sgraenitz created this revision.
sgraenitz added reviewers: aleksandr.urakov, rnk, aprantl, DavidSpickett, 
omjavaid, zturner, asmith.
Herald added a project: All.
sgraenitz requested review of this revision.
Herald added a project: LLDB.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146293

Files:
  lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/test/Shell/Expr/objc-gnustep-print-pdb.m

Index: lldb/test/Shell/Expr/objc-gnustep-print-pdb.m
===================================================================
--- /dev/null
+++ lldb/test/Shell/Expr/objc-gnustep-print-pdb.m
@@ -0,0 +1,69 @@
+// 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];
+}
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===================================================================
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -169,6 +169,8 @@
 
   void DumpClangAST(lldb_private::Stream &s) override;
 
+  bool IsaNSObjectOrNSProxy(const llvm::pdb::PDBSymbolTypeUDT &udt) const;
+
 private:
   struct SecContribInfo {
     uint32_t Offset;
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -46,6 +46,7 @@
 #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"
@@ -2070,3 +2071,29 @@
 
   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);
+}
Index: lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.h
===================================================================
--- lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.h
+++ lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.h
@@ -65,8 +65,7 @@
   }
 
 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>
@@ -106,7 +105,7 @@
   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;
Index: lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -22,6 +22,7 @@
 #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"
@@ -40,6 +41,8 @@
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
 
 #include "Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.h"
+#include "Plugins/Language/ObjC/ObjCLanguage.h"
+
 #include <optional>
 
 using namespace lldb;
@@ -402,13 +405,27 @@
 
     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.
+    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;
-    CompilerType clang_type = m_ast.GetTypeForIdentifier<clang::CXXRecordDecl>(
-        ConstString(name), decl_context);
     if (!clang_type.IsValid()) {
       auto access = GetAccessibilityForUdt(*udt);
 
@@ -418,21 +435,26 @@
       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;
+      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)));
 
-      auto inheritance_attr = clang::MSInheritanceAttr::CreateImplicit(
-          m_ast.getASTContext(), GetMSInheritance(*udt));
-      record_decl->addAttr(inheritance_attr);
+        TypeSystemClang::StartTagDeclarationDefinition(clang_type);
+      }
 
-      TypeSystemClang::StartTagDeclarationDefinition(clang_type);
+      m_uid_to_decl[type.getSymIndexId()] = cached_decl;
 
       auto children = udt->findAllChildren();
       if (!children || children->getChildCount() == 0) {
@@ -447,7 +469,7 @@
       } 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);
@@ -796,11 +818,21 @@
   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;
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] D1... Stefan Gränitz via Phabricator via lldb-commits

Reply via email to