Skip to content

Commit

Permalink
i#6416: Replace vdso raw bytes with per-block encodings (#6462)
Browse files Browse the repository at this point in the history 
Removes the vdso raw bytes we were storing in the module file for
offline drmemtraces. Switches to using per-block encodings instead. This
avoids problems with hooked vsysenter on 32-bit AMD.

Tested on tool.drcacheoff.simple on 32-bit AMD on a machine where that
test failed every time before this fix.

Removes the unused offline_instru_t::get_modoffs() rather than updating
it for the vdso change.

Issue: #6416, #2062
Fixes #6416
  • Loading branch information
@derekbruening
derekbruening authored Nov 17, 2023
1 parent 365b1a1 commit 63c701b
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 37 deletions.
9 changes: 6 additions & 3 deletions clients/drcachesim/tracer/instru.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,9 +432,6 @@ class offline_instru_t : public instru_t {
void
set_entry_addr(byte *buf_ptr, addr_t addr) override;

uint64_t
get_modoffs(void *drcontext, app_pc pc, DR_PARAM_OUT uint *modidx);

int
append_pid(byte *buf_ptr, process_id_t pid) override;
int
Expand Down Expand Up @@ -555,6 +552,10 @@ class offline_instru_t : public instru_t {
void
flush_instr_encodings();

bool
does_pc_require_encoding(void *drcontext, app_pc pc, uint *modidx_out,
app_pc *modbase_out);

// Custom module fields are global (since drmodtrack's support is global, we don't
// try to pass void* user data params through).
static void *(*user_load_)(module_data_t *module, int seg_idx);
Expand All @@ -566,6 +567,8 @@ class offline_instru_t : public instru_t {
print_custom_module_data(void *data, char *dst, size_t max_len);
static void
free_custom_module_data(void *data);
// Unfortunately this cached vdso base must be global as well.
static std::atomic<uintptr_t> vdso_modbase_;

// These identify the 4 fields we store in the label data area array.
static constexpr int LABEL_DATA_ELIDED_INDEX = 0; // Index among all operands.
Expand Down
68 changes: 37 additions & 31 deletions clients/drcachesim/tracer/instru_offline.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ static const uint MAX_INSTR_COUNT = 64 * 1024;
void *(*offline_instru_t::user_load_)(module_data_t *module, int seg_idx);
int (*offline_instru_t::user_print_)(void *data, char *dst, size_t max_len);
void (*offline_instru_t::user_free_)(void *data);
std::atomic<uintptr_t> offline_instru_t::vdso_modbase_;

// This constructor is for use in post-processing when we just need the
// elision utility functions.
Expand Down Expand Up @@ -153,26 +154,21 @@ offline_instru_t::load_custom_module_data(module_data_t *module, int seg_idx)
if (user_load_ != nullptr)
user_data = (*user_load_)(module, seg_idx);
const char *name = dr_module_preferred_name(module);
// For vdso we include the entire contents so we can decode it during
// post-processing.
// We use placement new for better isolation, esp w/ static linkage into the app.
if ((name != nullptr &&
(strstr(name, "linux-gate.so") == name ||
strstr(name, "linux-vdso.so") == name)) ||
(module->names.file_name != NULL && strcmp(name, "[vdso]") == 0)) {
void *alloc = dr_global_alloc(sizeof(custom_module_data_t));
#ifdef WINDOWS
byte *start = module->start;
byte *end = module->end;
#else
byte *start =
(module->num_segments > 0) ? module->segments[seg_idx].start : module->start;
byte *end =
(module->num_segments > 0) ? module->segments[seg_idx].end : module->end;
#endif
return new (alloc)
custom_module_data_t((const char *)start, end - start, user_data);
} else if (user_data != nullptr) {
// We used to store the vdso contents, but we now use separate block encodings
// for vdso code. So we just find the vdso here, and pass through the user's data
// for all modules.
if (seg_idx == 0 &&
((name != nullptr &&
(strstr(name, "linux-gate.so") == name ||
strstr(name, "linux-vdso.so") == name)) ||
(module->names.file_name != NULL && strcmp(name, "[vdso]") == 0))) {
DR_ASSERT(vdso_modbase_.load(std::memory_order_acquire) == 0 ||
vdso_modbase_.load(std::memory_order_acquire) ==
reinterpret_cast<uintptr_t>(module->start));
vdso_modbase_.store(reinterpret_cast<uintptr_t>(module->start),
std::memory_order_release);
}
if (user_data != nullptr) {
void *alloc = dr_global_alloc(sizeof(custom_module_data_t));
return new (alloc) custom_module_data_t(nullptr, 0, user_data);
}
Expand Down Expand Up @@ -461,15 +457,6 @@ offline_instru_t::insert_save_entry(void *drcontext, instrlist_t *ilist, instr_t
return sizeof(offline_entry_t);
}

uint64_t
offline_instru_t::get_modoffs(void *drcontext, app_pc pc, DR_PARAM_OUT uint *modidx)
{
app_pc modbase;
if (drmodtrack_lookup(drcontext, pc, modidx, &modbase) != DRCOVLIB_SUCCESS)
return 0;
return pc - modbase;
}

// Caller must hold the encoding_lock.
void
offline_instru_t::flush_instr_encodings()
Expand All @@ -490,6 +477,8 @@ offline_instru_t::record_instr_encodings(void *drcontext, app_pc tag_pc,
per_block_t *per_block, instrlist_t *ilist)
{
dr_mutex_lock(encoding_lock_);
log_(3, "%s: new block id " UINT64_FORMAT_STRING " for %p\n", __FUNCTION__,
encoding_id_, tag_pc);
per_block->id = encoding_id_++;

if (encoding_buf_ptr_ + max_block_encoding_size_ >=
Expand Down Expand Up @@ -542,6 +531,23 @@ offline_instru_t::record_instr_encodings(void *drcontext, app_pc tag_pc,
dr_mutex_unlock(encoding_lock_);
}

bool
offline_instru_t::does_pc_require_encoding(void *drcontext, app_pc pc, uint *modidx_out,
app_pc *modbase_out)
{
uint modidx;
app_pc modbase;
bool res = drmodtrack_lookup(drcontext, pc, &modidx, &modbase) != DRCOVLIB_SUCCESS ||
// We treat the VDSO as generated code, storing its encodings.
reinterpret_cast<uintptr_t>(modbase) ==
vdso_modbase_.load(std::memory_order_acquire);
if (modidx_out != nullptr)
*modidx_out = modidx;
if (modbase_out != nullptr)
*modbase_out = modbase;
return res;
}

int
offline_instru_t::insert_save_pc(void *drcontext, instrlist_t *ilist, instr_t *where,
reg_id_t reg_ptr, reg_id_t scratch, int adjust,
Expand All @@ -552,7 +558,7 @@ offline_instru_t::insert_save_pc(void *drcontext, instrlist_t *ilist, instr_t *w
app_pc modbase;
uint modidx;
uint64_t modoffs;
if (drmodtrack_lookup(drcontext, pc, &modidx, &modbase) == DRCOVLIB_SUCCESS) {
if (!does_pc_require_encoding(drcontext, pc, &modidx, &modbase)) {
// TODO i#2062: We need to also identify modified library code and record
// its encodings. The plan is to augment drmodtrack to track this for us;
// for now we will incorrectly use the original bits in the trace.
Expand Down Expand Up @@ -838,7 +844,7 @@ offline_instru_t::bb_analysis(void *drcontext, void *tag, void **bb_field,
memref_needs_full_info);

app_pc tag_pc = dr_fragment_app_pc(tag);
if (drmodtrack_lookup(drcontext, tag_pc, nullptr, nullptr) != DRCOVLIB_SUCCESS) {
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
10 changes: 7 additions & 3 deletions clients/drcachesim/tracer/raw2trace.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,9 @@ module_mapper_t::read_and_map_modules()
drmodtrack_info_t &info = *it;
custom_module_data_t *custom_data = (custom_module_data_t *)info.custom;
if (custom_data != nullptr && custom_data->contents_size > 0) {
// XXX i#2062: We could eliminate this raw bytes in the module data in
// favor of the new encoding file used for generated code.
// These raw bytes for vdso is only present for legacy traces; we
// use encoding entries for new traces.
// XXX i#2062: Delete this code once we stop supporting legacy traces.
VPRINT(1, "Using module %d %s stored %zd-byte contents @" PFX "\n",
(int)modvec_.size(), info.path, custom_data->contents_size,
custom_data->contents);
Expand Down Expand Up @@ -461,7 +462,10 @@ module_mapper_t::read_and_map_modules()
// We expect to fail to map dynamorio.dll for x64 Windows as it
// is built /fixed. (We could try to have the map succeed w/o relocs,
// but we expect to not care enough about code in DR).
if (strstr(info.path, "dynamorio") != NULL)
// We also expect to fail for vdso, for which we have encoding entries.
if (strstr(info.path, "dynamorio") != nullptr ||
strstr(info.path, "linux-gate") != nullptr ||
strstr(info.path, "vdso") != nullptr)
modvec_.push_back(module_t(info.path, info.start, NULL, 0, 0, 0));
else {
last_error_ = "Failed to map module " + std::string(info.path);
Expand Down

0 comments on commit 63c701b

Please sign in to comment.