[Lldb-commits] [lldb] f0c6d30 - [lldb][DWARFASTParserClang][NFC] Factor out unnamed bitfield creation into helper (#108196)
Author: Michael Buch Date: 2024-09-12T08:11:09+01:00 New Revision: f0c6d30a5d80dd42cb298f857987aa2c8f01e63e URL: https://github.com/llvm/llvm-project/commit/f0c6d30a5d80dd42cb298f857987aa2c8f01e63e DIFF: https://github.com/llvm/llvm-project/commit/f0c6d30a5d80dd42cb298f857987aa2c8f01e63e.diff LOG: [lldb][DWARFASTParserClang][NFC] Factor out unnamed bitfield creation into helper (#108196) This logic will need adjusting soon for https://github.com/llvm/llvm-project/pull/108155 This patch pulls out the logic for detecting/creating unnamed bitfields out of `ParseSingleMember` to make the latter (in my opinion) more readable. Otherwise we have a large number of similarly named variables in scope. Added: Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h Removed: diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 4d688f9a1358b2..5b9de6f1566b05 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,9 @@ void DWARFASTParserClang::ParseSingleMember( detect_unnamed_bitfields = die.GetCU()->Supports_unnamed_objc_bitfields(); -if (detect_unnamed_bitfields) { - std::optional 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, class_clang_type, + last_field_info, this_field_info); last_field_info = this_field_info; last_field_info.SetIsBitfield(true); @@ -3764,6 +3732,43 @@ bool DWARFASTParserClang::ShouldCreateUnnamedBitfield( return true; } +void DWARFASTParserClang::AddUnnamedBitfieldToRecordTypeIfNeeded( +ClangASTImporter::LayoutInfo &class_layout_info, +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; + + // 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.GetBuiltinTypeForEn
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFC] Factor out unnamed bitfield creation into helper (PR #108196)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/108196 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/108343 This bug surfaced after https://github.com/llvm/llvm-project/pull/105865 (currently reverted, but blocked on this to be relanded). Because Clang doesn't emit `DW_TAG_member`s for unnamed bitfields, LLDB has to make an educated guess about whether they existed in the source. It does so by checking whether there is a gap between where the last field began and the currently parsed field starts. In the example test case, the empty field `padding` was folded into the storage of `data`. Because the `bit_offset` of `padding` is `0x0` and its `DW_AT_byte_size` is `0x1`, LLDB thinks the field ends at `0x1` (not quite because we first round the size to a word size, but this is an implementation detail), erroneously deducing that there's a gap between `flag` and `padding`. This patch adds the notion of "effective field end", which accounts for fields that share storage. Its value is the end of the storage that the two fields occupy. Then we use this to check for gaps in the unnamed bitfield creation logic. >From 51bca430ecb8be09ccacef5e616ea4ed9d85ab30 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 12 Sep 2024 09:07:16 +0100 Subject: [PATCH] [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 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 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 +/// occupi
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes This bug surfaced after https://github.com/llvm/llvm-project/pull/105865 (currently reverted, but blocked on this to be relanded). Because Clang doesn't emit `DW_TAG_member`s for unnamed bitfields, LLDB has to make an educated guess about whether they existed in the source. It does so by checking whether there is a gap between where the last field began and the currently parsed field starts. In the example test case, the empty field `padding` was folded into the storage of `data`. Because the `bit_offset` of `padding` is `0x0` and its `DW_AT_byte_size` is `0x1`, LLDB thinks the field ends at `0x1` (not quite because we first round the size to a word size, but this is an implementation detail), erroneously deducing that there's a gap between `flag` and `padding`. This patch adds the notion of "effective field end", which accounts for fields that share storage. Its value is the end of the storage that the two fields occupy. Then we use this to check for gaps in the unnamed bitfield creation logic. --- Full diff: https://github.com/llvm/llvm-project/pull/108343.diff 3 Files Affected: - (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+10-4) - (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h (+31) - (modified) lldb/test/Shell/SymbolFile/DWARF/no_unique_address-with-bitfields.cpp (-6) ``diff 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 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 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
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
Michael137 wrote: This logic is getting very finicky, so I wouldn't be surprised if there are cases not covered by this. Will have a think https://github.com/llvm/llvm-project/pull/108343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
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 Date: Thu, 12 Sep 2024 09:07:16 +0100 Subject: [PATCH 1/2] [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 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 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 overlappi
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/108343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 7294396 - [lldb][test] Handle failure to get /proc/cpuinfo from a remote Linux platform (#108183)
Author: David Spickett Date: 2024-09-12T09:48:13+01:00 New Revision: 7294396a0878a6bd179fac9aa5c3743832c799f4 URL: https://github.com/llvm/llvm-project/commit/7294396a0878a6bd179fac9aa5c3743832c799f4 DIFF: https://github.com/llvm/llvm-project/commit/7294396a0878a6bd179fac9aa5c3743832c799f4.diff LOG: [lldb][test] Handle failure to get /proc/cpuinfo from a remote Linux platform (#108183) I've been testing against qemu-aarch64 using the qemu-user platform, which doesn't support get-file: ``` AssertionError: False is not true : Command 'platform get-file "/proc/cpuinfo" <...>/TestAArch64LinuxMTEMemoryRegion.test_mte_regions/cpuinfo Command output: get-file failed: unimplemented ' did not return successfully ``` QEMU itself does support overriding cpuinfo for the emulated process (https://gitlab.com/qemu-project/qemu/-/commit/a55b9e72267085957cadb0af0a8811cfbd7c61a9) however we'd need to be able to read the cpuinfo before the process starts, so I'm not attempting to use this feature. Instead if the get-file fails, assume empty cpuinfo so we can at least carry on testing. I've logged the failure and the reason to the trace so developers can find it. ``` runCmd: platform get-file "/proc/cpuinfo" <...>/TestAArch64LinuxMTEMemoryRegion.test_mte_regions/cpuinfo check of return status not required runCmd failed! Failed to get /proc/cpuinfo from remote: "get-file failed: unimplemented" All cpuinfo feature checks will fail. ``` For now this only helps AArch64 but I suspect that RISC-V, being even more mix and match when it comes to extensions, may need this in future. And I know we have some folks testing against qemu-riscv at the moment. Added: Modified: lldb/packages/Python/lldbsuite/test/lldbtest.py Removed: diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py index e0da7cbd1ddd6e..df5a110cb5b309 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py +++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py @@ -1317,7 +1317,18 @@ def getCPUInfo(self): # Need to do something diff erent for non-Linux/Android targets cpuinfo_path = self.getBuildArtifact("cpuinfo") if configuration.lldb_platform_name: -self.runCmd('platform get-file "/proc/cpuinfo" ' + cpuinfo_path) +self.runCmd( +'platform get-file "/proc/cpuinfo" ' + cpuinfo_path, check=False +) +if not self.res.Succeeded(): +if self.TraceOn(): +print( +'Failed to get /proc/cpuinfo from remote: "{}"'.format( +self.res.GetOutput().strip() +) +) +print("All cpuinfo feature checks will fail.") +return "" else: cpuinfo_path = "/proc/cpuinfo" ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Handle failure to get /proc/cpuinfo from a remote Linux platform (PR #108183)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/108183 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] adde85e - [lldb][test] Mark some more watchpoint tests
Author: David Spickett Date: 2024-09-12T10:35:11Z New Revision: adde85e7c3ade54b22c99d405fc9c3add869db0a URL: https://github.com/llvm/llvm-project/commit/adde85e7c3ade54b22c99d405fc9c3add869db0a DIFF: https://github.com/llvm/llvm-project/commit/adde85e7c3ade54b22c99d405fc9c3add869db0a.diff LOG: [lldb][test] Mark some more watchpoint tests Noticed when testing with qemu-aarch64 that does not support watchpoints. Added: lldb/test/API/functionalities/watchpoint/categories Modified: Removed: diff --git a/lldb/test/API/functionalities/watchpoint/categories b/lldb/test/API/functionalities/watchpoint/categories new file mode 100644 index 00..97934fb58afd42 --- /dev/null +++ b/lldb/test/API/functionalities/watchpoint/categories @@ -0,0 +1,2 @@ +watchpoint + ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] WoA HW Break and Watchpoint support in LLDB (PR #108072)
omjavaid wrote: > > Yes I think there might be a Windows API launch flag that can let us do > > early detection of hardware breakpoints and watchpoints. But so far after > > lots of experimentation I have not been able to find the right set. > > Is it possible this is a bug in Windows itself? Though it seems unlikely > given that Visual Studio supports WoA. > > I'm not sure about merging this until we know more about that part, at least > whether it's a bug or not. Perhaps we (Linaro) can get some information from > Microsoft on this? So further update from my offline (out of LLDB) testing with windows API and setting hardware breakpoints suggest that we might be doing something different on LLDB side during launch or while setting hardware breakpoints/watchpoints that stops us from hitting those exceptions. In simple console based debugger I have played with Windows API and set hardware breakpoints and found a good configuration that works. I will keep debugging this might get to fix it on LLDB side as well. And will talk to our friends at MS at some stage for any clarifications that might be needed. https://github.com/llvm/llvm-project/pull/108072 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: fix padding for current layout (PR #108362)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/108362 IIUC, the history of `std::string`'s `__short` structure in the alternate ABI layout (as recorded by the simulator test) looks as follows: * First layout ( `SUBCLASS_PADDING` is defined): ``` struct __short { value_type __data_[__min_cap]; struct : __padding { unsigned char __size_; }; }; ``` * Then: ``` struct __short { value_type __data_[__min_cap]; unsigned char __padding[sizeof(value_type) - 1]; unsigned char __size_; }; ``` * Then, post-`BITMASKS`: ``` struct __short { value_type __data_[__min_cap]; unsigned char __padding[sizeof(value_type) - 1]; unsigned char __size_ : 7; unsigned char __is_long_ : 1; }; ``` Which is the one that's [on top-of-tree](https://github.com/llvm/llvm-project/blob/89c10e27d8b4d5f44998aad9abd2590d9f96c5df/libcxx/include/string#L854-L859). But for `REVISION > 2`, `BITMASKS` is never set, so for those tests we lose the `__padding` member. This patch fixes this by Drive-by: * Also run expression evaluator on the string to provide is with some extra coverage. >From 8b999e3330f4b414d3ffe56c500d407cbde57be0 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 12 Sep 2024 11:05:27 +0100 Subject: [PATCH] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: fix padding for current layout Drive-by: * Also run expression evaluator on the string to provide is with some extra coverage. --- .../string/TestDataFormatterLibcxxStringSimulator.py| 3 +++ .../data-formatter-stl/libcxx-simulators/string/main.cpp| 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py index 3e5c493692c4f0..98d2c7320003e4 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py @@ -22,6 +22,9 @@ def _run_test(self, defines): self.expect_var_path("shortstring", summary='"short"') self.expect_var_path("longstring", summary='"I am a very long string"') +self.expect_expr("shortstring", result_summary='"short"') +self.expect_expr("longstring", result_summary='"I am a very long string"') + for v in [None, "ALTERNATE_LAYOUT"]: for r in range(5): diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp index 7beeb9c39de49e..74baeba6ec879b 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp @@ -71,15 +71,15 @@ template class basic_string { struct __short { value_type __data_[__min_cap]; -#ifdef BITMASKS #ifdef SUBCLASS_PADDING struct : __padding { unsigned char __size_; }; -#else +#else // !SUBCLASS_PADDING + unsigned char __padding[sizeof(value_type) - 1]; +#ifdef BITMASKS unsigned char __size_; -#endif #else // !BITMASKS unsigned char __size_ : 7; unsigned char __is_long_ : 1; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: fix padding for current layout (PR #108362)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/108362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: fix padding for current layout (PR #108362)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes IIUC, the history of `std::string`'s `__short` structure in the alternate ABI layout (as recorded by the simulator test) looks as follows: * First layout ( `SUBCLASS_PADDING` is defined): ``` struct __short { value_type __data_[__min_cap]; struct : __padding{ unsigned char __size_; }; }; ``` * Then: ``` struct __short { value_type __data_[__min_cap]; unsigned char __padding[sizeof(value_type) - 1]; unsigned char __size_; }; ``` * Then, post-`BITMASKS`: ``` struct __short { value_type __data_[__min_cap]; unsigned char __padding[sizeof(value_type) - 1]; unsigned char __size_ : 7; unsigned char __is_long_ : 1; }; ``` Which is the one that's [on top-of-tree](https://github.com/llvm/llvm-project/blob/89c10e27d8b4d5f44998aad9abd2590d9f96c5df/libcxx/include/string#L854-L859). But for `REVISION > 2`, `BITMASKS` is never set, so for those tests we lose the `__padding` member. This patch fixes this by Drive-by: * Also run expression evaluator on the string to provide is with some extra coverage. --- Full diff: https://github.com/llvm/llvm-project/pull/108362.diff 2 Files Affected: - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py (+3) - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp (+3-3) ``diff diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py index 3e5c493692c4f0..98d2c7320003e4 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py @@ -22,6 +22,9 @@ def _run_test(self, defines): self.expect_var_path("shortstring", summary='"short"') self.expect_var_path("longstring", summary='"I am a very long string"') +self.expect_expr("shortstring", result_summary='"short"') +self.expect_expr("longstring", result_summary='"I am a very long string"') + for v in [None, "ALTERNATE_LAYOUT"]: for r in range(5): diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp index 7beeb9c39de49e..74baeba6ec879b 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp @@ -71,15 +71,15 @@ template class basic_string { struct __short { value_type __data_[__min_cap]; -#ifdef BITMASKS #ifdef SUBCLASS_PADDING struct : __padding { unsigned char __size_; }; -#else +#else // !SUBCLASS_PADDING + unsigned char __padding[sizeof(value_type) - 1]; +#ifdef BITMASKS unsigned char __size_; -#endif #else // !BITMASKS unsigned char __size_ : 7; unsigned char __is_long_ : 1; `` https://github.com/llvm/llvm-project/pull/108362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: fix padding for current layout (PR #108362)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/108362 >From 8b999e3330f4b414d3ffe56c500d407cbde57be0 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 12 Sep 2024 11:05:27 +0100 Subject: [PATCH 1/2] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: fix padding for current layout Drive-by: * Also run expression evaluator on the string to provide is with some extra coverage. --- .../string/TestDataFormatterLibcxxStringSimulator.py| 3 +++ .../data-formatter-stl/libcxx-simulators/string/main.cpp| 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py index 3e5c493692c4f0..98d2c7320003e4 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py @@ -22,6 +22,9 @@ def _run_test(self, defines): self.expect_var_path("shortstring", summary='"short"') self.expect_var_path("longstring", summary='"I am a very long string"') +self.expect_expr("shortstring", result_summary='"short"') +self.expect_expr("longstring", result_summary='"I am a very long string"') + for v in [None, "ALTERNATE_LAYOUT"]: for r in range(5): diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp index 7beeb9c39de49e..74baeba6ec879b 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp @@ -71,15 +71,15 @@ template class basic_string { struct __short { value_type __data_[__min_cap]; -#ifdef BITMASKS #ifdef SUBCLASS_PADDING struct : __padding { unsigned char __size_; }; -#else +#else // !SUBCLASS_PADDING + unsigned char __padding[sizeof(value_type) - 1]; +#ifdef BITMASKS unsigned char __size_; -#endif #else // !BITMASKS unsigned char __size_ : 7; unsigned char __is_long_ : 1; >From bcb09ad70ba5a350035ef43eab37ff3d585af856 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 12 Sep 2024 11:56:50 +0100 Subject: [PATCH 2/2] fixup! add missing endif --- .../data-formatter-stl/libcxx-simulators/string/main.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp index 74baeba6ec879b..b010dc25f8f804 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp @@ -83,7 +83,8 @@ template class basic_string { #else // !BITMASKS unsigned char __size_ : 7; unsigned char __is_long_ : 1; -#endif +#endif // BITMASKS +#endif // SUBCLASS_PADDING }; #ifdef BITMASKS ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: fix padding for current layout (PR #108362)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/108362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Create Neon subregs when XML only includes SVE (PR #108365)
https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/108365 Fixes #107864 QEMU decided that when SVE is enabled it will only tell us about SVE registers in the XML, and not include Neon registers. On the grounds that the Neon V registers can be read from the bottom 128 bits of a SVE Z register (SVE's vector length is always >= 128 bits). To support this we create sub-registers just as we do for S and D registers of the V registers. Except this time we use part of the Z registers. This change also updates our fallback for registers with unknown types that are > 128 bit. This is detailed in https://github.com/llvm/llvm-project/issues/87471, though that covers more than this change fixes. We'll now treat any register of unknown type that is > 128 bit as a vector of bytes. So that the user gets to see something even if the order might be wrong. And until lldb supports vector and union types for registers, this is also the only way we can get a value to apply the sub-reg to, to make the V registers. >From a4e984b8a5027ff189b113a292b016a2ab37688d Mon Sep 17 00:00:00 2001 From: David Spickett Date: Tue, 10 Sep 2024 13:36:05 + Subject: [PATCH] [lldb][AArch64] Create Neon subregs when XML only includes SVE Fixes #107864 QEMU decided that when SVE is enabled it will only tell us about SVE registers in the XML, and not include Neon registers. On the grounds that the Neon V registers can be read from the bottom 128 bits of a SVE Z register (SVE's vector length is always >= 128 bits). To support this we create sub-registers just as we do for S and D registers of the V registers. Except this time we use part of the Z registers. This change also updates our fallback for registers with unknown types that are > 128 bit. This is detailed in https://github.com/llvm/llvm-project/issues/87471, though that covers more than this change fixes. We'll now treat any register of unknown type that is > 128 bit as a vector of bytes. So that the user gets to see something even if the order might be wrong. And until lldb supports vector and union types for registers, this is also the only way we can get a value to apply the sub-reg to, to make the V registers. --- .../source/Plugins/ABI/AArch64/ABIAArch64.cpp | 40 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 9 +- .../TestAArch64XMLRegistersSVEOnly.py | 121 ++ 3 files changed, 163 insertions(+), 7 deletions(-) create mode 100644 lldb/test/API/functionalities/gdb_remote_client/TestAArch64XMLRegistersSVEOnly.py diff --git a/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp b/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp index 256c1f828feb38..7d8d0a4d3d6711 100644 --- a/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp +++ b/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp @@ -136,6 +136,8 @@ void ABIAArch64::AugmentRegisterInfo( std::array, 32> x_regs; std::array, 32> v_regs; + std::array, 32> z_regs; + std::optional z_byte_size; for (auto it : llvm::enumerate(regs)) { lldb_private::DynamicRegisterInfo::Register &info = it.value(); @@ -157,16 +159,44 @@ void ABIAArch64::AugmentRegisterInfo( x_regs[reg_num] = it.index(); else if (get_reg("v")) v_regs[reg_num] = it.index(); +else if (get_reg("z")) { + z_regs[reg_num] = it.index(); + if (!z_byte_size) +z_byte_size = info.byte_size; +} // if we have at least one subregister, abort else if (get_reg("w") || get_reg("s") || get_reg("d")) return; } - // Create aliases for partial registers: wN for xN, and sN/dN for vN. + // Create aliases for partial registers. + + // Wn for Xn. addPartialRegisters(regs, x_regs, 8, "w{0}", 4, lldb::eEncodingUint, lldb::eFormatHex); - addPartialRegisters(regs, v_regs, 16, "s{0}", 4, lldb::eEncodingIEEE754, - lldb::eFormatFloat); - addPartialRegisters(regs, v_regs, 16, "d{0}", 8, lldb::eEncodingIEEE754, - lldb::eFormatFloat); + + auto bool_predicate = [](const auto ®_num) { return bool(reg_num); }; + bool saw_v_regs = std::any_of(v_regs.begin(), v_regs.end(), bool_predicate); + bool saw_z_regs = std::any_of(z_regs.begin(), z_regs.end(), bool_predicate); + + // Sn/Dn for Vn. + if (saw_v_regs) { +addPartialRegisters(regs, v_regs, 16, "s{0}", 4, lldb::eEncodingIEEE754, +lldb::eFormatFloat); +addPartialRegisters(regs, v_regs, 16, "d{0}", 8, lldb::eEncodingIEEE754, +lldb::eFormatFloat); + } else if (saw_z_regs && z_byte_size) { +// When SVE is enabled, some debug stubs will not describe the Neon V +// registers because they can be read from the bottom 128 bits of the SVE +// registers. + +// The size used here is the one sent by the debug server. This only needs +// to be correct right now. Later we will rely on the value of vg instead. +addPartialRegisters(regs, z_regs, *z_byte_size,
[Lldb-commits] [lldb] [lldb][AArch64] Create Neon subregs when XML only includes SVE (PR #108365)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/108365 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Create Neon subregs when XML only includes SVE (PR #108365)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: David Spickett (DavidSpickett) Changes Fixes #107864 QEMU decided that when SVE is enabled it will only tell us about SVE registers in the XML, and not include Neon registers. On the grounds that the Neon V registers can be read from the bottom 128 bits of a SVE Z register (SVE's vector length is always >= 128 bits). To support this we create sub-registers just as we do for S and D registers of the V registers. Except this time we use part of the Z registers. This change also updates our fallback for registers with unknown types that are > 128 bit. This is detailed in https://github.com/llvm/llvm-project/issues/87471, though that covers more than this change fixes. We'll now treat any register of unknown type that is >= 128 bit as a vector of bytes. So that the user gets to see something even if the order might be wrong. And until lldb supports vector and union types for registers, this is also the only way we can get a value to apply the sub-reg to, to make the V registers. --- Full diff: https://github.com/llvm/llvm-project/pull/108365.diff 3 Files Affected: - (modified) lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp (+35-5) - (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+7-2) - (added) lldb/test/API/functionalities/gdb_remote_client/TestAArch64XMLRegistersSVEOnly.py (+121) ``diff diff --git a/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp b/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp index 256c1f828feb38..7d8d0a4d3d6711 100644 --- a/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp +++ b/lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp @@ -136,6 +136,8 @@ void ABIAArch64::AugmentRegisterInfo( std::array, 32> x_regs; std::array, 32> v_regs; + std::array, 32> z_regs; + std::optional z_byte_size; for (auto it : llvm::enumerate(regs)) { lldb_private::DynamicRegisterInfo::Register &info = it.value(); @@ -157,16 +159,44 @@ void ABIAArch64::AugmentRegisterInfo( x_regs[reg_num] = it.index(); else if (get_reg("v")) v_regs[reg_num] = it.index(); +else if (get_reg("z")) { + z_regs[reg_num] = it.index(); + if (!z_byte_size) +z_byte_size = info.byte_size; +} // if we have at least one subregister, abort else if (get_reg("w") || get_reg("s") || get_reg("d")) return; } - // Create aliases for partial registers: wN for xN, and sN/dN for vN. + // Create aliases for partial registers. + + // Wn for Xn. addPartialRegisters(regs, x_regs, 8, "w{0}", 4, lldb::eEncodingUint, lldb::eFormatHex); - addPartialRegisters(regs, v_regs, 16, "s{0}", 4, lldb::eEncodingIEEE754, - lldb::eFormatFloat); - addPartialRegisters(regs, v_regs, 16, "d{0}", 8, lldb::eEncodingIEEE754, - lldb::eFormatFloat); + + auto bool_predicate = [](const auto ®_num) { return bool(reg_num); }; + bool saw_v_regs = std::any_of(v_regs.begin(), v_regs.end(), bool_predicate); + bool saw_z_regs = std::any_of(z_regs.begin(), z_regs.end(), bool_predicate); + + // Sn/Dn for Vn. + if (saw_v_regs) { +addPartialRegisters(regs, v_regs, 16, "s{0}", 4, lldb::eEncodingIEEE754, +lldb::eFormatFloat); +addPartialRegisters(regs, v_regs, 16, "d{0}", 8, lldb::eEncodingIEEE754, +lldb::eFormatFloat); + } else if (saw_z_regs && z_byte_size) { +// When SVE is enabled, some debug stubs will not describe the Neon V +// registers because they can be read from the bottom 128 bits of the SVE +// registers. + +// The size used here is the one sent by the debug server. This only needs +// to be correct right now. Later we will rely on the value of vg instead. +addPartialRegisters(regs, z_regs, *z_byte_size, "v{0}", 16, +lldb::eEncodingVector, lldb::eFormatVectorOfUInt8); +addPartialRegisters(regs, z_regs, *z_byte_size, "s{0}", 4, +lldb::eEncodingIEEE754, lldb::eFormatFloat); +addPartialRegisters(regs, z_regs, *z_byte_size, "d{0}", 8, +lldb::eEncodingIEEE754, lldb::eFormatFloat); + } } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 5eaf9ce2a302aa..342a1ec090b3fa 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -4715,9 +4715,14 @@ bool ParseRegisters( reg_info.encoding = eEncodingIEEE754; } else if (gdb_type == "aarch64v" || llvm::StringRef(gdb_type).starts_with("vec") || - gdb_type == "i387_ext" || gdb_type == "uint128") { + gdb_type == "i387_ext" || gdb_type == "uint128" || + reg_info.byte_size > 16) { // lldb doesn't handle 128-bit uints cor
[Lldb-commits] [lldb] [lldb][AArch64] Create Neon subregs when XML only includes SVE (PR #108365)
DavidSpickett wrote: The data order of SVE and Neon in this situation is probably wrong, but I'm not aiming to address that here. https://github.com/llvm/llvm-project/pull/108365 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: add new padding layout (PR #108169)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/108169 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: add new padding layout (PR #108375)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/108375 Depends on https://github.com/llvm/llvm-project/pull/108362 and https://github.com/llvm/llvm-project/pull/108343. Adds new layout for https://github.com/llvm/llvm-project/pull/105865. >From cac4ea7d5f3b4d3b6f221625c7f9c50a88df08f5 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 12 Sep 2024 11:05:27 +0100 Subject: [PATCH 1/3] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: fix padding for current layout Drive-by: * Also run expression evaluator on the string to provide is with some extra coverage. --- .../string/TestDataFormatterLibcxxStringSimulator.py| 3 +++ .../data-formatter-stl/libcxx-simulators/string/main.cpp| 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py index 3e5c493692c4f0..98d2c7320003e4 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py @@ -22,6 +22,9 @@ def _run_test(self, defines): self.expect_var_path("shortstring", summary='"short"') self.expect_var_path("longstring", summary='"I am a very long string"') +self.expect_expr("shortstring", result_summary='"short"') +self.expect_expr("longstring", result_summary='"I am a very long string"') + for v in [None, "ALTERNATE_LAYOUT"]: for r in range(5): diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp index 7beeb9c39de49e..74baeba6ec879b 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp @@ -71,15 +71,15 @@ template class basic_string { struct __short { value_type __data_[__min_cap]; -#ifdef BITMASKS #ifdef SUBCLASS_PADDING struct : __padding { unsigned char __size_; }; -#else +#else // !SUBCLASS_PADDING + unsigned char __padding[sizeof(value_type) - 1]; +#ifdef BITMASKS unsigned char __size_; -#endif #else // !BITMASKS unsigned char __size_ : 7; unsigned char __is_long_ : 1; >From 8caff2b4794bc8e8a45bdfa21d293fbe1cb95652 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 12 Sep 2024 11:56:50 +0100 Subject: [PATCH 2/3] fixup! add missing endif --- .../data-formatter-stl/libcxx-simulators/string/main.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp index 74baeba6ec879b..b010dc25f8f804 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp @@ -83,7 +83,8 @@ template class basic_string { #else // !BITMASKS unsigned char __size_ : 7; unsigned char __is_long_ : 1; -#endif +#endif // BITMASKS +#endif // SUBCLASS_PADDING }; #ifdef BITMASKS >From 4e4f1ea94aa4fde3953eb906252d16fa47d2ae31 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 12 Sep 2024 12:55:17 +0100 Subject: [PATCH 3/3] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: add new padding layout Depends on https://github.com/llvm/llvm-project/pull/108362. Adds new layout for https://github.com/llvm/llvm-project/pull/105865. --- .../TestDataFormatterLibcxxStringSimulator.py | 2 +- .../libcxx-simulators/string/main.cpp | 34 +++ 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py index 98d2c7320003e4..dd9f5de22c9057 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py @@ -27,7 +27,7 @@ def _run_test(self, defines): for v in [None, "ALTERNATE_LAYOUT"]: -for r in range(5): +for
[Lldb-commits] [lldb] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: add new padding layout (PR #108375)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes Depends on https://github.com/llvm/llvm-project/pull/108362 and https://github.com/llvm/llvm-project/pull/108343. Adds new layout for https://github.com/llvm/llvm-project/pull/105865. --- Full diff: https://github.com/llvm/llvm-project/pull/108375.diff 2 Files Affected: - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py (+4-1) - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp (+33-10) ``diff diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py index 3e5c493692c4f0..dd9f5de22c9057 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py @@ -22,9 +22,12 @@ def _run_test(self, defines): self.expect_var_path("shortstring", summary='"short"') self.expect_var_path("longstring", summary='"I am a very long string"') +self.expect_expr("shortstring", result_summary='"short"') +self.expect_expr("longstring", result_summary='"I am a very long string"') + for v in [None, "ALTERNATE_LAYOUT"]: -for r in range(5): +for r in range(6): name = "test_r%d" % r defines = ["REVISION=%d" % r] if v: diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp index 7beeb9c39de49e..5cf3a85007fb64 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp @@ -20,7 +20,11 @@ // Pre-D128285 layout. #define PACKED_ANON_STRUCT #endif -// REVISION == 4: current layout +#if REVISION <= 4 +// Pre-TODO layout. +#define UB_PADDING +#endif +// REVISION == 5: current layout #ifdef PACKED_ANON_STRUCT #define BEGIN_PACKED_ANON_STRUCT struct __attribute__((packed)) { @@ -34,6 +38,7 @@ namespace std { namespace __lldb { +#ifdef UB_PADDING #if defined(ALTERNATE_LAYOUT) && defined(SUBCLASS_PADDING) template struct __padding { unsigned char __xx[sizeof(_CharT) - 1]; @@ -41,6 +46,13 @@ template struct __padding { template struct __padding<_CharT, 1> {}; #endif +#else // !UB_PADDING +template struct __padding { + char __padding_[_PaddingSize]; +}; + +template <> struct __padding<0> {}; +#endif template class basic_string { public: @@ -71,19 +83,25 @@ template class basic_string { struct __short { value_type __data_[__min_cap]; -#ifdef BITMASKS #ifdef SUBCLASS_PADDING struct : __padding { unsigned char __size_; }; -#else +#else // !SUBCLASS_PADDING + +#ifdef UB_PADDING unsigned char __padding[sizeof(value_type) - 1]; -unsigned char __size_; +#else +[[no_unique_address]] __padding __padding_; #endif + +#ifdef BITMASKS +unsigned char __size_; #else // !BITMASKS unsigned char __size_ : 7; unsigned char __is_long_ : 1; -#endif +#endif // BITMASKS +#endif // SUBCLASS_PADDING }; #ifdef BITMASKS @@ -128,21 +146,26 @@ template class basic_string { union { #ifdef BITMASKS unsigned char __size_; -#else +#else // !BITMASKS struct { unsigned char __is_long_ : 1; unsigned char __size_ : 7; }; -#endif +#endif // BITMASKS value_type __lx; }; -#else +#else // !SHORT_UNION BEGIN_PACKED_ANON_STRUCT unsigned char __is_long_ : 1; unsigned char __size_ : 7; END_PACKED_ANON_STRUCT -char __padding_[sizeof(value_type) - 1]; -#endif +#ifdef UB_PADDING +unsigned char __padding[sizeof(value_type) - 1]; +#else // !UB_PADDING +[[no_unique_address]] __padding __padding_; +#endif // UB_PADDING + +#endif // SHORT_UNION value_type __data_[__min_cap]; }; `` https://github.com/llvm/llvm-project/pull/108375 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
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 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 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 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 overlappi
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 703ebca869e1e684147d316b7bdb15437c12206a 5d25eeb8cb993dc85869ace156d4a1505c9faa67 --extensions h,cpp -- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/test/Shell/SymbolFile/DWARF/no_unique_address-with-bitfields.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 9455bc7106..5b679bd5a6 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; } `` https://github.com/llvm/llvm-project/pull/108343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][testing] Check all stop reasons in get_threads_stopped_at_breakpoint_id (PR #108281)
https://github.com/felipepiovezan closed https://github.com/llvm/llvm-project/pull/108281 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 7e74472 - [lldb][testing] Check all stop reasons in get_threads_stopped_at_breakpoint_id (#108281)
Author: Felipe de Azevedo Piovezan Date: 2024-09-12T07:01:59-07:00 New Revision: 7e74472801486cc702fba3e831c8fcd77c120142 URL: https://github.com/llvm/llvm-project/commit/7e74472801486cc702fba3e831c8fcd77c120142 DIFF: https://github.com/llvm/llvm-project/commit/7e74472801486cc702fba3e831c8fcd77c120142.diff LOG: [lldb][testing] Check all stop reasons in get_threads_stopped_at_breakpoint_id (#108281) If multiple breakpoints are hit at the same time, multiple stop reasons are reported, one per breakpoint. Currently, `get_threads_stopped_at_breakpoint_id` only checks the first such reason. Added: Modified: lldb/packages/Python/lldbsuite/test/lldbutil.py Removed: diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index 629565b38ca1e6..660a3c085a908a 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -773,9 +773,16 @@ def get_threads_stopped_at_breakpoint_id(process, bpid): return threads for thread in stopped_threads: -# Make sure we've hit our breakpoint... -break_id = thread.GetStopReasonDataAtIndex(0) -if break_id == bpid: +# Make sure we've hit our breakpoint. +# From the docs of GetStopReasonDataAtIndex: "Breakpoint stop reasons +# will have data that consists of pairs of breakpoint IDs followed by +# the breakpoint location IDs". +# Iterate over all such pairs looking for `bpid`. +break_ids = [ +thread.GetStopReasonDataAtIndex(idx) +for idx in range(0, thread.GetStopReasonDataCount(), 2) +] +if bpid in break_ids: threads.append(thread) return threads ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
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 Date: Thu, 12 Sep 2024 09:07:16 +0100 Subject: [PATCH 1/4] [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 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 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 overlappi
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
@@ -258,9 +258,27 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { private: struct FieldInfo { +///< Size in bits that this field occupies. Can but adrian-prantl wrote: ``` /// comment decl d; ``` or ``` decl d; ///https://github.com/llvm/llvm-project/pull/108343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
https://github.com/adrian-prantl approved this pull request. Once we're in heuristic territory I think we can only get a hold of this problem with lots of tests. https://github.com/llvm/llvm-project/pull/108343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
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 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 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 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 overlappi
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
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 Date: Thu, 12 Sep 2024 09:07:16 +0100 Subject: [PATCH 1/6] [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 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 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 overlappi
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
Michael137 wrote: > Once we're in heuristic territory I think we can only get a hold of this > problem with lots of tests. Added more test in the latest iteration (the ones marked FIXME were still misbehaving on top-of-tree, although in other ways...but we still asserted in the expression evaluator, so don't think we really regress with this patch). There are some cases where it's pretty much impossible to differentiate between tail padding and unnamed bitfields. So as long as we don't emit those bitfields into DWARF I think we have to resort to this. https://github.com/llvm/llvm-project/pull/108343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change ValueObject::AddressOf() to return Expected (NFC) (PR #106831)
jimingham wrote: You also have to be careful when treating Errors in ValueObjects, since it is not always the case that a ValueObject that returns `GetError().Success() == true` on one stop will return that when the program is allowed to run. For instance, the ValueObject for a local variable might have an error because it is not available at the current PC due to optimization, but it will be when you step again. So an error state doesn't mean "discard me". To be clear, I'm have no problem with rationalizing the error handling for ValueObjects, but we need to have a higher level plan first, I don't think going in piecemeal like this is going to be productive. https://github.com/llvm/llvm-project/pull/106831 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
adrian-prantl wrote: Why don't we just make the unnamed fields explicit in DWARF and solve the problem the correct way? (We'll still needs this hack to support other/older compilers though) https://github.com/llvm/llvm-project/pull/108343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Avoid expression evaluation in libStdC++ std::vector synthetic children provider (PR #108414)
https://github.com/jeffreytan81 created https://github.com/llvm/llvm-project/pull/108414 Our customers is reporting a serious performance issue (expanding a this pointer takes 70 seconds in VSCode) in a specific execution context. Profiling shows the hot path is triggered by an expression evaluation from libStdC++ synthetic children provider for `std::vector` since it uses `CreateValueFromExpression()`. This PR added a new `SBValue::CreateBoolValue()` API and switch `std::vector` synthetic children provider to use the new API without performing expression evaluation. Note: there might be other cases of `CreateValueFromExpression()` in our summary/synthetic children providers which I will sweep through in later PRs. With this PR, the customer's scenario reduces from 70 seconds => 50 seconds. I will add other PRs to further optimize the remaining 50 seconds (mostly from type/namespace lookup). >From 6e84ab9a14e63c58e1facdbf9a695c093882b37b Mon Sep 17 00:00:00 2001 From: jeffreytan81 Date: Mon, 19 Aug 2024 10:57:35 -0700 Subject: [PATCH 1/2] Fix StartDebuggingRequestHandler/ReplModeRequestHandler in lldb-dap --- lldb/tools/lldb-dap/DAP.h| 2 -- lldb/tools/lldb-dap/lldb-dap.cpp | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 57562a14983519..7828272aa15a7d 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -192,8 +192,6 @@ struct DAP { std::mutex call_mutex; std::map inflight_reverse_requests; - StartDebuggingRequestHandler start_debugging_request_handler; - ReplModeRequestHandler repl_mode_request_handler; ReplMode repl_mode; std::string command_escape_prefix = "`"; lldb::SBFormat frame_format; diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index ea84f31aec3a6c..f50a6c17310739 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1627,12 +1627,12 @@ void request_initialize(const llvm::json::Object &request) { "lldb-dap", "Commands for managing lldb-dap."); if (GetBoolean(arguments, "supportsStartDebuggingRequest", false)) { cmd.AddCommand( -"startDebugging", &g_dap.start_debugging_request_handler, +"startDebugging", new StartDebuggingRequestHandler(), "Sends a startDebugging request from the debug adapter to the client " "to start a child debug session of the same type as the caller."); } cmd.AddCommand( - "repl-mode", &g_dap.repl_mode_request_handler, + "repl-mode", new ReplModeRequestHandler(), "Get or set the repl behavior of lldb-dap evaluation requests."); g_dap.progress_event_thread = std::thread(ProgressEventThreadFunction); >From d56c9b32f46e2c08ca134834b670bf3b1dad6c53 Mon Sep 17 00:00:00 2001 From: jeffreytan81 Date: Thu, 12 Sep 2024 09:05:33 -0700 Subject: [PATCH 2/2] Add CreateBoolValue API --- lldb/examples/synthetic/gnu_libstdcpp.py | 6 +- lldb/include/lldb/API/SBValue.h | 2 ++ lldb/source/API/SBValue.cpp | 27 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/lldb/examples/synthetic/gnu_libstdcpp.py b/lldb/examples/synthetic/gnu_libstdcpp.py index d98495b8a9df38..597298dfce36b4 100644 --- a/lldb/examples/synthetic/gnu_libstdcpp.py +++ b/lldb/examples/synthetic/gnu_libstdcpp.py @@ -473,11 +473,7 @@ def get_child_at_index(self, index): "[" + str(index) + "]", element_offset, element_type ) bit = element.GetValueAsUnsigned(0) & (1 << bit_offset) -if bit != 0: -value_expr = "(bool)true" -else: -value_expr = "(bool)false" -return self.valobj.CreateValueFromExpression("[%d]" % index, value_expr) +return self.valobj.CreateBooleanValue("[%d]" % index, bool(bit)) def update(self): try: diff --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h index bec816fb451844..aeda26cb6dd795 100644 --- a/lldb/include/lldb/API/SBValue.h +++ b/lldb/include/lldb/API/SBValue.h @@ -146,6 +146,8 @@ class LLDB_API SBValue { lldb::SBValue CreateValueFromData(const char *name, lldb::SBData data, lldb::SBType type); + lldb::SBValue CreateBoolValue(const char *name, bool value); + /// Get a child value by index from a value. /// /// Structs, unions, classes, arrays and pointers have child diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp index 273aac5ad47989..eb54213f4e60bc 100644 --- a/lldb/source/API/SBValue.cpp +++ b/lldb/source/API/SBValue.cpp @@ -645,6 +645,33 @@ lldb::SBValue SBValue::CreateValueFromData(const char *name, SBData data, return sb_value; } +lldb::SBValue SBValue::CreateBoolValue(const char *name, bool value) { + LLDB_INSTRUMENT_VA(this, name); + + lldb::SBValue sb_value; + lldb::ValueO
[Lldb-commits] [lldb] Avoid expression evaluation in libStdC++ std::vector synthetic children provider (PR #108414)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: None (jeffreytan81) Changes Our customers is reporting a serious performance issue (expanding a this pointer takes 70 seconds in VSCode) in a specific execution context. Profiling shows the hot path is triggered by an expression evaluation from libStdC++ synthetic children provider for `std::vector` since it uses `CreateValueFromExpression()`. This PR added a new `SBValue::CreateBoolValue()` API and switch `std::vector ` synthetic children provider to use the new API without performing expression evaluation. Note: there might be other cases of `CreateValueFromExpression()` in our summary/synthetic children providers which I will sweep through in later PRs. With this PR, the customer's scenario reduces from 70 seconds => 50 seconds. I will add other PRs to further optimize the remaining 50 seconds (mostly from type/namespace lookup). --- Full diff: https://github.com/llvm/llvm-project/pull/108414.diff 3 Files Affected: - (modified) lldb/examples/synthetic/gnu_libstdcpp.py (+1-5) - (modified) lldb/include/lldb/API/SBValue.h (+2) - (modified) lldb/source/API/SBValue.cpp (+27) ``diff diff --git a/lldb/examples/synthetic/gnu_libstdcpp.py b/lldb/examples/synthetic/gnu_libstdcpp.py index d98495b8a9df38..597298dfce36b4 100644 --- a/lldb/examples/synthetic/gnu_libstdcpp.py +++ b/lldb/examples/synthetic/gnu_libstdcpp.py @@ -473,11 +473,7 @@ def get_child_at_index(self, index): "[" + str(index) + "]", element_offset, element_type ) bit = element.GetValueAsUnsigned(0) & (1 << bit_offset) -if bit != 0: -value_expr = "(bool)true" -else: -value_expr = "(bool)false" -return self.valobj.CreateValueFromExpression("[%d]" % index, value_expr) +return self.valobj.CreateBooleanValue("[%d]" % index, bool(bit)) def update(self): try: diff --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h index bec816fb451844..aeda26cb6dd795 100644 --- a/lldb/include/lldb/API/SBValue.h +++ b/lldb/include/lldb/API/SBValue.h @@ -146,6 +146,8 @@ class LLDB_API SBValue { lldb::SBValue CreateValueFromData(const char *name, lldb::SBData data, lldb::SBType type); + lldb::SBValue CreateBoolValue(const char *name, bool value); + /// Get a child value by index from a value. /// /// Structs, unions, classes, arrays and pointers have child diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp index 273aac5ad47989..eb54213f4e60bc 100644 --- a/lldb/source/API/SBValue.cpp +++ b/lldb/source/API/SBValue.cpp @@ -645,6 +645,33 @@ lldb::SBValue SBValue::CreateValueFromData(const char *name, SBData data, return sb_value; } +lldb::SBValue SBValue::CreateBoolValue(const char *name, bool value) { + LLDB_INSTRUMENT_VA(this, name); + + lldb::SBValue sb_value; + lldb::ValueObjectSP new_value_sp; + ValueLocker locker; + lldb::ValueObjectSP value_sp(GetSP(locker)); + ProcessSP process_sp = m_opaque_sp->GetProcessSP(); + lldb::SBTarget target = GetTarget(); + if (!target.IsValid()) +return sb_value; + lldb::SBType boolean_type = target.GetBasicType(lldb::eBasicTypeBool); + lldb::TypeImplSP type_impl_sp(boolean_type.GetSP()); + if (value_sp && process_sp && type_impl_sp) { +int data_buf[1] = {value ? 1 : 0}; +lldb::SBData data = lldb::SBData::CreateDataFromSInt32Array( +process_sp->GetByteOrder(), sizeof(data_buf[0]), data_buf, +sizeof(data_buf) / sizeof(data_buf[0])); +ExecutionContext exe_ctx(value_sp->GetExecutionContextRef()); +new_value_sp = ValueObject::CreateValueObjectFromData( +name, **data, exe_ctx, type_impl_sp->GetCompilerType(true)); +new_value_sp->SetAddressTypeOfChildren(eAddressTypeLoad); + } + sb_value.SetSP(new_value_sp); + return sb_value; +} + SBValue SBValue::GetChildAtIndex(uint32_t idx) { LLDB_INSTRUMENT_VA(this, idx); `` https://github.com/llvm/llvm-project/pull/108414 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Avoid expression evaluation in libStdC++ std::vector synthetic children provider (PR #108414)
https://github.com/jeffreytan81 edited https://github.com/llvm/llvm-project/pull/108414 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/108343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a6a547f - [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (#108343)
Author: Michael Buch Date: 2024-09-12T17:26:17+01:00 New Revision: a6a547f18d99f0b0bf5ffac55443d687200f972d URL: https://github.com/llvm/llvm-project/commit/a6a547f18d99f0b0bf5ffac55443d687200f972d DIFF: https://github.com/llvm/llvm-project/commit/a6a547f18d99f0b0bf5ffac55443d687200f972d.diff LOG: [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (#108343) This bug surfaced after https://github.com/llvm/llvm-project/pull/105865 (currently reverted, but blocked on this to be relanded). Because Clang doesn't emit `DW_TAG_member`s for unnamed bitfields, LLDB has to make an educated guess about whether they existed in the source. It does so by checking whether there is a gap between where the last field ended and the currently parsed field starts. In the example test case, the empty field `padding` was folded into the storage of `data`. Because the `bit_offset` of `padding` is `0x0` and its `DW_AT_byte_size` is `0x1`, LLDB thinks the field ends at `0x1` (not quite because we first round the size to a word size, but this is an implementation detail), erroneously deducing that there's a gap between `flag` and `padding`. This patch adds the notion of "effective field end", which accounts for fields that share storage. It is set to the end of the storage that the two fields occupy. Then we use this to check for gaps in the unnamed bitfield creation logic. Added: Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/test/Shell/SymbolFile/DWARF/no_unique_address-with-bitfields.cpp Removed: diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 5b9de6f1566b05..5b679bd5a613e3 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2932,14 +2932,21 @@ 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 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.GetEffectiveFieldEnd()) + this_field_info.SetEffectiveFieldEnd( + last_field_info.GetEffectiveFieldEnd()); + +last_field_info = this_field_info; } // Don't turn artificial members such as vtable pointers into real FieldDecls @@ -3738,7 +3745,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..1ffb09b2fc1942 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 occupies. 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 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 bi
[Lldb-commits] [lldb] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: add new padding layout (PR #108375)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/108375 >From 5b38bd48254d552dca2eb51d7270b01734494d90 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 12 Sep 2024 11:05:27 +0100 Subject: [PATCH 1/3] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: fix padding for current layout Drive-by: * Also run expression evaluator on the string to provide is with some extra coverage. --- .../string/TestDataFormatterLibcxxStringSimulator.py| 3 +++ .../data-formatter-stl/libcxx-simulators/string/main.cpp| 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py index 3e5c493692c4f0..98d2c7320003e4 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py @@ -22,6 +22,9 @@ def _run_test(self, defines): self.expect_var_path("shortstring", summary='"short"') self.expect_var_path("longstring", summary='"I am a very long string"') +self.expect_expr("shortstring", result_summary='"short"') +self.expect_expr("longstring", result_summary='"I am a very long string"') + for v in [None, "ALTERNATE_LAYOUT"]: for r in range(5): diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp index 7beeb9c39de49e..74baeba6ec879b 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp @@ -71,15 +71,15 @@ template class basic_string { struct __short { value_type __data_[__min_cap]; -#ifdef BITMASKS #ifdef SUBCLASS_PADDING struct : __padding { unsigned char __size_; }; -#else +#else // !SUBCLASS_PADDING + unsigned char __padding[sizeof(value_type) - 1]; +#ifdef BITMASKS unsigned char __size_; -#endif #else // !BITMASKS unsigned char __size_ : 7; unsigned char __is_long_ : 1; >From 258b6fd43dddeb748cfd7e1dae5496ff156e33fa Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 12 Sep 2024 11:56:50 +0100 Subject: [PATCH 2/3] fixup! add missing endif --- .../data-formatter-stl/libcxx-simulators/string/main.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp index 74baeba6ec879b..b010dc25f8f804 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp @@ -83,7 +83,8 @@ template class basic_string { #else // !BITMASKS unsigned char __size_ : 7; unsigned char __is_long_ : 1; -#endif +#endif // BITMASKS +#endif // SUBCLASS_PADDING }; #ifdef BITMASKS >From 35971a4ee2e62552b57f644a2b1ff57c513a7934 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 12 Sep 2024 12:55:17 +0100 Subject: [PATCH 3/3] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: add new padding layout Depends on https://github.com/llvm/llvm-project/pull/108362. Adds new layout for https://github.com/llvm/llvm-project/pull/105865. --- .../TestDataFormatterLibcxxStringSimulator.py | 2 +- .../libcxx-simulators/string/main.cpp | 34 +++ 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py index 98d2c7320003e4..dd9f5de22c9057 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py @@ -27,7 +27,7 @@ def _run_test(self, defines): for v in [None, "ALTERNATE_LAYOUT"]: -for r in range(5): +for r in range(6): name = "test_r%d" % r defines = ["REVISION=%d" % r] if v: diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-si
[Lldb-commits] [lldb] [LLDB][Minidump] Add a progress bar to minidump (PR #108309)
Jlalond wrote: > The variable name should be `progress_tracker` but really we should just call > it `progress` for consistency with the rest of lldb. Good to know, I know this class has a lot of pascal casing from before and I'm always unsure to break the existing pattern and use snake or not https://github.com/llvm/llvm-project/pull/108309 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Add a progress bar to minidump (PR #108309)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/108309 >From e7054832dc6e54d4b9f3ce86a8babd1f62cac81a Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 11 Sep 2024 13:33:44 -0700 Subject: [PATCH 1/3] Add Progress bar to minidump memory emission --- .../Minidump/MinidumpFileBuilder.cpp | 18 -- .../ObjectFile/Minidump/MinidumpFileBuilder.h | 7 --- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index edc568a6b47e00..1765cc24721837 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -837,14 +837,16 @@ Status MinidumpFileBuilder::AddMemoryList() { error = m_process_sp->CalculateCoreFileSaveRanges(m_save_core_options, all_core_memory_ranges); + if (error.Fail()) +return error; + + lldb_private::Progress progress_tracker("Saving Minidump File", "", all_core_memory_ranges.GetSize()); std::vector all_core_memory_vec; // Extract all the data into just a vector of data. So we can mutate this in // place. for (const auto &core_range : all_core_memory_ranges) all_core_memory_vec.push_back(core_range.data); - if (error.Fail()) -return error; // Start by saving all of the stacks and ensuring they fit under the 32b // limit. @@ -892,13 +894,13 @@ Status MinidumpFileBuilder::AddMemoryList() { } } - error = AddMemoryList_32(ranges_32); + error = AddMemoryList_32(ranges_32, progress_tracker); if (error.Fail()) return error; // Add the remaining memory as a 64b range. if (!ranges_64.empty()) { -error = AddMemoryList_64(ranges_64); +error = AddMemoryList_64(ranges_64, progress_tracker); if (error.Fail()) return error; } @@ -973,7 +975,7 @@ GetLargestRangeSize(const std::vector &ranges) { } Status MinidumpFileBuilder::AddMemoryList_32( -std::vector &ranges) { +std::vector &ranges, Progress &progressTracker) { std::vector descriptors; Status error; if (ranges.size() == 0) @@ -1024,6 +1026,8 @@ Status MinidumpFileBuilder::AddMemoryList_32( error = AddData(data_up->GetBytes(), bytes_read); if (error.Fail()) return error; + +progressTracker.Increment(); } // Add a directory that references this list @@ -1050,7 +1054,7 @@ Status MinidumpFileBuilder::AddMemoryList_32( } Status MinidumpFileBuilder::AddMemoryList_64( -std::vector &ranges) { +std::vector &ranges, Progress &progressTracker) { Status error; if (ranges.empty()) return error; @@ -1132,6 +1136,8 @@ Status MinidumpFileBuilder::AddMemoryList_64( error = AddData(data_up->GetBytes(), bytes_read); if (error.Fail()) return error; + +progressTracker.Increment(); } // Early return if there is no cleanup needed. diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h index 71001e26c00e91..77c5dba6cdf991 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -30,6 +30,7 @@ #include "lldb/Utility/Status.h" #include "lldb/lldb-forward.h" #include "lldb/lldb-types.h" +#include "lldb/Core/Progress.h" #include "llvm/BinaryFormat/Minidump.h" #include "llvm/Object/Minidump.h" @@ -79,7 +80,7 @@ class MinidumpFileBuilder { const lldb::ProcessSP &process_sp, lldb_private::SaveCoreOptions &save_core_options) : m_process_sp(process_sp), m_core_file(std::move(core_file)), -m_save_core_options(save_core_options){} +m_save_core_options(save_core_options) {} MinidumpFileBuilder(const MinidumpFileBuilder &) = delete; MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete; @@ -121,9 +122,9 @@ class MinidumpFileBuilder { lldb_private::Status AddData(const void *data, uint64_t size); // Add MemoryList stream, containing dumps of important memory segments lldb_private::Status - AddMemoryList_64(std::vector &ranges); + AddMemoryList_64(std::vector &ranges, lldb_private::Progress &progressTracker); lldb_private::Status - AddMemoryList_32(std::vector &ranges); + AddMemoryList_32(std::vector &ranges, lldb_private::Progress &progressTracker); // Update the thread list on disk with the newly emitted stack RVAs. lldb_private::Status FixThreadStacks(); lldb_private::Status FlushBufferToDisk(); >From 7d62709164be7ce39685e2c99bfda8998c9c6d39 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 11 Sep 2024 17:11:28 -0700 Subject: [PATCH 2/3] run GCF --- .../ObjectFile/Minidump/MinidumpFileBuilder.cpp| 14 -- .../ObjectFile/Minidump/Minidu
[Lldb-commits] [lldb] Avoid expression evaluation in libStdC++ std::vector synthetic children provider (PR #108414)
@@ -645,6 +645,33 @@ lldb::SBValue SBValue::CreateValueFromData(const char *name, SBData data, return sb_value; } +lldb::SBValue SBValue::CreateBoolValue(const char *name, bool value) { + LLDB_INSTRUMENT_VA(this, name); + + lldb::SBValue sb_value; + lldb::ValueObjectSP new_value_sp; + ValueLocker locker; + lldb::ValueObjectSP value_sp(GetSP(locker)); + ProcessSP process_sp = m_opaque_sp->GetProcessSP(); + lldb::SBTarget target = GetTarget(); + if (!target.IsValid()) +return sb_value; + lldb::SBType boolean_type = target.GetBasicType(lldb::eBasicTypeBool); + lldb::TypeImplSP type_impl_sp(boolean_type.GetSP()); + if (value_sp && process_sp && type_impl_sp) { +int data_buf[1] = {value ? 1 : 0}; Jlalond wrote: Is a data buffer the only option here because of the bitwise logic? I ask because we could just have the address point to a singular byte instead right? https://github.com/llvm/llvm-project/pull/108414 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Avoid expression evaluation in libStdC++ std::vector synthetic children provider (PR #108414)
@@ -146,6 +146,8 @@ class LLDB_API SBValue { lldb::SBValue CreateValueFromData(const char *name, lldb::SBData data, lldb::SBType type); + lldb::SBValue CreateBoolValue(const char *name, bool value); Jlalond wrote: I would add a blurb this is created from data not address https://github.com/llvm/llvm-project/pull/108414 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Avoid expression evaluation in libStdC++ std::vector synthetic children provider (PR #108414)
https://github.com/Jlalond approved this pull request. https://github.com/llvm/llvm-project/pull/108414 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Add a progress bar to minidump (PR #108309)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/108309 >From e7054832dc6e54d4b9f3ce86a8babd1f62cac81a Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 11 Sep 2024 13:33:44 -0700 Subject: [PATCH 1/3] Add Progress bar to minidump memory emission --- .../Minidump/MinidumpFileBuilder.cpp | 18 -- .../ObjectFile/Minidump/MinidumpFileBuilder.h | 7 --- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index edc568a6b47e00..1765cc24721837 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -837,14 +837,16 @@ Status MinidumpFileBuilder::AddMemoryList() { error = m_process_sp->CalculateCoreFileSaveRanges(m_save_core_options, all_core_memory_ranges); + if (error.Fail()) +return error; + + lldb_private::Progress progress_tracker("Saving Minidump File", "", all_core_memory_ranges.GetSize()); std::vector all_core_memory_vec; // Extract all the data into just a vector of data. So we can mutate this in // place. for (const auto &core_range : all_core_memory_ranges) all_core_memory_vec.push_back(core_range.data); - if (error.Fail()) -return error; // Start by saving all of the stacks and ensuring they fit under the 32b // limit. @@ -892,13 +894,13 @@ Status MinidumpFileBuilder::AddMemoryList() { } } - error = AddMemoryList_32(ranges_32); + error = AddMemoryList_32(ranges_32, progress_tracker); if (error.Fail()) return error; // Add the remaining memory as a 64b range. if (!ranges_64.empty()) { -error = AddMemoryList_64(ranges_64); +error = AddMemoryList_64(ranges_64, progress_tracker); if (error.Fail()) return error; } @@ -973,7 +975,7 @@ GetLargestRangeSize(const std::vector &ranges) { } Status MinidumpFileBuilder::AddMemoryList_32( -std::vector &ranges) { +std::vector &ranges, Progress &progressTracker) { std::vector descriptors; Status error; if (ranges.size() == 0) @@ -1024,6 +1026,8 @@ Status MinidumpFileBuilder::AddMemoryList_32( error = AddData(data_up->GetBytes(), bytes_read); if (error.Fail()) return error; + +progressTracker.Increment(); } // Add a directory that references this list @@ -1050,7 +1054,7 @@ Status MinidumpFileBuilder::AddMemoryList_32( } Status MinidumpFileBuilder::AddMemoryList_64( -std::vector &ranges) { +std::vector &ranges, Progress &progressTracker) { Status error; if (ranges.empty()) return error; @@ -1132,6 +1136,8 @@ Status MinidumpFileBuilder::AddMemoryList_64( error = AddData(data_up->GetBytes(), bytes_read); if (error.Fail()) return error; + +progressTracker.Increment(); } // Early return if there is no cleanup needed. diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h index 71001e26c00e91..77c5dba6cdf991 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -30,6 +30,7 @@ #include "lldb/Utility/Status.h" #include "lldb/lldb-forward.h" #include "lldb/lldb-types.h" +#include "lldb/Core/Progress.h" #include "llvm/BinaryFormat/Minidump.h" #include "llvm/Object/Minidump.h" @@ -79,7 +80,7 @@ class MinidumpFileBuilder { const lldb::ProcessSP &process_sp, lldb_private::SaveCoreOptions &save_core_options) : m_process_sp(process_sp), m_core_file(std::move(core_file)), -m_save_core_options(save_core_options){} +m_save_core_options(save_core_options) {} MinidumpFileBuilder(const MinidumpFileBuilder &) = delete; MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete; @@ -121,9 +122,9 @@ class MinidumpFileBuilder { lldb_private::Status AddData(const void *data, uint64_t size); // Add MemoryList stream, containing dumps of important memory segments lldb_private::Status - AddMemoryList_64(std::vector &ranges); + AddMemoryList_64(std::vector &ranges, lldb_private::Progress &progressTracker); lldb_private::Status - AddMemoryList_32(std::vector &ranges); + AddMemoryList_32(std::vector &ranges, lldb_private::Progress &progressTracker); // Update the thread list on disk with the newly emitted stack RVAs. lldb_private::Status FixThreadStacks(); lldb_private::Status FlushBufferToDisk(); >From 7d62709164be7ce39685e2c99bfda8998c9c6d39 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 11 Sep 2024 17:11:28 -0700 Subject: [PATCH 2/3] run GCF --- .../ObjectFile/Minidump/MinidumpFileBuilder.cpp| 14 -- .../ObjectFile/Minidump/Minidu
[Lldb-commits] [lldb] [LLDB][Minidump] Minidump erase file on failure (PR #108259)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/108259 >From 022173d669e84c96362024feb6512342fdd02d09 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 11 Sep 2024 10:27:31 -0700 Subject: [PATCH 1/4] Unlink file on failure --- .../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 13 + .../ObjectFile/Minidump/MinidumpFileBuilder.h | 3 +++ .../ObjectFile/Minidump/ObjectFileMinidump.cpp | 8 3 files changed, 24 insertions(+) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index edc568a6b47e00..7b7745b2953665 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -1218,3 +1218,16 @@ Status MinidumpFileBuilder::DumpFile() { return error; } + + +void MinidumpFileBuilder::DeleteFile() { + Log *log = GetLog(LLDBLog::Object); + + if (m_core_file) { +Status error = m_core_file->Close(); +if (error.Fail()) + LLDB_LOGF(log, "Failed to close minidump file: %s", error.AsCString()); + +m_core_file.reset(); + } +} diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h index 71001e26c00e91..2dcabe893b496e 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -115,6 +115,9 @@ class MinidumpFileBuilder { // Run cleanup and write all remaining bytes to file lldb_private::Status DumpFile(); + // Delete the file if it exists + void DeleteFile(); + private: // Add data to the end of the buffer, if the buffer exceeds the flush level, // trigger a flush. diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp index 5da69dd4f2ce79..282e3fdda2daf6 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -81,28 +81,33 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, if (error.Fail()) { LLDB_LOGF(log, "AddHeaderAndCalculateDirectories failed: %s", error.AsCString()); +builder.DeleteFile(); return false; }; error = builder.AddSystemInfo(); if (error.Fail()) { LLDB_LOGF(log, "AddSystemInfo failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } error = builder.AddModuleList(); if (error.Fail()) { LLDB_LOGF(log, "AddModuleList failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } error = builder.AddMiscInfo(); if (error.Fail()) { LLDB_LOGF(log, "AddMiscInfo failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } error = builder.AddThreadList(); if (error.Fail()) { LLDB_LOGF(log, "AddThreadList failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } @@ -116,6 +121,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, error = builder.AddExceptions(); if (error.Fail()) { LLDB_LOGF(log, "AddExceptions failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } @@ -124,12 +130,14 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, error = builder.AddMemoryList(); if (error.Fail()) { LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } error = builder.DumpFile(); if (error.Fail()) { LLDB_LOGF(log, "DumpFile failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } >From 8a51bd0c3663c23644334506aa817d6a28739f42 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 11 Sep 2024 10:41:15 -0700 Subject: [PATCH 2/4] Add test to ensure file is deleted upon failure --- .../TestProcessSaveCoreMinidump.py| 26 +++ 1 file changed, 26 insertions(+) diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py index 2cbe20ee10b1af..31af96102f9d22 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -493,3 +493,29 @@ def test_save_minidump_custom_save_style_duplicated_regions(self): finally: self.assertTrue(self.dbg.DeleteTarget(target)) + +@skipUnlessPlatform(["linux"]) +def minidump_deleted_on_save_failure(self): +"""Test that verifies the minidump file is deleted after an error""" + +self.build() +exe = self.getBuildArtifact("a.o
[Lldb-commits] [lldb] Avoid expression evaluation in libStdC++ std::vector synthetic children provider (PR #108414)
https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/108414 >From 6e84ab9a14e63c58e1facdbf9a695c093882b37b Mon Sep 17 00:00:00 2001 From: jeffreytan81 Date: Mon, 19 Aug 2024 10:57:35 -0700 Subject: [PATCH 1/3] Fix StartDebuggingRequestHandler/ReplModeRequestHandler in lldb-dap --- lldb/tools/lldb-dap/DAP.h| 2 -- lldb/tools/lldb-dap/lldb-dap.cpp | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 57562a14983519..7828272aa15a7d 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -192,8 +192,6 @@ struct DAP { std::mutex call_mutex; std::map inflight_reverse_requests; - StartDebuggingRequestHandler start_debugging_request_handler; - ReplModeRequestHandler repl_mode_request_handler; ReplMode repl_mode; std::string command_escape_prefix = "`"; lldb::SBFormat frame_format; diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index ea84f31aec3a6c..f50a6c17310739 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1627,12 +1627,12 @@ void request_initialize(const llvm::json::Object &request) { "lldb-dap", "Commands for managing lldb-dap."); if (GetBoolean(arguments, "supportsStartDebuggingRequest", false)) { cmd.AddCommand( -"startDebugging", &g_dap.start_debugging_request_handler, +"startDebugging", new StartDebuggingRequestHandler(), "Sends a startDebugging request from the debug adapter to the client " "to start a child debug session of the same type as the caller."); } cmd.AddCommand( - "repl-mode", &g_dap.repl_mode_request_handler, + "repl-mode", new ReplModeRequestHandler(), "Get or set the repl behavior of lldb-dap evaluation requests."); g_dap.progress_event_thread = std::thread(ProgressEventThreadFunction); >From d56c9b32f46e2c08ca134834b670bf3b1dad6c53 Mon Sep 17 00:00:00 2001 From: jeffreytan81 Date: Thu, 12 Sep 2024 09:05:33 -0700 Subject: [PATCH 2/3] Add CreateBoolValue API --- lldb/examples/synthetic/gnu_libstdcpp.py | 6 +- lldb/include/lldb/API/SBValue.h | 2 ++ lldb/source/API/SBValue.cpp | 27 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/lldb/examples/synthetic/gnu_libstdcpp.py b/lldb/examples/synthetic/gnu_libstdcpp.py index d98495b8a9df38..597298dfce36b4 100644 --- a/lldb/examples/synthetic/gnu_libstdcpp.py +++ b/lldb/examples/synthetic/gnu_libstdcpp.py @@ -473,11 +473,7 @@ def get_child_at_index(self, index): "[" + str(index) + "]", element_offset, element_type ) bit = element.GetValueAsUnsigned(0) & (1 << bit_offset) -if bit != 0: -value_expr = "(bool)true" -else: -value_expr = "(bool)false" -return self.valobj.CreateValueFromExpression("[%d]" % index, value_expr) +return self.valobj.CreateBooleanValue("[%d]" % index, bool(bit)) def update(self): try: diff --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h index bec816fb451844..aeda26cb6dd795 100644 --- a/lldb/include/lldb/API/SBValue.h +++ b/lldb/include/lldb/API/SBValue.h @@ -146,6 +146,8 @@ class LLDB_API SBValue { lldb::SBValue CreateValueFromData(const char *name, lldb::SBData data, lldb::SBType type); + lldb::SBValue CreateBoolValue(const char *name, bool value); + /// Get a child value by index from a value. /// /// Structs, unions, classes, arrays and pointers have child diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp index 273aac5ad47989..eb54213f4e60bc 100644 --- a/lldb/source/API/SBValue.cpp +++ b/lldb/source/API/SBValue.cpp @@ -645,6 +645,33 @@ lldb::SBValue SBValue::CreateValueFromData(const char *name, SBData data, return sb_value; } +lldb::SBValue SBValue::CreateBoolValue(const char *name, bool value) { + LLDB_INSTRUMENT_VA(this, name); + + lldb::SBValue sb_value; + lldb::ValueObjectSP new_value_sp; + ValueLocker locker; + lldb::ValueObjectSP value_sp(GetSP(locker)); + ProcessSP process_sp = m_opaque_sp->GetProcessSP(); + lldb::SBTarget target = GetTarget(); + if (!target.IsValid()) +return sb_value; + lldb::SBType boolean_type = target.GetBasicType(lldb::eBasicTypeBool); + lldb::TypeImplSP type_impl_sp(boolean_type.GetSP()); + if (value_sp && process_sp && type_impl_sp) { +int data_buf[1] = {value ? 1 : 0}; +lldb::SBData data = lldb::SBData::CreateDataFromSInt32Array( +process_sp->GetByteOrder(), sizeof(data_buf[0]), data_buf, +sizeof(data_buf) / sizeof(data_buf[0])); +ExecutionContext exe_ctx(value_sp->GetExecutionContextRef()); +new_value_sp = ValueObject::CreateValueObjectFromData( +name, **data, exe_ctx, type_impl_sp->GetCompilerType(tru
[Lldb-commits] [lldb] [lldb] Print a warning on checksum mismatch (PR #107968)
mstorsjo wrote: > This broke building in mingw configurations, and possibly a few others as > well. > > The `ReportWarning` function takes a pointer to a `std::once_flag`, while > this is passing it with a `llvm::once_flag`. In many configurations, these > are the same type, but in a couple ones, they're not. > > LLDB seems to be using a mixture of `std::once_flag` and `llvm::once_flag` > throughout, with nontrivial numbers of both, so there's no one obvious > preferred way to go here... Or should `ReportWarning` be extended to accept > either kinds of flags, until the implementation is settled on either of them? If there are no suggestions on a way to go here, I'll push a fix within a couple hours, that fixes compilation - probably switching the newly added code to `std::once_flag` as that's what the existing function takes. https://github.com/llvm/llvm-project/pull/107968 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
pranavk wrote: Hmm, this seems to be breaking LLVM build: ``` error: initialization of non-aggregate type 'FieldInfo' with a designated initializer list | FieldInfo this_field_info{.is_bitfield = false}; ``` https://github.com/llvm/llvm-project/pull/108343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
kazutakahirata wrote: > Hmm, this seems to be breaking LLVM build: > > ``` > error: initialization of non-aggregate type 'FieldInfo' with a designated > initializer list > | FieldInfo this_field_info{.is_bitfield = false}; > ``` FWIW, with clang-16.0.6 as the host compiler, I get: ``` lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2935:31: error: designated initializers are a C++20 extension [-Werror,-Wc++20-designator] FieldInfo this_field_info{.is_bitfield = false}; ^ 1 error generated. ``` https://github.com/llvm/llvm-project/pull/108343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Minidump erase file on failure (PR #108259)
@@ -55,6 +55,21 @@ size_t ObjectFileMinidump::GetModuleSpecifications( return 0; } +struct SaveCoreRequest { jeffreytan81 wrote: Per our naming convention, let's call this RAII class `FileRemoveHolder/DumpFailRemoveHolder` or something similar, then the reader immediately know it is a RAII class. The current `SaveCoreRequest` is not indicating its purpose. https://github.com/llvm/llvm-project/pull/108259 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Minidump erase file on failure (PR #108259)
@@ -55,6 +55,21 @@ size_t ObjectFileMinidump::GetModuleSpecifications( return 0; } +struct SaveCoreRequest { + SaveCoreRequest(MinidumpFileBuilder &builder) : m_builder(builder) {} + + ~SaveCoreRequest() { +if (!m_success) + m_builder.DeleteFile(); jeffreytan81 wrote: We should emit visible error message telling end user that saving minidump has failed instead of silently fail and remove minidump file. https://github.com/llvm/llvm-project/pull/108259 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 5d17293 - [lldb] Fix a warning
Author: Kazu Hirata Date: 2024-09-12T10:43:32-07:00 New Revision: 5d17293caaf0f62ea94fecc137b9b6f07c659dac URL: https://github.com/llvm/llvm-project/commit/5d17293caaf0f62ea94fecc137b9b6f07c659dac DIFF: https://github.com/llvm/llvm-project/commit/5d17293caaf0f62ea94fecc137b9b6f07c659dac.diff LOG: [lldb] Fix a warning This patch fixes: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2935:31: error: designated initializers are a C++20 extension [-Werror,-Wc++20-designator] Added: Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Removed: diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 5b679bd5a613e3..70540fe7fada68 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2932,7 +2932,8 @@ void DWARFASTParserClang::ParseSingleMember( last_field_info = this_field_info; last_field_info.SetIsBitfield(true); } else { -FieldInfo this_field_info{.is_bitfield = false}; +FieldInfo this_field_info; +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 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
kazutakahirata wrote: I've fixed the warning with 5d17293caaf0f62ea94fecc137b9b6f07c659dac. https://github.com/llvm/llvm-project/pull/108343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Support inspecting memory (PR #104317)
vogelsgesang wrote: Hi @clayborg, sorry for the late reply - got knocked out by Covid for a while 😷 > Let me know what you think of my idea to always just show the load address of > any SBValue. I think that is the best for LLDB, but I am open to opinions. I did check the other lldb-based debug adapter. See https://github.com/vadimcn/codelldb/blob/05502bf75e4e7878a99b0bf0a7a81bba2922cbe3/adapter/codelldb/src/debug_session/variables.rs#L280. It turns out, they also always use the load address of the value itself, without dereferencing pointers. Given this prior art, I will update this commit to do the same. I will hopefully find time to update this pull request early next week https://github.com/llvm/llvm-project/pull/104317 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
https://github.com/cmtice updated https://github.com/llvm/llvm-project/pull/107485 >From 15541f354decf80586d590db9f9cb353be04b122 Mon Sep 17 00:00:00 2001 From: Caroline Tice Date: Thu, 5 Sep 2024 15:51:35 -0700 Subject: [PATCH 1/3] [lldb-dap] Add feature to remember last non-empty expression. Update lldb-dap so if the user just presses return, which sends an empty expression, it re-evaluates the most recent non-empty expression/command. Also udpated test to test this case. --- lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py | 3 +++ lldb/tools/lldb-dap/lldb-dap.cpp | 8 2 files changed, 11 insertions(+) diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py index 29548a835c6919..9ed0fc564268a7 100644 --- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py +++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py @@ -60,7 +60,10 @@ def run_test_evaluate_expressions( # Expressions at breakpoint 1, which is in main self.assertEvaluate("var1", "20") +# Empty expression should equate to the previous expression. +self.assertEvaluate("", "20") self.assertEvaluate("var2", "21") +self.assertEvaluate("", "21") self.assertEvaluate("static_int", "42") self.assertEvaluate("non_static_int", "43") self.assertEvaluate("struct1.foo", "15") diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index c5c4b09f15622b..a6a701dc2219fa 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1363,6 +1363,14 @@ void request_evaluate(const llvm::json::Object &request) { lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments); std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); + static std::string last_nonempty_expression; + + // Remember the last non-empty expression from the user, and use that if + // the current expression is empty (i.e. the user hit plain 'return'). + if (!expression.empty()) +last_nonempty_expression = expression; + else +expression = last_nonempty_expression; if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) == ExpressionContext::Command) { >From e35928e08f792163dd4886e797bc6de3d16ea6e6 Mon Sep 17 00:00:00 2001 From: Caroline Tice Date: Fri, 6 Sep 2024 10:02:18 -0700 Subject: [PATCH 2/3] [lldb] Add feature to remember last non-empty expression Make last_nonempty_spression part of DAP struct rather than a static variable. --- lldb/tools/lldb-dap/DAP.h| 1 + lldb/tools/lldb-dap/lldb-dap.cpp | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index f4fdec6e895ad1..4220c15d3ae70d 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -205,6 +205,7 @@ struct DAP { std::string command_escape_prefix = "`"; lldb::SBFormat frame_format; lldb::SBFormat thread_format; + std::string last_nonempty_expression; DAP(); ~DAP(); diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index a6a701dc2219fa..d3728df9183aa1 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1363,14 +1363,13 @@ void request_evaluate(const llvm::json::Object &request) { lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments); std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); - static std::string last_nonempty_expression; // Remember the last non-empty expression from the user, and use that if // the current expression is empty (i.e. the user hit plain 'return'). if (!expression.empty()) -last_nonempty_expression = expression; +g_dap.last_nonempty_expression = expression; else -expression = last_nonempty_expression; +expression = g_dap.last_nonempty_expression; if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) == ExpressionContext::Command) { >From 616017152f3f0611462e9863273754036b52f7eb Mon Sep 17 00:00:00 2001 From: Caroline Tice Date: Thu, 12 Sep 2024 10:52:32 -0700 Subject: [PATCH 3/3] [lldb-dap] Add feature to remember last non-empty expression Update to handle commands & variables separately: empty command expressions are passed to the CommandIntepreter to handle as it normally does; empty variable expressions are updated to use the last non-empty variable expression, if the last expression was a variable (not a command). Also updated the test case to test these cases properly, and added a 'memory read' followed by an empty expression, to make sure it handles that sequence correctly. --- .../lldb-dap/evaluate/TestDAP_evaluate.py | 16 +++-- ...
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
@@ -60,7 +60,10 @@ def run_test_evaluate_expressions( # Expressions at breakpoint 1, which is in main self.assertEvaluate("var1", "20") +# Empty expression should equate to the previous expression. cmtice wrote: Done https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
cmtice wrote: > I wrote a fairly long comment on Friday, but I don't see it anymore so, it > looks like github has swallowed it. Here's my reconstruction of it: > > LLDB commands have the notion of a "repeat command", which can sometimes be > more complicated than just running the same string over and over again. This > can be e.g. seen with the `memory read` command, which doesn't just print the > same memory again -- it actually prints the memory that comes _after_ it (a > pretty nifty feature actually): > > ``` > (lldb) memory read argv > 0x7fffd8a8: 49 dc ff ff ff 7f 00 00 00 00 00 00 00 00 00 00 > I... > 0x7fffd8b8: 6c dc ff ff ff 7f 00 00 ae dc ff ff ff 7f 00 00 > l... > (lldb) > 0x7fffd8c8: c4 dc ff ff ff 7f 00 00 cf dc ff ff ff 7f 00 00 > > 0x7fffd8d8: 09 dd ff ff ff 7f 00 00 18 dd ff ff ff 7f 00 00 > > (lldb) > 0x7fffd8e8: 40 dd ff ff ff 7f 00 00 7d dd ff ff ff 7f 00 00 > @...}... > 0x7fffd8f8: 81 e5 ff ff ff 7f 00 00 9d e5 ff ff ff 7f 00 00 > > (lldb) memory read argv > 0x7fffd8a8: 49 dc ff ff ff 7f 00 00 00 00 00 00 00 00 00 00 > I... > 0x7fffd8b8: 6c dc ff ff ff 7f 00 00 ae dc ff ff ff 7f 00 00 > l... > ``` > > Storing (and repeating) the command string in lldb-dap would break this > behavior. What we'd ideally want is to actually take the empty string and > pass it to lldb's command interpreter so that the proper repeat logic kicks > in. > > The thing which makes this tricky (but not too complicated I think) is that > lldb-dap multiplexes expression commands and CLI commands into the same > string (the `DetectExpressionContext` does the demultiplexing). > > I think the proper repeat handling could be two things about each command: > > * the type ("expression context") of the command > * the command string itself, if the command was not a CLI command (so we can > repeat the expression) > > Then, when we get an empty string, we check the type of the previous command: > > * if it was a CLI command (`ExpressionContext::Command`), we change send the > empty string to lldb command interpreter, so that it does the right thing > * otherwise, we take the expression string and re-evaluate it (like you do > here). I think I have done what you requested now. https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
@@ -1363,6 +1363,14 @@ void request_evaluate(const llvm::json::Object &request) { lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments); std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); + static std::string last_nonempty_expression; + + // Remember the last non-empty expression from the user, and use that if + // the current expression is empty (i.e. the user hit plain 'return'). + if (!expression.empty()) +last_nonempty_expression = expression; + else +expression = last_nonempty_expression; if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) == cmtice wrote: Done. https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
@@ -1364,6 +1364,13 @@ void request_evaluate(const llvm::json::Object &request) { std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); + // Remember the last non-empty expression from the user, and use that if + // the current expression is empty (i.e. the user hit plain 'return'). + if (!expression.empty()) +g_dap.last_nonempty_expression = expression; + else cmtice wrote: The SBCommandIntepreterRunOptions don't seem to be used on this execution path, so I couldn't make use of this. https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
cmtice wrote: I think I have addressed all the review comments; please take another look. https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
github-actions[bot] wrote: :warning: Python code formatter, darker found issues in your code. :warning: You can test this locally with the following command: ``bash darker --check --diff -r 247d3ea843cb20d8d75ec781cd603c8ececf8934...616017152f3f0611462e9863273754036b52f7eb lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py `` View the diff from darker here. ``diff --- TestDAP_evaluate.py 2024-09-12 17:52:32.00 + +++ TestDAP_evaluate.py 2024-09-12 17:59:35.638421 + @@ -61,15 +61,15 @@ # Expressions at breakpoint 1, which is in main self.assertEvaluate("var1", "20") # Empty expression should equate to the previous expression. if context == "variable": - self.assertEvaluate("", "20") +self.assertEvaluate("", "20") self.assertEvaluate("var2", "21") if context == "variable": - self.assertEvaluate("", "21") - self.assertEvaluate("", "21") +self.assertEvaluate("", "21") +self.assertEvaluate("", "21") self.assertEvaluate("static_int", "42") self.assertEvaluate("non_static_int", "43") self.assertEvaluate("struct1.foo", "15") self.assertEvaluate("struct2->foo", "16") @@ -198,15 +198,19 @@ self.continue_to_next_stop() self.assertEvaluate("my_bool_vec", "size=2") # Test memory read, especially with 'empty' repeat commands. if context == "repl": - self.continue_to_next_stop() - self.assertEvaluate("memory read &my_ints", - ".*00 00 00 00 02 00 00 00 04 00 00 00 06 00 00 00.*\n.*08 00 00 00 0a 00 00 00 0c 00 00 00 0e 00 00 00.*\n") - self.assertEvaluate("", - ".*10 00 00 00 12 00 00 00 14 00 00 00 16 00 00 00.*\n.*18 00 00 00 1a 00 00 00 1c 00 00 00.*\n") +self.continue_to_next_stop() +self.assertEvaluate( +"memory read &my_ints", +".*00 00 00 00 02 00 00 00 04 00 00 00 06 00 00 00.*\n.*08 00 00 00 0a 00 00 00 0c 00 00 00 0e 00 00 00.*\n", +) +self.assertEvaluate( +"", +".*10 00 00 00 12 00 00 00 14 00 00 00 16 00 00 00.*\n.*18 00 00 00 1a 00 00 00 1c 00 00 00.*\n", +) @skipIfWindows def test_generic_evaluate_expressions(self): # Tests context-less expression evaluations self.run_test_evaluate_expressions(enableAutoVariableSummaries=False) `` https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 247d3ea843cb20d8d75ec781cd603c8ececf8934 616017152f3f0611462e9863273754036b52f7eb --extensions cpp,h -- lldb/test/API/tools/lldb-dap/evaluate/main.cpp lldb/tools/lldb-dap/DAP.h lldb/tools/lldb-dap/LLDBUtils.cpp lldb/tools/lldb-dap/lldb-dap.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 322899804b..19a65e6b32 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1366,8 +1366,8 @@ void request_evaluate(const llvm::json::Object &request) { if (context == "repl" && ((!expression.empty() && - g_dap.DetectExpressionContext(frame, expression) == - ExpressionContext::Command) || +g_dap.DetectExpressionContext(frame, expression) == +ExpressionContext::Command) || (expression.empty() && g_dap.last_expression_context == ExpressionContext::Command))) { // If the current expression is empty, and the last expression context was `` https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Avoid expression evaluation in libStdC++ std::vector synthetic children provider (PR #108414)
jimingham wrote: The goal here is excellent! Not only is this a performance problem but if you call the version of GetValueFromExpression that doesn't take an expression options - as the code you are replacing does - you get the default value for "TryAllThreads" - which is `true`. So if you are unlucky and the expression takes a little bit of time (maybe your machine is heavily loaded or something) then printing this summary could cause other threads to make progress out from under you. In a GUI, that can result in the not helpful behavior where you stop at some point in Thread A, switch to Thread B - at which point the GUI loads the variables and runs the formatter for you and in doing so Thread A gets a chance to run. Then you switch back to Thread A and it wasn't where you left it. Bad debugger! We should not use expressions in data formatters because they make them slow as you have seen. But we for sure should never call an expression with `TryAllThreads` set to true in a data formatter. If you are going to use expressions at all you should do it on only one thread and make sure that along the path of the function you call nobody takes locks that might deadlock the expression against other threads... But we should try hard not to make that necessary, and shouldn't do so in ones we ship. Another general rule is that if you are adding a significant algorithm to an SB API implementation, you should really make an API in the underlying lldb_private API and then call that. We used to have all sorts of SB API specific implementations, which leads to code duplication and subtle differences in behavior depending on whether you find your way to the SB or private implementation. We try not to do that anymore. Can you do this instead with a ValueObject::CreateBooleanValue, and then just wrap that in the SBValue::CreateBooleanValue call? https://github.com/llvm/llvm-project/pull/108414 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Avoid expression evaluation in libStdC++ std::vector synthetic children provider (PR #108414)
https://github.com/jimingham requested changes to this pull request. This is good except the implementation should be in ValueObject not SBValue. https://github.com/llvm/llvm-project/pull/108414 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Prevent unnamed bitfield creation in the presence of overlapping fields (PR #108343)
Michael137 wrote: > I've fixed the warning with 5d17293caaf0f62ea94fecc137b9b6f07c659dac. > > Thanks! https://github.com/llvm/llvm-project/pull/108343 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][lldb-dap] Added readMemory and WriteMemory, var type correction (PR #108036)
vogelsgesang wrote: Hi @jennphilqc, Great minds think alike - I actually also implemented the `readMemory` request in #104317. Thankfully, we didn't literally do the same, duplicated work * your PR also implements `writeMemory` which my PR is still lacking. Here your PR is clearly further along * my PR already includes test cases which still seem to be lacking in your PR. When it comes to supporting `readMemory`, my PR seems to be further along Given this, I think we should take the best parts of both PRs 🙂 What do you think about the following path forward? * rebase this PR on top of #104317 * scope this PR down to `writeMemory` * modify this PR to also extend the test cases in `TestDAP_memory.py` (added by my PR) to introduce test coverage also for `writeMemory` Also, I noticed that you did not introduce any `memoryReference`s as part of your pull request. With your PR, how can a user even open the hex-viewer in VS-Code? Is there some way which I am not aware of to open the hex-viewer without a `memoryReference`? Asking, because I was considering introducing a "View memory address" via TypeScript, similar to [what VSCodeLLDB does](https://github.com/vadimcn/codelldb/blob/05502bf75e4e7878a99b0bf0a7a81bba2922cbe3/extension/main.ts#L476) - but if there already is some other way to pop up the memory-viewer, doing so would be unnecessary https://github.com/llvm/llvm-project/pull/108036 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make sure TestDAP_subtleFrames actually uses libc++ (PR #108227)
https://github.com/vogelsgesang approved this pull request. Thanks for fixing this! 🙂 https://github.com/llvm/llvm-project/pull/108227 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Provide `declarationLocation` for variables (PR #102928)
vogelsgesang wrote: Hi @clayborg, Are you still planning to review this (as requested by @walter-erquinigo)? Or are you fine with me landing this change without your review? https://github.com/llvm/llvm-project/pull/102928 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
@@ -191,6 +198,14 @@ def run_test_evaluate_expressions( self.continue_to_next_stop() self.assertEvaluate("my_bool_vec", "size=2") +# Test memory read, especially with 'empty' repeat commands. +if context == "repl": + self.continue_to_next_stop() + self.assertEvaluate("memory read &my_ints", + ".*00 00 00 00 02 00 00 00 04 00 00 00 06 00 00 00.*\n.*08 00 00 00 0a 00 00 00 0c 00 00 00 0e 00 00 00.*\n") + self.assertEvaluate("", + ".*10 00 00 00 12 00 00 00 14 00 00 00 16 00 00 00.*\n.*18 00 00 00 1a 00 00 00 1c 00 00 00.*\n") walter-erquinigo wrote: I don't think that this will reliably be the same across systems. Something that would be better would be to read from an `uint8_t` array and just read one byte at a time. That should work anywhere. https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
@@ -1364,8 +1364,20 @@ void request_evaluate(const llvm::json::Object &request) { std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); - if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) == - ExpressionContext::Command) { + if (context == "repl" && + ((!expression.empty() && + g_dap.DetectExpressionContext(frame, expression) == + ExpressionContext::Command) || + (expression.empty() && +g_dap.last_expression_context == ExpressionContext::Command))) { +// If the current expression is empty, and the last expression context was +// for a command, pass the empty expression along to the +// CommandInterpreter, to repeat the previous command. Also set the +// expression context properly for the next (possibly empty) expression. +g_dap.last_expression_context = ExpressionContext::Command; +// Since the current expression context is not for a variable, clear the +// last_nonempty_var_expression field. +g_dap.last_nonempty_var_expression.clear(); walter-erquinigo wrote: It would be cleaner if this gets stored in a sort of discriminated union: LastExpressionInfo: - Case Command (no metadata) - Case Var (expression) this way it's easier to maintain these two variables correctly in sync https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
@@ -1364,8 +1364,20 @@ void request_evaluate(const llvm::json::Object &request) { std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); - if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) == - ExpressionContext::Command) { + if (context == "repl" && + ((!expression.empty() && + g_dap.DetectExpressionContext(frame, expression) == + ExpressionContext::Command) || + (expression.empty() && +g_dap.last_expression_context == ExpressionContext::Command))) { +// If the current expression is empty, and the last expression context was +// for a command, pass the empty expression along to the walter-erquinigo wrote: ```suggestion // for a command, pass the empty expression along to the ``` https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb]Implement LLDB Telemetry (PR #98528)
@@ -754,6 +760,7 @@ class Debugger : public std::enable_shared_from_this, uint32_t m_interrupt_requested = 0; ///< Tracks interrupt requests std::mutex m_interrupt_mutex; + std::shared_ptr m_telemeter; oontvoo wrote: done https://github.com/llvm/llvm-project/pull/98528 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a81a4b2 - [lldb] Fix building with lldb::once_flag != std::once_flag
Author: Martin Storsjö Date: 2024-09-12T22:26:02+03:00 New Revision: a81a4b2a7ac2d0b8195bb008b2c0f464cfbda223 URL: https://github.com/llvm/llvm-project/commit/a81a4b2a7ac2d0b8195bb008b2c0f464cfbda223 DIFF: https://github.com/llvm/llvm-project/commit/a81a4b2a7ac2d0b8195bb008b2c0f464cfbda223.diff LOG: [lldb] Fix building with lldb::once_flag != std::once_flag This fixes build breakage on e.g. mingw platforms since ffa2f539ae2a4e79c01b3d54f8b12c63d8781a0c. The Debugger::ReportWarning function takes a pointer to a std::once_flag. On many platforms, llvm::once_flag is a typedef for std::once_flag, but on others, llvm::once_flag is a custom reimplementation. Added: Modified: lldb/include/lldb/Core/SourceManager.h Removed: diff --git a/lldb/include/lldb/Core/SourceManager.h b/lldb/include/lldb/Core/SourceManager.h index e38627182a9690..d929f7bd9bf221 100644 --- a/lldb/include/lldb/Core/SourceManager.h +++ b/lldb/include/lldb/Core/SourceManager.h @@ -74,7 +74,7 @@ class SourceManager { const Checksum &GetChecksum() const { return m_checksum; } -llvm::once_flag &GetChecksumWarningOnceFlag() { +std::once_flag &GetChecksumWarningOnceFlag() { return m_checksum_warning_once_flag; } @@ -92,7 +92,7 @@ class SourceManager { Checksum m_checksum; /// Once flag for emitting a checksum mismatch warning. -llvm::once_flag m_checksum_warning_once_flag; +std::once_flag m_checksum_warning_once_flag; // Keep the modification time that this file data is valid for llvm::sys::TimePoint<> m_mod_time; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Print a warning on checksum mismatch (PR #107968)
mstorsjo wrote: I pushed a fix in a81a4b2a7ac2d0b8195bb008b2c0f464cfbda223. https://github.com/llvm/llvm-project/pull/107968 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Minidump erase file on failure (PR #108259)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/108259 >From 022173d669e84c96362024feb6512342fdd02d09 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 11 Sep 2024 10:27:31 -0700 Subject: [PATCH 1/5] Unlink file on failure --- .../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 13 + .../ObjectFile/Minidump/MinidumpFileBuilder.h | 3 +++ .../ObjectFile/Minidump/ObjectFileMinidump.cpp | 8 3 files changed, 24 insertions(+) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index edc568a6b47e00..7b7745b2953665 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -1218,3 +1218,16 @@ Status MinidumpFileBuilder::DumpFile() { return error; } + + +void MinidumpFileBuilder::DeleteFile() { + Log *log = GetLog(LLDBLog::Object); + + if (m_core_file) { +Status error = m_core_file->Close(); +if (error.Fail()) + LLDB_LOGF(log, "Failed to close minidump file: %s", error.AsCString()); + +m_core_file.reset(); + } +} diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h index 71001e26c00e91..2dcabe893b496e 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -115,6 +115,9 @@ class MinidumpFileBuilder { // Run cleanup and write all remaining bytes to file lldb_private::Status DumpFile(); + // Delete the file if it exists + void DeleteFile(); + private: // Add data to the end of the buffer, if the buffer exceeds the flush level, // trigger a flush. diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp index 5da69dd4f2ce79..282e3fdda2daf6 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -81,28 +81,33 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, if (error.Fail()) { LLDB_LOGF(log, "AddHeaderAndCalculateDirectories failed: %s", error.AsCString()); +builder.DeleteFile(); return false; }; error = builder.AddSystemInfo(); if (error.Fail()) { LLDB_LOGF(log, "AddSystemInfo failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } error = builder.AddModuleList(); if (error.Fail()) { LLDB_LOGF(log, "AddModuleList failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } error = builder.AddMiscInfo(); if (error.Fail()) { LLDB_LOGF(log, "AddMiscInfo failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } error = builder.AddThreadList(); if (error.Fail()) { LLDB_LOGF(log, "AddThreadList failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } @@ -116,6 +121,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, error = builder.AddExceptions(); if (error.Fail()) { LLDB_LOGF(log, "AddExceptions failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } @@ -124,12 +130,14 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, error = builder.AddMemoryList(); if (error.Fail()) { LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } error = builder.DumpFile(); if (error.Fail()) { LLDB_LOGF(log, "DumpFile failed: %s", error.AsCString()); +builder.DeleteFile(); return false; } >From 8a51bd0c3663c23644334506aa817d6a28739f42 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 11 Sep 2024 10:41:15 -0700 Subject: [PATCH 2/5] Add test to ensure file is deleted upon failure --- .../TestProcessSaveCoreMinidump.py| 26 +++ 1 file changed, 26 insertions(+) diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py index 2cbe20ee10b1af..31af96102f9d22 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -493,3 +493,29 @@ def test_save_minidump_custom_save_style_duplicated_regions(self): finally: self.assertTrue(self.dbg.DeleteTarget(target)) + +@skipUnlessPlatform(["linux"]) +def minidump_deleted_on_save_failure(self): +"""Test that verifies the minidump file is deleted after an error""" + +self.build() +exe = self.getBuildArtifact("a.o
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
@@ -1364,8 +1364,20 @@ void request_evaluate(const llvm::json::Object &request) { std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); - if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) == - ExpressionContext::Command) { + if (context == "repl" && + ((!expression.empty() && + g_dap.DetectExpressionContext(frame, expression) == + ExpressionContext::Command) || + (expression.empty() && +g_dap.last_expression_context == ExpressionContext::Command))) { +// If the current expression is empty, and the last expression context was +// for a command, pass the empty expression along to the +// CommandInterpreter, to repeat the previous command. Also set the +// expression context properly for the next (possibly empty) expression. +g_dap.last_expression_context = ExpressionContext::Command; +// Since the current expression context is not for a variable, clear the +// last_nonempty_var_expression field. +g_dap.last_nonempty_var_expression.clear(); cmtice wrote: I don't think a union will work because last_expression_context is needed and used in all empty expression cases, while for the variable case we ALSO need the last_nonempty_var_expression. https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
https://github.com/cmtice updated https://github.com/llvm/llvm-project/pull/107485 >From 15541f354decf80586d590db9f9cb353be04b122 Mon Sep 17 00:00:00 2001 From: Caroline Tice Date: Thu, 5 Sep 2024 15:51:35 -0700 Subject: [PATCH 1/4] [lldb-dap] Add feature to remember last non-empty expression. Update lldb-dap so if the user just presses return, which sends an empty expression, it re-evaluates the most recent non-empty expression/command. Also udpated test to test this case. --- lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py | 3 +++ lldb/tools/lldb-dap/lldb-dap.cpp | 8 2 files changed, 11 insertions(+) diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py index 29548a835c6919..9ed0fc564268a7 100644 --- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py +++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py @@ -60,7 +60,10 @@ def run_test_evaluate_expressions( # Expressions at breakpoint 1, which is in main self.assertEvaluate("var1", "20") +# Empty expression should equate to the previous expression. +self.assertEvaluate("", "20") self.assertEvaluate("var2", "21") +self.assertEvaluate("", "21") self.assertEvaluate("static_int", "42") self.assertEvaluate("non_static_int", "43") self.assertEvaluate("struct1.foo", "15") diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index c5c4b09f15622b..a6a701dc2219fa 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1363,6 +1363,14 @@ void request_evaluate(const llvm::json::Object &request) { lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments); std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); + static std::string last_nonempty_expression; + + // Remember the last non-empty expression from the user, and use that if + // the current expression is empty (i.e. the user hit plain 'return'). + if (!expression.empty()) +last_nonempty_expression = expression; + else +expression = last_nonempty_expression; if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) == ExpressionContext::Command) { >From e35928e08f792163dd4886e797bc6de3d16ea6e6 Mon Sep 17 00:00:00 2001 From: Caroline Tice Date: Fri, 6 Sep 2024 10:02:18 -0700 Subject: [PATCH 2/4] [lldb] Add feature to remember last non-empty expression Make last_nonempty_spression part of DAP struct rather than a static variable. --- lldb/tools/lldb-dap/DAP.h| 1 + lldb/tools/lldb-dap/lldb-dap.cpp | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index f4fdec6e895ad1..4220c15d3ae70d 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -205,6 +205,7 @@ struct DAP { std::string command_escape_prefix = "`"; lldb::SBFormat frame_format; lldb::SBFormat thread_format; + std::string last_nonempty_expression; DAP(); ~DAP(); diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index a6a701dc2219fa..d3728df9183aa1 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1363,14 +1363,13 @@ void request_evaluate(const llvm::json::Object &request) { lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments); std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); - static std::string last_nonempty_expression; // Remember the last non-empty expression from the user, and use that if // the current expression is empty (i.e. the user hit plain 'return'). if (!expression.empty()) -last_nonempty_expression = expression; +g_dap.last_nonempty_expression = expression; else -expression = last_nonempty_expression; +expression = g_dap.last_nonempty_expression; if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) == ExpressionContext::Command) { >From 616017152f3f0611462e9863273754036b52f7eb Mon Sep 17 00:00:00 2001 From: Caroline Tice Date: Thu, 12 Sep 2024 10:52:32 -0700 Subject: [PATCH 3/4] [lldb-dap] Add feature to remember last non-empty expression Update to handle commands & variables separately: empty command expressions are passed to the CommandIntepreter to handle as it normally does; empty variable expressions are updated to use the last non-empty variable expression, if the last expression was a variable (not a command). Also updated the test case to test these cases properly, and added a 'memory read' followed by an empty expression, to make sure it handles that sequence correctly. --- .../lldb-dap/evaluate/TestDAP_evaluate.py | 16 +++-- ...
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
@@ -191,6 +198,14 @@ def run_test_evaluate_expressions( self.continue_to_next_stop() self.assertEvaluate("my_bool_vec", "size=2") +# Test memory read, especially with 'empty' repeat commands. +if context == "repl": + self.continue_to_next_stop() + self.assertEvaluate("memory read &my_ints", + ".*00 00 00 00 02 00 00 00 04 00 00 00 06 00 00 00.*\n.*08 00 00 00 0a 00 00 00 0c 00 00 00 0e 00 00 00.*\n") + self.assertEvaluate("", + ".*10 00 00 00 12 00 00 00 14 00 00 00 16 00 00 00.*\n.*18 00 00 00 1a 00 00 00 1c 00 00 00.*\n") cmtice wrote: Done. https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
@@ -1364,8 +1364,20 @@ void request_evaluate(const llvm::json::Object &request) { std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); - if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) == - ExpressionContext::Command) { + if (context == "repl" && + ((!expression.empty() && + g_dap.DetectExpressionContext(frame, expression) == + ExpressionContext::Command) || + (expression.empty() && +g_dap.last_expression_context == ExpressionContext::Command))) { +// If the current expression is empty, and the last expression context was +// for a command, pass the empty expression along to the cmtice wrote: Done. https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Minidump erase file on failure (PR #108259)
@@ -55,6 +55,21 @@ size_t ObjectFileMinidump::GetModuleSpecifications( return 0; } +struct SaveCoreRequest { + SaveCoreRequest(MinidumpFileBuilder &builder) : m_builder(builder) {} + + ~SaveCoreRequest() { +if (!m_success) + m_builder.DeleteFile(); Jlalond wrote: I'll have to get the patch, it's one of my SBSaveCore API changes, but we now print the error when Plugin Manager returns, we could print twice, one specific for minidump https://github.com/llvm/llvm-project/pull/108259 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Add breakpoint stop reasons to the minidump. (PR #108448)
https://github.com/Jlalond created https://github.com/llvm/llvm-project/pull/108448 Recently my coworker @jeffreytan81 pointed out that Minidumps don't show breakpoints when collected. This was prior blocked because Minidumps could only contain 1 exception, now that we support N signals/sections we can save all the threads stopped on breakpoints. >From adcc5e8065d2a1b7edf877bd74de9cebd2f84cc9 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 12 Sep 2024 13:10:58 -0700 Subject: [PATCH 1/2] Create set for the stop reasons we collect, and add breakpoint to it. --- .../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 13 +++-- .../ObjectFile/Minidump/MinidumpFileBuilder.h | 4 +++- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index edc568a6b47e00..38e71e0e485d55 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -75,8 +75,7 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() { StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); if (stop_info_sp) { const StopReason &stop_reason = stop_info_sp->GetStopReason(); - if (stop_reason == StopReason::eStopReasonException || - stop_reason == StopReason::eStopReasonSignal) + if (m_thread_stop_reasons.count(stop_reason) > 0) m_expected_directories++; } } @@ -686,16 +685,10 @@ Status MinidumpFileBuilder::AddExceptions() { for (const ThreadSP &thread_sp : thread_list) { StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); bool add_exception = false; -if (stop_info_sp) { - switch (stop_info_sp->GetStopReason()) { - case eStopReasonSignal: - case eStopReasonException: +if (stop_info_sp && m_thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) { add_exception = true; -break; - default: -break; - } } + if (add_exception) { constexpr size_t minidump_exception_size = sizeof(llvm::minidump::ExceptionStream); diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h index 71001e26c00e91..569ba7fb07d871 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -168,7 +168,9 @@ class MinidumpFileBuilder { m_tid_to_reg_ctx; std::unordered_set m_saved_stack_ranges; lldb::FileUP m_core_file; - lldb_private::SaveCoreOptions m_save_core_options; + lldb_private::SaveCoreOptions m_save_core_options; + const std::unordered_set m_thread_stop_reasons = {lldb::StopReason::eStopReasonException, lldb::StopReason::eStopReasonSignal, + lldb::StopReason::eStopReasonBreakpoint }; }; #endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H >From 030b74c9e763c86d217da39487478e7cf2dff074 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 12 Sep 2024 13:30:04 -0700 Subject: [PATCH 2/2] Make const set with the stop reasons --- .../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 11 +-- .../Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h | 4 +--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 38e71e0e485d55..0b69f1b8205610 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -54,6 +54,13 @@ using namespace lldb; using namespace lldb_private; using namespace llvm::minidump; +// Set of all the stop reasons minidumps will collect. +const std::unordered_set MinidumpFileBuilder::thread_stop_reasons { + lldb::StopReason::eStopReasonException, + lldb::StopReason::eStopReasonSignal, + lldb::StopReason::eStopReasonBreakpoint, +}; + Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() { // First set the offset on the file, and on the bytes saved m_saved_data_size = HEADER_SIZE; @@ -75,7 +82,7 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() { StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); if (stop_info_sp) { const StopReason &stop_reason = stop_info_sp->GetStopReason(); - if (m_thread_stop_reasons.count(stop_reason) > 0) + if (thread_stop_reasons.count(stop_reason) > 0) m_expected_directories++; } } @@ -685,7 +692,7 @@ Status MinidumpFileBuilder::AddExceptions() { for (const ThreadSP &thread_sp : thread_list) { StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); bool add_exception = false; -if (stop_info_sp && m_thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) { +if (
[Lldb-commits] [lldb] [LLDB][Minidump] Add breakpoint stop reasons to the minidump. (PR #108448)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jacob Lalonde (Jlalond) Changes Recently my coworker @jeffreytan81 pointed out that Minidumps don't show breakpoints when collected. This was prior blocked because Minidumps could only contain 1 exception, now that we support N signals/sections we can save all the threads stopped on breakpoints. --- Full diff: https://github.com/llvm/llvm-project/pull/108448.diff 2 Files Affected: - (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp (+10-10) - (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h (+2-2) ``diff diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index edc568a6b47e00..0b69f1b8205610 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -54,6 +54,13 @@ using namespace lldb; using namespace lldb_private; using namespace llvm::minidump; +// Set of all the stop reasons minidumps will collect. +const std::unordered_set MinidumpFileBuilder::thread_stop_reasons { + lldb::StopReason::eStopReasonException, + lldb::StopReason::eStopReasonSignal, + lldb::StopReason::eStopReasonBreakpoint, +}; + Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() { // First set the offset on the file, and on the bytes saved m_saved_data_size = HEADER_SIZE; @@ -75,8 +82,7 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() { StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); if (stop_info_sp) { const StopReason &stop_reason = stop_info_sp->GetStopReason(); - if (stop_reason == StopReason::eStopReasonException || - stop_reason == StopReason::eStopReasonSignal) + if (thread_stop_reasons.count(stop_reason) > 0) m_expected_directories++; } } @@ -686,16 +692,10 @@ Status MinidumpFileBuilder::AddExceptions() { for (const ThreadSP &thread_sp : thread_list) { StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); bool add_exception = false; -if (stop_info_sp) { - switch (stop_info_sp->GetStopReason()) { - case eStopReasonSignal: - case eStopReasonException: +if (stop_info_sp && thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) { add_exception = true; -break; - default: -break; - } } + if (add_exception) { constexpr size_t minidump_exception_size = sizeof(llvm::minidump::ExceptionStream); diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h index 71001e26c00e91..488eec7b9282bc 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -168,7 +168,7 @@ class MinidumpFileBuilder { m_tid_to_reg_ctx; std::unordered_set m_saved_stack_ranges; lldb::FileUP m_core_file; - lldb_private::SaveCoreOptions m_save_core_options; + lldb_private::SaveCoreOptions m_save_core_options; + static const std::unordered_set thread_stop_reasons; }; - #endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H `` https://github.com/llvm/llvm-project/pull/108448 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Add breakpoint stop reasons to the minidump. (PR #108448)
@@ -54,6 +54,13 @@ using namespace lldb; using namespace lldb_private; using namespace llvm::minidump; +// Set of all the stop reasons minidumps will collect. Jlalond wrote: @labath Hey Pavel, is there a more elegant way to define a constant set? I want to define these enums in a unified place so our two uses cases don't diverge, but I don't know a better way https://github.com/llvm/llvm-project/pull/108448 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Add breakpoint stop reasons to the minidump. (PR #108448)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 030c6da7af826b641db005be925b20f956c3a6bb 030b74c9e763c86d217da39487478e7cf2dff074 --extensions h,cpp -- lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h `` View the diff from clang-format here. ``diff diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 0b69f1b820..492df463a3 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -55,11 +55,12 @@ using namespace lldb_private; using namespace llvm::minidump; // Set of all the stop reasons minidumps will collect. -const std::unordered_set MinidumpFileBuilder::thread_stop_reasons { - lldb::StopReason::eStopReasonException, - lldb::StopReason::eStopReasonSignal, - lldb::StopReason::eStopReasonBreakpoint, -}; +const std::unordered_set +MinidumpFileBuilder::thread_stop_reasons{ +lldb::StopReason::eStopReasonException, +lldb::StopReason::eStopReasonSignal, +lldb::StopReason::eStopReasonBreakpoint, +}; Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() { // First set the offset on the file, and on the bytes saved @@ -692,8 +693,9 @@ Status MinidumpFileBuilder::AddExceptions() { for (const ThreadSP &thread_sp : thread_list) { StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); bool add_exception = false; -if (stop_info_sp && thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) { -add_exception = true; +if (stop_info_sp && +thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) { + add_exception = true; } if (add_exception) { diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h index 488eec7b92..9290290b64 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -168,7 +168,7 @@ private: m_tid_to_reg_ctx; std::unordered_set m_saved_stack_ranges; lldb::FileUP m_core_file; - lldb_private::SaveCoreOptions m_save_core_options; + lldb_private::SaveCoreOptions m_save_core_options; static const std::unordered_set thread_stop_reasons; }; #endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H `` https://github.com/llvm/llvm-project/pull/108448 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
@@ -1364,8 +1364,20 @@ void request_evaluate(const llvm::json::Object &request) { std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); - if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) == - ExpressionContext::Command) { + if (context == "repl" && + ((!expression.empty() && + g_dap.DetectExpressionContext(frame, expression) == + ExpressionContext::Command) || + (expression.empty() && +g_dap.last_expression_context == ExpressionContext::Command))) { +// If the current expression is empty, and the last expression context was +// for a command, pass the empty expression along to the +// CommandInterpreter, to repeat the previous command. Also set the +// expression context properly for the next (possibly empty) expression. +g_dap.last_expression_context = ExpressionContext::Command; +// Since the current expression context is not for a variable, clear the +// last_nonempty_var_expression field. +g_dap.last_nonempty_var_expression.clear(); cmtice wrote: Actually I have an idea for a way to make this work with only one variable; let me try that. https://github.com/llvm/llvm-project/pull/107485 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [NFC][LLDB] Remove CommandObjectTraceStartIntelPT.cpp call to protected Status Constructor (PR #108248)
https://github.com/Jlalond closed https://github.com/llvm/llvm-project/pull/108248 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Add breakpoint stop reasons to the minidump. (PR #108448)
https://github.com/jeffreytan81 commented: Can you add a test to check this? https://github.com/llvm/llvm-project/pull/108448 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Add breakpoint stop reasons to the minidump. (PR #108448)
https://github.com/jeffreytan81 edited https://github.com/llvm/llvm-project/pull/108448 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Minidump erase file on failure (PR #108259)
https://github.com/jeffreytan81 approved this pull request. I would suggest we added a unit test that checks that we do get an error message if saving minidump failed. Other than that, this looks good. https://github.com/llvm/llvm-project/pull/108259 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Add breakpoint stop reasons to the minidump. (PR #108448)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/108448 >From adcc5e8065d2a1b7edf877bd74de9cebd2f84cc9 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 12 Sep 2024 13:10:58 -0700 Subject: [PATCH 1/3] Create set for the stop reasons we collect, and add breakpoint to it. --- .../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 13 +++-- .../ObjectFile/Minidump/MinidumpFileBuilder.h | 4 +++- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index edc568a6b47e00..38e71e0e485d55 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -75,8 +75,7 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() { StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); if (stop_info_sp) { const StopReason &stop_reason = stop_info_sp->GetStopReason(); - if (stop_reason == StopReason::eStopReasonException || - stop_reason == StopReason::eStopReasonSignal) + if (m_thread_stop_reasons.count(stop_reason) > 0) m_expected_directories++; } } @@ -686,16 +685,10 @@ Status MinidumpFileBuilder::AddExceptions() { for (const ThreadSP &thread_sp : thread_list) { StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); bool add_exception = false; -if (stop_info_sp) { - switch (stop_info_sp->GetStopReason()) { - case eStopReasonSignal: - case eStopReasonException: +if (stop_info_sp && m_thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) { add_exception = true; -break; - default: -break; - } } + if (add_exception) { constexpr size_t minidump_exception_size = sizeof(llvm::minidump::ExceptionStream); diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h index 71001e26c00e91..569ba7fb07d871 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -168,7 +168,9 @@ class MinidumpFileBuilder { m_tid_to_reg_ctx; std::unordered_set m_saved_stack_ranges; lldb::FileUP m_core_file; - lldb_private::SaveCoreOptions m_save_core_options; + lldb_private::SaveCoreOptions m_save_core_options; + const std::unordered_set m_thread_stop_reasons = {lldb::StopReason::eStopReasonException, lldb::StopReason::eStopReasonSignal, + lldb::StopReason::eStopReasonBreakpoint }; }; #endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H >From 030b74c9e763c86d217da39487478e7cf2dff074 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 12 Sep 2024 13:30:04 -0700 Subject: [PATCH 2/3] Make const set with the stop reasons --- .../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 11 +-- .../Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h | 4 +--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 38e71e0e485d55..0b69f1b8205610 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -54,6 +54,13 @@ using namespace lldb; using namespace lldb_private; using namespace llvm::minidump; +// Set of all the stop reasons minidumps will collect. +const std::unordered_set MinidumpFileBuilder::thread_stop_reasons { + lldb::StopReason::eStopReasonException, + lldb::StopReason::eStopReasonSignal, + lldb::StopReason::eStopReasonBreakpoint, +}; + Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() { // First set the offset on the file, and on the bytes saved m_saved_data_size = HEADER_SIZE; @@ -75,7 +82,7 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() { StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); if (stop_info_sp) { const StopReason &stop_reason = stop_info_sp->GetStopReason(); - if (m_thread_stop_reasons.count(stop_reason) > 0) + if (thread_stop_reasons.count(stop_reason) > 0) m_expected_directories++; } } @@ -685,7 +692,7 @@ Status MinidumpFileBuilder::AddExceptions() { for (const ThreadSP &thread_sp : thread_list) { StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); bool add_exception = false; -if (stop_info_sp && m_thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) { +if (stop_info_sp && thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) { add_exception = true; } diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h index 56
[Lldb-commits] [lldb] [LLDB][Minidump] Minidump erase file on failure (PR #108259)
Jlalond wrote: @jeffreytan81 I'll add a test to ensure we get an error message https://github.com/llvm/llvm-project/pull/108259 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Add breakpoint stop reasons to the minidump. (PR #108448)
github-actions[bot] wrote: :warning: Python code formatter, darker found issues in your code. :warning: You can test this locally with the following command: ``bash darker --check --diff -r 030c6da7af826b641db005be925b20f956c3a6bb...099f017208ddadc07b3743ea7b04612f0f79617f lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py `` View the diff from darker here. ``diff --- TestProcessSaveCoreMinidump.py 2024-09-12 21:28:31.00 + +++ TestProcessSaveCoreMinidump.py 2024-09-12 21:31:56.194787 + @@ -521,11 +521,11 @@ thread = process.GetThreadAtIndex(thread_idx) stop = thread.stop_reason if stop == 1: foundSigInt = True break - + self.assertTrue(foundSigInt, "Breakpoint not included in minidump.") finally: self.assertTrue(self.dbg.DeleteTarget(target)) if os.path.isfile(custom_file): `` https://github.com/llvm/llvm-project/pull/108448 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Add breakpoint stop reasons to the minidump. (PR #108448)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/108448 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Add breakpoint stop reasons to the minidump. (PR #108448)
@@ -54,6 +54,13 @@ using namespace lldb; using namespace lldb_private; using namespace llvm::minidump; +// Set of all the stop reasons minidumps will collect. +const std::unordered_set MinidumpFileBuilder::thread_stop_reasons { + lldb::StopReason::eStopReasonException, + lldb::StopReason::eStopReasonSignal, + lldb::StopReason::eStopReasonBreakpoint, clayborg wrote: I would change `lldb::StopReason::eStopReasonBreakpoint` to `lldb::StopReason::eStopReasonLLDB` so we can encoded all the stuff we need. Then just encode all threads with a stop reason along with the needed data. I will detail more info below https://github.com/llvm/llvm-project/pull/108448 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Add breakpoint stop reasons to the minidump. (PR #108448)
@@ -686,16 +692,10 @@ Status MinidumpFileBuilder::AddExceptions() { for (const ThreadSP &thread_sp : thread_list) { StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); bool add_exception = false; -if (stop_info_sp) { - switch (stop_info_sp->GetStopReason()) { - case eStopReasonSignal: - case eStopReasonException: +if (stop_info_sp && thread_stop_reasons.count(stop_info_sp->GetStopReason()) > 0) { add_exception = true; -break; - default: -break; - } } + clayborg wrote: Down below we should do something like: ``` const StopReason stop_reason = stop_info_sp->GetStopReason(); if (stop_reason == eStopReasonSignal || stop_reason == eStopReasonException) exp_record.ExceptionCode = static_cast(stop_info_sp->GetValue()); else exp_record.ExceptionCode = lldb::StopReason::eStopReasonLLDB; ``` Then fill in the arguments for each stop reason into: ``` size_t data_count = ...; // See SBThread::GetStopReasonDataCount() exp_record.NumberParameters = data_count; // See SBThread::GetStopReasonDataCount() for (i=0; ihttps://github.com/llvm/llvm-project/pull/108448 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Avoid expression evaluation in libStdC++ std::vector synthetic children provider (PR #108414)
jeffreytan81 wrote: @jimingham, sounds good. I will move the logic into ValueObject. > TryAllThreads is true I did not see any client code setting this to true, I assume this happens because `m_try_others` is true by default? Maybe we should set `m_try_others`'s default value to false to avoid side effect like this? It sucks that we have to use timeout to guess and detect potential deadlock and resume all threads. On Windows, I remember I implemented the deadlock-free expression evaluation for VS debugger much accurately because Windows has an API to detect mutex block chain. With the APIs, I can even find out which dependency thread to resume to resolve the deadlock. It does not cover all synchronization situations though. https://github.com/llvm/llvm-project/pull/108414 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Minidump] Add a progress bar to minidump (PR #108309)
@@ -1024,6 +1027,8 @@ Status MinidumpFileBuilder::AddMemoryList_32( error = AddData(data_up->GetBytes(), bytes_read); if (error.Fail()) return error; + +progress.Increment(); clayborg wrote: might be nice to add detail before the ReadMemory above in case the first range is huge and give a description: ``` std::string descriotion = ...; // Put string in that give us the memory range as a string process.Increment(1, description); ``` https://github.com/llvm/llvm-project/pull/108309 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits