[LLDB][NativePDB] Fix struct layout when it has anonymous unions.

[zequanwu]

Summary

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 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.

[labath]

The fact that MSVC does not store all of the anonymous union data is unfortunate, though not entirely surprising, given that the goal of debug info never was to offer an exact reconstruction of the source AST.

That, I'm not sure if checking for matching initial offsets is sufficient to create a semblance of a consistent structure definition. What will this code do with structures like this:

struct S3 {
  char x[3];
};
struct A {
  union {
    struct {
      char c1;
      S3 s1;
    };
    struct {
      S3 s2;
      char c2;
    };
  };
} a;

In this case, a.s1 and a.c2 overlap, but they don't share the same starting offset. If the compiler does not provide data for anonymous structs as well, then I think this algorithm will need to be more complicated. I'm not even sure they can be reconstructed correctly, as the anonymous structs and unions can nest indefinitely. Maybe it would be sufficient to represent this as a "union of structs", where each struct gets (greedily) packed with as many members as it can hold, and we create as many structs as we need (we can replace the struct with a simple member if it would hold just one member)?

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

334

How are these opaque ids computed? Will they produce unique IDs if you have two structs like this close together? Might it be better to just make a global (local to a symbol file or something) counter and always increment/decrement that when creating another type ?

[zequanwu]

Updated to handle the nested anonymous union and struct situation.
It's done by:

1. Create the record layout with Field which could be a field member or an anonymous struct or an anonymous union. If it's struct/union, it has fields to refer to its members.
2. Create the AST from the record layout above.

[labath]

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

330

Now multiple symbol files can race when accessing this variable (and this also introduces a strange interaction between two supposedly-independent symbol files). Better make this member variable of the parent symbol file, or something like that.

391–442

This could use a high-level explanation of the algorithm. Like, I know it tries to stuff the fields into structs and unions, but I wasn't able to figure out how it does that, and what are the operating invariants.

The thing I like about this algorithm, is that the most complicated part (the thing I highlighted) is basically just playing with numbers and it is independent of any complex objects and state. If this part is separated out into a separate function, then it would be perfectly suitable for unit testing, and the unit tests could also serve as documentation/examples of the kinds of transformations that the algorithm does.

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h

54

according to https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly, these should be called Field, Struct, Union

63

m_start_offset ?

64

Can we drop the SP part? I think that the owning references (I guess that's this field) could be just plain Field instances (unique_ptr<Field> at worst), and the rest could just be plain pointers and references.

lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp

54–60

I think we should exclude all of these DefinitionData fields from the test expectation, as they are largely irrelevant to the test (and they also obfuscate it very well). Otherwise, people will have to update these whenever a new field gets added.

I don't know whether it contains the thing you wanted to test (as I don't know what that is), but the type lookup C output will contain the general structure of the type, and it will be a lot more readable.

[zequanwu]

Add unittests and address comments.

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

391–442

Moved into a separate function and added unit tests.

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h

64

The Field object is shared between fields and m_fields. And we can't have Field member instance inside Field class.

lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp

54–60

Thanks, I didn't know that before.

[labath]

The test looks great, and the comments have helped. I still have a couple of questions about the algorithm though.

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

431

This shadowing the fields member confused me for quite some time. Could you choose a different name for one of them?

459

Can parent be a union here? Would the algorithm still be correct?

471

IIUC, this code is reached when the parent object is a union. However, the parent is chosen such that it ends before the start of these new fields? Therefore its start address will be before the start of these fields as well. Is it correct to add the fields to the union under these circumstances, or am I misunderstanding something?

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h

64

You can't have a Field member, but you can have a Field*, unique_ptr<Field> and std::vector<Field> members. SmallVector<Field> is also not possible, for reasons that are mostly obvious, but then again, storing a pointer inside a SmallVector negates most of the benefits that one could hope to gain by using it.

My point is that that using a shared pointer makes it harder to understand the relationships between the objects because it obfuscates the ownership aspect. Sometimes that is unavoidable, like when there just isn't a single object that can own some other object (although llvm mostly avoids that by putting the ownership inside some "context" object), but it's not clear to me why that would be the case here.

[zequanwu]

Address comments.

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

459

parent could only be union when the top level record is a union (at this line Member *parent = &record;). That's the only case when we add an union into end_offset_map. In that case, all the fields would start at the same offset and we only iterate the loop for (auto &pair : fields) { once and then we are done.
Otherwise, we only insert {end offset, struct/field} into end_offset_map where field must be within an union.

471

This is a special case to handle the case when top level record is an union. That's the only case we would reach here. This is to avoid adding unnecessary nested union. For example, the unit test TestAnonymousUnionInUnion would become the following if we always add an anonymous union:

union {
  union {
    m1;
    m2;
    m3;
    m4;
  }
}

[labath]

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

459

Are you sure about that? I've created what I think is a counterexample to these statements. Here a top-level union contains a bunch of sequential fields, which is perfectly possible if the only member of that union is an anonymous struct which contains those fields.

I don't think that's what supposed to happen in this case, but I'm open to being convinced otherwise.

(I've also rewritten the test logic so it produces better error messages than "false is not true".)

[zequanwu]

• Handle the case when anonymous struct inside an union.
• Append labath's changes.

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

459

Thanks for writing the counter example and making better error message. I appended your changes in this file.
I updated to handle the case when parent is an union here by creating an anonymous struct to hold it.

[labath]

Looks good. I suggested some changes, which I hope will reduce duplication and better empasize the differences between the various branches in the code. I think I understand the algorithm now, and I believe it is ok, though I can't escape the feeling that this could have been done in a simpler way.

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

459–469

The current version will create a needless nested struct in case of a union with a single member. I think it should be possible to just insert a single field first, and let it be converted to a struct later -- if it is necessary.

471–491

This should be equivalent but shorter, and -- I hope -- more obvious.

[zequanwu]

Address comments

lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

bool PdbAstBuilder::CompleteTagDecl(clang::TagDecl &tag) {
   FieldListRecord field_list;
   if (llvm::Error error = TypeDeserializer::deserializeAs<FieldListRecord>(
           field_list_cvt, field_list))
     llvm::consumeError(std::move(error));
 
   // 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);
   completer.complete();
 
   m_decl_to_status[&tag].resolved = true;
   if (error) {
     llvm::consumeError(std::move(error));

lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp

size_t lldb_private::npdb::GetSizeOfType(PdbTypeSymId id,
   case LF_ARRAY:
     return GetSizeOfTypeInternal<ArrayRecord>(cvt);
   case LF_CLASS:
   case LF_STRUCTURE:
   case LF_INTERFACE:
     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;
   }
   return 0;
 }

lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h

size_t ParseSymbolArrayInScope(
           fn);
 
   void ParseInlineSite(PdbCompilandSymId inline_site_id, Address func_addr);
 
   llvm::BumpPtrAllocator m_allocator;
 
   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;
 
   llvm::DenseMap<lldb::user_id_t, lldb::VariableSP> m_global_vars;
   llvm::DenseMap<lldb::user_id_t, lldb::VariableSP> m_local_variables;
   llvm::DenseMap<lldb::user_id_t, lldb::BlockSP> m_blocks;
   llvm::DenseMap<lldb::user_id_t, lldb::FunctionSP> m_functions;

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h

 //===-- UdtRecordCompleter.h ------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #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;
 class TagDecl;
 } // namespace clang
 
 namespace llvm {
 namespace pdb {
union UdtTagRecord {
     llvm::codeview::ClassRecord cr;
     llvm::codeview::EnumRecord er;
   } m_cvr;
 
   PdbTypeSymId m_id;
   CompilerType &m_derived_ct;
   clang::TagDecl &m_tag_decl;
   PdbAstBuilder &m_ast_builder;
   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;
 
 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);
 
 #define MEMBER_RECORD(EnumName, EnumVal, Name)                                 \
   llvm::Error visitKnownMember(llvm::codeview::CVMemberRecord &CVR,            \
                                llvm::codeview::Name##Record &Record) override;
 #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>());
   void AddMethod(llvm::StringRef name, llvm::codeview::TypeIndex type_idx,
                  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
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_NATIVEPDB_UDTRECORDCOMPLETER_H

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

 #include "UdtRecordCompleter.h"
 
 #include "PdbAstBuilder.h"
 #include "PdbIndex.h"
 #include "PdbSymUid.h"
 #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"
 #include "llvm/DebugInfo/PDB/Native/GlobalsStream.h"
 #include "llvm/DebugInfo/PDB/Native/TpiStream.h"
 #include "llvm/DebugInfo/PDB/PDBTypes.h"
 
 using namespace llvm::codeview;
 using namespace llvm::pdb;
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::npdb;
 
 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:
     llvm::cantFail(TypeDeserializer::deserializeAs<EnumRecord>(cvt, m_cvr.er));
     break;
   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!");
   }
 }
 
 clang::QualType UdtRecordCompleter::AddBaseClassForTypeIndex(
     llvm::codeview::TypeIndex ti, llvm::codeview::MemberAccess access,
if (cvt.kind() == LF_BITFIELD) {
       ti = bfr.Type;
     }
   }
 
   clang::QualType member_qt = m_ast_builder.GetOrCreateType(PdbTypeSymId(ti));
   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();
 }
 
 Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
                                            OneMethodRecord &one_method) {
   AddMethod(one_method.Name, one_method.Type, one_method.getAccess(),
             one_method.getOptions(), one_method.Attrs);
 
void UdtRecordCompleter::complete() {
   bases.reserve(m_bases.size());
   for (auto &ib : m_bases)
     bases.push_back(std::move(ib.second));
 
   TypeSystemClang &clang = m_ast_builder.clang();
   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);
 
   if (auto *record_decl = llvm::dyn_cast<clang::CXXRecordDecl>(&m_tag_decl)) {
     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());
+      }
+    }
+  }
+}

lldb/test/Shell/SymbolFile/NativePDB/Inputs/class_layout.lldbinit

+expr a
+expr b.c
+expr b.u.c
+expr b.u.i
+expr c
+type lookup C
+exit

lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp

+// 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;
+}

lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp

 // CHECK: | |-FieldDecl {{.*}} o 'long long'
 // CHECK: | `-FieldDecl {{.*}} p 'int[5]'
 // CHECK: |-VarDecl {{.*}} ClassNoPaddingInstance 'const ClassNoPadding'
 // CHECK: |-EnumDecl {{.*}} EnumType
 // CHECK: | |-EnumConstantDecl {{.*}} A 'EnumType'
 // 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'
 // CHECK: |-VarDecl {{.*}} UnnamedClassInstance 'const <unnamed-type-UnnamedClassInstance>'
 // CHECK: |-CXXRecordDecl {{.*}} struct Pointers definition
 // CHECK: | |-FieldDecl {{.*}} a 'void *'
 // CHECK: | |-FieldDecl {{.*}} b 'char *'
 // CHECK: | |-FieldDecl {{.*}} c 'bool *'

lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp

-// 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;
-}

lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp

 // CHECK-NEXT:     const unsigned long long *PULL;
 // CHECK-NEXT:     const float *PF;
 // CHECK-NEXT:     const double *PD;
 // CHECK-NEXT:     const double *PLD;
 // CHECK-NEXT: }
 // 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;
 // CHECK-NEXT:     volatile OneMember VolatileMember;
 // CHECK-NEXT:     const volatile OneMember CVMember;
 // CHECK-NEXT:     OneMember *PtrMember;
 // CHECK-NEXT:     OneMember &RefMember;
 // CHECK-NEXT:     OneMember &&RValueRefMember;

lldb/unittests/SymbolFile/NativePDB/CMakeLists.txt

 add_lldb_unittest(SymbolFileNativePDBTests
   PdbFPOProgramToDWARFExpressionTests.cpp
+  UdtRecordCompleterTests.cpp
 
   LINK_LIBS
     lldbCore
     lldbHost
     lldbSymbol
     lldbPluginSymbolFileNativePDB
     lldbUtilityHelpers
   LINK_COMPONENTS
     Support
     DebugInfoPDB
   )

lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp

+//===-- 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));
+}