[Lldb-commits] [lldb] [LLDB][NativePDB] Add non-overlapping fields in root struct (PR #166243)

via lldb-commits lldb-commits at lists.llvm.org
Tue Nov 4 01:05:39 PST 2025


https://github.com/Nerixyz updated https://github.com/llvm/llvm-project/pull/166243

>From 22628e5e4ea56a5e06c8550a66cb86d1470cba0b Mon Sep 17 00:00:00 2001
From: Nerixyz <nerixdev at outlook.de>
Date: Mon, 3 Nov 2025 22:31:35 +0100
Subject: [PATCH 1/4] [LLDB][NativePDB] Add non-overlapping fields in root
 struct

---
 .../NativePDB/UdtRecordCompleter.cpp          | 13 ++++-
 .../NativePDB/UdtRecordCompleterTests.cpp     | 55 ++++++++++++++++++-
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
index 1c575e90bd72c..1458079483278 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -442,6 +442,10 @@ void UdtRecordCompleter::Record::ConstructRecord() {
 
   // The end offset to a vector of field/struct that ends at the offset.
   std::map<uint64_t, std::vector<Member *>> end_offset_map;
+  auto is_last_end_offset = [&](auto it) {
+    return it != end_offset_map.end() && ++it == end_offset_map.end();
+  };
+
   for (auto &pair : fields_map) {
     uint64_t offset = pair.first;
     auto &fields = pair.second;
@@ -463,7 +467,14 @@ void UdtRecordCompleter::Record::ConstructRecord() {
       if (iter->second.empty())
         continue;
       parent = iter->second.back();
-      iter->second.pop_back();
+
+      // For structs, if the new fields come after the already added ones
+      // without overlap, go back to the root struct.
+      if (parent != &record && iter->first <= offset &&
+          record.kind == Member::Struct && is_last_end_offset(iter))
+        parent = &record;
+      else
+        iter->second.pop_back();
     }
     // If it's a field, then the field is inside a union, so we can safely
     // increase its size by converting it to a struct to hold multiple fields.
diff --git a/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp b/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
index 17284b61b9a6e..3432e03bbf853 100644
--- a/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
+++ b/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
@@ -99,7 +99,7 @@ Member *AddField(Member *member, StringRef name, uint64_t byte_offset,
       std::make_unique<Member>(name, byte_offset * 8, byte_size * 8,
                                clang::QualType(), lldb::eAccessPublic, 0);
   field->kind = kind;
-  field->base_offset = base_offset;
+  field->base_offset = base_offset * 8;
   member->fields.push_back(std::move(field));
   return member->fields.back().get();
 }
@@ -111,6 +111,9 @@ TEST_F(UdtRecordCompleterRecordTests, TestAnonymousUnionInStruct) {
   CollectMember("m2", 0, 4);
   CollectMember("m3", 0, 1);
   CollectMember("m4", 0, 8);
+  CollectMember("m5", 8, 8);
+  CollectMember("m6", 16, 4);
+  CollectMember("m7", 16, 8);
   ConstructRecord();
 
   // struct {
@@ -120,6 +123,11 @@ TEST_F(UdtRecordCompleterRecordTests, TestAnonymousUnionInStruct) {
   //       m3;
   //       m4;
   //   };
+  //   m5;
+  //   union {
+  //       m6;
+  //       m7;
+  //   };
   // };
   Record record;
   record.start_offset = 0;
@@ -128,6 +136,10 @@ TEST_F(UdtRecordCompleterRecordTests, TestAnonymousUnionInStruct) {
   AddField(u, "m2", 0, 4, Member::Field);
   AddField(u, "m3", 0, 1, Member::Field);
   AddField(u, "m4", 0, 8, Member::Field);
+  AddField(&record.record, "m5", 8, 8, Member::Field);
+  Member *u2 = AddField(&record.record, "", 16, 0, Member::Union);
+  AddField(u2, "m6", 16, 4, Member::Field);
+  AddField(u2, "m7", 16, 8, Member::Field);
   EXPECT_EQ(WrappedRecord(this->record), WrappedRecord(record));
 }
 
@@ -243,3 +255,44 @@ TEST_F(UdtRecordCompleterRecordTests, TestNestedUnionStructInUnion) {
   AddField(s2, "m4", 2, 4, Member::Field);
   EXPECT_EQ(WrappedRecord(this->record), WrappedRecord(record));
 }
+
+TEST_F(UdtRecordCompleterRecordTests, TestNestedUnionStructInUnion2) {
+  SetKind(Member::Kind::Union);
+  CollectMember("m1", 0, 4);
+  CollectMember("m2", 0, 2);
+  CollectMember("m3", 0, 2);
+  CollectMember("m4", 2, 4);
+  CollectMember("m5", 6, 2);
+  CollectMember("m6", 6, 2);
+  CollectMember("m7", 8, 2);
+  ConstructRecord();
+
+  // union {
+  //   m1;
+  //   m2;
+  //   struct {
+  //       m3;
+  //       m4;
+  //       union {
+  //           m5;
+  //           struct {
+  //               m6;
+  //               m7;
+  //           };
+  //       };
+  //   };
+  // };
+  Record record;
+  record.start_offset = 0;
+  AddField(&record.record, "m1", 0, 4, Member::Field);
+  AddField(&record.record, "m2", 0, 2, Member::Field);
+  Member *s1 = AddField(&record.record, "", 0, 0, Member::Struct);
+  AddField(s1, "m3", 0, 2, Member::Field);
+  AddField(s1, "m4", 2, 4, Member::Field);
+  Member *u1 = AddField(s1, "", 6, 0, Member::Union);
+  AddField(u1, "m5", 6, 2, Member::Field);
+  Member *s2 = AddField(u1, "", 0, 0, Member::Struct, 6);
+  AddField(s2, "m6", 6, 2, Member::Field);
+  AddField(s2, "m7", 8, 2, Member::Field);
+  EXPECT_EQ(WrappedRecord(this->record), WrappedRecord(record));
+}

>From fc1478f60e9ad5262503f19cf79150012cb94eb1 Mon Sep 17 00:00:00 2001
From: Nerixyz <nerixdev at outlook.de>
Date: Tue, 4 Nov 2025 10:01:22 +0100
Subject: [PATCH 2/4] fix: class layout test

---
 lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp b/lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp
index 36bfdb9a8e565..83ed533eb13e3 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp
@@ -34,9 +34,6 @@
 // CHECK-NEXT:           s4 = {
 // CHECK-NEXT:             x = ([0] = 67, [1] = 68, [2] = 99)
 // CHECK-NEXT:           }
-// CHECK-NEXT:           s1 = {
-// CHECK-NEXT:             x = ([0] = 69, [1] = 70, [2] = 71)
-// CHECK-NEXT:           }
 // CHECK-NEXT:         }
 // CHECK-NEXT:       }
 // CHECK-NEXT:     }
@@ -47,6 +44,9 @@
 // CHECK-NEXT:       c2 = 'D'
 // CHECK-NEXT:     }
 // CHECK-NEXT:   }
+// CHECK-NEXT:   s1 = {
+// CHECK-NEXT:     x = ([0] = 69, [1] = 70, [2] = 71)
+// CHECK-NEXT:   }
 // CHECK-NEXT: }
 // CHECK-NEXT: (lldb) type lookup C
 // CHECK-NEXT: struct C {
@@ -63,7 +63,6 @@
 // CHECK-NEXT:                 struct {
 // CHECK-NEXT:                     char c4;
 // CHECK-NEXT:                     S3 s4;
-// CHECK-NEXT:                     S3 s1;
 // CHECK-NEXT:                 };
 // CHECK-NEXT:             };
 // CHECK-NEXT:         };
@@ -72,6 +71,7 @@
 // CHECK-NEXT:             char c2;
 // CHECK-NEXT:         };
 // CHECK-NEXT:     };
+// CHECK-NEXT:     S3 s1;
 // CHECK-NEXT: }
 
 

>From 5643c377eab0e4c8c9203ac2622ae38487fa9934 Mon Sep 17 00:00:00 2001
From: Nerixyz <nerixdev at outlook.de>
Date: Tue, 4 Nov 2025 10:04:06 +0100
Subject: [PATCH 3/4] fix: move `parent` assignment to else branch

---
 .../Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp  | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
index 1458079483278..8e544d5f2c505 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -466,15 +466,16 @@ void UdtRecordCompleter::Record::ConstructRecord() {
       }
       if (iter->second.empty())
         continue;
-      parent = iter->second.back();
 
       // For structs, if the new fields come after the already added ones
       // without overlap, go back to the root struct.
-      if (parent != &record && iter->first <= offset &&
-          record.kind == Member::Struct && is_last_end_offset(iter))
+      if (iter->first <= offset && record.kind == Member::Struct &&
+          is_last_end_offset(iter))
         parent = &record;
-      else
+      else {
+        parent = iter->second.back();
         iter->second.pop_back();
+      }
     }
     // If it's a field, then the field is inside a union, so we can safely
     // increase its size by converting it to a struct to hold multiple fields.

>From 2e0b691295b04e04eddeabe4ae1760d4d381cae9 Mon Sep 17 00:00:00 2001
From: Nerixyz <nerixdev at outlook.de>
Date: Tue, 4 Nov 2025 10:05:21 +0100
Subject: [PATCH 4/4] fix: test name

---
 lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp b/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
index 3432e03bbf853..81566807ab0c6 100644
--- a/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
+++ b/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
@@ -256,7 +256,7 @@ TEST_F(UdtRecordCompleterRecordTests, TestNestedUnionStructInUnion) {
   EXPECT_EQ(WrappedRecord(this->record), WrappedRecord(record));
 }
 
-TEST_F(UdtRecordCompleterRecordTests, TestNestedUnionStructInUnion2) {
+TEST_F(UdtRecordCompleterRecordTests, TestNestedStructInUnionInStructInUnion) {
   SetKind(Member::Kind::Union);
   CollectMember("m1", 0, 4);
   CollectMember("m2", 0, 2);



More information about the lldb-commits mailing list