[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 08:44:49 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/5] [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/5] 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/5] 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;
>From 567fcce1d0dce3516d3ea308fde78effaecb4f24 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 12 Sep 2024 14:12:41 +0100
Subject: [PATCH 4/5] fixup! clang-format
---
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 9455bc7106c6b4..5b679bd5a613e3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2943,7 +2943,8 @@ void DWARFASTParserClang::ParseSingleMember(
}
if (this_field_info.GetFieldEnd() <= last_field_info.GetEffectiveFieldEnd())
- this_field_info.SetEffectiveFieldEnd(last_field_info.GetEffectiveFieldEnd());
+ this_field_info.SetEffectiveFieldEnd(
+ last_field_info.GetEffectiveFieldEnd());
last_field_info = this_field_info;
}
>From 8416d05beeb299f478bcb799a604fe0b677097b3 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 12 Sep 2024 16:44:25 +0100
Subject: [PATCH 5/5] fixup! add more test-cases
---
.../no_unique_address-with-bitfields.cpp | 71 ++++++++++++++++++-
1 file changed, 69 insertions(+), 2 deletions(-)
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 a95baec6f0f08e..980180e7be9aef 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
@@ -2,6 +2,9 @@
// RUN: %lldb %t \
// RUN: -o "target var global" \
// RUN: -o "target var global2" \
+// RUN: -o "target var global3" \
+// RUN: -o "target var global4" \
+// RUN: -o "target var global5" \
// RUN: -o "image dump ast" \
// RUN: -o exit | FileCheck %s
@@ -9,7 +12,7 @@
// CHECK: CXXRecordDecl {{.*}} struct Foo definition
// CHECK: |-FieldDecl {{.*}} data 'char[5]'
// CHECK-NEXT: |-FieldDecl {{.*}} padding 'Empty'
-// CHECK-NEXT: `-FieldDecl {{.*}} sloc> flag 'unsigned long'
+// CHECK-NEXT: `-FieldDecl {{.*}} flag 'unsigned long'
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
struct Empty {};
@@ -29,7 +32,7 @@ Foo global;
// CHECK-NEXT: |-FieldDecl {{.*}} p1 'Empty'
// CHECK-NEXT: |-FieldDecl {{.*}} p2 'Empty2'
// CHECK-NEXT: |-FieldDecl {{.*}} p3 'Empty3'
-// CHECK-NEXT: `-FieldDecl {{.*}} sloc> flag 'unsigned long'
+// CHECK-NEXT: `-FieldDecl {{.*}} flag 'unsigned long'
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
struct ConsecutiveOverlap {
@@ -41,3 +44,67 @@ struct ConsecutiveOverlap {
};
ConsecutiveOverlap global2;
+
+// FIXME: we fail to deduce the unnamed bitfields here.
+//
+// CHECK: CXXRecordDecl {{.*}} struct MultipleAtOffsetZero definition
+// CHECK: |-FieldDecl {{.*}} data 'char[5]'
+// CHECK-NEXT: |-FieldDecl {{.*}} p1 'Empty'
+// CHECK-NEXT: |-FieldDecl {{.*}} f1 'unsigned long'
+// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+// CHECK-NEXT: |-FieldDecl {{.*}} p2 'Empty2'
+// CHECK-NEXT: `-FieldDecl {{.*}} f2 'unsigned long'
+// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+
+struct MultipleAtOffsetZero {
+ char data[5];
+ [[no_unique_address]] Empty p1;
+ int : 4;
+ unsigned long f1 : 1;
+ [[no_unique_address]] Empty2 p2;
+ int : 4;
+ unsigned long f2 : 1;
+};
+
+MultipleAtOffsetZero global3;
+
+// FIXME: we fail to deduce the unnamed bitfields here.
+//
+// CHECK: CXXRecordDecl {{.*}} struct MultipleEmpty definition
+// CHECK: |-FieldDecl {{.*}} data 'char[5]'
+// CHECK-NEXT: |-FieldDecl {{.*}} p1 'Empty'
+// CHECK-NEXT: |-FieldDecl {{.*}} f1 'unsigned long'
+// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+// CHECK-NEXT: |-FieldDecl {{.*}} p2 'Empty'
+// CHECK-NEXT: `-FieldDecl {{.*}} f2 'unsigned long'
+// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+
+struct MultipleEmpty {
+ char data[5];
+ [[no_unique_address]] Empty p1;
+ int : 4;
+ unsigned long f1 : 1;
+ [[no_unique_address]] Empty p2;
+ int : 4;
+ unsigned long f2 : 1;
+};
+
+MultipleEmpty global4;
+
+// CHECK: CXXRecordDecl {{.*}} struct FieldBitfieldOverlap definition
+// CHECK: |-FieldDecl {{.*}} a 'int'
+// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 3
+// CHECK-NEXT: |-FieldDecl {{.*}} p1 'Empty'
+// CHECK-NEXT: |-FieldDecl {{.*}} b 'int'
+// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 6
+// CHECK-NEXT: `-FieldDecl {{.*}} c 'int'
+// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+
+struct FieldBitfieldOverlap {
+ int a : 3;
+ [[no_unique_address]] Empty p1;
+ int b : 6;
+ int c : 1;
+};
+
+FieldBitfieldOverlap global5;
More information about the lldb-commits
mailing list