[lldb][DWARFASTParserClang] Eagerly search definitions for Objective-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.

Author: Michael137

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<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)) {
@@ -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
@@ -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();
@@ -1810,19 +1801,28 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
   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.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) {
@@ -1906,14 +1906,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 +4076,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;
+}

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.

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 <Foundation/Foundation.h>
+
+@implementation NSObject (Foo)
+- (NSError *)func {
+    NSLog(@"Hello, World!");
+    return 0;
+}
+@end
+
+NSObject * func() {
+  return 0;
+}