| Summary: | Can't find class methods on clang compiled libraries | ||
|---|---|---|---|
| Product: | gdb | Reporter: | Guinevere Larsen <guinevere> |
| Component: | c++ | Assignee: | Andrew Burgess <aburgess> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | aburgess, sam, ssbssa, tromey |
| Priority: | P2 | ||
| Version: | HEAD | ||
| Target Milestone: | 18.1 | ||
| See Also: |
https://sourceware.org/bugzilla/show_bug.cgi?id=16994 https://sourceware.org/bugzilla/show_bug.cgi?id=16998 https://sourceware.org/bugzilla/show_bug.cgi?id=33557 https://sourceware.org/bugzilla/show_bug.cgi?id=32299 |
||
| Host: | Target: | ||
| Build: | Last reconfirmed: | ||
| Project(s) to access: | ssh public key: | ||
|
Description
Guinevere Larsen
2025-09-18 10:40:17 UTC
Setting target milestone as this is a regression introduced since 17 branched. Entries for "a_function" have the wrong parent.
[199] ((cooked_index_entry *) 0x7fffd8015f40)
name: a_function
canonical: a_function
qualified: a_function
DWARF tag: DW_TAG_subprogram
flags: 0x0 []
DIE offset: 0x3167
CU index: 35
parent: ((cooked_index_entry *) 0)
I wonder if this is bug#32299.
It's not bug#32299, but I have a fix, I just need to finish testing and write a good commit message then I'll get this posted. Possible fix: https://inbox.sourceware.org/gdb-patches/3ea45eb5b7eba98d5b0261a7648720a56fe6d578.1787133721.git.aburgess@redhat.com The master branch has been updated by Simon Marchi <simark@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=53cacd285af9a1ae962f56d4134695ffbee2e461 commit 53cacd285af9a1ae962f56d4134695ffbee2e461 Author: Andrew Burgess <aburgess@redhat.com> Date: Thu Sep 24 11:09:10 2026 -0400 gdb/dwarf: resolve class name via DW_AT_signature in cooked index This commit fixes PR gdb/33447, an issue where looking up qualified member function names was not working for C++ binaries compiled with Clang when using the -fdebug-types-section flag. Before this commit we would see this behaviour: (gdb) print base1::a_function There is no field named a_function When what we expect to see is: (gdb) print base1::a_function $1 = {void (const base1 * const)} 0x403060 <base1::a_function() const> The problem is that the cooked index is unable to determine the name of the parent class `base1` in this case, and so decides not to index `base1` or any of its child DIEs, which includes its member functions. The problem was discovered by running gdb.cp/cpexprs-debug-types.exp with Clang: make check-gdb TESTS=gdb.cp/cpexprs-debug-types.exp \ RUNTESTFLAGS='CXX_FOR_TARGET=clang++ CC_FOR_TARGET=clang' The cpexprs-debug-types.exp test forces use of the '-fdebug-types-section' flag, which is not on by default. With this flag, class definitions are placed in type units, and the compile unit contains only a declaration stub for each class. Both clang++ and g++ emit these stubs, but they differ in one detail: GCC includes DW_AT_name on the stub, while clang++ does not, the stub carries only DW_AT_declaration and DW_AT_signature. The class name is only available in the type unit, reachable by following the signature. For example, Clang emits this in a CU: <1><2e7d>: DW_TAG_class_type DW_AT_declaration : 1 DW_AT_signature : 0x3abb... <2><2eac>: DW_TAG_subprogram DW_AT_name : a_function DW_AT_declaration : 1 The definition for a_function is elsewhere in the same CU: <1><3142>: DW_TAG_subprogram DW_AT_specification: <0x2eac> And in a TU elsewhere: Compilation Unit @ offset 0xd2e: ... snip ... Signature: 0x3abb... <0><d46>: Abbrev Number: 1 (DW_TAG_type_unit) ... snip ... <1><d51>: Abbrev Number: 30 (DW_TAG_class_type) ... snip ... <d57> DW_AT_name : (indexed string: 0xaa): base1 To find the DW_AT_name the cooked index needs to look up the type within the TU. Without the name the cooked indexer skips indexing `base1` as well as its children. This look up used to work; it works in GDB 17. PR gdb/33447 incorrectly identifies commit c879f4dc3e317cf6353a45a803ecf00d577a13d8 as the commit that introduced the regression. This is actually the last working commit. The problem was introduced by the next commit in the same series: commit 86ac8c546235a67d6a6bb29476a3a9ac8f7a620a Date: Thu Jan 2 15:28:18 2025 -0700 Convert lookup_symbol_in_objfile Prior to this commit GDB's symbol lookup had two phases, a search through already expanded symtabs, and a search via lookup_symbol_via_quick_fns. After the above commit only lookup_symbol_via_quick_fns remains. The lookup_symbol_via_quick_fns lookup, which relies on the indexer, was always broken, but the first phase, searching via expanded symtabs, could correctly find the type name via the signature. An initial attempt to solve this problem tried to fix this problem within cooked_indexer::scan_attributes, calling lookup_signatured_type and finding the name that way. However, there were three problems with this approach: 1. Possible thread safety issues; calling lookup_signatured_type for a DWO file ends up calling lookup_dwo_signatured_type, which can call add_type_unit and finalize_all_units, which modify state that is shared between parser threads. 2. The dwarf2_per_cu::type_offset_in_section for a TU is only set when the cutu_reader is constructed to parse that TU. Calling lookup_signatured_type doesn't fully parse the TU, it just finds the TU. The original code relied on type_offset_in_section being valid in order to then parse the TU and extract the name. This would break if the CU was processed before the TU. 3. For skeletonless TUs, these are not indexed until late in the indexing process, after all the CUs have been indexed. This means that if the TU referenced by the signature was skeletonless then the original approach would fail to find it. The new approach presented here is modeled more along the lines of the deferred parent handling that already exists within the indexer. The following changes have been made: 1. abbrev.c: Add DW_AT_signature to the set of attributes that mark a DIE as "interesting" in abbrev_table::read. Without this, the unnamed class stub was classified as uninteresting at the abbreviation level and scan_attributes was never called for it. 2. cooked-indexer.c (cooked_indexer::scan_attributes): Capture the DW_AT_signature attribute. 3. cooked-index-entry.h: Allow a cooked_index_entry to hold a deferred name, in the same way it can already hold a deferred parent. The name member becomes a union, cooked_index_entry_name_ref, holding either the resolved name or the signature of the DIE that carries the real name. The new IS_NAME_DEFERRED flag says which member is active, mirroring IS_PARENT_DEFERRED and cooked_index_entry_ref. cooked_index_entry also gets other methods mirroring the deferred parent mechanism: name(), name_is_deferred(), get_deferred_name() and resolve_name(). name() asserts that the name is no longer deferred, so any attempt to read the name too early is caught. All readers of the old public 'name' member are updated to call name(). 4. cooked-indexer.c (cooked_indexer::index_dies): There are two different jobs done here: (a) If a DIE has no name, but does have a signature, then create the entry anyway, storing the signature in the name union and setting IS_NAME_DEFERRED. (b) If we are indexing a TU, make a record of the signature, and the name of the primary type within the TU. This builds a signature to name map. 5. cooked-index-worker.c (cooked_index_worker::done_reading): Merge together all of the signature to name maps built in (4b). 6. cooked-index-shard.c (cooked_index_shard::resolve_deferred_names): A new function, look through all of the cooked_index_entry objects in the shard for those with IS_NAME_DEFERRED, and use the signature to name map created in (5) to look up the name and update the entry. resolve_deferred_names returns a bool, true if any deferred name couldn't be resolved. 7. cooked-index-shard.c (cooked_index_shard::prune_nameless_entries): A new function, look through all of the cooked_index_entry objects in the shard, if the entry's parent still has a deferred name, break the parent relationship. If the entry itself still has a deferred name, remove it from the index. This prevents a nameless entry from being found via the get_parent method. After this point every entry in the index has a resolved name, so name() can be called on any of them. 8. cooked-index.c (cooked_index::wait): After waiting, if we have completed finalization then emit any complaints that came from the finalization phase. 9. cooked-index.c: Add the "resolve deferred names" and "prune nameless entries" steps to the index finalization steps, implemented with the functions mentioned in points 6 and 7. The "resolve deferred names" step is only applied to shards that have m_have_deferred_names set. The "prune nameless entries" step is only run if cooked_index::m_have_nameless_entries is set. 10. cooked-index-entry.c (cooked_index_entry::write_scope): Add an assert that the entry's name is resolved. Entries that don't satisfy this should have been removed during finalization. Added three new DWARF assembler tests. gdb.dwarf2/sig-type-unnamed-class.exp gdb.dwarf2/sig-type-unnamed-class-dwo.exp: These reproduce the problem case; there's a nameless declaration stub that references its full type via DW_AT_signature. To match the Clang output as much as possible the member function definition is separate and makes use of DW_AT_specification. The -dwo test places the DWARF into a DWO file and uses a skeletonless TU. gdb.dwarf2/sig-type-unnamed-class-bad-sig.exp: This one tests some invalid DWARF, the type DIE references a signature that doesn't exist. In this case GDB discards the cooked_index_entries whose name could not be resolved, and orphans their children. I think this is fine though, the DWARF is corrupted in this case. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33447 Co-Authored-By: Simon Marchi <simon.marchi@efficios.com> Change-Id: I1e0ba20e58af169abbf99e97c14c788164f0d1b2 The gdb-18-branch branch has been updated by Simon Marchi <simark@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=39f0802fb020ad54f2dd57f1845c0c2a1d5cc7dd commit 39f0802fb020ad54f2dd57f1845c0c2a1d5cc7dd Author: Andrew Burgess <aburgess@redhat.com> Date: Thu Sep 24 11:09:10 2026 -0400 gdb/dwarf: resolve class name via DW_AT_signature in cooked index This commit fixes PR gdb/33447, an issue where looking up qualified member function names was not working for C++ binaries compiled with Clang when using the -fdebug-types-section flag. Before this commit we would see this behaviour: (gdb) print base1::a_function There is no field named a_function When what we expect to see is: (gdb) print base1::a_function $1 = {void (const base1 * const)} 0x403060 <base1::a_function() const> The problem is that the cooked index is unable to determine the name of the parent class `base1` in this case, and so decides not to index `base1` or any of its child DIEs, which includes its member functions. The problem was discovered by running gdb.cp/cpexprs-debug-types.exp with Clang: make check-gdb TESTS=gdb.cp/cpexprs-debug-types.exp \ RUNTESTFLAGS='CXX_FOR_TARGET=clang++ CC_FOR_TARGET=clang' The cpexprs-debug-types.exp test forces use of the '-fdebug-types-section' flag, which is not on by default. With this flag, class definitions are placed in type units, and the compile unit contains only a declaration stub for each class. Both clang++ and g++ emit these stubs, but they differ in one detail: GCC includes DW_AT_name on the stub, while clang++ does not, the stub carries only DW_AT_declaration and DW_AT_signature. The class name is only available in the type unit, reachable by following the signature. For example, Clang emits this in a CU: <1><2e7d>: DW_TAG_class_type DW_AT_declaration : 1 DW_AT_signature : 0x3abb... <2><2eac>: DW_TAG_subprogram DW_AT_name : a_function DW_AT_declaration : 1 The definition for a_function is elsewhere in the same CU: <1><3142>: DW_TAG_subprogram DW_AT_specification: <0x2eac> And in a TU elsewhere: Compilation Unit @ offset 0xd2e: ... snip ... Signature: 0x3abb... <0><d46>: Abbrev Number: 1 (DW_TAG_type_unit) ... snip ... <1><d51>: Abbrev Number: 30 (DW_TAG_class_type) ... snip ... <d57> DW_AT_name : (indexed string: 0xaa): base1 To find the DW_AT_name the cooked index needs to look up the type within the TU. Without the name the cooked indexer skips indexing `base1` as well as its children. This look up used to work; it works in GDB 17. PR gdb/33447 incorrectly identifies commit c879f4dc3e317cf6353a45a803ecf00d577a13d8 as the commit that introduced the regression. This is actually the last working commit. The problem was introduced by the next commit in the same series: commit 86ac8c546235a67d6a6bb29476a3a9ac8f7a620a Date: Thu Jan 2 15:28:18 2025 -0700 Convert lookup_symbol_in_objfile Prior to this commit GDB's symbol lookup had two phases, a search through already expanded symtabs, and a search via lookup_symbol_via_quick_fns. After the above commit only lookup_symbol_via_quick_fns remains. The lookup_symbol_via_quick_fns lookup, which relies on the indexer, was always broken, but the first phase, searching via expanded symtabs, could correctly find the type name via the signature. An initial attempt to solve this problem tried to fix this problem within cooked_indexer::scan_attributes, calling lookup_signatured_type and finding the name that way. However, there were three problems with this approach: 1. Possible thread safety issues; calling lookup_signatured_type for a DWO file ends up calling lookup_dwo_signatured_type, which can call add_type_unit and finalize_all_units, which modify state that is shared between parser threads. 2. The dwarf2_per_cu::type_offset_in_section for a TU is only set when the cutu_reader is constructed to parse that TU. Calling lookup_signatured_type doesn't fully parse the TU, it just finds the TU. The original code relied on type_offset_in_section being valid in order to then parse the TU and extract the name. This would break if the CU was processed before the TU. 3. For skeletonless TUs, these are not indexed until late in the indexing process, after all the CUs have been indexed. This means that if the TU referenced by the signature was skeletonless then the original approach would fail to find it. The new approach presented here is modeled more along the lines of the deferred parent handling that already exists within the indexer. The following changes have been made: 1. abbrev.c: Add DW_AT_signature to the set of attributes that mark a DIE as "interesting" in abbrev_table::read. Without this, the unnamed class stub was classified as uninteresting at the abbreviation level and scan_attributes was never called for it. 2. cooked-indexer.c (cooked_indexer::scan_attributes): Capture the DW_AT_signature attribute. 3. cooked-index-entry.h: Allow a cooked_index_entry to hold a deferred name, in the same way it can already hold a deferred parent. The name member becomes a union, cooked_index_entry_name_ref, holding either the resolved name or the signature of the DIE that carries the real name. The new IS_NAME_DEFERRED flag says which member is active, mirroring IS_PARENT_DEFERRED and cooked_index_entry_ref. cooked_index_entry also gets other methods mirroring the deferred parent mechanism: name(), name_is_deferred(), get_deferred_name() and resolve_name(). name() asserts that the name is no longer deferred, so any attempt to read the name too early is caught. All readers of the old public 'name' member are updated to call name(). 4. cooked-indexer.c (cooked_indexer::index_dies): There are two different jobs done here: (a) If a DIE has no name, but does have a signature, then create the entry anyway, storing the signature in the name union and setting IS_NAME_DEFERRED. (b) If we are indexing a TU, make a record of the signature, and the name of the primary type within the TU. This builds a signature to name map. 5. cooked-index-worker.c (cooked_index_worker::done_reading): Merge together all of the signature to name maps built in (4b). 6. cooked-index-shard.c (cooked_index_shard::resolve_deferred_names): A new function, look through all of the cooked_index_entry objects in the shard for those with IS_NAME_DEFERRED, and use the signature to name map created in (5) to look up the name and update the entry. resolve_deferred_names returns a bool, true if any deferred name couldn't be resolved. 7. cooked-index-shard.c (cooked_index_shard::prune_nameless_entries): A new function, look through all of the cooked_index_entry objects in the shard, if the entry's parent still has a deferred name, break the parent relationship. If the entry itself still has a deferred name, remove it from the index. This prevents a nameless entry from being found via the get_parent method. After this point every entry in the index has a resolved name, so name() can be called on any of them. 8. cooked-index.c (cooked_index::wait): After waiting, if we have completed finalization then emit any complaints that came from the finalization phase. 9. cooked-index.c: Add the "resolve deferred names" and "prune nameless entries" steps to the index finalization steps, implemented with the functions mentioned in points 6 and 7. The "resolve deferred names" step is only applied to shards that have m_have_deferred_names set. The "prune nameless entries" step is only run if cooked_index::m_have_nameless_entries is set. 10. cooked-index-entry.c (cooked_index_entry::write_scope): Add an assert that the entry's name is resolved. Entries that don't satisfy this should have been removed during finalization. Added three new DWARF assembler tests. gdb.dwarf2/sig-type-unnamed-class.exp gdb.dwarf2/sig-type-unnamed-class-dwo.exp: These reproduce the problem case; there's a nameless declaration stub that references its full type via DW_AT_signature. To match the Clang output as much as possible the member function definition is separate and makes use of DW_AT_specification. The -dwo test places the DWARF into a DWO file and uses a skeletonless TU. gdb.dwarf2/sig-type-unnamed-class-bad-sig.exp: This one tests some invalid DWARF, the type DIE references a signature that doesn't exist. In this case GDB discards the cooked_index_entries whose name could not be resolved, and orphans their children. I think this is fine though, the DWARF is corrupted in this case. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33447 Co-Authored-By: Simon Marchi <simon.marchi@efficios.com> Change-Id: I1e0ba20e58af169abbf99e97c14c788164f0d1b2 (cherry picked from commit 53cacd285af9a1ae962f56d4134695ffbee2e461) This should now be resolved in both master and gdb-18-branch. |