Skip to content

Commit a6a547f

Browse files
Michael137Michael137
authored
[lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (#108343)
This bug surfaced after #105865 (currently reverted, but blocked on this to be relanded). Because Clang doesn't emit `DW_TAG_member`s for unnamed bitfields, LLDB has to make an educated guess about whether they existed in the source. It does so by checking whether there is a gap between where the last field ended and the currently parsed field starts. In the example test case, the empty field `padding` was folded into the storage of `data`. Because the `bit_offset` of `padding` is `0x0` and its `DW_AT_byte_size` is `0x1`, LLDB thinks the field ends at `0x1` (not quite because we first round the size to a word size, but this is an implementation detail), erroneously deducing that there's a gap between `flag` and `padding`. This patch adds the notion of "effective field end", which accounts for fields that share storage. It is set to the end of the storage that the two fields occupy. Then we use this to check for gaps in the unnamed bitfield creation logic.
1 parent a16164d commit a6a547f

File tree

3 files changed

+131
-11
lines changed

3 files changed

+131
-11
lines changed

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

+11-4
Original file line numberDiff line numberDiff line change
@@ -2932,14 +2932,21 @@ void DWARFASTParserClang::ParseSingleMember(
29322932
last_field_info = this_field_info;
29332933
last_field_info.SetIsBitfield(true);
29342934
} else {
2935-
last_field_info.bit_offset = field_bit_offset;
2935+
FieldInfo this_field_info{.is_bitfield = false};
2936+
this_field_info.bit_offset = field_bit_offset;
29362937

2938+
// TODO: we shouldn't silently ignore the bit_size if we fail
2939+
// to GetByteSize.
29372940
if (std::optional<uint64_t> clang_type_size =
29382941
member_type->GetByteSize(nullptr)) {
2939-
last_field_info.bit_size = *clang_type_size * character_width;
2942+
this_field_info.bit_size = *clang_type_size * character_width;
29402943
}
29412944

2942-
last_field_info.SetIsBitfield(false);
2945+
if (this_field_info.GetFieldEnd() <= last_field_info.GetEffectiveFieldEnd())
2946+
this_field_info.SetEffectiveFieldEnd(
2947+
last_field_info.GetEffectiveFieldEnd());
2948+
2949+
last_field_info = this_field_info;
29432950
}
29442951

29452952
// Don't turn artificial members such as vtable pointers into real FieldDecls
@@ -3738,7 +3745,7 @@ void DWARFASTParserClang::AddUnnamedBitfieldToRecordTypeIfNeeded(
37383745
const FieldInfo &current_field) {
37393746
// TODO: get this value from target
37403747
const uint64_t word_width = 32;
3741-
uint64_t last_field_end = previous_field.bit_offset + previous_field.bit_size;
3748+
uint64_t last_field_end = previous_field.GetEffectiveFieldEnd();
37423749

37433750
if (!previous_field.IsBitfield()) {
37443751
// The last field was not a bit-field...

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

+31
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,27 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
258258

259259
private:
260260
struct FieldInfo {
261+
/// Size in bits that this field occupies. Can but
262+
/// need not be the DW_AT_bit_size of the field.
261263
uint64_t bit_size = 0;
264+
265+
/// Offset of this field in bits from the beginning
266+
/// of the containing struct. Can but need not
267+
/// be the DW_AT_data_bit_offset of the field.
262268
uint64_t bit_offset = 0;
269+
270+
/// In case this field is folded into the storage
271+
/// of a previous member's storage (for example
272+
/// with [[no_unique_address]]), the effective field
273+
/// end is the offset in bits from the beginning of
274+
/// the containing struct where the field we were
275+
/// folded into ended.
276+
std::optional<uint64_t> effective_field_end;
277+
278+
/// Set to 'true' if this field is a bit-field.
263279
bool is_bitfield = false;
280+
281+
/// Set to 'true' if this field is DW_AT_artificial.
264282
bool is_artificial = false;
265283

266284
FieldInfo() = default;
@@ -276,6 +294,19 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
276294
// bit offset than any previous bitfield + size.
277295
return (bit_size + bit_offset) <= next_bit_offset;
278296
}
297+
298+
/// Returns the offset in bits of where the storage this field
299+
/// occupies ends.
300+
uint64_t GetFieldEnd() const { return bit_size + bit_offset; }
301+
302+
void SetEffectiveFieldEnd(uint64_t val) { effective_field_end = val; }
303+
304+
/// If this field was folded into storage of a previous field,
305+
/// returns the offset in bits of where that storage ends. Otherwise,
306+
/// returns the regular field end (see \ref GetFieldEnd).
307+
uint64_t GetEffectiveFieldEnd() const {
308+
return effective_field_end.value_or(GetFieldEnd());
309+
}
279310
};
280311

281312
/// Parsed form of all attributes that are relevant for parsing type members.

‎lldb/test/Shell/SymbolFile/DWARF/no_unique_address-with-bitfields.cpp

+89-7
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,23 @@
1-
// LLDB currently erroneously adds an unnamed bitfield
2-
// into the AST when an overlapping no_unique_address
3-
// field precedes a bitfield.
4-
51
// RUN: %clang --target=x86_64-apple-macosx -c -gdwarf -o %t %s
62
// RUN: %lldb %t \
73
// RUN: -o "target var global" \
4+
// RUN: -o "target var global2" \
5+
// RUN: -o "target var global3" \
6+
// RUN: -o "target var global4" \
7+
// RUN: -o "target var global5" \
88
// RUN: -o "image dump ast" \
99
// RUN: -o exit | FileCheck %s
1010

1111
// CHECK: (lldb) image dump ast
1212
// CHECK: CXXRecordDecl {{.*}} struct Foo definition
1313
// CHECK: |-FieldDecl {{.*}} data 'char[5]'
1414
// CHECK-NEXT: |-FieldDecl {{.*}} padding 'Empty'
15-
// CHECK-NEXT: |-FieldDecl {{.*}} 'int'
16-
// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 8
17-
// CHECK-NEXT: `-FieldDecl {{.*}} sloc> flag 'unsigned long'
15+
// CHECK-NEXT: `-FieldDecl {{.*}} flag 'unsigned long'
1816
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
1917

2018
struct Empty {};
19+
struct Empty2 {};
20+
struct Empty3 {};
2121

2222
struct Foo {
2323
char data[5];
@@ -26,3 +26,85 @@ struct Foo {
2626
};
2727

2828
Foo global;
29+
30+
// CHECK: CXXRecordDecl {{.*}} struct ConsecutiveOverlap definition
31+
// CHECK: |-FieldDecl {{.*}} data 'char[5]'
32+
// CHECK-NEXT: |-FieldDecl {{.*}} p1 'Empty'
33+
// CHECK-NEXT: |-FieldDecl {{.*}} p2 'Empty2'
34+
// CHECK-NEXT: |-FieldDecl {{.*}} p3 'Empty3'
35+
// CHECK-NEXT: `-FieldDecl {{.*}} flag 'unsigned long'
36+
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
37+
38+
struct ConsecutiveOverlap {
39+
char data[5];
40+
[[no_unique_address]] Empty p1;
41+
[[no_unique_address]] Empty2 p2;
42+
[[no_unique_address]] Empty3 p3;
43+
unsigned long flag : 1;
44+
};
45+
46+
ConsecutiveOverlap global2;
47+
48+
// FIXME: we fail to deduce the unnamed bitfields here.
49+
//
50+
// CHECK: CXXRecordDecl {{.*}} struct MultipleAtOffsetZero definition
51+
// CHECK: |-FieldDecl {{.*}} data 'char[5]'
52+
// CHECK-NEXT: |-FieldDecl {{.*}} p1 'Empty'
53+
// CHECK-NEXT: |-FieldDecl {{.*}} f1 'unsigned long'
54+
// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
55+
// CHECK-NEXT: |-FieldDecl {{.*}} p2 'Empty2'
56+
// CHECK-NEXT: `-FieldDecl {{.*}} f2 'unsigned long'
57+
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
58+
59+
struct MultipleAtOffsetZero {
60+
char data[5];
61+
[[no_unique_address]] Empty p1;
62+
int : 4;
63+
unsigned long f1 : 1;
64+
[[no_unique_address]] Empty2 p2;
65+
int : 4;
66+
unsigned long f2 : 1;
67+
};
68+
69+
MultipleAtOffsetZero global3;
70+
71+
// FIXME: we fail to deduce the unnamed bitfields here.
72+
//
73+
// CHECK: CXXRecordDecl {{.*}} struct MultipleEmpty definition
74+
// CHECK: |-FieldDecl {{.*}} data 'char[5]'
75+
// CHECK-NEXT: |-FieldDecl {{.*}} p1 'Empty'
76+
// CHECK-NEXT: |-FieldDecl {{.*}} f1 'unsigned long'
77+
// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
78+
// CHECK-NEXT: |-FieldDecl {{.*}} p2 'Empty'
79+
// CHECK-NEXT: `-FieldDecl {{.*}} f2 'unsigned long'
80+
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
81+
82+
struct MultipleEmpty {
83+
char data[5];
84+
[[no_unique_address]] Empty p1;
85+
int : 4;
86+
unsigned long f1 : 1;
87+
[[no_unique_address]] Empty p2;
88+
int : 4;
89+
unsigned long f2 : 1;
90+
};
91+
92+
MultipleEmpty global4;
93+
94+
// CHECK: CXXRecordDecl {{.*}} struct FieldBitfieldOverlap definition
95+
// CHECK: |-FieldDecl {{.*}} a 'int'
96+
// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 3
97+
// CHECK-NEXT: |-FieldDecl {{.*}} p1 'Empty'
98+
// CHECK-NEXT: |-FieldDecl {{.*}} b 'int'
99+
// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 6
100+
// CHECK-NEXT: `-FieldDecl {{.*}} c 'int'
101+
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
102+
103+
struct FieldBitfieldOverlap {
104+
int a : 3;
105+
[[no_unique_address]] Empty p1;
106+
int b : 6;
107+
int c : 1;
108+
};
109+
110+
FieldBitfieldOverlap global5;

0 commit comments

Comments
 (0)