Simon Marchi [Mon, 17 Mar 2025 14:34:29 +0000 (10:34 -0400)]
gdb/dwarf: fix spurious error when encountering dummy CU
I built an application with -gsplit-dwarf (i.e. dwo), and some CUs are
considered "dummy" by the DWARF reader. That is, the top-level DIE
(DW_TAG_compile_unit) does not have any children. Here's the skeleton:
0x0000c0cb: Compile Unit: length = 0x0000001d, format = DWARF32, version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset = 0x529b, addr_size = 0x08, DWO_id = 0x0ed2693dd2a756dc (next unit at 0x0000c0ec)
When loading the binary in GDB, I see some warnings:
$ ./gdb -q -nx --data-directory=data-directory -ex 'maint set dwarf sync on' -ex "file /home/simark/src/tdesktop/build-relwithdebuginfo-split-nogz/telegram-desktop"
Reading symbols from /home/simark/src/tdesktop/build-relwithdebuginfo-split-nogz/telegram-desktop...
DWARF Error: unexpected tag 'DW_TAG_skeleton_unit' at offset 0xc0cb
DWARF Error: unexpected tag 'DW_TAG_skeleton_unit' at offset 0xc152
DWARF Error: unexpected tag 'DW_TAG_skeleton_unit' at offset 0xc194
DWARF Error: unexpected tag 'DW_TAG_skeleton_unit' at offset 0xc1b5
(gdb)
It turns out that these errors are not really justified. What happens
is:
- cutu_reader::read_cutu_die_from_dwo return 0, indicating that the CU
is "dummy"
- back in cutu_reader::cutu_reader, we omit setting m_top_level_die to
the DIE from the dwo file, meaning that m_top_level_die keeps
pointing to the DIE from the main file (DW_TAG_skeleton_unit)
- later, in cutu_reader::prepare_one_comp_unit, there is a check that
m_top_level_die->tag is one of DW_TAG_{compile,partial,type}_unit,
which triggers
My proposal to fix this is to set m_top_level_die even if the CU is
dummy. Even if the top-level DIE does not have any children, I don't
see any reason to leave cutu_reader::m_top_level_die in a different
state than when the CU is not dummy.
While at it, set m_dummy_p directly in read_cutu_die_from_dwo, instead
of returning a value and having the caller do it. This is all inside
cutu_reader anyway.
Change-Id: I483a68a369bb461a8dfa5bf2106ab1d6a0067198 Approved-By: Tom Tromey <tom@tromey.com>
Andrew Burgess [Sat, 2 Nov 2024 17:35:14 +0000 (17:35 +0000)]
gdb: split up construct_inferior_arguments
The function construct_inferior_arguments (gdbsupport/common-inferior.cc)
currently escapes all special shell characters. After this commit
there will be two "levels" of quoting:
1. The current "full" quoting, where all posix shell special
characters are quoted, and
2. a new "reduced" quoting, where only the characters that GDB sees
as special (quotes and whitespace) are quoted.
After this, almost all construct_inferior_arguments calls will use the
"full" quoting, which is the current quoting. The "reduced" quoting
will be used in this commit to restore the behaviour that was lost in
the previous commit (more details below).
In the future, the reduced quoting will be useful for some additional
inferior argument that I have planned. I already posted my full
inferior argument work here:
But that series is pretty long, and wasn't getting reviewed, so I'm
posted the series in parts now.
Before the previous commit, GDB behaved like this:
$ gdb -eiex 'set startup-with-shell off' --args /tmp/exec '$FOO'
(gdb) show args
Argument list to give program being debugged when it is started is "$FOO".
Notice that with 'startup-with-shell' off, the argument was left as
just '$FOO'. But after the previous commit, this changed to:
$ gdb -eiex 'set startup-with-shell off' --args /tmp/exec '$FOO'
(gdb) show args
Argument list to give program being debugged when it is started is "\$FOO".
Now the '$' is escaped with a backslash. This commit restores the
original behaviour, as this is (currently) the only way to unquoted
shell special characters into arguments from the GDB command line.
The series that I listed above includes a new command line option for
GDB which provides a better approach for controlling the quoting of
special shell characters, but that work requires these patches to be
merged first.
I've split out the core of construct_inferior_arguments into the new
function escape_characters, which takes a set of characters to escape.
Then the two functions escape_shell_characters and
escape_gdb_characters call escape_characters with the appropriate
character sets.
Finally, construct_inferior_arguments, now takes a boolean which
indicates if we should perform full shell escaping, or just perform
the reduced escaping.
I've updated all uses of construct_inferior_arguments to pass a
suitable value to indicate what escaping to perform (mostly just
'true', but one case in main.c is different), also I've updated
inferior::set_args to take the same boolean flag, and pass it through
to construct_inferior_arguments.
gdb: Support some escaping of args with startup-with-shell being off
nat/fork-inferior.c was updated such that when we are starting an
inferior without a shell we now remove escape characters. The
benefits of this are explained in that commit, but having made this
change we can now make an additional change.
Currently, in construct_inferior_arguments, when startup_with_shell is
false we construct the inferior argument string differently than when
startup_with_shell is true; when true we apply some escaping to
special shell character, when false we don't.
This commit simplifies construct_inferior_arguments by removing the
!startup_with_shell case, and instead we now apply escaping in all
cases. This is fine because, thanks to the above commit the escaping
will be correctly removed again when we call into nat/fork-inferior.c.
We should think of construct_inferior_arguments and
nat/fork-inferior.c as needing to cooperate in order for argument
handling to work correctly.
construct_inferior_arguments converts a list of separate arguments
into a single string, and nat/fork-inferior.c splits that single
string back into a list of arguments. It is critical that, if
nat/fork-inferior.c is expecting to remove a "layer" of escapes, then
construct_inferior_arguments must add that expected "layer",
otherwise, we end up stripping more escapes than expected.
The great thing (I think) about the new configuration, is that GDB no
longer cares about startup_with_shell at the point the arguments are
being setup. We only care about startup_with_shell at the point that
the inferior is started. This means that a user can set the inferior
arguments, and then change the startup-with-shell setting, and GDB
will do what they expect.
Under the previous system, where construct_inferior_arguments changed
its behaviour based on startup_with_shell, the user had to change the
setting, and then set the arguments, otherwise, GDB might not do what
they expect.
There is one slight issue with this commit though, which will be
addressed by the next commit.
For GDB's native targets construct_inferior_arguments is reached via
two code paths; first when GDB starts and we combine arguments from
the command line, and second when the Python API is used to set the
arguments from a sequence. It's the command line argument handling
which we are interested in.
Consider this:
$ gdb --args /tmp/exec '$FOO'
(gdb) show args
Argument list to give program being debugged when it is started is "\$FOO".
Notice that the argument has become \$FOO, the '$' is now quoted.
This is because, by quoting the argument in the shell command that
started GDB, GDB was passed a literal $FOO with no quotes. In order
to ensure that the inferior sees this same value, GDB added the extra
escape character. When GDB starts with a shell we pass \$FOO, which
results in the inferior seeing a literal $FOO.
But what if the user _actually_ wanted to have the shell GDB uses to
start the inferior expand $FOO? Well, it appears this can't be done
from the command line, but from the GDB prompt we can just do:
(gdb) set args $FOO
(gdb) show args
Argument list to give program being debugged when it is started is "$FOO".
And now the inferior will see the shell expanded version of $FOO.
It might seem like we cannot achieve the same result from the GDB
command line, however, it is possible with this trick:
$ gdb -eiex 'set startup-with-shell off' --args /tmp/exec '$FOO'
(gdb) show args
Argument list to give program being debugged when it is started is "$FOO".
(gdb) show startup-with-shell
Use of shell to start subprocesses is off.
And now the $FOO is not escaped, but GDB is no longer using a shell to
start the inferior, however, we can extend our command line like this:
$ gdb -eiex 'set startup-with-shell off' \
-ex 'set startup-with-shell on' \
--args /tmp/exec '$FOO'
(gdb) show args
Argument list to give program being debugged when it is started is "$FOO".
(gdb) show startup-with-shell
Use of shell to start subprocesses is on.
Use an early-initialisation option to disable startup-with-shell, this
is done before command line argument processing, then a normal
initialisation option turns startup-with-shell back on after GDB has
processed the command line arguments!
Is this useful? Yes, absolutely. Is this a good user experience?
Absolutely not. And I plan to add a new command line option to
GDB (and gdbserver) that will allow users to achieve the same
result (this trick doesn't work in gdbserver as there's no
early-initialisation there) without having to toggle the
startup-with-shell option. The new option can be found in the series
here:
The problem is that, that series is pretty long, and getting it
reviewed is just not possible. So instead I'm posting the individual
patches in smaller blocks, to make reviews easier.
So, what's the problem? Well, by removing the !startup_with_shell
code path from GDB, there is no longer a construct_inferior_arguments
code path that doesn't quote inferior arguments, and so there's no
longer a way, from the command line, to set an unquoted '$FOO' as an
inferior argument. Obviously, this can still be done from GDB's CLI
prompt.
The trick above is completely untested, so this regression isn't going
to show up in the testsuite.
And the breakage is only temporary. In the next commit I'll add a fix
which restores the above trick.
Of course, I hope that this fix will itself, only be temporary. Once
the new command line options that I mentioned above are added, then
the fix I add in the next commit can be removed, and user should start
using the new command line option.
After this commit a whole set of tests that were added as xfail in the
above commit are now passing.
A change similar to this one can be found in this series:
which I reviewed before writing this patch. I don't think there's any
one patch in that series that exactly corresponds with this patch
though, so I've listed the author of the original series as co-author
on this patch.
Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392
Tom Tromey [Mon, 17 Mar 2025 17:58:03 +0000 (11:58 -0600)]
Preserve a local variable in a gdb test
I found another Ada test where LLVM optimizes away an unused local
variable. This patch fixes this problem -- but note the test now
fails for a different (currently expected) reason.
Tom Tromey [Thu, 13 Mar 2025 22:44:05 +0000 (16:44 -0600)]
Use gdb unordered map in tui-io.c
This changes tui.c to use gdb::unordered_map. ui_file_style::color is
changed a little as well; operator< is no longer needed, but a simple
hash function is added.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 11 Mar 2025 17:22:05 +0000 (11:22 -0600)]
Use gdb unordered set and map in Python layer
This changes a couple of files in the Python layer to use
gdb:unordered_set and gdb::unordered_map. Another use exists but I
think it is being handled by Jan's series.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom de Vries [Tue, 18 Mar 2025 07:45:54 +0000 (08:45 +0100)]
[gdb/tdep] Use SYSCALL_MAP_RENAME for aarch64 and loongarch
There are currently two functions using macros SYSCALL_MAP and
UNSUPPORTED_SYSCALL_MAP: aarch64_canonicalize_syscall, and
loongarch_canonicalize_syscall.
Here [1] I propose to do the same in i386_canonicalize_syscall, using one
additional macro: SYSCALL_MAP_RENAME.
Add the same macro in aarch64_canonicalize_syscall and
loongarch_canonicalize_syscall, and use it to map aarch64_sys_mmap and
loongarch_sys_mmap to gdb_sys_mmap2.
While we're at it:
- reformat the macro definitions to be more readable,
- add missing macro undefs in aarch64_canonicalize_syscall, and
- fix indentation in aarch64_canonicalize_syscall.
No functional changes.
Tested by rebuilding on x86_64-linux.
Reviewed-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
[1] https://sourceware.org/pipermail/gdb-patches/2025-March/216230.html
Jerry Zhang Jian [Mon, 17 Mar 2025 12:16:35 +0000 (20:16 +0800)]
RISC-V: Support pointer masking extension 1.0
- Adding Ssnpm, Smnpm, Smmpm, Sspm, and Supm
- No new CSR added
- Pointer masking only applies to RV64
- Ref: https://github.com/riscv/riscv-j-extension/releases/download/pointer-masking-ratified/pointer-masking-ratified.pdf
Signed-off-by: Jerry Zhang Jian <jerry.zhangjian@sifive.com>
Jin Ma [Mon, 17 Mar 2025 06:07:35 +0000 (14:07 +0800)]
RISC-V: Add extension XTheadVdot for T-Head VECTOR vendor extension [1]
T-Head has a range of vendor-specific instructions. Therefore
it makes sense to group them into smaller chunks in form of
vendor extensions.
This patch adds the additional extension "XTheadVdot" based on the
"V" extension, and it provides four 8-bit multiply and add with
32-bit instructions for the "v" extension. The 'th' prefix and the
"XTheadVector" extension are documented in a PR for the
RISC-V toolchain conventions ([2]).
Nelson Chu [Thu, 13 Mar 2025 02:31:36 +0000 (10:31 +0800)]
RISC-V: Avoid parsing arch string repeatedly for dis-assembler
Since we now always generate $x+isa for now, these would increase the
dis-assemble time by parsing the same architecture string repeatedly. We
already have `arch_str' field into `subset_list' to record the current
architecture stirng, but it's only useful for assembler, since dis-assembler
and linker don't need it before. Now for dis-assembler, we just need to
update the `arch_str' after parsing the architecture stirng, and then avoid
parsing repeatedly if the strings are the same.
Nelson Chu [Thu, 13 Mar 2025 02:31:35 +0000 (10:31 +0800)]
RISC-V: Free the returned string of riscv_arch_str if we call it multiple times
The string returned from riscv_arch_str is allocated by xmalloc, so once we
called it multiple times, we should keep the newest one for the output elf
architecture attribute, but free the remaining unused strings.
Nelson Chu [Thu, 13 Mar 2025 02:31:34 +0000 (10:31 +0800)]
RISC-V: Fixed riscv_update_subset1 returning wrong boolean value
The riscv_update_subset1 returning wrong boolean value if the
riscv_parse_check_conflicts isn't called, though the current return value
doesn't really useful.
Simon Marchi [Sat, 15 Mar 2025 22:13:41 +0000 (18:13 -0400)]
gdbsupport: add some -Wunused-* warning flags
Add a few -Wunused-* diagnostic flags that look useful. Some are known
to gcc, some to clang, some to both. Fix the fallouts.
-Wunused-const-variable=1 is understood by gcc, but not clang.
-Wunused-const-variable would be undertsood by both, but for gcc at
least it would flag the unused const variables in headers. This doesn't
make sense to me, because as soon as one source file includes a header
but doesn't use a const variable defined in that header, it's an error.
With `=1`, gcc only warns about unused const variable in the main source
file. It's not a big deal that clang doesn't understand it though: any
instance of that problem will be flagged by any gcc build.
Change-Id: Ie20d99524b3054693f1ac5b53115bb46c89a5156 Approved-By: Tom Tromey <tom@tromey.com>
Andrew Burgess [Thu, 13 Mar 2025 16:29:07 +0000 (16:29 +0000)]
gdb-add-index: add --help and --version options
Update the gdb-add-index script to offer --help and --version options.
The script currently accepts the argument '-dwarf-5' with a single
leading '-'. As two '--' is more common for long options, the
preferred argument form is now '--dwarf-5', the docs have been
updated, and the new help text uses this form.
For backward compatibility, the old '-dwarf-5' form is still
accepted.
The new arguments are '--help' or '-h', but I also accept '-help' for
consistency with '-dwarf-5'. And likewise for the version argument.
Handling of the gdb-add-index script is done basically the same as for
gcore and gstack; we use config.status to create a .in file within the
build directory, which is then processed by the Makefile to create the
final script.
The difference with gdb-add-index is that I left the original script
as gdb/contrib/gdb-add-index.sh rather than renaming it to something
like gdb/contrib/gdb-add-index-1.in, which is how gcore and gstack are
handled (though they are not in the contrib directory).
The reason for this is that the contrib/cc-with-tweaks.sh script looks
for gdb-add-index.sh within the gdb/contrib/ source directory.
As the only reason we process gdb-add-index.sh into the build
directory is to support the PKGVERSION and VERSION variables, allowing
cc-with-tweaks to continue using the unprocessed version seems
harmless, and avoids having to change cc-with-tweaks.sh at all.
I tested that I can still run tests using the cc-with-gdb-index target
board, and that the installed gdb-add-index script correctly shows a
version number when asked.
Andrew Burgess [Mon, 17 Feb 2025 15:00:52 +0000 (15:00 +0000)]
gdb: make cli_styling static within cli/cli-style.c
The cli_styling variable is controlled by 'set style enabled on|off'
user setting, and is currently globally visible.
In a couple of places we access this variable directly, though in
ui-file.c the accesses are all performed through term_cli_styling(),
which is a function that wraps checking cli_styling along with a check
that GDB's terminal supports styling.
In a future commit, I'd plan to add a new parameter to gdb.execute()
which will allow styling to be temporarily suppressed. In an earlier
proposal, I made gdb.execute() disable styling by changing the value
of cli_styling, however, this approach has a problem.
If gdb.execute() is used to run 'show style enabled', the changing
cli_styling will change what is printed. Similarly, if gdb.execute()
is used to execute 'set style enabled on|off' then having
gdb.execute() save and restore the value of cli_styling will undo the
adjustment from 'set style enabled ...'.
So what I plan to do in the future, is add a new control flag which
can be used to temporarily disable styling.
To make this new control variable easier to add, lets force everyone
to call term_cli_styling() to check if styling is enabled or not. To
force everyone to use term_cli_styling() this commit makes cli_styling
static within gdb/cli/cli-style.c.
gdb/python: remove unused argument from builtin_disassemble
this commit fixes it.
I've also reworded the NEWS entry a little. Simon pointed out in
review that the unused argument was also documented in Python's help()
output, which I hadn't mentioned in the NEWS entry. I've updated the
NEWS entry to just highlight that the now removed argument was never
mentioned in the manual, I think that's all that really matters.
Simon Marchi [Mon, 17 Mar 2025 02:38:22 +0000 (22:38 -0400)]
gdb/dwarf: use gdb::unordered_set for seen_names
Direct replacement of an htab with a gdb::unordered_set.
Using a large test program, I see a small but consistent performance
improvement. The "file" command time goes on average from 7.88 to 7.73
seconds (~2%). To give a rough estimate of the scale of the test
program, the 8 seen_names hash tables (one for each worker thread) had
between 173846 and 866961 entries.
Change-Id: I0157cbd04bb55338bb1fcefd2690aeef52fe3afe Approved-By: Tom Tromey <tom@tromey.com>
Lucy Kingsbury [Sun, 16 Mar 2025 21:47:21 +0000 (17:47 -0400)]
Fix Guile pretty printer display hints
All 3 valid Guile pretty printer display hints are treated as the
value "string". As a result, if a printer specifies "array" or
"map", the output is instead formatted as a string.
ld/testsuite: add gnu property section in nto-stack-note*
A GNU property section is now always generated when `-z stack-size` is
passed. This was probably introduced by GNU Property refactoring
within elfxx-aarch64.c.
gdb/python: implement the print_insn extension language hook
added the gdb.disassembler.builtin_disassemble Python API function.
By mistake, the implementation accepted two arguments, the second
being a "memory_source".
However, this second argument was never used, it was left over from an
earlier proposed version of the API.
Luckily, the only place the unused argument was documented was in the
NEWS file and in the output of `help(gdb.builtin_disassemble)`, and
neither of these locations really describe what the argument was, or
how it would be used. The manual only describes the first (actually
used) argument, so I think we are safe enough to delete the unused
argument.
This allows some additional cleanup, with the store for the argument
also being deleted.
As the NEWS file did originally document the second argument, I have
added a NEWS entry to explain the argument has now been removed.
This could potentially break users code if they somehow decided to
pass a second argument, however, fixing things is as simple as
removing the second (unused) argument.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Andrew Burgess [Fri, 14 Feb 2025 11:51:41 +0000 (11:51 +0000)]
gdb/python: handle non-utf-8 character from gdb.execute()
I noticed that it was not possible to return a string containing non
utf-8 characters using gdb.execute(). For example, using the binary
from the gdb.python/py-source-styling.exp test:
(gdb) file ./gdb/testsuite/outputs/gdb.python/py-source-styling/py-source-styling
Reading symbols from ./gdb/testsuite/outputs/gdb.python/py-source-styling/py-source-styling...
(gdb) set style enabled off
(gdb) list 26
21 int some_variable = 1234;
22
23 /* The following line contains a character that is non-utf-8. This is a
24 critical part of the test as Python 3 can't convert this into a string
25 using its default mechanism. */
26 char c[] = "�"; /* List this line. */
27
28 return 0;
29 }
(gdb) python print(gdb.execute('list 26', to_string=True))
Python Exception <class 'UnicodeDecodeError'>: 'utf-8' codec can't decode byte 0xc0 in position 250: invalid start byte
Error occurred in Python: 'utf-8' codec can't decode byte 0xc0 in position 250: invalid start byte
It is necessary to disable styling before the initial 'list 26',
otherwise the source will be passed through GNU source highlight, and
GNU source highlight seems to be smart enough to figure out the
character encoding, and convert it to UTF-8. This conversion is then
cached in the source cache, and the later Python gdb.execute call will
get back a pure UTF-8 string.
If source styling is disabled, then GDB caches the string without the
conversion to UTF-8, now the gdb.execute call gets back the string
with a non-UTF-8 character within it, and Python throws an error
during its attempt to create a string object.
I'm not, at this point, proposing a solution that tries to guess the
source file encoding, though I guess such a thing could be done.
Instead, I think we should make use of the host_charset(), as set by
the user with 'set host-charset ....' during the creation of the
Python string.
To do this, in execute_gdb_command, we should switch from
PyUnicode_FromString, which requires the input be a UTF-8 string, to
using PyUnicode_Decode, which allows GDB to specify the string
encoding. We will use host_charset().
With this done, it is now possible to list the file contents using
gdb.execute(), with the contents passing through a string:
(gdb) set host-charset ISO-8859-1
(gdb) python print(gdb.execute('list 26', to_string=True), end='')
21 int some_variable = 1234;
22
23 /* The following line contains a character that is non-utf-8. This is a
24 critical part of the test as Python 3 can't convert this into a string
25 using its default mechanism. */
26 char c[] = "À"; /* List this line. */
27
28 return 0;
29 }
(gdb)
There are already plenty of other places in GDB's Python code where we
use PyUnicode_Decode to create a string from something that might
contain user generated content, so I believe this is the correct
approach.
H.J. Lu [Thu, 13 Mar 2025 18:52:00 +0000 (11:52 -0700)]
elf: Clear the SEC_ALLOC bit for NOLOAD note sections
When generating an ELF output file, if a note section is marked as
NOLOAD, clear the SEC_ALLOC bit so that it won't be treated as an
SHF_ALLOC section, like a .bss style section.
PR ld/32787
* ld.texi: Update NOLOAD for ELF output files.
* ldlang.c (lang_add_section): Clear the SEC_ALLOC bit for NOLOAD
note sections for ELF output files.
* testsuite/ld-elf/pr32787.d: New file.
* testsuite/ld-elf/pr32787.t: Likewise.
Tom Tromey [Tue, 11 Mar 2025 15:28:42 +0000 (09:28 -0600)]
Remove std::hash specialization
C++11 initially omitted specialization of std::hash for enumeration
types, but this was rectified in LWG issue 2148. This patch removes a
redundant specialization. Tested by rebuilding.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Simon Marchi [Fri, 14 Mar 2025 04:32:55 +0000 (00:32 -0400)]
gdb/dwarf: assume that no dwarf2_cu exist when calling load_full_comp_unit
After staring at the code, I got convinced that it was not possible for
load_full_comp_unit to be called while a dwarf2_cu object exists in
per_objfile for this_cu. If you follow all callers of
load_full_comp_unit, you can see that all calls to load_full_comp_unit
(except one, see below) are gated one way or another by the fact that:
per_objfile->get_cu (per_cu) == nullptr
Some calls are gated by maybe_queue_comp_unit returning true. If it
returns true, then necessarily the dwarf2_cu is unset for that per_cu.
The spot that didn't seem to check for whether the dwarf2_cu is already
set before calling load_full_comp_unit is dw2_do_instantiate_symtab. It
didn't trigger when running the testsuite, but I could imagine a made up
case where the dwarf2_cu would already be set because we looked up a DIE
reference to it (follow_die_ref) for whatever reason. Then, something
would cause the symtab for that CU to be expanded and
dw2_do_instantiate_symtab to be called.
I added a check in that function, because it seemed prudent to do so.
All other load_cu calls are gated by this check, so it makes this call
look just like the others.
Finally, because all call sites that use cutu_reader::release_cu pass
nullptr for `existing_cu` (and therefore cutu_reader creates a new
dwarf2_cu), we know that cutu_reader::release_cu will always return a
non-nullptr value. Add an assert in it and remove checks in
load_full_comp_unit and read_signatured_type.
Change-Id: I496be34bd4bf7edfa38d5135cf4bc4ccd960abe2 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Fri, 14 Mar 2025 04:32:53 +0000 (00:32 -0400)]
gdb/dwarf: assume that source_cu->dies is always set in follow_die_offset
After staring at the code for a while, I got convinced that it's not
possible for cu->dies to be nullptr in follow_die_offset. It might be a
leftover from the psymtab days.
In most cases, we see that the dwarf2_cu passedas `*ref_cu` has been
obtained by doing:
per_objfile->get_cu (per_cu);
The only way for a dwarf2_cu to end up in the per_objfile like this is
through load_full_comp_unit or read_signatured_type. Both of these
functions call `reader.read_all_dies ()` (which loads the DIEs in memory
and assigns dwarf2_cu::dies) before transferring the newly created
dwarf2_cu to the per_objfile. So any dwarf2_cu obtained through
per_objfile->get_cu (per_cu)
... will have its DIEs set.
The only case today I'm aware of of a dwarf2_cu without DIEs is in the
cooked indexer. It creates a cutu_reader, but does not call
read_all_dies. Instead, it gets the info_ptr from the cutu_reader and
reads the DIEs from the section buffer directly, on its own. But this
is an entirely different code path that doesn't assign dwarf2_cu
objects to per_objfile.
So, remove the code path in follow_die_offset that tests for
`source_cu->dies == NULL`. I added an assert at the top of the function
to verify that `source_cu->dies` is always non-nullptr, as a way to
test my hypothesis. We could probably get rid of it, but I left it
there because it doesn't cost much to have it.
Change-Id: I97f269f092128800850aa5e64eda7032c2edec60 Approved-By: Tom Tromey <tom@tromey.com>
This is a bit subjective, but I often struggle to understand what
cutu_reader::keep is meant to do (keep what, where). Perhaps it's just
a question of bad naming, but I think it's a bit confusing for
cutu_reader to transfer the ownership of the dwarf2_cu to the
per_objfile directly.
Add the cutu::release_cu method and make the caller of cutu_reader
transfer the ownership to the per_objfile object.
Right now, it is theoretically possible for release_cu to return
nullptr, so I made callers check the return value. A patch later in
this series will change release_cu to ensure it always return
non-nullptr, so those callers will get simplified.
Change-Id: I3103ff894d1654a95c9d69001073c218501c988a Approved-By: Tom Tromey <tom@tromey.com>
I don't really like this poking into cutu_reader's data structures from
the outside, I would prefer if that work was done by cutu_reader.
Rename the read_die_and_siblings method to read_all_dies, and do that
work inside cutu_reader.
I also moved these operations inside the read_all_dies method:
The rationale for this is that read_all_dies (and the functions it
calls) is responsible for filling the die_hash set. So I think it makes
sense for it to do the reserve.
It is also cutu_reader's job, currently, to create and fill the fields
of dwarf2_cu. So I think it makes sense for it to set cu->dies, after
having read the DIEs in memory.
Change-Id: I088c2e0b367db7d1f67e8c9e2d5b0d61165292fc Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Fri, 14 Mar 2025 04:32:47 +0000 (00:32 -0400)]
gdb/dwarf: access m_info_ptr directly instead of passing info_ptr around
The few methods of cutu_reader that read DIEs into memory generally
receive an info_ptr that says where to start reading and return another
one (either by return value or parameter) indicating where the caller
should continue reading.
We can avoid all this passing around by having these methods access
m_info_ptr directly. This allows changing some methods that read DIEs
to return `die_info *`, instead of returning it by parameter, which just
makes the code simpler to read, I think.
The only method that meaningfully reads and writes m_info_ptr (except
the places that initially set it up) is read_full_die_1. It reads and
increments m_info_ptr once to read the abbrev and once again to read
each attribute. Other methods use it for logging.
The methods cutu_reader::read_attribute and
cutu_reader::read_attribute_value do not touch m_info_ptr directly,
because they are used in cooked-indexer.c, which appears to read some
things in a non-linear fashion, unlike cutu_reader's DIE-reading
methods. The cooked indexer calls cutu_reader::info_ptr to get the
m_info_ptr value just after the top-level DIE, and then it does its own
attribute reading after that.
Change-Id: I251f63d13d453a2827b21349760da033171880e2 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Fri, 14 Mar 2025 04:32:46 +0000 (00:32 -0400)]
gdb/dwarf: factor out to cutu_reader::skip_one_attribute method
I was reading cutu_reader::skip_one_die, and thought that the code to
skip one attribute made it quite difficult to read. Factor this code
out to a new method, to get it out of the way.
As a bonus, it transforms one goto in a recursion call, which is also
easier to follow. Unfortunately, I have no idea how to test
DW_FORM_indirect, as it doesn't seem to appear anywhere in the
testsuite, and I don't think that compilers often emit that.
Change-Id: I2257b3e594aafb7c7da52ddd55baa651cefb802f Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Fri, 14 Mar 2025 04:32:45 +0000 (00:32 -0400)]
gdb/dwarf: remove pretend_language parameter from load_full_{comp,type}_unit
I noticed that load_full_comp_unit and load_full_type_unit didn't use
their pretend_language parameter. Remove them, and then remove more
things that were needed to get the language value to that point,
including the dwarf2_queue_item field.
Change-Id: Ie8cb21c54ae49da065a1b0a20bf18ccb93961d1a Approved-By: Tom Tromey <tom@tromey.com>
Richard Allen [Sat, 8 Mar 2025 08:44:18 +0000 (16:44 +0800)]
gprof: only process line numbers for intersection of vmas and histograms
Some programs like RTOS firmware may have a large number of symbols.
The profile information in the profile data file includes histogram
records, which capture low PC and high PC of program execution. If all
histogram records come in the profile data file before any call-graph
records and basic-block records, we can look up only the line numbers
within low PC and high PC in histogram records, which reduces processing
time for such a firmware from ~2 minutes to ~2 seconds.
Add symbol table access function, get_symtab, get_symtab_direct and
set_symtab to delay loading the symbol table until its first use.
* aarch64.c (aarch64_find_call): Call get_symtab to get the
symbol table pointer
* alpha.c (alpha_find_call): Likewise.
* basic_blocks.c (bb_read_rec): Likewise.
(bb_write_blocks): Likewise.
(print_exec_counts): Likewise.
(print_annotated_source): Likewise.
* call_graph.c (cg_tally): Likewise.
(cg_write_arcs): Likewise.
* cg_arcs.c (cycle_link): Likewise.
(propagate_flags): Likewise.
(cg_assemble): Likewise.
* cg_print.c (cg_print): Likewise.
(cg_print_index): Likewise.
(cg_print_function_ordering): Likewise.
* corefile.c: Include "gmon_io.h".
(core_create_syms_from): Call get_symtab_direct to get the
symbol table pointer.
(core_create_function_syms): Likewise.
(core_create_line_syms): Likewise. If all histogram records
come in the profile data file before any call-graph records and
basic-block records, we can look up only the line numbers within
low PC and high PC in histogram records.
* gmon_io.c (gmon_histograms_first): New.
(gmon_out_read): Set gmon_histograms_first to true if all
histogram records come first.
(gmon_out_write): Call get_symtab to get the symbol table
pointer.
* hist.c (scale_and_align_entries): Likewise.
(hist_assign_samples_1): Likewise.
(hist_print): Likewise.
* i386.c (i386_find_call): Likewise.
* mips.c (mips_find_call): Likewise.
* sparc.c (sparc_find_call): Likewise.
* sym_ids.c (sym_id_parse): Likewise.
* vax.c (vax_find_call): Likewise.
* gmon_io.h (gmon_histograms_first): New.
* gprof.c (man): Don't create profile info.
(symtab_init): New.
* gprof.h (symtab_init): New.
* symtab.c (symtab): Changed to static.
(get_symtab_direct): New.
(get_symtab): Likewise.
(set_symtab): Likewise.
* symtab.h (symtab): Removed.
(get_symtab_direct): New.
(get_symtab): Likewise.
(set_symtab): Likewise.
Signed-off-by: Richard Allen <rsaxvc@gmail.com> Co-Authored-By: H.J. Lu <hjl.tools@gmail.com>
Jan Beulich [Fri, 14 Mar 2025 09:32:42 +0000 (10:32 +0100)]
gas: permit wider-than-byte operands for .cfi_escape
Some DW_CFA_* and DW_OP_* take wider than byte, but non-LEB128 operands.
Having to hand-encode such when needing to resort to .cfi_escape isn't
very helpful.
Jan Beulich [Fri, 14 Mar 2025 09:30:47 +0000 (10:30 +0100)]
gas: make NO_LISTING work again
Presumably since no target enables this and there's also no configure
control, builds with NO_LISTING defined didn't really work anymore.
Convert fallback functions to macros and add #ifndef in a few places.
(Behavior is different for affected command line options vs directives:
The former are rejected as unrecognized, while the latter are silently
ignored. I think that's fair enough.)
Jan Beulich [Fri, 14 Mar 2025 09:30:18 +0000 (10:30 +0100)]
gas: include .cfi_* generated data in listing
These are data generating directives not overly different from e.g.
.byte and .long. Whatever (directly) results from should also be
represented in the listing, if one was requested. It's just that the
output data is generated much later than the parsing of the directive
arguments.
Jan Beulich [Fri, 14 Mar 2025 09:29:33 +0000 (10:29 +0100)]
gas: deal with the need for relocations from .cfi_{escape,fde_data}
Ignoring return values often isn't a good idea. The Sparc assembler in
particular would report an internal error if an expression with
relocation specifier is used with .cfi_escape, when the same works fine
with .byte. Propagate the relocation indicator up from
do_parse_cons_expression(), and eventually into emit_expr_with_reloc().
dot_cfi_fde_data(), only retaining the expression's X_add_number, would
require further work. Simply report the lack of support there. While
there, also check that what we were dealt is actually a constant.
Simon Marchi [Tue, 11 Mar 2025 19:02:47 +0000 (15:02 -0400)]
gdb/dwarf: keep going even if reading macro information fails
On Debian 12, with gcc 12 and ld 2.40, I get some failures when running:
$ make check TESTS="gdb.base/style.exp" RUNTESTFLAGS="--target_board=fission"
I think I stumble on this bug [1], preventing the test from doing
anything that requires expanding the compilation unit:
$ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.base/style/style
Reading symbols from testsuite/outputs/gdb.base/style/style...
(gdb) p main
DW_FORM_strp pointing outside of .debug_str section [in module /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/style/style]
(gdb)
The error is thrown here:
#0 0x00007ffff693f0a1 in __cxa_throw () from /lib/x86_64-linux-gnu/libstdc++.so.6
#1 0x0000555569ce6852 in throw_it(return_reason, errors, const char *, typedef __va_list_tag __va_list_tag *) (reason=RETURN_ERROR, error=GENERIC_ERROR, fmt=0x555562a9fc40 "%s pointing outside of %s section [in module %s]", ap=0x7fffffff8df0) at /home/smarchi/src/binutils-gdb/gdbsupport/common-exceptions.cc:203
#2 0x0000555569ce690f in throw_verror (error=GENERIC_ERROR, fmt=0x555562a9fc40 "%s pointing outside of %s section [in module %s]", ap=0x7fffffff8df0) at /home/smarchi/src/binutils-gdb/gdbsupport/common-exceptions.cc:211
#3 0x000055556879c0cb in verror (string=0x555562a9fc40 "%s pointing outside of %s section [in module %s]", args=0x7fffffff8df0) at /home/smarchi/src/binutils-gdb/gdb/utils.c:193
#4 0x0000555569cfa88d in error (fmt=0x555562a9fc40 "%s pointing outside of %s section [in module %s]") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:45
#5 0x000055556667dbff in dwarf2_section_info::read_string (this=0x61b000042a08, objfile=0x616000055e80, str_offset=262811, form_name=0x555562886b40 "DW_FORM_strp") at /home/smarchi/src/binutils-gdb/gdb/dwarf2/section.c:211
#6 0x00005555662486b7 in dwarf_decode_macro_bytes (per_objfile=0x616000056180, builder=0x614000006040, abfd=0x6120000f4b40, mac_ptr=0x60300004f5be "", mac_end=0x60300004f5bb "\002\004", current_file=0x62100007ad70, lh=0x60f000028bd0, section=0x61700008ba78, section_is_gnu=1, section_is_dwz=0, offset_size=4, str_section=0x61700008bac8, str_offsets_section=0x61700008baf0, str_offsets_base=std::optional<unsigned long> = {...}, include_hash=..., cu=0x61700008b600) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/macro.c:511
#7 0x000055556624af0e in dwarf_decode_macros (per_objfile=0x616000056180, builder=0x614000006040, section=0x61700008ba78, lh=0x60f000028bd0, offset_size=4, offset=0, str_section=0x61700008bac8, str_offsets_section=0x61700008baf0, str_offsets_base=std::optional<unsigned long> = {...}, section_is_gnu=1, cu=0x61700008b600) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/macro.c:934
#8 0x000055556642cb82 in dwarf_decode_macros (cu=0x61700008b600, offset=0, section_is_gnu=1) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:19435
#9 0x000055556639bd12 in read_file_scope (die=0x6210000885c0, cu=0x61700008b600) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:6366
#10 0x0000555566392d99 in process_die (die=0x6210000885c0, cu=0x61700008b600) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:5310
#11 0x0000555566390d72 in process_full_comp_unit (cu=0x61700008b600, pretend_language=language_minimal) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:5075
The exception is then only caught at the event-loop level
(start_event_loop), causing the whole debug info reading process to be
aborted. I think it's a little harsh, considering that a lot of things
could work even if we failed to read macro information.
Catch the exception inside read_file_scope, print the exception, and
carry on. We could go even more fine-grained: if reading the string for
one macro definition fails, we could continue reading the macro
information. Perhaps it's just that one macro definition that is
broken. However, I don't need this level of granularity, so I haven't
attempted this. Also, my experience is that macro reading fails when
the compiler or linker has a bug, in which case pretty much everything
is messed up.
With this patch, it now looks like:
$ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.base/style/style
Reading symbols from testsuite/outputs/gdb.base/style/style...
(gdb) p main
While reading section .debug_macro.dwo: DW_FORM_strp pointing outside of .debug_str section [in module /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/style/style]
$1 = {int (int, char **)} 0x684 <main>
(gdb)
In the test I am investigating (gdb.base/style.exp with the fission
board), it allows more tests to run:
-# of expected passes 107
-# of unexpected failures 17
+# of expected passes 448
+# of unexpected failures 19
Of course, we still see the error about the macro information, and some
macro-related tests still fail (those would be kfailed ideally), but
many tests that are not macro-dependent now pass.
The regexp in get_single_disassembled_insn fails to match, the insn
variable doesn't get set, and we get one of those unreadable TCL stack
traces:
ERROR: tcl error sourcing /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/style.exp.
ERROR: tcl error code TCL READ VARNAME
ERROR: can't read "insn": no such variable
while executing
"return $insn"
(procedure "get_single_disassembled_insn" line 4)
invoked from within
"get_single_disassembled_insn"
("uplevel" body line 18)
invoked from within
"uplevel 1 $body"
invoked from within
...
Check the return value of the regexp call, return an empty string on
failure. Log a failure, so that we have a trace that something went
wrong, in case the tests done by the caller happen to pass by change.
Andrew Burgess [Thu, 6 Mar 2025 15:50:43 +0000 (15:50 +0000)]
gcore/doc: fix mistake in the gcore man page
The gcore man page says that the default prefix for a generated core
file will be 'gcore', i.e. we'll create files like 'gcore.pid'. In
reality the default is 'core'.
As far as I can tell, the default has been 'core' for years, and the
docs used to say that the default was 'core', but the docs were
changed by mistake in commit:
And adds -h | --help options to the gcore script, and smartens up the
help and usage output messages.
The usage text is now split over several lines (as it was getting a
bit long), and an input error suggests using `--help` instead of
printing the full usage string.
These changes bring gcore and gstack closer in behaviour.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32325 Approved-By: Tom Tromey <tom@tromey.com> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
This commit adds a '-v' or '--version' option to the existing gcore
script. This new option causes the script to print its version
number, and then exit.
I needed to adjust the getopts handling a little in order to support
the long form '--version' argument, but as this makes gcore more
consistent with gstack, then this seems like a good thing.
The usage message is now getting a little long. Don't worry, I plan
to clean that up in the next commit.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32325 Approved-By: Tom Tromey <tom@tromey.com> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Tom de Vries [Thu, 13 Mar 2025 06:49:33 +0000 (07:49 +0100)]
[gdb/record] Fix out-of-bounds write in aarch64_record_asimd_load_store
After compiling gdb with -fstack-protector-all, and running test-case
gdb.reverse/getrandom.exp on aarch64-linux, we run into
"Stack smashing detected" in function aarch64_record_asimd_load_store.
This is reported in PR record/32784.
This happens due to an out-of-bounds write to local array record_buf_mem:
...
uint64_t record_buf_mem[24];
...
when recording insn:
...
B+>0xfffff7ff4d10 st1 {v0.16b-v3.16b}, [x0]
...
We can fix this by increasing the array size to 128, but rather than again
hardcoding a size, reimplement record_buf_mem as std::vector.
Tom de Vries [Thu, 13 Mar 2025 06:41:51 +0000 (07:41 +0100)]
[gdb/record] Support recording syscall accept4
While reviewing the enum gdb_syscall entries with values >= 500, I noticed
that gdb_sys_accept exists, but gdb_sys_accept4 doesn't, while recording
support is essentially the same, given that the difference in interface is
only an extra int parameter:
...
int accept (int sockfd, struct sockaddr *addr, socklen_t *addrlen);
int accept4 (int sockfd, struct sockaddr *addr, socklen_t *addrlen, int flags);
...
Fix this by:
- adding gdb_sys_accept4,
- supporting it in record_linux_system_call alongside gdb_sys_accept, and
- mapping to gdb_sys_accept4 in various syscall canonicalization functions.
The usual thing to do before the rewrite of i386_canonicalize_syscall would
have been to use the value from arch/x86/entry/syscalls/syscall_32.tbl:
...
gdb_sys_accept4 = 364,
...
but that's no longer necessary, so instead we use some >= 500 value:
...
gdb_sys_accept4 = 533,
...
to steer clear of the space where ppc_canonicalize_syscall and
s390_canonicalize_syscall do hard-coded number magic.
Tested on x86_64-linux, with and without target board unix/-m32, and
aarch64-linux.
Tom de Vries [Thu, 13 Mar 2025 06:41:51 +0000 (07:41 +0100)]
[gdb/tdep] Rewrite i386_canonicalize_syscall
On openSUSE Tumbleweed x86_64, with target board unix/-m32 and test-case
gdb.reverse/recvmsg-reverse.exp, I run into:
...
(gdb) continue^M
Continuing.^M
Process record and replay target doesn't support syscall number 360^M
Process record: failed to record execution log.^M
^M
Program stopped.^M
0xf7fc5575 in __kernel_vsyscall ()^M
(gdb) FAIL: $exp: continue to breakpoint: marker2
...
The syscall number 360 in i386 is for syscall socketpair, as we can see in
arch/x86/entry/syscalls/syscall_32.tbl:
...
<number> <abi> <name> <entry point>
360 i386 socketpair sys_socketpair
...
Function i386_canonicalize_syscall assumes that any syscall below 500 maps to
an identically valued enum in enum gdb_syscall:
...
static enum gdb_syscall
i386_canonicalize_syscall (int syscall)
{
enum { i386_syscall_max = 499 };
However, that's not the case. The value of gdb_sys_socketpair is not 360,
but 512:
...
enum gdb_syscall {
...
gdb_sys_getrandom = 355,
gdb_sys_statx = 383,
...
gdb_sys_socketpair = 512,
...
Consequently, when record_linux_system_call is called with
syscall == i386_canonicalize_syscall (360), we hit the default case here:
....
switch (syscall)
{
...
default:
gdb_printf (gdb_stderr,
_("Process record and replay target doesn't "
"support syscall number %d\n"), syscall);
return -1;
break;
}
...
rather than hitting the case for gdb_sys_socketpair.
I initially wrote a trivial fix for this, changing the value of
gdb_sys_socketpair to 360. However, Andreas Schwab pointed out that there are
other functions (ppc_canonicalize_syscall and s390_canonicalize_syscall) that
make assumptions about specific values of enum gdb_syscall, and fixing this
for i386 may break things for ppc or s390.
So instead, I decided to rewrite i386_canonicalize_syscall to match the
approach taken in aarch64_canonicalize_syscall, which allows
gdb_sys_socketpair to keep the same value.
So, fix this by:
- adding a new table file gdb/i386-syscalls.def, using a SYSCALL entry for
each syscall, generated from arch/x86/entry/syscalls/syscall_32.tbl,
- using gdb/i386-syscalls.def to define enum i386_syscall, and
- using macros SYSCALL_MAP, SYSCALL_MAP_RENAME and UNSUPPORTED_SYSCALL_MAP to
define the mapping from enum i386_syscall to enum gdb_syscall in
i386_canonicalize_syscall.
I've created the mapping as follows:
- I used arch/x86/entry/syscalls/syscall_32.tbl to generate an initial mapping
using SYSCALL_MAP for each syscall,
- I attempted to compile this and used the compilation errors about
non-existing gdb_sys_ values to change those entries to
UNSUPPORTED_SYSCALL_MAP, which got me a compiling version,
- I reviewed the UNSUPPORTED_SYSCALL_MAP entries, changing to
SYSCALL_MAP_RENAME where necessary,
- I then reviewed syscalls below 500 that mapped to a gdb_syscall value below
500, but not the same, and fixed those using SYSCALL_MAP_RENAME, and
- reviewed the mapping for gdb_syscall entries >= 500.
On the resulting mapping, I was able to do the following sanity check:
...
for (int i = 0; i < 500; ++i)
{
int res = i386_canonicalize_syscall (i);
if (res == i)
continue;
if (res == -1)
continue;
if (res >= 500)
continue;
gdb_assert_not_reached ("");
}
}
...
to make sure that any syscall below 500 either:
- maps to the same number,
- is unsupported, or
- maps to a number >= 500.
Coming back to our original problem, the socket pair syscall is addressed by
an entry:
...
SYSCALL_MAP (socketpair);
...
which maps i386_sys_socketpair (360) to gdb_sys_socketpair (512).
Tested on x86_64-linux with target board unix/-m32.
Simon Marchi [Thu, 13 Mar 2025 02:56:33 +0000 (22:56 -0400)]
gdb/dwarf: use all_units_range in dwarf2_base_index_functions::expand_all_symtabs
Commit 292041562289 ("gdb/dwarf: use ranged for loop in some spots")
broke some tests notably gdb.base/maint.exp with the fission board.
$ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.base/maint/maint -ex start -ex "maint expand-sym" -batch
...
Temporary breakpoint 1, main (argc=1, argv=0x7fffffffdc48, envp=0x7fffffffdc58) at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/break.c:43
43 if (argc == 12345) { /* an unlikely value < 2^16, in case uninited */ /* set breakpoint 6 here */
/usr/include/c++/14.2.1/debug/safe_iterator.h:392:
In function:
gnu_debug::_Safe_iterator<_Iterator, _Sequence, _Category>&
gnu_debug::_Safe_iterator<_Iterator, _Sequence, _Category>::operator++()
[with _Iterator = gnu_cxx::
normal_iterator<std::unique_ptr<dwarf2_per_cu, dwarf2_per_cu_deleter>*,
std::vector<std::unique_ptr<dwarf2_per_cu, dwarf2_per_cu_deleter>,
std::allocator<std::unique_ptr<dwarf2_per_cu, dwarf2_per_cu_deleter> > >
>; _Sequence = std::debug::vector<std::unique_ptr<dwarf2_per_cu,
dwarf2_per_cu_deleter> >; _Category = std::forward_iterator_tag]
Error: attempt to increment a singular iterator.
Note that this is caught because I build with -D_GLIBCXX_DEBUG=1.
Otherwise, it might crash more randomly, or just not crash at all (but
still be buggy).
While iterating on the all_units vector, some type units get added
there:
#0 add_type_unit (per_bfd=0x51b000044b80, section=0x50e0000c2280, sect_off=0, length=74, sig=4367013491293299229) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:2576
#1 0x00005555618a3a40 in lookup_dwo_signatured_type (cu=0x51700009b580, sig=4367013491293299229) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:2664
#2 0x00005555618ee176 in queue_and_load_dwo_tu (dwo_unit=0x521000120e00, cu=0x51700009b580) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:8329
#3 0x00005555618eeafe in queue_and_load_all_dwo_tus (cu=0x51700009b580) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:8366
#4 0x00005555618966a6 in dw2_do_instantiate_symtab (per_cu=0x50f0000043c0, per_objfile=0x516000065a80, skip_partial=true) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:1695
#5 0x00005555618968d4 in dw2_instantiate_symtab (per_cu=0x50f0000043c0, per_objfile=0x516000065a80, skip_partial=true) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:1719
#6 0x000055556189ac3f in dwarf2_base_index_functions::expand_all_symtabs (this=0x502000024390, objfile=0x516000065780) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:1977
This invalidates the iterator in
dwarf2_base_index_functions::expand_all_symtabs, which is caught by the
libstdc++ debug mode.
I'm not entirely sure that it is correct to append type units from dwo
files to the all_units vector like this. The
dwarf2_find_containing_comp_unit function expects a precise ordering of
the elements of the all_units vector, to be able to do a binary search.
Appending a type unit at the end at this point certainly doesn't respect
that ordering.
For now I'd just like to undo the regression. Do that by using
all_units_range in the ranged for loop. I will keep in mind to
investigate whether this insertion of type units in all_units after the
fact really makes sense or not.
Tom de Vries [Tue, 11 Mar 2025 08:38:50 +0000 (09:38 +0100)]
[gdb/testsuite] Fix gdb.base/step-over-syscall.exp with glibc 2.41
On openSUSE Tumbleweed, with glibc 2.41, when running test-case
gdb.base/step-over-syscall.exp I run into:
...
(gdb) stepi^M
0x00007ffff7cfd09b in __abort_lock_rdlock () from /lib64/libc.so.6^M
1: x/i $pc^M
=> 0x7ffff7cfd09b <__abort_lock_rdlock+29>: syscall^M
(gdb) p $eax^M
$1 = 14^M
(gdb) FAIL: $exp: fork: displaced=off: syscall number matches
FAIL: $exp: fork: displaced=off: find syscall insn in fork (timeout)
...
We're stepi-ing through fork trying to find the fork syscall, but encounter
another syscall.
The test-case attempts to handle this:
...
gdb_test_multiple "stepi" "find syscall insn in $syscall" {
-re ".*$syscall_insn.*$gdb_prompt $" {
# Is the syscall number the correct one?
if {[syscall_number_matches $syscall]} {
pass $gdb_test_name
} else {
exp_continue
}
}
-re "x/i .*=>.*\r\n$gdb_prompt $" {
incr steps
if {$steps == $max_steps} {
fail $gdb_test_name
} else {
send_gdb "stepi\n"
exp_continue
}
}
}
...
but fails to do so because it issues an exp_continue without issuing a new
stepi command, and consequently the "find syscall insn in fork" test times
out.
Also, the call to syscall_number_matches produces a PASS or FAIL, so skipping
one syscall would produce:
...
FAIL: $exp: fork: displaced=off: syscall number matches
PASS: $exp: fork: displaced=off: syscall number matches
DUPLICATE: $exp: fork: displaced=off: syscall number matches
...
Fix this by:
- not producing PASS or FAIL in syscall_number_matches, and
- issuing stepi when encountering another syscall.
While we're at it, fix indentation in syscall_number_matches.
Tested on x86_64-linux, specifically:
- openSUSE Tumbleweed (glibc 2.41), and
- openSUSE Leap 15.6 (glibc 2.38).
projects / thirdparty / binutils-gdb.git / log
