Skip to content

Commit

Permalink
i#4963 drreg state restore: handle non-drreg spills (#5000)
Browse files Browse the repository at this point in the history
Revamps drreg's state restoration logic (again) to obviate the need to observe
non-drreg spills and restores.

The previous strategy of tracking the app value across spills and restores
required drreg to be aware of non-drreg spilling mechanisms like spilling to
mcontext, to stack, or any other possible methods. This made it non-ideal.

Implements a new strategy for state restoration that does not need to be aware
of non-drreg spills, and also handles the various tricky drreg cases like
multi-phase use. The key observation behind this is that it is easier to find
the matching spill for a given restore, than to find the matching restore for a
given spill. This is because there may be other restores besides the final
restore (e.g. restores for app read, user prompted restores, etc.). This makes
it hard to find exactly where the spill region for a reg/aflags ends. Additional
complexities include the fact that aflags re-spills may not use the same slot,
which makes differentiating spills from multiple phases difficult. Now we
walk the ilist backwards from the last instr to the faulting instr. The first
restore (or last restore in the list) found is expected to be an app value
restore. We find the matching spill for that restore which uses the same slot
which has to be an app value spill too. We continue doing this until we reach
the faulting instr and if there's a spill-restore pair where the spill is before
the faulting instr, we use that slot to restore the reg/aflags. This way we see
only app value spills/restores and skip over tool value spills/restores.

Also makes a small fix in instr_is_reg_spill_or_restore_ex to not return true
for mcontext base loads to regs, which is not strictly a reg restore operation.

Adds multiple new test cases that use non-drreg spilling mechanisms: spilling
to mcontext, clean call instrumentation. Also modifies some previous tests so
that the interesting part of the test happens after the faulting instr; this is
required because we now walk backwards in the ilist, not forwards.

Verified on a GA runner that the original burst_threads failure didn't occur in
500 runs.

Issue: #3823
Fixes: #4963
  • Loading branch information
abhinav92003 authored Jul 12, 2021
1 parent c180473 commit be78c12
Show file tree
Hide file tree
Showing 7 changed files with 1,304 additions and 316 deletions.
3 changes: 2 additions & 1 deletion core/ir/instr_shared.c
Original file line number Diff line number Diff line change
Expand Up @@ -3697,7 +3697,8 @@ instr_is_reg_spill_or_restore_ex(void *drcontext, instr_t *instr, bool DR_only,
if (reg == NULL)
reg = &myreg;
if (instr_check_tls_spill_restore(instr, spill, reg, &check_disp)) {
if (!DR_only ||
/* We do not want to count an mcontext base load as a reg spill/restore. */
if ((!DR_only && check_disp != os_tls_offset((ushort)TLS_DCONTEXT_SLOT)) ||
(reg_spill_tls_offs(*reg) != -1 &&
/* Mangling may choose to spill registers to a not natural tls offset,
* e.g. rip-rel mangling will, if rax is used by the instruction. We
Expand Down
367 changes: 175 additions & 192 deletions ext/drreg/drreg.c

Large diffs are not rendered by default.

7 changes: 4 additions & 3 deletions ext/drx/drx.c
Original file line number Diff line number Diff line change
Expand Up @@ -2540,9 +2540,10 @@ drx_expand_scatter_gather(void *drcontext, instrlist_t *bb, OUT bool *expanded)
* in between recognized states. This is an approximation and could be broken in many
* ways, e.g. by a client adding more than DRX_RESTORE_EVENT_SKIP_UNKNOWN_INSTR_MAX
* number of instructions as instrumentation, or by altering the emulation sequence's
* code. A more safe way to do this would be along the lines of xref i#3801: if we had
* instruction lists available, we could see and pass down emulation labels instead of
* guessing the sequence based on decoding the code cache.
* code.
* TODO i#5005: A more safe way to do this would be along the lines of xref i#3801: if
* we had instruction lists available, we could see and pass down emulation labels
* instead of guessing the sequence based on decoding the code cache.
*
* AVX-512 gather sequence detection example:
*
Expand Down
40 changes: 39 additions & 1 deletion suite/tests/client-interface/drreg-test-shared.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,31 +33,45 @@
#ifdef X86
# define TEST_REG DR_REG_XDX
# define TEST_REG2 DR_REG_XBX
# define TEST_REG_CLEAN_CALL_MCONTEXT DR_REG_XAX
# define TEST_REG_ASM REG_XDX
# define TEST_REG2_ASM REG_XBX
# define TEST_REG_CLEAN_CALL_MCONTEXT_ASM REG_XAX
# define TEST_REG_ASM_LSB dl
# define TEST_REG_CXT IF_X64_ELSE(Rdx, Edx)
# define TEST_XAX_CXT IF_X64_ELSE(Rax, Eax)
# define TEST_REG_CLEAN_CALL_MCONTEXT_CXT IF_X64_ELSE(Rax, Eax)
# define TEST_REG_SIG SC_XDX
# define TEST_REG_CLEAN_CALL_MCONTEXT_SIG SC_XAX
#endif

#ifdef ARM
# define TEST_REG DR_REG_R12
# define TEST_REG2 DR_REG_R13
# define TEST_REG3 DR_REG_R14
# define TEST_REG_CLEAN_CALL_MCONTEXT DR_REG_R0
# define TEST_REG_ASM r12
# define TEST_REG2_ASM r13
# define TEST_REG3_ASM r14
# define TEST_REG_CLEAN_CALL_MCONTEXT_ASM r0
# define TEST_REG_SIG arm_ip
# define TEST_REG_CLEAN_CALL_MCONTEXT_SIG arm_r0
#endif

#ifdef AARCH64
# define TEST_REG DR_REG_X4
# define TEST_REG2 DR_REG_X5
# define TEST_REG3 DR_REG_X6
# define TEST_REG_CLEAN_CALL_MCONTEXT DR_REG_X0
# define TEST_REG_STOLEN DR_REG_X28
# define TEST_REG_ASM x4
# define TEST_REG2_ASM x5
# define TEST_REG3_ASM x6
# define TEST_REG_CLEAN_CALL_MCONTEXT_ASM x0
# define TEST_REG_STOLEN_ASM x28
# define TEST_REG_SIG regs[4]
# define TEST_FAUX_SPILL_TLS_OFFS 0x148
# define TEST_REG_CLEAN_CALL_MCONTEXT_SIG regs[0]
# define TEST_FAUX_SPILL_TLS_OFFS 0x150
#endif

#define TEST_FLAGS_SIG SC_XFLAGS
Expand Down Expand Up @@ -185,3 +199,27 @@

#define DRREG_TEST_29_ASM MAKE_HEX_ASM(DRREG_TEST_CONST(29))
#define DRREG_TEST_29_C MAKE_HEX_C(DRREG_TEST_CONST(29))

#define DRREG_TEST_30_ASM MAKE_HEX_ASM(DRREG_TEST_CONST(30))
#define DRREG_TEST_30_C MAKE_HEX_C(DRREG_TEST_CONST(30))

#define DRREG_TEST_31_ASM MAKE_HEX_ASM(DRREG_TEST_CONST(31))
#define DRREG_TEST_31_C MAKE_HEX_C(DRREG_TEST_CONST(31))

#define DRREG_TEST_32_ASM MAKE_HEX_ASM(DRREG_TEST_CONST(32))
#define DRREG_TEST_32_C MAKE_HEX_C(DRREG_TEST_CONST(32))

#define DRREG_TEST_33_ASM MAKE_HEX_ASM(DRREG_TEST_CONST(33))
#define DRREG_TEST_33_C MAKE_HEX_C(DRREG_TEST_CONST(33))

#define DRREG_TEST_34_ASM MAKE_HEX_ASM(DRREG_TEST_CONST(34))
#define DRREG_TEST_34_C MAKE_HEX_C(DRREG_TEST_CONST(34))

#define DRREG_TEST_35_ASM MAKE_HEX_ASM(DRREG_TEST_CONST(35))
#define DRREG_TEST_35_C MAKE_HEX_C(DRREG_TEST_CONST(35))

#define DRREG_TEST_36_ASM MAKE_HEX_ASM(DRREG_TEST_CONST(36))
#define DRREG_TEST_36_C MAKE_HEX_C(DRREG_TEST_CONST(36))

#define DRREG_TEST_37_ASM MAKE_HEX_ASM(DRREG_TEST_CONST(37))
#define DRREG_TEST_37_C MAKE_HEX_C(DRREG_TEST_CONST(37))
Loading

0 comments on commit be78c12

Please sign in to comment.