| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
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>
|
|
This patch proposes to require a C++17 compiler to build gdb /
gdbsupport / gdbserver. Before this patch, GDB required a C++11
compiler.
The general policy regarding bumping C++ language requirement in GDB (as
stated in [1]) is:
Our general policy is to wait until the oldest compiler that
supports C++NN is at least 3 years old.
Rationale: We want to ensure reasonably widespread compiler
availability, to lower barrier of entry to GDB contributions, and to
make it easy for users to easily build new GDB on currently
supported stable distributions themselves. 3 years should be
sufficient for latest stable releases of distributions to include a
compiler for the standard, and/or for new compilers to appear as
easily installable optional packages. Requiring everyone to build a
compiler first before building GDB, which would happen if we
required a too-new compiler, would cause too much inconvenience.
See the policy proposal and discussion
[here](https://sourceware.org/ml/gdb-patches/2016-10/msg00616.html).
The first GCC release which with full C++17 support is GCC-9[2],
released in 2019[3], which is over 4 years ago. Clang has had C++17
support since Clang-5[4] released in 2018[5].
A discussions with many distros showed that a C++17-able compiler is
always available, meaning that this no hard requirement preventing us to
require it going forward.
[1] https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#When_is_GDB_going_to_start_requiring_C.2B-.2B-NN_.3F
[2] https://gcc.gnu.org/projects/cxx-status.html#cxx17
[3] https://gcc.gnu.org/gcc-9/
[4] https://clang.llvm.org/cxx_status.html
[5] https://releases.llvm.org/
Change-Id: Id596f5db17ea346e8a978668825787b3a9a443fd
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Pedro Alves <pedro@palves.net>
|
|
This patch upgrades gdb/ax_cxx_compile_stdcxx.m4 to follow changes
available in [1] and regenerates the configure script.
[1] https://www.gnu.org/software/autoconf-archive/ax_cxx_compile_stdcxx.html
Change-Id: I5b16adc65c9e48a13ad65202d58ab7a9d487214e
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Pedro Alves <pedro@palves.net>
|
|
Overview
========
Consider the following situation, GDB is in non-stop mode, the main
thread is running while a second thread is stopped. The user has the
second thread selected as the current thread and asks GDB to detach.
At the exact moment of detach the main thread exits.
This situation currently causes crashes, assertion failures, and
unexpected errors to be reported from GDB for both native and remote
targets.
This commit addresses this situation for native and remote targets.
There are a number of different fixes, but all are required in order
to get this functionality working correct for native and remote
targets.
Native Linux Target
===================
For the native Linux target, detaching is handled in the function
linux_nat_target::detach. In here we call stop_wait_callback for each
thread, and it is this callback that will spot that the main thread
has exited.
GDB then detaches from everything except the main thread by calling
detach_callback.
After this the first problem is this assert:
/* Only the initial process should be left right now. */
gdb_assert (num_lwps (pid) == 1);
The num_lwps call will return 0 as the main thread has exited and all
of the other threads have now been detached. I fix this by changing
the assert to allow for 0 or 1 lwps at this point. As the 0 case can
only happen in non-stop mode, the assert becomes:
gdb_assert (num_lwps (pid) == 1
|| (target_is_non_stop_p () && num_lwps (pid) == 0));
The next problem is that we do:
main_lwp = find_lwp_pid (ptid_t (pid));
and then proceed assuming that main_lwp is not nullptr. In the case
that the main thread has exited though, main_lwp will be nullptr.
However, we only need main_lwp so that GDB can detach from the
thread. If the main thread has exited, and GDB has already detached
from every other thread, then GDB has finished detaching, GDB can skip
the calls that try to detach from the main thread, and then tell the
user that the detach was a success.
For Remote Targets
==================
On remote targets there are two problems.
First is that when the exit occurs during the early phase of the
detach, we see the stop notification arrive while GDB is removing the
breakpoints ahead of the detach. The 'set debug remote on' trace
looks like this:
[remote] Sending packet: $z0,7f1648fe0241,1#35
[remote] Notification received: Stop:W0;process:2a0ac8
# At this point an unpatched gdbserver segfaults, and the connection
# is broken. A patched gdbserver continues as below...
[remote] Packet received: E01
[remote] Sending packet: $z0,7f1648ff00a8,1#68
[remote] Packet received: E01
[remote] Sending packet: $z0,7f1648ff132f,1#6b
[remote] Packet received: E01
[remote] Sending packet: $D;2a0ac8#3e
[remote] Packet received: E01
I was originally running into Segmentation Faults, from within
gdbserver/mem-break.cc, in the function find_gdb_breakpoint. This
function calls current_process() and then dereferences the result to
find the breakpoint list.
However, in our case, the current process has already exited, and so
the current_process() call returns nullptr. At the point of failure,
the gdbserver backtrace looks like this:
#0 0x00000000004190e4 in find_gdb_breakpoint (z_type=48 '0', addr=4198762, kind=1) at ../../src/gdbserver/mem-break.cc:982
#1 0x000000000041930d in delete_gdb_breakpoint (z_type=48 '0', addr=4198762, kind=1) at ../../src/gdbserver/mem-break.cc:1093
#2 0x000000000042d8db in process_serial_event () at ../../src/gdbserver/server.cc:4372
#3 0x000000000042dcab in handle_serial_event (err=0, client_data=0x0) at ../../src/gdbserver/server.cc:4498
...
The problem is that, as a result non-stop being on, the process
exiting is only reported back to GDB after the request to remove a
breakpoint has been sent. Clearly gdbserver can't actually remove
this breakpoint -- the process has already exited -- so I think the
best solution is for gdbserver just to report an error, which is what
I've done.
The second problem I ran into was on the gdb side, as the process has
already exited, but GDB has not yet acknowledged the exit event, the
detach -- the 'D' packet in the above trace -- fails. This was being
reported to the user with a 'Can't detach process' error. As the test
actually calls detach from Python code, this error was then becoming a
Python exception.
Though clearly the detach has returned an error, and so, maybe, having
GDB throw an error would be fine, I think in this case, there's a good
argument that the remote error can be ignored -- if GDB tries to
detach and gets back an error, and if there's a pending exit event for
the pid we tried to detach, then just ignore the error and pretend the
detach worked fine.
We could possibly check for a pending exit event before sending the
detach packet, however, I believe that it might be possible (in
non-stop mode) for the stop notification to arrive after the detach is
sent, but before gdbserver has started processing the detach. In this
case we would still need to check for pending stop events after seeing
the detach fail, so I figure there's no point having two checks -- we
just send the detach request, and if it fails, check to see if the
process has already exited.
Testing
=======
In order to test this issue I needed to ensure that the exit event
arrives at the same time as the detach call. The window of
opportunity for getting the exit to arrive is so small I've never
managed to trigger this in real use -- I originally spotted this issue
while working on another patch, which did manage to trigger this
issue.
However, if we trigger both the exit and the detach from a single
Python function then we never return to GDB's event loop, as such GDB
never processes the exit event, and so the first time GDB gets a
chance to see the exit is during the detach call. And so that is the
approach I've taken for testing this patch.
Tested-By: Kevin Buettner <kevinb@redhat.com>
Approved-By: Kevin Buettner <kevinb@redhat.com>
|
|
I noticed that in handle_v_run (gdbserver/server.cc) we leak
new_program_name (a string) each time GDB starts an inferior, in the
case where GDB passes a program name to gdbserver.
This bug was introduced with this commit:
commit 7ab2607f97e5deaeae91018edf3ef5b4255a842c
Date: Wed Apr 13 17:31:02 2022 -0400
gdbsupport: make gdb_abspath return an std::string
When gdbserver receives a program name from GDB, this is first placed
into a malloc'd buffer within handle_v_run, and this buffer is then
used in this call:
program_path.set (new_program_name);
Prior to the above commit this call took ownership of the buffer
passed to it, but now this call uses the buffer to initialise a
std::string, which copies the buffer contents, leaving ownership with
the caller. So now, after this call (in handle_v_run)
new_program_name still owns a buffer.
At no point in handle_v_run do we free new_program_name, as a result
we are leaking the program name each time GDB starts a remote
inferior.
I could solve this by adding a 'free' call into handle_v_run, but I'd
rather automate the memory management.
So, to this end, I have added a new function in gdbserver/server.cc,
decode_v_run_arg. This function takes care of allocating the memory
buffer and decoding the vRun packet into the buffer, but returns a
gdb::unique_xmalloc_ptr<char> (or nullptr on error).
Back in handle_v_run I have converted new_program_name to also be a
gdb::unique_xmalloc_ptr<char>.
Now, after we call program_path.set(), the allocated buffer will be
automatically released when it is no longer needed.
It is worth highlighting that within the new decode_v_run_arg
function, I have wrapped the call to hex2bin in a try/catch block.
The hex2bin function can throw an exception if it encounters an
invalid (non-hex) character. Back in handle_v_run, we have a local
argument new_argv, which is of type std::vector<char *>. Each
'char *' in this vector is a malloc'd buffer. If we allow
hex2bin to throw an exception and don't catch it in either
decode_v_run_arg or handle_v_run then we are going to leak memory from
new_argv.
I chose to catch the exception in decode_v_run_arg, this seemed
cleanest, but I'm not sure it really matters, so long as the exception
is caught before we leave handle_v_run. I am working on a patch that
changes new_argv to automatically manage its memory, but that isn't
ready for posting yet. I think what I have here would be fine if my
follow on patch never arrives.
Additionally, within the handle_v_run loop I have changed an
assignment of nullptr to new_program_name into an assert. Previously,
the assignment could only trigger on the first iteration of the loop,
if we had no new program name to assign. However, new_program_name
always starts as nullptr, so, on the first loop iteration, if we have
nothing to assign to new_program_name, its value must already be
nullptr.
There should be no user visible changes after this commit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
A user pointed out that the -lsocket check in gdb should also apply to
gdbserver -- otherwise it can't find the Solaris socketpair. This
patch makes the change. It also removes a couple of redundant
function checks from gdb's configure.ac.
This was tested by the person who reported the bug.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30927
Approved-By: Pedro Alves <pedro@palves.net>
|
|
After this commit:
commit 6a65998a8a94abaaae7ca4ff0ab9c3f25dc2e766
Date: Mon Sep 11 12:42:00 2023 +0100
Convert tdesc's expedite_regs to a string vector
The risc-v, loongarch, and csky gdbserver builds were broken. A use
of target_desc::expedite_regs (for each architecture) was not updated
to take account of the type change.
I've tested that this fixes the risc-v build. I haven't tested the
other architectures, but they should be fine.
|
|
After the previous commit there is now a redundant string copy in
handle_v_run, this commit cleans that up.
There should be no functional change after this commit.
During review I was pointed to this older series:
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/
which also includes this fix as part of a larger set of changes. I'm
giving a Co-Authored-By credit to the author of that original series.
I believe this smaller fix brings some benefits on its own, though the
original series does offer additional improvements. Once this is
merged I'll take a look at rebasing and resubmitting the original series.
Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Similarly to how single quotes were mishandled, which was fixed two
commits ago, this commit fixes handling of newlines in arguments
passed to gdbserver.
We already had a test that covered this, gdb.base/args.exp, which,
when run with the native-extended-gdbserver board contained several
KFAIL covering this situation.
In this commit I remove the unnecessary, attempt to quote incoming
newlines within arguments, and do some minimal cleanup of the related
code. There is additional cleanup that can be done, but I'm leaving
that for the next commit.
Then I've removed the KFAIL from the test case, and performed some
minimal cleanup there too.
After this commit the gdb.base/args.exp is 100% passing with the
native-extended-gdbserver board file.
During review I was pointed to this older series:
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/
which also includes this fix as part of a larger set of changes. I'm
giving a Co-Authored-By credit to the author of that original series.
I believe this smaller fix brings some benefits on its own, though the
original series does offer additional improvements. Once this is
merged I'll take a look at rebasing and resubmitting the original series.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27989
Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
When I posted the previous patch for review Andreas Schwab pointed out
that passing a trailing empty argument also doesn't work.
The fix for this is in the same area of code as the previous patch,
but is sufficiently different that I felt it deserved a patch of its
own.
I noticed that passing arguments containing single quotes to gdbserver
didn't work correctly:
gdb -ex 'set sysroot' --args /tmp/show-args
Reading symbols from /tmp/show-args...
(gdb) target extended-remote | gdbserver --once --multi - /tmp/show-args
Remote debugging using | gdbserver --once --multi - /tmp/show-args
stdin/stdout redirected
Process /tmp/show-args created; pid = 176054
Remote debugging using stdio
Reading symbols from /lib64/ld-linux-x86-64.so.2...
(No debugging symbols found in /lib64/ld-linux-x86-64.so.2)
0x00007ffff7fd3110 in _start () from /lib64/ld-linux-x86-64.so.2
(gdb) set args abc ""
(gdb) run
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /tmp/show-args \'
stdin/stdout redirected
Process /tmp/show-args created; pid = 176088
2 args are:
/tmp/show-args
abc
Done.
[Inferior 1 (process 176088) exited normally]
(gdb) target native
Done. Use the "run" command to start a process.
(gdb) run
Starting program: /tmp/show-args \'
2 args are:
/tmp/show-args
abc
Done.
[Inferior 1 (process 176095) exited normally]
(gdb) q
The 'shows-args' program used here just prints the arguments passed to
the inferior.
Notice that when starting the inferior using the extended-remote
target there is only a single argument 'abc', while when using the
native target there is a second argument, the blank line, representing
the empty argument.
The problem here is that the vRun packet coming from GDB looks like
this (I've removing the trailing checksum):
$vRun;PROGRAM_NAME;616263;
If we compare this to a packet with only a single argument and no
trailing empty argument:
$vRun;PROGRAM_NAME;616263
Notice the lack of the trailing ';' character here.
The problem is that gdbserver processes this string in a loop. At
each point we maintain a pointer to the character just after a ';',
and then we process everything up to either the next ';' character, or
to the end of the string.
We break out of this loop when the character we start with (in that
loop iteration) is the null-character. This means in the trailing
empty argument case, we abort the loop before doing anything with the
empty argument.
In this commit I've updated the loop, we now break out using a 'break'
statement at the end of the loop if the (sub-)string we just processed
was empty, with this change we now notice the trailing empty
argument.
I've updated the test case to cover this issue.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I noticed that passing arguments containing single quotes to gdbserver
didn't work correctly:
gdb -ex 'set sysroot' --args /tmp/show-args
Reading symbols from /tmp/show-args...
(gdb) target extended-remote | gdbserver --once --multi - /tmp/show-args
Remote debugging using | gdbserver --once --multi - /tmp/show-args
stdin/stdout redirected
Process /tmp/show-args created; pid = 176054
Remote debugging using stdio
Reading symbols from /lib64/ld-linux-x86-64.so.2...
(No debugging symbols found in /lib64/ld-linux-x86-64.so.2)
0x00007ffff7fd3110 in _start () from /lib64/ld-linux-x86-64.so.2
(gdb) set args \'
(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /tmp/show-args \'
stdin/stdout redirected
Process /tmp/show-args created; pid = 176088
2 args are:
/tmp/show-args
\'
Done.
[Inferior 1 (process 176088) exited normally]
(gdb) target native
Done. Use the "run" command to start a process.
(gdb) run
Starting program: /tmp/show-args \'
2 args are:
/tmp/show-args
'
Done.
[Inferior 1 (process 176095) exited normally]
(gdb) q
The 'shows-args' program used here just prints the arguments passed to
the inferior.
Notice that when starting the inferior using the extended-remote
target the second argument is "\'", while when running using native
target the argument is "'". The second of these is correct, the \'
used with the "set args" command is just to show GDB that the single
quote is not opening an argument string.
It turns out that the extra backslash is injected on the gdbserver
side when gdbserver processes the arguments that GDB passes it, the
code that does this was added as part of this much larger commit:
commit 2090129c36c7e582943b7d300968d19b46160d84
Date: Thu Dec 22 21:11:11 2016 -0500
Share fork_inferior et al with gdbserver
In this commit I propose removing the specific code that adds what I
believe is a stray backslash. I've extended an existing test to cover
this case, and I now see identical behaviour when using an
extended-remote target as with the native target.
This partially fixes PR gdb/27989, though there are still some issues
with newline handling which I'll address in a later commit.
During review I was pointed to this older series:
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/
which also includes this fix as part of a larger set of changes. I'm
giving a Co-Authored-By credit to the author of that original series.
I believe this smaller fix brings some benefits on its own, though the
original series does offer additional improvements. Once this is
merged I'll take a look at rebasing and resubmitting the original series.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27989
Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This patch teaches gdbserver about the SME2 and the ZT0 register.
Since most of the code used by gdbserver for SME2 is shared with gdb, this
is a rather small patch that reuses most of the code put in place for native
AArch64 Linux.
Validated under Fast Models.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
|
|
Enable SME support in gdbserver by adjusting the usual fields. There is
not much to this patch because the code is either in gdb or it is shared
between gdbserver and gdb. One exception is the bump to gdbserver's
PBUFSIZ from 18432 to 131104.
Since the ZA register can be quite big (256 * 256 bytes), the g/G remote
packet will also become quite big
From gdbserver/tdesc.cc:init_target_desc, I estimated the new size should
be at least (2 * 256 * 256 + 32), which yields 131104.
It is also unlikely we will find a process starting up with SVL set to 256.
Ideally we'd adjust the packet size dynamically based on what we need, but
for now this should do.
Please note we have the same limitation for SME that we have for SVE, and
that is the fact gdbserver cannot communicate vector length changes to gdb
via the remote protocol.
Thiago is working on this improvement, which hopefully will be able to be
adapted to SME in an easy way.
Co-Authored-By: Ezra Sitorus <ezra.sitorus@arm.com>
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
|
|
Instead of using static arrays, build the list of expedited registers
dynamically using a std::vector.
This refactor shouldn't cause any user-visible changes.
Regression-tested for aarch64-linux Ubuntu 22.04/20.04.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
|
|
Right now the list of expedited registers is stored as an array of char *,
with a nullptr element at the end to signal its last element.
Convert expedite_regs to a std::vector of std::string so it is easier to
manage the elements and the storage is handled automatically.
Eventually we might want to convert all the target functions so they pass a
std::vector of std::string as well. Or maybe expose an interface that target can
use to add expedited registers on-by-one depending on the target description
discovery needs, as opposed to just a static list of char *.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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>
|
|
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>
|
|
On a machine with AVX512 support (AMD EPYC 9634), I see these failures:
$ make check TESTS="gdb.arch/i386-avx512.exp" RUNTESTFLAGS="--target_board=native-gdbserver"
...
FAIL: gdb.arch/i386-avx512.exp: check contents of zmm_data[16] after writing ZMM regs
FAIL: gdb.arch/i386-avx512.exp: check contents of zmm_data[17] after writing ZMM regs
FAIL: gdb.arch/i386-avx512.exp: check contents of zmm_data[18] after writing ZMM regs
...
The problem can be reduced to:
(gdb) print $zmm16.v8_int64
$1 = {0, 0, 0, 0, 0, 0, 0, 0}
(gdb) print $zmm16.v8_int64 = {11,22,33,44,55,66,77,88}
$2 = {11, 22, 33, 44, 55, 66, 77, 88}
(gdb) print $zmm16.v8_int64
$3 = {11, 22, 33, 44, 55, 66, 77, 88}
(gdb) step
5 ++x;
(gdb) print $zmm16.v8_int64
$4 = {11, 22, 77, 88, 0, 0, 0, 0}
Writing to the local regcache in GDB works fine, but the writeback to
gdbserver (which happens when resuming / stepping) doesn't work (the
code being stepped doesn't touch AVX registers, so we don't expect the
value of zmm16 to change when stepping).
The problem is on the gdbserver side, the zmmh and ymmh portions of the
zmm register are not memcpied at the right place in the xsave buffer. Fix
that. Note now how the two modified memcpy calls match the memcmp calls
just above them.
With this patch, gdb.arch/i386-avx512.exp passes completely for me.
Change-Id: I22c417e0f5e88d4bc635a0f08f8817a031c76433
Reviewed-by: John Baldwin <jhb@FreeBSD.org>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30818
|
|
This safeguards a couple of places that may theoretically return NULL but
must not in this specific context. These were found by a static analysis tool.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
|
|
- Reuse num_xmm_registers directly for the count of ZMM0-15 registers
as is already done for the YMM registers for AVX rather than using
a new variable that is always the same.
- Replace 3 identical variables for the count of upper ZMM16-31
registers with a single variable. Make use of this to merge
various loops working on the ZMM XSAVE region so that all of the
handling for the various sub-registers in this region are always
handled in a single loop.
- While here, fix some bugs in i387_cache_to_xsave where if
X86_XSTATE_ZMM was set on i386 (e.g. a 32-bit process on a 64-bit
kernel), the -1 register nums would wrap around and store the value
of GPRs in the XSAVE area. This should be harmless, but is
definitely odd. Instead, check num_zmm_high_registers directly when
checking X86_XSTATE_ZMM and skip the ZMM region handling entirely if
the register count is 0.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Replace the extended state area fields of i387_xsave with methods which
return an offset into the XSAVE buffer.
The two changed functions are called within all tests which runs
gdbserver.
Signed-off-by: Aleksandar Paunovic <aleksandar.paunovic@intel.com>
Co-authored-by: John Baldwin <jhb@FreeBSD.org>
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Legacy fields of the XSAVE area are already defined within fx_save
struct. Use class inheritance to remove code duplication.
The two changed functions are called within all tests which run
gdbserver.
Signed-off-by: Aleksandar Paunovic <aleksandar.paunovic@intel.com>
Co-authored-by: John Baldwin <jhb@FreeBSD.org>
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Make x86_xcr0 private to i387-fp.cc and use i387_set_xsave_mask to set
the value instead. Add a static global instance of x86_xsave_layout
and initialize it in the new function as well to be used in a future
commit to parse XSAVE extended state regions.
Update the Linux port to use this function rather than setting
x86_xcr0 directly. In the case that XML is not supported, don't
bother setting x86_xcr0 to the default value but just omit the call to
i387_set_xsave_mask as i387-fp.cc defaults to the SSE case used for
non-XML.
In addition, use x86_xsave_length to determine the size of the XSAVE
register set via CPUID.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
resume_stopped_resumed_lwps
At the moment, while performing a software single-step, gdbserver fails
to reinsert software single-step breakpoints for a LWP when
interrupted by a signal in another thread. This commit fixes this
problem by reinstalling software single-step breakpoints in
linux_process_target::resume_stopped_resumed_lwps in
gdbserver/linux-low.cc.
This bug was discovered due to a failing assert in maybe_hw_step()
in gdbserver/linux-low.cc. Looking at the backtrace revealed
that the caller was linux_process_target::resume_stopped_resumed_lwps.
I was uncertain whether the assert should still be valid when called
from that method, so I tried hoisting the assert from maybe_hw_step
to all callers except resume_stopped_resumed_lwps. But running the
new test case, described below, showed that merely eliminating the
assert for this case was NOT a good fix - a study of the log file for
the test showed that the single-step operation failed to occur.
Instead GDB (via gdbserver) stopped at the next breakpoint that was
hit.
Zhiyong Yan had proposed a fix which resinserted software single-step
breakpoints, albeit at a different location in linux-low.cc. Testing
revealed that, while running gdb.threads/pending-fork-event-detach,
the executable associated with that test would die due to a SIGTRAP
after the test program was detached. Examination of the core file(s)
showed that a breakpoint instruction had been left in program memory.
Test results were otherwise very good, so Zhiyong was definitely on
the right track!
This commit causes software single-step breakpoint(s) to be inserted
before the call to maybe_hw_step in resume_stopped_resumed_lwps. This
will cause 'has_single_step_breakpoints (thread)' to be true, so that
the assert in maybe_hw_step...
/* GDBserver must insert single-step breakpoint for software
single step. */
gdb_assert (has_single_step_breakpoints (thread));
...will no longer fail. And better still, the single-step breakpoints
are reinstalled, so that stepping will actually work, even when
interrupted.
The C code for the test case was loosely adapted from the reproducer
provided in Zhiyong's bug report for this problem. The .exp file was
copied from next-fork-other-thread.exp and then tweaked slightly. As
noted in a comment in next-fork-exec-other-thread.exp, I had to remove
"on" from the loop for non-stop as it was failing on all architectures
(including x86-64) that I tested. I have a feeling that it ought to
work, but this can be investigated separately and (re)enabled once it
works. I also increased the number of iterations for the loop running
the "next" commands. I've had some test runs which don't show the bug
until the loop counter exceeded 100 iterations. The C file for the
new test uses shorter delays than next-fork-other-thread.c though, so
it doesn't take overly long (IMO) to run this new test.
Running the new test on a Raspberry Pi w/ a 32-bit (Arm) kernel and
userland using a gdbserver build without the fix in this commit shows
the following results:
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=fork: target-non-stop=auto: non-stop=off: displaced-stepping=auto: i=12: next to other line
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=fork: target-non-stop=auto: non-stop=off: displaced-stepping=on: i=9: next to other line
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=fork: target-non-stop=auto: non-stop=off: displaced-stepping=off: i=18: next to other line
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=fork: target-non-stop=off: non-stop=off: displaced-stepping=auto: i=3: next to other line
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=fork: target-non-stop=off: non-stop=off: displaced-stepping=on: i=11: next to other line
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=fork: target-non-stop=off: non-stop=off: displaced-stepping=off: i=1: next to other line
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=vfork: target-non-stop=auto: non-stop=off: displaced-stepping=auto: i=1: next to break here
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=vfork: target-non-stop=auto: non-stop=off: displaced-stepping=on: i=3: next to break here
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=vfork: target-non-stop=auto: non-stop=off: displaced-stepping=off: i=1: next to break here
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=vfork: target-non-stop=on: non-stop=off: displaced-stepping=auto: i=47: next to other line
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=vfork: target-non-stop=on: non-stop=off: displaced-stepping=on: i=57: next to other line
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=vfork: target-non-stop=off: non-stop=off: displaced-stepping=auto: i=1: next to break here
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=vfork: target-non-stop=off: non-stop=off: displaced-stepping=on: i=10: next to break here
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=vfork: target-non-stop=off: non-stop=off: displaced-stepping=off: i=1: next to break here
=== gdb Summary ===
# of unexpected core files 12
# of expected passes 3011
# of unexpected failures 14
Each of the 12 core files were caused by the failed assertion in
maybe_hw_step in linux-low.c. These correspond to 12 of the
unexpected failures.
When the tests are run using a gdbserver build which includes the fix
in this commit, the results are significantly better, but not perfect:
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=vfork: target-non-stop=on: non-stop=off: displaced-stepping=auto: i=143: next to other line
FAIL: gdb.threads/next-fork-exec-other-thread.exp: fork_func=vfork: target-non-stop=on: non-stop=off: displaced-stepping=on: i=25: next to other line
=== gdb Summary ===
# of expected passes 10178
# of unexpected failures 2
I think that the two remaining failures are due to some different
problem. They are also racy - I've seen runs with no failures or only
one failure, but never more than two. Also, those runs were conducted
with the loop count in next-fork-exec-other-thread.exp set to 200.
During his testing of this fix and the new test case, Luis Machado
found that this test was taking a long time and asked about ways to
speed it up. I then conducted additional tests in which I gradually
reduced the loop count, timing each one, also noting the number of
failures. With the loop count set to 30, I found that I could still
reliably reproduce the failures that Zhiyong reported (in which, with
the proper settings, core files are created). But, with the loop
count set to 30, the other failures noted above were much less likely
to show up. Anyone wishing to investigate those other failures should
set the loop count back up to 200.
Running the new test on x86-64 and aarch64, both native and
native-gdbserver shows no failures.
Also, I see no regressions when running the entire test suite for
armv7l-unknown-linux-gnueabihf (i.e. the Raspberry Pi w/ 32-bit
kernel+userland) with --target_board=native-gdbserver. Additionally,
using --target_board=native-gdbserver, I also see no regressions for
the entire test suite for x86-64 and aarch64 running Fedora 38.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30387
Co-Authored-By: Zhiyong Yan <zhiyong.yan@windriver.com>
Tested-By: Zhiyong Yan <zhiyong.yan@windriver.com>
Tested-By: Luis Machado <luis.machado@arm.com>
|
|
It was pointed out[1] that after this commit:
commit 3812b38d8de5804ad3eadd6c7a5d532402ddabab
Date: Thu Oct 20 11:14:33 2022 +0100
gdbserver: allow agent expressions to fail with invalid memory access
Now that agent expressions might fail with the error
expr_eval_invalid_memory_access, we might overflow the
eval_result_names array in tracepoint.cc. This is because the
eval_result_names array does not include a string for either
expr_eval_invalid_goto or expr_eval_invalid_memory_access.
I don't know if having expr_eval_invalid_goto missing is also a
problem, but it feels like eval_result_names should just include a
string for every possible error.
I could just add two more strings into the array, but I figure that a
more robust solution will be to move all of the error types, and their
associated strings, into a new ax-result-types.def file, and to then
include this file in both ax.h and tracepoint.cc in order to build
the enum eval_result_type and the eval_result_names string array.
Doing this means it is impossible to have a missing error string in
the future.
[1] https://inbox.sourceware.org/gdb-patches/01059f8a-0e59-55b5-f530-190c26df5ba3@palves.net/
Approved-By: Pedro Alves <pedro@palves.net>
|
|
Since commit 05c06f318fd9 ("Linux: Access memory even if threads are
running"), GDB prefers pread64/pwrite64 to access inferior memory
instead of ptrace. That change broke reading shared libraries on
SPARC64 Linux, as reported by PR gdb/30525 ("gdb cannot read shared
libraries on SPARC64").
On SPARC64 Linux, surprisingly (to me), userspace shared libraries are
mapped at high 64-bit addresses:
(gdb) info sharedlibrary
Cannot access memory at address 0xfff80001002011e0
Cannot access memory at address 0xfff80001002011d8
Cannot access memory at address 0xfff80001002011d8
From To Syms Read Shared Object Library
0xfff80001000010a0 0xfff8000100021f80 Yes (*) /lib64/ld-linux.so.2
(*): Shared library is missing debugging information.
Those addresses are 64-bit addresses with the high bits set. When
interpreted as signed, they're negative.
The Linux kernel rejects pread64/pwrite64 if the offset argument of
type off_t (a signed type) is negative, which happens if the memory
address we're accessing has its high bit set. See
linux/fs/read_write.c sys_pread64 and sys_pwrite64 in Linux.
Thankfully, lseek does not fail in that situation. So the fix is to
use the 'lseek + read|write' path if the offset would be negative.
Fix this in both native GDB and GDBserver.
Tested on a SPARC64 GNU/Linux and x86-64 GNU/Linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30525
Change-Id: I79c724f918037ea67b7396fadb521bc9d1b10dc5
|
|
Use the to_string method of target_waitstatus in
'prepare_resume_reply' for a more readable log message.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Fix some more typos:
- distinquish -> distinguish
- actualy -> actually
- singe -> single
- frash -> frame
- chid -> child
- dissassembler -> disassembler
- uninitalized -> uninitialized
- precontidion -> precondition
- regsiters -> registers
- marge -> merge
- sate -> state
- garanteed -> guaranteed
- explictly -> explicitly
- prefices (nonstandard plural) -> prefixes
- bondary -> boundary
- formated -> formatted
- ithe -> the
- arrav -> array
- coresponding -> corresponding
- owend -> owned
- fials -> fails
- diasm -> disasm
- ture -> true
- tpye -> type
There's one code change, the name of macro SIG_CODE_BONDARY_FAULT changed to
SIG_CODE_BOUNDARY_FAULT.
Tested on x86_64-linux.
|
|
Fix a few typos:
- implemention -> implementation
- convertion(s) -> conversion(s)
- backlashes -> backslashes
- signoring -> ignoring
- (un)ambigious -> (un)ambiguous
- occured -> occurred
- hidding -> hiding
- temporarilly -> temporarily
- immediatelly -> immediately
- sillyness -> silliness
- similiar -> similar
- porkuser -> pokeuser
- thats -> that
- alway -> always
- supercede -> supersede
- accomodate -> accommodate
- aquire -> acquire
- priveleged -> privileged
- priviliged -> privileged
- priviledges -> privileges
- privilige -> privilege
- recieve -> receive
- (p)refered -> (p)referred
- succesfully -> successfully
- successfuly -> successfully
- responsability -> responsibility
- wether -> whether
- wich -> which
- disasbleable -> disableable
- descriminant -> discriminant
- construcstor -> constructor
- underlaying -> underlying
- underyling -> underlying
- structureal -> structural
- appearences -> appearances
- terciarily -> tertiarily
- resgisters -> registers
- reacheable -> reachable
- likelyhood -> likelihood
- intepreter -> interpreter
- disassemly -> disassembly
- covnersion -> conversion
- conviently -> conveniently
- atttribute -> attribute
- struction -> struct
- resonable -> reasonable
- popupated -> populated
- namespaxe -> namespace
- intialize -> initialize
- identifer(s) -> identifier(s)
- expection -> exception
- exectuted -> executed
- dungerous -> dangerous
- dissapear -> disappear
- completly -> completely
- (inter)changable -> (inter)changeable
- beakpoint -> breakpoint
- automativ -> automatic
- alocating -> allocating
- agressive -> aggressive
- writting -> writing
- reguires -> requires
- registed -> registered
- recuding -> reducing
- opeartor -> operator
- ommitted -> omitted
- modifing -> modifying
- intances -> instances
- imbedded -> embedded
- gdbaarch -> gdbarch
- exection -> execution
- direcive -> directive
- demanged -> demangled
- decidely -> decidedly
- argments -> arguments
- agrument -> argument
- amespace -> namespace
- targtet -> target
- supress(ed) -> suppress(ed)
- startum -> stratum
- squence -> sequence
- prompty -> prompt
- overlow -> overflow
- memember -> member
- languge -> language
- geneate -> generate
- funcion -> function
- exising -> existing
- dinking -> syncing
- destroh -> destroy
- clenaed -> cleaned
- changep -> changedp (name of variable)
- arround -> around
- aproach -> approach
- whould -> would
- symobl -> symbol
- recuse -> recurse
- outter -> outer
- freeds -> frees
- contex -> context
Tested on x86_64-linux.
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
I noticed in emit_ops_insns in gdbserver/linux-aarch64-low.cc:
...
threads_debug_printf ("Adding %d instrucions at %s",
...
Fix the typo by using "instructions" instead.
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
This was previously clearing the upper 32 bytes of ZMM0-15 rather than
ZMM16-31.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This commit extends gdbserver to take account of a failed memory
access from agent_mem_read, and to return a new eval_result_type
expr_eval_invalid_memory_access.
I have only updated the agent_mem_read calls related directly to
reading memory, I have not updated any of the calls related to
tracepoint data collection. This is just because I'm not familiar
with that area of gdb/gdbserver, and I don't want to break anything,
so leaving the existing behaviour untouched seems like the safest
approach.
I've then updated gdb.base/bp-cond-failure.exp to test evaluating the
breakpoints on the target, and have also extended the test so that it
checks for different sizes of memory access.
|
|
Currently the gdbserver function agent_mem_read ignores any errors
from calling read_inferior_memory. This means that if there is an
attempt to access invalid memory then this will appear to succeed.
In this patch I update agent_mem_read so that if read_inferior_memory
fails, agent_mem_read will return an error code.
However, none of the callers of agent_mem_read actually check the
return value, so this commit will have no effect on anything. In the
next commit I will update the users of agent_mem_read to check for the
error code.
I've also updated the header comments on agent_mem_read to better
reflect what the function does, and its possible return values.
|
|
Replace spaces with tabs in a bunch of places.
Change-Id: If0f87180f1d13028dc178e5a8af7882a067868b0
|
|
Tom de Vries pointed out that my earlier patch:
commit 873a185be258ad2552b9579005852815b4da5baf
Date: Fri Dec 16 07:56:57 2022 -0700
Don't use struct buffer in handle_qxfer_btrace
regressed gdb.btrace/reconnect.exp. I didn't notice this because I
did not have libipt installed.
This patch fixes the bug.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30169
Tested-By: Bruno Larsen <blarsen@redhat.com>
|
|
Signed-off-by: Khem Raj <raj.khem@gmail.com>
|
|
I've long wanted to remove 'struct buffer', and thanks to Simon's
earlier patch, I was finally able to do so. My feeling has been that
gdb already has several decent structures available for growing
strings: std::string of course, but also obstack and even objalloc
from BFD and dyn-string from libiberty. The previous patches in this
series removed all the uses of struct buffer, so this one can remove
the code and the remaining #includes.
|
|
This changes handle_qxfer_threads, in gdbserver, to use std::string
rather than struct buffer.
|
|
This changes handle_qxfer_btrace and handle_qxfer_btrace_conf, in
gdbserver, to use std::string rather than struct buffer.
|
|
This changes handle_qxfer_traceframe_info, in gdbserver, to use
std::string rather than struct buffer.
|
|
While investigating something else, I noticed some weird code in
agent_run_command (use of memcpy rather than strcpy). Then I noticed
that 'cmd' is used as both an in and out parameter, despite being
const.
Casting away const like this is bad. This patch removes the const and
fixes the memcpy. I also added a static assert to assure myself that
the code in gdbserver is correct -- gdbserver is passing its own
buffer directly to agent_run_command.
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
|
|
gdbsupport/errors.h declares perror_with_name and leaves the
implementation to the clients.
However gdb and gdbserver's implementations are essentially the
same, resulting in unnecessary code duplication.
Fix this by implementing perror_with_name in gdbsupport. Add an
optional parameter for specifying the errno used to generate the
error message.
Also move the implementation of perror_string to gdbsupport since
perror_with_name requires it.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This patch doesn't change gdbserver behaviour, but after later changes are
made it avoids a null pointer dereference when HWCAP needs to be obtained
for a specific process while current_thread is nullptr.
Fixing linux_read_auxv, linux_get_hwcap and linux_get_hwcap2 to take a PID
parameter seems more correct than setting current_thread in one particular
code path.
Changes are propagated to allow passing the new parameter through the call
chain.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
It helped me during development, catching bugs closer to when they actually
happened.
Also remove the equivalent gdb_assert in regcache_raw_read_unsigned, since
it's checking the same condition a few frames above.
Suggested-By: Simon Marchi <simon.marchi@efficios.com>
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
PR build/29003 points out that "make TAGS" is broken in gdbserver.
This patch fixes the problem that is pointed out there, plus another
one I noticed while working on that -- namely that the "sed" computes
the wrong names for some source files. Finally, a couple of obsolete
variable references are removed.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29003
|
|
Just adding some comments to the gdbserver read_inferior_memory
function. No actual code changes.
|
|
GCC recently added support for the Windows thread model, enabling
libstdc++ to support Windows natively. However, this supporrt
requires a version of Windows later than the minimum version that is
supported by GDB.
PR build/29966 points out that the GDB configure test for std::thread
does not work in this situation, because _WIN32_WINNT is not defined
in test program, and so <thread> seems to be fine.
This patch is an attempt to fix the problem, by using the same setting
for _WIN32_WINNT at configure time as is used at build time.
I don't have access to one of the older systems so I don't think I can
truly test this. I did do a mingw cross build, though. I'm going to
ask the bug reporter to test it.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29966
|
