[Lldb-commits] [PATCH] D154930: [lldb][AArch64] Add the tpidr2 TLS register that comes with SME

[Muhammad Omair Javaid via Phabricator via lldb-commits]

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

This looks good just a minor suggestion inline.




Comment at: 
lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py:84
+
+self.check_tls_reg("tpidr2")
+

Should we try to read/write both tpidr and tpidr2 here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154930

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

[Lldb-commits] [PATCH] D154930: [lldb][AArch64] Add the tpidr2 TLS register that comes with SME

[David Spickett via Phabricator via lldb-commits]

DavidSpickett marked 2 inline comments as done.
DavidSpickett added a comment.

Thanks for the review, I'll try making the test a bit more flexible so we can 
read both registers at once for a bit more coverage.




Comment at: 
lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py:84
+
+self.check_tls_reg("tpidr2")
+

omjavaid wrote:
> Should we try to read/write both tpidr and tpidr2 here?
We could but on a system with SME `test_tpidr` will also run. So you're 
checking both regardless.

I could change the test program so that it always sets tpidr, but only sets 
tpdir2 if told to. Then we could check both here just in case there's an issue 
reading one after the other. I'll give that a go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154930

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

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

The only part of this that's maybe tricky is the HighMem parts of it. Given the 
niche use case it should be ok if they turn out a bit awkward for general use.

You are missing a way to fix a high mem address, is that intentional? Or would 
you get the high mem mask, change the setting, fix the address, as needed by 
context.

The rest are passing on existing calls we know are useful within lldb.




Comment at: lldb/include/lldb/API/SBProcess.h:401
 
+  /// Get the current address mask that may be applied to addresses
+  /// before reading from memory.  The bits which are not relevant/valid

may -> should?

Though for API uses that only run on simple systems, calling this would just be 
unnecessary overhead.



Comment at: lldb/include/lldb/API/SBProcess.h:402-403
+  /// Get the current address mask that may be applied to addresses
+  /// before reading from memory.  The bits which are not relevant/valid
+  /// for addressing should be set.
+  ///

"should be set in the mask". Or in reverse "should be cleared by the mask". 
Just be clear whether it's the mask or the address.



Comment at: lldb/include/lldb/API/SBProcess.h:416
+  lldb::addr_t GetCodeAddressMask();
+  lldb::addr_t GetDataAddressMask();
+

Should we note somewhere, maybe below where the `any` method is, the logic 
behind the code/data split?

Such as it is. Overall it's that data (load store) and code (fetch) addresses 
may have a different arrangement of addressing bits, so choose the one that 
makes sense in context. If we don't know, use any. It'd be an obvious question 
from anyone integrating this into an IDE.



Comment at: lldb/include/lldb/API/SBProcess.h:453
+  lldb::addr_t FixDataAddress(lldb::addr_t addr);
+  lldb::addr_t FixAnyAddress(lldb::addr_t addr);
+

Here is a good place for a note about which to pick.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155905

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

[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

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

[Lldb-commits] [PATCH] D154926: [lldb][AArch64] Add support for SME's SVE streaming mode registers

[Muhammad Omair Javaid via Phabricator via lldb-commits]

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

This looks very good much simpler from the first version. just a minor nit.




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:81
   opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskSVE);
 
+  // We may also have the Scalable Matrix Extension (SME) which adds a

Should we test for SSVE first that way we wont have to check for SVE once SSVE 
is found?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154926

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

[Lldb-commits] [PATCH] D154927: [lldb][AArch64] Add SME's streaming vector control register

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

I guess there is not much gain from telling the user between sve/ssve mode for 
the moment. Can we defer this patch till SME registers are implemented?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154927

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

[Lldb-commits] [PATCH] D154927: [lldb][AArch64] Add SME's streaming vector control register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett planned changes to this revision.
DavidSpickett added a comment.

In some sense it already is, it'll go in after the streaming mode SVE 
registers. Should we wait until we can show the full content including the ZA 
bit? Yeah, why not, we're in no rush.

I have a patch locally for ZA support that I'm working on. I'll reorder this to 
after that and when I update this, it will have the full SVCR contents.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154927

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

[Lldb-commits] [PATCH] D154930: [lldb][AArch64] Add the tpidr2 TLS register that comes with SME

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 542910.
DavidSpickett added a comment.

Refactor the test so there are tests for:

- reading tpidr on non-SME systems
- reading tpidr and tpidr2 on SME systems
- tpidr2 not being present on non-SME systems

You could try to fold the last one into the first one but
it gets fiddly for not much saving.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154930

Files:
  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/test/API/linux/aarch64/tls_register/Makefile
  lldb/test/API/linux/aarch64/tls_register/TestAArch64LinuxTLSRegister.py
  lldb/test/API/linux/aarch64/tls_register/main.c
  lldb/test/API/linux/aarch64/tls_registers/Makefile
  lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
  lldb/test/API/linux/aarch64/tls_registers/main.c

Index: lldb/test/API/linux/aarch64/tls_registers/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_registers/main.c
@@ -0,0 +1,60 @@
+#include 
+#include 
+
+uint64_t get_tpidr(void) {
+  uint64_t tpidr = 0;
+  __asm__ volatile("mrs %0, tpidr_el0" : "=r"(tpidr));
+  return tpidr;
+}
+
+uint64_t get_tpidr2(void) {
+  uint64_t tpidr2 = 0;
+  // S3_3_C13_C0_5 means tpidr2, and will work with older tools.
+  __asm__ volatile("mrs %0, S3_3_C13_C0_5" : "=r"(tpidr2));
+  return tpidr2;
+}
+
+void set_tpidr(uint64_t value) {
+  __asm__ volatile("msr tpidr_el0, %0" ::"r"(value));
+}
+
+void set_tpidr2(uint64_t value) {
+  __asm__ volatile("msr S3_3_C13_C0_5, %0" ::"r"(value));
+}
+
+int main(int argc, char *argv[]) {
+  bool use_tpidr2 = argc > 1;
+
+  uint64_t original_tpidr = get_tpidr();
+  // Accessing this on a core without it produces SIGILL. Only do this if
+  // requested.
+  uint64_t original_tpidr2 = 0;
+  if (use_tpidr2)
+original_tpidr2 = get_tpidr2();
+
+  uint64_t tpidr_pattern = 0x1122334455667788;
+  set_tpidr(tpidr_pattern);
+
+  uint64_t tpidr2_pattern = 0x8877665544332211;
+  if (use_tpidr2)
+set_tpidr2(tpidr2_pattern);
+
+  // Set break point at this line.
+  // lldb will now set its own pattern(s) for us to find.
+
+  uint64_t new_tpidr = get_tpidr();
+  volatile bool tpidr_was_set = new_tpidr == 0x;
+
+  uint64_t new_tpidr2 = 0;
+  volatile bool tpidr2_was_set = false;
+  if (use_tpidr2) {
+new_tpidr2 = get_tpidr2();
+tpidr2_was_set = new_tpidr2 == 0x;
+  }
+
+  set_tpidr(original_tpidr);
+  if (use_tpidr2)
+set_tpidr2(original_tpidr2);
+
+  return 0; // Set break point 2 at this line.
+}
Index: lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -0,0 +1,115 @@
+"""
+Test lldb's ability to read and write the AArch64 TLS registers.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxTLSRegisters(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def setup(self, registers):
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"main.c",
+line_number("main.c", "// Set break point at this line."),
+num_expected_locations=1,
+)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"main.c",
+line_number("main.c", "// Set break point 2 at this line."),
+num_expected_locations=1,
+)
+
+if "tpidr2" in registers:
+self.runCmd("settings set target.run-args 1")
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stopped", "stop reason = breakpoint"],
+)
+
+def check_tls_reg(self, registers):
+self.setup(registers)
+
+regs = self.thread().GetSelectedFrame().GetRegisters()
+tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
+self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
+
+# Since we can't predict what the value will be, the program has set
+# a target value for us to find.
+initial_values = {
+"tpidr": 0x1122334455667788,
+"tpidr2": 0x8877665544332211,
+}
+
+for register in registe

[Lldb-commits] [PATCH] D154930: [lldb][AArch64] Add the tpidr2 TLS register that comes with SME

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 542911.
DavidSpickett added a comment.

Correct missing "tls" in test name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154930

Files:
  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/test/API/linux/aarch64/tls_register/Makefile
  lldb/test/API/linux/aarch64/tls_register/TestAArch64LinuxTLSRegister.py
  lldb/test/API/linux/aarch64/tls_register/main.c
  lldb/test/API/linux/aarch64/tls_registers/Makefile
  lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
  lldb/test/API/linux/aarch64/tls_registers/main.c

Index: lldb/test/API/linux/aarch64/tls_registers/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_registers/main.c
@@ -0,0 +1,60 @@
+#include 
+#include 
+
+uint64_t get_tpidr(void) {
+  uint64_t tpidr = 0;
+  __asm__ volatile("mrs %0, tpidr_el0" : "=r"(tpidr));
+  return tpidr;
+}
+
+uint64_t get_tpidr2(void) {
+  uint64_t tpidr2 = 0;
+  // S3_3_C13_C0_5 means tpidr2, and will work with older tools.
+  __asm__ volatile("mrs %0, S3_3_C13_C0_5" : "=r"(tpidr2));
+  return tpidr2;
+}
+
+void set_tpidr(uint64_t value) {
+  __asm__ volatile("msr tpidr_el0, %0" ::"r"(value));
+}
+
+void set_tpidr2(uint64_t value) {
+  __asm__ volatile("msr S3_3_C13_C0_5, %0" ::"r"(value));
+}
+
+int main(int argc, char *argv[]) {
+  bool use_tpidr2 = argc > 1;
+
+  uint64_t original_tpidr = get_tpidr();
+  // Accessing this on a core without it produces SIGILL. Only do this if
+  // requested.
+  uint64_t original_tpidr2 = 0;
+  if (use_tpidr2)
+original_tpidr2 = get_tpidr2();
+
+  uint64_t tpidr_pattern = 0x1122334455667788;
+  set_tpidr(tpidr_pattern);
+
+  uint64_t tpidr2_pattern = 0x8877665544332211;
+  if (use_tpidr2)
+set_tpidr2(tpidr2_pattern);
+
+  // Set break point at this line.
+  // lldb will now set its own pattern(s) for us to find.
+
+  uint64_t new_tpidr = get_tpidr();
+  volatile bool tpidr_was_set = new_tpidr == 0x;
+
+  uint64_t new_tpidr2 = 0;
+  volatile bool tpidr2_was_set = false;
+  if (use_tpidr2) {
+new_tpidr2 = get_tpidr2();
+tpidr2_was_set = new_tpidr2 == 0x;
+  }
+
+  set_tpidr(original_tpidr);
+  if (use_tpidr2)
+set_tpidr2(original_tpidr2);
+
+  return 0; // Set break point 2 at this line.
+}
Index: lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -0,0 +1,115 @@
+"""
+Test lldb's ability to read and write the AArch64 TLS registers.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxTLSRegisters(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def setup(self, registers):
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"main.c",
+line_number("main.c", "// Set break point at this line."),
+num_expected_locations=1,
+)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"main.c",
+line_number("main.c", "// Set break point 2 at this line."),
+num_expected_locations=1,
+)
+
+if "tpidr2" in registers:
+self.runCmd("settings set target.run-args 1")
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stopped", "stop reason = breakpoint"],
+)
+
+def check_tls_reg(self, registers):
+self.setup(registers)
+
+regs = self.thread().GetSelectedFrame().GetRegisters()
+tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
+self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
+
+# Since we can't predict what the value will be, the program has set
+# a target value for us to find.
+initial_values = {
+"tpidr": 0x1122334455667788,
+"tpidr2": 0x8877665544332211,
+}
+
+for register in registers:
+tls_reg = tls_regs.GetChildMemberWithName(register)
+self.assertTrue(tls_reg.IsValid(), "{} register not found.".format(
+register))
+self.assertEqual(tls_reg.GetValueA

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

Thanks for the feedback David.

> You are missing a way to fix a high mem address, is that intentional? Or 
> would you get the high mem mask, change the setting, fix the address, as 
> needed by context.

The FixAddress method will choose whether to use the high mem mask or the low 
mem mask based on the address.




Comment at: lldb/include/lldb/API/SBProcess.h:401
 
+  /// Get the current address mask that may be applied to addresses
+  /// before reading from memory.  The bits which are not relevant/valid

DavidSpickett wrote:
> may -> should?
> 
> Though for API uses that only run on simple systems, calling this would just 
> be unnecessary overhead.
Probably the right call.  The masking is implemented in the ABI so there are 
systems that don't have this concept and have no mask function, which is what I 
was thinking of with "maybe".  

It makes me wonder if doing it in the ABI is correct; if we have an address 
mask for the system, is it a part of the ABI?  On an ARMv8.3 ptrauth using 
process, the ABI defines what things are signed and need to be cleared before 
dereferencing (where we need to call FixAddress from within lldb), but we solve 
this by calling FixAddress at all sites that any ABI might use; ultimately they 
all need to resolve to an addressable address before reading, so if one ABI 
signs the C++ vtable pointer and one does not, having both call FixAddress on 
that is fine.

Of course I'm coming at this with an AArch64 perspective.  On AArch64 we need 
to look at b55 to decide if the bits that are masked off are set to 0 or 1  
(bits 56-63 possibly being TBI non-address bits), which is not a generic 
behavior that would apply on other processors that might need to apply an 
address mask.  So maybe it's an architecture specific method instead of an ABI 
method, I should say.



Comment at: lldb/include/lldb/API/SBProcess.h:402-403
+  /// Get the current address mask that may be applied to addresses
+  /// before reading from memory.  The bits which are not relevant/valid
+  /// for addressing should be set.
+  ///

DavidSpickett wrote:
> "should be set in the mask". Or in reverse "should be cleared by the mask". 
> Just be clear whether it's the mask or the address.
Ah very good point.



Comment at: lldb/include/lldb/API/SBProcess.h:416
+  lldb::addr_t GetCodeAddressMask();
+  lldb::addr_t GetDataAddressMask();
+

DavidSpickett wrote:
> Should we note somewhere, maybe below where the `any` method is, the logic 
> behind the code/data split?
> 
> Such as it is. Overall it's that data (load store) and code (fetch) addresses 
> may have a different arrangement of addressing bits, so choose the one that 
> makes sense in context. If we don't know, use any. It'd be an obvious 
> question from anyone integrating this into an IDE.
Yeah I mean the reality is that on all our systems where code and data 
addresses are the same, FixCodeAddress may take advantage of the alignment of 
instructions to add a couple of bits, whereas FixDataAddress has to be able to 
address at a byte boundary so we really have "FixCodeAddress" and 
"FixAnyAddress", and it's only the Linux address mask namings that got us 
"FixDataAddress".  

Of course there exist systems where instruction addresses and data addresses 
may not be as interchangeable, and I could even imagine some Harvard 
architecture system where the number of bits used for addressing of 
instructions is different than data, and applying the Data address mask to an 
instruction could corrupt it.  

But I should explain the ideal use of these methods.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155905

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

[Lldb-commits] [PATCH] D154926: [lldb][AArch64] Add support for SME's SVE streaming mode registers

[David Spickett via Phabricator via lldb-commits]

DavidSpickett marked an inline comment as done.
DavidSpickett added inline comments.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:81
   opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskSVE);
 
+  // We may also have the Scalable Matrix Extension (SME) which adds a

omjavaid wrote:
> Should we test for SSVE first that way we wont have to check for SVE once 
> SSVE is found?
Good idea, but according to the architecture supplement:
```
If SME is implemented, this does not imply that FEAT_SVE and FEAT_SVE2 are 
implemented by the PE when it is not in Streaming SVE mode.
```
I don't know how realistic such a core is but at a glance Linux doesn't say it 
wouldn't support it. I'll leave this as it is on that basis.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154926

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

[Lldb-commits] [PATCH] D154926: [lldb][AArch64] Add support for SME's SVE streaming mode registers

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 542930.
DavidSpickett marked an inline comment as done.
DavidSpickett added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154926

Files:
  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/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
  lldb/test/API/commands/register/register/aarch64_sve_simd_registers/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
  lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
@@ -0,0 +1,108 @@
+#include 
+#include 
+
+void write_simd_regs() {
+#define WRITE_SIMD(NUM)\
+  asm volatile("MOV v" #NUM ".d[0], %0\n\t"\
+   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(NUM))
+
+  WRITE_SIMD(0);
+  WRITE_SIMD(1);
+  WRITE_SIMD(2);
+  WRITE_SIMD(3);
+  WRITE_SIMD(4);
+  WRITE_SIMD(5);
+  WRITE_SIMD(6);
+  WRITE_SIMD(7);
+  WRITE_SIMD(8);
+  WRITE_SIMD(9);
+  WRITE_SIMD(10);
+  WRITE_SIMD(11);
+  WRITE_SIMD(12);
+  WRITE_SIMD(13);
+  WRITE_SIMD(14);
+  WRITE_SIMD(15);
+  WRITE_SIMD(16);
+  WRITE_SIMD(17);
+  WRITE_SIMD(18);
+  WRITE_SIMD(19);
+  WRITE_SIMD(20);
+  WRITE_SIMD(21);
+  WRITE_SIMD(22);
+  WRITE_SIMD(23);
+  WRITE_SIMD(24);
+  WRITE_SIMD(25);
+  WRITE_SIMD(26);
+  WRITE_SIMD(27);
+  WRITE_SIMD(28);
+  WRITE_SIMD(29);
+  WRITE_SIMD(30);
+  WRITE_SIMD(31);
+}
+
+unsigned verify_simd_regs() {
+  uint64_t got_low = 0;
+  uint64_t got_high = 0;
+  uint64_t target = 0;
+
+#define VERIFY_SIMD(NUM)   \
+  do { \
+got_low = 0;   \
+got_high = 0;  \
+asm volatile("MOV %0, v" #NUM ".d[0]\n\t"  \
+ "MOV %1, v" #NUM ".d[1]\n\t"  \
+ : "=r"(got_low), "=r"(got_high)); \
+target = NUM + 1;  \
+if ((got_low != target) || (got_high != target))   \
+  return 1;\
+  } while (0)
+
+  VERIFY_SIMD(0);
+  VERIFY_SIMD(1);
+  VERIFY_SIMD(2);
+  VERIFY_SIMD(3);
+  VERIFY_SIMD(4);
+  VERIFY_SIMD(5);
+  VERIFY_SIMD(6);
+  VERIFY_SIMD(7);
+  VERIFY_SIMD(8);
+  VERIFY_SIMD(9);
+  VERIFY_SIMD(10);
+  VERIFY_SIMD(11);
+  VERIFY_SIMD(12);
+  VERIFY_SIMD(13);
+  VERIFY_SIMD(14);
+  VERIFY_SIMD(15);
+  VERIFY_SIMD(16);
+  VERIFY_SIMD(17);
+  VERIFY_SIMD(18);
+  VERIFY_SIMD(19);
+  VERIFY_SIMD(20);
+  VERIFY_SIMD(21);
+  VERIFY_SIMD(22);
+  VERIFY_SIMD(23);
+  VERIFY_SIMD(24);
+  VERIFY_SIMD(25);
+  VERIFY_SIMD(26);
+  VERIFY_SIMD(27);
+  VERIFY_SIMD(28);
+  VERIFY_SIMD(29);
+  VERIFY_SIMD(30);
+  VERIFY_SIMD(31);
+
+  return 0;
+}
+
+int main() {
+#ifdef SSVE
+  asm volatile("msr  s0_3_c4_c7_3, xzr" /*smstart*/);
+#elif defined SVE
+  // Make the non-streaming SVE registers active.
+  asm volatile("cpy  z0.b, p0/z, #1\n\t");
+#endif
+  // else test plain SIMD access.
+
+  write_simd_regs();
+
+  return verify_simd_regs(); // Set a break point here.
+}
Index: lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
@@ -0,0 +1,108 @@
+"""
+Test that LLDB correctly reads and writes AArch64 SIMD registers in SVE,
+streaming SVE and normal SIMD modes.
+
+There are a few operating modes and we use different strategies for each:
+* Without SVE, in SIMD mode - read the SIMD regset.
+* With SVE, but SVE is inactive - read the SVE regset, but get 

[Lldb-commits] [PATCH] D154930: [lldb][AArch64] Add the tpidr2 TLS register that comes with SME

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 542932.
DavidSpickett added a comment.

Rebase and put this after the SSVE registers patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154930

Files:
  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/test/API/linux/aarch64/tls_register/Makefile
  lldb/test/API/linux/aarch64/tls_register/TestAArch64LinuxTLSRegister.py
  lldb/test/API/linux/aarch64/tls_register/main.c
  lldb/test/API/linux/aarch64/tls_registers/Makefile
  lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
  lldb/test/API/linux/aarch64/tls_registers/main.c

Index: lldb/test/API/linux/aarch64/tls_registers/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_registers/main.c
@@ -0,0 +1,60 @@
+#include 
+#include 
+
+uint64_t get_tpidr(void) {
+  uint64_t tpidr = 0;
+  __asm__ volatile("mrs %0, tpidr_el0" : "=r"(tpidr));
+  return tpidr;
+}
+
+uint64_t get_tpidr2(void) {
+  uint64_t tpidr2 = 0;
+  // S3_3_C13_C0_5 means tpidr2, and will work with older tools.
+  __asm__ volatile("mrs %0, S3_3_C13_C0_5" : "=r"(tpidr2));
+  return tpidr2;
+}
+
+void set_tpidr(uint64_t value) {
+  __asm__ volatile("msr tpidr_el0, %0" ::"r"(value));
+}
+
+void set_tpidr2(uint64_t value) {
+  __asm__ volatile("msr S3_3_C13_C0_5, %0" ::"r"(value));
+}
+
+int main(int argc, char *argv[]) {
+  bool use_tpidr2 = argc > 1;
+
+  uint64_t original_tpidr = get_tpidr();
+  // Accessing this on a core without it produces SIGILL. Only do this if
+  // requested.
+  uint64_t original_tpidr2 = 0;
+  if (use_tpidr2)
+original_tpidr2 = get_tpidr2();
+
+  uint64_t tpidr_pattern = 0x1122334455667788;
+  set_tpidr(tpidr_pattern);
+
+  uint64_t tpidr2_pattern = 0x8877665544332211;
+  if (use_tpidr2)
+set_tpidr2(tpidr2_pattern);
+
+  // Set break point at this line.
+  // lldb will now set its own pattern(s) for us to find.
+
+  uint64_t new_tpidr = get_tpidr();
+  volatile bool tpidr_was_set = new_tpidr == 0x;
+
+  uint64_t new_tpidr2 = 0;
+  volatile bool tpidr2_was_set = false;
+  if (use_tpidr2) {
+new_tpidr2 = get_tpidr2();
+tpidr2_was_set = new_tpidr2 == 0x;
+  }
+
+  set_tpidr(original_tpidr);
+  if (use_tpidr2)
+set_tpidr2(original_tpidr2);
+
+  return 0; // Set break point 2 at this line.
+}
Index: lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -0,0 +1,115 @@
+"""
+Test lldb's ability to read and write the AArch64 TLS registers.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxTLSRegisters(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def setup(self, registers):
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"main.c",
+line_number("main.c", "// Set break point at this line."),
+num_expected_locations=1,
+)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"main.c",
+line_number("main.c", "// Set break point 2 at this line."),
+num_expected_locations=1,
+)
+
+if "tpidr2" in registers:
+self.runCmd("settings set target.run-args 1")
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stopped", "stop reason = breakpoint"],
+)
+
+def check_tls_reg(self, registers):
+self.setup(registers)
+
+regs = self.thread().GetSelectedFrame().GetRegisters()
+tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
+self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
+
+# Since we can't predict what the value will be, the program has set
+# a target value for us to find.
+initial_values = {
+"tpidr": 0x1122334455667788,
+"tpidr2": 0x8877665544332211,
+}
+
+for register in registers:
+tls_reg = tls_regs.GetChildMemberWithName(register)
+self.assertTrue(tls_reg.IsValid(), "{} register not found.".format(
+register))
+self.assertEqual(t

[Lldb-commits] [PATCH] D155161: [lldb] Convert script native types to StructuredData counterpart

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere requested changes to this revision.
JDevlieghere added a comment.
This revision now requires changes to proceed.

I'm okay with the ABI break given that this (1) hasn't made it into a release 
yet, (2) it is under active development by Ismail himself and (3) has existing 
adopters beyond the ones motivating the original and current change.

However, now that it's more than a typedef, we should rename ScriptObject to 
`SBScriptObject`, move it to its own file and hide its implementation with 
PIMPL. There is no precedent for having small structs in lldb-types and I don't 
see why we would want lock ourselves into the current implementation. If 
anything this patch itself serves as an example that we want the ability to 
change this class without breaking the ABI (again).


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

https://reviews.llvm.org/D155161

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

[Lldb-commits] [PATCH] D149213: [lldb] Add basic support for Rust enums in TypeSystemClang

[Vladimir Makaev via Phabricator via lldb-commits]

VladimirMakaev marked an inline comment as done.
VladimirMakaev added a comment.

In D149213#4520309 , @tom.tromey 
wrote:

>> Apart from just implementing type system itself (which is much bigger scope 
>> than this change) there are other non-trivial issues:
>>
>> 1. There is no "compiler-as-a-service" in Rust so getting expressions to 
>> work is non-trivial. An interpreter of some sort needs to be built with 
>> subset of Rust support
>
> My work also included a parser for some Rust expressions.

Yeah I reviewed your fork and I saw there was a parser. So it could potentially 
be used with some success. With this approach we can use Clang for expression 
evaluations which might be inconvenient for Rust developers but it's decent 
enough. Other concerns with having this tested with CI still stands. Overall I 
agree that having TypeSystemRust will offer a better experience and more tuning 
but this fix is quite cheap and should be a big improvement over current state 
of things.  Do you know by chance if there are other areas apart from enums 
which are currently broken in LLDB and cannot be fixed via synthetic provider?


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

https://reviews.llvm.org/D149213

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

[Lldb-commits] [PATCH] D149213: [lldb] Add basic support for Rust enums in TypeSystemClang

[J. Ryan Stinnett via Phabricator via lldb-commits]

jryans added a comment.

In D149213#4491889 , @VladimirMakaev 
wrote:

> I think there was an attempt in the past to build TypeSystemRust. Rust 
> Project had a fork of LLDB with this implemented by Tom Tromey and CodeLLDB 
> maintainer(vadimcn) has one. Both of them were not able to merge that into 
> LLDB codebase. A relevant link to a discussion 
> https://rust-lang.zulipchat.com/#narrow/stream/317568-t-compiler.2Fwg-debugging/topic/upstreaming.20rust.20support.20into.20lldb
>
> Apart from just implementing type system itself (which is much bigger scope 
> than this change) there are other non-trivial issues:
>
> 1. There is no "compiler-as-a-service" in Rust so getting expressions to work 
> is non-trivial. An interpreter of some sort needs to be built with subset of 
> Rust support
> 2. Infra changes (DEVOPS) changes are required to start building Rust 
> inferiors to run tests on CI
> 3. LLVM / rustc cross repository logistics. Rustc probably needs to live in 
> LLVM to make this work well.

I know this is not exactly the focus of this patch, but I just wanted to reply 
to this portion (since you've mentioned it). While some people did try to 
suggest things like your points 2 and 3 in previous Rust support attempts, I 
would say it seems impractical to require pulling a whole language 
implementation into LLVM just to add debugging support. If LLDB really wants to 
be a many-language debugger, I would strongly urge looking for testing 
solutions that do not require somehow adding an full implementation of each one 
into the tree. After all, the whole point of debug info formats like DWARF is 
that the producer and consumer do not need to be tightly coupled (since there's 
a standard format in between).

I would argue a more practical approach would involve test cases based on 
whatever IR or AST might be appropriate for the feature under test, e.g. DWARF 
output from Rust, Rust IR or MIR, etc. These things are just data, and LLDB 
(and / or small helper tools) could process this data without the need for an 
entire language implementation.

A discussion from 2019 
(https://discourse.llvm.org/t/rust-support-in-lldb-again/53210) seemed to 
potentially be open to such an idea, but perhaps it's worth starting a new 
discussion thread focused on the testing requirements for language support in 
LLDB which attempts to work out guidelines which allow for languages whose 
implementations live outside of the LLVM tree.


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

https://reviews.llvm.org/D149213

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

Re: [Lldb-commits] [PATCH] D149213: [lldb] Add basic support for Rust enums in TypeSystemClang

[Jim Ingham via lldb-commits]

> On Jul 21, 2023, at 9:52 AM, J. Ryan Stinnett via Phabricator via 
> lldb-commits  wrote:
> 
> jryans added a comment.
> 
> In D149213#4491889 , 
> @VladimirMakaev wrote:
> 
>> I think there was an attempt in the past to build TypeSystemRust. Rust 
>> Project had a fork of LLDB with this implemented by Tom Tromey and CodeLLDB 
>> maintainer(vadimcn) has one. Both of them were not able to merge that into 
>> LLDB codebase. A relevant link to a discussion 
>> https://rust-lang.zulipchat.com/#narrow/stream/317568-t-compiler.2Fwg-debugging/topic/upstreaming.20rust.20support.20into.20lldb
>> 
>> Apart from just implementing type system itself (which is much bigger scope 
>> than this change) there are other non-trivial issues:
>> 
>> 1. There is no "compiler-as-a-service" in Rust so getting expressions to 
>> work is non-trivial. An interpreter of some sort needs to be built with 
>> subset of Rust support
>> 2. Infra changes (DEVOPS) changes are required to start building Rust 
>> inferiors to run tests on CI
>> 3. LLVM / rustc cross repository logistics. Rustc probably needs to live in 
>> LLVM to make this work well.
> 
> I know this is not exactly the focus of this patch, but I just wanted to 
> reply to this portion (since you've mentioned it). While some people did try 
> to suggest things like your points 2 and 3 in previous Rust support attempts, 
> I would say it seems impractical to require pulling a whole language 
> implementation into LLVM just to add debugging support. If LLDB really wants 
> to be a many-language debugger, I would strongly urge looking for testing 
> solutions that do not require somehow adding an full implementation of each 
> one into the tree. After all, the whole point of debug info formats like 
> DWARF is that the producer and consumer do not need to be tightly coupled 
> (since there's a standard format in between).
> 
> I would argue a more practical approach would involve test cases based on 
> whatever IR or AST might be appropriate for the feature under test, e.g. 
> DWARF output from Rust, Rust IR or MIR, etc. These things are just data, and 
> LLDB (and / or small helper tools) could process this data without the need 
> for an entire language implementation.
> 
> A discussion from 2019 
> (https://discourse.llvm.org/t/rust-support-in-lldb-again/53210) seemed to 
> potentially be open to such an idea, but perhaps it's worth starting a new 
> discussion thread focused on the testing requirements for language support in 
> LLDB which attempts to work out guidelines which allow for languages whose 
> implementations live outside of the LLVM tree.
> 

From our experience supporting other languages, It is not enough to write tests 
that check static predicates.  You need to run expressions and move the program 
around to test the expression parser, and runtime introspection features, etc.  
Imagine trying to write tests for stepping through Rust async dispatch w/o 
being able to run programs in the test suite...  

A sustainable language plugin is going to need those sorts of tests, and trying 
to write those tests w/o a compiler to build the test subjects will get really 
tedious very quickly.  So whatever people plan to do, you need to make it more 
testable than just "did I get the DWARF right".

IMO to make a supportable Rust plugin that wider testing has to go on in CI 
somewhere.  Note, that doesn't require a full copy of the Rust compiler in the 
llvm sources.   As Greg pointed out in that thread you mentioned, you can back 
the TypeSystemRust and the ExpressionParserRust with whatever implementation 
suits you, and if you can do that in a simpler way that doesn't involve the 
actual language implementation, that's fine.  

But I do think there needs to be bots & testing for the plugins that can rely 
on being able to build & test more complex scenarios than just what you can get 
from static analysis. Otherwise, all the people working on support for other 
languages are going to keep breaking the Rust implementation, which won't make 
for a productive community.

Jim

> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D149213/new/
> 
> https://reviews.llvm.org/D149213
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Lldb-commits] [PATCH] D155198: [lldb] Consider OP_addrx in DWARFExpression::Update_DW_OP_addr

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision as: JDevlieghere.
JDevlieghere added a comment.

LGTM. I agree with Adrian: we can go ahead and land this to unblock the bots.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155198

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

[Lldb-commits] [PATCH] D155161: [lldb] Convert script native types to StructuredData counterpart

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 543101.
mib edited the summary of this revision.
mib added a comment.

Address @JDevlieghere & @bulbazord comments:

- Add `SBScriptObject` with PIMPL


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

https://reviews.llvm.org/D155161

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBScriptObjectExtensions.i
  lldb/bindings/interfaces.swig
  lldb/bindings/python/python-typemaps.swig
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBProcess.h
  lldb/include/lldb/API/SBScriptObject.h
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Utility/ScriptObject.h
  lldb/include/lldb/lldb-types.h
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBScriptObject.cpp
  lldb/source/API/SBStructuredData.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py

Index: lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
===
--- lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -76,6 +76,40 @@
 # Tests for array data type
 self.array_struct_test(dict_struct)
 
+s.Clear()
+self.assertSuccess(example.GetAsJSON(s))
+py_obj = json.loads(s.GetData())
+self.assertTrue(py_obj)
+self.assertIn("key_dict", py_obj)
+
+py_dict = py_obj["key_dict"]
+self.assertEqual(py_dict["key_string"], "STRING")
+self.assertEqual(py_dict["key_uint"], 0x)
+self.assertEqual(py_dict["key_sint"], -42)
+self.assertEqual(py_dict["key_float"], 2.99)
+self.assertEqual(py_dict["key_bool"], True)
+self.assertEqual(py_dict["key_array"], ["23", "arr"])
+
+class MyRandomClass:
+payload = "foo"
+
+py_dict["key_generic"] = MyRandomClass()
+
+stp = lldb.SBScriptObject(py_dict, lldb.eScriptLanguagePython)
+self.assertEqual(stp.ptr, py_dict)
+
+sd = lldb.SBStructuredData(stp, self.dbg)
+self.assertTrue(sd.IsValid())
+self.assertEqual(sd.GetSize(), len(py_dict))
+
+generic_sd = sd.GetValueForKey("key_generic")
+self.assertTrue(generic_sd.IsValid())
+self.assertEqual(generic_sd.GetType(), lldb.eStructuredDataTypeGeneric)
+
+my_random_class = generic_sd.GetGenericValue()
+self.assertTrue(my_random_class)
+self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+
 def invalid_struct_test(self, example):
 invalid_struct = lldb.SBStructuredData()
 invalid_struct = example.GetValueForKey("invalid_key")
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -83,6 +83,9 @@
std::string &error_str,
lldb::ThreadPlanSP thread_plan) override;
 
+  StructuredData::ObjectSP
+  CreateStructuredDataFromScriptObject(ScriptObject obj) override;
+
   bool ScriptedThreadPlanExplainsStop(StructuredData::ObjectSP implementor_sp,
   Event *event,
   bool &script_error) override;
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1519,6 +1519,17 @@
   return std::make_unique(*this);
 }
 
+StructuredData::ObjectSP
+ScriptInterpreterPythonImpl::CreateStructuredDataFromScriptObject(
+ScriptObject obj) {
+  void *ptr = const_cast(obj.GetPointer());
+  PythonObject py_obj(PyRefType::Borrowed, static_cast(ptr));
+  if (!py_obj.IsValid() || py_obj.IsNone())
+return {};
+  Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
+  return py_obj.CreateStructuredObject();
+}
+
 StructuredData::GenericSP
 ScriptInterpreterPythonImpl::OSPlugin_CreatePluginObject(
 const char *class_name, lldb::ProcessSP process_sp) {
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++

[Lldb-commits] [PATCH] D155161: [lldb] Convert script native types to StructuredData counterpart

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/API/SBScriptObject.h:47
+private:
+  std::shared_ptr m_opaque_sp;
+};

Does this need to be a shared pointer as opposed to a unique pointer? If so 
please add a comment why. 



Comment at: lldb/include/lldb/Utility/ScriptObject.h:9
+
+#ifndef LLDB_INTERPRETER_SCRIPTOBJECT_H
+#define LLDB_INTERPRETER_SCRIPTOBJECT_H

Not sure if this really belongs in Utility. What about putting it under 
`Plugins/ScriptInterpreter/` or `Interpreter/` next  to `ScriptInterpreter.h`. 
The header guard seems to suggest that it was there at some point? 



Comment at: lldb/source/API/SBScriptObject.cpp:46
+  LLDB_INSTRUMENT_VA(this);
+  return this->operator bool();
+}

Missing newline.


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

https://reviews.llvm.org/D155161

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

[Lldb-commits] [lldb] f09f0a6 - [lldb] Consider OP_addrx in DWARFExpression::Update_DW_OP_addr

[Adrian Prantl via lldb-commits]

Author: Felipe de Azevedo Piovezan
Date: 2023-07-21T16:42:16-07:00
New Revision: f09f0a6b10765f0993255e52a62268472f586830

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

LOG: [lldb] Consider OP_addrx in DWARFExpression::Update_DW_OP_addr

This rewrites DW_OP_addrx inside DWARFExpression as an DW_OP_addr so
that we can update addresses that are originally located in the
debug_addr section.

The full discussion behind this can be found in
https://discourse.llvm.org/t/dwarfexpression-and-dw-op-addrx/71627/12, but a
summary follows.

When SymbolFileDWARF::ParseVariableDIE creates DWARFExpressions for
variables whose location is an OP_addr, it knows how to remap
addresses appropriately in the DebugMap case. It then calls
DWARFExpression::Update_DW_OP_addr, which updates the value associated
with OP_addr.

However, when we have an OP_addrx, the update function does
nothing. This makes sense, as the DWARFExpression does not have a
mutable view of the debug_addr section. In non-DebugMap flows this is
not an issue, as the debug_addr contains the correct addresses of
variables. In DebugMap flows, this is problematic because the work
done by RelinkOSOAddress is lost. By updating the OP to OP_addr, we
can also update the address as required,

We also explored the alternative of relinking the debug_addr section
when we are initializing OSOs (InitOSO). However, this creates an
inconsistent story for users of
DWARFExpression::GetLocation_DW_OP_addr. This function returns an
address without telling callers whether that address came from an
OP_addr or an OP_addrx. If we preemptively relink OP_addrx results
without doing the same for OP_addr results, then callers can’t know
whether the address they got was an executable address or an object
file address. In other words, they can’t know whether they need to
call LinkOSOFileAddress on those results or not.

This patch addresses the majority of test failures when enabling DWARF
5 for MachO (over 200 test failures).

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

Added: 


Modified: 
lldb/source/Expression/DWARFExpression.cpp
lldb/unittests/Expression/DWARFExpressionTest.cpp

Removed: 




diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index b829a6f7c86477..93fcf0579be0b1 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -408,13 +408,33 @@ bool DWARFExpression::Update_DW_OP_addr(const DWARFUnit 
*dwarf_cu,
   // the heap data so "m_data" will now correctly manage the heap data.
   m_data.SetData(encoder.GetDataBuffer());
   return true;
-} else {
-  const offset_t op_arg_size =
-  GetOpcodeDataSize(m_data, offset, op, dwarf_cu);
-  if (op_arg_size == LLDB_INVALID_OFFSET)
-break;
-  offset += op_arg_size;
 }
+if (op == DW_OP_addrx) {
+  // Replace DW_OP_addrx with DW_OP_addr, since we can't modify the
+  // read-only debug_addr table.
+  // Subtract one to account for the opcode.
+  llvm::ArrayRef data_before_op = m_data.GetData().take_front(offset - 1);
+
+  // Read the addrx index to determine how many bytes it needs.
+  const lldb::offset_t old_offset = offset;
+  m_data.GetULEB128(&offset);
+  if (old_offset == offset)
+return false;
+  llvm::ArrayRef data_after_op = m_data.GetData().drop_front(offset);
+
+  DataEncoder encoder(m_data.GetByteOrder(), m_data.GetAddressByteSize());
+  encoder.AppendData(data_before_op);
+  encoder.AppendU8(DW_OP_addr);
+  encoder.AppendAddress(file_addr);
+  encoder.AppendData(data_after_op);
+  m_data.SetData(encoder.GetDataBuffer());
+  return true;
+}
+const offset_t op_arg_size =
+GetOpcodeDataSize(m_data, offset, op, dwarf_cu);
+if (op_arg_size == LLDB_INVALID_OFFSET)
+  break;
+offset += op_arg_size;
   }
   return false;
 }

diff  --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp 
b/lldb/unittests/Expression/DWARFExpressionTest.cpp
index e35f35b369ea1e..b8b5b39422a4f6 100644
--- a/lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -522,6 +522,11 @@ TEST_F(DWARFExpressionMockProcessTest, 
WASM_DW_OP_addr_index) {
   ASSERT_TRUE(evaluate(expr, status, result)) << status.ToError();
   ASSERT_EQ(result.GetValueType(), Value::ValueType::LoadAddress);
   ASSERT_EQ(result.GetScalar().UInt(), 0x5678u);
+
+  ASSERT_TRUE(expr.Update_DW_OP_addr(dwarf_cu, 0xdeadbeef));
+  ASSERT_TRUE(evaluate(expr, status, result)) << status.ToError();
+  ASSERT_EQ(result.GetValueType(), Value::ValueType::LoadAddress);
+  ASSERT_EQ(result.GetScalar().UInt(), 0xdeadbeefu);
 }
 
 class CustomSymbo

[Lldb-commits] [PATCH] D155198: [lldb] Consider OP_addrx in DWARFExpression::Update_DW_OP_addr

[Adrian Prantl via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf09f0a6b1076: [lldb] Consider OP_addrx in 
DWARFExpression::Update_DW_OP_addr (authored by fdeazeve, committed by aprantl).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155198

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/unittests/Expression/DWARFExpressionTest.cpp


Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -522,6 +522,11 @@
   ASSERT_TRUE(evaluate(expr, status, result)) << status.ToError();
   ASSERT_EQ(result.GetValueType(), Value::ValueType::LoadAddress);
   ASSERT_EQ(result.GetScalar().UInt(), 0x5678u);
+
+  ASSERT_TRUE(expr.Update_DW_OP_addr(dwarf_cu, 0xdeadbeef));
+  ASSERT_TRUE(evaluate(expr, status, result)) << status.ToError();
+  ASSERT_EQ(result.GetValueType(), Value::ValueType::LoadAddress);
+  ASSERT_EQ(result.GetScalar().UInt(), 0xdeadbeefu);
 }
 
 class CustomSymbolFileDWARF : public SymbolFileDWARF {
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -408,13 +408,33 @@
   // the heap data so "m_data" will now correctly manage the heap data.
   m_data.SetData(encoder.GetDataBuffer());
   return true;
-} else {
-  const offset_t op_arg_size =
-  GetOpcodeDataSize(m_data, offset, op, dwarf_cu);
-  if (op_arg_size == LLDB_INVALID_OFFSET)
-break;
-  offset += op_arg_size;
 }
+if (op == DW_OP_addrx) {
+  // Replace DW_OP_addrx with DW_OP_addr, since we can't modify the
+  // read-only debug_addr table.
+  // Subtract one to account for the opcode.
+  llvm::ArrayRef data_before_op = m_data.GetData().take_front(offset - 1);
+
+  // Read the addrx index to determine how many bytes it needs.
+  const lldb::offset_t old_offset = offset;
+  m_data.GetULEB128(&offset);
+  if (old_offset == offset)
+return false;
+  llvm::ArrayRef data_after_op = m_data.GetData().drop_front(offset);
+
+  DataEncoder encoder(m_data.GetByteOrder(), m_data.GetAddressByteSize());
+  encoder.AppendData(data_before_op);
+  encoder.AppendU8(DW_OP_addr);
+  encoder.AppendAddress(file_addr);
+  encoder.AppendData(data_after_op);
+  m_data.SetData(encoder.GetDataBuffer());
+  return true;
+}
+const offset_t op_arg_size =
+GetOpcodeDataSize(m_data, offset, op, dwarf_cu);
+if (op_arg_size == LLDB_INVALID_OFFSET)
+  break;
+offset += op_arg_size;
   }
   return false;
 }


Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -522,6 +522,11 @@
   ASSERT_TRUE(evaluate(expr, status, result)) << status.ToError();
   ASSERT_EQ(result.GetValueType(), Value::ValueType::LoadAddress);
   ASSERT_EQ(result.GetScalar().UInt(), 0x5678u);
+
+  ASSERT_TRUE(expr.Update_DW_OP_addr(dwarf_cu, 0xdeadbeef));
+  ASSERT_TRUE(evaluate(expr, status, result)) << status.ToError();
+  ASSERT_EQ(result.GetValueType(), Value::ValueType::LoadAddress);
+  ASSERT_EQ(result.GetScalar().UInt(), 0xdeadbeefu);
 }
 
 class CustomSymbolFileDWARF : public SymbolFileDWARF {
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -408,13 +408,33 @@
   // the heap data so "m_data" will now correctly manage the heap data.
   m_data.SetData(encoder.GetDataBuffer());
   return true;
-} else {
-  const offset_t op_arg_size =
-  GetOpcodeDataSize(m_data, offset, op, dwarf_cu);
-  if (op_arg_size == LLDB_INVALID_OFFSET)
-break;
-  offset += op_arg_size;
 }
+if (op == DW_OP_addrx) {
+  // Replace DW_OP_addrx with DW_OP_addr, since we can't modify the
+  // read-only debug_addr table.
+  // Subtract one to account for the opcode.
+  llvm::ArrayRef data_before_op = m_data.GetData().take_front(offset - 1);
+
+  // Read the addrx index to determine how many bytes it needs.
+  const lldb::offset_t old_offset = offset;
+  m_data.GetULEB128(&offset);
+  if (old_offset == offset)
+return false;
+  llvm::ArrayRef data_after_op = m_data.GetData().drop_front(offset);
+
+  DataEncoder encoder(m_data.GetByteOrder(), m_data.GetAddressByteSize());
+  encoder.AppendData(data_before_op);
+  encoder.AppendU8(DW_OP_addr);
+  encoder.AppendAddress(file_addr);
+   

[Lldb-commits] [PATCH] D155161: [lldb] Convert script native types to StructuredData counterpart

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 543122.
mib marked 3 inline comments as done.
mib added a comment.

Address @JDevlieghere last comments.

- Use `unique_ptr` instead of `shared_ptr` for PIMPL
- Add missing new line after instrumentation macros
- Move `ScriptObject` from `Utility` to `Interpreter`


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

https://reviews.llvm.org/D155161

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBScriptObjectExtensions.i
  lldb/bindings/interfaces.swig
  lldb/bindings/python/python-typemaps.swig
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBProcess.h
  lldb/include/lldb/API/SBScriptObject.h
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Interpreter/ScriptObject.h
  lldb/include/lldb/lldb-types.h
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBScriptObject.cpp
  lldb/source/API/SBStructuredData.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py

Index: lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
===
--- lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -76,6 +76,40 @@
 # Tests for array data type
 self.array_struct_test(dict_struct)
 
+s.Clear()
+self.assertSuccess(example.GetAsJSON(s))
+py_obj = json.loads(s.GetData())
+self.assertTrue(py_obj)
+self.assertIn("key_dict", py_obj)
+
+py_dict = py_obj["key_dict"]
+self.assertEqual(py_dict["key_string"], "STRING")
+self.assertEqual(py_dict["key_uint"], 0x)
+self.assertEqual(py_dict["key_sint"], -42)
+self.assertEqual(py_dict["key_float"], 2.99)
+self.assertEqual(py_dict["key_bool"], True)
+self.assertEqual(py_dict["key_array"], ["23", "arr"])
+
+class MyRandomClass:
+payload = "foo"
+
+py_dict["key_generic"] = MyRandomClass()
+
+stp = lldb.SBScriptObject(py_dict, lldb.eScriptLanguagePython)
+self.assertEqual(stp.ptr, py_dict)
+
+sd = lldb.SBStructuredData(stp, self.dbg)
+self.assertTrue(sd.IsValid())
+self.assertEqual(sd.GetSize(), len(py_dict))
+
+generic_sd = sd.GetValueForKey("key_generic")
+self.assertTrue(generic_sd.IsValid())
+self.assertEqual(generic_sd.GetType(), lldb.eStructuredDataTypeGeneric)
+
+my_random_class = generic_sd.GetGenericValue()
+self.assertTrue(my_random_class)
+self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+
 def invalid_struct_test(self, example):
 invalid_struct = lldb.SBStructuredData()
 invalid_struct = example.GetValueForKey("invalid_key")
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -83,6 +83,9 @@
std::string &error_str,
lldb::ThreadPlanSP thread_plan) override;
 
+  StructuredData::ObjectSP
+  CreateStructuredDataFromScriptObject(ScriptObject obj) override;
+
   bool ScriptedThreadPlanExplainsStop(StructuredData::ObjectSP implementor_sp,
   Event *event,
   bool &script_error) override;
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1519,6 +1519,17 @@
   return std::make_unique(*this);
 }
 
+StructuredData::ObjectSP
+ScriptInterpreterPythonImpl::CreateStructuredDataFromScriptObject(
+ScriptObject obj) {
+  void *ptr = const_cast(obj.GetPointer());
+  PythonObject py_obj(PyRefType::Borrowed, static_cast(ptr));
+  if (!py_obj.IsValid() || py_obj.IsNone())
+return {};
+  Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
+  return py_obj.CreateStructuredObject();
+}
+
 StructuredData::GenericSP
 ScriptInterpreterPythonImpl::OSPlugin_CreatePluginObject(
 const char *class_name, lldb::ProcessSP process_sp) {
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
==

[Lldb-commits] [PATCH] D155161: [lldb] Convert script native types to StructuredData counterpart

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Thanks for bearing with me. LGTM.


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

https://reviews.llvm.org/D155161

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

[Lldb-commits] [lldb] 57bd882 - [lldb] Convert script native types to StructuredData counterpart

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2023-07-21T18:47:46-07:00
New Revision: 57bd882343f8e4cca598b6ad47da93476cffb987

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

LOG: [lldb] Convert script native types to StructuredData counterpart

This patch adds the ability to pass native types from the script
interpreter to methods that use a {SB,}StructuredData argument.

To do so, this patch changes the `ScriptedObject` struture that holds
the pointer to the script object as well as the originating script
interpreter language. It also exposes that to the SB API via a new class
called `SBScriptObject`.

This structure allows the debugger to parse the script object and
convert it to a StructuredData object. If the type is not compatible
with the StructuredData types, we will store its pointer in a
`StructuredData::Generic` object.

This patch also adds some SWIG typemaps that checks the input argument to
ensure it's either an SBStructuredData object, in which case it just
passes it throught, or a python object that is NOT another SB type, to
provide some guardrails for the user.

rdar://111467140

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

Signed-off-by: Med Ismail Bennani 

Added: 
lldb/bindings/interface/SBScriptObjectExtensions.i
lldb/include/lldb/API/SBScriptObject.h
lldb/include/lldb/Interpreter/ScriptObject.h
lldb/source/API/SBScriptObject.cpp

Modified: 
lldb/bindings/headers.swig
lldb/bindings/interfaces.swig
lldb/bindings/python/python-typemaps.swig
lldb/include/lldb/API/SBDebugger.h
lldb/include/lldb/API/SBDefines.h
lldb/include/lldb/API/SBProcess.h
lldb/include/lldb/API/SBStructuredData.h
lldb/include/lldb/Core/StructuredDataImpl.h
lldb/include/lldb/Interpreter/ScriptInterpreter.h
lldb/include/lldb/lldb-types.h
lldb/source/API/CMakeLists.txt
lldb/source/API/SBProcess.cpp
lldb/source/API/SBStructuredData.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py

Removed: 




diff  --git a/lldb/bindings/headers.swig b/lldb/bindings/headers.swig
index 3c2cd85f504da9..eabf4e12241600 100644
--- a/lldb/bindings/headers.swig
+++ b/lldb/bindings/headers.swig
@@ -49,6 +49,7 @@
 #include "lldb/API/SBQueue.h"
 #include "lldb/API/SBQueueItem.h"
 #include "lldb/API/SBReproducer.h"
+#include "lldb/API/SBScriptObject.h"
 #include "lldb/API/SBSection.h"
 #include "lldb/API/SBSourceManager.h"
 #include "lldb/API/SBStream.h"

diff  --git a/lldb/bindings/interface/SBScriptObjectExtensions.i 
b/lldb/bindings/interface/SBScriptObjectExtensions.i
new file mode 100644
index 00..279854c3d73ccf
--- /dev/null
+++ b/lldb/bindings/interface/SBScriptObjectExtensions.i
@@ -0,0 +1,8 @@
+%extend lldb::SBScriptObject {
+#ifdef SWIGPYTHON
+%pythoncode %{
+ptr = property(GetPointer, None, doc='''A read only property that 
returns the underlying script object.''')
+lang = property(GetLanguage, None, doc='''A read only property that 
returns the script language associated with with this script object.''')
+%}
+#endif
+}

diff  --git a/lldb/bindings/interfaces.swig b/lldb/bindings/interfaces.swig
index c77523834de940..db8eda06464c1a 100644
--- a/lldb/bindings/interfaces.swig
+++ b/lldb/bindings/interfaces.swig
@@ -124,6 +124,7 @@
 %include "lldb/API/SBQueue.h"
 %include "lldb/API/SBQueueItem.h"
 %include "lldb/API/SBReproducer.h"
+%include "lldb/API/SBScriptObject.h"
 %include "lldb/API/SBSection.h"
 %include "lldb/API/SBSourceManager.h"
 %include "lldb/API/SBStream.h"
@@ -176,6 +177,7 @@
 %include "./interface/SBModuleExtensions.i"
 %include "./interface/SBModuleSpecExtensions.i"
 %include "./interface/SBProcessExtensions.i"
+%include "./interface/SBScriptObjectExtensions.i"
 %include "./interface/SBSectionExtensions.i"
 %include "./interface/SBStreamExtensions.i"
 %include "./interface/SBStringListExtensions.i"

diff  --git a/lldb/bindings/python/python-typemaps.swig 
b/lldb/bindings/python/python-typemaps.swig
index f3d0c918819b10..7660e0282c8fcf 100644
--- a/lldb/bindings/python/python-typemaps.swig
+++ b/lldb/bindings/python/python-typemaps.swig
@@ -59,9 +59,78 @@ AND call SWIG_fail at the same time, because it will result 
in a double free.
   free((char *) $1);
 }
 
-%typemap(out) lldb::ScriptedObject {
+%typecheck(SWIG_TYPECHECK_POINTER) lldb::ScriptObjectPtr {
+  PythonObject obj(PyRefType::Borrowed, $input);
+  if (!obj.IsValid()) {
+PyErr_Clear();
+$1 = 0;
+  } else {
+$1 = 1;
+  }
+}
+
+%typemap(in) lldb::ScriptObjectPtr {
+  if ($input == Py_No

[Lldb-commits] [PATCH] D155161: [lldb] Convert script native types to StructuredData counterpart

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG57bd882343f8: [lldb] Convert script native types to 
StructuredData counterpart (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155161

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBScriptObjectExtensions.i
  lldb/bindings/interfaces.swig
  lldb/bindings/python/python-typemaps.swig
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBProcess.h
  lldb/include/lldb/API/SBScriptObject.h
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Interpreter/ScriptObject.h
  lldb/include/lldb/lldb-types.h
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBScriptObject.cpp
  lldb/source/API/SBStructuredData.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py

Index: lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
===
--- lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -76,6 +76,40 @@
 # Tests for array data type
 self.array_struct_test(dict_struct)
 
+s.Clear()
+self.assertSuccess(example.GetAsJSON(s))
+py_obj = json.loads(s.GetData())
+self.assertTrue(py_obj)
+self.assertIn("key_dict", py_obj)
+
+py_dict = py_obj["key_dict"]
+self.assertEqual(py_dict["key_string"], "STRING")
+self.assertEqual(py_dict["key_uint"], 0x)
+self.assertEqual(py_dict["key_sint"], -42)
+self.assertEqual(py_dict["key_float"], 2.99)
+self.assertEqual(py_dict["key_bool"], True)
+self.assertEqual(py_dict["key_array"], ["23", "arr"])
+
+class MyRandomClass:
+payload = "foo"
+
+py_dict["key_generic"] = MyRandomClass()
+
+stp = lldb.SBScriptObject(py_dict, lldb.eScriptLanguagePython)
+self.assertEqual(stp.ptr, py_dict)
+
+sd = lldb.SBStructuredData(stp, self.dbg)
+self.assertTrue(sd.IsValid())
+self.assertEqual(sd.GetSize(), len(py_dict))
+
+generic_sd = sd.GetValueForKey("key_generic")
+self.assertTrue(generic_sd.IsValid())
+self.assertEqual(generic_sd.GetType(), lldb.eStructuredDataTypeGeneric)
+
+my_random_class = generic_sd.GetGenericValue()
+self.assertTrue(my_random_class)
+self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+
 def invalid_struct_test(self, example):
 invalid_struct = lldb.SBStructuredData()
 invalid_struct = example.GetValueForKey("invalid_key")
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -83,6 +83,9 @@
std::string &error_str,
lldb::ThreadPlanSP thread_plan) override;
 
+  StructuredData::ObjectSP
+  CreateStructuredDataFromScriptObject(ScriptObject obj) override;
+
   bool ScriptedThreadPlanExplainsStop(StructuredData::ObjectSP implementor_sp,
   Event *event,
   bool &script_error) override;
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1519,6 +1519,17 @@
   return std::make_unique(*this);
 }
 
+StructuredData::ObjectSP
+ScriptInterpreterPythonImpl::CreateStructuredDataFromScriptObject(
+ScriptObject obj) {
+  void *ptr = const_cast(obj.GetPointer());
+  PythonObject py_obj(PyRefType::Borrowed, static_cast(ptr));
+  if (!py_obj.IsValid() || py_obj.IsNone())
+return {};
+  Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
+  return py_obj.CreateStructuredObject();
+}
+
 StructuredData::GenericSP
 ScriptInterpreterPythonImpl::OSPlugin_CreatePluginObject(
 const char *class_name, lldb::ProcessSP process_sp) {
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins