[LLDB][NativePDB] Complete array member types in AST builder #156370

Nerixyz · 2025-09-01T19:44:39Z

In native PDB, types of array members were not completed when completing a struct. This lead to an assertion failure later when trying to print a variable of the struct (Shell/SymbolFile/PDB/udt-layout.test, see #114906).
The following shows the issue:

struct G {
  int foo = 1;
};
struct H {
  G g[2];
};
H h;

Running target variable h would result in a request to complete H. This will run the UdtRecordCompleter over the struct fields. When it encounters a data member, it tries to complete its type. This would previously only complete records. However, types of constant array members (in our case G) need to be completed as well, as they are required to compute the layout of the record.

With this PR, PdbASTBuilder::CreateArrayType will also complete the types of constant array members.

To hit the assertion, the layout needs to be requested (e.g. by getting a child member). That's why I used both target variable and expression.

Unfortunately, this doesn't allow Shell/SymbolFile/PDB/udt-layout.test to run with the native plugin yet, because it checks for the order in which base classes are printed. The DIA plugin puts virtual bases first, while the native plugin puts them last (where they are in memory).

llvmbot · 2025-09-01T19:45:10Z

@llvm/pr-subscribers-lldb

Author: nerix (Nerixyz)

Changes

In native PDB, types of constant array members were not completed when completing a struct. This lead to an assertion failure later when trying to print a variable of the struct (Shell/SymbolFile/PDB/udt-layout.test, see #114906).
The following shows the issue:

struct G {
  int foo = 1;
};
struct H {
  G g[2];
};
H h;

Running target variable h would result in a request to complete H. This will run the UdtRecordCompleter over the struct fields. When it encounters a data member, it tries to complete its type. This would previously only complete records. However, types of constant array members (in our case G) need to be completed as well, as they are required to compute the layout of the record.

With this PR, TypeSystemClang::RequireCompleteType will also complete the types of constant array members.

To hit the assertion, the layout needs to be requested (e.g. by getting a child member). That's why I used both target variable and expression.

Unfortunately, this doesn't allow Shell/SymbolFile/PDB/udt-layout.test to run with the native plugin yet, because it checks for the order in which base classes are printed. The DIA plugin puts virtual bases first, while the native plugin puts them last (where they are in memory).

Full diff: https://github.com/llvm/llvm-project/pull/156370.diff

4 Files Affected:

(modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+12-3)
(modified) lldb/test/Shell/SymbolFile/NativePDB/Inputs/incomplete-tag-type.cpp (+5)
(removed) lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp (-45)
(added) lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.test (+109)

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 39aacdb58e694..038677f68b991 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9577,12 +9577,21 @@ TypeSystemClang::DeclContextGetTypeSystemClang(const CompilerDeclContext &dc) {
 }
 
 void TypeSystemClang::RequireCompleteType(CompilerType type) {
+  if (!type)
+    return;
+
+  clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
+  if (qual_type.isNull())
+    return;
+
   // 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;
+  bool is_constant_array = qual_type->isConstantArrayType();
+  bool is_cxx_record = qual_type->getAsCXXRecordDecl() != nullptr;
+  if (is_constant_array || is_cxx_record)
+    type.GetCompleteType();
 
-  if (type.GetCompleteType())
+  if (!is_cxx_record)
     return;
 
   // No complete definition in this module.  Mark the class as complete to
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/incomplete-tag-type.cpp b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/incomplete-tag-type.cpp
index c930338905445..d08f49d1014ba 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/incomplete-tag-type.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/incomplete-tag-type.cpp
@@ -13,3 +13,8 @@ struct E {
   E();
 };
 E::E() = default;
+
+struct I {
+  I();
+};
+I::I() = default;
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp b/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp
deleted file mode 100644
index 7bc7e618667f7..0000000000000
--- a/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.cpp
+++ /dev/null
@@ -1,45 +0,0 @@
-// 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: %lldb -f %t.exe -o \
-// RUN:   "settings set interpreter.stop-command-source-on-error false" \
-// RUN:   -o "expression b" -o "expression d" -o "expression static_e_ref" -o "exit" 2>&1 | FileCheck %s
-
-// CHECK: (lldb) expression b
-// CHECK: (B) $0 = {}
-// CHECK: (lldb) expression d
-// CHECK: (D) $1 = {}
-// CHECK: (lldb) expression static_e_ref
-// CHECK: error:{{.*}}incomplete type 'E' where a complete type is required
-
-// 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(){}
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.test b/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.test
new file mode 100644
index 0000000000000..f30866ccdd6f0
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/NativePDB/incomplete-tag-type.test
@@ -0,0 +1,109 @@
+# REQUIRES: lld, x86
+
+# RUN: split-file %s %t
+
+# 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 -- %t/main.cpp
+# RUN: lld-link /debug:full /nodefaultlib /entry:main %t1.obj %t2.obj /out:%t.exe /pdb:%t.pdb
+
+# RUN: %lldb -f %t.exe -s %t/target-var.input 2>&1 | FileCheck %s --check-prefix=TARGET-VAR
+# RUN: %lldb -f %t.exe -s %t/expr.input 2>&1 | FileCheck %s --check-prefix=EXPR
+
+#--- main.cpp
+
+// 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;
+
+struct G {
+  int foo = 1;
+};
+struct H {
+  G g[2];
+};
+H h;
+
+struct I {
+  I();
+};
+struct J {
+  I i[2];
+};
+J j;
+
+
+int main(){}
+
+#--- target-var.input
+
+target variable b
+target variable d
+target variable h
+target variable j
+target variable static_e_ref
+exit
+
+#--- expr.input
+
+settings set interpreter.stop-command-source-on-error false
+expression b
+expression d
+expression h
+expression j
+expression static_e_ref
+exit
+
+# TARGET-VAR:      (lldb) target variable b
+# TARGET-VAR-NEXT: (B) b = (A = <incomplete type>)
+# TARGET-VAR-NEXT: (lldb) target variable d
+# TARGET-VAR-NEXT: (D) d = {}
+# TARGET-VAR-NEXT: (lldb) target variable h
+# TARGET-VAR-NEXT: (H) h = {
+# TARGET-VAR-NEXT:   g = {
+# TARGET-VAR-NEXT:     [0] = (foo = 1)
+# TARGET-VAR-NEXT:     [1] = (foo = 1)
+# TARGET-VAR-NEXT:   }
+# TARGET-VAR-NEXT: }
+# TARGET-VAR-NEXT: (lldb) target variable j
+# TARGET-VAR-NEXT: (J) j = {}
+# TARGET-VAR-NEXT: (lldb) target variable static_e_ref
+# TARGET-VAR-NEXT: (E &) static_e_ref = 0x{{.*}} <incomplete type "E">
+
+# EXPR:      (lldb) expression b
+# EXPR-NEXT: (B) $0 = {}
+# EXPR-NEXT: (lldb) expression d
+# EXPR-NEXT: (D) $1 = {}
+# EXPR-NEXT: (lldb) expression h
+# EXPR-NEXT: (H) $2 = {
+# EXPR-NEXT:   g = {
+# EXPR-NEXT:     [0] = (foo = 1)
+# EXPR-NEXT:     [1] = (foo = 1)
+# EXPR-NEXT:   }
+# EXPR-NEXT: }
+# EXPR-NEXT: (lldb) expression j
+# EXPR-NEXT: (J) $3 = {}
+# EXPR-NEXT: (lldb) expression static_e_ref
+# EXPR:      error:{{.*}}incomplete type 'E' where a complete type is required

Michael137

The way we do it for DWARF (for better or for worse) is that we complete the array element type, not the array type itself. FWIW, that's what completing a ConstantArrayType does anyway. It completes the element type.

Is there a place in the PDB plugin where we can do the same thing we do for DWARF?

llvm-project/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Lines 1507 to 1508 in 74b9484

    
           CompilerType array_element_type = element_type->GetForwardCompilerType(); 
        
           TypeSystemClang::RequireCompleteType(array_element_type);

Nerixyz · 2025-09-02T10:16:45Z

It completes the element type.

As in, "if an array type is encountered, it's immediately completed"? Because in native PDB (with this PR too), it would be delayed.

Is there a place in the PDB plugin where we can do the same thing we do for DWARF?

Yes, in PdbAstBuilder::createArrayType a CompleteType(element_type) could be added, and the test would pass too:

llvm-project/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

Lines 1170 to 1172 in 74b9484

    
           clang::QualType PdbAstBuilder::CreateArrayType(const ArrayRecord &ar) { 
        
             clang::QualType element_type = GetOrCreateType(ar.ElementType);

Would this be a better place?

Michael137 · 2025-09-02T12:39:32Z

It completes the element type.

As in, "if an array type is encountered, it's immediately completed"? Because in native PDB (with this PR too), it would be delayed.

I meant, "completing a ConstantArrayType" is the same as "completing the element type". This PR implements the former, whereas the DWARF plugin we opted for the latter.

Is there a place in the PDB plugin where we can do the same thing we do for DWARF?

Yes, in PdbAstBuilder::createArrayType a CompleteType(element_type) could be added, and the test would pass too:

llvm-project/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

Lines 1170 to 1172 in 74b9484

clang::QualType PdbAstBuilder::CreateArrayType(const ArrayRecord &ar) {

clang::QualType element_type = GetOrCreateType(ar.ElementType);

Would this be a better place?

Yea that seems like the right thing to do

Michael137 · 2025-09-02T12:45:12Z

Actually, what you want to do is probably just call TypeSystemClang::RequireCompleteType on the element type. This is basically what the PDB plugin does:

llvm-project/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp

Lines 685 to 695 in 6042e09

    
           CompilerType element_ast_type = element_type->GetForwardCompilerType(); 
        
           // If element type is UDT, it needs to be complete. 
        
           if (TypeSystemClang::IsCXXClassType(element_ast_type) && 
        
               !element_ast_type.GetCompleteType()) { 
        
             if (TypeSystemClang::StartTagDeclarationDefinition(element_ast_type)) { 
        
               TypeSystemClang::CompleteTagDeclarationDefinition(element_ast_type); 
        
             } else { 
        
               // We are not able to start definition. 
        
               return nullptr; 
        
             } 
        
           }

(I suspect this is why the PDB plugin passes this test?)

But I suspect this code came before RequireCompleteType got introduced. We can probably just change the PDB plugin to call RequireCompleteType instead too. Though since we're removing that plugin anyway, leaving it as-is is fine too.

Michael137 · 2025-09-02T15:11:36Z

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

+struct I {
+  I();
+};
+I::I() = default;


What's this used for?

~~Oops, was testing something unrelated...~~

It wasn't unrelated (I undid that locally).

It's to test that arrays of forward declared types work. I had to add a new type there to make ensure there was no attempt to complete it from the previous target variable calls that don't go through arrays.

This reverts commit 28ee7f4.

ZequanWu

LGTM as it aligns with dwarf plugin's behaviour.

[LLDB] Complete constant array member types in class members

967d345

Nerixyz requested review from labath and ZequanWu September 1, 2025 19:44

Nerixyz requested a review from JDevlieghere as a code owner September 1, 2025 19:44

llvmbot added the lldb label Sep 1, 2025

fix: return if complete

4a08289

Michael137 reviewed Sep 2, 2025

View reviewed changes

fix: require element type when creating array type

d978d59

Nerixyz changed the title ~~[LLDB] Complete constant array member types in class members~~ [LLDB][NativePDB] Complete array member types in AST builder Sep 2, 2025

Michael137 reviewed Sep 2, 2025

View reviewed changes

Michael137 approved these changes Sep 2, 2025

View reviewed changes

Nerixyz added 2 commits September 2, 2025 18:09

fix: remove unused type

28ee7f4

Revert "fix: remove unused type"

f90ce9a

This reverts commit 28ee7f4.

ZequanWu approved these changes Sep 2, 2025

View reviewed changes

Nerixyz merged commit 3c7bf3b into llvm:main Sep 2, 2025
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[LLDB][NativePDB] Complete array member types in AST builder #156370

[LLDB][NativePDB] Complete array member types in AST builder #156370

Nerixyz commented Sep 1, 2025 •

edited

Loading

Uh oh!

llvmbot commented Sep 1, 2025

Uh oh!

Michael137 left a comment

Uh oh!

Nerixyz commented Sep 2, 2025

Uh oh!

Michael137 commented Sep 2, 2025

Uh oh!

Michael137 commented Sep 2, 2025 •

edited

Loading

Uh oh!

Michael137 Sep 2, 2025

Uh oh!

Nerixyz Sep 2, 2025 •

edited

Loading

Uh oh!

ZequanWu left a comment

Uh oh!

Uh oh!

Uh oh!

	CompilerType array_element_type = element_type->GetForwardCompilerType();
	TypeSystemClang::RequireCompleteType(array_element_type);

[LLDB][NativePDB] Complete array member types in AST builder #156370

[LLDB][NativePDB] Complete array member types in AST builder #156370

Conversation

Nerixyz commented Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 1, 2025

Uh oh!

Michael137 left a comment

Choose a reason for hiding this comment

Uh oh!

Nerixyz commented Sep 2, 2025

Uh oh!

Michael137 commented Sep 2, 2025

Uh oh!

Michael137 commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Michael137 Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Nerixyz Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZequanWu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Nerixyz commented Sep 1, 2025 •

edited

Loading

Michael137 commented Sep 2, 2025 •

edited

Loading

Nerixyz Sep 2, 2025 •

edited

Loading