[LLDB][NativePDB] Fix struct layout when it has anonymous unions.
authorZequan Wu <zequanwu@google.com>
Thu, 29 Sep 2022 02:40:35 +0000 (19:40 -0700)
committerZequan Wu <zequanwu@google.com>
Thu, 13 Oct 2022 19:43:45 +0000 (12:43 -0700)
Previously, lldb mistook fields in anonymous union in a struct as the direct
field of the struct, which causes lldb crashes due to multiple fields sharing
the same offset in a struct. This patch fixes it.

MSVC generated pdb doesn't have the debug info entity representing a anonymous
union in a struct. It looks like the following:
```
struct S {
union {
  char c;
  int i;
};
};

0x1004 | LF_FIELDLIST [size = 40]
         - LF_MEMBER [name = `c`, Type = 0x0070 (char), offset = 0, attrs = public]
         - LF_MEMBER [name = `i`, Type = 0x0074 (int), offset = 0, attrs = public]
0x1005 | LF_STRUCTURE [size = 32] `S`
         unique name: `.?AUS@@`
         vtable: <no type>, base list: <no type>, field list: 0x1004
```
Clang generated pdb is similar, though due to the [[ https://github.com/llvm/llvm-project/issues/57999 | bug ]],
it's not more useful than the debug info above. But that's not very relavent,
lldb should still be able to understand MSVC geneerated pdb.
```
0x1003 | LF_UNION [size = 60] `S::<unnamed-tag>`
         unique name: `.?AT<unnamed-type-$S1>@S@@`
         field list: <no type>
         options: forward ref (= 0x1003) | has unique name | is nested, sizeof 0
0x1004 | LF_FIELDLIST [size = 40]
         - LF_MEMBER [name = `c`, Type = 0x0070 (char), offset = 0, attrs = public]
         - LF_MEMBER [name = `i`, Type = 0x0074 (int), offset = 0, attrs = public]
         - LF_NESTTYPE [name = ``, parent = 0x1003]
0x1005 | LF_STRUCTURE [size = 32] `S`
         unique name: `.?AUS@@`
         vtable: <no type>, base list: <no type>, field list: 0x1004
         options: contains nested class | has unique name, sizeof 4
0x1006 | LF_FIELDLIST [size = 28]
         - LF_MEMBER [name = `c`, Type = 0x0070 (char), offset = 0, attrs = public]
         - LF_MEMBER [name = `i`, Type = 0x0074 (int), offset = 0, attrs = public]
0x1007 | LF_UNION [size = 60] `S::<unnamed-tag>`
         unique name: `.?AT<unnamed-type-$S1>@S@@`
         field list: 0x1006
         options: has unique name | is nested | sealed, sizeof
```
This patch delays the FieldDecl creation when travesing LF_FIELDLIST so we know
if there are multiple fields are in the same offsets and are able to group them
into different anonymous unions based on offsets. Nested anonymous union will
be flatten into one anonymous union, because we simply don't have that info, but
they are equivalent in terms of union layout.

Differential Revision: https://reviews.llvm.org/D134849

12 files changed:
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 [new file with mode: 0644]
lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp [new file with mode: 0644]
lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp
lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp [deleted file]
lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp
lldb/unittests/SymbolFile/NativePDB/CMakeLists.txt
lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp [new file with mode: 0644]

index 2e17aa1..fb27d0a 100644 (file)
@@ -477,7 +477,7 @@ bool PdbAstBuilder::CompleteTagDecl(clang::TagDecl &tag) {
   // 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);
index 695bb85..164bfa8 100644 (file)
@@ -1104,6 +1104,11 @@ size_t lldb_private::npdb::GetSizeOfType(PdbTypeSymId id,
     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 51ed121..7da4ffb 100644 (file)
@@ -267,6 +267,9 @@ private:
 
   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 183ee5a..2a0954a 100644 (file)
@@ -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 @@ using Error = llvm::Error;
 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 @@ UdtRecordCompleter::UdtRecordCompleter(
   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 @@ Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
   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 @@ void UdtRecordCompleter::complete() {
   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 @@ void UdtRecordCompleter::complete() {
     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 9c6b5ed..794305f 100644 (file)
@@ -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 @@ class UdtRecordCompleter : public llvm::codeview::TypeVisitorCallbacks {
   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 @@ public:
   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 @@ public:
 #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 @@ private:
                  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
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/class_layout.lldbinit b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/class_layout.lldbinit
new file mode 100644 (file)
index 0000000..bbce1e8
--- /dev/null
@@ -0,0 +1,7 @@
+expr a
+expr b.c
+expr b.u.c
+expr b.u.i
+expr c
+type lookup C
+exit
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp b/lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp
new file mode 100644 (file)
index 0000000..0941dc7
--- /dev/null
@@ -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 30293a7..8f4aab6 100644 (file)
@@ -311,41 +311,41 @@ constexpr References ReferencesInstance;
 // 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'
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp b/lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
deleted file mode 100644 (file)
index 0389ae2..0000000
+++ /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 2073cec..03cf25d 100644 (file)
@@ -242,6 +242,7 @@ int main(int argc, char **argv) {
 // 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 b82199f..4562c3f 100644 (file)
@@ -1,5 +1,6 @@
 add_lldb_unittest(SymbolFileNativePDBTests
   PdbFPOProgramToDWARFExpressionTests.cpp
+  UdtRecordCompleterTests.cpp
 
   LINK_LIBS
     lldbCore
diff --git a/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp b/lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
new file mode 100644 (file)
index 0000000..03c5c9a
--- /dev/null
@@ -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));
+}