From 37b6878b9125c314c75053f7d5b0ba520111e9a3 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Tue, 9 Jul 2024 15:28:19 -0700 Subject: [PATCH 1/5] 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 8e297141f4e132..7b93f6941dddaf 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(); - 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(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(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(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(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(this), def_die.GetID(), - DW_TAG_value_to_name(tag), tag, attrs.name.GetCString()); + static_cast(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(); // 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 f2ff3a8b259fa4..e1c0ddd8e93857 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 & +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 8469248872a44f..4967b37d753a09 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 - CompilerTypeToDIE; - - virtual CompilerTypeToDIE &GetForwardDeclCompilerTypeToDIE() { - return m_forward_decl_compiler_type_to_die; - } + virtual llvm::DenseMap & + GetForwardDeclCompilerTypeToDIE(); typedef llvm::DenseMap DIEToVariableSP; @@ -537,9 +533,14 @@ class SymbolFileDWARF : public SymbolFileCommon { NameToOffsetMap m_function_scope_qualified_name_map; std::unique_ptr 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 + m_forward_decl_compiler_type_to_die; llvm::DenseMap> m_type_unit_support_files; std::vector 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 7d5516b92737b9..34cb52e5b601c4 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 & + 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 m_func_indexes; // Sorted by address std::vector m_glob_indexes; std::map>, 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 + 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 4a8c532a0d2a5e..49632e1d8911cb 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 & SymbolFileDWARFDwo::GetForwardDeclCompilerTypeToDIE() { return GetBaseSymbolFile().GetForwardDeclCompilerTypeToDIE(); } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h index 3bd0a2d25a5a6a..15c28fefd81f9d 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 & + GetForwardDeclCompilerTypeToDIE() override; UniqueDWARFASTTypeMap &GetUniqueDWARFASTTypeMap() override; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp index 223518f0ae8241..3d201e96f92c3c 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 bf3cbae55e5c7b..29e5c02dcbe176 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 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 00000000000000..d253981b498c81 --- /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' +# CHECK: DWARFASTParserClang::ParseTypeFromDWARF{{.*}}DW_TAG_structure_type (DW_TAG_structure_type) name = 't1' +# CHECK: DW_TAG_structure_type (DW_TAG_structure_type) 't2' resolving forward declaration... +# CHECK: (t2) {} +# 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 +struct t2 { +}; +struct t1; +t2 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 8070b7a19abccc..80f22e50f6a36e 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 From 1e86ac615e751f0c98836690a1a8ca2b007be03b Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Thu, 11 Jul 2024 16:50:57 -0700 Subject: [PATCH 2/5] Address comments --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 38 +++++++++++++++++-- .../SymbolFile/DWARF/DWARFASTParserClang.h | 4 ++ .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 30 +++++++-------- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 7b93f6941dddaf..ee170feaf6b4db 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -824,6 +824,36 @@ DWARFASTParserClang::GetDIEClassTemplateParams(const DWARFDIE &die) { return {}; } +void DWARFASTParserClang::MappingDeclDIEToDefDIE( + const lldb_private::plugin::dwarf::DWARFDIE &decl_die, + const lldb_private::plugin::dwarf::DWARFDIE &def_die) { + LinkDeclContextToDIE(GetCachedClangDeclContextForDIE(decl_die), def_die); + SymbolFileDWARF *dwarf = def_die.GetDWARF(); + ParsedDWARFTypeAttributes decl_attrs(decl_die); + ParsedDWARFTypeAttributes def_attrs(def_die); + ConstString unique_typename(decl_attrs.name); + Declaration decl_declaration(decl_attrs.decl); + if (Language::LanguageIsCPlusPlus( + SymbolFileDWARF::GetLanguage(*decl_die.GetCU()))) { + std::string qualified_name = GetCPlusPlusQualifiedName(decl_die); + if (!qualified_name.empty()) + unique_typename = ConstString(qualified_name); + decl_declaration.Clear(); + } + if (UniqueDWARFASTType *unique_ast_entry_type = + dwarf->GetUniqueDWARFASTTypeMap().Find( + unique_typename, decl_die, decl_declaration, + decl_attrs.byte_size.value_or(0), + decl_attrs.is_forward_declaration)) { + unique_ast_entry_type->m_die = def_die; + if (int32_t def_byte_size = def_attrs.byte_size.value_or(0)) + unique_ast_entry_type->m_byte_size = def_byte_size; + if (def_attrs.decl.IsValid()) + unique_ast_entry_type->m_declaration = def_attrs.decl; + unique_ast_entry_type->m_is_forward_declaration = false; + } +} + TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc, const DWARFDIE &decl_die, ParsedDWARFTypeAttributes &attrs) { @@ -1654,11 +1684,13 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, 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; + if (byte_size) + unique_ast_entry_type->m_byte_size = byte_size; + if (unique_decl.IsValid()) + 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 + // it's used in ParseCXXMethod 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(); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h index 7b5ddbaa2a6b52..5eb6da6bbe2c32 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -109,6 +109,10 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { std::string GetDIEClassTemplateParams( const lldb_private::plugin::dwarf::DWARFDIE &die) override; + void MappingDeclDIEToDefDIE( + const lldb_private::plugin::dwarf::DWARFDIE &decl_die, + const lldb_private::plugin::dwarf::DWARFDIE &def_die); + protected: /// Protected typedefs and members. /// @{ diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index e1c0ddd8e93857..9799eb81d6513d 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -1653,22 +1653,24 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) { } } if (!def_die) { - // No definition found. Proceed with the declaration die. We can use it to - // create a forward-declared type. + // If we don't have definition DIE, CompleteTypeFromDWARF will forcefully + // complete this type. def_die = decl_die; } - Type *type = ResolveType(def_die); - if (!type) + DWARFASTParser *dwarf_ast = GetDWARFParser(*def_die.GetCU()); + if (!dwarf_ast) return false; - - 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()); + Type *type = GetDIEToType().lookup(decl_die.GetDIE()); + if (decl_die != def_die) { + GetDIEToType()[def_die.GetDIE()] = type; + // Need to update Type ID to refer to the definition DIE. because + // it's used in ParseCXXMethod to determine if we need to copy cxx + // method types from a declaration DIE to this definition DIE. + type->SetID(def_die.GetID()); + if (DWARFASTParserClang *ast_parser = + static_cast(dwarf_ast)) + ast_parser->MappingDeclDIEToDefDIE(decl_die, def_die); } Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion); @@ -1678,9 +1680,7 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) { 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; + return dwarf_ast->CompleteTypeFromDWARF(def_die, type, compiler_type); } Type *SymbolFileDWARF::ResolveType(const DWARFDIE &die, From 9c320bd6269eee9f742c0ef1b61fd9839e06d650 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Fri, 12 Jul 2024 11:42:15 -0700 Subject: [PATCH 3/5] Address comments --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 27 +++++++------------ .../SymbolFile/DWARF/DWARFASTParserClang.h | 5 ++-- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 10 +++---- .../SymbolFile/DWARF/UniqueDWARFASTType.h | 16 +++++++++++ ...context.cpp => type-definition-search.cpp} | 5 ++++ 5 files changed, 36 insertions(+), 27 deletions(-) rename lldb/test/Shell/SymbolFile/DWARF/x86/{simple-template-names-context.cpp => type-definition-search.cpp} (81%) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index ee170feaf6b4db..201e43fefc69a4 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -824,7 +824,7 @@ DWARFASTParserClang::GetDIEClassTemplateParams(const DWARFDIE &die) { return {}; } -void DWARFASTParserClang::MappingDeclDIEToDefDIE( +void DWARFASTParserClang::MapDeclDIEToDefDIE( const lldb_private::plugin::dwarf::DWARFDIE &decl_die, const lldb_private::plugin::dwarf::DWARFDIE &def_die) { LinkDeclContextToDIE(GetCachedClangDeclContextForDIE(decl_die), def_die); @@ -845,12 +845,14 @@ void DWARFASTParserClang::MappingDeclDIEToDefDIE( unique_typename, decl_die, decl_declaration, decl_attrs.byte_size.value_or(0), decl_attrs.is_forward_declaration)) { - unique_ast_entry_type->m_die = def_die; - if (int32_t def_byte_size = def_attrs.byte_size.value_or(0)) - unique_ast_entry_type->m_byte_size = def_byte_size; - if (def_attrs.decl.IsValid()) - unique_ast_entry_type->m_declaration = def_attrs.decl; - unique_ast_entry_type->m_is_forward_declaration = false; + unique_ast_entry_type->UpdateToDefDIE(def_die, def_attrs.decl, + def_attrs.byte_size.value_or(0)); + } else if (Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups)) { + const dw_tag_t tag = decl_die.Tag(); + LLDB_LOG(log, + "Failed to find {0:x16} {1} ({2}) type \"{3}\" in " + "UniqueDWARFASTTypeMap", + decl_die.GetID(), DW_TAG_value_to_name(tag), tag, unique_typename); } } @@ -1683,16 +1685,7 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, // 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; - if (byte_size) - unique_ast_entry_type->m_byte_size = byte_size; - if (unique_decl.IsValid()) - 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 ParseCXXMethod to determine if we need to copy cxx - // method types from a declaration DIE to this definition DIE. - type_sp->SetID(die.GetID()); + unique_ast_entry_type->UpdateToDefDIE(die, unique_decl, byte_size); clang_type = type_sp->GetForwardCompilerType(); CompilerType compiler_type_no_qualifiers = diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h index 5eb6da6bbe2c32..a1f0c86af880f5 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -109,9 +109,8 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { std::string GetDIEClassTemplateParams( const lldb_private::plugin::dwarf::DWARFDIE &die) override; - void MappingDeclDIEToDefDIE( - const lldb_private::plugin::dwarf::DWARFDIE &decl_die, - const lldb_private::plugin::dwarf::DWARFDIE &def_die); + void MapDeclDIEToDefDIE(const lldb_private::plugin::dwarf::DWARFDIE &decl_die, + const lldb_private::plugin::dwarf::DWARFDIE &def_die); protected: /// Protected typedefs and members. diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 9799eb81d6513d..838e7074ce1ee1 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -1664,13 +1664,9 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) { Type *type = GetDIEToType().lookup(decl_die.GetDIE()); if (decl_die != def_die) { GetDIEToType()[def_die.GetDIE()] = type; - // Need to update Type ID to refer to the definition DIE. because - // it's used in ParseCXXMethod to determine if we need to copy cxx - // method types from a declaration DIE to this definition DIE. - type->SetID(def_die.GetID()); - if (DWARFASTParserClang *ast_parser = - static_cast(dwarf_ast)) - ast_parser->MappingDeclDIEToDefDIE(decl_die, def_die); + DWARFASTParserClang *ast_parser = + static_cast(dwarf_ast); + ast_parser->MapDeclDIEToDefDIE(decl_die, def_die); } Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.h b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.h index 29e5c02dcbe176..9215484fa2ea22 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.h @@ -15,6 +15,7 @@ #include "DWARFDIE.h" #include "lldb/Core/Declaration.h" +#include "lldb/Symbol/Type.h" namespace lldb_private::plugin { namespace dwarf { @@ -30,6 +31,21 @@ class UniqueDWARFASTType { ~UniqueDWARFASTType() = default; + // This UniqueDWARFASTType might be created from declaration, update its info + // to definition DIE. + void UpdateToDefDIE(const DWARFDIE &def_die, Declaration &declaration, + int32_t byte_size) { + // Need to update Type ID to refer to the definition DIE, because + // it's used in DWARFASTParserClang::ParseCXXMethod to determine if we need + // to copy cxx method types from a declaration DIE to this definition DIE. + m_type_sp->SetID(def_die.GetID()); + if (declaration.IsValid()) + m_declaration = declaration; + if (byte_size) + m_byte_size = byte_size; + m_is_forward_declaration = false; + } + lldb::TypeSP m_type_sp; DWARFDIE m_die; Declaration m_declaration; diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/type-definition-search.cpp similarity index 81% rename from lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp rename to lldb/test/Shell/SymbolFile/DWARF/x86/type-definition-search.cpp index 80f22e50f6a36e..aeb7d7494510e8 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/type-definition-search.cpp @@ -4,6 +4,11 @@ // 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: 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 + // 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 From 9f723f0e59c337e6069916cb9e36c016b1e16d55 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Mon, 15 Jul 2024 10:41:14 -0700 Subject: [PATCH 4/5] address comment and format --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 44 ++++++++----------- .../SymbolFile/DWARF/DWARFASTParserClang.h | 6 ++- .../DWARF/x86/type-definition-search.cpp | 6 +-- 3 files changed, 24 insertions(+), 32 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 201e43fefc69a4..23f21ec6eafe40 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -833,13 +833,9 @@ void DWARFASTParserClang::MapDeclDIEToDefDIE( ParsedDWARFTypeAttributes def_attrs(def_die); ConstString unique_typename(decl_attrs.name); Declaration decl_declaration(decl_attrs.decl); - if (Language::LanguageIsCPlusPlus( - SymbolFileDWARF::GetLanguage(*decl_die.GetCU()))) { - std::string qualified_name = GetCPlusPlusQualifiedName(decl_die); - if (!qualified_name.empty()) - unique_typename = ConstString(qualified_name); - decl_declaration.Clear(); - } + GetUniqueTypeNameAndDeclaration( + decl_die, SymbolFileDWARF::GetLanguage(*decl_die.GetCU()), + unique_typename, decl_declaration); if (UniqueDWARFASTType *unique_ast_entry_type = dwarf->GetUniqueDWARFASTTypeMap().Find( unique_typename, decl_die, decl_declaration, @@ -1578,13 +1574,17 @@ TypeSP DWARFASTParserClang::UpdateSymbolContextScopeForType( return type_sp; } -std::string -DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) { - if (!die.IsValid()) - return ""; - const char *name = die.GetName(); - if (!name) - return ""; +void DWARFASTParserClang::GetUniqueTypeNameAndDeclaration( + const lldb_private::plugin::dwarf::DWARFDIE &die, + lldb::LanguageType language, lldb_private::ConstString &unique_typename, + lldb_private::Declaration &decl_declaration) { + // 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. + if (!die.IsValid() || !Language::LanguageIsCPlusPlus(language) || + unique_typename.IsEmpty()) + return; + decl_declaration.Clear(); std::string qualified_name; DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE(); // TODO: change this to get the correct decl context parent.... @@ -1627,10 +1627,10 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) { if (qualified_name.empty()) qualified_name.append("::"); - qualified_name.append(name); + qualified_name.append(unique_typename.GetCString()); qualified_name.append(GetDIEClassTemplateParams(die)); - return qualified_name; + unique_typename = ConstString(qualified_name); } TypeSP @@ -1662,16 +1662,8 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, } 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(die); - if (!qualified_name.empty()) - unique_typename = ConstString(qualified_name); - unique_decl.Clear(); - } - + GetUniqueTypeNameAndDeclaration(die, cu_language, unique_typename, + unique_decl); if (UniqueDWARFASTType *unique_ast_entry_type = dwarf->GetUniqueDWARFASTTypeMap().Find( unique_typename, die, unique_decl, byte_size, diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h index a1f0c86af880f5..4b0ae026bce7e9 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -171,8 +171,10 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { lldb_private::TypeSystemClang::TemplateParameterInfos &template_param_infos); - std::string - GetCPlusPlusQualifiedName(const lldb_private::plugin::dwarf::DWARFDIE &die); + void GetUniqueTypeNameAndDeclaration( + const lldb_private::plugin::dwarf::DWARFDIE &die, + lldb::LanguageType language, lldb_private::ConstString &unique_typename, + lldb_private::Declaration &decl_declaration); bool ParseChildMembers( const lldb_private::plugin::dwarf::DWARFDIE &die, diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/type-definition-search.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/type-definition-search.cpp index aeb7d7494510e8..bce6ed36b0968e 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/type-definition-search.cpp +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/type-definition-search.cpp @@ -24,13 +24,11 @@ // CHECK-NEXT: (Outer<'B'>::Inner *) b = 0x{{[0-9A-Fa-f]*}} {} // CHECK-NEXT: } -template -struct Outer { +template struct Outer { struct Inner {}; }; -template -struct ReferencesBoth { +template struct ReferencesBoth { Outer<'A'>::Inner *a; Outer<'B'>::Inner *b; }; From c394e70312178eb994c7d3a9828d1b5653b6403e Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Tue, 16 Jul 2024 10:49:00 -0700 Subject: [PATCH 5/5] Address comments --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 5 +---- .../Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 13 +++++++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 23f21ec6eafe40..85c59a605c675c 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -23,7 +23,6 @@ #include "Plugins/ExpressionParser/Clang/ClangUtil.h" #include "Plugins/Language/ObjC/ObjCLanguage.h" #include "lldb/Core/Module.h" -#include "lldb/Core/Progress.h" #include "lldb/Core/Value.h" #include "lldb/Host/Host.h" #include "lldb/Symbol/CompileUnit.h" @@ -1682,11 +1681,9 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, CompilerType compiler_type_no_qualifiers = ClangUtil::RemoveFastQualifiers(clang_type); - auto result = dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace( + dwarf->GetForwardDeclCompilerTypeToDIE().insert_or_assign( compiler_type_no_qualifiers.GetOpaqueQualType(), *die.GetDIERef()); - if (!result.second) - result.first->second = *die.GetDIERef(); } return type_sp; } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 838e7074ce1ee1..7cd3a33c7de575 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -3072,11 +3072,16 @@ TypeSP SymbolFileDWARF::FindCompleteObjCDefinitionTypeForDIE( DWARFDIE SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE &die) { - if (!die || !die.GetName()) + const char *name = die.GetName(); + if (!name) return {}; if (!die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0)) return die; + Progress progress(llvm::formatv( + "Searching definition DIE in {0}: '{1}'", + GetObjectFile()->GetFileSpec().GetFilename().GetString(), name)); + const dw_tag_t tag = die.Tag(); Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups); @@ -3085,7 +3090,7 @@ SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE &die) { log, "SymbolFileDWARF::FindDefinitionDIE(tag={0} " "({1}), name='{2}')", - DW_TAG_value_to_name(tag), tag, die.GetName()); + DW_TAG_value_to_name(tag), tag, name); } // Get the type system that we are looking to find a type for. We will @@ -3159,7 +3164,7 @@ SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE &die) { log, "SymbolFileDWARF::FindDefinitionDIE(tag={0} ({1}), " "name='{2}') ignoring die={3:x16} ({4})", - DW_TAG_value_to_name(tag), tag, die.GetName(), type_die.GetOffset(), + DW_TAG_value_to_name(tag), tag, name, type_die.GetOffset(), type_die.GetName()); } return true; @@ -3171,7 +3176,7 @@ SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE &die) { log, "SymbolFileDWARF::FindDefinitionTypeDIE(tag={0} ({1}), name='{2}') " "trying die={3:x16} ({4})", - DW_TAG_value_to_name(tag), tag, die.GetName(), type_die.GetOffset(), + DW_TAG_value_to_name(tag), tag, name, type_die.GetOffset(), type_dwarf_decl_ctx.GetQualifiedName()); }