[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 13:36:24 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/5] [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/5] 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/5] 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/5] 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);
>From 8207a5fd8e3e374e8f1d457ffe8157fe4c21193c Mon Sep 17 00:00:00 2001
From: Nerixyz <nerixdev at outlook.de>
Date: Tue, 4 Nov 2025 22:36:07 +0100
Subject: [PATCH 5/5] fix: handle unions as well
---
.../NativePDB/UdtRecordCompleter.cpp | 19 +++++++++++------
.../NativePDB/UdtRecordCompleterTests.cpp | 21 ++++++++-----------
2 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
index 8e544d5f2c505..01674d5993dcd 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -467,12 +467,19 @@ void UdtRecordCompleter::Record::ConstructRecord() {
if (iter->second.empty())
continue;
- // For structs, if the new fields come after the already added ones
- // without overlap, go back to the root struct.
- if (iter->first <= offset && record.kind == Member::Struct &&
- is_last_end_offset(iter))
- parent = &record;
- else {
+ // If the new fields come after the already added ones
+ // without overlap, go back to the root.
+ if (iter->first <= offset && is_last_end_offset(iter)) {
+ if (record.kind == Member::Struct)
+ parent = &record;
+ else {
+ lldbassert(record.kind == Member::Union &&
+ "Current record must be a union");
+ lldbassert(!record.fields.empty());
+ // For unions, append the field to the last struct
+ parent = record.fields.back().get();
+ }
+ } else {
parent = iter->second.back();
iter->second.pop_back();
}
diff --git a/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp b/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
index 81566807ab0c6..cd6db5fcb1f4c 100644
--- a/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
+++ b/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
@@ -275,24 +275,21 @@ TEST_F(UdtRecordCompleterRecordTests, TestNestedStructInUnionInStructInUnion) {
// m4;
// union {
// m5;
- // struct {
- // m6;
- // m7;
- // };
+ // 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);
+ Member *s = AddField(&record.record, "", 0, 0, Member::Struct);
+ AddField(s, "m3", 0, 2, Member::Field);
+ AddField(s, "m4", 2, 4, Member::Field);
+ Member *u = AddField(s, "", 6, 0, Member::Union);
+ AddField(u, "m5", 6, 2, Member::Field);
+ AddField(u, "m6", 6, 2, Member::Field);
+ AddField(s, "m7", 8, 2, Member::Field);
EXPECT_EQ(WrappedRecord(this->record), WrappedRecord(record));
}
More information about the lldb-commits
mailing list