[Lldb-commits] [PATCH] D154705: [lldb][AArch64] Fix flakiness in TestSVEThreadedDynamic

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This test runs to a breakpoint on thread 0. Thread 0 then starts
thread 2 and 3, which both have breakpoints in them.

In https://lab.llvm.org/buildbot/#/builders/96/builds/41674
I think that we managed to do the first check on thread 2 before
thread 3 had started. Therefore "thread continue 3" failed.

So wait for all three to startup before we check their status.

I considered putting a timeout on the while like the wait_for... methods,
but the test itself already has a global timeout. Plus, I'd rather
not be tuning a timeout per piece of hardware this runs on.

99% of the time we will already have 3 threads when the check is done.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154705

Files:
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py


Index: 
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/TestSVEThreadedDynamic.py
+++ 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -125,8 +125,6 @@
 
 process = self.dbg.GetSelectedTarget().GetProcess()
 
-thread1 = process.GetThreadAtIndex(0)
-
 self.expect(
 "thread info 1",
 STOPPED_DUE_TO_BREAKPOINT,
@@ -140,6 +138,10 @@
 
 self.runCmd("process continue", RUN_SUCCEEDED)
 
+# If we start the checks too quickly, thread 3 may not have started.
+while (process.GetNumThreads() < 3):
+pass
+
 for idx in range(1, process.GetNumThreads()):
 thread = process.GetThreadAtIndex(idx)
 if thread.GetStopReason() != lldb.eStopReasonBreakpoint:


Index: 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/TestSVEThreadedDynamic.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -125,8 +125,6 @@
 
 process = self.dbg.GetSelectedTarget().GetProcess()
 
-thread1 = process.GetThreadAtIndex(0)
-
 self.expect(
 "thread info 1",
 STOPPED_DUE_TO_BREAKPOINT,
@@ -140,6 +138,10 @@
 
 self.runCmd("process continue", RUN_SUCCEEDED)
 
+# If we start the checks too quickly, thread 3 may not have started.
+while (process.GetNumThreads() < 3):
+pass
+
 for idx in range(1, process.GetNumThreads()):
 thread = process.GetThreadAtIndex(idx)
 if thread.GetStopReason() != lldb.eStopReasonBreakpoint:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154705: [lldb][AArch64] Fix flakiness in TestSVEThreadedDynamic

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

My brain says this is not a good way to ensure a state, but I couldn't think of 
anything that wouldn't have its own race conditions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154705

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

[Lldb-commits] [PATCH] D154757: [lldb][NFCI] TestEmulation should take a Stream ref

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a reviewer: DavidSpickett.
DavidSpickett accepted this revision.
DavidSpickett added a comment.
This revision is now accepted and ready to land.

LGTM. Always happy to see never-null pointers get updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154757

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

[Lldb-commits] [PATCH] D154823: [lldb][AArch64] Add test predicate for systems with SME enabled

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

"sme" is just one of many cpuinfo features for SME but it's the
only one we need for testing.

The rest are related to the use of certain instructions and
don't change the register state available.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154823

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1271,6 +1271,9 @@
 def isAArch64SVE(self):
 return self.isAArch64() and "sve" in self.getCPUInfo()
 
+def isAArch64SME(self):
+return self.isAArch64() and "sme" in self.getCPUInfo()
+
 def isAArch64MTE(self):
 return self.isAArch64() and "mte" in self.getCPUInfo()
 


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1271,6 +1271,9 @@
 def isAArch64SVE(self):
 return self.isAArch64() and "sve" in self.getCPUInfo()
 
+def isAArch64SME(self):
+return self.isAArch64() and "sme" in self.getCPUInfo()
+
 def isAArch64MTE(self):
 return self.isAArch64() and "mte" in self.getCPUInfo()
 
___
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 created this revision.
Herald added subscribers: ctetreau, kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added subscribers: lldb-commits, alextsao1999.
Herald added a project: LLDB.

The Scalable Matrix Extension (SME) adds a new Scalable Vector mode
called "streaming SVE mode".

In this mode a lot of things change, but my understanding overall
is that this mode assumes you are not going to move data out of
the vector unit very often or read flags.

Based on "E1.3" of "Arm® Architecture Reference Manual Supplement,
The Scalable Matrix Extension (SME), for Armv9-A".

https://developer.arm.com/documentation/ddi0616/latest/

The important details for debug are that this adds another set
of SVE registers. This set is only active when we are in streaming
mode and is read from a new ptrace regset NT_ARM_SSVE.
We are able to read the header of either mode at all times but
only one will be active and contain register data.

SIMD registers must be read and written via the SVE regset when
in SSVE mode. Writing to them exits streaming mode.

"Note that when SME is present and streaming SVE mode is in use the
FPSIMD subset of registers will be read via NT_ARM_SVE and NT_ARM_SVE
writes will exit streaming mode in the target."

https://kernel.org/doc/html/v6.2/arm64/sve.html

The streaming mode registers do not have different names in the
architecture, so I do not plan to allow users to read or write the
inactive mode's registers. "z0" will always mean "z0" of the active
mode.

(ptrace does allow this but you would be reading invalid state,
and writing switches you into that mode which is probably not what
you want)

I've chosen to have 2 sets of state in the register context.
I did try reusing the same one, but it gets tricky to read SIMD
while in streaming mode.

Existing SVE tests have been updated to check streaming mode and
mode switches. However, we are limited in what we can check given
that state for the other mode is trashed on mode switch.

The only way to know what mode you are in for testing purposes would
be to execute a streaming only, or non-streaming only instruction in
the opposite mode. However, the CPU feature smefa64 actually allows
all non-streaming mode instructions in streaming mode.

This is enabled by default in QEMU emulation and rather than mess
about trying to disable it I'm just going to use the pseduo streaming
control register added in a later patch to make the testing a bit
more robust.

A new test has been added to check SIMD read/write from all the modes.


Repository:
  rG LLVM Github Monorepo

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)

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added subscribers: ctetreau, kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added subscribers: lldb-commits, alextsao1999.
Herald added a project: LLDB.

Software can tell if it is in streaming SVE mode by checking
the Streaming Vector Control Register (SVCR).

"E3.1.9 SVCR, Streaming Vector Control Register" in
"Arm® Architecture Reference Manual Supplement, The Scalable Matrix
Extension (SME), for Armv9-A"

This is especially useful for debug because the names of the
SVE registers are the same betweeen non-streaming and streaming mode.

The Linux Kernel chose to not put this register behind ptrace,
and it can be read from EL0. However, this would mean running code
in process to read it. That can be done but we already know all
the information just from ptrace.

So this is a pseudo register that matches the architectural
content, and it is read only for now until all bits are implemented.
The name is just "svcr", which aligns with GDB's proposed naming.

It is in a new dynamic register set named for the SME exention.
There will also be a streaming vector length pseduo register here
in future.

The register contains two bits:
0 : Whether streaming SVE mode is enabled.
1 : Whether the array storage is enabled.

This patch only adds bit 0, bit 1 will be added in a later patch
after the array storage (ZA) is supported. The register is read
only and may be made writeable in a later patch.

An existing test has been updated to check the expected SVCR value.
There's not much point adding a dedicated test given that the content
is just whatever m_sme_state is. If lldb gets it wrong, svcr will be
wrong too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154927

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_static_config/TestSVERegisters.py

Index: 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/TestSVERegisters.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
@@ -22,7 +22,12 @@
 reg_value.GetByteSize(), expected, 'Verify "%s" == %i' % (name, expected)
 )
 
-def check_sve_regs_read(self, z_reg_size):
+def check_sve_regs_read(self, z_reg_size, expected_mode):
+if self.isAArch64SME():
+expected_value = "1" if expected_mode == Mode.SSVE else "0"
+self.expect("register read svcr", substrs=[
+"0x000" + expected_value])
+
 p_reg_size = int(z_reg_size / 8)
 
 for i in range(32):
@@ -162,7 +167,7 @@
 
 vg_reg_value = sve_registers.GetChildMemberWithName("vg").GetValueAsUnsigned()
 z_reg_size = vg_reg_value * 8
-self.check_sve_regs_read(z_reg_size)
+self.check_sve_regs_read(z_reg_size, start_mode)
 
 # Evaluate simple expression and print function expr_eval_func address.
 self.expect("expression expr_eval_func", substrs=["= 0x"])
@@ -174,7 +179,7 @@
 
 # We called a jitted function above which must not have changed SVE
 # vector length or register values.
-self.check_sve_regs_read(z_reg_size)
+self.check_sve_regs_read(z_reg_size, start_mode)
 
 self.check_sve_regs_read_after_write(z_reg_size)
 
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -106,6 +106,8 @@
 
   void AddRegSetTLS();
 
+  void AddRegSetSME();
+
   uint32_t ConfigureVectorLength(uint32_t sve_vq);
 
   bool VectorSizeIsValid(uint32_t vq) {
@@ -127,6 +129,7 @@
   bool IsPAuthReg(unsigned reg) const;
   bool IsMTEReg(unsigned reg) const;
   bool IsTLSReg(unsigned reg) const;
+  bool IsSMEReg(unsigned reg) const;
 
   uint32_t GetRegNumSVEZ0() const;
   uint32_t GetRegNumSVEFFR() const;
@@ -136,6 +139,7 @@
   uint32_t GetPAuthOffset() const;
   uint32_t GetMTEOffset() const;
   uint32_t GetTLSOffset() const;
+  uint32_t GetSMEOffset() const;
 
 private:
   typedef std::map>
@@ -163,6 +167,7 @@
   std::vector pauth_regnum_collection;
   std::vector m_mte_regnum_collection;
   std::vector m_tls_regnum_collection;
+  std::vector m_sme_regnum_collection;
 };
 
 #endif
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This changes the TLS regset to not only be dynamic in that it could
exist or not (though it always does) but also of a dynamic size.

If SME is present then the regset is 16 bytes and contains both tpidr
and tpidr2.

Testing is the same as tpidr. Write from assembly, read from lldb and
vice versa since we have no way to predict what its value should be
by just running a program.


Repository:
  rG LLVM Github Monorepo

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,57 @@
+#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[]) {
+  uint64_t (*getter)(void) = get_tpidr;
+  void (*setter)(uint64_t) = set_tpidr;
+
+  // Expect just the register number.
+  if (argc != 2)
+return 1;
+
+  switch (argv[1][0]) {
+  case '1':
+break;
+  case '2':
+getter = get_tpidr2;
+setter = set_tpidr2;
+break;
+  default:
+return 1;
+  }
+
+  uint64_t original = getter();
+  uint64_t pattern = 0x1122334455667788;
+  setter(pattern);
+
+  // Set break point at this line.
+  // lldb will now set its own pattern for us to find.
+
+  uint64_t new_value = getter();
+  volatile bool reg_was_set = new_value == 0x8877665544332211;
+
+  setter(original);
+
+  return 0; // Set break point 2 at this line.
+}
Index: lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
===
--- lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
+++ lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -1,5 +1,5 @@
 """
-Test lldb's ability to read and write the AArch64 TLS register tpidr.
+Test lldb's ability to read and write the AArch64 TLS registers.
 """
 
 import lldb
@@ -8,12 +8,10 @@
 from lldbsuite.test import lldbutil
 
 
-class AArch64LinuxTLSRegister(TestBase):
+class AArch64LinuxTLSRegisters(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
-@skipUnlessArch("aarch64")
-@skipUnlessPlatform(["linux"])
-def test_tls(self):
+def setup(self, register_name):
 self.build()
 self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
@@ -31,6 +29,8 @@
 num_expected_locations=1,
 )
 
+self.runCmd("settings set target.run-args {}".format(
+{"tpidr": 1, "tpidr2": 2}[register_name]))
 self.runCmd("run", RUN_SUCCEEDED)
 
 if self.process().GetState() == lldb.eStateExited:
@@ -42,19 +42,24 @@
 substrs=["stopped", "stop reason = breakpoint"],
 )
 
+def check_tls_reg(self, register_name):
+self.setup(register_name)
+
 # Since we can't predict what the value will be, the program has set
 # a target value for us to find.
 
 regs = self.thread().GetSelectedFrame().GetRegisters()
 tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
 self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
-tpidr = tls_regs.GetChildMemberWithName("tpidr")
-self.assertTrue(tpidr.IsValid(), "No tpidr register found.")
+tls_reg = tls_regs.GetChildMemberWithName(register_name)
+self.assertTrue(tls_reg.IsValid(), "{} register not found.".format(
+register_name))
 
-self.assertEqual(tpidr.GetValueAsUnsigned(), 0x1122334455667788)
+self.assertEqual(tls_reg.GetValueAsUnsigned(), 0x1122334455667788)
 
 # Set our own value fo

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a reviewer: omjavaid.
DavidSpickett added a comment.

Feel free to bombard me with questions about SME, if it's quicker than reading 
the entire spec yourself. It has some quirks to it for sure.


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 added a comment.

> SIMD registers must be read and written via the SVE regset when in SSVE mode. 
> Writing to them exits streaming mode.

In my testing it did actually work if you always used the current mode. I think 
that's an artifact of QEMU or the kernel's implementation choices though, and 
perhaps the data is actually stale in certain circumstances.


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 added a comment.

> You mention a couple of times that when in SSVE mode, we can only write to 
> the SVE registers and doing so forces the processor out of SSVE Mode. We can 
> read the SSVE registers while in SSVE mode though right?

We are always able to read the header for either mode, which tells us the 
vector length among other useful metadata. Register data though...

Maybe I should just enumerate the states.

Write to SVE while in SSVE - switch to SVE mode
Write to SSVE while in SVE - switch to SSVE mode
Write to SVE while in SVE - no change
Write to SSVE while in SSVE - no change

Read SVE while in SSVE - no register data returned
Read SSVE while in SVE - no register data returned
Read SSVE while in SSVE - SSVE registers returned
Read SVE while in SVE - SVE registers returned

Then we have SIMD (v0-31) which is a bit of a wrench in this. We must read SIMD 
via the SVE regset even while SSVE is active, but writing to that same set 
brings us out of SSVE mode (into SIMD mode, I think, but it could just be SVE 
mode).

So perhaps you see from that why I don't plan to use the mode switching this 
way in lldb.

I will in follow on patches report the current mode via a pseudo control 
register, and report the streaming vector length again with a pseudo. To 
resolve the ambiguity of naming the registers the same thing.


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 added a comment.

> Then we have SIMD (v0-31) which is a bit of a wrench in this. We must read 
> SIMD via the SVE regset even while SSVE is active, but writing to that same 
> set brings us out of SSVE mode (into SIMD mode, I think, but it could just be 
> SVE mode).

In my testing it did work to just use the active mode instead but this is one 
of those things that is likely unintentional or a result of QEMU's 
implementation choices.


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] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added subscribers: ctetreau, kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Reading the SVE registers of streaming mode from non-streaming mode,
and vice versa, returns invalid data. As their state is reset each
time you switch between them.

However, the vector length is distinct between the two modes.

The existing register "vg" will always report the vector length for
the current mode and this change adds "svg" which will always return
the streaming vector length.

non-streaming mode: vg  = non-streaming vector length

  svg = streaming vector length
  streaming mode: vg  = streaming vector length
  svg = streaming vector length

The content of svg is read from the NT_ARM_SSVE header, even if
we are in non-streaming mode. Which we are allowed to do
(the result is just missing the register data in this situation).

It is read only for the moment. It may be made writeable in future
patches. It has been added to the SME register set and I've converted
that into a struct for easier handling.

The SVE dynamic size test has been updated to check the expected
svg values as it is already setup to break just after a mode switch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155269

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/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py

Index: 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/TestSVEThreadedDynamic.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -96,6 +96,12 @@
 
 self.expect("register read ffr", substrs=[p_regs_value])
 
+def build_for_mode(self, mode):
+cflags = "-march=armv8-a+sve -lpthread"
+if mode == Mode.SSVE:
+cflags += " -DUSE_SSVE"
+self.build(dictionary={"CFLAGS_EXTRAS": cflags})
+
 def run_sve_test(self, mode):
 if (mode == Mode.SVE) and not self.isAArch64SVE():
 self.skipTest("SVE registers must be supported.")
@@ -103,12 +109,8 @@
 if (mode == Mode.SSVE) and not self.isAArch64SME():
 self.skipTest("Streaming SVE registers must be supported.")
 
-cflags = "-march=armv8-a+sve -lpthread"
-if mode == Mode.SSVE:
-cflags += " -DUSE_SSVE"
-self.build(dictionary={"CFLAGS_EXTRAS": cflags})
+self.build_for_mode(mode)
 
-self.build()
 supported_vg = self.get_supported_vg()
 
 if not (2 in supported_vg and 4 in supported_vg):
@@ -198,3 +200,62 @@
 def test_ssve_registers_dynamic_config(self):
 """Test AArch64 SSVE registers multi-threaded dynamic resize."""
 self.run_sve_test(Mode.SSVE)
+
+def setup_svg_test(self, mode):
+# Even when running in SVE mode, we need access to SVG for these tests.
+if not self.isAArch64SME():
+self.skipTest("Streaming SVE registers must be supported.")
+
+self.build_for_mode(mode)
+
+supported_vg = self.get_supported_vg()
+
+main_thread_stop_line = line_number("main.c", "// Break in main thread")
+lldbutil.run_break_set_by_file_and_line(self, "main.c", main_thread_stop_line)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+process = self.dbg.GetSelectedTarget().GetProcess()
+
+self.expect(
+"thread info 1",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint"],
+)
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+
+return process, supported_vg
+
+def read_vg(self, process):
+registerSets = process.GetThreadAtIndex(0).GetFrameAtIndex(0).GetRegisters()
+sve_registers = registerSets.GetFirstValueByName("Scalable Vector Extension Registers")
+return sve_registers.GetChildMemberWithName("vg").GetValueAsUnsigned()
+
+def read_svg(self, process):
+registerSets = process.GetThreadAtIndex(0).GetFrameAtIndex(0).GetRegisters()
+sve_registers = registerSets.GetFirstValueByName("Scalable Matrix Extension Registers")
+return sve_registers.GetChildMemberWithName("svg").GetValueAsUnsigned()
+
+def do_svg_test(self, process, vgs, expected_svgs):
+for vg, svg in zip(vgs, expected_svgs):
+self.runCmd("register write vg {}".format(vg))
+self.assertEqual(svg, se

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: 
lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py:73
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])

omjavaid wrote:
> These three tests have a lot of commonalities may be merge them into one 
> testing the whole logic. Doesn't look like we are getting much out of 
> emitting three tests from this fairly basic test class.
The tradeoff is execution time vs. a HWCAP check in the program file and a 
bunch of ifs in Python.

Let me see what I can do, but I'm leaning toward the implementation complexity 
outweighing the performance gained.



Comment at: 
lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py:90
+if self.isAArch64SME():
+self.skipTest("SME must not be enabled.")
+

And speaking of words, let me change this skip reason to "not be present".



Comment at: lldb/test/API/linux/aarch64/tls_registers/main.c:37
+  case '2':
+getter = get_tpidr2;
+setter = set_tpidr2;

omjavaid wrote:
> It would be interesting to test reading/writing tpidr2 when SME is not 
> enabled.
Not enabled, or not present? (I admit, these two words are used interchangeably 
in places)

Not enabled is actually the state here, as there's no SMTART used here. 
Architecture wise, I don't see anything to indicate it makes a difference if 
SME is active or not.

Not present is covered by test_tpidr2_no_sme.


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] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

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

> As you say above, we can show svg when in non-streaming mode but we can't 
> show vg when in streaming mode. Should we only show a single vg for the 
> currently-available registers?

I will admit I don't have practical experience to justify adding svg, I'm just 
guessing it would be nice to know what vector length you're about to switch 
into. That argument plays just as well for streaming -> non-streaming switches 
though.

The problem with that is we don't have an obvious name (or place) to show the 
non-streaming vg. For `svg`, we're adding the new register set to hold `svcr` 
anyway (which is essential because it's the only way to know the active mode). 
The architecture doesn't assign a new name to the non-streaming mode, so you 
could call it "svevg" and "ssvevg" but that's no help to the average person who 
hasn't (and shouldn't have to) read the manual.

Given that this isn't an essential feature, I'll put this on the backburner. At 
least until after 17, once I have more experience writing SME code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

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

[Lldb-commits] [PATCH] D154705: [lldb][AArch64] Fix flakiness in TestSVEThreadedDynamic

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfdf36c3d4b46: [lldb][AArch64] Fix flakiness in 
TestSVEThreadedDynamic (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154705

Files:
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py


Index: 
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/TestSVEThreadedDynamic.py
+++ 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -125,8 +125,6 @@
 
 process = self.dbg.GetSelectedTarget().GetProcess()
 
-thread1 = process.GetThreadAtIndex(0)
-
 self.expect(
 "thread info 1",
 STOPPED_DUE_TO_BREAKPOINT,
@@ -140,6 +138,10 @@
 
 self.runCmd("process continue", RUN_SUCCEEDED)
 
+# If we start the checks too quickly, thread 3 may not have started.
+while (process.GetNumThreads() < 3):
+pass
+
 for idx in range(1, process.GetNumThreads()):
 thread = process.GetThreadAtIndex(idx)
 if thread.GetStopReason() != lldb.eStopReasonBreakpoint:


Index: 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/TestSVEThreadedDynamic.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -125,8 +125,6 @@
 
 process = self.dbg.GetSelectedTarget().GetProcess()
 
-thread1 = process.GetThreadAtIndex(0)
-
 self.expect(
 "thread info 1",
 STOPPED_DUE_TO_BREAKPOINT,
@@ -140,6 +138,10 @@
 
 self.runCmd("process continue", RUN_SUCCEEDED)
 
+# If we start the checks too quickly, thread 3 may not have started.
+while (process.GetNumThreads() < 3):
+pass
+
 for idx in range(1, process.GetNumThreads()):
 thread = process.GetThreadAtIndex(idx)
 if thread.GetStopReason() != lldb.eStopReasonBreakpoint:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154823: [lldb][AArch64] Add test predicate for systems with SME enabled

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3ebd6f65cb87: [lldb][AArch64] Add test predicate for systems 
with SME enabled (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154823

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1271,6 +1271,9 @@
 def isAArch64SVE(self):
 return self.isAArch64() and "sve" in self.getCPUInfo()
 
+def isAArch64SME(self):
+return self.isAArch64() and "sme" in self.getCPUInfo()
+
 def isAArch64MTE(self):
 return self.isAArch64() and "mte" in self.getCPUInfo()
 


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1271,6 +1271,9 @@
 def isAArch64SVE(self):
 return self.isAArch64() and "sve" in self.getCPUInfo()
 
+def isAArch64SME(self):
+return self.isAArch64() and "sme" in self.getCPUInfo()
+
 def isAArch64MTE(self):
 return self.isAArch64() and "mte" in self.getCPUInfo()
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Could you offer higher abstractions? Show me the current SVME vector length? 
> Show me the current SVME mode?

Adding it to `process status` is along those lines, we have stuff like the 
number of addressable bits there right now. Overall I prefer the registers 
route just for visibility but we'll see what the early users find.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

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

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> I would never question giving low-level access to the registers.

Well in your defense, both `svg` and `svcr` will actually be pseudo registers. 
So the user isn't getting access to the "real" ones either way, we're emulating 
the behaviour with ptrace commands.

> As you mentioned less experienced users could accidentally switch between the 
> modes with knowing.

If we follow the kernel to the letter you can also mode switch by writing 
floating point registers while in streaming mode. That currently doesn't happen 
due to the way we model `v` registers as subsets of `z` but I might have to 
change that and if I do, that's another potential pitfall.

Ideally we would have as few routes to mode switch via the debugger as 
possible. Writing to the streaming vector control register is the single route 
I'd support given the choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

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

[Lldb-commits] [PATCH] D155256: Add fs_base/gs_base support for Linux

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Caveat: I have 0 prior knowledge about these registers.

What's the testing story here? I see one for fs_base on a live process but none 
for gs_base and neither for core files. If one test can hit all the code paths 
those would hit, then fine, but otherwise this needs more coverage.

Side note: please put a tag in the commit title like `[lldb][x86] ...`. It 
helps people like me who skim through the logs looking for build problems later.




Comment at: 
lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64_with_base.h:71
+ {kind1, kind2, kind3, kind4,  
\
+  x86_64_with_base::lldb_##reg },  
 \
+  nullptr, nullptr, nullptr,   
\

Has this file been put through clang-format? These lines look off.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155256

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

[Lldb-commits] [PATCH] D155107: Add support for llvm::MCInstPrinter::setPrintBranchImmAsAddress

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Also you can test it as long as you put the right skipif annotations on it as 
it'll be architecture specific. Or use a corefile and just check that the 
backend for that architecture is enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155107

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

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I think in https://reviews.llvm.org/D154926, 
`lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py`
 addresses this. If what you mean is you are stopped in streaming mode, you 
evaluate an expression that may call a function which takes you into another 
mode.

If not, give me an example and I'll try to test it. This is the first I'm 
hearing of QSaveRegisterState / QRestoreRegisterState.

> g/G are probably going to fetch / write all the floating point registers and 
> reset the mode if you did a function call while in streaming mode?

We'd have to order them carefully I expect. Or say something like if we're 
restoring floating point and SVE registers, just ignore the floating point 
because we're about to supersede it.

I am not 100% sure that one cannot implement streaming SVE as a completely 
separate register state, I will be checking that today. If you can then it will 
complicate things in theory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

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

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> As a simplification of all of this, and to avoid using g/G, we added 
> QSaveRegisterState which tells the stub (debugserver etc) to save the current 
> register context, and then after the inferior function call has completed, 
> QRestoreRegisterState to restore them all.

Looks like we do support that in lldb-server, I just hadn't come across it 
because I've been down at the native process level. I've updated 
`ReadAllRegisterValues` down there so it will restore to whatever the saved 
mode was.

> In the process of restoring / writing the registers, I expect we will try to 
> write the floating point register contents into the process which would drop 
> it out of SSVE mode?

Right, yes it would if we follow this statement from the kernel docs:

  Note that when SME is present and streaming SVE mode is in use the FPSIMD 
subset of registers will be read via NT_ARM_SVE and NT_ARM_SVE writes will exit 
streaming mode in the target.

I am talking to our kernel folks to understand the background to that. I 
suspect that it may be the case that for example, writing to the bottom 128 
bits of streaming mode z0 may not be reflected in the SIMD unit's v0. Or at 
least, one could build a core that acted that way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

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

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> I suspect that it may be the case that for example, writing to the bottom 128 
> bits of streaming mode z0 may not be reflected in the SIMD unit's v0. Or at 
> least, one could build a core that acted that way.

But the user would be very confused by this given that if you are stopped here 
in streaming mode:

  mov v0.d[0] x0

That instruction would actually see the bottom 128 bits of streaming z0, even 
if elsewhere there is another, inactive v0 register in the core. If the user 
then does `register write v0 {}` I doubt they would expect it to mode 
switch and write to a whole different v0, it should update z0.

So even if on a hardware level this configuration is possible, I don't think 
it's good to have the debugger act this way. Better that we think of this from 
the perspective of a running instruction, what would it see and therefore what 
would the user expect to happen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

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

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> I am talking to our kernel folks to understand the background to that.

The result is that yes cores an implement it as separate state but as mentioned 
here, taking that into account in lldb would be rather confusing in 99% of 
situations. If we simply want to read what instructions in the current context 
will see, using the bottom 128 bits of the Z registers is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

___
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 541520.
DavidSpickett added a comment.

Turns out I was misinterpreting this setence from the kernel docs:

  Note that when SME is present and streaming SVE mode is in use the FPSIMD 
subset of registers will be read via NT_ARM_SVE and NT_ARM_SVE writes will exit 
streaming mode in the target.

https://kernel.org/doc/html/v6.2/arm64/sve.html

I read this as "should be read" not, "will be read". The intent of the statement
is to make you aware that the register sets are connected in that one can effect
the other.

However, our strategy of using the bottom part of the Z registers to read the V
registers is still valid as long as we do not want to switch modes, which we 
never
do.

So I've done what Omair suggested and reverted to a single set of state for
the non-streaming and streaming modes. With m_sve_state to tell the difference.


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

[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 541526.
DavidSpickett added a comment.

Address some comments from the previous version.


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 SIMD da

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett marked 3 inline comments as done.
DavidSpickett added inline comments.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:422
+m_sve_state = SVEState::Unknown;
+m_sve_state_data.Invalidate();
+m_ssve_state_data.Invalidate();

omjavaid wrote:
> shouldnt we also invalidate all other dynamic regsets here?
This is not needed anymore as we're no longer going to trigger a mode switch 
here. When we write to FPSIMD, we'll use the bottom 128 bits of the streaming 
mode Z register. Instead of going back to NT_ARM_SVE, which would cause a mode 
switch.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h:116
+
+  SVEStateData m_sve_state_data;
+  SVEStateData m_ssve_state_data;

omjavaid wrote:
> When we are in streaming mode normal state data will be invalid? If yes then 
> can we convert this into a pointer which should be pointing to a valid state 
> data based on current state?
I've reverted to the previous scheme.


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 marked 4 inline comments as done.
DavidSpickett added inline comments.



Comment at: 
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py:1
+"""
+Test that LLDB correctly reads and writes AArch64 SIMD registers in SVE,

I wrote this test when I thought we had to read/write SIMD via NT_ARM_SVE 
during streaming mode, so its main reason for existing has gone now.

However, I think it's worth keeping because no other test checks SIMD 
read/write as literally as this does. And if it turns out I did part of this 
incorrectly, we have tests to refer to.


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 added a comment.

On the off chance anyone was going to try and run this, you'll need a kernel 
that includes 
https://lore.kernel.org/lkml/20230713-arm64-fix-sve-sme-vl-change-v1-3-129dd8611...@kernel.org/T/.
 This fixes a bug found while writing these tests.


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

[David Spickett via Phabricator via lldb-commits]

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

Rebase, fix typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154927

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_static_config/TestSVERegisters.py

Index: 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/TestSVERegisters.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
@@ -22,7 +22,12 @@
 reg_value.GetByteSize(), expected, 'Verify "%s" == %i' % (name, expected)
 )
 
-def check_sve_regs_read(self, z_reg_size):
+def check_sve_regs_read(self, z_reg_size, expected_mode):
+if self.isAArch64SME():
+expected_value = "1" if expected_mode == Mode.SSVE else "0"
+self.expect("register read svcr", substrs=[
+"0x000" + expected_value])
+
 p_reg_size = int(z_reg_size / 8)
 
 for i in range(32):
@@ -162,7 +167,7 @@
 
 vg_reg_value = sve_registers.GetChildMemberWithName("vg").GetValueAsUnsigned()
 z_reg_size = vg_reg_value * 8
-self.check_sve_regs_read(z_reg_size)
+self.check_sve_regs_read(z_reg_size, start_mode)
 
 # Evaluate simple expression and print function expr_eval_func address.
 self.expect("expression expr_eval_func", substrs=["= 0x"])
@@ -174,7 +179,7 @@
 
 # We called a jitted function above which must not have changed SVE
 # vector length or register values.
-self.check_sve_regs_read(z_reg_size)
+self.check_sve_regs_read(z_reg_size, start_mode)
 
 self.check_sve_regs_read_after_write(z_reg_size)
 
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -106,6 +106,8 @@
 
   void AddRegSetTLS();
 
+  void AddRegSetSME();
+
   uint32_t ConfigureVectorLength(uint32_t sve_vq);
 
   bool VectorSizeIsValid(uint32_t vq) {
@@ -127,6 +129,7 @@
   bool IsPAuthReg(unsigned reg) const;
   bool IsMTEReg(unsigned reg) const;
   bool IsTLSReg(unsigned reg) const;
+  bool IsSMEReg(unsigned reg) const;
 
   uint32_t GetRegNumSVEZ0() const;
   uint32_t GetRegNumSVEFFR() const;
@@ -136,6 +139,7 @@
   uint32_t GetPAuthOffset() const;
   uint32_t GetMTEOffset() const;
   uint32_t GetTLSOffset() const;
+  uint32_t GetSMEOffset() const;
 
 private:
   typedef std::map>
@@ -163,6 +167,7 @@
   std::vector pauth_regnum_collection;
   std::vector m_mte_regnum_collection;
   std::vector m_tls_regnum_collection;
+  std::vector m_sme_regnum_collection;
 };
 
 #endif
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -81,6 +81,9 @@
 static lldb_private::RegisterInfo g_register_infos_tls[] = {
 DEFINE_EXTENSION_REG(tpidr)};
 
+static lldb_private::RegisterInfo g_register_infos_sme[] = {
+DEFINE_EXTENSION_REG(svcr)};
+
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,
@@ -89,6 +92,7 @@
   k_num_mte_register = 1,
   k_num_tls_register = 1,
   k_num_pauth_register = 2,
+  k_num_sme_register = 1,
   k_num_register_sets_default = 2,
   k_num_register_sets = 3
 };
@@ -196,6 +200,9 @@
 static const lldb_private::RegisterSet g_reg_set_tls_arm64 = {
 "Thread Local Storage Registers", "tls", k_num_tls_register, nullptr};
 
+static const lldb_private::RegisterSet g_reg_set_sme_arm64 = {
+"Scalable Matrix Extension Registers", "sme", k_num_sme_register, nullptr};
+
 RegisterInfoPOSIX_arm64::RegisterInfoPOSIX_arm64(
 const lldb_private::ArchSpec &target_arch, lldb_private::Flags opt_regsets)
 : lldb_private::RegisterInfoAndSetInterface(target_arch),
@@ -240,6 +247,9 @@
   // done as a dynamic set.
   AddRegSetTLS();
 
+  if (m_opt_regsets.AllSet(eRegsetMaskSSVE))
+AddRegSetSME();
+
   m_register_info_count = m_dynamic_reg_infos.size();
   m_register_info_p = m_dynamic_reg_infos.data();
   m_register_set_p = m_dynamic_reg_sets.data();
@@ -338,6 +348,21 @@
   m_dyn

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

[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:1003
+  // Bit 2 indicates whether the array storage is active (not yet implemented).
+  m_sme_ctrl_reg = m_sve_state == SVEState::Streaming;
+  return {};

omjavaid wrote:
> is there a need here to check if m_sve_state is valid?
The only "invalid" state we can check at the moment is SVEState::Unknown. Which 
would translate here to a value of 0 in the register, which isn't incorrect, 
it's just pesimistic.

By the time you're printing this register you'd likely have checked the state 
fully. Maybe it would come back as SVEState::Disabled, but in that case you 
wouldn't have SME anyway so this register isn't available to you.


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 updated this revision to Diff 541542.
DavidSpickett marked an inline comment as done.
DavidSpickett added a comment.

Add missing newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154927

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_static_config/TestSVERegisters.py

Index: 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/TestSVERegisters.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
@@ -22,7 +22,12 @@
 reg_value.GetByteSize(), expected, 'Verify "%s" == %i' % (name, expected)
 )
 
-def check_sve_regs_read(self, z_reg_size):
+def check_sve_regs_read(self, z_reg_size, expected_mode):
+if self.isAArch64SME():
+expected_value = "1" if expected_mode == Mode.SSVE else "0"
+self.expect("register read svcr", substrs=[
+"0x000" + expected_value])
+
 p_reg_size = int(z_reg_size / 8)
 
 for i in range(32):
@@ -162,7 +167,7 @@
 
 vg_reg_value = sve_registers.GetChildMemberWithName("vg").GetValueAsUnsigned()
 z_reg_size = vg_reg_value * 8
-self.check_sve_regs_read(z_reg_size)
+self.check_sve_regs_read(z_reg_size, start_mode)
 
 # Evaluate simple expression and print function expr_eval_func address.
 self.expect("expression expr_eval_func", substrs=["= 0x"])
@@ -174,7 +179,7 @@
 
 # We called a jitted function above which must not have changed SVE
 # vector length or register values.
-self.check_sve_regs_read(z_reg_size)
+self.check_sve_regs_read(z_reg_size, start_mode)
 
 self.check_sve_regs_read_after_write(z_reg_size)
 
@@ -206,4 +211,4 @@
 @skipIf(archs=no_match(["aarch64"]))
 @skipIf(oslist=no_match(["linux"]))
 def test_registers_expr_read_write_ssve_sve(self):
-self.sve_registers_read_write_impl(Mode.SSVE, Mode.SVE)
\ No newline at end of file
+self.sve_registers_read_write_impl(Mode.SSVE, Mode.SVE)
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -106,6 +106,8 @@
 
   void AddRegSetTLS();
 
+  void AddRegSetSME();
+
   uint32_t ConfigureVectorLength(uint32_t sve_vq);
 
   bool VectorSizeIsValid(uint32_t vq) {
@@ -127,6 +129,7 @@
   bool IsPAuthReg(unsigned reg) const;
   bool IsMTEReg(unsigned reg) const;
   bool IsTLSReg(unsigned reg) const;
+  bool IsSMEReg(unsigned reg) const;
 
   uint32_t GetRegNumSVEZ0() const;
   uint32_t GetRegNumSVEFFR() const;
@@ -136,6 +139,7 @@
   uint32_t GetPAuthOffset() const;
   uint32_t GetMTEOffset() const;
   uint32_t GetTLSOffset() const;
+  uint32_t GetSMEOffset() const;
 
 private:
   typedef std::map>
@@ -163,6 +167,7 @@
   std::vector pauth_regnum_collection;
   std::vector m_mte_regnum_collection;
   std::vector m_tls_regnum_collection;
+  std::vector m_sme_regnum_collection;
 };
 
 #endif
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -81,6 +81,9 @@
 static lldb_private::RegisterInfo g_register_infos_tls[] = {
 DEFINE_EXTENSION_REG(tpidr)};
 
+static lldb_private::RegisterInfo g_register_infos_sme[] = {
+DEFINE_EXTENSION_REG(svcr)};
+
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,
@@ -89,6 +92,7 @@
   k_num_mte_register = 1,
   k_num_tls_register = 1,
   k_num_pauth_register = 2,
+  k_num_sme_register = 1,
   k_num_register_sets_default = 2,
   k_num_register_sets = 3
 };
@@ -196,6 +200,9 @@
 static const lldb_private::RegisterSet g_reg_set_tls_arm64 = {
 "Thread Local Storage Registers", "tls", k_num_tls_register, nullptr};
 
+static const lldb_private::RegisterSet g_reg_set_sme_arm64 = {
+"Scalable Matrix Extension Registers", "sme", k_num_sme_register, nullptr};
+
 RegisterInfoPOSIX_arm64::RegisterInfoPOSIX_arm64(
 const lldb_private::ArchSpec &target_arch, lldb_private::Flags opt_regsets)
 : lldb_private::RegisterInfoAndSetInterfa

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py:30
+"0x000" + expected_value])
+
 p_reg_size = int(z_reg_size / 8)

This is the check promised in the previous patch. It ensures that when we 
return from evaluating the expression we do restore the previous operating mode.


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 541548.
DavidSpickett added a comment.

Add "Buffer" to method names.

enabled -> present in skipped messages. I realised that "enabled" is ambiguous
does it mean enabled in the CPU or in the process, present is more clearly
meaning is it on the CPU at all.


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,57 @@
+#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[]) {
+  uint64_t (*getter)(void) = get_tpidr;
+  void (*setter)(uint64_t) = set_tpidr;
+
+  // Expect just the register number.
+  if (argc != 2)
+return 1;
+
+  switch (argv[1][0]) {
+  case '1':
+break;
+  case '2':
+getter = get_tpidr2;
+setter = set_tpidr2;
+break;
+  default:
+return 1;
+  }
+
+  uint64_t original = getter();
+  uint64_t pattern = 0x1122334455667788;
+  setter(pattern);
+
+  // Set break point at this line.
+  // lldb will now set its own pattern for us to find.
+
+  uint64_t new_value = getter();
+  volatile bool reg_was_set = new_value == 0x8877665544332211;
+
+  setter(original);
+
+  return 0; // Set break point 2 at this line.
+}
Index: lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
===
--- lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
+++ lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -1,5 +1,5 @@
 """
-Test lldb's ability to read and write the AArch64 TLS register tpidr.
+Test lldb's ability to read and write the AArch64 TLS registers.
 """
 
 import lldb
@@ -8,12 +8,10 @@
 from lldbsuite.test import lldbutil
 
 
-class AArch64LinuxTLSRegister(TestBase):
+class AArch64LinuxTLSRegisters(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
-@skipUnlessArch("aarch64")
-@skipUnlessPlatform(["linux"])
-def test_tls(self):
+def setup(self, register_name):
 self.build()
 self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
@@ -31,6 +29,8 @@
 num_expected_locations=1,
 )
 
+self.runCmd("settings set target.run-args {}".format(
+{"tpidr": 1, "tpidr2": 2}[register_name]))
 self.runCmd("run", RUN_SUCCEEDED)
 
 if self.process().GetState() == lldb.eStateExited:
@@ -42,19 +42,24 @@
 substrs=["stopped", "stop reason = breakpoint"],
 )
 
+def check_tls_reg(self, register_name):
+self.setup(register_name)
+
 # Since we can't predict what the value will be, the program has set
 # a target value for us to find.
 
 regs = self.thread().GetSelectedFrame().GetRegisters()
 tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
 self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
-tpidr = tls_regs.GetChildMemberWithName("tpidr")
-self.assertTrue(tpidr.IsValid(), "No tpidr register found.")
+tls_reg = tls_regs.GetChildMemberWithName(register_name)
+self.assertTrue(tls_reg.IsValid(), "{} register not found.".format(
+register_name))
 
-self.assertEqual(tpidr.GetValueAsUnsigned(), 0x1122334455667788)
+self.assertEqual(tls_reg.GetValueAsUnsigned(), 0x1122334455667788)
 
 # Set our own value for the program to find.
-self.expect("register write tpidr 0x{:x}".format(0x8877665544332211))
+self.expect("register write {} 0x{:x}".format(
+register_name, 0x8877665544332211))
 self.expect("conti

[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 541570.
DavidSpickett added a comment.

Note the behaviour of tpidr2 on a system without SME.


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,59 @@
+#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[]) {
+  uint64_t (*getter)(void) = get_tpidr;
+  void (*setter)(uint64_t) = set_tpidr;
+
+  // Expect just the register number.
+  if (argc != 2)
+return 1;
+
+  // Note that accessing tpidr2 on a system without it will SIGILL. That is why
+  // we have this option instead of trying to set both regardless.
+  switch (argv[1][0]) {
+  case '1':
+break;
+  case '2':
+getter = get_tpidr2;
+setter = set_tpidr2;
+break;
+  default:
+return 1;
+  }
+
+  uint64_t original = getter();
+  uint64_t pattern = 0x1122334455667788;
+  setter(pattern);
+
+  // Set break point at this line.
+  // lldb will now set its own pattern for us to find.
+
+  uint64_t new_value = getter();
+  volatile bool reg_was_set = new_value == 0x8877665544332211;
+
+  setter(original);
+
+  return 0; // Set break point 2 at this line.
+}
Index: lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
===
--- lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
+++ lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -1,5 +1,5 @@
 """
-Test lldb's ability to read and write the AArch64 TLS register tpidr.
+Test lldb's ability to read and write the AArch64 TLS registers.
 """
 
 import lldb
@@ -8,12 +8,10 @@
 from lldbsuite.test import lldbutil
 
 
-class AArch64LinuxTLSRegister(TestBase):
+class AArch64LinuxTLSRegisters(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
-@skipUnlessArch("aarch64")
-@skipUnlessPlatform(["linux"])
-def test_tls(self):
+def setup(self, register_name):
 self.build()
 self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
@@ -31,6 +29,8 @@
 num_expected_locations=1,
 )
 
+self.runCmd("settings set target.run-args {}".format(
+{"tpidr": 1, "tpidr2": 2}[register_name]))
 self.runCmd("run", RUN_SUCCEEDED)
 
 if self.process().GetState() == lldb.eStateExited:
@@ -42,19 +42,24 @@
 substrs=["stopped", "stop reason = breakpoint"],
 )
 
+def check_tls_reg(self, register_name):
+self.setup(register_name)
+
 # Since we can't predict what the value will be, the program has set
 # a target value for us to find.
 
 regs = self.thread().GetSelectedFrame().GetRegisters()
 tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
 self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
-tpidr = tls_regs.GetChildMemberWithName("tpidr")
-self.assertTrue(tpidr.IsValid(), "No tpidr register found.")
+tls_reg = tls_regs.GetChildMemberWithName(register_name)
+self.assertTrue(tls_reg.IsValid(), "{} register not found.".format(
+register_name))
 
-self.assertEqual(tpidr.GetValueAsUnsigned(), 0x1122334455667788)
+self.assertEqual(tls_reg.GetValueAsUnsigned(), 0x1122334455667788)
 
 # Set our own value for the program to find.
-self.expect("register write tpidr 0x{:x}".format(0x8877665544332211))
+self.expect("register write {} 0x{:x}".format(
+register_name, 0x8877665544332211))
 self.expect("continue")
 
   

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: 
lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py:73
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])

DavidSpickett wrote:
> omjavaid wrote:
> > These three tests have a lot of commonalities may be merge them into one 
> > testing the whole logic. Doesn't look like we are getting much out of 
> > emitting three tests from this fairly basic test class.
> The tradeoff is execution time vs. a HWCAP check in the program file and a 
> bunch of ifs in Python.
> 
> Let me see what I can do, but I'm leaning toward the implementation 
> complexity outweighing the performance gained.
I've looked into this. A major thing to note is that reading tpidr2 (or rather, 
the system register operands that represent it) on a system without SME will 
SIGILL. I tried this on a Mountain Jade system.

This means we cannot have the program file set both regardless and only test 
what we expect to be present. We still need the option to the program and still 
need to run it again each time we want to check a different register.

(I guess you could have an option for each register, but again we're trading 
complexity here)

That does mean that the program file is the same between the 3 tests, so you 
could merge them to only build once. That said I think the time saved 
rebuilding a small example is not much when put against the pile of `if` you 
would need to use to merge the 3 tests into one (and therefore have to unpick 
later).

I could merge `test_tpidr2_no_sme` into `test_tpidr` but again, I'd rather have 
the clear separation of it being its own test than add an `if not 
self.isAArch64SME...` in there.

If I'm addressing completely different aspects than you had considered here, 
let me know and I'll fix those.


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] D155256: [lldb][x86_64] Add fs_base/gs_base support for Linux

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: 
lldb/test/API/commands/register/register/register_command/TestRegisters.py:613
+def test_fs_gs_base(self):
+"""Tests fs_base register can be read and equals to pthread_self() 
return value."""
+self.build()

Update this comment.



Comment at: 
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py:635
+error = lldb.SBError()
+bytesread = process.ReadMemory(0x400FF0, 20, error)
+

Stray line?



Comment at: 
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py:647
+self.assertTrue(reg_gs_base.IsValid(), "gs_base is not available")
+# The fs_base/gs_base registers in linux-x86_64.core are both zero.
+# Use "eu-readelf -n linux-x86_64.core" to verify.

Is it possible to make them non-zero? Looks like gs_base is often 0 anyway, so 
making fs_base non-zero would at least prove that we don't just always return 
0, and that we don't get fs_base and gs_base mixed up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155256

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

[Lldb-commits] [PATCH] D155256: [lldb][x86_64] Add fs_base/gs_base support for Linux

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: 
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py:647
+self.assertTrue(reg_gs_base.IsValid(), "gs_base is not available")
+# The fs_base/gs_base registers in linux-x86_64.core are both zero.
+# Use "eu-readelf -n linux-x86_64.core" to verify.

DavidSpickett wrote:
> Is it possible to make them non-zero? Looks like gs_base is often 0 anyway, 
> so making fs_base non-zero would at least prove that we don't just always 
> return 0, and that we don't get fs_base and gs_base mixed up.
Hmm, I see that this core is created from a very simple file that doesn't use 
pthreads. And adding pthreads in for all the supported platforms might be a can 
of worms.

There is a thread_crash test you could tack this onto instead, assuming the cpp 
threads are built on pthreads. It's not great because the test is really 
focusing on something else, but it's convenient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155256

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

[Lldb-commits] [PATCH] D155256: [lldb][x86_64] Add fs_base/gs_base support for Linux

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Tests LGTM, thanks.

@clayborg please approve if they look good to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155256

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

[Lldb-commits] [PATCH] D155768: [lldb] [NFC] Remove some dead code from Watchpoint class, and a method that makes no sense

[David Spickett via Phabricator via lldb-commits]

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

I don't understand the logic for if the hit counter is < the number of false 
alarms. I'm not even sure that's possible given that a false alarm would have 
to come from a reported hit, wouldn't it?

Maybe there is a behaviour difference when using UndohitCount but I don't think 
it's worth worrying about. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155768

___
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] 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] 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] D154926: [lldb][AArch64] Add support for SME's SVE streaming mode registers

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I will land this and the next patch once the 17 branch has been taken. I don't 
want SME changes on 17 with A: not enough testing and B: missing features (ZA).


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] D155256: [lldb][x86_64] Add fs_base/gs_base support for Linux

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

If you feel it's significant enough, it is worth adding a release note in 
`llvm/docs/ReleaseNotes.rst` before/on to the llvm 17 branch is taken.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155256

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

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

What I understand is that `MCInstrAnalysis` defaults to using `MCInstrDesc`, 
however not all targets have `MCInstrAnalysis` at all. So we have this 
awkwardness here with the two pointers, which is fine.

Sounds good to me, I am just wondering about the changes over in llvm.




Comment at: llvm/include/llvm/MC/MCInstrAnalysis.h:80
+  }
+
   /// Returns true if at least one of the register writes performed by

Does this have test coverage already?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156086

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

[Lldb-commits] [PATCH] D156118: [lldb][AArch64] Add reading of TLS tpidr register from core files

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added subscribers: lldb-commits, alextsao1999.
Herald added a project: LLDB.

7e229217f4215b519b886e7881bae4da3742a7d2 
 did live 
processes, this does
core files. Pretty simple, there is an NT_ARM_TLS note that contains
at least tpidr, and on systems with the Scalable Matrix Extension (SME), tpidr2
as well.

tpidr2 will be handled in future patches for SME support.

This NT_ARM_TLS note has always been present but it seems convenient to
handle it as "optional" inside of LLDB. We'll probably want the flexibility
when supporting tpidr2.

Normally the C library would set tpidr but all our test sources build
without it. So I've updated the neon test program to write to tpidr
and regenerated the corefile.

I've removed the LLDB_PTRACE_NT_ARM_TLS that was unused, we get
what we need from llvm's defs instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156118

Files:
  lldb/include/lldb/Host/linux/Ptrace.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.core

Index: lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
===
--- lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
+++ lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
@@ -1,6 +1,8 @@
 // compile with -march=armv8-a+simd on compatible aarch64 compiler
 // linux-aarch64-neon.core was generated by: aarch64-linux-gnu-gcc-8
 // commandline: -march=armv8-a+simd -nostdlib -static -g linux-aarch64-neon.c
+#include 
+
 static void bar(char *boom) {
   char F = 'b';
   asm volatile("fmov d0,  #0.5\n\t");
@@ -14,6 +16,9 @@
   asm volatile("movi v8.16b, #0x11\n\t");
   asm volatile("movi v31.16b, #0x30\n\t");
 
+  uint64_t pattern = 0x1122334455667788;
+  asm volatile("msr tpidr_el0, %0" ::"r"(pattern));
+
   *boom = 47; // Frame bar
 }
 
Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -304,10 +304,10 @@
 values = {}
 values["x1"] = "0x002f"
 values["w1"] = "0x002f"
-values["fp"] = "0x007fc5dd7f20"
-values["lr"] = "0x00400180"
-values["sp"] = "0x007fc5dd7f00"
-values["pc"] = "0x0040014c"
+values["fp"] = "0xc2ba77e0"
+values["lr"] = "0x0040019c"
+values["sp"] = "0xc2ba77c0"
+values["pc"] = "0x00400168"
 values[
 "v0"
 ] = "{0x00 0x00 0x00 0x00 0x00 0x00 0xe0 0x3f 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
@@ -366,6 +366,7 @@
 values["d31"] = "1.3980432860952889E-76"
 values["fpsr"] = "0x"
 values["fpcr"] = "0x"
+values["tpidr"] = "0x1122334455667788"
 
 for regname, value in values.items():
 self.expect(
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -123,6 +123,10 @@
 {llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_PAC_MASK},
 };
 
+constexpr RegsetDesc AARCH64_TLS_Desc[] = {
+{llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_TLS},
+};
+
 constexpr RegsetDesc PPC_VMX_Desc[] = {
 {llvm::Triple::FreeBSD, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
 {llvm::Triple::Linux, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -57,6 +57,7 @@
   lldb_private::DataExtractor m_fpr_data;
   lldb_private::DataExtractor m_sve_data;
   lldb_private::DataExtractor m_pac_data;
+  lldb_private::DataExtractor 

[Lldb-commits] [PATCH] D156118: [lldb][AArch64] Add reading of TLS tpidr register from core files

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a reviewer: omjavaid.
DavidSpickett added a comment.
Herald added a subscriber: JDevlieghere.

I wanted to add tpidr to the release notes and realised I left it half 
finished. Will update the release notes afterwards on branch or main depending 
on when this lands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156118

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

[Lldb-commits] [PATCH] D156118: [lldb][AArch64] Add reading of TLS tpidr register from core files

[David Spickett via Phabricator via lldb-commits]

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

Lower stack limit when making corefile to shrink it some. 16k was about as low
as I could go before it failed to even get to the program itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156118

Files:
  lldb/include/lldb/Host/linux/Ptrace.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.core

Index: lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
===
--- lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
+++ lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
@@ -1,6 +1,8 @@
 // compile with -march=armv8-a+simd on compatible aarch64 compiler
 // linux-aarch64-neon.core was generated by: aarch64-linux-gnu-gcc-8
 // commandline: -march=armv8-a+simd -nostdlib -static -g linux-aarch64-neon.c
+#include 
+
 static void bar(char *boom) {
   char F = 'b';
   asm volatile("fmov d0,  #0.5\n\t");
@@ -14,6 +16,9 @@
   asm volatile("movi v8.16b, #0x11\n\t");
   asm volatile("movi v31.16b, #0x30\n\t");
 
+  uint64_t pattern = 0x1122334455667788;
+  asm volatile("msr tpidr_el0, %0" ::"r"(pattern));
+
   *boom = 47; // Frame bar
 }
 
Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -304,10 +304,10 @@
 values = {}
 values["x1"] = "0x002f"
 values["w1"] = "0x002f"
-values["fp"] = "0x007fc5dd7f20"
-values["lr"] = "0x00400180"
-values["sp"] = "0x007fc5dd7f00"
-values["pc"] = "0x0040014c"
+values["fp"] = "0xdab7c770"
+values["lr"] = "0x0040019c"
+values["sp"] = "0xdab7c750"
+values["pc"] = "0x00400168"
 values[
 "v0"
 ] = "{0x00 0x00 0x00 0x00 0x00 0x00 0xe0 0x3f 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
@@ -366,6 +366,7 @@
 values["d31"] = "1.3980432860952889E-76"
 values["fpsr"] = "0x"
 values["fpcr"] = "0x"
+values["tpidr"] = "0x1122334455667788"
 
 for regname, value in values.items():
 self.expect(
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -123,6 +123,10 @@
 {llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_PAC_MASK},
 };
 
+constexpr RegsetDesc AARCH64_TLS_Desc[] = {
+{llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_TLS},
+};
+
 constexpr RegsetDesc PPC_VMX_Desc[] = {
 {llvm::Triple::FreeBSD, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
 {llvm::Triple::Linux, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -57,6 +57,7 @@
   lldb_private::DataExtractor m_fpr_data;
   lldb_private::DataExtractor m_sve_data;
   lldb_private::DataExtractor m_pac_data;
+  lldb_private::DataExtractor m_tls_data;
 
   SVEState m_sve_state;
   uint16_t m_sve_vector_length = 0;
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -34,6 +34,12 @@
   if (pac_data.GetByteSize() >= sizeof(uint64_t) * 2)
 opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskPAuth);
 
+  DataExtractor tls_data = getRegset(notes, arch.GetTriple(), AARCH64_TLS_Desc);
+  // A valid note will always contain at least one register, "tpidr". It may
+  // expand in future.
+  if (tls_data.GetByteSize() >= sizeof(uint64_t))
+

[Lldb-commits] [PATCH] D156118: [lldb][AArch64] Add reading of TLS tpidr register from core files

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 543999.
DavidSpickett added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Rebase, add release note before I forget.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156118

Files:
  lldb/include/lldb/Host/linux/Ptrace.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.core
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -131,6 +131,9 @@
 Changes to LLDB
 -
 
+* AArch64 Linux targets now provide access to the Thread Local Storage
+  register ``tpidr``.
+
 Changes to Sanitizers
 -
 
Index: lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
===
--- lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
+++ lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
@@ -1,6 +1,8 @@
 // compile with -march=armv8-a+simd on compatible aarch64 compiler
 // linux-aarch64-neon.core was generated by: aarch64-linux-gnu-gcc-8
 // commandline: -march=armv8-a+simd -nostdlib -static -g linux-aarch64-neon.c
+#include 
+
 static void bar(char *boom) {
   char F = 'b';
   asm volatile("fmov d0,  #0.5\n\t");
@@ -14,6 +16,9 @@
   asm volatile("movi v8.16b, #0x11\n\t");
   asm volatile("movi v31.16b, #0x30\n\t");
 
+  uint64_t pattern = 0x1122334455667788;
+  asm volatile("msr tpidr_el0, %0" ::"r"(pattern));
+
   *boom = 47; // Frame bar
 }
 
Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -304,10 +304,10 @@
 values = {}
 values["x1"] = "0x002f"
 values["w1"] = "0x002f"
-values["fp"] = "0x007fc5dd7f20"
-values["lr"] = "0x00400180"
-values["sp"] = "0x007fc5dd7f00"
-values["pc"] = "0x0040014c"
+values["fp"] = "0xdab7c770"
+values["lr"] = "0x0040019c"
+values["sp"] = "0xdab7c750"
+values["pc"] = "0x00400168"
 values[
 "v0"
 ] = "{0x00 0x00 0x00 0x00 0x00 0x00 0xe0 0x3f 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
@@ -366,6 +366,7 @@
 values["d31"] = "1.3980432860952889E-76"
 values["fpsr"] = "0x"
 values["fpcr"] = "0x"
+values["tpidr"] = "0x1122334455667788"
 
 for regname, value in values.items():
 self.expect(
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -123,6 +123,10 @@
 {llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_PAC_MASK},
 };
 
+constexpr RegsetDesc AARCH64_TLS_Desc[] = {
+{llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_TLS},
+};
+
 constexpr RegsetDesc PPC_VMX_Desc[] = {
 {llvm::Triple::FreeBSD, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
 {llvm::Triple::Linux, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -57,6 +57,7 @@
   lldb_private::DataExtractor m_fpr_data;
   lldb_private::DataExtractor m_sve_data;
   lldb_private::DataExtractor m_pac_data;
+  lldb_private::DataExtractor m_tls_data;
 
   SVEState m_sve_state;
   uint16_t m_sve_vector_length = 0;
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.c

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

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG03d8cd1d722d: [lldb][AArch64] Add support for SME's SVE 
streaming mode registers (authored by DavidSpickett).

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.
+

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

[David Spickett 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 rGefa43d785ee6: [lldb][AArch64] Add the tpidr2 TLS register 
that comes with SME (authored by DavidSpickett).

Changed prior to commit:
  https://reviews.llvm.org/D154930?vs=542932&id=544269#toc

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:

[Lldb-commits] [PATCH] D155107: Add support for llvm::MCInstPrinter::setPrintBranchImmAsAddress

[David Spickett via Phabricator via lldb-commits]

DavidSpickett accepted this revision.
DavidSpickett added a comment.

> Also you can test it as long as you put the right skipif annotations on it as 
> it'll be architecture specific. Or use a corefile and just check that the 
> backend for that architecture is enabled.

Given that:

- We are a thin layer around llvm-mc
- There is some confusion about exactly what each target does with its assembly 
already
- The target that benefits most from this doesn't have a buildbot

This is ok to go in without specific testing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155107

___
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.

> I like it the above approach with more enums for the high and low code/data. 
> Not sure if eTypeAny makes sense in the GetAddressMask(eTypeAny) scenario

It could be like the `FixAny` method, a fallback when you don't know either 
way. `eTypeAny` returns `FixAnyAddress(~0)` essentially.

Or it could or together all the masks but while that's good for curiosity's 
sake (what address bits are "safe"?) you would still need specific masks to 
construct an address for specific purposes.

It could fail, but then we're carrying around a Status object for one case, and 
that seems a shame. Unless we can think of other failure modes we haven't 
considered yet.


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] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> but I could imagine some harvard architecture target that behaved differently 
> (surely this is why Linux has two address masks)

I'm not privy to the exact reasoning, but at least part of it comes from the 
architecture itself. You could have a target that enables top byte ignore and 
pointer authentication for data addresses, but only enables pointer 
authentication for code addresses. So ptrace will show you different values for 
the pointer authentication masks in that case. I'm not sure you can actually 
configure a kernel that way today, but it's viable.

For the debugger, the result is the same. When top byte ignore is off, pointer 
authentication just uses that free space for itself. We end up removing the 
same set of bits either way.

For very specific tools you might want to only remove pointer authentication 
bits. Making this up, but maybe you want to take pointers from a pointer 
authenticated ABI application and pass them to a shared library without those 
protections. Niche, but ptrace leaves the door open for that rather than 
breaking userspace later by adding it.

Finally, you are right that one day there may be some scheme that truly does 
need different handling for code and data and again, let's keep the door open 
for that up here in lldb (though such an architecture would be major surgery in 
lldb anyway).


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] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

And +1 to the enum. This mitigates the MacOS specific-ness of the high and low 
masks at this time, and makes expansion easier if we do manage to generalise 
them later.


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] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett abandoned this revision.
DavidSpickett added a comment.

Turns out that for ZA support, we need to know the streaming vector length 
regardless of current mode. So SVG will be implemented as part of ZA support 
instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

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

[Lldb-commits] [PATCH] D156512: [lldb][AArch64] Save/restore TLS registers around expressions

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Previously we were storing them but not restoring them. This fixes
that and expands the testing to check for it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156512

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  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
===
--- lldb/test/API/linux/aarch64/tls_registers/main.c
+++ lldb/test/API/linux/aarch64/tls_registers/main.c
@@ -22,8 +22,18 @@
   __asm__ volatile("msr S3_3_C13_C0_5, %0" ::"r"(value));
 }
 
+bool use_tpidr2 = false;
+const uint64_t tpidr_pattern = 0x1122334455667788;
+const uint64_t tpidr2_pattern = 0x8877665544332211;
+
+void expr_func() {
+  set_tpidr(~tpidr_pattern);
+  if (use_tpidr2)
+set_tpidr2(~tpidr2_pattern);
+}
+
 int main(int argc, char *argv[]) {
-  bool use_tpidr2 = argc > 1;
+  use_tpidr2 = argc > 1;
 
   uint64_t original_tpidr = get_tpidr();
   // Accessing this on a core without it produces SIGILL. Only do this if
@@ -32,10 +42,8 @@
   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);
 
Index: lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
===
--- lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
+++ lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -42,13 +42,20 @@
 substrs=["stopped", "stop reason = breakpoint"],
 )
 
-def check_tls_reg(self, registers):
-self.setup(registers)
-
+def check_registers(self, registers, values):
 regs = self.thread().GetSelectedFrame().GetRegisters()
 tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
 self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
 
+for register in registers:
+tls_reg = tls_regs.GetChildMemberWithName(register)
+self.assertTrue(tls_reg.IsValid(), "{} register not found.".format(
+register))
+self.assertEqual(tls_reg.GetValueAsUnsigned(), values[register])
+
+def check_tls_reg(self, registers):
+self.setup(registers)
+
 # Since we can't predict what the value will be, the program has set
 # a target value for us to find.
 initial_values = {
@@ -56,11 +63,12 @@
 "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.GetValueAsUnsigned(), initial_values[register])
+self.check_registers(registers, initial_values)
+
+# Their values should be restored if an expression modifies them.
+self.runCmd("expression expr_func()")
+
+self.check_registers(registers, initial_values)
 
 set_values = {
 "tpidr": 0x,
@@ -95,7 +103,7 @@
 @skipUnlessPlatform(["linux"])
 def test_tls_sme(self):
 if not self.isAArch64SME():
-self.skipTest("SME must present.")
+self.skipTest("SME must be present.")
 
 self.check_tls_reg(["tpidr", "tpidr2"])
 
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -559,8 +559,10 @@
 dst += GetFPRSize();
   }
 
-  if (GetRegisterInfo().IsMTEEnabled())
+  if (GetRegisterInfo().IsMTEEnabled()) {
 ::memcpy(dst, GetMTEControl(), GetMTEControlSize());
+dst += GetMTEControlSize();
+  }
 
   ::memcpy(dst, GetTLSBuffer(), GetTLSBufferSize());
 
@@ -671,8 +673,17 @@
 ::memcpy(GetMTEControl(), src, GetMTEControlSize());
 m_mte_ctrl_is_valid = true;
 error = WriteMTEControl();
+if (error.Fail())
+  return error;
+src += GetMTEControlSize();
   }
 
+  // There is always a TLS set. It changes size based on system properties, it's
+  // not something an expression can change.
+  ::memcpy(GetTLSBuffer(), src, GetTLSBufferSize());
+  m_tls_is_valid = true;
+  error = WriteTLS();
+
   return error;
 }
 
___
lldb-commits mailing li

[Lldb-commits] [PATCH] D156118: [lldb][AArch64] Add reading of TLS tpidr register from core files

[David Spickett via Phabricator via lldb-commits]

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

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156118

Files:
  lldb/include/lldb/Host/linux/Ptrace.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.core
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -131,6 +131,9 @@
 Changes to LLDB
 -
 
+* AArch64 Linux targets now provide access to the Thread Local Storage
+  register ``tpidr``.
+
 Changes to Sanitizers
 -
 * HWASan now defaults to detecting use-after-scope bugs.
Index: lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
===
--- lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
+++ lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
@@ -1,6 +1,8 @@
 // compile with -march=armv8-a+simd on compatible aarch64 compiler
 // linux-aarch64-neon.core was generated by: aarch64-linux-gnu-gcc-8
 // commandline: -march=armv8-a+simd -nostdlib -static -g linux-aarch64-neon.c
+#include 
+
 static void bar(char *boom) {
   char F = 'b';
   asm volatile("fmov d0,  #0.5\n\t");
@@ -14,6 +16,9 @@
   asm volatile("movi v8.16b, #0x11\n\t");
   asm volatile("movi v31.16b, #0x30\n\t");
 
+  uint64_t pattern = 0x1122334455667788;
+  asm volatile("msr tpidr_el0, %0" ::"r"(pattern));
+
   *boom = 47; // Frame bar
 }
 
Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -304,10 +304,10 @@
 values = {}
 values["x1"] = "0x002f"
 values["w1"] = "0x002f"
-values["fp"] = "0x007fc5dd7f20"
-values["lr"] = "0x00400180"
-values["sp"] = "0x007fc5dd7f00"
-values["pc"] = "0x0040014c"
+values["fp"] = "0xdab7c770"
+values["lr"] = "0x0040019c"
+values["sp"] = "0xdab7c750"
+values["pc"] = "0x00400168"
 values[
 "v0"
 ] = "{0x00 0x00 0x00 0x00 0x00 0x00 0xe0 0x3f 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
@@ -366,6 +366,7 @@
 values["d31"] = "1.3980432860952889E-76"
 values["fpsr"] = "0x"
 values["fpcr"] = "0x"
+values["tpidr"] = "0x1122334455667788"
 
 for regname, value in values.items():
 self.expect(
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -123,6 +123,10 @@
 {llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_PAC_MASK},
 };
 
+constexpr RegsetDesc AARCH64_TLS_Desc[] = {
+{llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_TLS},
+};
+
 constexpr RegsetDesc PPC_VMX_Desc[] = {
 {llvm::Triple::FreeBSD, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
 {llvm::Triple::Linux, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -57,6 +57,7 @@
   lldb_private::DataExtractor m_fpr_data;
   lldb_private::DataExtractor m_sve_data;
   lldb_private::DataExtractor m_pac_data;
+  lldb_private::DataExtractor m_tls_data;
 
   SVEState m_sve_state;
   uint16_t m_sve_vector_length = 0;
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -34,6 +34,12 @@
   if (pac_data.GetByteSize(

[Lldb-commits] [PATCH] D156512: [lldb][AArch64] Save/restore TLS registers around expressions

[David Spickett via Phabricator via lldb-commits]

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

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156512

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  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
===
--- lldb/test/API/linux/aarch64/tls_registers/main.c
+++ lldb/test/API/linux/aarch64/tls_registers/main.c
@@ -22,8 +22,18 @@
   __asm__ volatile("msr S3_3_C13_C0_5, %0" ::"r"(value));
 }
 
+bool use_tpidr2 = false;
+const uint64_t tpidr_pattern = 0x1122334455667788;
+const uint64_t tpidr2_pattern = 0x8877665544332211;
+
+void expr_func() {
+  set_tpidr(~tpidr_pattern);
+  if (use_tpidr2)
+set_tpidr2(~tpidr2_pattern);
+}
+
 int main(int argc, char *argv[]) {
-  bool use_tpidr2 = argc > 1;
+  use_tpidr2 = argc > 1;
 
   uint64_t original_tpidr = get_tpidr();
   // Accessing this on a core without it produces SIGILL. Only do this if
@@ -32,10 +42,8 @@
   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);
 
Index: lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
===
--- lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
+++ lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -42,13 +42,20 @@
 substrs=["stopped", "stop reason = breakpoint"],
 )
 
-def check_tls_reg(self, registers):
-self.setup(registers)
-
+def check_registers(self, registers, values):
 regs = self.thread().GetSelectedFrame().GetRegisters()
 tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
 self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
 
+for register in registers:
+tls_reg = tls_regs.GetChildMemberWithName(register)
+self.assertTrue(tls_reg.IsValid(), "{} register not found.".format(
+register))
+self.assertEqual(tls_reg.GetValueAsUnsigned(), values[register])
+
+def check_tls_reg(self, registers):
+self.setup(registers)
+
 # Since we can't predict what the value will be, the program has set
 # a target value for us to find.
 initial_values = {
@@ -56,11 +63,12 @@
 "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.GetValueAsUnsigned(), initial_values[register])
+self.check_registers(registers, initial_values)
+
+# Their values should be restored if an expression modifies them.
+self.runCmd("expression expr_func()")
+
+self.check_registers(registers, initial_values)
 
 set_values = {
 "tpidr": 0x,
@@ -95,7 +103,7 @@
 @skipUnlessPlatform(["linux"])
 def test_tls_sme(self):
 if not self.isAArch64SME():
-self.skipTest("SME must present.")
+self.skipTest("SME must be present.")
 
 self.check_tls_reg(["tpidr", "tpidr2"])
 
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -559,8 +559,10 @@
 dst += GetFPRSize();
   }
 
-  if (GetRegisterInfo().IsMTEEnabled())
+  if (GetRegisterInfo().IsMTEEnabled()) {
 ::memcpy(dst, GetMTEControl(), GetMTEControlSize());
+dst += GetMTEControlSize();
+  }
 
   ::memcpy(dst, GetTLSBuffer(), GetTLSBufferSize());
 
@@ -671,8 +673,17 @@
 ::memcpy(GetMTEControl(), src, GetMTEControlSize());
 m_mte_ctrl_is_valid = true;
 error = WriteMTEControl();
+if (error.Fail())
+  return error;
+src += GetMTEControlSize();
   }
 
+  // There is always a TLS set. It changes size based on system properties, it's
+  // not something an expression can change.
+  ::memcpy(GetTLSBuffer(), src, GetTLSBufferSize());
+  m_tls_is_valid = true;
+  error = WriteTLS();
+
   return error;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156493: [lldb-vsocde] Adding support for the "disassemble" request.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I have no excuse to not try vcsode with lldb now, thanks for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156493

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

[Lldb-commits] [PATCH] D156687: [LLDB][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added subscribers: ctetreau, kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

While working in support for SME's ZA register, I found a circumstance
where restoring ZA after SVE, when the current SVE mode is non-streaming,
will kick the process back into FPSIMD mode. Meaning the SVE values that
you just wrote are now cut off at 128 bit.

The fix for that, I think, is to write ZA then SVE. Problem with that
is, the current ReadAll/WriteAll makes a lot of assumptions about the
saved data length.

This patch changes the format so there is a "kind" written before
each data block. This tells WriteAllRegisterValues what it's looking at
without brittle checks on length, or assumptions about ordering.

If we want to change the order of restoration, all we now have to
do is change the order of saving.

This exposes a bug where the TLS registers are not restored.
This will be fixed by https://reviews.llvm.org/D156512 in some form,
depending on what lands first.

Existing SVE tests certainly check restoration and when I got this
wrong, many, many tests failed. So I think we have enough coverage
already, and more will be coming with future ZA changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156687

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp

Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -499,6 +499,30 @@
   return Status("Failed to write register value");
 }
 
+enum SavedRegistersKind : uint32_t {
+  GPR,
+  SVE, // Used for SVE and SSVE.
+  FPR, // When there is no SVE, or SVE in FPSIMD mode.
+  MTE,
+  TLS,
+};
+
+static uint8_t *AddSavedRegistersKind(uint8_t *dst, SavedRegistersKind kind) {
+  *(reinterpret_cast(dst)) = kind;
+  return dst + sizeof(uint32_t);
+}
+
+static uint8_t *AddSavedRegistersData(uint8_t *dst, void *src, size_t size) {
+  ::memcpy(dst, src, size);
+  return dst + size;
+}
+
+static uint8_t *AddSavedRegisters(uint8_t *dst, enum SavedRegistersKind kind,
+  void *src, size_t size) {
+  dst = AddSavedRegistersKind(dst, kind);
+  return AddSavedRegistersData(dst, src, size);
+}
+
 Status NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
 lldb::WritableDataBufferSP &data_sp) {
   // AArch64 register data must contain GPRs and either FPR or SVE registers.
@@ -512,33 +536,33 @@
   // corresponds to register sets enabled by current register context.
 
   Status error;
-  uint32_t reg_data_byte_size = GetGPRBufferSize();
+  uint32_t reg_data_byte_size = sizeof(SavedRegistersKind) + GetGPRBufferSize();
   error = ReadGPR();
   if (error.Fail())
 return error;
 
   // If SVE is enabled we need not copy FPR separately.
   if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
-reg_data_byte_size += GetSVEBufferSize();
-// Also store the current SVE mode.
-reg_data_byte_size += sizeof(uint32_t);
+// Store mode and register data.
+reg_data_byte_size +=
+sizeof(SavedRegistersKind) + sizeof(uint32_t) + GetSVEBufferSize();
 error = ReadAllSVE();
   } else {
-reg_data_byte_size += GetFPRSize();
+reg_data_byte_size += sizeof(SavedRegistersKind) + GetFPRSize();
 error = ReadFPR();
   }
   if (error.Fail())
 return error;
 
   if (GetRegisterInfo().IsMTEEnabled()) {
-reg_data_byte_size += GetMTEControlSize();
+reg_data_byte_size += sizeof(SavedRegistersKind) + GetMTEControlSize();
 error = ReadMTEControl();
 if (error.Fail())
   return error;
   }
 
   // tpidr is always present but tpidr2 depends on SME.
-  reg_data_byte_size += GetTLSBufferSize();
+  reg_data_byte_size += sizeof(SavedRegistersKind) + GetTLSBufferSize();
   error = ReadTLS();
   if (error.Fail())
 return error;
@@ -546,23 +570,26 @@
   data_sp.reset(new DataBufferHeap(reg_data_byte_size, 0));
   uint8_t *dst = data_sp->GetBytes();
 
-  ::memcpy(dst, GetGPRBuffer(), GetGPRBufferSize());
-  dst += GetGPRBufferSize();
+  dst = AddSavedRegisters(dst, SavedRegistersKind::GPR, GetGPRBuffer(),
+  GetGPRBufferSize());
 
   if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
-*dst = static_cast(m_sve_state);
-dst += sizeof(m_sve_state);
-::memcpy(dst, GetSVEBuffer(), GetSVEBufferSize());
-dst += GetSVEBufferSize();
+dst = AddSavedRegistersKind(dst, SavedRegistersKind::SVE);
+*(reinterpret_cast(dst)) = static_cast(m_sve_state);
+dst += sizeof(uint32_t);
+dst = AddSavedRegistersData(dst, GetSVEBuffer(), GetSVEBufferSize());
   } else {
-::memcpy(dst, GetFPRBuffer(), G

[Lldb-commits] [PATCH] D156687: [LLDB][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a reviewer: omjavaid.
DavidSpickett added a comment.
Herald added a subscriber: JDevlieghere.

Even if this turns out not to be needed for Za handling, I think 
`WriteAllRegisterValues` not having to care about the layout is an improvement 
in itself.

Plus, this will give us a warning if we forget to save/restore new registers in 
future. It would have told me I had not done TLS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156687

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

[Lldb-commits] [PATCH] D156687: [LLDB][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

And an important note, you don't need the size for each block because the 
scalable blocks have headers from ptrace embedded in them anyway (SVE/SSVE/ZA).

I did remove some checks that seemed redundant because all data read by 
WriteAll is going to have been written by ReadALL, so they were unlikely to 
fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156687

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

[Lldb-commits] [PATCH] D156687: [LLDB][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I have some failing tests on Graviton to debug:

  Failed Tests (2):
lldb-api :: 
commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
lldb-api :: tools/lldb-server/TestGdbRemoteRegisterState.py

But the idea still stands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156687

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

[Lldb-commits] [PATCH] D156687: [LLDB][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

[David Spickett via Phabricator via lldb-commits]

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

Correct m_sve_state size to fix tests on SVE machines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156687

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp

Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -499,6 +499,30 @@
   return Status("Failed to write register value");
 }
 
+enum SavedRegistersKind : uint32_t {
+  GPR,
+  SVE, // Used for SVE and SSVE.
+  FPR, // When there is no SVE, or SVE in FPSIMD mode.
+  MTE,
+  TLS,
+};
+
+static uint8_t *AddSavedRegistersKind(uint8_t *dst, SavedRegistersKind kind) {
+  *(reinterpret_cast(dst)) = kind;
+  return dst + sizeof(uint32_t);
+}
+
+static uint8_t *AddSavedRegistersData(uint8_t *dst, void *src, size_t size) {
+  ::memcpy(dst, src, size);
+  return dst + size;
+}
+
+static uint8_t *AddSavedRegisters(uint8_t *dst, enum SavedRegistersKind kind,
+  void *src, size_t size) {
+  dst = AddSavedRegistersKind(dst, kind);
+  return AddSavedRegistersData(dst, src, size);
+}
+
 Status NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
 lldb::WritableDataBufferSP &data_sp) {
   // AArch64 register data must contain GPRs and either FPR or SVE registers.
@@ -512,33 +536,33 @@
   // corresponds to register sets enabled by current register context.
 
   Status error;
-  uint32_t reg_data_byte_size = GetGPRBufferSize();
+  uint32_t reg_data_byte_size = sizeof(SavedRegistersKind) + GetGPRBufferSize();
   error = ReadGPR();
   if (error.Fail())
 return error;
 
   // If SVE is enabled we need not copy FPR separately.
   if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
-reg_data_byte_size += GetSVEBufferSize();
-// Also store the current SVE mode.
-reg_data_byte_size += sizeof(uint32_t);
+// Store mode and register data.
+reg_data_byte_size +=
+sizeof(SavedRegistersKind) + sizeof(m_sve_state) + GetSVEBufferSize();
 error = ReadAllSVE();
   } else {
-reg_data_byte_size += GetFPRSize();
+reg_data_byte_size += sizeof(SavedRegistersKind) + GetFPRSize();
 error = ReadFPR();
   }
   if (error.Fail())
 return error;
 
   if (GetRegisterInfo().IsMTEEnabled()) {
-reg_data_byte_size += GetMTEControlSize();
+reg_data_byte_size += sizeof(SavedRegistersKind) + GetMTEControlSize();
 error = ReadMTEControl();
 if (error.Fail())
   return error;
   }
 
   // tpidr is always present but tpidr2 depends on SME.
-  reg_data_byte_size += GetTLSBufferSize();
+  reg_data_byte_size += sizeof(SavedRegistersKind) + GetTLSBufferSize();
   error = ReadTLS();
   if (error.Fail())
 return error;
@@ -546,23 +570,26 @@
   data_sp.reset(new DataBufferHeap(reg_data_byte_size, 0));
   uint8_t *dst = data_sp->GetBytes();
 
-  ::memcpy(dst, GetGPRBuffer(), GetGPRBufferSize());
-  dst += GetGPRBufferSize();
+  dst = AddSavedRegisters(dst, SavedRegistersKind::GPR, GetGPRBuffer(),
+  GetGPRBufferSize());
 
   if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
-*dst = static_cast(m_sve_state);
+dst = AddSavedRegistersKind(dst, SavedRegistersKind::SVE);
+*(reinterpret_cast(dst)) = m_sve_state;
 dst += sizeof(m_sve_state);
-::memcpy(dst, GetSVEBuffer(), GetSVEBufferSize());
-dst += GetSVEBufferSize();
+dst = AddSavedRegistersData(dst, GetSVEBuffer(), GetSVEBufferSize());
   } else {
-::memcpy(dst, GetFPRBuffer(), GetFPRSize());
-dst += GetFPRSize();
+dst = AddSavedRegisters(dst, SavedRegistersKind::FPR, GetFPRBuffer(),
+GetFPRSize());
   }
 
-  if (GetRegisterInfo().IsMTEEnabled())
-::memcpy(dst, GetMTEControl(), GetMTEControlSize());
+  if (GetRegisterInfo().IsMTEEnabled()) {
+dst = AddSavedRegisters(dst, SavedRegistersKind::MTE, GetMTEControl(),
+GetMTEControlSize());
+  }
 
-  ::memcpy(dst, GetTLSBuffer(), GetTLSBufferSize());
+  dst = AddSavedRegisters(dst, SavedRegistersKind::TLS, GetTLSBuffer(),
+  GetTLSBufferSize());
 
   return error;
 }
@@ -597,7 +624,8 @@
 return error;
   }
 
-  uint64_t reg_data_min_size = GetGPRBufferSize() + GetFPRSize();
+  uint64_t reg_data_min_size =
+  GetGPRBufferSize() + GetFPRSize() + 2 * (sizeof(SavedRegistersKind));
   if (data_sp->GetByteSize() < reg_data_min_size) {
 error.SetErrorStringWithFormat(
 "NativeRegisterContextLinux_arm64::%s data_sp contained insufficient "
@@ -606,71 +634,76 @@
 return error;
   }
 
-  // Register data starts with GPRs
-  ::memcpy(GetGPRBuffer(), src, Get

[Lldb-commits] [PATCH] D156687: [LLDB][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

[David Spickett via Phabricator via lldb-commits]

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

Remove misleading comment. We save the whole SVE context, FPSIMD or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156687

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp

Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -499,6 +499,30 @@
   return Status("Failed to write register value");
 }
 
+enum SavedRegistersKind : uint32_t {
+  GPR,
+  SVE, // Used for SVE and SSVE.
+  FPR,
+  MTE,
+  TLS,
+};
+
+static uint8_t *AddSavedRegistersKind(uint8_t *dst, SavedRegistersKind kind) {
+  *(reinterpret_cast(dst)) = kind;
+  return dst + sizeof(uint32_t);
+}
+
+static uint8_t *AddSavedRegistersData(uint8_t *dst, void *src, size_t size) {
+  ::memcpy(dst, src, size);
+  return dst + size;
+}
+
+static uint8_t *AddSavedRegisters(uint8_t *dst, enum SavedRegistersKind kind,
+  void *src, size_t size) {
+  dst = AddSavedRegistersKind(dst, kind);
+  return AddSavedRegistersData(dst, src, size);
+}
+
 Status NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
 lldb::WritableDataBufferSP &data_sp) {
   // AArch64 register data must contain GPRs and either FPR or SVE registers.
@@ -512,33 +536,33 @@
   // corresponds to register sets enabled by current register context.
 
   Status error;
-  uint32_t reg_data_byte_size = GetGPRBufferSize();
+  uint32_t reg_data_byte_size = sizeof(SavedRegistersKind) + GetGPRBufferSize();
   error = ReadGPR();
   if (error.Fail())
 return error;
 
   // If SVE is enabled we need not copy FPR separately.
   if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
-reg_data_byte_size += GetSVEBufferSize();
-// Also store the current SVE mode.
-reg_data_byte_size += sizeof(uint32_t);
+// Store mode and register data.
+reg_data_byte_size +=
+sizeof(SavedRegistersKind) + sizeof(m_sve_state) + GetSVEBufferSize();
 error = ReadAllSVE();
   } else {
-reg_data_byte_size += GetFPRSize();
+reg_data_byte_size += sizeof(SavedRegistersKind) + GetFPRSize();
 error = ReadFPR();
   }
   if (error.Fail())
 return error;
 
   if (GetRegisterInfo().IsMTEEnabled()) {
-reg_data_byte_size += GetMTEControlSize();
+reg_data_byte_size += sizeof(SavedRegistersKind) + GetMTEControlSize();
 error = ReadMTEControl();
 if (error.Fail())
   return error;
   }
 
   // tpidr is always present but tpidr2 depends on SME.
-  reg_data_byte_size += GetTLSBufferSize();
+  reg_data_byte_size += sizeof(SavedRegistersKind) + GetTLSBufferSize();
   error = ReadTLS();
   if (error.Fail())
 return error;
@@ -546,23 +570,26 @@
   data_sp.reset(new DataBufferHeap(reg_data_byte_size, 0));
   uint8_t *dst = data_sp->GetBytes();
 
-  ::memcpy(dst, GetGPRBuffer(), GetGPRBufferSize());
-  dst += GetGPRBufferSize();
+  dst = AddSavedRegisters(dst, SavedRegistersKind::GPR, GetGPRBuffer(),
+  GetGPRBufferSize());
 
   if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
-*dst = static_cast(m_sve_state);
+dst = AddSavedRegistersKind(dst, SavedRegistersKind::SVE);
+*(reinterpret_cast(dst)) = m_sve_state;
 dst += sizeof(m_sve_state);
-::memcpy(dst, GetSVEBuffer(), GetSVEBufferSize());
-dst += GetSVEBufferSize();
+dst = AddSavedRegistersData(dst, GetSVEBuffer(), GetSVEBufferSize());
   } else {
-::memcpy(dst, GetFPRBuffer(), GetFPRSize());
-dst += GetFPRSize();
+dst = AddSavedRegisters(dst, SavedRegistersKind::FPR, GetFPRBuffer(),
+GetFPRSize());
   }
 
-  if (GetRegisterInfo().IsMTEEnabled())
-::memcpy(dst, GetMTEControl(), GetMTEControlSize());
+  if (GetRegisterInfo().IsMTEEnabled()) {
+dst = AddSavedRegisters(dst, SavedRegistersKind::MTE, GetMTEControl(),
+GetMTEControlSize());
+  }
 
-  ::memcpy(dst, GetTLSBuffer(), GetTLSBufferSize());
+  dst = AddSavedRegisters(dst, SavedRegistersKind::TLS, GetTLSBuffer(),
+  GetTLSBufferSize());
 
   return error;
 }
@@ -597,7 +624,8 @@
 return error;
   }
 
-  uint64_t reg_data_min_size = GetGPRBufferSize() + GetFPRSize();
+  uint64_t reg_data_min_size =
+  GetGPRBufferSize() + GetFPRSize() + 2 * (sizeof(SavedRegistersKind));
   if (data_sp->GetByteSize() < reg_data_min_size) {
 error.SetErrorStringWithFormat(
 "NativeRegisterContextLinux_arm64::%s data_sp contained insufficient "
@@ -606,71 +634,76 @@
 return error;
   }
 
-  // Register data starts with GPRs
-  ::memcpy(GetGPRBuffer(), src, GetGPRBufferSize());
-  m_gpr_is_

[Lldb-commits] [PATCH] D156817: _wsopen_s does not accept bits other than `_S_IREAD | _S_IWRITE`

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I just commented on the issue, thanks for the fix!

What exactly are the other bits in the mode here, are we losing something 
important potentially? I guess it can't be that important if Windows rejects 
them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156817

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

[Lldb-commits] [PATCH] D156817: _wsopen_s does not accept bits other than `_S_IREAD | _S_IWRITE`

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Also please put `[lldb][Windows]` at the start of the commit title.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156817

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

[Lldb-commits] [PATCH] D156817: _wsopen_s does not accept bits other than `_S_IREAD | _S_IWRITE`

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/source/Host/windows/FileSystem.cpp:104
 return -1;
+  mode = mode & (_S_IREAD | _S_IWRITE); // All other bits are rejected by 
_wsopen_s
   int result;

Nitpick: comments are usually on the line before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156817

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

[Lldb-commits] [PATCH] D156493: [lldb-vsocde] Adding support for the "disassemble" request.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I've marked one of these tests skipped on Arm because it's failing on our bot: 
https://github.com/llvm/llvm-project/commit/54458c525aa47219a3ef2bee2be33d6096b1585c

This is a 32 Arm machine, so unless you've got ready access to one, we'll 
figure out what the problem is. Other tests in this change may also be failing, 
but I'll see what the bot thinks of this first skip.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156493

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

[Lldb-commits] [PATCH] D156493: [lldb-vsocde] Adding support for the "disassemble" request.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Also it would be very useful if the logs could be printed in a pretty form like:

  >>> from pprint import pprint
  >>> pprint(j)
  {u'body': {u'instructions': [{u'address': u'0x400584',
u'column': 9,
u'instruction': u'ldr r0, [sp, #0x4]',
u'instructionBytes': u'04 00 9d e5',
u'line': 12,
u'location': {u'name': u'main.c',
  u'path': 
u'/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/tools/lldb-vscode/disassemble/main.c'}},
   {u'address': u'0x400588',
u'column': 16,
u'instruction': u'ldr r1, [sp]',
u'instructionBytes': u'00 10 9d e5',
u'line': 12,
u'location': {u'name': u'main.c',
  u'path': 
u'/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/tools/lldb-vscode/disassemble/main.c'}},

Probably only want to do that when we've had a failure and not waste time 
otherwise, but it would be a lot more readable in the log file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156493

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

[Lldb-commits] [PATCH] D156493: [lldb-vsocde] Adding support for the "disassemble" request.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

It's failing the test, but actually it looks like a cleanup step is failing. 
Whether that is also the timeout I don't know. I will restart the machine to 
rule out any lingering processes and get back to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156493

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

[Lldb-commits] [PATCH] D156804: [lldb] Bump SWIG minimum version to 4

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

We are on 4.01 (AArch64), 4.02 (Arm) and 4.1.1 (Windows on Arm), so we have no 
issues with this.


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

https://reviews.llvm.org/D156804

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

[Lldb-commits] [PATCH] D156949: [lldb] Update LLDB Code Ownership

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/CodeOwners.rst:7-8
+particular part of LLDB are reviewed, either by themself or by someone else.
+They are also the gatekeepers for their part of LLDB, with the final word on
+what goes in or not.
+

This could be taken to mean every review must have approval from a code owner, 
on top of whatever other review has been done. Is that the intent? Someone 
coming from a project with strong maintainer rules (e.g. GDB, so I gather) may 
take it that way.



Comment at: lldb/CodeOwners.rst:214
+
+
+Tools

Is the extra space an RST thing, seems placed randomly.


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

https://reviews.llvm.org/D156949

___
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 inline comments.



Comment at: lldb/include/lldb/API/SBProcess.h:433
+
+  /// Clear the non-addressable bits of an \a addr value and return a
+  /// virtual address in memory.

"Clear bits from an addr value that are not used for addressing" is clearer to 
me.

Addressable makes me think I can craft an address to point to that bit, and 
I've been saying "non-address" but that term is also inside baseball, so 
something more wordy I think is better.



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

jasonmolenda wrote:
> clayborg wrote:
> > What does this function do? Does it try to auto detect code/data low/hi on 
> > its own assuming that specific regions are easily identifiable?
> We have the same method in Process.  Linux has separate masks for code & data 
> addresses, so we have FixCode and FixData methods, but sometimes you just 
> have an addr_t and you don't know the context (e.g. a register value that 
> you're testing to see if it points to a symbol so you can print that as 
> additional info to the user), and so in that case we have a 
> Process::FixAddress.
> 
> There is one armv7 abi that clears the 0th bit on FixCode address, and I 
> could see other ABIs doing similar things for the fixed nature of code 
> addresses.  But Data addresses are the most general, having to point to any 
> addressable byte.
> 
> Our Process/SBProcess say "FixCode if you know it's code, FixData if you know 
> it's data.  And if you don't know, FixData".  But I think having a 
> "FixAddress" method, making it clear that you don't know the origin of the 
> value, makes the intention a little clearer.  Even if it's just calling 
> FixData inside Process.
FixAnyAddress is also a marker for the, hopefully far from now, time, that lldb 
has to track address provenance. That's not an API concern though.

Should these functions also be one function that takes the enum value?

It's `FixAddress(addr, lldb.eMaskTypeData` instead of `FixDataAddress(addr)`, 
but it keeps the theme consistent between all the functions.



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

jasonmolenda wrote:
> 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.
I don't think it being in the ABI plugin is particularly deliberate, even if it 
is correct.

The ABI states something like the top byte is reserved for the OS and language 
runtimes, lldb is dealing with a consequence of that choice. So you could argue 
the ABI plugin should say "yes this should be fixed" and the fixing should 
happen elsewhere.

But right now those two things are so close together I don't think it matters.


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] D156493: [lldb-vsocde] Adding support for the "disassemble" request.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Also it would be very useful if the logs could be printed in a pretty form 
> like:

And I'll have a patch for this shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156493

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

[Lldb-commits] [PATCH] D156977: [lldb][lldb-vscode] Fix nullptr dereference when JSON is not an object

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added subscribers: lldb-commits, wangpc.
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156977

Files:
  lldb/tools/lldb-vscode/VSCode.cpp


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -523,12 +523,13 @@
 }
 return PacketStatus::JSONMalformed;
   }
-  object = *json_value->getAsObject();
-  if (!json_value->getAsObject()) {
+  llvm::json::Object *object_ptr = json_value->getAsObject();
+  if (!object_ptr) {
 if (log)
   *log << "error: json packet isn't a object" << std::endl;
 return PacketStatus::JSONNotObject;
   }
+  object = *object_ptr;
   return PacketStatus::Success;
 }
 


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -523,12 +523,13 @@
 }
 return PacketStatus::JSONMalformed;
   }
-  object = *json_value->getAsObject();
-  if (!json_value->getAsObject()) {
+  llvm::json::Object *object_ptr = json_value->getAsObject();
+  if (!object_ptr) {
 if (log)
   *log << "error: json packet isn't a object" << std::endl;
 return PacketStatus::JSONNotObject;
   }
+  object = *object_ptr;
   return PacketStatus::Success;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156979: [lldb][lldb-vscode] Pretty print JSON to log files

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This makes anlysing test failures much more easy.

For SendJSON this is simple, just use llvm::format instead.

For GetNextObject/ReadJSON it's a bit more tricky.

- Print the "Content-Length:" line in ReadJSON, but not the json.
- Back in GetNextObject, if the JSON doesn't parse, it'll be printed as a 
normal string along with an error message.
- If we didn't error before, we have a JSON value so we pretty print it.
- Finally, if it isn't an object we'll log an error for that, not including the 
JSON.

Before:

  <--
  Content-Length: 81
  
  
{"command":"disconnect","request_seq":5,"seq":0,"success":true,"type":"response"}

After:

  <--
  Content-Length: 81
  
  {
"command": "disconnect",
"request_seq": 5,
"seq": 0,
"success": true,
"type": "response"
  }

There appear to be some responses that include strings that are themselves JSON,
and this won't pretty print those but I think it's still worth doing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156979

Files:
  lldb/tools/lldb-vscode/VSCode.cpp


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -89,12 +89,6 @@
   output.write_full(llvm::utostr(json_str.size()));
   output.write_full("\r\n\r\n");
   output.write_full(json_str);
-
-  if (log) {
-*log << "<-- " << std::endl
- << "Content-Length: " << json_str.size() << "\r\n\r\n"
- << json_str << std::endl;
-  }
 }
 
 // Serialize the JSON value into a string and send the JSON packet to
@@ -105,7 +99,14 @@
   strm << json;
   static std::mutex mutex;
   std::lock_guard locker(mutex);
-  SendJSON(strm.str());
+  std::string json_str = strm.str();
+  SendJSON(json_str);
+
+  if (log) {
+*log << "<-- " << std::endl
+ << "Content-Length: " << json_str.size() << "\r\n\r\n"
+ << llvm::formatv("{0:2}", json).str() << std::endl;
+  }
 }
 
 // Read a JSON packet from the "in" stream.
@@ -129,11 +130,8 @@
   if (!input.read_full(log.get(), length, json_str))
 return json_str;
 
-  if (log) {
-*log << "--> " << std::endl
- << "Content-Length: " << length << "\r\n\r\n"
- << json_str << std::endl;
-  }
+  if (log)
+*log << "--> " << std::endl << "Content-Length: " << length << "\r\n\r\n";
 
   return json_str;
 }
@@ -523,6 +521,11 @@
 }
 return PacketStatus::JSONMalformed;
   }
+
+  if (log) {
+*log << llvm::formatv("{0:2}", *json_value).str() << std::endl;
+  }
+
   llvm::json::Object *object_ptr = json_value->getAsObject();
   if (!object_ptr) {
 if (log)


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -89,12 +89,6 @@
   output.write_full(llvm::utostr(json_str.size()));
   output.write_full("\r\n\r\n");
   output.write_full(json_str);
-
-  if (log) {
-*log << "<-- " << std::endl
- << "Content-Length: " << json_str.size() << "\r\n\r\n"
- << json_str << std::endl;
-  }
 }
 
 // Serialize the JSON value into a string and send the JSON packet to
@@ -105,7 +99,14 @@
   strm << json;
   static std::mutex mutex;
   std::lock_guard locker(mutex);
-  SendJSON(strm.str());
+  std::string json_str = strm.str();
+  SendJSON(json_str);
+
+  if (log) {
+*log << "<-- " << std::endl
+ << "Content-Length: " << json_str.size() << "\r\n\r\n"
+ << llvm::formatv("{0:2}", json).str() << std::endl;
+  }
 }
 
 // Read a JSON packet from the "in" stream.
@@ -129,11 +130,8 @@
   if (!input.read_full(log.get(), length, json_str))
 return json_str;
 
-  if (log) {
-*log << "--> " << std::endl
- << "Content-Length: " << length << "\r\n\r\n"
- << json_str << std::endl;
-  }
+  if (log)
+*log << "--> " << std::endl << "Content-Length: " << length << "\r\n\r\n";
 
   return json_str;
 }
@@ -523,6 +521,11 @@
 }
 return PacketStatus::JSONMalformed;
   }
+
+  if (log) {
+*log << llvm::formatv("{0:2}", *json_value).str() << std::endl;
+  }
+
   llvm::json::Object *object_ptr = json_value->getAsObject();
   if (!object_ptr) {
 if (log)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156979: [lldb][lldb-vscode] Pretty print JSON to log files

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added reviewers: ashgti, wallace.
DavidSpickett added a comment.

The one disadvantage here is that pretty printing adds more characters, so 
before you could copy paste the single JSON line and (I presume) it would have 
the same number of characters.

If that's a required use case then instead I can do the printing in the testing 
wrapper in lldb, by just going line by line and anything beginning with `{`, 
attempt to parse as JSON. I just figured that this formatting may be useful 
outside of the lldb context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156979

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

[Lldb-commits] [PATCH] D156118: [lldb][AArch64] Add reading of TLS tpidr register from core files

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156118

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

[Lldb-commits] [PATCH] D156512: [lldb][AArch64] Save/restore TLS registers around expressions

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156512

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

[Lldb-commits] [PATCH] D156118: [lldb][AArch64] Add reading of TLS tpidr register from core files

[David Spickett 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 rG43ad521f2fa9: [lldb][AArch64] Add reading of TLS tpidr 
register from core files (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156118

Files:
  lldb/include/lldb/Host/linux/Ptrace.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.core
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -135,6 +135,9 @@
 Changes to LLDB
 -
 
+* AArch64 Linux targets now provide access to the Thread Local Storage
+  register ``tpidr``.
+
 Changes to Sanitizers
 -
 * HWASan now defaults to detecting use-after-scope bugs.
Index: lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
===
--- lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
+++ lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-neon.c
@@ -1,6 +1,8 @@
 // compile with -march=armv8-a+simd on compatible aarch64 compiler
 // linux-aarch64-neon.core was generated by: aarch64-linux-gnu-gcc-8
 // commandline: -march=armv8-a+simd -nostdlib -static -g linux-aarch64-neon.c
+#include 
+
 static void bar(char *boom) {
   char F = 'b';
   asm volatile("fmov d0,  #0.5\n\t");
@@ -14,6 +16,9 @@
   asm volatile("movi v8.16b, #0x11\n\t");
   asm volatile("movi v31.16b, #0x30\n\t");
 
+  uint64_t pattern = 0x1122334455667788;
+  asm volatile("msr tpidr_el0, %0" ::"r"(pattern));
+
   *boom = 47; // Frame bar
 }
 
Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -304,10 +304,10 @@
 values = {}
 values["x1"] = "0x002f"
 values["w1"] = "0x002f"
-values["fp"] = "0x007fc5dd7f20"
-values["lr"] = "0x00400180"
-values["sp"] = "0x007fc5dd7f00"
-values["pc"] = "0x0040014c"
+values["fp"] = "0xdab7c770"
+values["lr"] = "0x0040019c"
+values["sp"] = "0xdab7c750"
+values["pc"] = "0x00400168"
 values[
 "v0"
 ] = "{0x00 0x00 0x00 0x00 0x00 0x00 0xe0 0x3f 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
@@ -366,6 +366,7 @@
 values["d31"] = "1.3980432860952889E-76"
 values["fpsr"] = "0x"
 values["fpcr"] = "0x"
+values["tpidr"] = "0x1122334455667788"
 
 for regname, value in values.items():
 self.expect(
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -123,6 +123,10 @@
 {llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_PAC_MASK},
 };
 
+constexpr RegsetDesc AARCH64_TLS_Desc[] = {
+{llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_TLS},
+};
+
 constexpr RegsetDesc PPC_VMX_Desc[] = {
 {llvm::Triple::FreeBSD, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
 {llvm::Triple::Linux, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -57,6 +57,7 @@
   lldb_private::DataExtractor m_fpr_data;
   lldb_private::DataExtractor m_sve_data;
   lldb_private::DataExtractor m_pac_data;
+  lldb_private::DataExtractor m_tls_data;
 
   SVEState m_sve_state;
   uint16_t m_sve_vector_length = 0;
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/

[Lldb-commits] [PATCH] D156512: [lldb][AArch64] Save/restore TLS registers around expressions

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6239227172cd: [lldb][AArch64] Save/restore TLS registers 
around expressions (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156512

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  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
===
--- lldb/test/API/linux/aarch64/tls_registers/main.c
+++ lldb/test/API/linux/aarch64/tls_registers/main.c
@@ -22,8 +22,18 @@
   __asm__ volatile("msr S3_3_C13_C0_5, %0" ::"r"(value));
 }
 
+bool use_tpidr2 = false;
+const uint64_t tpidr_pattern = 0x1122334455667788;
+const uint64_t tpidr2_pattern = 0x8877665544332211;
+
+void expr_func() {
+  set_tpidr(~tpidr_pattern);
+  if (use_tpidr2)
+set_tpidr2(~tpidr2_pattern);
+}
+
 int main(int argc, char *argv[]) {
-  bool use_tpidr2 = argc > 1;
+  use_tpidr2 = argc > 1;
 
   uint64_t original_tpidr = get_tpidr();
   // Accessing this on a core without it produces SIGILL. Only do this if
@@ -32,10 +42,8 @@
   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);
 
Index: lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
===
--- lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
+++ lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -42,13 +42,20 @@
 substrs=["stopped", "stop reason = breakpoint"],
 )
 
-def check_tls_reg(self, registers):
-self.setup(registers)
-
+def check_registers(self, registers, values):
 regs = self.thread().GetSelectedFrame().GetRegisters()
 tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
 self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
 
+for register in registers:
+tls_reg = tls_regs.GetChildMemberWithName(register)
+self.assertTrue(tls_reg.IsValid(), "{} register not found.".format(
+register))
+self.assertEqual(tls_reg.GetValueAsUnsigned(), values[register])
+
+def check_tls_reg(self, registers):
+self.setup(registers)
+
 # Since we can't predict what the value will be, the program has set
 # a target value for us to find.
 initial_values = {
@@ -56,11 +63,12 @@
 "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.GetValueAsUnsigned(), initial_values[register])
+self.check_registers(registers, initial_values)
+
+# Their values should be restored if an expression modifies them.
+self.runCmd("expression expr_func()")
+
+self.check_registers(registers, initial_values)
 
 set_values = {
 "tpidr": 0x,
@@ -95,7 +103,7 @@
 @skipUnlessPlatform(["linux"])
 def test_tls_sme(self):
 if not self.isAArch64SME():
-self.skipTest("SME must present.")
+self.skipTest("SME must be present.")
 
 self.check_tls_reg(["tpidr", "tpidr2"])
 
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -559,8 +559,10 @@
 dst += GetFPRSize();
   }
 
-  if (GetRegisterInfo().IsMTEEnabled())
+  if (GetRegisterInfo().IsMTEEnabled()) {
 ::memcpy(dst, GetMTEControl(), GetMTEControlSize());
+dst += GetMTEControlSize();
+  }
 
   ::memcpy(dst, GetTLSBuffer(), GetTLSBufferSize());
 
@@ -671,8 +673,17 @@
 ::memcpy(GetMTEControl(), src, GetMTEControlSize());
 m_mte_ctrl_is_valid = true;
 error = WriteMTEControl();
+if (error.Fail())
+  return error;
+src += GetMTEControlSize();
   }
 
+  // There is always a TLS set. It changes size based on system properties, it's
+  // not something an expression can change.
+  ::memcpy(GetTLSBuffer(), src, GetTLSBufferSize());
+  m_tls_is_valid = true;
+  error = WriteTLS();
+
   return error;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinf

[Lldb-commits] [PATCH] D157000: [lldb][AArch64] Check SIMD save/restore in SVE SIMD test

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added subscribers: ctetreau, kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

While doing some refactoring I forgot to carry over the copying in of
SIMD data in normal mode, but no tests failed.

Turns out, it's very easy for us to get the restore wrong because
even if you forget the memcopy, setting the buffer to valid may
just read the data you had before the expression evaluation.

So I've extended the SVE SIMD testing (which includes the plain SIMD mode)
to check expression save/restore. This is the only test that fails
if you forget to do `m_fpu_is_valid = true` so I take from that, that
prior to this it wasn't tested at all.

As a bonus, we now have coverage of the same thing for SVE and SSVE modes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157000

Files:
  
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
===
--- lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
+++ lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
@@ -40,6 +40,45 @@
   WRITE_SIMD(31);
 }
 
+void write_simd_regs_expr() {
+#define WRITE_SIMD(NUM)\
+  asm volatile("MOV v" #NUM ".d[0], %0\n\t"\
+   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(NUM + 1))
+
+  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;
Index: lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
===
--- lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
+++ lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
@@ -1,6 +1,6 @@
 """
-Test that LLDB correctly reads and writes AArch64 SIMD registers in SVE,
-streaming SVE and normal SIMD modes.
+Test that LLDB correctly reads and writes and restores 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.
@@ -46,6 +46,13 @@
 pad = " ".join(["0x00"] * 7)
 return "{{0x{:02x} {} 0x{:02x} {}}}".format(n, pad, n, pad)
 
+def check_simd_values(self, value_offset):
+# These are 128 bit registers, so getting them from the API as unsigned
+# values doesn't work. Check the command output instead.
+for i in range(32):
+self.expect("register read v{}".format(i),
+substrs=[self.make_simd_value(i+value_offset)])
+
 def sve_simd_registers_impl(self, mode):
 self.skip_if_needed(mode)
 
@@ -66,11 +73,9 @@
 substrs=["stop reason = breakpoint 1."],
 )
 
-# These are 128 bit registers, so getting them from the API as unsigned
-# values doesn't work. Check the command output instead.
-for i in range(32):
-self.expect("register read v{}".format(i),
-substrs=[self.make_simd_value(i)])
+self.check_simd_values(0)
+self.runCmd("expression write_simd_regs_expr()")
+self.check_simd_values(0)
 
 # Write a new set of values. The kernel will move the program back to
 # non-streaming mode here.
@@ -79,9 +84,7 @@
 i, self.make_simd_value(i+1)))
 
 # Should be visible within lldb.
-for i in range(32):
-self.expect("register read v{}".format(i),
-substrs=[self.make_simd_value(i+1)])
+self.check_simd_values(1)
 
 # The program should agree with lldb.
 self.expect("continue", substrs=["exited with status = 0"])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

[David Spickett via Phabricator via lldb-commits]

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

Use uint8_t for kind, fix some places doing sizeof uint32_t not of the kind 
type.

Put back memcopy of fpr registers, which is now tested by the parent patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156687

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp

Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -499,6 +499,30 @@
   return Status("Failed to write register value");
 }
 
+enum SavedRegistersKind : uint8_t {
+  GPR,
+  SVE, // Used for SVE and SSVE.
+  FPR,
+  MTE,
+  TLS,
+};
+
+static uint8_t *AddSavedRegistersKind(uint8_t *dst, SavedRegistersKind kind) {
+  *(reinterpret_cast(dst)) = kind;
+  return dst + sizeof(SavedRegistersKind);
+}
+
+static uint8_t *AddSavedRegistersData(uint8_t *dst, void *src, size_t size) {
+  ::memcpy(dst, src, size);
+  return dst + size;
+}
+
+static uint8_t *AddSavedRegisters(uint8_t *dst, enum SavedRegistersKind kind,
+  void *src, size_t size) {
+  dst = AddSavedRegistersKind(dst, kind);
+  return AddSavedRegistersData(dst, src, size);
+}
+
 Status NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
 lldb::WritableDataBufferSP &data_sp) {
   // AArch64 register data must contain GPRs and either FPR or SVE registers.
@@ -512,33 +536,33 @@
   // corresponds to register sets enabled by current register context.
 
   Status error;
-  uint32_t reg_data_byte_size = GetGPRBufferSize();
+  uint32_t reg_data_byte_size = sizeof(SavedRegistersKind) + GetGPRBufferSize();
   error = ReadGPR();
   if (error.Fail())
 return error;
 
   // If SVE is enabled we need not copy FPR separately.
   if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
-reg_data_byte_size += GetSVEBufferSize();
-// Also store the current SVE mode.
-reg_data_byte_size += sizeof(uint32_t);
+// Store mode and register data.
+reg_data_byte_size +=
+sizeof(SavedRegistersKind) + sizeof(m_sve_state) + GetSVEBufferSize();
 error = ReadAllSVE();
   } else {
-reg_data_byte_size += GetFPRSize();
+reg_data_byte_size += sizeof(SavedRegistersKind) + GetFPRSize();
 error = ReadFPR();
   }
   if (error.Fail())
 return error;
 
   if (GetRegisterInfo().IsMTEEnabled()) {
-reg_data_byte_size += GetMTEControlSize();
+reg_data_byte_size += sizeof(SavedRegistersKind) + GetMTEControlSize();
 error = ReadMTEControl();
 if (error.Fail())
   return error;
   }
 
   // tpidr is always present but tpidr2 depends on SME.
-  reg_data_byte_size += GetTLSBufferSize();
+  reg_data_byte_size += sizeof(SavedRegistersKind) + GetTLSBufferSize();
   error = ReadTLS();
   if (error.Fail())
 return error;
@@ -546,25 +570,26 @@
   data_sp.reset(new DataBufferHeap(reg_data_byte_size, 0));
   uint8_t *dst = data_sp->GetBytes();
 
-  ::memcpy(dst, GetGPRBuffer(), GetGPRBufferSize());
-  dst += GetGPRBufferSize();
+  dst = AddSavedRegisters(dst, SavedRegistersKind::GPR, GetGPRBuffer(),
+  GetGPRBufferSize());
 
   if (GetRegisterInfo().IsSVEEnabled() || GetRegisterInfo().IsSSVEEnabled()) {
-*dst = static_cast(m_sve_state);
+dst = AddSavedRegistersKind(dst, SavedRegistersKind::SVE);
+*(reinterpret_cast(dst)) = m_sve_state;
 dst += sizeof(m_sve_state);
-::memcpy(dst, GetSVEBuffer(), GetSVEBufferSize());
-dst += GetSVEBufferSize();
+dst = AddSavedRegistersData(dst, GetSVEBuffer(), GetSVEBufferSize());
   } else {
-::memcpy(dst, GetFPRBuffer(), GetFPRSize());
-dst += GetFPRSize();
+dst = AddSavedRegisters(dst, SavedRegistersKind::FPR, GetFPRBuffer(),
+GetFPRSize());
   }
 
   if (GetRegisterInfo().IsMTEEnabled()) {
-::memcpy(dst, GetMTEControl(), GetMTEControlSize());
-dst += GetMTEControlSize();
+dst = AddSavedRegisters(dst, SavedRegistersKind::MTE, GetMTEControl(),
+GetMTEControlSize());
   }
 
-  ::memcpy(dst, GetTLSBuffer(), GetTLSBufferSize());
+  dst = AddSavedRegisters(dst, SavedRegistersKind::TLS, GetTLSBuffer(),
+  GetTLSBufferSize());
 
   return error;
 }
@@ -599,7 +624,8 @@
 return error;
   }
 
-  uint64_t reg_data_min_size = GetGPRBufferSize() + GetFPRSize();
+  uint64_t reg_data_min_size =
+  GetGPRBufferSize() + GetFPRSize() + 2 * (sizeof(SavedRegistersKind));
   if (data_sp->GetByteSize() < reg_data_min_size) {
 error.SetErrorStringWithFormat(
 "NativeRegisterContextLinux_arm64::%s data_sp contained insufficient "
@@ -608,82 +634,84 @@
 return error;
   }
 
-  // Register 

[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Oh, and fixed the FIXME now that the TLS fixes have landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156687

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

[Lldb-commits] [PATCH] D157000: [lldb][AArch64] Check SIMD save/restore in SVE SIMD test

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

MTE control needs a test too, I'm working on that as its own change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157000

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

[Lldb-commits] [PATCH] D156949: [lldb] Update LLDB Code Ownership

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/CodeOwners.rst:7-8
+particular part of LLDB are reviewed, either by themself or by someone else.
+They are also the gatekeepers for their part of LLDB, with the final word on
+what goes in or not.
+

JDevlieghere wrote:
> DavidSpickett wrote:
> > This could be taken to mean every review must have approval from a code 
> > owner, on top of whatever other review has been done. Is that the intent? 
> > Someone coming from a project with strong maintainer rules (e.g. GDB, so I 
> > gather) may take it that way.
> I copied this from the Clang `CodeOwners.rst` with the aim of being 
> consistent, but I'm happy to tweak it. We  could qualify the last sentence 
> with something like "when consensus cannot be reached" or if we think 
> "gatekeeper" is too strong of a work maybe we can use "tie-breaker", though I 
> like that the former implies a sense of duty. Happy to take suggestions!
My understanding was that llvm in general didn't have this hard requirement for 
an owner to acknowledge every review.

So yeah:
"They are also the gatekeepers for their part of LLDB, with the final word on 
what goes in or not when consensus cannot be reached."

Sounds good to me.


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

https://reviews.llvm.org/D156949

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

[Lldb-commits] [PATCH] D156977: [lldb][lldb-vscode] Fix nullptr dereference when JSON is not an object

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbdeb35bda438: [lldb][lldb-vscode] Fix nullptr dereference 
when JSON is not an object (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156977

Files:
  lldb/tools/lldb-vscode/VSCode.cpp


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -523,12 +523,13 @@
 }
 return PacketStatus::JSONMalformed;
   }
-  object = *json_value->getAsObject();
-  if (!json_value->getAsObject()) {
+  llvm::json::Object *object_ptr = json_value->getAsObject();
+  if (!object_ptr) {
 if (log)
   *log << "error: json packet isn't a object" << std::endl;
 return PacketStatus::JSONNotObject;
   }
+  object = *object_ptr;
   return PacketStatus::Success;
 }
 


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -523,12 +523,13 @@
 }
 return PacketStatus::JSONMalformed;
   }
-  object = *json_value->getAsObject();
-  if (!json_value->getAsObject()) {
+  llvm::json::Object *object_ptr = json_value->getAsObject();
+  if (!object_ptr) {
 if (log)
   *log << "error: json packet isn't a object" << std::endl;
 return PacketStatus::JSONNotObject;
   }
+  object = *object_ptr;
   return PacketStatus::Success;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156979: [lldb][lldb-vscode] Pretty print JSON to log files

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG165f45a87774: [lldb][lldb-vscode] Pretty print JSON to log 
files (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156979

Files:
  lldb/tools/lldb-vscode/VSCode.cpp


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -89,12 +89,6 @@
   output.write_full(llvm::utostr(json_str.size()));
   output.write_full("\r\n\r\n");
   output.write_full(json_str);
-
-  if (log) {
-*log << "<-- " << std::endl
- << "Content-Length: " << json_str.size() << "\r\n\r\n"
- << json_str << std::endl;
-  }
 }
 
 // Serialize the JSON value into a string and send the JSON packet to
@@ -105,7 +99,14 @@
   strm << json;
   static std::mutex mutex;
   std::lock_guard locker(mutex);
-  SendJSON(strm.str());
+  std::string json_str = strm.str();
+  SendJSON(json_str);
+
+  if (log) {
+*log << "<-- " << std::endl
+ << "Content-Length: " << json_str.size() << "\r\n\r\n"
+ << llvm::formatv("{0:2}", json).str() << std::endl;
+  }
 }
 
 // Read a JSON packet from the "in" stream.
@@ -129,11 +130,8 @@
   if (!input.read_full(log.get(), length, json_str))
 return json_str;
 
-  if (log) {
-*log << "--> " << std::endl
- << "Content-Length: " << length << "\r\n\r\n"
- << json_str << std::endl;
-  }
+  if (log)
+*log << "--> " << std::endl << "Content-Length: " << length << "\r\n\r\n";
 
   return json_str;
 }
@@ -523,6 +521,11 @@
 }
 return PacketStatus::JSONMalformed;
   }
+
+  if (log) {
+*log << llvm::formatv("{0:2}", *json_value).str() << std::endl;
+  }
+
   llvm::json::Object *object_ptr = json_value->getAsObject();
   if (!object_ptr) {
 if (log)


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -89,12 +89,6 @@
   output.write_full(llvm::utostr(json_str.size()));
   output.write_full("\r\n\r\n");
   output.write_full(json_str);
-
-  if (log) {
-*log << "<-- " << std::endl
- << "Content-Length: " << json_str.size() << "\r\n\r\n"
- << json_str << std::endl;
-  }
 }
 
 // Serialize the JSON value into a string and send the JSON packet to
@@ -105,7 +99,14 @@
   strm << json;
   static std::mutex mutex;
   std::lock_guard locker(mutex);
-  SendJSON(strm.str());
+  std::string json_str = strm.str();
+  SendJSON(json_str);
+
+  if (log) {
+*log << "<-- " << std::endl
+ << "Content-Length: " << json_str.size() << "\r\n\r\n"
+ << llvm::formatv("{0:2}", json).str() << std::endl;
+  }
 }
 
 // Read a JSON packet from the "in" stream.
@@ -129,11 +130,8 @@
   if (!input.read_full(log.get(), length, json_str))
 return json_str;
 
-  if (log) {
-*log << "--> " << std::endl
- << "Content-Length: " << length << "\r\n\r\n"
- << json_str << std::endl;
-  }
+  if (log)
+*log << "--> " << std::endl << "Content-Length: " << length << "\r\n\r\n";
 
   return json_str;
 }
@@ -523,6 +521,11 @@
 }
 return PacketStatus::JSONMalformed;
   }
+
+  if (log) {
+*log << llvm::formatv("{0:2}", *json_value).str() << std::endl;
+  }
+
   llvm::json::Object *object_ptr = json_value->getAsObject();
   if (!object_ptr) {
 if (log)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156817: [lldb][windows] _wsopen_s does not accept bits other than `_S_IREAD | _S_IWRITE`

[David Spickett via Phabricator via lldb-commits]

DavidSpickett accepted this revision.
DavidSpickett added a comment.

I think this could be tested with an lldb-server test, but it would likely go 
in `lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py` which has 
every test marked as skipped on Windows. For reasons lost to time.

If you feel like trying to re-enable them, great, otherwise I'm ok for such a 
simple fix to go in as is. Thanks again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156817

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

[Lldb-commits] [PATCH] D156493: [lldb-vsocde] Adding support for the "disassemble" request.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Do you have any additional information about the failure? The link looks like 
> a timeout, so I'm not sure where things are timing out.

Sorry for the false alarm, 
https://github.com/llvm/llvm-project/commit/9a3f0cd717f68ccf9e348bce2d76a2372482f4f2
 fixed a few vscode tests including this one. I've enabled it again.

> Also it would be very useful if the logs could be printed in a pretty form 
> like:

And I've done this in 
https://github.com/llvm/llvm-project/commit/165f45a877742a74988d63f36aee635c8e0a47da.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156493

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

[Lldb-commits] [PATCH] D156949: [lldb] Update LLDB Code Ownership

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/CodeOwners.rst:150
+
+ELF
+~~~

labath wrote:
> @DavidSpickett 
Ok with me.



Comment at: lldb/CodeOwners.rst:220
+
+lldb-server
+~~~

labath wrote:
> @DavidSpickett 
Ok with me.


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

https://reviews.llvm.org/D156949

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

[Lldb-commits] [PATCH] D157214: [lldb] Fix typo in comments and in test

[David Spickett via Phabricator via lldb-commits]

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

LGTM, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157214

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