| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
After the x86 target description changes that I committed recently,
the first commit in the series being:
commit 8a29222b85f28a2201db50a34ac4144f961311db
Date: Sat Jan 27 10:40:35 2024 +0000
gdb/gdbserver: share I386_LINUX_XSAVE_XCR0_OFFSET definition
and the last commit in the series being:
commit 646d754d14c2fe70a492a893506a74b0463b6ae8
Author: Andrew Burgess <aburgess@redhat.com>
Date: Tue Jan 30 15:37:23 2024 +0000
gdb/gdbserver: share x86/linux tdesc caching
The sourceware buildbot highlighted a regression on i386. On the GDB
side we'd see this:
Remote debugging using :54321
warning: Architecture rejected target-supplied description
Remote connection closed
(gdb)
while on the gdbserver side we'd see this:
$ ./gdbserver/gdbserver --once :54321 ~/empty
Process /srv/aburgess/empty created; pid = 31406
Listening on port 54321
Remote debugging from host ::1, port 39488
../../src/gdbserver/regcache.cc:272: A problem internal to GDBserver has been detected.
Unknown register st0 requested
Aborted (core dumped)
When I tried to reproduce this regression on my local i386 VM the
issue would not reproduce.
I eventually tracked the problem down to x86_linux_tdesc_for_tid in
gdb/nat/x86-linux-tdesc.c. In this function we have this line:
/* Check if PTRACE_GETREGSET works. */
if (ptrace (PTRACE_GETREGSET, tid,
(unsigned int) NT_X86_XSTATE, &iov) < 0)
{
... handle failure ...
}
else
{
... handle success ...
}
The problem is that on my VM the PTRACE_GETREGSET feature is
supported, while on sourceware's buildbot machine this feature is not
supported.
I did a quick search and it seems like the 'xsave' feature in
/proc/cpuinfo might be the indicator for whether PTRACE_GETREGSET is
supported or not, and indeed my machine has the 'xsave' feature while
the sourceware machine does not.
The point of divergence then is this ptrace call, on my machine the
call succeeds and we extract the xcr0 value from the iov vector, while
on the sourceware machine the ptrace call fails and we use a default
xcr0 value of 0.
This xcr0 value is then passed to i386_linux_read_description at the
end of x86_linux_tdesc_for_tid.
In gdb/arch/i386-linux-tdesc.c we find i386_linux_read_description
which does some caching but calls i386_create_target_description to
actually create the target descriptions when needed. The xcr0 value
is masked to only the bits that are interesting, but given a value of
0 we'll just pass 0 through to i386_create_target_description.
In gdb/arch/i386.c we find i386_create_target_description which checks
the xcr0 bits and builds the target description. What we can see is
that if no bits are set in the xcr0 value then no features will be
added to the created target description. This featureless target
description is then transmitted back to GDB, which is then rejected
due to lack of essential core registers.
So, how did things work prior to the above commit series? There are
three places of interest, on the GDB side there is
x86_linux_nat_target::read_description and
i386_linux_core_read_description. Then on the gdbserver side there is
x86_linux_read_description.
All of these locations have a call to i386_linux_read_description
followed by a check if the return value was nullptr. If we do get
back nullptr then we perform another call to
i386_linux_read_description with a default xcr0 value.
Looking in i386_linux_read_description we see a specific check for
xcr0 being 0 in which case we return nullptr.
And so, prior to the above series, if xcr0 was 0 due to
PTRACE_GETREGSET being unavailable we'd use a default xcr0 value.
After the above series this is no longer the case, the 'xcr0 == 0'
check has been removed from i386_linux_read_description and the
calling code is streamlined to remove the use of default xcr0 values.
The fix I propose here is to setup the default xcr0 value at the point
where we find that PTRACE_GETREGSET is unavailable. The default value
used is X86_XSTATE_SSE_MASK. This is the default used in
x86_linux_nat_target::read_description (for GDB) and in
x86_linux_read_description (for gdbserver). The above commit series
already fixed i386_linux_core_read_description to ensure that the
correct default xcr0 value was used, this case is a little special in
that it uses different defaults depending on which sections are
present in the core file, so that case always needed to be handled
differently.
The choice of X86_XSTATE_SSE_MASK corresponds to the default used for
i386 before the above series was committed. This mask includes the
X87 and SSE bits only, neither of these bits are checked for on amd64
or x32, so this default doesn't change the behaviour on these targets.
By setting the default xcr0 value at this early stage we ensure that
the cached xcr0 value on the gdbserver side is correct. This is
critical as this cached xcr0 value is passed through to the in process
agent (IPA). If we leave the cached xcr0 value as 0 and apply the
defaults later in the series we also have to encode the knowledge of
the default into the IPA, this just means we have the default encoded
in multiple locations, which seems like a bad idea. The approach used
in this patch means the default is present in just one location.
This commit should fix the i386 regressions seen on the sourceware
buildbot.
In addition to the fix in nat/x86-linux-tdesc.c I've also fixed the
layout of the declaration of x86_linux_tdesc_for_tid in the header
file.
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>
|
|
This patch is part of a series that has the aim sharing the x86 Linux
target description creation code between GDB and gdbserver.
Within GDB part of this process involves reading the cs and ds state
from the 'struct user_regs_struct' using a ptrace call.
This isn't done by gdbserver, which is part of the motivation for this
whole series; the approach gdbserver takes is inferior to the approach
GDB takes (gdbserver relies on reading the file being debugged, and
extracting similar information from the file headers).
This commit moves the reading of cs and ds, which is used to figure
out if a thread is 32-bit or 64-bit (or in x32 mode), into the gdb/nat
directory so that the code can be shared with gdbserver, but at this
point I'm not actually using the code in gdbserver, that will come
later.
As such there should be no user visible changes after this commit, GDB
continues to do things as it did before (reading cs/ds), while
gdbserver continues to use its own approach (which doesn't require
reading cs/ds).
Approved-By: John Baldwin <jhb@FreeBSD.org>
Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
|
|
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>
|
|
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>
|
|
The `fork_inferior()` function accepts multiple callbacks, making its
signature a bit hard to read. Define some type aliases to make it a bit
clearer. Use function view for all, while at it.
Change-Id: Ide8d1fa533d0c5eaf3249860f8c0d339baa09bce
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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>
|
|
When GDB attaches to a multi-threaded process, it calls
linux_proc_attach_tgid_threads () to go through all threads found in
/proc/PID/task/ and call attach_proc_task_lwp_callback () on each of
them. If it does that twice without the callback reporting that a new
thread was found, then it considers that all inferior threads have been
found and returns.
The problem is that the callback considers any thread that it hasn't
attached to yet as new. This causes problems if the process has one or
more zombie threads, because GDB can't attach to it and the loop will
always "find" a new thread (the zombie one), and get stuck in an
infinite loop.
This is easy to trigger (at least on aarch64-linux and powerpc64le-linux)
with the gdb.threads/attach-many-short-lived-threads.exp testcase, because
its test program constantly creates and finishes joinable threads so the
chance of having zombie threads is high.
This problem causes the following failures:
FAIL: gdb.threads/attach-many-short-lived-threads.exp: iter 8: attach (timeout)
FAIL: gdb.threads/attach-many-short-lived-threads.exp: iter 8: no new threads (timeout)
FAIL: gdb.threads/attach-many-short-lived-threads.exp: iter 8: set breakpoint always-inserted on (timeout)
FAIL: gdb.threads/attach-many-short-lived-threads.exp: iter 8: break break_fn (timeout)
FAIL: gdb.threads/attach-many-short-lived-threads.exp: iter 8: break at break_fn: 1 (timeout)
FAIL: gdb.threads/attach-many-short-lived-threads.exp: iter 8: break at break_fn: 2 (timeout)
FAIL: gdb.threads/attach-many-short-lived-threads.exp: iter 8: break at break_fn: 3 (timeout)
FAIL: gdb.threads/attach-many-short-lived-threads.exp: iter 8: reset timer in the inferior (timeout)
FAIL: gdb.threads/attach-many-short-lived-threads.exp: iter 8: print seconds_left (timeout)
FAIL: gdb.threads/attach-many-short-lived-threads.exp: iter 8: detach (timeout)
FAIL: gdb.threads/attach-many-short-lived-threads.exp: iter 8: set breakpoint always-inserted off (timeout)
FAIL: gdb.threads/attach-many-short-lived-threads.exp: iter 8: delete all breakpoints, watchpoints, tracepoints, and catchpoints in delete_breakpoints (timeout)
ERROR: breakpoints not deleted
The iteration number is random, and all tests in the subsequent iterations
fail too, because GDB is stuck in the attach command at the beginning of
the iteration.
The solution is to make linux_proc_attach_tgid_threads () remember when it
has already processed a given LWP and skip it in the subsequent iterations.
PR testsuite/31312
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31312
Reviewed-By: Luis Machado <luis.machado@arm.com>
Approved-By: Pedro Alves <pedro@palves.net>
|
|
The new function will be used in a subsequent patch to read a different
stat field.
The new code is believed to be equivalent to the old code, so there
should be no change in GDB behaviour. The only material change was to
use std::string and string_printf rather than a fixed char array to
build the path to the stat file.
Also, take the opportunity to move the function's documentation comment to
the header file, to conform with GDB practice.
Reviewed-By: Luis Machado <luis.machado@arm.com>
Approved-By: Pedro Alves <pedro@palves.net>
|
|
The code and comment reference stat fields by made-up indexes. The
procfs(5) man page, which describes the /proc/PID/stat file, has a numbered
list of these fields so it's more convenient to use those numbers instead.
This is currently an implementation detail inside the function so it's
not really relevant with the code as-is, but a future patch will do some
refactoring which will make the index more prominent.
Therefore, make this change in a separate patch so that it's simpler to
review.
Reviewed-By: Luis Machado <luis.machado@arm.com>
Approved-By: Pedro Alves <pedro@palves.net>
|
|
clangd tells me that the gdb_signals.h include in target/waitstatus.h is
unused. This include was probably to give access to `enum gdb_signal`,
but this is in fact defined in gdb/signals.h. Change the include to
gdb/signals.h. Include gdbsupport/gdb_signals.h in some files that were
relying on the transitive include.
Change-Id: I6f4361b3d801394bf29abe8c1393aff110aa0ad6
|
|
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
|
|
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>
|
|
This reverts commit 01ed1674d4435aa4e194fd9373b7705e425ef354.
|
|
This reverts commit 7816b81e9b36ea0f57662bfd7446b573bf0c9e54.
|
|
This reverts commit cd9b374ffe372dcaf7e4c15548cf53a301d8dcdd.
|
|
This reverts commit 198ff6ff819c240545f9fc68b39636fd376d4ba9.
|
|
This reverts commit f4c19f89ef43dbce8065532c808e1aeb05d08994.
|
|
This commit:
commit 198ff6ff819c240545f9fc68b39636fd376d4ba9
Date: Tue Jan 30 15:37:23 2024 +0000
gdb/gdbserver: share x86/linux tdesc caching
added some functions which are always defined, but their use is
guarded within various #ifdef blocks. As a result we were seeing
errors about defined, but unused, functions.
I've fixed this problem in this commit by wrapping the function
definitions within #ifdef blocks.
I'm a little worried that there might be too many #ifdef blocks within
this file, however, I'm going to commit this fix for now as this will
fix the build, then I'll think about if there's a better way to split
this file so we might avoid some of these #ifdef blocks.
|
|
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 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 patch is part of a series that has the aim of making the code
that, for x86, reads the target description for a native process
shared between GDB and gdbserver.
Within GDB part of this process involves reading the cs and ds state
from the 'struct user_regs_struct' using a ptrace call.
This isn't done by gdbserver, which is part of the motivation for this
whole series; the approach gdbserver takes is inferior to the approach
GDB takes.
This commit moves the reading of cs and ds, which is used to figure
out if a thread is 32-bit or 64-bit (or in x32 mode), into the gdb/nat
directory so that the code could be shared with gdbserver, but at this
point I'm not actually using the code in gdbserver, that will come
later.
As such there should be no user visible changes after this commit, GDB
continues to do things as it did before (reading cs/ds), while
gdbserver continues to use its own approach (which doesn't require
reading cs/ds).
Approved-By: John Baldwin <jhb@FreeBSD.org>
|
|
On aarch64-linux, with test-case gdb.base/watch-bitfields.exp I run into:
...
(gdb) continue^M
Continuing.^M
^M
Hardware watchpoint 2: -location q.a^M
^M
Old value = 1^M
New value = 0^M
main () at watch-bitfields.c:42^M
42 q.h--;^M
(gdb) FAIL: $exp: -location watch against bitfields: q.e: 0->5: continue
...
In a minimal form, if we step past line 37 which sets q.e, and we have a
watchpoint set on q.e, it triggers:
...
$ gdb -q -batch watch-bitfields -ex "b 37" -ex run -ex "watch q.e" -ex step
Breakpoint 1 at 0x410204: file watch-bitfields.c, line 37.
Breakpoint 1, main () at watch-bitfields.c:37
37 q.e = 5;
Hardware watchpoint 2: q.e
Hardware watchpoint 2: q.e
Old value = 0
New value = 5
main () at /home/vries/gdb/src/gdb/testsuite/gdb.base/watch-bitfields.c:38
38 q.f = 6;
...
However, if we set in addition a watchpoint on q.a, the watchpoint on q.e
doesn't trigger.
How does this happen?
Bitfield q.a is just bit 0 of byte 0, and bitfield q.e is bit 4..7 of byte 1
and bit 1 of byte 2. So, watch q.a should watch byte 0, and watch q.e should
watch bytes 1 and 2.
Using "maint set show-debug-regs on" (and some more detailed debug prints) we
get:
...
WP2: addr=0x440028 (orig=0x440029), ctrl=0x000000d5, ref.count=1
ctrl: enabled=1, offset=1, len=2
WP3: addr=0x440028 (orig=0x440028), ctrl=0x00000035, ref.count=1
ctrl: enabled=1, offset=0, len=1
...
which matches that.
When executing line 37, a hardware watchpoint trap triggers and we hit
aarch64_stopped_data_address with addr_trap == 0x440028:
...
(gdb) p /x addr_trap
$1 = 0x440028
....
and since the loop in aarch64_stopped_data_address walks backward, we check
WP3 first, which matches, and consequently target_stopped_by_watchpoint
returns true in watchpoints_triggered.
Likewise for target_stopped_data_address, which also returns addr == 0x440028.
Watchpoints_triggered matches watchpoint q.a to that address, and sets
watch_triggered_yes.
However, subsequently the value of q.a is checked, and it's the same value as
before (becase the insn in line 37 didn't change q.a), so the watchpoint
hardware trap is not reported to the user.
The problem originates from that fact that aarch64_stopped_data_address picked
WP3 instead of WP2.
There's something we can do about this. In the example above, both
target_stopped_by_watchpoint and target_stopped_data_address returned true.
Instead we can return true in target_stopped_by_watchpoint but false in
target_stopped_data_address. This lets watchpoints_triggered known that a
watchpoint was triggered, but we don't know where, and both watchpoints
get set to watch_triggered_unknown.
Subsequently, the values of both q.a and q.e are checked, and since q.e is not
the same value as before, the watchpoint hardware trap is reported to the user.
Note that this works well for regular (write) watchpoints (watch command), but
not for read watchpoints (rwatch command), because for those no value is
checked. Likewise for access watchpoints (awatch command).
So, fix this by:
- passing a nullptr in aarch64_fbsd_nat_target::stopped_by_watchpoint and
aarch64_linux_nat_target::stopped_by_watchpoint to make clear we're not
interested in the stop address,
- introducing a two-phase approach in aarch64_stopped_data_address, where:
- phase one handles access and read watchpoints, as before, and
- phase two handles write watchpoints, where multiple matches cause:
- return true if addr_p == null, and
- return false if addr_p != null.
Tested on aarch64-linux.
Approved-By: Luis Machado <luis.machado@arm.com>
PR tdep/31214
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31214
|
|
When we improved error messages in
cd393cec3ab gdb, btrace: improve error messages
we cleared the original errno. When the error reason can not be explained
in a more detailed error message, and we fall back to the default error
message, it now gives Success as error.
Restore the original errno to fix that.
|
|
Bug PR gdb/28313 describes attaching to a process when the executable
has been deleted. The bug is for S390 and describes how a user sees a
message 'PC not saved'.
On x86-64 (GNU/Linux) I don't see a 'PC not saved' message, but
instead I see this:
(gdb) attach 901877
Attaching to process 901877
No executable file now.
warning: Could not load vsyscall page because no executable was specified
0x00007fa9d9c121e7 in ?? ()
(gdb) bt
#0 0x00007fa9d9c121e7 in ?? ()
#1 0x00007fa9d9c1211e in ?? ()
#2 0x0000000000000007 in ?? ()
#3 0x000000002dc8b18d in ?? ()
#4 0x0000000000000000 in ?? ()
(gdb)
Notice that the addresses in the backtrace don't seem right, quickly
heading to 0x7 and finally ending at 0x0.
What's going on, in both the s390 case and the x86-64 case is that the
architecture's prologue scanner is going wrong and causing the stack
unwinding to fail.
The prologue scanner goes wrong because GDB has no unwind information.
And GDB has no unwind information because, of course, the executable
has been deleted.
Notice in the example session above we get this line in the output:
No executable file now.
which indicates that GDB failed to find an executable to debug.
For GNU/Linux when GDB tries to find an executable for a given pid we
end up calling linux_proc_pid_to_exec_file in gdb/nat/linux-procfs.c.
Within this function we call `readlink` on /proc/PID/exe to find the
path of the actual executable.
If the `readlink` call fails then we already fallback on using
/proc/PID/exe as the path to the executable to debug.
However, when the executable has been deleted the `readlink` call
doesn't fail, but the path that is returned points to a non-existent
file.
I propose that we add an `access` call to linux_proc_pid_to_exec_file
to check that the target file exists and can be read. If the target
can't be read then we should fall back to /proc/PID/exe (assuming that
/proc/PID/exe can be read).
Now on x86-64 the output looks like this:
(gdb) attach 901877
Attaching to process 901877
Reading symbols from /proc/901877/exe...
Reading symbols from /lib64/libc.so.6...
(No debugging symbols found in /lib64/libc.so.6)
Reading symbols from /lib64/ld-linux-x86-64.so.2...
(No debugging symbols found in /lib64/ld-linux-x86-64.so.2)
0x00007fa9d9c121e7 in nanosleep () from /lib64/libc.so.6
(gdb) bt
#0 0x00007fa9d9c121e7 in nanosleep () from /lib64/libc.so.6
#1 0x00007fa9d9c1211e in sleep () from /lib64/libc.so.6
#2 0x000000000040117e in spin_forever () at attach-test.c:17
#3 0x0000000000401198 in main () at attach-test.c:24
(gdb)
which is much better.
I've also tagged the bug PR gdb/29782 which concerns the test
gdb.server/connect-with-no-symbol-file.exp. After making this change,
when running gdb.server/connect-with-no-symbol-file.exp GDB would now
pick up the /proc/PID/exe file as the executable in some cases.
As GDB is not restarted for the multiple iterations of this test
GDB (or rather BFD) would given a warning/error like:
(gdb) PASS: gdb.server/connect-with-no-symbol-file.exp: sysroot=target:: action=permission: setup: disconnect
set sysroot target:
BFD: reopening /proc/3283001/exe: No such file or directory
(gdb) FAIL: gdb.server/connect-with-no-symbol-file.exp: sysroot=target:: action=permission: setup: adjust sysroot
What's happening is that an executable found for an earlier iteration
of the test is still registered for the inferior when we are setting
up for a second iteration of the test. When the sysroot changes, if
there's an executable registered GDB tries to reopen it, but in this
case the file has disappeared (the previous inferior has exited by
this point).
I did think about maybe, when the executable is /proc/PID/exe, we
should auto-delete the file from the inferior. But in the end I
thought this was a bad idea. Not only would this require a lot of
special code in GDB just to support this edge case: we'd need to track
if the exe file name came from /proc and should be auto-deleted, or
we'd need target specific code to check if a path should be
auto-deleted.....
... in addition, we'd still want to warn the user when we auto-deleted
the file from the inferior, otherwise they might be surprised to find
their inferior suddenly has no executable attached, so we wouldn't
actually reduce the number of warnings the user sees.
So in the end I figured that the best solution is to just update the
test to avoid the warning. This is easily done by manually removing
the executable from the inferior once each iteration of the test has
completed.
Now, in bug PR gdb/29782 GDB is clearly managing to pick up an
executable from the NFS cache somehow. I guess what's happening is
that when the original file is deleted /proc/PID/exe is actually
pointing to a file in the NFS cache which is only deleted at some
later point, and so when GDB starts up we do manage to associate a
file with the inferior, this results in the same message being emitted
from BFD as I was seeing. The fix included in this commit should also
fix that bug.
One final note: On x86-64 GNU/Linux, the
gdb.server/connect-with-no-symbol-file.exp test will produce 2 core
files. This is due to a bug in gdbserver that is nothing to do with
this test. These core files are created before and after this
commit. I am working on a fix for the gdbserver issue, but will post
that separately.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28313
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29782
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This commit is the result of the following actions:
- Running gdb/copyright.py to update all of the copyright headers to
include 2024,
- Manually updating a few files the copyright.py script told me to
update, these files had copyright headers embedded within the
file,
- Regenerating gdbsupport/Makefile.in to refresh it's copyright
date,
- Using grep to find other files that still mentioned 2023. If
these files were updated last year from 2022 to 2023 then I've
updated them this year to 2024.
I'm sure I've probably missed some dates. Feel free to fix them up as
you spot them.
|
|
Change most of regcache (and base classes) to use array_view when
possible, instead of raw pointers. By propagating the use of array_view
further, it enables having some runtime checks to make sure the what we
read from or write to regcaches has the expected length (such as the one
in the `copy(array_view, array_view)` function. It also integrates well
when connecting with other APIs already using gdb::array_view.
Add some overloads of the methods using raw pointers to avoid having to
change all call sites at once (which is both a lot of work and risky).
I tried to do this change in small increments, but since many of these
functions use each other, it ended up simpler to do it in one shot than
having a lot of intermediary / transient changes.
This change extends into gdbserver as well, because there is some part
of the regcache interface that is shared.
Changing the reg_buffer_common interface to use array_view caused some
build failures in nat/aarch64-scalable-linux-ptrace.c. That file
currently "takes advantage" of the fact that
reg_buffer_common::{raw_supply,raw_collect} operates on `void *`, which
IMO is dangerous. It uses raw_supply/raw_collect directly on
uint64_t's, which I guess is fine because it is expected that native
code will have the same endianness as the debugged process. To
accomodate that, add some overloads of raw_collect and raw_supply that
work on uint64_t.
This file also uses raw_collect and raw_supply on `char` pointers.
Change it to use `gdb_byte` pointers instead. Add overloads of
raw_collect and raw_supply that work on `gdb_byte *` and make an
array_view on the fly using the register's size. Those call sites could
be converted to use array_view with not much work, in which case these
overloads could be removed, but I didn't want to do it in this patch, to
avoid starting to dig in arch-specific code.
During development, I inadvertently changed reg_buffer::raw_compare's
behavior to not accept an offset equal to the register size. This
behavior (effectively comparing 0 bytes, returning true) change was
caught by the AArch64 SME core tests. Add a selftest to make sure that
this raw_compare behavior is preserved in the future.
Change-Id: I9005f04114543ddff738949e12d85a31855304c2
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
|
|
Right now, gdbsupport/common-regcache.h contains two abstractons for a
regcache. An opaque type `regcache` (gdb and gdbserver both have their
own regcache that is the concrete version of this) and an abstract base
class `reg_buffer_common`, that is the base of regcaches on both sides.
These abstractions allow code to be written for both gdb and gdbserver,
for instance in the gdb/arch sub-directory.
However, having two
different abstractions is impractical. If some common code has a regcache,
and wants to use an operation defined on reg_buffer_common, it can't.
It would be better to have just one. Change all instances of `regcache
*` in gdbsupport/common-regcache.h to be `reg_buffer_common *`, then fix
fallouts.
Implementations in gdb and gdbserver now need to down-cast (using
gdb::checked_static_cast) from reg_buffer_common to their concrete
regcache type. Some of them could be avoided by changing free functions
(like regcache_register_size) to be virtual methods on
reg_buffer_common. I tried it, it seems to work, but I did not include
it in this series to avoid adding unnecessary changes.
Change-Id: Ia5503adb6b5509a0f4604bd2a68b4642cc5283fd
Reviewed-by: John Baldwin <jhb@FreeBSD.org>
|
|
This changes linux_proc_attach_tgid_threads to use gdb_dir_up. This
makes it robust against exceptions.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
linux_proc_attach_tgid_threads computes a file name, and then
re-computes it for a warning. It is better to reuse the
already-computed name here.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
C++17 makes the second parameter to static_assert optional, so we can
remove gdb_static_assert now.
|
|
This changes gdb to use the C++17 [[fallthrough]] attribute rather
than special comments.
This was mostly done by script, but I neglected a few spellings and so
also fixed it up by hand.
I suspect this fixes the bug mentioned below, by switching to a
standard approach that, presumably, clang supports.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23159
Approved-By: John Baldwin <jhb@FreeBSD.org>
Approved-By: Luis Machado <luis.machado@arm.com>
Approved-By: Pedro Alves <pedro@palves.net>
|
|
The previous commit describes PR gdb/30547, a segfault when running test-case
gdb.base/vfork-follow-parent.exp on powerpc64 (likewise on s390x).
The root cause for the segmentation fault is that linux_is_uclinux gives an
incorrect result: it returns true instead of false.
So, why does linux_is_uclinux:
...
int
linux_is_uclinux (void)
{
CORE_ADDR dummy;
return (target_auxv_search (AT_NULL, &dummy) > 0
&& target_auxv_search (AT_PAGESZ, &dummy) == 0);
...
return true?
This is because ppc_linux_target_wordsize returns 4 instead of 8, causing
ppc_linux_nat_target::auxv_parse to misinterpret the auxv vector.
So, why does ppc_linux_target_wordsize:
...
int
ppc_linux_target_wordsize (int tid)
{
int wordsize = 4;
/* Check for 64-bit inferior process. This is the case when the host is
64-bit, and in addition the top bit of the MSR register is set. */
long msr;
errno = 0;
msr = (long) ptrace (PTRACE_PEEKUSER, tid, PT_MSR * 8, 0);
if (errno == 0 && ppc64_64bit_inferior_p (msr))
wordsize = 8;
return wordsize;
}
...
return 4?
Specifically, we get this result because because tid == 0, so we get
errno == ESRCH.
The tid == 0 is caused by the switch_to_no_thread in
handle_vfork_child_exec_or_exit:
...
/* Switch to no-thread while running clone_program_space, so
that clone_program_space doesn't want to read the
selected frame of a dead process. */
scoped_restore_current_thread restore_thread;
switch_to_no_thread ();
inf->pspace = new program_space (maybe_new_address_space ());
...
but moving the maybe_new_address_space call to before that gives us the
same result. The tid is no longer 0, but we still get ESRCH because the
thread has exited.
Fix this in handle_vfork_child_exec_or_exit by doing the
maybe_new_address_space call in the context of the vfork parent.
Tested on top of trunk on x86_64-linux and ppc64le-linux.
Tested on top of gdb-14-branch on ppc64-linux.
Co-Authored-By: Simon Marchi <simon.marchi@polymtl.ca>
PR gdb/30547
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30547
|
|
This introduces throw_winerror_with_name, a Windows analog of
perror_with_name, and changes various places in gdb to call it.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30770
|
|
Since GDB now requires C++17, we don't need the internally maintained
gdb::optional implementation. This patch does the following replacing:
- gdb::optional -> std::optional
- gdb::in_place -> std::in_place
- #include "gdbsupport/gdb_optional.h" -> #include <optional>
This change has mostly been done automatically. One exception is
gdbsupport/thread-pool.* which did not use the gdb:: prefix as it
already lives in the gdb namespace.
Change-Id: I19a92fa03e89637bab136c72e34fd351524f65e9
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Pedro Alves <pedro@palves.net>
|
|
gdb::make_unique is a wrapper around std::make_unique when compiled with
C++17. Now that C++17 is required, use std::make_unique directly in the
codebase, and remove gdb::make_unique.
Change-Id: I80b615e46e4b7c097f09d78e579a9bdce00254ab
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Pedro Alves <pedro@palves.net
|
|
common-defs.h has a few defines that I suspect were used during the
transition to C++. These aren't needed any more, so remove them.
Tested by rebuilding.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
GCC doesn't complain, but it's still wrong.
|
|
This header is only suitable for use on x86 hosts and is only included
there, so these fallbacks should not be needed.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
__SVE_VQ_BYTES is only available if SVE definitions are available in
the system's headers, and this is not true for all systems.
For this purpose, we define SVE_VQ_BYTES. This patch fixes the
name of the constant being used.
|
|
SME2 defines a new 512-bit register named ZT0, and it is only available
if SME is also supported. The ZT0 state is valid only if the SVCR ZA bit
is enabled. Otherwise its contents are empty (0).
The target description is dynamic and gets generated at runtime based on the
availability of the feature.
Validated under Fast Models.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
|
|
Teach gdb about the ZA/SSVE state on signal frames and how to restore
the contents of the registers.
There is a new ZA_MAGIC context that the Linux Kernel uses to communicate
the ZA register state to gdb.
The SVE_MAGIC context has also been adjusted to contain a flag indicating
whether it is a SVE or SSVE state.
Regression-tested on aarch64-linux Ubuntu 22.04/20.04.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
|
|
The SME (Scalable Matrix Extension) [1] exposes a new matrix register ZA with
variable sizes. It also exposes a new mode called streaming mode.
Similarly to SVE, the ZA register size is dictated by a vector length, but the
SME vector length is called streaming vetor length. The total size for
ZA in a given moment is svl x svl.
In streaming mode, the SVE registers have their sizes based on svl rather than
the regular vector length (vl).
The feature detection is controlled by the HWCAP2_SME bit, but actual support
should be validated by attempting a ptrace call for one of the new register
sets: NT_ARM_ZA and NT_ARM_SSVE.
Due to its large size, the ZA register is exposed as a vector of bytes, but we
introduce a number of pseudo-registers that gives various different views
into the ZA contents. These can be arranged in a couple categories: tiles and
tile slices.
Tiles are matrices the same size or smaller than ZA. Tile slices are vectors
which map to ZA's rows/columns in different ways.
A new dynamic target description is provided containing the ZA register, the SVG
register and the SVCR register. The size of ZA, like the SVE vector registers,
is based on the vector length register SVG (VG for SVE).
This patch enables SME register support for gdb.
[1] https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/scalable-matrix-extension-armv9-a-architecture
Co-Authored-By: Ezra Sitorus <ezra.sitorus@arm.com>
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
|
|
This is a patch in preparation to upcoming patches enabling SME support. It
attempts to simplify the gdb/gdbserver shared interface used to read/write
SVE registers.
Where the current code makes use of unique_ptr, allocating a new buffer by
hand and passing a buffer around, this patch makes that code use
gdb::byte_vector and passes a reference to this byte vector to the functions,
allowing the functions to have ready access to the size of the buffer.
It also shares a bit more code between gdb and gdbserver, in particular around
handling of ptrace get/set requests for SVE.
I think gdbserver could be refactored to handle register reads/writes more
like gdb's native layer as opposed to letting the generic linux-low layer do
the ptrace calls. This is not very flexible and assumes one size for the
responses. If you have something like NT_ARM_SVE, where you can have either
FPSIMD or SVE contents, it doesn't work that well.
I didn't want to change that interface right now as it is a bit too much work
and touches all the targets, some of which I can't easily test.
Hence the reason why the buffer the generic linux-now passes down to
linux-aarch64-low is unused or ignored.
No user-visible changes should happen as part of this refactor other than a
slightly reworded warning message.
While doing the refactor, I also noticed what seems to be a mistake in checking
if the register cache contains active (non-zero) SVE data.
For instance, the original code did something like this in
aarch64_sve_regs_copy_from_reg_buf:
has_sve_state |= reg_buf->raw_compare (AARCH64_SVE_Z0_REGNUM + i
reg, sizeof (__int128_t));
"reg" is a zeroed-out buffer that we compare the Z register contents
past the first 128 bits. The problem here is that raw_compare returns
1 if the contents compare the same, which means has_sve_state will be
true. But if we compared the Z register contents to 0, it means we
*do not* have SVE state, and therefore has_sve_state should be false.
The consequence of this mistake is that we convert the initial
FPSIMD-formatted data we get from ptrace for the NT_ARM_SVE register
set to a SVE-formatted one.
In the end, this doesn't cause user-visible differences because the
values of both the Z and V registers will still be the same. But the
logic is not correct.
I used the opportunity to fix this, and it gets tested later on by
the additional SME tests.
I do plan on submitting some SVE-specific tests to make sure we have
a bit more coverage in GDB's testsuite.
Regression-tested on aarch64-linux Ubuntu 22.04/20.04.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
|
|
In preparation to the SME support patches, rename the SVE-specific files to
something a bit more meaningful that can be shared with the SME code.
In this case, I've renamed the "sve" in the names to "scalable".
No functional changes.
Regression-tested on aarch64-linux Ubuntu 22.04/20.04.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
|
|
Since commit b42405a1594 ("gdb: Update x86 Linux architectures to
support XSAVE layouts."), the test gdb.base/gcore.exp fails on my AMD
Ryzen 3700X machine:
FAIL: gdb.base/gcore.exp: corefile restored all registers
The test gets the register state (saves the output of "info
all-registers"), saves a core with the "gcore" command, loads the core,
and checks the register state against the one previously saved. The
problem is that when reading registers from the core file, the last half
of ymm registers is unavailable:
(gdb) print $ymm0.v32_int8
$1 = {0, -77, -23, -9, -1, 127, 0, 0, 0, -77, -23, -9, -1, 127, 0, 0, <unavailable> <repeats 16 times>}
One strange thing with this machine is that the bitset of state
components supported by XCR0 is 0x207, meaning "x87 | SSE | AVX | PKRU",
but XCR0 at runtime is 0x7, meaning "x87 | SSE | AVX". So, PKRU appears
to be supported by the processor, but disabled by the kernel. I didn't
find why yet.
From CPUID leaf EAX=0Dh, ECX=00h, GDB can get:
- from EBX: max size of the XSAVE area required by features currently
enabled in XCR0. On my machine, it's 0x340 (832).
- from ECX: max size of the XSAVE area required by all features
supported by XCR0. On my machine, it's 0x380 (896).
At runtime, GDB uses ECX (max size required by all supported features)
to fill the x86_xsave_layout::sizeof_xsave. So, when writing the core
file note for the XSAVE state, it writes a note of size 896, even though
it doesn't write the PKRU state. When loading back the core, GDB tries
to figure out the layout of the XSAVE area based on what features are
enabled in XCR0 and the size of the note (the size of the XSAVE area).
Since my combination of XCR0 and size of XSAVE area doesn't match any
combination known by GDB, GDB falls back to a gdbarch supporting only
x87 and SSE.
This patch changes GDB to populate the x86_xsave_layout::sizeof_xsave
field (and consequently the size of the XSAVE state note in core files)
using EBX, the size of the XSAVE area required by currently enabled
features in XCR0. This makes i387_guess_xsave_layout recognize my case
with this condition:
else if (HAS_AVX (xcr0) && xsave_size == 832)
{
/* Intel and AMD CPUs supporting AVX. */
layout.avx_offset = 576;
}
In other words, just as if my machine didn't support PKRU at all.
Another reason why I think this change makes sense is that XSAVE state
notes in kernel-generated cores on this machine have size 832. So this
change makes GDB-generated cores more similar to kernel-generated ones,
reducing the diversity of XSAVE state notes that GDB needs to be able to
figure out.
Note that if PKRU was enabled on my machine, then the effective XSAVE
area size would be 896 bytes. We would need to add a case in
i387_guess_xsave_layout for that combination, since there is no
currently. But I don't have a way to test that right now, since I don't
know why PKRU is disabled.
Relevant review note from John Baldwin:
One further note is that the Linux x86 arches use x86_xsave_length()
to infer ("guess") the size of the XSAVE register set that the Linux
kernel writes out in core dumps. On FreeBSD x86 arches, GDB is able
to query this size directly from the kernel via ptrace. My use of ECX
for this guess earlier was just not the best guess. In the case that
the kernel enables all of the available features, then ECX and EBX
have the same values, so this only matters for a system where the
kernel has enabled a subset of available XSAVE extensions.
Change-Id: If64f30307f3a2e5ca3e1fd1cb7379ea840805a85
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
|
|
I noticed a comment by an include and remembered that I think these
don't really provide much value -- sometimes they are just editorial,
and sometimes they are obsolete. I think it's better to just remove
them. Tested by rebuilding.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
