Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. #98361
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ng when parsing declaration DIEs.
|
@llvm/pr-subscribers-lldb Author: Zequan Wu (ZequanWu) ChangesThis is a reapply of #92328 and #93839. It now passes the test, 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:
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]
|
labath
reviewed
Jul 11, 2024
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
labath
reviewed
Jul 12, 2024
lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp
Outdated
Show resolved
Hide resolved
labath
approved these changes
Jul 16, 2024
Michael137
approved these changes
Jul 16, 2024
| qualified_name.append(GetDIEClassTemplateParams(die)); | ||
|
|
||
| return qualified_name; | ||
| unique_typename = ConstString(qualified_name); |
There was a problem hiding this comment.
Nit: I'd find it slightly clearer if we returned the new name like the old function did (probably don't even need the parameter to be a ConstString)
There was a problem hiding this comment.
This functions modifies both unique_typename and decl_declaration if it's c++, so there're two out values. Returning one out value while modifying another out value via reference parameter looks strange.
| if (attrs.is_forward_declaration) { | ||
| Progress progress(llvm::formatv( |
There was a problem hiding this comment.
Can we move this into FindDefinitionDIE? We previously put this here to keep track of when we call into FindDefinitionDIE during DWARF parsing
There was a problem hiding this comment.
I moved it inside FindDefinitionDIE and changed the logging message to Searching definition DIE in ... as that function just do searching not parsing.
|
This buildkite seems got stuck somehow, no logging at all: https://buildkite.com/llvm-project/github-pull-requests/builds/81790#0190bca9-bde7-4fad-8478-9dffd4f669f7. Will merge without waiting for it to finish. |
sgundapa
pushed a commit
to sgundapa/upstream_effort
that referenced
this pull request
Jul 23, 2024
…ng when parsing declaration DIEs. (llvm#98361) This is a reapply of llvm#92328 and llvm#93839. It now passes the [test](llvm@de3f1b6), which crashes with the original reverted changes.
yuxuanchen1997
pushed a commit
that referenced
this pull request
Jul 25, 2024
…ng when parsing declaration DIEs. (#98361) Summary: This is a reapply of #92328 and #93839. It now passes the [test](de3f1b6), which crashes with the original reverted changes. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251550
Michael137
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Dec 16, 2024
…C classes This patch essentially reverts the definition DIE delay changes (in llvm#98361) for Objective-C. The problem we've been seeing is as follows: 1. An Objetive-C class extension is represented in DWARF as: ``` DW_TAG_structure_type DW_AT_declaration (true) DW_AT_name ("ExtendedClass") DW_TAG_subprogram DW_AT_name ("extensionMethod") ... ``` I.e., it is a forward declaration of the extended class, but that forward declaration has children methods. 2. When we set a breakpoint in one of those methods we parse the subprogram DIE and try to create an `ObjCMethodDecl` for it (and attach it to the context). 3. When parsing the subprogram DIE, we first try to complete the DeclContext. Because `ExtendedClass` is only a forward declaration and we don't try to fetch the definition DIE eagerly anymore, LLDB has no idea that we're dealing with an Objective-C type. So it goes ahead and constructs it as a `CXXRecordDecl`. This confuses `DWARFASTParserClang::ParseObjCMethod` because it expects the context to be an `clang::ObjCObjectOrInterfaceType`. So it bails and we end up crashing because we try to attach a `FunctionDecl` to an incomplete `CXXRecordDecl` (which wasn't even forcefully completed). Since there's no good way to tell whether a forward declaration is an Objective-C type, the only solution I can really see here is to eagerly fetch the definition for Objective-C types.
Michael137
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Dec 16, 2024
…C classes This patch essentially reverts the definition DIE delay changes (in llvm#98361) for Objective-C. The problem we've been seeing is as follows: 1. An Objetive-C class extension is represented in DWARF as: ``` DW_TAG_structure_type DW_AT_declaration (true) DW_AT_name ("ExtendedClass") DW_TAG_subprogram DW_AT_name ("extensionMethod") ... ``` I.e., it is a forward declaration of the extended class, but that forward declaration has children methods. 2. When we set a breakpoint in one of those methods we parse the subprogram DIE and try to create an `ObjCMethodDecl` for it (and attach it to the context). 3. When parsing the subprogram DIE, we first try to complete the DeclContext. Because `ExtendedClass` is only a forward declaration and we don't try to fetch the definition DIE eagerly anymore, LLDB has no idea that we're dealing with an Objective-C type. So it goes ahead and constructs it as a `CXXRecordDecl`. This confuses `DWARFASTParserClang::ParseObjCMethod` because it expects the context to be an `clang::ObjCObjectOrInterfaceType`. So it bails and we end up crashing because we try to attach a `FunctionDecl` to an incomplete `CXXRecordDecl` (which wasn't even forcefully completed). Since there's no good way to tell whether a forward declaration is an Objective-C type, the only solution I can really see here is to eagerly fetch the definition for Objective-C types.
Michael137
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Dec 16, 2024
…C classes This patch essentially reverts the definition DIE delay changes (in llvm#98361) for Objective-C. The problem we've been seeing is as follows: 1. An Objetive-C class extension is represented in DWARF as: ``` DW_TAG_structure_type DW_AT_declaration (true) DW_AT_name ("ExtendedClass") DW_TAG_subprogram DW_AT_name ("extensionMethod") ... ``` I.e., it is a forward declaration of the extended class, but that forward declaration has children methods. 2. When we set a breakpoint in one of those methods we parse the subprogram DIE and try to create an `ObjCMethodDecl` for it (and attach it to the context). 3. When parsing the subprogram DIE, we first try to complete the DeclContext. Because `ExtendedClass` is only a forward declaration and we don't try to fetch the definition DIE eagerly anymore, LLDB has no idea that we're dealing with an Objective-C type. So it goes ahead and constructs it as a `CXXRecordDecl`. This confuses `DWARFASTParserClang::ParseObjCMethod` because it expects the context to be an `clang::ObjCObjectOrInterfaceType`. So it bails and we end up crashing because we try to attach a `FunctionDecl` to an incomplete `CXXRecordDecl` (which wasn't even forcefully completed). Since there's no good way to tell whether a forward declaration is an Objective-C type, the only solution I can really see here is to eagerly fetch the definition for Objective-C types.
Michael137
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Dec 16, 2024
…C classes This patch essentially reverts the definition DIE delay changes (in llvm#98361) for Objective-C. The problem we've been seeing is as follows: 1. An Objetive-C class extension is represented in DWARF as: ``` DW_TAG_structure_type DW_AT_declaration (true) DW_AT_name ("ExtendedClass") DW_TAG_subprogram DW_AT_name ("extensionMethod") ... ``` I.e., it is a forward declaration of the extended class, but that forward declaration has children methods. 2. When we set a breakpoint in one of those methods we parse the subprogram DIE and try to create an `ObjCMethodDecl` for it (and attach it to the context). 3. When parsing the subprogram DIE, we first try to complete the DeclContext. Because `ExtendedClass` is only a forward declaration and we don't try to fetch the definition DIE eagerly anymore, LLDB has no idea that we're dealing with an Objective-C type. So it goes ahead and constructs it as a `CXXRecordDecl`. This confuses `DWARFASTParserClang::ParseObjCMethod` because it expects the context to be an `clang::ObjCObjectOrInterfaceType`. So it bails and we end up crashing because we try to attach a `FunctionDecl` to an incomplete `CXXRecordDecl` (which wasn't even forcefully completed). Since there's no good way to tell whether a forward declaration is an Objective-C type, the only solution I can really see here is to eagerly fetch the definition for Objective-C types.
Michael137
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Dec 16, 2024
…C classes This patch essentially reverts the definition DIE delay changes (in llvm#98361) for Objective-C. The problem we've been seeing is as follows: 1. An Objetive-C class extension is represented in DWARF as: ``` DW_TAG_structure_type DW_AT_declaration (true) DW_AT_name ("ExtendedClass") DW_TAG_subprogram DW_AT_name ("extensionMethod") ... ``` I.e., it is a forward declaration of the extended class, but that forward declaration has children methods. 2. When we set a breakpoint in one of those methods we parse the subprogram DIE and try to create an `ObjCMethodDecl` for it (and attach it to the context). 3. When parsing the subprogram DIE, we first try to complete the DeclContext. Because `ExtendedClass` is only a forward declaration and we don't try to fetch the definition DIE eagerly anymore, LLDB has no idea that we're dealing with an Objective-C type. So it goes ahead and constructs it as a `CXXRecordDecl`. This confuses `DWARFASTParserClang::ParseObjCMethod` because it expects the context to be an `clang::ObjCObjectOrInterfaceType`. So it bails and we end up crashing because we try to attach a `FunctionDecl` to an incomplete `CXXRecordDecl` (which wasn't even forcefully completed). Since there's no good way to tell whether a forward declaration is an Objective-C type, the only solution I can really see here is to eagerly fetch the definition for Objective-C types.
Michael137
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Dec 16, 2024
…C classes This patch essentially reverts the definition DIE delay changes (in llvm#98361) for Objective-C. The problem we've been seeing is as follows: 1. An Objetive-C class extension is represented in DWARF as: ``` DW_TAG_structure_type DW_AT_declaration (true) DW_AT_name ("ExtendedClass") DW_TAG_subprogram DW_AT_name ("extensionMethod") ... ``` I.e., it is a forward declaration of the extended class, but that forward declaration has children methods. 2. When we set a breakpoint in one of those methods we parse the subprogram DIE and try to create an `ObjCMethodDecl` for it (and attach it to the context). 3. When parsing the subprogram DIE, we first try to complete the DeclContext. Because `ExtendedClass` is only a forward declaration and we don't try to fetch the definition DIE eagerly anymore, LLDB has no idea that we're dealing with an Objective-C type. So it goes ahead and constructs it as a `CXXRecordDecl`. This confuses `DWARFASTParserClang::ParseObjCMethod` because it expects the context to be an `clang::ObjCObjectOrInterfaceType`. So it bails and we end up crashing because we try to attach a `FunctionDecl` to an incomplete `CXXRecordDecl` (which wasn't even forcefully completed). Since there's no good way to tell whether a forward declaration is an Objective-C type, the only solution I can really see here is to eagerly fetch the definition for Objective-C types.
Michael137
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Dec 16, 2024
…C classes This patch essentially reverts the definition DIE delay changes (in llvm#98361) for Objective-C. The problem we've been seeing is as follows: 1. An Objetive-C class extension is represented in DWARF as: ``` DW_TAG_structure_type DW_AT_declaration (true) DW_AT_name ("ExtendedClass") DW_TAG_subprogram DW_AT_name ("extensionMethod") ... ``` I.e., it is a forward declaration of the extended class, but that forward declaration has children methods. 2. When we set a breakpoint in one of those methods we parse the subprogram DIE and try to create an `ObjCMethodDecl` for it (and attach it to the context). 3. When parsing the subprogram DIE, we first try to complete the DeclContext. Because `ExtendedClass` is only a forward declaration and we don't try to fetch the definition DIE eagerly anymore, LLDB has no idea that we're dealing with an Objective-C type. So it goes ahead and constructs it as a `CXXRecordDecl`. This confuses `DWARFASTParserClang::ParseObjCMethod` because it expects the context to be an `clang::ObjCObjectOrInterfaceType`. So it bails and we end up crashing because we try to attach a `FunctionDecl` to an incomplete `CXXRecordDecl` (which wasn't even forcefully completed). Since there's no good way to tell whether a forward declaration is an Objective-C type, the only solution I can really see here is to eagerly fetch the definition for Objective-C types.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a reapply of #92328 and #93839.
It now passes the test, which crashes with the original reverted changes.