| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
On an aarch64-linux system with 32-bit userland running in a chroot, and using
target board unix/mthumb I get:
...
(gdb) hbreak hbreak.c:27^M
Hardware assisted breakpoint 2 at 0x4004e2: file hbreak.c, line 27.^M
(gdb) PASS: gdb.base/hbreak.exp: hbreak
continue^M
Continuing.^M
Unexpected error setting breakpoint: Invalid argument.^M
(gdb) XFAIL: gdb.base/hbreak.exp: continue to break-at-exit after hbreak
...
due to this call in arm_linux_nat_target::low_prepare_to_resume:
...
if (ptrace (PTRACE_SETHBPREGS, pid,
(PTRACE_TYPE_ARG3) ((i << 1) + 1), &bpts[i].address) < 0)
perror_with_name (_("Unexpected error setting breakpoint"));
...
This problem does not happen if instead we use a 4-byte aligned address.
This may or may not be a kernel bug.
Work around this by first using an inoffensive address bpts[i].address & ~0x7.
Likewise in arm_target::low_prepare_to_resume, which fixes the same fail on
target board native-gdbserver/mthumb.
While we're at it:
- use arm_hwbp_control_is_initialized in
arm_linux_nat_target::low_prepare_to_resume,
- handle the !arm_hwbp_control_is_initialized case explicitly,
- add missing '_()' in arm_target::low_prepare_to_resume,
- make error messages identical between arm_target::low_prepare_to_resume and
arm_linux_nat_target::low_prepare_to_resume,
- factor out sethbpregs_hwbp_address and sethbpregs_hwbp_control to
make the implementation more readable.
Remove the tentative xfail added in d0af16d5a10 ("[gdb/testsuite] Add xfail in
gdb.base/hbreak.exp") by simply reverting the commit.
Tested on arm-linux.
Approved-By: Luis Machado <luis.machado@arm.com>
Tested-By: Luis Machado <luis.machado@arm.com>
|
|
After the previous two commits, this commit adds support for the
vFile::stat packet to gdbserver. This is pretty similar to the
handling for vFile::fstat, but instead calls 'lstat'.
There's still no users of target_fileio_stat in GDB, that will come in
a later commit.
|
|
C++ 11 has a built-in attribute for this, no need to use a compat macro.
Change-Id: I90e4220d26e8f3949d91761f8a13cd9c37da3875
Reviewed-by: Lancelot Six <lancelot.six@amd.com>
|
|
Like the previous commit, add two overloads of gdb_tilde_expand, one
takes std::string and other takes gdb::unique_xmalloc_ptr<char>. Make
use of these overloads throughout GDB and gdbserver.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Add two overloads of gdb_abspath, one which takes std::string and one
which takes gdb::unique_xmalloc_ptr<char>, then make use of these
overloads throughout GDB and gdbserver.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This reverts commit e874cbd3879843a83e4bcc4b54cd7107387b1df6.
The patch was wrong. LIBINTL_DEP is needed with an in-tree gettext.
|
|
The intl directory in the source no longer exists. LIBINTL_DEP is
thus always empty. Remove references to it.
config/
* gettext-sister.m4: Don't AC_SUBST LIBINTL_DEP.
bfd/
* Makefile.in: Regenerate.
* configure: Regenerate.
binutils/
* Makefile.am (*_DEPENDENCIES): Remove LIBINTL_DEP.
* Makefile.in: Regenerate.
* configure: Regenerate.
gas/
* Makefile.am (as_new_DEPENDENCIES): Remove LIBINTL_DEP.
* Makefile.in: Regenerate.
* configure: Regenerate.
gdb/
* Makefile.in (INTL_DEPS): Don't set or reference.
* configure: Regenerate.
gdbserver/
* Makefile.in (INTL_DEPS): Don't set or reference.
gdbsupport/
* Makefile.in: Regenerate.
* configure: Regenerate.
gold/
* Makefile.am (deps_var): Remove LIBINTL_DEP.
(incremental_dump_DEPENDENCIES, dwp_DEPENDENCIES): Likewise.
* Makefile.in: Regenerate.
* configure: Regenerate.
* testsuite/Makefile.am (DEPENDENCIES): Remove LIBINTL_DEP.
* testsuite/Makefile.in: Regenerate.
gprof/
* Makefile.am (gprof_DEPENDENCIES): Remove LIBINTL_DEP.
* Makefile.in: Regenerate.
* configure: Regenerate.
ld/
* Makefile.am (ld_new_DEPENDENCIES): Remove LIBINTL_DEP.
* Makefile.in: Regenerate.
* configure: Regenerate.
libctf/
* Makefile.in: Regenerate.
* configure: Regenerate.
opcodes/
* configure.ac (BUILD_LIBS): Remove LIBINTL.
(BUILD_LIB_DEPS): Remove LIBINTL_DEP.
* Makefile.in: Regenerate.
* configure: Regenerate.
|
|
Since this commit:
commit 0ee6b1c511c0e2a6793568692d2e5418cd6bc10d
Date: Wed May 18 13:32:04 2022 -0700
Use aarch64_features to describe register features in target descriptions.
There has been an issue with how aarch64 target descriptions are
cached within gdbserver, and specifically, how this caching impacts
the in process agent (IPA).
The function initialize_tracepoint_ftlib (gdbserver/tracepoint.cc) is
part of the IPA, this function is a constructor function, i.e. is
called as part of the global initialisation process. We can't
guarantee the ordering of when this function is called vs when other
global state is initialised.
Now initialize_tracepoint_ftlib calls initialize_tracepoint, which
calls initialize_low_tracepoint, which for aarch64 calls
aarch64_linux_read_description.
The aarch64_linux_read_description function lives in
linux-aarch64-tdesc.cc and after the above commit, depends on a
std::unordered_map having been initialized.
Prior to the above commit aarch64_linux_read_description used a global
C style array, which obviously requires no runtime initialization.
The consequence of the above is that any inferior linked with the IPA
(for aarch64) will experience undefined behaviour (access to an
uninitialized std::unordered_map) during startup, which for me
manifests as a segfault.
I propose fixing this by moving the std::unordered_map into the
function body, but leaving it static. The map will now be initialized
the first time the function is called, which removes the undefiend
behaviour.
The same problem exists for the expedited_registers global, however
this global can just be made into a function local instead. The
expedited_registers variable is used to build a pointer list which is
then passed to init_target_desc, however init_target_desc copies the
values it is given so expedited_registers does not need to live longer
than its containing function.
On most of the AArch64 machines I have access too tracing is not
supported, and so the gdb.trace/*.exp tests that use the IPA just exit
early reporting unsupported. I've added a test which links an
inferior with the IPA and just starts the inferior. No tracing is
performed. This exposes the current issue even on hosts that don't
support tracing. After this patch the test passes.
|
|
This commit builds on the previous series of commits to share the
target description caching code between GDB and gdbserver for
x86/Linux targets.
The objective of this commit is to move the four functions (2 each of)
i386_linux_read_description and amd64_linux_read_description into the
gdb/arch/ directory and combine them so we have just a single copy of
each. Then GDB, gdbserver, and the in-process-agent (IPA) will link
against these shared functions.
One curiosity with this patch is the function
x86_linux_post_init_tdesc. On the gdbserver side the two functions
amd64_linux_read_description and i386_linux_read_description have some
functionality that is not present on the GDB side, there is some
additional configuration that is performed as each target description
is created, to setup the expedited registers.
To support this I've added the function x86_linux_post_init_tdesc.
This function is called from the two *_linux_read_description
functions, but is implemented separately for GDB and gdbserver.
An alternative approach that avoids adding x86_linux_post_init_tdesc
would be to have x86_linux_tdesc_for_tid return a non-const target
description, then in x86_target::low_arch_setup we could inspect the
target description to figure out if it is 64-bit or not, and modify
the target description as needed. In the end I think that adding the
x86_linux_post_init_tdesc function is the simpler solution.
The contents of gdbserver/linux-x86-low.cc have moved to
gdb/arch/x86-linux-tdesc-features.c, and gdbserver/linux-x86-tdesc.h
has moved to gdb/arch/x86-linux-tdesc-features.h, this change leads to
some updates in the #includes in the gdbserver/ directory.
This commit also changes how target descriptions are cached.
Previously both GDB and gdbserver used static C-style arrays to act as
the tdesc cache. This was fine, except for two problems. Either the
C-style arrays would need to be placed in x86-linux-tdesc-features.c,
which would allow us to use the x86_linux_*_tdesc_count_1() functions
to size the arrays for us, or we'd need to hard code the array sizes
using separate #defines, which we'd then have to keep in sync with the
rest of the code in x86-linux-tdesc-features.c.
Given both of these problems I decided a better solution would be to
just switch to using a std::unordered_map to act as the cache. This
will resize automatically, and we can use the xcr0 value as the key.
At first inspection, using xcr0 might seem to be a problem; after all
the {i386,amd64}_create_target_description functions take more than
just the xcr0 value. However, this patch is only for x86/Linux
targets, and for x86/Linux all of the other flags passed to the tdesc
creation functions have constant values and so are irrelevant when we
consider tdesc caching.
For testing I've done the following:
- Built on x86-64 GNU/Linux for all targets, and just for the native
target,
- Build on i386 GNU/Linux for all targets, and just for the native
target,
- Build on a 64-bit, non-x86 GNU/Linux for all targets, just for the
native target, and for targets x86_64-*-linux and i386-*-linux.
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
|
|
This commit is part of a series which aims to share more of the target
description creation between GDB and gdbserver for x86/Linux.
After some refactoring earlier in this series the shared
x86_linux_tdesc_for_tid function was added into nat/x86-linux-tdesc.c.
However, this function still relies on amd64_linux_read_description
and i386_linux_read_description which are implemented separately for
both gdbserver and GDB. Given that at their core, all these functions
do is:
1. take an xcr0 value as input,
2. mask out some feature bits,
3. look for a cached pre-generated target description and return it
if found,
4. if no cached target description is found then call either
amd64_create_target_description or
i386_create_target_description to create a new target
description, which is then added to the cache. Return the newly
created target description.
The inner functions amd64_create_target_description and
i386_create_target_description are already shared between GDB and
gdbserver (in the gdb/arch/ directory), so the only thing that
the *_read_description functions really do is add the caching layer,
and it feels like this really could be shared.
However, we have a small problem.
Despite using the same {amd64,i386}_create_target_description
functions in both GDB and gdbserver to create the target descriptions,
on the gdbserver side we cache target descriptions based on a reduced
set of xcr0 feature bits.
What this means is that, in theory, different xcr0 values can map to
the same cache entry, which could result in the wrong target
description being used.
However, I'm not sure if this can actually happen in reality. Within
gdbserver we already split the target description cache based on i386,
amd64, and x32. I suspect within a given gdbserver session we'll only
see at most one target description for each of these.
The cache conflicting problem is caused by xcr0_to_tdesc_idx, which
maps an xcr0 value to a enum x86_linux_tdesc value, and there are only
7 usable values in enum x86_linux_tdesc.
In contrast, on the GDB side there are 64, 32, and 16 cache slots for
i386, amd64, and x32 respectively.
On the GDB side it is much more important to cache things correctly as
a single GDB session might connect to multiple different remote
targets, each of which might have slightly different x86
architectures.
And so, if we want to merge the target description caching between GDB
and gdbserver, then we need to first update gdbserver so that it
caches in the same way as GDB, that is, it needs to adopt a mechanism
that allows for the same number of cache slots of each of i386, amd64,
and x32. In this way, when the caching is shared, GDB's behaviour
will not change.
Unfortunately it's a little more complex than that due to the in
process agent (IPA).
When the IPA is in use, gdbserver sends a target description index to
the IPA, and the IPA uses this to find the correct target description
to use, the IPA having first generated every possible target
description.
Interestingly, there is certainly a bug here which results from only
having 7 values in enum x86_linux_tdesc. As multiple possible target
descriptions in gdbserver map to the same enum x86_linux_tdesc value,
then, when the enum x86_linux_tdesc value is sent to the IPA there is
no way for gdbserver to know that the IPA will select the correct
target description. This bug will get fixed by this commit.
** START OF AN ASIDE **
Back in the day I suspect this approach of sending a target
description index made perfect sense. However since this commit:
commit a8806230241d201f808d856eaae4d44088117b0c
Date: Thu Dec 7 17:07:01 2017 +0000
Initialize target description early in IPA
I think that passing an index was probably a bad idea.
We used to pass the index, and then use that index to lookup which
target description to instantiate and use, the target description was
not generated until the index arrived.
However, the above commit fixed an issue where we can't call malloc()
within (certain parts of) the IPA (apparently), so instead we now
pre-compute _every_ possible target description within the IPA. The
index is only used to lookup which of the (many) pre-computed target
descriptions to use.
It would (I think) have been easier all around if the IPA just
self-inspected, figured out its own xcr0 value, and used that to
create the one target description that is required. So long as the
xcr0 to target description code is shared (at compile time) with
gdbserver, then we can be sure that the IPA will derive the same
target description as gdbserver, and we would avoid all this index
passing business, which has made this commit so very, very painful.
I did look at how a process might derive its own xcr0 value, but I
don't believe this is actually that simple, so for now I've just
doubled down on the index passing approach.
While reviewing earlier iterations of this patch there has been
discussion about the possibility of removing the IPA from GDB. That
would certainly make all of the code touched in this patch much
simpler, but I don't really want to do that as part of this series.
** END OF AN ASIDE **
Currently then for x86/linux, gdbserver sends a number between 0 and 7
to the IPA, and the IPA uses this to create a target description.
However, I am proposing that gdbserver should now create one of (up
to) 64 different target descriptions for i386, so this 0 to 7 index
isn't going to be good enough any more (amd64 and x32 have slightly
fewer possible target descriptions, but still more than 8, so the
problem is the same).
For a while I wondered if I was going to have to try and find some
backward compatible solution for this mess. But after seeing how
lightly the IPA is actually documented, I wonder if it is not the case
that there is a tight coupling between a version of gdbserver and a
version of the IPA? At least I'm hoping so, and that's what I've
assumed in this commit.
In this commit I have thrown out the old IPA target description index
numbering scheme, and switched to a completely new numbering scheme.
Instead of the index that is passed being arbitrary, the index is
instead calculated from the set of xcr0 features that are present on
the target. Within the IPA we can then reverse this logic to recreate
the xcr0 value based on the index, and from the xcr0 value we can
choose the correct target description.
With the gdbserver to IPA numbering scheme issue resolved I have then
update the gdbserver versions of amd64_linux_read_description and
i386_linux_read_description so that they cache target descriptions
using the same set of xcr0 features as GDB itself.
After this gdbserver should now always come up with the same target
description as GDB does on any x86/Linux target.
This commit does not introduce any new code sharing between GDB and
gdbserver as previous commits in this series have done. Instead this
commit is all about bringing GDB and gdbserver into alignment
functionally so that the next commit(s) can merge the GDB and
gdbserver versions of these functions.
Notes On The Implementation
---------------------------
Previously, within gdbserver, target descriptions were cached within
arrays. These arrays were sized based on enum x86_linux_tdesc and
xcr0_to_tdesc_idx returned the array (cache) index.
Now we need different array lengths for each of i386, amd64, and x32.
And the index to use within each array is calculated based on which
xcr0 bits are set and valid for a particular target type.
I really wanted to avoid having fixed array sizes, or having the set
of relevant xcr0 bits encoded in multiple places.
The solution I came up with was to create a single data structure
which would contain a list of xcr0 bits along with flags to indicate
which of the i386, amd64, and x32 targets the bit is relevant for. By
making the table constexpr, and adding some constexpr helper
functions, it is possible to calculate the sizes for the cache arrays
at compile time, as well as the bit masks needed to each target type.
During review it was pointed out[1] that possibly the failure to check
the SSE and X87 bits for amd64/x32 targets might be an error, however,
if this is the case then this is an issue that existed long before
this patch. I'd really like to keep this patch focused on reworking
the existing code and try to avoid changing how target descriptions
are actually created, mostly out of fear that I'll break something.
[1] https://inbox.sourceware.org/gdb-patches/MN2PR11MB4566070607318EE7E669A5E28E1B2@MN2PR11MB4566.namprd11.prod.outlook.com
Approved-By: John Baldwin <jhb@FreeBSD.org>
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
|
|
This commit is part of a series to share more of the x86 target
description creation code between GDB and gdbserver.
Unlike previous commits which were mostly refactoring, this commit is
the first that makes a real change, though that change should mostly
be for gdbserver; I've largely adopted the "GDB" way of doing things
for gdbserver, and this fixes a real gdbserver bug.
On a x86-64 Linux target, running the test:
gdb.server/connect-with-no-symbol-file.exp
results in two core files being created. Both of these core files are
from the inferior process, created after gdbserver has detached.
In this test a gdbserver process is started and then, after gdbserver
has started, but before GDB attaches, we either delete the inferior
executable, or change its permissions so it can't be read. Only after
doing this do we attempt to connect with GDB.
As GDB connects to gdbserver, gdbserver attempts to figure out the
target description so that it can send the description to GDB, this
involves a call to x86_linux_read_description.
In x86_linux_read_description one of the first things we do is try to
figure out if the process is 32-bit or 64-bit. To do this we look up
the executable via the thread-id, and then attempt to read the
architecture size from the executable. This isn't going to work if
the executable has been deleted, or is no longer readable.
And so, as we can't read the executable, we default to an i386 target
and use an i386 target description.
A consequence of using an i386 target description is that addresses
are assumed to be 32-bits. Here's an example session that shows the
problems this causes. This is run on an x86-64 machine, and the test
binary (xx.x) is a standard 64-bit x86-64 binary:
shell_1$ gdbserver --once localhost :54321 /tmp/xx.x
shell_2$ gdb -q
(gdb) set sysroot
(gdb) shell chmod 000 /tmp/xx.x
(gdb) target remote :54321
Remote debugging using :54321
warning: /tmp/xx.x: Permission denied.
0xf7fd3110 in ?? ()
(gdb) show architecture
The target architecture is set to "auto" (currently "i386").
(gdb) p/x $pc
$1 = 0xf7fd3110
(gdb) info proc mappings
process 2412639
Mapped address spaces:
Start Addr End Addr Size Offset Perms objfile
0x400000 0x401000 0x1000 0x0 r--p /tmp/xx.x
0x401000 0x402000 0x1000 0x1000 r-xp /tmp/xx.x
0x402000 0x403000 0x1000 0x2000 r--p /tmp/xx.x
0x403000 0x405000 0x2000 0x2000 rw-p /tmp/xx.x
0xf7fcb000 0xf7fcf000 0x4000 0x0 r--p [vvar]
0xf7fcf000 0xf7fd1000 0x2000 0x0 r-xp [vdso]
0xf7fd1000 0xf7fd3000 0x2000 0x0 r--p /usr/lib64/ld-2.30.so
0xf7fd3000 0xf7ff3000 0x20000 0x2000 r-xp /usr/lib64/ld-2.30.so
0xf7ff3000 0xf7ffb000 0x8000 0x22000 r--p /usr/lib64/ld-2.30.so
0xf7ffc000 0xf7ffe000 0x2000 0x2a000 rw-p /usr/lib64/ld-2.30.so
0xf7ffe000 0xf7fff000 0x1000 0x0 rw-p
0xfffda000 0xfffff000 0x25000 0x0 rw-p [stack]
0xff600000 0xff601000 0x1000 0x0 r-xp [vsyscall]
(gdb) info inferiors
Num Description Connection Executable
* 1 process 2412639 1 (remote :54321)
(gdb) shell cat /proc/2412639/maps
00400000-00401000 r--p 00000000 fd:03 45907133 /tmp/xx.x
00401000-00402000 r-xp 00001000 fd:03 45907133 /tmp/xx.x
00402000-00403000 r--p 00002000 fd:03 45907133 /tmp/xx.x
00403000-00405000 rw-p 00002000 fd:03 45907133 /tmp/xx.x
7ffff7fcb000-7ffff7fcf000 r--p 00000000 00:00 0 [vvar]
7ffff7fcf000-7ffff7fd1000 r-xp 00000000 00:00 0 [vdso]
7ffff7fd1000-7ffff7fd3000 r--p 00000000 fd:00 143904 /usr/lib64/ld-2.30.so
7ffff7fd3000-7ffff7ff3000 r-xp 00002000 fd:00 143904 /usr/lib64/ld-2.30.so
7ffff7ff3000-7ffff7ffb000 r--p 00022000 fd:00 143904 /usr/lib64/ld-2.30.so
7ffff7ffc000-7ffff7ffe000 rw-p 0002a000 fd:00 143904 /usr/lib64/ld-2.30.so
7ffff7ffe000-7ffff7fff000 rw-p 00000000 00:00 0
7ffffffda000-7ffffffff000 rw-p 00000000 00:00 0 [stack]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall]
(gdb)
Notice the difference between the mappings reported via GDB and those
reported directly from the kernel via /proc/PID/maps, the addresses of
every mapping is clamped to 32-bits for GDB, while the kernel reports
real 64-bit addresses.
Notice also that the $pc value is a 32-bit value. It appears to be
within one of the mappings reported by GDB, but is outside any of the
mappings reported from the kernel.
And this is where the problem arises. When gdbserver detaches from
the inferior we pass the inferior the address from which it should
resume. Due to the 32/64 bit confusion we tell the inferior to resume
from the 32-bit $pc value, which is not within any valid mapping, and
so, as soon as the inferior resumes, it segfaults.
If we look at how GDB (not gdbserver) figures out its target
description then we see an interesting difference. GDB doesn't try to
read the executable. Instead GDB uses ptrace to query the thread's
state, and uses this to figure out the if the thread is 32 or 64 bit.
If we update gdbserver to do it the "GDB" way then the above problem
is resolved, gdbserver now sees the process as 64-bit, and when we
detach from the inferior we give it the correct 64-bit address, and
the inferior no longer segfaults.
Now, I could just update the gdbserver code, but better, I think, to
share one copy of the code between GDB and gdbserver in gdb/nat/.
That is what this commit does.
The cores of x86_linux_read_description from gdbserver and
x86_linux_nat_target::read_description from GDB are moved into a new
file gdb/nat/x86-linux-tdesc.c and combined into a single function
x86_linux_tdesc_for_tid which is called from each location.
This new function does things mostly the GDB way, some changes are
needed to allow for the sharing; we now take some pointers for where
the shared code can cache the xcr0 and xsave layout values.
Another thing to note about this commit is how the functions
i386_linux_read_description and amd64_linux_read_description are
handled. For now I've left these function as implemented separately
in GDB and gdbserver. I've moved the declarations of these functions
into gdb/arch/{i386,amd64}-linux-tdesc.h, but the implementations are
left where they are.
A later commit in this series will make these functions shared too,
but doing this is not trivial, so I've left that for a separate
commit. Merging the declarations as I've done here ensures that
everyone implements the function to the same API, and once these
functions are shared (in a later commit) we'll want a shared
declaration anyway.
Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
Acked-By: John Baldwin <jhb@FreeBSD.org>
|
|
In a later commit I want to access have_ptrace_getregset from a .c
file in the nat/ directory. To achieve this I need access to the
declaration of have_ptrace_getregset.
Currently have_ptrace_getregset is declared (and defined) twice, once
in GDB and once in gdbserver.
This commit moves the declaration into nat/linux-nat.h, but leaves the
two definitions where they are. Now, in my later commit, I can pull
in the declaration from nat/linux-nat.h.
There should be no user visible changes after this commit.
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
|
|
The have_ptrace_getfpxregs global tracks whether GDB or gdbserver is
running on a kernel that supports the GETFPXREGS ptrace request.
Currently this global is declared twice (once in GDB and once in
gdbserver), I think it makes sense to move this global into the nat/
directory, and have a single declaration and definition.
While moving this variable I have converted it to a tribool, as that
was what it really was, if even used the same numbering as the tribool
enum (-1, 0, 1). Where have_ptrace_getfpxregs was used I have updated
in the obvious way.
However, while making this change I noticed what I think is a bug in
x86_linux_nat_target::read_description and x86_linux_read_description,
both of these functions can be called multiple times, but in both
cases we only end up calling i386_linux_read_description the first
time through in the event that PTRACE_GETFPXREGS is not supported.
This is because initially have_ptrace_getfpxregs will be
TRIBOOL_UNKNOWN, but after the ptrace call fails we set
have_ptrace_getfpxregs to TRIBOOL_FALSE. The next time we attempt to
read the target description we'll skip the ptrace call, and so skip
the call to i386_linux_read_description.
I've not tried to address this preexisting bug in this commit, this is
purely a refactor, there should be no user visible changes after this
commit. In later commits I'll merge the gdbserver and GDB code
together into the nat/ directory, and after that I'll try to address
this bug.
Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
|
|
This commit is part of a series that aims to share more of the x86
target description reading/generation code between GDB and gdbserver.
There are a huge number of similarities between the code in
gdbserver's x86_linux_read_description function and GDB's
x86_linux_nat_target::read_description function, and it is this
similarity that I plan, in a later commit, to share between GDB and
gdbserver.
However, one thing that is different in x86_linux_read_description is
the code inside the '!use_xml' block. This is the code that handles
the case where gdbserver is not allowed to send an XML target
description back to GDB. In this case gdbserver uses some predefined,
fixed, target descriptions.
First, it's worth noting that I suspect this code is not tested any
more. I couldn't find anything in the testsuite that tries to disable
XML target description support. And the idea of having a single
"fixed" target description really doesn't work well when we think
about all the various x86 extensions that exist. Part of me would
like to rip out the no-xml support in gdbserver (at least for x86),
and if a GDB connects that doesn't support XML target descriptions,
gdbserver can just give an error and drop the connection. GDB has
supported XML target descriptions for 16 years now, I think it would
be reasonable for our shipped gdbserver to drop support for the old
way of doing things.
Anyway.... this commit doesn't do that.
What I did notice was that, over time, the '!use_xml' block appears to
have "drifted" within the x86_linux_read_description function; it's
now not the first check we do. Instead we make some ptrace calls and
return a target description generated based on the result of these
ptrace calls. Surely it only makes sense to generate variable target
descriptions if we can send these back to GDB?
So in this commit I propose to move the '!use_xml' block earlier in
the x86_linux_read_description function.
The benefit of this is that this leaves the later half of
x86_linux_read_description much more similar to the GDB function
x86_linux_nat_target::read_description and sets us up for potentially
sharing code between GDB and gdbserver in a later commit.
Approved-By: John Baldwin <jhb@FreeBSD.org>
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
|
|
Share the definition of I386_LINUX_XSAVE_XCR0_OFFSET between GDB and
gdbserver.
This commit moves the definition into gdbsupport/x86-xstate.h, which
allows the #define to be shared.
There should be no user visible changes after this commit.
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
|
|
Add a new 'error_message' feature to the qSupported packet. When GDB
supports this feature then gdbserver is able to send
errors in the E.errtext format for the qRcmd and m packets.
Update qRcmd packet and m packets documentation as qRcmd newly
accepts errors in a E.errtext format.
Previously these two packets didn't support E.errtext style errors.
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
I believe that the get_exec_file function is unnecessary, and the code
can be simplified if we remove it.
Consider for instance when you "run" a program on Linux with native
debugging.
1. run_command_1 obtains the executable file from
`current_program_space->exec_filename ()`
2. it passes it to `run_target->create_inferior()`, which is
`inf_ptrace_target::create_inferior()` in this case, which then
passes it to `fork_inferior()`
3. `fork_inferior()` then has a fallback, where if the passed exec file
is nullptr, it gets its from `get_exec_file()`.
4. `get_exec_file()` returns `current_program_space->exec_filename ()`
- just like the things we started with - or errors out if the
current program space doesn't have a specified executable.
If there's no exec filename passed in step 1, there's not going to be
any in step 4, so it seems pointless to call `get_exec_file()`, we could
just error out when `exec_file` is nullptr. But we can't error out
directly in `fork_inferior()`, since the error is GDB-specific, and that
function is shared with GDBserver.
Speaking of GDBserver, all code paths that lead to `fork_inferior()`
provide a non-nullptr exec file.
Therefore, to simplify things:
- Make `fork_inferior()` assume that the passed exec file is not
nullptr, don't call `get_exec_file()`
- Change some targets (darwin-nat, go32-nat, gnu-nat, inf-ptrace,
nto-procfs, procfs) to error out when the exec file passed to their
create_inferior method is nullptr. Some targets are fine with a
nullptr exec file, so we can't check that in `run_command_1()`.
- Add the `no_executable_specified_error()` function, which re-uses the
error message that `get_exec_file()` had.
- Change some targets (go32-nat, nto-procfs) to not call
`get_exec_file()`, since it's pointless for the same reason as in the
example above, if it returns, it's going the be the same value as the
`exec_file` parameter. Just rely on `exec_file`.
- Remove the final use of `get_exec_file()`, in `load_command()`.
- Remove the `get_exec_file()` implementations in GDB and GDBserver and
remove the shared declaration.
Change-Id: I601c16498e455f7baa1f111a179da2f6c913baa3
Approved-By: Tom Tromey <tom@tromey.com>
|
|
`current_program_space->exec_filename ()`
Calls of `get_exec_file (0)` are equivalent to just getting the exec
filename from the current program space. I'm looking to remove
`get_exec_file`, so replace these uses with
`current_program_space->exec_filename ()`.
Remove the `err` parameter of `get_exec_wrapper` since all the calls
that remain pass 1, meaning to error out if no executable is specified.
Change-Id: I7729ea4c7f03dbb046211cc5aa3858ab3a551965
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Currently, if you configure gdb with explicit --enable-threading, but
then configure detects std::thread does not work, configure silently
disables threading support and continues configuring.
This patch makes that scenario cause a configuration error, like so:
$ /home/pedro/gdb/src/configure --enable-threading && make
...
configure: error: std::thread does not work; disable threading
make[1]: *** [Makefile:11225: configure-gdbsupport] Error 1
make[1]: Leaving directory '/home/pedro/gdb/build-windows-threads'
make: *** [Makefile:1041: all] Error 2
$
Additionally, if you don't explicitly pass --enable-threading, and
std::thread does not work, we will now get a warning (and the build
continues):
$ /home/pedro/gdb/src/configure && make
...
configure: WARNING: std::thread does not work; disabling threading
...
This is similar to how we handle --enable-tui and missing curses. The
code and error/warning messages were borrowed from there.
Change-Id: I73a8b580d1e2a796b23136920c0e181408ae1b22
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Convert the have_ptrace_getregset global within gdbserver to a
tribool. This brings the flag into alignment with the corresponding
flag in GDB.
The gdbserver have_ptrace_getregset variable is already used as a
tribool, it just doesn't have the tribool type.
In a future commit I plan to share more code between GDB and
gdbserver, and having this variable be the same type in both code
bases will make the sharing much easier.
There should be no user visible changes after this commit.
Approved-By: John Baldwin <jhb@FreeBSD.org>
Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
|
|
Spotted some declarations in gdbserver/linux-amd64-ipa.cc that are no
longer needed. These are:
1. 'init_registers_amd64_linux' - the comment claims this function
is auto generated, but I don't believe that this is still the case.
Also the function is not used in this file,
2. 'tdesc_amd64_linux' - this variable doesn't seem to exist any
more, I suspect this was renamed to 'tdesc_amd64_linux_no_xml', but
neither are used in this file, so lets remove the declaration.
The amd64 in-process-agent still builds fine after this commit.
There should be no user visible changes after this commit.
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
|
|
With a x86_64-pc-mingw32 toolchain there is a build issue
whether or not the --disable-threading option is used.
The problem happens because _WIN32_WINNT is defined to 0x501
before #include <mutex> which makes the compilation abort
due to missing support for __gthread_cond_t in std_mutex.h,
which is conditional on _WIN32_WINNT >= 0x600.
Fix the case when --disable-threading is used, by only
including <mutex> in gdb/complaints.c when STD_CXX_THREAD
is defined.
Additionally make the configure script try to #include <mutex>
to automatically select --disable-threading when the header file
is not able to compile.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Commit 0ee25f97d21e ("Fix regression on aarch64-linux gdbserver")
removed the last use of i in gdbserver/linux-aarch64-low.cc
(aarch64_target::low_stopped_data_address). Breaking the build on
aarch64 with:
gdbserver/linux-aarch64-low.cc: In member function ?virtual CORE_ADDR aarch64_target::low_stopped_data_address()?:
gdbserver/linux-aarch64-low.cc:557:12: error: unused variable ?i? [-Werror=unused-variable]
557 | int pid, i;
| ^
cc1plus: all warnings being treated as errors
Fix this by removing the variable i completely.
Fixes: 0ee25f97d21e ("Fix regression on aarch64-linux gdbserver")
|
|
Commit 9a03f218 ("Fix gdb.base/watchpoint-unaligned.exp on aarch64")
fixed a watchpoint bug in gdb -- but did not touch the corresponding
code in gdbserver.
This patch moves the gdb code into gdb/nat, so that it can be shared
with gdbserver, and then changes gdbserver to use it, fixing the bug.
This is yet another case where having a single back end would prevent
bugs.
I tested this using the AdaCore internal gdb testsuite.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29423
Approved-By: Luis Machado <luis.machado@arm.com>
|
|
handle_v_attach calls attach_inferior, which says:
"return -1 if attaching is unsupported, 0 if it succeeded, and call
error() otherwise."
So if attach_inferior return != 0, we have the unsupported case,
meaning we should return the empty packet instead of an error.
In practice, this shouldn't trigger, as vAttach support is supposed to
be reported via qSupported. But it doesn't hurt to be pedantic here.
Change-Id: I99cce6fa678f2370571e6bca0657451300956127
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This fixes the same issue as the previous patch, but for "attach"
instead of "run".
If attaching to a process with "attach" (vAttach packet) fails,
GDBserver throws an error that escapes all the way to the top level.
When an error escapes all the way like that, GDBserver interprets it
as a disconnection, and either goes back to waiting for a new GDB
connection, or exits, if --once was specified.
Here's an example:
On the GDB side:
...
(gdb) tar extended-remote :9999
...
Remote debugging using :9999
(gdb) attach 1
Attaching to process 1
Attaching to process 1 failed
(gdb)
On the GDBserver side:
$ gdbserver --once --multi :9999
Listening on port 9999
Remote debugging from host 127.0.0.1, port 37464
gdbserver: Cannot attach to process 1: Operation not permitted (1)
$ # gdbserver exited
This is wrong, as we've connected with extended-remote/--multi.
GDBserver should just report an error to vAttach, and continue
connected to GDB, waiting for other commands.
This commit fixes GDBserver by catching the error locally in
handle_v_attach.
Note we now let pid == 0 pass down to attach_inferior. That is so we
get a useful textual error message to report to GDB.
This fixes a couple KFAILs in gdb.base/attach.exp. Still, I thought
it would be useful to add a new testcase specifically for this
scenario, in case gdb.base/attach.exp is ever split and stops trying
to attach again after a failed attach, with the same GDB session.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19558
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31554
Change-Id: I25314c7e5f1435eff69cb84d57ecac13d8de3393
Approved-By: Tom Tromey <tom@tromey.com>
|
|
If starting the inferior process with "run" (vRun packet) fails,
GDBserver throws an error that escapes all the way to the top level.
When an error escapes all the way like that, GDBserver interprets it
as a disconnection, and either goes back to waiting for a new GDB
connection, or exits, if --once was specified.
E.g., with the testcase program added by this commit, we see:
On GDB side:
...
(gdb) tar extended-remote :999
...
Remote debugging using :9999
(gdb) r
Starting program:
Running ".../gdb.base/run-fail-twice/run-fail-twice.nox" on the remote target failed
(gdb)
On GDBserver side:
$ gdbserver --once --multi :9999
Remote debugging from host 127.0.0.1, port 34344
bash: line 1: .../gdb.base/run-fail-twice/run-fail-twice.nox: Permission denied
bash: line 1: exec: .../gdb.base/run-fail-twice/run-fail-twice.nox: cannot execute: Permission denied
gdbserver: During startup program exited with code 126.
$ # gdbserver exited
This is wrong, as we've connected with extended-remote/--multi.
GDBserver should just report an error to vCont, and continue connected
to GDB, waiting for other commands.
This commit fixes GDBserver by catching the error locally in
handle_v_run.
Change-Id: Ib386f267522603f554b52a885b15229c9639e870
Approved-By: Tom Tromey <tom@tromey.com>
|
|
There is no reason the callers of these functions need to change the
returned string, so change the `char *` return types to `const char *`.
Update a few callers to also use `const char *`.
Change-Id: I94adff574d5e1b326e8cc688cf1817a15b408b96
Approved-By: Tom Tromey <tom@tromey.com>
|
|
It's been over 9 years (since commit faf09f0119da) since Linux GDB and
GDBserver started relying on SIGTRAP si_code to tell whether a
breakpoint triggered, which is important for non-stop mode. When that
then-new code was added, I had left the then-old code as fallback, in
case some architectured still needed it. Given AFAIK there haven't
been complaints since, this commit finally removes the fallback code,
along with USE_SIGTRAP_SIGINFO.
Change-Id: I140a5333a9fe70e90dbd186aca1f081549b2e63d
|
|
When building with clang 18, I see:
CXX aarch64-linux-tdep.o
/home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]
1299 | gdb_byte za_zeroed[za_bytes];
| ^~~~~~~~
/home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: note: read of non-const variable 'za_bytes' is not allowed in a constant expression
/home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1282:10: note: declared here
1282 | size_t za_bytes = std::pow (sve_vl_from_vg (svg), 2);
| ^
Since we are using VLAs right now, that warning doesn't make sense for
us. add `-Wno-vla-cxx-extension` to the list of warning flags we try to
enable. If we ever choose to disallow VLAs, we can remove that flag.
Change-Id: Ie41feafc50c343f6e75333d4f836ce32fbeb6d8c
|
|
For some reason install-dvi is missing although other targets of the
same family are present. This looks like an oversight.
This enables calling 'make install-dvi' from the top-level build
directory.
Fix what looks like another oversight: include 'pdf' in 'all-doc' in
gdb/doc/Makefile.in.
Approved-By: Luis Machado <luis.machado@arm.com>
Tested-By: Luis Machado <luis.machado@arm.com>
|
|
Now that defs.h, server.h and common-defs.h are included via the
`-include` option, it is no longer necessary for source files to include
them. Remove all the inclusions of these files I could find. Update
the generation scripts where relevant.
Change-Id: Ia026cff269c1b7ae7386dd3619bc9bb6a5332837
Approved-By: Pedro Alves <pedro@palves.net>
|
|
The motivation for this change is for analysis tools and IDEs to be
better at analyzing header files on their own.
There are some definitions and includes we want to occur at the very
beginning of all translation units. The way we currently do that is by
requiring all source files (.c and .cc files) to include one of defs.h
(for gdb), server.h (for gdbserver) of common-defs.h (for gdbsupport and
shared source files). These special header files define and include
everything that needs to be included at the very beginning. Other
header files are written in a way that assume that these special
"prologue" header files have already been included.
My problem with that is that my editor (clangd-based) provides a very
bad experience when editing header files. Since clangd doesn't know
that one of defs.h/server.h/common-defs.h was included already, a lot of
things are flagged as errors. For instance, CORE_ADDR is not known.
It's possible to edit the files in this state, but a lot of the power of
the editor is unavailable.
My proposal to help with this is to include those things we always want
to be there using the compilers' `-include` option. Tom Tromey said
that the current approach might exist because not all compilers used to
have an option like this. But I believe that it's safe to assume they
do today.
With this change, clangd picks up the -include option from the compile
command, and is able to analyze the header file correctly, as it sees
all that stuff included or defined by that -include option. That works
because when editing a header file, clangd tries to get the compilation
flags from a source file that includes said header file.
This change is a bit self-serving, because it addresses one of my
frustrations when editing header files, but it might help others too.
I'd be curious to know if others encounter the same kinds of problems
when editing header files. Also, even if the change is not necessary by
any means, I think the solution of using -include for stuff we always
want to be there is more elegant than the current solution.
Even with this -include flag, many header files currently don't include
what they use, but rather depend on files included before them. This
will still cause errors when editing them, but it should be easily
fixable by adding the appropriate include. There's no rush to do so, as
long as the code still compiles, it's just a convenience thing.
The changes are:
- Add the appropriate `-include` option to the various Makefiles.
- There is one particularity for gdbserver's Makefile: we do not want
to include server.h when building `gdbreplay.o`, as `gdbreplay.cc`
doesn't include it. So we can't simply put the `-include` in
`INTERNAL_CFLAGS`. Add the `-include server.h` option to the
`COMPILE` and `IPAGENT_COMPILE` variables, and added a special rule
to compile `gdbreplay.o` with `-include gdbsupport/common-defs.h`.
- Remove the `-include` option from the `check-headers` rule in
gdb/Makefile.in, since it is already included in `INTERNAL_CFLAGS`.
Change-Id: If3e345d00a9fc42336322f1d8286687d22134340
Approved-By: Pedro Alves <pedro@palves.net>
|
|
Remove `INTERNAL_CFLAGS_BASE` and `INTERNAL_WARN_CFLAGS`, inline their
contents in `INTERNAL_CFLAGS`. Not functional changes expected.
Change-Id: I6a09794835ca2cfd4a88a3e9f2e627c8f5bd569f
Approved-By: Pedro Alves <pedro@palves.net>
|
|
Reformat some variables definitions. I think it makes them easier to
read, and it also makes diffs clearer.
Change-Id: I82f63ba0e6d0fe268eb1f1ad5ab22c3cd016ab02
Approved-By: Pedro Alves <pedro@palves.net>
|
|
This reverts commit 5920765d7513aaae9241a1850d62d73e0477f81c.
|
|
This reverts commit 0a7bb97ad2f2fe2d18a442dad265051e34eab13e.
|
|
This reverts commit 7816b81e9b36ea0f57662bfd7446b573bf0c9e54.
|
|
This reverts commit cd9b374ffe372dcaf7e4c15548cf53a301d8dcdd.
|
|
This reverts commit 61bb321605fc74703adc994fd7a78e9d2495ca7a.
|
|
This reverts commit 198ff6ff819c240545f9fc68b39636fd376d4ba9.
|
|
This reverts commit c7c9820071f8b81a64221f5cfafb3cbfeafe7916.
|
|
When building with Clang, I get:
CXX nat/x86-linux-tdesc-ipa.o
clang++: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated [-Werror,-Wdeprecated]
Fix that by adding the missing `-x c++` in the rule building
`gdb/nat/*.c` files for the in-process agent.
Change-Id: Ie53e4b9a8b57bef9669397fdfaf21617107c7180
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This commit builds on the previous series of commits to share the
target description caching code between GDB and gdbserver for
x86/Linux targets.
The objective of this commit is to move the four functions (2 each of)
i386_linux_read_description and amd64_linux_read_description into
gdb/nat/x86-linux-tdesc.c and combine them so we have just a single
copy of each. Then both GDB and gdbserver will link against these
shared functions.
It is worth reading the description of the previous commit to see why
this merging is not as simple as it seems: on the gdbserver side we
actually have two users of these functions, gdbserver itself, and the
in process agent (IPA).
However, the previous commit streamlined the gdbserver code, and so
now it is simple to move the two functions along with all their
support functions from the gdbserver directory into the gdb/nat/
directory, and then GDB is fine to call these functions.
One small curiosity with this patch is the function
x86_linux_post_init_tdesc. On the gdbserver side the two functions
amd64_linux_read_description and i386_linux_read_description have some
functionality that is not present on the GDB side, that is some
additional configuration that is performed as each target description
is created to setup the expedited registers.
To support this I've added the function x86_linux_post_init_tdesc.
This function is called from the two *_linux_read_description
functions, but is implemented separately for GDB and gdbserver.
This does mean adding back some non-shared code when this whole series
has been about sharing code, but now the only non-shared bit is the
single line that is actually different between GDB and gdbserver, all
the rest, which is identical, is now shared.
I did need to add a new rule to the gdbserver Makefile, this is to
allow the nat/x86-linux-tdesc.c file to be compiled for the IPA.
Approved-By: John Baldwin <jhb@FreeBSD.org>
|
|
This commit is part of a series which aims to share more of the target
description creation between GDB and gdbserver for x86/Linux.
After some refactoring, the previous commit actually started to share
some code, we added the shared x86_linux_tdesc_for_tid function into
nat/x86-linux-tdesc.c. However, this function still relies on
amd64_linux_read_description and i386_linux_read_description which are
implemented separately for both gdbserver and GDB. Given that at
their core, all these functions to is:
1. take an xcr0 value as input,
2. mask out some feature bits,
3. look for a cached pre-generated target description and return it
if found,
4. if no cached target description is found then call either
amd64_create_target_description or
i386_create_target_description to create a new target
description, which is then added to the cache. Return the newly
created target description.
The inner functions amd64_create_target_description and
i386_create_target_description are already shared between GDB and
gdbserver (in the gdb/arch/ directory), so the only thing that
the *_read_description functions really do is add the caching layer,
and it feels like this really could be shared.
However, we have a small problem.
On the GDB side we create target descriptions using a different set of
cpu features than on the gdbserver side! This means that for the
exact same target, we might get a different target description when
using native GDB vs using gdbserver. This surely feels like a
mistake, I would expect to get the same target description on each.
The table below shows the number of possible different target
descriptions that we can create on the GDB side vs on the gdbserver
side for each target type:
| GDB | gdbserver
------|-----|----------
i386 | 64 | 7
amd64 | 32 | 7
x32 | 16 | 7
So in theory, all I want to do is move the GDB version
of *_read_description into the nat/ directory and have gdbserver use
that, then both GDB and gdbserver would be able to create any of the
possible target descriptions.
Unfortunately it's a little more complex than that due to the in
process agent (IPA).
When the IPA is in use, gdbserver sends a target description index to
the IPA, and the IPA uses this to find the correct target description
to use.
** START OF AN ASIDE **
Back in the day I suspect this approach made perfect sense. However
since this commit:
commit a8806230241d201f808d856eaae4d44088117b0c
Date: Thu Dec 7 17:07:01 2017 +0000
Initialize target description early in IPA
I think passing the index is now more trouble than its worth.
We used to pass the index, and then use that index to lookup which
target description to instantiate and use. However, the above commit
fixed an issue where we can't call malloc() within (certain parts of)
the IPA (apparently), so instead we now pre-compute _every_ possible
target description within the IPA. The index is now only used to
lookup which of the (many) pre-computed target descriptions to use.
It would (I think) have been easier all around if the IPA just
self-inspected, figured out its own xcr0 value, and used that to
create the one target description that is required. So long as the
xcr0 to target description code is shared (at compile time) with
gdbserver, then we can be sure that the IPA will derive the same
target description as gdbserver, and we would avoid all this index
passing business, which has made this commit so very, very painful.
** END OF AN ASIDE **
Currently then for x86/linux, gdbserver sends a number between 0 and 7
to the IPA, and the IPA uses this to create a target description.
However, I am proposing that gdbserver should now create one of (up
to) 64 different target descriptions for i386, so this 0 to 7 index
isn't going to be good enough any more (amd64 and x32 have slightly
fewer possible target descriptions, but still more than 8, so the
problem is the same).
For a while I wondered if I was going to have to try and find some
backward compatible solution for this mess. But after seeing how
lightly the IPA is actually documented, I wonder if it is not the case
that there is a tight coupling between a version of gdbserver and a
version of the IPA? At least I'm hoping so.
In this commit I have thrown out the old IPA target description index
numbering scheme, and switched to a completely new numbering scheme.
Instead of the index that is passed being arbitrary, the index is
instead calculated from the set of cpu features that are present on
the target. Within the IPA we can then reverse this logic to recreate
the xcr0 value based on the index, and from the xcr0 value we can
create the correct target description.
With the gdbserver to IPA numbering scheme issue resolved I have then
update the gdbserver versions of amd64_linux_read_description and
i386_linux_read_description so that they create target descriptions
using the same set of cpu features as GDB itself.
After this gdbserver should now always come up with the same target
description as GDB does on any x86/Linux target.
This commit does not introduce any new code sharing between GDB and
gdbserver as previous commits in this series does. Instead this
commit is all about bringing GDB and gdbserver into alignment
functionally so that the next commit can merge the GDB and gdbserver
versions of these functions.
Approved-By: John Baldwin <jhb@FreeBSD.org>
|
|
This commit is part of a series to share more of the x86 target
description creation code between GDB and gdbserver.
Unlike previous commits which were mostly refactoring, this commit is
the first that makes a real change, though that change should mostly
be for gdbserver; I've largely adopted the "GDB" way of doing things
for gdbserver, and this fixes a real gdbserver bug.
On a x86-64 Linux target, running the test:
gdb.server/connect-with-no-symbol-file.exp
results in two core files being created. Both of these core files are
from the inferior process, created after gdbserver has detached.
In this test a gdbserver process is started and then, after gdbserver
has started, but before GDB attaches, we either delete the inferior
executable, or change its permissions so it can't be read. Only after
doing this do we attempt to connect with GDB.
As GDB connects to gdbserver, gdbserver attempts to figure out the
target description so that it can send the description to GDB, this
involves a call to x86_linux_read_description.
In x86_linux_read_description one of the first things we do is try to
figure out if the process is 32-bit or 64-bit. To do this we look up
the executable via the thread-id, and then attempt to read the
architecture size from the executable. This isn't going to work if
the executable has been deleted, or is no longer readable.
And so, as we can't read the executable, we default to an i386 target
and use an i386 target description.
A consequence of using an i386 target description is that addresses
are assumed to be 32-bits. Here's an example session that shows the
problems this causes. This is run on an x86-64 machine, and the test
binary (xx.x) is a standard 64-bit x86-64 binary:
shell_1$ gdbserver --once localhost :54321 /tmp/xx.x
shell_2$ gdb -q
(gdb) set sysroot
(gdb) shell chmod 000 /tmp/xx.x
(gdb) target remote :54321
Remote debugging using :54321
warning: /tmp/xx.x: Permission denied.
0xf7fd3110 in ?? ()
(gdb) show architecture
The target architecture is set to "auto" (currently "i386").
(gdb) p/x $pc
$1 = 0xf7fd3110
(gdb) info proc mappings
process 2412639
Mapped address spaces:
Start Addr End Addr Size Offset Perms objfile
0x400000 0x401000 0x1000 0x0 r--p /tmp/xx.x
0x401000 0x402000 0x1000 0x1000 r-xp /tmp/xx.x
0x402000 0x403000 0x1000 0x2000 r--p /tmp/xx.x
0x403000 0x405000 0x2000 0x2000 rw-p /tmp/xx.x
0xf7fcb000 0xf7fcf000 0x4000 0x0 r--p [vvar]
0xf7fcf000 0xf7fd1000 0x2000 0x0 r-xp [vdso]
0xf7fd1000 0xf7fd3000 0x2000 0x0 r--p /usr/lib64/ld-2.30.so
0xf7fd3000 0xf7ff3000 0x20000 0x2000 r-xp /usr/lib64/ld-2.30.so
0xf7ff3000 0xf7ffb000 0x8000 0x22000 r--p /usr/lib64/ld-2.30.so
0xf7ffc000 0xf7ffe000 0x2000 0x2a000 rw-p /usr/lib64/ld-2.30.so
0xf7ffe000 0xf7fff000 0x1000 0x0 rw-p
0xfffda000 0xfffff000 0x25000 0x0 rw-p [stack]
0xff600000 0xff601000 0x1000 0x0 r-xp [vsyscall]
(gdb) info inferiors
Num Description Connection Executable
* 1 process 2412639 1 (remote :54321)
(gdb) shell cat /proc/2412639/maps
00400000-00401000 r--p 00000000 fd:03 45907133 /tmp/xx.x
00401000-00402000 r-xp 00001000 fd:03 45907133 /tmp/xx.x
00402000-00403000 r--p 00002000 fd:03 45907133 /tmp/xx.x
00403000-00405000 rw-p 00002000 fd:03 45907133 /tmp/xx.x
7ffff7fcb000-7ffff7fcf000 r--p 00000000 00:00 0 [vvar]
7ffff7fcf000-7ffff7fd1000 r-xp 00000000 00:00 0 [vdso]
7ffff7fd1000-7ffff7fd3000 r--p 00000000 fd:00 143904 /usr/lib64/ld-2.30.so
7ffff7fd3000-7ffff7ff3000 r-xp 00002000 fd:00 143904 /usr/lib64/ld-2.30.so
7ffff7ff3000-7ffff7ffb000 r--p 00022000 fd:00 143904 /usr/lib64/ld-2.30.so
7ffff7ffc000-7ffff7ffe000 rw-p 0002a000 fd:00 143904 /usr/lib64/ld-2.30.so
7ffff7ffe000-7ffff7fff000 rw-p 00000000 00:00 0
7ffffffda000-7ffffffff000 rw-p 00000000 00:00 0 [stack]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall]
(gdb)
Notice the difference between the mappings reported via GDB and those
reported directly from the kernel via /proc/PID/maps, the addresses of
every mapping is clamped to 32-bits for GDB, while the kernel reports
real 64-bit addresses.
Notice also that the $pc value is a 32-bit value. It appears to be
within one of the mappings reported by GDB, but is outside any of the
mappings reported from the kernel.
And this is where the problem arises. When gdbserver detaches from
the inferior we pass the inferior the address from which it should
resume. Due to the 32/64 bit confusion we tell the inferior to resume
from the 32-bit $pc value, which is not within any valid mapping, and
so, as soon as the inferior resumes, it segfaults.
If we look at how GDB (not gdbserver) figures out its target
description then we see an interesting difference. GDB doesn't try to
read the executable. Instead GDB uses ptrace to query the thread's
state, and uses this to figure out the if the thread is 32 or 64 bit.
If we update gdbserver to do it the "GDB" way then the above problem
is resolved, gdbserver now sees the process as 64-bit, and when we
detach from the inferior we give it the correct 64-bit address, and
the inferior no longer segfaults.
Now, I could just update the gdbserver code, but better, I think, to
share one copy of the code between GDB and gdbserver in gdb/nat/.
That is what this commit does.
The cores of x86_linux_read_description from gdbserver and
x86_linux_nat_target::read_description from GDB are moved into a new
file gdb/nat/x86-linux-tdesc.c and combined into a single function
x86_linux_tdesc_for_tid which is called from each location.
This new function does things the GDB way, the only changes are to
allow for the sharing; we now have a callback function to call the
first time that the xcr0 state is read, this allows for GDB and
gdbserver to perform their own initialisation as needed, and
additionally, the new function takes a pointer for where to cache the
xcr0 value, this isn't needed for this commit, but will be useful in a
later commit where gdbserver will want to read this cached xcr0
value.
Another thing to note about this commit is how the functions
i386_linux_read_description and amd64_linux_read_description are
handled. For now I've left these function as implemented separately
in GDB and gdbserver. I've moved the declarations of these functions
into gdb/nat/x86-linux-tdesc.h, but the implementations are left as
separate.
A later commit in this series will make these functions shared too,
but doing this is not trivial, so I've left that for a separate
commit. Merging the declarations as I've done here ensures that
everyone implements the function to the same API, and once these
functions are shared (in a later commit) we'll want a shared
declaration anyway.
Approved-By: John Baldwin <jhb@FreeBSD.org>
|
|
Share the definition of I386_LINUX_XSAVE_XCR0_OFFSET between GDB and
gdbserver.
This commit is part of a series that aims to share more of the x86
target description creation code between GDB and gdbserver. The
I386_LINUX_XSAVE_XCR0_OFFSET #define is used as part of the target
description creation, and I noticed that this constant is defined
separately for GDB and gdbserver.
This commit moves the definition into gdb/nat/x86-linux.h, which
allows the #define to be shared.
There should be no user visible changes after this commit.
Approved-By: John Baldwin <jhb@FreeBSD.org>
|
|
This commit is part of a series that aims to share more of the x86
target description reading/generation code between GDB and gdbserver.
There are a huge number of similarities between the code in
gdbserver's x86_linux_read_description function and GDB's
x86_linux_nat_target::read_description function, and it is this
similarity that I plan, in a later commit, to share between GDB and
gdbserver.
However, one thing that is different in x86_linux_read_description is
the code inside the '!use_xml' block. This is the code that handles
the case where gdbserver is not allowed to send an XML target
description back to GDB. In this case gdbserver uses some predefined,
fixed, target descriptions.
First, it's worth noting that I suspect this code is not tested any
more. I couldn't find anything in the testsuite that tries to disable
XML target description support. And the idea of having a single
"fixed" target description really doesn't work well when we think
about all the various x86 extensions that exist. Part of me would
like to rip out the no-xml support in gdbserver (at least for x86),
and if a GDB connects that doesn't support XML target descriptions,
gdbserver can just give an error and drop the connection. GDB has
supported XML target descriptions for 16 years now, I think it would
be reasonable for our shipped gdbserver to drop support for the old
way of doing things.
Anyway.... this commit doesn't do that.
What I did notice was that, over time, the '!use_xml' block appears to
have "drifted" within the x86_linux_read_description function; it's
now not the first check we do. Instead we make some ptrace calls and
return a target description generated based on the result of these
ptrace calls. Surely it only makes sense to generate variable target
descriptions if we can send these back to GDB?
So in this commit I propose to move the '!use_xml' block earlier in
the x86_linux_read_description function.
The benefit of this is that this leaves the later half of
x86_linux_read_description much more similar to the GDB function
x86_linux_nat_target::read_description and sets us up for potentially
sharing code between GDB and gdbserver in a later commit.
Approved-By: John Baldwin <jhb@FreeBSD.org>
|
|
Convert the have_ptrace_getregset global within gdbserver to a
tribool. This brings the flag into alignment with the corresponding
flag in GDB.
The gdbserver have_ptrace_getregset variable is already used as a
tribool, it just doesn't have the tribool type.
In a future commit I plan to share more code between GDB and
gdbserver, and having this variable be the same type in both code
bases will make the sharing much easier.
There should be no user visible changes after this commit.
Approved-By: John Baldwin <jhb@FreeBSD.org>
|
|
Since MPX isn't available for x32, we should clear X86_XSTATE_MPX bits
on x32.
PR server/31511
* linux-x86-low.cc (x86_linux_read_description): Clear
X86_XSTATE_MPX bits in xcr0 on x32.
Reviewed-by: Felix Willgerodt <felix.willgerodt@intel.com>
|
