[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Jul 10 11:01:37 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Zequan Wu (ZequanWu)
<details>
<summary>Changes</summary>
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.
---
Patch is 36.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/98361.diff
10 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+118-161)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+49-18)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h (+8-7)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h (+9)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp (+1-1)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h (+2-1)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp (+63-54)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.h (+13-23)
- (added) lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test (+36)
- (modified) lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp (+2-2)
``````````diff
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...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/98361
More information about the lldb-commits
mailing list