Skip to content

Commit

Permalink
[lldb][DWARFASTParserClang] Eagerly search definitions for Objective-…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
Michael137 committed Dec 16, 2024
1 parent 5a5002e commit 7b2f5c5
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 46 deletions.
113 changes: 73 additions & 40 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -302,6 +303,22 @@ static void PrepareContextToReceiveMembers(TypeSystemClang &ast,
}
}

static std::optional<TypeSP> 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)) {
Expand Down Expand Up @@ -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<void *>(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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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<void *>(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;
}
10 changes: 6 additions & 4 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down
22 changes: 20 additions & 2 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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 <Foundation/Foundation.h>

@implementation NSObject (Foo)
- (NSError *)func {
NSLog(@"Hello, World!");
return 0;
}
@end

NSObject * func() {
return 0;
}

0 comments on commit 7b2f5c5

Please sign in to comment.