Author: Nerixyz Date: 2026-08-26T20:53:40+02:00 New Revision: 90a5ddf2a9775540c9b6c24567cd96dcc9d02bea
URL: https://github.com/llvm/llvm-project/commit/90a5ddf2a9775540c9b6c24567cd96dcc9d02bea DIFF: https://github.com/llvm/llvm-project/commit/90a5ddf2a9775540c9b6c24567cd96dcc9d02bea.diff LOG: [lldb][NativePDB] Keep declaration order of struct fields (#218731) Unlike in C/C++, struct fields in Rust may be reordered by the compiler to reduce padding. When completing records, we only tracked the offset of fields inside the struct. For Rust this could result in an order that didn't match the declaration (#218613). This PR tracks the declaration order and sorts fields after the record is constructed. It's assumed that the members inside the `LF_FIELDLIST` are present in declaration order. Fixes #218613. Added: Modified: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h 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 bdc0a54c9fc39..1681b07ea5f86 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp +++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp @@ -60,6 +60,12 @@ UdtRecordCompleter::UdtRecordCompleter( } } +llvm::Error +UdtRecordCompleter::visitMemberEnd(llvm::codeview::CVMemberRecord &Record) { + ++m_member_index; + return Error::success(); +} + clang::QualType UdtRecordCompleter::AddBaseClassForTypeIndex( llvm::codeview::TypeIndex ti, llvm::codeview::MemberAccess access, std::optional<uint64_t> vtable_idx) { @@ -300,8 +306,8 @@ Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr, bitfield_width ? bitfield_width : GetSizeOfType(ti, m_index.tpi()) * 8; if (field_size == 0) return Error::success(); - m_record.CollectMember(data_member.Name, offset, field_size, member_qt, access, - bitfield_width); + m_record.CollectMember(data_member.Name, offset, field_size, member_qt, + access, bitfield_width, m_member_index); return Error::success(); } @@ -465,11 +471,22 @@ void UdtRecordCompleter::FinishRecord() { } } +void UdtRecordCompleter::Member::RestoreOriginalOrder() { + llvm::stable_sort(fields, [](const MemberUP &lhs, const MemberUP &rhs) { + return std::tie(lhs->original_index, lhs->bit_offset) < + std::tie(rhs->original_index, rhs->bit_offset); + }); + + for (MemberUP &field : fields) + field->RestoreOriginalOrder(); +} + void UdtRecordCompleter::Record::CollectMember( llvm::StringRef name, uint64_t offset, uint64_t field_size, - clang::QualType qt, lldb::AccessType access, uint64_t bitfield_width) { + clang::QualType qt, lldb::AccessType access, uint64_t bitfield_width, + uint32_t member_index) { fields_map[offset].push_back(std::make_unique<Member>( - name, offset, field_size, qt, access, bitfield_width)); + name, offset, field_size, qt, access, bitfield_width, member_index)); if (start_offset > offset) start_offset = offset; } @@ -555,6 +572,7 @@ void UdtRecordCompleter::Record::ConstructRecord() { if (parent->kind == Member::Struct) { parent->fields.push_back(std::make_unique<Member>(Member::Union)); parent = parent->fields.back().get(); + parent->original_index = std::numeric_limits<uint32_t>::max(); parent->bit_offset = offset; } else { assert(parent == &record && @@ -562,10 +580,15 @@ void UdtRecordCompleter::Record::ConstructRecord() { } for (auto &field : fields) { int64_t bit_size = field->bit_size; + // Use the lowest index of the union members. + parent->original_index = + std::min(parent->original_index, field->original_index); parent->fields.push_back(std::move(field)); end_offset_map[offset + bit_size].push_back( parent->fields.back().get()); } } } + + record.RestoreOriginalOrder(); } diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h index 0a6aedefa69e8..0b4dd30a435ff 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h +++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h @@ -53,6 +53,8 @@ class UdtRecordCompleter : public llvm::codeview::TypeVisitorCallbacks { llvm::DenseMap<lldb::opaque_compiler_type_t, llvm::SmallSet<std::pair<llvm::StringRef, CompilerType>, 8>> &m_cxx_record_map; + /// Index of the current member. + uint32_t m_member_index = 0; public: UdtRecordCompleter( @@ -63,6 +65,8 @@ class UdtRecordCompleter : public llvm::codeview::TypeVisitorCallbacks { llvm::SmallSet<std::pair<llvm::StringRef, CompilerType>, 8>> &cxx_record_map); + llvm::Error visitMemberEnd(llvm::codeview::CVMemberRecord &Record) override; + #define MEMBER_RECORD(EnumName, EnumVal, Name) \ llvm::Error visitKnownMember(llvm::codeview::CVMemberRecord &CVR, \ llvm::codeview::Name##Record &Record) override; @@ -81,6 +85,8 @@ class UdtRecordCompleter : public llvm::codeview::TypeVisitorCallbacks { clang::QualType qt; lldb::AccessType access; uint32_t bitfield_width; + /// Index of the member inside the LF_FIELDLIST. + uint32_t original_index = 0; // Following are Only used for struct or union. uint64_t base_offset; llvm::SmallVector<MemberUP, 1> fields; @@ -90,20 +96,25 @@ class UdtRecordCompleter : public llvm::codeview::TypeVisitorCallbacks { : kind(kind), name(), bit_offset(0), bit_size(0), qt(), access(lldb::eAccessPublic), bitfield_width(0), base_offset(0) {} Member(llvm::StringRef name, uint64_t bit_offset, uint64_t bit_size, - clang::QualType qt, lldb::AccessType access, uint32_t bitfield_width) + clang::QualType qt, lldb::AccessType access, uint32_t bitfield_width, + uint32_t original_index) : kind(Field), name(name), bit_offset(bit_offset), bit_size(bit_size), qt(qt), access(access), bitfield_width(bitfield_width), - base_offset(0) {} + original_index(original_index), base_offset(0) {} void ConvertToStruct() { kind = Struct; base_offset = bit_offset; fields.push_back(std::make_unique<Member>(name, bit_offset, bit_size, qt, - access, bitfield_width)); + access, bitfield_width, + original_index)); name = llvm::StringRef(); qt = clang::QualType(); access = lldb::eAccessPublic; bit_offset = bit_size = bitfield_width = 0; + // Keep original_index. } + + void RestoreOriginalOrder(); }; struct Record { @@ -113,7 +124,8 @@ class UdtRecordCompleter : public llvm::codeview::TypeVisitorCallbacks { std::map<uint64_t, llvm::SmallVector<MemberUP, 1>> fields_map; void CollectMember(llvm::StringRef name, uint64_t offset, uint64_t field_size, clang::QualType qt, - lldb::AccessType access, uint64_t bitfield_width); + lldb::AccessType access, uint64_t bitfield_width, + uint32_t member_index); void ConstructRecord(); }; void complete(); diff --git a/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp b/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp index cd6db5fcb1f4c..7553834f3ce99 100644 --- a/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp +++ b/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp @@ -41,10 +41,11 @@ class WrappedMember { friend llvm::raw_ostream &operator<<(llvm::raw_ostream &os, const WrappedMember &w) { - os << llvm::formatv("Member{.kind={0}, .name=\"{1}\", .bit_offset={2}, " - ".bit_size={3}, .base_offset={4}, .fields=[", - w.m_obj.kind, w.m_obj.name, w.m_obj.bit_offset, - w.m_obj.bit_size, w.m_obj.base_offset); + os << llvm::formatv( + "Member{.kind={0}, .name=\"{1}\", .bit_offset={2}, " + ".bit_size={3}, .base_offset={4}, .original_index={5}, .fields=[", + w.m_obj.kind, w.m_obj.name, w.m_obj.bit_offset, w.m_obj.bit_size, + w.m_obj.base_offset, w.m_obj.original_index); llvm::ListSeparator sep; for (auto &f : w.m_obj.fields) os << sep << WrappedMember(*f); @@ -83,21 +84,27 @@ class WrappedRecord { class UdtRecordCompleterRecordTests : public testing::Test { protected: Record record; + uint32_t field_index = 0; public: void SetKind(Member::Kind kind) { record.record.kind = kind; } void CollectMember(StringRef name, uint64_t byte_offset, uint64_t byte_size) { record.CollectMember(name, byte_offset * 8, byte_size * 8, - clang::QualType(), lldb::eAccessPublic, 0); + clang::QualType(), lldb::eAccessPublic, 0, + ++field_index); + } + void ConstructRecord() { + record.ConstructRecord(); + field_index = 0; } - void ConstructRecord() { record.ConstructRecord(); } }; + Member *AddField(Member *member, StringRef name, uint64_t byte_offset, uint64_t byte_size, Member::Kind kind, uint64_t base_offset = 0) { - auto field = - std::make_unique<Member>(name, byte_offset * 8, byte_size * 8, - clang::QualType(), lldb::eAccessPublic, 0); + auto field = std::make_unique<Member>( + name, byte_offset * 8, byte_size * 8, clang::QualType(), + lldb::eAccessPublic, /*bitfield_width=*/0, /*original_index=*/0); field->kind = kind; field->base_offset = base_offset * 8; member->fields.push_back(std::move(field)); @@ -293,3 +300,30 @@ TEST_F(UdtRecordCompleterRecordTests, TestNestedStructInUnionInStructInUnion) { AddField(s, "m7", 8, 2, Member::Field); EXPECT_EQ(WrappedRecord(this->record), WrappedRecord(record)); } + +TEST_F(UdtRecordCompleterRecordTests, TestReorderedStuctFields) { + SetKind(Member::Kind::Struct); + CollectMember("__0", 12, 2); + CollectMember("__1", 14, 2); + CollectMember("__2", 8, 4); + CollectMember("__3", 0, 8); + ConstructRecord(); + + // From Rust. + // struct Foo(i16, u16, i32, u64) + // Is reordered by Rust to: + // struct Foo { + // u64 __3; + // i32 __2; + // i16 __0; + // u16 __1: + // }; + + Record record; + record.start_offset = 0; + AddField(&record.record, "__0", 12, 2, Member::Field); + AddField(&record.record, "__1", 14, 2, Member::Field); + AddField(&record.record, "__2", 8, 4, Member::Field); + AddField(&record.record, "__3", 0, 8, 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
