zequanwu updated this revision to Diff 467573. zequanwu marked 2 inline comments as done. zequanwu added a comment.
Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134849/new/ https://reviews.llvm.org/D134849 Files: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h lldb/test/Shell/SymbolFile/NativePDB/Inputs/class_layout.lldbinit lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp lldb/unittests/SymbolFile/NativePDB/CMakeLists.txt lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
Index: lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp =================================================================== --- /dev/null +++ lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp @@ -0,0 +1,244 @@ +//===-- UdtRecordCompleterTests.cpp ---------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace lldb_private::npdb; +using namespace llvm; + +namespace { +using Member = UdtRecordCompleter::Member; +using MemberUP = std::unique_ptr<Member>; +using Record = UdtRecordCompleter::Record; + +class WrappedMember { +public: + WrappedMember(const Member &obj) : m_obj(obj) {} + +private: + const Member &m_obj; + + friend bool operator==(const WrappedMember &lhs, const WrappedMember &rhs) { + return lhs.m_obj.kind == rhs.m_obj.kind && + lhs.m_obj.name == rhs.m_obj.name && + lhs.m_obj.bit_offset == rhs.m_obj.bit_offset && + lhs.m_obj.bit_size == rhs.m_obj.bit_size && + lhs.m_obj.base_offset == rhs.m_obj.base_offset && + std::equal(lhs.m_obj.fields.begin(), lhs.m_obj.fields.end(), + rhs.m_obj.fields.begin(), rhs.m_obj.fields.end(), + [](const MemberUP &lhs, const MemberUP &rhs) { + return WrappedMember(*lhs) == WrappedMember(*rhs); + }); + } + + 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); + llvm::ListSeparator sep; + for (auto &f : w.m_obj.fields) + os << sep << WrappedMember(*f); + return os << "]}"; + } +}; + +class WrappedRecord { +public: + WrappedRecord(const Record &obj) : m_obj(obj) {} + +private: + const Record &m_obj; + + friend bool operator==(const WrappedRecord &lhs, const WrappedRecord &rhs) { + return lhs.m_obj.start_offset == rhs.m_obj.start_offset && + std::equal( + lhs.m_obj.record.fields.begin(), lhs.m_obj.record.fields.end(), + rhs.m_obj.record.fields.begin(), rhs.m_obj.record.fields.end(), + [](const MemberUP &lhs, const MemberUP &rhs) { + return WrappedMember(*lhs) == WrappedMember(*rhs); + }); + } + + friend llvm::raw_ostream &operator<<(llvm::raw_ostream &os, + const WrappedRecord &w) { + os << llvm::formatv("Record{.start_offset={0}, .record.fields=[", + w.m_obj.start_offset); + llvm::ListSeparator sep; + for (const MemberUP &f : w.m_obj.record.fields) + os << sep << WrappedMember(*f); + return os << "]}"; + } +}; + +class UdtRecordCompleterRecordTests : public testing::Test { +protected: + Record record; + +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); + } + 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); + field->kind = kind; + field->base_offset = base_offset; + member->fields.push_back(std::move(field)); + return member->fields.back().get(); +} +} // namespace + +TEST_F(UdtRecordCompleterRecordTests, TestAnonymousUnionInStruct) { + SetKind(Member::Kind::Struct); + CollectMember("m1", 0, 4); + CollectMember("m2", 0, 4); + CollectMember("m3", 0, 1); + CollectMember("m4", 0, 8); + ConstructRecord(); + + // struct { + // union { + // m1; + // m2; + // m3; + // m4; + // }; + // }; + Record record; + record.start_offset = 0; + Member *u = AddField(&record.record, "", 0, 0, Member::Union); + AddField(u, "m1", 0, 4, Member::Field); + AddField(u, "m2", 0, 4, Member::Field); + AddField(u, "m3", 0, 1, Member::Field); + AddField(u, "m4", 0, 8, Member::Field); + EXPECT_EQ(WrappedRecord(this->record), WrappedRecord(record)); +} + +TEST_F(UdtRecordCompleterRecordTests, TestAnonymousUnionInUnion) { + SetKind(Member::Kind::Union); + CollectMember("m1", 0, 4); + CollectMember("m2", 0, 4); + CollectMember("m3", 0, 1); + CollectMember("m4", 0, 8); + ConstructRecord(); + + // union { + // m1; + // m2; + // m3; + // m4; + // }; + Record record; + record.start_offset = 0; + AddField(&record.record, "m1", 0, 4, Member::Field); + AddField(&record.record, "m2", 0, 4, Member::Field); + AddField(&record.record, "m3", 0, 1, Member::Field); + AddField(&record.record, "m4", 0, 8, Member::Field); + EXPECT_EQ(WrappedRecord(this->record), WrappedRecord(record)); +} + +TEST_F(UdtRecordCompleterRecordTests, TestAnonymousStructInUnion) { + SetKind(Member::Kind::Union); + CollectMember("m1", 0, 4); + CollectMember("m2", 4, 4); + CollectMember("m3", 8, 1); + ConstructRecord(); + + // union { + // struct { + // m1; + // m2; + // m3; + // }; + // }; + Record record; + record.start_offset = 0; + Member *s = AddField(&record.record, "", 0, 0, Member::Kind::Struct); + AddField(s, "m1", 0, 4, Member::Field); + AddField(s, "m2", 4, 4, Member::Field); + AddField(s, "m3", 8, 1, Member::Field); + EXPECT_EQ(WrappedRecord(this->record), WrappedRecord(record)); +} + +TEST_F(UdtRecordCompleterRecordTests, TestNestedUnionStructInStruct) { + SetKind(Member::Kind::Struct); + CollectMember("m1", 0, 4); + CollectMember("m2", 0, 2); + CollectMember("m3", 0, 2); + CollectMember("m4", 2, 4); + CollectMember("m5", 3, 2); + ConstructRecord(); + + // struct { + // union { + // m1; + // struct { + // m2; + // m5; + // }; + // struct { + // m3; + // m4; + // }; + // }; + // }; + Record record; + record.start_offset = 0; + Member *u = AddField(&record.record, "", 0, 0, Member::Union); + AddField(u, "m1", 0, 4, Member::Field); + Member *s1 = AddField(u, "", 0, 0, Member::Struct); + Member *s2 = AddField(u, "", 0, 0, Member::Struct); + AddField(s1, "m2", 0, 2, Member::Field); + AddField(s1, "m5", 3, 2, Member::Field); + AddField(s2, "m3", 0, 2, Member::Field); + AddField(s2, "m4", 2, 4, Member::Field); + EXPECT_EQ(WrappedRecord(this->record), WrappedRecord(record)); +} + +TEST_F(UdtRecordCompleterRecordTests, TestNestedUnionStructInUnion) { + SetKind(Member::Kind::Union); + CollectMember("m1", 0, 4); + CollectMember("m2", 0, 2); + CollectMember("m3", 0, 2); + CollectMember("m4", 2, 4); + CollectMember("m5", 3, 2); + ConstructRecord(); + + // union { + // m1; + // struct { + // m2; + // m5; + // }; + // struct { + // m3; + // m4; + // }; + // }; + Record record; + record.start_offset = 0; + AddField(&record.record, "m1", 0, 4, Member::Field); + Member *s1 = AddField(&record.record, "", 0, 0, Member::Struct); + Member *s2 = AddField(&record.record, "", 0, 0, Member::Struct); + AddField(s1, "m2", 0, 2, Member::Field); + AddField(s1, "m5", 3, 2, Member::Field); + AddField(s2, "m3", 0, 2, Member::Field); + AddField(s2, "m4", 2, 4, Member::Field); + EXPECT_EQ(WrappedRecord(this->record), WrappedRecord(record)); +} Index: lldb/unittests/SymbolFile/NativePDB/CMakeLists.txt =================================================================== --- lldb/unittests/SymbolFile/NativePDB/CMakeLists.txt +++ lldb/unittests/SymbolFile/NativePDB/CMakeLists.txt @@ -1,5 +1,6 @@ add_lldb_unittest(SymbolFileNativePDBTests PdbFPOProgramToDWARFExpressionTests.cpp + UdtRecordCompleterTests.cpp LINK_LIBS lldbCore Index: lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp =================================================================== --- lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp +++ lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp @@ -242,6 +242,7 @@ // CHECK-NEXT: (lldb) type lookup -- Derived // CHECK-NEXT: class Derived : public Class { // CHECK-NEXT: public: +// CHECK-NEXT: Derived(); // CHECK-NEXT: Derived &Reference; // CHECK-NEXT: OneMember Member; // CHECK-NEXT: const OneMember ConstMember; Index: lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp =================================================================== --- lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp +++ /dev/null @@ -1,39 +0,0 @@ -// clang-format off -// REQUIRES: lld, x86 - -// Make sure class layout is correct. -// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s -// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb -// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe \ -// RUN: -o "expr a" -o "expr b.c" -o "expr b.u.c" -o "expr b.u.i" -o "exit" | FileCheck %s - -// CHECK: (lldb) expr a -// CHECK-NEXT: (A) $0 = (d1 = 'a', d2 = 1, d3 = 2, d4 = 'b') -// CHECK-NEXT: (lldb) expr b.c -// CHECK-NEXT: (char) $1 = 'a' -// CHECK-NEXT: (lldb) expr b.u.c -// CHECK-NEXT: (char[2]) $2 = "b" -// CHECK-NEXT: (lldb) expr b.u.i -// CHECK-NEXT: (int) $3 = 98 - -struct __attribute__((packed, aligned(1))) A { - char d1; - int d2; - int d3; - char d4; -}; - -struct __attribute__((packed, aligned(1))) B { - char c; - union { - char c[2]; - int i; - } u; -}; - -A a = {'a', 1, 2, 'b'}; -B b = {'a', {"b"}}; - -int main() { - return 0; -} Index: lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp =================================================================== --- lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp +++ lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp @@ -311,41 +311,41 @@ // CHECK: | `-EnumConstantDecl {{.*}} B 'EnumType' // CHECK: |-CXXRecordDecl {{.*}} struct DerivedClass definition // CHECK: | |-public 'BaseClass<int>' -// CHECK: | |-FieldDecl {{.*}} DerivedMember 'int' -// CHECK: | `-CXXConstructorDecl {{.*}} DerivedClass 'void (int, int)' -// CHECK: | |-ParmVarDecl {{.*}} 'int' -// CHECK: | `-ParmVarDecl {{.*}} 'int' +// CHECK: | |-CXXConstructorDecl {{.*}} DerivedClass 'void (int, int)' +// CHECK: | | |-ParmVarDecl {{.*}} 'int' +// CHECK: | | `-ParmVarDecl {{.*}} 'int' +// CHECK: | `-FieldDecl {{.*}} DerivedMember 'int' // CHECK: |-VarDecl {{.*}} DC 'const DerivedClass' // CHECK: |-CXXRecordDecl {{.*}} struct BaseClass<int> definition -// CHECK: | |-FieldDecl {{.*}} BaseMember 'int' -// CHECK: | `-CXXMethodDecl {{.*}} BaseClass 'void (int)' -// CHECK: | `-ParmVarDecl {{.*}} 'int' +// CHECK: | |-CXXMethodDecl {{.*}} BaseClass 'void (int)' +// CHECK: | | `-ParmVarDecl {{.*}} 'int' +// CHECK: | `-FieldDecl {{.*}} BaseMember 'int' // CHECK: |-CXXRecordDecl {{.*}} struct EBO definition // CHECK: | |-public 'EmptyBase' -// CHECK: | |-FieldDecl {{.*}} Member 'int' -// CHECK: | `-CXXConstructorDecl {{.*}} EBO 'void (int)' -// CHECK: | `-ParmVarDecl {{.*}} 'int' +// CHECK: | |-CXXConstructorDecl {{.*}} EBO 'void (int)' +// CHECK: | | `-ParmVarDecl {{.*}} 'int' +// CHECK: | `-FieldDecl {{.*}} Member 'int' // CHECK: |-VarDecl {{.*}} EBOC 'const EBO' // CHECK: |-CXXRecordDecl {{.*}} struct EmptyBase definition // CHECK: |-CXXRecordDecl {{.*}} struct PaddedBases definition // CHECK: | |-public 'BaseClass<char>' // CHECK: | |-public 'BaseClass<short>' // CHECK: | |-public 'BaseClass<int>' -// CHECK: | |-FieldDecl {{.*}} DerivedMember 'long long' -// CHECK: | `-CXXConstructorDecl {{.*}} PaddedBases 'void (char, short, int, long long)' -// CHECK: | |-ParmVarDecl {{.*}} 'char' -// CHECK: | |-ParmVarDecl {{.*}} 'short' -// CHECK: | |-ParmVarDecl {{.*}} 'int' -// CHECK: | `-ParmVarDecl {{.*}} 'long long' +// CHECK: | |-CXXConstructorDecl {{.*}} PaddedBases 'void (char, short, int, long long)' +// CHECK: | | |-ParmVarDecl {{.*}} 'char' +// CHECK: | | |-ParmVarDecl {{.*}} 'short' +// CHECK: | | |-ParmVarDecl {{.*}} 'int' +// CHECK: | | `-ParmVarDecl {{.*}} 'long long' +// CHECK: | `-FieldDecl {{.*}} DerivedMember 'long long' // CHECK: |-VarDecl {{.*}} PBC 'const PaddedBases' // CHECK: |-CXXRecordDecl {{.*}} struct BaseClass<char> definition -// CHECK: | |-FieldDecl {{.*}} BaseMember 'int' -// CHECK: | `-CXXMethodDecl {{.*}} BaseClass 'void (int)' -// CHECK: | `-ParmVarDecl {{.*}} 'int' +// CHECK: | |-CXXMethodDecl {{.*}} BaseClass 'void (int)' +// CHECK: | | `-ParmVarDecl {{.*}} 'int' +// CHECK: | `-FieldDecl {{.*}} BaseMember 'int' // CHECK: |-CXXRecordDecl {{.*}} struct BaseClass<short> definition -// CHECK: | |-FieldDecl {{.*}} BaseMember 'int' -// CHECK: | `-CXXMethodDecl {{.*}} BaseClass 'void (int)' -// CHECK: | `-ParmVarDecl {{.*}} 'int' +// CHECK: | |-CXXMethodDecl {{.*}} BaseClass 'void (int)' +// CHECK: | | `-ParmVarDecl {{.*}} 'int' +// CHECK: | `-FieldDecl {{.*}} BaseMember 'int' // CHECK: |-CXXRecordDecl {{.*}} struct <unnamed-type-UnnamedClassInstance> definition // CHECK: | |-FieldDecl {{.*}} x 'int' // CHECK: | `-FieldDecl {{.*}} EBOC 'EBO' Index: lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp =================================================================== --- /dev/null +++ lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp @@ -0,0 +1,131 @@ +// clang-format off +// REQUIRES: lld, x86 + +// Make sure class layout is correct. +// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s +// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb +// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \ +// RUN: %p/Inputs/class_layout.lldbinit 2>&1 | FileCheck %s + +// CHECK: (lldb) expr a +// CHECK-NEXT: (A) $0 = (d1 = 'a', d2 = 1, d3 = 2, d4 = 'b') +// CHECK-NEXT: (lldb) expr b.c +// CHECK-NEXT: (char) $1 = 'a' +// CHECK-NEXT: (lldb) expr b.u.c +// CHECK-NEXT: (char[2]) $2 = "b" +// CHECK-NEXT: (lldb) expr b.u.i +// CHECK-NEXT: (int) $3 = 98 +// CHECK-NEXT: (lldb) expr c +// CHECK-NEXT: (C) $4 = { +// CHECK-NEXT: c = 'a' +// CHECK-NEXT: x = 65 +// CHECK-NEXT: = { +// CHECK-NEXT: = { +// CHECK-NEXT: c1 = 'b' +// CHECK-NEXT: = { +// CHECK-NEXT: = { +// CHECK-NEXT: s3 = { +// CHECK-NEXT: x = ([0] = 66, [1] = 67, [2] = 68) +// CHECK-NEXT: } +// CHECK-NEXT: c3 = 'c' +// CHECK-NEXT: } +// CHECK-NEXT: = { +// CHECK-NEXT: c4 = 'B' +// 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: } +// CHECK-NEXT: = { +// CHECK-NEXT: s2 = { +// CHECK-NEXT: x = ([0] = 98, [1] = 66, [2] = 67) +// CHECK-NEXT: } +// CHECK-NEXT: c2 = 'D' +// CHECK-NEXT: } +// CHECK-NEXT: } +// CHECK-NEXT: } +// CHECK-NEXT: (lldb) type lookup C +// CHECK-NEXT: struct C { +// CHECK-NEXT: char c; +// CHECK-NEXT: int x; +// CHECK-NEXT: union { +// CHECK-NEXT: struct { +// CHECK-NEXT: char c1; +// CHECK-NEXT: union { +// CHECK-NEXT: struct { +// CHECK-NEXT: S3 s3; +// CHECK-NEXT: char c3; +// CHECK-NEXT: }; +// CHECK-NEXT: struct { +// CHECK-NEXT: char c4; +// CHECK-NEXT: S3 s4; +// CHECK-NEXT: S3 s1; +// CHECK-NEXT: }; +// CHECK-NEXT: }; +// CHECK-NEXT: }; +// CHECK-NEXT: struct { +// CHECK-NEXT: S3 s2; +// CHECK-NEXT: char c2; +// CHECK-NEXT: }; +// CHECK-NEXT: }; +// CHECK-NEXT: } + + + +// Test packed stuct layout. +struct __attribute__((packed, aligned(1))) A { + char d1; + int d2; + int d3; + char d4; +}; + +struct __attribute__((packed, aligned(1))) B { + char c; + union { + char c[2]; + int i; + } u; +}; + +// Test struct layout with anonymous union/struct. +struct S3 { + short x[3]; +}; + +struct C { + char c; + int x; + union { + struct { + char c1; + union { + struct { + S3 s3; + char c3; + }; + struct { + char c4; + S3 s4; + }; + }; + S3 s1; + }; + struct { + S3 s2; + char c2; + }; + }; +}; + +A a = {'a', 1, 2, 'b'}; +B b = {'a', {"b"}}; +C c = {.c = 'a', .x = 65, .c1 = 'b', .s3 = {66, 67, 68}, .c3 = 'c', .s1={69, 70, 71}}; + +int main() { + return 0; +} Index: lldb/test/Shell/SymbolFile/NativePDB/Inputs/class_layout.lldbinit =================================================================== --- /dev/null +++ lldb/test/Shell/SymbolFile/NativePDB/Inputs/class_layout.lldbinit @@ -0,0 +1,7 @@ +expr a +expr b.c +expr b.u.c +expr b.u.i +expr c +type lookup C +exit Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h =================================================================== --- lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h +++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h @@ -9,13 +9,13 @@ #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_NATIVEPDB_UDTRECORDCOMPLETER_H #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_NATIVEPDB_UDTRECORDCOMPLETER_H +#include "PdbAstBuilder.h" +#include "PdbSymUid.h" #include "Plugins/ExpressionParser/Clang/ClangASTImporter.h" #include "llvm/DebugInfo/CodeView/CVRecord.h" #include "llvm/DebugInfo/CodeView/TypeRecord.h" #include "llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h" -#include "PdbSymUid.h" - namespace clang { class CXXBaseSpecifier; class QualType; @@ -54,6 +54,7 @@ PdbIndex &m_index; std::vector<IndexedBase> m_bases; ClangASTImporter::LayoutInfo m_layout; + llvm::DenseMap<clang::Decl *, DeclStatus> &m_decl_to_status; llvm::DenseMap<lldb::opaque_compiler_type_t, llvm::SmallSet<std::pair<llvm::StringRef, CompilerType>, 8>> &m_cxx_record_map; @@ -62,6 +63,7 @@ UdtRecordCompleter( PdbTypeSymId id, CompilerType &derived_ct, clang::TagDecl &tag_decl, PdbAstBuilder &ast_builder, PdbIndex &index, + llvm::DenseMap<clang::Decl *, DeclStatus> &decl_to_status, llvm::DenseMap<lldb::opaque_compiler_type_t, llvm::SmallSet<std::pair<llvm::StringRef, CompilerType>, 8>> &cxx_record_map); @@ -72,9 +74,57 @@ #define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName) #include "llvm/DebugInfo/CodeView/CodeViewTypes.def" + struct Member; + using MemberUP = std::unique_ptr<Member>; + + struct Member { + enum Kind { Field, Struct, Union } kind; + // Following are only used for field. + llvm::StringRef name; + uint64_t bit_offset; + uint64_t bit_size; + clang::QualType qt; + lldb::AccessType access; + uint32_t bitfield_width; + // Following are Only used for struct or union. + uint64_t base_offset; + llvm::SmallVector<MemberUP, 1> fields; + + Member() = default; + Member(Kind kind) + : 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) + : kind(Field), name(name), bit_offset(bit_offset), bit_size(bit_size), + qt(qt), access(access), bitfield_width(bitfield_width), + 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)); + name = llvm::StringRef(); + qt = clang::QualType(); + access = lldb::eAccessPublic; + bit_offset = bit_size = bitfield_width = 0; + } + }; + + struct Record { + // Top level record. + Member record; + uint64_t start_offset = UINT64_MAX; + 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); + void ConstructRecord(); + }; void complete(); private: + Record m_record; clang::QualType AddBaseClassForTypeIndex( llvm::codeview::TypeIndex ti, llvm::codeview::MemberAccess access, llvm::Optional<uint64_t> vtable_idx = llvm::Optional<uint64_t>()); @@ -82,6 +132,11 @@ llvm::codeview::MemberAccess access, llvm::codeview::MethodOptions options, llvm::codeview::MemberAttributes attrs); + void FinishRecord(); + uint64_t AddMember(TypeSystemClang &clang, Member *field, uint64_t bit_offset, + CompilerType parent_ct, + ClangASTImporter::LayoutInfo &parent_layout, + clang::DeclContext *decl_ctx); }; } // namespace npdb Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp +++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp @@ -6,14 +6,18 @@ #include "PdbUtil.h" #include "Plugins/ExpressionParser/Clang/ClangASTImporter.h" +#include "Plugins/ExpressionParser/Clang/ClangASTMetadata.h" #include "Plugins/ExpressionParser/Clang/ClangUtil.h" #include "Plugins/TypeSystem/Clang/TypeSystemClang.h" +#include "SymbolFileNativePDB.h" +#include "lldb/Core/Address.h" #include "lldb/Symbol/Type.h" #include "lldb/Utility/LLDBAssert.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/lldb-enumerations.h" #include "lldb/lldb-forward.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/DebugInfo/CodeView/SymbolDeserializer.h" #include "llvm/DebugInfo/CodeView/TypeDeserializer.h" #include "llvm/DebugInfo/CodeView/TypeIndex.h" @@ -32,12 +36,13 @@ UdtRecordCompleter::UdtRecordCompleter( PdbTypeSymId id, CompilerType &derived_ct, clang::TagDecl &tag_decl, PdbAstBuilder &ast_builder, PdbIndex &index, + llvm::DenseMap<clang::Decl *, DeclStatus> &decl_to_status, llvm::DenseMap<lldb::opaque_compiler_type_t, llvm::SmallSet<std::pair<llvm::StringRef, CompilerType>, 8>> &cxx_record_map) : m_id(id), m_derived_ct(derived_ct), m_tag_decl(tag_decl), m_ast_builder(ast_builder), m_index(index), - m_cxx_record_map(cxx_record_map) { + m_decl_to_status(decl_to_status), m_cxx_record_map(cxx_record_map) { CVType cvt = m_index.tpi().getType(m_id.index); switch (cvt.kind()) { case LF_ENUM: @@ -46,11 +51,13 @@ case LF_UNION: llvm::cantFail(TypeDeserializer::deserializeAs<UnionRecord>(cvt, m_cvr.ur)); m_layout.bit_size = m_cvr.ur.getSize() * 8; + m_record.record.kind = Member::Union; break; case LF_CLASS: case LF_STRUCTURE: llvm::cantFail(TypeDeserializer::deserializeAs<ClassRecord>(cvt, m_cvr.cr)); m_layout.bit_size = m_cvr.cr.getSize() * 8; + m_record.record.kind = Member::Struct; break; default: llvm_unreachable("unreachable!"); @@ -247,17 +254,13 @@ if (member_qt.isNull()) return Error::success(); m_ast_builder.CompleteType(member_qt); - lldb::AccessType access = TranslateMemberAccess(data_member.getAccess()); - - clang::FieldDecl *decl = TypeSystemClang::AddFieldToRecordType( - m_derived_ct, data_member.Name, m_ast_builder.ToCompilerType(member_qt), - access, bitfield_width); - // FIXME: Add a PdbSymUid namespace for field list members and update - // the m_uid_to_decl map with this decl. - - m_layout.field_offsets.insert(std::make_pair(decl, offset)); - + size_t field_size = + 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); return Error::success(); } @@ -310,6 +313,7 @@ clang.TransferBaseClasses(m_derived_ct.GetOpaqueQualType(), std::move(bases)); clang.AddMethodOverridesForCXXRecordType(m_derived_ct.GetOpaqueQualType()); + FinishRecord(); TypeSystemClang::BuildIndirectFields(m_derived_ct); TypeSystemClang::CompleteTagDeclarationDefinition(m_derived_ct); @@ -317,3 +321,164 @@ m_ast_builder.GetClangASTImporter().SetRecordLayout(record_decl, m_layout); } } + +uint64_t +UdtRecordCompleter::AddMember(TypeSystemClang &clang, Member *field, + uint64_t bit_offset, CompilerType parent_ct, + ClangASTImporter::LayoutInfo &parent_layout, + clang::DeclContext *parent_decl_ctx) { + SymbolFileNativePDB *pdb = static_cast<SymbolFileNativePDB *>( + clang.GetSymbolFile()->GetBackingSymbolFile()); + clang::FieldDecl *field_decl = nullptr; + uint64_t bit_size = 0; + switch (field->kind) { + case Member::Field: { + field_decl = TypeSystemClang::AddFieldToRecordType( + parent_ct, field->name, m_ast_builder.ToCompilerType(field->qt), + field->access, field->bitfield_width); + bit_size = field->bit_size; + break; + }; + case Member::Struct: + case Member::Union: { + clang::TagTypeKind kind = field->kind == Member::Struct + ? clang::TagTypeKind::TTK_Struct + : clang::TagTypeKind::TTK_Union; + ClangASTMetadata metadata; + metadata.SetUserID(pdb->anonymous_id); + metadata.SetIsDynamicCXXType(false); + CompilerType record_ct = clang.CreateRecordType( + parent_decl_ctx, OptionalClangModuleID(), lldb::eAccessPublic, "", kind, + lldb::eLanguageTypeC_plus_plus, &metadata); + TypeSystemClang::StartTagDeclarationDefinition(record_ct); + ClangASTImporter::LayoutInfo layout; + clang::DeclContext *decl_ctx = clang.GetDeclContextForType(record_ct); + for (const auto &member : field->fields) { + uint64_t member_offset = field->kind == Member::Struct + ? member->bit_offset - field->base_offset + : 0; + uint64_t member_bit_size = AddMember(clang, member.get(), member_offset, + record_ct, layout, decl_ctx); + if (field->kind == Member::Struct) + bit_size = std::max(bit_size, member_offset + member_bit_size); + else + bit_size = std::max(bit_size, member_bit_size); + } + layout.bit_size = bit_size; + TypeSystemClang::CompleteTagDeclarationDefinition(record_ct); + clang::RecordDecl *record_decl = clang.GetAsRecordDecl(record_ct); + m_ast_builder.GetClangASTImporter().SetRecordLayout(record_decl, layout); + field_decl = TypeSystemClang::AddFieldToRecordType( + parent_ct, "", record_ct, lldb::eAccessPublic, 0); + // Mark this record decl as completed. + DeclStatus status; + status.resolved = true; + status.uid = pdb->anonymous_id--; + m_decl_to_status.insert({record_decl, status}); + break; + }; + } + // FIXME: Add a PdbSymUid namespace for field list members and update + // the m_uid_to_decl map with this decl. + parent_layout.field_offsets.insert({field_decl, bit_offset}); + return bit_size; +} + +void UdtRecordCompleter::FinishRecord() { + TypeSystemClang &clang = m_ast_builder.clang(); + clang::DeclContext *decl_ctx = + m_ast_builder.GetOrCreateDeclContextForUid(m_id); + m_record.ConstructRecord(); + // Maybe we should check the construsted record size with the size in pdb. If + // they mismatch, it might be pdb has fields info missing. + for (const auto &field : m_record.record.fields) { + AddMember(clang, field.get(), field->bit_offset, m_derived_ct, m_layout, + decl_ctx); + } +} + +void UdtRecordCompleter::Record::CollectMember( + llvm::StringRef name, uint64_t offset, uint64_t field_size, + clang::QualType qt, lldb::AccessType access, uint64_t bitfield_width) { + fields_map[offset].push_back(std::make_unique<Member>( + name, offset, field_size, qt, access, bitfield_width)); + if (start_offset > offset) + start_offset = offset; +} + +void UdtRecordCompleter::Record::ConstructRecord() { + // For anonymous unions in a struct, msvc generated pdb doesn't have the + // entity for that union. So, we need to construct anonymous union and struct + // based on field offsets. The final AST is likely not matching the exact + // original AST, but the memory layout is preseved. + // After we collecting all fields in visitKnownMember, we have all fields in + // increasing offset order in m_fields. Since we are iterating in increase + // offset order, if the current offset is equal to m_start_offset, we insert + // it as direct field of top level record. If the current offset is greater + // than m_start_offset, we should be able to find a field in end_offset_map + // whose end offset is less than or equal to current offset. (if not, it might + // be missing field info. We will ignore the field in this case. e.g. Field A + // starts at 0 with size 4 bytes, and Field B starts at 2 with size 4 bytes. + // Normally, there must be something which ends at/before 2.) Then we will + // append current field to the end of parent record. If parent is struct, we + // can just grow it. If parent is a field, it's a field inside an union. We + // convert it into an anonymous struct containing old field and new field. + + // The end offset to a vector of field/struct that ends at the offset. + std::map<uint64_t, std::vector<Member *>> end_offset_map; + for (auto &pair : fields_map) { + uint64_t offset = pair.first; + auto &fields = pair.second; + lldbassert(offset >= start_offset); + Member *parent = &record; + if (offset > start_offset) { + // Find the field with largest end offset that is <= offset. If it's less + // than offset, it indicates there are padding bytes between end offset + // and offset. + lldbassert(!end_offset_map.empty()); + auto iter = end_offset_map.lower_bound(offset); + if (iter == end_offset_map.end()) + --iter; + else if (iter->first > offset) { + if (iter == end_offset_map.begin()) + continue; + --iter; + } + if (iter->second.empty()) + continue; + 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. + if (parent->kind == Member::Field) + parent->ConvertToStruct(); + + if (fields.size() == 1) { + uint64_t end_offset = offset + fields.back()->bit_size; + parent->fields.push_back(std::move(fields.back())); + if (parent->kind == Member::Struct) { + end_offset_map[end_offset].push_back(parent); + } else { + lldbassert(parent == &record && + "If parent is union, it must be the top level record."); + end_offset_map[end_offset].push_back(parent->fields.back().get()); + } + } else { + if (parent->kind == Member::Struct) { + parent->fields.push_back(std::make_unique<Member>(Member::Union)); + parent = parent->fields.back().get(); + parent->bit_offset = offset; + } else { + lldbassert(parent == &record && + "If parent is union, it must be the top level record."); + } + for (auto &field : fields) { + int64_t bit_size = field->bit_size; + parent->fields.push_back(std::move(field)); + end_offset_map[offset + bit_size].push_back( + parent->fields.back().get()); + } + } + } +} Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h =================================================================== --- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h +++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h @@ -267,6 +267,9 @@ lldb::addr_t m_obj_load_address = 0; bool m_done_full_type_scan = false; + // UID for anonymous union and anonymous struct as they don't have entities in + // pdb debug info. + lldb::user_id_t anonymous_id = LLDB_INVALID_UID - 1; std::unique_ptr<llvm::pdb::PDBFile> m_file_up; std::unique_ptr<PdbIndex> m_index; Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp +++ lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp @@ -1104,6 +1104,11 @@ return GetSizeOfTypeInternal<ClassRecord>(cvt); case LF_UNION: return GetSizeOfTypeInternal<UnionRecord>(cvt); + case LF_BITFIELD: { + BitFieldRecord record; + llvm::cantFail(TypeDeserializer::deserializeAs<BitFieldRecord>(cvt, record)); + return GetSizeOfType({record.Type}, tpi); + } default: break; } Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp +++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp @@ -477,7 +477,7 @@ // Visit all members of this class, then perform any finalization necessary // to complete the class. CompilerType ct = ToCompilerType(tag_qt); - UdtRecordCompleter completer(best_ti, ct, tag, *this, index, + UdtRecordCompleter completer(best_ti, ct, tag, *this, index, m_decl_to_status, m_cxx_record_map); llvm::Error error = llvm::codeview::visitMemberRecordStream(field_list.Data, completer);
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
