Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
**Summary**
When filling out the LayoutInfo for a structure with the offsets
from DWARF, LLDB fills gaps in the layout by creating unnamed
bitfields and adding them to the AST. If we don't do this correctly
and our layout has overlapping fields, we will hat an assertion
in `clang::CGRecordLowering::lower()`. Specifically, if we have
a derived class with a VTable and a bitfield immediately following
the vtable pointer, we create a layout with overlapping fields.
This is an oversight in some of the previous cleanups done around this
area.
In `D76808`, we prevented LLDB from creating unnamed bitfields if there
was a gap between the last field of a base class and the start of a bitfield
in the derived class.
In `D112697`, we started accounting for the vtable pointer. The intention
there was to make sure the offset bookkeeping accounted for the
existence of a vtable pointer (but we didn't actually want to create
any AST nodes for it). Now that `last_field_info.bit_size` was being
set even for artifical fields, the previous fix `D76808` broke
specifically for cases where the bitfield was the first member of a
derived class with a vtable (this scenario wasn't tested so we didn't
notice it). I.e., we started creating redundant unnamed bitfields for
where the vtable pointer usually sits. This confused the lowering logic
in clang.
This patch adds a condition to `ShouldCreateUnnamedBitfield` which
checks whether the first field in the derived class is a vtable ptr.
**Testing**
- Added API test case
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D150591
Files:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
lldb/test/API/lang/cpp/bitfields/main.cpp
Index: lldb/test/API/lang/cpp/bitfields/main.cpp
===================================================================
--- lldb/test/API/lang/cpp/bitfields/main.cpp
+++ lldb/test/API/lang/cpp/bitfields/main.cpp
@@ -113,6 +113,12 @@
};
HasBaseWithVTable base_with_vtable;
+struct DerivedWithVTable : public Base {
+ virtual ~DerivedWithVTable() {}
+ unsigned a : 1;
+};
+DerivedWithVTable derived_with_vtable;
+
int main(int argc, char const *argv[]) {
lba.a = 2;
@@ -153,5 +159,8 @@
base_with_vtable.b = 0;
base_with_vtable.c = 5;
+ derived_with_vtable.b_a = 2;
+ derived_with_vtable.a = 1;
+
return 0; // break here
}
Index: lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
===================================================================
--- lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
+++ lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
@@ -168,3 +168,14 @@
result_children=with_vtable_and_unnamed_children)
self.expect_var_path("with_vtable_and_unnamed",
children=with_vtable_and_unnamed_children)
+
+ derived_with_vtable_children = [
+ ValueCheck(name="Base", children=[
+ ValueCheck(name="b_a", value="2", type="uint32_t")
+ ]),
+ ValueCheck(name="a", value="1", type="unsigned int:1")
+ ]
+ self.expect_expr("derived_with_vtable",
+ result_children=derived_with_vtable_children)
+ self.expect_var_path("derived_with_vtable",
+ children=derived_with_vtable_children)
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -223,12 +223,16 @@
uint64_t bit_size = 0;
uint64_t bit_offset = 0;
bool is_bitfield = false;
+ bool is_artificial = false;
FieldInfo() = default;
void SetIsBitfield(bool flag) { is_bitfield = flag; }
bool IsBitfield() { return is_bitfield; }
+ void SetIsArtificial(bool flag) { is_artificial = flag; }
+ bool IsArtificial() const { return is_artificial; }
+
bool NextBitfieldOffsetIsValid(const uint64_t next_bit_offset) const {
// Any subsequent bitfields must not overlap and must be at a higher
// bit offset than any previous bitfield + size.
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2935,8 +2935,10 @@
// 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)
+ if (attrs.is_artificial) {
+ last_field_info.SetIsArtificial(true);
return;
+ }
if (!member_clang_type.IsCompleteType())
member_clang_type.GetCompleteType();
@@ -3697,17 +3699,22 @@
return false;
// 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.
+ // bit-field if either of the following is true:
+ // (a) this is the first field since the gap can be
+ // attributed to the members from the base class.
+ // FIXME: 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.
+ // (b) Or, the gap in the layout is due to a vtable pointer
+ // in the beginning of the derived class.
const bool have_base = layout_info.base_offsets.size() != 0;
const bool this_is_first_field =
last_field_info.bit_offset == 0 && last_field_info.bit_size == 0;
+ const bool first_field_is_vptr =
+ last_field_info.bit_offset == 0 && last_field_info.IsArtificial();
- if (have_base && this_is_first_field)
+ if (have_base && (this_is_first_field || first_field_is_vptr))
return false;
return true;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
