llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

<details>
<summary>Changes</summary>

In https://reviews.llvm.org/D136620 I changed debugserver to stop using the 
kernel-provided functions
arm_thread_state64_get_{pc,lr,sp,fp} to postprocess those four registers on 
aarch64 systems after we thread_get_state() them. The kernel stores these four 
registers with signing internally, either from the inferior process' actual 
signing, or its own.

When a program had crashed by doing an authenticated BL to an address with 
improper signing, the inferior process would crash and that improperly signed 
pc would be given to debugserver via thread_get_state. debugserver would run 
that through arm_thread_state64_get_pc() and then debugserver would crash when 
authenticating &amp; stripping the value, on newer Mac hardware.

To avoid debugserver crashing on a crashed inferior process, I switched from 
using these system functions to strip the values, to simply clearing the bits 
outright in debugserver.

However, lr is a special case where the inferior may have signed this value 
(against the stack pointer value at the time).  Or it may not yet have any 
authentication bits, right after a BL.  In the latter case, the kernel will add 
its own auth bits for while it is stored inside the kernel.  In the case of a 
user lr value, we cannot authenticate it in debugserver without knowing the sp 
value it was signed against (and the way it is signed is not specified by the 
ABI) so an "improperly" signed lr (whatever that means) won't cause debugserver 
to crash.

debugserver can thread_get_state the inferior's lr, run it through 
arm_thread_state64_get_lr(), and get the actual signed 64-bit value that the 
inferior process is using.  And the specifics of how that lr is signed may be 
important for debugging the process, instead of how I am currently clearing the 
auth bits outright.

This patch reverts that change for lr only, and also adds a new logging to 
debugserver specifically for the four sp/fp/lr/pc values that thread_get_state 
hands to us, before we process them at all.

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


1 Files Affected:

- (modified) lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp 
(+18-2) 


``````````diff
diff --git a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp 
b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
index bdecae81d31a71a..e02ef6353640a44 100644
--- a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -169,6 +169,23 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
                          (thread_state_t)&m_state.context.gpr, &count);
   if (DNBLogEnabledForAny(LOG_THREAD)) {
     uint64_t *x = &m_state.context.gpr.__x[0];
+    DNBLogThreaded("thread_get_state signed regs "
+                   "\n   fp=%16.16llx"
+                   "\n   lr=%16.16llx"
+                   "\n   sp=%16.16llx"
+                   "\n   pc=%16.16llx",
+#if __has_feature(ptrauth_calls) && defined(__LP64__)
+                   reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_fp),
+                   reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_lr),
+                   reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_sp),
+                   reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_pc)
+#else
+                   m_state.context.gpr.__fp,
+                   m_state.context.gpr.__lr, 
+                   m_state.context.gpr.__sp,
+                   m_state.context.gpr.__pc
+#endif
+    );
 
 #if __has_feature(ptrauth_calls) && defined(__LP64__)
     uint64_t log_fp = clear_pac_bits(
@@ -2173,8 +2190,7 @@ bool DNBArchMachARM64::GetRegisterValue(uint32_t set, 
uint32_t reg,
               reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_pc));
           break;
         case gpr_lr:
-          value->value.uint64 = clear_pac_bits(
-              reinterpret_cast<uint64_t>(m_state.context.gpr.__opaque_lr));
+          value->value.uint64 = arm_thread_state64_get_lr(m_state.context.gpr);
           break;
         case gpr_sp:
           value->value.uint64 = clear_pac_bits(

``````````

</details>


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

Reply via email to