[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 09:37:29 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/3] [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 &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;
+
+  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 &current_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/3] 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/3] 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



More information about the lldb-commits mailing list