| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
After this series, expand_symtabs_matching is now misnamed. This
patch renames it, renames some associated types, and also fixes up
some comments that I previously missed.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
|
|
dw_expand_symtabs_matching_file_matcher is no longer needed outside of
read.c, so it can be made static.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This patch rewrites the .gdb_index reader to create the same data
structures that are created by the cooked indexer and the .debug_names
reader.
This is done in support of this series; but also because, from what I
can tell, the "templates.exp" change didn't really work properly with
this reader.
In addition to fixing that problem, this patch removes a lot of code.
Implementing this required a couple of hacks, as .gdb_index does not
contain all the information that's used by the cooked index
implementation.
* The index-searching code likes to differentiate between the various
DWARF tags when matching, but .gdb_index lumps many things into a
single "other" category. To handle this, we introduce a phony tag
that's used so that the match method can match on multiple domains.
* Similarly, .gdb_index doesn't distinguish between the type and
struct domains, so another phony tag is used for this.
* The reader must attempt to guess the language of various symbols.
This is somewhat finicky. "Plain" (unqualified) symbols are marked
as language_unknown and then a couple of hacks are used to handle
these -- one in expand_symtabs_matching and another when recognizing
"main".
For what it's worth, I consider .gdb_index to be near the end of its
life. While .debug_names is not perfect -- we found a number of bugs
in the standard while implementing it -- it is better than .gdb_index
and also better documented.
After this patch, we could conceivably remove dwarf_scanner_base.
However, I have not done this.
Finally, this patch also changes this reader to dump the content of
the index, as the other DWARF readers do. This can be handy when
debugging gdb.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33316
|
|
Currently, gdb will search the already-expanded symtabs in one loop,
and then also expand matching symtabs in another loop. However, this
is somewhat inefficient -- when searching the already-expanded
symtabs, all such symtabs are examined. However, the various "quick"
implementations already know which subset of symtabs might have a
match.
This changes the contract of expand_symtabs_matching to also call the
callback for an already-expanded symtab. With this change, and some
subsequent enabling changes, the number of searched symtabs should
sometimes be reduced. This also cuts down on the amount of redundant
code.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30736
Acked-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This removes dwarf2_per_cu_data::mark, replacing it with a
locally-allocated boolean vector. It also inverts the sense of the
flag -- now, the flag is true when a CU should be skipped, and false
when the CU should be further examined. Also, the validity of the
flag is no longer dependent on 'file_matcher != NULL'.
This patch makes the subsequent patch to searching a bit simpler, so
I've separated it out.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
Acked-By: Simon Marchi <simon.marchi@efficios.com>
|
|
g++ will sometimes use a typedef to give a name to an otherwise
anonymous type for linkage purposes. gdb tries to handle this odd
scenario, which is enforced by anon-struct.exp.
It's difficult to detect this problem in the current tree, but the
cooked index does not include an entry for these DIEs.
This patch changes gdb to add these to the index. This is needed by
subsequent changes in this series.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32519
Acked-By: Simon Marchi <simon.marchi@efficios.com>
|
|
When I rewrote the .debug_names writer (commit 91a42a61), I changed
the writer to not import .debug_str into the debug_str_lookup object.
However, a later patch in this series needed this again. The issue
here was that if a name occurs in the DWARF, and is also allocated,
then there is a race, where the created index depends on which DIE is
read first. This can cause index-file.exp failures.
This patch restores the old approach, avoiding this problem. I also
applied a couple of small cleanups to the class. And, I removed the
old complaint from the "ingestion" function, as this was not
necessary.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Currently the full symbol reader puts DW_TAG_imported_declaration in
TYPE_DOMAIN, in the global scope. This patch changes the cooked
indexer to follow.
Without this patch, a later patch in the series would cause
nsalias.exp to regress.
This also updates read-gdb-index.c to do something similar.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
|
|
The cooked index does not currently contain entries for Ada import
functions. This means that whether or not these are visible to
"break" depends on which CUs were previously expanded -- clearly a
bug.
This patch fixes the issue. I think the comments in the patch explain
the fix reasonably well.
Perhaps one to-do item here is to change GNAT to use
DW_TAG_imported_declaration for these imports. This may eventually
let us remove some of the current hacks.
This version includes a fix from Simon to initialize the new member.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32511
Acked-By: Simon Marchi <simon.marchi@efficios.com>
|
|
If you run struct-decl.exp with the .gdb_index board, you will see
that "the_type" is not emitted in the index. This would cause a
failure in this series. The fix is to ensure that certain necessary
type declarations are emitted.
However, a naive fix here will regress stub-array-size.exp, where a
type declaration and a type definition are both seen -- but the
declaration is seen first and causes a failure. This is handled by
adding some code (including a mild hack) to filter out type
declarations when a corresponding type definition is seen.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This patch is needed to avoid regressions later in the series.
The issue here is that ada_decode, when called with wide=false, would
act as though the input needed verbatim quoting. That would happen
because the 'W' character would be passed through; and then a later
loop would reject the result due to that character.
Similarly, with operators=false the upper-case-checking loop would be
skipped, but then some names that did need verbatim quoting would pass
through.
Furthermore I noticed that there isn't a need to distinguish between
the "wide" and "operators" cases -- all callers pass identical values
to both.
This patch cleans up the above, consolidating the parameters and
changing how upper-case detection is handled, so that both the
operator and wide cases pass-through without issue. I've added new
unit tests for this.
Acked-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This patch adds another minor hack to cooked_index_entry::full_name.
In particular, if GNAT emits non-hierarchical names (still the default
as the hierarchical series is blocked on one tricky problem), then a
request to compute the "linkage-style" name will now just return the
'name' field.
Without this tweak, this series would regress ada-cold-name.exp,
because the search would look for "name.cold" but the index would
return "name[cold]" as the "linkage" name (which would be wrong).
This area is a bit difficult to unravel. The best plan here, IMO, is
to change Ada to work like the other languages in gdb: store the
natural name and do searches with that name. I think this is
achievable, but I didn't want to try it here.
I've updated the relevant bug (tagged below) to reflect this.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32766
Acked-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This patch removes support for .gdb_index versions less than 7. See
the patch that rewrites the reader to understand why this is needed.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Acked-By: Simon Marchi <simon.marchi@efficios.com>
|
|
PR ada/33217 points out that gdb incorrectly calls the <ctype.h>
functions. In particular, gdb feels free to pass a 'char' like:
char *str = ...;
... isdigit (*str)
This is incorrect as isdigit only accepts EOF and values that can be
represented as 'unsigned char' -- that is, a cast is needed here to
avoid undefined behavior when 'char' is signed and a character in the
string might be sign-extended. (As an aside, I think this API seems
obviously bad, but unfortunately this is what the standard says, and
some systems check this.)
Rather than adding casts everywhere, this changes all the code in gdb
that uses any <ctype.h> API to instead call the corresponding c-ctype
function.
Now, c-ctype has some limitations compared to <ctype.h>. It works as
if the C locale is in effect, so in theory some non-ASCII characters
may be misclassified. This would only affect a subset of character
sets, though, and in most places I think ASCII is sufficient -- for
example the many places in gdb that check for whitespace.
Furthermore, in practice most users are using UTF-8-based locales,
where these functions aren't really informative for non-ASCII
characters anyway; see the existing workarounds in gdb/c-support.h.
Note that safe-ctype.h cannot be used because it causes conflicts with
readline.h. And, we canot poison the <ctype.h> identifiers as this
provokes errors from some libstdc++ headers.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33217
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes gdb and related programs to use the gnulib c-ctype code
rather than safe-ctype.h. The gdb-safe-ctype.h header is removed.
This changes common-defs.h to include the c-ctype header, making it
available everywhere in gdb.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33217
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes dwarf_record_line_1 to be a method of lnp_state_machine,
simplifying it a bit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes dwarf_finish_line to be a method of lnp_state_machine,
simplifying it a bit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
I noticed that several spots in lnp_state_machine fetch the CU's
builder. Since this can't change over the lifetime of the object, it
seemed nicer to simply cache it.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes dwarf_record_line_p to be a method of lnp_state_machine.
This simplifies it, as it can refer to members of the object.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes some code in line-program.c to use bool rather than int.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
I noticed that the compute_include_file_name intro comment was
slightly wrong, and while looking at this, I also noticed that it has
a single caller. This patch hoists it slightly so that a forward
declaration isn't needed.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This patch moves lnp_state_machine and some supporting code to a new
file, dwarf2/line-program.c. The main benefit of this is shrinking
dwarf2/read.c a bit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Rather than issuing a complaint, which is off by default, warn when returning
false in create_addrmap_from_gdb_index, informing the user that the .gdb_index
was ignored, and why.
Tested on aarch64-linux.
|
|
In create_addrmap_from_gdb_index, use the return value of
addrmap_mutable::insert_empty to detect overlapping ranges.
Tested on x86_64-linux.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Currently, in create_addrmap_from_gdb_index, when finding an incorrect entry
in the address table of a .gdb_index section:
- a (by default silent) complaint is made,
- the entry is skipped, and
- the rest of the entries is processed.
This is the use-what-you-can approach, which make sense in general.
But in the case that the .gdb_index section is incorrect while the other debug
info is correct, this approach prevents gdb from building a correct cooked
index (assuming there's no bug in gdb that would cause an incorrect index to
be generated).
Instead, bail out of create_addrmap_from_gdb_index on finding errors in the
address table.
I wonder about the following potential drawback of this approach: in the case
that the .gdb_index section is incorrect because the debug info is incorrect,
this approach rejects the .gdb_index section and spents time rebuilding a
likewise incorrect index. But I'm not sure if this is a real problem.
Perhaps gdb will refuse to generate such an index, in which case this is a
non-issue.
Tested on aarch64-linux.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
The enum address_class and related fields and methods seem misnamed to
me. Generalize it to "location_class". The enumerators in
address_class are already prefixed with LOC, so the new name seems
logical to me. Rename related fields and methods as well.
Plus, address_class could easily be mistaken for other unrelated things
named "address class" in GDB or DWARF.
Tested by rebuilding.
Change-Id: I0dca3738df412b350715286c608041b08e9b4d82
Approved-by: Kevin Buettner <kevinb@redhat.com>
|
|
Commit bedd6a7a44 ("gdb/dwarf: track compilation and type unit count")
causes this internal error:
$ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.dwarf2/debug-names-duplicate-cu/debug-names-duplicate-cu -ex "save gdb-index -dwarf-5 /tmp" -batch
warning: Section .debug_names has incorrect number of CUs in CU table, ignoring .debug_names.
/home/smarchi/src/binutils-gdb/gdb/dwarf2/index-write.c:1454: internal-error: write_debug_names: Assertion `comp_unit_counter == per_bfd->num_comp_units' failed.
This is visible when running this test:
$ make check TESTS="gdb.dwarf2/debug-names-duplicate-cu.exp" RUNTESTFLAGS="--target_board=cc-with-debug-names"
...
Running /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.dwarf2/debug-names-duplicate-cu.exp ...
gdb compile failed, warning: Section .debug_names has incorrect number of CUs in CU table, ignoring .debug_names.
/home/smarchi/src/binutils-gdb/gdb/dwarf2/index-write.c:1454: internal-error: write_debug_names: Assertion `comp_unit_counter == per_bfd->num_comp_units' failed.
...
=== gdb Summary ===
# of untested testcases 1
However, it's easy to miss because it only causes an "UNTESTED" to be
recorded, not a FAIL or UNRESOLVED. This is because the problem happens
while trying to create the .debug_names index, as part of the test case
compilation.
The problem is: when we bail out from using .debug_names because we
detect it is inconsistent with the units in .debug_info, we clear
per_bfd->all_units, to destroy all units previously created, before
proceeding to read the units with an index. However, we don't clear
per_bfd->num_{comp,type}_units. As a result, per_bfd->all_units
contains one unit, while per_bfd->num_comp_units is 2. Whenever we
clear per_bfd->all_units, we should also clear
per_bfd->num_{comp,type}_units.
While at it, move this logic inside a scoped object.
I added an assertion in finalize_all_units to verify that the size of
per_bfd->all_units equals per_bfd->num_comp_units +
per_bfd->num_type_units. This makes the problem (if omitting the fix)
visible when running gdb.dwarf2/debug-names-duplicate-cu.exp with the
unix (default) target board:
ERROR: Couldn't load debug-names-duplicate-cu into GDB (GDB internal error).
FAIL: gdb.dwarf2/debug-names-duplicate-cu.exp: find index type (GDB internal error)
FAIL: gdb.dwarf2/debug-names-duplicate-cu.exp: find index type, check type is valid
=== gdb Summary ===
# of expected passes 1
# of unexpected failures 2
# of unresolved testcases 1
I considered changing the code to build a local vector of units first,
then move it in per_bfd->all_units on success, that would avoid having
to clean it up on error. I did not do it because it's a much larger
change, but we could consider it.
Change-Id: I49bcc0cb4b34aba3d882b27c8a93c168e8875c08
Approved-By: Tom Tromey <tom@tromey.com>
|
|
PR symtab/33247 points out that this check in
create_addrmap_from_gdb_index:
if (lo > hi)
{
complaint (_(".gdb_index address table has invalid range (%s - %s)"),
hex_string (lo), hex_string (hi));
... should probably use ">=" instead. Reading a bit further the
reason seems obvious:
mutable_map.set_empty (lo, hi - 1, index->units[cu_index]);
Here if lo==hi, then this will insert a "reversed" range into the
addrmap.
Apparently some LLVM tool can erroneously create a .gdb_index like
this.
No test because it seems like more trouble to write than it's worth.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33247
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This patch changes type::fields to return an array_view of the fields,
then fixes up the fallout.
More cleanups would be possible here (in particular in the field
initialization code) but I haven't done so.
The main motivation for this patch was to make it simpler to iterate
over the fields of a type.
Regression tested on x86-64 Fedora 41.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This patch started as a fix for PR 29518 ("GDB doesn't handle
DW_FORM_ref_addr DIE references correctly with .debug_types sections")
[1], but the scope has expanded a bit to fix the problem more generally,
after I spotted a few issues related to the order of all_units. The
first version of this patch is here [2].
PR 29518 shows that dwarf2_find_containing_comp_unit can erroneously
find a type unit. The obvious problem is that the
dwarf2_find_containing_comp_unit function searches the whole all_units
vector (containing both comp and type units), when really it should just
search the compilation units. A simple solution would be to make it
search the all_comp_units view (which is removed in a patch earlier in
this series).
I then realized that in DWARF 5, since type units are in .debug_info
(versus .debug_types in DWARF 4), type units can be interleaved with
comp type in the all_units vector. That would make the all_comp_units
and all_type_units views erroneous, and dwarf2_find_containing_comp_unit
could still return something wrong. In v1, I added a sort in
finalize_all_units to make sure all_units is in the order that
dwarf2_find_containing_comp_unit expects:
- comp units from the main file
- type units from the main file
- comp units from the dwz file
- type units from the dwz file (not actually supported, see PR 30838)
Another problem I spotted is that the .gdb_index reader creates units in
this order:
- comp units from .gdb_index from main file
- comp units from .gdb_index from dwz file
- type units from .gdb_index from main file
This isn't the same order as above, so it would need the same sort step.
Finally, I'm not exactly sure if and when it happens, but it looks like
lookup_signatured_type can be called at a later time (after the initial
scan and creation of dwarf2_per_cu object creation), when expanding a
symtab. And that could lead to the creation of a new type unit (see
function add_type_unit), which would place the new type unit at the end
of the all_units vector, possibly screwing up the previous order.
To handle all this in a nice and generic way, Tom Tromey proposed to
change the all_units order, so that units are sorted by section, then
section offset. This is what this patch implements. The sorting is
done in finalize_all_units.
This works well, because when looking up a unit by section offset, the
caller knows which section the unit is in. Passing down a (section,
section offset) tuple makes it clear and unambiguous what unit the
caller is referring to. It should help eliminate some bugs where the
callee used the section offset in the wrong section. Passing down the
section along with the section offset replaces the "is_dwz" flag passed
to dwarf2_find_containing_comp_unit and a bunch of other functions in a
more general way.
dwarf2_find_containing_comp_unit can now legitimately find and return
type units even though it should be needed (type units are typically
referred to by signature). But I don't think there is harm for this
function to be more generic than needed. I therefore I renamed it to
dwarf2_find_containing_unit.
The sort criterion for "section" can be anything, as long as we use the
same for sorting and searching. In this patch, I use the pointer to
dwarf2_section_info, because it's easy. The downside is that the actual
order depends on what the memory allocator decided to return, so could
change from run to run, or machine to machine. Later, I might change it
so that sections are ordered based on their properties, making the order
stable across the board. This logic is encapsulated in the
all_units_less_than function, so it's easy to change.
The .debug_names reader can no longer rely on the order of the all_units
vector for its checks, since all_units won't be the same order as found
in the .debug_names lists. In fact, even before, it wasn't: this check
assumed that .debug_info had all CUs before TUs, and that the index
listed them in the exact same order. When I build a file with gcc and
"-gdwarf-5 -fdebug-types-section", type units appear first in
.debug_info. This caused GDB to reject a .debug_names index that is had
produced:
$ GDB="./gdb -nx -q --data-directory=data-directory" /home/smarchi/src/binutils-gdb/gdb/contrib/gdb-add-index.sh -dwarf-5 hello.so
$ ./gdb -nx -q --data-directory=data-directory hello.so
Reading symbols from hello.so...
⚠️ warning: Section .debug_names has incorrect entry in CU table, ignoring .debug_names.
To make it work, add a new dwarf2_find_unit function that allows looking
up a unit by start address (unlike dwarf2_find_containing_unit, which
can find by any containing address), and make the .debug_names reader
use it. It might make the load time of .debug_names a bit longer (the
build and check step is now going to be O(n*log(n)) instead of O(n)
where n is the number of units, or something like that), but I think
it's important to be correct here.
This patch adds a test
(gdb.dwarf2/dw-form-ref-addr-with-type-units.exp), which tries to
replicate the problem as shown by PR 29518.
gdb.base/varval.exp needs a small change, because an error message
changes (for the better, I think)
gdb.dwarf2/debug-names-non-ascending-cu.exp now fails, because GDB no
longer rejects a .debug_names index which lists CUs in a different order
than .debug_info. Given the change I did to the .debug_names reader,
explained above, I don't think this is a problem anymore (GDB can accept
an index like that). I also don't think that DWARF 5 mandates that CUs
are in ascending order. Delete this test.
[1] https://sourceware.org/bugzilla/show_bug.cgi?id=29518
[2] https://inbox.sourceware.org/gdb-patches/20250218193443.118139-1-simon.marchi@efficios.com/
Change-Id: I45f982d824d3842ac1eb73f8cce721a0a24b5faa
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The order of all_units can't be relied on when writing the CU and TU
lists to .gdb_index or .debug_names.
Both the .gdb_index and .debug_names writers expect that all_units
contains comp units followed by type units. As of this commit, when
reading a DWARF 5 .debug_info, the all_units vector is ordered based on
the order the units appear in .debug_info, where type units can be
interleaved with comp units.
It probably worked fine with DWARF 4, where type units were in a section
of their own (.debug_types). They were read after comp units, and
therefore after them in the all_units vector.
Change the writers to use a common function that splits the units in two
lists (comp units and type units). Sort both lists by section offset.
This is more than required, but it should help produce a stable and
predictable output.
Change-Id: I5a22e2e354145e3d6b5b2822dc2a3af2f9d6bb76
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The .gdb_index reader currently uses per_bfd::all_units when translating
a numerical index (as found in an index entry) to a dwarf2_per_cu. The
order of per_bfd::all_units is going to change in a subsequent patch, so
the indices as found in the index won't map to the right unit in
all_units. Change the .gdb_index reader to maintain its own vector,
with the units in the same order as found in the .gdb_index header.
This is similar to what the .debug_names reader does. But unlike
.debug_names, .gdb_index treats the CUs and TUs as a single list, as far
as the numerical indices are concerned, so we only need a single list
here (versus two for .debug_names).
Change-Id: I235e9b99bf02fc160dfcdaa610c9aca471f298a7
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The all_comp_units_index_cus and all_comp_units_index_tus vectors
contain the CU and TU lists as found in the .debug_names list. It seems
like they are meant to be used by the .debug_names reader when handling
a DW_IDX_compile_unit or DW_IDX_type_unit attribute. The value of the
attribute would translate directly into an index into one of these
vectors.
However, it looks like these vectors aren't actually used in practice.
They are used in the dwarf2_per_bfd::get_index_{c,t}u methods, which in
turn aren't used anywhere.
The handlers of DW_IDX_compile_unit and DW_IDX_type_unit use the
dwarf2_per_bfd::get_unit method, with the assumption that
dwarf2_per_bfd::all_units has comp units before type units. This is not
the case: the .debug_names reader creates the units in
dwarf2_per_bfd::all_units using the create_all_units function, which
creates the units in the order found in .debug_info, where type units
can be interleaved with comp units.
Since those vectors are specific to the .debug_names reader, move them
to the mapped_debug_names_reader struct. Then, update the handlers of
DW_IDX_compile_unit and DW_IDX_type_unit to actually use them.
Change-Id: Ie7db81f4442f634ac6d02280a60c6c671bcd22a5
Approved-By: Tom Tromey <tom@tromey.com>
|
|
In DWARF 5, type units appear in the .debug_info section, interleaved
with comp units, and the order in all_units reflects that. The
all_comp_units and all_type_units views are wrong in that case
(all_comp_units contains some type units, and vice-versa).
It would be possible to manually sort all_units to ensure that type
units follow comp units, but this series takes the approach of sorting
the units by section and section offset.
Remove those views, and replace their uses with num_comp_units and
num_type_units. It appears that the views were only used to know the
number of each kind.
The finalize_all_units function is now empty, but I am keeping it
because a subsequent patch adds a call to std::sort in there to sort the
all_units vector.
Change-Id: I42a65b6f1b6192957b55cea0e2eaff097e13a33b
Approved-By: Tom Tromey <tom@tromey.com>
|
|
A subsequent commit will remove the all_comp_units and all_type_units
array views, since it's not possible to assume that that all_units
vector is segmented between comp and type units. Some callers still
need to know the number of each kind, so track that separately.
Change-Id: I712fbdfbf10b333c431b688b881cc0987e74f688
Approved-By: Tom Tromey <tom@tromey.com>
|
|
constant block
Since commit 420d030e88 ("Handle field with dynamic bit offset"), I see:
$ make check TESTS="gdb.trace/unavailable-dwarf-piece.exp" RUNTESTFLAGS="--target_board=native-extended-gdbserver"
FAIL: gdb.trace/unavailable-dwarf-piece.exp: tracing bar: p/d x
FAIL: gdb.trace/unavailable-dwarf-piece.exp: tracing bar: p/d y
FAIL: gdb.trace/unavailable-dwarf-piece.exp: tracing bar: p/d z
The first FAIL is:
p/d x
$4 = {a = 0, b = <unavailable>, c = <unavailable>, d = <unavailable>, e = <unavailable>, f = <unavailable>, g = <unavailable>, h = <unavailable>, i = <unavailable>, j = 0}
(gdb) FAIL: gdb.trace/unavailable-dwarf-piece.exp: tracing bar: p/d x
When we should see:
p/d x
$4 = {a = 0, b = <unavailable>, c = 0, d = 0, e = 0, f = 0, g = 0, h = 0, i = 0, j = 0}
(gdb) PASS: gdb.trace/unavailable-dwarf-piece.exp: tracing bar: p/d x
The structure we print is:
0x0000004f: DW_TAG_structure_type
DW_AT_name [DW_FORM_string] ("t")
DW_AT_byte_size [DW_FORM_sdata] (3)
DW_AT_decl_file [DW_FORM_udata] (0)
DW_AT_decl_line [DW_FORM_udata] (1)
0x00000055: DW_TAG_member
DW_AT_name [DW_FORM_string] ("a")
DW_AT_type [DW_FORM_ref4] (0x00000019 "unsigned char")
DW_AT_data_member_location [DW_FORM_exprloc] (DW_OP_plus_uconst 0x0)
0x0000005f: DW_TAG_member
DW_AT_name [DW_FORM_string] ("b")
DW_AT_type [DW_FORM_ref4] (0x00000019 "unsigned char")
DW_AT_byte_size [DW_FORM_sdata] (1)
DW_AT_bit_size [DW_FORM_sdata] (1)
DW_AT_bit_offset [DW_FORM_sdata] (7)
DW_AT_data_member_location [DW_FORM_exprloc] (DW_OP_plus_uconst 0x1)
...
The particularity of field "b" (and the following ones, not shown here)
is that they have:
- a DW_AT_data_member_location of expression form, but that GDB reduces
to a constant
- a DW_AT_bit_offset
What I think happens is that the code path taken in this particular
scenario never ends up using the DW_AT_bit_offset value. Fix it by
calling apply_bit_offset_to_field, like what is done when
data_member_location_attr is using a constant form.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33136
Change-Id: I18e838e6c56a548495d3af332aeff3051188eaa9
Approved-By: Tom Tromey <tom@tromey.com>
|
|
For legibility, use more specific names for attribute variables and
don't reuse them for different attributes.
Change-Id: I98d8bb32fc64b5f6357fbc88f6fe93f2ddc8ef7c
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This patch introduces a new macro, INIT_GDB_FILE. This is used to
replace the current "_initialize_" idiom when introducing a per-file
initialization function. That is, rather than write:
void _initialize_something ();
void
_initialize_something ()
{
...
}
... now you would write:
INIT_GDB_FILE (something)
{
...
}
The macro handles both the declaration and definition of the function.
The point of this approach is that it makes it harder to accidentally
cause an initializer to be omitted; see commit 2711e475 ("Ensure
cooked_index_entry self-tests are run"). Specifically, the regexp now
used by make-init-c seems harder to trick.
New in v2: un-did some erroneous changes made by the script.
The bulk of this patch was written by script.
Regression tested on x86-64 Fedora 41.
|
|
Change the messages to reflect that these numbers includes type units,
not only compile units.
Change-Id: Id2f511d4666e5cf92112be917d72ff76791b7e1d
Approved-by: Kevin Buettner <kevinb@redhat.com>
|
|
This method returns type units too, so "get_unit" is a better name.
Change-Id: I6ec9de3f783637a3e206bcaaec96a4e00b4b7d31
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Change-Id: Iaac252aa2abbe169153e79b84f956cda172c69d1
|
|
When writing commit 28f15782adab ("gdb/dwarf: read multiple .debug_info.dwo
sections"), I initially thought that the gcc behavior of producing multiple
.debug_info.dwo sections was a bug (it is not). I updated the commit
message, but it looks like this comment stayed. Remove it, since it can
be misleading.
Change-Id: I027712d44b778e836f41afbfafab993da02726ef
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Internal AdaCore testing using -gdwarf-4 found a spot where GCC will
emit a negative DW_AT_bit_offset. However, my recent signed/unsigned
changes assumed that this value had to be positive.
I feel this bug somewhat invalidates my previous thinking about how
DWARF attributes should be handled.
In particular, both GCC and LLVM at understand that a negative bit
offset can be generated -- but for positive offsets they might use a
smaller "data" form, which is expected not to be sign-extended. LLVM
has similar code but GCC does:
if (bit_offset < 0)
add_AT_int (die, DW_AT_bit_offset, bit_offset);
else
add_AT_unsigned (die, DW_AT_bit_offset, (unsigned HOST_WIDE_INT) bit_offset);
What this means is that this attribute is "signed but default
unsigned".
To fix this, I've added a new attribute::confused_constant method.
This should be used when a constant value might be signed, but where
narrow forms (e.g., DW_FORM_data1) should *not* cause sign extension.
I examined the GCC and LLVM DWARF writers to come up with the list of
attributes where this applies, namely DW_AT_bit_offset,
DW_AT_const_value and DW_AT_data_member_location (GCC only, but LLVM
always emits it as unsigned, so we're safe here).
This patch corrects the bug and imports the relevant test case.
Regression tested on x86-64 Fedora 41.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32680
Bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118837
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This commit allows a user to enable or disable dwarf support at
compilation time. To do that, a new configure option is introduced, in
the form of --enable-gdb-dwarf-support (and the accompanying --disable
version). By default, dwarf support is enabled, so no behavior changes
occur if a user doesn't use the new feature. If support is disabled, no
.c files inside the dwarf2/ subfolder will be compiled into the final
binary.
To achieve this, this commit also introduces the new macro
DWARF_FORMAT_AVAILABLE, which guards the definitions of functions
exported from the dwarf reader. If the macro is not defined, there are a
couple behaviors that exported functions may have:
* no-ops: several functions are used to register things at
initialization time, like unwinders. These are turned into no-ops
because the user hasn't attempted to read DWARF yet, there's no point
in warning that DWARF is unavailable.
* warnings: similar to the previous commit, if dwarf would be read or
used, the funciton will emit the warning "No dwarf support available."
* throw exceptions: If the code that calls a function expects an
exceptin in case of errors, and has a try-catch block, an error with
the previous message is thrown.
I believe that the changed functions should probalby be moved to the
dwarf2/public.h header, but that require a much larger refactor, so it
is left as a future improvement.
Finally, the --enable-gdb-compile configure option has been slightly
changed, since compile requires dwarf support. If compile was requested
and dwarf was disabled, configure will throw an error. If the option was
not used, support will follow what was requested for dwarf (warning the
user of what is decided).
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
In Ada, a field can have a dynamic bit offset in its enclosing record.
In DWARF 3, this was handled using a dynamic
DW_AT_data_member_location, combined with a DW_AT_bit_offset -- this
combination worked out ok because in practice GNAT only needs a
dynamic byte offset with a fixed offset within the byte.
However, this approach was deprecated in DWARF 4 and then removed in
DWARF 5. No replacement approach was given, meaning that in strict
mode there is no way to express this.
This is a DWARF bug, see
https://dwarfstd.org/issues/250501.1.html
In a discussion on the DWARF mailing list, a couple people mentioned
that compilers could use the obvious extension of a dynamic
DW_AT_data_bit_offset. I've implemented this for LLVM:
https://github.com/llvm/llvm-project/pull/141106
In preparation for that landing, this patch implements support for
this construct in gdb.
New in v2: renamed some constants and added a helper method, per
Simon's review.
New in v3: more renamings.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
With test-case gdb.base/maint.exp, I ran into:
...
(gdb) file maint^M
Reading symbols from maint...^M
(gdb) mt set per-command on^M
(gdb) Time for "DWARF indexing worker": ...^M
Time for "DWARF indexing worker": ...^M
Time for "DWARF indexing worker": ...^M
Time for "DWARF indexing worker": ...^M
Time for "DWARF skeletonless type units": ...^M
Time for "DWARF add parent map": ...^M
Time for "DWARF finalize worker": ...^M
Time for "DWARF finalize worker": ...^M
Time for "DWARF finalize worker": ...^M
Time for "DWARF finalize worker": ...^M
Time for "DWARF finalize worker": ...^M
FAIL: $exp: warnings: per-command: mt set per-command on (timeout)
mt set per-command off^M
2025-05-31 09:33:44.711 - command started^M
(gdb) PASS: $exp: warnings: per-command: mt set per-command off
...
I didn't manage to reproduce this by rerunning the test-case, but it's fairly
easy to reproduce using a file with more debug info, for instance gdb:
...
$ gdb -q -batch -ex "file build/gdb/gdb" -ex "mt set per-command on"
...
Due to the default "mt dwarf synchronous" == off, the file command starts
building the cooked index in the background, and returns immediately without
waiting for the result.
The subsequent "mt set per-command on" implies "mt set per-command time on",
which switches on displaying of per-command execution time.
The "Time for" lines are the result of those two commands, but these lines
shouldn't be there because "mt per-command time" == off at the point of
issuing the file command.
Fix this by capturing the per_command_time variable, and using the captured
value instead.
Tested on x86_64-linux.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
PR cli/33039
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33039
|
|
This comment refers to the field location kind enum, even though call
sites were moved to their own enum in 7eb21cc70224 ("Change
call_site_target to use custom type and enum"). Update it.
Change-Id: I089923170c919853efb2946529221a4b55e720c1
|
|
While (mistakenly) grepping for DW_AT_compile_unit, I found this typo.
Change-Id: I04d97d7b1b27eacfca9da3853711b6092d330575
|
|
With a hello world a.out, and using the compiler flags from target board
dwarf5-fission-debug-types:
...
$ gcc -gdwarf-5 -fdebug-types-section -gsplit-dwarf ~/data/hello.c
...
I run into:
...
$ gdb -q -batch a.out
terminate called after throwing an instance of 'gdb_exception_error'
...
What happens is that an error is thrown due to invalid dwarf, but the error is
not caught, causing gdb to terminate.
In a way, this is a duplicate of PR32861, in the sense that we no longer run
into this after:
- applying the proposed patch (work around compiler bug), or
- using gcc 9 or newer (compiler bug fixed).
But in this case, the failure mode is worse than in PR32861.
Fix this by catching the error in
cooked_index_worker_debug_info::process_skeletonless_type_units.
With the patch, we get instead:
...
$ gdb -q -batch a.out
Offset from DW_FORM_GNU_str_index or DW_FORM_strx pointing outside of \
.debug_str.dwo section in CU at offset 0x0 [in module a.out]
...
While we're at it, absorb the common use of
cooked_index_worker_result::note_error:
...
try
{
...
}
catch (gdb_exception &exc)
{
(...).note_error (std::move (exc));
}
...
into the method and rename it to catch_error, resulting in more compact code
for the fix:
...
(...).catch_error ([&] ()
{
...
});
...
While we're at it, also use it in
cooked_index_worker_debug_info::process_units which looks like it needs the
same fix.
Tested on x86_64-linux.
PR symtab/32979
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32979
|
|
On x86_64-linux, with gcc 7.5.0 I ran into a build breaker:
...
gdb/dwarf2/read.c: In function ‘dwo_unit* lookup_dwo_unit_in_dwp()’:
gdb/dwarf2/read.c:7403:22: error: unused variable ‘inserted’ \
[-Werror=unused-variable]
auto [it, inserted] = dwo_unit_set.emplace (std::move (dwo_unit));
^
...
Fix this by dropping the unused variable.
Tested on x86_64-linux, by completing a build.
|
