Fix handling of bit-fields when there is a base class when parsing DWARF

[shafik]

Summary

When parsing DWARF and laying out bit-fields we currently don't take into account whether we have a base class or not. Currently if the first field is a bit-field but the bit offset is due a field we inherit from a base class we currently treat it as an unnamed bit-field and therefore add an extra field.

This fix will not check if we have a base class and assume that this offset is due to members we are inheriting from the base. We are currently seeing asserts during codegen when debugging clang::DiagnosticOptions.

This assumption will fail in the case where the first field in the derived class in an unnamed bit-field. Fixing the first field being an unnamed bit-field looks like it will require a larger change since we will need a way to track or discover the last field offset of the bases(s).

[aprantl]

Couple of minor suggestions inside.

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

2671–2678

do we need to amend the comment to describe the new conditions?

2680–2683

(this_field_info.bit_offset > last_field_end) ?

2681

(last_field_info.bit_offset > 0) ||
(last_field_info.bit_size > 0) ||
!layout_info.base_offsets.size())

Does this get more readable if you sink the ! into the subexpressions?

[shafik]

• Expanded comments
• Adjusted conditionals

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

2671–2678

Good catch, I forgot about that.

2681

I see what your doing but it feels too clever, I would never get what it means coming back to it a couple of months from now.

[aprantl]

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

2674

memembers

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

90

clang-format

[shafik]

Minor fixes

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

if (!is_artificial) {
                 // but if it did take up the entire word then we need to extend
                 // last_field_end so the bit-field does not step into the last
                 // fields padding.
                 if (last_field_end != 0 && ((last_field_end % word_width) != 0))
                   last_field_end += word_width - (last_field_end % word_width);
               }
 
               // If we have a gap between the last_field_end and the current
-              // field we have an unnamed bit-field
+              // field we have an unnamed bit-field.
+              // If we have a base class, we assume there is no unnamed
+              // bit-field if this is the first field since the gap can be
+              // attributed to the members from the base class. This assumption
+              // is not correct if the first field of the derived class is
+              // indeed an unnamed bit-field. We currently do not have the
+              // machinary to track the offset of the last field of classes we
+              // have seen before, so we are not handling this case.
               if (this_field_info.bit_offset != last_field_end &&
-                  !(this_field_info.bit_offset < last_field_end)) {
+                  this_field_info.bit_offset > last_field_end &&
+                  !(last_field_info.bit_offset == 0 &&
+                    last_field_info.bit_size == 0 &&
+                    layout_info.base_offsets.size() != 0)) {
                 unnamed_field_info = FieldInfo{};
                 unnamed_field_info->bit_size =
                     this_field_info.bit_offset - last_field_end;
                 unnamed_field_info->bit_offset = last_field_end;
               }
 
               if (unnamed_field_info) {
                 clang::FieldDecl *unnamed_bitfield_decl =

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

def test_and_run_command(self):
                    '(uint64_t:1) f = 0',
                    '(uint64_t:1) g = 1',
                    '(uint64_t:1) h = 0',
                    '(uint64_t:1) i = 1',
                    '(uint64_t:1) j = 0',
                    '(uint64_t:1) k = 1',
                 ])
 
+        self.expect(
+            "frame variable --show-types derived",
+            VARIABLES_DISPLAYED_CORRECTLY,
+            substrs=[
+                '(uint32_t) b_a = 2',
+                '(uint32_t:1) d_a = 1',
+                ])

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

int main(int argc, char const *argv[]) {
   f.g = 1;
   f.h = 0;
   f.i = 1;
   f.j = 0;
   f.k = 1;
     } 
   } clang_example;
 
+  class B {
+  public:
+    uint32_t b_a;
+  };
+
+  class D : public B {
+  public:
+    uint32_t d_a : 1;
+  } derived;
+
   lba.a = 2;
 
   lbb.a = 1;
   lbb.b = 3;
 
   lbc.a = 1;
   lbc.b = 0;
   lbc.c = 4;
   lbc.d = 1;
   lbc.e = 20;
 
   lbd.arr[0] = 'a';
   lbd.arr[1] = 'b';
   lbd.arr[2] = '\0';
   lbd.a = 5;
 
+  derived.b_a = 2;
+  derived.d_a = 1;
 
   return 0; // Set break point at this line.
 }