Pedro Alves [Thu, 11 Sep 2025 12:14:16 +0000 (13:14 +0100)]
Make get_compiler_info use gdb_caching_proc
While running tests on Windows with:
$ make check-parallel RUNTESTFLAGS="-v"
I noticed that get_compiler_info was invoking the compiler over and
over for each testcase, even though the result is supposed to be
cached.
This isn't normally very visible in gdb.log, because we suppress it
there:
# Run $ifile through the right preprocessor.
# Toggle gdb.log to keep the compiler output out of the log.
set saved_log [log_file -info]
log_file
...
I'm not sure it's a good idea to do that suppression, BTW. I was very
confused when I couldn't find the compiler invocation in gdb.log, and
it took me a while to notice that code.
The reason get_compiler_info in parallel mode isn't hitting the cache
is that in that mode each testcase runs under its own expect/dejagnu
process, and the way get_compiler_info caches results currently
doesn't handle that -- the result is simply cached in a global
variable, which is private to each expect.
So improve this by switching get_compiler_info's caching mechanism to
gdb_caching_proc instead, so that results are cached across parallel
invocations of dejagnu.
On an x86-64 GNU/Linux run with "make check-parallel -j32", before the
patch I get 2223 calls to get_compiler_info that result in a compiler
invocation. After the patch, I get 7.
On GNU/Linux, those compiler invocations don't cost much, but on
Windows, they add up. On my machine each invocation takes around
500ms to 700ms. Here is one representative run:
$ time x86_64-w64-mingw32-gcc \
/c/msys2/home/alves/gdb/build-testsuite/temp/14826/compiler.c \
-fdiagnostics-color=never -E
...
real 0m0.639s
user 0m0.061s
sys 0m0.141s
This reference to a 'compiler_info' global:
# N.B. compiler_info is intended to be local to this file.
# Call test_compiler_info with no arguments to fetch its value.
# Yes, this is counterintuitive when there's get_compiler_info,
# but that's the current API.
if [info exists compiler_info] {
unset compiler_info
}
is outdated, even before this patch, as "compiler_info" is a local
variable in get_compiler_info. Remove all that code.
Since test_compiler_info now calls get_compiler_info directly, the
"Requires get_compiler_info" comments in skip_inline_frame_tests and
skip_inline_var_tests are no longer accurate. Remove them.
test_compiler_info's intro comment is also outdated; improve it.
Changing the return value of get_compiler_info to be the
'compiler_info' string directly instead of 0/-1 was simpler. It would
be possible to support the current 0/-1 interface by making
get_compiler_info_1 still return the 'compiler_info' string, and then
having the get_compiler_info wrapper convert to 0/-1, and I considered
doing that. But the only caller of get_compiler_info outside gdb.exp
is gdb.python/py-event-load.exp, and it seems that one simply crossed
wires with:
as the test as added at roughly the same time as that commit.
So simply remove that call in gdb.python/py-event-load.exp, otherwise
we get something like:
ERROR: -------------------------------------------
ERROR: in testcase src/gdb/testsuite/gdb.python/py-event-load.exp
ERROR: expected boolean value but got "gcc-13-3-0"
ERROR: tcl error code TCL VALUE NUMBER
ERROR: tcl error info:
expected boolean value but got "gcc-13-3-0"
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ia3d3dc34f7cdcf9a2013f1054128c62a108eabfb
Simon Marchi [Wed, 20 Aug 2025 16:50:08 +0000 (12:50 -0400)]
gdbsupport: remove xmalloc in format_pieces
Remove the use of xmalloc (and the arbitrary allocation size) in
format_pieces. This turned out a bit more involved than expected, but
not too bad.
format_pieces::m_storage is a buffer with multiple concatenated
null-terminated strings, referenced by format_piece::string. Change
this to an std::string, while keeping its purpose (use the std::string
as a buffer with embedded null characters).
However, because the std::string's internal buffer can be reallocated as
it grows, and I do not want to hardcode a big reserved size like we have
now, it's not possible to store the direct pointer to the string in
format_piece::string. Those pointers would become stale as the buffer
gets reallocated. Therefore, change format_piece to hold an index into
the storage instead. Add format_pieces::piece_str for the callers to be
able to access the piece's string. This requires changing the few
callers, but in a trivial way.
The selftest also needs to be updated. I want to keep the test cases
as-is, where the expected pieces contain the expected string, and not
hard-code an expected index. To achieve this, add the
expected_format_piece structure. Note that the previous
format_piece::operator== didn't compare the n_int_args fields, while the
test provides expected values for that field. I guess that was a
mistake. The new code checks it, and the test still passes.
Change-Id: I80630ff60e01c8caaa800ae22f69a9a7660bc9e9 Reviewed-By: Keith Seitz <keiths@redhat.com>
Richard Earnshaw [Mon, 15 Sep 2025 10:42:52 +0000 (11:42 +0100)]
arm: Rename some tests to avoid duplicate tests names
A number of arm-specific tests print the same name. This can cause problems
if one of those tests fails, since then comparing tests with GCC's
compare_tests script can result in ambiguities in the changes summary.
Avoid this by giving tests unique names.
Still to do is where a test is run more than once (eg by having multiple
'#as: ' lines). This will require a tweak to the framework.
Andrew Burgess [Thu, 20 Jul 2023 10:12:40 +0000 (11:12 +0100)]
gdb: improve show text and help text for 'remote exec-file'
The current behaviour for 'show remote exec-file' is this:
(gdb) show remote exec-file
(gdb) set remote exec-file /abc
(gdb) show remote exec-file
/abc
(gdb)
The first output, the blank line, is just GDB showing the default
empty value.
This output is not really inline with GDB's more full sentence style
output, so in this commit I've updated things, the output is now:
(gdb) show remote exec-file
The remote exec-file is unset, the default remote executable will be used.
(gdb) set remote exec-file /abc
(gdb) show remote exec-file
The remote exec-file is "/abc".
(gdb)
Which I think is more helpful to the user.
I have also updated the help text for this setting. Previously we had
a set/show header line, but no body text, now we have:
(gdb) help show remote exec-file
Show the remote file name for starting inferiors.
This is the file name, on the remote target, used when starting an
inferior, for example with the \"run\", \"start\", or \"starti\"
commands. This setting is only useful when debugging a remote target,
otherwise, this setting is not used.
(gdb)
Which I think is more helpful.
Reviewed-By: Mark Wielaard <mark@klomp.org> Tested-By: Mark Wielaard <mark@klomp.org> Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-By: Tom Tromey <tom@tromey.com>
Andrew Burgess [Thu, 20 Jul 2023 09:25:43 +0000 (10:25 +0100)]
gdb: improve how 'remote exec-file' is stored and accessed
This commit makes two related changes. The goal of the commit is to
update the 'remote exec-file' setting to work correctly in a
multi-inferior setup. To do this I have switched from the older
style add_setshow_* function, which uses a single backing variable, to
the newer style add_setshow_* functions that uses a get/set callback.
The get/set callbacks now directly access the state held in the
progspace which ensures that the correct value is always returned.
However, the new get/set API requires that the get callback return a
reference to the setting's value, which in this case needs to be a
std::string.
Currently the 'remote exec-file' setting is stored as a 'char *'
string, which isn't going to work.
And so, this commit also changes 'remote exec-file' to be stored as a
std::string within the progspace.
Now, when switching between multiple inferiors, GDB can correctly
inform the user about the value of the 'remote exec-file' setting.
Andrew Burgess [Thu, 20 Jul 2023 09:20:33 +0000 (10:20 +0100)]
gdb: have remote_target::extended_remote_run take the exec filename
Small refactor, have remote_target::extended_remote_run take the name
of the executable to run rather than looking it up directly, the one
caller of this function has already looked up the remote-exec
filename.
There should be no user visible changes after this commit.
Tom de Vries [Sat, 13 Sep 2025 10:36:33 +0000 (12:36 +0200)]
[gdb/testsuite] Fix test names in gdb.tui/{empty,new-layout}.exp
Post-commit review [1] pointed out that this change in gdb.tui/empty.exp:
...
- eval Term::check_box [list "box $boxno"] $box
+ Term::check_box [list "box $boxno"] {*}$box
...
is incomplete because it leaves the "[list ...]" in place.
Indeed, it changes the test name like this:
...
-PASS: gdb.tui/empty.exp: src: 80x24: box 1
+PASS: gdb.tui/empty.exp: src: 80x24: {box 1}
...
Add DRAFT marker to be emitted in the info, pdf and html outputs. This
is done in two places: one in the @ifnottex block meant for PDF output
and another in @titlepage block meant for info and html output.
While at it, also add date to non-pdf outputs.
The marker lines:
@center @strong{*** DRAFT - NOT FOR DISTRIBUTION ***}
should be removed before a release.
libsframe/doc/
* sframe-spec.texi: Add marker for DRAFT.
Older versions of ncurses (including the version that ships inside
macos, and Centos 7) do not include the A_ITALIC macro. This patch
simply hides any use of A_ITALIC behind a preprocessor guard.
The result of this is that italics won't be rendered in the tui
if ncurses isn't supported. We do have other options if we think
it's important - for instance we could show italics as bold if
italics aren't supported. From my understanding, that might be
overthinking it - so I took the simplest approach here, just to
fix the build.
Those versions also define tgetnum as:
int tgetnum(char *id);
so attempting to compile for c++ results in the error:
ISO C++ forbids converting a string constant to 'char*' [-Werror=write-strings]
This is just a dated API issue, so a const cast resolves the issue.
Tom Tromey [Fri, 12 Sep 2025 16:59:20 +0000 (10:59 -0600)]
Fix 32-bit failure in array_long_idx.exp
Testing on the AdaCore-internal equivalent to array_long_idx.exp
showed that it failed on 32-bit targets. This patch fixes the problem
by arranging to use types that aren't target-dependent.
Andrew Burgess [Wed, 16 Jul 2025 14:31:28 +0000 (15:31 +0100)]
gdb: clear proceed status before starting a new inferior
This patch fixes a bug when 'set schedule-multiple on' is in use and a
second inferior is started using the 'run' command (or 'start' or
'starti'). This bug was reported as PR gdb/28777.
The problem appears as the first inferior terminating with an
unexpected SIGTRAP. The bug can be reproduced like this:
The final 'run' can be replaced with 'start' or 'starti'. The output
is different in the 'starti' case, but the cause is the same. For the
'run' and 'start' cases the final output is:
Starting program: /tmp/spin
Program terminated with signal SIGTRAP, Trace/breakpoint trap.
The program no longer exists.
In the 'starti' case the output is:
Starting program: /tmp/spin
Thread 2.1 "spin" stopped.
Cannot remove breakpoints because program is no longer writable.
Further execution is probably impossible.
0x00007ffff7fd3110 in _start () from /lib64/ld-linux-x86-64.so.2
What's happening is that GDB is failing to clear the previous proceed
status from inferior 1 before starting inferior 2. Normally when
schedule-multiple is off, this isn't a problem as 'run' only starts
the new inferior, and the new inferior will have no previous proceed
status that needs clearing.
But when schedule-multiple is on, starting a new inferior, with 'run'
and friends, will actually start all inferiors, including those that
previous stopped at a breakpoint with a SIGTRAP signal.
By failing to clear out the proceed status for those threads, when GDB
restarts inferior 1 it arranges for the thread to receive the SIGTRAP,
which is delivered, and, as GDB isn't expecting a SIGTRAP, is allowed
to kill the process.
Fix this by calling clear_proceed_status from run_command_1.
Andrew Burgess [Thu, 17 Jul 2025 14:50:51 +0000 (15:50 +0100)]
gdb: ensure thread state is updated when remote target starts up
This patch fixes a bug that was exposed by a test added in the
previous commit. After writing this patch I also discovered that this
issue had been reported as PR gdb/27322.
When 'maint set target-non-stop on' is in effect, then the remote
targets will be running in non-stop mode. The previous commit
revealed a bug where, in this mode, GDB can fail to copy the thread
state from the target to the GDB frontend, this leaves the thread
marked as running in the frontend, even though the thread is actually
stopped. When this happens the user is no longer able to interrupt
the thread (it's already stopped), nor can the user resume the
thread (GDB thinks the threads is running).
The 'info threads' output will look something like:
Id Target Id Frame
1.1 process 1746383 "ls" (running)
* 2.1 Thread 1746389.1746389 "ls" 0x00007ffff7fd3110 in _start () from /lib64/ld-linux-x86-64.so.2
The thread 1.1 should be stopped. GDB is running in all-stop mode
after all.
The problem is that in remote_target::process_initial_stop_replies,
there is a call to stop_all_threads, however, the changes in the
thread state are never copied back to the GDB frontend. This leaves
the threads stopped, but still marked running.
Solve this by adding a scoped_finish_thread_state. This is similar to
how scoped_finish_thread_state is used in run_command_1 when we start
a new inferior running.
The problem is that target_stop is being called for a target when
commit_resumed_state is true, the comment on
process_stratum_target::commit_resumed_state is pretty clear:
To simplify the implementation of targets, the following methods
are guaranteed to be called with COMMIT_RESUMED_STATE set to
false:
- resume
- stop
- wait
So clearly we're breaking a precondition of target_stop. In this
example there are two target, the native target (inferior 1), and the
remote target (inferior 2). It is the first, the native target, for
which commit_resumed_state is set incorrectly.
At the point target_stop is called looks like this:
#11 0x00000000009a3c19 in target_stop (ptid=...) at ../../src/gdb/target.c:3760
#12 target_stop (ptid=...) at ../../src/gdb/target.c:3756
#13 0x00000000007042f2 in stop_all_threads (reason=<optimized out>, inf=<optimized out>) at ../../src/gdb/infrun.c:5739
#14 0x0000000000711d3a in wait_for_inferior (inf=0x2b90fd0) at ../../src/gdb/infrun.c:4412
#15 start_remote (from_tty=from_tty@entry=1) at ../../src/gdb/infrun.c:3829
#16 0x0000000000897014 in remote_target::start_remote_1 (this=this@entry=0x2c4a520, from_tty=from_tty@entry=1, extended_p=extended_p@entry=0) at ../../src/gdb/remote.c:5350
#17 0x00000000008976e7 in remote_target::start_remote (extended_p=0, from_tty=1, this=0x2c4a520) at ../../src/gdb/remote.c:5441
#18 remote_target::open_1 (name=<optimized out>, from_tty=1, extended_p=0) at ../../src/gdb/remote.c:6312
#19 0x00000000009a815f in open_target (args=0x7fffffffa93c "| gdbserver - ls", from_tty=1, command=<optimized out>) at ../../src/gdb/target.c:838
For new inferiors commit_resumed_state starts set to false, for this
reason, if we only start a remote inferior, then when
wait_for_inferior is called commit_resumed_state will be false, and
everything will work.
Further, as target_stop is only called for running threads, if, when
the remote inferior is started, all other threads (in other targets)
are already stopped, then GDB will never need to call target_stop for
the other targets, and so GDB will not notice that
commit_resumed_state for those target is set to true.
In this case though, as the first (native) inferior is left running in
the background while the remote inferior is created, and because GDB
is running in all-stop mode (so needs to stop all threads in all
targets), then GDB does call target_stop for the other targets, and so
spots that commit_resumed_state is not set correctly and asserts.
The fix is to add scoped_disable_commit_resumed somewhere in the call
stack. Initially I planned to add the scoped_disable_commit_resumed
in `wait_for_inferior`, however, this isn't good enough. This
location would solve the problem as described in the bug, but when
writing the test I extended the problem to also cover non-stop mode,
and this runs into a second problem, the same assertion, but triggered
from a different call path. For this new case the stack looks like
this:
#1 0x0000000000fb0e50 in target_stop (ptid=...) at ../../src/gdb/target.c:3771
#2 0x0000000000a7f0ae in stop_all_threads (reason=0x1d0ff74 "remote connect in all-stop", inf=0x0) at ../../src/gdb/infrun.c:5756
#3 0x0000000000d9c028 in remote_target::process_initial_stop_replies (this=0x3e10670, from_tty=1) at ../../src/gdb/remote.c:5017
#4 0x0000000000d9cdf0 in remote_target::start_remote_1 (this=0x3e10670, from_tty=1, extended_p=0) at ../../src/gdb/remote.c:5405
#5 0x0000000000d9d0d4 in remote_target::start_remote (this=0x3e10670, from_tty=1, extended_p=0) at ../../src/gdb/remote.c:5457
#6 0x0000000000d9e8ac in remote_target::open_1 (name=0x7fffffffa931 "| gdbserver - /bin/ls", from_tty=1, extended_p=0) at ../../src/gdb/remote.c:6329
#7 0x0000000000d9d167 in remote_target::open (name=0x7fffffffa931 "| gdbserver - /bin/ls", from_tty=1) at ../../src/gdb/remote.c:5479
#8 0x0000000000f9914d in open_target (args=0x7fffffffa931 "| gdbserver - /bin/ls", from_tty=1, command=0x35d1a40) at ../../src/gdb/target.c:838
So I'm now thinking that stop_all_threads would be the best place for
the scoped_disable_commit_resumed. I did leave an assert in
wait_for_inferior as, having thought about the assert some, I do still
think the logic of it is true, and it doesn't hurt to leave it in
place I think.
However, it's not quite that simple, the test throws up yet another
bug when we 'maint set target-non-stop on', but then 'set non-stop
off'. This bug leaves a stopped thread marked as "(running)" in the
'info threads' output. I have a fix for this issue, but I'm leaving
that for the next commit. For now I've just disabled part of the test
in the problem case.
I've also tagged this patch with PR gdb/27322. That bug was created
before the above assert was added, but if you follow the steps to
reproduce for that bug today you will hit the above assert. The
actual issue described in PR gdb/27322 is fixed in the next patch.
Andrew Burgess [Wed, 16 Jul 2025 18:29:55 +0000 (19:29 +0100)]
gdb: ensure normal stop finishes the thread state of all threads
This patch fixes a multi-target issue where the normal_stop function
can fail to finish the thread state of threads from a non current
target, this leaves the threads marked as running in GDB core, while
the threads is actually stopped.
For testing I used this test program:
#include <unistd.h>
int
main ()
{
while (1)
sleep (1);
return 0;
}
Compile this to make '/tmp/spin', then the bug can be shown using this
command:
Id Target Id Frame
1.1 process 1610445 "spin" (running)
* 2.1 Thread 1610451.1610451 "spin" main () at spin.c:7
(gdb)
Notice that thread 1.1 is marked as running when it should be
stopped. We can see that the thread is actually stopped if we try
this:
(gdb) inferior 1
[Switching to inferior 1 [process 1610445] (/tmp/spin)]
[Switching to thread 1.1 (process 1610445)](running)
(gdb) continue
Cannot execute this command while the selected thread is running.
(gdb) interrupt
(gdb) info threads
Id Target Id Frame
* 1.1 process 1610445 "spin" (running)
2.1 Thread 1610451.1610451 "spin" main () at spin.c:7
(gdb)
We can see the expected behaviour if both inferiors run on the same
target, like this:
The 'info threads' from this series of commands looks like this:
Id Target Id Frame
1.1 process 1611589 "spin" 0x00007ffff7e951e7 in nanosleep () from /lib64/libc.so.6
* 2.1 process 1611593 "spin" main () at spin.c:7
(gdb)
Now both threads are stopped as we'd expect.
The problem is in normal_stop. The scoped_finish_thread_state uses
user_visible_resume_target to select the target(s) over which GDB will
iterate to find the threads to update.
The problem with this is that when the ptid_t is minus_one_ptid,
meaning all threads, user_visible_resume_target only returns nullptr,
meaning all targets, when sched_multi is true.
This dependency on sched_multi makes sense when _resuming_ threads.
If we are resuming all threads, then when sched_multi (the
schedule-multiple setting) is off (the default), all threads actually
means all threads in the current inferior only. When sched_multi is
true (schedule-multiple is on) then this means all threads, from all
inferiors, which means GDB needs to consider every target.
However, when stopping an inferior in all-stop mode (non_stop is
false), then GDB wants to stop all threads from all inferiors,
regardless of the sched_multi setting.
What this means is that, when 'non_stop' is false, then we should be
passing nullptr as the target selection to scoped_finish_thread_state.
My proposal is that we should stop using user_visible_resume_target in
the normal_stop function for the target selection of the
scoped_finish_thread_state, instead we should manually figure out the
correct target value and pass this in.
There is precedent for this in GDB, see run_command_1, where
'finish_target' is calculated directly within the function rather than
using user_visible_resume_target.
After this commit, when using two different targets (native and
remote) as in my first example above, both threads will be correctly
stopped.
Tom de Vries [Fri, 12 Sep 2025 13:04:50 +0000 (15:04 +0200)]
[gdb/testsuite, tclint] Fix gdb.rocm
Running tclint on the test-cases in gdb.rocm shows a few problems:
...
precise-memory-multi-inferiors.exp:33:5: expected braced word or word \
without substitutions in argument interpreted as expr [command-args]
precise-memory-multi-inferiors.exp:43:5: expected braced word or word \
without substitutions in argument interpreted as expr [command-args]
precise-memory-multi-inferiors.exp:55:5: expected braced word or word \
without substitutions in argument interpreted as expr [command-args]
...
Fix these.
The gdb.rocm test-cases are unsupported for me, so I can't test this.
Tom de Vries [Fri, 12 Sep 2025 13:04:50 +0000 (15:04 +0200)]
[gdb/testsuite, tclint] Fix gdb.rust
Running tclint on the test-cases in gdb.rust shows a few problems:
...
modules.exp:37:1: expected braced word or word without substitutions in \
argument interpreted as expr [command-args]
traits.exp:28:13: expected braced word or word without substitutions in \
argument interpreted as script [command-args]
...
Tom de Vries [Fri, 12 Sep 2025 13:04:50 +0000 (15:04 +0200)]
[gdb/testsuite, tclint] Fix gdb.server
Running tclint on the test-cases in gdb.server shows a few problems:
...
connect-with-no-symbol-file.exp:72:1: line has trailing whitespace \
[trailing-whitespace]
exit-multiple-threads.exp:73:5: expected braced word or word without \
substitutions in argument interpreted as expr [command-args]
extended-remote-restart.exp:73:5: expected braced word or word without \
substitutions in argument interpreted as expr [command-args]
monitor-exit-quit.exp:73:5: expected braced word or word without \
substitutions in argument interpreted as expr [command-args]
reconnect-ctrl-c.exp:54:5: expected braced word or word without \
substitutions in argument interpreted as expr [command-args]
server-exec-info.exp:24:1: expected braced word or word without \
substitutions in argument interpreted as expr [command-args]
stop-reply-no-thread.exp:73:5: expected braced word or word without \
substitutions in argument interpreted as expr [command-args]
/stop-reply-no-thread-multi.exp:81:5: expected braced word or word without \
substitutions in argument interpreted as expr [command-args]
...
Tom de Vries [Fri, 12 Sep 2025 13:04:50 +0000 (15:04 +0200)]
[gdb/testsuite, tclint] Fix lib/tuiterm.exp
Running tclint on lib/tuiterm.exp shows a few problems:
...
$ tclint --ignore line-length gdb/testsuite/lib/tuiterm.exp
tuiterm.exp:105:3: expression with substitutions should be enclosed by \
braces [unbraced-expr]
tuiterm.exp:1576:28: unnecessary command substitution within expression \
[redundant-expr]
tuiterm.exp:1582:25: unnecessary command substitution within expression \
[redundant-expr]
...
Tom de Vries [Fri, 12 Sep 2025 13:04:50 +0000 (15:04 +0200)]
[gdb/testsuite, tclint] Fix gdb.tui
Running tclint on the test-cases in gdb.tui shows a few problems:
...
$ ( cd gdb/testsuite/gdb.tui; tclint --ignore line-length *.exp )
compact-source.exp:58:28: expression with substitutions should be enclosed by \
braces [unbraced-expr]
compact-source.exp:60:27: expression with substitutions should be enclosed by \
braces [unbraced-expr]
compact-source.exp:68:32: expression with substitutions should be enclosed by \
braces [unbraced-expr]
empty.exp:68:2: eval received an argument with a substitution, unable to \
parse its arguments [command-args]
new-layout.exp:84:2: eval received an argument with a substitution, unable \
to parse its arguments [command-args]
source-search.exp:33:25: expression with substitutions should be enclosed by \
braces [unbraced-expr]
wrap-line.exp:40:21: expression with substitutions should be enclosed by \
braces [unbraced-expr]
wrap-line.exp:44:14: expression with substitutions should be enclosed by \
braces [unbraced-expr]
wrap-line.exp:62:40: expression with substitutions should be enclosed by \
braces [unbraced-expr]
...
Andrew Burgess [Thu, 23 Nov 2023 18:46:54 +0000 (18:46 +0000)]
gdb/gdbserver: pass inferior arguments as a single string
GDB holds the inferior arguments as a single string. Currently when
GDB needs to pass the inferior arguments to a remote target as part of
a vRun packet, this is done by splitting the single argument string
into its component arguments by calling gdb::remote_args::split, which
uses the gdb_argv class to split the arguments for us.
The same gdb_argv class is used when the user has asked GDB/gdbserver
to start the inferior without first invoking a shell; the gdb_argv
class is used to split the argument string into it component
arguments, and each is passed as a separate argument to the execve
call which spawns the inferior.
There is however, a problem with using gdb_argv to split the arguments
before passing them to a remote target. To understand this problem we
must first understand how gdb_argv is used when invoking an inferior
without a shell.
And to understand how gdb_argv is used to start an inferior without a
shell, I feel we need to first look at an example of starting an
inferior with a shell.
Consider these two cases:
(a) (gdb) set args \$VAR
(b) (gdb) set args $VAR
When starting with a shell, in case (a) the user expects the inferior
to receive a literal '$VAR' string as an argument, while in case (b)
the user expects to see the shell expanded value of the variable $VAR.
If the user does 'set startup-with-shell off', then in (a) GDB will
strip the '\' while splitting the arguments, and the inferior will be
passed a literal '$VAR'. In (b) there is no '\' to strip, so also in
this case the inferior will receive a literal '$VAR', remember
startup-with-shell is off, so there is no shell that can ever expand
$VAR.
Notice, that when startup-with-shell is off, we end up with a many to
one mapping, both (a) and (b) result in the literal string $VAR being
passed to the inferior. I think this is the correct behaviour in this
case.
However, as we use gdb_argv to split the remote arguments we have the
same many to one mapping within the vRun packet. But the vRun packet
will be used when startup-with-shell is both on and off. What this
means is that when gdbserver receives a vRun packet containing '$VAR'
it doesn't know if GDB actually had '$VAR', or if GDB had '\$VAR'.
And this is a huge problem.
We can address this by making the argument splitting for remote
targets smarter, and I do have patches that try to do this in this
series:
That series was pretty long, and wasn't getting reviewed, so I'm
pulling the individual patches out and posting them separately.
This patch doesn't try to improve remote argument splitting. I think
that splitting and then joining the arguments is a mistake which can
only introduce problems. The patch in the above series which tries to
make the splitting and joining "smarter" handles unquoted, single
quoted, and double quoted strings. But that doesn't really address
parameter substitution, command substitution, or arithmetic expansion.
And even if we did try to address these cases, what rules exactly
would we implement? Probably POSIX shell rules, but what if the
remote target doesn't have a POSIX shell? The only reason we're
talking about which shell rules to follow is because the splitting and
joining logic needs to mirror those rules. If we stop splitting and
joining then we no longer need to care about the target's shell.
Clearly, for backward compatibility we need to maintain some degree of
argument splitting and joining as we currently have; and that's why I
have a later patch (see the series above) that tries to improve that
splitting and joining a little. But I think, what we should really
do, is add a new feature flag (as used by the qSupported packet) and,
if GDB and the remote target agree, we should pass the inferior
arguments as a single string.
This solves all our problems. In the startup with shell case, we no
longer need to worry about splitting at all. The arguments are passed
unmodified to the remote target, that can then pass the arguments to
the shell directly.
In the 'startup-with-shell off' case it is now up to the remote target
to split the arguments, though in gdbserver we already did this, so
nothing really changes in this case. And if the remote target doesn't
have a POSIX shell, well GDB just doesn't need to worry about it!
Something similar to this was originally suggested in this series:
though this series didn't try to maintain backward compatibility,
which I think is an issue that my patch solves. Additionally, this
series only passed the arguments as a single string in some cases,
I've simplified this so that, when GDB and the remote agree, the
arguments are always passed as a single string. I think this is a
little cleaner.
I've also added documentation and some tests with this commit,
including ensuring that we test both the new single string approach,
and the fallback split/join approach.
I've credited the author of the referenced series as co-author as they
did come to a similar conclusion, though I think my implementation is
different enough that I'm happy to list myself as primary author.
I have changed things slightly from the original series. I think this
work is close enough that I've left the original author (Michael) in
place and added myself as co-author. Any bugs introduced by my
modifications to the original patch should be considered mine. I've
also added documentation and tests which were missing from the
originally proposed patch.
When the startup-with-shell option is enabled, arguments passed
directly as 'gdb --args <args>' or 'gdbserver <args>', are by default
escaped so that they are passed to the inferior as passed on the
command line, no globbing or variable substitution happens within the
shell GDB uses to start the inferior.
Use construct_inferior_arguments which handles special chars
Only arguments set via 'set args <args>', 'run <args>', or through the
Python API are not escaped in standard upstream GDB right now.
For the 'gdb --args' case, directly setting unescaped args on gdb
invocation is possible e.g. by using the "--eval-command='set args
<args>'", while this possibility does not exist for gdbserver.
This commit adds a new '--no-escape-args' command line option for GDB
and gdbserver. This option is used with GDB as a replacement for the
current '--args' option, and for gdbserver this new option is a flag
which changes how gdbserver handles inferior arguments on the command
line. When '--no-escape-args' is used inferior arguments passed on
the command line will not have escaping added by GDB or gdbserver.
For gdbserver, using this new option allows having the behaviour from
before commit bea571ebd78ee29cb94adf648fbcda1e109e1be6, while keeping
the default behaviour unified between GDB and GDBserver.
For GDB the --no-escape-args option can be used as a replacement for
--args, like this:
libsframe: testsuite: Fix testsuite build on Solaris [PR33168]
As reported in PR libsframe/33168, the libsframe tests don't build on
Solaris. The failure is
In file included from libsframe/testsuite/libsframe.decode/be-flipping.c:28:
/usr/include/dejagnu.h:48:1: error: conflicting types for ‘wait’; have ‘void(void)’
48 | wait (void)
| ^~~~
In file included from /usr/include/stdlib.h:16,
from libsframe/testsuite/libsframe.decode/be-flipping.c:21:
/usr/include/sys/wait.h:85:14: note: previous declaration of ‘wait’ with type ‘pid_t(int *)’ {aka ‘long int(int *)’}
85 | extern pid_t wait(int *);
| ^~~~
We have a combination of two factors here:
* Solaris <stdlib.h> has
and configure.ac predefines __EXTENSIONS__ due to the use of
AC_USE_SYSTEM_EXTENSIONS.
* This conflicts with <dejagnu.h>'s definition
void
wait (void)
{
...
}
While this version of wait was removed in upstream DejaGnu, the removal
only happened after the latest release, 1.6.3.
To avoid this, I've moved all testsuite includes into a new
sframe-test.h, adding a workaround for the wait conflict.
-Wall and -I$(srcdir) have been removed from AM_CPPFLAGS since they
don't seem to be needed. To fix the Makefile fragment duplication, the
local.mk files now use $(testsuite_LDADD) and $(testsuite_CPPFLAGS)
throughout.
Tested on {i386,amd64}-pc-solaris2.11, {sparc,sparcv9}-sun-solaris2.11,
{x86_64,i686}-pc-linux-gnu, and amd64-pc-freebsd14.0.
Coauthored-By: Alan Modra <amodra@gmail.com>
2025-08-31 Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
Alan Modra <amodra@gmail.com>
Tom de Vries [Fri, 12 Sep 2025 04:59:57 +0000 (06:59 +0200)]
[gdb/testsuite, tclint] Fix gdb.testsuite
Running tclint on the test-cases in gdb.testsuite shows a few problems:
...
board-sanity.exp:47:38: line has trailing whitespace [trailing-whitespace]
board-sanity.exp:83:1: line has trailing whitespace [trailing-whitespace]
board-sanity.exp:95:38: line has trailing whitespace [trailing-whitespace]
gdb-caching-proc-consistency.exp:66:2: expected braced word or word without \
substitutions in argument interpreted as expr [command-args]
gdb-caching-proc-consistency.exp:117:12: eval received an argument with a \
substitution, unable to parse its arguments [command-args]
with-override.exp:53:18: expression with substitutions should be enclosed by \
braces [unbraced-expr]
with-override.exp:57:18: expression with substitutions should be enclosed by \
braces [unbraced-expr]
...
Nelson Chu [Fri, 12 Sep 2025 04:24:19 +0000 (12:24 +0800)]
RISC-V: Also, fixed more ld testcases for --with-arch and --with-abi
Well these testcases cannot be fixed by .option norvc simply, that is because
current linker needs to check mapping symbols before doing any rvc relaxations,
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/393
Once we support the above features, we can revert this patch.
Tom Tromey [Wed, 27 Aug 2025 19:30:46 +0000 (13:30 -0600)]
Add Ada test case with long array indices
This patch adds a test case to test that the previous two patches did
their job.
With the current gdb, this test fails:
(gdb) print some_regular_access.all
Value out of range.
The bug here is that the array has an index type that is wider than
'int', which is perfectly acceptable in Ada.
Note that this series doesn't quite go far enough: in Ada the index
could be a 128-bit integer. This change would be more invasive; and
in practice this doesn't really seem to come up much -- so I've
deferred it.
Tom Tromey [Wed, 27 Aug 2025 19:08:13 +0000 (13:08 -0600)]
Use LONGEST rather than int for array slices
This patch started by removing the remaining calls to longest_to_int
from ada-lang.c, then chasing down the callees to make sure they were
also using LONGEST. This ended up with a small change to value_slice
as well.
Tom Tromey [Wed, 27 Aug 2025 18:56:02 +0000 (12:56 -0600)]
Remove some uses of longest_to_int from ada-lang.c
A few spots in ada-lang.c use longest_to_int -- but in a context where
the value is immediately passed to a function accepting LONGEST. This
patch removes the offending calls. It turned out to be easy to change
find_struct_field as well, so I've included that in this patch.
Tom de Vries [Thu, 11 Sep 2025 16:31:37 +0000 (18:31 +0200)]
[gdb/testsuite, tclint] Drop lreverse
When running tclint with lib/future.exp, I get:
...
$ tclint lib/future.exp
$exp:756:5: redefinition of built-in command 'lreverse' [redefined-builtin]
...
The code was added to handle pre-7.5 tcl versions without lreverse.
Since we now require Tcl 8.6.2 (as per PR testsuite/33205), drop this.
Tested by rerunning tclint.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
PR testsuite/33403
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33403
Tom de Vries [Thu, 11 Sep 2025 16:31:37 +0000 (18:31 +0200)]
[gdb/testsuite, tclint] Fix syntax error in gdb.base/dtrace-probe.exp
When running tclint with gdb.base/dtrace-probe.exp I get:
...
$ tclint gdb.base/dtrace-probe.exp
$exp:67:45: syntax error: expected newline or semicolon, got ]
...
due to these lines:
...
67 runto "-probe-dtrace test:two-locations"]
68 runto "-probe-dtrace test:two-locations"]
...
Fix this by dropping the trailing ']'.
Tested by rerunning tclint.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
PR testsuite/33403
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33403
Copy non_got_ref_without_indirect_extern_access when copying indirect
symbol for weak alias so that _bfd_x86_elf_adjust_dynamic_symbol will
properly handle GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS.
testsuite: RISC-V: Add '.option norvc' to ensure consistent results.
Add `.option norvc` to several RISC-V tests to avoid compressed
instruction generation. This ensures consistent disassembly and
alignment behavior regardless of assembler default options.
Kevin Buettner [Fri, 11 Jul 2025 23:18:13 +0000 (16:18 -0700)]
Fix unwinding when restoring a register from one of a greater size
When debugging functions where a callee-saved register is moved to a
register of a larger size (e.g., a 64-bit general-purpose register to
a 128-bit vector register), GDB would crash when the user issued the
"return" command. For example:
ldgr %f0, %r11 ; Move 64-bit general-purpose register (r11)
; to 128-bit vector register (f0)
.cfi_register r11, f0 ; DW_CFA_register: r11 is stored in f0
...
lgdr %r11, %f0 ; Restore r11 from f0
.cfi_restore r11 ; DW_CFA_restore: r11 is restored to its original
; register
(This example uses instructions and registers for the S390x architecture,
where this bug was originally found.)
If GDB is stopped in the "..." section and the user issues the
"return" command, GDB crashes due to a buffer size mismatch during
unwinding. Specifically, in frame_register_unwind in frame.c, a
buffer the size of the original register (the 64-bit r11 in this
example) has been allocated and GDB would like to use memcpy to copy
the contents of the register where the original register was saved
(the 128-bit f0) to the buffer for the original register. But,
fortunately, GDB has an assertion which prevents this from happening:
This patch ensures that GDB uses the original register's type (e.g.,
r11's type) when unwinding, even if it was marked as saved to a differently
typed/sized register (e.g., f0) via .cfi_register (DW_CFA_register).
The fix adds a 'struct type *' parameter to value_of_register_lazy() to
explicitly track the original register's type. The function
frame_unwind_got_register is updated to pass the correct type for the
original register.
The call chain from frame_register_unwind to frame_unwind_got_register
is shown by this backtrace:
#0 frame_unwind_got_register (frame=..., regnum=13, new_regnum=128)
at gdb/frame-unwind.c:300
#1 0x000000000135d894 in dwarf2_frame_prev_register (this_frame=...,
this_cache=0x2204528, regnum=13)
at gdb/dwarf2/frame.c:1187
#2 0x00000000014d9186 in frame_unwind_legacy::prev_register (
this=0x211f428 <dwarf2_frame_unwind>, this_frame=...,
this_prologue_cache=0x2204528, regnum=13) at gdb/frame-unwind.c:401
#3 0x00000000014e1d12 in frame_unwind_register_value (next_frame=...,
regnum=13) at gdb/frame.c:1263
#4 0x00000000014e16b8 in frame_register_unwind (next_frame=..., regnum=13,
optimizedp=0x3ffffff813c, unavailablep=0x3ffffff8138,
lvalp=0x3ffffff8134, addrp=0x3ffffff8128, realnump=0x3ffffff8124,
buffer=...) at gdb/frame.c:1189
The register numbers shown above are for s390x. On s390x,
S390_R11_REGNUM has value 13. Vector registers (like f0) are numbered
differently from floating-point registers of the same name, leading to
regnum 128 for f0 despite S390_F0_REGNUM being assigned a different
value in s390-tdep.h.
New test cases for aarch64 and x86_64 check for this on more popular
architectures and also without dependency on a particular compiler to
generate an unusual prologue in which a general purpose register is
being moved to a vector register. In both cases, the test simulates
the bug found on s390x where a 64-bit frame pointer was being moved to
a much wider vector register. These test cases will cause an internal
error on their respective architecture, but will pass with this fix in
place.
When tested on s390x linux (native), this change fixes 59 GDB internal
errors and around 200 failures overall. This is the list of internal
errors that no longer occur on s390x:
I have tested this commit on Fedora Linux, with architectures s390x,
x86_64, x86_64/-m32, aarch64, ppc64le, and riscv64, with no
regressions found.
This v2 version makes some changes suggested by Andrew Burgess: It
adds an assert to frame_unwind_got_register() and always passes the
type of REGNUM to value_of_register_lazy(). It also updates value.h's
comment describing value_of_register_lazy().
In his approval message, Andrew requested some changes to the tests.
Those have been made exactly as requested.
Tom Tromey [Thu, 24 Apr 2025 21:24:52 +0000 (15:24 -0600)]
Rename expand_symtabs_matching
After this series, expand_symtabs_matching is now misnamed. This
patch renames it, renames some associated types, and also fixes up
some comments that I previously missed.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 11 Feb 2025 20:39:16 +0000 (13:39 -0700)]
Remove enter_symbol_lookup
The "enter_symbol_lookup" class was introduced to work around the lack
of reentrancy in symbol lookup. There were two problems here:
1. The DWARF reader kept a mark bit on the dwarf2_per_cu_data object.
This bit is gone now, replaced with a local mark vector.
2. Some spots in gdb first examined the expanded symbol tables, and
then on failure expanded some symtabs and searched the newly
expanded ones (skipping previousy-expanded ones). Fixing this has
been the main point of this series.
Now that both of these barriers are gone, I think enter_symbol_lookup
can be removed.
One proof of this idea is that, without the first fix mentioned above,
py-symbol.exp regressed because gdbpy_lookup_static_symbols did not
first ensure that the current language was set -- i.e., there was a
latent bug in the enter_symbol_lookup patch anyway.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Thu, 2 Jan 2025 22:28:18 +0000 (15:28 -0700)]
Convert lookup_symbol_in_objfile
This converts lookup_symbol_in_objfile to the callback approach by
removing the call to lookup_symbol_in_objfile_symtabs. (The latter is
not removed as there are still other callers.)
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998 Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 31 Dec 2024 20:30:18 +0000 (13:30 -0700)]
Convert lookup_symbol_via_quick_fns
This converts lookup_symbol_via_quick_fns to the callback approach,
merging the search loop and the call to expand_symtabs_matching.
Note that this changes lookup_symbol_via_quick_fns to use a
best_symbol_tracker. Before this patch there was a discrepancy here
between the two search functions.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998 Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Thu, 2 Jan 2025 22:21:46 +0000 (15:21 -0700)]
Add best_symbol_tracker
This adds a new best_symbol_tracker struct. This is used to implement
the "best symbol" logic that is used sometimes in symtab.c. This
approach makes it simpler and more efficient to track the "best"
symbol when searching across multiple blocks.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Thu, 2 Jan 2025 22:17:25 +0000 (15:17 -0700)]
Simplify block_lookup_symbol
One loop in block_lookup_symbol is identical to the code in
block_lookup_symbol_primary. This patch simplifies the former by
having it call the latter.
This removes an assert. However, note that the assert is not needed
-- it does not check any invariant that must be maintained.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Thu, 2 Jan 2025 22:15:32 +0000 (15:15 -0700)]
Pass lookup_name_info to block_lookup_symbol_primary
This changes block_lookup_symbol_primary to accept a lookup_name_info.
This follows the general trend of hoisting these objects to the
outermost layer where they make sense -- somewhat reducing the cost of
using them.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 31 Dec 2024 20:11:50 +0000 (13:11 -0700)]
Remove objfile::expand_symtabs_for_function
objfile::expand_symtabs_for_function only has a single caller now, so
it can be removed. This also allows us to merge the expansion and
searching phases, as done in other patches in this series.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998 Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 31 Dec 2024 17:30:15 +0000 (10:30 -0700)]
Simplify basic_lookup_transparent_type
This patch changes basic_lookup_transparent_type to always work via
the "quick" API -- that is, no separate search of the already-expanded
symtabs is needed.
This is more efficient when many CUs have already been expanded. It
also makes the lookup more consistent, as the result is no longer
dependent on the order in which CUs were previously expanded.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998 Acked-By: Simon Marchi <simon.marchi@efficios.com>
This converts ada_language_defn::collect_symbol_completion_matches to
the callback approach, merging the search loop and the call to
expand_symtabs_matching.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998 Acked-By: Simon Marchi <simon.marchi@efficios.com>
This converts default_collect_symbol_completion_matches_break_on to
the callback approach, merging the search loop and the call to
expand_symtabs_matching.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998 Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Fri, 7 Mar 2025 01:00:37 +0000 (18:00 -0700)]
Rewrite the .gdb_index reader
This patch rewrites the .gdb_index reader to create the same data
structures that are created by the cooked indexer and the .debug_names
reader.
This is done in support of this series; but also because, from what I
can tell, the "templates.exp" change didn't really work properly with
this reader.
In addition to fixing that problem, this patch removes a lot of code.
Implementing this required a couple of hacks, as .gdb_index does not
contain all the information that's used by the cooked index
implementation.
* The index-searching code likes to differentiate between the various
DWARF tags when matching, but .gdb_index lumps many things into a
single "other" category. To handle this, we introduce a phony tag
that's used so that the match method can match on multiple domains.
* Similarly, .gdb_index doesn't distinguish between the type and
struct domains, so another phony tag is used for this.
* The reader must attempt to guess the language of various symbols.
This is somewhat finicky. "Plain" (unqualified) symbols are marked
as language_unknown and then a couple of hacks are used to handle
these -- one in expand_symtabs_matching and another when recognizing
"main".
For what it's worth, I consider .gdb_index to be near the end of its
life. While .debug_names is not perfect -- we found a number of bugs
in the standard while implementing it -- it is better than .gdb_index
and also better documented.
After this patch, we could conceivably remove dwarf_scanner_base.
However, I have not done this.
Finally, this patch also changes this reader to dump the content of
the index, as the other DWARF readers do. This can be handy when
debugging gdb.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33316
Tom Tromey [Sat, 7 Dec 2024 23:26:06 +0000 (16:26 -0700)]
Have expand_symtabs_matching work for already-expanded CUs
Currently, gdb will search the already-expanded symtabs in one loop,
and then also expand matching symtabs in another loop. However, this
is somewhat inefficient -- when searching the already-expanded
symtabs, all such symtabs are examined. However, the various "quick"
implementations already know which subset of symtabs might have a
match.
This changes the contract of expand_symtabs_matching to also call the
callback for an already-expanded symtab. With this change, and some
subsequent enabling changes, the number of searched symtabs should
sometimes be reduced. This also cuts down on the amount of redundant
code.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30736 Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Sat, 7 Dec 2024 22:51:24 +0000 (15:51 -0700)]
Remove dwarf2_per_cu_data::mark
This removes dwarf2_per_cu_data::mark, replacing it with a
locally-allocated boolean vector. It also inverts the sense of the
flag -- now, the flag is true when a CU should be skipped, and false
when the CU should be further examined. Also, the validity of the
flag is no longer dependent on 'file_matcher != NULL'.
This patch makes the subsequent patch to searching a bit simpler, so
I've separated it out.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998 Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Sun, 19 Jan 2025 01:15:21 +0000 (18:15 -0700)]
Entries from anon-struct.exp not in cooked index
g++ will sometimes use a typedef to give a name to an otherwise
anonymous type for linkage purposes. gdb tries to handle this odd
scenario, which is enforced by anon-struct.exp.
It's difficult to detect this problem in the current tree, but the
cooked index does not include an entry for these DIEs.
This patch changes gdb to add these to the index. This is needed by
subsequent changes in this series.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32519 Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Fri, 24 Jan 2025 01:56:51 +0000 (18:56 -0700)]
Restore "ingestion" of .debug_str when writing .debug_names
When I rewrote the .debug_names writer (commit 91a42a61), I changed
the writer to not import .debug_str into the debug_str_lookup object.
However, a later patch in this series needed this again. The issue
here was that if a name occurs in the DWARF, and is also allocated,
then there is a race, where the created index depends on which DIE is
read first. This can cause index-file.exp failures.
This patch restores the old approach, avoiding this problem. I also
applied a couple of small cleanups to the class. And, I removed the
old complaint from the "ingestion" function, as this was not
necessary.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Mon, 20 Jan 2025 18:06:21 +0000 (11:06 -0700)]
Put all CTF symbols in global scope
The new approach to searching (solely via the quick API) is more
sensitive to discrepancies between the partial and full readers. In
CTF, there is some disagreement about which scope to use. CTF doesn't
seem to really distinguish between the file and global scope, so this
patch takes the simple approach of putting all CTF symbols into the
global scope.
This changes one test as well. It seems to me that the behavior here
is arbitrary and the test is making unwarranted assumptions.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Sun, 19 Jan 2025 23:53:33 +0000 (16:53 -0700)]
Fix index's handling of DW_TAG_imported_declaration
Currently the full symbol reader puts DW_TAG_imported_declaration in
TYPE_DOMAIN, in the global scope. This patch changes the cooked
indexer to follow.
Without this patch, a later patch in the series would cause
nsalias.exp to regress.
This also updates read-gdb-index.c to do something similar.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Thu, 2 Jan 2025 20:40:27 +0000 (13:40 -0700)]
Ada import functions not in index
The cooked index does not currently contain entries for Ada import
functions. This means that whether or not these are visible to
"break" depends on which CUs were previously expanded -- clearly a
bug.
This patch fixes the issue. I think the comments in the patch explain
the fix reasonably well.
Perhaps one to-do item here is to change GNAT to use
DW_TAG_imported_declaration for these imports. This may eventually
let us remove some of the current hacks.
This version includes a fix from Simon to initialize the new member.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32511 Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 11 Feb 2025 18:48:09 +0000 (11:48 -0700)]
Emit some type declarations in .gdb_index
If you run struct-decl.exp with the .gdb_index board, you will see
that "the_type" is not emitted in the index. This would cause a
failure in this series. The fix is to ensure that certain necessary
type declarations are emitted.
However, a naive fix here will regress stub-array-size.exp, where a
type declaration and a type definition are both seen -- but the
declaration is seen first and causes a failure. This is handled by
adding some code (including a mild hack) to filter out type
declarations when a corresponding type definition is seen.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Sat, 22 Feb 2025 21:07:57 +0000 (14:07 -0700)]
Change ada_decode to preserve upper-case in some situations
This patch is needed to avoid regressions later in the series.
The issue here is that ada_decode, when called with wide=false, would
act as though the input needed verbatim quoting. That would happen
because the 'W' character would be passed through; and then a later
loop would reject the result due to that character.
Similarly, with operators=false the upper-case-checking loop would be
skipped, but then some names that did need verbatim quoting would pass
through.
Furthermore I noticed that there isn't a need to distinguish between
the "wide" and "operators" cases -- all callers pass identical values
to both.
This patch cleans up the above, consolidating the parameters and
changing how upper-case detection is handled, so that both the
operator and wide cases pass-through without issue. I've added new
unit tests for this.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Mon, 10 Mar 2025 21:52:17 +0000 (15:52 -0600)]
Add another minor hack to cooked_index_entry::full_name
This patch adds another minor hack to cooked_index_entry::full_name.
In particular, if GNAT emits non-hierarchical names (still the default
as the hierarchical series is blocked on one tricky problem), then a
request to compute the "linkage-style" name will now just return the
'name' field.
Without this tweak, this series would regress ada-cold-name.exp,
because the search would look for "name.cold" but the index would
return "name[cold]" as the "linkage" name (which would be wrong).
This area is a bit difficult to unravel. The best plan here, IMO, is
to change Ada to work like the other languages in gdb: store the
natural name and do searches with that name. I think this is
achievable, but I didn't want to try it here.
I've updated the relevant bug (tagged below) to reflect this.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32766 Acked-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Sat, 26 Jul 2025 21:28:34 +0000 (15:28 -0600)]
Skip some tests with "readnow" board
This series pointed out a few tests that check that a particular index
is in use. It seems to me that this does not really make sense when
the "readnow" board is in use, as this actually skips index creation.
The tests do pass today, but by accident. This patch adds the
appropriate "require" line to the tests in question.
Approved-By: Simon Marchi <simon.marchi@efficios.com> Acked-By: Simon Marchi <simon.marchi@efficios.com>
gas: sframe: skip DW_CFA_GNU_args_size when safe to ignore
Currently, gas warns and skips generating SFrame FDE when it sees:
.cfi_escape 0x2e,XX
From the documentation of DW_CFA_GNU_args_size:
"The DW_CFA_GNU_args_size instruction takes an unsigned LEB128 operand
representing an argument size. This instruction specifies the total of
the size of the arguments which have been pushed onto the stack."
With origins seemingly for VAX architecture, the usage of
DW_CFA_GNU_args_size seems to have evolved. The purpose of
DW_CFA_GNU_args_size is to adjust SP when performing stack unwinding for
exception handling.
For the purpose of stack tracing using SFrame, DW_CFA_GNU_args_size is
safe to skip, especially when the CFA restoration is known to be FP
based. A previous summary of the reasoning and intent was indicated
here [1].
R_X86_64_TPOFF32 relocation of local-exec TLS model can only be used in
executable, not in a shared library, even if the source code is compiled
with -fPIC. Change the linker error message from
relocation R_X86_64_TPOFF32 against symbol `foo' can not be used when making a shared object; recompile with -fPIC
to
relocation R_X86_64_TPOFF32 against symbol `foo' can not be used when making a shared object; replace local-exec with initial-exec TLS model
bfd/
PR ld/33408
* elf64-x86-64.c (elf_x86_64_need_pic): Suggest "replace
local-exec with initial-exec TLS model" for R_X86_64_TPOFF32.
(elf_x86_64_scan_relocs): Drop ABI_64_P check for
R_X86_64_TPOFF32.
Matthieu Longo [Thu, 4 Sep 2025 14:25:21 +0000 (15:25 +0100)]
ld: fix segfault when linker script is not found
ld previously crashed with a segmentation fault if the specified linker
script could not be found. The issue seems to have been introduced
recently by d048eee2910 [1].
This patch adds a check to ensure that a filename was found after
searching the possible prefixes. If no filename was found, the function
returns NULL, and ldfile_open_command_file_1() emits a proper fatal
error message.
This change prevents the crash and provides a clear diagnostic.
A new generic test was also added to cover this error case.
Tom Tromey [Sat, 23 Aug 2025 17:51:03 +0000 (11:51 -0600)]
Change type_stack::insert to take gdbarch
This changes type_stack::insert to take a gdbarch rather than an
expr_builder. This is simpler and means that type_stack doesn't have
to know anything about expression building; the parser-defs.h include
can be removed.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Sat, 23 Aug 2025 17:46:48 +0000 (11:46 -0600)]
Make type_stack popping a bit safer
This changes type_stack so that an element that has an argument can't
be popped in isolation. The idea is to make type stack use a little
safer, making it so that the stack can't end up in an invalid state.
This also fixes up a few related comments.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Sat, 23 Aug 2025 17:39:35 +0000 (11:39 -0600)]
Make type_stack pushing a bit safer
This changes type_stack to make pushing elements a bit safer: if an
element requires an argument, these are now always pushed at the same
time, rather than separately.
This patch also adds a few comments to help document a bit better.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Alan Modra [Tue, 9 Sep 2025 10:45:44 +0000 (20:15 +0930)]
readelf: tidy dump_relr_relocations
A comment in display_relocations said "RELRS has been freed by
dump_relr_relocations". Except that hadn't happened on all return
paths. Tidy that by freeing relrs allocated in dump_relr_relocations
in that function, and relrs allocated in display_relocation in that
function.
* readelf.c (dump_relr_relocations): Only free relrs allocated
in this function.
(display_relocations): Free relrs here, on error return paths
too.
Tom Tromey [Mon, 4 Aug 2025 16:39:02 +0000 (10:39 -0600)]
Use gnulib c-ctype module in gdb
PR ada/33217 points out that gdb incorrectly calls the <ctype.h>
functions. In particular, gdb feels free to pass a 'char' like:
char *str = ...;
... isdigit (*str)
This is incorrect as isdigit only accepts EOF and values that can be
represented as 'unsigned char' -- that is, a cast is needed here to
avoid undefined behavior when 'char' is signed and a character in the
string might be sign-extended. (As an aside, I think this API seems
obviously bad, but unfortunately this is what the standard says, and
some systems check this.)
Rather than adding casts everywhere, this changes all the code in gdb
that uses any <ctype.h> API to instead call the corresponding c-ctype
function.
Now, c-ctype has some limitations compared to <ctype.h>. It works as
if the C locale is in effect, so in theory some non-ASCII characters
may be misclassified. This would only affect a subset of character
sets, though, and in most places I think ASCII is sufficient -- for
example the many places in gdb that check for whitespace.
Furthermore, in practice most users are using UTF-8-based locales,
where these functions aren't really informative for non-ASCII
characters anyway; see the existing workarounds in gdb/c-support.h.
Note that safe-ctype.h cannot be used because it causes conflicts with
readline.h. And, we canot poison the <ctype.h> identifiers as this
provokes errors from some libstdc++ headers.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33217 Approved-By: Simon Marchi <simon.marchi@efficios.com>