It is based on a misconception, that hidden types in the deduplicator
input should always be hidden in the output. For cu-mapped links,
and final links following cu-mapped links, this is not true: we want
to hide inputs if they were conflicting on the output and no more.
We will reintroduce the testcase once a better fix is found.
The mistake in the above patch is in how gdbpy_parse_command_name is
used. The BASE_LIST output argument from this function points to the
list of commands for the prefix, not to the prefix command itself.
So when gdbpy_parse_command_name is called for 'prefix-1 prefix-2',
BASE_LIST points to the list of commands associated with 'prefix-1',
not to the actual 'prefix-1' cmd_list_element.
Back in cmdpy_init, from where gdbpy_parse_command_name was called, I
was walking back from the first entry in BASE_LIST to figure out if
this was a "show" prefix command or not. However, if BASE_LIST is
empty then there is no first item, and this would trigger the
segfault.
The solution it to extend gdbpy_parse_command_name to also return the
prefix cmd_list_element in addition to the existing values. With this
done, and cmdpy_init updated, the segfault is now avoided.
There's a new test that would trigger the crash without the patch.
And, of course, the above commit also broke guile in the exact same
way. And the fix is exactly the same. And there's a guile test too.
NOTE: We should investigate possibly sharing some of this boiler plate
helper code between Python and Guile. But not in this commit.
Since commit d462550c91c ("gdb/testsuite: also compile foll-exec.exp as C++"),
we run into:
...
Running gdb.base/exec-invalid-sysroot.exp ...
gdb compile failed, foll-exec.c: In function 'main':
foll-exec.c:35:52: error: 'EXECD_PROG' undeclared (first use in this function)
printf ("foll-exec is about to execlp(%s)...\n", EXECD_PROG);
^~~~~~~~~~
foll-exec.c:35:52: note: each undeclared identifier is reported only once \
for each function it appears in
...
Fix this by default-defining EXECD_PROG to "execd-prog".
Rui Ueyama [Tue, 3 Jun 2025 02:16:02 +0000 (11:16 +0900)]
Reject compressed sections exceding 4 GiB on LLP64 machines
According to the zlib FAQ (*1), zlib does not support compressed data
larger than 4 GiB when the compiler's long type is 32 bits. Therefore,
we need to report an error if a zlib-compressed debug section exceeds
4 GiB on LLP64 machines.
Indu Bhagat [Wed, 4 Jun 2025 06:10:46 +0000 (23:10 -0700)]
sframe: fix PR libsframe/33051
Fix PR libsframe/Bug 33051 - ASAN: heap-buffer-overflow
../../src/libsframe/sframe.c:1054 in
sframe_get_funcdesc_with_addr_internal
The previous commit 9d2a24349e2 (libsframe: correct binary search for
SFrame FDE) adapted the binary search logic in
sframe_get_funcdesc_with_addr_internal. Adjusting the upper end of the
search index was missed.
The search must only be done for FDEs starting at index 0 and up until
num_fdes - 1. Prior logic of searching (before commit 9d2a24349e2) was
a bit different.
libsframe/
* sframe.c: Use the correct high index.
Andrew Burgess [Mon, 2 Jun 2025 13:58:51 +0000 (14:58 +0100)]
gdb/testsuite: also compile foll-exec.exp as C++
For a long time, Fedora GDB has carried a test that performs some
basic testing that GDB can handle 'catch exec' related commands for a
C++ executable.
The exact motivation for this test has been lost in the mists of time,
but looking at the test script, the concern seems to be that GDB would
have problems inserting C++ related internal breakpoints if a non C++
process is execd from a C++ one.
There's no actual GDB fix associated with the Fedora test. This
usually means that the issue was fixed upstream long ago. This patch
does seem to date from around 2010ish (or maybe earlier).
Having a look through the upstream tests, I cannot see anything that
covers this sort of thing (C++ to C exec calls), and I figure it
cannot hurt to have some additional testing in this area, and so I
wrote this patch.
I've taken the existing foll-exec.exp test, which compiles a C
executable and then execs a different C executable, and split it into
two copies.
We now have foll-exec-c.exp and foll-exec-c++.exp. These tests
compile a C and C++ executable respectively. Then within each of
these scripts both a C and C++ helper application is built, which can
then be execd from the main test executable.
And so, we now cover 4 cases, the initial executable can be C or C++,
and the execd process can be C or C++.
As expected, everything passes. This is just increasing test
coverage.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Guinevere Larsen [Wed, 12 Feb 2025 11:25:46 +0000 (08:25 -0300)]
gdb: Make dwarf support optional at compile time
This commit allows a user to enable or disable dwarf support at
compilation time. To do that, a new configure option is introduced, in
the form of --enable-gdb-dwarf-support (and the accompanying --disable
version). By default, dwarf support is enabled, so no behavior changes
occur if a user doesn't use the new feature. If support is disabled, no
.c files inside the dwarf2/ subfolder will be compiled into the final
binary.
To achieve this, this commit also introduces the new macro
DWARF_FORMAT_AVAILABLE, which guards the definitions of functions
exported from the dwarf reader. If the macro is not defined, there are a
couple behaviors that exported functions may have:
* no-ops: several functions are used to register things at
initialization time, like unwinders. These are turned into no-ops
because the user hasn't attempted to read DWARF yet, there's no point
in warning that DWARF is unavailable.
* warnings: similar to the previous commit, if dwarf would be read or
used, the funciton will emit the warning "No dwarf support available."
* throw exceptions: If the code that calls a function expects an
exceptin in case of errors, and has a try-catch block, an error with
the previous message is thrown.
I believe that the changed functions should probalby be moved to the
dwarf2/public.h header, but that require a much larger refactor, so it
is left as a future improvement.
Finally, the --enable-gdb-compile configure option has been slightly
changed, since compile requires dwarf support. If compile was requested
and dwarf was disabled, configure will throw an error. If the option was
not used, support will follow what was requested for dwarf (warning the
user of what is decided).
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-By: Tom Tromey <tom@tromey.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
This commit aims to allow a user to enable or disable mdebug support at
compilation time. To do that, a new configure option is added, called
--enable-gdb-mdebug-support (and the accompanying --disable version). By
default, support is enabled, and if a user decides to disable support,
the file mdebugread.c won't be compiled in the final binary, and the
macro MDEBUG_FORMAT_AVAILABLE won't be defined.
That macro is used to control the definitions of mdebug reading, either
the actual definition in mdebugread.c, or a static inline version that
only emits the following warning:
> No mdebug support available.
Ideally, we'd like to guard the entirity of mdebugread in the macro, but
the alpha-mdebug-tdep file uses those directly, and I don't think we
should restrict alpha hosts to requiring that debug format compiled in,
nor do I understand the tdep file enough to be comfortable disentangling
the requirements.
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-By: Andrew Burgess <aburgess@redhat.com>
Guinevere Larsen [Thu, 12 Dec 2024 12:37:36 +0000 (09:37 -0300)]
gdb: Use multiple minimal_symbol_readers in mipscoff_symfile_read
Currently, mipscoff_symfile_read uses a single minimal_symbol_reader to
get all minimal symbols from mdebug-formatted debuginfo, and from
alphacoff. This pattern has been around since minimal_symbol_reader has
been introduced, and from own research, there's no need to use the same
reader. This made it so mipscoff_symfile_read could call
mdebug_build_psymtabs directly, since the latter needs a reference to
the minsym reader object. The issue is that future commits need a
unified entrance point to read debuginfo, and this pattern is very
different to how elf does mdebug reading.
In fact, the elf mdebug reader does some preparatory steps and then
calls mdebug_build_psymtabs, so what the mips version does is just
spread these preparatory steps through the mipscoff function instead.
To make it easier for future commits to query debuginfo support
dynamically (as opposed to assuming things at compile time), this commit
introduces a new mipsmdebug_build_psymtabs function, which does similar
preparatory steps as elfmdebug_build_psymtabs. It is added to
mdebugread.c to help with maintaining a separation between reading an
objfile (in mipsread.c) and its debuginfo (mdebug), so that in the
future we have an easier time selectively disabling debuginfo formats
at compilation time. This should have no visible changes for the end
user.
The new function must receive the pointers to ecoff_debug_swap and
ecoff_debug_info because finding those structures based on the bfd
object necessitates including the headers libcoff.h and libecoff.h,
and those headers can't be included at the same time as libaout.h
- which mdebugread.c already uses.
Andrew Burgess [Sat, 12 Apr 2025 08:15:53 +0000 (09:15 +0100)]
gdb/python/guile: user created prefix commands get help list
Consider GDB's builtin prefix set/show prefix sub-commands, if they
are invoked with no sub-command name then they work like this:
(gdb) show print
print address: Printing of addresses is on.
print array: Pretty formatting of arrays is off.
print array-indexes: Printing of array indexes is off.
print asm-demangle: Demangling of C++/ObjC names in disassembly listings is off.
... cut lots of lines ...
(gdb) set print
List of set print subcommands:
set print address -- Set printing of addresses.
set print array -- Set pretty formatting of arrays.
set print array-indexes -- Set printing of array indexes.
set print asm-demangle -- Set demangling of C++/ObjC names in disassembly listings.
... cut lots of lines ...
Type "help set print" followed by set print subcommand name for full documentation.
Type "apropos word" to search for commands related to "word".
Type "apropos -v word" for full documentation of commands related to "word".
Command name abbreviations are allowed if unambiguous.
(gdb)
That is 'show print' lists the values of all settings under the
'print' prefix, and 'set print' lists the help text for all settings
under the 'set print' prefix.
Now, if we try to create something similar using the Python API:
(gdb) python gdb.ParameterPrefix("my-prefix", gdb.COMMAND_NONE)
(gdb) python gdb.Parameter("my-prefix foo", gdb.COMMAND_OBSCURE, gdb.PARAM_BOOLEAN)
(gdb) show my-prefix
(gdb) set my-prefix
Neither 'show my-prefix' or 'set my-prefix' gives us the same details
relating to the sub-commands that we get with the builtin prefix
commands.
This commit aims to address this.
Currently, in cmdpy_init, when a new command is created, we always set
the commands callback function to cmdpy_function. It is within
cmdpy_function that we spot that the command is a prefix command, and
that there is no gdb.Command.invoke method, and so return early.
This commit changes things so that the rules are now:
1. For NON prefix commands, we continue to use cmdpy_function.
2. For prefix commands that do have a gdb.Command.invoke
method (i.e. can handle unknown sub-commands), continue to use
cmdpy_function.
3. For all other prefix commands, don't use cmdpy_function, instead
use GDB's normal callback function for set/show prefixes.
This requires splitting the current call to add_prefix_cmd into either
a call to add_prefix_cmd, add_show_prefix_cmd, or
add_basic_prefix_cmd, as appropriate.
After these changes, we now see this:
(gdb) python gdb.ParameterPrefix("my-prefix", gdb.COMMAND_NONE) │
(gdb) python gdb.Parameter("my-prefix foo", gdb.COMMAND_OBSCURE, gdb.PARAM_BOOLEAN)
(gdb) show my-prefix │
my-prefix foo: The current value of 'my-prefix foo' is "off".
(gdb) set my-prefix
List of "set my-prefix" subcommands:
set my-prefix foo -- Set the current value of 'my-prefix foo'.
Type "help set my-prefix" followed by subcommand name for full documentation.
Type "apropos word" to search for commands related to "word".
Type "apropos -v word" for full documentation of commands related to "word".
Command name abbreviations are allowed if unambiguous.
(gdb)
Which matches how a prefix defined within GDB would act.
Tom Tromey [Tue, 3 Jun 2025 14:05:41 +0000 (08:05 -0600)]
Clean up comment in dw2-ranges-psym-warning.exp
This removes a trailing backslash from a comment in
dw2-ranges-psym-warning.exp. This backslash causes Emacs to try to
reindent the next line. This happens because comments are weird in
Tcl -- they are not exactly syntactic and the backslash still acts as
a line-continuation marker here.
Tom Tromey [Tue, 20 May 2025 16:45:07 +0000 (10:45 -0600)]
Handle dynamic DW_AT_data_bit_offset
In Ada, a field can have a dynamic bit offset in its enclosing record.
In DWARF 3, this was handled using a dynamic
DW_AT_data_member_location, combined with a DW_AT_bit_offset -- this
combination worked out ok because in practice GNAT only needs a
dynamic byte offset with a fixed offset within the byte.
However, this approach was deprecated in DWARF 4 and then removed in
DWARF 5. No replacement approach was given, meaning that in strict
mode there is no way to express this.
This is a DWARF bug, see
https://dwarfstd.org/issues/250501.1.html
In a discussion on the DWARF mailing list, a couple people mentioned
that compilers could use the obvious extension of a dynamic
DW_AT_data_bit_offset. I've implemented this for LLVM:
https://github.com/llvm/llvm-project/pull/141106
In preparation for that landing, this patch implements support for
this construct in gdb.
New in v2: renamed some constants and added a helper method, per
Simon's review.
New in v3: more renamings.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Andrew Burgess [Wed, 7 May 2025 11:58:41 +0000 (12:58 +0100)]
gdb: support zero inode in generate-core-file command
It is possible, when creating a shared memory segment (i.e. with
shmget), that the id of the segment will be zero.
When looking at the segment in /proc/PID/smaps, the inode field of the
entry holds the shared memory segment id.
And so, it can be the case that an entry (in the smaps file) will have
an inode of zero.
When GDB generates a core file, with the generate-core-file (or its
gcore alias) command, the shared memory segment should be written into
the core file.
Fedora GDB has, since 2008, carried a patch that tests this case.
There is no fix for GDB associated with the test, and unfortunately,
the motivation for the test has been lost to the mists of time. This
likely means that a fix was merged upstream without a suitable test,
but I've not been able to find and relevant commit. The test seems to
be checking that the shared memory segment with id zero, is being
written to the core file.
While looking at this test and trying to work out if it should be
posted upstream, I saw that GDB does appear to write the shared memory
segment into the core file (as expected), which is good. However, GDB
still isn't getting this case exactly right, there appears to be no
NT_FILE entry for the shared memory mapping if the mapping had an id
of zero.
In gcore_memory_sections (gcore.c) we call back into linux-tdep.c (via
the gdbarch_find_memory_regions call) to correctly write the shared
memory segment into the core file, however, in
linux_make_mappings_corefile_notes, when we use
linux_find_memory_regions_full to create the NT_FILE note, we call
back in to dump_note_entry_p for each mapping, and in here we reject
any mapping with a zero inode.
The result of this, is that, for a shared memory segment with a
non-zero id, after loading the core file, the shared memory segment
will appear in the 'proc info mappings' output. But, for a shared
memory segment with a zero id, the segment will not appear in the
'proc info mappings' output.
I initially tried just dropping the inode check in this function (see
previous commit 1e21c846c27, which I then reverted in commit 998165ba99a.
The problem with dropping the inode check is that the special kernel
mappings, e.g. '[vvar]' would now get a NT_FILE entry. In fact, any
special entry except '[vdso]' and '[vsyscall]' which are specifically
checked for in dump_note_entry_p would get a NT_FILE entry, which is
not correct.
So, instead, I propose that if the inode is zero, and the filename
starts with '[' and finished with ']' then we should not create a
NT_FILE entry. But otherwise a zero inode should not prevent a
NT_FILE entry being created.
The test for this change is a bit tricky. The original Fedora
test (mentioned above) has a loop that tries to grab the shared memory
mapping with id zero. This was, unfortunately, not very reliable.
I tried to make this more reliable by going multi-threaded, and
waiting for longer, see my proposal here:
But this was still not great. On further testing this was only
passing (i.e. managing to find the shared memory mapping with id zero)
about 60% of the time.
However, I realised that GDB finds the shared memory id by reading the
/proc/PID/smaps file. But we don't really _need_ the shared memory id
for anything, we just use the value (as an inode) to decide if the
segment should be included in the core file or not. The id isn't even
written to the core file. So, if we could intercept the read of the
smaps file, then maybe, we could lie to GDB, and tell it that the id
was zero, and then see how GDB handles this.
And luckily, we can do that using a preload library!
We already have a test that uses a preload library to modify GDB, see
gdb.threads/attach-slow-waitpid.exp.
So, I have created a new preload library. This one intercepts open,
open64, close, read, and pread. When GDB attempts to open
/proc/PID/smaps, the library spots this and loads the file contents
into a memory buffer. The buffer is then modified to change the id of
any shared memory mapping to zero. Any reads from this file are
served from the modified memory buffer.
I tested on x86-64, AArch64, PPC, s390, and ARM, all running various
versions of GNU/Linux. The requirement for open64() came from my ARM
testing. The other targets used plain open().
And so, the test is now simple. Start GDB with the preload library in
place, start the inferior and generate a core file. Then restart GDB,
load the core file, and check the shared memory mapping was included.
This test will fail with an unpatched GDB, and succeed with the patch
applied.
Piotr Rudnicki [Fri, 30 May 2025 13:21:49 +0000 (15:21 +0200)]
gdb: handle struct and union types in evaluate_subexp_for_address_base
Suppose a function returns a struct and a method of that struct is
called. E.g.:
struct S
{
int a;
int get () { return a; }
};
S f ()
{
S s;
s.a = 42;
return s;
}
...
int z = f().get();
...
GDB is able to evaluate the expression:
(gdb) print f().get()
$1 = 42
However, type-checking the expression fails:
(gdb) ptype f().get()
Attempt to take address of value not located in memory.
This happens because the `get` function takes an implicit `this`
pointer, which in this case is the value returned by `f()`, and GDB
wants to get an address for that value, as if passing the implicit
this pointer. However, during type-checking, the struct value
returned by `f()` is a `not_lval`.
A similar issue exists for union types, where methods called on
temporary union objects would fail type-checking in the same way.
Address the problems by handling `TYPE_CODE_STRUCT` and
`TYPE_CODE_UNION` in `evaluate_subexp_for_address_base`.
With this change, for struct's method call, we get
(gdb) ptype f().get()
type = int
Add new test cases to file gdb.cp/chained-calls.exp to test this change.
Tom de Vries [Tue, 3 Jun 2025 06:59:58 +0000 (08:59 +0200)]
[gdb/cli] Use captured per_command_time in worker threads
With test-case gdb.base/maint.exp, I ran into:
...
(gdb) file maint^M
Reading symbols from maint...^M
(gdb) mt set per-command on^M
(gdb) Time for "DWARF indexing worker": ...^M
Time for "DWARF indexing worker": ...^M
Time for "DWARF indexing worker": ...^M
Time for "DWARF indexing worker": ...^M
Time for "DWARF skeletonless type units": ...^M
Time for "DWARF add parent map": ...^M
Time for "DWARF finalize worker": ...^M
Time for "DWARF finalize worker": ...^M
Time for "DWARF finalize worker": ...^M
Time for "DWARF finalize worker": ...^M
Time for "DWARF finalize worker": ...^M
FAIL: $exp: warnings: per-command: mt set per-command on (timeout)
mt set per-command off^M
2025-05-31 09:33:44.711 - command started^M
(gdb) PASS: $exp: warnings: per-command: mt set per-command off
...
I didn't manage to reproduce this by rerunning the test-case, but it's fairly
easy to reproduce using a file with more debug info, for instance gdb:
...
$ gdb -q -batch -ex "file build/gdb/gdb" -ex "mt set per-command on"
...
Due to the default "mt dwarf synchronous" == off, the file command starts
building the cooked index in the background, and returns immediately without
waiting for the result.
The subsequent "mt set per-command on" implies "mt set per-command time on",
which switches on displaying of per-command execution time.
The "Time for" lines are the result of those two commands, but these lines
shouldn't be there because "mt per-command time" == off at the point of
issuing the file command.
Fix this by capturing the per_command_time variable, and using the captured
value instead.
Tested on x86_64-linux.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
PR cli/33039
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33039
Simon Marchi [Mon, 2 Jun 2025 20:31:12 +0000 (20:31 +0000)]
gdb/dwarf2: update call_site::target comment
This comment refers to the field location kind enum, even though call
sites were moved to their own enum in 7eb21cc70224 ("Change
call_site_target to use custom type and enum"). Update it.
Andrew Burgess [Mon, 19 May 2025 19:54:54 +0000 (20:54 +0100)]
gdb: use quoted filename completion for the shell command
With the quoted filename completion work that I did last year the
deprecated_filename_completer function will now only complete a single
word as a filename, for example:
(gdb) save breakpoints /tm<TAB>
The 'save breakpoints' command uses the deprecated_filename_completer
completion function. In the above '/tm' will complete to '/tmp/' as
expected. However, if you try this:
(gdb) save breakpoints /tmp/ /tm<TAB>
The second '/tm' will not complete for GDB 16.x, but will complete
with GDB 15.x as GDB 15.x is before my changes were merged.
What's actually happening here is that, before my changes, the
filename completion was breaking words on white space, so in the above
the first '/tmp/' and the second '/tm' are seen as separate words for
completion, the second word is therefore seen as the start of a new
filename.
After my changes, deprecated_filename_completer allows spaces to be
part of the filename, so in the above, GDB is actually trying to
complete a filename '/tmp/ /tm' which likely doesn't exist, and so
completion stops.
This change for how deprecated_filename_completer works makes sense,
commands like 'save breakpoints' take their complete command arguments
and treat it as a single filename, so given this:
(gdb) save breakpoints /tmp/ /tm<ENTER>
GDB really will try to save breakpoints to a file called '/tmp/ /tm',
weird as that may seem. How GDB interprets the command arguments
didn't change with my completion patches, I simply brought completion
into line with how GDB interprets the arguments.
The patches I'm talking about here are this set:
* 4076f962e8c gdb: split apart two different types of filename completion
* dc22ab49e9b gdb: deprecated filename_completer and associated functions
* 35036875913 gdb: improve escaping when completing filenames
* 1d1df753977 gdb: move display of completion results into completion_result class
* bbbfe4af4fb gdb: simplify completion_result::print_matches
* 2bebc9ee270 gdb: add match formatter mechanism for 'complete' command output
* f2f866c6ca8 gdb: apply escaping to filenames in 'complete' results
* 8f87fcb1daf gdb: improve gdb_rl_find_completion_word for quoted words
* 67b8e30af90 gdb: implement readline rl_directory_rewrite_hook callback
* 1be3b2e82f7 gdb: extend completion of quoted filenames to work in brkchars phase
* 9dedc2ac713 gdb: fix for completing a second filename for a command
* 4339a3ffc39 gdb: fix filename completion in the middle of a line
Bug PR gdb/32982 identifies a problem with the shell command;
completion broke between 15.x and 16.x. The shell command also uses
deprecated_filename_completer for completion. But consider a shell
command line:
(gdb) shell ls /tm<TAB>
The arguments to the shell command are 'ls /tm' at the point <TAB> is
pressed. Under the old 15.x completion GDB would split the words on
white space and then try to complete '/tm' as a filename.
Under the 16.x model, GDB completes all the arguments as a single
filename, that is 'ls /tm', which is unlikely to match any filenames,
and so completion fails.
The fix is to write a custom completion function for the shell_command
function (cli/cli-cmds.c), this custom completion function will skip
forward to find the last word in the arguments, and then try to
complete that, so in the above example, GDB will skip over 'ls ', and
then tries to complete '/tm', which is exactly what we want.
Given that the filenames passed to the shell command are forwarded to
an actual shell, I have switched over the new quoted filename
completion function for the shell command, this means that white space
within a filename will be escaped with a backslash by the completion
function, which is likely what the user wants, this means the filename
will arrive in the (actual) shell as a single word, rather than
splitting on white space and arriving as two words.
Tom Tromey [Thu, 13 Feb 2025 14:24:08 +0000 (07:24 -0700)]
Fix DAP defer_stop_events implementation
DAP requests have a "defer_stop_events" option that is intended to
defer the emission of any "stopped" event until after the current
request completes. This was needed to handle async continues like
"finish &".
However, I noticed that sometimes DAP tests can fail, because a stop
event does arrive before the response to the "stepOut" request. I've
only noticed this when the machine is fairly loaded -- for instance
when I'm regression-testing a series, it may occur in some of the
tests mid-series.
I believe the problem is that the implementation in the "request"
function is incorrect -- the flag is set when "request" is invoked,
but instead it must be deferred until the request itself is run. That
is, the setting must be captured in one of the wrapper functions.
Following up on this, Simon pointed out that introducing a delay
before sending a request's response will cause test case failures.
That is, there's a race here that is normally hidden.
Investigation showed that that deferred requests can't force event
deferral. This patch implements this; but more testing showed many
more race failures. Some of these are due to how the test suite is
written.
Anyway, in the end I took the radical approach of deferring all events
by default. Most DAP requests are asynchronous by nature, so this
seemed ok. The only case I found that really required this is
pause.exp, where the test (rightly) expects to see a 'continued' event
while performing an inferior function call.
I went through all events and all requests and tried to convince
myself that this patch will cause acceptable behavior in every case.
However, it's hard to be completely sure about this approach. Maybe
there are cases that do still need an event before the response, but
we just don't have tests for them.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32685 Acked-By: Simon Marchi <simon.marchi@efficios.com>
Patrick Monnerat [Fri, 30 May 2025 17:16:56 +0000 (19:16 +0200)]
gdb: introduce a per-interpreter event servicing method
This allows an interpreter to override internal calls to
gdb_do_one_event in case the former needs to handle alternate event
sources.
The default action is to call gdb_do_one_event and this is not overriden
in current internal interpreters. However this feature allows to easily
embed Tcl/Tk in insight that needs to concurrently handle Tcl events for
GUI handling.
In all cases, an interpreter event servicing method must call
gdb_do_one_event at some point.
All internal event servicing calls from gdb now direct to the
interpreter-specific method rather than gdb_do_one_event itself.
Tom de Vries [Mon, 2 Jun 2025 04:44:59 +0000 (06:44 +0200)]
[gdb/python] Reimplement F405 fix
At commit 34b0776fd73^, flake8 reports the following F405 warnings:
...
$ pre-commit run flake8 --file gdb/python/lib/gdb/__init__.py
flake8...................................................................Failed
- hook id: flake8
- exit code: 1
F405 'flush' may be undefined, or defined from star imports: _gdb
F405 'write' may be undefined, or defined from star imports: _gdb
F405 'STDOUT' may be undefined, or defined from star imports: _gdb
F405 'STDERR' may be undefined, or defined from star imports: _gdb
...
F405 'selected_inferior' may be undefined, or defined from star imports: _gdb
F405 'execute' may be undefined, or defined from star imports: _gdb
F405 'parameter' may be undefined, or defined from star imports: _gdb
...
The F405s are addressed by commit 34b0776fd73 ('Suppress some "undefined"
warnings from flake8').
The problem indicated by the first F405 is that the use of flush here:
...
class _GdbFile(object):
...
def flush(self):
flush(stream=self.stream)
...
cannot be verified by flake8. It concludes that either, flush is undefined,
or it is defined by this "star import":
...
from _gdb import * # noqa: F401,F403
...
In this particular case, indeed flush is defined by the star import.
This can be addressed by simply adding:
...
flush(stream=self.stream) # noqa: F405
...
but that has only effect for flake8, so other analyzers may report the same
problem.
The commit 34b0776fd73 addresses it instead by adding an "import _gdb" and
adding a "_gdb." prefix:
...
_gdb.flush(stream=self.stream)
...
This introduces a second way to specify _gdb names, but the first one still
remains, and occasionally someone will use the first one, which then requires
fixing once flake8 is run [1].
While this works to silence the warnings, there is a problem: if a developer
makes a typo:
...
_gdb.flash(stream=self.stream)
...
this is not detected by flake8.
This matters because although the python import already complains:
...
$ gdb -q -batch -ex "python import gdb"
Exception ignored in: <gdb._GdbFile object at 0x7f6186d4d7f0>
Traceback (most recent call last):
File "__init__.py", line 63, in flush
_gdb.flash(stream=self.stream)
AttributeError: module '_gdb' has no attribute 'flash'
...
that doesn't trigger if the code is hidden behind some control flow:
...
if _var_mostly_false:
flash(stream=self.stream)
...
Instead, fix the F405s by reverting commit 34b0776fd73 and adding a second
import of _gdb alongside the star import which lists the names used locally:
...
from _gdb import * # noqa: F401,F403
+from _gdb import (
+ STDERR,
+ STDOUT,
+ Command,
+ execute,
+ flush,
+ parameter,
+ selected_inferior,
+ write,
+)
...
This gives the following warnings for the flash typo:
...
31:1: F401 '_gdb.flush' imported but unused
70:5: F811 redefinition of unused 'flush' from line 31
71:9: F405 'flash' may be undefined, or defined from star imports: _gdb
...
The benefits of this approach compared to the previous one are that:
- the typo is noticed, and
- when using a new name, the F405 fix needs to be done once (by adding it to
the explicit import list), while previously the fix had to be applied to
each use (by adding the "_gdb." prefix).
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
[1] Commit 475799b692e ("Fix some pre-commit nits in gdb/__init__.py")
Tom Tromey [Sun, 1 Jun 2025 19:58:18 +0000 (13:58 -0600)]
Have bfd_thread_init fail when thread-local storage is unavailable
If thread-local storage is unavailable, bfd_thread_init should fail,
because in this case BFD can't be used from multiple threads -- it
relies on TLS working.
Tom de Vries [Sun, 1 Jun 2025 12:06:01 +0000 (14:06 +0200)]
[gdb/tdep] Fix gdb.ada/finish-var-size.exp on ppc64le-linux
On openSUSE Tumbleweed ppc64le-linux using gcc 14.3.0, with a gdb 16.3 based
package and test-case gdb.ada/finish-var-size.exp, I run into:
...
(gdb) finish^M
Run till exit from #0 pck.get (value=true) at pck.adb:19^M
0x0000000100004a20 in p () at finish-var-size/p.adb:18^M
18 V : Result_T := Get (True);^M
Value returned is $1 = <error reading variable: \
Cannot access memory at address 0x0>^M
(gdb) FAIL: gdb.ada/finish-var-size.exp: finish
...
Function pck.get returns type Result_T:
...
type Array_Type is array (1 .. 64) of Integer;
type Maybe_Array (Defined : Boolean := False) is
record
Arr : Array_Type;
Arr2 : Array_Type;
end record;
type Result_T (Defined : Boolean := False) is
record
case Defined is
when False =>
Arr : Maybe_Array;
when True =>
Payload : Boolean;
end case;
end record;
...
and uses r3 as address of the return value, which means
RETURN_VALUE_STRUCT_CONVENTION, but while executing finish_command we do:
...
return_value
= gdbarch_return_value_as_value (gdbarch,
read_var_value (sm->function, nullptr,
callee_frame),
val_type, nullptr, nullptr, nullptr);
...
and get:
...
(gdb) p return_value
$1 = RETURN_VALUE_REGISTER_CONVENTION
...
This is caused by this check in ppc64_sysv_abi_return_value:
...
/* In the ELFv2 ABI, aggregate types of up to 16 bytes are
returned in registers r3:r4. */
if (tdep->elf_abi == POWERPC_ELF_V2
&& valtype->length () <= 16
...
which succeeds because valtype->length () == 0.
Fix this by also checking for !TYPE_HAS_DYNAMIC_LENGTH (valtype).
[ I also tested a version of this patch using "!is_dynamic_type (valtype)"
instead, but ran into a regression in test-case gdb.ada/variant-record.exp,
because type T:
...
Length : constant Positive := 8;
subtype Name_T is String (1 .. Length);
type A_Record_T is
record
X1 : Natural;
X2 : Natural;
end record;
type Yes_No_T is (Yes, No);
type T (Well : Yes_No_T := Yes) is
record
case Well is
when Yes =>
Name : Name_T;
when No =>
Unique_Name : A_Record_T;
end case;
end record;
...
while being dynamic, also has a non-zero size, and is small enough to be
returned in registers r3:r4. ]
Fixing this causes the test-case to fail with the familiar:
...
warning: Cannot determine the function return value.
Try compiling with -fvar-tracking.
...
and indeed using -fvar-tracking makes the test-case pass.
Alan Modra [Sat, 31 May 2025 04:19:24 +0000 (13:49 +0930)]
weakref gas internal error
This horrible testcase (cleaned up from oss-fuzz)
r=x*2
x=r-r
.weakref r,x
r=r-5
triggers resolve_symbol_value "gas_assert (final_val == 0)" in weakref
handling.
Rui Ueyama [Sat, 31 May 2025 12:16:34 +0000 (21:16 +0900)]
PR 33033, Support compressed debug sections larger than 4 GiB
z_stream's avail_in and avail_out are defined as "unsigned int", so it
cannot decode an entire compressed stream in one pass if the stream is
larger than 4 GiB. The simplest solution to this problem is to use zlib's
convenient uncompress2() function, which handles the details for us.
Tom Tromey [Fri, 30 May 2025 13:04:03 +0000 (07:04 -0600)]
Define TLS in bfd.c if not already defined
If configure decides that thread-local storage isn't available, it
does not define "TLS". However, this is used unconditionally in a
definition. So, define it if it isn't already defined.
Alan Modra [Fri, 30 May 2025 22:43:20 +0000 (08:13 +0930)]
PR 33020 segv in _bfd_elf_strtab_offset
The PR fuzzer testcase creates a SHT_NOBITS .debug_info section, then
triggers a bug in --compress-debug-sections=zlib whereby sh_name is
set to -1 in elf_fake_sections as a flag to indicate the name is not
set yet (may change to zdebug_*), but the section never hits the debug
compression code in assign_file_positions_for_non_load_sections that
is responsible for setting sh_name.
PR 33020
* elf.c (_bfd_elf_init_reloc_shdr): Rename delay_st_name_p
param to delay_sh_name_p.
(elf_fake_sections): Rename delay_st_name_p to delay_sh_name_p.
Don't set delay_sh_name_p for no contents debug sections.
Tom Tromey [Fri, 30 May 2025 20:07:57 +0000 (14:07 -0600)]
Remove some Rust expression helpers
When I did the big expression conversion, I left some helper functions
lying around, primarily because the conversion was already quite large
and I didn't want to add on.
This patch removes a couple such helpers, turning them into methods on
the appropriate operation objects.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Wed, 16 Oct 2024 15:58:14 +0000 (09:58 -0600)]
Require Python 3.4
I believe we previously agreed that the minimum supported Python
version should be 3.4. This patch makes this change, harmonizing the
documentation (which was inconsistent about the minimum version) and
the code.
New in v2: rebased, and removed a pre-3.4 workaround from __init__.py.
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-by: Kevin Buettner <kevinb@redhat.com> Acked-By: Tom de Vries <tdevries@suse.de>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31870
Alan Modra [Thu, 29 May 2025 10:40:28 +0000 (20:10 +0930)]
elf symbol size
This changes elf_obj_sy.size from being malloc'd to being on the notes
obstack. That means no code needs to free these expressions, which in
turn means that the size expression can be shared when cloning
symbols. Nothing modifies the size expressions except when resolving.
In all cases I could see, if the size changes the entire expression is
replaced.
The patch also extracts code from elf_copy_symbol_attributes into a
separate function for use by riscv and aarch64.
* config/obj-elf.c (elf_obj_symbol_clone_hook): Delete.
(elf_copy_symbol_size): New function, extracted and modified from..
(elf_copy_symbol_attributes): ..here.
(obj_elf_size): Don't free size and use notes_alloc.
(elf_frob_symbol): Don't free size.
(elf_format_ops): Zero symbol_clone_hook.
* config/obj-elf.h (elf_obj_symbol_clone_hook): Delete.
(obj_symbol_clone_hook): Don't define.
(elf_copy_symbol_size): Declare.
* config/tc-aarch64.c (aarch64_elf_copy_symbol_attributes): Delete.
* config/tc-aarch64.h (OBJ_COPY_SYMBOL_ATTRIBUTES): Define as
elf_copy_symbol_size.
* config/tc-alpha.c (s_alpha_end): notes_alloc symbol size exp.
* config/tc-ia64.c (dot_endp): Likewise.
* config/tc-kvx.c (kvx_endp): Likewise.
* config/tc-mips.c (s_mips_end): Likewise.
* config/tc-riscv.c (riscv_elf_copy_symbol_attributes): Delete.
* config/tc-riscv.h (OBJ_COPY_SYMBOL_ATTRIBUTES): Define as
elf_copy_symbol_size.
Alan Modra [Thu, 29 May 2025 03:20:45 +0000 (12:50 +0930)]
gas symbol_remove
If a symbol is at the start of the symbol chain then symbol_rootP
points at the symbol and symbol->x->previous is NULL. Testing either
condition is sufficient, there is no need to test both. Similarly for
the symbol at the end of the symbol chain.
I'm testing the previous/next pointer because it's most likely that
the symbol is not at the start/finish of the chain and thus we need to
use that pointer.
* symbols.c (symbol_remove): Tidy list handling.
(symbol_append, symbol_clone, symbol_insert): Likewise.
Alan Modra [Tue, 27 May 2025 01:10:59 +0000 (10:40 +0930)]
Reduce rs_align_code memory for small alignments
On x86, MAX_MEM_FOR_RS_ALIGN_CODE is 35, when the most common
alignment is 2**3 or 2**4, where the max memory required for the
alignment nops is 7 and 15 bytes respectively. So there is some
memory wasted since commit 83d94ae428b1. It's not a large amount,
especially considering that frag overhead on x86_46 is 144 bytes,
but even so I'd rather not be blamed for increasing gas memory usage.
So to reduce the memory we'd like to take the alignment into
consideration when initialising an rs_align_code frag. The only
difficulty here is start_bundle making an rs_align_code frag with an
alignment of zero initially, then later increasing the alignment. We
change that to use the bundle alignment when setting up the frag. I
think that is sufficient as bundle_align_p2 can't change in the middle
of a start_bundle/finish_bundle sequence.
I haven't modified any targets other than x86 in this patch. Most
won't benefit much due to using fairly small MAX_MEM_FOR_RS_ALIGN_CODE.
* read.c (start_bundle): Create rs_align_code frag with
bundle_align_p2 alignment, then set to zero alignment.
(finish_bundle): Adjust comment.
* frags.c (MAX_MEM_FOR_RS_ALIGN_CODE): Pass p2align and max
to macro.
* config/tc-i386.h (HANDLE_ALIGN): Assert that max_bytes is
sufficient for nop padding.
(max_mem_for_rs_align_code): New inline function.
(MAX_MEM_FOR_RS_ALIGN_CODE): Use it.
* config/tc-aarch64.h: Adjust MAX_MEM_FOR_RS_ALIGN_CODE.
* config/tc-alpha.h: Likewise.
* config/tc-arc.h: Likewise.
* config/tc-arm.h: Likewise.
* config/tc-epiphany.h: Likewise.
* config/tc-frv.h: Likewise.
* config/tc-ia64.h: Likewise.
* config/tc-kvx.h: Likewise.
* config/tc-loongarch.h: Likewise.
* config/tc-m32r.h: Likewise.
* config/tc-metag.h: Likewise.
* config/tc-mips.h: Likewise.
* config/tc-nds32.h: Likewise.
* config/tc-ppc.h: Likewise.
* config/tc-riscv.h: Likewise.
* config/tc-rl78.h: Likewise.
* config/tc-rx.h: Likewise.
* config/tc-score.h: Likewise.
* config/tc-sh.h: Likewise.
* config/tc-sparc.h: Likewise.
* config/tc-spu.h: Likewise.
* config/tc-tilegx.h: Likewise.
* config/tc-tilepro.h: Likewise.
* config/tc-visium.h: Likewise.
* config/tc-xtensa.h: Likewise.
gdb: update corner case when canonicalizing riscv syscall names
The script syscalls/riscv-canonicalize-syscall-gen.py has been recently
introduced to help support record-full in riscv systems. However, it
was developed before commit 432eca4113d5748ad284a068873455f9962b44fe,
which made the GDB enum more consistent, which forced the python script
to have a corner case for the "gdb_old_mmap" case.
Since the aforementioned commit has already been merged, we need to
update the special case for the mmap syscall. A special case is still
needed because the script would expect that the glibc sources call the
syscall "old_mmap", or that gdb call it "gdb_sys_mmap", neither of which
happens unfortunately.
This commit doesn't change the .c file because it was already fixed by a
different commit, 65ab41b7d5c612b6000b28f4c50bb256b2a9e22b, which was
pushed as obvious to fix the build issues.
Jorenar [Sun, 25 May 2025 15:40:25 +0000 (17:40 +0200)]
gdb/dap: fix completion request for empty strings
When DAP completion requests receives empty string to complete,
the script crashes due trying to access element -1 from list
being a result of `text.splitlines()` (which for `text == ""`
evaluates into empty list).
This patch adds simple check if `text` is populated, and when it
is not, skips transformations and assigns correct result directly.
Pedro Alves [Sat, 17 May 2025 21:30:56 +0000 (22:30 +0100)]
Fix build when RUSAGE_THREAD is not available & add warning
Building current GDB on Cygwin, fails like so:
/home/pedro/gdb/src/gdbsupport/run-time-clock.cc: In function ‘void get_run_time(user_cpu_time_clock::time_point&, system_cpu_time_clock::time_point&, run_time_scope ’:
/home/pedro/gdb/src/gdbsupport/run-time-clock.cc:52:13: error: ‘RUSAGE_THREAD’ was not declared in this scope; did you mean ‘SIGEV_THREAD’?
52 | who = RUSAGE_THREAD;
| ^~~~~~~~~~~~~
| SIGEV_THREAD
Cygwin does not implement RUSAGE_THREAD. Googling around, I see
Cygwin is not alone, other platforms don't support it either. For
example, here is someone suggesting an alternative for darwin/macos:
https://stackoverflow.com/questions/5652463/equivalent-to-rusage-thread-darwin
Fix this by falling back to process scope if thread scope can't be
supported. I chose this instead of returning zero usage or some other
constant, because if gdb is built without threading support, then
process-scope run time usage is the right info to return.
But instead of falling back silently, print a warning (just once),
like so:
(gdb) maint set per-command time on
⚠️ warning: per-thread run time information not available on this platform
... so that developers on other platforms at least have a hint
upfront.
This new warning also shows on platforms that don't have getrusage in
the first place, but does not show if the build doesn't support
threading at all.
New tests are added to gdb.base/maint.exp, to expect the warning, and
also to ensure other "mt per-command" sub commands don't trigger the
new warning.
Change-Id: Ie01b916b62f87006f855e31594a5ac7cf09e4c02 Approved-By: Simon Marchi <simon.marchi@efficios.com> Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Mon, 26 May 2025 20:26:15 +0000 (16:26 -0400)]
gdb/solib: make implementation of solib_ops::open_symbol_file_object optional
The only solib implementation that implements open_symbol_file_object is
SVR4. All others just return 0. Make it optional, to avoid having
these empty functions.
Simon Marchi [Mon, 26 May 2025 20:26:13 +0000 (16:26 -0400)]
gdb/solib: move solist.h content to solib.h
I don't think that the file solist.h is useful. It would make sense to
have `struct solib` in solib.h. And then, all that would remain is
`struct solib_ops` and some solib-related function declarations. So,
move it all to solib.h.
Tom de Vries [Wed, 28 May 2025 20:17:19 +0000 (22:17 +0200)]
[gdb/symtab] Note errors in process_skeletonless_type_units
With a hello world a.out, and using the compiler flags from target board
dwarf5-fission-debug-types:
...
$ gcc -gdwarf-5 -fdebug-types-section -gsplit-dwarf ~/data/hello.c
...
I run into:
...
$ gdb -q -batch a.out
terminate called after throwing an instance of 'gdb_exception_error'
...
What happens is that an error is thrown due to invalid dwarf, but the error is
not caught, causing gdb to terminate.
In a way, this is a duplicate of PR32861, in the sense that we no longer run
into this after:
- applying the proposed patch (work around compiler bug), or
- using gcc 9 or newer (compiler bug fixed).
But in this case, the failure mode is worse than in PR32861.
Fix this by catching the error in
cooked_index_worker_debug_info::process_skeletonless_type_units.
With the patch, we get instead:
...
$ gdb -q -batch a.out
Offset from DW_FORM_GNU_str_index or DW_FORM_strx pointing outside of \
.debug_str.dwo section in CU at offset 0x0 [in module a.out]
...
While we're at it, absorb the common use of
cooked_index_worker_result::note_error:
...
try
{
...
}
catch (gdb_exception &exc)
{
(...).note_error (std::move (exc));
}
...
into the method and rename it to catch_error, resulting in more compact code
for the fix:
...
(...).catch_error ([&] ()
{
...
});
...
While we're at it, also use it in
cooked_index_worker_debug_info::process_units which looks like it needs the
same fix.
Call restore_original_signal_state after GDB forks.
When I run GDB under Valgrind, GDB seems to segfault
displaying:
Fatal signal: Segmentation fault
----- Backtrace -----
0x2803f7 ???
0x3c9696 ???
0x3c9899 ???
0x55f8fcf ???
0x486c000 ???
---------------------
A fatal error internal to GDB has been detected, further
debugging is not possible. GDB will now terminate.
This is a bug, please report it. For instructions, see:
<https://www.gnu.org/software/gdb/bugs/>.
warning: linux_ptrace_test_ret_to_nx: PC 0x5821c09d is neither near return address 0x486c000 nor is the return instruction 0x4f8f4a!
but then, acts like nothing happened and excutes normally. This is
because it's the child from linux_ptrace_test_ret_to_nx that
segfaults and parent GDB carries on normally. Restore the original
signal states to not to print confusing backtrace. After restoring,
only such warning is displayed:
warning: linux_ptrace_test_ret_to_nx: WSTOPSIG 19 is neither SIGTRAP nor SIGSEGV!
gdb/testsuite: Clarify -lbl option in gdb_test_multiple
I was a bit confused about the -lbl option in gdb_test_multiple, and needed
to read its implementation to determine that it would be useful for my
needs. Explicitly mention what the option does and why it's useful to
hopefully help other developers.
Reviewed-By: Keith Seitz <keiths@redhat.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
gdb/testsuite: Fix flakiness in gdb.base/default.exp
The Linaro CI runs the GDB testsuite using the read1 tool, which
significantly increases the time it takes DejaGNU to read inferior output.
On top of that sometimes the test machine has higher than normal load,
which causes tests to run even slower.
Because gdb.base/default.exp tests some verbose commands such as "info
set", it sometimes times out while waiting for the complete command
output when running in the Linaro CI environment.
Fix this problem by consuming each line of output from the most verbose
commands with gdb_test_multiple's -lbl (line-by-line) option — which
causes DejaGNU to reset the timeout after each match — and also by
breaking up regular expressions that try to match more than 2000
characters (the default Expect buffer size) in one go into multiple
matches.
Some tests use the same regular expression, so I created a procedure for
them. This is the case for "i" / "info", "info set" / "show", and "set
print" tests.
The tests for "show print" don't actually test their output, so this
patch improves them by checking some lines of the output.
Reviewed-By: Keith Seitz <keiths@redhat.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
Alan Modra [Mon, 26 May 2025 02:16:05 +0000 (11:46 +0930)]
ALPHA_R_OP_STORE
In commit db4ab410dec3 I rewrote OP_STORE handling to support writing
near the end of a section. The rewrite had some bugs, fixed in commit 3e02c4891dcb. However I wasn't entirely happy with the code writing
the bitfield:
- it doesn't support 64-bit fields with a bit offset,
- the code is duplicated and inelegant,
- the stack ought to be popped whenever seeing one of these relocs,
even if the reloc can't be applied.
This patch fixes all of the above.
In addition, it is clear from the OP_STORE description in the ABI that
a 64-bit field is encoded as 0 in r_size, so I've decoded that in
alpha_ecoff_swap_reloc_in. The aborts there are not appropriate as
they can be triggered by user input (fuzzed object files). Also,
stack underflow wasn't checked in alpha_relocate_section.
* coff-alpha.c (alpha_ecoff_swap_reloc_in): Replace aborts
with asserts. Decode ALPHA_R_OP_STORE r_size of zero.
(write_bit_field): New function.
(alpha_ecoff_get_relocated_section_contents): Use it.
(alpha_relocate_section): Here too. Catch stack underflow.
Jens Remus [Mon, 26 May 2025 18:02:47 +0000 (11:02 -0700)]
libsframe: handle SFrame FRE start/end IP offsets as unsigned
The SFrame FRE start address (fre_start_addr) is defined as unsigned
32-bit integer, as it is an offset from SFrame FDE function start
address (sfde_func_start_address) and functions only grow upwards
(towards higher addresses).
The SFrame FRE start IP offset is a synonym to the SFrame FRE start
address. The SFrame FRE end IP offset is either the value of the
subsequent FDE start address minus one, if that exists, or the FDE
function size minus one otherwise. Both should therefore be handled
as unsigned 32-bit integer.
In libsframe the "lookup PC" (pc) and SFrame FDE function start address
(sfde_func_start_address) are both signed integers, as they are actually
offsets from the SFrame section. The unsigned FDE start/end IP offsets
may therefore only be safely compared against the offset of the lookup
PC from FDE function start address if the FDE function start address is
lower or equal to the lookup PC, as this guarantees the offset to be
always positive:
If the FDE function start address is lower or equal than the lookup PC,
which both are signed offsets from SFrame section, then the function
start address is also lower or equal to the PC, which are both unsigned:
sfde_func_start_address <= lookup_pc
func_start_addr - sframe_addr <= pc - sframe_addr
func_start_addr <= pc
With that the offset of the lookup PC from FDE function start address
(lookup_pc - sfde_func_start_address) must always be positive, if
FDE function start address is lower or equal to the lookup PC:
lookup_pc - sfde_func_start_address
= pc - sframe_addr - (func_start_addr - sframe_addr)
= pc - func_start_addr
libsframe/
* sframe.c (sframe_find_fre): Define and handle start_ip_offset
and end_ip_offset as unsigned (same as FRE fre_start_addr).
(sframe_fre_check_range_p): Likewise. Define PC offset (from
function start address) as unsigned.
Jens Remus [Mon, 26 May 2025 18:02:29 +0000 (11:02 -0700)]
libsframe: stop search for SFrame FRE if its start IP is greater than PC
The SFrame FREs for an SFrame FDE are sorted on their start address.
Therefore the linear search for a matching SFrame FRE can be stopped,
if its start address is greater than the searched for PC.
libsframe/
* sframe.c (sframe_find_fre): Stop search if FRE's start IP is
greater than PC.
Jens Remus [Mon, 26 May 2025 18:01:14 +0000 (11:01 -0700)]
libsframe: correct binary search for SFrame FDE
sframe_get_funcdesc_with_addr_internal erroneously returns the last FDE,
if its function start address is lower than the searched for address.
Simplify the binary search for a SFrame FDE for a given address. Only
return an FDE, if the searched for address is within the bounds of the
FDE function start address and function size.
libsframe/
* sframe.c (sframe_get_funcdesc_with_addr_internal): Correct
binary search for SFrame FDE.
libsframe/testsuite/
* libsframe.find/plt-findfre-1.c: Add test for out of range
PLT6.
Indu Bhagat [Mon, 26 May 2025 17:54:06 +0000 (10:54 -0700)]
libsframe: fix issue finding FRE in PCMASK type SFrame FDEs
SFrame FDEs of type SFRAME_FDE_TYPE_PCMASK are used for repetitive code
patterns, e.g., pltN entries. For SFrame FDEs of type
SFRAME_FDE_TYPE_PCMASK, sframe_fre_check_range_p erroneously tested the
given PC instead of the masked PC offset from function start address.
Therefore it only worked correctly by chance, e.g., if the function start
address was aligned on the repetition block size.
For regular SFrame FDEs the PC offset from function start address must
be within a SFrame FRE's start IP offset and end IP offset. For SFrame
FDEs of type SFRAME_FDE_TYPE_PCMASK, the masked PC offset must be within
that range.
SFrame FRE start/end IP offsets are relative to the SFrame FDE function
start address. For regular SFrame FDEs, the PC offset from function
start address must be within a SFrame FRE's start IP offset and end IP
offset. For SFRAME_FDE_TYPE_PCMASK type FDEs, the masked PC offset must
be within that range.
Exercise the testcase for a variety of placements; without the fix some
of these tests will fail. Also, make the testcase itself easier to
follow by adding appropriate vars where applicable.
libsframe/
* sframe.c (sframe_fre_check_range_p): Fix logic for
SFRAME_FDE_TYPE_PCMASK type FDE.
libsframe/testsuite/
* libsframe.find/plt-findfre-1.c: Adjust the test for a variety
of placements of .sframe and .plt.
if (uia < uib)
return true;
if (uia > uib)
return false;
...
we compare pointers to struct program_space, which gives unstable sorting
results.
The assumption is that this doesn't matter, but as PR32202 demonstrates,
sometimes it does.
While PR32202 is fixed elsewhere, it seems like a good idea to stabilize this
comparison, because it comes at a small cost and possibly prevents
hard-to-reproduce user-visible ordering issues.
Fix this by comparing the program space IDs instead of the pointers.
Likewise in compare_msymbols.
Tested on x86_64-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
Tom de Vries [Mon, 26 May 2025 13:15:31 +0000 (15:15 +0200)]
[gdb/breakpoints] Stabilize info breakpoints output
With test-case gdb.multi/pending-bp-del-inferior.exp, occasionally I run into:
...
(gdb) info breakpoints^M
Num Type Disp Enb Address What^M
3 dprintf keep y <MULTIPLE> ^M
printf "in foo"^M
3.1 y 0x004004dc in foo at $c:21 inf 2^M
3.2 y 0x004004dc in foo at $c:21 inf 1^M
(gdb) FAIL: $exp: bp_pending=false: info breakpoints before inferior removal
...
The FAIL happens because the test-case expects:
- breakpoint location 3.1 to be in inferior 1, and
- breakpoint location 3.2 to be in inferior 2
but it's the other way around.
I managed to reproduce this with a trigger patch in
compare_symbols from gdb/linespec.c:
...
uia = (uintptr_t) a.symbol->symtab ()->compunit ()->objfile ()->pspace ();
uib = (uintptr_t) b.symbol->symtab ()->compunit ()->objfile ()->pspace ();
- if (uia < uib)
+ if (uia > uib)
return true;
- if (uia > uib)
+ if (uia < uib)
return false;
...
The order enforced by compare_symbols shows up in the "info breakpoints"
output because breakpoint::add_location doesn't enforce an ordering for equal
addresses:
...
auto ub = std::upper_bound (m_locations.begin (), m_locations.end (),
loc,
[] (const bp_location &left,
const bp_location &right)
{ return left.address < right.address; });
m_locations.insert (ub, loc);
...
Fix this by using new function bp_location_is_less_than
(forwarding to bp_location_ptr_is_less_than) in breakpoint::add_location.
Tom de Vries [Mon, 26 May 2025 13:15:31 +0000 (15:15 +0200)]
[gdb/breakpoints] Rename bp_location_is_less_than to bp_location_ptr_is_less_than
In breakpoint.c, we have:
...
/* A comparison function for bp_location AP and BP being interfaced to
std::sort. Sort elements primarily by their ADDRESS (no matter what
bl_address_is_meaningful says), secondarily by ordering first
permanent elements and tertiarily just ensuring the array is sorted
stable way despite std::sort being an unstable algorithm. */
static int
bp_location_is_less_than (const bp_location *a, const bp_location *b)
...
There are few problems here:
- the return type is int. While std::sort allows this, because int is
convertible to bool, it's clearer to use bool directly,
- it's not abundantly clear from either function name or comment that we can
use this to sort std::vector<bp_location *> but not
std::vector<bp_location>, and
- the comment mentions AP and BP, but there are no such parameters.
Fix this by:
- changing the return type to bool,
- renaming the function to bp_location_ptr_is_less_than and mentioning
std::vector<bp_location *> in the comment, and
- updating the comment to use the correct parameter names.
Tested on x86_64-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
Janne Ramstedt [Sun, 25 May 2025 17:17:20 +0000 (20:17 +0300)]
alpha, bfd: Fixes for ALPHA_R_OP_STORE
ALPHA_R_OP_STORE copies one byte too many and also will cause out of
range error when it tries to copy from the end of section. Since
"endbyte" is already rounded to next full byte, there is enough bits
to copy and the additional "+ 1" is erroneous in bytes count. I also
believe size is incorrectly decreased.
Simon Marchi [Fri, 23 May 2025 16:30:52 +0000 (12:30 -0400)]
gdb/solib-svr4: check that solib is SVR4 in tls_maybe_fill_slot and tls_maybe_erase_slot
Functions tls_maybe_fill_slot and tls_maybe_erase_slot blindly assume
that the passe solibs come from solib-svr4. This is not always the
case, because they are called even on the systems where the solib
implementation isn't solib-svr4. Add some checks to return early in
that case.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32990
Change-Id: I0a281e1f4826aa1914460c2213f0fae1bdc9af7c Tested-By: Hannes Domani <ssbssa@yahoo.de> Approved-By: Andrew Burgess <aburgess@redhat.com>
Tom de Vries [Sat, 24 May 2025 08:27:12 +0000 (10:27 +0200)]
[gdb/build] Fix unused var in lookup_dwo_unit_in_dwp
On x86_64-linux, with gcc 7.5.0 I ran into a build breaker:
...
gdb/dwarf2/read.c: In function ‘dwo_unit* lookup_dwo_unit_in_dwp()’:
gdb/dwarf2/read.c:7403:22: error: unused variable ‘inserted’ \
[-Werror=unused-variable]
auto [it, inserted] = dwo_unit_set.emplace (std::move (dwo_unit));
^
...
Simon Marchi [Fri, 23 May 2025 18:00:29 +0000 (14:00 -0400)]
gdb: include <mutex> in dwarf2/read.h
The buildbot pointed out this compilation failure on AlmaLinux, with g++
8.5.0, which is not seen on more recent systems:
CXX gdbtypes.o
In file included from ../../binutils-gdb/gdb/gdbtypes.c:39:
../../binutils-gdb/gdb/dwarf2/read.h:639:8: error: ‘mutex’ in namespace ‘std’ does not name a type
std::mutex dwo_files_lock;
^~~~~
../../binutils-gdb/gdb/dwarf2/read.h:639:3: note: ‘std::mutex’ is defined in header ‘<mutex>’; did you forget to ‘#include <mutex>’?
../../binutils-gdb/gdb/dwarf2/read.h:35:1:
+#include <mutex>
projects / thirdparty / binutils-gdb.git / log
