Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields #108343

Merged
merged 6 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2932,14 +2932,21 @@ void DWARFASTParserClang::ParseSingleMember(
last_field_info = this_field_info;
last_field_info.SetIsBitfield(true);
} else {
last_field_info.bit_offset = field_bit_offset;
FieldInfo this_field_info{.is_bitfield = false};
this_field_info.bit_offset = field_bit_offset;

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

last_field_info.SetIsBitfield(false);
if (this_field_info.GetFieldEnd() <= last_field_info.GetEffectiveFieldEnd())
this_field_info.SetEffectiveFieldEnd(
last_field_info.GetEffectiveFieldEnd());

last_field_info = this_field_info;
}

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

if (!previous_field.IsBitfield()) {
// The last field was not a bit-field...
Expand Down
31 changes: 31 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,27 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {

private:
struct FieldInfo {
///< Size in bits that this field occupies. Can but
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/// comment
decl d;

or

decl d; ///<comment 

///< need not be the DW_AT_bit_size of the field.
uint64_t bit_size = 0;

///< Offset of this field in bits from the beginning
///< of the containing struct. Can but need not
///< be the DW_AT_data_bit_offset of the field.
uint64_t bit_offset = 0;

///< In case this field is folded into the storage
///< of a previous member's storage (for example
///< with [[no_unique_address]]), the effective field
///< end is the offset in bits from the beginning of
///< the containing struct where the field we were
///< folded into ended.
std::optional<uint64_t> effective_field_end;

///< Set to 'true' if this field is a bit-field.
bool is_bitfield = false;

///< Set to 'true' if this field is DW_AT_artificial.
bool is_artificial = false;

FieldInfo() = default;
Expand All @@ -276,6 +294,19 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
// bit offset than any previous bitfield + size.
return (bit_size + bit_offset) <= next_bit_offset;
}

/// Returns the offset in bits of where the storage this field
/// occupies ends.
uint64_t GetFieldEnd() const { return bit_size + bit_offset; }

void SetEffectiveFieldEnd(uint64_t val) { effective_field_end = val; }

/// If this field was folded into storage of a previous field,
/// returns the offset in bits of where that storage ends. Otherwise,
/// returns the regular field end (see \ref GetFieldEnd).
uint64_t GetEffectiveFieldEnd() const {
return effective_field_end.value_or(GetFieldEnd());
}
};

/// Parsed form of all attributes that are relevant for parsing type members.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,20 @@
// LLDB currently erroneously adds an unnamed bitfield
// into the AST when an overlapping no_unique_address
// field precedes a bitfield.

// RUN: %clang --target=x86_64-apple-macosx -c -gdwarf -o %t %s
// RUN: %lldb %t \
// RUN: -o "target var global" \
// RUN: -o "target var global2" \
// RUN: -o "image dump ast" \
// RUN: -o exit | FileCheck %s

// CHECK: (lldb) image dump ast
// CHECK: CXXRecordDecl {{.*}} struct Foo definition
// CHECK: |-FieldDecl {{.*}} data 'char[5]'
// CHECK-NEXT: |-FieldDecl {{.*}} padding 'Empty'
// CHECK-NEXT: |-FieldDecl {{.*}} 'int'
// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 8
// CHECK-NEXT: `-FieldDecl {{.*}} sloc> flag 'unsigned long'
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1

struct Empty {};
struct Empty2 {};
struct Empty3 {};

struct Foo {
char data[5];
Expand All @@ -26,3 +23,21 @@ struct Foo {
};

Foo global;

// CHECK: CXXRecordDecl {{.*}} struct ConsecutiveOverlap definition
// CHECK: |-FieldDecl {{.*}} data 'char[5]'
// CHECK-NEXT: |-FieldDecl {{.*}} p1 'Empty'
// CHECK-NEXT: |-FieldDecl {{.*}} p2 'Empty2'
// CHECK-NEXT: |-FieldDecl {{.*}} p3 'Empty3'
// CHECK-NEXT: `-FieldDecl {{.*}} sloc> flag 'unsigned long'
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1

struct ConsecutiveOverlap {
char data[5];
[[no_unique_address]] Empty p1;
[[no_unique_address]] Empty2 p2;
[[no_unique_address]] Empty3 p3;
unsigned long flag : 1;
};

ConsecutiveOverlap global2;
Loading