zequanwu created this revision.
zequanwu added reviewers: labath, rnk.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

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 
<https://github.com/llvm/llvm-project/issues/57999>,
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.


Repository:
  rG LLVM Github Monorepo

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

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,202 @@
+// 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 d
+// CHECK-NEXT: (D) $4 = {
+// CHECK-NEXT:    = (c = 'a', s = 97, i = 97)
+// CHECK-NEXT:    = {
+// CHECK-NEXT:     c2 = 'b'
+// CHECK-NEXT:     a = (i = 98)
+// CHECK-NEXT:   }
+// CHECK-NEXT:    = (c3 = 'c', i2 = 99, s2 = 99)
+// CHECK-NEXT:   s1 = (x = 10)
+// CHECK-NEXT: }
+// CHECK-NEXT: (lldb) target module dump ast
+// CHECK-NEXT: Dumping clang ast for 1 modules.
+// CHECK-NEXT: TranslationUnitDecl 
+// CHECK-NEXT: |-CXXRecordDecl {{.*}} struct A definition
+// CHECK-NEXT: | |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
+// CHECK-NEXT: | | |-DefaultConstructor exists trivial needs_implicit
+// CHECK-NEXT: | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | |-MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT: | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | |-MoveAssignment exists simple trivial needs_implicit
+// CHECK-NEXT: | | `-Destructor simple irrelevant trivial needs_implicit
+// CHECK-NEXT: | |-FieldDecl {{.*}} d1 'char'
+// CHECK-NEXT: | |-FieldDecl {{.*}} d2 'int'
+// CHECK-NEXT: | |-FieldDecl {{.*}} d3 'int'
+// CHECK-NEXT: | `-FieldDecl {{.*}} d4 'char'
+// CHECK-NEXT: |-VarDecl {{.*}} a 'A'
+// CHECK-NEXT: |-CXXRecordDecl {{.*}} struct B definition
+// CHECK-NEXT: | |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
+// CHECK-NEXT: | | |-DefaultConstructor exists trivial needs_implicit
+// CHECK-NEXT: | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | |-MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT: | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | |-MoveAssignment exists simple trivial needs_implicit
+// CHECK-NEXT: | | `-Destructor simple irrelevant trivial needs_implicit
+// CHECK-NEXT: | |-FieldDecl {{.*}} c 'char'
+// CHECK-NEXT: | |-CXXRecordDecl {{.*}} union <unnamed-type-u> definition
+// CHECK-NEXT: | | |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal has_variant_members
+// CHECK-NEXT: | | | |-DefaultConstructor exists trivial needs_implicit
+// CHECK-NEXT: | | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT: | | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveAssignment exists simple trivial needs_implicit
+// CHECK-NEXT: | | | `-Destructor simple irrelevant trivial needs_implicit
+// CHECK-NEXT: | | |-FieldDecl {{.*}} c 'char[2]'
+// CHECK-NEXT: | | `-FieldDecl {{.*}} i 'int'
+// CHECK-NEXT: | `-FieldDecl {{.*}} u 'B::<unnamed-type-u>'
+// CHECK-NEXT: |-VarDecl {{.*}} b 'B'
+// CHECK-NEXT: |-CXXRecordDecl {{.*}} struct D definition
+// CHECK-NEXT: | |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal has_variant_members
+// CHECK-NEXT: | | |-DefaultConstructor exists trivial needs_implicit
+// CHECK-NEXT: | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | |-MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT: | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | |-MoveAssignment exists simple trivial needs_implicit
+// CHECK-NEXT: | | `-Destructor simple irrelevant trivial needs_implicit
+// CHECK-NEXT: | |-CXXRecordDecl {{.*}} union definition
+// CHECK-NEXT: | | |-DefinitionData is_anonymous pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal has_variant_members
+// CHECK-NEXT: | | | |-DefaultConstructor exists trivial needs_implicit
+// CHECK-NEXT: | | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT: | | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveAssignment exists simple trivial needs_implicit
+// CHECK-NEXT: | | | `-Destructor simple irrelevant trivial needs_implicit
+// CHECK-NEXT: | | |-FieldDecl {{.*}} c 'char'
+// CHECK-NEXT: | | |-FieldDecl {{.*}} s 'short'
+// CHECK-NEXT: | | `-FieldDecl {{.*}} i 'int'
+// CHECK-NEXT: | |-FieldDecl {{.*}} implicit 'D::(anonymous union)'
+// CHECK-NEXT: | |-CXXRecordDecl {{.*}} union definition
+// CHECK-NEXT: | | |-DefinitionData is_anonymous pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal has_variant_members
+// CHECK-NEXT: | | | |-DefaultConstructor exists trivial needs_implicit
+// CHECK-NEXT: | | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT: | | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveAssignment exists simple trivial needs_implicit
+// CHECK-NEXT: | | | `-Destructor simple irrelevant trivial needs_implicit
+// CHECK-NEXT: | | |-FieldDecl {{.*}} c2 'char'
+// CHECK-NEXT: | | `-FieldDecl {{.*}} a 'C'
+// CHECK-NEXT: | |-FieldDecl {{.*}} implicit 'D::(anonymous union)'
+// CHECK-NEXT: | |-CXXRecordDecl {{.*}} union definition
+// CHECK-NEXT: | | |-DefinitionData is_anonymous pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal has_variant_members
+// CHECK-NEXT: | | | |-DefaultConstructor exists trivial needs_implicit
+// CHECK-NEXT: | | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT: | | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveAssignment exists simple trivial needs_implicit
+// CHECK-NEXT: | | | `-Destructor simple irrelevant trivial needs_implicit
+// CHECK-NEXT: | | |-FieldDecl {{.*}} c3 'char'
+// CHECK-NEXT: | | |-FieldDecl {{.*}} i2 'int'
+// CHECK-NEXT: | | `-FieldDecl {{.*}} s2 'short'
+// CHECK-NEXT: | |-FieldDecl {{.*}} implicit 'D::(anonymous union)'
+// CHECK-NEXT: | |-CXXRecordDecl {{.*}} struct <unnamed-type-s1> definition
+// CHECK-NEXT: | | |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
+// CHECK-NEXT: | | | |-DefaultConstructor exists trivial needs_implicit
+// CHECK-NEXT: | | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT: | | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | | |-MoveAssignment exists simple trivial needs_implicit
+// CHECK-NEXT: | | | `-Destructor simple irrelevant trivial needs_implicit
+// CHECK-NEXT: | | `-FieldDecl {{.*}} x 'int'
+// CHECK-NEXT: | |-FieldDecl {{.*}} s1 'D::<unnamed-type-s1>'
+// CHECK-NEXT: | |-IndirectFieldDecl {{.*}} implicit c 'char'
+// CHECK-NEXT: | | |-Field {{.*}} '' 'D::(anonymous union)'
+// CHECK-NEXT: | | `-Field {{.*}} 'c' 'char'
+// CHECK-NEXT: | |-IndirectFieldDecl {{.*}} implicit s 'short'
+// CHECK-NEXT: | | |-Field {{.*}} '' 'D::(anonymous union)'
+// CHECK-NEXT: | | `-Field {{.*}} 's' 'short'
+// CHECK-NEXT: | |-IndirectFieldDecl {{.*}} implicit i 'int'
+// CHECK-NEXT: | | |-Field {{.*}} '' 'D::(anonymous union)'
+// CHECK-NEXT: | | `-Field {{.*}} 'i' 'int'
+// CHECK-NEXT: | |-IndirectFieldDecl {{.*}} implicit c2 'char'
+// CHECK-NEXT: | | |-Field {{.*}} '' 'D::(anonymous union)'
+// CHECK-NEXT: | | `-Field {{.*}} 'c2' 'char'
+// CHECK-NEXT: | |-IndirectFieldDecl {{.*}} implicit a 'C'
+// CHECK-NEXT: | | |-Field {{.*}} '' 'D::(anonymous union)'
+// CHECK-NEXT: | | `-Field {{.*}} 'a' 'C'
+// CHECK-NEXT: | |-IndirectFieldDecl {{.*}} implicit c3 'char'
+// CHECK-NEXT: | | |-Field {{.*}} '' 'D::(anonymous union)'
+// CHECK-NEXT: | | `-Field {{.*}} 'c3' 'char'
+// CHECK-NEXT: | |-IndirectFieldDecl {{.*}} implicit i2 'int'
+// CHECK-NEXT: | | |-Field {{.*}} '' 'D::(anonymous union)'
+// CHECK-NEXT: | | `-Field {{.*}} 'i2' 'int'
+// CHECK-NEXT: | `-IndirectFieldDecl {{.*}} implicit s2 'short'
+// CHECK-NEXT: |   |-Field {{.*}} '' 'D::(anonymous union)'
+// CHECK-NEXT: |   `-Field {{.*}} 's2' 'short'
+// CHECK-NEXT: |-VarDecl {{.*}} d 'D'
+// CHECK-NEXT: `-CXXRecordDecl {{.*}} struct C definition
+// CHECK-NEXT:   |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
+// CHECK-NEXT:   | |-DefaultConstructor exists trivial needs_implicit
+// CHECK-NEXT:   | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT:   | |-MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT:   | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT:   | |-MoveAssignment exists simple trivial needs_implicit
+// CHECK-NEXT:   | `-Destructor simple irrelevant trivial needs_implicit
+// CHECK-NEXT:   `-FieldDecl {{.*}} i 'int'
+
+// 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 with anonymous union layout.
+struct C {
+  int i;
+};
+
+struct D {
+  union {
+    char c;
+    short s;
+    int i;
+  };
+  union {
+    char c2;
+    C a;
+  };
+  union {
+    union {
+      char c3;
+      int i2;
+    };
+    short s2;
+  };
+  struct {
+    int x;
+  } s1;
+};
+
+A a = {'a', 1, 2, 'b'};
+B b = {'a', {"b"}};
+D d = {.c = 'a', .a = {98}, .c3 = 'c', .s1 = { 10 }};
+
+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 d
+target module dump ast
+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;
@@ -47,6 +47,13 @@
     llvm::codeview::EnumRecord er;
   } m_cvr;
 
+  struct Field {
+    llvm::StringRef name;
+    PdbTypeSymId ti;
+    lldb::AccessType access;
+    uint32_t bitfield_width;
+  };
+
   PdbTypeSymId m_id;
   CompilerType &m_derived_ct;
   clang::TagDecl &m_tag_decl;
@@ -54,14 +61,18 @@
   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;
+  std::map<uint64_t, llvm::SmallVector<Field, 1>> m_fields;
+  bool m_is_struct;
 
 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);
@@ -82,6 +93,7 @@
                  llvm::codeview::MemberAccess access,
                  llvm::codeview::MethodOptions options,
                  llvm::codeview::MemberAttributes attrs);
+  void FinishFields();
 };
 
 } // 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,6 +6,7 @@
 #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 "lldb/Symbol/Type.h"
@@ -32,12 +33,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), m_is_struct(false) {
   CVType cvt = m_index.tpi().getType(m_id.index);
   switch (cvt.kind()) {
   case LF_ENUM:
@@ -51,6 +53,7 @@
   case LF_STRUCTURE:
     llvm::cantFail(TypeDeserializer::deserializeAs<ClassRecord>(cvt, m_cvr.cr));
     m_layout.bit_size = m_cvr.cr.getSize() * 8;
+    m_is_struct = true;
     break;
   default:
     llvm_unreachable("unreachable!");
@@ -242,22 +245,9 @@
       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));
-
+  m_fields[offset].push_back({data_member.Name, ti, access, bitfield_width});
   return Error::success();
 }
 
@@ -310,6 +300,7 @@
   clang.TransferBaseClasses(m_derived_ct.GetOpaqueQualType(), std::move(bases));
 
   clang.AddMethodOverridesForCXXRecordType(m_derived_ct.GetOpaqueQualType());
+  FinishFields();
   TypeSystemClang::BuildIndirectFields(m_derived_ct);
   TypeSystemClang::CompleteTagDeclarationDefinition(m_derived_ct);
 
@@ -317,3 +308,70 @@
     m_ast_builder.GetClangASTImporter().SetRecordLayout(record_decl, m_layout);
   }
 }
+
+void UdtRecordCompleter::FinishFields() {
+  TypeSystemClang &clang = m_ast_builder.clang();
+  clang::DeclContext *decl_ctx =
+      m_ast_builder.GetOrCreateDeclContextForUid(m_id);
+
+  // For anonymous unions in a struct, msvc generated pdb doesn't have the
+  // entity for that union. So, we need to group members with same offsets into
+  // an anonymous union and use the record type index to calculate the uid for
+  // the artificial anonymous union.
+  lldb::user_id_t au_id = LLDB_INVALID_UID - toOpaqueUid(m_id);
+  uint64_t au_field_bitoffset = 0;
+  for (const auto &pair : m_fields) {
+    uint64_t offset = pair.first;
+    const auto &fields = pair.second;
+    auto add_field_to_record = [&](CompilerType record_ct,
+                                   ClangASTImporter::LayoutInfo &record_layout,
+                                   uint64_t field_offset) {
+      for (const auto &field : fields) {
+        clang::QualType member_qt = m_ast_builder.GetOrCreateType(field.ti);
+        if (member_qt.isNull())
+          continue;
+        m_ast_builder.CompleteType(member_qt);
+        clang::FieldDecl *field_decl = TypeSystemClang::AddFieldToRecordType(
+            record_ct, field.name, m_ast_builder.ToCompilerType(member_qt),
+            field.access, field.bitfield_width);
+        size_t field_size = GetSizeOfType(field.ti, m_index.tpi()) * 8;
+        if (record_layout.bit_size < field_size)
+          record_layout.bit_size = field_size;
+        // FIXME: Add a PdbSymUid namespace for field list members and
+        // update the m_uid_to_decl map with this decl.
+        record_layout.field_offsets.insert({field_decl, field_offset});
+      }
+    };
+    if (m_is_struct && fields.size() > 1) {
+      // There are at least two fields in the same offset in the struct,
+      // create a anonymous union for them.
+      ClangASTMetadata metadata;
+      metadata.SetUserID(au_id--);
+      metadata.SetIsDynamicCXXType(false);
+      CompilerType au_ct = clang.CreateRecordType(
+          decl_ctx, OptionalClangModuleID(), fields[0].access, "",
+          clang::TagTypeKind::TTK_Union, lldb::eLanguageTypeC_plus_plus,
+          &metadata);
+      TypeSystemClang::StartTagDeclarationDefinition(au_ct);
+      ClangASTImporter::LayoutInfo au_layout;
+      add_field_to_record(au_ct, au_layout, 0);
+      TypeSystemClang::CompleteTagDeclarationDefinition(au_ct);
+      clang::RecordDecl *au_decl = clang.GetAsRecordDecl(au_ct);
+      m_ast_builder.GetClangASTImporter().SetRecordLayout(au_decl, au_layout);
+
+      clang::FieldDecl *au_field_decl = TypeSystemClang::AddFieldToRecordType(
+          m_derived_ct, "", au_ct, fields[0].access, fields[0].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({au_field_decl, au_field_bitoffset});
+
+      au_field_bitoffset += au_layout.bit_size;
+      DeclStatus status;
+      status.resolved = true;
+      status.uid = au_id;
+      m_decl_to_status.insert({au_decl, status});
+    } else {
+      add_field_to_record(m_derived_ct, m_layout, offset);
+    }
+  }
+}
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

Reply via email to