[Lldb-commits] [lldb] d1556e5 - [lldb][lldb-server] Enable sending RegisterFlags as XML (#69951)

[via lldb-commits]

Author: David Spickett
Date: 2023-10-26T08:33:30+01:00
New Revision: d1556e5efbf0cb671c0f6e403fc1eaf9153f8713

URL: 
https://github.com/llvm/llvm-project/commit/d1556e5efbf0cb671c0f6e403fc1eaf9153f8713
DIFF: 
https://github.com/llvm/llvm-project/commit/d1556e5efbf0cb671c0f6e403fc1eaf9153f8713.diff

LOG: [lldb][lldb-server] Enable sending RegisterFlags as XML (#69951)

This adds ToXML methods to encode RegisterFlags and its fields into XML
according to GDB's target XML format:

https://sourceware.org/gdb/onlinedocs/gdb/Target-Description-Format.html#Target-Description-Format

lldb-server does not use libXML to build XML, so this follows the
existing code that uses strings. Indentation is used so the result is
still human readable.

```

  

```

This is used by lldb-server when building target XML, though no one sets
any fields yet. That'll come in a later commit.

Added: 


Modified: 
lldb/include/lldb/Target/RegisterFlags.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Target/RegisterFlags.cpp
lldb/unittests/Target/RegisterFlagsTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/RegisterFlags.h 
b/lldb/include/lldb/Target/RegisterFlags.h
index d98bc0263e35e23..7c5b97c2265fda3 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -9,20 +9,21 @@
 #ifndef LLDB_TARGET_REGISTERFLAGS_H
 #define LLDB_TARGET_REGISTERFLAGS_H
 
-#include "lldb/Utility/Log.h"
+#include 
+#include 
 
 namespace lldb_private {
 
+class StreamString;
+class Log;
+
 class RegisterFlags {
 public:
   class Field {
   public:
 /// Where start is the least significant bit and end is the most
 /// significant bit. The start bit must be <= the end bit.
-Field(std::string name, unsigned start, unsigned end)
-: m_name(std::move(name)), m_start(start), m_end(end) {
-  assert(m_start <= m_end && "Start bit must be <= end bit.");
-}
+Field(std::string name, unsigned start, unsigned end);
 
 /// Construct a field that occupies a single bit.
 Field(std::string name, unsigned bit_position)
@@ -51,6 +52,11 @@ class RegisterFlags {
 /// covered by either field.
 unsigned PaddingDistance(const Field &other) const;
 
+/// Output XML that describes this field, to be inserted into a target XML
+/// file. Reserved characters in field names like "<" are replaced with
+/// their XML safe equivalents like ">".
+void ToXML(StreamString &strm) const;
+
 bool operator<(const Field &rhs) const {
   return GetStart() < rhs.GetStart();
 }
@@ -106,6 +112,9 @@ class RegisterFlags {
   /// be split into many tables as needed.
   std::string AsTable(uint32_t max_width) const;
 
+  // Output XML that describes this set of flags.
+  void ToXML(StreamString &strm) const;
+
 private:
   const std::string m_id;
   /// Size in bytes

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 23c2f18cd388a86..187c23a206094c0 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3094,6 +3094,12 @@ GDBRemoteCommunicationServerLLGS::BuildTargetXml() {
   continue;
 }
 
+if (reg_info->flags_type) {
+  response.IndentMore();
+  reg_info->flags_type->ToXML(response);
+  response.IndentLess();
+}
+
 response.Indent();
 response.Printf("flags_type)
+  response << "type=\"" << reg_info->flags_type->GetID() << "\" ";
+
 const char *const register_set_name =
 reg_context.GetRegisterSetNameForRegisterAtIndex(reg_index);
 if (register_set_name)

diff  --git a/lldb/source/Target/RegisterFlags.cpp 
b/lldb/source/Target/RegisterFlags.cpp
index 06fb45d777ec36f..49974718ccb514a 100644
--- a/lldb/source/Target/RegisterFlags.cpp
+++ b/lldb/source/Target/RegisterFlags.cpp
@@ -7,13 +7,21 @@
 
//===--===//
 
 #include "lldb/Target/RegisterFlags.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 
+#include "llvm/ADT/StringExtras.h"
+
 #include 
 #include 
 
 using namespace lldb_private;
 
+RegisterFlags::Field::Field(std::string name, unsigned start, unsigned end)
+: m_name(std::move(name)), m_start(start), m_end(end) {
+  assert(m_start <= m_end && "Start bit must be <= end bit.");
+}
+
 void RegisterFlags::Field::log(Log *log) const {
   LLDB_LOG(log, "  Name: \"{0}\" Start: {1} End: {2}", m_name.c_str(), m_start,
m_end);
@@ -175,3 +183,41 @@ std::string RegisterFlags::AsTable(uint32_t max_width) 
const {
 
   return table;
 }
+
+void RegisterFlags::ToXML(StreamString &strm) const {
+  // Example XML:
+  // 
+  //   
+  /

[Lldb-commits] [lldb] [lldb][lldb-server] Enable sending RegisterFlags as XML (PR #69951)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett closed 
https://github.com/llvm/llvm-project/pull/69951
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AArch64][Linux] Add field information for the CPSR register (PR #70300)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett created 
https://github.com/llvm/llvm-project/pull/70300

The contents of which are mostly SPSR_EL1 as shown in the Arm manual, with a 
few adjustments for things Linux says userspace shouldn't concern itself with.

```
(lldb) register read cpsr
cpsr = 0x80001000
 = (N = 1, Z = 0, C = 0, V = 0, SS = 0, IL = 0, ...
```

Some fields are always present, some depend on extensions. I've checked for 
those extensions using HWCAP and HWCAP2.

To provide this for core files and live processes I've added a new class 
LinuxArm64RegisterFlags. This is a container for all the registers we'll want 
to have fields and handles detecting fields and updating register info.

This is used by the native process as follows:
* There is a global LinuxArm64RegisterFlags object.
* The first thread takes a mutex on it, and updates the fields.
* Subsequent threads see that detection is already done, and skip it.
* All threads then update their own copy of the register information with 
pointers to the field information contained in the global object.

This means that even though every thread will have the same fields, we only 
detect them once and have one copy of the information.

Core files instead have a LinuxArm64RegisterFlags as a member, because each 
core file could have different saved capabilities. The logic from there is the 
same but we get HWACP values from the corefile note.

This handler class is Linux specific right now, but it can easily be made more 
generic if needed. For example by using LLVM's FeatureBitset instead of HWCAPs.

Updating register info is done with string comparison, which isn't ideal. For 
CPSR, we do know the register number ahead of time but we do not for other 
registers in dynamic register sets. So in the interest of consistency, I'm 
going to use string comparison for all registers including cpsr.

I've added tests with a core file and live process. Only checking for fields 
that are always present to account for CPU variance.

>From 45a9d131ce6c9fb31355519cd810ceff32c05ee7 Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Wed, 11 Oct 2023 14:54:07 +0100
Subject: [PATCH] [lldb][AArch64][Linux] Add field information for the CPSR
 register

The contents of which are mostly SPSR_EL1 as shown in the Arm manual,
with a few adjustments for things Linux says userspace shouldn't concern
itself with.

```
(lldb) register read cpsr
cpsr = 0x80001000
 = (N = 1, Z = 0, C = 0, V = 0, SS = 0, IL = 0, ...
```

Some fields are always present, some depend on extensions. I've checked
for those extensions using HWCAP and HWCAP2.

To provide this for core files and live processes I've added a new class
LinuxArm64RegisterFlags. This is a container for all the registers we'll want
to have fields and handles detecting fields and updating register info.

This is used by the native process as follows:
* There is a global LinuxArm64RegisterFlags object.
* The first thread takes a mutex on it, and updates the fields.
* Subsequent threads see that detection is already done, and skip it.
* All threads then update their own copy of the register information
  with pointers to the field information contained in the global object.

This means that even though every thread will have the same fields,
we only detect them once and have one copy of the information.

Core files instead have a LinuxArm64RegisterFlags as a member, because
each core file could have different saved capabilities. The logic from
there is the same but we get HWACP values from the corefile note.

This handler class is Linux specific right now, but it can easily be made
more generic if needed. For example by using LLVM's FeatureBitset
instead of HWCAPs.

Updating register info is done with string comparison, which isn't ideal.
For CPSR, we do know the register number ahead of time but we do not for
other registers in dynamic register sets. So in the interest of
consistency, I'm going to use string comparison for all registers including 
cpsr.

I've added tests with a core file and live process. Only checking for
fields that are always present to account for CPU variance.
---
 lldb/include/lldb/Target/RegisterFlags.h  |   5 +
 lldb/include/lldb/lldb-private-types.h|   5 +-
 .../NativeRegisterContextLinux_arm64.cpp  |  18 
 .../Plugins/Process/Utility/CMakeLists.txt|   1 +
 .../Utility/RegisterFlagsLinux_arm64.cpp  | 100 ++
 .../Utility/RegisterFlagsLinux_arm64.h|  75 +
 .../RegisterContextPOSIXCore_arm64.cpp|  17 +++
 .../elf-core/RegisterContextPOSIXCore_arm64.h |   3 +
 lldb/source/Target/RegisterFlags.cpp  |  14 ++-
 .../register_command/TestRegisters.py |  12 +++
 .../postmortem/elf-core/TestLinuxCore.py  |   4 +
 11 files changed, 249 insertions(+), 5 deletions(-)
 create mode 100644 
lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
 create mode 100644 
lldb/source/Plugins/Process/Utilit

[Lldb-commits] [lldb] [lldb][AArch64][Linux] Add field information for the CPSR register (PR #70300)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)


Changes

The contents of which are mostly SPSR_EL1 as shown in the Arm manual, with a 
few adjustments for things Linux says userspace shouldn't concern itself with.

```
(lldb) register read cpsr
cpsr = 0x80001000
 = (N = 1, Z = 0, C = 0, V = 0, SS = 0, IL = 0, ...
```

Some fields are always present, some depend on extensions. I've checked for 
those extensions using HWCAP and HWCAP2.

To provide this for core files and live processes I've added a new class 
LinuxArm64RegisterFlags. This is a container for all the registers we'll want 
to have fields and handles detecting fields and updating register info.

This is used by the native process as follows:
* There is a global LinuxArm64RegisterFlags object.
* The first thread takes a mutex on it, and updates the fields.
* Subsequent threads see that detection is already done, and skip it.
* All threads then update their own copy of the register information with 
pointers to the field information contained in the global object.

This means that even though every thread will have the same fields, we only 
detect them once and have one copy of the information.

Core files instead have a LinuxArm64RegisterFlags as a member, because each 
core file could have different saved capabilities. The logic from there is the 
same but we get HWACP values from the corefile note.

This handler class is Linux specific right now, but it can easily be made more 
generic if needed. For example by using LLVM's FeatureBitset instead of HWCAPs.

Updating register info is done with string comparison, which isn't ideal. For 
CPSR, we do know the register number ahead of time but we do not for other 
registers in dynamic register sets. So in the interest of consistency, I'm 
going to use string comparison for all registers including cpsr.

I've added tests with a core file and live process. Only checking for fields 
that are always present to account for CPU variance.

---
Full diff: https://github.com/llvm/llvm-project/pull/70300.diff


11 Files Affected:

- (modified) lldb/include/lldb/Target/RegisterFlags.h (+5) 
- (modified) lldb/include/lldb/lldb-private-types.h (+4-1) 
- (modified) 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp (+18) 
- (modified) lldb/source/Plugins/Process/Utility/CMakeLists.txt (+1) 
- (added) lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp 
(+100) 
- (added) lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h (+75) 
- (modified) 
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp (+17) 
- (modified) 
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h (+3) 
- (modified) lldb/source/Target/RegisterFlags.cpp (+10-4) 
- (modified) 
lldb/test/API/commands/register/register/register_command/TestRegisters.py 
(+12) 
- (modified) lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py 
(+4) 


``diff
diff --git a/lldb/include/lldb/Target/RegisterFlags.h 
b/lldb/include/lldb/Target/RegisterFlags.h
index 7c5b97c2265fda3..1d8c8e943813e77 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -83,6 +83,11 @@ class RegisterFlags {
   RegisterFlags(std::string id, unsigned size,
 const std::vector &fields);
 
+  /// Replace all the fields with the new set of fields. All the assumptions
+  /// and checks apply as when you use the constructor. Intended to only be 
used
+  /// when runtime field detection is needed.
+  void SetFields(const std::vector &fields);
+
   // Reverse the order of the fields, keeping their values the same.
   // For example a field from bit 31 to 30 with value 0b10 will become bits
   // 1 to 0, with the same 0b10 value.
diff --git a/lldb/include/lldb/lldb-private-types.h 
b/lldb/include/lldb/lldb-private-types.h
index e6717836331f590..f2ced61b7cc315b 100644
--- a/lldb/include/lldb/lldb-private-types.h
+++ b/lldb/include/lldb/lldb-private-types.h
@@ -62,7 +62,10 @@ struct RegisterInfo {
   /// rax ax, ah, and al.
   uint32_t *invalidate_regs;
   /// If not nullptr, a type defined by XML descriptions.
-  const RegisterFlags *flags_type;
+  /// This is mutable so that it may be updated after the register info tables
+  /// have been constructed. For example a specific target OS may have a
+  /// different layout.
+  mutable const RegisterFlags *flags_type;
 
   llvm::ArrayRef data(const uint8_t *context_base) const {
 return llvm::ArrayRef(context_base + byte_offset, byte_size);
diff --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index b5210c368144206..b55d60f3d9bbd13 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -23,6 +23,7 @@
 #include "Plugins/Proces

[Lldb-commits] [lldb] [lldb][AArch64] Add SME2's ZT0 register (PR #70205)

[David Spickett via lldb-commits]

@@ -625,6 +662,18 @@ 
NativeRegisterContextLinux_arm64::CacheAllRegisters(uint32_t &cached_size) {
 error = ReadZA();
 if (error.Fail())
   return error;
+
+// We will only be restoring ZT data if ZA is active. As writing to an
+// inactive ZT enables ZA, which may not be desireable.
+if (GetRegisterInfo().IsZTEnabled() &&
+m_za_header.size > sizeof(m_za_header)) {

DavidSpickett wrote:

It's actually "if ZT0 is present and ZA is active", and ZA being active implies 
that zt0 is also active.

We have a bit of a word salad here as the existing checks are `IsFooEnabled()` 
because if for example, PAC is enabled it means we have the registers.

ZA threw a spanner in these works because `IsZAEnabled` can be true as in, it 
is present. Whether it's enabled, meaning it's "active" and SVCR.ZA is set, 
that's what we have to check the header for.

...which is to say I agree this is very confusing.

What the current code says is:
```
if (
  // Do we have ZT0 present, meaning, do we have SME2?
  GetRegisterInfo().IsZTEnabled() &&
  // Do we have a ZA header and is that header telling us that ZA is active?
  m_za_header.size > sizeof(m_za_header)) {
```

Because ironically, you can tell if zt0 would be active by checking if za is 
active, since they both use the svcr.za bit.

Let me see what I can do about this code right now, and I think I'll change all 
the `IsFooEnabled` to `IsFooPresent` in another PR for clarity sake.

https://github.com/llvm/llvm-project/pull/70205
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AArch64][Linux] Rename IsEnabled to IsPresent (PR #70303)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett created 
https://github.com/llvm/llvm-project/pull/70303

For most register sets, if it was enabled this meant you could use it, it was 
present in the process. There was no present but turned off state. So "enabled" 
made sense.

Then ZA came along (and soon to be ZT0) where ZA can be present in the hardware 
when you have SME, but ZA itself can be made inactive. This means that 
"IsZAEnabled()" doesn't mean is it active, it means do you have SME. Which is 
very confusing when we actually want to know if ZA is active.

So instead say "IsZAPresent", to make these checks more specific. For things 
that can't be made inactive, present will imply "active" as they're never 
inactive.

>From c513a486814ac03d3150f59c1b2c835e379ebdb6 Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Thu, 26 Oct 2023 08:28:02 +
Subject: [PATCH] [lldb][AArch64][Linux] Reanme IsEnabled to
 IsPresent

For most register sets, if it was enabled this meant you could use it,
it was present in the process. There was no present but turned off state.

So "enabled" made sense.

Then ZA came along (and soon to be ZT0) where ZA can be present in the
hardware when you have SME, but ZA itself can be made inactive.

This means that "IsZAEnabled()" doesn't mean is it active, it means
do you have SME. Which is very confusing when we actually want to know
if ZA is active.

So instead say "IsZAPresent", to make these checks more specific.
For things that can't be made inactive, present will imply "active"
as they're never inactive.
---
 .../NativeRegisterContextLinux_arm64.cpp  | 20 +--
 .../Process/Utility/RegisterInfoPOSIX_arm64.h | 12 +--
 .../RegisterContextPOSIXCore_arm64.cpp| 12 +--
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index b5210c368144206..b23b4ed2171b546 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -166,10 +166,10 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   m_tls_is_valid = false;
 
   // SME adds the tpidr2 register
-  m_tls_size = GetRegisterInfo().IsSSVEEnabled() ? sizeof(m_tls_regs)
+  m_tls_size = GetRegisterInfo().IsSSVEPresent() ? sizeof(m_tls_regs)
  : 
sizeof(m_tls_regs.tpidr_reg);
 
-  if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled())
+  if (GetRegisterInfo().IsSVEPresent() || GetRegisterInfo().IsSSVEPresent())
 m_sve_state = SVEState::Unknown;
   else
 m_sve_state = SVEState::Disabled;
@@ -610,7 +610,7 @@ 
NativeRegisterContextLinux_arm64::CacheAllRegisters(uint32_t &cached_size) {
 return error;
 
   // Here this means, does the system have ZA, not whether it is active.
-  if (GetRegisterInfo().IsZAEnabled()) {
+  if (GetRegisterInfo().IsZAPresent()) {
 error = ReadZAHeader();
 if (error.Fail())
   return error;
@@ -628,7 +628,7 @@ 
NativeRegisterContextLinux_arm64::CacheAllRegisters(uint32_t &cached_size) {
   }
 
   // If SVE is enabled we need not copy FPR separately.
-  if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
+  if (GetRegisterInfo().IsSVEPresent() || GetRegisterInfo().IsSSVEPresent()) {
 // Store mode and register data.
 cached_size +=
 sizeof(RegisterSetType) + sizeof(m_sve_state) + GetSVEBufferSize();
@@ -640,7 +640,7 @@ 
NativeRegisterContextLinux_arm64::CacheAllRegisters(uint32_t &cached_size) {
   if (error.Fail())
 return error;
 
-  if (GetRegisterInfo().IsMTEEnabled()) {
+  if (GetRegisterInfo().IsMTEPresent()) {
 cached_size += sizeof(RegisterSetType) + GetMTEControlSize();
 error = ReadMTEControl();
 if (error.Fail())
@@ -708,7 +708,7 @@ Status 
NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
   // constants and the functions vec_set_vector_length, sve_set_common and
   // za_set in the Linux Kernel.
 
-  if ((m_sve_state != SVEState::Streaming) && GetRegisterInfo().IsZAEnabled()) 
{
+  if ((m_sve_state != SVEState::Streaming) && GetRegisterInfo().IsZAPresent()) 
{
 // Use the header size not the buffer size, as we may be using the buffer
 // for fake data, which we do not want to write out.
 assert(m_za_header.size <= GetZABufferSize());
@@ -716,7 +716,7 @@ Status 
NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
 m_za_header.size);
   }
 
-  if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
+  if (GetRegisterInfo().IsSVEPresent() || GetRegisterInfo().IsSSVEPresent()) {
 dst = AddRegisterSetType(dst, RegisterSetType::SVE);
 *(reinterpret_cast(dst)) = m_sve_state;
 dst += sizeof(m_sve_state);
@@ -726,13 +726,13 @@ Status 
NativeRegisterContextLinux_arm64::

[Lldb-commits] [lldb] [lldb][AArch64][Linux] Rename IsEnabled to IsPresent (PR #70303)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)


Changes

For most register sets, if it was enabled this meant you could use it, it was 
present in the process. There was no present but turned off state. So "enabled" 
made sense.

Then ZA came along (and soon to be ZT0) where ZA can be present in the hardware 
when you have SME, but ZA itself can be made inactive. This means that 
"IsZAEnabled()" doesn't mean is it active, it means do you have SME. Which is 
very confusing when we actually want to know if ZA is active.

So instead say "IsZAPresent", to make these checks more specific. For things 
that can't be made inactive, present will imply "active" as they're never 
inactive.

---
Full diff: https://github.com/llvm/llvm-project/pull/70303.diff


3 Files Affected:

- (modified) 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp (+10-10) 
- (modified) lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h 
(+6-6) 
- (modified) 
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp (+6-6) 


``diff
diff --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index b5210c368144206..b23b4ed2171b546 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -166,10 +166,10 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   m_tls_is_valid = false;
 
   // SME adds the tpidr2 register
-  m_tls_size = GetRegisterInfo().IsSSVEEnabled() ? sizeof(m_tls_regs)
+  m_tls_size = GetRegisterInfo().IsSSVEPresent() ? sizeof(m_tls_regs)
  : 
sizeof(m_tls_regs.tpidr_reg);
 
-  if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled())
+  if (GetRegisterInfo().IsSVEPresent() || GetRegisterInfo().IsSSVEPresent())
 m_sve_state = SVEState::Unknown;
   else
 m_sve_state = SVEState::Disabled;
@@ -610,7 +610,7 @@ 
NativeRegisterContextLinux_arm64::CacheAllRegisters(uint32_t &cached_size) {
 return error;
 
   // Here this means, does the system have ZA, not whether it is active.
-  if (GetRegisterInfo().IsZAEnabled()) {
+  if (GetRegisterInfo().IsZAPresent()) {
 error = ReadZAHeader();
 if (error.Fail())
   return error;
@@ -628,7 +628,7 @@ 
NativeRegisterContextLinux_arm64::CacheAllRegisters(uint32_t &cached_size) {
   }
 
   // If SVE is enabled we need not copy FPR separately.
-  if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
+  if (GetRegisterInfo().IsSVEPresent() || GetRegisterInfo().IsSSVEPresent()) {
 // Store mode and register data.
 cached_size +=
 sizeof(RegisterSetType) + sizeof(m_sve_state) + GetSVEBufferSize();
@@ -640,7 +640,7 @@ 
NativeRegisterContextLinux_arm64::CacheAllRegisters(uint32_t &cached_size) {
   if (error.Fail())
 return error;
 
-  if (GetRegisterInfo().IsMTEEnabled()) {
+  if (GetRegisterInfo().IsMTEPresent()) {
 cached_size += sizeof(RegisterSetType) + GetMTEControlSize();
 error = ReadMTEControl();
 if (error.Fail())
@@ -708,7 +708,7 @@ Status 
NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
   // constants and the functions vec_set_vector_length, sve_set_common and
   // za_set in the Linux Kernel.
 
-  if ((m_sve_state != SVEState::Streaming) && GetRegisterInfo().IsZAEnabled()) 
{
+  if ((m_sve_state != SVEState::Streaming) && GetRegisterInfo().IsZAPresent()) 
{
 // Use the header size not the buffer size, as we may be using the buffer
 // for fake data, which we do not want to write out.
 assert(m_za_header.size <= GetZABufferSize());
@@ -716,7 +716,7 @@ Status 
NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
 m_za_header.size);
   }
 
-  if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
+  if (GetRegisterInfo().IsSVEPresent() || GetRegisterInfo().IsSSVEPresent()) {
 dst = AddRegisterSetType(dst, RegisterSetType::SVE);
 *(reinterpret_cast(dst)) = m_sve_state;
 dst += sizeof(m_sve_state);
@@ -726,13 +726,13 @@ Status 
NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
 GetFPRSize());
   }
 
-  if ((m_sve_state == SVEState::Streaming) && GetRegisterInfo().IsZAEnabled()) 
{
+  if ((m_sve_state == SVEState::Streaming) && GetRegisterInfo().IsZAPresent()) 
{
 assert(m_za_header.size <= GetZABufferSize());
 dst = AddSavedRegisters(dst, RegisterSetType::SME, GetZABuffer(),
 m_za_header.size);
   }
 
-  if (GetRegisterInfo().IsMTEEnabled()) {
+  if (GetRegisterInfo().IsMTEPresent()) {
 dst = AddSavedRegisters(dst, RegisterSetType::MTE, GetMTEControl(),
 GetMTEControlSize());
   }
@@ -1411,7 +1411,7 @@ std::vector 
NativeRegisterContextLinux_arm64::G

[Lldb-commits] [lldb] [lldb][AArch64] Add SME2's ZT0 register (PR #70205)

[David Spickett via lldb-commits]

@@ -625,6 +662,18 @@ 
NativeRegisterContextLinux_arm64::CacheAllRegisters(uint32_t &cached_size) {
 error = ReadZA();
 if (error.Fail())
   return error;
+
+// We will only be restoring ZT data if ZA is active. As writing to an
+// inactive ZT enables ZA, which may not be desireable.
+if (GetRegisterInfo().IsZTEnabled() &&
+m_za_header.size > sizeof(m_za_header)) {

DavidSpickett wrote:

https://github.com/llvm/llvm-project/pull/70303 Addresses the enabled/present 
thing.

I can't make a `IsZAEnabled` helper until that goes in so whichever one of 
these lands first I'll sort that out.

For now I'll add some comments to clarify the current code.

https://github.com/llvm/llvm-project/pull/70205
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AArch64] Add SME2's ZT0 register (PR #70205)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett updated 
https://github.com/llvm/llvm-project/pull/70205

>From 80e01960f247ab9ee06daa59d9c033fda5fc3978 Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Tue, 3 Oct 2023 13:24:39 +0100
Subject: [PATCH 1/3] [lldb][AArch64] Add SME2's ZT0 register

SME2 is documented as part of the main SME supplement:
https://developer.arm.com/documentation/ddi0616/latest/

The one change for debug is this new ZT0 register. This register
contains data to be used with new table lookup instructions.
It's size is always 512 bits (not scalable) and can be
interpreted in many different ways depending on the instructions
that use it.

The kernel has implemented this as a new register set containing
this single register. It always returns register data (with no header,
unlike ZA which does have a header).

https://docs.kernel.org/arch/arm64/sme.html

ZT0 is only active when ZA is active (when SVCR.ZA is 1). In the
inactive state the kernel returns 0s for its contents. Therefore
lldb doesn't need to create 0s like it does for ZA.

However, we will skip restoring the value of ZT0 if we know that
ZA is inactive. As writing to an inactive ZT0 sets SVCR.ZA to 1,
which is not desireable as it would activate ZA also. Whether
SVCR.ZA is set will be determined only by the ZA data we restore.

Due to this, I've added a new save/restore kind SME2. This is easier
than accounting for the variable length ZA in the SME data. We'll only
save an SME2 data block if ZA is active. If it's not we can get fresh
0s back from the kernel for ZT0 anyway so there's nothing for us to restore.

This new register will only show up if the system has SME2 therefore
the SME set presented to the user may change, and I've had to account
for that in in a few places.

I've referred to it internally as simply "ZT" as the kernel does in
NT_ARM_ZT, but the architecture refers to the specific register as "ZT0"
so that's what you'll see in lldb.

```
(lldb) register read -s 6
Scalable Matrix Extension Registers:
  svcr = 0x
   svg = 0x0004
za = {0x00 <...> 0x00}
   zt0 = {0x00 <...> 0x00}
```
---
 .../NativeRegisterContextLinux_arm64.cpp  | 133 --
 .../Linux/NativeRegisterContextLinux_arm64.h  |  12 ++
 .../Utility/RegisterInfoPOSIX_arm64.cpp   |  50 +--
 .../Process/Utility/RegisterInfoPOSIX_arm64.h |   5 +-
 .../Process/elf-core/RegisterUtilities.h  |   4 +
 5 files changed, 177 insertions(+), 27 deletions(-)

diff --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index b5210c368144206..d4c83dcba6363f5 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -45,6 +45,11 @@
 #define NT_ARM_ZA 0x40c /* ARM Scalable Matrix Extension, Array Storage */
 #endif
 
+#ifndef NT_ARM_ZT
+#define NT_ARM_ZT  
\
+  0x40d /* ARM Scalable Matrix Extension 2, lookup table register */
+#endif
+
 #ifndef NT_ARM_PAC_MASK
 #define NT_ARM_PAC_MASK 0x406 /* Pointer authentication code masks */
 #endif
@@ -104,6 +109,17 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
 .Success())
   opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskZA);
 
+// SME's ZT0 is a 512 bit register.
+std::array zt_reg;
+ioVec.iov_base = zt_reg.data();
+ioVec.iov_len = zt_reg.size();
+regset = NT_ARM_ZT;
+if (NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET,
+  native_thread.GetID(), ®set,
+  &ioVec, zt_reg.size())
+.Success())
+  opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskZT);
+
 NativeProcessLinux &process = native_thread.GetProcess();
 
 std::optional auxv_at_hwcap =
@@ -148,6 +164,7 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   ::memset(&m_pac_mask, 0, sizeof(m_pac_mask));
   ::memset(&m_tls_regs, 0, sizeof(m_tls_regs));
   ::memset(&m_sme_pseudo_regs, 0, sizeof(m_sme_pseudo_regs));
+  std::fill(m_zt_reg.begin(), m_zt_reg.end(), 0);
 
   m_mte_ctrl_reg = 0;
 
@@ -164,6 +181,7 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   m_pac_mask_is_valid = false;
   m_mte_ctrl_is_valid = false;
   m_tls_is_valid = false;
+  m_zt_buffer_is_valid = false;
 
   // SME adds the tpidr2 register
   m_tls_size = GetRegisterInfo().IsSSVEEnabled() ? sizeof(m_tls_regs)
@@ -355,6 +373,15 @@ NativeRegisterContextLinux_arm64::ReadRegister(const 
RegisterInfo *reg_info,
   // storage. Therefore its effective byte offset is always 0 even if it
   // isn't 0 within the SME register set.
   src = (uint8_t *)GetZABuffer() + GetZAHeaderSize();
+} else if (GetRegisterInfo().IsSMERegZT(reg)) {
+  // Unlike ZA, the 

[Lldb-commits] [lldb] [lldb][AArch64] Add SME2's ZT0 register (PR #70205)

[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 23d6a6dfc1bc6e79bdcc48a59a0698a5b79262e9 
3ed10ca4d37abce5114fe22110b587a0c78d2d0a -- 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h 
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp 
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h 
lldb/source/Plugins/Process/elf-core/RegisterUtilities.h 
lldb/test/API/commands/register/register/aarch64_sme_z_registers/save_restore/main.c
 
lldb/test/API/commands/register/register/aarch64_sme_z_registers/za_dynamic_resize/main.c
``





View the diff from clang-format here.


``diff
diff --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index b4ae5dd7c35c..c93782e2d411 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -666,9 +666,9 @@ 
NativeRegisterContextLinux_arm64::CacheAllRegisters(uint32_t &cached_size) {
 // We will only be restoring ZT data if ZA is active. As writing to an
 // inactive ZT enables ZA, which may not be desireable.
 if (
-  // If we have ZT0, or in other words, if we have SME2.
-  GetRegisterInfo().IsZTEnabled() &&
-  // And ZA is active, which means that ZT0 is also active.
+// If we have ZT0, or in other words, if we have SME2.
+GetRegisterInfo().IsZTEnabled() &&
+// And ZA is active, which means that ZT0 is also active.
 m_za_header.size > sizeof(m_za_header)) {
   cached_size += sizeof(RegisterSetType) + GetZTBufferSize();
   // Unlike ZA where we have to fake data for an inactive ZA, the kernel
@@ -790,10 +790,10 @@ Status 
NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
   // If we leave ZA/ZT0 inactive and read ZT0, the kernel returns 0s. Therefore
   // there's nothing for us to restore if ZA was originally inactive.
   if (
-// If we have SME2 and therefore ZT0.
-GetRegisterInfo().IsZTEnabled() &&
-// And ZA is enabled.
-m_za_header.size > sizeof(m_za_header))
+  // If we have SME2 and therefore ZT0.
+  GetRegisterInfo().IsZTEnabled() &&
+  // And ZA is enabled.
+  m_za_header.size > sizeof(m_za_header))
 dst = AddSavedRegisters(dst, RegisterSetType::SME2, GetZTBuffer(),
 GetZTBufferSize());
 

``




https://github.com/llvm/llvm-project/pull/70205
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AArch64] Add SME2's ZT0 register (PR #70205)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett updated 
https://github.com/llvm/llvm-project/pull/70205

>From 80e01960f247ab9ee06daa59d9c033fda5fc3978 Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Tue, 3 Oct 2023 13:24:39 +0100
Subject: [PATCH 1/4] [lldb][AArch64] Add SME2's ZT0 register

SME2 is documented as part of the main SME supplement:
https://developer.arm.com/documentation/ddi0616/latest/

The one change for debug is this new ZT0 register. This register
contains data to be used with new table lookup instructions.
It's size is always 512 bits (not scalable) and can be
interpreted in many different ways depending on the instructions
that use it.

The kernel has implemented this as a new register set containing
this single register. It always returns register data (with no header,
unlike ZA which does have a header).

https://docs.kernel.org/arch/arm64/sme.html

ZT0 is only active when ZA is active (when SVCR.ZA is 1). In the
inactive state the kernel returns 0s for its contents. Therefore
lldb doesn't need to create 0s like it does for ZA.

However, we will skip restoring the value of ZT0 if we know that
ZA is inactive. As writing to an inactive ZT0 sets SVCR.ZA to 1,
which is not desireable as it would activate ZA also. Whether
SVCR.ZA is set will be determined only by the ZA data we restore.

Due to this, I've added a new save/restore kind SME2. This is easier
than accounting for the variable length ZA in the SME data. We'll only
save an SME2 data block if ZA is active. If it's not we can get fresh
0s back from the kernel for ZT0 anyway so there's nothing for us to restore.

This new register will only show up if the system has SME2 therefore
the SME set presented to the user may change, and I've had to account
for that in in a few places.

I've referred to it internally as simply "ZT" as the kernel does in
NT_ARM_ZT, but the architecture refers to the specific register as "ZT0"
so that's what you'll see in lldb.

```
(lldb) register read -s 6
Scalable Matrix Extension Registers:
  svcr = 0x
   svg = 0x0004
za = {0x00 <...> 0x00}
   zt0 = {0x00 <...> 0x00}
```
---
 .../NativeRegisterContextLinux_arm64.cpp  | 133 --
 .../Linux/NativeRegisterContextLinux_arm64.h  |  12 ++
 .../Utility/RegisterInfoPOSIX_arm64.cpp   |  50 +--
 .../Process/Utility/RegisterInfoPOSIX_arm64.h |   5 +-
 .../Process/elf-core/RegisterUtilities.h  |   4 +
 5 files changed, 177 insertions(+), 27 deletions(-)

diff --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index b5210c368144206..d4c83dcba6363f5 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -45,6 +45,11 @@
 #define NT_ARM_ZA 0x40c /* ARM Scalable Matrix Extension, Array Storage */
 #endif
 
+#ifndef NT_ARM_ZT
+#define NT_ARM_ZT  
\
+  0x40d /* ARM Scalable Matrix Extension 2, lookup table register */
+#endif
+
 #ifndef NT_ARM_PAC_MASK
 #define NT_ARM_PAC_MASK 0x406 /* Pointer authentication code masks */
 #endif
@@ -104,6 +109,17 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
 .Success())
   opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskZA);
 
+// SME's ZT0 is a 512 bit register.
+std::array zt_reg;
+ioVec.iov_base = zt_reg.data();
+ioVec.iov_len = zt_reg.size();
+regset = NT_ARM_ZT;
+if (NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET,
+  native_thread.GetID(), ®set,
+  &ioVec, zt_reg.size())
+.Success())
+  opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskZT);
+
 NativeProcessLinux &process = native_thread.GetProcess();
 
 std::optional auxv_at_hwcap =
@@ -148,6 +164,7 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   ::memset(&m_pac_mask, 0, sizeof(m_pac_mask));
   ::memset(&m_tls_regs, 0, sizeof(m_tls_regs));
   ::memset(&m_sme_pseudo_regs, 0, sizeof(m_sme_pseudo_regs));
+  std::fill(m_zt_reg.begin(), m_zt_reg.end(), 0);
 
   m_mte_ctrl_reg = 0;
 
@@ -164,6 +181,7 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   m_pac_mask_is_valid = false;
   m_mte_ctrl_is_valid = false;
   m_tls_is_valid = false;
+  m_zt_buffer_is_valid = false;
 
   // SME adds the tpidr2 register
   m_tls_size = GetRegisterInfo().IsSSVEEnabled() ? sizeof(m_tls_regs)
@@ -355,6 +373,15 @@ NativeRegisterContextLinux_arm64::ReadRegister(const 
RegisterInfo *reg_info,
   // storage. Therefore its effective byte offset is always 0 even if it
   // isn't 0 within the SME register set.
   src = (uint8_t *)GetZABuffer() + GetZAHeaderSize();
+} else if (GetRegisterInfo().IsSMERegZT(reg)) {
+  // Unlike ZA, the 

[Lldb-commits] [lldb] Colorize output when searching for symbols in lldb (PR #69422)

[David Spickett via lldb-commits]

@@ -0,0 +1,23 @@
+UNSUPPORTED: system-windows
+
+# RUN: %clang_host -g %S/Inputs/main.c -o %t
+# RUN: %lldb %t -b -o 'image lookup -r -s ma' | FileCheck %s

DavidSpickett wrote:

I should have brought this up earlier but the way you're coloring the output 
doesn't actually do a regex match, it looks for substrings. This works fine if 
the input is itself basically a substring like here, but would not for `m[abc]`.

(keep these test lines they're good, but add more to cover this)

So you'll need some way to either do the matching again while formatting, or 
pass instead of `name`, a list of matched locations from the initial search.

I suggest you start by doing the first idea, redo the regex match in the 
formatting function. That'll give you an idea of what data you'd require, if we 
were to instead pass the result of the first matching to the formatter.

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Colorize output when searching for symbols in lldb (PR #69422)

[David Spickett via lldb-commits]

@@ -0,0 +1,23 @@
+UNSUPPORTED: system-windows
+
+# RUN: %clang_host -g %S/Inputs/main.c -o %t
+# RUN: %lldb %t -b -o 'image lookup -r -s ma' | FileCheck %s
+
+# CHECK: 3 symbols match the regular expression 'ma' in {{.*}}
+# The [[ confuses FileCheck so regex match it.
+# CHECK-NEXT: Name: {{.+}}31mma{{.+}}0min.c

DavidSpickett wrote:

Only the lines that are supposed to have colour are important here, and 
ordering isn't really an issue.

Keep the "3 symbols match" check, just as a sanity check in case the output 
changes, but then only check for the lines with colour (you won't need -NEXT).

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Colorize output when searching for symbols in lldb (PR #69422)

[David Spickett via lldb-commits]

@@ -0,0 +1,23 @@
+UNSUPPORTED: system-windows
+
+# RUN: %clang_host -g %S/Inputs/main.c -o %t
+# RUN: %lldb %t -b -o 'image lookup -r -s ma' | FileCheck %s
+
+# CHECK: 3 symbols match the regular expression 'ma' in {{.*}}
+# The [[ confuses FileCheck so regex match it.
+# CHECK-NEXT: Name: {{.+}}31mma{{.+}}0min.c

DavidSpickett wrote:

Well, ordering is something we want to be correct - but that's tested by other 
tests. You can assume it will be correct.

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Colorize output when searching for symbols in lldb (PR #69422)

[via lldb-commits]

taalhaataahir0102 wrote:

Hi David, I thought writing a few simple and very basic test cases will be 
better before updating the `printRed` function so that while updating the 
`printRed` function it easier and quicker for us to test :) 
We've written 2 test cases for colorized output search in 
`tests/Shell/Commands/TestImageLookupColor` by taking help from the following 2 
already written test cases:

- 
https://github.com/llvm/llvm-project/blob/main/lldb/test/Shell/Driver/TestUseColor.test
- 
https://github.com/llvm/llvm-project/blob/main/lldb/test/Shell/Commands/command-target-modules-lookup.test

First test is a basic regex symbol search for "ma"
The second test (which is failing right now as we have not added the support of 
| in `printRed` function) searches for 2 symbols using or (|). I'll update the 
`printRed` function to first split the input string i.e., `name` on | and then 
apply colors to each of the string in the searched result

Also, we've incorporated the previous feedback.

Next we'll update the `printRed` function:

- Will add or (|) feature in printRed.
- Will update printRed so that we don't have to write it in every separate file.
- Will add support of no use color i.e., `settings set use-color false`

Can you please confirm weather the test cases good for now and the future plan 
looks okay? ;)

P.S: Currently The test case tests/Shell/Commands/command-target-modules-lookup 
is failing as it is not checking for ANSI color codes in the output. Once we 
update the `printRed` function, We'll add no use color in this test case and 
hopefully it will work fine.

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Colorize output when searching for symbols in lldb (PR #69422)

[David Spickett via lldb-commits]

DavidSpickett wrote:

> Can you please confirm weather the test cases good for now and the future 
> plan looks okay? ;)

Sounds good but see my comment about substrings vs. regex. If you go and 
special case all the characters you're gonna end up reinventing a regex engine, 
but you don't need to do that, we have functions to handle that already.

https://github.com/llvm/llvm-project/pull/69422
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/Interpreter] Make ScriptedInterface Object creation more generic (PR #68052)

[David Spickett via lldb-commits]

DavidSpickett wrote:

We still have a failure on Windows after the follow ups:
```
# .---command stderr
# | Cannot create an instance of the ScriptedProcessInterface!
# | UNREACHABLE executed at 
C:\Users\tcwg\david.spickett\llvm-project\lldb\include\lldb/Interpreter/Interfaces/ScriptedProcessInterface.h:29!
# | PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace.
# | Stack dump:
# | 0.  Program arguments: 
c:\\users\\tcwg\\david.spickett\\build-llvm\\bin\\lldb-test.exe ir-memory-map 
C:\\Users\\tcwg\\david.spickett\\build-llvm\\tools\\lldb\\test\\Shell\\Expr\\Output\\TestIRMemoryMapWindows.test.tmp
 
C:\\Users\\tcwg\\david.spickett\\llvm-project\\lldb\\test\\Shell\\Expr/Inputs/ir-memory-map-basic
# | 1.  HandleCommand(command = "run")
# | Exception Code: 0xC01D
# |  #0 0x7ff71e493e0c HandleAbort 
C:\Users\tcwg\david.spickett\llvm-project\llvm\lib\Support\Windows\Signals.inc:424:0
# |  #1 0x7ffe20f30ef8 (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0xa0ef8)
# |  #2 0x7ffe20f32a50 (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0xa2a50)
# |  #3 0x7ff71e460358 llvm::llvm_unreachable_internal(char const *, char 
const *, unsigned int) 
C:\Users\tcwg\david.spickett\llvm-project\llvm\lib\Support\ErrorHandling.cpp:212:0
# |  #4 0x7ff71e999fc4 
lldb_private::ScriptedProcessInterface::CreatePluginObject(class 
llvm::StringRef, class lldb_private::ExecutionContext &, class 
std::shared_ptr, class 
lldb_private::StructuredData::Generic *) 
C:\Users\tcwg\david.spickett\llvm-project\lldb\include\lldb\Interpreter\Interfaces\ScriptedProcessInterface.h:28:0
```
I'm looking into it.

https://github.com/llvm/llvm-project/pull/68052
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/Interpreter] Make ScriptedInterface Object creation more generic (PR #68052)

[David Spickett via lldb-commits]

DavidSpickett wrote:

Somehow it is getting to:
```
  virtual llvm::Expected
  CreatePluginObject(llvm::StringRef class_name, ExecutionContext &exe_ctx,
 StructuredData::DictionarySP args_sp,
 StructuredData::Generic *script_obj = nullptr) {
llvm_unreachable(
"Cannot create an instance of the ScriptedProcessInterface!");
  }
```
Starting to wonder if we're relying on some undefined order of evaluation 
somewhere.

https://github.com/llvm/llvm-project/pull/68052
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 53096f9 - [lldb] Try to fix build after d1556e5efbf0

[Nico Weber via lldb-commits]

Author: Nico Weber
Date: 2023-10-26T07:37:42-07:00
New Revision: 53096f910ca874cf446417d0bf551c5d63e917b2

URL: 
https://github.com/llvm/llvm-project/commit/53096f910ca874cf446417d0bf551c5d63e917b2
DIFF: 
https://github.com/llvm/llvm-project/commit/53096f910ca874cf446417d0bf551c5d63e917b2.diff

LOG: [lldb] Try to fix build after d1556e5efbf0

Getting lots of `error: unknown type name 'uint64_t'` and `uint32_t`
in this file on Linux.

Added: 


Modified: 
lldb/include/lldb/Target/RegisterFlags.h

Removed: 




diff  --git a/lldb/include/lldb/Target/RegisterFlags.h 
b/lldb/include/lldb/Target/RegisterFlags.h
index 7c5b97c2265fda3..a088981918cb349 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_TARGET_REGISTERFLAGS_H
 #define LLDB_TARGET_REGISTERFLAGS_H
 
+#include 
 #include 
 #include 
 



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/Interpreter] Make ScriptedInterface Object creation more generic (PR #68052)

[David Spickett via lldb-commits]

DavidSpickett wrote:

Never mind, this could have been the result of using an incompatible Python 
version. I'll get a stacktrace with one I know is used on the bot.

https://github.com/llvm/llvm-project/pull/68052
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Diagnose use of VLAs in a coroutine (PR #70341)

[Aaron Ballman via lldb-commits]

https://github.com/AaronBallman updated 
https://github.com/llvm/llvm-project/pull/70341

>From f068b471efa81d60d1d6e2c98da56be58958a015 Mon Sep 17 00:00:00 2001
From: Aaron Ballman 
Date: Thu, 26 Oct 2023 10:53:06 -0400
Subject: [PATCH 1/2] Diagnose use of VLAs in a coroutine

Fixes https://github.com/llvm/llvm-project/issues/65858
---
 .../clang/Basic/DiagnosticSemaKinds.td|  2 ++
 clang/include/clang/Sema/ScopeInfo.h  |  8 +
 clang/lib/Sema/SemaCoroutine.cpp  |  5 
 clang/lib/Sema/SemaType.cpp   | 18 
 clang/test/SemaCXX/coroutine-vla.cpp  | 29 +++
 5 files changed, 56 insertions(+), 6 deletions(-)
 create mode 100644 clang/test/SemaCXX/coroutine-vla.cpp

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a673ce726d6c220..453bd8a9a340425 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -166,6 +166,8 @@ def ext_vla_folded_to_constant : ExtWarn<
   InGroup;
 def err_vla_unsupported : Error<
   "variable length arrays are not supported for %select{the current 
target|'%1'}0">;
+def err_vla_in_coroutine_unsupported : Error<
+  "variable length arrays in a coroutine are not supported">;
 def note_vla_unsupported : Note<
   "variable length arrays are not supported for the current target">;
 
diff --git a/clang/include/clang/Sema/ScopeInfo.h 
b/clang/include/clang/Sema/ScopeInfo.h
index 02b22af89ff035d..b2f6e3289f41fce 100644
--- a/clang/include/clang/Sema/ScopeInfo.h
+++ b/clang/include/clang/Sema/ScopeInfo.h
@@ -189,6 +189,9 @@ class FunctionScopeInfo {
   /// First SEH '__try' statement in the current function.
   SourceLocation FirstSEHTryLoc;
 
+  /// First use of a VLA within the current function.
+  SourceLocation FirstVLALoc;
+
 private:
   /// Used to determine if errors occurred in this function or block.
   DiagnosticErrorTrap ErrorTrap;
@@ -473,6 +476,11 @@ class FunctionScopeInfo {
 FirstSEHTryLoc = TryLoc;
   }
 
+  void setHasVLA(SourceLocation VLALoc) {
+if (FirstVLALoc.isInvalid())
+  FirstVLALoc = VLALoc;
+  }
+
   bool NeedsScopeChecking() const {
 return !HasDroppedStmt && (HasIndirectGoto || HasMustTail ||
(HasBranchProtectedScope && 
HasBranchIntoScope));
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index d2b0922a4bb9c4c..e1f3b5acb3cadec 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1198,6 +1198,11 @@ void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, 
Stmt *&Body) {
   if (FD->hasAttr())
 Diag(FD->getLocation(), diag::warn_always_inline_coroutine);
 
+  // We don't allow use of VLAs within a coroutine, so diagnose if we've seen
+  // a VLA in the body of this function.
+  if (Fn->FirstVLALoc.isValid())
+Diag(Fn->FirstVLALoc, diag::err_vla_in_coroutine_unsupported);
+
   // [stmt.return.coroutine]p1:
   //   A coroutine shall not enclose a return statement ([stmt.return]).
   if (Fn->FirstReturnLoc.isValid()) {
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 28b81c1768a3004..dea77fae4cadb59 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -2706,12 +2706,18 @@ QualType Sema::BuildArrayType(QualType T, 
ArrayType::ArraySizeModifier ASM,
 }
   }
 
-  if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) {
-// CUDA device code and some other targets don't support VLAs.
-bool IsCUDADevice = (getLangOpts().CUDA && getLangOpts().CUDAIsDevice);
-targetDiag(Loc,
-   IsCUDADevice ? diag::err_cuda_vla : diag::err_vla_unsupported)
-<< (IsCUDADevice ? CurrentCUDATarget() : 0);
+  if (T->isVariableArrayType()) {
+if (!Context.getTargetInfo().isVLASupported()) {
+  // CUDA device code and some other targets don't support VLAs.
+  bool IsCUDADevice = (getLangOpts().CUDA && getLangOpts().CUDAIsDevice);
+  targetDiag(Loc,
+ IsCUDADevice ? diag::err_cuda_vla : diag::err_vla_unsupported)
+  << (IsCUDADevice ? CurrentCUDATarget() : 0);
+} else if (sema::FunctionScopeInfo *FSI = getCurFunction()) {
+  // VLAs are supported on this target, but we may need to do delayed
+  // checking that the VLA is not being used within a coroutine.
+  FSI->setHasVLA(Loc);
+}
   }
 
   // If this is not C99, diagnose array size modifiers on non-VLAs.
diff --git a/clang/test/SemaCXX/coroutine-vla.cpp 
b/clang/test/SemaCXX/coroutine-vla.cpp
new file mode 100644
index 000..176e35f346e2b45
--- /dev/null
+++ b/clang/test/SemaCXX/coroutine-vla.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -Wno-vla-cxx-extension -verify
+#include "Inputs/std-coroutine.h"
+
+struct promise;
+
+struct coroutine : std::coroutine_handle {
+  using promise_type = ::promise;
+};
+
+stru

[Lldb-commits] [lldb] Diagnose use of VLAs in a coroutine (PR #70341)

[Aaron Ballman via lldb-commits]

https://github.com/AaronBallman closed 
https://github.com/llvm/llvm-project/pull/70341
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 45ccc16 - [lldb][test][Windows] XFAIL IR memory map test

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2023-10-26T16:17:14Z
New Revision: 45ccc1666c723e11d7b0148b2ef5c37c7a36e916

URL: 
https://github.com/llvm/llvm-project/commit/45ccc1666c723e11d7b0148b2ef5c37c7a36e916
DIFF: 
https://github.com/llvm/llvm-project/commit/45ccc1666c723e11d7b0148b2ef5c37c7a36e916.diff

LOG: [lldb][test][Windows] XFAIL IR memory map test

Since https://github.com/llvm/llvm-project/pull/68052 this has been failing.

https://lab.llvm.org/buildbot/#/builders/219/builds/6545

Follow up changes have not fixed it, XFAIL while I debug it.

Added: 


Modified: 
lldb/test/Shell/Expr/TestIRMemoryMapWindows.test

Removed: 




diff  --git a/lldb/test/Shell/Expr/TestIRMemoryMapWindows.test 
b/lldb/test/Shell/Expr/TestIRMemoryMapWindows.test
index ae29492c9ccc9fb..f9f4da3c4092029 100644
--- a/lldb/test/Shell/Expr/TestIRMemoryMapWindows.test
+++ b/lldb/test/Shell/Expr/TestIRMemoryMapWindows.test
@@ -1,4 +1,5 @@
 # REQUIRES: system-windows
+# XFAIL: system-windows
 
 # RUN: %clang_cl_host /Zi /GS- %p/Inputs/call-function.cpp /c /o %t.obj
 # RUN: %msvc_link /debug:full %t.obj /out:%t



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/Target] Delay image loading after corefile process creation (PR #70351)

[Med Ismail Bennani via lldb-commits]

https://github.com/medismailben created 
https://github.com/llvm/llvm-project/pull/70351

This patch is a follow-up to db223b7f01f7. Similarly to it, it changes the 
timing of binary image loading for the ProcessMachCore plugin.

This issue came up after getting reports of scripting resources that would fail 
to execute because they relied on data provided by the corefile process (i.e. 
for reading memory). However, rior to this change, the scripting resource 
loading would happen as part of the binary image loading, which in turns 
happened before the process finished being created.

This patch address that issue by delaying the binary image loading phase until 
we receive the corefile process stop event event, ensuring that the process is 
fully formed.

>From 81ace8b87271098c3c9dd615d2dc401b9414ea37 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani 
Date: Thu, 26 Oct 2023 01:11:02 -0700
Subject: [PATCH] [lldb/Target] Delay image loading after corefile process
 creation

This patch is a follow-up to db223b7f01f7. Similarly to it, it changes
the timing of binary image loading for the ProcessMachCore plugin.

This issue came up after getting reports of scripting resources that
would fail to execute because they relied on data provided by the corefile
process (i.e. for reading memory). However, rior to this change, the
scripting resource loading would happen as part of the binary image
loading, which in turns happened before the process finished being created.

This patch address that issue by delaying the binary image loading phase
until we receive the corefile process stop event event, ensuring that the
process is fully formed.

Signed-off-by: Med Ismail Bennani 
---
 lldb/include/lldb/Target/Process.h|  2 ++
 .../Process/mach-core/ProcessMachCore.cpp |  4 +--
 .../Process/mach-core/ProcessMachCore.h   |  2 ++
 lldb/source/Target/Process.cpp| 29 ++-
 4 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index a6d3e6c2d16926e..e25e82302a56dd9 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -614,6 +614,8 @@ class Process : public 
std::enable_shared_from_this,
 return error;
   }
 
+  virtual void DidLoadCore() {}
+
   /// The "ShadowListener" for a process is just an ordinary Listener that 
   /// listens for all the Process event bits.  It's convenient because you can
   /// specify it in the LaunchInfo or AttachInfo, so it will get events from
diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp 
b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index b11062a0224abc2..9b10a0b832915d3 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -570,8 +570,6 @@ Status ProcessMachCore::DoLoadCore() {
 
   CreateMemoryRegions();
 
-  LoadBinariesAndSetDYLD();
-
   CleanupMemoryRegionPermissions();
 
   AddressableBits addressable_bits = core_objfile->GetAddressableBits();
@@ -580,6 +578,8 @@ Status ProcessMachCore::DoLoadCore() {
   return error;
 }
 
+void ProcessMachCore::DidLoadCore() { LoadBinariesAndSetDYLD(); }
+
 lldb_private::DynamicLoader *ProcessMachCore::GetDynamicLoader() {
   if (m_dyld_up.get() == nullptr)
 m_dyld_up.reset(DynamicLoader::FindPlugin(this, m_dyld_plugin_name));
diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h 
b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
index c8820209e3f3830..0e61daa625b53cc 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
@@ -46,6 +46,8 @@ class ProcessMachCore : public 
lldb_private::PostMortemProcess {
   // Creating a new process, or attaching to an existing one
   lldb_private::Status DoLoadCore() override;
 
+  void DidLoadCore() override;
+
   lldb_private::DynamicLoader *GetDynamicLoader() override;
 
   // PluginInterface protocol
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index f82ab05362fbee9..f4bacf314dd746a 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2639,19 +2639,6 @@ Status Process::LoadCore() {
 else
   StartPrivateStateThread();
 
-DynamicLoader *dyld = GetDynamicLoader();
-if (dyld)
-  dyld->DidAttach();
-
-GetJITLoaders().DidAttach();
-
-SystemRuntime *system_runtime = GetSystemRuntime();
-if (system_runtime)
-  system_runtime->DidAttach();
-
-if (!m_os_up)
-  LoadOperatingSystemPlugin(false);
-
 // We successfully loaded a core file, now pretend we stopped so we can
 // show all of the threads in the core file and explore the crashed state.
 SetPrivateState(eStateStopped);
@@ -2668,7 +2655,23 @@ Status Process::LoadCore() {
 StateAsCString(state));
   error.SetErrorString(
   "Did not

[Lldb-commits] [lldb] [lldb/Target] Delay image loading after corefile process creation (PR #70351)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Med Ismail Bennani (medismailben)


Changes

This patch is a follow-up to db223b7f01f7. Similarly to it, it changes the 
timing of binary image loading for the ProcessMachCore plugin.

This issue came up after getting reports of scripting resources that would fail 
to execute because they relied on data provided by the corefile process (i.e. 
for reading memory). However, rior to this change, the scripting resource 
loading would happen as part of the binary image loading, which in turns 
happened before the process finished being created.

This patch address that issue by delaying the binary image loading phase until 
we receive the corefile process stop event event, ensuring that the process is 
fully formed.

---
Full diff: https://github.com/llvm/llvm-project/pull/70351.diff


4 Files Affected:

- (modified) lldb/include/lldb/Target/Process.h (+2) 
- (modified) lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp (+2-2) 
- (modified) lldb/source/Plugins/Process/mach-core/ProcessMachCore.h (+2) 
- (modified) lldb/source/Target/Process.cpp (+16-13) 


``diff
diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index a6d3e6c2d16926e..e25e82302a56dd9 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -614,6 +614,8 @@ class Process : public 
std::enable_shared_from_this,
 return error;
   }
 
+  virtual void DidLoadCore() {}
+
   /// The "ShadowListener" for a process is just an ordinary Listener that 
   /// listens for all the Process event bits.  It's convenient because you can
   /// specify it in the LaunchInfo or AttachInfo, so it will get events from
diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp 
b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index b11062a0224abc2..9b10a0b832915d3 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -570,8 +570,6 @@ Status ProcessMachCore::DoLoadCore() {
 
   CreateMemoryRegions();
 
-  LoadBinariesAndSetDYLD();
-
   CleanupMemoryRegionPermissions();
 
   AddressableBits addressable_bits = core_objfile->GetAddressableBits();
@@ -580,6 +578,8 @@ Status ProcessMachCore::DoLoadCore() {
   return error;
 }
 
+void ProcessMachCore::DidLoadCore() { LoadBinariesAndSetDYLD(); }
+
 lldb_private::DynamicLoader *ProcessMachCore::GetDynamicLoader() {
   if (m_dyld_up.get() == nullptr)
 m_dyld_up.reset(DynamicLoader::FindPlugin(this, m_dyld_plugin_name));
diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h 
b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
index c8820209e3f3830..0e61daa625b53cc 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
@@ -46,6 +46,8 @@ class ProcessMachCore : public 
lldb_private::PostMortemProcess {
   // Creating a new process, or attaching to an existing one
   lldb_private::Status DoLoadCore() override;
 
+  void DidLoadCore() override;
+
   lldb_private::DynamicLoader *GetDynamicLoader() override;
 
   // PluginInterface protocol
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index f82ab05362fbee9..f4bacf314dd746a 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2639,19 +2639,6 @@ Status Process::LoadCore() {
 else
   StartPrivateStateThread();
 
-DynamicLoader *dyld = GetDynamicLoader();
-if (dyld)
-  dyld->DidAttach();
-
-GetJITLoaders().DidAttach();
-
-SystemRuntime *system_runtime = GetSystemRuntime();
-if (system_runtime)
-  system_runtime->DidAttach();
-
-if (!m_os_up)
-  LoadOperatingSystemPlugin(false);
-
 // We successfully loaded a core file, now pretend we stopped so we can
 // show all of the threads in the core file and explore the crashed state.
 SetPrivateState(eStateStopped);
@@ -2668,7 +2655,23 @@ Status Process::LoadCore() {
 StateAsCString(state));
   error.SetErrorString(
   "Did not get stopped event after loading the core file.");
+} else {
+  DidLoadCore();
+
+  DynamicLoader *dyld = GetDynamicLoader();
+  if (dyld)
+dyld->DidAttach();
+
+  GetJITLoaders().DidAttach();
+
+  SystemRuntime *system_runtime = GetSystemRuntime();
+  if (system_runtime)
+system_runtime->DidAttach();
+
+  if (!m_os_up)
+LoadOperatingSystemPlugin(false);
 }
+
 RestoreProcessEvents();
   }
   return error;

``




https://github.com/llvm/llvm-project/pull/70351
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added a comment.

In D140996#4646880 , @cor3ntin wrote:

> It would be nice to figure out a plan for this PR in the next few weeks, 
> before we get kicked out of Phab!

Agreed -- @bolshakov-a are you still planning to actively work on this patch 
(it's okay if you're not, but in that case, do you mind if someone commandeers 
the patch)? We've got a few weeks left until Phabricator is going into 
read-only mode, so it would be good if we could get this review across the 
finish line before mid-Nov if possible.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140996/new/

https://reviews.llvm.org/D140996

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

[Andrey Ali Khan Bolshakov via Phabricator via lldb-commits]

bolshakov-a added a comment.

Sorry, but I don't know what remains to be done here. It seems that the only 
important question is about ABI, but I've already answered that the changes 
under discussion seem to be already fixed in the Itanium ABI document.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140996/new/

https://reviews.llvm.org/D140996

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D159314: [lldb] Introduce OperatingSystem{, Python}Interface and make use it

[Alex Langford via Phabricator via lldb-commits]

bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

Ok, makes sense to me. Since this is a refactor, I assume that the current test 
suite should be enough to cover this change?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159314/new/

https://reviews.llvm.org/D159314

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [OpenACC] Initial commits to support OpenACC (PR #70234)

[Erich Keane via lldb-commits]

https://github.com/erichkeane updated 
https://github.com/llvm/llvm-project/pull/70234

>From b3d64b3f744ccb37e334e3aae8d6874cd8391c56 Mon Sep 17 00:00:00 2001
From: erichkeane 
Date: Mon, 23 Oct 2023 11:09:11 -0700
Subject: [PATCH 1/5] [OpenACC] Initial commit for OpenACC Support

This is the initial commit to support OpenACC in Clang, which adds a
clang-command line argument '-fopenacc', and starts to define _OPENACC,
albeit to '1' instead of the standardized value (since we don't
properly implement OpenACC yet).
---
 clang/docs/ReleaseNotes.rst   | 12 
 clang/include/clang/Basic/LangOptions.def |  2 ++
 clang/include/clang/Driver/Options.td | 10 --
 clang/lib/Driver/ToolChains/Clang.cpp | 11 +++
 clang/lib/Frontend/CompilerInvocation.cpp |  7 +++
 clang/lib/Frontend/InitPreprocessor.cpp   |  6 ++
 clang/test/Driver/openacc.c   |  2 ++
 clang/test/Preprocessor/openacc.c |  4 
 8 files changed, 52 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/Driver/openacc.c
 create mode 100644 clang/test/Preprocessor/openacc.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f94e4e10b805911..8f40872b539322a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -211,6 +211,11 @@ New Compiler Flags
   the preprocessed text to the output. This can greatly reduce the size of the
   preprocessed output, which can be helpful when trying to reduce a test case.
 
+* ``-Wbitfield-conversion`` was added to detect assignments of integral
+  types to a bitfield that may change the value.
+
+* ``-fopenacc`` was added as a part of the effort to support OpenACC in clang.
+
 Deprecated Compiler Flags
 -
 
@@ -665,6 +670,13 @@ Miscellaneous Clang Crashes Fixed
 - Fixed a crash when an ObjC ivar has an invalid type. See
   (`#68001 `_)
 
+OpenACC Specific Changes
+
+- OpenACC Implementation effort is beginning with semantic analysis and parsing
+  of OpenACC pragmas. The ``-fopenacc`` flag was added to enable these new,
+  albeit incomplete changes. The ``_OPENACC`` macro is currently defined to
+  ``1``, as support is too incomplete to update to a standards-required value.
+
 Target Specific Changes
 ---
 
diff --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index c0ea4ecb9806a5b..872d693cc3ebbff 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -283,6 +283,8 @@ LANGOPT(OffloadUniformBlock, 1, 0, "Assume that kernels are 
launched with unifor
 LANGOPT(HIPStdPar, 1, 0, "Enable Standard Parallel Algorithm Acceleration for 
HIP (experimental)")
 LANGOPT(HIPStdParInterposeAlloc, 1, 0, "Replace allocations / deallocations 
with HIP RT calls when Standard Parallel Algorithm Acceleration for HIP is 
enabled (Experimental)")
 
+LANGOPT(OpenACC   , 1, 0, "OpenACC Enabled")
+
 LANGOPT(SizedDeallocation , 1, 0, "sized deallocation")
 LANGOPT(AlignedAllocation , 1, 0, "aligned allocation")
 LANGOPT(AlignedAllocationUnavailable, 1, 0, "aligned allocation functions are 
unavailable")
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index c6b1903a32a0621..ab28c3e394afe93 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3340,6 +3340,14 @@ def fno_openmp_target_debug : Flag<["-"], 
"fno-openmp-target-debug">;
 } // let Visibility = [ClangOption, CC1Option, FC1Option]
 } // let Flags = [NoArgumentUnused]
 
+//===--===//
+// FlangOption + FC1 + ClangOption + CC1Option
+//===--===//
+let Visibility = [FC1Option, FlangOption, CC1Option, ClangOption] in {
+def fopenacc : Flag<["-"], "fopenacc">, Group,
+  HelpText<"Enable OpenACC">;
+} // let Visibility = [FC1Option, FlangOption, CC1Option, ClangOption]
+
 
//===--===//
 // Optimisation remark options
 
//===--===//
@@ -6256,8 +6264,6 @@ file}]>;
 def ffixed_line_length_VALUE : Joined<["-"], "ffixed-line-length-">, 
Group, Alias;
 def fconvert_EQ : Joined<["-"], "fconvert=">, Group,
   HelpText<"Set endian conversion of data for unformatted files">;
-def fopenacc : Flag<["-"], "fopenacc">, Group,
-  HelpText<"Enable OpenACC">;
 def fdefault_double_8 : Flag<["-"],"fdefault-double-8">, Group,
   HelpText<"Set the default double precision kind to an 8 byte wide type">;
 def fdefault_integer_8 : Flag<["-"],"fdefault-integer-8">, Group,
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 601bbfb927746fc..cf65773de0d010d 100644
--- a/clang/l

[Lldb-commits] [lldb] [lldb][AArch64][Linux] Rename IsEnabled to IsPresent (PR #70303)

[Alex Langford via lldb-commits]

https://github.com/bulbazord edited 
https://github.com/llvm/llvm-project/pull/70303
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AArch64][Linux] Rename IsEnabled to IsPresent (PR #70303)

[Alex Langford via lldb-commits]

https://github.com/bulbazord approved this pull request.

LGTM 😃 

https://github.com/llvm/llvm-project/pull/70303
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AArch64][Linux] Rename IsEnabled to IsPresent (PR #70303)

[Alex Langford via lldb-commits]

@@ -610,7 +610,7 @@ 
NativeRegisterContextLinux_arm64::CacheAllRegisters(uint32_t &cached_size) {
 return error;
 
   // Here this means, does the system have ZA, not whether it is active.

bulbazord wrote:

You could probably remove this comment

https://github.com/llvm/llvm-project/pull/70303
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/Target] Delay image loading after corefile process creation (PR #70351)

[Jason Molenda via lldb-commits]

https://github.com/jasonmolenda approved this pull request.

Yes this looks good to me.  I originally had a little concern about changing 
the ordering of how we call the methods in the corefile process plugins, but I 
don't think that's going to be an issue for them.

https://github.com/llvm/llvm-project/pull/70351
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/Target] Delay image loading after corefile process creation (PR #70351)

[via lldb-commits]

jimingham wrote:

Is there a reason this is DidLoadCore only has a non-trivial implementation for 
Mach?

The generic and Mach parts of this are fine, so if there's a reason why this 
isn't needed for the other platform's core files, then this LGTM as well.

https://github.com/llvm/llvm-project/pull/70351
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D159314: [lldb] Introduce OperatingSystem{, Python}Interface and make use it

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D159314#4655289 , @bulbazord wrote:

> Ok, makes sense to me. Since this is a refactor, I assume that the current 
> test suite should be enough to cover this change?

@bulbazord yes!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159314/new/

https://reviews.llvm.org/D159314

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 7a1e878 - [lldb] Introduce OperatingSystem{, Python}Interface and make use it

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2023-10-26T15:12:22-07:00
New Revision: 7a1e8783586ecc90ee15f12c7b76799313bb32e8

URL: 
https://github.com/llvm/llvm-project/commit/7a1e8783586ecc90ee15f12c7b76799313bb32e8
DIFF: 
https://github.com/llvm/llvm-project/commit/7a1e8783586ecc90ee15f12c7b76799313bb32e8.diff

LOG: [lldb] Introduce OperatingSystem{,Python}Interface and make use it

This patch aims to consolidate the OperatingSystem scripting affordance
by introducing a stable interface that conforms to the
Scripted{,Python}Interface.

This unify the way we call into python methods from lldb while
also improving its capabilities by allowing us to pass lldb_private
objects are arguments.

Differential Revision: https://reviews.llvm.org/D159314

Signed-off-by: Med Ismail Bennani 

Added: 
lldb/include/lldb/Interpreter/Interfaces/OperatingSystemInterface.h

lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.cpp

lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h

Modified: 
lldb/include/lldb/Interpreter/ScriptInterpreter.h
lldb/include/lldb/lldb-forward.h
lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/CMakeLists.txt

lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Removed: 




diff  --git 
a/lldb/include/lldb/Interpreter/Interfaces/OperatingSystemInterface.h 
b/lldb/include/lldb/Interpreter/Interfaces/OperatingSystemInterface.h
new file mode 100644
index 000..3c46f99f3b356fe
--- /dev/null
+++ b/lldb/include/lldb/Interpreter/Interfaces/OperatingSystemInterface.h
@@ -0,0 +1,33 @@
+//===-- OperatingSystemInterface.h --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_INTERPRETER_INTERFACES_OPERATINGSYSTEMINTERFACE_H
+#define LLDB_INTERPRETER_INTERFACES_OPERATINGSYSTEMINTERFACE_H
+
+#include "ScriptedThreadInterface.h"
+#include "lldb/Core/StructuredDataImpl.h"
+
+#include "lldb/lldb-private.h"
+
+namespace lldb_private {
+class OperatingSystemInterface : virtual public ScriptedThreadInterface {
+public:
+  virtual StructuredData::DictionarySP CreateThread(lldb::tid_t tid,
+lldb::addr_t context) {
+return {};
+  }
+
+  virtual StructuredData::ArraySP GetThreadInfo() { return {}; }
+
+  virtual std::optional GetRegisterContextForTID(lldb::tid_t tid) 
{
+return std::nullopt;
+  }
+};
+} // namespace lldb_private
+
+#endif // LLDB_INTERPRETER_INTERFACES_OPERATINGSYSTEMINTERFACE_H

diff  --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h 
b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index eacd10d5279be6f..0146eeb86262003 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -21,6 +21,7 @@
 #include "lldb/Core/ThreadedCommunication.h"
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/StreamFile.h"
+#include "lldb/Interpreter/Interfaces/OperatingSystemInterface.h"
 #include "lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h"
 #include "lldb/Interpreter/Interfaces/ScriptedProcessInterface.h"
 #include "lldb/Interpreter/Interfaces/ScriptedThreadInterface.h"
@@ -252,34 +253,6 @@ class ScriptInterpreter : public PluginInterface {
 return lldb::ValueObjectListSP();
   }
 
-  virtual StructuredData::GenericSP
-  OSPlugin_CreatePluginObject(const char *class_name,
-  lldb::ProcessSP process_sp) {
-return StructuredData::GenericSP();
-  }
-
-  virtual StructuredData::DictionarySP
-  OSPlugin_RegisterInfo(StructuredData::ObjectSP os_plugin_object_sp) {
-return StructuredData::DictionarySP();
-  }
-
-  virtual StructuredData::ArraySP
-  OSPlugin_ThreadsInfo(StructuredData::ObjectSP os_plugin_object_sp) {
-return StructuredData::ArraySP();
-  }
-
-  virtual StructuredData::StringSP
-  OSPlugin_RegisterContextData(StructuredData::ObjectSP os_plugin_object_sp,
-   lldb::tid_t thread_id) {
-return StructuredData::StringSP();
-  }
-
-  virtual StructuredData::DictionarySP
-  OSPlugin_CreateThread(StructuredData::ObjectSP os_plugin_object_sp,
-lldb::tid_t tid, lldb::addr_t context) {
-retur

[Lldb-commits] [lldb] ec456ba - [lldb] Add OperatingSystem base class to the lldb python module

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2023-10-26T15:12:22-07:00
New Revision: ec456ba9ca0a097da63daafbb031cb2024f5513a

URL: 
https://github.com/llvm/llvm-project/commit/ec456ba9ca0a097da63daafbb031cb2024f5513a
DIFF: 
https://github.com/llvm/llvm-project/commit/ec456ba9ca0a097da63daafbb031cb2024f5513a.diff

LOG: [lldb] Add OperatingSystem base class to the lldb python module

This patch introduces an `OperatingSystem` base implementation in the
`lldb` python module to make it easier for lldb users to write their own
implementation.

The `OperatingSystem` base implementation is derived itself from the
`ScriptedThread` base implementation since they share some common grounds.

To achieve that, this patch makes changes to the `ScriptedThread`
initializer since it gets called by the `OperatingSystem` initializer.

I also took the opportunity to document the `OperatingSystem` base
class and methods.

Differential Revision: https://reviews.llvm.org/D159315

Signed-off-by: Med Ismail Bennani 

Added: 
lldb/examples/python/templates/operating_system.py

Modified: 
lldb/bindings/python/CMakeLists.txt
lldb/examples/python/templates/scripted_process.py
lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py

Removed: 




diff  --git a/lldb/bindings/python/CMakeLists.txt 
b/lldb/bindings/python/CMakeLists.txt
index c4806bda27049c6..c941f764dfc92ac 100644
--- a/lldb/bindings/python/CMakeLists.txt
+++ b/lldb/bindings/python/CMakeLists.txt
@@ -104,7 +104,8 @@ function(finish_swig_python swig_target 
lldb_python_bindings_dir lldb_python_tar
 "plugins"
 FILES
 "${LLDB_SOURCE_DIR}/examples/python/templates/scripted_process.py"
-"${LLDB_SOURCE_DIR}/examples/python/templates/scripted_platform.py")
+"${LLDB_SOURCE_DIR}/examples/python/templates/scripted_platform.py"
+"${LLDB_SOURCE_DIR}/examples/python/templates/operating_system.py")
 
   if(APPLE)
 create_python_package(

diff  --git a/lldb/examples/python/templates/operating_system.py 
b/lldb/examples/python/templates/operating_system.py
new file mode 100644
index 000..a8053bcaa21afe7
--- /dev/null
+++ b/lldb/examples/python/templates/operating_system.py
@@ -0,0 +1,103 @@
+from abc import abstractmethod
+
+import lldb
+import struct
+
+from lldb.plugins.scripted_process import ScriptedThread
+
+
+class OperatingSystem(ScriptedThread):
+"""
+Class that provides data for an instance of a LLDB 'OperatingSystemPython' 
plug-in class.
+
+```
+thread_info = {
+"tid": tid,
+"name": "four",
+"queue": "queue4",
+"state": "stopped",
+"stop_reason": "none",
+"core" : 2
+}
+```
+
+- tid : thread ID (mandatory)
+- name : thread name (optional key/value pair)
+- queue : thread dispatch queue name (optional key/value pair)
+- state : thread state (mandatory, set to 'stopped' for now)
+- core : the index of the core (lldb) thread that this OS Thread should 
shadow
+- stop_reason : thread stop reason. (mandatory, usually set to 'none')
+Possible values include:
+- 'breakpoint': thread is stopped at a breakpoint
+- 'none': thread is stopped because the process is stopped
+- 'trace': thread is stopped after single stepping
+The usual value for this while threads are in memory is 'none'
+- register_data_addr : the address of the register data in memory 
(optional key/value pair)
+Specifying this key/value pair for a thread will avoid a call to 
get_register_data()
+and can be used when your registers are in a thread context structure 
that is contiguous
+in memory. Don't specify this if your register layout in memory 
doesn't match the layout
+described by the dictionary returned from a call to the 
get_register_info() method.
+"""
+
+def __init__(self, process):
+"""Initialization needs a valid lldb.SBProcess object. This plug-in
+will get created after a live process is valid and has stopped for the
+first time.
+
+Args:
+process (lldb.SBProcess): The process owning this thread.
+"""
+self.registers = None
+super().__init__(process, None)
+self.registers = self.register_info
+self.threads = []
+
+def create_thread(self, tid, context):
+"""Lazily create an operating system thread using a thread information
+dictionary and an optional operating system thread context address.
+This method is called manually, using the SBAPI
+`lldb.SBProcess.CreateOSPluginThread` affordance.
+
+Args:
+tid (int): Thread ID to get `thread_info` dictionary for.
+context (int): Address of the operating system thread struct.
+
+Returns:
+Dict: The `thread_info` dictionary containing the various 
information
+   

[Lldb-commits] [PATCH] D159314: [lldb] Introduce OperatingSystem{, Python}Interface and make use it

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7a1e8783586e: [lldb] Introduce 
OperatingSystem{,Python}Interface and make use it (authored by mib).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159314/new/

https://reviews.llvm.org/D159314

Files:
  lldb/include/lldb/Interpreter/Interfaces/OperatingSystemInterface.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
  lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h
  lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/CMakeLists.txt
  
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.cpp
  
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h
  
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -320,6 +320,11 @@
 }
 
 python::PythonObject
-lldb_private::python::SWIGBridge::ToSWIGWrapper(const StructuredDataImpl &) {
+lldb_private::python::SWIGBridge::ToSWIGWrapper(lldb::ProcessSP) {
+  return python::PythonObject();
+}
+
+python::PythonObject lldb_private::python::SWIGBridge::ToSWIGWrapper(
+const lldb_private::StructuredDataImpl &) {
   return python::PythonObject();
 }
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -134,24 +134,9 @@
 
   lldb::ScriptedProcessInterfaceUP CreateScriptedProcessInterface() override;
 
-  StructuredData::GenericSP
-  OSPlugin_CreatePluginObject(const char *class_name,
-  lldb::ProcessSP process_sp) override;
-
-  StructuredData::DictionarySP
-  OSPlugin_RegisterInfo(StructuredData::ObjectSP os_plugin_object_sp) override;
   lldb::ScriptedThreadInterfaceSP CreateScriptedThreadInterface() override;
 
-  StructuredData::ArraySP
-  OSPlugin_ThreadsInfo(StructuredData::ObjectSP os_plugin_object_sp) override;
-
-  StructuredData::StringSP
-  OSPlugin_RegisterContextData(StructuredData::ObjectSP os_plugin_object_sp,
-   lldb::tid_t thread_id) override;
-
-  StructuredData::DictionarySP
-  OSPlugin_CreateThread(StructuredData::ObjectSP os_plugin_object_sp,
-lldb::tid_t tid, lldb::addr_t context) override;
+  lldb::OperatingSystemInterfaceSP CreateOperatingSystemInterface() override;
 
   StructuredData::ObjectSP
   LoadPluginModule(const FileSpec &file_spec,
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -14,6 +14,7 @@
 // LLDB Python header must be included first
 #include "lldb-python.h"
 
+#include "Interfaces/OperatingSystemPythonInterface.h"
 #include "Interfaces/ScriptedPlatformPythonInterface.h"
 #include "Interfaces/ScriptedProcessPythonInterface.h"
 #include "Interfaces/ScriptedThreadPythonInterface.h"
@@ -1521,6 +1522,11 @@
   return std::make_shared(*this);
 }
 
+OperatingSystemInterfaceSP
+ScriptInterpreterPythonImpl::CreateOperatingSystemInterface() {
+  return std::make_shared(*this);
+}
+
 StructuredData::ObjectSP
 ScriptInterpreterPythonImpl::CreateStructuredDataFromScriptObject(
 ScriptObject obj) {
@@ -1532,159 +1538,6 @@
   return py_obj.CreateStructuredObject();
 }
 
-StructuredData::GenericSP
-ScriptInterpreterPythonImpl::OSPlugin_CreatePluginObject(
-const char *class_name, lldb::ProcessSP process_sp) {
-  if (class_name == nullptr || class_name[0] == '\0')
-return StructuredData::GenericSP();
-
-  if (!process_sp)
-return StructuredData::GenericSP();
-
-  Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
-  PythonObject ret_val = SWIGBridge::LLDBSWIGPythonCreateOSPlugin(
-  class_name, m_dictionary_name.c_str(), process_sp);
-
-  return StructuredData::GenericSP(
-  new StructuredPythonObject(std::move(ret_val)));
-}
-
-StructuredData::DictionarySP ScriptInterpreterPythonImpl::OSPlugin_RegisterInfo(
-Structured

[Lldb-commits] [PATCH] D159315: [lldb] Add OperatingSystem base class to the lldb python module

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGec456ba9ca0a: [lldb] Add OperatingSystem base class to the 
lldb python module (authored by mib).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159315/new/

https://reviews.llvm.org/D159315

Files:
  lldb/bindings/python/CMakeLists.txt
  lldb/examples/python/templates/operating_system.py
  lldb/examples/python/templates/scripted_process.py
  lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py

Index: lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py
===
--- lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py
+++ lldb/test/API/functionalities/plugins/python_os_plugin/operating_system.py
@@ -1,29 +1,14 @@
-#!/usr/bin/env python
-
 import lldb
 import struct
 
+from lldb.plugins.operating_system import OperatingSystem
+
 
-class OperatingSystemPlugIn(object):
+class OperatingSystemPlugIn(OperatingSystem):
 """Class that provides data for an instance of a LLDB 'OperatingSystemPython' plug-in class"""
 
 def __init__(self, process):
-"""Initialization needs a valid.SBProcess object.
-
-This plug-in will get created after a live process is valid and has stopped for the
-first time."""
-self.process = None
-self.registers = None
-self.threads = None
-if isinstance(process, lldb.SBProcess) and process.IsValid():
-self.process = process
-self.threads = None  # Will be an dictionary containing info for each thread
-
-def get_target(self):
-# NOTE: Don't use "lldb.target" when trying to get your target as the "lldb.target"
-# tracks the current target in the LLDB command interpreter which isn't the
-# correct thing to use for this plug-in.
-return self.process.target
+super().__init__(process)
 
 def create_thread(self, tid, context):
 if tid == 0x4:
@@ -40,23 +25,6 @@
 
 def get_thread_info(self):
 if not self.threads:
-# The sample dictionary below shows the values that can be returned for a thread
-# tid => thread ID (mandatory)
-# name => thread name (optional key/value pair)
-# queue => thread dispatch queue name (optional key/value pair)
-# state => thred state (mandatory, set to 'stopped' for now)
-# stop_reason => thread stop reason. (mandatory, usually set to 'none')
-#  Possible values include:
-#   'breakpoint' if the thread is stopped at a breakpoint
-#   'none' thread is just stopped because the process is stopped
-#   'trace' the thread just single stepped
-#   The usual value for this while threads are in memory is 'none'
-# register_data_addr => the address of the register data in memory (optional key/value pair)
-#   Specifying this key/value pair for a thread will avoid a call to get_register_data()
-#   and can be used when your registers are in a thread context structure that is contiguous
-#   in memory. Don't specify this if your register layout in memory doesn't match the layout
-# described by the dictionary returned from a call to the
-# get_register_info() method.
 self.threads = [
 {
 "tid": 0x1,
Index: lldb/examples/python/templates/scripted_process.py
===
--- lldb/examples/python/templates/scripted_process.py
+++ lldb/examples/python/templates/scripted_process.py
@@ -244,16 +244,16 @@
 """
 
 @abstractmethod
-def __init__(self, scripted_process, args):
+def __init__(self, process, args):
 """Construct a scripted thread.
 
 Args:
-process (ScriptedProcess): The scripted process owning this thread.
+process (ScriptedProcess/lldb.SBProcess): The process owning this thread.
 args (lldb.SBStructuredData): A Dictionary holding arbitrary
 key/value pairs used by the scripted thread.
 """
 self.target = None
-self.scripted_process = None
+self.originating_process = None
 self.process = None
 self.args = None
 self.idx = 0
@@ -268,9 +268,13 @@
 self.frames = []
 self.extended_info = []
 
-if isinstance(scripted_process, ScriptedProcess):
-self.target = scripted_process.target
-self.scripted_process = scripted_process
+if (
+isinstance(process, ScriptedProcess)
+or isinstance(process, lldb.SBProcess)
+and process.IsValid()
+):
+self.target = process.target
+self.originating_process = proces

[Lldb-commits] [lldb] [lldb/Interpreter] Make ScriptedInterface Object creation more generic (PR #70392)

[Med Ismail Bennani via lldb-commits]

https://github.com/medismailben created 
https://github.com/llvm/llvm-project/pull/70392

This patch changes the way plugin objects used with Scripted Interfaces are 
created.

Instead of implementing a different SWIG method to create the object for every 
scripted interface, this patch makes the creation more generic by re-using some 
of the ScriptedPythonInterface templated Dispatch code.

This patch also improves error handling of the object creation by returning an 
`llvm::Expected`.

>From ef90c8a7f2f555cf312807d2bc83ffda45e8c2af Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani 
Date: Tue, 24 Oct 2023 20:40:43 -0700
Subject: [PATCH] [lldb/Interpreter] Make ScriptedInterface Object creation
 more generic

This patch changes the way plugin objects used with Scripted Interfaces
are created.

Instead of implementing a different SWIG method to create the object for
every scripted interface, this patch makes the creation more generic by
re-using some of the ScriptedPythonInterface templated Dispatch code.

This patch also improves error handling of the object creation by
returning an `llvm::Expected`.

Signed-off-by: Med Ismail Bennani 
---
 lldb/bindings/python/python-wrapper.swig  |  43 ---
 .../Interfaces/ScriptedInterface.h|   5 -
 .../Interfaces/ScriptedPlatformInterface.h|   7 +-
 .../Interfaces/ScriptedProcessInterface.h |   7 +-
 .../Interfaces/ScriptedThreadInterface.h  |   7 +-
 .../Process/scripted/ScriptedProcess.cpp  |   9 +-
 .../Process/scripted/ScriptedThread.cpp   |  12 +-
 .../ScriptedPlatformPythonInterface.cpp   |  26 +
 .../ScriptedPlatformPythonInterface.h |   2 +-
 .../ScriptedProcessPythonInterface.cpp|  26 +
 .../ScriptedProcessPythonInterface.h  |   2 +-
 .../Interfaces/ScriptedPythonInterface.h  | 106 +-
 .../ScriptedThreadPythonInterface.cpp |  36 ++
 .../ScriptedThreadPythonInterface.h   |   2 +-
 .../Python/SWIGPythonBridge.h |   6 -
 .../Python/PythonTestSuite.cpp|  18 +--
 16 files changed, 161 insertions(+), 153 deletions(-)

diff --git a/lldb/bindings/python/python-wrapper.swig 
b/lldb/bindings/python/python-wrapper.swig
index cb54901e66d03c6..17bc7b1f2198709 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -229,49 +229,6 @@ PythonObject 
lldb_private::python::SWIGBridge::LLDBSwigPythonCreateCommandObject
   return pfunc(SWIGBridge::ToSWIGWrapper(std::move(debugger_sp)), dict);
 }
 
-PythonObject 
lldb_private::python::SWIGBridge::LLDBSwigPythonCreateScriptedObject(
-const char *python_class_name, const char *session_dictionary_name,
-lldb::ExecutionContextRefSP exe_ctx_sp,
-const lldb_private::StructuredDataImpl &args_impl,
-std::string &error_string) {
-  if (python_class_name == NULL || python_class_name[0] == '\0' ||
-  !session_dictionary_name)
-return PythonObject();
-
-  PyErr_Cleaner py_err_cleaner(true);
-
-  auto dict = PythonModule::MainModule().ResolveName(
-  session_dictionary_name);
-  auto pfunc = PythonObject::ResolveNameWithDictionary(
-  python_class_name, dict);
-
-  if (!pfunc.IsAllocated()) {
-error_string.append("could not find script class: ");
-error_string.append(python_class_name);
-return PythonObject();
-  }
-
-  llvm::Expected arg_info = pfunc.GetArgInfo();
-  if (!arg_info) {
-llvm::handleAllErrors(
-arg_info.takeError(),
-[&](PythonException &E) { error_string.append(E.ReadBacktrace()); },
-[&](const llvm::ErrorInfoBase &E) {
-  error_string.append(E.message());
-});
-return PythonObject();
-  }
-
-  PythonObject result = {};
-  if (arg_info.get().max_positional_args == 2) {
-  result = pfunc(SWIGBridge::ToSWIGWrapper(exe_ctx_sp), 
SWIGBridge::ToSWIGWrapper(args_impl));
-  } else {
-error_string.assign("wrong number of arguments in __init__, should be 2 "
-"(not including self)");
-  }
-  return result;
-}
-
 PythonObject 
lldb_private::python::SWIGBridge::LLDBSwigPythonCreateScriptedThreadPlan(
 const char *python_class_name, const char *session_dictionary_name,
 const lldb_private::StructuredDataImpl &args_impl,
diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h 
b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h
index 948f763e95ecea4..2406f0f1f9aee27 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h
@@ -25,11 +25,6 @@ class ScriptedInterface {
   ScriptedInterface() = default;
   virtual ~ScriptedInterface() = default;
 
-  virtual StructuredData::GenericSP
-  CreatePluginObject(llvm::StringRef class_name, ExecutionContext &exe_ctx,
- StructuredData::DictionarySP args_sp,
- StructuredData::Generic *script_obj = nullptr) = 0;
-
   StructuredData::

[Lldb-commits] [lldb] [lldb/Interpreter] Make ScriptedInterface Object creation more generic (PR #70392)

[Med Ismail Bennani via lldb-commits]

https://github.com/medismailben updated 
https://github.com/llvm/llvm-project/pull/70392

>From 76f655166b36928c1fe10c3124025c50d6eb1ceb Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani 
Date: Thu, 26 Oct 2023 16:30:20 -0700
Subject: [PATCH] [lldb] Make use of Scripted{Python,}Interface for
 ScriptedThreadPlan

This patch makes ScriptedThreadPlan conforming to the ScriptedInterface
& ScriptedPythonInterface facilities by introducing 2
ScriptedThreadPlanInterface & ScriptedThreadPlanPythonInterface classes.

This allows us to get rid of every ScriptedThreadPlan-specific SWIG
method and re-use the same affordances as other scripting offordances,
like Scripted{Process,Thread,Platform} & OperatingSystem.

To do so, this adds new transformer methods for `ThreadPlan`, `Stream` &
`Event`, to allow the bijection between C++ objects and their python
counterparts.

Signed-off-by: Med Ismail Bennani 
---
 lldb/bindings/python/python-swigsafecast.swig |  13 +-
 lldb/bindings/python/python-wrapper.swig  | 153 +++---
 lldb/include/lldb/API/SBEvent.h   |   4 +-
 lldb/include/lldb/API/SBStream.h  |   9 ++
 .../Interfaces/ScriptedInterface.h|   9 +-
 .../Interfaces/ScriptedThreadPlanInterface.h  |  40 +
 .../lldb/Interpreter/ScriptInterpreter.h  |  56 ++-
 lldb/include/lldb/Target/ThreadPlanPython.h   |   2 +
 lldb/include/lldb/lldb-forward.h  |   3 +
 lldb/source/Interpreter/ScriptInterpreter.cpp |  13 ++
 .../Python/Interfaces/CMakeLists.txt  |   1 +
 .../ScriptedPlatformPythonInterface.cpp   |   2 +
 .../Interfaces/ScriptedPythonInterface.cpp|  34 
 .../Interfaces/ScriptedPythonInterface.h  |  20 +++
 .../ScriptedThreadPlanPythonInterface.cpp |  92 +++
 .../ScriptedThreadPlanPythonInterface.h   |  44 +
 .../ScriptedThreadPythonInterface.cpp |   1 +
 .../Python/SWIGPythonBridge.h |  21 +--
 .../Python/ScriptInterpreterPython.cpp| 122 +-
 .../Python/ScriptInterpreterPythonImpl.h  |  28 +---
 lldb/source/Target/ThreadPlanPython.cpp   |  92 ++-
 .../functionalities/step_scripted/Steps.py|   4 +-
 .../Python/PythonTestSuite.cpp|  50 +++---
 23 files changed, 403 insertions(+), 410 deletions(-)
 create mode 100644 
lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h
 create mode 100644 
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp
 create mode 100644 
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h

diff --git a/lldb/bindings/python/python-swigsafecast.swig 
b/lldb/bindings/python/python-swigsafecast.swig
index d5ea5148727134d..fba3a77d8f2df44 100644
--- a/lldb/bindings/python/python-swigsafecast.swig
+++ b/lldb/bindings/python/python-swigsafecast.swig
@@ -37,10 +37,6 @@ PythonObject SWIGBridge::ToSWIGWrapper(const Status& status) 
{
   return ToSWIGHelper(new lldb::SBError(status), SWIGTYPE_p_lldb__SBError);
 }
 
-PythonObject SWIGBridge::ToSWIGWrapper(std::unique_ptr 
stream_sb) {
-  return ToSWIGHelper(stream_sb.release(), SWIGTYPE_p_lldb__SBStream);
-}
-
 PythonObject SWIGBridge::ToSWIGWrapper(std::unique_ptr 
data_sb) {
   return ToSWIGHelper(data_sb.release(), SWIGTYPE_p_lldb__SBStructuredData);
 }
@@ -115,9 +111,12 @@ SWIGBridge::ToSWIGWrapper(CommandReturnObject &cmd_retobj) 
{
   SWIGTYPE_p_lldb__SBCommandReturnObject);
 }
 
-ScopedPythonObject SWIGBridge::ToSWIGWrapper(Event *event) {
-  return ScopedPythonObject(new lldb::SBEvent(event),
-   SWIGTYPE_p_lldb__SBEvent);
+PythonObject SWIGBridge::ToSWIGWrapper(const Stream *s) {
+  return ToSWIGHelper(new lldb::SBStream(), SWIGTYPE_p_lldb__SBStream);
+}
+
+PythonObject SWIGBridge::ToSWIGWrapper(Event *event) {
+  return ToSWIGHelper(new lldb::SBEvent(event), SWIGTYPE_p_lldb__SBEvent);
 }
 
 PythonObject SWIGBridge::ToSWIGWrapper(
diff --git a/lldb/bindings/python/python-wrapper.swig 
b/lldb/bindings/python/python-wrapper.swig
index 17bc7b1f2198709..5c28d652824073a 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -229,133 +229,6 @@ PythonObject 
lldb_private::python::SWIGBridge::LLDBSwigPythonCreateCommandObject
   return pfunc(SWIGBridge::ToSWIGWrapper(std::move(debugger_sp)), dict);
 }
 
-PythonObject 
lldb_private::python::SWIGBridge::LLDBSwigPythonCreateScriptedThreadPlan(
-const char *python_class_name, const char *session_dictionary_name,
-const lldb_private::StructuredDataImpl &args_impl,
-std::string &error_string, const lldb::ThreadPlanSP &thread_plan_sp) {
-  if (python_class_name == NULL || python_class_name[0] == '\0' ||
-  !session_dictionary_name)
-return PythonObject();
-
-  PyErr_Cleaner py_err_cleaner(true);
-
-  auto dict = PythonModule::MainModule().ResolveName(
-  session_dictionary_name);
-  auto pfunc = PythonObject::R

[Lldb-commits] [lldb] [lldb/Interpreter] Make ScriptedInterface Object creation more generic (PR #70392)

[Med Ismail Bennani via lldb-commits]

https://github.com/medismailben updated 
https://github.com/llvm/llvm-project/pull/70392

>From 7366abec305c6f53d3906b54c86f0dabe84ef01e Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani 
Date: Thu, 26 Oct 2023 16:35:53 -0700
Subject: [PATCH] [lldb] Make use of Scripted{Python,}Interface for
 ScriptedThreadPlan

This patch makes ScriptedThreadPlan conforming to the ScriptedInterface
& ScriptedPythonInterface facilities by introducing 2
ScriptedThreadPlanInterface & ScriptedThreadPlanPythonInterface classes.

This allows us to get rid of every ScriptedThreadPlan-specific SWIG
method and re-use the same affordances as other scripting offordances,
like Scripted{Process,Thread,Platform} & OperatingSystem.

To do so, this adds new transformer methods for `ThreadPlan`, `Stream` &
`Event`, to allow the bijection between C++ objects and their python
counterparts.

Signed-off-by: Med Ismail Bennani 
---
 lldb/bindings/python/python-swigsafecast.swig |  13 +-
 lldb/bindings/python/python-wrapper.swig  | 153 +++---
 lldb/include/lldb/API/SBEvent.h   |   4 +-
 lldb/include/lldb/API/SBStream.h  |   9 ++
 .../Interfaces/ScriptedInterface.h|   9 +-
 .../Interfaces/ScriptedThreadPlanInterface.h  |  40 +
 .../lldb/Interpreter/ScriptInterpreter.h  |  56 ++-
 lldb/include/lldb/Target/ThreadPlanPython.h   |   2 +
 lldb/include/lldb/lldb-forward.h  |   3 +
 lldb/source/Interpreter/ScriptInterpreter.cpp |  13 ++
 .../Python/Interfaces/CMakeLists.txt  |   1 +
 .../ScriptedPlatformPythonInterface.cpp   |   2 +
 .../Interfaces/ScriptedPythonInterface.cpp|  34 
 .../Interfaces/ScriptedPythonInterface.h  |  20 +++
 .../ScriptedThreadPlanPythonInterface.cpp |  92 +++
 .../ScriptedThreadPlanPythonInterface.h   |  44 +
 .../ScriptedThreadPythonInterface.cpp |   1 +
 .../Python/SWIGPythonBridge.h |  21 +--
 .../Python/ScriptInterpreterPython.cpp| 122 +-
 .../Python/ScriptInterpreterPythonImpl.h  |  28 +---
 lldb/source/Target/ThreadPlanPython.cpp   |  92 ++-
 .../functionalities/step_scripted/Steps.py|   4 +-
 .../Python/PythonTestSuite.cpp|  45 +++---
 23 files changed, 398 insertions(+), 410 deletions(-)
 create mode 100644 
lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h
 create mode 100644 
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp
 create mode 100644 
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h

diff --git a/lldb/bindings/python/python-swigsafecast.swig 
b/lldb/bindings/python/python-swigsafecast.swig
index d5ea5148727134d..fba3a77d8f2df44 100644
--- a/lldb/bindings/python/python-swigsafecast.swig
+++ b/lldb/bindings/python/python-swigsafecast.swig
@@ -37,10 +37,6 @@ PythonObject SWIGBridge::ToSWIGWrapper(const Status& status) 
{
   return ToSWIGHelper(new lldb::SBError(status), SWIGTYPE_p_lldb__SBError);
 }
 
-PythonObject SWIGBridge::ToSWIGWrapper(std::unique_ptr 
stream_sb) {
-  return ToSWIGHelper(stream_sb.release(), SWIGTYPE_p_lldb__SBStream);
-}
-
 PythonObject SWIGBridge::ToSWIGWrapper(std::unique_ptr 
data_sb) {
   return ToSWIGHelper(data_sb.release(), SWIGTYPE_p_lldb__SBStructuredData);
 }
@@ -115,9 +111,12 @@ SWIGBridge::ToSWIGWrapper(CommandReturnObject &cmd_retobj) 
{
   SWIGTYPE_p_lldb__SBCommandReturnObject);
 }
 
-ScopedPythonObject SWIGBridge::ToSWIGWrapper(Event *event) {
-  return ScopedPythonObject(new lldb::SBEvent(event),
-   SWIGTYPE_p_lldb__SBEvent);
+PythonObject SWIGBridge::ToSWIGWrapper(const Stream *s) {
+  return ToSWIGHelper(new lldb::SBStream(), SWIGTYPE_p_lldb__SBStream);
+}
+
+PythonObject SWIGBridge::ToSWIGWrapper(Event *event) {
+  return ToSWIGHelper(new lldb::SBEvent(event), SWIGTYPE_p_lldb__SBEvent);
 }
 
 PythonObject SWIGBridge::ToSWIGWrapper(
diff --git a/lldb/bindings/python/python-wrapper.swig 
b/lldb/bindings/python/python-wrapper.swig
index 17bc7b1f2198709..5c28d652824073a 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -229,133 +229,6 @@ PythonObject 
lldb_private::python::SWIGBridge::LLDBSwigPythonCreateCommandObject
   return pfunc(SWIGBridge::ToSWIGWrapper(std::move(debugger_sp)), dict);
 }
 
-PythonObject 
lldb_private::python::SWIGBridge::LLDBSwigPythonCreateScriptedThreadPlan(
-const char *python_class_name, const char *session_dictionary_name,
-const lldb_private::StructuredDataImpl &args_impl,
-std::string &error_string, const lldb::ThreadPlanSP &thread_plan_sp) {
-  if (python_class_name == NULL || python_class_name[0] == '\0' ||
-  !session_dictionary_name)
-return PythonObject();
-
-  PyErr_Cleaner py_err_cleaner(true);
-
-  auto dict = PythonModule::MainModule().ResolveName(
-  session_dictionary_name);
-  auto pfunc = PythonObject::R

[Lldb-commits] [lldb] [lldb/Interpreter] Make ScriptedInterface Object creation more generic (PR #70392)

[Med Ismail Bennani via lldb-commits]

https://github.com/medismailben edited 
https://github.com/llvm/llvm-project/pull/70392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make use of Scripted{Python, }Interface for ScriptedThreadPlan (PR #70392)

[Med Ismail Bennani via lldb-commits]

https://github.com/medismailben edited 
https://github.com/llvm/llvm-project/pull/70392
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make use of Scripted{Python, }Interface for ScriptedThreadPlan (PR #70392)

[Med Ismail Bennani via lldb-commits]

https://github.com/medismailben updated 
https://github.com/llvm/llvm-project/pull/70392

>From 836bafbf3956750c99d38814a4a0a445f9136141 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani 
Date: Thu, 26 Oct 2023 16:35:53 -0700
Subject: [PATCH] [lldb] Make use of Scripted{Python,}Interface for
 ScriptedThreadPlan

This patch makes ScriptedThreadPlan conforming to the ScriptedInterface
& ScriptedPythonInterface facilities by introducing 2
ScriptedThreadPlanInterface & ScriptedThreadPlanPythonInterface classes.

This allows us to get rid of every ScriptedThreadPlan-specific SWIG
method and re-use the same affordances as other scripting offordances,
like Scripted{Process,Thread,Platform} & OperatingSystem.

To do so, this adds new transformer methods for `ThreadPlan`, `Stream` &
`Event`, to allow the bijection between C++ objects and their python
counterparts.

Signed-off-by: Med Ismail Bennani 
---
 lldb/bindings/python/python-swigsafecast.swig |  13 +-
 lldb/bindings/python/python-wrapper.swig  | 153 +++---
 lldb/include/lldb/API/SBEvent.h   |   4 +-
 lldb/include/lldb/API/SBStream.h  |   9 ++
 .../Interfaces/ScriptedInterface.h|   9 +-
 .../Interfaces/ScriptedThreadPlanInterface.h  |  40 +
 .../lldb/Interpreter/ScriptInterpreter.h  |  56 ++-
 lldb/include/lldb/Target/ThreadPlanPython.h   |   2 +
 lldb/include/lldb/lldb-forward.h  |   3 +
 lldb/source/Interpreter/ScriptInterpreter.cpp |  13 ++
 .../Python/Interfaces/CMakeLists.txt  |   1 +
 .../ScriptedPlatformPythonInterface.cpp   |   2 +
 .../Interfaces/ScriptedPythonInterface.cpp|  34 
 .../Interfaces/ScriptedPythonInterface.h  |  20 +++
 .../ScriptedThreadPlanPythonInterface.cpp |  92 +++
 .../ScriptedThreadPlanPythonInterface.h   |  44 +
 .../ScriptedThreadPythonInterface.cpp |   1 +
 .../Python/SWIGPythonBridge.h |  21 +--
 .../Python/ScriptInterpreterPython.cpp| 122 +-
 .../Python/ScriptInterpreterPythonImpl.h  |  28 +---
 lldb/source/Target/ThreadPlanPython.cpp   |  92 ++-
 .../functionalities/step_scripted/Steps.py|   4 +-
 .../Python/PythonTestSuite.cpp|  45 +++---
 23 files changed, 398 insertions(+), 410 deletions(-)
 create mode 100644 
lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h
 create mode 100644 
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp
 create mode 100644 
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h

diff --git a/lldb/bindings/python/python-swigsafecast.swig 
b/lldb/bindings/python/python-swigsafecast.swig
index d5ea5148727134d..fba3a77d8f2df44 100644
--- a/lldb/bindings/python/python-swigsafecast.swig
+++ b/lldb/bindings/python/python-swigsafecast.swig
@@ -37,10 +37,6 @@ PythonObject SWIGBridge::ToSWIGWrapper(const Status& status) 
{
   return ToSWIGHelper(new lldb::SBError(status), SWIGTYPE_p_lldb__SBError);
 }
 
-PythonObject SWIGBridge::ToSWIGWrapper(std::unique_ptr 
stream_sb) {
-  return ToSWIGHelper(stream_sb.release(), SWIGTYPE_p_lldb__SBStream);
-}
-
 PythonObject SWIGBridge::ToSWIGWrapper(std::unique_ptr 
data_sb) {
   return ToSWIGHelper(data_sb.release(), SWIGTYPE_p_lldb__SBStructuredData);
 }
@@ -115,9 +111,12 @@ SWIGBridge::ToSWIGWrapper(CommandReturnObject &cmd_retobj) 
{
   SWIGTYPE_p_lldb__SBCommandReturnObject);
 }
 
-ScopedPythonObject SWIGBridge::ToSWIGWrapper(Event *event) {
-  return ScopedPythonObject(new lldb::SBEvent(event),
-   SWIGTYPE_p_lldb__SBEvent);
+PythonObject SWIGBridge::ToSWIGWrapper(const Stream *s) {
+  return ToSWIGHelper(new lldb::SBStream(), SWIGTYPE_p_lldb__SBStream);
+}
+
+PythonObject SWIGBridge::ToSWIGWrapper(Event *event) {
+  return ToSWIGHelper(new lldb::SBEvent(event), SWIGTYPE_p_lldb__SBEvent);
 }
 
 PythonObject SWIGBridge::ToSWIGWrapper(
diff --git a/lldb/bindings/python/python-wrapper.swig 
b/lldb/bindings/python/python-wrapper.swig
index 17bc7b1f2198709..5c28d652824073a 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -229,133 +229,6 @@ PythonObject 
lldb_private::python::SWIGBridge::LLDBSwigPythonCreateCommandObject
   return pfunc(SWIGBridge::ToSWIGWrapper(std::move(debugger_sp)), dict);
 }
 
-PythonObject 
lldb_private::python::SWIGBridge::LLDBSwigPythonCreateScriptedThreadPlan(
-const char *python_class_name, const char *session_dictionary_name,
-const lldb_private::StructuredDataImpl &args_impl,
-std::string &error_string, const lldb::ThreadPlanSP &thread_plan_sp) {
-  if (python_class_name == NULL || python_class_name[0] == '\0' ||
-  !session_dictionary_name)
-return PythonObject();
-
-  PyErr_Cleaner py_err_cleaner(true);
-
-  auto dict = PythonModule::MainModule().ResolveName(
-  session_dictionary_name);
-  auto pfunc = PythonObject::R

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -0,0 +1,155 @@
+//===-- WatchpointResource.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+#define LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+
+#include "lldb/Utility/Iterable.h"
+#include "lldb/lldb-public.h"
+
+#include 
+#include 
+
+namespace lldb_private {
+
+class WatchpointResource
+: public std::enable_shared_from_this {
+
+public:
+  // Constructors and Destructors

JDevlieghere wrote:

Nit: these comments are useless. We've been gradually removing them. Please 
remove this. 

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -0,0 +1,155 @@
+//===-- WatchpointResource.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+#define LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+
+#include "lldb/Utility/Iterable.h"
+#include "lldb/lldb-public.h"
+
+#include 
+#include 
+
+namespace lldb_private {
+
+class WatchpointResource
+: public std::enable_shared_from_this {
+
+public:
+  // Constructors and Destructors
+  WatchpointResource(lldb::addr_t addr, size_t size, bool read, bool write);
+
+  ~WatchpointResource();
+
+  lldb::addr_t GetAddress() const;
+
+  size_t GetByteSize() const;
+
+  bool WatchpointResourceRead() const;
+
+  bool WatchpointResourceWrite() const;
+
+  void SetType(bool read, bool write);
+
+  typedef std::vector WatchpointCollection;
+  typedef LockingAdaptedIterable
+  WatchpointIterable;
+
+  /// Iterate over the watchpoint owners for this resource
+  ///
+  /// \return
+  /// An Iterable object which can be used to loop over the watchpoints
+  /// that are owners of this resource.
+  WatchpointIterable Owners() {
+return WatchpointIterable(m_owners, m_owners_mutex);
+  }
+
+  /// The "Owners" are the watchpoints that share this resource.
+  /// The method adds the \a owner to this resource's owner list.
+  ///
+  /// \param[in] owner
+  ///\a owner is the Wachpoint to add.
+  void AddOwner(const lldb::WatchpointSP &owner);
+
+  /// The method removes the owner at \a owner from this watchpoint
+  /// resource.
+  void RemoveOwner(lldb::WatchpointSP &owner);
+
+  /// This method returns the number of Watchpoints currently using
+  /// watchpoint resource.
+  ///
+  /// \return
+  ///The number of owners.
+  size_t GetNumberOfOwners();
+
+  /// This method returns the Watchpoint at index \a index using this
+  /// Resource.  The owners are listed ordinally from 0 to
+  /// GetNumberOfOwners() - 1 so you can use this method to iterate over the
+  /// owners.
+  ///
+  /// \param[in] idx
+  /// The index in the list of owners for which you wish the owner 
location.
+  ///
+  /// \return
+  ///The Watchpoint at that index.
+  lldb::WatchpointSP GetOwnerAtIndex(size_t idx);
+
+  /// Check if the owners includes a watchpoint.
+  ///
+  /// \param[in] wp_sp
+  /// The WatchpointSP to search for.
+  ///
+  /// \result
+  /// true if this resource's owners includes the watchpoint.
+  bool OwnersContains(lldb::WatchpointSP &wp_sp);
+
+  /// Check if the owners includes a watchpoint.
+  ///
+  /// \param[in] wp
+  /// The Watchpoint to search for.
+  ///
+  /// \result
+  /// true if this resource's owners includes the watchpoint.
+  bool OwnersContains(const lldb_private::Watchpoint *wp);
+
+  /// This method copies the watchpoint resource's owners into a new 
collection.
+  /// It does this while the owners mutex is locked.
+  ///
+  /// \param[out] out_collection
+  ///The BreakpointLocationCollection into which to put the owners
+  ///of this breakpoint site.
+  ///
+  /// \return
+  ///The number of elements copied into out_collection.
+  size_t CopyOwnersList(WatchpointCollection &out_collection);
+
+  // The ID of the WatchpointResource is set by the WatchpointResourceList
+  // when the Resource has been set in the inferior and is being added
+  // to the List, in an attempt to match the hardware watchpoint register
+  // ordering.  If a Process can correctly identify the hardware watchpoint
+  // register index when it has created the Resource, it may initialize it
+  // before it is inserted in the WatchpointResourceList.
+  void SetID(lldb::wp_resource_id_t);
+
+  lldb::wp_resource_id_t GetID() const;
+
+  bool Contains(lldb::addr_t addr);
+
+protected:
+  // The StopInfoWatchpoint knows when it is processing a hit for a thread for
+  // a site, so let it be the one to manage setting the location hit count once
+  // and only once.
+  friend class StopInfoWatchpoint;
+
+  void BumpHitCounts();
+
+private:
+  lldb::wp_resource_id_t m_id;
+
+  // start address & size aligned & expanded to be a valid watchpoint

JDevlieghere wrote:

Nit: Capitalize

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -0,0 +1,97 @@
+//===-- WatchpointResource.cpp 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Breakpoint/WatchpointResource.h"
+#include "lldb/Utility/LLDBAssert.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+WatchpointResource::WatchpointResource(lldb::addr_t addr, size_t size,
+   bool read, bool write)
+: m_id(LLDB_INVALID_WATCHPOINT_RESOURCE_ID), m_addr(addr), m_size(size),
+  m_watch_read(read), m_watch_write(write) {}
+
+WatchpointResource::~WatchpointResource() {
+  std::lock_guard guard(m_owners_mutex);
+  m_owners.clear();
+}
+
+addr_t WatchpointResource::GetAddress() const { return m_addr; }
+
+size_t WatchpointResource::GetByteSize() const { return m_size; }
+
+bool WatchpointResource::WatchpointResourceRead() const { return m_watch_read; 
}
+
+bool WatchpointResource::WatchpointResourceWrite() const {
+  return m_watch_write;
+}
+
+void WatchpointResource::SetType(bool read, bool write) {
+  m_watch_read = read;
+  m_watch_write = write;
+}
+
+wp_resource_id_t WatchpointResource::GetID() const { return m_id; }
+
+void WatchpointResource::SetID(wp_resource_id_t id) { m_id = id; }
+
+bool WatchpointResource::Contains(addr_t addr) {
+  if (addr >= m_addr && addr < m_addr + m_size)
+return true;
+  return false;
+}
+
+void WatchpointResource::AddOwner(const WatchpointSP &wp_sp) {
+  std::lock_guard guard(m_owners_mutex);
+  m_owners.push_back(wp_sp);
+}
+
+void WatchpointResource::RemoveOwner(WatchpointSP &wp_sp) {
+  std::lock_guard guard(m_owners_mutex);
+  const auto &it = std::find(m_owners.begin(), m_owners.end(), wp_sp);
+  if (it != m_owners.end())
+m_owners.erase(it);
+}
+
+size_t WatchpointResource::GetNumberOfOwners() {
+  std::lock_guard guard(m_owners_mutex);
+  return m_owners.size();
+}
+
+bool WatchpointResource::OwnersContains(WatchpointSP &wp_sp) {
+  std::lock_guard guard(m_owners_mutex);
+  const auto &it = std::find(m_owners.begin(), m_owners.end(), wp_sp);
+  return it != m_owners.end();
+}
+
+bool WatchpointResource::OwnersContains(const Watchpoint *wp) {
+  std::lock_guard guard(m_owners_mutex);
+  WatchpointCollection::const_iterator match =
+  std::find_if(m_owners.begin(), m_owners.end(),
+   [&wp](const WatchpointSP &x) { return x.get() == wp; });
+  return match != m_owners.end();
+}
+
+WatchpointSP WatchpointResource::GetOwnerAtIndex(size_t idx) {
+  std::lock_guard guard(m_owners_mutex);
+  lldbassert(idx < m_owners.size());

JDevlieghere wrote:

I'd just make this a normal assert.

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -0,0 +1,155 @@
+//===-- WatchpointResource.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+#define LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+
+#include "lldb/Utility/Iterable.h"
+#include "lldb/lldb-public.h"
+
+#include 
+#include 
+
+namespace lldb_private {
+
+class WatchpointResource
+: public std::enable_shared_from_this {
+
+public:
+  // Constructors and Destructors
+  WatchpointResource(lldb::addr_t addr, size_t size, bool read, bool write);
+
+  ~WatchpointResource();
+
+  lldb::addr_t GetAddress() const;
+
+  size_t GetByteSize() const;
+
+  bool WatchpointResourceRead() const;
+
+  bool WatchpointResourceWrite() const;
+
+  void SetType(bool read, bool write);
+
+  typedef std::vector WatchpointCollection;
+  typedef LockingAdaptedIterable
+  WatchpointIterable;
+
+  /// Iterate over the watchpoint owners for this resource
+  ///
+  /// \return
+  /// An Iterable object which can be used to loop over the watchpoints
+  /// that are owners of this resource.
+  WatchpointIterable Owners() {
+return WatchpointIterable(m_owners, m_owners_mutex);
+  }
+
+  /// The "Owners" are the watchpoints that share this resource.
+  /// The method adds the \a owner to this resource's owner list.
+  ///
+  /// \param[in] owner
+  ///\a owner is the Wachpoint to add.
+  void AddOwner(const lldb::WatchpointSP &owner);
+
+  /// The method removes the owner at \a owner from this watchpoint
+  /// resource.
+  void RemoveOwner(lldb::WatchpointSP &owner);
+
+  /// This method returns the number of Watchpoints currently using
+  /// watchpoint resource.
+  ///
+  /// \return
+  ///The number of owners.
+  size_t GetNumberOfOwners();
+
+  /// This method returns the Watchpoint at index \a index using this
+  /// Resource.  The owners are listed ordinally from 0 to
+  /// GetNumberOfOwners() - 1 so you can use this method to iterate over the
+  /// owners.
+  ///
+  /// \param[in] idx
+  /// The index in the list of owners for which you wish the owner 
location.
+  ///
+  /// \return
+  ///The Watchpoint at that index.
+  lldb::WatchpointSP GetOwnerAtIndex(size_t idx);
+
+  /// Check if the owners includes a watchpoint.
+  ///
+  /// \param[in] wp_sp
+  /// The WatchpointSP to search for.
+  ///
+  /// \result
+  /// true if this resource's owners includes the watchpoint.
+  bool OwnersContains(lldb::WatchpointSP &wp_sp);
+
+  /// Check if the owners includes a watchpoint.
+  ///
+  /// \param[in] wp
+  /// The Watchpoint to search for.
+  ///
+  /// \result
+  /// true if this resource's owners includes the watchpoint.
+  bool OwnersContains(const lldb_private::Watchpoint *wp);

JDevlieghere wrote:

Can this be a const reference?

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -0,0 +1,155 @@
+//===-- WatchpointResource.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+#define LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+
+#include "lldb/Utility/Iterable.h"
+#include "lldb/lldb-public.h"
+
+#include 
+#include 
+
+namespace lldb_private {
+
+class WatchpointResource
+: public std::enable_shared_from_this {
+
+public:
+  // Constructors and Destructors
+  WatchpointResource(lldb::addr_t addr, size_t size, bool read, bool write);
+
+  ~WatchpointResource();
+
+  lldb::addr_t GetAddress() const;
+
+  size_t GetByteSize() const;
+
+  bool WatchpointResourceRead() const;
+
+  bool WatchpointResourceWrite() const;
+
+  void SetType(bool read, bool write);
+
+  typedef std::vector WatchpointCollection;
+  typedef LockingAdaptedIterable
+  WatchpointIterable;
+
+  /// Iterate over the watchpoint owners for this resource
+  ///
+  /// \return
+  /// An Iterable object which can be used to loop over the watchpoints
+  /// that are owners of this resource.
+  WatchpointIterable Owners() {
+return WatchpointIterable(m_owners, m_owners_mutex);
+  }
+
+  /// The "Owners" are the watchpoints that share this resource.
+  /// The method adds the \a owner to this resource's owner list.
+  ///
+  /// \param[in] owner
+  ///\a owner is the Wachpoint to add.
+  void AddOwner(const lldb::WatchpointSP &owner);
+
+  /// The method removes the owner at \a owner from this watchpoint
+  /// resource.
+  void RemoveOwner(lldb::WatchpointSP &owner);
+
+  /// This method returns the number of Watchpoints currently using
+  /// watchpoint resource.
+  ///
+  /// \return
+  ///The number of owners.
+  size_t GetNumberOfOwners();
+
+  /// This method returns the Watchpoint at index \a index using this
+  /// Resource.  The owners are listed ordinally from 0 to
+  /// GetNumberOfOwners() - 1 so you can use this method to iterate over the
+  /// owners.
+  ///
+  /// \param[in] idx
+  /// The index in the list of owners for which you wish the owner 
location.
+  ///
+  /// \return
+  ///The Watchpoint at that index.
+  lldb::WatchpointSP GetOwnerAtIndex(size_t idx);
+
+  /// Check if the owners includes a watchpoint.
+  ///
+  /// \param[in] wp_sp
+  /// The WatchpointSP to search for.
+  ///
+  /// \result
+  /// true if this resource's owners includes the watchpoint.
+  bool OwnersContains(lldb::WatchpointSP &wp_sp);
+
+  /// Check if the owners includes a watchpoint.
+  ///
+  /// \param[in] wp
+  /// The Watchpoint to search for.
+  ///
+  /// \result
+  /// true if this resource's owners includes the watchpoint.
+  bool OwnersContains(const lldb_private::Watchpoint *wp);
+
+  /// This method copies the watchpoint resource's owners into a new 
collection.
+  /// It does this while the owners mutex is locked.
+  ///
+  /// \param[out] out_collection
+  ///The BreakpointLocationCollection into which to put the owners
+  ///of this breakpoint site.
+  ///
+  /// \return
+  ///The number of elements copied into out_collection.
+  size_t CopyOwnersList(WatchpointCollection &out_collection);
+
+  // The ID of the WatchpointResource is set by the WatchpointResourceList
+  // when the Resource has been set in the inferior and is being added
+  // to the List, in an attempt to match the hardware watchpoint register
+  // ordering.  If a Process can correctly identify the hardware watchpoint
+  // register index when it has created the Resource, it may initialize it
+  // before it is inserted in the WatchpointResourceList.
+  void SetID(lldb::wp_resource_id_t);
+
+  lldb::wp_resource_id_t GetID() const;
+
+  bool Contains(lldb::addr_t addr);
+
+protected:
+  // The StopInfoWatchpoint knows when it is processing a hit for a thread for
+  // a site, so let it be the one to manage setting the location hit count once
+  // and only once.
+  friend class StopInfoWatchpoint;
+
+  void BumpHitCounts();
+
+private:
+  lldb::wp_resource_id_t m_id;
+
+  // start address & size aligned & expanded to be a valid watchpoint
+  // memory granule on this target.
+  lldb::addr_t m_addr;
+  size_t m_size;
+
+  bool m_watch_read;  // true if we stop when the watched data is read from
+  bool m_watch_write; // true if we stop when the watched data is written to
+
+  // The watchpoints using this WatchpointResource.

JDevlieghere wrote:

`///`

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -0,0 +1,146 @@
+//===-- WatchpointResourceList.h *- C++ 
-*-===//

JDevlieghere wrote:

Do we really need this class? It looks like a pretty trivial wrapper around a 
vector and a mutex. How much overlap is there with the `BreakpointSiteList.h`? 
Can we template this into a `StopPointList`? 

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -0,0 +1,155 @@
+//===-- WatchpointResource.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+#define LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+
+#include "lldb/Utility/Iterable.h"
+#include "lldb/lldb-public.h"
+
+#include 
+#include 
+
+namespace lldb_private {
+
+class WatchpointResource
+: public std::enable_shared_from_this {
+
+public:
+  // Constructors and Destructors
+  WatchpointResource(lldb::addr_t addr, size_t size, bool read, bool write);
+
+  ~WatchpointResource();
+
+  lldb::addr_t GetAddress() const;
+
+  size_t GetByteSize() const;
+
+  bool WatchpointResourceRead() const;
+
+  bool WatchpointResourceWrite() const;
+
+  void SetType(bool read, bool write);
+
+  typedef std::vector WatchpointCollection;
+  typedef LockingAdaptedIterable
+  WatchpointIterable;
+
+  /// Iterate over the watchpoint owners for this resource
+  ///
+  /// \return
+  /// An Iterable object which can be used to loop over the watchpoints
+  /// that are owners of this resource.
+  WatchpointIterable Owners() {
+return WatchpointIterable(m_owners, m_owners_mutex);
+  }
+
+  /// The "Owners" are the watchpoints that share this resource.
+  /// The method adds the \a owner to this resource's owner list.
+  ///
+  /// \param[in] owner
+  ///\a owner is the Wachpoint to add.
+  void AddOwner(const lldb::WatchpointSP &owner);
+
+  /// The method removes the owner at \a owner from this watchpoint
+  /// resource.
+  void RemoveOwner(lldb::WatchpointSP &owner);
+
+  /// This method returns the number of Watchpoints currently using
+  /// watchpoint resource.
+  ///
+  /// \return
+  ///The number of owners.
+  size_t GetNumberOfOwners();
+
+  /// This method returns the Watchpoint at index \a index using this
+  /// Resource.  The owners are listed ordinally from 0 to
+  /// GetNumberOfOwners() - 1 so you can use this method to iterate over the
+  /// owners.
+  ///
+  /// \param[in] idx
+  /// The index in the list of owners for which you wish the owner 
location.
+  ///
+  /// \return
+  ///The Watchpoint at that index.
+  lldb::WatchpointSP GetOwnerAtIndex(size_t idx);
+
+  /// Check if the owners includes a watchpoint.
+  ///
+  /// \param[in] wp_sp
+  /// The WatchpointSP to search for.
+  ///
+  /// \result
+  /// true if this resource's owners includes the watchpoint.
+  bool OwnersContains(lldb::WatchpointSP &wp_sp);
+
+  /// Check if the owners includes a watchpoint.
+  ///
+  /// \param[in] wp
+  /// The Watchpoint to search for.
+  ///
+  /// \result
+  /// true if this resource's owners includes the watchpoint.
+  bool OwnersContains(const lldb_private::Watchpoint *wp);
+
+  /// This method copies the watchpoint resource's owners into a new 
collection.
+  /// It does this while the owners mutex is locked.
+  ///
+  /// \param[out] out_collection
+  ///The BreakpointLocationCollection into which to put the owners
+  ///of this breakpoint site.
+  ///
+  /// \return
+  ///The number of elements copied into out_collection.
+  size_t CopyOwnersList(WatchpointCollection &out_collection);
+
+  // The ID of the WatchpointResource is set by the WatchpointResourceList
+  // when the Resource has been set in the inferior and is being added
+  // to the List, in an attempt to match the hardware watchpoint register
+  // ordering.  If a Process can correctly identify the hardware watchpoint
+  // register index when it has created the Resource, it may initialize it
+  // before it is inserted in the WatchpointResourceList.
+  void SetID(lldb::wp_resource_id_t);
+
+  lldb::wp_resource_id_t GetID() const;
+
+  bool Contains(lldb::addr_t addr);
+
+protected:
+  // The StopInfoWatchpoint knows when it is processing a hit for a thread for
+  // a site, so let it be the one to manage setting the location hit count once
+  // and only once.
+  friend class StopInfoWatchpoint;
+
+  void BumpHitCounts();
+
+private:
+  lldb::wp_resource_id_t m_id;
+
+  // start address & size aligned & expanded to be a valid watchpoint
+  // memory granule on this target.
+  lldb::addr_t m_addr;
+  size_t m_size;
+
+  bool m_watch_read;  // true if we stop when the watched data is read from
+  bool m_watch_write; // true if we stop when the watched data is written to
+
+  // The watchpoints using this WatchpointResource.
+  WatchpointCollection m_owners;
+
+  std::recursive_mutex
+  m_owners_mutex; ///< This mutex protects the owners collection.

JDevlieghere wrote:

Move the comment to the line above. Does this have to be a recursive mutex?

https://github.com/llvm/llvm-project/pull/68845

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -0,0 +1,155 @@
+//===-- WatchpointResource.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+#define LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+
+#include "lldb/Utility/Iterable.h"
+#include "lldb/lldb-public.h"
+
+#include 
+#include 
+
+namespace lldb_private {
+
+class WatchpointResource
+: public std::enable_shared_from_this {
+
+public:
+  // Constructors and Destructors
+  WatchpointResource(lldb::addr_t addr, size_t size, bool read, bool write);
+
+  ~WatchpointResource();
+
+  lldb::addr_t GetAddress() const;
+
+  size_t GetByteSize() const;
+
+  bool WatchpointResourceRead() const;
+
+  bool WatchpointResourceWrite() const;
+
+  void SetType(bool read, bool write);
+
+  typedef std::vector WatchpointCollection;
+  typedef LockingAdaptedIterable
+  WatchpointIterable;
+
+  /// Iterate over the watchpoint owners for this resource
+  ///
+  /// \return
+  /// An Iterable object which can be used to loop over the watchpoints
+  /// that are owners of this resource.
+  WatchpointIterable Owners() {
+return WatchpointIterable(m_owners, m_owners_mutex);
+  }
+
+  /// The "Owners" are the watchpoints that share this resource.
+  /// The method adds the \a owner to this resource's owner list.
+  ///
+  /// \param[in] owner
+  ///\a owner is the Wachpoint to add.
+  void AddOwner(const lldb::WatchpointSP &owner);
+
+  /// The method removes the owner at \a owner from this watchpoint
+  /// resource.
+  void RemoveOwner(lldb::WatchpointSP &owner);
+
+  /// This method returns the number of Watchpoints currently using
+  /// watchpoint resource.
+  ///
+  /// \return
+  ///The number of owners.
+  size_t GetNumberOfOwners();
+
+  /// This method returns the Watchpoint at index \a index using this
+  /// Resource.  The owners are listed ordinally from 0 to
+  /// GetNumberOfOwners() - 1 so you can use this method to iterate over the
+  /// owners.
+  ///
+  /// \param[in] idx
+  /// The index in the list of owners for which you wish the owner 
location.
+  ///
+  /// \return
+  ///The Watchpoint at that index.
+  lldb::WatchpointSP GetOwnerAtIndex(size_t idx);
+
+  /// Check if the owners includes a watchpoint.
+  ///
+  /// \param[in] wp_sp
+  /// The WatchpointSP to search for.
+  ///
+  /// \result
+  /// true if this resource's owners includes the watchpoint.
+  bool OwnersContains(lldb::WatchpointSP &wp_sp);

JDevlieghere wrote:

Does this method care about the shared pointer or the underlying watchpoint? If 
it's the former, the docs should make that explicit. 

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -0,0 +1,154 @@
+//===-- WatchpointResourceList.cpp 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Breakpoint/WatchpointResourceList.h"
+#include "lldb/Breakpoint/WatchpointResource.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+WatchpointResourceList::WatchpointResourceList() : m_resources(), m_mutex() {}
+
+WatchpointResourceList::~WatchpointResourceList() { Clear(); }
+
+wp_resource_id_t
+WatchpointResourceList::Add(const WatchpointResourceSP &wp_res_sp) {
+  Log *log = GetLog(LLDBLog::Watchpoints);
+  std::lock_guard guard(m_mutex);
+
+  LLDB_LOGF(log, "WatchpointResourceList::Add(addr 0x%" PRIx64 " size %zu)",
+wp_res_sp->GetAddress(), wp_res_sp->GetByteSize());
+
+  // The goal is to have the wp_resource_id_t match the actual hardware
+  // watchpoint register number.  If we assume that the remote stub is
+  // setting them in the register context in the same order that they
+  // are sent from lldb, and if a watchpoint is removed and then a new
+  // one is added and gets the same register number, then we can
+  // iterate over all used IDs looking for the first unused number.
+
+  // If the Process was able to find the actual hardware watchpoint register
+  // number that was used, it can set the ID for the WatchpointResource
+  // before we get here.
+
+  if (wp_res_sp->GetID() == LLDB_INVALID_WATCHPOINT_RESOURCE_ID) {
+std::set used_ids;
+size_t size = m_resources.size();
+for (size_t i = 0; i < size; ++i)
+  used_ids.insert(m_resources[i]->GetID());
+
+wp_resource_id_t best = 0;
+for (wp_resource_id_t id : used_ids)
+  if (id == best)
+best++;
+  else
+break;
+
+LLDB_LOGF(log,
+  "WatchpointResourceList::Add assigning next "
+  "available WatchpointResource ID, %u",
+  best);
+wp_res_sp->SetID(best);
+  }
+
+  m_resources.push_back(wp_res_sp);
+  return wp_res_sp->GetID();
+}
+
+bool WatchpointResourceList::Remove(wp_resource_id_t id) {
+  std::lock_guard guard(m_mutex);
+  Log *log = GetLog(LLDBLog::Watchpoints);
+  for (collection::iterator pos = m_resources.begin(); pos != 
m_resources.end();
+   ++pos) {
+if ((*pos)->GetID() == id) {
+  LLDB_LOGF(log,
+"WatchpointResourceList::Remove(addr 0x%" PRIx64 " size %zu)",
+(*pos)->GetAddress(), (*pos)->GetByteSize());
+  m_resources.erase(pos);
+  return true;
+}
+  }
+  return false;
+}
+
+bool WatchpointResourceList::RemoveByAddress(addr_t addr) {
+  std::lock_guard guard(m_mutex);
+  Log *log = GetLog(LLDBLog::Watchpoints);
+  for (collection::iterator pos = m_resources.begin(); pos != 
m_resources.end();
+   ++pos) {
+if ((*pos)->Contains(addr)) {
+  LLDB_LOGF(log,
+"WatchpointResourceList::RemoveByAddress(addr 0x%" PRIx64
+" size %zu)",
+(*pos)->GetAddress(), (*pos)->GetByteSize());
+  m_resources.erase(pos);
+  return true;
+}
+  }
+  return false;
+}
+
+WatchpointResourceSP WatchpointResourceList::FindByAddress(addr_t addr) {
+  std::lock_guard guard(m_mutex);
+  for (collection::iterator pos = m_resources.begin(); pos != 
m_resources.end();
+   ++pos)
+if ((*pos)->Contains(addr))
+  return *pos;

JDevlieghere wrote:

Can you use a range-based for loop here and below?

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -0,0 +1,140 @@
+//===-- WatchpointResource.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+#define LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+
+#include "lldb/Breakpoint/WatchpointCollection.h"
+#include "lldb/lldb-public.h"
+
+#include 
+
+namespace lldb_private {
+
+class WatchpointResource
+: public std::enable_shared_from_this {
+
+public:
+  // Constructors and Destructors
+  WatchpointResource(lldb::addr_t addr, size_t size, bool read, bool write);
+
+  ~WatchpointResource();
+
+  void GetMemoryRange(lldb::addr_t &addr, size_t &size) const;
+
+  lldb::addr_t GetAddress() const;
+
+  size_t GetByteSize() const;
+
+  void GetType(bool &read, bool &write) const;
+
+  void SetType(bool read, bool write);
+
+  /// The "Owners" are the watchpoints that share this resource.
+  /// The method adds the \a owner to this resource's owner list.
+  ///
+  /// \param[in] owner
+  ///\a owner is the Wachpoint to add.
+  void AddOwner(const lldb::WatchpointSP &owner);
+
+  /// The method removes the owner at \a owner from this watchpoint
+  /// resource.
+  void RemoveOwner(lldb::WatchpointSP &owner);
+
+  /// This method returns the number of Watchpoints currently using
+  /// watchpoint resource.
+  ///
+  /// \return
+  ///The number of owners.
+  size_t GetNumberOfOwners();
+
+  /// This method returns the Watchpoint at index \a index using this
+  /// Resource.  The owners are listed ordinally from 0 to
+  /// GetNumberOfOwners() - 1 so you can use this method to iterate over the
+  /// owners.
+  ///
+  /// \param[in] idx
+  /// The index in the list of owners for which you wish the owner 
location.
+  ///
+  /// \return
+  ///The Watchpoint at that index.
+  lldb::WatchpointSP GetOwnerAtIndex(size_t idx);
+
+  /// Check if the owners includes a watchpoint.
+  ///
+  /// \param[in] wp_sp
+  /// The WatchpointSP to search for.
+  ///
+  /// \result
+  /// true if this resource's owners includes the watchpoint.
+  bool OwnersContains(lldb::WatchpointSP &wp_sp);
+
+  /// Check if the owners includes a watchpoint.
+  ///
+  /// \param[in] wp
+  /// The Watchpoint to search for.
+  ///
+  /// \result
+  /// true if this resource's owners includes the watchpoint.
+  bool OwnersContains(const lldb_private::Watchpoint *wp);
+
+  /// This method copies the watchpoint resource's owners into a new 
collection.
+  /// It does this while the owners mutex is locked.
+  ///
+  /// \param[out] out_collection
+  ///The BreakpointLocationCollection into which to put the owners
+  ///of this breakpoint site.
+  ///
+  /// \return
+  ///The number of elements copied into out_collection.
+  size_t CopyOwnersList(WatchpointCollection &out_collection);

JDevlieghere wrote:

Starting with C++17, copy elision is guaranteed. 

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -331,6 +373,16 @@ void Watchpoint::DumpWithLevel(Stream *s,
 
 bool Watchpoint::IsEnabled() const { return m_enabled; }
 
+uint32_t Watchpoint::GetHardwareIndex() const {
+  if (IsEnabled())
+return m_target.GetProcessSP()
+->GetWatchpointResourceList()
+.FindByWatchpoint(this)
+->GetID();
+  else
+return UINT32_MAX;

JDevlieghere wrote:

+1

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -266,33 +268,73 @@ void Watchpoint::Dump(Stream *s) const {
 
 // If prefix is nullptr, we display the watch id and ignore the prefix
 // altogether.
-void Watchpoint::DumpSnapshots(Stream *s, const char *prefix) const {
-  if (!prefix) {
-s->Printf("\nWatchpoint %u hit:", GetID());
-prefix = "";
-  }
+bool Watchpoint::DumpSnapshots(Stream *s, const char *prefix) const {
+  bool printed_anything = false;

JDevlieghere wrote:

Doesn't look like you actually need this. You change the value exactly once 
below. I would just return false on line 276 and 337 and true on line 334. I 
think the return value of a `Dump` function is more self explanatory than 
having to scroll up and see if `printed_anything` happened to be modified in 
the 50 lines between 275 and 325. 

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -0,0 +1,97 @@
+//===-- WatchpointResource.cpp 
===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Breakpoint/WatchpointResource.h"
+#include "lldb/Utility/LLDBAssert.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+WatchpointResource::WatchpointResource(lldb::addr_t addr, size_t size,
+   bool read, bool write)
+: m_id(LLDB_INVALID_WATCHPOINT_RESOURCE_ID), m_addr(addr), m_size(size),
+  m_watch_read(read), m_watch_write(write) {}
+
+WatchpointResource::~WatchpointResource() {
+  std::lock_guard guard(m_owners_mutex);
+  m_owners.clear();
+}
+
+addr_t WatchpointResource::GetAddress() const { return m_addr; }
+
+size_t WatchpointResource::GetByteSize() const { return m_size; }
+
+bool WatchpointResource::WatchpointResourceRead() const { return m_watch_read; 
}
+
+bool WatchpointResource::WatchpointResourceWrite() const {
+  return m_watch_write;
+}
+
+void WatchpointResource::SetType(bool read, bool write) {
+  m_watch_read = read;
+  m_watch_write = write;
+}
+
+wp_resource_id_t WatchpointResource::GetID() const { return m_id; }
+
+void WatchpointResource::SetID(wp_resource_id_t id) { m_id = id; }
+
+bool WatchpointResource::Contains(addr_t addr) {
+  if (addr >= m_addr && addr < m_addr + m_size)
+return true;
+  return false;
+}
+
+void WatchpointResource::AddOwner(const WatchpointSP &wp_sp) {
+  std::lock_guard guard(m_owners_mutex);
+  m_owners.push_back(wp_sp);
+}
+
+void WatchpointResource::RemoveOwner(WatchpointSP &wp_sp) {
+  std::lock_guard guard(m_owners_mutex);
+  const auto &it = std::find(m_owners.begin(), m_owners.end(), wp_sp);
+  if (it != m_owners.end())
+m_owners.erase(it);
+}
+
+size_t WatchpointResource::GetNumberOfOwners() {
+  std::lock_guard guard(m_owners_mutex);
+  return m_owners.size();
+}
+
+bool WatchpointResource::OwnersContains(WatchpointSP &wp_sp) {
+  std::lock_guard guard(m_owners_mutex);
+  const auto &it = std::find(m_owners.begin(), m_owners.end(), wp_sp);
+  return it != m_owners.end();
+}
+
+bool WatchpointResource::OwnersContains(const Watchpoint *wp) {
+  std::lock_guard guard(m_owners_mutex);
+  WatchpointCollection::const_iterator match =
+  std::find_if(m_owners.begin(), m_owners.end(),
+   [&wp](const WatchpointSP &x) { return x.get() == wp; });
+  return match != m_owners.end();
+}
+
+WatchpointSP WatchpointResource::GetOwnerAtIndex(size_t idx) {
+  std::lock_guard guard(m_owners_mutex);
+  lldbassert(idx < m_owners.size());
+  if (idx >= m_owners.size())
+return {};
+
+  return m_owners[idx];
+}
+
+size_t
+WatchpointResource::CopyOwnersList(WatchpointCollection &out_collection) {
+  std::lock_guard guard(m_owners_mutex);
+  for (auto wp_sp : m_owners) {
+out_collection.push_back(wp_sp);
+  }

JDevlieghere wrote:

No braces

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -0,0 +1,146 @@
+//===-- WatchpointResourceList.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_BREAKPOINT_WATCHPOINTRESOURCELIST_H
+#define LLDB_BREAKPOINT_WATCHPOINTRESOURCELIST_H
+
+#include "lldb/Utility/Iterable.h"
+#include "lldb/lldb-private.h"
+#include "lldb/lldb-public.h"
+
+#include 
+#include 
+
+namespace lldb_private {
+
+class WatchpointResourceList {
+
+public:
+  // Constructors and Destructors

JDevlieghere wrote:

Remove

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] [mostly NFC] Large WP foundation: WatchpointResources (PR #68845)

[Jonas Devlieghere via lldb-commits]

@@ -0,0 +1,140 @@
+//===-- WatchpointResource.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+#define LLDB_BREAKPOINT_WATCHPOINTRESOURCE_H
+
+#include "lldb/Breakpoint/WatchpointCollection.h"
+#include "lldb/lldb-public.h"
+
+#include 
+
+namespace lldb_private {
+
+class WatchpointResource
+: public std::enable_shared_from_this {
+
+public:
+  // Constructors and Destructors
+  WatchpointResource(lldb::addr_t addr, size_t size, bool read, bool write);
+
+  ~WatchpointResource();
+
+  void GetMemoryRange(lldb::addr_t &addr, size_t &size) const;
+
+  lldb::addr_t GetAddress() const;
+
+  size_t GetByteSize() const;
+
+  void GetType(bool &read, bool &write) const;
+
+  void SetType(bool read, bool write);
+
+  /// The "Owners" are the watchpoints that share this resource.
+  /// The method adds the \a owner to this resource's owner list.
+  ///
+  /// \param[in] owner
+  ///\a owner is the Wachpoint to add.
+  void AddOwner(const lldb::WatchpointSP &owner);
+
+  /// The method removes the owner at \a owner from this watchpoint
+  /// resource.
+  void RemoveOwner(lldb::WatchpointSP &owner);
+
+  /// This method returns the number of Watchpoints currently using
+  /// watchpoint resource.
+  ///
+  /// \return
+  ///The number of owners.
+  size_t GetNumberOfOwners();
+
+  /// This method returns the Watchpoint at index \a index using this
+  /// Resource.  The owners are listed ordinally from 0 to
+  /// GetNumberOfOwners() - 1 so you can use this method to iterate over the
+  /// owners.
+  ///
+  /// \param[in] idx
+  /// The index in the list of owners for which you wish the owner 
location.
+  ///
+  /// \return
+  ///The Watchpoint at that index.
+  lldb::WatchpointSP GetOwnerAtIndex(size_t idx);
+
+  /// Check if the owners includes a watchpoint.
+  ///
+  /// \param[in] wp_sp
+  /// The WatchpointSP to search for.
+  ///
+  /// \result
+  /// true if this resource's owners includes the watchpoint.
+  bool OwnersContains(lldb::WatchpointSP &wp_sp);
+
+  /// Check if the owners includes a watchpoint.
+  ///
+  /// \param[in] wp
+  /// The Watchpoint to search for.
+  ///
+  /// \result
+  /// true if this resource's owners includes the watchpoint.
+  bool OwnersContains(const lldb_private::Watchpoint *wp);
+
+  /// This method copies the watchpoint resource's owners into a new 
collection.
+  /// It does this while the owners mutex is locked.
+  ///
+  /// \param[out] out_collection
+  ///The BreakpointLocationCollection into which to put the owners
+  ///of this breakpoint site.
+  ///
+  /// \return
+  ///The number of elements copied into out_collection.
+  size_t CopyOwnersList(WatchpointCollection &out_collection);
+
+  // The ID of the WatchpointResource is set by the WatchpointResourceList
+  // when the Resource has been set in the inferior and is being added
+  // to the List, in an attempt to match the hardware watchpoint register
+  // ordering.  If a Process can correctly identify the hardware watchpoint
+  // register index when it has created the Resource, it may initialize it
+  // before it is inserted in the WatchpointResourceList.
+  void SetID(lldb::wp_resource_id_t);
+
+  lldb::wp_resource_id_t GetID() const;
+
+  bool Contains(lldb::addr_t addr);
+
+protected:
+  // The StopInfoWatchpoint knows when it is processing a hit for a thread for
+  // a site, so let it be the one to manage setting the location hit count once
+  // and only once.
+  friend class StopInfoWatchpoint;
+
+  void BumpHitCounts();
+
+private:
+  lldb::wp_resource_id_t m_id;
+
+  // start address & size aligned & expanded to be a valid watchpoint
+  // memory granule on this target.
+  lldb::addr_t m_addr;
+  size_t m_size;
+
+  bool m_watch_read;  // true if we stop when the watched data is read from
+  bool m_watch_write; // true if we stop when the watched data is written to

JDevlieghere wrote:

+1. Otherwise they should go above the line, with `///` and make it a 
capitalized sentence with a period. 

https://github.com/llvm/llvm-project/pull/68845
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 4ec9cda - [lldb/test] Fix failures following ec456ba9ca0a

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2023-10-26T21:21:54-07:00
New Revision: 4ec9cda656cc2fde41d4a4415ae363d9a3290c80

URL: 
https://github.com/llvm/llvm-project/commit/4ec9cda656cc2fde41d4a4415ae363d9a3290c80
DIFF: 
https://github.com/llvm/llvm-project/commit/4ec9cda656cc2fde41d4a4415ae363d9a3290c80.diff

LOG: [lldb/test] Fix failures following ec456ba9ca0a

This patch fixes the various crashlog test failures following
ec456ba9ca0a, which renamed the process member variable in the Scripted
Thread python base class.

This patch updates the crashlog scripted process implementation to
reflect that change.

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/examples/python/crashlog_scripted_process.py

Removed: 




diff  --git a/lldb/examples/python/crashlog_scripted_process.py 
b/lldb/examples/python/crashlog_scripted_process.py
index 43f767de138cd50..c69985b1a072d09 100644
--- a/lldb/examples/python/crashlog_scripted_process.py
+++ b/lldb/examples/python/crashlog_scripted_process.py
@@ -159,14 +159,14 @@ def resolve_stackframes(thread, addr_mask, target):
 return frames
 
 def create_stackframes(self):
-if not (self.scripted_process.load_all_images or self.has_crashed):
+if not (self.originating_process.load_all_images or self.has_crashed):
 return None
 
 if not self.backing_thread or not len(self.backing_thread.frames):
 return None
 
 self.frames = CrashLogScriptedThread.resolve_stackframes(
-self.backing_thread, self.scripted_process.addr_mask, self.target
+self.backing_thread, self.originating_process.addr_mask, 
self.target
 )
 
 return self.frames
@@ -182,7 +182,7 @@ def __init__(self, process, args, crashlog_thread):
 else:
 self.name = self.backing_thread.name
 self.queue = self.backing_thread.queue
-self.has_crashed = self.scripted_process.crashed_thread_idx == self.idx
+self.has_crashed = self.originating_process.crashed_thread_idx == 
self.idx
 self.create_stackframes()
 
 def get_state(self):
@@ -195,8 +195,8 @@ def get_stop_reason(self) -> Dict[str, Any]:
 return {"type": lldb.eStopReasonNone}
 # TODO: Investigate what stop reason should be reported when crashed
 stop_reason = {"type": lldb.eStopReasonException, "data": {}}
-if self.scripted_process.exception:
-stop_reason["data"]["mach_exception"] = 
self.scripted_process.exception
+if self.originating_process.exception:
+stop_reason["data"]["mach_exception"] = 
self.originating_process.exception
 return stop_reason
 
 def get_register_context(self) -> str:
@@ -209,5 +209,5 @@ def get_register_context(self) -> str:
 
 def get_extended_info(self):
 if self.has_crashed:
-self.extended_info = self.scripted_process.extended_thread_info
+self.extended_info = self.originating_process.extended_thread_info
 return self.extended_info



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits