Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i#5121: Add test for getting app value in multi-phase drreg #5126

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

i#5121: Add test for getting app value in multi-phase drreg #5126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
25ca8fd
i#5121: Add test for getting app value in multi-phase drreg
abhinav92003 Sep 26, 2021
7da2d72
Remove debug statement.
abhinav92003 Sep 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions ext/drreg/drreg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,11 @@ DR_EXPORT
* \p *swap. It is up to the caller to un-reserve the register in
* that case.
*
* For regs that were reserved in a prior phase also, this routine
* restores the reg's meta value from that prior phase, OR if the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem good enough: users don't know what registers the scatter/gather expansion used, and so users can't write correct clients. It seems like we have to not overlap scratch registers between app2app and insertion or something, or have the scatter expansion provide guarantees on its behavior so a client knows where it can get app values.

A real-world case to focus on is #3995 where one method of implementation is phases of instrumentation implemented with flushes. With flushes invoked from clean calls, the call has to get the proper app state. Maybe having just one point at the last instruction of the basic block where the state is available is good enough there.

* current phase is the first one where the reg is reserved, it
* restores the actual app value.
*
* @return whether successful or an error code on failure. On failure,
* any values that were already restored are not undone.
*/
Expand Down Expand Up @@ -508,6 +513,10 @@ DR_EXPORT
*
* On ARM, passing \p reg equal to dr_get_stolen_reg() is not supported.
*
* For regs that were reserved in a prior phase also, this routine restores the reg's
* meta value from that prior phase, OR if the current phase is the first one where the
* reg is reserved, it restores the actual app value.
*
* @return whether successful or an error code on failure.
*/
drreg_status_t
Expand Down
3 changes: 3 additions & 0 deletions suite/tests/client-interface/drreg-test-shared.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,3 +223,6 @@

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

#define DRREG_TEST_39_ASM MAKE_HEX_ASM(DRREG_TEST_CONST(39))
#define DRREG_TEST_39_C MAKE_HEX_C(DRREG_TEST_CONST(39))
64 changes: 63 additions & 1 deletion suite/tests/client-interface/drreg-test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1021,10 +1021,72 @@ GLOBAL_LABEL(FUNCNAME:)
jne test28_fail
cmp al, 1
jne test28_fail
jmp epilog
jmp test39
test28_fail:
ud2
/* Unreachable, but we want this bb to end here. */
jmp test39

/* Test 39: Get app value of register spilled in multiple phases.
* This test demonstrates how to get the app value for a reg that
* was reserved in multiple phases (app2app and insertion phase).
*
* TODO i#5121: Simplify getting the app value for a reg reserved
* in multiple phases.
*/
test39:
mov TEST_REG_ASM, DRREG_TEST_39_ASM
mov TEST_REG_ASM, DRREG_TEST_39_ASM

/* app2app phase reserves TEST_REG_ASM here. */
/* app2app phase writes TEST_REG_ASM here. */
/* insertion phase reserves TEST_REG_ASM here. */
/* insertion phase writes TEST_REG_ASM here. */
mov TEST_REG2_ASM, TEST_INSTRUMENTATION_MARKER_1
/***********************************************************
* This section demonstrates how to get the app2app value of
* TEST_REG_ASM from insertion phase. See DRREG_TEST_39_C in
* event_app_instruction for code.
*/
/* insertion phase gets the app2app value of TEST_REG_ASM
* here using drreg_get_app_value or
* drreg_statelessly_restore_app_value.
*/
/***********************************************************
* Done.
*/
mov TEST_REG2_ASM, TEST_INSTRUMENTATION_MARKER_2
/**********************************************************
* This section demonstrates how to get the app value of
* TEST_REG_ASM from insertion phase. This requires the
* app2app and insertion phases to work together. See
* DRREG_TEST_39_C in event_app2app and event_app_instruction
* for code.
*/
/* app2app phase gets the app value of TEST_REG_ASM here. */
/* Seeing the above write, insertion phase automatically
* re-spills the new value of TEST_REG_ASM to its own slot,
* thus making it available using drreg_get_app_value or
* drreg_statelessly_get_app_value in the insertion phase.
*/
/* insertion phase gets the app value of TEST_REG_ASM here
* using drreg_get_app_value or drreg_statelessly_restore_app_value.
*/
/* app2app inserts a label that marks where insertion phase
* should try to get the app value.
*/
/* app2app adds a dummy instr that reads TEST_REG_ASM, just to
* make sure it is live (if it is dead then insertion phase won't
* re-spill the new value to its slot).
*/
/**********************************************************
* Done.
*/
mov TEST_REG2_ASM, TEST_INSTRUMENTATION_MARKER_3
/* app2app phase unreserves TEST_REG_ASM here. */
/* insertion phase unreserves TEST_REG_ASM here. */
jmp test39_done
test39_done:
jmp epilog

epilog:
Expand Down
117 changes: 115 additions & 2 deletions suite/tests/client-interface/drreg-test.dll.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ static ptr_uint_t note_base;
#define NOTE_VAL(enum_val) ((void *)(ptr_int_t)(note_base + (enum_val)))

/* Enum describing the different types of notes in drreg-test. */
enum { DRREG_TEST_LABEL_MARKER, DRREG_TEST_NOTE_COUNT };
enum {
DRREG_TEST_LABEL_MARKER,
DRREG_TEST_APP_VAL_RESTORE_MARKER,
DRREG_TEST_NOTE_COUNT
};

uint tls_offs_app2app_spilled_aflags;
uint tls_offs_app2app_spilled_reg;
Expand All @@ -69,6 +73,15 @@ is_drreg_test_label_marker(instr_t *inst)
return false;
}

static bool
is_drreg_app_val_restore_marker(instr_t *inst)
{
if (instr_is_label(inst) &&
instr_get_note(inst) == NOTE_VAL(DRREG_TEST_APP_VAL_RESTORE_MARKER))
return true;
return false;
}

static uint
spill_test_reg_to_slot(void *drcontext, instrlist_t *bb, instr_t *inst,
reg_id_t reg_to_reserve, drvector_t *allowed, bool overwrite)
Expand Down Expand Up @@ -380,6 +393,60 @@ event_app2app(void *drcontext, void *tag, instrlist_t *bb, bool for_trace,
"cannot unreserve aflags");
}
}
} else if (subtest == DRREG_TEST_39_C) {
CHECK(drreg_set_bb_properties(
drcontext, DRREG_HANDLE_MULTI_PHASE_SLOT_RESERVATIONS) == DRREG_SUCCESS,
"unable to set bb properties");
/* Reset for this bb. */
tls_offs_app2app_spilled_reg = -1;
dr_log(drcontext, DR_LOG_ALL, 1, "drreg test #39: app2app phase\n");
for (inst = instrlist_first_app(bb); inst != NULL;
inst = instr_get_next_app(inst)) {
if (instr_is_mov_constant(inst, &val) &&
val == TEST_INSTRUMENTATION_MARKER_1) {
tls_offs_app2app_spilled_reg =
spill_test_reg_to_slot(drcontext, bb, inst, TEST_REG, &allowed, true);
} else if (instr_is_mov_constant(inst, &val) &&
val == TEST_INSTRUMENTATION_MARKER_3) {
/* Make sure that the app2app value of TEST_REG isn't dead. If it is
* dead, the insertion phase spill will only reserve a slot, but not
* actually write to it.
*/
instrlist_meta_preinsert(bb, inst,
XINST_CREATE_add(drcontext,
opnd_create_reg(TEST_REG),
OPND_CREATE_INT32(1)));

/*******************************************************************
* Read app value for reg reserved in app2app and insertion phases:
* step 1/2 (here): get app value in app2app phase
* step 2/2: See DRREG_TEST_39_C in event_app_instruction
*/
CHECK(drreg_get_app_value(drcontext, bb, inst, TEST_REG, TEST_REG) ==
DRREG_SUCCESS,
"unable to get app value");
instr_t *label = INSTR_CREATE_label(drcontext);
instr_set_note(label, NOTE_VAL(DRREG_TEST_APP_VAL_RESTORE_MARKER));
instrlist_meta_preinsert(bb, inst, label);
/* Make sure that the TEST_REG value is live, so that the insertion
* phase re-spills the app value to its slot. This makes the app
* value available to the insertion phase.
*/
instrlist_meta_preinsert(bb, inst,
XINST_CREATE_add(drcontext,
opnd_create_reg(TEST_REG),
OPND_CREATE_INT32(0)));

/*******************************************************************
* Step 1 done.
*/

} else if (inst == instrlist_last(bb)) {
CHECK(drreg_unreserve_register(drcontext, bb, inst, TEST_REG) ==
DRREG_SUCCESS,
"cannot unreserve register");
}
}
}
drvector_delete(&allowed);
return DR_EMIT_DEFAULT;
Expand Down Expand Up @@ -1042,6 +1109,52 @@ event_app_instruction(void *drcontext, void *tag, instrlist_t *bb, instr_t *inst
CHECK(drreg_unreserve_aflags(drcontext, bb, inst) == DRREG_SUCCESS,
"cannot unreserve aflags");
}
} else if (subtest == DRREG_TEST_39_C) {
dr_log(drcontext, DR_LOG_ALL, 1, "drreg test #39: insertion phase\n");
if (instr_is_mov_constant(inst, &val) && val == TEST_INSTRUMENTATION_MARKER_1) {
CHECK(tls_offs_app2app_spilled_reg != -1,
"unable to use any spill slot in app2app phase.");
uint tls_offs = spill_test_reg_to_slot(drcontext, bb, inst, TEST_REG,
&allowed_test_reg_1, false);
/* Overwrite with a different constant than app2app. */
instrlist_meta_preinsert(bb, inst,
XINST_CREATE_load_int(drcontext,
opnd_create_reg(TEST_REG),
OPND_CREATE_INT32(0x8bad)));
CHECK(tls_offs_app2app_spilled_reg != tls_offs,
"found conflict in use of spill slots across multiple phases");
} else if (instr_is_mov_constant(inst, &val) &&
val == TEST_INSTRUMENTATION_MARKER_2) {
/**********************************************************************
* Read app2app value for reg reserved in app2app and insertion phases.
*/
CHECK(drreg_statelessly_restore_app_value(drcontext, bb, TEST_REG, inst, inst,
NULL, NULL) == DRREG_SUCCESS,
"unable to get app value");
dr_insert_clean_call(drcontext, bb, inst, (void *)check_const_eq, false, 2,
opnd_create_reg(TEST_REG), OPND_CREATE_INT32(MAGIC_VAL));
}
/*******************************************************************
* Read app value for reg reserved in app2app and insertion phases:
* step 1/2: See DRREG_TEST_39_C in event_app2app
* step 2/2 (here): get value from insertion phase slot.
*/
else if (is_drreg_app_val_restore_marker(inst)) {
CHECK(drreg_statelessly_restore_app_value(drcontext, bb, TEST_REG, inst, inst,
NULL, NULL) == DRREG_SUCCESS,
"unable to get app value");
dr_insert_clean_call(drcontext, bb, inst, (void *)check_const_eq, false, 2,
opnd_create_reg(TEST_REG),
OPND_CREATE_INT32(DRREG_TEST_39_C));
}
/*******************************************************************
* Step 2 done.
*/
else if (drmgr_is_last_instr(drcontext, inst)) {
CHECK(drreg_unreserve_register(drcontext, bb, inst, TEST_REG) ==
DRREG_SUCCESS,
"cannot unreserve register");
}
}
#ifdef X86
drvector_delete(&allowed_test_reg_xax);
Expand Down Expand Up @@ -1081,7 +1194,7 @@ event_instru2instru(void *drcontext, void *tag, instrlist_t *bb, bool for_trace,
subtest == DRREG_TEST_31_C || subtest == DRREG_TEST_32_C ||
subtest == DRREG_TEST_33_C || subtest == DRREG_TEST_34_C ||
subtest == DRREG_TEST_35_C || subtest == DRREG_TEST_36_C ||
subtest == DRREG_TEST_37_C) {
subtest == DRREG_TEST_37_C || subtest == DRREG_TEST_39_C) {
return DR_EMIT_DEFAULT;
}

Expand Down