Author: nerix Date: 2025-11-05T19:07:44+01:00 New Revision: 3c162ba247d30c9d8113e66fe5d96e24156ce797
URL: https://github.com/llvm/llvm-project/commit/3c162ba247d30c9d8113e66fe5d96e24156ce797 DIFF: https://github.com/llvm/llvm-project/commit/3c162ba247d30c9d8113e66fe5d96e24156ce797.diff LOG: [LLDB][NativePDB] Add non-overlapping fields in root struct (#166243) When anonymous unions are used in a struct or vice versa, their fields are merged into the parent record when using PDB. LLDB tries to recreate the original definition of the record _with_ the anonymous unions/structs. For tagged unions (like `std::optional`) where the tag followed the anonymous union, the result was suboptimal: ```cpp // input: struct Foo { union { Bar b; char c; }; bool tag; }; // reconstructed: struct Foo { union { Bar b; struct { char c; bool tag; }; }; }; ``` Once the algorithm is in some nested union, it can't get out. In the above case, we can get to the correct reconstructed record if we always add fields that don't overlap others in the root struct. So when we see `tag`, we'll see that it comes after all other fields, so it's possible to add it in the root `Foo`. Added: Modified: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp index 1c575e90bd72c..46cf9b8524ede 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; @@ -462,8 +466,23 @@ void UdtRecordCompleter::Record::ConstructRecord() { } if (iter->second.empty()) continue; - parent = iter->second.back(); - iter->second.pop_back(); + + // 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 { + assert(record.kind == Member::Union && + "Current record must be a union"); + assert(!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(); + } } // 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/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: } diff --git a/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp b/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp index 17284b61b9a6e..cd6db5fcb1f4c 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,41 @@ TEST_F(UdtRecordCompleterRecordTests, TestNestedUnionStructInUnion) { AddField(s2, "m4", 2, 4, Member::Field); EXPECT_EQ(WrappedRecord(this->record), WrappedRecord(record)); } + +TEST_F(UdtRecordCompleterRecordTests, TestNestedStructInUnionInStructInUnion) { + 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; + // 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 *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)); +} _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
