Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs.

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h

@@ -60,6 +60,8 @@ class DWARFASTParser {
 
   virtual ConstString GetDIEClassTemplateParams(const DWARFDIE &die) = 0;
 
+  virtual lldb_private::Type *FindDefinitionTypeForDIE(const DWARFDIE &die) = 0;
+
   static std::optional<SymbolFile::ArrayInfo>
   ParseChildArrayInfo(const DWARFDIE &parent_die,
                       const ExecutionContext *exe_ctx = nullptr);

lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp

@@ -83,6 +83,10 @@ bool DebugNamesDWARFIndex::ProcessEntry(
   DWARFDIE die = dwarf.GetDIE(*ref);
   if (!die)
     return true;
+  // Clang erroneously emits index entries for declaration DIEs in case when the
+  // definition is in a type unit (llvm.org/pr77696). Weed those out.
+  if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0))
+    return true;
   return callback(die);
 }
 

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

@@ -481,6 +481,13 @@ static ConstString GetDWARFMachOSegmentName() {
   return g_dwarf_section_name;
 }
 
+llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> &
+SymbolFileDWARF::GetForwardDeclCompilerTypeToDIE() {
+  if (SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile())
+    return debug_map_symfile->GetForwardDeclCompilerTypeToDIE();
+  return m_forward_decl_compiler_type_to_die;
+}
+
 UniqueDWARFASTTypeMap &SymbolFileDWARF::GetUniqueDWARFASTTypeMap() {
   SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
   if (debug_map_symfile)
@@ -1632,27 +1639,33 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) {
     return true;
   }
 
-  DWARFDIE dwarf_die = GetDIE(die_it->getSecond());
-  if (dwarf_die) {
-    // Once we start resolving this type, remove it from the forward
-    // declaration map in case anyone child members or other types require this
-    // type to get resolved. The type will get resolved when all of the calls
-    // to SymbolFileDWARF::ResolveClangOpaqueTypeDefinition are done.
-    GetForwardDeclCompilerTypeToDIE().erase(die_it);
-
-    Type *type = GetDIEToType().lookup(dwarf_die.GetDIE());
+  // Once we start resolving this type, remove it from the forward
+  // declaration map in case anyone's child members or other types require this
+  // type to get resolved.
+  DWARFDIE dwarf_die = GetDIE(die_it->second);
+  GetForwardDeclCompilerTypeToDIE().erase(die_it);
+  Type *type = nullptr;
+  if (DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU()))
+    type = dwarf_ast->FindDefinitionTypeForDIE(dwarf_die);
+  if (!type)
+    return false;
 
-    Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion);
-    if (log)
-      GetObjectFile()->GetModule()->LogMessageVerboseBacktrace(
-          log, "{0:x8}: {1} ({2}) '{3}' resolving forward declaration...",
-          dwarf_die.GetID(), DW_TAG_value_to_name(dwarf_die.Tag()),
-          dwarf_die.Tag(), type->GetName().AsCString());
-    assert(compiler_type);
-    if (DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU()))
-      return dwarf_ast->CompleteTypeFromDWARF(dwarf_die, type, compiler_type);
+  die_it = GetForwardDeclCompilerTypeToDIE().find(
+      compiler_type_no_qualifiers.GetOpaqueQualType());
+  if (die_it != GetForwardDeclCompilerTypeToDIE().end()) {
+    dwarf_die = GetDIE(die_it->getSecond());
+    GetForwardDeclCompilerTypeToDIE().erase(die_it);
   }
-  return false;
+
+  if (Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion))
+    GetObjectFile()->GetModule()->LogMessageVerboseBacktrace(
+        log, "{0:x8}: {1} ({2}) '{3}' resolving forward declaration...",
+        dwarf_die.GetID(), DW_TAG_value_to_name(dwarf_die.Tag()),
+        dwarf_die.Tag(), type->GetName().AsCString());
+  assert(compiler_type);
+  if (DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU()))
+    return dwarf_ast->CompleteTypeFromDWARF(dwarf_die, type, compiler_type);
+  return true;
 }
 
 Type *SymbolFileDWARF::ResolveType(const DWARFDIE &die,

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

@@ -335,12 +335,8 @@ class SymbolFileDWARF : public SymbolFileCommon {
 
   virtual DIEToTypePtr &GetDIEToType() { return m_die_to_type; }
 
-  typedef llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef>
-      CompilerTypeToDIE;
-
-  virtual CompilerTypeToDIE &GetForwardDeclCompilerTypeToDIE() {
-    return m_forward_decl_compiler_type_to_die;
-  }
+  virtual llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> &
+  GetForwardDeclCompilerTypeToDIE();
 
   typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb::VariableSP>
       DIEToVariableSP;
@@ -533,9 +529,14 @@ class SymbolFileDWARF : public SymbolFileCommon {
   NameToOffsetMap m_function_scope_qualified_name_map;
   std::unique_ptr<DWARFDebugRanges> m_ranges;
   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;
   DIEToVariableSP m_die_to_variable_sp;
-  CompilerTypeToDIE m_forward_decl_compiler_type_to_die;
+  // A map from CompilerType to the struct/class/union/enum DIE (might be a
+  // declaration or a definition) that is used to construct it.
+  llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef>
+      m_forward_decl_compiler_type_to_die;
   llvm::DenseMap<dw_offset_t, std::unique_ptr<SupportFileList>>
       m_type_unit_support_files;
   std::vector<uint32_t> m_lldb_cu_to_dwarf_unit;

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h

@@ -284,6 +284,11 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon {
   lldb::TypeSP FindCompleteObjCDefinitionTypeForDIE(
       const DWARFDIE &die, ConstString type_name, bool must_be_implementation);
 
+  llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> &
+  GetForwardDeclCompilerTypeToDIE() {
+    return m_forward_decl_compiler_type_to_die;
+  }
+
   UniqueDWARFASTTypeMap &GetUniqueDWARFASTTypeMap() {
     return m_unique_ast_type_map;
   }
@@ -321,6 +326,10 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon {
   std::vector<uint32_t> m_func_indexes; // Sorted by address
   std::vector<uint32_t> m_glob_indexes;
   std::map<std::pair<ConstString, llvm::sys::TimePoint<>>, OSOInfoSP> m_oso_map;
+  // A map from CompilerType to the struct/class/union/enum DIE (might be a
+  // declaration or a definition) that is used to construct it.
+  llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef>
+      m_forward_decl_compiler_type_to_die;
   UniqueDWARFASTTypeMap m_unique_ast_type_map;
   LazyBool m_supports_DW_AT_APPLE_objc_complete_type;
   DebugMap m_debug_map;

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

@@ -110,7 +110,7 @@ SymbolFileDWARF::DIEToVariableSP &SymbolFileDWARFDwo::GetDIEToVariable() {
   return GetBaseSymbolFile().GetDIEToVariable();
 }
 
-SymbolFileDWARF::CompilerTypeToDIE &
+llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> &
 SymbolFileDWARFDwo::GetForwardDeclCompilerTypeToDIE() {
   return GetBaseSymbolFile().GetForwardDeclCompilerTypeToDIE();
 }

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

@@ -72,7 +72,8 @@ class SymbolFileDWARFDwo : public SymbolFileDWARF {
 
   DIEToVariableSP &GetDIEToVariable() override;
 
-  CompilerTypeToDIE &GetForwardDeclCompilerTypeToDIE() override;
+  llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> &
+  GetForwardDeclCompilerTypeToDIE() override;
 
   UniqueDWARFASTTypeMap &GetUniqueDWARFASTTypeMap() override;
 

lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp

@@ -13,66 +13,67 @@
 using namespace lldb_private::dwarf;
 using namespace lldb_private::plugin::dwarf;
 
-bool UniqueDWARFASTTypeList::Find(const DWARFDIE &die,
-                                  const lldb_private::Declaration &decl,
-                                  const int32_t byte_size,
-                                  UniqueDWARFASTType &entry) const {
-  for (const UniqueDWARFASTType &udt : m_collection) {
+UniqueDWARFASTType *UniqueDWARFASTTypeList::Find(
+    const DWARFDIE &die, const lldb_private::Declaration &decl,
+    const int32_t byte_size, bool is_forward_declaration) {
+  for (UniqueDWARFASTType &udt : m_collection) {
     // Make sure the tags match
     if (udt.m_die.Tag() == die.Tag()) {
-      // Validate byte sizes of both types only if both are valid.
-      if (udt.m_byte_size < 0 || byte_size < 0 ||
-          udt.m_byte_size == byte_size) {
-        // Make sure the file and line match
-        if (udt.m_declaration == decl) {
-          // 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();
-          DWARFDIE parent_pos_die = udt.m_die.GetParent();
-          bool match = true;
-          bool done = false;
-          while (!done && match && parent_arg_die && parent_pos_die) {
-            const dw_tag_t parent_arg_tag = parent_arg_die.Tag();
-            const dw_tag_t parent_pos_tag = parent_pos_die.Tag();
-            if (parent_arg_tag == parent_pos_tag) {
-              switch (parent_arg_tag) {
-              case DW_TAG_class_type:
-              case DW_TAG_structure_type:
-              case DW_TAG_union_type:
-              case DW_TAG_namespace: {
-                const char *parent_arg_die_name = parent_arg_die.GetName();
-                if (parent_arg_die_name ==
-                    nullptr) // Anonymous (i.e. no-name) struct
-                {
-                  match = false;
-                } else {
-                  const char *parent_pos_die_name = parent_pos_die.GetName();
-                  if (parent_pos_die_name == nullptr ||
-                      ((parent_arg_die_name != parent_pos_die_name) &&
-                       strcmp(parent_arg_die_name, parent_pos_die_name)))
-                    match = false;
-                }
-              } break;
-
-              case DW_TAG_compile_unit:
-              case DW_TAG_partial_unit:
-                done = true;
-                break;
-              default:
-                break;
-              }
+      // 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)
+        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();
+      DWARFDIE parent_pos_die = udt.m_die.GetParent();
+      bool match = true;
+      bool done = false;
+      while (!done && match && parent_arg_die && parent_pos_die) {
+        const dw_tag_t parent_arg_tag = parent_arg_die.Tag();
+        const dw_tag_t parent_pos_tag = parent_pos_die.Tag();
+        if (parent_arg_tag == parent_pos_tag) {
+          switch (parent_arg_tag) {
+          case DW_TAG_class_type:
+          case DW_TAG_structure_type:
+          case DW_TAG_union_type:
+          case DW_TAG_namespace: {
+            const char *parent_arg_die_name = parent_arg_die.GetName();
+            if (parent_arg_die_name == nullptr) {
+              // Anonymous (i.e. no-name) struct
+              match = false;
+            } else {
+              const char *parent_pos_die_name = parent_pos_die.GetName();
+              if (parent_pos_die_name == nullptr ||
+                  ((parent_arg_die_name != parent_pos_die_name) &&
+                   strcmp(parent_arg_die_name, parent_pos_die_name)))
+                match = false;
             }
-            parent_arg_die = parent_arg_die.GetParent();
-            parent_pos_die = parent_pos_die.GetParent();
-          }
+          } break;
 
-          if (match) {
-            entry = udt;
-            return true;
+          case DW_TAG_compile_unit:
+          case DW_TAG_partial_unit:
+            done = true;
+            break;
+          default:
+            break;
           }
         }
+        parent_arg_die = parent_arg_die.GetParent();
+        parent_pos_die = parent_pos_die.GetParent();
+      }
+
+      if (match) {
+        return &udt;
       }
     }
   }
-  return false;
+  return nullptr;
 }

lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.h

@@ -23,31 +23,19 @@ class UniqueDWARFASTType {
   // Constructors and Destructors
   UniqueDWARFASTType() : m_type_sp(), m_die(), m_declaration() {}
 
-  UniqueDWARFASTType(lldb::TypeSP &type_sp, const DWARFDIE &die,
-                     const Declaration &decl, int32_t byte_size)
-      : m_type_sp(type_sp), m_die(die), m_declaration(decl),
-        m_byte_size(byte_size) {}
-
   UniqueDWARFASTType(const UniqueDWARFASTType &rhs)
       : m_type_sp(rhs.m_type_sp), m_die(rhs.m_die),
-        m_declaration(rhs.m_declaration), m_byte_size(rhs.m_byte_size) {}
+        m_declaration(rhs.m_declaration), m_byte_size(rhs.m_byte_size),
+        m_is_forward_declaration(rhs.m_is_forward_declaration) {}
 
   ~UniqueDWARFASTType() = default;
 
-  UniqueDWARFASTType &operator=(const UniqueDWARFASTType &rhs) {
-    if (this != &rhs) {
-      m_type_sp = rhs.m_type_sp;
-      m_die = rhs.m_die;
-      m_declaration = rhs.m_declaration;
-      m_byte_size = rhs.m_byte_size;
-    }
-    return *this;
-  }
-
   lldb::TypeSP m_type_sp;
   DWARFDIE m_die;
   Declaration m_declaration;
   int32_t m_byte_size = -1;
+  // True if the m_die is a forward declaration DIE.
+  bool m_is_forward_declaration = true;
 };
 
 class UniqueDWARFASTTypeList {
@@ -62,8 +50,9 @@ class UniqueDWARFASTTypeList {
     m_collection.push_back(entry);
   }
 
-  bool Find(const DWARFDIE &die, const Declaration &decl,
-            const int32_t byte_size, UniqueDWARFASTType &entry) const;
+  UniqueDWARFASTType *Find(const DWARFDIE &die, const Declaration &decl,
+                           const int32_t byte_size,
+                           bool is_forward_declaration);
 
 protected:
   typedef std::vector<UniqueDWARFASTType> collection;
@@ -80,14 +69,15 @@ class UniqueDWARFASTTypeMap {
     m_collection[name.GetCString()].Append(entry);
   }
 
-  bool Find(ConstString name, const DWARFDIE &die, const Declaration &decl,
-            const int32_t byte_size, UniqueDWARFASTType &entry) const {
+  UniqueDWARFASTType *Find(ConstString name, const DWARFDIE &die,
+                           const Declaration &decl, const int32_t byte_size,
+                           bool is_forward_declaration) {
     const char *unique_name_cstr = name.GetCString();
-    collection::const_iterator pos = m_collection.find(unique_name_cstr);
+    collection::iterator pos = m_collection.find(unique_name_cstr);
     if (pos != m_collection.end()) {
-      return pos->second.Find(die, decl, byte_size, entry);
+      return pos->second.Find(die, decl, byte_size, is_forward_declaration);
     }
-    return false;
+    return nullptr;
   }
 
 protected:

lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test

@@ -0,0 +1,36 @@
+# Test definition DIE searching is delayed until complete type is required.
+
+# UNSUPPORTED: system-windows
+
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -gdwarf -o %t.out
+# RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s
+
+# CHECK: (lldb) p v1
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type (DW_TAG_structure_type) name = 't2<t1>'
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type (DW_TAG_structure_type) name = 't1'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't2<t1>' resolving forward declaration...
+# CHECK: (t2<t1>)  {}
+# CHECK: (lldb) p v2
+# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type (DW_TAG_structure_type) name = 't1'
+# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't1' resolving forward declaration...
+
+#--- lldb.cmd
+log enable dwarf comp
+p v1
+p v2
+
+#--- main.cpp
+template<typename T>
+struct t2 {
+};
+struct t1;
+t2<t1> v1; // this CU doesn't have definition DIE for t1, but only declaration DIE for it.
+int main() {
+}
+
+#--- t1_def.cpp
+struct t1 { // this CU contains definition DIE for t1.
+  int x;
+};
+t1 v2;