Address Yichao's review

Author: Keno

src/Monkeypatcher.cc

@@ -500,8 +500,8 @@ remote_code_ptr Monkeypatcher::get_jump_stub_exit_breakpoint(remote_code_ptr ip,
   --it;
   patched_syscall *ps = &syscall_stub_list[it->second];
   auto bp = it->first + ps->size - ps->safe_suffix;
-  if (pp == bp - 4 || pp == bp - 8) {
-    return remote_code_ptr((it->first + ps->size - 4).as_int());
+  if (pp == bp - 4 || pp == bp - 8 || pp == bp - 12) {
+    return remote_code_ptr((it->first + ps->size - 12).as_int());
   }
   return nullptr;
 }
@@ -717,13 +717,13 @@ bool patch_syscall_with_hook_arch<ARM64Arch>(Monkeypatcher& patcher,
     2 * 4,
     /**
      * safe_suffix:
-     * We've returned from syscallbuf and continue execution
-     * won't hit syscallbuf breakpoint
-     * (this also include the 8 bytes that stores the return address)
-     * Note that stack restore instruction also belongs to the syscallbuf return path
-     * However, since it is still using the scratch memory,
-     * it doesn't belong to the safe area.
-     * The caller needs to have special handling for that instruction.
+     * The safe suffix are all instructions that are no longer using syscallbuf
+     * private stack memory. On aarch64, that is the bail path svc instruction
+     * and the final jump instruction (including the 8 byte return address).
+     * See the detailed extended jump patch assembly above for details.
+     * Note that the stack restore instructions also occurr on the syscallbuf
+     * return path, but are not considered part of the safe suffix, since they
+     * still rely on the syscallbuf stack memory to function properly.
      */
     2 * 4 + 8
   });

src/preload/syscall_hook.S

@@ -891,7 +891,9 @@ retq
 _syscallbuf_code_start:
 
 _syscall_hook_trampoline:
-        // stack frame:
+        // parent frame:
+        // 0 (688): lr from the extended jump patch [this gets rewritten here in the bail path]
+        // this stack frame:
         // 208-688: q2 - q31
         // 128-200: x10 - x18
         // 112-128: x7, x9
@@ -952,6 +954,7 @@ _syscall_hook_trampoline:
         cbnz x0, 1f
 
         // If the function requested the bail path, rewrite the return address
+        // N.B.: This modifies the stack address saved in the parent frame.
         ldr x0, [sp, 688]
         add x0, x0, 8
         str x0, [sp, 688]

src/record_signal.cc

@@ -298,7 +298,7 @@ bool handle_syscallbuf_breakpoint(RecordTask* t) {
     LOG(debug) << "Reached syscallstub exit instruction, singlestepping to "
                   "enable signal dispatch";
     ASSERT(t, t->arch() == aarch64 && t->syscallstub_exit_breakpoint);
-    auto retaddr_addr = t->syscallstub_exit_breakpoint.to_data_ptr<uint8_t>() + 3 * 4;
+    auto retaddr_addr = t->syscallstub_exit_breakpoint.to_data_ptr<uint8_t>() + 4;
     uint64_t retaddr;
     t->read_bytes_helper(retaddr_addr, sizeof(retaddr), &retaddr);
     Registers r = t->regs();