Skip to content

Commit

Permalink
i#2062: Support filtering of non-module blocks
Browse files Browse the repository at this point in the history
Adds support for instrumenting non-module code instr-by-instr instead of
whole-block-at-a-time. This is required for the L0 filter mode which
instruments individual instructions rather than one pc entry for the
whole block.

Adds a new variant of the tool.drcacheoff.gencode test that runs with
the L0_filter enabled. Modifies the test to add a sequence of instrs
that produces an error in raw2trace due to an apparently out-of-block
memref.

This changes how we store the location of non-module code in trace PC
entries. Previously all PC_MODOFFS_BITS (33)
bits were used to store the gencode block id, with no bits assigned to
offset into the block. Now we use PC_BLOCKOFFS_BITS
(equal to PC_INSTR_COUNT_BITS, 12) bits for offset into the module
 and PC_BLOCKIDX_BITS (21) bits for block idx.
  • Loading branch information
abhinav92003 committed Nov 21, 2023
1 parent b9bb6d0 commit bd56135
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 21 deletions.
10 changes: 10 additions & 0 deletions clients/drcachesim/common/trace_entry.h
Original file line number Diff line number Diff line change
Expand Up @@ -809,6 +809,16 @@ typedef enum {
#define PC_MODIDX_INVALID ((1 << PC_MODIDX_BITS) - 1)
#define PC_INSTR_COUNT_BITS 12
#define PC_TYPE_BITS 3
// Block idx and block offset are stored in the slot of module offset for
// generated code.
// We let PC_BLOCKOFFS_BITS be large enough to accommodate roughly the
// max instr count we can store in a PC trace entry.
#define PC_BLOCKOFFS_BITS (PC_INSTR_COUNT_BITS + 2)
// TODO i#2062: We can have only 2^19 gencode blocks with this configuration.
// Allow more gencode blocks by using multiple modidx (and not just
// PC_MODIDX_INVALID) for pointing to non-module code, growing downward from
// PC_MODIDX_INVALID.
#define PC_BLOCKIDX_BITS (PC_MODOFFS_BITS - PC_BLOCKOFFS_BITS)

#define OFFLINE_FILE_VERSION_NO_ELISION 2
#define OFFLINE_FILE_VERSION_OLDEST_SUPPORTED OFFLINE_FILE_VERSION_NO_ELISION
Expand Down
55 changes: 41 additions & 14 deletions clients/drcachesim/tests/burst_gencode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,23 @@ class code_generator_t {
XINST_CREATE_store(dc, OPND_CREATE_MEMPTR(base, -ptrsz),
opnd_create_reg(base)));

// Test raw2trace for filtered traces.
instr_t *nop1 = XINST_CREATE_nop(dc);
// Start new basic block.
instrlist_append(ilist, XINST_CREATE_jump(dc, opnd_create_instr(nop1)));
// First instr is one without a memref.
instrlist_append(ilist, nop1);
// Second instr has a memref. If raw2trace always picks the first bb instr for
// gencode bb instrs, this will result in a "memref entry found outside of bb"
// error.
instrlist_append(ilist,
XINST_CREATE_load_int(dc, opnd_create_reg(base4imm),
OPND_CREATE_INT32(kGencodeMagic2)));
instr_t *nop2 = XINST_CREATE_nop(dc);
instrlist_append(ilist, XINST_CREATE_jump(dc, opnd_create_instr(nop2)));
// End basic block.
instrlist_append(ilist, nop2);

#ifdef LINUX
// Test a signal in non-module code.
# ifdef X86
Expand Down Expand Up @@ -287,38 +304,40 @@ post_process()
}

static std::string
gather_trace()
gather_trace(const std::string &add_env)
{
#ifdef LINUX
intercept_signal(SIGILL, handle_signal, false);
#endif

if (!my_setenv("DYNAMORIO_OPTIONS",
std::string env =
#if defined(LINUX) && defined(X64)
// We pass -satisfy_w_xor_x to further stress that option
// interacting with standalone mode (xref i#5621).
"-satisfy_w_xor_x "
// We pass -satisfy_w_xor_x to further stress that option
// interacting with standalone mode (xref i#5621).
"-satisfy_w_xor_x "
#endif
"-stderr_mask 0xc -client_lib ';;-offline"))
"-stderr_mask 0xc -client_lib ';;-offline";
env += add_env;
if (!my_setenv("DYNAMORIO_OPTIONS", env.c_str()))
std::cerr << "failed to set env var!\n";
code_generator_t gen(false);
std::cerr << "pre-DR init\n";
std::cerr << "pre-DR init\n" << std::flush;
dr_app_setup();
assert(!dr_app_running_under_dynamorio());
drmemtrace_status_t res = drmemtrace_buffer_handoff(nullptr, exit_cb, nullptr);
assert(res == DRMEMTRACE_SUCCESS);
std::cerr << "pre-DR start\n";
std::cerr << "pre-DR start\n" << std::flush;
dr_app_start();
if (do_some_work(gen) < 0)
std::cerr << "error in computation\n";
std::cerr << "pre-DR detach\n";
std::cerr << "pre-DR detach\n" << std::flush;
dr_app_stop_and_cleanup();
std::cerr << "all done\n";
std::cerr << "all done\n" << std::flush;
return post_process();
}

static int
look_for_gencode(std::string trace_dir)
look_for_gencode(std::string trace_dir, bool look_for_magic)
{
void *dr_context = dr_standalone_init();
scheduler_t scheduler;
Expand Down Expand Up @@ -372,15 +391,23 @@ look_for_gencode(std::string trace_dir)
instr_free(dr_context, &instr);
}
dr_standalone_exit();
assert(found_magic2);
assert(!look_for_magic || found_magic2);
return 0;
}

int
test_main(int argc, const char *argv[])
{
std::string trace_dir = gather_trace();
return look_for_gencode(trace_dir);
std::string extra_client_opts = "";
for (int i = 1; i < argc; ++i) {
extra_client_opts += " ";
extra_client_opts += argv[i];
}
std::string trace_dir = gather_trace(extra_client_opts);
bool look_for_magic = true;
if (extra_client_opts.find("-L0_filter") != std::string::npos)
look_for_magic = false;
return look_for_gencode(trace_dir, look_for_magic);
}

} // namespace drmemtrace
Expand Down
6 changes: 6 additions & 0 deletions clients/drcachesim/tests/offline-gencode_filtered.templatex
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pre-DR init
pre-DR start
pre-DR detach
all done
Opcode mix tool results:
.*
1 change: 1 addition & 0 deletions clients/drcachesim/tracer/instru.h
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ class offline_instru_t : public instru_t {
struct per_block_t {
uint64_t id = 0;
uint instr_count = 0;
app_pc start_pc = 0;
};

bool
Expand Down
10 changes: 8 additions & 2 deletions clients/drcachesim/tracer/instru_offline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,11 @@ offline_instru_t::insert_save_pc(void *drcontext, instrlist_t *ilist, instr_t *w
modidx = PC_MODIDX_INVALID;
// For generated code we store the id for matching with the encodings recorded
// into the encoding file.
modoffs = per_block->id;
uint64 blockoffs = pc - per_block->start_pc;
uint64 blockidx = per_block->id;
DR_ASSERT(blockoffs < uint64_t(1) << PC_BLOCKOFFS_BITS);
DR_ASSERT(blockidx < uint64_t(1) << PC_BLOCKIDX_BITS);
modoffs = (blockidx << PC_BLOCKOFFS_BITS) | blockoffs;
}
// Check that the values we want to assign to the bitfields in offline_entry_t do not
// overflow. In i#2956 we observed an overflow for the modidx field.
Expand Down Expand Up @@ -840,10 +844,12 @@ offline_instru_t::bb_analysis(void *drcontext, void *tag, void **bb_field,

per_block->instr_count = instru_t::count_app_instrs(ilist);

app_pc tag_pc = dr_fragment_app_pc(tag);
per_block->start_pc = tag_pc;

identify_elidable_addresses(drcontext, ilist, OFFLINE_FILE_VERSION,
memref_needs_full_info);

app_pc tag_pc = dr_fragment_app_pc(tag);
if (does_pc_require_encoding(drcontext, tag_pc, nullptr, nullptr)) {
// For (unmodified) library code we do not need to record encodings as we
// rely on access to the binary during post-processing.
Expand Down
15 changes: 10 additions & 5 deletions clients/drcachesim/tracer/raw2trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,8 @@ class module_mapper_t {
get_orig_pc_from_map_pc(app_pc map_pc, uint64 modidx, uint64 modoffs) const
{
if (modidx == PC_MODIDX_INVALID) {
auto const it = encodings_.find(modoffs);
uint64 blockidx = (modoffs >> PC_BLOCKOFFS_BITS);
auto const it = encodings_.find(blockidx);
if (it == encodings_.end())
return nullptr;
encoding_entry_t *entry = it->second;
Expand All @@ -476,11 +477,13 @@ class module_mapper_t {
get_orig_pc(uint64 modidx, uint64 modoffs) const
{
if (modidx == PC_MODIDX_INVALID) {
auto const it = encodings_.find(modoffs);
uint64 blockidx = (modoffs >> PC_BLOCKOFFS_BITS);
uint64 blockoffs = (modoffs & ((uint64_t(1) << PC_BLOCKOFFS_BITS) - 1));
auto const it = encodings_.find(blockidx);
if (it == encodings_.end())
return nullptr;
encoding_entry_t *entry = it->second;
return reinterpret_cast<app_pc>(entry->start_pc);
return reinterpret_cast<app_pc>(entry->start_pc + blockoffs);
} else {
size_t idx = static_cast<size_t>(modidx); // Avoid win32 warnings.
// Cast to unsigned pointer-sized int first to avoid sign-extending.
Expand All @@ -494,11 +497,13 @@ class module_mapper_t {
get_map_pc(uint64 modidx, uint64 modoffs) const
{
if (modidx == PC_MODIDX_INVALID) {
auto const it = encodings_.find(modoffs);
uint64 blockidx = (modoffs >> PC_BLOCKOFFS_BITS);
uint64 blockoffs = (modoffs & ((uint64_t(1) << PC_BLOCKOFFS_BITS) - 1));
auto const it = encodings_.find(blockidx);
if (it == encodings_.end())
return nullptr;
encoding_entry_t *entry = it->second;
return entry->encodings;
return &entry->encodings[blockoffs];
} else {
size_t idx = static_cast<size_t>(modidx); // Avoid win32 warnings.
return modvec_[idx].map_seg_base + (modoffs - modvec_[idx].seg_offs);
Expand Down
5 changes: 5 additions & 0 deletions suite/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4249,6 +4249,11 @@ if (BUILD_CLIENTS)
torunonly_drcacheoff(gencode tool.drcacheoff.gencode
"" "@-simulator_type@opcode_mix" "")
unset(tool.drcacheoff.gencode_rawtemp) # Use preprocessor on template.

set(tool.drcacheoff.gencode_filtered_nodr ON)
torunonly_drcacheoff(gencode_filtered tool.drcacheoff.gencode
"" "@-simulator_type@opcode_mix" "-L0_filter -subdir_prefix drmemtrace.tool.drcacheoff.gencode_filtered")
unset(tool.drcacheoff.gencode_filtered_rawtemp) # Use preprocessor on template.
endif ()

# XXX i#1551: startstop API is NYI on ARM
Expand Down

0 comments on commit bd56135

Please sign in to comment.