https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/120569
>From 9aa49efdcde5887e8de6bcd6cfbb08c0a499e24b Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 19 Dec 2024 12:25:19 +0000 Subject: [PATCH 1/7] [lldb][SymbolFileDWARF] CompleteType: Lookup type in the declaration DIE's SymbolFile 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 surrounding code used to assume we don't call `CompleteTypeFromDWARF` with a `nullptr` `Type*`. This used to be more of an issue when `CompleteRecordType` actually made use of the `Type*` parameter (see https://github.com/llvm/llvm-project/pull/120456). 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. --- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 4 ++- .../lang/cpp/typedef-to-outer-fwd/Makefile | 3 ++ .../TestTypedefToOuterFwd.py | 32 +++++++++++++++++++ .../API/lang/cpp/typedef-to-outer-fwd/lib.cpp | 15 +++++++++ .../API/lang/cpp/typedef-to-outer-fwd/lib.h | 10 ++++++ .../lang/cpp/typedef-to-outer-fwd/main.cpp | 8 +++++ 6 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile create mode 100644 lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py create mode 100644 lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp create mode 100644 lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h create mode 100644 lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 68e50902d641a2..936a44865b4a7e 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -1593,7 +1593,9 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) { DWARFASTParser *dwarf_ast = GetDWARFParser(*def_die.GetCU()); if (!dwarf_ast) return false; - Type *type = GetDIEToType().lookup(decl_die.GetDIE()); + Type *type = decl_die.GetDWARF()->GetDIEToType().lookup(decl_die.GetDIE()); + assert (type); + if (decl_die != def_die) { GetDIEToType()[def_die.GetDIE()] = type; DWARFASTParserClang *ast_parser = 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..ad26d503c545b2 --- /dev/null +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py @@ -0,0 +1,32 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestCaseTypedefToOuterFwd(TestBase): + ''' + We are stopped in main.o, which only sees a forward declaration + of FooImpl. We then try to get the FooImpl::Ref typedef (whose + definition is in lib.o). Make sure we correctly resolve this + typedef. + ''' + def test(self): + self.build() + (_, _, thread, _) = lldbutil.run_to_source_breakpoint( + self, "return", lldb.SBFileSpec("main.cpp") + ) + + foo = thread.frames[0].FindVariable('foo') + self.assertSuccess(foo.GetError(), "Found foo") + + foo_type = foo.GetType() + self.assertTrue(foo_type) + + impl = foo_type.GetPointeeType() + self.assertTrue(impl) + + ref = impl.FindDirectNestedType('Ref') + self.assertTrue(ref) + + self.assertEqual(ref.GetCanonicalType(), foo_type) 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..2bc38f6503396d --- /dev/null +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp @@ -0,0 +1,15 @@ +#include "lib.h" + +template <typename T> struct FooImpl { + using Ref = FooImpl<T> *; + + Ref Create() { return new FooImpl<T>(); } +}; + +Foo getString() { + FooImpl<char> impl; + Foo ret; + ret.impl = impl.Create(); + + return ret; +} 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..d5dfddf2246209 --- /dev/null +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h @@ -0,0 +1,10 @@ +#ifndef LIB_H_IN +#define LIB_H_IN + +template <typename T> struct FooImpl; + +struct Foo { + FooImpl<char> *impl = nullptr; +}; + +#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..61c819ca0d31a1 --- /dev/null +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp @@ -0,0 +1,8 @@ +#include "lib.h" + +extern Foo getString(); + +int main() { + FooImpl<char> *foo = getString().impl; + return 0; +} >From be4c4c14e100cbdce5b983f18a53c888b7368a1c Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 19 Dec 2024 12:49:17 +0000 Subject: [PATCH 2/7] fixup! clang-format --- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 936a44865b4a7e..7e5841c890cfda 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -1594,7 +1594,7 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) { if (!dwarf_ast) return false; Type *type = decl_die.GetDWARF()->GetDIEToType().lookup(decl_die.GetDIE()); - assert (type); + assert(type); if (decl_die != def_die) { GetDIEToType()[def_die.GetDIE()] = type; >From 52ca0891445343c864b833d5c6b494dfac679963 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 19 Dec 2024 12:50:27 +0000 Subject: [PATCH 3/7] fixup! python format --- .../cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) 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 index ad26d503c545b2..b6663b1ff504f0 100644 --- a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py @@ -5,19 +5,20 @@ class TestCaseTypedefToOuterFwd(TestBase): - ''' + """ We are stopped in main.o, which only sees a forward declaration of FooImpl. We then try to get the FooImpl::Ref typedef (whose definition is in lib.o). Make sure we correctly resolve this typedef. - ''' + """ + def test(self): self.build() (_, _, thread, _) = lldbutil.run_to_source_breakpoint( self, "return", lldb.SBFileSpec("main.cpp") ) - foo = thread.frames[0].FindVariable('foo') + foo = thread.frames[0].FindVariable("foo") self.assertSuccess(foo.GetError(), "Found foo") foo_type = foo.GetType() @@ -26,7 +27,7 @@ def test(self): impl = foo_type.GetPointeeType() self.assertTrue(impl) - ref = impl.FindDirectNestedType('Ref') + ref = impl.FindDirectNestedType("Ref") self.assertTrue(ref) self.assertEqual(ref.GetCanonicalType(), foo_type) >From 918efaa2bd67e42455358201bf318d89bf2d4657 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Fri, 20 Dec 2024 12:56:50 +0000 Subject: [PATCH 4/7] fixup! share DIEToType map between debug-map SymbolFiles --- .../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 3 +-- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 9 ++++++++- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h | 6 ++---- .../Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h | 5 +++++ .../Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp | 3 ++- .../source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h | 2 +- 6 files changed, 19 insertions(+), 9 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 58f7b805abe2fd..32b0d32c22f6c1 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -3649,8 +3649,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 7e5841c890cfda..1b993f835f5c7b 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()) @@ -1593,7 +1600,7 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) { DWARFASTParser *dwarf_ast = GetDWARFParser(*def_die.GetCU()); if (!dwarf_ast) return false; - Type *type = decl_die.GetDWARF()->GetDIEToType().lookup(decl_die.GetDIE()); + Type *type = GetDIEToType().lookup(decl_die.GetDIE()); assert(type); if (decl_die != def_die) { diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index 76f4188fdf4afb..cab3a89a175854 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -337,9 +337,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(); @@ -532,7 +530,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 34cb52e5b601c4..26d97a4419d6bc 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h @@ -293,6 +293,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: @@ -331,6 +335,7 @@ 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; LazyBool m_supports_DW_AT_APPLE_objc_complete_type; DebugMap m_debug_map; 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; >From ca0c533dfe8658a6c7e0ac9aa141f1ab99845405 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Fri, 20 Dec 2024 15:11:25 +0000 Subject: [PATCH 5/7] fixup! make test bi-directional --- .../TestTypedefToOuterFwd.py | 39 +++++++++++-------- .../API/lang/cpp/typedef-to-outer-fwd/lib.cpp | 9 +---- .../API/lang/cpp/typedef-to-outer-fwd/lib.h | 5 +-- .../lang/cpp/typedef-to-outer-fwd/main.cpp | 14 ++++--- 4 files changed, 35 insertions(+), 32 deletions(-) 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 index b6663b1ff504f0..526afeb71c5877 100644 --- a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py @@ -6,28 +6,35 @@ class TestCaseTypedefToOuterFwd(TestBase): """ - We are stopped in main.o, which only sees a forward declaration - of FooImpl. We then try to get the FooImpl::Ref typedef (whose - definition is in lib.o). Make sure we correctly resolve this - typedef. - """ + We a global variable whose type is forward declared. We then + try to get the Ref typedef (whose definition is in either main.o + or lib.o). Make sure we correctly resolve this typedef. - def test(self): - self.build() - (_, _, thread, _) = lldbutil.run_to_source_breakpoint( - self, "return", lldb.SBFileSpec("main.cpp") - ) + We test this for two cases, where the definition lives + in main.o or lib.o. + """ - foo = thread.frames[0].FindVariable("foo") - self.assertSuccess(foo.GetError(), "Found foo") + def check_global_var(self, target, name: str): + var = target.FindFirstGlobalVariable(name) + self.assertSuccess(var.GetError(), f"Found {name}") - foo_type = foo.GetType() - self.assertTrue(foo_type) + var_type = var.GetType() + self.assertTrue(var_type) - impl = foo_type.GetPointeeType() + impl = var_type.GetPointeeType() self.assertTrue(impl) ref = impl.FindDirectNestedType("Ref") self.assertTrue(ref) - self.assertEqual(ref.GetCanonicalType(), foo_type) + self.assertEqual(ref.GetCanonicalType(), var_type) + + def test_definition_in_main(self): + self.build() + target = self.dbg.CreateTarget(self.getBuildArtifact("a.out")) + self.check_global_var(target, "gLibExternalDef") + + def test_definition_in_lib(self): + self.build() + target = self.dbg.CreateTarget(self.getBuildArtifact("a.out")) + 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 index 2bc38f6503396d..b50a13afe1acc7 100644 --- a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp @@ -6,10 +6,5 @@ template <typename T> struct FooImpl { Ref Create() { return new FooImpl<T>(); } }; -Foo getString() { - FooImpl<char> impl; - Foo ret; - ret.impl = impl.Create(); - - return ret; -} +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 index d5dfddf2246209..d909fe58df12b3 100644 --- a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h @@ -2,9 +2,6 @@ #define LIB_H_IN template <typename T> struct FooImpl; - -struct Foo { - FooImpl<char> *impl = nullptr; -}; +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 index 61c819ca0d31a1..acf8337a69d0bb 100644 --- a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp @@ -1,8 +1,12 @@ #include "lib.h" -extern Foo getString(); +template <typename T> struct BarImpl { + using Ref = BarImpl<T> *; -int main() { - FooImpl<char> *foo = getString().impl; - return 0; -} + Ref Create() { return new BarImpl<T>(); } +}; + +BarImpl<char> gMainLocalDef; +FooImpl<char> *gMainExternalDef = nullptr; + +int main() { return 0; } >From b156070dd7d667e28fe86a021785faeefd5783eb Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Fri, 20 Dec 2024 23:37:49 +0000 Subject: [PATCH 6/7] [lldb][SymbolFileDWARF] Ignore Declaration when searching through UniqueDWARFASTTypeList in C++ --- .../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 43 ++++-- .../DWARF/DWARFASTParserClangTests.cpp | 143 ++++++++++++++++++ 2 files changed, 176 insertions(+), 10 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp index 3d201e96f92c3c..b598768b6e49f9 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp @@ -7,8 +7,10 @@ //===----------------------------------------------------------------------===// #include "UniqueDWARFASTType.h" +#include "SymbolFileDWARF.h" #include "lldb/Core/Declaration.h" +#include "lldb/Target/Language.h" using namespace lldb_private::dwarf; using namespace lldb_private::plugin::dwarf; @@ -18,6 +20,33 @@ static bool IsStructOrClassTag(llvm::dwarf::Tag Tag) { Tag == llvm::dwarf::Tag::DW_TAG_structure_type; } +static bool IsSizeAndDeclarationMatching(UniqueDWARFASTType const &udt, + DWARFDIE const &die, + const lldb_private::Declaration &decl, + const int32_t byte_size, + bool is_forward_declaration) { + + // If they are not both definition DIEs or both declaration DIEs, then + // don't check for byte size and declaration location, because declaration + // DIEs usually don't have those info. + if (udt.m_is_forward_declaration != is_forward_declaration) + return true; + + if (udt.m_byte_size > 0 && byte_size > 0 && udt.m_byte_size != byte_size) + return false; + + // For C++, we match the behaviour of + // DWARFASTParserClang::GetUniqueTypeNameAndDeclaration. We rely on the + // one-definition-rule: for a given fully qualified name there exists only one + // definition, and there should only be one entry for such name, so ignore + // location of where it was declared vs. defined. + if (lldb_private::Language::LanguageIsCPlusPlus( + SymbolFileDWARF::GetLanguage(*die.GetCU()))) + return true; + + return udt.m_declaration == decl; +} + UniqueDWARFASTType *UniqueDWARFASTTypeList::Find( const DWARFDIE &die, const lldb_private::Declaration &decl, const int32_t byte_size, bool is_forward_declaration) { @@ -25,17 +54,11 @@ UniqueDWARFASTType *UniqueDWARFASTTypeList::Find( // Make sure the tags match if (udt.m_die.Tag() == die.Tag() || (IsStructOrClassTag(udt.m_die.Tag()) && IsStructOrClassTag(die.Tag()))) { - // If they are not both definition DIEs or both declaration DIEs, then - // don't check for byte size and declaration location, because declaration - // DIEs usually don't have those info. - bool matching_size_declaration = - udt.m_is_forward_declaration != is_forward_declaration - ? true - : (udt.m_byte_size < 0 || byte_size < 0 || - udt.m_byte_size == byte_size) && - udt.m_declaration == decl; - if (!matching_size_declaration) + + if (!IsSizeAndDeclarationMatching(udt, die, decl, byte_size, + is_forward_declaration)) continue; + // The type has the same name, and was defined on the same file and // line. Now verify all of the parent DIEs match. DWARFDIE parent_arg_die = die.GetParent(); diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp index ee90855437a71e..f22d76b3973e5f 100644 --- a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp +++ b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp @@ -598,3 +598,146 @@ TEST_F(DWARFASTParserClangTests, TestDefaultTemplateParamParsing) { } } } + +TEST_F(DWARFASTParserClangTests, TestUniqueDWARFASTTypeMap_CppInsertMapFind) { + // This tests the behaviour of UniqueDWARFASTTypeMap under + // following scenario: + // 1. DWARFASTParserClang parses a forward declaration and + // inserts it into the UniqueDWARFASTTypeMap. + // 2. We then MapDeclDIEToDefDIE which updates the map + // entry with the line number/file information of the definition. + // 3. Parse the definition DIE, which should return the previously + // parsed type from the UniqueDWARFASTTypeMap. + + const char *yamldata = R"( +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + Type: ET_EXEC + Machine: EM_AARCH64 +DWARF: + debug_str: + - Foo + + debug_line: + - Version: 4 + MinInstLength: 1 + MaxOpsPerInst: 1 + DefaultIsStmt: 1 + LineBase: 0 + LineRange: 0 + Files: + - Name: main.cpp + DirIdx: 0 + ModTime: 0 + Length: 0 + + debug_abbrev: + - ID: 0 + Table: + - Code: 0x01 + Tag: DW_TAG_compile_unit + Children: DW_CHILDREN_yes + Attributes: + - Attribute: DW_AT_language + Form: DW_FORM_data2 + - Attribute: DW_AT_stmt_list + Form: DW_FORM_sec_offset + - Code: 0x02 + Tag: DW_TAG_structure_type + Children: DW_CHILDREN_no + Attributes: + - Attribute: DW_AT_name + Form: DW_FORM_strp + - Attribute: DW_AT_declaration + Form: DW_FORM_flag_present + - Code: 0x03 + Tag: DW_TAG_structure_type + Children: DW_CHILDREN_no + Attributes: + - Attribute: DW_AT_name + Form: DW_FORM_strp + - Attribute: DW_AT_decl_file + Form: DW_FORM_data1 + - Attribute: DW_AT_decl_line + Form: DW_FORM_data1 + + debug_info: + - Version: 5 + UnitType: DW_UT_compile + AddrSize: 8 + Entries: +# 0x0c: DW_TAG_compile_unit +# DW_AT_language [DW_FORM_data2] (DW_LANG_C_plus_plus) +# DW_AT_stmt_list [DW_FORM_sec_offset] + - AbbrCode: 0x01 + Values: + - Value: 0x04 + - Value: 0x0000000000000000 + +# 0x0d: DW_TAG_structure_type +# DW_AT_name [DW_FORM_strp] (\"Foo\") +# DW_AT_declaration [DW_FORM_flag_present] (true) + - AbbrCode: 0x02 + Values: + - Value: 0x00 + +# 0x0f: DW_TAG_structure_type +# DW_AT_name [DW_FORM_strp] (\"Foo\") +# DW_AT_decl_file [DW_FORM_data1] (main.cpp) +# DW_AT_decl_line [DW_FORM_data1] (3) + - AbbrCode: 0x03 + Values: + - Value: 0x00 + - Value: 0x01 + - Value: 0x03 + + - AbbrCode: 0x00 # end of child tags of 0x0c +... +)"; + YAMLModuleTester t(yamldata); + + DWARFUnit *unit = t.GetDwarfUnit(); + ASSERT_NE(unit, nullptr); + const DWARFDebugInfoEntry *cu_entry = unit->DIE().GetDIE(); + ASSERT_EQ(cu_entry->Tag(), DW_TAG_compile_unit); + ASSERT_EQ(unit->GetDWARFLanguageType(), DW_LANG_C_plus_plus); + DWARFDIE cu_die(unit, cu_entry); + + auto holder = std::make_unique<clang_utils::TypeSystemClangHolder>("ast"); + auto &ast_ctx = *holder->GetAST(); + DWARFASTParserClangStub ast_parser(ast_ctx); + + DWARFDIE decl_die; + DWARFDIE def_die; + for (auto const &die : cu_die.children()) { + if (die.Tag() != DW_TAG_structure_type) + continue; + + if (die.GetAttributeValueAsOptionalUnsigned(llvm::dwarf::DW_AT_declaration)) + decl_die = die; + else + def_die = die; + } + + ASSERT_TRUE(decl_die.IsValid()); + ASSERT_TRUE(def_die.IsValid()); + ASSERT_NE(decl_die, def_die); + + ParsedDWARFTypeAttributes attrs(def_die); + ASSERT_TRUE(attrs.decl.IsValid()); + + SymbolContext sc; + bool new_type = false; + lldb::TypeSP type_sp = ast_parser.ParseTypeFromDWARF(sc, decl_die, &new_type); + ASSERT_NE(type_sp, nullptr); + + ast_parser.MapDeclDIEToDefDIE(decl_die, def_die); + + lldb::TypeSP reparsed_type_sp = + ast_parser.ParseTypeFromDWARF(sc, def_die, &new_type); + ASSERT_NE(reparsed_type_sp, nullptr); + + ASSERT_EQ(type_sp, reparsed_type_sp); +} >From 8397543c42e15b92e41dfb5af3a829b6670f3082 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Mon, 23 Dec 2024 10:31:29 +0000 Subject: [PATCH 7/7] fixup! merge test cases; fix comment --- .../typedef-to-outer-fwd/TestTypedefToOuterFwd.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) 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 index 526afeb71c5877..7e9f6967a1288c 100644 --- a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py +++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py @@ -6,9 +6,11 @@ class TestCaseTypedefToOuterFwd(TestBase): """ - We a global variable whose type is forward declared. We then - try to get the Ref typedef (whose definition is in either main.o - or lib.o). Make sure we correctly resolve this typedef. + 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. @@ -29,12 +31,8 @@ def check_global_var(self, target, name: str): self.assertEqual(ref.GetCanonicalType(), var_type) - def test_definition_in_main(self): + def test(self): self.build() target = self.dbg.CreateTarget(self.getBuildArtifact("a.out")) self.check_global_var(target, "gLibExternalDef") - - def test_definition_in_lib(self): - self.build() - target = self.dbg.CreateTarget(self.getBuildArtifact("a.out")) self.check_global_var(target, "gMainExternalDef") _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
