Author: Michael Buch Date: 2024-12-23T11:51:28Z New Revision: 28d14904c00b74154b8dfa71d5b062a7e590c44c
URL: https://github.com/llvm/llvm-project/commit/28d14904c00b74154b8dfa71d5b062a7e590c44c DIFF: https://github.com/llvm/llvm-project/commit/28d14904c00b74154b8dfa71d5b062a7e590c44c.diff LOG: [lldb][SymbolFileDWARF] Share GetDIEToType between SymbolFiles of a SymbolFileDWARFDebugMap (#120569) The problem here manifests as follows: 1. We are stopped in main.o, so the first `ParseTypeFromDWARF` on `FooImpl<char>` gets called on `main.o`'s SymbolFile. This adds a mapping from *declaration die* -> `TypeSP` into `main.o`'s `GetDIEToType` map. 2. We then `CompleteType(FooImpl<char>)`. Depending on the order of entries in the debug-map, this might call `CompleteType` on `lib.o`'s SymbolFile. In which case, `GetDIEToType().lookup(decl_die)` will return a `nullptr`. This is already a bit iffy because some of the surrounding code assumes we don't call `CompleteTypeFromDWARF` with a `nullptr` `Type*`. E.g., `CompleteEnumType` blindly dereferences it (though enums will never encounter this because their definition is fetched in ParseEnum, unlike for structures). 3. While in `CompleteTypeFromDWARF`, we call `ParseTypeFromDWARF` again. This will parse the member function `FooImpl::Create` and its return type which is a typedef to `FooImpl*`. But now we're inside `lib.o`'s SymbolFile, so we call it on the definition DIE. In step (2) we just inserted a `nullptr` into `GetDIEToType` for the definition DIE, so we trivially return a `nullptr` from `ParseTypeFromDWARF`. Instead of reporting back this parse failure to the user LLDB trucks on and marks `FooImpl::Ref` to be `void*`. This test-case will trigger an assert in `TypeSystemClang::VerifyDecl` even if we just `frame var` (but only in debug-builds). In release builds where this function is a no-op, we'll create an incorrect Clang AST node for the `Ref` typedef. The proposed fix here is to share the `GetDIEToType` map between SymbolFiles if a debug-map exists. **Alternatives considered** * Check the `GetDIEToType` map of the `SymbolFile` that the declaration DIE belongs to. The assumption here being that if we called `ParseTypeFromDWARF` on a declaration, the `GetDIEToType` map that the result was inserted into was the one on that DIE's SymbolFile. This was the first version of this patch, but that felt like a weaker version sharing the map. It complicates the code in `CompleteType` and is less consistent with the other bookkeeping structures we already share between SymbolFiles * Return from `SymbolFileDWARF::CompleteType` if there is no type in the current `GetDIEToType`. Then `SymbolFileDWARFDebugMap::CompleteType` could continue to the next `SymbolFile` which does own the type. But that didn't quite work because we remove the `GetForwardCompilerTypeToDie` entry in `SymbolFile::CompleteType`, which `SymbolFileDWARFDebugMap::CompleteType` relies upon for iterating Added: lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h Removed: ################################################################################ diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 71a77a4ed7458f..28e7cceb397154 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -3633,8 +3633,7 @@ bool DWARFASTParserClang::CopyUniqueClassMethodTypes( static_cast<DWARFASTParserClang *>( SymbolFileDWARF::GetDWARFParser(*dst_class_die.GetCU())); auto link = [&](DWARFDIE src, DWARFDIE dst) { - SymbolFileDWARF::DIEToTypePtr &die_to_type = - dst_class_die.GetDWARF()->GetDIEToType(); + auto &die_to_type = dst_class_die.GetDWARF()->GetDIEToType(); clang::DeclContext *dst_decl_ctx = dst_dwarf_ast_parser->m_die_to_decl_ctx[dst.GetDIE()]; if (dst_decl_ctx) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 87517266fced5c..360dbaa1beb5eb 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -482,6 +482,13 @@ static ConstString GetDWARFMachOSegmentName() { return g_dwarf_section_name; } +llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> & +SymbolFileDWARF::GetDIEToType() { + if (SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile()) + return debug_map_symfile->GetDIEToType(); + return m_die_to_type; +} + llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> & SymbolFileDWARF::GetForwardDeclCompilerTypeToDIE() { if (SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile()) @@ -1594,6 +1601,8 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) { if (!dwarf_ast) return false; Type *type = GetDIEToType().lookup(decl_die.GetDIE()); + assert(type); + if (decl_die != def_die) { GetDIEToType()[def_die.GetDIE()] = type; DWARFASTParserClang *ast_parser = diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index 6ecc8855380411..7309f7a86b659c 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -335,9 +335,7 @@ class SymbolFileDWARF : public SymbolFileCommon { m_file_index = file_index; } - typedef llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> DIEToTypePtr; - - virtual DIEToTypePtr &GetDIEToType() { return m_die_to_type; } + virtual llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> &GetDIEToType(); virtual llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> & GetForwardDeclCompilerTypeToDIE(); @@ -529,7 +527,7 @@ class SymbolFileDWARF : public SymbolFileCommon { UniqueDWARFASTTypeMap m_unique_ast_type_map; // A map from DIE to lldb_private::Type. For record type, the key might be // either declaration DIE or definition DIE. - DIEToTypePtr m_die_to_type; + llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> m_die_to_type; DIEToVariableSP m_die_to_variable_sp; // A map from CompilerType to the struct/class/union/enum DIE (might be a // declaration or a definition) that is used to construct it. diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h index 0ebcad2866a72e..df41d6a2a4e42f 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h @@ -291,6 +291,10 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon { return m_unique_ast_type_map; } + llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> &GetDIEToType() { + return m_die_to_type; + } + // OSOEntry class OSOEntry { public: @@ -329,6 +333,8 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon { llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> m_forward_decl_compiler_type_to_die; UniqueDWARFASTTypeMap m_unique_ast_type_map; + llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> m_die_to_type; + DebugMap m_debug_map; // When an object file from the debug map gets parsed in diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp index 49632e1d8911cb..c1829abe72933a 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp @@ -102,7 +102,8 @@ bool SymbolFileDWARFDwo::ParseVendorDWARFOpcode( return GetBaseSymbolFile().ParseVendorDWARFOpcode(op, opcodes, offset, stack); } -SymbolFileDWARF::DIEToTypePtr &SymbolFileDWARFDwo::GetDIEToType() { +llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> & +SymbolFileDWARFDwo::GetDIEToType() { return GetBaseSymbolFile().GetDIEToType(); } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h index 15c28fefd81f9d..75f5986f14014c 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h @@ -70,7 +70,7 @@ class SymbolFileDWARFDwo : public SymbolFileDWARF { SymbolFileDWARF *GetDIERefSymbolFile(const DIERef &die_ref) override; protected: - DIEToTypePtr &GetDIEToType() override; + llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> &GetDIEToType() override; DIEToVariableSP &GetDIEToVariable() override; diff --git a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile new file mode 100644 index 00000000000000..d9db5666f9b6cd --- /dev/null +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := lib.cpp main.cpp + +include Makefile.rules diff --git a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py new file mode 100644 index 00000000000000..7e9f6967a1288c --- /dev/null +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py @@ -0,0 +1,38 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestCaseTypedefToOuterFwd(TestBase): + """ + We find a global variable whose type is forward declared + (whose definition is in either main.o or lib.o). We then + try to get the 'Ref' typedef nested within that forward + declared type. This test makes sure we correctly resolve + this typedef. + + We test this for two cases, where the definition lives + in main.o or lib.o. + """ + + def check_global_var(self, target, name: str): + var = target.FindFirstGlobalVariable(name) + self.assertSuccess(var.GetError(), f"Found {name}") + + var_type = var.GetType() + self.assertTrue(var_type) + + impl = var_type.GetPointeeType() + self.assertTrue(impl) + + ref = impl.FindDirectNestedType("Ref") + self.assertTrue(ref) + + self.assertEqual(ref.GetCanonicalType(), var_type) + + def test(self): + self.build() + target = self.dbg.CreateTarget(self.getBuildArtifact("a.out")) + self.check_global_var(target, "gLibExternalDef") + self.check_global_var(target, "gMainExternalDef") diff --git a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp new file mode 100644 index 00000000000000..b50a13afe1acc7 --- /dev/null +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp @@ -0,0 +1,10 @@ +#include "lib.h" + +template <typename T> struct FooImpl { + using Ref = FooImpl<T> *; + + Ref Create() { return new FooImpl<T>(); } +}; + +FooImpl<char> gLibLocalDef; +BarImpl<char> *gLibExternalDef = nullptr; diff --git a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h new file mode 100644 index 00000000000000..d909fe58df12b3 --- /dev/null +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h @@ -0,0 +1,7 @@ +#ifndef LIB_H_IN +#define LIB_H_IN + +template <typename T> struct FooImpl; +template <typename T> struct BarImpl; + +#endif // LIB_H_IN diff --git a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp new file mode 100644 index 00000000000000..acf8337a69d0bb --- /dev/null +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp @@ -0,0 +1,12 @@ +#include "lib.h" + +template <typename T> struct BarImpl { + using Ref = BarImpl<T> *; + + Ref Create() { return new BarImpl<T>(); } +}; + +BarImpl<char> gMainLocalDef; +FooImpl<char> *gMainExternalDef = nullptr; + +int main() { return 0; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
