Skip to content

Commit

Permalink
i#2019: bound signal delivery for arrivals in DR or gencode
Browse files Browse the repository at this point in the history
Adds a new check for pending signals at the top of fcache_enter, to avoid
entering a linked fragment after a signal arrived while in DR (where we
can't safely unlink).  This works by returning back to dispatch for signal
delivery.

This raises a complication for interrupted system call entries, for which
we cannot easily remember multiple pieces of state (the post-syscall
continuation plus the fact that we need to do a syscall): we instead go
backward and point at the syscall instruction itself (which will produce a
tail-bb).

Adds unlinking of the last-entered fragment for signals that arrive in
gencode, which handles signals arriving in the middle of fcache_enter.
This also unlinks the target of an IBL for signals that arrive while there,
except for the final jmp* which is punted to i#2042.

Adds two new tests based on code from Edmund Grimley Evans from the issue
tracker.

A64's fcache_enter signal pending check is NYI here and left for i#2043 due
to lack of conditional branch generation support.

Fixes #2019
Fixes #2025

Review-URL: https://codereview.appspot.com/313910043
  • Loading branch information
derekbruening committed Oct 23, 2016
1 parent 98657e1 commit 5d3c252
Show file tree
Hide file tree
Showing 17 changed files with 603 additions and 47 deletions.
6 changes: 5 additions & 1 deletion core/arch/aarch64/emit_utils.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* **********************************************************
* Copyright (c) 2014-2015 Google, Inc. All rights reserved.
* Copyright (c) 2014-2016 Google, Inc. All rights reserved.
* Copyright (c) 2016 ARM Limited. All rights reserved.
* **********************************************************/

Expand Down Expand Up @@ -392,6 +392,10 @@ append_fcache_enter_prologue(dcontext_t *dcontext, instrlist_t *ilist, bool abso
opnd_create_reg(DR_REG_X0)));
/* set up stolen reg */
insert_load_dr_tls_base(dcontext, ilist, NULL/*append*/, SCRATCH_REG0);

/* FIXME i#2019: check dcontext->signals_pending. First we need a way to
* create a conditional branch b.le.
*/
}

void
Expand Down
31 changes: 31 additions & 0 deletions core/arch/arch.c
Original file line number Diff line number Diff line change
Expand Up @@ -3157,6 +3157,37 @@ get_app_sysenter_addr()
return app_sysenter_instr_addr;
}

size_t
syscall_instr_length(dr_isa_mode_t mode)
{
size_t syslen;
IF_X86_ELSE({
ASSERT(INT_LENGTH == SYSCALL_LENGTH);
ASSERT(SYSENTER_LENGTH == SYSCALL_LENGTH);
syslen = SYSCALL_LENGTH;
}, {
syslen = IF_ARM_ELSE((mode == DR_ISA_ARM_THUMB ?
SVC_THUMB_LENGTH : SVC_ARM_LENGTH),
SVC_LENGTH);
});
return syslen;
}

bool
is_syscall_at_pc(dcontext_t *dcontext, app_pc pc)
{
instr_t instr;
bool res = false;
instr_init(dcontext, &instr);
TRY_EXCEPT(dcontext, {
pc = decode(dcontext, pc, &instr);
res = (pc != NULL && instr_valid(&instr) && instr_is_syscall(&instr));
}, {
});
instr_free(dcontext, &instr);
return res;
}

void
copy_mcontext(priv_mcontext_t *src, priv_mcontext_t *dst)
{
Expand Down
2 changes: 2 additions & 0 deletions core/arch/arch.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ mixed_mode_enabled(void)
# define APP_STACK_LIMIT_OFFSET ((PROT_OFFS)+offsetof(dcontext_t, app_stack_limit))
# define APP_STACK_BASE_OFFSET ((PROT_OFFS)+offsetof(dcontext_t, app_stack_base))
# define NONSWAPPED_SCRATCH_OFFSET ((PROT_OFFS)+offsetof(dcontext_t, nonswapped_scratch))
#else
# define SIGPENDING_OFFSET ((PROT_OFFS)+offsetof(dcontext_t, signals_pending))
#endif

#ifdef TRACE_HEAD_CACHE_INCR
Expand Down
2 changes: 2 additions & 0 deletions core/arch/arch_exports.h
Original file line number Diff line number Diff line change
Expand Up @@ -1634,6 +1634,8 @@ int decode_syscall_num(dcontext_t *dcontext, byte *entry);
void link_shared_syscall(dcontext_t *dcontext);
void unlink_shared_syscall(dcontext_t *dcontext);
#endif
size_t syscall_instr_length(dr_isa_mode_t mode);
bool is_syscall_at_pc(dcontext_t *dcontext, app_pc pc);

/* Coarse-grain fragment support */
cache_pc
Expand Down
15 changes: 15 additions & 0 deletions core/arch/arm/emit_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,9 @@ insert_load_dr_tls_base(dcontext_t *dcontext, instrlist_t *ilist, instr_t *where
void
append_fcache_enter_prologue(dcontext_t *dcontext, instrlist_t *ilist, bool absolute)
{
#ifdef UNIX
instr_t *no_signals = INSTR_CREATE_label(dcontext);
#endif
ASSERT_NOT_IMPLEMENTED(!absolute &&
!TEST(SELFPROT_DCONTEXT, dynamo_options.protect_mask));
/* grab gen routine's parameter dcontext and put it into REG_DCXT */
Expand All @@ -636,6 +639,18 @@ append_fcache_enter_prologue(dcontext_t *dcontext, instrlist_t *ilist, bool abso
OPND_ARG1/*r0*/));
/* set up stolen reg */
insert_load_dr_tls_base(dcontext, ilist, NULL/*append*/, SCRATCH_REG0);

#ifdef UNIX
APP(ilist, INSTR_CREATE_ldrsb
(dcontext, opnd_create_reg(DR_REG_R2),
OPND_DC_FIELD(absolute, dcontext, OPSZ_1, SIGPENDING_OFFSET)));
APP(ilist, XINST_CREATE_cmp
(dcontext, opnd_create_reg(DR_REG_R2), OPND_CREATE_INT8(0)));
APP(ilist, INSTR_PRED(INSTR_CREATE_b(dcontext, opnd_create_instr(no_signals)),
DR_PRED_LE));
APP(ilist, INSTR_CREATE_bx(dcontext, opnd_create_reg(DR_REG_LR)));
APP(ilist, no_signals);
#endif
}

void
Expand Down
4 changes: 4 additions & 0 deletions core/arch/emit_utils_shared.c
Original file line number Diff line number Diff line change
Expand Up @@ -1923,6 +1923,10 @@ append_jmp_to_fcache_target(dcontext_t *dcontext, instrlist_t *ilist,
* RESTORE_FROM_UPCONTEXT PROT_OFFSET, %xsi
* endif
* endif
* cmp signals_pending_OFFSET(SCRATCH_REG5), 0
* jle no_signals
* ret
* no_signals:
*
* # append_load_tls_base (ARM only)
* mrc p15, 0, r0, c13, c0, 2
Expand Down
5 changes: 1 addition & 4 deletions core/arch/interp.c
Original file line number Diff line number Diff line change
Expand Up @@ -7080,10 +7080,7 @@ decode_fragment(dcontext_t *dcontext, fragment_t *f, byte *buf, /*IN/OUT*/uint *
* the following conditions are satisfied. */
bool possible_ignorable_sysenter = DYNAMO_OPTION(ignore_syscalls) &&
(get_syscall_method() == SYSCALL_METHOD_SYSENTER) &&
/* FIXME Traces don't have FRAG_HAS_SYSCALL set so we can't filter on
* that flag for all fragments. We should propagate this flag from
* a BB to a trace. */
(TEST(FRAG_HAS_SYSCALL, f->flags) || TEST(FRAG_IS_TRACE, f->flags));
TEST(FRAG_HAS_SYSCALL, f->flags);
#endif
instrlist_t intra_ctis;
coarse_info_t *info = NULL;
Expand Down
23 changes: 15 additions & 8 deletions core/arch/x86/emit_utils.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* **********************************************************
* Copyright (c) 2010-2015 Google, Inc. All rights reserved.
* Copyright (c) 2010-2016 Google, Inc. All rights reserved.
* Copyright (c) 2000-2010 VMware, Inc. All rights reserved.
* **********************************************************/

Expand Down Expand Up @@ -1161,16 +1161,23 @@ insert_fragment_prefix(dcontext_t *dcontext, fragment_t *f)
void
append_fcache_enter_prologue(dcontext_t *dcontext, instrlist_t *ilist, bool absolute)
{
#ifdef UNIX
instr_t *no_signals = INSTR_CREATE_label(dcontext);
#endif
if (!absolute) {
/* grab gen routine's parameter dcontext and put it into edi */
APP(ilist, XINST_CREATE_load(dcontext,
opnd_create_reg(REG_DCXT),
OPND_ARG1));
APP(ilist, XINST_CREATE_load(dcontext, opnd_create_reg(REG_DCXT), OPND_ARG1));
if (TEST(SELFPROT_DCONTEXT, dynamo_options.protect_mask))
APP(ilist, RESTORE_FROM_DC(dcontext,
REG_DCXT_PROT,
PROT_OFFS));
}
APP(ilist, RESTORE_FROM_DC(dcontext, REG_DCXT_PROT, PROT_OFFS));
}
#ifdef UNIX
APP(ilist, XINST_CREATE_cmp
(dcontext, OPND_DC_FIELD(absolute, dcontext, OPSZ_1, SIGPENDING_OFFSET),
OPND_CREATE_INT8(0)));
APP(ilist, INSTR_CREATE_jcc(dcontext, OP_jle, opnd_create_instr(no_signals)));
APP(ilist, XINST_CREATE_return(dcontext));
APP(ilist, no_signals);
#endif
}

/* # append instructions to call exit_dr_hook
Expand Down
90 changes: 81 additions & 9 deletions core/dispatch.c
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,11 @@ dispatch_enter_fcache(dcontext_t *dcontext, fragment_t *targetf)
mcontext->pc = save_pc;
}

#ifdef UNIX
/* We store this for purposes like signal unlinking (i#2019) */
dcontext->asynch_target = dcontext->next_tag;
#endif

#if defined(UNIX) && defined(DEBUG)
/* i#238/PR 499179: check that libc errno hasn't changed. It's
* not worth actually saving+restoring since to we'd also need to
Expand Down Expand Up @@ -493,8 +498,28 @@ dispatch_enter_fcache(dcontext_t *dcontext, fragment_t *targetf)
FCACHE_ENTRY_PC(targetf))
#endif
);
#ifdef UNIX
if (dcontext->signals_pending) {
/* i#2019: the fcache_enter generated code starts with a check for pending
* signals, allowing the signal handling code to simply queue signals that
* arrive in DR code and only attempt to unlink for interruption points known
* to be safe for unlinking.
*/
KSTOP_NOT_MATCHING(fcache_default);
dcontext->whereami = WHERE_DISPATCH;
enter_couldbelinking(dcontext, NULL, true);
dcontext->next_tag = dcontext->asynch_target;
LOG(THREAD, LOG_DISPATCH, 2,
"Signal arrived while in DR: aborting fcache_enter; next_tag is "PFX"\n",
dcontext->next_tag);
STATS_INC(num_entrances_aborted);
trace_abort(dcontext);
receive_pending_signal(dcontext);
return false;
}
#endif
ASSERT_NOT_REACHED();
return true;
return false;
}

/* Enters the cache at the specified entrance routine to execute the
Expand Down Expand Up @@ -535,7 +560,7 @@ enter_fcache(dcontext_t *dcontext, fcache_enter_func_t entry, cache_pc pc)

dcontext->whereami = WHERE_FCACHE;
(*entry)(dcontext);
ASSERT_NOT_REACHED();
IF_WINDOWS(ASSERT_NOT_REACHED()); /* returns for signals on unix */
}

/* Handles special tags in DR or elsewhere that do interesting things.
Expand Down Expand Up @@ -1741,6 +1766,8 @@ handle_system_call(dcontext_t *dcontext)
priv_mcontext_t *mc = get_mcontext(dcontext);
int sysnum = os_normalized_sysnum((int)MCXT_SYSNUM_REG(mc), NULL, dcontext);
#endif
app_pc saved_next_tag = dcontext->next_tag;
bool repeat = false;
#ifdef WINDOWS
/* make sure to ask about syscall before pre_syscall, which will swap new mc in! */
bool use_prev_dcontext = is_cb_return_syscall(dcontext);
Expand Down Expand Up @@ -1975,13 +2002,58 @@ handle_system_call(dcontext_t *dcontext)

set_at_syscall(dcontext, true);
KSTART_DC(dcontext, syscall_fcache); /* stopped in dispatch_exit_fcache_stats */
enter_fcache(dcontext, (fcache_enter_func_t)
/* DEFAULT_ISA_MODE as we want the ISA mode of our gencode */
convert_data_to_function
(PC_AS_JMP_TGT(DEFAULT_ISA_MODE, (app_pc)fcache_enter)),
PC_AS_JMP_TGT(DEFAULT_ISA_MODE, do_syscall));
/* will handle post processing in handle_post_system_call */
ASSERT_NOT_REACHED();
do {
#ifdef UNIX
/* We've already updated the signal mask as though the handler is
* completely finished, so we cannot go and receive a signal before
* executing the sigreturn syscall.
* Sigreturn will come back to dispatch so there's no worry about
* unbounded delay.
*/
if (is_sigreturn_syscall(dcontext) && dcontext->signals_pending > 0)
dcontext->signals_pending = -1;
#endif
enter_fcache(dcontext, (fcache_enter_func_t)
/* DEFAULT_ISA_MODE as we want the ISA mode of our gencode */
convert_data_to_function
(PC_AS_JMP_TGT(DEFAULT_ISA_MODE, (app_pc)fcache_enter)),
PC_AS_JMP_TGT(DEFAULT_ISA_MODE, do_syscall));
#ifdef UNIX
if (is_sigreturn_syscall(dcontext) && dcontext->signals_pending)
repeat = true;
else
break;
#endif
} while (repeat);
#ifdef UNIX
if (dcontext->signals_pending) {
/* i#2019: see comments in dispatch_enter_fcache() */
KSTOP(syscall_fcache);
dcontext->whereami = WHERE_DISPATCH;
set_at_syscall(dcontext, false);
/* We need to remember both the post-syscall resumption point and
* the fact that we need to execute a syscall, but we only have
* a single PC field to place it into inside our sigreturn frame
* and other places. Our solution is to point back at the
* syscall instruction itself. The walk-backward scheme here is a
* little hacky perhaps. We'll make a bb just for this syscall, which
* will not know the syscall number: but any re-execution in a loop
* will go back to the main bb.
*/
dcontext->next_tag = saved_next_tag -
syscall_instr_length(dcontext->last_fragment == NULL ? DEFAULT_ISA_MODE :
FRAG_ISA_MODE(dcontext->last_fragment->flags));
ASSERT(is_syscall_at_pc(dcontext, dcontext->next_tag));
LOG(THREAD, LOG_DISPATCH, 2,
"Signal arrived in DR: aborting syscall enter; interrupted "PFX"\n",
dcontext->next_tag);
STATS_INC(num_entrances_aborted);
trace_abort(dcontext);
receive_pending_signal(dcontext);
} else
#endif
/* will handle post processing in handle_post_system_call */
ASSERT_NOT_REACHED();
}
else {
#ifdef CLIENT_INTERFACE
Expand Down
1 change: 1 addition & 0 deletions core/dispatch.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* **********************************************************
* Copyright (c) 2016 Google, Inc. All rights reserved.
* Copyright (c) 2000-2008 VMware, Inc. All rights reserved.
* **********************************************************/

Expand Down
6 changes: 4 additions & 2 deletions core/globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,10 @@ struct _dcontext_t {
coarse_info_t *dir_exit;
} coarse_exit;

where_am_i_t whereami; /* where control is at the moment */
where_am_i_t whereami; /* where control is at the moment */
#ifdef UNIX
char signals_pending; /* != 0: pending; < 0: currently handling one */
#endif

/************* end of offset-crucial fields *********************/

Expand Down Expand Up @@ -843,7 +846,6 @@ struct _dcontext_t {
#ifdef UNIX
void * signal_field;
void * pcprofile_field;
bool signals_pending;
#endif
void * private_code; /* various thread-private routines */

Expand Down
Loading

0 comments on commit 5d3c252

Please sign in to comment.