[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

Zequan Wu via lldb-commits lldb-commits at lists.llvm.org
Wed Jul 10 11:01:07 PDT 2024


https://github.com/ZequanWu created https://github.com/llvm/llvm-project/pull/98361

This is a reapply of https://github.com/llvm/llvm-project/pull/92328 and https://github.com/llvm/llvm-project/pull/93839.

It now passes the [test](https://github.com/llvm/llvm-project/commit/de3f1b6d68ab8a0e827db84b328803857a4f60df), which crashes with the original reverted changes.

>From 37b6878b9125c314c75053f7d5b0ba520111e9a3 Mon Sep 17 00:00:00 2001
From: Zequan Wu <zequanwu at google.com>
Date: Tue, 9 Jul 2024 15:28:19 -0700
Subject: [PATCH] Reapply [lldb][DWARF] Delay struct/class/union definition DIE
 searching when parsing declaration DIEs.

---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 279 ++++++++----------
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      |  67 +++--
 .../SymbolFile/DWARF/SymbolFileDWARF.h        |  15 +-
 .../DWARF/SymbolFileDWARFDebugMap.h           |   9 +
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp   |   2 +-
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.h     |   3 +-
 .../SymbolFile/DWARF/UniqueDWARFASTType.cpp   | 117 ++++----
 .../SymbolFile/DWARF/UniqueDWARFASTType.h     |  36 +--
 .../delayed-definition-die-searching.test     |  36 +++
 .../x86/simple-template-names-context.cpp     |   4 +-
 10 files changed, 301 insertions(+), 267 deletions(-)
 create mode 100644 lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 8e297141f4e13..7b93f6941ddda 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1603,41 +1603,74 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
 
 TypeSP
 DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
-                                           const DWARFDIE &decl_die,
+                                           const DWARFDIE &die,
                                            ParsedDWARFTypeAttributes &attrs) {
   CompilerType clang_type;
-  const dw_tag_t tag = decl_die.Tag();
-  SymbolFileDWARF *dwarf = decl_die.GetDWARF();
-  LanguageType cu_language = SymbolFileDWARF::GetLanguage(*decl_die.GetCU());
+  const dw_tag_t tag = die.Tag();
+  SymbolFileDWARF *dwarf = die.GetDWARF();
+  LanguageType cu_language = SymbolFileDWARF::GetLanguage(*die.GetCU());
   Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
 
-  // UniqueDWARFASTType is large, so don't create a local variables on the
-  // stack, put it on the heap. This function is often called recursively and
-  // clang isn't good at sharing the stack space for variables in different
-  // blocks.
-  auto unique_ast_entry_up = std::make_unique<UniqueDWARFASTType>();
-
   ConstString unique_typename(attrs.name);
   Declaration unique_decl(attrs.decl);
+  uint64_t byte_size = attrs.byte_size.value_or(0);
+  if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
+      !die.HasChildren() && cu_language == eLanguageTypeObjC) {
+    // Work around an issue with clang at the moment where forward
+    // declarations for objective C classes are emitted as:
+    //  DW_TAG_structure_type [2]
+    //  DW_AT_name( "ForwardObjcClass" )
+    //  DW_AT_byte_size( 0x00 )
+    //  DW_AT_decl_file( "..." )
+    //  DW_AT_decl_line( 1 )
+    //
+    // Note that there is no DW_AT_declaration and there are no children,
+    // and the byte size is zero.
+    attrs.is_forward_declaration = true;
+  }
 
   if (attrs.name) {
     if (Language::LanguageIsCPlusPlus(cu_language)) {
       // For C++, we rely solely upon the one definition rule that says
       // only one thing can exist at a given decl context. We ignore the
       // file and line that things are declared on.
-      std::string qualified_name = GetCPlusPlusQualifiedName(decl_die);
+      std::string qualified_name = GetCPlusPlusQualifiedName(die);
       if (!qualified_name.empty())
         unique_typename = ConstString(qualified_name);
       unique_decl.Clear();
     }
 
-    if (dwarf->GetUniqueDWARFASTTypeMap().Find(
-            unique_typename, decl_die, unique_decl,
-            attrs.byte_size.value_or(-1), *unique_ast_entry_up)) {
-      if (TypeSP type_sp = unique_ast_entry_up->m_type_sp) {
+    if (UniqueDWARFASTType *unique_ast_entry_type =
+            dwarf->GetUniqueDWARFASTTypeMap().Find(
+                unique_typename, die, unique_decl, byte_size,
+                attrs.is_forward_declaration)) {
+      if (TypeSP type_sp = unique_ast_entry_type->m_type_sp) {
+        dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
         LinkDeclContextToDIE(
-            GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die),
-            decl_die);
+            GetCachedClangDeclContextForDIE(unique_ast_entry_type->m_die), die);
+        // If the DIE being parsed in this function is a definition and the
+        // entry in the map is a declaration, then we need to update the entry
+        // to point to the definition DIE.
+        if (!attrs.is_forward_declaration &&
+            unique_ast_entry_type->m_is_forward_declaration) {
+          unique_ast_entry_type->m_die = die;
+          unique_ast_entry_type->m_byte_size = byte_size;
+          unique_ast_entry_type->m_declaration = unique_decl;
+          unique_ast_entry_type->m_is_forward_declaration = false;
+          // Need to update Type ID to refer to the definition DIE. because
+          // it's used in ParseSubroutine to determine if we need to copy cxx
+          // method types from a declaration DIE to this definition DIE.
+          type_sp->SetID(die.GetID());
+          clang_type = type_sp->GetForwardCompilerType();
+
+          CompilerType compiler_type_no_qualifiers =
+              ClangUtil::RemoveFastQualifiers(clang_type);
+          auto result = dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace(
+              compiler_type_no_qualifiers.GetOpaqueQualType(),
+              *die.GetDIERef());
+          if (!result.second)
+            result.first->second = *die.GetDIERef();
+        }
         return type_sp;
       }
     }
@@ -1659,128 +1692,56 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
     default_accessibility = eAccessPrivate;
   }
 
-  if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
-      !decl_die.HasChildren() && cu_language == eLanguageTypeObjC) {
-    // Work around an issue with clang at the moment where forward
-    // declarations for objective C classes are emitted as:
-    //  DW_TAG_structure_type [2]
-    //  DW_AT_name( "ForwardObjcClass" )
-    //  DW_AT_byte_size( 0x00 )
-    //  DW_AT_decl_file( "..." )
-    //  DW_AT_decl_line( 1 )
-    //
-    // Note that there is no DW_AT_declaration and there are no children,
-    // and the byte size is zero.
-    attrs.is_forward_declaration = true;
-  }
+  if ((attrs.class_language == eLanguageTypeObjC ||
+       attrs.class_language == eLanguageTypeObjC_plus_plus) &&
+      !attrs.is_complete_objc_class &&
+      die.Supports_DW_AT_APPLE_objc_complete_type()) {
+    // We have a valid eSymbolTypeObjCClass class symbol whose name
+    // matches the current objective C class that we are trying to find
+    // and this DIE isn't the complete definition (we checked
+    // is_complete_objc_class above and know it is false), so the real
+    // definition is in here somewhere
+    TypeSP type_sp =
+        dwarf->FindCompleteObjCDefinitionTypeForDIE(die, attrs.name, true);
 
-  if (attrs.class_language == eLanguageTypeObjC ||
-      attrs.class_language == eLanguageTypeObjC_plus_plus) {
-    if (!attrs.is_complete_objc_class &&
-        decl_die.Supports_DW_AT_APPLE_objc_complete_type()) {
-      // We have a valid eSymbolTypeObjCClass class symbol whose name
-      // matches the current objective C class that we are trying to find
-      // and this DIE isn't the complete definition (we checked
-      // is_complete_objc_class above and know it is false), so the real
-      // definition is in here somewhere
-      TypeSP type_sp =
-          dwarf->FindCompleteObjCDefinitionTypeForDIE(decl_die, attrs.name, true);
-
-      if (!type_sp) {
-        SymbolFileDWARFDebugMap *debug_map_symfile =
-            dwarf->GetDebugMapSymfile();
-        if (debug_map_symfile) {
-          // We weren't able to find a full declaration in this DWARF,
-          // see if we have a declaration anywhere else...
-          type_sp = debug_map_symfile->FindCompleteObjCDefinitionTypeForDIE(
-              decl_die, attrs.name, true);
-        }
+    if (!type_sp) {
+      SymbolFileDWARFDebugMap *debug_map_symfile = dwarf->GetDebugMapSymfile();
+      if (debug_map_symfile) {
+        // We weren't able to find a full declaration in this DWARF,
+        // see if we have a declaration anywhere else...
+        type_sp = debug_map_symfile->FindCompleteObjCDefinitionTypeForDIE(
+            die, attrs.name, true);
       }
+    }
 
-      if (type_sp) {
-        if (log) {
-          dwarf->GetObjectFile()->GetModule()->LogMessage(
-              log,
-              "SymbolFileDWARF({0:p}) - {1:x16}: {2} ({3}) type \"{4}\" is an "
-              "incomplete objc type, complete type is {5:x8}",
-              static_cast<void *>(this), decl_die.GetOffset(),
-              DW_TAG_value_to_name(tag), tag, attrs.name.GetCString(),
-              type_sp->GetID());
-        }
-        return type_sp;
+    if (type_sp) {
+      if (log) {
+        dwarf->GetObjectFile()->GetModule()->LogMessage(
+            log,
+            "SymbolFileDWARF({0:p}) - {1:x16}: {2} ({3}) type \"{4}\" is an "
+            "incomplete objc type, complete type is {5:x8}",
+            static_cast<void *>(this), die.GetID(), DW_TAG_value_to_name(tag),
+            tag, attrs.name.GetCString(), type_sp->GetID());
       }
+      return type_sp;
     }
   }
 
-  DWARFDIE def_die;
   if (attrs.is_forward_declaration) {
-    Progress progress(llvm::formatv(
-        "Parsing type in {0}: '{1}'",
-        dwarf->GetObjectFile()->GetFileSpec().GetFilename().GetString(),
-        attrs.name.GetString()));
-
-    // We have a forward declaration to a type and we need to try and
-    // find a full declaration. We look in the current type index just in
-    // case we have a forward declaration followed by an actual
-    // declarations in the DWARF. If this fails, we need to look
-    // elsewhere...
-    if (log) {
-      dwarf->GetObjectFile()->GetModule()->LogMessage(
-          log,
-          "SymbolFileDWARF({0:p}) - {1:x16}: {2} ({3}) type \"{4}\" is a "
-          "forward declaration, trying to find complete type",
-          static_cast<void *>(this), decl_die.GetID(),
-          DW_TAG_value_to_name(tag), tag, attrs.name.GetCString());
-    }
-
     // See if the type comes from a Clang module and if so, track down
     // that type.
-    if (TypeSP type_sp = ParseTypeFromClangModule(sc, decl_die, log))
+    TypeSP type_sp = ParseTypeFromClangModule(sc, die, log);
+    if (type_sp)
       return type_sp;
-
-    def_die = dwarf->FindDefinitionDIE(decl_die);
-
-    if (!def_die) {
-      SymbolFileDWARFDebugMap *debug_map_symfile = dwarf->GetDebugMapSymfile();
-      if (debug_map_symfile) {
-        // We weren't able to find a full declaration in this DWARF, see
-        // if we have a declaration anywhere else...
-        def_die = debug_map_symfile->FindDefinitionDIE(decl_die);
-      }
-    }
-
-    if (log) {
-      dwarf->GetObjectFile()->GetModule()->LogMessage(
-          log,
-          "SymbolFileDWARF({0:p}) - {1:x16}: {2} ({3}) type \"{4}\" is a "
-          "forward declaration, complete type is {5}",
-          static_cast<void *>(this), def_die.GetID(), DW_TAG_value_to_name(tag),
-          tag, attrs.name.GetCString(),
-          def_die ? llvm::utohexstr(def_die.GetID()) : "not found");
-    }
   }
 
-  if (def_die) {
-    if (auto [it, inserted] = dwarf->GetDIEToType().try_emplace(
-            def_die.GetDIE(), DIE_IS_BEING_PARSED);
-        !inserted) {
-      if (it->getSecond() == nullptr || it->getSecond() == DIE_IS_BEING_PARSED)
-        return nullptr;
-      return it->getSecond()->shared_from_this();
-    }
-    attrs = ParsedDWARFTypeAttributes(def_die);
-  } else {
-    // No definition found. Proceed with the declaration die. We can use it to
-    // create a forward-declared type.
-    def_die = decl_die;
-  }
   assert(tag_decl_kind != -1);
   UNUSED_IF_ASSERT_DISABLED(tag_decl_kind);
-  bool clang_type_was_created = false;
-  clang::DeclContext *containing_decl_ctx = GetClangDeclContextContainingDIE(def_die, nullptr);
+  clang::DeclContext *containing_decl_ctx =
+      GetClangDeclContextContainingDIE(die, nullptr);
 
   PrepareContextToReceiveMembers(m_ast, GetClangASTImporter(),
-                                 containing_decl_ctx, def_die,
+                                 containing_decl_ctx, die,
                                  attrs.name.GetCString());
 
   if (attrs.accessibility == eAccessNone && containing_decl_ctx) {
@@ -1793,50 +1754,47 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
   }
 
   ClangASTMetadata metadata;
-  metadata.SetUserID(def_die.GetID());
-  metadata.SetIsDynamicCXXType(dwarf->ClassOrStructIsVirtual(def_die));
+  metadata.SetUserID(die.GetID());
+  metadata.SetIsDynamicCXXType(dwarf->ClassOrStructIsVirtual(die));
 
   TypeSystemClang::TemplateParameterInfos template_param_infos;
-  if (ParseTemplateParameterInfos(def_die, template_param_infos)) {
+  if (ParseTemplateParameterInfos(die, template_param_infos)) {
     clang::ClassTemplateDecl *class_template_decl =
         m_ast.ParseClassTemplateDecl(
-            containing_decl_ctx, GetOwningClangModule(def_die),
-            attrs.accessibility, attrs.name.GetCString(), tag_decl_kind,
-            template_param_infos);
+            containing_decl_ctx, GetOwningClangModule(die), attrs.accessibility,
+            attrs.name.GetCString(), tag_decl_kind, template_param_infos);
     if (!class_template_decl) {
       if (log) {
         dwarf->GetObjectFile()->GetModule()->LogMessage(
             log,
             "SymbolFileDWARF({0:p}) - {1:x16}: {2} ({3}) type \"{4}\" "
             "clang::ClassTemplateDecl failed to return a decl.",
-            static_cast<void *>(this), def_die.GetID(),
-            DW_TAG_value_to_name(tag), tag, attrs.name.GetCString());
+            static_cast<void *>(this), die.GetID(), DW_TAG_value_to_name(tag),
+            tag, attrs.name.GetCString());
       }
       return TypeSP();
     }
 
     clang::ClassTemplateSpecializationDecl *class_specialization_decl =
         m_ast.CreateClassTemplateSpecializationDecl(
-            containing_decl_ctx, GetOwningClangModule(def_die),
-            class_template_decl, tag_decl_kind, template_param_infos);
+            containing_decl_ctx, GetOwningClangModule(die), class_template_decl,
+            tag_decl_kind, template_param_infos);
     clang_type =
         m_ast.CreateClassTemplateSpecializationType(class_specialization_decl);
-    clang_type_was_created = true;
 
     m_ast.SetMetadata(class_template_decl, metadata);
     m_ast.SetMetadata(class_specialization_decl, metadata);
   }
 
-  if (!clang_type_was_created) {
-    clang_type_was_created = true;
+  if (!clang_type) {
     clang_type = m_ast.CreateRecordType(
-        containing_decl_ctx, GetOwningClangModule(def_die), attrs.accessibility,
+        containing_decl_ctx, GetOwningClangModule(die), attrs.accessibility,
         attrs.name.GetCString(), tag_decl_kind, attrs.class_language, &metadata,
         attrs.exports_symbols);
   }
 
   TypeSP type_sp = dwarf->MakeType(
-      def_die.GetID(), attrs.name, attrs.byte_size, nullptr, LLDB_INVALID_UID,
+      die.GetID(), attrs.name, attrs.byte_size, nullptr, LLDB_INVALID_UID,
       Type::eEncodingIsUID, &attrs.decl, clang_type,
       Type::ResolveState::Forward,
       TypePayloadClang(OptionalClangModuleID(), attrs.is_complete_objc_class));
@@ -1846,39 +1804,38 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
   // function prototypes.
   clang::DeclContext *type_decl_ctx =
       TypeSystemClang::GetDeclContextForType(clang_type);
-  LinkDeclContextToDIE(type_decl_ctx, decl_die);
-  if (decl_die != def_die) {
-    LinkDeclContextToDIE(type_decl_ctx, def_die);
-    dwarf->GetDIEToType()[def_die.GetDIE()] = type_sp.get();
-    // Declaration DIE is inserted into the type map in ParseTypeFromDWARF
-  }
+  LinkDeclContextToDIE(type_decl_ctx, die);
 
+  // UniqueDWARFASTType is large, so don't create a local variables on the
+  // stack, put it on the heap. This function is often called recursively and
+  // clang isn't good at sharing the stack space for variables in different
+  // blocks.
+  auto unique_ast_entry_up = std::make_unique<UniqueDWARFASTType>();
   // Add our type to the unique type map so we don't end up creating many
   // copies of the same type over and over in the ASTContext for our
   // module
   unique_ast_entry_up->m_type_sp = type_sp;
-  unique_ast_entry_up->m_die = def_die;
+  unique_ast_entry_up->m_die = die;
   unique_ast_entry_up->m_declaration = unique_decl;
-  unique_ast_entry_up->m_byte_size = attrs.byte_size.value_or(0);
+  unique_ast_entry_up->m_byte_size = byte_size;
+  unique_ast_entry_up->m_is_forward_declaration = attrs.is_forward_declaration;
   dwarf->GetUniqueDWARFASTTypeMap().Insert(unique_typename,
                                            *unique_ast_entry_up);
 
-  if (clang_type_was_created) {
-    // Leave this as a forward declaration until we need to know the
-    // details of the type. lldb_private::Type will automatically call
-    // the SymbolFile virtual function
-    // "SymbolFileDWARF::CompleteType(Type *)" When the definition
-    // needs to be defined.
-    bool inserted =
-        dwarf->GetForwardDeclCompilerTypeToDIE()
-            .try_emplace(
-                ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(),
-                *def_die.GetDIERef())
-            .second;
-    assert(inserted && "Type already in the forward declaration map!");
-    (void)inserted;
-    m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
-  }
+  // Leave this as a forward declaration until we need to know the
+  // details of the type. lldb_private::Type will automatically call
+  // the SymbolFile virtual function
+  // "SymbolFileDWARF::CompleteType(Type *)" When the definition
+  // needs to be defined.
+  bool inserted =
+      dwarf->GetForwardDeclCompilerTypeToDIE()
+          .try_emplace(
+              ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(),
+              *die.GetDIERef())
+          .second;
+  assert(inserted && "Type already in the forward declaration map!");
+  (void)inserted;
+  m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
 
   // If we made a clang type, set the trivial abi if applicable: We only
   // do this for pass by value - which implies the Trivial ABI. There
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index f2ff3a8b259fa..e1c0ddd8e9385 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/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)
@@ -1631,26 +1638,48 @@ 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);
+  DWARFDIE decl_die = GetDIE(die_it->getSecond());
+  // 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.
+  GetForwardDeclCompilerTypeToDIE().erase(die_it);
+  DWARFDIE def_die = FindDefinitionDIE(decl_die);
+  if (!def_die) {
+    SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
+    if (debug_map_symfile) {
+      // We weren't able to find a full declaration in this DWARF, see
+      // if we have a declaration anywhere else...
+      def_die = debug_map_symfile->FindDefinitionDIE(decl_die);
+    }
+  }
+  if (!def_die) {
+    // No definition found. Proceed with the declaration die. We can use it to
+    // create a forward-declared type.
+    def_die = decl_die;
+  }
 
-    Type *type = GetDIEToType().lookup(dwarf_die.GetDIE());
+  Type *type = ResolveType(def_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);
+  if (def_die != decl_die) {
+    // After the call to FindDefinitionDIE, we have a new mapping from the old
+    // CompilerType to definition DIE. Remove it to mark it as completed.
+    // TODO: Maybe this requires a more robust way to mark the type is already
+    // completed.
+    GetForwardDeclCompilerTypeToDIE().erase(
+        compiler_type_no_qualifiers.GetOpaqueQualType());
   }
+
+  Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion);
+  if (log)
+    GetObjectFile()->GetModule()->LogMessageVerboseBacktrace(
+        log, "{0:x8}: {1} ({2}) '{3}' resolving forward declaration...",
+        def_die.GetID(), DW_TAG_value_to_name(def_die.Tag()), def_die.Tag(),
+        type->GetName().AsCString());
+  assert(compiler_type);
+  if (DWARFASTParser *dwarf_ast = GetDWARFParser(*def_die.GetCU()))
+    return dwarf_ast->CompleteTypeFromDWARF(def_die, type, compiler_type);
   return false;
 }
 
@@ -3047,8 +3076,10 @@ TypeSP SymbolFileDWARF::FindCompleteObjCDefinitionTypeForDIE(
 
 DWARFDIE
 SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE &die) {
-  if (!die.GetName())
+  if (!die || !die.GetName())
     return {};
+  if (!die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0))
+    return die;
 
   const dw_tag_t tag = die.Tag();
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index 8469248872a44..4967b37d753a0 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -342,12 +342,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;
@@ -537,9 +533,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;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
index 7d5516b92737b..34cb52e5b601c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ b/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;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
index 4a8c532a0d2a5..49632e1d8911c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ b/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();
 }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
index 3bd0a2d25a5a6..15c28fefd81f9 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -74,7 +74,8 @@ class SymbolFileDWARFDwo : public SymbolFileDWARF {
 
   DIEToVariableSP &GetDIEToVariable() override;
 
-  CompilerTypeToDIE &GetForwardDeclCompilerTypeToDIE() override;
+  llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> &
+  GetForwardDeclCompilerTypeToDIE() override;
 
   UniqueDWARFASTTypeMap &GetUniqueDWARFASTTypeMap() override;
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
index 223518f0ae824..3d201e96f92c3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
@@ -13,66 +13,75 @@
 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) {
-    // 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;
+static bool IsStructOrClassTag(llvm::dwarf::Tag Tag) {
+  return Tag == llvm::dwarf::Tag::DW_TAG_class_type ||
+         Tag == llvm::dwarf::Tag::DW_TAG_structure_type;
+}
 
-              case DW_TAG_compile_unit:
-              case DW_TAG_partial_unit:
-                done = true;
-                break;
-              default:
-                break;
-              }
+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() || (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;
             }
-            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;
 }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.h b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.h
index bf3cbae55e5c7..29e5c02dcbe17 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.h
+++ b/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:
diff --git a/lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test b/lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test
new file mode 100644
index 0000000000000..d253981b498c8
--- /dev/null
+++ b/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;
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp
index 8070b7a19abcc..80f22e50f6a36 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp
@@ -4,8 +4,8 @@
 
 // REQUIRES: lld
 
-// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-a.o -g -gsimple-template-names -DFILE_A
-// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-b.o -g -gsimple-template-names -DFILE_B
+// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-a.o -g -fdebug-types-section -DFILE_A
+// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-b.o -g -fdebug-types-section -DFILE_B
 // RUN: ld.lld %t-a.o %t-b.o -o %t
 // RUN: %lldb %t -o "target variable --ptr-depth 1 --show-types both_a both_b" -o exit | FileCheck %s
 



More information about the lldb-commits mailing list