[lldb] Update field offset/sizes when encountering artificial members such as vtable pointers

[teemperor]

Summary

DWARFASTParserClang::ParseSingleMember turns DWARF DIEs that describe
struct/class members into their respective Clang representation (e.g.,
clang::FieldDecl). It also updates a record of where the last field started/ended
so that we can speculatively fill any holes between a field and a bitfield with
unnamed bitfield padding.

Right now we are completely ignoring 'artificial' members when parsing the
DWARF of a struct/class. The only artificial member that seems to be emitted
in practice for C/C++ seems to be the vtable pointer.

By completely skipping both the Clang AST node creation and the updating of
the last-field record, we essentially leave a hole in our layout with the size of
our artificial member. If the next member is a bitfield we then speculatively
fill the hole with an unnamed bitfield. During CodeGen Clang inserts an
artificial vtable pointer into the layout again which now occupies the same
offset as the unnamed bitfield. This later brings down Clang's
CGRecordLowering::insertPadding when it checks that none of the fields
of the generated record layout overlap.

Note that this is not a Clang bug. We explicitly set the offset of our fields
in LLDB and overwrite whatever Clang makes up.

[teemperor]

lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py

165

The test changes are just fixing up the existing tests. It previously failed (it's hard to write XFAIL'ing tests when everything crashes)

[labath]

You get a badge for writing a longer commit message than the patch itself.

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

void DWARFASTParserClang::ParseSingleMember(
   // Get the parent byte size so we can verify any members will fit
   const uint64_t parent_byte_size =
       parent_die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
   const uint64_t parent_bit_size =
       parent_byte_size == UINT64_MAX ? UINT64_MAX : parent_byte_size * 8;
 
   // FIXME: Remove the workarounds below and make this const.
   MemberAttributes attrs(die, parent_die, module_sp);
-  // Skip artificial members such as vtable pointers.
-  // FIXME: This check should verify that this is indeed an artificial member
-  // we are supposed to ignore.
-  if (attrs.is_artificial)
-    return;
 
   const bool class_is_objc_object_or_interface =
       TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type);
 
   // FIXME: Make Clang ignore Objective-C accessibility for expressions
   if (class_is_objc_object_or_interface)
     attrs.accessibility = eAccessNone;
 
if (attrs.bit_size > 0) {
     if (llvm::Optional<uint64_t> clang_type_size =
             member_type->GetByteSize(nullptr)) {
       last_field_info.bit_size = *clang_type_size * character_width;
     }
 
     last_field_info.SetIsBitfield(false);
   }
 
+  // Don't turn artificial members such as vtable pointers into real FieldDecls
+  // in our AST. Clang will re-create those articial members and they would
+  // otherwise just overlap in the layout with the FieldDecls we add here.
+  // This needs to be done after updating FieldInfo which keeps track of where
+  // field start/end so we don't later try to fill the the space of this
+  // artificial member with (unnamed bitfield) padding.
+  // FIXME: This check should verify that this is indeed an artificial member
+  // we are supposed to ignore.
+  if (attrs.is_artificial)
+    return;
+
   if (!member_clang_type.IsCompleteType())
     member_clang_type.GetCompleteType();
 
   {
     // Older versions of clang emit array[0] and array[1] in the
     // same way (<rdar://problem/12566646>). If the current field
     // is at the end of the structure, then there is definitely no
     // room for extra elements and we override the type to

lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py

def test_bitfields(self):
         base_with_vtable_children = [
             ValueCheck(name="a", type="unsigned int:4", value="5"),
             ValueCheck(name="b", type="unsigned int:4", value="0"),
             ValueCheck(name="c", type="unsigned int:4", value="5")
         ]
         self.expect_expr("base_with_vtable", result_children=base_with_vtable_children)
         self.expect_var_path("base_with_vtable", children=base_with_vtable_children)
 
-    # FIXME: These all crash due the vtable ptr.
-    @skipIf
     @no_debug_info_test
     def test_bitfield_behind_vtable_ptr(self):
         self.build()
         lldbutil.run_to_source_breakpoint(self, '// break here',
             lldb.SBFileSpec("main.cpp", False))
 
         # Test a class with a vtable ptr and bitfields.
         with_vtable_children = [
             ValueCheck(name="a", type="unsigned int:4", value="5"),
             ValueCheck(name="b", type="unsigned int:4", value="0"),
             ValueCheck(name="c", type="unsigned int:4", value="5")
         ]
         self.expect_expr("with_vtable", result_children=with_vtable_children)
         self.expect_var_path("with_vtable", children=with_vtable_children)
 
         # Test a class with a vtable ptr and unnamed bitfield directly after.
         with_vtable_and_unnamed_children = [
-            ValueCheck(name="", type="unsigned int:4", value="0"),
+            ValueCheck(name="", type="int:4", value="0"),
             ValueCheck(name="b", type="unsigned int:4", value="0"),
             ValueCheck(name="c", type="unsigned int:4", value="5")
         ]
         self.expect_expr("with_vtable_and_unnamed",
                          result_children=with_vtable_and_unnamed_children)
         self.expect_var_path("with_vtable_and_unnamed",
                          children=with_vtable_and_unnamed_children)

lldb/test/API/lang/cpp/bitfields/main.cpp

struct WithVTable {
   unsigned a : 4;
   unsigned b : 4;
   unsigned c : 4;
 };
 WithVTable with_vtable;
 
 struct WithVTableAndUnnamed {
   virtual ~WithVTableAndUnnamed() {}
-  unsigned a : 4;
+  unsigned : 4;
   unsigned b : 4;
   unsigned c : 4;
 };
 WithVTableAndUnnamed with_vtable_and_unnamed;
 
 struct BaseWithVTable {
   virtual ~BaseWithVTable() {}
 };
int main(int argc, char const *argv[]) {
   bb.b = 0b0;
   bb.c = 0b11;
   bb.d = 0b01;
 
   with_vtable.a = 5;
   with_vtable.b = 0;
   with_vtable.c = 5;
 
-  with_vtable_and_unnamed.a = 5;
   with_vtable_and_unnamed.b = 0;
   with_vtable_and_unnamed.c = 5;
 
   base_with_vtable.a = 5;
   base_with_vtable.b = 0;
   base_with_vtable.c = 5;
 
   return 0; // break here
 }