Skip to content

Commit d3207b7

Browse files
committed
[lldb][DWARFASTParserClang] Don't create unnamed bitfields to account for vtable pointer
**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 Differential Revision: https://reviews.llvm.org/D150591 (cherry picked from commit 3c30f22)
1 parent 4c5a02c commit d3207b7

File tree

4 files changed

+40
-8
lines changed

4 files changed

+40
-8
lines changed

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

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2894,8 +2894,10 @@ void DWARFASTParserClang::ParseSingleMember(
28942894
// artificial member with (unnamed bitfield) padding.
28952895
// FIXME: This check should verify that this is indeed an artificial member
28962896
// we are supposed to ignore.
2897-
if (attrs.is_artificial)
2897+
if (attrs.is_artificial) {
2898+
last_field_info.SetIsArtificial(true);
28982899
return;
2900+
}
28992901

29002902
if (!member_clang_type.IsCompleteType())
29012903
member_clang_type.GetCompleteType();
@@ -3658,17 +3660,23 @@ bool DWARFASTParserClang::ShouldCreateUnnamedBitfield(
36583660
return false;
36593661

36603662
// If we have a base class, we assume there is no unnamed
3661-
// bit-field if this is the first field since the gap can be
3662-
// attributed to the members from the base class. This assumption
3663-
// is not correct if the first field of the derived class is
3664-
// indeed an unnamed bit-field. We currently do not have the
3665-
// machinary to track the offset of the last field of classes we
3666-
// have seen before, so we are not handling this case.
3663+
// bit-field if either of the following is true:
3664+
// (a) this is the first field since the gap can be
3665+
// attributed to the members from the base class.
3666+
// FIXME: This assumption is not correct if the first field of
3667+
// the derived class is indeed an unnamed bit-field. We currently
3668+
// do not have the machinary to track the offset of the last field
3669+
// of classes we have seen before, so we are not handling this case.
3670+
// (b) Or, the first member of the derived class was a vtable pointer.
3671+
// In this case we don't want to create an unnamed bitfield either
3672+
// since those will be inserted by clang later.
36673673
const bool have_base = layout_info.base_offsets.size() != 0;
36683674
const bool this_is_first_field =
36693675
last_field_info.bit_offset == 0 && last_field_info.bit_size == 0;
3676+
const bool first_field_is_vptr =
3677+
last_field_info.bit_offset == 0 && last_field_info.IsArtificial();
36703678

3671-
if (have_base && this_is_first_field)
3679+
if (have_base && (this_is_first_field || first_field_is_vptr))
36723680
return false;
36733681

36743682
return true;

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,16 @@ class DWARFASTParserClang : public DWARFASTParser {
206206
uint64_t bit_size = 0;
207207
uint64_t bit_offset = 0;
208208
bool is_bitfield = false;
209+
bool is_artificial = false;
209210

210211
FieldInfo() = default;
211212

212213
void SetIsBitfield(bool flag) { is_bitfield = flag; }
213214
bool IsBitfield() { return is_bitfield; }
214215

216+
void SetIsArtificial(bool flag) { is_artificial = flag; }
217+
bool IsArtificial() const { return is_artificial; }
218+
215219
bool NextBitfieldOffsetIsValid(const uint64_t next_bit_offset) const {
216220
// Any subsequent bitfields must not overlap and must be at a higher
217221
// bit offset than any previous bitfield + size.

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,3 +168,14 @@ def test_bitfield_behind_vtable_ptr(self):
168168
result_children=with_vtable_and_unnamed_children)
169169
self.expect_var_path("with_vtable_and_unnamed",
170170
children=with_vtable_and_unnamed_children)
171+
172+
derived_with_vtable_children = [
173+
ValueCheck(name="Base", children=[
174+
ValueCheck(name="b_a", value="2", type="uint32_t")
175+
]),
176+
ValueCheck(name="a", value="1", type="unsigned int:1")
177+
]
178+
self.expect_expr("derived_with_vtable",
179+
result_children=derived_with_vtable_children)
180+
self.expect_var_path("derived_with_vtable",
181+
children=derived_with_vtable_children)

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ struct HasBaseWithVTable : BaseWithVTable {
113113
};
114114
HasBaseWithVTable base_with_vtable;
115115

116+
struct DerivedWithVTable : public Base {
117+
virtual ~DerivedWithVTable() {}
118+
unsigned a : 1;
119+
};
120+
DerivedWithVTable derived_with_vtable;
121+
116122
int main(int argc, char const *argv[]) {
117123
lba.a = 2;
118124

@@ -153,5 +159,8 @@ int main(int argc, char const *argv[]) {
153159
base_with_vtable.b = 0;
154160
base_with_vtable.c = 5;
155161

162+
derived_with_vtable.b_a = 2;
163+
derived_with_vtable.a = 1;
164+
156165
return 0; // break here
157166
}

0 commit comments

Comments
 (0)