From 271d357cd5609cd1668e67c4617b17364c33c67e Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Mon, 28 Jun 2021 17:13:08 -0400 Subject: [PATCH] i#4274 kernel pc: Provide rseq abort interrupted pc (#4973) Adds source context information to the kernel xfer event for rseq aborts, which was previously missing. To provide the interrupted rseq abort PC as the PC of the final committing store, adds logic inside DR to record that PC when it is discovered during block creation. Updates the rseq test to check the new information. Issue: #4274 --- api/docs/release.dox | 2 ++ core/arch/mangle_shared.c | 1 + core/lib/dr_events.h | 9 ++++++--- core/unix/os_exports.h | 5 ++++- core/unix/rseq_linux.c | 41 +++++++++++++++++++++++++++++++++------ suite/tests/linux/rseq.c | 34 ++++++++++++++++++++++++++++++++ 6 files changed, 82 insertions(+), 10 deletions(-) diff --git a/api/docs/release.dox b/api/docs/release.dox index 298197bbf1f..e97de132ab1 100644 --- a/api/docs/release.dox +++ b/api/docs/release.dox @@ -222,6 +222,8 @@ Further non-compatibility-affecting changes include: - Added the reconstructed #instrlist_t when available for the faulting fragment to #dr_fault_fragment_info_t. This makes it available to the restore state event callback(s) via the #dr_restore_state_info_t arg. + - Added the source context for restartable sequence aborts (#DR_XFER_RSEQ_ABORT) + which was previously missing. **************************************************
diff --git a/core/arch/mangle_shared.c b/core/arch/mangle_shared.c index e792f18cdf0..8e1d357324a 100644 --- a/core/arch/mangle_shared.c +++ b/core/arch/mangle_shared.c @@ -1364,6 +1364,7 @@ mangle_rseq(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr, "Rseq sequences must fall through their endpoints"); ASSERT_NOT_REACHED(); } + rseq_set_final_instr_pc(start, pc); mangle_rseq_insert_native_sequence(dcontext, ilist, instr, next_instr, flags, start, end, handler, scratch_reg, reg_written, reg_written_count); diff --git a/core/lib/dr_events.h b/core/lib/dr_events.h index f00253ffd20..67903fe2a62 100644 --- a/core/lib/dr_events.h +++ b/core/lib/dr_events.h @@ -946,9 +946,12 @@ typedef struct _dr_kernel_xfer_info_t { dr_kernel_xfer_type_t type; /** * The source machine context which is about to be changed. This may be NULL - * if it is unknown, which is the case for #DR_XFER_CALLBACK_DISPATCHER and - * #DR_XFER_RSEQ_ABORT (where the PC is not known but the rest of the state - * matches the current state). + * if it is unknown, which is the case for #DR_XFER_CALLBACK_DISPATCHER. + * For #DR_XFER_RSEQ_ABORT, due to the constraints of handling restartable + * sequences, the abort PC will point prior to the committing store, while + * that store already executed during instrumentation. We recommend that + * clients treat the store as never-executed in that situation, if possible, + * to produce a more-representative sequence. */ const dr_mcontext_t *source_mcontext; /** diff --git a/core/unix/os_exports.h b/core/unix/os_exports.h index b87d2435c83..01b71577f5d 100644 --- a/core/unix/os_exports.h +++ b/core/unix/os_exports.h @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2011-2020 Google, Inc. All rights reserved. + * Copyright (c) 2011-2021 Google, Inc. All rights reserved. * Copyright (c) 2000-2010 VMware, Inc. All rights reserved. * **********************************************************/ @@ -546,6 +546,9 @@ bool rseq_get_region_info(app_pc pc, app_pc *start OUT, app_pc *end OUT, app_pc *handler OUT, bool **reg_written OUT, int *reg_written_size OUT); +bool +rseq_set_final_instr_pc(app_pc start, app_pc final_instr_pc); + int rseq_get_tls_ptr_offset(void); diff --git a/core/unix/rseq_linux.c b/core/unix/rseq_linux.c index 310018e0705..0c0623fc57f 100644 --- a/core/unix/rseq_linux.c +++ b/core/unix/rseq_linux.c @@ -88,6 +88,7 @@ typedef struct _rseq_region_t { app_pc start; app_pc end; app_pc handler; + app_pc final_instr_pc; /* We need to preserve input registers for targeting "start" instead of "handler" * for our 2nd invocation, if they're written in the rseq region. We only support * GPR inputs. We document that we do not support any other inputs (no flags, no @@ -188,6 +189,18 @@ rseq_get_region_info(app_pc pc, app_pc *start OUT, app_pc *end OUT, app_pc *hand return true; } +bool +rseq_set_final_instr_pc(app_pc start, app_pc final_instr_pc) +{ + rseq_region_t *info; + if (!vmvector_lookup_data(d_r_rseq_areas, start, NULL, NULL, (void **)&info)) + return false; + if (final_instr_pc < start || final_instr_pc >= info->end) + return false; + info->final_instr_pc = final_instr_pc; + return true; +} + int rseq_get_tls_ptr_offset(void) { @@ -378,6 +391,7 @@ rseq_process_entry(struct rseq_cs *entry, ssize_t load_offs) info->start = (app_pc)(ptr_uint_t)entry->start_ip + load_offs; info->end = info->start + entry->post_commit_offset; info->handler = (app_pc)(ptr_uint_t)entry->abort_ip + load_offs; + info->final_instr_pc = NULL; /* Only set later at block building time. */ int signature; if (!d_r_safe_read(info->handler - sizeof(signature), sizeof(signature), &signature)) { @@ -740,14 +754,29 @@ rseq_process_native_abort(dcontext_t *dcontext) { /* Raise a transfer event. */ LOG(THREAD, LOG_INTERP | LOG_VMAREAS, 2, "Abort triggered in rseq native code\n"); + /* We do not know the precise interruption point but we try to present something + * reasonable. + */ + rseq_region_t *info = NULL; + priv_mcontext_t *source_mc = NULL; + if (dcontext->last_fragment != NULL && + vmvector_lookup_data(d_r_rseq_areas, dcontext->last_fragment->tag, NULL, NULL, + (void **)&info)) { + /* An artifact of our run-twice solution is that clients have already seen + * the whole sequence when any abort anywhere in the native execution occurs. + * We thus give the source PC as the final instr in the region, and use the + * target context as the rest of the context. + */ + source_mc = HEAP_TYPE_ALLOC(dcontext, priv_mcontext_t, ACCT_CLIENT, PROTECTED); + *source_mc = *get_mcontext(dcontext); + source_mc->pc = info->final_instr_pc; + } get_mcontext(dcontext)->pc = dcontext->next_tag; - if (instrument_kernel_xfer(dcontext, DR_XFER_RSEQ_ABORT, osc_empty, - /* We do not know the source PC so we do not - * supply a source state. - */ - NULL, NULL, dcontext->next_tag, - get_mcontext(dcontext)->xsp, osc_empty, + if (instrument_kernel_xfer(dcontext, DR_XFER_RSEQ_ABORT, osc_empty, NULL, source_mc, + dcontext->next_tag, get_mcontext(dcontext)->xsp, osc_empty, get_mcontext(dcontext), 0)) { dcontext->next_tag = canonicalize_pc_target(dcontext, get_mcontext(dcontext)->pc); } + if (source_mc != NULL) + HEAP_TYPE_FREE(dcontext, source_mc, priv_mcontext_t, ACCT_CLIENT, PROTECTED); } diff --git a/suite/tests/linux/rseq.c b/suite/tests/linux/rseq.c index 6aa5ccfdb4d..d4939010c03 100644 --- a/suite/tests/linux/rseq.c +++ b/suite/tests/linux/rseq.c @@ -59,6 +59,7 @@ #include #include #include +#undef NDEBUG #include #define EXPANDSTR(x) #x @@ -95,6 +96,9 @@ static __thread volatile struct rseq rseq_tls; /* Make it harder to find rseq_tls for DR's heuristic by adding more static TLS. */ static __thread volatile struct rseq fill_up_tls[128]; +extern void +test_rseq_native_abort_pre_commit(); + #ifdef RSEQ_TEST_ATTACH static atomic_int exit_requested; static void *thread_ready; @@ -602,6 +606,8 @@ test_rseq_native_abort(void) "leaq sched_mask_2(%%rip), %%rdx\n\t" "mov %[sysnum_setaffinity], %%eax\n\t" "syscall\n\t" + ".global test_rseq_native_abort_pre_commit\n\t" + "test_rseq_native_abort_pre_commit:\n\t" "11:\n\t" "nop\n\t" @@ -670,6 +676,8 @@ test_rseq_native_abort(void) "add x2, x2, :lo12:sched_mask_2\n\t" "mov w8, #%[sysnum_setaffinity]\n\t" "svc #0\n\t" + ".global test_rseq_native_abort_pre_commit\n\t" + "test_rseq_native_abort_pre_commit:\n\t" "11:\n\t" "nop\n\t" @@ -906,6 +914,32 @@ kernel_xfer_event(void *drcontext, const dr_kernel_xfer_info_t *info) mc.flags = DR_MC_ALL; ok = dr_get_mcontext(drcontext, &mc); assert(ok); + /* All transfer cases in this test should have source info. */ + assert(info->source_mcontext != NULL); + if (info->type == DR_XFER_RSEQ_ABORT) { + /* The interrupted context should be identical except the pc. */ + assert(info->source_mcontext->pc != mc.pc); +# ifdef X86 + assert(info->source_mcontext->xax == mc.xax); + assert(info->source_mcontext->xcx == mc.xcx); + assert(info->source_mcontext->xdx == mc.xdx); + assert(info->source_mcontext->xbx == mc.xbx); + assert(info->source_mcontext->xsi == mc.xsi); + assert(info->source_mcontext->xdi == mc.xdi); + assert(info->source_mcontext->xbp == mc.xbp); + assert(info->source_mcontext->xsp == mc.xsp); +# elif defined(AARCH64) + assert(memcmp(&info->source_mcontext->r0, &mc.r0, sizeof(mc.r0) * 32) == 0); +# else +# error Unsupported arch +# endif +# ifdef DEBUG /* See above: special code in core/ is DEBUG-only> */ + /* Check that the interrupted PC for the true abort case is *prior* to the + * committing store. + */ + assert(info->source_mcontext->pc == (app_pc)test_rseq_native_abort_pre_commit); +# endif + } } DR_EXPORT void