[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFC] Factor out unnamed bitfield creation into helper (PR #108196)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Wed Sep 11 10:05:49 PDT 2024
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/108196
>From 0484639d79dc79eba39d203a35487a0cc1064caa Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 11 Sep 2024 12:15:02 +0100
Subject: [PATCH 1/6] [lldb][DWARFASTParserClang][NFC] Factor out unnamed
bitfield creation into helper
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 77 ++++++++++---------
.../SymbolFile/DWARF/DWARFASTParserClang.h | 8 +-
2 files changed, 49 insertions(+), 36 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 4d688f9a1358b2..4f651a78f67918 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2858,7 +2858,6 @@ void DWARFASTParserClang::ParseSingleMember(
}
const uint64_t character_width = 8;
- const uint64_t word_width = 32;
CompilerType member_clang_type = member_type->GetLayoutCompilerType();
const auto accessibility = attrs.accessibility == eAccessNone
@@ -2926,40 +2925,10 @@ void DWARFASTParserClang::ParseSingleMember(
detect_unnamed_bitfields =
die.GetCU()->Supports_unnamed_objc_bitfields();
- if (detect_unnamed_bitfields) {
- std::optional<FieldInfo> unnamed_field_info;
- uint64_t last_field_end =
- last_field_info.bit_offset + last_field_info.bit_size;
-
- if (!last_field_info.IsBitfield()) {
- // The last field was not a bit-field...
- // 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);
- }
-
- if (ShouldCreateUnnamedBitfield(last_field_info, last_field_end,
- this_field_info, layout_info)) {
- unnamed_field_info = FieldInfo{};
- unnamed_field_info->bit_size =
- this_field_info.bit_offset - last_field_end;
- unnamed_field_info->bit_offset = last_field_end;
- }
-
- if (unnamed_field_info) {
- clang::FieldDecl *unnamed_bitfield_decl =
- TypeSystemClang::AddFieldToRecordType(
- class_clang_type, llvm::StringRef(),
- m_ast.GetBuiltinTypeForEncodingAndBitSize(eEncodingSint,
- word_width),
- accessibility, unnamed_field_info->bit_size);
-
- layout_info.field_offsets.insert(std::make_pair(
- unnamed_bitfield_decl, unnamed_field_info->bit_offset));
- }
- }
+ if (detect_unnamed_bitfields)
+ AddUnnamedBitfieldToRecordTypeIfNeeded(layout_info, accessibility,
+ class_clang_type, last_field_info,
+ this_field_info);
last_field_info = this_field_info;
last_field_info.SetIsBitfield(true);
@@ -3764,6 +3733,44 @@ bool DWARFASTParserClang::ShouldCreateUnnamedBitfield(
return true;
}
+void DWARFASTParserClang::AddUnnamedBitfieldToRecordTypeIfNeeded(
+ ClangASTImporter::LayoutInfo &class_layout_info,
+ lldb::AccessType accessibility, const CompilerType &class_clang_type,
+ const FieldInfo &previous_field, 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;
+
+ if (!previous_field.IsBitfield()) {
+ // The last field was not a bit-field...
+ // 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);
+ }
+
+ // Nothing to be done.
+ if (!ShouldCreateUnnamedBitfield(previous_field, last_field_end,
+ current_field, class_layout_info))
+ return;
+
+ FieldInfo unnamed_field_info{.bit_size =
+ current_field.bit_offset - last_field_end,
+ .bit_offset = last_field_end,
+ .is_bitfield = false,
+ .is_artificial = false};
+
+ clang::FieldDecl *unnamed_bitfield_decl =
+ TypeSystemClang::AddFieldToRecordType(
+ class_clang_type, llvm::StringRef(),
+ m_ast.GetBuiltinTypeForEncodingAndBitSize(eEncodingSint, word_width),
+ accessibility, unnamed_field_info.bit_size);
+
+ class_layout_info.field_offsets.insert(
+ std::make_pair(unnamed_bitfield_decl, unnamed_field_info.bit_offset));
+}
+
void DWARFASTParserClang::ParseRustVariantPart(
DWARFDIE &die, const DWARFDIE &parent_die,
const CompilerType &class_clang_type,
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 2806fbbeb99d24..732d9eff4b9217 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -266,7 +266,7 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
FieldInfo() = default;
void SetIsBitfield(bool flag) { is_bitfield = flag; }
- bool IsBitfield() { return is_bitfield; }
+ bool IsBitfield() const { return is_bitfield; }
void SetIsArtificial(bool flag) { is_artificial = flag; }
bool IsArtificial() const { return is_artificial; }
@@ -318,6 +318,12 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
FieldInfo const &this_field_info,
lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const;
+ void AddUnnamedBitfieldToRecordTypeIfNeeded(
+ lldb_private::ClangASTImporter::LayoutInfo &class_layout_info,
+ lldb::AccessType accessibility,
+ const lldb_private::CompilerType &class_clang_type,
+ const FieldInfo &previous_field, const FieldInfo ¤t_field);
+
/// Parses a DW_TAG_APPLE_property DIE and appends the parsed data to the
/// list of delayed Objective-C properties.
///
>From 69bd43aec6461d8f97b0eed11d1c7218e899b3ee Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 11 Sep 2024 17:36:59 +0100
Subject: [PATCH 2/6] fixup! add doxygen comment
---
.../SymbolFile/DWARF/DWARFASTParserClang.h | 26 +++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 732d9eff4b9217..9e27405469011f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -318,6 +318,32 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
FieldInfo const &this_field_info,
lldb_private::ClangASTImporter::LayoutInfo const &layout_info) const;
+ /// Tries to detect whether \ref class_clang_type contained an unnamed
+ /// bit-field between \ref previous_field and \ref current_field, and if
+ /// so, adds a clang::FieldDecl representing that bit-field to
+ /// \ref class_clang_type.
+ ///
+ /// This is necessary because Clang (and GCC) doesn't emit a DW_TAG_member
+ /// entry for unnamed bit-fields. So we derive it (with some exceptions),
+ /// by checking whether there is a gap between where the storage of a DW_TAG_member
+ /// ended and the subsequent DW_TAG_member began.
+ ///
+ /// \param[in,out] layout_info Layout information of all decls parsed by the
+ /// current parser. Will contain an entry for
+ /// the unnamed bit-field if this function created
+ /// one.
+ ///
+ /// \param[in] accessibility AccessType of the unnamed bitfield.
+ ///
+ /// \param[in] class_clang_type The RecordType to which the unnamed bit-field
+ /// will be added (if any).
+ ///
+ /// \param[in] previous_field FieldInfo of the previous DW_TAG_member
+ /// we parsed.
+ ///
+ /// \param[in] current_field FieldInfo of the current DW_TAG_member
+ /// being parsed.
+ ///
void AddUnnamedBitfieldToRecordTypeIfNeeded(
lldb_private::ClangASTImporter::LayoutInfo &class_layout_info,
lldb::AccessType accessibility,
>From 465216b341db0ba3513d9a6fe294436fde3bd185 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 11 Sep 2024 17:37:13 +0100
Subject: [PATCH 3/6] fixup! clang-format
---
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 9e27405469011f..81835985c2356f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -325,8 +325,8 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
///
/// This is necessary because Clang (and GCC) doesn't emit a DW_TAG_member
/// entry for unnamed bit-fields. So we derive it (with some exceptions),
- /// by checking whether there is a gap between where the storage of a DW_TAG_member
- /// ended and the subsequent DW_TAG_member began.
+ /// by checking whether there is a gap between where the storage of a
+ /// DW_TAG_member ended and the subsequent DW_TAG_member began.
///
/// \param[in,out] layout_info Layout information of all decls parsed by the
/// current parser. Will contain an entry for
>From 860106f1e6863bdc2e0070fe0629d717ffe06ffa Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 11 Sep 2024 18:04:42 +0100
Subject: [PATCH 4/6] fixup! no need to create a FieldInfo for the unnamed
bitfield; default to eAccessPrivate always
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 22 +++++++++++--------
.../SymbolFile/DWARF/DWARFASTParserClang.h | 3 ---
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 4f651a78f67918..0497725badb151 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2925,8 +2925,13 @@ void DWARFASTParserClang::ParseSingleMember(
detect_unnamed_bitfields =
die.GetCU()->Supports_unnamed_objc_bitfields();
+ // TODO:
+ // for each ParseSingleMember, check if the field start <= last_field_end.
+ // In that case we have an overlap, so we need to adjust the this_field_info
+ // in such a way that the next last_field_end will be that of the field that we
+ // just overlapped with.
if (detect_unnamed_bitfields)
- AddUnnamedBitfieldToRecordTypeIfNeeded(layout_info, accessibility,
+ AddUnnamedBitfieldToRecordTypeIfNeeded(layout_info,
class_clang_type, last_field_info,
this_field_info);
@@ -3735,7 +3740,7 @@ bool DWARFASTParserClang::ShouldCreateUnnamedBitfield(
void DWARFASTParserClang::AddUnnamedBitfieldToRecordTypeIfNeeded(
ClangASTImporter::LayoutInfo &class_layout_info,
- lldb::AccessType accessibility, const CompilerType &class_clang_type,
+ const CompilerType &class_clang_type,
const FieldInfo &previous_field, const FieldInfo ¤t_field) {
// TODO: get this value from target
const uint64_t word_width = 32;
@@ -3755,20 +3760,19 @@ void DWARFASTParserClang::AddUnnamedBitfieldToRecordTypeIfNeeded(
current_field, class_layout_info))
return;
- FieldInfo unnamed_field_info{.bit_size =
- current_field.bit_offset - last_field_end,
- .bit_offset = last_field_end,
- .is_bitfield = false,
- .is_artificial = false};
+ // Place the unnamed bitfield into the gap between the previous field's end
+ // and the current field's start.
+ const uint64_t unnamed_bit_size = current_field.bit_offset - last_field_end;
+ const uint64_t unnamed_bit_offset = last_field_end;
clang::FieldDecl *unnamed_bitfield_decl =
TypeSystemClang::AddFieldToRecordType(
class_clang_type, llvm::StringRef(),
m_ast.GetBuiltinTypeForEncodingAndBitSize(eEncodingSint, word_width),
- accessibility, unnamed_field_info.bit_size);
+ lldb::AccessType::eAccessPrivate, unnamed_bit_size);
class_layout_info.field_offsets.insert(
- std::make_pair(unnamed_bitfield_decl, unnamed_field_info.bit_offset));
+ std::make_pair(unnamed_bitfield_decl, unnamed_bit_offset));
}
void DWARFASTParserClang::ParseRustVariantPart(
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 81835985c2356f..3809ee912cec6f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -333,8 +333,6 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
/// the unnamed bit-field if this function created
/// one.
///
- /// \param[in] accessibility AccessType of the unnamed bitfield.
- ///
/// \param[in] class_clang_type The RecordType to which the unnamed bit-field
/// will be added (if any).
///
@@ -346,7 +344,6 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
///
void AddUnnamedBitfieldToRecordTypeIfNeeded(
lldb_private::ClangASTImporter::LayoutInfo &class_layout_info,
- lldb::AccessType accessibility,
const lldb_private::CompilerType &class_clang_type,
const FieldInfo &previous_field, const FieldInfo ¤t_field);
>From 8d8b0a1ea6c21ab47b2df1d2d826603f8007ed34 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 11 Sep 2024 18:04:57 +0100
Subject: [PATCH 5/6] fixup! clang-format
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 0497725badb151..541dbbe0cb2ead 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2927,13 +2927,12 @@ void DWARFASTParserClang::ParseSingleMember(
// TODO:
// for each ParseSingleMember, check if the field start <= last_field_end.
- // In that case we have an overlap, so we need to adjust the this_field_info
- // in such a way that the next last_field_end will be that of the field that we
- // just overlapped with.
+ // In that case we have an overlap, so we need to adjust the
+ // this_field_info in such a way that the next last_field_end will be that
+ // of the field that we just overlapped with.
if (detect_unnamed_bitfields)
- AddUnnamedBitfieldToRecordTypeIfNeeded(layout_info,
- class_clang_type, last_field_info,
- this_field_info);
+ AddUnnamedBitfieldToRecordTypeIfNeeded(layout_info, class_clang_type,
+ last_field_info, this_field_info);
last_field_info = this_field_info;
last_field_info.SetIsBitfield(true);
@@ -3740,8 +3739,8 @@ bool DWARFASTParserClang::ShouldCreateUnnamedBitfield(
void DWARFASTParserClang::AddUnnamedBitfieldToRecordTypeIfNeeded(
ClangASTImporter::LayoutInfo &class_layout_info,
- const CompilerType &class_clang_type,
- const FieldInfo &previous_field, const FieldInfo ¤t_field) {
+ const CompilerType &class_clang_type, const FieldInfo &previous_field,
+ 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;
>From f80e9d9b30f8abf4f0b0f8c268ed3fe541b80cda Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 11 Sep 2024 18:05:36 +0100
Subject: [PATCH 6/6] fixup! remove TODO
---
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 5 -----
1 file changed, 5 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 541dbbe0cb2ead..9239c8f607d90e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2925,11 +2925,6 @@ void DWARFASTParserClang::ParseSingleMember(
detect_unnamed_bitfields =
die.GetCU()->Supports_unnamed_objc_bitfields();
- // TODO:
- // for each ParseSingleMember, check if the field start <= last_field_end.
- // In that case we have an overlap, so we need to adjust the
- // this_field_info in such a way that the next last_field_end will be that
- // of the field that we just overlapped with.
if (detect_unnamed_bitfields)
AddUnnamedBitfieldToRecordTypeIfNeeded(layout_info, class_clang_type,
last_field_info, this_field_info);
More information about the lldb-commits
mailing list