[Lldb-commits] [lldb] 6239227 - [lldb][AArch64] Save/restore TLS registers around expressions

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2023-08-03T12:33:49Z
New Revision: 6239227172cdc92f3bb72131333f50f83a6439cf

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

LOG: [lldb][AArch64] Save/restore TLS registers around expressions

Previously lldb was storing them but not restoring them. Meaning that this function:
```
void expr(uint64_t value) {
  __asm__ volatile("msr tpidr_el0, %0" ::"r"(value));
}
```
When run from lldb:
```
(lldb) expression expr()
```
Would leave tpidr as `value` instead of the original value of the register.

A check for this scenario has been added to TestAArch64LinuxTLSRegisters.py,
which covers tpidr and the SME excluisve tpidr2 register when it's present.

Reviewed By: omjavaid

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

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index ae9fe7039a9204..490b4d619edb59 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -559,8 +559,10 @@ Status NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
     dst += GetFPRSize();
   }
 
-  if (GetRegisterInfo().IsMTEEnabled())
+  if (GetRegisterInfo().IsMTEEnabled()) {
     ::memcpy(dst, GetMTEControl(), GetMTEControlSize());
+    dst += GetMTEControlSize();
+  }
 
   ::memcpy(dst, GetTLSBuffer(), GetTLSBufferSize());
 
@@ -671,8 +673,17 @@ Status NativeRegisterContextLinux_arm64::WriteAllRegisterValues(
     ::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;
 }
 

diff  --git a/lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py b/lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
index f1bc6b45a9d815..53a60a4e3d3b72 100644
--- a/lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
+++ b/lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -42,13 +42,20 @@ def setup(self, registers):
             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 @@ def check_tls_reg(self, registers):
             "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": 0x1111222233334444,
@@ -95,7 +103,7 @@ def test_tls_no_sme(self):
     @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"])
 

diff  --git a/lldb/test/API/linux/aarch64/tls_registers/main.c b/lldb/test/API/linux/aarch64/tls_registers/main.c
index bd8d7b4d1b6028..d32560867045ee 100644
--- a/lldb/test/API/linux/aarch64/tls_registers/main.c
+++ b/lldb/test/API/linux/aarch64/tls_registers/main.c
@@ -22,8 +22,18 @@ void set_tpidr2(uint64_t value) {
   __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 @@ int main(int argc, char *argv[]) {
   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);