[Lldb-commits] [lldb] f0c6d30 - [lldb][DWARFASTParserClang][NFC] Factor out unnamed bitfield creation into helper (#108196)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread David Spickett via lldb-commits

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

2024-09-12 Thread David Spickett via lldb-commits

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)

2024-09-12 Thread Omair Javaid via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread David Spickett via lldb-commits

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)

2024-09-12 Thread David Spickett via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread David Spickett via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Felipe de Azevedo Piovezan via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread Adrian Prantl via lldb-commits


@@ -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)

2024-09-12 Thread Adrian Prantl via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Adrian Prantl via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread Jacob Lalonde via lldb-commits

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)

2024-09-12 Thread Jacob Lalonde via lldb-commits

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)

2024-09-12 Thread Jacob Lalonde via lldb-commits


@@ -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)

2024-09-12 Thread Jacob Lalonde via lldb-commits


@@ -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)

2024-09-12 Thread Jacob Lalonde via lldb-commits

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)

2024-09-12 Thread Jacob Lalonde via lldb-commits

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)

2024-09-12 Thread Jacob Lalonde via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Martin Storsjö via lldb-commits

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)

2024-09-12 Thread Pranav Kant via lldb-commits

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)

2024-09-12 Thread Kazu Hirata via lldb-commits

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)

2024-09-12 Thread via lldb-commits


@@ -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)

2024-09-12 Thread via lldb-commits


@@ -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

2024-09-12 Thread Kazu Hirata via lldb-commits

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)

2024-09-12 Thread Kazu Hirata via lldb-commits

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)

2024-09-12 Thread Adrian Vogelsgesang via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread via lldb-commits


@@ -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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread via lldb-commits


@@ -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)

2024-09-12 Thread via lldb-commits


@@ -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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Michael Buch via lldb-commits

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)

2024-09-12 Thread Adrian Vogelsgesang via lldb-commits

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)

2024-09-12 Thread Adrian Vogelsgesang via lldb-commits

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)

2024-09-12 Thread Adrian Vogelsgesang via lldb-commits

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)

2024-09-12 Thread Walter Erquinigo via lldb-commits


@@ -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)

2024-09-12 Thread Walter Erquinigo via lldb-commits


@@ -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)

2024-09-12 Thread Walter Erquinigo via lldb-commits


@@ -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)

2024-09-12 Thread Vy Nguyen via lldb-commits


@@ -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

2024-09-12 Thread Martin Storsjö via lldb-commits

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)

2024-09-12 Thread Martin Storsjö via lldb-commits

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)

2024-09-12 Thread Jacob Lalonde via lldb-commits

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)

2024-09-12 Thread via lldb-commits


@@ -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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread via lldb-commits


@@ -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)

2024-09-12 Thread via lldb-commits


@@ -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)

2024-09-12 Thread Jacob Lalonde via lldb-commits


@@ -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)

2024-09-12 Thread Jacob Lalonde via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Jacob Lalonde via lldb-commits


@@ -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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread via lldb-commits


@@ -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)

2024-09-12 Thread Jacob Lalonde via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Jacob Lalonde via lldb-commits

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)

2024-09-12 Thread Jacob Lalonde via lldb-commits

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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Greg Clayton via lldb-commits

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)

2024-09-12 Thread Greg Clayton via lldb-commits


@@ -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)

2024-09-12 Thread Greg Clayton via lldb-commits


@@ -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)

2024-09-12 Thread via lldb-commits

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)

2024-09-12 Thread Greg Clayton via lldb-commits


@@ -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


  1   2   >