aboutsummaryrefslogtreecommitdiff
path: root/gdb/python/py-function.c
diff options
context:
space:
mode:
authorSimon Marchi <simon.marchi@efficios.com>2025-07-09 11:35:13 -0400
committerSimon Marchi <simon.marchi@polymtl.ca>2025-08-01 00:25:54 -0400
commit6474c699a5257f6edfce1132b341f84c0f867f82 (patch)
tree0b67f9200917561fff973bf538774a27a1962a5f /gdb/python/py-function.c
parentcb208105eb5674fcc0fa2a4999ea0cc4f06bc8c6 (diff)
downloadbinutils-6474c699a5257f6edfce1132b341f84c0f867f82.zip
binutils-6474c699a5257f6edfce1132b341f84c0f867f82.tar.gz
binutils-6474c699a5257f6edfce1132b341f84c0f867f82.tar.bz2
gdb/dwarf: sort dwarf2_per_bfd::all_units by (section, offset)
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>
Diffstat (limited to 'gdb/python/py-function.c')
0 files changed, 0 insertions, 0 deletions