Skip to content

Commit

Permalink
i#3801,#3823: Make drreg state restoration more robust (#4944)
Browse files Browse the repository at this point in the history
Revamps the drreg state restoration logic in drreg_event_restore_state to
correctly handle various corner cases like multiple restores (i#4939), multiple
spills, aflags in reg (i#4933), multi-phase use (i#3823) leading to nested
or otherwise overlapping spill regions, and app instrs that look like drreg
spill/restore instrs.

Makes the reconstructed ilist available to drreg_event_restore_state (i#3801).
The additional metadata that is used by the new logic is only whether an instr
is an app or meta instr. For cases when the reconstructed ilist is not
available, we continue to use the old best-effort logic. Highlighted its caveats
in comments.

The new logic looks at the complete ilist to track the movement of the app value
of gprs/aflags across spills and restores. The native value can be in the
gpr/aflags itself or one or more spill slots. We do not forget a spill slot on
seeing one restore (which caused i#4939), but remember that the app value can
still be found in the spill slot, in addition to the gpr itself. Instead, a
spill slot is forgotten when it is used to spill some other reg or if there's an
app instr that writes to the spilled gpr/aflags.

We also track whether gprs/aflags contain their native app value or not. This
helps in handling cases of overlapping or nested spill regions arising out of
multi-phase use of drreg. Tool writes (except restores) to gprs/aflags clobber
the native value. App writes to gprs/aflags install a native value and
invalidate the value in the gpr's/aflags' spill slot. Restores from spill slots
also install a native value.

Adds tests for multi-phase gpr reservation cases (nested/overlapping spill
regions), multiple spills for gpr and multiple restores for gpr. Tests for these
scenarios for aflags will be added when multi-phase drreg support is added for
aflags.

Tracks movement of aflags to registers more robustly to fix i#4933. Also adds a
test for this case.

Avoids confusing app instrs that write using the stolen reg on AArch64 with
drreg spill or restore instrs. Also adds a test that verifies this fix.

Adds documentation about faulting fragment's reconstructed ilist being available
in restore state event callbacks.

Issue: #4933, #4939, #3823, #3801
Fixes: #4933
Fixes: #4939
  • Loading branch information
abhinav92003 committed Jun 14, 2021
1 parent 4b6d1ca commit f62441b
Show file tree
Hide file tree
Showing 10 changed files with 1,037 additions and 30 deletions.
10 changes: 9 additions & 1 deletion api/docs/bt.dox
Original file line number Diff line number Diff line change
Expand Up @@ -1145,7 +1145,15 @@ Furthermore, if the client's modifications change any part of the machine
state besides the program counter, the client should use
dr_register_restore_state_event() or dr_register_restore_state_ex_event()
(see \ref sec_events_translation) to restore the registers to their
original application values.
original application values. DR attempts to reconstruct the #instrlist_t
for the faulting fragment; this list contains all instrs added by the
basic block event(s) with \p translating set to true, and also DR's own
mangling of some instrs. If this reconstructed #instrlist_t is available,
it will be passed on to the registered callback as part of
#dr_fault_fragment_info_t in #dr_restore_state_info_t. It may not be
available when some or all clients returned #DR_EMIT_STORE_TRANSLATIONS,
or for DR internal reasons when the app code may not be consistent: for
pending deletion or self-modifying fragments.

For meta instructions that do not reference application memory (i.e., they
should not fault), leave the translation field as NULL. A NULL value
Expand Down
3 changes: 3 additions & 0 deletions api/docs/release.dox
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,9 @@ Further non-compatibility-affecting changes include:
enable logic that avoids conflicts in spill slots when drreg is used to reserve
registers in multiple phases.
- Added drmgr_in_emulation_region() for more conveniently handling emulation.
- 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.

**************************************************
<hr>
Expand Down
6 changes: 6 additions & 0 deletions core/arch/asm_defines.asm
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@

/****************************************************/
#if defined(ASSEMBLE_WITH_GAS)
# define START_DATA .data
# define START_FILE .text
# define END_FILE /* nothing */
# define DECLARE_FUNC(symbol) \
Expand Down Expand Up @@ -154,6 +155,7 @@
# define DECL_EXTERN(symbol) /* nothing */
/* include newline so we can put multiple on one line */
# define RAW(n) .byte HEX(n) @N@
# define BYTES_ARR(symbol, n) symbol: .skip n, 0
# define DECLARE_FUNC_SEH(symbol) DECLARE_FUNC(symbol)
# define PUSH_SEH(reg) push reg
# define PUSH_NONCALLEE_SEH(reg) push reg
Expand All @@ -171,6 +173,7 @@
# define VAR_VIA_GOT(base, sym) [sym @GOTOFF + base]
/****************************************************/
#elif defined(ASSEMBLE_WITH_MASM)
#define START_DATA .DATA
# ifdef X64
# define START_FILE \
/* We add blank lines to match the 32-bit line count */ \
Expand Down Expand Up @@ -214,6 +217,7 @@ ASSUME fs:_DATA @N@\
# define DECL_EXTERN(symbol) EXTERN symbol:PROC
/* include newline so we can put multiple on one line */
# define RAW(n) DB HEX(n) @N@
# define BYTES_ARR(symbol, n) symbol byte n dup (0)
# define ADD_STACK_ALIGNMENT_NOSEH sub REG_XSP, FRAME_ALIGNMENT - ARG_SZ
# define RESTORE_STACK_ALIGNMENT add REG_XSP, FRAME_ALIGNMENT - ARG_SZ
# ifdef X64
Expand All @@ -237,6 +241,7 @@ ASSUME fs:_DATA @N@\
# endif
/****************************************************/
#elif defined(ASSEMBLE_WITH_NASM)
# define START_DATA SECTION .data
# define START_FILE SECTION .text
# define END_FILE /* nothing */
/* for MacOS, at least, we have to add _ ourselves */
Expand Down Expand Up @@ -265,6 +270,7 @@ ASSUME fs:_DATA @N@\
# define SEGMEM(seg,mem) [seg:mem]
# define DECL_EXTERN(symbol) EXTERN GLOBAL_REF(symbol)
# define RAW(n) DB HEX(n) @N@
# define BYTES_ARR(symbol, n) symbol times n DB 0
# define DECLARE_FUNC_SEH(symbol) DECLARE_FUNC(symbol)
# define PUSH_SEH(reg) push reg
# define PUSH_NONCALLEE_SEH(reg) push reg
Expand Down
12 changes: 12 additions & 0 deletions core/lib/dr_events.h
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,18 @@ typedef struct _dr_fault_fragment_info_t {
* depending on the type of cache consistency being used by DR.
*/
bool app_code_consistent;
/**
* The recreated ilist for this fragment, which contains instrs added
* by the basic block event(s) with \p translating set to true and also
* DR's own mangling of some instrs. This includes client-added metadata
* in the form of notes and label instrs too. This may be helpful in
* restoring app state on a fault.
* When the recreated ilist is not available, this is set to NULL. This
* may happen when a client returns #DR_EMIT_STORE_TRANSLATIONS, or for
* DR internal reasons when the app code may not be consistent: for pending
* deletion or self-modifying fragments.
*/
instrlist_t *ilist;
} dr_fault_fragment_info_t;

/**
Expand Down
1 change: 1 addition & 0 deletions core/translate.c
Original file line number Diff line number Diff line change
Expand Up @@ -1495,6 +1495,7 @@ recreate_app_state_internal(dcontext_t *tdcontext, priv_mcontext_t *mcontext,
client_info.fragment_info.is_trace = TEST(FRAG_IS_TRACE, f->flags);
client_info.fragment_info.app_code_consistent =
!TESTANY(FRAG_WAS_DELETED | FRAG_SELFMOD_SANDBOXED, f->flags);
client_info.fragment_info.ilist = ilist;
/* i#220/PR 480565: client has option of failing the translation */
if (!instrument_restore_state(tdcontext, restore_memory, &client_info))
res = RECREATE_FAILURE;
Expand Down
Loading

0 comments on commit f62441b

Please sign in to comment.