[Lldb-commits] [lldb] b7961f2 - [lldb] [debugserver] Preserve signing bits on lr in debugserver (#67384)

[via lldb-commits]

Author: Jason Molenda
Date: 2023-09-25T17:02:25-07:00
New Revision: b7961f2cb97556bfc50f7828d5f869d96ab9352e

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

LOG: [lldb] [debugserver] Preserve signing bits on lr in debugserver (#67384)

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

Added: 
    

Modified: 
    lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp

Removed: 
    


################################################################################
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(