Page MenuHomePhabricator

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

Authored by shafik on Mar 25 2020, 3:19 PM.

Details

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

Diff Detail

Event Timeline

shafik created this revision.Mar 25 2020, 3:19 PM
shafik added a reviewer: vsk.
aprantl accepted this revision.Mar 25 2020, 4:00 PM

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?

This revision is now accepted and ready to land.Mar 25 2020, 4:00 PM
shafik updated this revision to Diff 252984.Mar 26 2020, 2:48 PM
shafik marked 4 inline comments as done.
  • 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 added inline comments.Mar 26 2020, 6:46 PM
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
2674

memembers

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

clang-format

shafik updated this revision to Diff 253147.Mar 27 2020, 9:36 AM
shafik marked 2 inline comments as done.

Minor fixes

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 27 2020, 11:30 AM