Revert "[lldb][DebugNames] Only skip processing of DW_AT_declarations for class/union types"

and two follow-up commits. The reason is the crash we've discovered when
processing -gsimple-template-names binaries. I'm committing a minimal
reproducer as a separate patch.

This reverts the following commits:
- 51dd4ea (#92328)
- 3d9d485 (#93839)
- afe6ab7 (#94400)

Author: labath

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

@@ -60,8 +60,6 @@ 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/DWARFASTParserClang.cpp

@@ -85,11 +85,6 @@ bool DebugNamesDWARFIndex::ProcessEntry(
   DWARFDIE die = GetDIE(entry);
   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.IsStructUnionOrClass() &&
-      die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0))
-    return true;
   return callback(die);
 }
 

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

@@ -481,13 +481,6 @@ 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)
@@ -1639,33 +1632,27 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) {
     return true;
   }
 
-  // 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;
-
-  die_it = GetForwardDeclCompilerTypeToDIE().find(
-      compiler_type_no_qualifiers.GetOpaqueQualType());
-  if (die_it != GetForwardDeclCompilerTypeToDIE().end()) {
-    dwarf_die = GetDIE(die_it->getSecond());
+  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);
-  }
 
-  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 *type = GetDIEToType().lookup(dwarf_die.GetDIE());
+
+    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);
+  }
+  return false;
 }
 
 Type *SymbolFileDWARF::ResolveType(const DWARFDIE &die,

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

@@ -335,8 +335,12 @@ class SymbolFileDWARF : public SymbolFileCommon {
 
   virtual DIEToTypePtr &GetDIEToType() { return m_die_to_type; }
 
-  virtual llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> &
-  GetForwardDeclCompilerTypeToDIE();
+  typedef llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef>
+      CompilerTypeToDIE;
+
+  virtual CompilerTypeToDIE &GetForwardDeclCompilerTypeToDIE() {
+    return m_forward_decl_compiler_type_to_die;
+  }
 
   typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb::VariableSP>
       DIEToVariableSP;
@@ -529,14 +533,9 @@ 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;
-  // 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;
+  CompilerTypeToDIE 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/SymbolFileDWARF.cpp

@@ -284,11 +284,6 @@ 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;
   }
@@ -326,10 +321,6 @@ 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/SymbolFileDWARF.h

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

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

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

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

@@ -13,75 +13,66 @@
 using namespace lldb_private::dwarf;
 using namespace lldb_private::plugin::dwarf;
 
-static bool IsStructOrClassTag(llvm::dwarf::Tag Tag) {
-  return Tag == llvm::dwarf::Tag::DW_TAG_class_type ||
-         Tag == llvm::dwarf::Tag::DW_TAG_structure_type;
-}
-
-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) {
+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) {
     // 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)
-        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 ||
-            (IsStructOrClassTag(parent_arg_tag) &&
-             IsStructOrClassTag(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;
+    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;
+              }
             }
-          } break;
+            parent_arg_die = parent_arg_die.GetParent();
+            parent_pos_die = parent_pos_die.GetParent();
+          }
 
-          case DW_TAG_compile_unit:
-          case DW_TAG_partial_unit:
-            done = true;
-            break;
-          default:
-            break;
+          if (match) {
+            entry = udt;
+            return true;
           }
         }
-        parent_arg_die = parent_arg_die.GetParent();
-        parent_pos_die = parent_pos_die.GetParent();
-      }
-
-      if (match) {
-        return &udt;
       }
     }
   }
-  return nullptr;
+  return false;
 }

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

@@ -23,19 +23,31 @@ 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_is_forward_declaration(rhs.m_is_forward_declaration) {}
+        m_declaration(rhs.m_declaration), m_byte_size(rhs.m_byte_size) {}
 
   ~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 {
@@ -50,9 +62,8 @@ class UniqueDWARFASTTypeList {
     m_collection.push_back(entry);
   }
 
-  UniqueDWARFASTType *Find(const DWARFDIE &die, const Declaration &decl,
-                           const int32_t byte_size,
-                           bool is_forward_declaration);
+  bool Find(const DWARFDIE &die, const Declaration &decl,
+            const int32_t byte_size, UniqueDWARFASTType &entry) const;
 
 protected:
   typedef std::vector<UniqueDWARFASTType> collection;
@@ -69,15 +80,14 @@ class UniqueDWARFASTTypeMap {
     m_collection[name.GetCString()].Append(entry);
   }
 
-  UniqueDWARFASTType *Find(ConstString name, const DWARFDIE &die,
-                           const Declaration &decl, const int32_t byte_size,
-                           bool is_forward_declaration) {
+  bool Find(ConstString name, const DWARFDIE &die, const Declaration &decl,
+            const int32_t byte_size, UniqueDWARFASTType &entry) const {
     const char *unique_name_cstr = name.GetCString();
-    collection::iterator pos = m_collection.find(unique_name_cstr);
+    collection::const_iterator pos = m_collection.find(unique_name_cstr);
     if (pos != m_collection.end()) {
-      return pos->second.Find(die, decl, byte_size, is_forward_declaration);
+      return pos->second.Find(die, decl, byte_size, entry);
     }
-    return nullptr;
+    return false;
   }
 
 protected: