[lldb][gnustep][PDB] Recognize ObjC classes and start definition only once the complete type is requested

[sgraenitz]

Summary

This patch allows the PDB parser to recognize UDT symbols as ObjCInterfaceDecls and delay the definition process until we resolve the complete compiler type. This corresponds to the approach in the DWARF parser: https://github.com/llvm/llvm-project/blob/release/16.x/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L1869-L1871

In order to cache forward declarations for both, CXXRecordDecl and ObjCInterfaceDecl, the map type is changed to store NamedDecls now (first common base). The test shows that we get the names of type and members for a simple ObjC class now.

[sgraenitz]

In order to run the test yourself locally, you also need to apply D146058, D146154 and D146221 first (right now they are still in review).

lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp

414

I am not an ObjC expert, but from those who are I know that in practice this should holds for all common scenarios. So far it works reliably.

Right now I'd like to avoid patching the PDB contents from the compiler side. Would be great to see how far we get without. If there's a better way to do the same based on current PDB info, please let me know.

lldb/test/Shell/Expr/objc-gnustep-print-pdb.m

1

This test is supposed to demonstrate what works with this patch. I will remove it once we match the features in the Linux/DWARF baseline proposed in: D146154 test objc-gnustep-print.m

lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.h

public:
   clang::NamespaceDecl *FindNamespaceDecl(const clang::DeclContext *parent,
                                           llvm::StringRef name);
 
   lldb_private::ClangASTImporter &GetClangASTImporter() {
     return m_ast_importer;
   }
 
 private:
-  typedef llvm::DenseMap<clang::CXXRecordDecl *, lldb::user_id_t>
-      CXXRecordDeclToUidMap;
+  typedef llvm::DenseMap<clang::NamedDecl *, lldb::user_id_t> NamedDeclToUidMap;
   typedef llvm::DenseMap<lldb::user_id_t, clang::Decl *> UidToDeclMap;
   typedef std::set<clang::NamespaceDecl *> NamespacesSet;
   typedef llvm::DenseMap<clang::DeclContext *, NamespacesSet>
       ParentToNamespacesMap;
   typedef llvm::DenseMap<clang::DeclContext *, lldb::user_id_t>
       DeclContextToUidMap;
   typedef llvm::pdb::ConcreteSymbolEnumerator<llvm::pdb::PDBSymbolData>
       PDBDataSymbolEnumerator;
private:
   clang::CXXMethodDecl *
   AddRecordMethod(lldb_private::SymbolFile &symbol_file,
                   lldb_private::CompilerType &record_type,
                   const llvm::pdb::PDBSymbolFunc &method) const;
 
   lldb_private::TypeSystemClang &m_ast;
   lldb_private::ClangASTImporter m_ast_importer;
 
-  CXXRecordDeclToUidMap m_forward_decl_to_uid;
+  NamedDeclToUidMap m_forward_decl_to_uid;
   UidToDeclMap m_uid_to_decl;
   ParentToNamespacesMap m_parent_to_namespaces;
   NamespacesSet m_namespaces;
   DeclContextToUidMap m_decl_context_to_uid;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_PDB_PDBASTPARSER_H

lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp

 #include "Plugins/ExpressionParser/Clang/ClangASTMetadata.h"
 #include "Plugins/ExpressionParser/Clang/ClangUtil.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/Declaration.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/TypeMap.h"
 #include "lldb/Symbol/TypeSystem.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "llvm/DebugInfo/PDB/ConcreteSymbolEnumerator.h"
 #include "llvm/DebugInfo/PDB/IPDBLineNumber.h"
 #include "llvm/DebugInfo/PDB/IPDBSourceFile.h"
 #include "llvm/DebugInfo/PDB/PDBSymbol.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolData.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFunc.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeArray.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeBaseClass.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeBuiltin.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionArg.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypePointer.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
 
 #include "Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.h"
+#include "Plugins/Language/ObjC/ObjCLanguage.h"
+
 #include <optional>
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace llvm::pdb;
 
 static int TranslateUdtKind(PDB_UdtType pdb_kind) {
   switch (pdb_kind) {
case PDB_SymType::UDT: {
     // Ignore unnamed-tag UDTs.
     std::string name =
         std::string(MSVCUndecoratedNameParser::DropScope(udt->getName()));
     if (name.empty())
       return nullptr;
 
     auto decl_context = GetDeclContextContainingSymbol(type);
 
+    // PDB has no attribute to encode the language per symbol. We assume
+    // all ObjCInterfaceDecls to derive from specific base classes.
+    LanguageType lang = eLanguageTypeC_plus_plus;
+    auto *symbol_file = static_cast<SymbolFilePDB *>(m_ast.GetSymbolFile());
+    if (symbol_file->IsaNSObjectOrNSProxy(*udt))
+      lang = eLanguageTypeObjC;
+
     // Check if such an UDT already exists in the current context.
     // This may occur with const or volatile types. There are separate type
     // symbols in PDB for types with const or volatile modifiers, but we need
     // to create only one declaration for them all.
-    Type::ResolveState type_resolve_state;
-    CompilerType clang_type = m_ast.GetTypeForIdentifier<clang::CXXRecordDecl>(
+    CompilerType clang_type;
+    if (lang == eLanguageTypeObjC) {
+      clang_type = m_ast.GetTypeForIdentifier<clang::ObjCInterfaceDecl>(
+          ConstString(name), decl_context);
+    } else {
+      clang_type = m_ast.GetTypeForIdentifier<clang::CXXRecordDecl>(
           ConstString(name), decl_context);
+    }
+
+    Type::ResolveState type_resolve_state;
     if (!clang_type.IsValid()) {
       auto access = GetAccessibilityForUdt(*udt);
 
       auto tag_type_kind = TranslateUdtKind(udt->getUdtKind());
 
       ClangASTMetadata metadata;
       metadata.SetUserID(type.getSymIndexId());
       metadata.SetIsDynamicCXXType(false);
 
-      clang_type = m_ast.CreateRecordType(
-          decl_context, OptionalClangModuleID(), access, name, tag_type_kind,
-          lldb::eLanguageTypeC_plus_plus, &metadata);
+      clang_type =
+          m_ast.CreateRecordType(decl_context, OptionalClangModuleID(), access,
+                                 name, tag_type_kind, lang, &metadata);
       assert(clang_type.IsValid());
 
-      auto record_decl =
-          m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
-      assert(record_decl);
-      m_uid_to_decl[type.getSymIndexId()] = record_decl;
-
-      auto inheritance_attr = clang::MSInheritanceAttr::CreateImplicit(
-          m_ast.getASTContext(), GetMSInheritance(*udt));
-      record_decl->addAttr(inheritance_attr);
+      clang::NamedDecl *cached_decl;
+      if (TypeSystemClang::IsObjCObjectOrInterfaceType(clang_type)) {
+        // We start the declaration definition only once the full compiler type
+        // is requested since the underlying decls for ObjC types don't respond
+        // to isCompleteDefinition().
+        cached_decl = m_ast.GetAsObjCInterfaceDecl(clang_type);
+      } else {
+        cached_decl = m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
+        cached_decl->addAttr(clang::MSInheritanceAttr::CreateImplicit(
+            m_ast.getASTContext(), GetMSInheritance(*udt)));
 
         TypeSystemClang::StartTagDeclarationDefinition(clang_type);
+      }
+
+      m_uid_to_decl[type.getSymIndexId()] = cached_decl;
 
       auto children = udt->findAllChildren();
       if (!children || children->getChildCount() == 0) {
         // PDB does not have symbol of forwarder. We assume we get an udt w/o
         // any fields. Just complete it at this point.
         TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
 
         TypeSystemClang::SetHasExternalStorage(clang_type.GetOpaqueQualType(),
                                                false);
 
         type_resolve_state = Type::ResolveState::Full;
       } else {
         // Add the type to the forward declarations. It will help us to avoid
         // an endless recursion in CompleteTypeFromUdt function.
-        m_forward_decl_to_uid[record_decl] = type.getSymIndexId();
+        m_forward_decl_to_uid[cached_decl] = type.getSymIndexId();
 
         TypeSystemClang::SetHasExternalStorage(clang_type.GetOpaqueQualType(),
                                                true);
 
         type_resolve_state = Type::ResolveState::Forward;
       }
     } else
       type_resolve_state = Type::ResolveState::Forward;
lldb::TypeSP PDBASTParser::CreateLLDBTypeFromPDBType(const PDBSymbol &type) {
   return nullptr;
 }
 
 bool PDBASTParser::CompleteTypeFromPDB(
     lldb_private::CompilerType &compiler_type) {
   if (GetClangASTImporter().CanImport(compiler_type))
     return GetClangASTImporter().CompleteType(compiler_type);
 
+  clang::NamedDecl *lookup_decl;
+  if (TypeSystemClang::IsObjCObjectOrInterfaceType(compiler_type)) {
+    // Start definition now that we resolve the entire type info
+    clang::ObjCInterfaceDecl *decl =
+        m_ast.GetAsObjCInterfaceDecl(compiler_type);
+    assert(!decl->hasDefinition() && "Definition must not have started");
+    TypeSystemClang::StartTagDeclarationDefinition(compiler_type);
+    lookup_decl = decl;
+  } else {
+    lookup_decl = m_ast.GetAsCXXRecordDecl(compiler_type.GetOpaqueQualType());
+  }
+
   // Remove the type from the forward declarations to avoid
   // an endless recursion for types like a linked list.
-  clang::CXXRecordDecl *record_decl =
-      m_ast.GetAsCXXRecordDecl(compiler_type.GetOpaqueQualType());
-  auto uid_it = m_forward_decl_to_uid.find(record_decl);
+  auto uid_it = m_forward_decl_to_uid.find(lookup_decl);
   if (uid_it == m_forward_decl_to_uid.end())
     return true;
 
   auto symbol_file = static_cast<SymbolFilePDB *>(
       m_ast.GetSymbolFile()->GetBackingSymbolFile());
   if (!symbol_file)
     return false;
 

lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h

public:
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 
   llvm::pdb::IPDBSession &GetPDBSession();
 
   const llvm::pdb::IPDBSession &GetPDBSession() const;
 
   void DumpClangAST(lldb_private::Stream &s) override;
 
+  bool IsaNSObjectOrNSProxy(const llvm::pdb::PDBSymbolTypeUDT &udt) const;
+
 private:
   struct SecContribInfo {
     uint32_t Offset;
     uint32_t Size;
     uint32_t CompilandId;
   };
   using SecContribsMap = std::map<uint32_t, std::vector<SecContribInfo>>;
 

lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

 #include "llvm/DebugInfo/PDB/PDBSymbolCompiland.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolCompilandDetails.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolData.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFunc.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFuncDebugEnd.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFuncDebugStart.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolPublicSymbol.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolTypeBaseClass.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeBuiltin.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 #include "Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.h"
while (auto LexParent = m_session_up->getSymbolById(LexParentId)) {
       if (LexParent->getSymTag() == PDB_SymType::Compiland)
         return LexParentId;
       LexParentId = LexParent->getRawSymbol().getLexicalParentId();
     }
   }
 
   return 0;
 }
+
+bool SymbolFilePDB::IsaNSObjectOrNSProxy(
+    const PDBSymbolTypeUDT &pdb_udt) const {
+  if (pdb_udt.getName() == "NSObject")
+    return true;
+  if (pdb_udt.getName() == "NSProxy")
+    return true;
+
+  auto bases_up = pdb_udt.findAllChildren<PDBSymbolTypeBaseClass>();
+  std::unique_ptr<llvm::pdb::PDBSymbolTypeBaseClass> pdb_base_up =
+      bases_up->getNext();
+  if (!pdb_base_up)
+    return false; // No further bases classes (we assume it's C/C++)
+
+  if (bases_up->getNext())
+    return false; // ObjC has single inheritance only (this must be C/C++)
+
+  user_id_t base_uid = pdb_base_up->getRawSymbol().getTypeId();
+  std::unique_ptr<PDBSymbol> pdb_base_raw_up =
+      m_session_up->getSymbolById(base_uid);
+  if (pdb_base_raw_up->getSymTag() != PDB_SymType::UDT)
+    return false; // Error: base class is not a user-defined type
+
+  auto *pdb_base_udt = llvm::dyn_cast<PDBSymbolTypeUDT>(pdb_base_raw_up.get());
+  return IsaNSObjectOrNSProxy(*pdb_base_udt);
+}

lldb/test/Shell/Expr/objc-gnustep-print-pdb.m

+// REQUIRES: objc-gnustep && system-windows
+//
+// RUN: %build %s --compiler=clang --objc-gnustep --output=%t
+
+#import "objc/runtime.h"
+
+@protocol NSCoding
+@end
+
+#ifdef __has_attribute
+#if __has_attribute(objc_root_class)
+__attribute__((objc_root_class))
+#endif
+#endif
+@interface NSObject<NSCoding> {
+  id isa;
+  int refcount;
+}
+@end
+@implementation NSObject
+- (id)class {
+  return object_getClass(self);
+}
++ (id)new {
+  return class_createInstance(self, 0);
+}
+@end
+@interface TestObj : NSObject {
+  int _int;
+  float _float;
+  char _char;
+  void *_ptr_void;
+  NSObject *_ptr_nsobject;
+  id _id_objc;
+}
+- (int)ok;
+@end
+@implementation TestObj
+- (int)ok {
+  return self ? 0 : 1;
+}
+@end
+
+// RUN: %lldb -b -o "b objc-gnustep-print-pdb.m:67" -o "run" -o "p ptr" -o "p *ptr" -- %t | FileCheck %s
+//
+// CHECK: (lldb) b objc-gnustep-print-pdb.m:67
+// CHECK: Breakpoint {{.*}} at objc-gnustep-print-pdb.m:67
+//
+// CHECK: (lldb) run
+// CHECK: Process {{[0-9]+}} stopped
+// CHECK: frame #0: {{.*}}`main  at objc-gnustep-print-pdb.m:67
+//
+// CHECK: (lldb) p ptr
+// CHECK: (TestObj *) $0 = 0x{{[0-9]+}}
+//
+// CHECK: (lldb) p *ptr
+// CHECK: (TestObj) $1 = {
+// CHECK:   _int
+// CHECK:   _float
+// CHECK:   _char
+// CHECK:   _ptr_void
+// CHECK:   _ptr_nsobject
+// CHECK:   _id_objc
+// CHECK: }
+
+int main() {
+  TestObj *ptr = [TestObj new];
+  return [ptr ok];
+}