Skip to content

Commit

Permalink
i#1929 slow memtrace: use drreg in memtrace samples
Browse files Browse the repository at this point in the history 
Switches memtrace_simple and memtrace_x86 to use drreg instead of
hardcoding scratch registers.

Updates the drreg docs about drreg taking one slot away for flags, and
increases the slots used in the instrace samples to match the extra one
asked for here.

Combined with the earlier split into separate text and binary clients, this
resolves the slow memtrace issue.

Fixes #1929

Review-URL: https://codereview.appspot.com/297660043
  • Loading branch information
@derekbruening
derekbruening committed Jul 4, 2016
1 parent f0d2fcd commit 4a587f8
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 61 deletions.
6 changes: 3 additions & 3 deletions api/samples/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,16 +211,16 @@ if (NOT AARCH64) # FIXME i#1569: port to AArch64
endif ()
add_sample_client(bbsize "bbsize.c" "drmgr")
add_sample_client(empty "empty.c" "")
add_sample_client(memtrace_simple "memtrace_simple.c;utils.c" "drmgr;drutil;drx")
add_sample_client(memtrace_simple "memtrace_simple.c;utils.c" "drmgr;drreg;drutil;drx")
add_sample_client(instrace_simple "instrace_simple.c;utils.c" "drmgr;drreg;drx")
if (X86) # FIXME i#1551, i#1569: port to ARM and AArch64
add_sample_client(bbcount "bbcount.c" "drmgr;drreg")
add_sample_client(cbr "cbr.c" "drmgr")
add_sample_client(countcalls "countcalls.c" "")
add_sample_client(div "div.c" "drmgr")
add_sample_client(inc2add "inc2add.c" "")
add_sample_client(memtrace_x86_binary "memtrace_x86.c;utils.c" "drmgr;drutil;drx")
add_sample_client(memtrace_x86_text "memtrace_x86.c;utils.c" "drmgr;drutil;drx")
add_sample_client(memtrace_x86_binary "memtrace_x86.c;utils.c" "drmgr;drreg;drutil;drx")
add_sample_client(memtrace_x86_text "memtrace_x86.c;utils.c" "drmgr;drreg;drutil;drx")
_DR_append_property_string(TARGET memtrace_x86_text COMPILE_DEFINITIONS "OUTPUT_TEXT")
add_sample_client(instrace_x86_binary "instrace_x86.c;utils.c" "drmgr;drreg;drx")
add_sample_client(instrace_x86_text "instrace_x86.c;utils.c" "drmgr;drreg;drx")
Expand Down
3 changes: 2 additions & 1 deletion api/samples/instrace_simple.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,8 @@ event_exit(void)
DR_EXPORT void
dr_client_main(client_id_t id, int argc, const char *argv[])
{
drreg_options_t ops = {sizeof(ops), 2 /*max slots needed*/, false};
/* We need 2 reg slots beyond drreg's eflags slots => 3 slots */
drreg_options_t ops = {sizeof(ops), 3, false};
dr_set_client_name("DynamoRIO Sample Client 'instrace'",
"http://dynamorio.org/issues");
if (!drmgr_init() || drreg_init(&ops) != DRREG_SUCCESS)
Expand Down
3 changes: 2 additions & 1 deletion api/samples/instrace_x86.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ static void instrument_instr(void *drcontext, instrlist_t *ilist, instr_t *where
DR_EXPORT void
dr_client_main(client_id_t id, int argc, const char *argv[])
{
drreg_options_t ops = {sizeof(ops), 2 /*max slots needed*/, false};
/* We need 2 reg slots beyond drreg's eflags slots => 3 slots */
drreg_options_t ops = {sizeof(ops), 3, false};
/* Specify priority relative to other instrumentation operations: */
drmgr_priority_t priority = {
sizeof(priority), /* size of struct */
Expand Down
55 changes: 30 additions & 25 deletions api/samples/memtrace_simple.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
#include <stddef.h> /* for offsetof */
#include "dr_api.h"
#include "drmgr.h"
#include "drreg.h"
#include "drutil.h"
#include "utils.h"

Expand Down Expand Up @@ -235,15 +236,15 @@ insert_save_addr(void *drcontext, instrlist_t *ilist, instr_t *where,
static void
instrument_instr(void *drcontext, instrlist_t *ilist, instr_t *where)
{
reg_id_t reg_ptr = IF_X86_ELSE(DR_REG_XCX, DR_REG_R1);
reg_id_t reg_tmp = IF_X86_ELSE(DR_REG_XBX, DR_REG_R2);
ushort slot_ptr = SPILL_SLOT_2;
ushort slot_tmp = SPILL_SLOT_3;

/* We need two scratch registers */
dr_save_reg(drcontext, ilist, where, reg_ptr, slot_ptr);
dr_save_reg(drcontext, ilist, where, reg_tmp, slot_tmp);

reg_id_t reg_ptr, reg_tmp;
if (drreg_reserve_register(drcontext, ilist, where, NULL, &reg_ptr) !=
DRREG_SUCCESS ||
drreg_reserve_register(drcontext, ilist, where, NULL, &reg_tmp) !=
DRREG_SUCCESS) {
DR_ASSERT(false); /* cannot recover */
return;
}
insert_load_buf_ptr(drcontext, ilist, where, reg_ptr);
insert_save_type(drcontext, ilist, where, reg_ptr, reg_tmp,
(ushort)instr_get_opcode(where));
Expand All @@ -252,36 +253,37 @@ instrument_instr(void *drcontext, instrlist_t *ilist, instr_t *where)
insert_save_pc(drcontext, ilist, where, reg_ptr, reg_tmp,
instr_get_app_pc(where));
insert_update_buf_ptr(drcontext, ilist, where, reg_ptr, sizeof(mem_ref_t));
/* restore scratch registers */
dr_restore_reg(drcontext, ilist, where, reg_ptr, slot_ptr);
dr_restore_reg(drcontext, ilist, where, reg_tmp, slot_tmp);
/* Restore scratch registers */
if (drreg_unreserve_register(drcontext, ilist, where, reg_ptr) != DRREG_SUCCESS ||
drreg_unreserve_register(drcontext, ilist, where, reg_tmp) != DRREG_SUCCESS)
DR_ASSERT(false);
}

/* insert inline code to add a memory reference info entry into the buffer */
static void
instrument_mem(void *drcontext, instrlist_t *ilist, instr_t *where,
opnd_t ref, bool write)
{
reg_id_t reg_ptr = IF_X86_ELSE(DR_REG_XCX, DR_REG_R1);
reg_id_t reg_tmp = IF_X86_ELSE(DR_REG_XBX, DR_REG_R2);
ushort slot_ptr = SPILL_SLOT_2;
ushort slot_tmp = SPILL_SLOT_3;

/* We need two scratch registers */
dr_save_reg(drcontext, ilist, where, reg_ptr, slot_ptr);
dr_save_reg(drcontext, ilist, where, reg_tmp, slot_tmp);

reg_id_t reg_ptr, reg_tmp;
if (drreg_reserve_register(drcontext, ilist, where, NULL, &reg_ptr) !=
DRREG_SUCCESS ||
drreg_reserve_register(drcontext, ilist, where, NULL, &reg_tmp) !=
DRREG_SUCCESS) {
DR_ASSERT(false); /* cannot recover */
return;
}
/* save_addr should be called first as reg_ptr or reg_tmp maybe used in ref */
insert_save_addr(drcontext, ilist, where, ref, reg_ptr, reg_tmp);
insert_save_type(drcontext, ilist, where, reg_ptr, reg_tmp,
write ? REF_TYPE_WRITE : REF_TYPE_READ);
insert_save_size(drcontext, ilist, where, reg_ptr, reg_tmp,
(ushort)drutil_opnd_mem_size_in_bytes(ref, where));
insert_update_buf_ptr(drcontext, ilist, where, reg_ptr, sizeof(mem_ref_t));

/* restore scratch registers */
dr_restore_reg(drcontext, ilist, where, reg_ptr, slot_ptr);
dr_restore_reg(drcontext, ilist, where, reg_tmp, slot_tmp);
/* Restore scratch registers */
if (drreg_unreserve_register(drcontext, ilist, where, reg_ptr) != DRREG_SUCCESS ||
drreg_unreserve_register(drcontext, ilist, where, reg_tmp) != DRREG_SUCCESS)
DR_ASSERT(false);
}

/* For each memory reference app instr, we insert inline code to fill the buffer
Expand Down Expand Up @@ -410,7 +412,8 @@ event_exit(void)
!drmgr_unregister_thread_init_event(event_thread_init) ||
!drmgr_unregister_thread_exit_event(event_thread_exit) ||
!drmgr_unregister_bb_app2app_event(event_bb_app2app) ||
!drmgr_unregister_bb_insertion_event(event_app_instruction))
!drmgr_unregister_bb_insertion_event(event_app_instruction) ||
drreg_exit() != DRREG_SUCCESS)
DR_ASSERT(false);

dr_mutex_destroy(mutex);
Expand All @@ -421,9 +424,11 @@ event_exit(void)
DR_EXPORT void
dr_client_main(client_id_t id, int argc, const char *argv[])
{
/* We need 2 reg slots beyond drreg's eflags slots => 3 slots */
drreg_options_t ops = {sizeof(ops), 3, false};
dr_set_client_name("DynamoRIO Sample Client 'memtrace'",
"http://dynamorio.org/issues");
if (!drmgr_init() || !drutil_init())
if (!drmgr_init() || drreg_init(&ops) != DRREG_SUCCESS || !drutil_init())
DR_ASSERT(false);

/* register events */
Expand Down
63 changes: 32 additions & 31 deletions api/samples/memtrace_x86.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
#include <stddef.h> /* for offsetof */
#include "dr_api.h"
#include "drmgr.h"
#include "drreg.h"
#include "drutil.h"
#include "utils.h"

Expand Down Expand Up @@ -107,10 +108,6 @@ static void event_thread_exit(void *drcontext);
static dr_emit_flags_t event_bb_app2app(void *drcontext, void *tag, instrlist_t *bb,
bool for_trace, bool translating);

static dr_emit_flags_t event_bb_analysis(void *drcontext, void *tag, instrlist_t *bb,
bool for_trace, bool translating,
OUT void **user_data);

static dr_emit_flags_t event_bb_insert(void *drcontext, void *tag, instrlist_t *bb,
instr_t *instr, bool for_trace, bool translating,
void *user_data);
Expand All @@ -128,6 +125,8 @@ static void instrument_mem(void *drcontext,
DR_EXPORT void
dr_client_main(client_id_t id, int argc, const char *argv[])
{
/* We need 2 reg slots beyond drreg's eflags slots => 3 slots */
drreg_options_t ops = {sizeof(ops), 3, false};
/* Specify priority relative to other instrumentation operations: */
drmgr_priority_t priority = {
sizeof(priority), /* size of struct */
Expand All @@ -144,11 +143,9 @@ dr_client_main(client_id_t id, int argc, const char *argv[])
dr_register_exit_event(event_exit);
if (!drmgr_register_thread_init_event(event_thread_init) ||
!drmgr_register_thread_exit_event(event_thread_exit) ||
!drmgr_register_bb_app2app_event(event_bb_app2app,
&priority) ||
!drmgr_register_bb_instrumentation_event(event_bb_analysis,
event_bb_insert,
&priority)) {
!drmgr_register_bb_app2app_event(event_bb_app2app, &priority) ||
!drmgr_register_bb_instrumentation_event(NULL, event_bb_insert, &priority) ||
drreg_init(&ops) != DRREG_SUCCESS) {
/* something is wrong: can't continue */
DR_ASSERT(false);
return;
Expand Down Expand Up @@ -186,7 +183,14 @@ event_exit()
DISPLAY_STRING(msg);
#endif /* SHOW_RESULTS */
code_cache_exit();
drmgr_unregister_tls_field(tls_index);

if (!drmgr_unregister_tls_field(tls_index) ||
!drmgr_unregister_thread_init_event(event_thread_init) ||
!drmgr_unregister_thread_exit_event(event_thread_exit) ||
!drmgr_unregister_bb_insertion_event(event_bb_insert) ||
drreg_exit() != DRREG_SUCCESS)
DR_ASSERT(false);

dr_mutex_destroy(mutex);
drutil_exit();
drmgr_exit();
Expand Down Expand Up @@ -265,17 +269,6 @@ event_bb_app2app(void *drcontext, void *tag, instrlist_t *bb,
return DR_EMIT_DEFAULT;
}

/* our operations here only need to see a single-instruction window so
* we do not need to do any whole-bb analysis
*/
static dr_emit_flags_t
event_bb_analysis(void *drcontext, void *tag, instrlist_t *bb,
bool for_trace, bool translating,
OUT void **user_data)
{
return DR_EMIT_DEFAULT;
}

/* event_bb_insert calls instrument_mem to instrument every
* application memory reference.
*/
Expand Down Expand Up @@ -394,19 +387,26 @@ instrument_mem(void *drcontext, instrlist_t *ilist, instr_t *where,
{
instr_t *instr, *call, *restore, *first, *second;
opnd_t ref, opnd1, opnd2;
reg_id_t reg1 = DR_REG_XBX; /* We can optimize it by picking dead reg */
reg_id_t reg2 = DR_REG_XCX; /* reg2 must be ECX or RCX for jecxz */
reg_id_t reg1, reg2;
drvector_t allowed;
per_thread_t *data;
app_pc pc;

data = drmgr_get_tls_field(drcontext, tls_index);

/* Steal the register for memory reference address *
* We can optimize away the unnecessary register save and restore
* by analyzing the code and finding the register is dead.
/* Steal two scratch registers.
* reg2 must be ECX or RCX for jecxz.
*/
dr_save_reg(drcontext, ilist, where, reg1, SPILL_SLOT_2);
dr_save_reg(drcontext, ilist, where, reg2, SPILL_SLOT_3);
drreg_init_and_fill_vector(&allowed, false);
drreg_set_vector_entry(&allowed, DR_REG_XCX, true);
if (drreg_reserve_register(drcontext, ilist, where, &allowed, &reg2) !=
DRREG_SUCCESS ||
drreg_reserve_register(drcontext, ilist, where, NULL, &reg1) != DRREG_SUCCESS) {
DR_ASSERT(false); /* cannot recover */
drvector_delete(&allowed);
return;
}
drvector_delete(&allowed);

if (write)
ref = instr_get_dst(where, pos);
Expand Down Expand Up @@ -524,8 +524,9 @@ instrument_mem(void *drcontext, instrlist_t *ilist, instr_t *where,
instr = INSTR_CREATE_jmp(drcontext, opnd1);
instrlist_meta_preinsert(ilist, where, instr);

/* restore %reg */
/* Restore scratch registers */
instrlist_meta_preinsert(ilist, where, restore);
dr_restore_reg(drcontext, ilist, where, reg1, SPILL_SLOT_2);
dr_restore_reg(drcontext, ilist, where, reg2, SPILL_SLOT_3);
if (drreg_unreserve_register(drcontext, ilist, where, reg1) != DRREG_SUCCESS ||
drreg_unreserve_register(drcontext, ilist, where, reg2) != DRREG_SUCCESS)
DR_ASSERT(false);
}
1 change: 1 addition & 0 deletions ext/drreg/drreg.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ typedef struct _drreg_options_t {
* across application instructions, an additional slot must be
* requested for adjusting the saved application value with
* respect to application reads and writes.
* drreg always reserves one slot for use in preserving the arithmetic flags.
*/
uint num_spill_slots;
/**
Expand Down

0 comments on commit 4a587f8

Please sign in to comment.