[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] Share GetDIEToType between SymbolFiles of a SymbolFileDWARFDebugMap (PR #120569)

Michael Buch via lldb-commits lldb-commits at lists.llvm.org
Mon Dec 23 02:33:18 PST 2024


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 <michaelbuch12 at 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 <michaelbuch12 at 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 <michaelbuch12 at 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 <michaelbuch12 at 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 <michaelbuch12 at 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 <michaelbuch12 at 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 <michaelbuch12 at 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")



More information about the lldb-commits mailing list