Skip to content

Commit

Permalink
i#1569 AArch64: Fix polluted X1 in handle_sigreturn.
Browse files Browse the repository at this point in the history
This affected interrupted system calls.

Review-URL: https://codereview.appspot.com/295650043
  • Loading branch information
Kevin Zhou authored and egrimley-arm committed Jun 29, 2016
1 parent e445404 commit 3375189
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 12 deletions.
26 changes: 16 additions & 10 deletions core/arch/aarch64/emit_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -480,8 +480,9 @@ append_restore_gpr(dcontext_t *dcontext, instrlist_t *ilist, bool absolute)

/* Append instructions to save gpr on fcache return, called after
* append_fcache_return_prologue.
* Assuming the execution comes from an exit stub,
* Assuming the execution comes from an exit stub via br DR_REG_X1,
* dcontext base is held in REG_DCXT, and exit stub in X0.
* App's x0 and x1 is stored in TLS_REG0_SLOT and TLS_REG1_SLOT
* - store all registers into dcontext's mcontext
* - restore REG_DCXT app value from TLS slot to mcontext
* - restore dr_reg_stolen app value from TLS slot to mcontext
Expand All @@ -492,7 +493,13 @@ append_save_gpr(dcontext_t *dcontext, instrlist_t *ilist, bool ibl_end, bool abs
{
int i;

for (i = 0; i < 30; i += 2) {
/* X0 and X1 will always have been saved in TLS slots before executing
* the code generated here. See, for example:
* emit_do_syscall_common, emit_indirect_branch_lookup, handle_sigreturn,
* insert_exit_stub_other_flags, execute_handler_from_{cache,dispatch},
* transfer_from_sig_handler_to_fcache_return
*/
for (i = 2; i < 30; i += 2) {
/* stp x(i), x(i+1), [x(dcxt), #xi_offset] */
APP(ilist, INSTR_CREATE_xx(dcontext,
0xa9000000 | i | (i + 1) << 10 |
Expand All @@ -507,19 +514,18 @@ append_save_gpr(dcontext_t *dcontext, instrlist_t *ilist, bool ibl_end, bool abs
(REG_DCXT - DR_REG_X0) << 5 |
REG_OFFSET(DR_REG_X30) >> 3 << 15));

/* It would be a bit more efficient to use LDP + STP here:
* app's x0 was spilled to DIRECT_STUB_SPILL_SLOT by exit stub.
/* ldp x1, x2, [x(stolen)]
* stp x1, x2, [x(dcxt)]
*/
APP(ilist, RESTORE_FROM_TLS(dcontext, SCRATCH_REG1, DIRECT_STUB_SPILL_SLOT));
APP(ilist, INSTR_CREATE_xx(dcontext, 0xa9400000 | 1 | 2 << 10 |
(dr_reg_stolen - DR_REG_X0) << 5));
APP(ilist, INSTR_CREATE_xx(dcontext, 0xa9000000 | 1 | 2 << 10 |
(REG_DCXT - DR_REG_X0) << 5));

if (linkstub != NULL) {
/* FIXME i#1575: NYI for coarse-grain stub */
ASSERT_NOT_IMPLEMENTED(false);
} else {
APP(ilist, SAVE_TO_DC(dcontext, SCRATCH_REG1, R0_OFFSET));
}
/* App's x1 was spilled to DIRECT_STUB_SPILL_SLOT2. */
APP(ilist, RESTORE_FROM_TLS(dcontext, SCRATCH_REG1, DIRECT_STUB_SPILL_SLOT2));
APP(ilist, SAVE_TO_DC(dcontext, SCRATCH_REG1, R1_OFFSET));

/* REG_DCXT's app value is stored in DCONTEXT_BASE_SPILL_SLOT by
* append_prepare_fcache_return, so copy it to mcontext.
Expand Down
12 changes: 10 additions & 2 deletions core/arch/emit_utils_shared.c
Original file line number Diff line number Diff line change
Expand Up @@ -4581,13 +4581,21 @@ emit_do_syscall_common(dcontext_t *dcontext, generated_code_t *code,
*syscall_offs += AARCH64_INSTR_SIZE;
#endif

#ifdef AARCHXX
#if defined(ARM)
/* We have to save r0 in case the syscall is interrupted. We can't
* easily do this from dispatch b/c fcache_enter clobbers some TLS slots.
*/
APP(&ilist, instr_create_save_to_tls(dcontext, DR_REG_R0, TLS_REG0_SLOT));
/* XXX: should have a proper patch list entry */
*syscall_offs += IF_X64_ELSE(AARCH64_INSTR_SIZE, THUMB_LONG_INSTR_SIZE);
*syscall_offs += THUMB_LONG_INSTR_SIZE;
#elif defined(AARCH64)
/* For AArch64, we need to save both x0 and x1 into SLOT 0 and SLOT 1
* in case the syscall is interrupted. See append_save_gpr.
* stp x0, x1, [x28]
*/
APP(&ilist, INSTR_CREATE_xx(dcontext, 0xa9000000 | 0 | 1 << 10 |
(dr_reg_stolen - DR_REG_X0) << 5));
*syscall_offs += AARCH64_INSTR_SIZE;
#endif

/* system call itself -- using same method we've observed OS using */
Expand Down
4 changes: 4 additions & 0 deletions core/unix/signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -2833,6 +2833,10 @@ transfer_from_sig_handler_to_fcache_return(dcontext_t *dcontext, sigcontext_t *s
/* x64 always uses shared gencode */
get_local_state_extended()->spill_space.IF_X86_ELSE(xax, r0) =
sc->IF_X86_ELSE(SC_XAX, SC_R0);
# ifdef AARCH64
/* X1 needs to be spilled because of br x1 in exit stubs. */
get_local_state_extended()->spill_space.r1 = sc->SC_R1;
# endif
#else
get_mcontext(dcontext)->IF_X86_ELSE(xax, r0) = sc->IF_X86_ELSE(SC_XAX, SC_R0);
#endif
Expand Down

0 comments on commit 3375189

Please sign in to comment.