| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Refactor amd64_analyze_prologue so it clearly reflects what is the order
of operations in the prologue that we expect to encounter, as is the
case for i386's implementation.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Update the make-check-all.sh script to use TESTS rather than passing
the test names within RUNTESTFLAGS. This addresses the following
issue:
I was running some tests like this:
make -C gdb check-all-boards TESTS="gdb.base/break*.exp"
And I was finding that I would get lots of DUPLICATE test results,
which is not what I expected.
What's happening here is that the 'make check-all-boards' rule runs
the 'make-check-all.sh' script, which then runs 'make check' with
various board files.
However, passing TESTS=... to the initial 'make check-all-boards'
command invocation automatically causes the TESTS value to be added to
the MAKEFLAGS environment variable, this is then picked up by the
later calls to 'make check'.
Now, in GDB's testfile/Makefile, we check for TESTS, and if this is
set, we expand the value and set `expanded_tests_or_none`. Otherwise,
if TESTS is not set, expanded_tests_or_none is left empty.
Finally, when handling 'make check', the value of
`expanded_tests_or_none` is passed through to dejagnu, along with the
RUNTESTFLAGS value.
What this means is that, when make-check-all.sh passes the test names
in the RUNTESTFLAGS, then dejagnu ends up seeing the list of tests
twice, once from RUNTESTFLAGS, and once from expanded_tests_or_none,
and this is why I was seeing duplicate testnames.
The easiest fix for the above is to have make-check-all.sh pass the
test names using TESTS="...", this will override the TESTS="..." value
already present in MAKEFLAGS, and means dejagnu will see the test
names just once.
Additionally, this is a start towards allowing parallel test running
from the make-check-all.sh script. Parallel test running only works
if the test names are passed in TESTS, and not in RUNTESTFLAGS.
Currently, in testsuite/Makefile, if RUNTESTFLAGS is not empty, then
we force single threaded test running. But with this change, at least
for the `local` board, we can now benefit from multi-threaded test
running, as this board has an empty RUNTESTFLAGS now. For the other
boards we'd need to set FORCE_PARALLEL in order to benefit from
parallel test running, but we'll need to double check that all the
board files actually support parallel test running first, so I'm
leaving that for another day.
|
|
This commit adds filename completion for the shell command part of
the pipe command. This is a follow on from this commit:
commit 036e5c0c9121d0ac691dbf408a3bdf2bf3501d0f
Date: Mon May 19 20:54:54 2025 +0100
gdb: use quoted filename completion for the shell command
which fixed the completion for the 'shell' command itself.
Like with the 'shell' command, we don't offer completions of command
names pulled from $PATH, we just offer filename completion, which is
often useful for arguments being passed to commands. Maybe in the
future we could add completion for command names too (for both 'pipe'
and the 'shell' command), but that is left for a future commit.
There's some additional testing.
|
|
The use of user namespaces is required for normal users to use mount
namespaces. Consider trying this as an unprivileged user:
$ unshare --mount /bin/true
unshare: unshare failed: Operation not permitted
The problem here is that an unprivileged user doesn't have the
required permissions to create a new mount namespace. If, instead, we
do this:
$ unshare --mount --map-root-user /bin/true
then this will succeed. The new option causes unshare to create a
user namespace in which the unprivileged user is mapped to UID/GID 0,
and so gains all privileges (inside the namespace), the user is then
able to create the mount namespace as required.
So, how does this relate to GDB?
When a user attaches to a process running in a separate mount
namespace, GDB makes use of a separate helper process (see
linux_mntns_get_helper in nat/linux-namespaces.c), which will then use
the `setns` function to enter (or try to enter) the mount namespace of
the process GDB is attaching too. The helper process will then handle
file I/O requests received from GDB, and return the results back to
GDB, this allows GDB to access files within the mount namespace.
The problem here is that, switching to a mount namespace requires that
a process hold CAP_SYS_CHROOT and CAP_SYS_ADMIN capabilities within
its user namespace (actually it's a little more complex, see 'man 2
setns'). Assuming GDB is running as an unprivileged user, then GDB
will not have the required permissions.
However, if GDB enters the user namespace that the `unshare` process
created, then the current user will be mapped to UID/GID 0, and will
have the required permissions.
And so, this patch extends linux_mntns_access_fs (in
nat/linux-namespace.c) to first try and switch to the user namespace
of the inferior before trying to switch to the mount namespace. If
the inferior does have a user namespace, and does have elevated
privileges within that namespace, then this first switch by GDB will
mean that the second step, into the mount namespace, will succeed.
If there is no user namespace, or the inferior doesn't have elevated
privileges within the user namespace, then the switch into the mount
namespace will fail, just as it currently does, and the user will need
to give elevated privileges to GDB via some other mechanism (e.g. run
as root).
This work was originally posted here:
https://inbox.sourceware.org/gdb-patches/20230321120126.1418012-1-benjamin@sipsolutions.net
I (Andrew Burgess) have made some cleanups to the code to comply with
GDB's coding standard, and the test is entirely mine. This commit
message is also entirely mine -- the original message was very terse
and required the reader to understand how the various namespaces
work and interact. The above is my attempt to document what I now
understand about the problem being fixed.
I've left the original author in place as the core of the GDB change
itself is largely as originally presented, but any inaccuracies in the
commit message, or problems with the test, are all mine.
Co-Authored-by: Andrew Burgess <aburgess@redhat.com>
|
|
This commit works around a problem introduced by commit:
commit e58beedf2c8a1e0c78e0f57aeab3934de9416bfc
Date: Tue Jan 23 16:00:59 2024 +0000
gdb: attach to a process when the executable has been deleted
The above commit extended GDB for Linux, so that, of the executable
for a process had been deleted, GDB would instead try to use
/proc/PID/exe as the executable.
This worked by updating linux_proc_pid_to_exec_file to introduce the
/proc/PID/exe fallback. However, the result of
linux_proc_pid_to_exec_file is then passed to exec_file_find to
actually find the executable, and exec_file_find, will take into
account the sysroot. In addition, if GDB is attaching to a process in
a different MNT and/or PID namespace then the executable lookup is
done within that namespace.
This all means two things:
1. Just because linux_proc_pid_to_exec_file cannot see the
executable doesn't mean that GDB is actually going to fail to
find the executable, and
2. returning /proc/PID/exe isn't useful if we know GDB is then going
to look for this within a sysroot, or within some other
namespace (where PIDs might be different).
There was an initial attempt to fix this issue here:
https://inbox.sourceware.org/gdb-patches/20250511141517.2455092-4-kilger@sec.in.tum.de/
This proposal addresses the issue in PR gdb/32955, which is all about
the namespace side of the problem. The fix in this original proposal
is to check the MNT namespace inside linux_proc_pid_to_exec_file, and
for the namespace problem this is fine. But we should also consider
the sysroot problem.
And for the sysroot problem, the fix cannot fully live inside
linux_proc_pid_to_exec_file, as linux_proc_pid_to_exec_file is shared
between GDB and gdbserver, and gdbserver has no sysroot.
And so, I propose a slightly bigger change.
Now, linux_proc_pid_to_exec_file takes a flag which indicates if
GDB (or gdbserver) will look for the inferior executable in the
local file system, where local means the same file system as GDB (or
gdbserver) is running in.
This local file system check is true if:
1. The MNT namespace of the inferior is the same as for GDB, and
2. for GDB only, the sysroot must either be empty, or 'target:'.
If the local file system check is false then GDB (or gdbserver) is
going to look elsewhere for the inferior executable, and so, falling
back to /proc/PID/exe should not be done, as GDB will end up looking
for this file in the sysroot, or within the alternative MNT
namespace (which in also likely to be a different PID namespace).
Now this is all a bit of a shame really. It would be nice if
linux_proc_pid_to_exec_file could return /proc/PID/exe in such a way
that exec_file_find would know that the file should NOT be looked for
in the sysroot, or in the alternative namespace. But fixing that
problem would be a much bigger change, so for now lets just disable
the /proc/PID/exe fallback for cases where it might not work.
For testing, the sysroot case is now tested.
I don't believe we have any alternative namespace testing. It would
certainly be interesting to add some, but I'm not proposing any with
this patch, so the code for checking the MNT namespace has been tested
manually by me, but isn't covered by a new test I'm adding here.
Author of the original fix is listed as co-author here. Credit for
identifying the original problem, and proposing a solution belongs to
them.
Co-Authored-By: Fabian Kilger <kilger@sec.in.tum.de>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32955
|
|
Currently, when attaching to a process, if the user hasn't told GDB
which executable they are going to be debugging, GDB will try to
figure out the executable from the running process.
There are two (for this patch) interesting places where this can fail,
both in exec_file_locate_attach.
First GDB calls target_pid_to_exec_file, this does target specific
"stuff" to find the name of the executable file. If this returns NULL
then GDB will give a warning and return.
After this we need to "find" the executable. This is where we apply
things like the sysroot in order to transform the executable path.
This is done by calling exec_file_find, and this too can return NULL
to indicate that the executable couldn't be found.
Currently, if exec_file_find returns NULL then GDB doesn't give a
warning, instead we push on and call try_open_exec_file passing in the
NULL pointer as the filename string. This has the effect of removing
the current executable from the current program space.
However, exec_file_locate_attach already checks there is no executable
attached to the current program space. If there was, then there would
be no need to try and lookup the executable from the running process.
So calling try_open_exec_file with a NULL string is, I claim,
pointless.
But worse, calling try_open_exec_file with a NULL string means that
GDB prints the message: "No executable file now.", which, while
correct, isn't (I think) very helpful. To me this message indicates
that we've moved from a state of having an executable to a state of
not having one, which isn't correct.
I think we should introduce a new warning in exec_file_locate_attach,
which is printed if the executable cannot be found.
So, before this patch GDB's output looked like this:
(gdb) attach 12345
Attaching to process 12345
No executable file now.
warning: Could not load vsyscall page because no executable was specified
0x00007f0978b94557 in ?? ()
(gdb)
After this patch the output now looks like this:
(gdb) attach 12345
Attaching to process 12345
No executable has been specified, and target executable /tmp/my-exec (deleted) could not be found. Try using the "file" command.
warning: Could not load vsyscall page because no executable was specified
0x00007f0978b94557 in ?? ()
(gdb)
This warning includes the name of the file that GDB was looking for,
and gives a hint that the 'file' command should be used to tell GDB
which executable is being debugged. Much better.
There's no test for this change in this commit. The next commit fixes
another (semi-related) bug, and includes a test that checks for this
warning string.
|
|
This patch simplifies the code at two points by removing redundant
null checks. There is no functional impact.
Reviewed-By: Keith Seitz <keiths@redhat.com>
Approved-By: Pedro Alves <pedro@palves.net>
Change-Id: I76e1c7fad00e8fcb24ced7bfd75d19cdd6266c32
|
|
Currently gdbserver uses the require_int() function to parse the
requested offset (in vFile::pread packet and the like). This function
allows integers up to 0x7fffffff (to fit in 32-bit int), however the
offset (for the pread system call) has an off_t type which can be
larger than 32-bit.
This patch allows require_int() function to parse offset up to the
maximum value implied by the off_t type.
Approved-By: Pedro Alves <pedro@palves.net>
Change-Id: I3691bcc1ab1838c0db7f8b82d297d276a5419c8c
|
|
`pre-commit run --all-files` found this.
Change-Id: I8db09b12cf184d32351ff2c579bdaa8cf6f80ac3
|
|
Change the messages to reflect that these numbers includes type units,
not only compile units.
Change-Id: Id2f511d4666e5cf92112be917d72ff76791b7e1d
Approved-by: Kevin Buettner <kevinb@redhat.com>
|
|
This commit adds a new gdb.warning() function. This function takes a
string and then calls GDB's internal warning() function. This will
display the string as a warning.
Using gdb.warning() means that the message will get the new emoji
prefix if the user has that feature turned on. Also, the message will
be sent to gdb.STDERR without the user having to remember to print to
the correct stream.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This fixes a bug related to build-id files with linux namespaces.
Specifically, we expect the debug files to be present inside the container,
thus the container filesystem should be queried if the program is running
inside one.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32956
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
The new algorithm to look for a build-id-based debug file
(introduced by commit 22836ca88591ac7efacf06d5b6db191763fd8aba)
makes use of fileio_lstat. As lstat was not supported by
linux-namespace.c, all lstat calls would be performed on the host
and not inside the namespace. Fixed by adding namespace lstat
support.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32956
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
This commit continues the work of the previous two commits.
In the following commits I added the target_fileio_stat function, and
the target_ops::fileio_stat member function:
* 08a115cc1c4 gdb: add target_fileio_stat, but no implementations yet
* 3055e3d2f13 gdb: add GDB side target_ops::fileio_stat implementation
* 6d45af96ea5 gdbserver: add gdbserver support for vFile::stat packet
* 22836ca8859 gdb: check for multiple matching build-id files
Unfortunately I messed up, despite being called 'stat' these function
actually performed an 'lstat'. The 'lstat' is the correct (required)
implementation, it's the naming that is wrong.
Additionally, to support remote targets, these commit added the
vFile::stat packet, which again, performed an 'lstat'.
In the previous two commits I changed the GDB code to replace 'stat'
with 'lstat' in the fileio function names. I then added a new
vFile:lstat packet which GDB now uses instead of vFile:stat.
And that just leaves the vFile:stat packet which is, right now,
performing an 'lstat'.
Now, clearly when I wrote this code I fully intended for this packet
to perform an lstat, it's the lstat that I needed. But now, I think,
we should "fix" vFile:stat to actually perform a 'stat'.
This is risky. This is a change in remote protocol behaviour.
Reasons why this might be OK:
- vFile:stat was only added in GDB 16, so it's not been "in the
wild" for too long yet. If we're quick, we might be able to "fix"
this before anyone realises I messed up.
- The documentation for vFile:stat is pretty vague. It certainly
doesn't explicitly say "this does an lstat". Most implementers
would (I think), given the name, start by assuming this should be
a 'stat' (given the name). Only if they ran the full GDB
testsuite, or examined GDB's implementation, would they know to
use lstat.
Reasons why this might not be OK:
- Some other debug client could be connecting to gdbserver, sending
vFile:stat and expecting to get lstat behaviour. This would break
after this patch.
- Some other remote server might have implemented vFile:stat
support, and either figured out, or copied, the lstat behaviour
from gdbserver. This remote server would technically be wrong
after this commit, but as GDB no longer uses vFile:stat, then this
will only become a problem if/when GDB or some other client starts
to use vFile:stat in the future.
Given the vague documentation for vFile:stat, and that it was only
added in GDB 16, I think we should fix it now to perform a 'stat', and
that is what this commit does.
The change in behaviour is documented in the NEWS file. I've improved
the vFile:stat documentation in the manual to better explain what is
expected from this packet, and I've extended the existing test to
cover vFile:stat.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
In the following commits I added the target_fileio_stat function, and
the target_ops::fileio_stat member function:
* 08a115cc1c4 gdb: add target_fileio_stat, but no implementations yet
* 3055e3d2f13 gdb: add GDB side target_ops::fileio_stat implementation
* 6d45af96ea5 gdbserver: add gdbserver support for vFile::stat packet
* 22836ca8859 gdb: check for multiple matching build-id files
Unfortunately I messed up, despite being called 'stat' these function
actually performed an 'lstat'. The 'lstat' is the correct (required)
implementation, it's the naming that is wrong.
In the previous commit I fixed the naming within GDB, renaming 'stat'
to 'lstat' throughout.
However, in order to support target_fileio_stat (as was) on remote
targets, the above patches added the vFile:stat packet, which actually
performed an 'lstat' call. This is really quite unfortunate, and I'd
like to do as much as I can to try and clean up this mess. But I'm
mindful that changing packets is not really the done thing.
So, this commit doesn't change anything.
Instead, this commit adds vFile:lstat as a new packet.
Currently, this packet is handled identically as vFile:stat, the
packet performs an 'lstat' call.
I then update GDB to send the new vFile:lstat instead of vFile:stat
for the remote_target::fileio_lstat implementation.
After this commit GDB will never send the vFile:stat packet.
However, I have retained the 'set remote hostio-stat-packet' control
flag, just in case someone was trying to set this somewhere.
Then there's one test in the testsuite which used to disable the
vFile:stat packet, that test is updated to now disable vFile:lstat.
There's a new test that does a more direct test of vFile:lstat. This
new test can be extended to also test vFile:stat, but that is left for
the next commit.
And so, after this commit, GDB sends the new vFile:lstat packet in
order to implement target_ops::fileio_lstat. The new packet is more
clearly documented than vFile:stat is. But critically, this change
doesn't risk breaking any other clients or servers that implement
GDB's remote protocol.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
In the following commits I added the target_fileio_stat function, and
the target_ops::fileio_stat member function:
* 08a115cc1c4 gdb: add target_fileio_stat, but no implementations yet
* 3055e3d2f13 gdb: add GDB side target_ops::fileio_stat implementation
* 6d45af96ea5 gdbserver: add gdbserver support for vFile::stat packet
* 22836ca8859 gdb: check for multiple matching build-id files
Unfortunately, I messed up when adding this API. The actual
underlying call is lstat, not stat.
This commit tries to clear up some of the confusion by renaming things
to target_fileio_lstat and target_ops::fileio_lstat.
After this change the function names now match the underlying
implementation.
One problem remains though. In order to support target_fileio_stat
for remote target the above patches added the vFile:stat packet to GDB
and gdbserver. The implementation of this packet still does an lstat
though, which is a bit of a shame. I'm going to try and fix that in
later commits.
This commit is just a rename within GDB, there should be no user
visible changes.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This method returns type units too, so "get_unit" is a better name.
Change-Id: I6ec9de3f783637a3e206bcaaec96a4e00b4b7d31
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Makes it possible to set and remove other types of breakpoints while the
process is running. Makes debugging more convenient.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
During testing csr instructions in risc-v, it occurs that instruction csrrci
is unsupported for recording process and there is such warning:
'warning: Currently this instruction with len 4(100174f3) is unsupported', so
recording failed. This patch fixes this error.
|
|
|
|
With MSYS2 and test-case gdb.ada/assign_1.exp, we get:
...
(gdb) dir^M
Reinitialize source path to empty? (y or n) \
[answered Y; input not from terminal]^M^M
Source directories searched: $cdir;$cwd^M^M
(gdb)
...
GDB automatically answers the query, because interactive-mode is off:
...
(gdb) show interactive-mode^M
Debugger's interactive mode is auto (currently off).^M^M
...
The correct value is on, because GDB was started in a terminal.
For some reason, the auto value of interactive-mode is off instead. According
to this patch [1], gdb doesn't recognize the pipes used by DejaGnu testsuite
as an interactive setup.
Fix this by adding "set interactive-mode on" to INTERNAL_GDBFLAGS, such that
we get:
...
(gdb) dir^M
Reinitialize source path to empty? (y or n) y^M
Source directories searched: $cdir;$cwd^M^M
(gdb)
...
and no longer need fixes like commit be740e7cc62 ("testsuite: skip
confirmation in 'gdb_reinitialize_dir'")
The fix is essentially the same as in aforementioned patch.
For consistency, we apply the fix for all platforms.
Co-Authored-By: Pierre Muller <muller@sourceware.org>
Approved-By: Tom Tromey <tom@tromey.com>
[1] https://sourceware.org/legacy-ml/gdb-patches/2013-09/msg00940.html
|
|
With MSYS2 and default TERM=xterm-256color (as well as with xterm and ansi), I
get:
...
builtin_spawn gdb -q ...
^[[6n(gdb) ERROR: GDB never initialized.
...
This is not specific to gdb, other tools produce the same CSI sequence, and
consequently we run into trouble in other places (like get_compiler_info).
Fix this by default-setting TERM to dumb.
We do this for all platforms, to avoid test-cases passing on one platform but
failing on another.
For test-cases that set TERM to something other than dumb, handle the CSI
sequence in default_gdb_start.
Approved-By: Tom Tromey <tom@tromey.com>
PR testsuite/33072
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33072
|
|
Recent GDB commits added more features related to linker namespaces and
documented them on the manual, but did not add a convenient way for a
user to understand what they are. This commit adds a quick explanation
of what they are.
It also fixes the inconsistency of using "linker namespaces" and
"linkage namespaces", by always using the first form to avoid user
confusion.
Approved-By: Eli Zaretskii <eliz@gnu.org>
|
|
Remove comma from: gdb.flush([, stream]) . I suspect this was a copy
and paste from gdb.write(string [, stream]) where the comma is
correct.
|
|
amd_dbgapi_target_breakpoint::check_status
ROCgdb handles target events very slowly when running a test case like
this, where a breakpoint is preset on HipTest::vectorADD:
for (int i=0; i < numDevices; ++i) {
HIPCHECK(hipSetDevice(i));
hipLaunchKernelGGL(HipTest::vectorADD, dim3(blocks), dim3(threadsPerBlock), 0, stream[i],
static_cast<const int*>(A_d[i]), static_cast<const int*>(B_d[i]), C_d[i], N);
}
What happens is:
- A kernel is launched
- The internal runtime breakpoint is hit during the second
hipLaunchKernelGGL call, which causes
amd_dbgapi_target_breakpoint::check_status to be called
- Meanwhile, all waves of the kernel hit the breakpoint on vectorADD
- amd_dbgapi_target_breakpoint::check_status calls process_event_queue,
which pulls the thousand of breakpoint hit events from the kernel
- As part of handling the breakpoint hit events, we write the PC of the
waves that stopped to decrement it. Because the forward progress
requirement is not disabled, this causes a suspend/resume of the
queue each time, which is time-consuming.
The stack trace where this all happens is:
#32 0x00007ffff6b9abda in amd_dbgapi_write_register (wave_id=..., register_id=..., offset=0, value_size=8, value=0x7fffea9fdcc0) at /home/smarchi/src/amd-dbgapi/src/register.cpp:587
#33 0x00005555588c0bed in amd_dbgapi_target::store_registers (this=0x55555c7b1d20 <the_amd_dbgapi_target>, regcache=0x507000002240, regno=470) at /home/smarchi/src/wt/amd/gdb/amd-dbgapi-target.c:2504
#34 0x000055555a5186a1 in target_store_registers (regcache=0x507000002240, regno=470) at /home/smarchi/src/wt/amd/gdb/target.c:3973
#35 0x0000555559fab831 in regcache::raw_write (this=0x507000002240, regnum=470, src=...) at /home/smarchi/src/wt/amd/gdb/regcache.c:890
#36 0x0000555559fabd2b in regcache::cooked_write (this=0x507000002240, regnum=470, src=...) at /home/smarchi/src/wt/amd/gdb/regcache.c:915
#37 0x0000555559fc3ca5 in regcache::cooked_write<unsigned long, void> (this=0x507000002240, regnum=470, val=140737323456768) at /home/smarchi/src/wt/amd/gdb/regcache.c:850
#38 0x0000555559fab09a in regcache_cooked_write_unsigned (regcache=0x507000002240, regnum=470, val=140737323456768) at /home/smarchi/src/wt/amd/gdb/regcache.c:858
#39 0x0000555559fb0678 in regcache_write_pc (regcache=0x507000002240, pc=0x7ffff62bd900) at /home/smarchi/src/wt/amd/gdb/regcache.c:1460
#40 0x00005555588bb37d in process_one_event (event_id=..., event_kind=AMD_DBGAPI_EVENT_KIND_WAVE_STOP) at /home/smarchi/src/wt/amd/gdb/amd-dbgapi-target.c:1873
#41 0x00005555588bbf7b in process_event_queue (process_id=..., until_event_kind=AMD_DBGAPI_EVENT_KIND_BREAKPOINT_RESUME) at /home/smarchi/src/wt/amd/gdb/amd-dbgapi-target.c:2006
#42 0x00005555588b1aca in amd_dbgapi_target_breakpoint::check_status (this=0x511000140900, bs=0x50600014ed00) at /home/smarchi/src/wt/amd/gdb/amd-dbgapi-target.c:890
#43 0x0000555558c50080 in bpstat_stop_status (aspace=0x5070000061b0, bp_addr=0x7fffed0b9ab0, thread=0x518000026c80, ws=..., stop_chain=0x50600014ed00) at /home/smarchi/src/wt/amd/gdb/breakpoint.c:6126
#44 0x000055555984f4ff in handle_signal_stop (ecs=0x7fffeaa40ef0) at /home/smarchi/src/wt/amd/gdb/infrun.c:7169
#45 0x000055555984b889 in handle_inferior_event (ecs=0x7fffeaa40ef0) at /home/smarchi/src/wt/amd/gdb/infrun.c:6621
#46 0x000055555983eab6 in fetch_inferior_event () at /home/smarchi/src/wt/amd/gdb/infrun.c:4750
#47 0x00005555597caa5f in inferior_event_handler (event_type=INF_REG_EVENT) at /home/smarchi/src/wt/amd/gdb/inf-loop.c:42
#48 0x00005555588b838e in handle_target_event (client_data=0x0) at /home/smarchi/src/wt/amd/gdb/amd-dbgapi-target.c:1513
Fix that performance problem by disabling the forward progress
requirement in amd_dbgapi_target_breakpoint::check_status, before
calling process_event_queue, so that we can process all events
efficiently.
Since the same performance problem could theoritically happen any time
process_event_queue is called with forward progress requirement enabled,
add an assert to ensure that forward progress requirement is disabled
when process_event_queue is invoked. This makes it necessary to add a
require_forward_progress call to amd_dbgapi_finalize_core_attach. It
looks a bit strange, since core files don't have execution, but it
doesn't hurt.
Add a test that replicates this scenario. The test launches a kernel
that hits a breakpoint (with an always false condition) repeatedly.
Meanwhile, the host process loads an unloads a code object, causing
check_status to be called.
Bug: SWDEV-482511
Change-Id: Ida86340d679e6bd8462712953458c07ba3fd49ec
Approved-by: Lancelot Six <lancelot.six@amd.com>
|
|
inferior
A following patch will want to call require_forward_progress for a given
inferior. Extract a new require_forward_progress overload from the
existing require_forward_progress function that targets a specific
inferior.
Change-Id: I54f42b83eb8443d4d91747ffbc86eaeb017f1e49
Approved-by: Lancelot Six <lancelot.six@amd.com>
|
|
Pass the amd_dbgapi_inferior_info object from process_event_queue to
process_one_event. Since process_event_queue pulls events for one
specific inferior, we know for which inferior the event is. This
removes the need for process_one_event to do two dbgapi calls to get the
relevant pid. If also removes one inferior lookup.
Change-Id: I22927e4b6251513eb3be95785082058aa3d09954
Approved-by: Lancelot Six <lancelot.six@amd.com>
|
|
A following patch will make process_event_queue access a field of
amd_dbgapi_inferior_info. Prepare for this by making
process_event_queue accept an amd_dbgapi_inferior_info object, instead
of a process id.
Change-Id: I9adc491dd1ff64ff74c40aa7662fffb11bd8332b
Approved-by: Lancelot Six <lancelot.six@amd.com>
|
|
I didn't have a problem in this area, but it seems to me that this
pre-condition should always hold. We should only disable forward
progress requirement if the target says it's ok to do so. Otherwise, we
could get in a situation where we wait for events from amd-dbgapi, which
will never arrive, because amd-dbgapi didn't actually resume things.
Change-Id: Ifc49f55c7874924b7c47888b8391a07a01d960fc
Approved-by: Lancelot Six <lancelot.six@amd.com>
|
|
Rely on the default value.
Change-Id: I08c683de005806c5c5d29ed7f9b0c6de81b49a01
Approved-By: Lancelot Six <lancelot.six@amd.com>
|
|
When running test-case gdb.python/py-source-styling-2.exp with TERM=dumb, I
get:
...
(gdb) set style enabled on^M
warning: The current terminal doesn't support styling. \
Styled output might not appear as expected.^M
(gdb) FAIL: $exp: set style enabled on
...
Fix this by using with_ansi_styling_terminal on clean_restart.
Tested on x86_64-linux.
|
|
The manual section on using GDB under Emacs is out-of-date and
duplicates existing and comprehensive documentation in the Emacs
manual.
Replace the section by a short introduction and reference.
Approved-By: Eli Zaretskii <eliz@gnu.org>
|
|
This value will likely never change at runtime, so we might as well make
it a template parameter. This has the "advantage" of being able to
remove the unnecessary param from gdb::sequential_for_each.
Change-Id: Ia172ab8e08964e30d4e3378a95ccfa782abce674
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I find this file difficult to work with and modify, due to how it uses
the preprocessor to include itself, to generate variations of the test
functions. Change it to something a bit more C++-y, with a test
function that accepts a callback to invoke the foreach function under
test.
Change-Id: Ibf1e2907380a88a4f8e4b4b88df2b0dfd0e9b6c8
|
|
Change-Id: Iaac252aa2abbe169153e79b84f956cda172c69d1
|
|
I noticed a minor grammer issue in a comment in DAP.
|
|
Setting a BP on a line like this would incorrectly yield two BP locations:
01 void two () { {int var = 0;} }
(gdb) break 1
Breakpoint 1 at 0x1164: main.cpp:1. (2 locations)
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x0000000000001164 in two() at main.cpp:1
1.2 y 0x0000000000001164 in two() at main.cpp:1
In this case decode_digits_ordinary () returns two SALs, exactly matching the
requested line. One for the entry PC and one for the prologue end PC. This
was
tested with GCC, CLANG and ICPX. Subsequent code tries to skip the prologue
on these PCs, which in turn makes them the same.
To fix this, ignore SALs with the same PC and program space when adding to the
list of SALs.
This will then properly set only one location:
(gdb) break 1
Breakpoint 1 at 0x1164: file main.cpp, line 1
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y 0x0000000000001164 in two() at main.cpp:1
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Convert 'set debug linux-namespaces' to the new(er) debug scheme. As
part of this change I converted the mnsh_debug_print_message function,
which previously printed its output, to instead return a std::string,
this string is then printed using linux_namespaces_debug_printf. The
mnsh_debug_print_message function is only used as part of the debug
output.
I also updated one place in the code where debug_linux_namespaces, the
debug control variable, which is a boolean, was assigned an integer.
When debug is turned on then clearly the output is now different, but
in all other cases, there should be no user visible change in GDB
after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The tables in z80-tdep.c previously either gave these instructions the
wrong size, or failed to recognize them by using the wrong masks, or
both. The fixed instructions alongside their representation in octal are:
* add ii,rr: [0335] 00r1 (where r & 1 == 1)
[0375] 00r1
* ld (ii+d,n): [0335] 0066 <d> <n>
[0375] 0066 <d> <n>
Prefix bytes inside [] do not count towards instruction length in
these tables.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33066
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Remove period from subsubsection titles in the AArch64 configuration-specific
subsection, and expand acronyms.
Regarding @cindex entries, remove periods and standardise their order
and the position of "AArch64" to make it easier to find them by
using the index-searching commands of Info readers that offer TAB
completion.
Approved-By: Eli Zaretskii <eliz@gnu.org>
|
|
The Windows port does not support multi-process debugging. Testcases
that want to exercise multi-process currently FAIL and some hit
cascading timeouts. Add a new allow_multi_inferior_tests procedure,
meant to be used with require, and sprinkle it throughout testcases as
needed.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Change-Id: I4a10d8f04f9fa10f4b751f140ad0a6d31fbd9dfb
|
|
Cygwin debugging does not support follow fork. There is currently no
interface between the debugger and the Cygwin runtime to be able to
intercept forks and execs. Consequently, testcases that try to
exercise fork/exec all FAIL, and several hit long cascading timeouts.
Add a new allow_fork_tests procedure, meant to be used with require,
and sprinkle it throughout testcases that exercise fork.
Note that some tests currently are skipped on targets other than
Linux, with something like:
# Until "set follow-fork-mode" and "catch vfork" are implemented on
# other targets...
#
if {![istarget "*-linux*"]} {
continue
}
However, some BSD ports also support fork debugging nowadays, and the
testcases were never adjusted... That is why the new allow_fork_tests
procedure doesn't look for linux.
With this patch, on Cygwin, I get this:
$ make check TESTS="*/*fork*.exp"
...
=== gdb Summary ===
# of expected passes 6
# of untested testcases 1
# of unsupported tests 31
Reviewed-By: Keith Seitz <keiths@redhat.com>
Change-Id: I0c5e8c574d1f61b28d370c22a0b0b6bc3efaf978
|
|
Running gdb.multi/attach-no-multi-process.exp on Cygwin, where
GDBserver does not support non-stop mode, I see:
FAIL: gdb.multi/attach-no-multi-process.exp: target_non_stop=off: info threads
FAIL: gdb.multi/attach-no-multi-process.exp: target_non_stop=on: attach to the program via remote (timeout)
FAIL: gdb.multi/attach-no-multi-process.exp: target_non_stop=on: info threads (timeout)
Let's ignore the first "info threads" fail. The timeouts look like
this:
builtin_spawn /home/alves/gdb-cache-cygwin/gdb/../gdbserver/gdbserver --once --multi localhost:2346
Listening on port 2346
target extended-remote localhost:2346
Remote debugging using localhost:2346
Non-stop mode requested, but remote does not support non-stop
(gdb) gdb_do_cache: can_spawn_for_attach ( )
builtin_spawn /home/alves/gdb/build-cygwin-testsuite/outputs/gdb.multi/attach-no-multi-process/attach-no-multi-process
attach 14540
FAIL: gdb.multi/attach-no-multi-process.exp: target_non_stop=on: attach to the program via remote (timeout)
info threads
FAIL: gdb.multi/attach-no-multi-process.exp: target_non_stop=on: info threads (timeout)
Note the "Non-stop mode requested, but remote does not support
non-stop" line.
The intro to gdb_target_cmd_ext says:
# gdb_target_cmd_ext
# Send gdb the "target" command. Returns 0 on success, 1 on failure, 2 on
# unsupported.
That's perfect here, we can just use gdb_target_cmd_ext instead of
gdb_target_cmd, and check for 2 (unsupported). That's what this patch
does.
However gdb_target_cmd_ext incorrectly returns 1 instead of 2 for the
case where the remote target says it does not support non-stop. That
is also fixed by this patch.
With this, we no longer get those timeout fails. We get instead:
target extended-remote localhost:2346
Remote debugging using localhost:2346
Non-stop mode requested, but remote does not support non-stop
(gdb) UNSUPPORTED: gdb.multi/attach-no-multi-process.exp: target_non_stop=on: non-stop RSP
Approved-by: Kevin Buettner <kevinb@redhat.com>
Change-Id: I1ab3162f74200c6c02a17a0600b102d2d12db236
|
|
On Cygwin, starting an inferior under GDB, and detaching it, quitting
GDB, and then closing the shell, like so:
(gdb) start
(gdb) detach
(gdb) quit
# close shell
... hangs the parent shell of GDB (not GDB!) until the inferior
process that was detached (as it is still using the same terminal GDB
was using) exits too.
This leads to odd failures in gdb.base/watchpoint-hw-attach.exp like
so:
detach
Detaching from program: .../outputs/gdb.base/watchpoint-hw-attach/watchpoint-hw-attach, process 16580
[Inferior 1 (process 16580) detached]
(gdb) FAIL: gdb.base/watchpoint-hw-attach.exp: detach
Fix this by converting the testcase to spawn the inferior outside GDB,
with spawn_wait_for_attach.
With this patch, the testcase passes cleanly on Cygwin, for me.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I8e3884073a510d6fd2fff611e1d26fc808adc4fa
|
|
Commit 58984e4a ("Use gdb::function_view in iterate_over_threads")
broke the Solaris build. This patch attempts to fix it, changing
find_signalled_thread to have the correct signature, and correcting a
couple of problems in sol_thread_target::get_ada_task_ptid.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33073
|
|
When writing commit 28f15782adab ("gdb/dwarf: read multiple .debug_info.dwo
sections"), I initially thought that the gcc behavior of producing multiple
.debug_info.dwo sections was a bug (it is not). I updated the commit
message, but it looks like this comment stayed. Remove it, since it can
be misleading.
Change-Id: I027712d44b778e836f41afbfafab993da02726ef
Approved-By: Tom Tromey <tom@tromey.com>
|
|
While C++ifying the solib code, I concluded that all arches that use
SVR4 libraries do provide link map offsets, so I think this function is
unnecessary now.
Change-Id: Ifaae2560d92f658df3724def6219e2f89054e4b7
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Running gdb.cp/cpexprs.exp on x86-64 GNU/Linux, I see:
break base::~base
Breakpoint 117 at 0x555555555d90: file .../src/gdb/testsuite/gdb.cp/cpexprs.cc, line 135.
(gdb) continue
Continuing.
Breakpoint 117, base::~base (this=0x7fffffffd0f8, __in_chrg=<optimized out>) at .../src/gdb/testsuite/gdb.cp/cpexprs.cc:135
135 ~base (void) { } // base::~base
(gdb) PASS: gdb.cp/cpexprs.exp: continue to base::~base
Here, the breakpoint only got one location because both the in-charge
and the not-in-charge dtors are identical and got the same address:
$ nm -A ./testsuite/outputs/gdb.cp/cpexprs/cpexprs| c++filt |grep "~base"
./testsuite/outputs/gdb.cp/cpexprs/cpexprs:0000000000001d84 W base::~base()
./testsuite/outputs/gdb.cp/cpexprs/cpexprs:0000000000001d84 W base::~base()
While on Cygwin, we get two locations for the same breakpoint, which
the testcase isn't expecting:
break base::~base
Breakpoint 117 at 0x100402678: base::~base. (2 locations)
(gdb) continue
Continuing.
Thread 1 "cpexprs" hit Breakpoint 117.1, base::~base (this=0x7ffffcaf8, __in_chrg=<optimized out>) at .../src/gdb/testsuite/gdb.cp/cpexprs.cc:135
135 ~base (void) { } // base::~base
(gdb) FAIL: gdb.cp/cpexprs.exp: continue to base::~base
We got two locations because the in-charge and the not-in-charge dtors
have different addresses:
$ nm -A outputs/gdb.cp/cpexprs/cpexprs.exe | c++filt | grep "~base"
outputs/gdb.cp/cpexprs/cpexprs.exe:0000000100402680 T base::~base()
outputs/gdb.cp/cpexprs/cpexprs.exe:0000000100402690 T base::~base()
On Cygwin, we also see the typical failure due to not expecting the
inferior to be multi-threaded:
(gdb) continue
Continuing.
[New Thread 628.0xe08]
Thread 1 "cpexprs" hit Breakpoint 200, test_function (argc=1, argv=0x7ffffcc20) at .../src/gdb/testsuite/gdb.cp/cpexprs.cc:336
336 derived d;
(gdb) FAIL: gdb.cp/cpexprs.exp: continue to test_function for policyd3::~policyd
Both issues are fixed by this patch, and now the testcase passes
cleanly on Cygwin, for me.
Reviewed-By: Keith Seitz <keiths@redhat.com>
Change-Id: If7eb95d595f083f36dfebf9045c0fc40ef5c5df1
|
|
I noticed on Cygwin, gdb.thread/thread-execl.exp would hang, (not that
surprising since we can't follow-exec on Cygwin). Looking at the
process list running on the machine, we end up with a thread-execl.exe
process constantly respawning another process [1].
We see the same constant-reexec if we launch gdb.thread/thread-execl
manually on the shell:
$ ./testsuite/outputs/gdb.threads/thread-execl/thread-execl
# * doesn't exit, constantly re-execing *
^C
Prevent this leftover constantly-re-execing scenario by making the
testcase program only exec once. We now get:
$ ./testsuite/outputs/gdb.threads/thread-execl/thread-execl
$ # exits immediately after one exec.
On Cygwin, the testcase now fails reasonably quickly, and doesn't
leave stale processes behind.
Still passes cleanly on x86-64 GNU/Linux.
[1] Cygwin's exec emulation spawns a new Windows process for the new
image.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Change-Id: I0de1136cf2ef7e89465189bc43489a2139a80efb
|
|
Cygwin supports dumping ELF cores via a dumper.exe utility, see
https://www.cygwin.com/cygwin-ug-net/dumper.html.
When I run a testcase that has the "kernel" generate a corefile, like
gdb.base/corefile.exp, Cygwin invokes dumper.exe correctly and
generates an ELF core file, however, the testsuite doesn't find the
generated core:
Running /home/alves/gdb/src/gdb/testsuite/gdb.base/corefile.exp ...
WARNING: can't generate a core file - core tests suppressed - check ulimit -c
The file is correctly put under $coredir, e.g., like so:
outputs/gdb.base/corefile/coredir.8926/corefile.exe.core
The problem is in this line in core_find:
foreach i "${coredir}/core ${coredir}/core.coremaker.c ${binfile}.core" {
Note that that isn't looking for "${binfile}.core" inside
${coredir}... That is fixed in this patch.
However, that still isn't sufficient for Cygwin + dumper, as in that
case the core is going to be called foo.exe.core, not foo.core. Fix
that by looking for foo.exe.core in the core dir as well.
With this, gdb.base/corefile.exp and other tests that use core_find
now run. They don't pass cleanly, but at least now they're exercised.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ic807dd2d7f22c5df291360a18c1d4fbbbb9b993e
|
