From 7b2f5c50a495002bc2c7b5d44e85d32563a8b92e Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 16 Dec 2024 11:46:04 +0000 Subject: [PATCH] [lldb][DWARFASTParserClang] Eagerly search definitions for Objective-C classes This patch essentially reverts the definition DIE delay changes (in https://github.com/llvm/llvm-project/pull/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. --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 113 +++++++++++------- .../SymbolFile/DWARF/DWARFASTParserClang.h | 10 +- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 22 +++- .../DWARF/objc-gmodules-class-extension.test | 33 +++++ 4 files changed, 132 insertions(+), 46 deletions(-) create mode 100644 lldb/test/Shell/SymbolFile/DWARF/objc-gmodules-class-extension.test diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 586e385edb80eff..60bb2588046fddc 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -40,6 +40,7 @@ #include "lldb/Utility/LLDBAssert.h" #include "lldb/Utility/Log.h" #include "lldb/Utility/StreamString.h" +#include "lldb/lldb-enumerations.h" #include "clang/AST/CXXInheritance.h" #include "clang/AST/Decl.h" @@ -302,6 +303,22 @@ static void PrepareContextToReceiveMembers(TypeSystemClang &ast, } } +static std::optional TryEmplaceDIEToType(DWARFDIE const &def_die, + SymbolFileDWARF &dwarf) { + auto [it, inserted] = + dwarf.GetDIEToType().try_emplace(def_die.GetDIE(), DIE_IS_BEING_PARSED); + + // First time we're parsing this DIE, nothing to return. + if (inserted) + return std::nullopt; + + // DIE is currently being parsed. + if (it->getSecond() == nullptr || it->getSecond() == DIE_IS_BEING_PARSED) + return nullptr; + + return it->getSecond()->shared_from_this(); +} + void DWARFASTParserClang::RegisterDIE(DWARFDebugInfoEntry *die, CompilerType type) { if (clang::TagDecl *td = ClangUtil::GetAsTagDecl(type)) { @@ -981,42 +998,18 @@ TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc, ParsedDWARFTypeAttributes &attrs) { Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups); SymbolFileDWARF *dwarf = decl_die.GetDWARF(); - const dw_tag_t tag = decl_die.Tag(); DWARFDIE def_die; if (attrs.is_forward_declaration) { if (TypeSP type_sp = ParseTypeFromClangModule(sc, decl_die, log)) 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 DIE is {5}", - static_cast(this), decl_die.GetID(), DW_TAG_value_to_name(tag), - tag, attrs.name.GetCString(), - def_die ? llvm::utohexstr(def_die.GetID()) : "not found"); - } + def_die = FindDefinitionDIE(decl_die, *dwarf); } 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(); - } + if (auto maybe_type = TryEmplaceDIEToType(def_die, *dwarf)) + return *maybe_type; + attrs = ParsedDWARFTypeAttributes(def_die); } else { // No definition found. Proceed with the declaration die. We can use it to @@ -1791,10 +1784,8 @@ static void adjustArgPassing(TypeSystemClang &ast, } } -TypeSP -DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, - const DWARFDIE &die, - ParsedDWARFTypeAttributes &attrs) { +TypeSP DWARFASTParserClang::ParseStructureLikeDIE( + const SymbolContext &sc, DWARFDIE die, ParsedDWARFTypeAttributes &attrs) { CompilerType clang_type; const dw_tag_t tag = die.Tag(); SymbolFileDWARF *dwarf = die.GetDWARF(); @@ -1825,6 +1816,29 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, attrs.is_forward_declaration = true; } + if (attrs.is_forward_declaration) { + // See if the type comes from a Clang module and if so, track down + // that type. + if (TypeSP type_sp = ParseTypeFromClangModule(sc, die, log)) + return type_sp; + + // Objetive-C forward declared types are represented in the same way + // that they are in C++. Since we don't want to create a CXXRecordDecl + // for the Objective-C type here, we need to fetch the definition DIE, + // which will have a DW_AT_APPLE_runtime_class attribute indicating + // we're dealing with Objective-C. + if (cu_language == eLanguageTypeObjC_plus_plus || + cu_language == eLanguageTypeObjC) { + if (DWARFDIE def_die = FindDefinitionDIE(die, *dwarf)) { + if (auto maybe_type = TryEmplaceDIEToType(def_die, *dwarf)) + return *maybe_type; + + attrs = ParsedDWARFTypeAttributes(def_die); + die = def_die; + } + } + } + if (attrs.name) { GetUniqueTypeNameAndDeclaration(die, cu_language, unique_typename, unique_decl); @@ -1906,14 +1920,6 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, } } - if (attrs.is_forward_declaration) { - // See if the type comes from a Clang module and if so, track down - // that type. - TypeSP type_sp = ParseTypeFromClangModule(sc, die, log); - if (type_sp) - return type_sp; - } - assert(tag_decl_kind != -1); UNUSED_IF_ASSERT_DISABLED(tag_decl_kind); @@ -4084,3 +4090,30 @@ void DWARFASTParserClang::ParseRustVariantPart( layout_info.field_offsets.insert({inner_field, 0}); } + +DWARFDIE DWARFASTParserClang::FindDefinitionDIE(DWARFDIE const &decl_die, + SymbolFileDWARF &dwarf) { + DWARFDIE 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 (auto *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups)) { + char const *name = decl_die.GetName(); + dwarf.GetObjectFile()->GetModule()->LogMessage( + log, + "SymbolFileDWARF({0:p}) - {1:x16}}: {2} ({3}) type \"{4}\" is a " + "forward declaration, complete DIE is {5}", + static_cast(this), decl_die.GetID(), + DW_TAG_value_to_name(decl_die.Tag()), decl_die.Tag(), name ? name : "", + def_die ? llvm::utohexstr(def_die.GetID()) : "not found"); + } + + return def_die; +} diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h index 9b740e06afdd70b..beadab4cd6b8338 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -219,10 +219,9 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { const lldb_private::plugin::dwarf::DWARFDIE &parent_die); /// Parse a structure, class, or union type DIE. - lldb::TypeSP - ParseStructureLikeDIE(const lldb_private::SymbolContext &sc, - const lldb_private::plugin::dwarf::DWARFDIE &die, - ParsedDWARFTypeAttributes &attrs); + lldb::TypeSP ParseStructureLikeDIE(const lldb_private::SymbolContext &sc, + lldb_private::plugin::dwarf::DWARFDIE die, + ParsedDWARFTypeAttributes &attrs); clang::Decl * GetClangDeclForDIE(const lldb_private::plugin::dwarf::DWARFDIE &die); @@ -498,6 +497,9 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { bool IsSwiftInteropType(const lldb_private::plugin::dwarf::DWARFDIE &die); // END SWIFT + lldb_private::plugin::dwarf::DWARFDIE + FindDefinitionDIE(lldb_private::plugin::dwarf::DWARFDIE const &decl_die, + lldb_private::plugin::dwarf::SymbolFileDWARF &dwarf); }; /// Parsed form of all attributes that are relevant for type reconstruction. diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index b249edfcf03f820..1333ef482b91476 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -3136,7 +3136,26 @@ SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE &die) { const char *name = die.GetName(); if (!name) return {}; - if (!die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0)) + + const LanguageType language = GetLanguage(*die.GetCU()); + bool is_fwd_decl = die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0); + const auto byte_size = + die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX); + // 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. + if (byte_size < UINT64_MAX && byte_size > 0 && !die.HasChildren() && + language == eLanguageTypeObjC) + is_fwd_decl = true; + + if (!is_fwd_decl) return die; Progress progress(llvm::formatv( @@ -3157,7 +3176,6 @@ SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE &die) { // Get the type system that we are looking to find a type for. We will // use this to ensure any matches we find are in a language that this // type system supports - const LanguageType language = GetLanguage(*die.GetCU()); TypeSystemSP type_system = nullptr; if (language != eLanguageTypeUnknown) { auto type_system_or_err = GetTypeSystemForLanguage(language); diff --git a/lldb/test/Shell/SymbolFile/DWARF/objc-gmodules-class-extension.test b/lldb/test/Shell/SymbolFile/DWARF/objc-gmodules-class-extension.test new file mode 100644 index 000000000000000..c78eb30c3889ec0 --- /dev/null +++ b/lldb/test/Shell/SymbolFile/DWARF/objc-gmodules-class-extension.test @@ -0,0 +1,33 @@ +# REQUIRES: system-darwin + +# Test that we can set a breakpoint in a method of a class extension. +# This requires us to parse the method into an AST type, and the context +# too (which in DWARF is just a forward declaration). +# +# RUN: split-file %s %t +# RUN: %clangxx_host %t/lib.m -c -g -gmodules -fmodules -o %t/lib.o +# RUN: %clangxx_host %t/main.m -g -gmodules -fmodules %t/lib.o -o %t/a.out -framework Foundation +# +# RUN: %lldb %t/a.out -o "breakpoint set -f lib.m -l 6" -o exit | FileCheck %s + +# CHECK: (lldb) breakpoint set -f lib.m -l 6 +# CHECK: Breakpoint 1: where = a.out`-[NSObject(Foo) func] + +#--- main.m +int main() { + return 0; +} + +#--- lib.m +#import + +@implementation NSObject (Foo) +- (NSError *)func { + NSLog(@"Hello, World!"); + return 0; +} +@end + +NSObject * func() { + return 0; +}