[PATCH 0/1] MR3365: ntdll: Pass the trampoline ctx to the exception dispatcher on aarch64.

[Billy Laws (＠bylaws)]

CFI directives allow the context stored on the stack by raise_func_trampoline
to be used to unwind to any exception handlers as required when dispatching
an exception. As the dispatcher may change its input context in e.g.
BTCpuResetToConsistentState and these changes also need to be used when
unwinding make sure to pass this copy of the context to
KiUserExceptionDispatcher as opposed to the copy initially stored on the stack,
which is not taken into account when unwinding.

[PATCH 1/1] ntdll: Pass the trampoline ctx to the exception dispatcher on aarch64.

[Billy Laws]

From: Billy Laws

CFI directives allow the context stored on the stack by raise_func_trampoline
to be used to unwind to any exception handlers as required when dispatching
an exception. As the dispatcher may change its input context in e.g.
BTCpuResetToConsistentState and these changes also need to be used when
unwinding make sure to pass this copy of the context to
KiUserExceptionDispatcher as opposed to the copy initially stored on the stack,
which is not taken into account when unwinding.
---
 dlls/ntdll/unix/signal_arm64.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/dlls/ntdll/unix/signal_arm64.c b/dlls/ntdll/unix/signal_arm64.c
index d98268bcd54..47bf94eee23 100644
--- a/dlls/ntdll/unix/signal_arm64.c
+++ b/dlls/ntdll/unix/signal_arm64.c
@@ -953,12 +953,11 @@ NTSTATUS get_thread_wow64_context( HANDLE handle, void *ctx, ULONG size )
  * function stores things on the stack; if modified, modify that one in
  * sync as well. */
 __ASM_GLOBAL_FUNC( raise_func_trampoline,
-                   "stp x29, x30, [sp, #-0x30]!\n\t"
-                   __ASM_CFI(".cfi_def_cfa_offset 48\n\t")
-                   __ASM_CFI(".cfi_offset 29, -48\n\t")
-                   __ASM_CFI(".cfi_offset 30, -40\n\t")
-                   "stp x0,  x1,  [sp, #0x10]\n\t"
-                   "str x2,       [sp, #0x20]\n\t"
+                   "stp x29, x30, [sp, #-0x20]!\n\t"
+                   __ASM_CFI(".cfi_def_cfa_offset 32\n\t")
+                   __ASM_CFI(".cfi_offset 29, -32\n\t")
+                   __ASM_CFI(".cfi_offset 30, -24\n\t")
+                   "stp x0,  x2,  [sp, #0x10]\n\t"
                    "mov x29, sp\n\t"
                    __ASM_CFI(".cfi_def_cfa_register 29\n\t")
                    __ASM_CFI(".cfi_remember_state\n\t")
@@ -990,8 +989,9 @@ __ASM_GLOBAL_FUNC( raise_func_trampoline,
                    __ASM_CFI(".cfi_escape 0x10,0x4d,0x03,0x8f,0xb8,0x03\n\t") /* d13 */
                    __ASM_CFI(".cfi_escape 0x10,0x4e,0x03,0x8f,0xc0,0x03\n\t") /* d14 */
                    __ASM_CFI(".cfi_escape 0x10,0x4f,0x03,0x8f,0xc8,0x03\n\t") /* d15 */
-                   "ldp x0,  x1,  [x29, #0x10]\n\t"
-                   "ldr x2,       [x29, #0x20]\n\t"
+                   "ldp x0,  x2,  [x29, #0x10]\n\t"
+                   /* Set the context argument of KiUserExceptionDispatcher to the copy stored on the stack */
+                   "mov x1,  sp\n\t"
                    "blr x2\n\t"
                    __ASM_CFI(".cfi_restore_state\n\t")
                    "brk #1")
@@ -1032,7 +1032,7 @@ static void setup_exception( ucontext_t *sigcontext, EXCEPTION_RECORD *rec )
     LR_sig(sigcontext) = PC_sig(sigcontext);
     PC_sig(sigcontext) = (ULONG_PTR)raise_func_trampoline;
     REGn_sig(0, sigcontext) = (ULONG_PTR)&stack->rec;  /* first arg for KiUserExceptionDispatcher */
-    REGn_sig(1, sigcontext) = (ULONG_PTR)&stack->context; /* second arg for KiUserExceptionDispatcher */
+    REGn_sig(1, sigcontext) = (ULONG_PTR)&stack->context; /* context arg for raise_func_trampoline */
     REGn_sig(2, sigcontext) = (ULONG_PTR)pKiUserExceptionDispatcher; /* dispatcher arg for raise_func_trampoline */
     REGn_sig(18, sigcontext) = (ULONG_PTR)NtCurrentTeb();
 }

[Alexandre Julliard (＠julliard)]

Do we actually need the extra copy? Couldn't the CFI point to the initial context?

[Billy Laws (＠bylaws)]

On Fri Jul 21 10:07:27 2023 +0000, Alexandre Julliard wrote:
> Do we actually need the extra copy? Couldn't the CFI point to the
> initial context?

Yeah that should work, just need to store a pointer to the context on the stack in the trampoline to allow for LR  recovery (LR requires explicit handling in builtin_unwind_dll as CFI on ARM64 isn't really capable of recovering both PC and LR).

I'll switch to using the CFI macros from dwarf.h rather than manually escaping while I'm at it.