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 <michaelbuc...@gmail.com> Date: Wed, 11 Sep 2024 12:15:02 +0100 Subject: [PATCH 1/7] [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 <michaelbuc...@gmail.com> Date: Wed, 11 Sep 2024 17:36:59 +0100 Subject: [PATCH 2/7] 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 <michaelbuc...@gmail.com> Date: Wed, 11 Sep 2024 17:37:13 +0100 Subject: [PATCH 3/7] 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 <michaelbuc...@gmail.com> Date: Wed, 11 Sep 2024 18:04:42 +0100 Subject: [PATCH 4/7] 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 <michaelbuc...@gmail.com> Date: Wed, 11 Sep 2024 18:04:57 +0100 Subject: [PATCH 5/7] 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 <michaelbuc...@gmail.com> Date: Wed, 11 Sep 2024 18:05:36 +0100 Subject: [PATCH 6/7] 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); >From c640717a9053b8d65dd3eebefd6c6e18384d242d Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Wed, 11 Sep 2024 23:29:57 +0100 Subject: [PATCH 7/7] fixup! make default access public --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 9239c8f607d90e..5b9de6f1566b05 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -3763,7 +3763,7 @@ void DWARFASTParserClang::AddUnnamedBitfieldToRecordTypeIfNeeded( TypeSystemClang::AddFieldToRecordType( class_clang_type, llvm::StringRef(), m_ast.GetBuiltinTypeForEncodingAndBitSize(eEncodingSint, word_width), - lldb::AccessType::eAccessPrivate, unnamed_bit_size); + lldb::AccessType::eAccessPublic, unnamed_bit_size); class_layout_info.field_offsets.insert( std::make_pair(unnamed_bitfield_decl, unnamed_bit_offset)); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits