[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Thu Sep 12 06:12:41 PDT 2024
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/108343
>From 51bca430ecb8be09ccacef5e616ea4ed9d85ab30 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 12 Sep 2024 09:07:16 +0100
Subject: [PATCH 1/3] [lldb][DWARFASTParserClang] Prevent unnamed bitfield
creation in the presence of overlapping fields
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 14 ++++++---
.../SymbolFile/DWARF/DWARFASTParserClang.h | 31 +++++++++++++++++++
.../no_unique_address-with-bitfields.cpp | 6 ----
3 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 5b9de6f1566b05..88511e036fd8c2 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2932,14 +2932,20 @@ 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.GetFieldEnd())
+ this_field_info.SetEffectiveFieldEnd(last_field_info.GetFieldEnd());
+
+ last_field_info = this_field_info;
}
// Don't turn artificial members such as vtable pointers into real FieldDecls
@@ -3738,7 +3744,7 @@ void DWARFASTParserClang::AddUnnamedBitfieldToRecordTypeIfNeeded(
const FieldInfo ¤t_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...
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 3809ee912cec6f..4ff464ce26c548 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -258,9 +258,27 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
private:
struct FieldInfo {
+ ///< Size in bits that this field occopies. Can but
+ ///< 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;
@@ -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.
diff --git a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-with-bitfields.cpp b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-with-bitfields.cpp
index 1c9cc36a711b47..b1672a384c9fe4 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-with-bitfields.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-with-bitfields.cpp
@@ -1,7 +1,3 @@
-// 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" \
@@ -12,8 +8,6 @@
// 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
>From bae2a96f87d229812157da587b7086830665b0fa Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 12 Sep 2024 09:22:29 +0100
Subject: [PATCH 2/3] fixup! fix comment typo
---
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 4ff464ce26c548..ac9b2b94fc28f7 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -258,7 +258,7 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
private:
struct FieldInfo {
- ///< Size in bits that this field occopies. Can but
+ ///< Size in bits that this field occupies. Can but
///< need not be the DW_AT_bit_size of the field.
uint64_t bit_size = 0;
>From 5d25eeb8cb993dc85869ace156d4a1505c9faa67 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 12 Sep 2024 14:12:25 +0100
Subject: [PATCH 3/3] fixup! fix case with consecutive overlapping fields
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 4 ++--
.../no_unique_address-with-bitfields.cpp | 21 +++++++++++++++++++
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 88511e036fd8c2..9455bc7106c6b4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2942,8 +2942,8 @@ void DWARFASTParserClang::ParseSingleMember(
this_field_info.bit_size = *clang_type_size * character_width;
}
- if (this_field_info.GetFieldEnd() < last_field_info.GetFieldEnd())
- this_field_info.SetEffectiveFieldEnd(last_field_info.GetFieldEnd());
+ if (this_field_info.GetFieldEnd() <= last_field_info.GetEffectiveFieldEnd())
+ this_field_info.SetEffectiveFieldEnd(last_field_info.GetEffectiveFieldEnd());
last_field_info = this_field_info;
}
diff --git a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-with-bitfields.cpp b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-with-bitfields.cpp
index b1672a384c9fe4..a95baec6f0f08e 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-with-bitfields.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-with-bitfields.cpp
@@ -1,6 +1,7 @@
// 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
@@ -12,6 +13,8 @@
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
struct Empty {};
+struct Empty2 {};
+struct Empty3 {};
struct Foo {
char data[5];
@@ -20,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;
More information about the lldb-commits
mailing list