Transform "typedef void (*foo) ()" into "using foo = void (*) ()".
Generated by a script written by Claude Code.
Approved-By: Tom Tromey <tom@tromey.com>
I was reminded that while we turned off ARI reports a little while
ago, we never did remove ARI from gdb itself. This patch does so.
ARI was an attempt at enforcing some gdb coding standards. However,
it seemed to get in the way more than it helped.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33842
Reviewed-By: Tom de Vries <tdevries@suse.de>
After commit 1eed06ae51 ("[gdbsupport] Use using instead of typedef in
next_iterator") I wondered if I could do something similar using sed.
Result of:
...
$ find gdb* -type f -name "*.[ch]" -o -name "*.cc" \
| egrep -v /testsuite/ \
| xargs sed -i \
's/^\([ \t]*\)typedef \([a-zA-Z_0-9:<>,.() ]*\) \([\*&][\*&]*\)\([a-zA-Z_0-9]*\);/\1using \4 = \2 \3;/'
$ find gdb* -type f -name "*.[ch]" -o -name "*.cc" \
| egrep -v /testsuite/ \
| xargs sed -i \
's/^\([ \t]*\)typedef \([a-zA-Z_0-9:<>,.() ]*\) \([a-zA-Z_0-9]*\);/\1using \3 = \2;/'
...
Tested on x86_64-linux.
I came across "get_selected_frame (nullptr)", and realized nullptr is the
default argument, so this can be simplified to "get_selected_frame ()".
Do so using the following script:
...
$ find gdb* -type f -name "*.[ch]" -o -name "*.cc" \
| xargs sed -i 's/get_selected_frame (nullptr)/get_selected_frame ()/'
...
and likewise for "get_selected_frame (NULL)".
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
Doxygen supports a multi-line comment marker '/**'. In GDB we're using
something slightly similar: '/* *' [1].
Drop this and just use '/*'.
Result of:
...
$ find gdb* -type f -name "*.[ch]" -o -name "*.def" \
| egrep -v /testsuite/ \
| xargs sed -i 's%/\* \* %/* %'
...
and manually reverting the change in the comment for BINOP_MUL in
gdb/std-operator.def.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34186
Approved-By: Kevin Buettner <kevinb@redhat.com>
Approved-By: Tom Tromey <tom@tromey.com>
[1] https://sourceware.org/gdb/wiki/DoxygenForGDB
This reverts commit b083976272.
The new assert triggers in test-cases gdb.ada/uninitialized-variable-record.exp and
gdb.ada/uninitialized_vars.exp on x86_64-linux.
C++ always allows enum bitfields, so gdb no longer needs to use the
ENUM_BITFIELD macro.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32821
Approved-By: Andrew Burgess <aburgess@redhat.com>
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
I wrote a patch containing:
...
dw2_instantiate_symtab (cu->per_cu, ...);
...
and ran into a use-after-free at a following use of cu.
The problem is that dw2_instantiate_symtab contains:
...
free_cached_comp_units freer (per_objfile);
...
and that the destructor does:
...
~free_cached_comp_units ()
{
m_per_objfile->remove_all_cus ();
}
...
which also frees the cu we used in the cu->per_cu argument to
dw2_instantiate_symtab.
Detect this situation using an assert in the free_cached_comp_units
constructor.
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
The call to bfd_check_format in try_open_dwop_file:
...
/* The operations below are not thread-safe, use a lock to synchronize
concurrent accesses. */
static gdb::mutex mutex;
gdb::lock_guard<gdb::mutex> lock (mutex);
if (!bfd_check_format (sym_bfd.get (), bfd_object))
return NULL;
...
accesses the sym_bfd.get () BFD, so it should be guarded by the global BFD
lock.
Fix this by:
- using the global BFD lock in gdb_bfd_check_format, and
- removing the local lock.
Likewise, use the global BFD lock in gdb_bfd_check_format_matches.
The local lock also guarded a call to gdb_bfd_record_inclusion, which doesn't
do any locking, so likewise use the global BFD lock in
gdb_bfd_record_inclusion.
Approved-By: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33809
Add new bfd wrappers:
- gdb_bfd_check_format for bfd_check_format, and
- gdb_bfd_check_format_matches for bfd_check_format_matches.
and run this command to use them:
...
$ sed -i 's/bfd_check_format/gdb_bfd_check_format/' \
$(find gdb* -type f | egrep -v "/testsuite/|ChangeLog")
...
No functional changes.
Approved-By: Tom Tromey <tom@tromey.com>
This method can't return nullptr, switch it to return a reference.
Change all_units_iterator::operator* too.
Change-Id: I15c945553abfebdcc8834438a3b45d9895d628f0
Approved-By: Tom Tromey <tom@tromey.com>
dwarf2_find_unit performs a binary search over dwarf2_per_bfd::all_units
using std::lower_bound. Before calling std::lower_bound, we want to
make sure that all_units is properly sorted.
Track the state of whether all_units is considered sorted with a new
dwarf2_per_bfd::all_units_sorted flag, and assert it in
dwarf2_find_unit. This will help catch bugs where we call
dwarf2_find_unit on a non sorted vector, for instance what was fixed by
commit fbaef7de77 ("gdb/dwarf: fix order of operations when reading
.debug_names").
The flag is set:
- At initialization time, because an empty vector is sorted.
- Whenever the vector is cleared, for the same reason (added a helper
method for that).
- After sorting in dwarf2_per_bfd::sort_all_unit.
It is cleared:
- Whenever a unit is appended to all_units. To keep this centralized,
add a new dwarf2_per_bfd::add_unit helper that appends the unit and
clears the flag.
- Whenever a unit's sort key (section / sect_off) is modified. That is
done in dwarf2_per_cu::set_section and dwarf2_per_cu::set_sect_off.
Note that the flag is cleared when appending a unit even if the vector
would by chance already be sorted. This is good, because it will catch
mistakes even on machines where a problem with std::lower_bound would
not manifest.
I checked that this patch would have caught the problem before commit
fbaef7de77:
(gdb) file /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/debug-names-bad-cu-index/debug-names-bad-cu-index
Reading symbols from /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/debug-names-bad-cu-index/debug-names-bad-cu-index...
/home/simark/src/binutils-gdb/gdb/dwarf2/read.c:18092: internal-error: dwarf2_find_unit: Assertion `per_bfd->all_units_sorted' failed.
Change-Id: I9b3e52bb33b02763594efa381761f383ee344317
Approved-By: Tom Tromey <tom@tromey.com>
We currently have:
using dwarf2_per_cu_up = std::unique_ptr<dwarf2_per_cu, dwarf2_per_cu_deleter>;
using signatured_type_up = std::unique_ptr<signatured_type>;
Meaning that it's not possible to pass a signatured_type_up as a
dwarf2_per_cu_up, even though the target types are related (it is
possible to pass a `signatured_type *` as a `dwarf2_per_cu *`).
If we give signatured_type_up the same deleter as dwarf2_per_cu_up, then
it becomes possible to pass a signatured_type_up as a dwarf2_per_cu_up.
This lets us avoid releasing a signatured_type_up only to create a
dwarf2dwarf2_per_cu_up immediately after in some spots. The only
downside is that we can't use make_unique anymore, but it's already the
case for dwarf2_per_cu_up.
Swap the order of things in add_type_unit so that we don't need a
special holder variable.
Change-Id: Iee34e5d1711d601297f109e58cbaeccb5a0c6cde
Approved-By: Tom Tromey <tom@tromey.com>
PR 34149 shows a hard to reproduce bug. The problem sometimes shows up
when reading in `.debug_names` indexes, manifesting like this:
warning: Section .debug_names has incorrect entry in CU table, ignoring .debug_names.
This is shown even if the CU table in the `.debug_names` index
accurately describes the CUs in `.debug_info`.
The bug was introduced by commit b301725d35 ("gdb/dwarf: read foreign
type units").
Before b301725d35, the steps when reading a `.debug_names` index were:
- call to create_all_units(), which filled the all_units vector by
walking `.debug_info` and `.debug_types` and then sorted it.
- calls to the build_and_check_* functions, which do some lookups in
the all_units vector, requiring it to be sorted.
All good. Post-b301725d35c1, we have:
- call to create_all_units(), which still fills the all_units vector by
walking `.debug_info` and `.debug_types`, but does not sort it
anymore.
- calls to the build_and_check_* functions, which do some lookups in
the all_units vector, requiring it to be sorted (oops)
- call to create_foreign_type_units_from_debug_names(), which may add
more items to the all_units vector.
- call to finalize_all_units(), to sort the all_units vector.
The sorting of all_units was taken out of create_all_units() on purpose.
Because create_foreign_type_units_from_debug_names() may add more units,
we don't want to sort the vector in create_all_units() and then sort it
again after create_foreign_type_units_from_debug_names().
The build_and_check_* functions do some lookups in the all_units vector
with dwarf2_find_unit(), which uses std::lower_bound() internally. This
happens while the all_units vector is not sorted according to the
sorting key, all_units_less_than(), which makes the search is bogus.
dwarf2_find_unit() might return "no such unit", when in fact, the given
unit exists. This leads build_and_check_cu_list_from_debug_names() to
believe that the list of CUs in the `.debug_names` index does not match
what was built from reading `.debug_info`, we get the warning shown
above, and the index gets wrongfully rejected.
For this bug to show up, it is necessary to have type units in a
`.debug_types`. The sorting key for units in the all_units vector is:
section pointer (`dwarf2_section_info *`), then the unit offset into the
section. If all units come from the same section (`.debug_info`), then
this bug does not show up because the all_units vector happens to be
sorted after create_all_units() runs. This happens for example in test
gdb.dwarf2/debug-names-bad-cu-index.exp, where we create some DWARF 4
units and then a `.debug_names` index out of them.
It must also happen for the `dwarf2_section_info *` for `.debug_types`
to be "less than" the `dwarf2_section_info *` for `.debug_info`,
otherwise the all_units also happens to be sorted after
create_all_units() runs. This entirely depends on the whims of the
memory allocator.
I was not able to reproduce it in my normal GDB dev build, but I was
able in a "thread sanitizer" build, for some reason. Not because there
is an actual threading issue, but I think because it shuffled the memory
allocations in just the right way to make the bug happen. And then, it
only happened when doing "make check", I could not get it to reproduce
when running interactively. Using -D_GLIBCXX_DEBUG was immensely
useful, because it pointed out that the "range must be sorted"
precondition for std::lower_bound() was not met, which made the problem
immediately obvious:
(gdb) file /home/simark/build/binutils-gdb-tsan/gdb/testsuite/outputs/gdb.dwarf2/debug-names-bad-cu-index/debug-names-bad-cu-index
Reading symbols from /home/simark/build/binutils-gdb-tsan/gdb/testsuite/outputs/gdb.dwarf2/debug-names-bad-cu-index/debug-names-bad-cu-index...
/usr/include/c++/16.1.1/bits/stl_algo.h:1981:
In function:
constexpr _FIter std::lower_bound(_FIter, _FIter, const _Tp&, _Compare)
[with _FIter = gnu_debug::_Safe_iterator<gnu_cxx::
normal_iterator<unique_ptr<dwarf2_per_cu, dwarf2_per_cu_deleter>*,
vector<unique_ptr<dwarf2_per_cu, dwarf2_per_cu_deleter>,
allocator<unique_ptr<dwarf2_per_cu, dwarf2_per_cu_deleter> > > >,
debug::vector<unique_ptr<dwarf2_per_cu, dwarf2_per_cu_deleter> >,
random_access_iterator_tag>; _Tp = section_and_offset; _Compare =
dwarf2_find_unit(const section_and_offset&,
dwarf2_per_bfd*)::<lambda(const dwarf2_per_cu_up&, const
section_and_offset&)>]
Error: elements in iterator range [first, last) are not partitioned by the
predicate __comp and value __val.
Knowing all this, is it easy to artificially reproduce the problem by
making create_all_units() reverse the all_units vector, like so:
// Reverse the all_units vector
std::reverse (per_objfile->per_bfd->all_units.begin (),
per_objfile->per_bfd->all_units.end ());
The fix is to reorder the operations in this way
- call create_all_units(), to fill the all_units vector by walking
`.debug_info` and `.debug_types`
- call create_foreign_type_units_from_debug_names(), which may add more
items to the all_units vector.
- call finalize_all_units(), to sort the all_units vector.
- call the build_and_check_* functions, which is fine now that
all_units is sorted.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34149
Change-Id: I315f391605af549b55341a167683d2dc6203bcea
Approved-By: Tom Tromey <tom@tromey.com>
A user pointed out a problem when printing certain values from an Ada
enumeration type. Investigation showed that the problem was that some
enumeration constants were emitted using DW_FORM_data1, and were
incorrectly sign-extended by gdb.
First, this is yet another instance of a general problem with DWARF.
See https://sourceware.org/bugzilla/show_bug.cgi?id=32680 for the
analysis.
Meanwhile, it turns out that GCC implements an extension to handle
this scenario. In particular, in non-strict mode, it will emit
DW_AT_encoding using either DW_ATE_signed or DW_ATE_unsigned. This
was done back in 2017 by Pierre-Marie, in support of Ada -- but then
somehow nothing was ever implemented on the gdb side. For this see
GCC commit f76f096e ("DWARF: add DW_AT_encoding attributes for
DW_TAG_enumeration_type DIEs").
This patch adds the missing code to gdb. The included test case shows
the bug that was originally reported. I've also included the snippet
from Pierre-Marie's commit message for good measure.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32680
Approved-By: Andrew Burgess <aburgess@redhat.com>
This commit builds on the previous two commits. You should go and
read them both for context.
Looking at the users of value::bits_synthetic_pointer, all callers but
two are now passing either the offset of a field within the value, or
hard-coded zero as they want to ask about the entire value.
The two exceptions are in coerce_pieced_ref (in dwarf2/expr.c) and in
value_addr (in valops.c) where we still pass value::embedded_offset.
The problem is that some of the other callers, where we currently pass
just a field offset, might also have a non-zero embedded offset,
specifically, the call in cp_print_value_fields is in this category.
In cp_print_value_fields we are printing fields from a value which is
potentially a base class contained within an instance of a derived
class, this will be represented by a non-zero embedded_offset, which
is currently not taken into account.
Additionally, the two callers that currently pass the
value::embedded_offset don't scale the embedded offset from bytes to
bits.
The failure to scale from bytes to bits is interesting, I originally
tried to create some tests that exposed this, but constantly failed.
It turns out, that in both these locations, in all code paths that I
could find, the value::embedded_offset will always be zero. For
example in coerce_pieced_ref, a reference cannot be a base class, and
so we never expect to see a non-zero embedded_offset in this case.
Similarly, in value_addr, the use of value::bits_synthetic_pointer is
within a block that only applies to reference types, which means the
embedded_offset will be zero.
Now within check_pieced_synthetic_pointer we already add the
value::offset into the bit_offset which is passed in. Remember, after
the previous two commits, the incoming bit_offset is (almost) always
the offset of a field within the value. The almost here is the two
cases mentioned above.
I propose that within check_pieced_synthetic_pointer we should take
into account both value::offset and value::embedded_offset. Then the
two cases that currently pass value::embedded_offset can be changed to
just pass zero.
With this done, and the previous two commits, we now have a consistent
model. value::bits_synthetic_pointer expects the (bit) offset of a
field within the value to check. In many cases this will be zero
meaning we want to check from the start of the value, but in some
cases it can be non-zero.
Then within value::bits_synthetic_pointer implementations, like
check_pieced_synthetic_pointer, we will take the value::offset and
value::embedded_offset into account, remembering to scale them from
bytes to bits.
I have also changed indirect_pieced_value to also take the
embedded_offset into account. I have done this for consistency rather
than necessity. I believe that the embedded_offset will always be
zero within indirect_pieced_value. The indirect_pieced_value function
is only called from value_ind (in valops.c), and only operates on
TYPE_CODE_PTR types (checked for in indirect_pieced_value). As a
TYPE_CODE_PTR cannot be the base class for a derived type, then we
don't expect to ever see a TYPE_CODE_PTR value with a non-zero
embedded_offset. But, having indirect_pieced_value take the embedded
offset into account is simple enough, and future proofs the code.
In both check_pieced_synthetic_pointer and indirect_pieced_value I
have changed uses of '8' to 'TARGET_CHAR_BIT', I was already touching
some of these lines, and I think TARGET_CHAR_BIT is clearer, but one
line in indirect_pieced_value was just updated to use TARGET_CHAR_BIT,
this is done for consistency.
The gdb.dwarf2/multi-piece-inherited-bitfield.exp test fails without
this patch, this exposes the case where the embedded_offset is
non-zero and we were previously failing to take this into account.
The gdb.dwarf2/multi-piece-primitive-field.exp test was something I
wrote while trying to exercise the coerce_pieced_ref code path some
more. It is an inheritance based version of the existing test, I was
wondering if this would result in a value with a non-zero
embedded_offset, but due to how the fields are extracted from the
aggregate prior to calling coerce_pieced_ref the embedded_offset is
always zero in this function.
Approved-By: Tom Tromey <tom@tromey.com>
Bug PR gdb/30693 describes a case where the following assertion can be
triggered:
../../gdb/dwarf2/loc.c:2213: internal-error: value* coerce_pieced_ref(const value*): Assertion `closure->pieces.size () == 1' failed.
The problem is that coerce_pieced_ref makes the following claim:
/* gdb represents synthetic pointers as pieced values with a single
piece. */
gdb_assert (closure != NULL);
gdb_assert (closure->pieces.size () == 1);
But this is not really true. If an aggregate type contains a synthetic
pointer, then it is possible that the aggregate type will have a
computed location consisting of multiple pieces. When GDB prints the
fields of that aggregate type these fields are extracted by calling
value::primitive_field. Within value::primitive_field the location of
the field is set by calling value::set_component_location.
When the parent value that holds the field has a computed location, the
field value gains a reference to the parent value's closure, this can be
seen in copy_pieced_value_closure in dwarf2/expr.c.
What this means is that, if the aggregate value has a multi-piece
computed location, then the synthetic pointer field will also have a
reference to that same multi-piece computed location, even if there is
really only a single piece that describes the synthetic pointer itself.
Some parts of GDB are already aware of this. If we look at
check_pieced_synthetic_pointer which implements the
value::bits_synthetic_pointer function, you'll see that this function
searches through all of the pieces to find the piece that covers the
value we are looking for, it then checks if that piece is an implicit
pointer location. But back in coerce_pieced_ref, after calling
value::bits_synthetic_pointer, we still make the assertion that there
will be just a single piece.
Fix this by copying the search through all pieces logic into
coerce_pieced_ref (see note on efficiency below). We now search through
all the pieces looking for a piece that describes the location of the
synthetic pointer, and we then use that piece to form the pointer's
value.
There are some assertions in the new code, these align with how
check_pieced_synthetic_pointer operates.
In addition, there is an error for the case where multiple pieces are
used to describe the location of a synthetic pointer. This case is
technically allowed by check_pieced_synthetic_pointer, but supporting
this would require changes to indirect_synthetic_pointer, so I propose
leaving that until we see such a case in the wild.
On efficiency, you'll notice that check_pieced_synthetic_pointer
performs a search through all the location pieces, and
coerce_pieced_ref also has to search through the pieces. It would be
nice if this could be avoided in order to avoid multiple searches.
Currently though coerce_pieced_ref calls
value->bits_synthetic_pointer, which is an API that should be agnostic
to the underlying implementation, i.e. shouldn't need to know that the
implementation is computed, so passing pieces back would be harder.
Maybe coerce_pieced_ref could avoid the value::bits_synthetic_pointer
call, and instead call check_pieced_synthetic_pointer directly, or
some related helper function, and could get the pieces back that way.
But this breaks the cleanly structured API that we currently have.
For now I'm leaving things as they are. My assumption is that the
number of pieces used to represent a value is pretty low, so the
search is actually pretty cheap.
There's a new test that uses the DWARF assembler to create a
representative example of a multi-piece aggregate that contains a
synthetic pointer member variable. This test triggers the assertion
before this commit.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30693
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=2467251
Approved-By: Tom Tromey <tom@tromey.com>
In DWARF 5, foreign type units are type units present in .dwo files.
Type units in .dwo files don't have a matching skeleton in the main
file. When an .debug_names index is present, it can't include those
type units in the regular type unit list, since that list specifies
offsets in the main file. They are instead listed in the foreign TU
list, which is basically just a list of type signatures.
In order to help the debugger locate these units (i.e. find the .dwo
file containing them), individual index entries referencing foreign type
units may also include a reference to a compile unit that the debugger
can follow to find the appropriate .dwo file.
This patch implements reading the .debug_names foreign TU list and using
these "hint" CUs to locate foreign type units. I use the term "hint"
throughout the code, but I don't mind another name if someone has a
better idea.
The first part is to read the actual foreign TU list from the
.debug_names index and create signatured_type objects out of them. This
is done in the new function create_foreign_type_units_from_debug_names.
Append the newly created signatured_type to the
mapped_debug_names_reader::foreign_type_units vector, which will be used
later to resolve DW_IDX_type_unit indices. Populate the
dwarf2_per_bfd::signatured_types set, which contains signatured_types
indexed by signature. And finally, transfer ownership of the object to
the dwarf2_per_bfd::all_units vector.
Previously, all dwarf2_per_cu (including signatured_type) objects were
created with a non-nullptr section. With foreign type units, we don't
know the section at creation time. We also don't know the offset into
section nor the size of the unit. Therefore, add a
dwarf2_per_bfd::allocate_signatured_type overload that takes just the
signature. Remove the "section != nullptr" assert from the
dwarf2_per_cu constructor, but add it to the other allocate_* methods.
Since the new create_foreign_type_units_from_debug_names function adds
items to the dwarf2_per_bfd::all_units vector, the vector needs to be
sorted after create_foreign_type_units_from_debug_names runs. Remove
the finalize_all_units call from create_all_units, making the callers
responsible to call it.
The next step is to read the hint CU attributes when scanning the index
entries. Rework mapped_debug_names_reader::scan_one_entry to remember
which kind of unit references it saw for the entry (comp, type and/or
foreign type) and then figure out what this means. The logic is:
- Did the entry reference a foreign type unit? If so, it's a foreign
type unit. Does it also reference a hint CU? If not, drop the
entry, there's nothing useful we can do with it.
- Otherwise, did the entry reference a (non-foreign) type unit? Then
it's a regular type unit. If so, it shouldn't also have a
DW_IDX_compile_unit.
- Otherwise, did the entry reference a comp unit? If so, it's a comp
unit.
- Otherwise, we don't know what unit the entry references, it's an
error.
Since the .debug_name index attaches hint CU attributes to individual
index entries, my initial implementation added the hint CU information
to the cooked_index_entry structure. I am not sure why DWARF 5 chose to
do it this way, as opposed to attaching one hint per foreign TU. Does
this mean that two type units with the same signature could be different
and, for a specific index entry, it would be important to find one
specific instance of the type unit over the others? I have no idea.
However, I know that the current GDB DWARF reader is not able to load
multiple type units with the same signature but different content. Once
it loads one type unit with a given signature, all subsequent references
to that signature will use that loaded type unit. I therefore chose to
have the .debug_names reader record just one hint CU per foreign TU.
This avoids growing the cooked_index_entry structure for nothing, and
having to pass through this information through multiple layers.
The next step is to locate the .dwo file containing the foreign TUs when
we need them. This sometimes makes use of the hint CU, but not always.
I identified these 3 code paths:
1. When the type unit gets expanded directly, for instance if you use
"ptype" and there is a direct match into the type unit. This case
is handled in load_full_type_unit, calling a new function
fill_in_sig_entry_from_per_cu_hint. This one uses the hint recorded
by the .debug_names reader. When a cooked index entry exists and
refers to a foreign TU for which the section is not yet known, we
know that there exists a hint, otherwise we wouldn't have created
the entry in the first place.
2. The second one is when the type unit is referenced by some other
unit. This case is handled in follow_die_sig_1, calling another new
function fill_in_sig_entry_from_dwo_file. In this case, we know
which unit is referring to the TU, so we use that unit's dwo file to
fill in the details. As explained in the comment, this is sometimes
just an optimization, but sometimes also necessary, if the TU
does not have a hint, due to it not containing any indexed name.
3. Similarly, in dwarf2_base_index_functions::expand_all_symtabs, we
might have to handle foreign type units for which we don't have a
hint. I initially implemented something in two passes, to go dig in
the dwo_file structures to find those TUs, but ended up choosing to
just skip them, for the reasons explained in the comment there.
Setting a dwarf2_per_cu's section a posteriori breaks the assumed
ordering of the dwarf2_per_bfd::all_units vector. After setting the
section, re-sort the vector.
Add a target board to exercise this new code. This board builds with:
- type units (-fdebug-types-section)
- split DWARF (-gsplit-dwarf)
- .debug_names index (created by GDB)
I ran the whole testsuite with this board file and it's not perfect, but
the results are comparable to the dwarf5-fission-debug-types board, for
instance.
There is one known failure that I am unable to get to the bottom of. It
seems orthogonal to my change though, more like an indexer or symbol
reader issue. There are maybe more of this kind, but this is one
example:
FAIL: gdb.ada/tick_length_array_enum_idx.exp: ptype variable_table'length (GDB internal error)
/home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:1839: internal-error: search_one: Assertion `symtab != nullptr' failed.
The problem appears to be that a cooked index lookup for symbol
variable_table says that a given TU should contain a match. But then
trying to expand the TU makes dw2_instantiate_symtab yield a nullptr
compunit_symtab, I think because the symbol reader found nothing
interesting symbol-wise. And then the assert in search_one triggers.
The issue seems sensitive to some aspects of the environment (gnat
version?). I am able to reproduce the issue on Arch Linux (gnat 15)
with:
$ make check TESTS="gdb.ada/tick_length_array_enum_idx.exp" RUNTESTFLAGS="--target_board=dwarf5-fission-debug-types-debug-names"
But it doesn't reproduce on Debian 13 (gnat 14), Ubuntu 24.04 (gnat
13) or Fedora Rawhide (gnat 16).
Change-Id: I0d4ccc1cbbce3a337794341744d24091e8549d7f
Approved-By: Tom Tromey <tom@tromey.com>
I realize that complaints are not often enabled and shown, but if we
emit them, I think they should include enough context to help a user who
wishes to dig in the problem to pinpoint where the problem happened
exactly. For this reason, change the complaints in
mapped_debug_names_reader::scan_one_entry to include precisions
regarding which index entry is problematic exactly.
I am not attached to the particular format. I thought that because this
is extra contextual information, I would put it in the square brackets
at the end (which only shows the module name currently). However, it
would be nice to converge towards a format that we could replicate
elsewhere in the DWARF reader, so feel free to chime in.
I factored out the formatting to a separate function, because I will be
adding more of these in a subsequent patch, and it's easier to have the
formatting centralized.
I have not touched the "Unsupported .debug_names form" warning. If that
comes up, we probably don't care about the specific index entry, we just
need to implement support for that form.
Change-Id: Id68a0ae406a3ca620408576b81f893f05c7e3df2
Approved-By: Tom Tromey <tom@tromey.com>
Add a bunch of debug prints in read-debug-names.c to dump what we are
reading. Some of if was helpful to me when debugging my changes to the
.debug_names reader and writer.
The debug prints are activated by "set debug dwarf-read" (I didn't feel
like we needed a separate knob for that). There are two levels of
verbosity. At level 1, we print everything that happens once, like the
header, counts, offsets of important parts of the index, etc. At
levels > 1, we print information about each abbrev, CU/TU and index
entry.
The output is made such that it is easy look at the output of readelf of
llvm-dwarfdump on the side and confirm that GDB is reading the index
right.
Here are some examples of the output.
Header and some abbrevs:
[dwarf-read] read_debug_names_from_section: start: reading .debug_names from /home/simark/build/babeltrace/src/lib/.libs/libbabeltrace2.so.0.0.0
[dwarf-read] read_debug_names_from_section: section size: 0x19d4e, initial length: 0x19d4a, dwarf64: 0, offset_size: 4
[dwarf-read] read_debug_names_from_section: version: 5
[dwarf-read] read_debug_names_from_section: cu_count: 56, tu_count: 0, foreign_tu_count: 0
[dwarf-read] read_debug_names_from_section: bucket_count: 0, name_count: 3418, abbrev_table_size: 374, augmentation_string_size: 8
[dwarf-read] read_debug_names_from_section: augmentation string: "GDB3\x00\x00\x00\x00"
[dwarf-read] read_debug_names_from_section: abbrev 1: tag DW_TAG_namespace, 4 attributes
[dwarf-read] read_debug_names_from_section: DW_IDX_compile_unit DW_FORM_udata
[dwarf-read] read_debug_names_from_section: DW_IDX_die_offset DW_FORM_ref_addr
[dwarf-read] read_debug_names_from_section: DW_IDX_GNU_language DW_FORM_udata
[dwarf-read] read_debug_names_from_section: DW_IDX_parent DW_FORM_data4
[dwarf-read] read_debug_names_from_section: abbrev 2: tag DW_TAG_enumerator, 4 attributes
[dwarf-read] read_debug_names_from_section: DW_IDX_compile_unit DW_FORM_udata
[dwarf-read] read_debug_names_from_section: DW_IDX_die_offset DW_FORM_ref_addr
[dwarf-read] read_debug_names_from_section: DW_IDX_GNU_language DW_FORM_udata
[dwarf-read] read_debug_names_from_section: DW_IDX_GNU_internal DW_FORM_flag_present
Excerpt of the CU list:
[dwarf-read] build_and_check_cu_list_from_debug_names: building CU list from .debug_names (56 CUs)
[dwarf-read] build_and_check_cu_list_from_debug_names: CU 0: offset 0x0
[dwarf-read] build_and_check_cu_list_from_debug_names: CU 1: offset 0x1bac
[dwarf-read] build_and_check_cu_list_from_debug_names: CU 2: offset 0x4262
[dwarf-read] build_and_check_cu_list_from_debug_names: CU 3: offset 0x4ff0
[dwarf-read] build_and_check_cu_list_from_debug_names: CU 4: offset 0x59e9
[dwarf-read] build_and_check_cu_list_from_debug_names: CU 5: offset 0x67af
[dwarf-read] build_and_check_cu_list_from_debug_names: CU 6: offset 0x8e1a
[dwarf-read] build_and_check_cu_list_from_debug_names: CU 7: offset 0xa5cc
[dwarf-read] build_and_check_cu_list_from_debug_names: CU 8: offset 0xcd39
Some entries:
[dwarf-read] scan_all_names: start: scanning 3418 names from .debug_names
[dwarf-read] scan_entries: scanning entries for name 1: "(anonymous namespace)" (entry pool offset 0x0)
[dwarf-read] scan_one_entry: entry pool offset 0x0: abbrev 1, tag DW_TAG_namespace
[dwarf-read] scan_one_entry: DW_IDX_compile_unit (DW_FORM_udata): 55
[dwarf-read] scan_one_entry: DW_IDX_die_offset (DW_FORM_ref_addr): 0x7d06b
[dwarf-read] scan_one_entry: DW_IDX_GNU_language (DW_FORM_udata): 33
[dwarf-read] scan_one_entry: DW_IDX_parent (DW_FORM_data4): 38201
[dwarf-read] scan_one_entry: -> die_offset 0x7d06b, per_cu offset 0x5af0e
[dwarf-read] scan_one_entry: entry pool offset 0xb: end of entries (abbrev 0)
[dwarf-read] scan_entries: scanning entries for name 2: "AUTO_SEEK_STREAM_STATE_PACKET_BEGAN" (entry pool offset 0xc)
[dwarf-read] scan_one_entry: entry pool offset 0xc: abbrev 2, tag DW_TAG_enumerator
[dwarf-read] scan_one_entry: DW_IDX_compile_unit (DW_FORM_udata): 17
[dwarf-read] scan_one_entry: DW_IDX_die_offset (DW_FORM_ref_addr): 0x1beea
[dwarf-read] scan_one_entry: DW_IDX_GNU_language (DW_FORM_udata): 29
[dwarf-read] scan_one_entry: DW_IDX_GNU_internal (DW_FORM_flag_present): 1
[dwarf-read] scan_one_entry: -> die_offset 0x1beea, per_cu offset 0x1a28e
[dwarf-read] scan_one_entry: entry pool offset 0x13: end of entries (abbrev 0)
Change-Id: I4a40bfb73fa7feccb5038814a7d1a1bcd4f6231b
Approved-By: Tom Tromey <tom@tromey.com>
The DWARF 5 .debug_names index makes the distinction between local type
units and foreign type units. Local type units are those present
directly in the main file. Foreign type units are those present in .dwo
files. GDB only knows how to produce local type units today, which
leads to invalid indexes whenever there are type units in .dwo files.
To observe this, run test gdb.dwarf2/fission-with-type-unit.exp with
board cc-with-debug-names. The test passes, but we see this in the log:
(gdb) file
/home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/fission-with-type-unit/fission-with-type-unit^M
Reading symbols from
/home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/fission-with-type-unit/fission-with-type-unit...^M
warning: Section .debug_names has incorrect entry in TU table,
ignoring .debug_names.^M
These are the units involved in this test. The first and last CUs are
dummy CUs added by the DWARF assembler, they are not important.
- CU at offset 0x0 of .debug_info in fission-with-type-unit -- dummy
- CU at offset 0xc of .debug_info in fission-with-type-unit
- CU at offset 0x25 of .debug_info in fission-with-type-unit -- dummy
- TU at offset 0x0 of .debug_info.dwo in fission-with-type-unit-dw.dwo
This is the content of the produced .debug_names:
Contents of the .debug_names section:
$ readelf --debug-dump=gdb_index testsuite/outputs/gdb.dwarf2/fission-with-type-unit/fission-with-type-unit
...
Version 5
Augmentation string: 47 44 42 33 00 00 00 00 ("GDB3")
CU table:
[ 0] 0
[ 1] 0xc
[ 2] 0x25
TU table:
[ 0] 0
Foreign TU table:
Used 0 of 0 buckets.
Symbol table:
[ 1] global_var: <0><1> DW_TAG_variable DW_IDX_compile_unit=(udata)=1 DW_IDX_die_offset=(ref_addr)=<0x1c> DW_IDX_GNU_language=(udata)=0 DW_IDX_GNU_internal=(flag_present)=1
[ 2] int:
<0x8><2> DW_TAG_base_type DW_IDX_compile_unit=(udata)=1 DW_IDX_die_offset=(ref_addr)=<0x15> DW_IDX_GNU_language=(udata)=0
<0xf><3> DW_TAG_base_type DW_IDX_type_unit=(udata)=0 DW_IDX_die_offset=(ref_addr)=<0x19> DW_IDX_GNU_language=(udata)=0
The TU table claims that there is a TU at offset 0 in the main file
(fission-with-type-unit). This is wrong: the TU is in
fission-with-type-unit-dw.dwo. It should be listed in the Foreign TU
table instead.
This patch therefore teaches GDB to use the foreign TU table for TUs in
.dwo files.
A note about foreign type units and skeletons: in the early history of
split DWARF and type units, gcc 4.x used to create skeletons for type
units in .dwo files, but subsequent versions don't. DWARF 5 doesn't
have support for type unit skeletons at all. So skeletons for type
units are mostly a historical curiosity at this point, the norm is to
not have them. But if for some reason a type unit in a .dwo file had a
matching skeleton in the main file, then it would be ok for that TU to
be listed in the "TU table". The offset would be that of the skeleton.
While the list of CUs and local TUs contain the offset within the
.debug_info section where to find the unit, the foreign TU list only
contains the 8-byte signature of the types. With just that, a reader
wouldn't be able to easily locate a .dwo that contain the type with a
given signature.
To help with this, index entries for foreign type units may also include
a reference to a compilation unit that can be followed in order to find
a .dwo file containing the type. This patch implements it.
Implementation details
----------------------
The first change is the addition of the dwo_unit::per_cu field, which
allows going from the dwo_unit to the dwarf2_per_cu structure (which
describes the skeleton) that was used to lookup this dwo_unit. This
fields starts at nullptr, and it gets set in lookup_dwo_cutu whenever we
look up the dwo_unit for a given dwarf2_per_cu. This will come handy
later. I made this field an std::atomic, because I think it would be
possible to craft a weird test case that would make two indexer threads
try to set the field on the same dwo_unit. During normal operation, we
expect the field for each dwo_unit representing a CU to be written
exactly once.
In index-write.c, change the get_unit_lists function in
dwarf2/index-write.c to segregate local and foreign type units. Then,
update write_debug_names to emit the list of foreign TUs in the
.debug_names header. This consists of a list of type signatures.
In debug_names::build, for foreign type units, emit a
DW_IDX_compile_unit field. This is the reference to the CU that can be
used to locate the .dwo file containing that type unit. To obtain the
value for this field, look up a CU in the same dwo_file that has its
dwo_unit::per_cu field set (typically there will be exactly one CU, and
the field will be set).
With this patch, the index for the test case above looks like:
$ readelf --debug-dump=gdb_index testsuite/outputs/gdb.dwarf2/fission-with-type-unit/fission-with-type-unit
...
Version 5
Augmentation string: 47 44 42 33 00 00 00 00 ("GDB3")
CU table:
[ 0] 0
[ 1] 0xc
[ 2] 0x25
TU table:
Foreign TU table:
[ 0] 000000000000cafe
Used 0 of 0 buckets.
Symbol table:
[ 1] global_var: <0><1> DW_TAG_variable DW_IDX_compile_unit=(udata)=1 DW_IDX_die_offset=(ref_addr)=<0x1c> DW_IDX_GNU_language=(udata)=0 DW_IDX_GNU_internal=(flag_present)=1
[ 2] int:
<0x8><2> DW_TAG_base_type DW_IDX_compile_unit=(udata)=1 DW_IDX_die_offset=(ref_addr)=<0x15> DW_IDX_GNU_language=(udata)=0
<0xf><3> DW_TAG_base_type DW_IDX_type_unit=(udata)=0 DW_IDX_compile_unit=(udata)=1 DW_IDX_die_offset=(ref_addr)=<0x19> DW_IDX_GNU_language=(udata)=0 ...
We can see that the TU is correctly placed in the foreign TU list, and
that the index entry (the last line) points to the TU at index 0, but
also to the CU at index 1, which is indeed the CU that the reader can
follow to find the type unit.
With this patch, GDB still rejects the index:
(gdb) file /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/fission-with-type-unit/fission-with-type-unit
Reading symbols from /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/fission-with-type-unit/fission-with-type-unit...
warning: Section .debug_names in /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/fission-with-type-unit/fission-with-type-unit has unsupported 1 foreign TUs, ignoring .debug_names.
But at least, we don't produce a bogus index anymore, that's already an
improvement. A following patch in this series implements the reading
side
Change-Id: I311fd7b4ca57d9ff6d64ae08df805c6635961eed
Approved-By: Tom Tromey <tom@tromey.com>
Add this little helper to make finding a TU a bit simpler. I always
find the STL way of doing things cryptic, so I think that hiding it in
small helper methods makes the code clearer.
A subsequent patch will add more uses of it
Change-Id: Ibfb20d0e44c65d2ff729f3e0980ec4435f223aef
Approved-By: Tom Tromey <tom@tromey.com>
Following the previous patch that moves the dwo_unit structure from
dwarf2/read.c to dwarf2/dwo.h, dwarf2_cu::section has no reason to be
implemented in dwarf2/read.c anymore. Move it to dwarf2/cu.c.
Change-Id: I67e2bb42d878ac18e4bf3460d75f1394477a46ce
Approved-By: Tom Tromey <tom@tromey.com>
Move these types and some related things to a new dwo.h header file. My
goal is to allow index-write.c to see these types, in a later patch. I
initially moved them to read.h, but Tom suggested in review that read.h
was getting a bit too large.
Change-Id: Ia32e0643f95561d3a1bfb67d501c8e20f5682f0e
Approved-By: Tom Tromey <tom@tromey.com>
Running test gdb.dwarf2/fission-with-type-unit.exp with the
cc-with-gdb-index target board fails with:
(gdb) maint expand-symtabs
/home/simark/src/binutils-gdb/gdb/dwarf2/read.c:3064: internal-error: cutu_reader: Assertion `sig_type->signature == cu->header.signature' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
FAIL: gdb.dwarf2/fission-with-type-unit.exp: maint expand-symtabs (GDB internal error)
This is a consequence of .gdb_index not supporting skeletonless type
units in .dwo files. That is, type units in .dwo files that don't have
a corresponding skeleton (or stub) in the main file.
For context: in DWARF 4, gcc 4.x used to create skeletons for type units
in .dwo files, but subsequent versions don't. DWARF 5 doesn't have
support for type unit skeletons at all. So skeletons for type units are
mostly a historical curiosity at this point, the norm is to not have
them.
Here's what leads up to the crash. First, this is what is in the main
file's .debug_info section (the first and last CUs are dummy CUs added
by the testsuite):
Compilation Unit @ offset 0:
Length: 0x8 (32-bit)
Version: 4
Abbrev Offset: 0
Pointer Size: 8
<0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
Compilation Unit @ offset 0xc:
Length: 0x15 (32-bit)
Version: 5
Unit Type: DW_UT_skeleton (4)
Abbrev Offset: 0x6
Pointer Size: 8
DWO ID: 0xf00d
<0><20>: Abbrev Number: 1 (DW_TAG_compile_unit)
<21> DW_AT_dwo_name : (strp) (offset: 0): fission-with-type-unit-dw.dwo
Compilation Unit @ offset 0x25:
Length: 0x8 (32-bit)
Version: 4
Abbrev Offset: 0xe
Pointer Size: 8
<0><30>: Abbrev Number: 1 (DW_TAG_compile_unit)
And here is what is in the fission-with-type-unit-dw.dwo file (one TU
and the CU):
Contents of the .debug_info.dwo section:
Compilation Unit @ offset 0:
Length: 0x1d (32-bit)
Version: 5
Unit Type: DW_UT_type (2)
Abbrev Offset: 0
Pointer Size: 8
Signature: 0xcafe
Type Offset: 0x19
<0><18>: Abbrev Number: 1 (DW_TAG_type_unit)
<1><19>: Abbrev Number: 2 (DW_TAG_base_type)
<1a> DW_AT_byte_size : (sdata) 4
<1b> DW_AT_encoding : (sdata) 5 (signed)
<1c> DW_AT_name : (string) int
<1><20>: Abbrev Number: 0
Contents of the .debug_info.dwo section:
Compilation Unit @ offset 0:
Length: 0x2d (32-bit)
Version: 5
Unit Type: DW_UT_split_compile (5)
Abbrev Offset: 0
Pointer Size: 8
DWO ID: 0xf00d
<0><14>: Abbrev Number: 3 (DW_TAG_compile_unit)
<1><15>: Abbrev Number: 4 (DW_TAG_base_type)
<16> DW_AT_byte_size : (sdata) 4
<17> DW_AT_encoding : (sdata) 5 (signed)
<18> DW_AT_name : (string) int
<1><1c>: Abbrev Number: 5 (DW_TAG_variable)
<1d> DW_AT_name : (string) global_var
<28> DW_AT_type : (ref4) <0x15>, int
<2c> DW_AT_location : (exprloc) 3 byte block: 8 c 9f (DW_OP_const1u: 12; DW_OP_stack_value)
<1><30>: Abbrev Number: 0
After loading the above in GDB, here is what is in GDB's mind (contents
of dwarf2_per_bfd::all_units):
- CU at offset 0x0 of .debug_info in fission-with-type-unit -- dummy
- CU at offset 0xc of .debug_info in fission-with-type-unit
- CU at offset 0x25 of .debug_info in fission-with-type-unit -- dummy
- TU at offset 0x0 of .debug_info.dwo in fission-with-type-unit-dw.dwo
This is correct. Then, this is the generated .gdb_index:
Contents of the .gdb_index section:
Version 9
CU table:
[ 0] 0 - 0xb -- dummy
[ 1] 0xc - 0x24
[ 2] 0x25 - 0x30 -- dummy
TU table:
[ 0] 0 0x19 000000000000cafe
Address table:
Symbol table:
[ 3] global_var: 1 [static, variable]
[754] int: 1 [static, type]
Shortcut table:
Language of main: unknown: 0
Name of main: <unknown>
The TU table says that there exists a TU at offset 0. Unfortunately,
there is no way for a reader of that index to know that this TU is
really in a .dwo file, not in the main file. So when GDB loads this
index back (creating dwarf2_per_bfd::all_units from .gdb_index this
time, rather than walking the debug info), this is what is in its mind:
- CU at offset 0x0 of .debug_info in fission-with-type-unit -- dummy
- TU at offset 0x0 of .debug_info in fission-with-type-unit
- CU at offset 0xc of .debug_info in fission-with-type-unit
- CU at offset 0x25 of .debug_info in fission-with-type-unit -- dummy
GDB now incorrectly believes there's a TU at offset 0 of .debug_info in
the main file, which is wrong. When trying to expand that TU with
"maint expand-symtabs", we're not really reading the TU, so we hit the
assert checking that the signature in the TU header matches what was
given by the index.
The .debug_names format has a way to list the TUs found in the .dwo
files, called the "foreign TU list" (see section 6.1.1.2 "Structure of
the Name Index" of DWARF 5). That list only includes the signature of
the type, and there is some capability to figure out which .dwo file
contains that type unit. The .gdb_index format does not have something
like that. We could try to retrofit such a feature in the .gdb_index
format, but I think we prefer to put our efforts on the standard
.debug_names format.
To avoid producing a misleading index like shown above, I propose to
make GDB refuse to produce an index if there exists a skeletonless type
unit. This patch implements that by looking at section of all
signatured_types. If the containing section ends with .dwo, then this
is a skeletonless type unit.
As a reminder: if a unit has a skeleton, the dwarf2_per_cu section will
point at the skeleton section, in the main file. If the unit does not
have a skeleton, the dwarf2_per_cu section will point at the section in
the .dwo file. All .dwo section names end with ".dwo".
Add a "endswith" utils function to help with that.
With this patch, running the gdb.dwarf/fission-with-type-unit.exp leads
to a compilation failure:
gdb compile failed, Error while writing index for `/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/fission-with-type-unit/.tmp/fission-with-type-unit': Found skeletonless type units, unable to produce .gdb_index. Consider using .debug_names instead.
... which makes the test "untested".
Add a new test, gdb.dwarf2/gdb-index-skeletonless-tu.exp, to verify the new error path.
Change-Id: I1e2e0204c9c2b48763aa99ce63521ae4a5262b22
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
All Fortran imported variable aliases (`use module, alias => var`)
were being added to "global scope", regardless of whether they appeared
in:
- Program/module scope (should be global)
- Function scope (should be local)
This caused conflicts when different functions had the same alias name
pointing to different variables.
DW_TAG_imported_declaration and DW_TAG_namespace cases are now handled
separately. This patch modifies the case for DW_TAG_imported_
declaration in the function new_symbol () to use cu->list_in_scope
instead of global symbols for all languages. This ensures that
function-scoped aliases use the current scope rather than being forced
into the global scope.
Bug Scenario:
subroutine sub1
use mod1, var_i_alias=>var_i ! alias points to mod1::var_i
var_i_alias = 3
var_i = 4
end subroutine
subroutine sub2
use mod2, var_i_alias=>var_i ! alias points to mod2::var_i
var_i_alias = 23
var_i = 25
end subroutine
Before: var_i_alias in sub2 incorrectly resolved to mod1::var_i
(value 25)
After: Each function's alias correctly resolves to its own imported
variable (value 23)
New test files verify the fix and include regression tests for global
program-scope imports:
- gdb/testsuite/gdb.fortran/module_declarations.exp
- gdb/testsuite/gdb.fortran/module_declarations.f90
Before the change:
(gdb) print var_i_alias
$4 = 25
FAIL: gdb.fortran/module_declarations.exp: sub2_test: print var_i_alias
After the change:
(gdb) print var_i_alias
$4 = 23
PASS: gdb.fortran/module_declarations.exp: sub2_test: print var_i_alias
Approved-By: Tom Tromey <tom@tromey.com>
This patch converts a bunch of functions to take or return DWARF
expressions as `gdb::array_view<const gdb_byte>`, instead of raw pointer
and size. It doesn't do any non-trivial change to function
implementations, but the idea is that we could change them (for example
dwarf_expr_context::execute_stack_op) to operate on the array view
directly, giving us bounds checking when building in debug mode. But
that is not as trivial.
This patch also doesn't change structure fields to array_views (for
instance, dwarf2_loclist_baton), because that would make them
non-trivially constructible, and we'd (technically) need to change how
they are allocated.
Change-Id: I45a40e4d00edfb54b7fdff1447806da5bbe06183
Approved-By: Tom Tromey <tom@tromey.com>
While testing gdb against a version of gnat-llvm that emits
unqualified names (in a hierarchical structure), I found that
gdb.ada/array_of_symbolic_length.exp would fail.
The DWARF in question looks like this:
<1><11a1>: Abbrev Number: 2 (DW_TAG_module)
<11a2> DW_AT_name : (indirect string, offset: 0xe65): pck
<2><11a6>: Abbrev Number: 3 (DW_TAG_variable)
<11a7> DW_AT_name : (indirect string, offset: 0xe69): my_length_LAST
[...]
<3><11df>: Abbrev Number: 5 (DW_TAG_subrange_type)
<11e0> DW_AT_type : <0x12ab>
<11e4> DW_AT_upper_bound : <0x11a6>
That is, an array's upper bound refers to my_length_LAST. gdb
represents this as a PROP_VARIABLE_NAME -- but only the local name of
the variable was used in this case.
This patch changes var_decl_name to use the correct full name for the
variable.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
I found a gdb crash when using some changes to gnat-llvm to have it
emit unqualified names in the DWARF. The crash happens because
determine_prefix does this:
return dwarf2_full_name (nullptr, parent, cu);
However, dwarf2_full_name can return NULL, causing a crash in the
caller.
The particular DWARF causing this is pretty strange -- it is a
function nested inside another nameless function. This may be a bug
in gnat-llvm, something I plan to investigate.
Meanwhile, gdb shouldn't crash. This patch changes determine_prefix
to avoid possible crashes here, by following its contract and not
returning NULL.
I'm not sure if it's worthwhile to write a test case for this.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Factor out a new_symbol variant out of new_symbol, containing mostly the
die->tag switch.
While we're at it, modernize the code using bool and nullptr. Also, remove
some unnecessary braces, and apply this simplification:
...
if (c)
foo (a);
else
foo (b);
->
foo (c
? a
: b);
...
I wondered about naming the new variant new_symbol_tag or new_symbol_1, but
I stuck with new_symbol.
Refactoring made we wonder why we use linkagename instead of sym->linkagename ()
after sym->set_linkage_name (), but since there are cornercases where these are
different, I've left it as is.
Also, I suspect the cp_scan_for_anonymous_namespaces can be hoisted, but I
also left that as is.
Finally, I noticed that physname == linkagename uses a pointer equivalence
test, which seems fragile to me. Also that I left as is.
The patch is easier to view with git show -w.
Approved-By: Tom Tromey <tom@tromey.com>
Running gdb.dwarf2/pr13961.exp with clang fails with an internal error:
gdb/dwarf2/read.c: internal-error: decode_line_header_for_cu:
Assertion `!cu->per_cu->is_debug_types ()' failed.
pr13961 is a legacy .S test that references a .debug_line label from
both a CU and a TU. The assembly includes an empty .debug_line section
declaration:
.section .debug_line,"",%progbits
.Ldebug_line0:
With gcc this results in a dummy (valid, but content-less) .debug_line
header being emitted, so GDB can build a line header and resolve file
indices. With clang the .debug_line section can be missing/empty,
leaving the line header unset.
During symbol creation, new_symbol may then try to lazily decode the
CU-only line header while processing a type unit, which triggers the
assertion above.
Fix this by only decoding the line header for non-type units. If no
line header is available, emit a complaint and continue without setting
the symtab rather than attempting CU-only line decoding from a TU.
Tested: gdb.dwarf2/pr13961.exp (CC_FOR_TARGET=clang-23)
Commit a910478f65 ("gdb/dwarf: fix internal error when FDEs do not
describe the CFA") fixed the switch on cfa_how in dwarf2_frame_cache,
but there is a very similar switch in dwarf2_fetch_cfa_info that has
the same issue. If the unwind info does not set a rule for the CFA,
fs.regs.cfa_how is left as CFA_UNSET and we hit the default case,
triggering an internal error.
dwarf2_fetch_cfa_info is called from two places, both while compiling
a DWARF expression that contains DW_OP_call_frame_cfa:
- dwarf2_compile_expr_to_ax, when compiling to agent-expression
bytecode
- do_compile_dwarf_expr_to_c, when compiling to C source (used by
the "compile" commands).
Unlike dwarf2_frame_cache, where we can degrade gracefully by setting
undefined_retaddr, the callers of dwarf2_fetch_cfa_info need a concrete
CFA to generate code, so we can't continue. Throw an error with the
same message used a few lines above, when the FDE itself cannot be
found.
Extend the debug-frame-no-cfa.exp test to reproduce this case: add a
compile unit describing the "main" function and with a DW_AT_frame_base
that uses DW_OP_call_frame_cfa and a local variable "x" that uses
DW_OP_fbreg (the evaluation of which requires the CFA). Then, try to
translate the location of "x" to agent-expression bytecode with "maint"
agent. This hits the internal error without the corresponding fix.
Change-Id: I82349e3d9259c8f943eabee5c2fce360876feee8
Approved-by: Kevin Buettner <kevinb@redhat.com>
I was looking at a patch [1] modifying the DW_AT_decl_file / DW_AT_call_file
handling of new_symbol, and noticed that:
- the code is fairly nested, and
- the patch adds another nesting layer.
Factor out function new_symbol_file_line out of new_symbol, handling both the
DW_AT_decl_file / DW_AT_call_file and DW_AT_call_line / DW_AT_decl_line
attributes.
Having factored out the code, simplify it using return, and modernize it using
bool and nullptr.
Approved-By: Tom Tromey <tom@tromey.com>
[1] https://sourceware.org/pipermail/gdb-patches/2026-March/226065.html
The OBJSTATS macro seems pretty pointless, so I removed it. Then when
looking at the OBJSTAT macro as well, I decided to remove it and also
struct objstats.
After this patch, symbols are allocated using a template method that
automatically updates the n_syms member. This cleans up the code a
little.
Also, nothing ever set objstats::sz_strtab, so this is removed.
Regression tested on x86-64 Fedora 43.
Approved-by: Kevin Buettner <kevinb@redhat.com>
There is one caller that uses the result of
buildsym_compunit::push_context. This patch changes this method to
return void and changes that spot to instead call a new methods on
buildsym_compunit.
This patch also removes the get_current_context_stack method in favor
of a new method that checks the exact condition needed by the one
caller.
This patch enables a subsequent cleanup; in particular now the
'context_stack' type isn't used outside of buildsym.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This changes buildsym_compunit::pop_context to create and return the
block, if needed. It also arranges to reset some fields in the
buildsym_compunit object to their saved values.
This also enables the removal of the set_local_using_directives
method.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Some functions in the DWARF reader temporarily set
dwarf2_cu::list_in_scope and then reset it when returning. This patch
changes these spots to use scoped_restore.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
There are a bunch of iteration functions that take a callback returning
true or false to indicate whether to continue or stop iterating. These
functions then return the same value, indicate whether the iteration was
done until the end of interrupted. I think this is confusing and
error-prone, as I never know which value means what. It is especially
confusing when two opposite conventions collide, such as in
objfile::map_symtabs_matching_filename.
I propose to make that more obvious by introducing a new
iteration_status enum with self-documenting values.
I started to change the callback type
compunit_symtab_iteration_callback, taken by
quick_symbol_functions::search, and then followed that path to update a
bunch of other functions.
I chose the name to be kind of generic, so that it can be used for other
similar iteration patterns. I also put it in gdbsupport, in case we
want to use it in gdbserver too.
Change-Id: I55d84d0c1af8ac0b82cc9f49ccf0d6b60e1769e0
Approved-By: Andrew Burgess <aburgess@redhat.com>
I think that the name search_symtabs_expansion_listener function type
no longer makes sense for the following reasons:
- Since we have both `symtab` and `compunit_symtab` structs, I like
when we are specific about which one we're talking about. In this
case, the callback takes a `compunit_symtab`.
- Following series "Search symbols via quick API" [1] last year (I
believe commit f88f9f42db ("Have expand_symtabs_matching work for
already-expanded CUs")), the callback gets called for all matching
compunit_symtabs, not just those that get expanded.
I therefore propose to rename it to compunit_symtab_iteration_callback.
I chose "callback" over "listener", because I think that listener
implies that there is some event happening, that we listen for. That
made sense before where we would listen for the "expansion" event. But
now since it just gets called back for all matching CUs, I think that
"callback" makes more sense.
I renamed the parameters accordingly.
[1] https://inbox.sourceware.org/gdb-patches/20250909-search-in-psyms-v4-0-e7ef9e3b6479@tromey.com/
Change-Id: Ia5a662e6d37caf8e0272424ddbbd82f05bdb5ebe
Approved-By: Tom Tromey <tom@tromey.com>
The `file_matcher` parameter is unused.
The information from `file_matcher` is actually encoded in
`cus_to_skip`. All callers to this:
auto_bool_vector cus_to_skip;
dw_search_file_matcher (per_objfile, cus_to_skip, file_matcher);
... which populates `cus_to_skip` with all the CUs that do not match
`file_matcher`.
Change-Id: I7fd642f84d72ad2595c1e6de38db3869cc555ce9
Approved-By: Andrew Burgess <aburgess@redhat.com>
New in v2: change how undefined_retaddr is set, to avoid regressions on
AArch64 (among possibly others).
This patch fixes an internal error problem that happens when a frame
description entry does not define the Canonical Frame Address (CFA).
This problem was initially reported downstream as a ROCgdb issue (see
Bug trailer below), but I wrote a reproducer that uses the .debug_frame
functionality added to the DWARF assembler in the previous patch.
The error is:
/home/smarchi/src/binutils-gdb/gdb/dwarf2/frame.c:1046: internal-error: Unknown CFA rule.
The original bug was encountered while debugging a GPU kernel written
with Triton [1]. From what I understand, the generated kernel does not
really use a stack, so the .debug_frame contents generated is quite
bare:
$ readelf --debug-dump=frames k
Contents of the .debug_frame section:
00000000 000000000000000c ffffffff CIE
Version: 4
Augmentation: ""
Pointer Size: 8
Segment Size: 0
Code alignment factor: 4
Data alignment factor: 4
Return address column: 16
DW_CFA_nop
00000010 0000000000000014 00000000 FDE cie=00000000 pc=0000000000001600..0000000000001704
For those who don't speak fluent .debug_frame, what we see here is a
Frame Description Entry (FDE) that doesn't define any register rule,
referring to a Common Information Entry (CIE) that also doesn't define
any initial register rule. This is equivalent to having no unwind
information at all. One question is: why generate these at all? I
suppose that this is an edge case, that the compiler is written in a way
that that presumes there will always be some unwind info. That there is
no "if unwind info is empty, skip emitting the FDE" check. Anyway, the
important thing for us is that these can be found in the wild, so GDB
shouldn't crash.
The fix consists of handling CFA_UNSET in the dwarf2_frame_cache switch.
CFA_UNSET is the initial state when we start interpreting a CFA program,
meaning that we don't know yet how the CFA is defined. In our case, it
remains unset after interpreting the CFA program.
With just the fix above, we get:
(gdb) bt
#0 0x000055555555511d in main ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
Which is good (better than crashing), but it would be good to avoid the
error. To do so, set the undefined_retaddr flag to true. This has
two effects:
- dwarf2_frame_this_id won't try to build a frame id from the CFA
(which is good, we don't have a CFA)
- dwarf2_frame_unwind_stop_reason will return UNWIND_OUTERMOST, which
is the most accurate thing we can return here (there is no outer
frame)
The result is the expected:
(gdb) bt
#0 0x000055555555511d in main ()
My initial implementation changed this condition:
if (fs.retaddr_column < fs.regs.reg.size ()
&& fs.regs.reg[fs.retaddr_column].how == DWARF2_FRAME_REG_UNDEFINED)
cache->undefined_retaddr = true;
such that we would enter it if
fs.retaddr_column <+ fs.regs.reg.size ()
However, this broke the unwinding on AArch64 (and possibly others).
Add a test case written using the DWARF assembler that reproduces the
issue.
[1] https://triton-lang.org/
Change-Id: I67c717ff03a41c0630a73ce9549d88ff363e8cea
Bug: https://github.com/ROCm/ROCgdb/issues/47
Approved-By: Tom Tromey <tom@tromey.com>
Currently, gdb does not properly handle inline functions when dwz is
used. This can be seen by running gdb.cp/breakpoint-locs.exp with the
cc-with-dwz target board.
The problem here is that inline functions need special handling in the
dwz case.
First, recall that a DWARF partial unit cannot, in general, be read in
isolation, as it may not have a language. To handle this, gdb defers
scanning partial units directly, and instead scans them in the context
of some including CU.
Entries coming from the PU are attributed to this reading CU. If
multiple CUs import a PU, gdb has the reader threads race to see which
one does the actual reading.
However, if an inline function is moved into a partial unit, then that
means it has potentially been inlined somewhere in every including CU.
Thus, when linespec searches for this function, each such including CU
should be expanded. But because gdb only attributed the function's
index entry to one CU, only that particular one is expanded.
This patch fixes this bug. All inclusions of a PU are recorded.
Entries coming from a PU are attributed to that PU. For most entries
coming from the PU, a single "canonical" outer CU is chosen to expand.
However, when an inline function is found, all such CUs are expanded.
A change was also needed to the index writer to handle this case.
There, entries coming from a PU should note the correct including CU.
This must be done because, with .debug_names or .gdb_index, gdb does
not have information about unit imports. Handling inline functions
here means writing out a separate entry for each outermost CU that
includes the function's PU.
I did consider changing the cooked indexer to create an internal table
more similar to what would be created by the .debug_names (e.g.)
reader: that is, multiple entries for each inline function. However,
this seemed more expensive at read time, and a main goal of the cooked
indexer is to be fast.
This version updates an assert in the .debug_names writer, and adds a
regression test for that.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30728
Acked-By: Tom de Vries <tdevries@suse.de>
process_imported_unit_die has a special case for C++, added as a
performance improvement.
While I somewhat agree with the general idea of this snippet --
importing a compilation unit seems like a strange thing to do, given
that partial units exist -- I think there are two issues with it.
First, it is specific to C++. I don't see any real reason that this
should be the case.
Second, it does not have a corresponding bit of code in the indexer.
This means that the cooked index's view of the DWARF differs from the
full reader's view here. This causes regressions in this series,
because the indexer assumes that reading a CU will cause all the
imported CUs to be read as a side effect -- but that does not happen
here.
I think fixing this in the indexer is not trivial. The reason for
this is that the indexer works in parallel, and once a reader has
acquired the "scan" bit for a CU, it is obligated to read it.
However, in this case this would require making a new cooked indexer.
Instead, because (1) this is weird and rare DWARF anyway, and (2) this
is just a performance optimization, I propose removing this.
Acked-By: Tom de Vries <tdevries@suse.de>
Approved-By: Simon Marchi <simark@simark.ca>
