shafik created this revision.
shafik added reviewers: aprantl, teemperor, jingham.
We ran into an assert when debugging clang and performing an expression on a
class derived from `DeclContext`. The assert was indicating we were getting the
offsets wrong for `RecordDeclBitfields`. We were getting both the size and
offset of unnamed bit-field members wrong. We could fix this case with a quick
change but as I extended the test suite to include more combinations we kept
finding more cases that were being handled incorrectly. A fix that handled all
the new cases as well as the cases already covered required a refactor of the
existing technique.
I removed a duplicate of `BitfieldInfo` and renamed `BitfieldInfo` ->
`FieldInfo` since it is being used for both bit-fields and non-bit-fields. I
extended `TestBitfields.py` to cover five more cases we uncovered while fixing
this bug.
https://reviews.llvm.org/D72953
Files:
lldb/packages/Python/lldbsuite/test/lang/c/bitfields/TestBitfields.py
lldb/packages/Python/lldbsuite/test/lang/c/bitfields/main.c
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -170,11 +170,11 @@
lldb::ModuleSP GetModuleForType(const DWARFDIE &die);
private:
- struct BitfieldInfo {
+ struct FieldInfo {
uint64_t bit_size;
uint64_t bit_offset;
- BitfieldInfo()
+ FieldInfo()
: bit_size(LLDB_INVALID_ADDRESS), bit_offset(LLDB_INVALID_ADDRESS) {}
void Clear() {
@@ -194,7 +194,7 @@
// + size.
return (bit_size + bit_offset) <= next_bit_offset;
} else {
- // If the this BitfieldInfo is not valid, then any offset isOK
+ // If the this FieldInfo is not valid, then any offset isOK
return true;
}
}
@@ -208,7 +208,7 @@
lldb::AccessType &default_accessibility,
DelayedPropertyList &delayed_properties,
lldb_private::ClangASTImporter::LayoutInfo &layout_info,
- BitfieldInfo &last_field_info);
+ FieldInfo &last_bitfield_info, FieldInfo &last_field_info);
bool CompleteRecordType(const DWARFDIE &die, lldb_private::Type *type,
lldb_private::CompilerType &clang_type);
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -85,35 +85,6 @@
return false;
}
-struct BitfieldInfo {
- uint64_t bit_size;
- uint64_t bit_offset;
-
- BitfieldInfo()
- : bit_size(LLDB_INVALID_ADDRESS), bit_offset(LLDB_INVALID_ADDRESS) {}
-
- void Clear() {
- bit_size = LLDB_INVALID_ADDRESS;
- bit_offset = LLDB_INVALID_ADDRESS;
- }
-
- bool IsValid() const {
- return (bit_size != LLDB_INVALID_ADDRESS) &&
- (bit_offset != LLDB_INVALID_ADDRESS);
- }
-
- bool NextBitfieldOffsetIsValid(const uint64_t next_bit_offset) const {
- if (IsValid()) {
- // This bitfield info is valid, so any subsequent bitfields must not
- // overlap and must be at a higher bit offset than any previous bitfield
- // + size.
- return (bit_size + bit_offset) <= next_bit_offset;
- } else {
- // If the this BitfieldInfo is not valid, then any offset isOK
- return true;
- }
- }
-};
ClangASTImporter &DWARFASTParserClang::GetClangASTImporter() {
if (!m_clang_ast_importer_up) {
@@ -2419,7 +2390,7 @@
lldb::AccessType &default_accessibility,
DelayedPropertyList &delayed_properties,
lldb_private::ClangASTImporter::LayoutInfo &layout_info,
- BitfieldInfo &last_field_info) {
+ FieldInfo &last_bitfield_info, FieldInfo &last_field_info) {
ModuleSP module_sp = parent_die.GetDWARF()->GetObjectFile()->GetModule();
const dw_tag_t tag = die.Tag();
// Get the parent byte size so we can verify any members will fit
@@ -2453,6 +2424,14 @@
const dw_attr_t attr = attributes.AttributeAtIndex(i);
DWARFFormValue form_value;
if (attributes.ExtractFormValueAtIndex(i, form_value)) {
+ // AT_data_member_location indicates the byte offset of the
+ // word from the base address of the structure.
+ //
+ // AT_bit_offset indicates how many bits into the word
+ // (according to the host endianness) the low-order bit of the
+ // field starts. AT_bit_offset can be negative.
+ //
+ // AT_bit_size indicates the size of the field in bits.
switch (attr) {
case DW_AT_name:
name = form_value.AsCString();
@@ -2603,35 +2582,24 @@
Type *member_type = die.ResolveTypeUID(encoding_form.Reference());
clang::FieldDecl *field_decl = nullptr;
+ const uint64_t character_width = 8;
+ const uint64_t word_width = 32;
if (tag == DW_TAG_member) {
if (member_type) {
+ CompilerType member_clang_type = member_type->GetLayoutCompilerType();
+
if (accessibility == eAccessNone)
accessibility = default_accessibility;
member_accessibilities.push_back(accessibility);
uint64_t field_bit_offset =
(member_byte_offset == UINT32_MAX ? 0 : (member_byte_offset * 8));
- if (bit_size > 0) {
- BitfieldInfo this_field_info;
+ if (bit_size > 0) {
+ FieldInfo this_field_info;
this_field_info.bit_offset = field_bit_offset;
this_field_info.bit_size = bit_size;
- /////////////////////////////////////////////////////////////
- // How to locate a field given the DWARF debug information
- //
- // AT_byte_size indicates the size of the word in which the bit
- // offset must be interpreted.
- //
- // AT_data_member_location indicates the byte offset of the
- // word from the base address of the structure.
- //
- // AT_bit_offset indicates how many bits into the word
- // (according to the host endianness) the low-order bit of the
- // field starts. AT_bit_offset can be negative.
- //
- // AT_bit_size indicates the size of the field in bits.
- /////////////////////////////////////////////////////////////
if (data_bit_offset != UINT64_MAX) {
this_field_info.bit_offset = data_bit_offset;
@@ -2649,7 +2617,7 @@
}
if ((this_field_info.bit_offset >= parent_bit_size) ||
- !last_field_info.NextBitfieldOffsetIsValid(
+ !last_bitfield_info.NextBitfieldOffsetIsValid(
this_field_info.bit_offset)) {
ObjectFile *objfile = die.GetDWARF()->GetObjectFile();
objfile->GetModule()->ReportWarning(
@@ -2666,33 +2634,6 @@
// Update the field bit offset we will report for layout
field_bit_offset = this_field_info.bit_offset;
- // If the member to be emitted did not start on a character
- // boundary and there is empty space between the last field and
- // this one, then we need to emit an anonymous member filling
- // up the space up to its start. There are three cases here:
- //
- // 1 If the previous member ended on a character boundary, then
- // we can emit an
- // anonymous member starting at the most recent character
- // boundary.
- //
- // 2 If the previous member did not end on a character boundary
- // and the distance
- // from the end of the previous member to the current member
- // is less than a
- // word width, then we can emit an anonymous member starting
- // right after the
- // previous member and right before this member.
- //
- // 3 If the previous member did not end on a character boundary
- // and the distance
- // from the end of the previous member to the current member
- // is greater than
- // or equal a word width, then we act as in Case 1.
-
- const uint64_t character_width = 8;
- const uint64_t word_width = 32;
-
// Objective-C has invalid DW_AT_bit_offset values in older
// versions of clang, so we have to be careful and only insert
// unnamed bitfields if we have a new enough clang.
@@ -2704,53 +2645,64 @@
die.GetCU()->Supports_unnamed_objc_bitfields();
if (detect_unnamed_bitfields) {
- BitfieldInfo anon_field_info;
-
- if ((this_field_info.bit_offset % character_width) !=
- 0) // not char aligned
- {
- uint64_t last_field_end = 0;
-
- if (last_field_info.IsValid())
- last_field_end =
- last_field_info.bit_offset + last_field_info.bit_size;
-
- if (this_field_info.bit_offset != last_field_end) {
- if (((last_field_end % character_width) == 0) || // case 1
- (this_field_info.bit_offset - last_field_end >=
- word_width)) // case 3
- {
- anon_field_info.bit_size =
- this_field_info.bit_offset % character_width;
- anon_field_info.bit_offset =
- this_field_info.bit_offset - anon_field_info.bit_size;
- } else // case 2
- {
- anon_field_info.bit_size =
- this_field_info.bit_offset - last_field_end;
- anon_field_info.bit_offset = last_field_end;
- }
- }
+ FieldInfo unnamed_field_info;
+ uint64_t last_field_end = 0;
+
+ // The last field was not a bit-field...
+ if (last_field_info.IsValid()) {
+ last_field_end =
+ last_field_info.bit_offset + last_field_info.bit_size;
+
+ // 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);
+ } else if (last_bitfield_info.IsValid())
+ last_field_end =
+ last_bitfield_info.bit_offset + last_bitfield_info.bit_size;
+
+ // If we have a gap between the last_field_end and the current
+ // field we have an unnamed bit-field
+ if (this_field_info.bit_offset != last_field_end &&
+ !(this_field_info.bit_offset < last_field_end)) {
+ unnamed_field_info.bit_size =
+ this_field_info.bit_offset - last_field_end;
+ unnamed_field_info.bit_offset = last_field_end;
}
- if (anon_field_info.IsValid()) {
+ if (unnamed_field_info.IsValid()) {
+ if (data_bit_offset != UINT64_MAX)
+ field_bit_offset = data_bit_offset;
+ else
+ field_bit_offset = unnamed_field_info.bit_offset +
+ unnamed_field_info.bit_size;
+
clang::FieldDecl *unnamed_bitfield_decl =
ClangASTContext::AddFieldToRecordType(
class_clang_type, llvm::StringRef(),
m_ast.GetBuiltinTypeForEncodingAndBitSize(eEncodingSint,
word_width),
- accessibility, anon_field_info.bit_size);
+ accessibility, unnamed_field_info.bit_size);
layout_info.field_offsets.insert(std::make_pair(
- unnamed_bitfield_decl, anon_field_info.bit_offset));
- }
+ unnamed_bitfield_decl, unnamed_field_info.bit_offset));
+ } else
+ field_bit_offset = this_field_info.bit_offset;
}
- last_field_info = this_field_info;
- } else {
+
last_field_info.Clear();
+ last_bitfield_info = this_field_info;
+ } else {
+ last_bitfield_info.Clear();
+ last_field_info.bit_offset = field_bit_offset;
+
+ if (llvm::Optional<uint64_t> clang_type_size =
+ member_clang_type.GetByteSize(nullptr)) {
+ last_field_info.bit_size = *clang_type_size * character_width;
+ }
}
- CompilerType member_clang_type = member_type->GetLayoutCompilerType();
if (!member_clang_type.IsCompleteType())
member_clang_type.GetCompleteType();
@@ -2835,7 +2787,6 @@
bit_size);
m_ast.SetMetadataAsUserID(field_decl, die.GetID());
-
layout_info.field_offsets.insert(
std::make_pair(field_decl, field_bit_offset));
} else {
@@ -2885,7 +2836,10 @@
if (!parent_die)
return false;
- BitfieldInfo last_field_info;
+ FieldInfo last_bitfield_info;
+ FieldInfo last_field_info;
+ last_field_info.bit_offset = 0;
+ last_field_info.bit_size = 0;
ModuleSP module_sp = parent_die.GetDWARF()->GetObjectFile()->GetModule();
ClangASTContext *ast =
@@ -2902,7 +2856,8 @@
case DW_TAG_APPLE_property:
ParseSingleMember(die, parent_die, class_clang_type, class_language,
member_accessibilities, default_accessibility,
- delayed_properties, layout_info, last_field_info);
+ delayed_properties, layout_info, last_bitfield_info,
+ last_field_info);
break;
case DW_TAG_subprogram:
Index: lldb/packages/Python/lldbsuite/test/lang/c/bitfields/main.c
===================================================================
--- lldb/packages/Python/lldbsuite/test/lang/c/bitfields/main.c
+++ lldb/packages/Python/lldbsuite/test/lang/c/bitfields/main.c
@@ -9,11 +9,9 @@
#include <stdio.h>
#include <string.h>
-int main (int argc, char const *argv[])
-{
- struct Bits
- {
- uint32_t : 1, // Unnamed bitfield
+int main(int argc, char const *argv[]) {
+ struct Bits {
+ uint32_t : 1, // Unnamed bitfield
b1 : 1,
b2 : 2,
: 2, // Unnamed bitfield
@@ -24,80 +22,122 @@
b6 : 6,
b7 : 7,
four : 4;
- };
+ };
- printf("%lu", sizeof(struct Bits));
-
- struct Bits bits;
- int i;
- for (i=0; i<(1<<1); i++)
- bits.b1 = i; //// break $source:$line
- for (i=0; i<(1<<2); i++)
- bits.b2 = i; //// break $source:$line
- for (i=0; i<(1<<3); i++)
- bits.b3 = i; //// break $source:$line
- for (i=0; i<(1<<4); i++)
- bits.b4 = i; //// break $source:$line
- for (i=0; i<(1<<5); i++)
- bits.b5 = i; //// break $source:$line
- for (i=0; i<(1<<6); i++)
- bits.b6 = i; //// break $source:$line
- for (i=0; i<(1<<7); i++)
- bits.b7 = i; //// break $source:$line
- for (i=0; i<(1<<4); i++)
- bits.four = i; //// break $source:$line
-
- struct MoreBits
- {
- uint32_t a : 3;
- uint8_t : 1;
- uint8_t b : 1;
- uint8_t c : 1;
- uint8_t d : 1;
- };
+ printf("%lu", sizeof(struct Bits));
- struct MoreBits more_bits;
+ struct Bits bits;
+ int i;
+ for (i = 0; i < (1 << 1); i++)
+ bits.b1 = i; //// break $source:$line
+ for (i = 0; i < (1 << 2); i++)
+ bits.b2 = i; //// break $source:$line
+ for (i = 0; i < (1 << 3); i++)
+ bits.b3 = i; //// break $source:$line
+ for (i = 0; i < (1 << 4); i++)
+ bits.b4 = i; //// break $source:$line
+ for (i = 0; i < (1 << 5); i++)
+ bits.b5 = i; //// break $source:$line
+ for (i = 0; i < (1 << 6); i++)
+ bits.b6 = i; //// break $source:$line
+ for (i = 0; i < (1 << 7); i++)
+ bits.b7 = i; //// break $source:$line
+ for (i = 0; i < (1 << 4); i++)
+ bits.four = i; //// break $source:$line
- more_bits.a = 3;
- more_bits.b = 0;
- more_bits.c = 1;
- more_bits.d = 0;
+ struct MoreBits {
+ uint32_t a : 3;
+ uint8_t : 1;
+ uint8_t b : 1;
+ uint8_t c : 1;
+ uint8_t d : 1;
+ };
- struct EvenMoreBits
- {
- uint8_t b1 : 1, b2 : 1, b3 : 1, b4 : 1, b5 : 1, b6 : 1,
- b7 : 1, b8 : 1, b9 : 1, b10 : 1, b11 : 1, b12 : 1,
- b13 : 1, b14 : 1, b15 : 1, b16 : 1, b17 : 1;
- };
+ struct MoreBits more_bits;
+
+ more_bits.a = 3;
+ more_bits.b = 0;
+ more_bits.c = 1;
+ more_bits.d = 0;
- struct EvenMoreBits even_more_bits;
- memset(&even_more_bits, 0, sizeof(even_more_bits));
- even_more_bits.b1 = 1;
- even_more_bits.b5 = 1;
- even_more_bits.b7 = 1;
- even_more_bits.b13 = 1;
+ struct EvenMoreBits {
+ uint8_t b1 : 1, b2 : 1, b3 : 1, b4 : 1, b5 : 1, b6 : 1, b7 : 1, b8 : 1,
+ b9 : 1, b10 : 1, b11 : 1, b12 : 1, b13 : 1, b14 : 1, b15 : 1, b16 : 1,
+ b17 : 1;
+ };
+
+ struct EvenMoreBits even_more_bits;
+ memset(&even_more_bits, 0, sizeof(even_more_bits));
+ even_more_bits.b1 = 1;
+ even_more_bits.b5 = 1;
+ even_more_bits.b7 = 1;
+ even_more_bits.b13 = 1;
#pragma pack(1)
- struct PackedBits
- {
- char a;
- uint32_t b : 5,
- c : 27;
+ struct PackedBits {
+ char a;
+ uint32_t b : 5, c : 27;
+ };
+#pragma pack()
+ struct PackedBits packed;
+ packed.a = 'a';
+ packed.b = 10;
+ packed.c = 0x7112233;
+
+ struct LargePackedBits {
+ uint64_t a : 36;
+ uint64_t b : 36;
+ } __attribute__((packed));
+
+ struct LargePackedBits large_packed =
+ (struct LargePackedBits){0xcbbbbaaaa, 0xdffffeeee};
+
+ struct LargerBitsA {
+ unsigned int : 30, a : 20;
+ } lba;
+
+ struct LargerBitsB {
+ unsigned int a : 1, : 11, : 12, b : 20;
+ } lbb;
+
+ struct LargerBitsC {
+ unsigned int : 13, : 9, a : 1, b : 1, c : 5, d : 1, e : 20;
+ } lbc;
+
+ struct LargerBitsD {
+ char arr[3];
+ unsigned int : 30, a : 20;
+ } lbd;
+
+ // This case came up when debugging clang
+ struct BitExampleFromClangDeclContext {
+ struct fields {
+ uint64_t : 13;
+ uint64_t : 9;
+
+ uint64_t HasFlexibleArrayMember : 1;
+ uint64_t AnonymousStructOrUnion : 1;
+ uint64_t HasObjectMember : 1;
+ uint64_t HasVolatileMember : 1;
+ uint64_t LoadedFieldsFromExternalStorage : 1;
+ uint64_t NonTrivialToPrimitiveDefaultInitialize : 1;
+ uint64_t NonTrivialToPrimitiveCopy : 1;
+ uint64_t NonTrivialToPrimitiveDestroy : 1;
+ uint64_t HasNonTrivialToPrimitiveDefaultInitializeCUnion : 1;
+ uint64_t HasNonTrivialToPrimitiveDestructCUnion : 1;
+ uint64_t HasNonTrivialToPrimitiveCopyCUnion : 1;
+ };
+
+ union {
+ struct fields f;
};
-#pragma pack()
- struct PackedBits packed;
- packed.a = 'a';
- packed.b = 10;
- packed.c = 0x7112233;
-
- struct LargePackedBits {
- uint64_t a: 36;
- uint64_t b: 36;
- } __attribute__((packed));
-
- struct LargePackedBits large_packed =
- (struct LargePackedBits){ 0xcbbbbaaaa, 0xdffffeeee };
-
- return 0; //// Set break point at this line.
+ } clang_example;
+
+ lba.a = 2;
+ lbb.b = 3;
+ lbc.c = 4;
+ lbd.a = 5;
+ clang_example.f.HasFlexibleArrayMember = 1;
+ return 0; //// Set break point at this line.
}
Index: lldb/packages/Python/lldbsuite/test/lang/c/bitfields/TestBitfields.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/lang/c/bitfields/TestBitfields.py
+++ lldb/packages/Python/lldbsuite/test/lang/c/bitfields/TestBitfields.py
@@ -147,6 +147,74 @@
self.expect("v/x large_packed", VARIABLES_DISPLAYED_CORRECTLY,
substrs=["a = 0x0000000cbbbbaaaa", "b = 0x0000000dffffeee"])
+ self.expect(
+ "frame variable --show-types lba",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=[
+ '(int:32) = ',
+ '(unsigned int:20) a =',
+ ])
+
+ self.expect(
+ "frame variable --show-types lbb",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=[
+ '(unsigned int:1) a =',
+ '(int:31) =',
+ '(unsigned int:20) b =',
+ ])
+
+ self.expect(
+ "frame variable --show-types lbc",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=[
+ '(int:22) =',
+ '(unsigned int:1) a =',
+ '(unsigned int:1) b =',
+ '(unsigned int:5) c =',
+ '(unsigned int:1) d =',
+ '(int:2) =',
+ '(unsigned int:20) e =',
+ ])
+
+ self.expect(
+ "frame variable --show-types lbd",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=[
+ '(char [3]) arr =',
+ '(int:32) =',
+ '(unsigned int:20) a =',
+ ])
+
+ self.expect(
+ "frame variable --show-types clang_example",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=[
+ '(int:22) =',
+ '(uint64_t:1) HasFlexibleArrayMember =',
+ '(uint64_t:1) AnonymousStructOrUnion =',
+ '(uint64_t:1) HasObjectMember =',
+ '(uint64_t:1) HasVolatileMember =',
+ '(uint64_t:1) LoadedFieldsFromExternalStorage =',
+ '(uint64_t:1) NonTrivialToPrimitiveDefaultInitialize =',
+ '(uint64_t:1) NonTrivialToPrimitiveCopy =',
+ '(uint64_t:1) NonTrivialToPrimitiveDestroy =',
+ '(uint64_t:1) HasNonTrivialToPrimitiveDefaultInitializeCUnion =',
+ '(uint64_t:1) HasNonTrivialToPrimitiveDestructCUnion =',
+ '(uint64_t:1) HasNonTrivialToPrimitiveCopyCUnion =',
+ ])
+
+ self.expect("expr (lba.a)", VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=['unsigned int', '2'])
+ self.expect("expr (lbb.b)", VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=['unsigned int', '3'])
+ self.expect("expr (lbc.c)", VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=['unsigned int', '4'])
+ self.expect("expr (lbd.a)", VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=['unsigned int', '5'])
+ self.expect("expr (clang_example.f.HasFlexibleArrayMember)", VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=['uint64_t', '1'])
+
@add_test_categories(['pyapi'])
# BitFields exhibit crashes in record layout on Windows
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
