[Lldb-commits] [lldb] 6435fe6 - [LLDB][NativePDB] Forcefully complete a record type if it has empty debug info and is required to have complete type.

Zequan Wu via lldb-commits lldb-commits at lists.llvm.org
Wed Nov 16 11:17:14 PST 2022


Author: Zequan Wu
Date: 2022-11-16T11:17:07-08:00
New Revision: 6435fe699c2978aa8a91dc1cf5daa82fb4b28d95

URL: https://github.com/llvm/llvm-project/commit/6435fe699c2978aa8a91dc1cf5daa82fb4b28d95
DIFF: https://github.com/llvm/llvm-project/commit/6435fe699c2978aa8a91dc1cf5daa82fb4b28d95.diff

LOG: [LLDB][NativePDB] Forcefully complete a record type if it has empty debug info and is required to have complete type.

It's required in following situations:
1. As a base class.
2. As a data member.
3. As an array element type.

Reviewed By: labath

Differential Revision: https://reviews.llvm.org/D134066

Added: 
    lldb/test/Shell/SymbolFile/NativePDB/Inputs/incomplete-tag-type.cpp
    lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp

Modified: 
    lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
    lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
    lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
    lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 037e11895b106..80709d1f0b0dd 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -227,28 +227,6 @@ static void ForcefullyCompleteType(CompilerType type) {
   ts.GetMetadata(td)->SetIsForcefullyCompleted();
 }
 
-/// Complete a type from debug info, or mark it as forcefully completed if
-/// there is no definition of the type in the current Module. Call this function
-/// in contexts where the usual C++ rules require a type to be complete (base
-/// class, member, etc.).
-static void RequireCompleteType(CompilerType type) {
-  // Technically, enums can be incomplete too, but we don't handle those as they
-  // are emitted even under -flimit-debug-info.
-  if (!TypeSystemClang::IsCXXClassType(type))
-    return;
-
-  if (type.GetCompleteType())
-    return;
-
-  // No complete definition in this module.  Mark the class as complete to
-  // satisfy local ast invariants, but make a note of the fact that
-  // it is not _really_ complete so we can later search for a definition in a
-  // 
diff erent module.
-  // Since we provide layout assistance, layouts of types containing this class
-  // will be correct even if we  are not able to find the definition elsewhere.
-  ForcefullyCompleteType(type);
-}
-
 /// This function serves a similar purpose as RequireCompleteType above, but it
 /// avoids completing the type if it is not immediately necessary. It only
 /// ensures we _can_ complete the type later.
@@ -1323,7 +1301,7 @@ DWARFASTParserClang::ParseArrayType(const DWARFDIE &die,
   if (byte_stride == 0 && bit_stride == 0)
     byte_stride = element_type->GetByteSize(nullptr).value_or(0);
   CompilerType array_element_type = element_type->GetForwardCompilerType();
-  RequireCompleteType(array_element_type);
+  TypeSystemClang::RequireCompleteType(array_element_type);
 
   uint64_t array_element_bit_stride = byte_stride * 8 + bit_stride;
   CompilerType clang_type;
@@ -2212,7 +2190,8 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
         clang::TypeSourceInfo *type_source_info =
             base_class->getTypeSourceInfo();
         if (type_source_info)
-          RequireCompleteType(m_ast.GetType(type_source_info->getType()));
+          TypeSystemClang::RequireCompleteType(
+              m_ast.GetType(type_source_info->getType()));
       }
 
       m_ast.TransferBaseClasses(clang_type.GetOpaqueQualType(),
@@ -3018,7 +2997,7 @@ void DWARFASTParserClang::ParseSingleMember(
     }
   }
 
-  RequireCompleteType(member_clang_type);
+  TypeSystemClang::RequireCompleteType(member_clang_type);
 
   clang::FieldDecl *field_decl = TypeSystemClang::AddFieldToRecordType(
       class_clang_type, attrs.name, member_clang_type, attrs.accessibility,

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
index 2a0954a20aaa7..b4125ca26d04f 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -92,9 +92,10 @@ void UdtRecordCompleter::AddMethod(llvm::StringRef name, TypeIndex type_idx,
       m_ast_builder.GetOrCreateType(PdbTypeSymId(type_idx));
   if (method_qt.isNull())
     return;
-  m_ast_builder.CompleteType(method_qt);
   CompilerType method_ct = m_ast_builder.ToCompilerType(method_qt);
-  lldb::opaque_compiler_type_t derived_opaque_ty = m_derived_ct.GetOpaqueQualType();
+  TypeSystemClang::RequireCompleteType(method_ct);
+  lldb::opaque_compiler_type_t derived_opaque_ty =
+      m_derived_ct.GetOpaqueQualType();
   auto iter = m_cxx_record_map.find(derived_opaque_ty);
   if (iter != m_cxx_record_map.end()) {
     if (iter->getSecond().contains({name, method_ct})) {
@@ -253,7 +254,7 @@ Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
   clang::QualType member_qt = m_ast_builder.GetOrCreateType(PdbTypeSymId(ti));
   if (member_qt.isNull())
     return Error::success();
-  m_ast_builder.CompleteType(member_qt);
+  TypeSystemClang::RequireCompleteType(m_ast_builder.ToCompilerType(member_qt));
   lldb::AccessType access = TranslateMemberAccess(data_member.getAccess());
   size_t field_size =
       bitfield_width ? bitfield_width : GetSizeOfType(ti, m_index.tpi()) * 8;
@@ -310,6 +311,18 @@ void UdtRecordCompleter::complete() {
     bases.push_back(std::move(ib.second));
 
   TypeSystemClang &clang = m_ast_builder.clang();
+  // Make sure all base classes refer to complete types and not forward
+  // declarations. If we don't do this, clang will crash with an
+  // assertion in the call to clang_type.TransferBaseClasses()
+  for (const auto &base_class : bases) {
+    clang::TypeSourceInfo *type_source_info =
+        base_class->getTypeSourceInfo();
+    if (type_source_info) {
+      TypeSystemClang::RequireCompleteType(
+          clang.GetType(type_source_info->getType()));
+    }
+  }
+
   clang.TransferBaseClasses(m_derived_ct.GetOpaqueQualType(), std::move(bases));
 
   clang.AddMethodOverridesForCXXRecordType(m_derived_ct.GetOpaqueQualType());

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index a868abb46a73a..2993b98f967e5 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9812,6 +9812,29 @@ TypeSystemClang::DeclContextGetTypeSystemClang(const CompilerDeclContext &dc) {
   return nullptr;
 }
 
+void TypeSystemClang::RequireCompleteType(CompilerType type) {
+  // Technically, enums can be incomplete too, but we don't handle those as they
+  // are emitted even under -flimit-debug-info.
+  if (!TypeSystemClang::IsCXXClassType(type))
+    return;
+
+  if (type.GetCompleteType())
+    return;
+
+  // No complete definition in this module.  Mark the class as complete to
+  // satisfy local ast invariants, but make a note of the fact that
+  // it is not _really_ complete so we can later search for a definition in a
+  // 
diff erent module.
+  // Since we provide layout assistance, layouts of types containing this class
+  // will be correct even if we  are not able to find the definition elsewhere.
+  bool started = TypeSystemClang::StartTagDeclarationDefinition(type);
+  lldbassert(started && "Unable to start a class type definition.");
+  TypeSystemClang::CompleteTagDeclarationDefinition(type);
+  const clang::TagDecl *td = ClangUtil::GetAsTagDecl(type);
+  auto &ts = llvm::cast<TypeSystemClang>(*type.GetTypeSystem());
+  ts.GetMetadata(td)->SetIsForcefullyCompleted();
+}
+
 namespace {
 /// A specialized scratch AST used within ScratchTypeSystemClang.
 /// These are the ASTs backing the 
diff erent IsolatedASTKinds. They behave

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 1c0e120aadbfe..1a5ec3075a8ac 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1045,6 +1045,12 @@ class TypeSystemClang : public TypeSystem {
     return m_source_manager_up.get();
   }
 
+  /// Complete a type from debug info, or mark it as forcefully completed if
+  /// there is no definition of the type in the current Module. Call this
+  /// function in contexts where the usual C++ rules require a type to be
+  /// complete (base class, member, etc.).
+  static void RequireCompleteType(CompilerType type);
+
 private:
   /// Returns the PrintingPolicy used when generating the internal type names.
   /// These type names are mostly used for the formatter selection.

diff  --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/incomplete-tag-type.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/incomplete-tag-type.cpp
new file mode 100644
index 0000000000000..c930338905445
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/incomplete-tag-type.cpp
@@ -0,0 +1,15 @@
+struct A {
+  int x;
+  A();
+};
+A::A() : x(47) {}
+
+struct C {
+  C();
+};
+C::C() = default;
+
+struct E {
+  E();
+};
+E::E() = default;

diff  --git a/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp b/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp
new file mode 100644
index 0000000000000..b6e7ab3a7b1a9
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp
@@ -0,0 +1,48 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// RUN: %clang_cl --target=x86_64-windows-msvc -c /Fo%t1.obj -- %p/Inputs/incomplete-tag-type.cpp
+// RUN: %clang_cl --target=x86_64-windows-msvc /O1 /Z7 -c /Fo%t2.obj -- %s
+// RUN: lld-link /debug:full /nodefaultlib /entry:main %t1.obj %t2.obj /out:%t.exe /pdb:%t.pdb
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -o \
+// RUN:   "settings set interpreter.stop-command-source-on-error false" \
+// RUN:   -o "p b" -o "p d" -o "p static_e_ref" -o "exit" 2>&1 | FileCheck %s
+
+// CHECK: (lldb) p b
+// CHECK: (B) $0 = {}
+// CHECK: (lldb) p d
+// CHECK: (D) $1 = {}
+// CHECK: (lldb) p static_e_ref
+// CHECK: error: expression failed to parse:
+// CHECK: error: {{.*}}: incomplete type 'E' where a complete type is required
+// CHECK: static_e_ref
+// CHECK: ^
+
+// Complete base class.
+struct A { int x; A(); };
+struct B : A {};
+B b;
+
+// Complete data member.
+struct C {
+  C();
+};
+
+struct D {
+  C c;
+};
+D d;
+
+// Incomplete static data member should return error.
+struct E {
+  E();
+};
+
+struct F {
+  static E static_e;
+};
+
+E F::static_e = E();
+E& static_e_ref = F::static_e;
+
+int main(){}


        


More information about the lldb-commits mailing list