From a2fbfe472d5f8a04ef4cbf0da36c3314a1274a83 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Tue, 16 Aug 2022 14:31:14 -0400 Subject: [PATCH 1/7] i#2062 memtrace nonmod: Raw2trace parsing of encoding file Adds raw2trace parsing of the encoding file used by the tracer to store instruction encodings for generated code. This involves the following changes: + Adds encoding file parsing to module_mapper_t. + Changes module map queries to use new module_mapper_t interfaces instead, which handle generated code. + Changes block lookup to use the modidx,modoffs pair as the key rather than the absolute pc. The changes are compatibility-breaking for raw2trace_t which now takes an encoding file parameter in the middle of existing parameters. Updates existing uses. For module_mapper_t the encoding file is added last with a default value to preserve compatibility for existing analysis tools like opcode_mix and view. It is assumed that encodings for generated code will be added to the final trace file and thus these tools will not need a module_mapper_t interface for generated code. Augments the tool.drcacheoff.gencode test to post-process the trace and ensure the generated code PC is observed. Fixes a -loglevel 4 signal dump_unmaksed() crash on detach i#5618 hit in the gencode test; confirmed the test is crash-free at loglevel 4 with the fix. Issue: #2062 Fixes #5618 --- api/docs/release.dox | 1 + clients/drcachesim/analyzer_multi.cpp | 4 +- .../drcachesim/tests/burst_aarch64_sys.cpp | 2 +- clients/drcachesim/tests/burst_gencode.cpp | 147 +++++++++++--- clients/drcachesim/tests/burst_replace.cpp | 8 +- clients/drcachesim/tests/burst_traceopts.cpp | 2 +- .../tests/offline-gencode.templatex | 9 - clients/drcachesim/tests/raw2trace_io.cpp | 2 +- .../drcachesim/tests/raw2trace_unit_tests.cpp | 40 ++-- clients/drcachesim/tools/opcode_mix.cpp | 4 +- clients/drcachesim/tools/view.cpp | 2 + clients/drcachesim/tracer/raw2trace.cpp | 84 ++++++-- clients/drcachesim/tracer/raw2trace.h | 187 ++++++++++++++---- .../drcachesim/tracer/raw2trace_launcher.cpp | 8 +- core/unix/signal.c | 8 +- suite/tests/CMakeLists.txt | 8 +- 16 files changed, 393 insertions(+), 123 deletions(-) diff --git a/api/docs/release.dox b/api/docs/release.dox index bc016e4d72b..85a0d6da6b0 100644 --- a/api/docs/release.dox +++ b/api/docs/release.dox @@ -165,6 +165,7 @@ Further non-compatibility-affecting changes include: and available only on Intel processors that support the Intel@ Processor Trace feature. - Added drmemtrace_get_encoding_path(). + - Added preliminary support for generated code to drmemtrace. The changes between version 9.0.1 and 9.0.0 include the following compatibility changes: diff --git a/clients/drcachesim/analyzer_multi.cpp b/clients/drcachesim/analyzer_multi.cpp index 5b2dfa43b8e..0d5a10daae0 100644 --- a/clients/drcachesim/analyzer_multi.cpp +++ b/clients/drcachesim/analyzer_multi.cpp @@ -106,8 +106,8 @@ analyzer_multi_t::analyzer_multi_t() return; } raw2trace_t raw2trace(dir.modfile_bytes_, dir.in_files_, dir.out_files_, - nullptr, op_verbose.get_value(), op_jobs.get_value(), - op_alt_module_dir.get_value()); + dir.encoding_file_, nullptr, op_verbose.get_value(), + op_jobs.get_value(), op_alt_module_dir.get_value()); std::string error = raw2trace.do_conversion(); if (!error.empty()) { success_ = false; diff --git a/clients/drcachesim/tests/burst_aarch64_sys.cpp b/clients/drcachesim/tests/burst_aarch64_sys.cpp index 1c80d522d02..bd4d91f99de 100644 --- a/clients/drcachesim/tests/burst_aarch64_sys.cpp +++ b/clients/drcachesim/tests/burst_aarch64_sys.cpp @@ -162,7 +162,7 @@ post_process() std::string dir_err = dir.initialize(raw_dir, outdir); assert(dir_err.empty()); raw2trace_t raw2trace(dir.modfile_bytes_, dir.in_files_, dir.out_files_, - dr_context); + dir.encoding_file_, dr_context); std::string error = raw2trace.do_conversion(); if (!error.empty()) { std::cerr << "raw2trace failed: " << error << "\n"; diff --git a/clients/drcachesim/tests/burst_gencode.cpp b/clients/drcachesim/tests/burst_gencode.cpp index 8b9f4023956..5ca16dfe78b 100644 --- a/clients/drcachesim/tests/burst_gencode.cpp +++ b/clients/drcachesim/tests/burst_gencode.cpp @@ -39,16 +39,25 @@ #include "configure.h" #include "dr_api.h" #include "drmemtrace/drmemtrace.h" +#include "drmemtrace/raw2trace.h" +#include "raw2trace_directory.h" +#include "analyzer.h" #include #include #include #include -#include "tools.h" // Included after system headers to avoid printf warning. +#undef ALIGN_FORWARD // Conflicts with drcachesim utils.h. +#include "tools.h" // Included after system headers to avoid printf warning. + +namespace { /*************************************************************************** * Code generation. */ +// XXX i#2062: Remove this once we have encodings in the final trace. +static app_pc gencode_start; + class code_generator_t { public: explicit code_generator_t(bool verbose = false) @@ -66,6 +75,15 @@ class code_generator_t { reinterpret_cast(map_)(); } + static constexpr int kGencodeMagic1 = 0x742; + static constexpr int kGencodeMagic2 = 0x427; + + app_pc + get_gencode_start() const + { + return map_; + } + private: byte *map_ = nullptr; size_t map_size_ = 0; @@ -84,9 +102,15 @@ class code_generator_t { instrlist_t *ilist = instrlist_create(dc); reg_id_t base = IF_X86_ELSE(IF_X64_ELSE(DR_REG_RAX, DR_REG_EAX), DR_REG_R0); + reg_id_t base4imm = IF_X86_64_ELSE(reg_64_to_32(base), base); int ptrsz = static_cast(sizeof(void *)); - // TODO i#2062: Add more instructions and look for them in the final trace. - // For now we are testing that drmemtrace detects generated code. + // A two-immediate pattern we look for in the trace. + instrlist_append(ilist, + XINST_CREATE_load_int(dc, opnd_create_reg(base4imm), + OPND_CREATE_INT32(kGencodeMagic1))); + instrlist_append(ilist, + XINST_CREATE_load_int(dc, opnd_create_reg(base4imm), + OPND_CREATE_INT32(kGencodeMagic2))); instrlist_append( ilist, XINST_CREATE_move(dc, opnd_create_reg(base), opnd_create_reg(DR_REG_XSP))); @@ -119,9 +143,6 @@ class code_generator_t { * Top-level tracing. */ -// TODO i#2062: Run raw2trace and examine the final trace looking for the gencode -// instrs; use a unique start and end pattern to identify. - static int do_some_work(const code_generator_t &gen) { @@ -147,27 +168,107 @@ exit_cb(void *) assert(stream.good()); } -int -main(int argc, const char *argv[]) +static std::string +post_process() +{ + const char *raw_dir; + drmemtrace_status_t mem_res = drmemtrace_get_output_path(&raw_dir); + assert(mem_res == DRMEMTRACE_SUCCESS); + std::string outdir = std::string(raw_dir) + DIRSEP + "post_processed"; + void *dr_context = dr_standalone_init(); + // Use a new scope to free raw2trace_directory_t before dr_standalone_exit(). + { + raw2trace_directory_t dir; + if (!dr_create_dir(outdir.c_str())) { + std::cerr << "Failed to create output dir"; + assert(false); + } + std::string dir_err = dir.initialize(raw_dir, outdir); + assert(dir_err.empty()); + raw2trace_t raw2trace(dir.modfile_bytes_, dir.in_files_, dir.out_files_, + dir.encoding_file_, dr_context); + std::string error = raw2trace.do_conversion(); + if (!error.empty()) { + std::cerr << "raw2trace failed: " << error << "\n"; + assert(false); + } + } + dr_standalone_exit(); + return outdir; +} + +static std::string +gather_trace() { if (!my_setenv("DYNAMORIO_OPTIONS", "-stderr_mask 0xc -client_lib ';;-offline")) std::cerr << "failed to set env var!\n"; - code_generator_t gen(false); - for (int i = 0; i < 3; i++) { - std::cerr << "pre-DR init\n"; - 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"; - dr_app_start(); - if (do_some_work(gen) < 0) - std::cerr << "error in computation\n"; - std::cerr << "pre-DR detach\n"; - dr_app_stop_and_cleanup(); - std::cerr << "all done\n"; - } + gencode_start = gen.get_gencode_start(); + std::cerr << "pre-DR init\n"; + 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"; + dr_app_start(); + if (do_some_work(gen) < 0) + std::cerr << "error in computation\n"; + std::cerr << "pre-DR detach\n"; + dr_app_stop_and_cleanup(); + std::cerr << "all done\n"; + return post_process(); +} +static int +look_for_gencode(std::string trace_dir) +{ + void *dr_context = dr_standalone_init(); + analyzer_t analyzer(trace_dir); + if (!analyzer) { + std::cerr << "Failed to initialize: " << analyzer.get_error_string() << "\n"; + } + bool found_magic1 = false, found_magic2 = false; + bool have_instr_encodings = false; // XXX i#5520: See comment below. + for (reader_t &iter = analyzer.begin(); iter != analyzer.end(); ++iter) { + memref_t memref = *iter; + if (!type_is_instr(memref.instr.type)) { + found_magic1 = false; + continue; + } + app_pc pc = (app_pc)memref.instr.addr; + // TODO i#5520: Once we have instruction encodings in the final trace, we want + // to perform the decoding below and look for the immediates. + // Until then, we just look for the gencode PC. + if (pc == gencode_start) { + found_magic2 = true; + } + if (have_instr_encodings) { + instr_t instr; + instr_init(dr_context, &instr); + app_pc next_pc = decode(dr_context, pc, &instr); + assert(next_pc != nullptr && instr_valid(&instr)); + ptr_int_t immed; + if (!found_magic1 && instr_is_mov_constant(&instr, &immed) && + immed == code_generator_t::kGencodeMagic1) + found_magic1 = true; + else if (found_magic1 && instr_is_mov_constant(&instr, &immed) && + immed == code_generator_t::kGencodeMagic2) + found_magic2 = true; + else + found_magic1 = false; + instr_free(dr_context, &instr); + } + } + dr_standalone_exit(); + assert(found_magic2); return 0; } + +} // namespace + +int +main(int argc, const char *argv[]) +{ + std::string trace_dir = gather_trace(); + return look_for_gencode(trace_dir); +} diff --git a/clients/drcachesim/tests/burst_replace.cpp b/clients/drcachesim/tests/burst_replace.cpp index cba8977a0c6..01d7045bfc5 100644 --- a/clients/drcachesim/tests/burst_replace.cpp +++ b/clients/drcachesim/tests/burst_replace.cpp @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2016-2021 Google, Inc. All rights reserved. + * Copyright (c) 2016-2022 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -196,7 +196,7 @@ post_process() dir.modfile_bytes_, parse_cb, process_cb, MAGIC_VALUE, free_cb); assert(module_mapper->get_last_error().empty()); // Test back-compat of deprecated APIs. - raw2trace_t raw2trace(dir.modfile_bytes_, dir.in_files_, dir.out_files_, NULL); + raw2trace_t raw2trace(dir.modfile_bytes_, dir.in_files_, dir.out_files_); std::string error = raw2trace.handle_custom_data(parse_cb, process_cb, MAGIC_VALUE, free_cb); assert(error.empty()); @@ -218,8 +218,8 @@ post_process() raw2trace_directory_t dir; std::string dir_err = dir.initialize(raw_dir, ""); assert(dir_err.empty()); - raw2trace_t raw2trace(dir.modfile_bytes_, dir.in_files_, dir.out_files_, dr_context, - 0); + raw2trace_t raw2trace(dir.modfile_bytes_, dir.in_files_, dir.out_files_, + dir.encoding_file_, dr_context, 0); std::string error = raw2trace.handle_custom_data(parse_cb, process_cb, MAGIC_VALUE, free_cb); assert(error.empty()); diff --git a/clients/drcachesim/tests/burst_traceopts.cpp b/clients/drcachesim/tests/burst_traceopts.cpp index 6789ab72e54..706519ffa7c 100644 --- a/clients/drcachesim/tests/burst_traceopts.cpp +++ b/clients/drcachesim/tests/burst_traceopts.cpp @@ -175,7 +175,7 @@ post_process(const std::string &out_subdir) std::string dir_err = dir.initialize(raw_dir, outdir); assert(dir_err.empty()); raw2trace_t raw2trace(dir.modfile_bytes_, dir.in_files_, dir.out_files_, - dr_context, + dir.encoding_file_, dr_context, 0 # ifdef WINDOWS /* FIXME i#3983: Creating threads in standalone mode diff --git a/clients/drcachesim/tests/offline-gencode.templatex b/clients/drcachesim/tests/offline-gencode.templatex index 35d15c96991..c0129467608 100644 --- a/clients/drcachesim/tests/offline-gencode.templatex +++ b/clients/drcachesim/tests/offline-gencode.templatex @@ -2,15 +2,6 @@ pre-DR init pre-DR start pre-DR detach all done -pre-DR init -pre-DR start -pre-DR detach -all done -pre-DR init -pre-DR start -pre-DR detach -all done -\[drmemtrace\]: WARNING: Skipping ifetch for instructions not in a module Basic counts tool results: Total counts: .* diff --git a/clients/drcachesim/tests/raw2trace_io.cpp b/clients/drcachesim/tests/raw2trace_io.cpp index ee869ed6cd2..ff1aded1448 100644 --- a/clients/drcachesim/tests/raw2trace_io.cpp +++ b/clients/drcachesim/tests/raw2trace_io.cpp @@ -148,7 +148,7 @@ test_raw2trace(raw2trace_directory_t *dir) * raw2trace::read_and_map_modules(). */ raw2trace_t raw2trace(dir->modfile_bytes_, dir->in_files_, dir->out_files_, - GLOBAL_DCONTEXT, 1); + dir->encoding_file_, GLOBAL_DCONTEXT, 1); std::string error = raw2trace.do_conversion(); if (!error.empty()) { std::cerr << "raw2trace failed " << error << "\n"; diff --git a/clients/drcachesim/tests/raw2trace_unit_tests.cpp b/clients/drcachesim/tests/raw2trace_unit_tests.cpp index 7843f57aa31..95ac5af027a 100644 --- a/clients/drcachesim/tests/raw2trace_unit_tests.cpp +++ b/clients/drcachesim/tests/raw2trace_unit_tests.cpp @@ -65,36 +65,44 @@ # error Unsupported arch #endif -// Subclasses raw2trace_t and replaces the module loading with a buffer -// of encoded instr_t. -class raw2trace_test_t : public raw2trace_t { +// Subclasses raw2trace_t and module_mapper_t and replaces the module loading with +// a buffer of encoded instr_t. +class module_mapper_test_t : public module_mapper_t { public: - raw2trace_test_t(const std::vector &input, - const std::vector &output, instrlist_t &instrs, - void *drcontext) - : raw2trace_t(nullptr, input, output, drcontext, - // The sequences are small so we print everything for easier - // debugging and viewing of what's going on. - 4) + module_mapper_test_t(instrlist_t &instrs, void *drcontext) + : module_mapper_t(nullptr) { byte *pc = instrlist_encode(drcontext, &instrs, decode_buf_, true); ASSERT(pc - decode_buf_ < MAX_DECODE_SIZE, "decode buffer overflow"); - set_modvec_(&modules_); } protected: - std::string + void read_and_map_modules() override { - modules_.push_back(module_t("fake_exe", 0, decode_buf_, 0, MAX_DECODE_SIZE, - MAX_DECODE_SIZE, true)); - return ""; + modvec_.push_back(module_t("fake_exe", 0, decode_buf_, 0, MAX_DECODE_SIZE, + MAX_DECODE_SIZE, true)); } private: static const int MAX_DECODE_SIZE = 1024; byte decode_buf_[MAX_DECODE_SIZE]; - std::vector modules_; +}; + +class raw2trace_test_t : public raw2trace_t { +public: + raw2trace_test_t(const std::vector &input, + const std::vector &output, instrlist_t &instrs, + void *drcontext) + : raw2trace_t(nullptr, input, output, INVALID_FILE, drcontext, + // The sequences are small so we print everything for easier + // debugging and viewing of what's going on. + 4) + { + module_mapper_ = + std::unique_ptr(new module_mapper_test_t(instrs, drcontext)); + set_modmap_(module_mapper_.get()); + } }; offline_entry_t diff --git a/clients/drcachesim/tools/opcode_mix.cpp b/clients/drcachesim/tools/opcode_mix.cpp index df803a36bf0..7ec845b1120 100644 --- a/clients/drcachesim/tools/opcode_mix.cpp +++ b/clients/drcachesim/tools/opcode_mix.cpp @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2017-2021 Google, Inc. All rights reserved. + * Copyright (c) 2017-2022 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -71,6 +71,8 @@ opcode_mix_t::initialize() std::string error = directory_.initialize_module_file(module_file_path_); if (!error.empty()) return "Failed to initialize directory: " + error; + // Non-module instruction entries will be in the final trace (i#2062,i#5520) so + // we do not need the encoding file and leave it as its default INVALID_FILE. module_mapper_ = module_mapper_t::create(directory_.modfile_bytes_, nullptr, nullptr, nullptr, nullptr, knob_verbose_, knob_alt_module_dir_); diff --git a/clients/drcachesim/tools/view.cpp b/clients/drcachesim/tools/view.cpp index 638499a406c..d56ebc94cc9 100644 --- a/clients/drcachesim/tools/view.cpp +++ b/clients/drcachesim/tools/view.cpp @@ -93,6 +93,8 @@ view_t::initialize() // not available. return ""; } + // Non-module instruction entries will be in the final trace (i#2062,i#5520) so + // we do not need the encoding file and leave it as its default INVALID_FILE. module_mapper_ = module_mapper_t::create(directory_.modfile_bytes_, nullptr, nullptr, nullptr, nullptr, knob_verbose_, knob_alt_module_dir_); diff --git a/clients/drcachesim/tracer/raw2trace.cpp b/clients/drcachesim/tracer/raw2trace.cpp index e9a855b6391..d9957744fe0 100644 --- a/clients/drcachesim/tracer/raw2trace.cpp +++ b/clients/drcachesim/tracer/raw2trace.cpp @@ -124,11 +124,12 @@ module_mapper_t::module_mapper_t( const char *module_map, const char *(*parse_cb)(const char *src, OUT void **data), std::string (*process_cb)(drmodtrack_info_t *info, void *data, void *user_data), void *process_cb_user_data, void (*free_cb)(void *data), uint verbosity, - const std::string &alt_module_dir) + const std::string &alt_module_dir, file_t encoding_file) : modmap_(module_map) , cached_user_free_(free_cb) , verbosity_(verbosity) , alt_module_dir_(alt_module_dir) + , encoding_file_(encoding_file) { // We mutate global state because do_module_parsing() uses drmodtrack, which // wants global functions. The state isn't needed past do_module_parsing(), so @@ -145,7 +146,10 @@ module_mapper_t::module_mapper_t( // It is assumed to be set to 'true' initially. has_custom_data_global_ = true; - last_error_ = do_module_parsing(); + if (modmap_ != nullptr) + last_error_ = do_module_parsing(); + if (encoding_file_ != INVALID_FILE) + last_error_ += do_encoding_parsing(); // capture has_custom_data_global_'s value for this instance. has_custom_data_ = has_custom_data_global_; @@ -274,9 +278,9 @@ std::string raw2trace_t::do_module_parsing() { if (!module_mapper_) { - module_mapper_ = module_mapper_t::create(modmap_, user_parse_, user_process_, - user_process_data_, user_free_, - verbosity_, alt_module_dir_); + module_mapper_ = module_mapper_t::create( + modmap_, user_parse_, user_process_, user_process_data_, user_free_, + verbosity_, alt_module_dir_, encoding_file_); } return module_mapper_->get_last_error(); } @@ -309,6 +313,32 @@ module_mapper_t::do_module_parsing() return ""; } +std::string +module_mapper_t::do_encoding_parsing() +{ + if (encoding_file_ == INVALID_FILE) + return ""; + uint64 file_size; + if (!dr_file_size(encoding_file_, &file_size)) + return "Failed to obtain size of encoding file"; + size_t map_size = (size_t)file_size; + byte *map_start = reinterpret_cast( + dr_map_file(encoding_file_, &map_size, 0, NULL, DR_MEMPROT_READ, 0)); + if (map_start == nullptr || map_size < file_size) + return "Failed to map encoding file"; + if (*reinterpret_cast(map_start) != ENCODING_FILE_VERSION) + return "Encoding file has invalid version"; + size_t offs = sizeof(uint64_t); + while (offs < map_size) { + encoding_entry_t *entry = reinterpret_cast(map_start + offs); + if (offs + entry->length > map_size) + return "Encoding file is truncated"; + encodings_[entry->id] = entry; + offs += entry->length; + } + return ""; +} + std::string raw2trace_t::read_and_map_modules() { @@ -319,6 +349,7 @@ raw2trace_t::read_and_map_modules() } set_modvec_(&module_mapper_->get_loaded_modules()); + set_modmap_(module_mapper_.get()); return module_mapper_->get_last_error(); } @@ -339,6 +370,8 @@ 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. VPRINT(1, "Using module %d %s stored %zd-byte contents @" PFX "\n", (int)modvec_.size(), info.path, custom_data->contents_size, custom_data->contents); @@ -568,6 +601,9 @@ raw2trace_t::process_next_thread_buffer(raw2trace_thread_data_t *tdata, tdata->file_type); if (!tdata->error.empty()) return tdata->error; + if (tdata->version >= OFFLINE_FILE_VERSION_ENCODINGS && + encoding_file_ == INVALID_FILE) + return "Expected encoding file but none was passed"; if (tdata->saw_header) { tdata->error = process_header(tdata); if (!tdata->error.empty()) @@ -730,19 +766,23 @@ raw2trace_t::do_conversion() } raw2trace_t::block_summary_t * -raw2trace_t::lookup_block_summary(void *tls, app_pc block_start) +raw2trace_t::lookup_block_summary(void *tls, uint64 modidx, uint64 modoffs, + app_pc block_start) { auto tdata = reinterpret_cast(tls); - if (block_start == tdata->last_decode_block_start) { + if (block_start == tdata->last_decode_block_start && + modidx == tdata->last_decode_modidx && modoffs == tdata->last_decode_modoffs) { VPRINT(5, "Using last block summary " PFX " for " PFX "\n", tdata->last_block_summary, tdata->last_decode_block_start); return tdata->last_block_summary; } - block_summary_t *ret = static_cast( - hashtable_lookup(&decode_cache_[tdata->worker], block_start)); + block_summary_t *ret = static_cast(hashtable_lookup( + &decode_cache_[tdata->worker], decode_cache_key(modidx, modoffs))); if (ret != nullptr) { DEBUG_ASSERT(ret->start_pc == block_start); tdata->last_decode_block_start = block_start; + tdata->last_decode_modidx = modidx; + tdata->last_decode_modoffs = modoffs; tdata->last_block_summary = ret; VPRINT(5, "Caching last block summary " PFX " for " PFX "\n", tdata->last_block_summary, tdata->last_decode_block_start); @@ -755,7 +795,7 @@ raw2trace_t::lookup_instr_summary(void *tls, uint64 modidx, uint64 modoffs, app_pc block_start, int index, app_pc pc, OUT block_summary_t **block_summary) { - block_summary_t *block = lookup_block_summary(tls, block_start); + block_summary_t *block = lookup_block_summary(tls, modidx, modoffs, block_start); if (block_summary != nullptr) *block_summary = block; if (block == nullptr) @@ -785,15 +825,24 @@ raw2trace_t::create_instr_summary(void *tls, uint64 modidx, uint64 modoffs, if (block == nullptr) { block = new block_summary_t(block_start, instr_count); DEBUG_ASSERT(index >= 0 && index < static_cast(block->instrs.size())); - hashtable_add(&decode_cache_[tdata->worker], block_start, block); - VPRINT(5, "Created new block summary " PFX " for " PFX "\n", block, block_start); + hashtable_add(&decode_cache_[tdata->worker], decode_cache_key(modidx, modoffs), + block); + VPRINT(5, + "Created new block summary " PFX " for " PFX " modidx=" INT64_FORMAT_STRING + " modoffs=" HEX64_FORMAT_STRING " key=%p\n", + block, block_start, modidx, modoffs, decode_cache_key(modidx, modoffs)); tdata->last_decode_block_start = block_start; + tdata->last_decode_modidx = modidx; + tdata->last_decode_modoffs = modoffs; tdata->last_block_summary = block; } instr_summary_t *desc = &block->instrs[index]; if (!instr_summary_t::construct(dcontext_, block_start, pc, orig, desc, verbosity_)) { - WARN("Encountered invalid/undecodable instr @ %s+" PIFX, - modvec_()[static_cast(modidx)].path, IF_NOT_X64((uint)) modoffs); + WARN("Encountered invalid/undecodable instr @ idx=" INT64_FORMAT_STRING + " offs=" INT64_FORMAT_STRING " %s", + modidx, modoffs, + modidx == PC_MODIDX_INVALID ? "" + : modvec_()[static_cast(modidx)].path); return nullptr; } return desc; @@ -1116,14 +1165,15 @@ raw2trace_t::get_file_type(void *tls) raw2trace_t::raw2trace_t(const char *module_map, const std::vector &thread_files, - const std::vector &out_files, void *dcontext, - unsigned int verbosity, int worker_count, - const std::string &alt_module_dir) + const std::vector &out_files, + file_t encoding_file, void *dcontext, unsigned int verbosity, + int worker_count, const std::string &alt_module_dir) : trace_converter_t(dcontext) , worker_count_(worker_count) , user_process_(nullptr) , user_process_data_(nullptr) , modmap_(module_map) + , encoding_file_(encoding_file) , verbosity_(verbosity) , alt_module_dir_(alt_module_dir) { diff --git a/clients/drcachesim/tracer/raw2trace.h b/clients/drcachesim/tracer/raw2trace.h index 24203eee8ce..df4ff4185c1 100644 --- a/clients/drcachesim/tracer/raw2trace.h +++ b/clients/drcachesim/tracer/raw2trace.h @@ -357,7 +357,8 @@ struct trace_metadata_reader_t { }; /** - * module_mapper_t maps and unloads application modules. + * module_mapper_t maps and unloads application modules, as well as non-module + * instruction encodings (for raw traces, or if not present in the final trace). * Using it assumes a dr_context has already been setup. * This class is not thread-safe. */ @@ -376,6 +377,9 @@ class module_mapper_t { * * The callbacks will only be called during object construction. * + * Additionally parses the non-module instruction encodings file if 'encoding_file' + * is not nullptr. + * * On success, calls the \p process_cb function for every module in the list. * On failure, get_last_error() is non-empty, and indicates the cause. */ @@ -385,11 +389,12 @@ class module_mapper_t { std::string (*process_cb)(drmodtrack_info_t *info, void *data, void *user_data) = nullptr, void *process_cb_user_data = nullptr, void (*free_cb)(void *data) = nullptr, - uint verbosity = 0, const std::string &alt_module_dir = "") + uint verbosity = 0, const std::string &alt_module_dir = "", + file_t encoding_file = INVALID_FILE) { return std::unique_ptr( new module_mapper_t(module_map, parse_cb, process_cb, process_cb_user_data, - free_cb, verbosity, alt_module_dir)); + free_cb, verbosity, alt_module_dir, encoding_file)); } /** @@ -417,13 +422,61 @@ class module_mapper_t { return modvec_; } + app_pc + 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); + if (it == encodings_.end()) + return nullptr; + encoding_entry_t *entry = it->second; + return (map_pc - entry->encodings) + + reinterpret_cast(entry->start_pc); + } else { + return map_pc - modvec_[modidx].map_seg_base + modvec_[modidx].orig_seg_base; + } + } + + app_pc + get_orig_pc(uint64 modidx, uint64 modoffs) const + { + if (modidx == PC_MODIDX_INVALID) { + auto const it = encodings_.find(modoffs); + if (it == encodings_.end()) + return nullptr; + encoding_entry_t *entry = it->second; + return reinterpret_cast(entry->start_pc); + } else { + // Cast to unsigned pointer-sized int first to avoid sign-extending. + return reinterpret_cast( + reinterpret_cast(modvec_[modidx].orig_seg_base)) + + (modoffs - modvec_[modidx].seg_offs); + } + } + + app_pc + get_map_pc(uint64 modidx, uint64 modoffs) const + { + if (modidx == PC_MODIDX_INVALID) { + auto const it = encodings_.find(modoffs); + if (it == encodings_.end()) + return nullptr; + encoding_entry_t *entry = it->second; + return entry->encodings; + } else { + return modvec_[modidx].map_seg_base + (modoffs - modvec_[modidx].seg_offs); + } + } + /** * This interface is meant to be used with a final trace rather than a raw * trace, using the module log file saved from the raw2trace conversion. - * After the a call to get_loaded_modules(), this routine may be used + * After a call to get_loaded_modules(), this routine may be used * to convert an instruction program counter in a trace into an address in the * current process where the instruction bytes for that instruction are mapped, * allowing decoding for obtaining further information than is stored in the trace. + * This interface only supports code inside modules; generated code is + * expected to have instruction encodings in the trace itself. * Returns the mapped address. Check get_last_error_() if an error occurred. */ app_pc @@ -462,7 +515,8 @@ class module_mapper_t { void *user_data) = nullptr, void *process_cb_user_data = nullptr, void (*free_cb)(void *data) = nullptr, uint verbosity = 0, - const std::string &alt_module_dir = ""); + const std::string &alt_module_dir = "", + file_t encoding_file = INVALID_FILE); module_mapper_t(const module_mapper_t &) = delete; module_mapper_t & @@ -486,6 +540,9 @@ class module_mapper_t { std::string do_module_parsing(); + std::string + do_encoding_parsing(); + const char *modmap_ = nullptr; void *modhandle_ = nullptr; std::vector modvec_; @@ -517,6 +574,9 @@ class module_mapper_t { uint verbosity_ = 0; std::string alt_module_dir_; std::string last_error_; + + file_t encoding_file_ = INVALID_FILE; + std::unordered_map encodings_; }; /** @@ -610,7 +670,7 @@ struct trace_header_t { * * *
  • bool raw2trace_t::instr_summary_exists(void *tls, uint64 modidx, uint64 modoffs, - * app_pc block_start_pc, app_pc pc) const + * int index) const * * Returns whether an #instr_summary_t representation of the instruction at pc inside * the block that begins at block_start_pc in the specified module exists. @@ -699,7 +759,8 @@ template class trace_converter_t { /** * Convert starting from in_entry, and reading more entries as required. * Sets end_of_record to true if processing hit the end of a record. - * set_modvec_() must have been called by the implementation before calling this API. + * read_and_map_modules() must have been called by the implementation before + * calling this API. */ std::string process_offline_entry(void *tls, const offline_entry_t *in_entry, thread_id_t tid, @@ -843,6 +904,9 @@ template class trace_converter_t { */ bool passed_dcontext_ = false; + /* TODO i#2062: Remove this after replacing all uses in favor of queries to + * module_mapper_t to handle both generated and module code. + */ /** * Get the module map. */ @@ -852,6 +916,9 @@ template class trace_converter_t { return *modvec_ptr_; } + /* TODO i#2062: Remove this after replacing all uses in favor of queries to + * module_mapper_t to handle both generated and module code. + */ /** * Set the module map. Must be called before process_offline_entry() is called. */ @@ -861,6 +928,26 @@ template class trace_converter_t { modvec_ptr_ = modvec; } + /** + * Get the module mapper. + */ + const module_mapper_t & + modmap_() const + { + return *modmap_ptr_; + } + + /** + * Set the module mapper. Must be called before process_offline_entry() is called. + */ + void + set_modmap_(const module_mapper_t *modmap) + { + modmap_ptr_ = modmap; + } + + const module_mapper_t *modmap_ptr_ = nullptr; + private: T * impl() @@ -881,8 +968,6 @@ template class trace_converter_t { OFFLINE_FILE_TYPE_INSTRUCTION_ONLY, impl()->get_file_type(tls))) return ""; - // Avoid type complaints for 32-bit. - size_t modidx_typed = static_cast(modidx); // We build an ilist to use identify_elidable_addresses() and fill in // state needed to reconstruct elided addresses. instrlist_t *ilist = instrlist_create(dcontext_); @@ -914,8 +999,7 @@ template class trace_converter_t { pc = instr_get_app_pc(meminst); int index_in_bb = static_cast(reinterpret_cast(instr_get_note(meminst))); - app_pc orig_pc = pc - modvec_()[modidx_typed].map_seg_base + - modvec_()[modidx_typed].orig_seg_base; + app_pc orig_pc = modmap_().get_orig_pc_from_map_pc(pc, modidx, modoffs); impl()->log(5, "Marking < " PFX ", " PFX "> %s #%d to use remembered base\n", start_pc, pc, write ? "write" : "read", memop_index); if (!impl()->set_instr_summary_flags( @@ -977,8 +1061,8 @@ template class trace_converter_t { if (remember_index == -1) continue; app_pc pc_prev = instr_get_app_pc(prev); - app_pc orig_pc_prev = pc_prev - modvec_()[modidx_typed].map_seg_base + - modvec_()[modidx_typed].orig_seg_base; + app_pc orig_pc_prev = + modmap_().get_orig_pc_from_map_pc(pc_prev, modidx, modoffs); int index_prev = static_cast(reinterpret_cast(instr_get_note(prev))); if (!impl()->set_instr_summary_flags( @@ -1021,32 +1105,39 @@ template class trace_converter_t { std::string error = ""; uint instr_count = in_entry->pc.instr_count; const instr_summary_t *instr = nullptr; - if ((in_entry->pc.modidx == 0 && in_entry->pc.modoffs == 0) || - in_entry->pc.modidx == PC_MODIDX_INVALID || - modvec_()[in_entry->pc.modidx].map_seg_base == NULL) { - // FIXME i#2062: add support for code not in a module (vsyscall, JIT, etc.). - // Once that support is in we can remove the bool return value and handle - // the memrefs up here. - // A race is fine for our visible ~one-time warning at level 0. + app_pc start_pc = modmap_().get_map_pc(in_entry->pc.modidx, in_entry->pc.modoffs); + app_pc pc, decode_pc = start_pc; + if (in_entry->pc.modidx == PC_MODIDX_INVALID) { + impl()->log(3, "Appending %u instrs in bb " PFX " in generated code\n", + instr_count, (ptr_uint_t)start_pc); + } else if ((in_entry->pc.modidx == 0 && in_entry->pc.modoffs == 0) || + modvec_()[in_entry->pc.modidx].map_seg_base == NULL) { + if (impl()->get_version(tls) >= OFFLINE_FILE_VERSION_ENCODINGS) { + // This is a fatal error if this trace should have encodings. + return "Non-module instructions found with no encoding information."; + } + // This is a legacy trace without generated code support. + // A race is fine for our visible ~one-time warning at level 0. static volatile bool warned_once; if (!warned_once) { impl()->log( - 0, "WARNING: Skipping ifetch for instructions not in a module\n"); + 0, "WARNING: Skipping ifetch for unknown-encoding instructions\n"); warned_once = true; } impl()->log( - 1, "Skipping ifetch for %u instrs not in a module (idx %d, +" PIFX ")\n", + 1, "Skipping ifetch for %u unknown-encoding instrs (idx %d, +" PIFX ")\n", instr_count, in_entry->pc.modidx, in_entry->pc.modoffs); + // XXX i#2062: If we abandon this graceful skip (maybe once all legacy + // traces have expired), we can remove the bool return value and handle the + // memrefs in this function. *handled = false; return ""; + } else { + impl()->log(3, "Appending %u instrs in bb " PFX " in mod %u +" PIFX " = %s\n", + instr_count, (ptr_uint_t)start_pc, (uint)in_entry->pc.modidx, + (ptr_uint_t)in_entry->pc.modoffs, + modvec_()[in_entry->pc.modidx].path); } - app_pc start_pc = modvec_()[in_entry->pc.modidx].map_seg_base + - (in_entry->pc.modoffs - modvec_()[in_entry->pc.modidx].seg_offs); - app_pc pc, decode_pc = start_pc; - impl()->log(3, "Appending %u instrs in bb " PFX " in mod %u +" PIFX " = %s\n", - instr_count, (ptr_uint_t)start_pc, (uint)in_entry->pc.modidx, - (ptr_uint_t)in_entry->pc.modoffs, - modvec_()[in_entry->pc.modidx].path); bool skip_icache = false; // This indicates that each memref has its own PC entry and that each // icache entry does not need to be considered a memref PC entry as well. @@ -1055,10 +1146,8 @@ template class trace_converter_t { impl()->get_file_type(tls)); bool is_instr_only_trace = TESTANY(OFFLINE_FILE_TYPE_INSTRUCTION_ONLY, impl()->get_file_type(tls)); - // Cast to unsigned pointer-sized int first to avoid sign-extending. - uint64_t cur_pc = static_cast(reinterpret_cast( - modvec_()[in_entry->pc.modidx].orig_seg_base)) + - (in_entry->pc.modoffs - modvec_()[in_entry->pc.modidx].seg_offs); + uint64_t cur_pc = reinterpret_cast( + modmap_().get_orig_pc(in_entry->pc.modidx, in_entry->pc.modoffs)); // Legacy traces need the offset, not the pc. uint64_t cur_offs = in_entry->pc.modoffs; std::unordered_map reg_vals; @@ -1086,8 +1175,8 @@ template class trace_converter_t { for (uint i = 0; i < instr_count; ++i) { trace_entry_t *buf_start = impl()->get_write_buffer(tls); trace_entry_t *buf = buf_start; - app_pc orig_pc = decode_pc - modvec_()[in_entry->pc.modidx].map_seg_base + - modvec_()[in_entry->pc.modidx].orig_seg_base; + app_pc orig_pc = modmap_().get_orig_pc_from_map_pc( + decode_pc, in_entry->pc.modidx, in_entry->pc.modoffs); // To avoid repeatedly decoding the same instruction on every one of its // dynamic executions, we cache the decoding in a hashtable. pc = decode_pc; @@ -1372,8 +1461,12 @@ template class trace_converter_t { (*entry)->extended.valueB == TRACE_MARKER_TYPE_KERNEL_XFER) { if (impl()->get_version(tls) >= OFFLINE_FILE_VERSION_KERNEL_INT_PC) { // We convert the idx:offs to an absolute PC. + // TODO i#2062: For non-module code we don't have the block id and so + // cannot use this format. We should probably just abandon this and + // always store the absolute PC. kernel_interrupted_raw_pc_t raw_pc; raw_pc.combined_value = marker_val; + DR_ASSERT(raw_pc.pc.modidx != PC_MODIDX_INVALID); app_pc pc = modvec_()[raw_pc.pc.modidx].orig_seg_base + (raw_pc.pc.modoffs - modvec_()[raw_pc.pc.modidx].seg_offs); impl()->log(3, @@ -1529,10 +1622,12 @@ template class trace_converter_t { */ class raw2trace_t : public trace_converter_t { public: - // module_map, thread_files and out_files are all owned and opened/closed by the - // caller. module_map is not a string and can contain binary data. + // module_map, encoding_file, thread_files, and out_files are all owned and + // opened/closed by the caller. module_map is not a string and can contain binary + // data. raw2trace_t(const char *module_map, const std::vector &thread_files, - const std::vector &out_files, void *dcontext = NULL, + const std::vector &out_files, + file_t encoding_file = INVALID_FILE, void *dcontext = nullptr, unsigned int verbosity = 0, int worker_count = -1, const std::string &alt_module_dir = ""); virtual ~raw2trace_t(); @@ -1659,6 +1754,8 @@ class raw2trace_t : public trace_converter_t { , last_entry_is_split(false) , prev_instr_was_rep_string(false) , last_decode_block_start(nullptr) + , last_decode_modidx(0) + , last_decode_modoffs(0) , last_block_summary(nullptr) { } @@ -1685,7 +1782,10 @@ class raw2trace_t : public trace_converter_t { offline_entry_t last_split_first_entry; std::array out_buf; bool prev_instr_was_rep_string; + // There is no sentinel available for modidx+modoffs so we use the pc for that. app_pc last_decode_block_start; + uint64 last_decode_modidx; + uint64 last_decode_modoffs; block_summary_t *last_block_summary; uint64 last_window = 0; @@ -1708,6 +1808,8 @@ class raw2trace_t : public trace_converter_t { uint64 count_elided_ = 0; + std::unique_ptr module_mapper_; + private: friend class trace_converter_t; @@ -1731,7 +1833,7 @@ class raw2trace_t : public trace_converter_t { instr_summary_exists(void *tls, uint64 modidx, uint64 modoffs, app_pc block_start, int index, app_pc pc); block_summary_t * - lookup_block_summary(void *tls, app_pc block_start); + lookup_block_summary(void *tls, uint64 modidx, uint64 modoffs, app_pc block_start); instr_summary_t * lookup_instr_summary(void *tls, uint64 modidx, uint64 modoffs, app_pc block_start, int index, app_pc pc, OUT block_summary_t **block_summary); @@ -1789,6 +1891,11 @@ class raw2trace_t : public trace_converter_t { // instruction pc. Now that we use block_summary_t and only look up each block, // the hashtable performance matters much less. // We use a per-worker cache to avoid locks. + static inline void * + decode_cache_key(uint64 modidx, uint64 modoffs) + { + return reinterpret_cast((modidx << PC_MODOFFS_BITS) | modoffs); + } std::vector decode_cache_; // Store optional parameters for the module_mapper_t until we need to construct it. @@ -1799,7 +1906,7 @@ class raw2trace_t : public trace_converter_t { void *user_process_data_ = nullptr; const char *modmap_; - std::unique_ptr module_mapper_; + file_t encoding_file_ = INVALID_FILE; unsigned int verbosity_ = 0; diff --git a/clients/drcachesim/tracer/raw2trace_launcher.cpp b/clients/drcachesim/tracer/raw2trace_launcher.cpp index 50f5ff3f487..ae90fc0ad2f 100644 --- a/clients/drcachesim/tracer/raw2trace_launcher.cpp +++ b/clients/drcachesim/tracer/raw2trace_launcher.cpp @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2016-2020 Google, Inc. All rights reserved. + * Copyright (c) 2016-2022 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -98,9 +98,9 @@ _tmain(int argc, const TCHAR *targv[]) std::string dir_err = dir.initialize(op_indir.get_value(), op_outdir.get_value()); if (!dir_err.empty()) FATAL_ERROR("Directory parsing failed: %s", dir_err.c_str()); - raw2trace_t raw2trace(dir.modfile_bytes_, dir.in_files_, dir.out_files_, NULL, - op_verbose.get_value(), op_jobs.get_value(), - op_alt_module_dir.get_value()); + raw2trace_t raw2trace(dir.modfile_bytes_, dir.in_files_, dir.out_files_, + dir.encoding_file_, nullptr, op_verbose.get_value(), + op_jobs.get_value(), op_alt_module_dir.get_value()); std::string error = raw2trace.do_conversion(); if (!error.empty()) FATAL_ERROR("Conversion failed: %s", error.c_str()); diff --git a/core/unix/signal.c b/core/unix/signal.c index 2f7e3acea6b..838f257ee63 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -333,6 +333,8 @@ dump_unmasked(dcontext_t *dcontext, const char *where) if (dcontext == GLOBAL_DCONTEXT || dcontext == NULL) return; thread_sig_info_t *info = (thread_sig_info_t *)dcontext->signal_field; + if (info->sighand == NULL) + return; LOG(THREAD, LOG_ASYNCH, 3, "%s: threads_unmasked: ", where); for (int i = 1; i <= MAX_SIGNUM; i++) { LOG(THREAD, LOG_ASYNCH, 3, "[%d]=%d ", i, info->sighand->threads_unmasked[i]); @@ -1110,9 +1112,9 @@ signal_thread_inherit(dcontext_t *dcontext, void *clone_record) ? "vfork" : #endif - (IF_LINUX(record->clone_sysnum == SYS_clone - ? "clone" - : record->clone_sysnum == SYS_clone3 ? "clone3" :) + (IF_LINUX(record->clone_sysnum == SYS_clone ? "clone" + : record->clone_sysnum == SYS_clone3 ? "clone3" + :) IF_MACOS(record->clone_sysnum == SYS_bsdthread_create ? "bsdthread_create" :) "unexpected"), diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index bd650a271b9..a86c4da1d76 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -3832,8 +3832,14 @@ if (BUILD_CLIENTS) ${PROJECT_SOURCE_DIR}/clients/drcachesim/tests/burst_gencode.cpp OFF ON) use_DynamoRIO_static_client(tool.drcacheoff.gencode drmemtrace_static) + use_DynamoRIO_drmemtrace_tracer(tool.drcacheoff.gencode) + target_link_libraries(tool.drcacheoff.gencode drmemtrace_raw2trace + drmemtrace_analyzer) target_include_directories(tool.drcacheoff.gencode PUBLIC - "${PROJECT_BINARY_DIR}/clients/include") + "${PROJECT_SOURCE_DIR}/clients/drcachesim" + "${PROJECT_SOURCE_DIR}/clients/drcachesim/common" + "${PROJECT_SOURCE_DIR}/clients/drcachesim/reader" + "${PROJECT_SOURCE_DIR}/clients/drcachesim/tracer") set(tool.drcacheoff.gencode_nodr ON) torunonly_drcacheoff(gencode tool.drcacheoff.gencode "" "@-simulator_type@basic_counts" "") From 645d7e11a4587542a2ed2bbc4f4f8fd08d1e8e32 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Thu, 18 Aug 2022 14:58:37 -0400 Subject: [PATCH 2/7] Fix Windows warnings; remove up-front version complaint b/c some tests don't have encoding files --- clients/drcachesim/tracer/raw2trace.cpp | 6 +++--- clients/drcachesim/tracer/raw2trace.h | 11 +++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/clients/drcachesim/tracer/raw2trace.cpp b/clients/drcachesim/tracer/raw2trace.cpp index d9957744fe0..f7a8bfd5a17 100644 --- a/clients/drcachesim/tracer/raw2trace.cpp +++ b/clients/drcachesim/tracer/raw2trace.cpp @@ -601,9 +601,9 @@ raw2trace_t::process_next_thread_buffer(raw2trace_thread_data_t *tdata, tdata->file_type); if (!tdata->error.empty()) return tdata->error; - if (tdata->version >= OFFLINE_FILE_VERSION_ENCODINGS && - encoding_file_ == INVALID_FILE) - return "Expected encoding file but none was passed"; + // We do not complain if tdata->version >= OFFLINE_FILE_VERSION_ENCODINGS + // and encoding_file_ == INVALID_FILE since we have several tests with + // that setup. We do complain during processing about unknown instructions. if (tdata->saw_header) { tdata->error = process_header(tdata); if (!tdata->error.empty()) diff --git a/clients/drcachesim/tracer/raw2trace.h b/clients/drcachesim/tracer/raw2trace.h index df4ff4185c1..97991af6563 100644 --- a/clients/drcachesim/tracer/raw2trace.h +++ b/clients/drcachesim/tracer/raw2trace.h @@ -433,7 +433,8 @@ class module_mapper_t { return (map_pc - entry->encodings) + reinterpret_cast(entry->start_pc); } else { - return map_pc - modvec_[modidx].map_seg_base + modvec_[modidx].orig_seg_base; + size_t idx = static_cast(modidx); // Avoid win32 warnings. + return map_pc - modvec_[idx].map_seg_base + modvec_[idx].orig_seg_base; } } @@ -447,10 +448,11 @@ class module_mapper_t { encoding_entry_t *entry = it->second; return reinterpret_cast(entry->start_pc); } else { + size_t idx = static_cast(modidx); // Avoid win32 warnings. // Cast to unsigned pointer-sized int first to avoid sign-extending. return reinterpret_cast( - reinterpret_cast(modvec_[modidx].orig_seg_base)) + - (modoffs - modvec_[modidx].seg_offs); + reinterpret_cast(modvec_[idx].orig_seg_base)) + + (modoffs - modvec_[idx].seg_offs); } } @@ -464,7 +466,8 @@ class module_mapper_t { encoding_entry_t *entry = it->second; return entry->encodings; } else { - return modvec_[modidx].map_seg_base + (modoffs - modvec_[modidx].seg_offs); + size_t idx = static_cast(modidx); // Avoid win32 warnings. + return modvec_[idx].map_seg_base + (modoffs - modvec_[idx].seg_offs); } } From 4c0e9688daf58eb39733f08ffd9fac6f9c5d46a7 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Thu, 18 Aug 2022 22:58:41 -0400 Subject: [PATCH 3/7] Fix 32-bit hash key limitations by using unordered_map there; keeps hashtable_t for 64-bit by adding a wrapper class block_hashtable_t --- clients/drcachesim/tracer/raw2trace.cpp | 37 ++-------- clients/drcachesim/tracer/raw2trace.h | 89 +++++++++++++++++++++---- 2 files changed, 83 insertions(+), 43 deletions(-) diff --git a/clients/drcachesim/tracer/raw2trace.cpp b/clients/drcachesim/tracer/raw2trace.cpp index f7a8bfd5a17..f28192cbe87 100644 --- a/clients/drcachesim/tracer/raw2trace.cpp +++ b/clients/drcachesim/tracer/raw2trace.cpp @@ -776,8 +776,7 @@ raw2trace_t::lookup_block_summary(void *tls, uint64 modidx, uint64 modoffs, tdata->last_block_summary, tdata->last_decode_block_start); return tdata->last_block_summary; } - block_summary_t *ret = static_cast(hashtable_lookup( - &decode_cache_[tdata->worker], decode_cache_key(modidx, modoffs))); + block_summary_t *ret = decode_cache_[tdata->worker].lookup(modidx, modoffs); if (ret != nullptr) { DEBUG_ASSERT(ret->start_pc == block_start); tdata->last_decode_block_start = block_start; @@ -825,12 +824,11 @@ raw2trace_t::create_instr_summary(void *tls, uint64 modidx, uint64 modoffs, if (block == nullptr) { block = new block_summary_t(block_start, instr_count); DEBUG_ASSERT(index >= 0 && index < static_cast(block->instrs.size())); - hashtable_add(&decode_cache_[tdata->worker], decode_cache_key(modidx, modoffs), - block); + decode_cache_[tdata->worker].add(modidx, modoffs, block); VPRINT(5, "Created new block summary " PFX " for " PFX " modidx=" INT64_FORMAT_STRING - " modoffs=" HEX64_FORMAT_STRING " key=%p\n", - block, block_start, modidx, modoffs, decode_cache_key(modidx, modoffs)); + " modoffs=" HEX64_FORMAT_STRING "\n", + block, block_start, modidx, modoffs); tdata->last_decode_block_start = block_start; tdata->last_decode_modidx = modidx; tdata->last_decode_modoffs = modoffs; @@ -1210,35 +1208,14 @@ raw2trace_t::raw2trace_t(const char *module_map, } } else cache_count = 1; - decode_cache_.resize(cache_count); - for (int i = 0; i < cache_count; ++i) { - // We go ahead and start with a reasonably large capacity. - // We do not want the built-in mutex: this is per-worker so it can be lockless. - hashtable_init_ex(&decode_cache_[i], 16, HASH_INTPTR, false, false, nullptr, - nullptr, nullptr); - // We pay a little memory to get a lower load factor, unless we have - // many duplicated tables. - hashtable_config_t config = { sizeof(config), true, - worker_count_ <= 8 - ? 40U - : (worker_count_ <= 16 ? 50U : 60U) }; - hashtable_configure(&decode_cache_[i], &config); - } + decode_cache_.reserve(cache_count); + for (int i = 0; i < cache_count; ++i) + decode_cache_.emplace_back(cache_count); } raw2trace_t::~raw2trace_t() { module_mapper_.reset(); - for (size_t i = 0; i < decode_cache_.size(); ++i) { - // XXX: We can't use a free-payload function b/c we can't get the dcontext there, - // so we have to explicitly free the payloads. - for (uint j = 0; j < HASHTABLE_SIZE(decode_cache_[i].table_bits); j++) { - for (hash_entry_t *e = decode_cache_[i].table[j]; e != NULL; e = e->next) { - delete (static_cast(e->payload)); - } - } - hashtable_delete(&decode_cache_[i]); - } } bool diff --git a/clients/drcachesim/tracer/raw2trace.h b/clients/drcachesim/tracer/raw2trace.h index 97991af6563..5123b44c9cd 100644 --- a/clients/drcachesim/tracer/raw2trace.h +++ b/clients/drcachesim/tracer/raw2trace.h @@ -1886,20 +1886,83 @@ class raw2trace_t : public trace_converter_t { int worker_count_; std::vector> worker_tasks_; - // We use a hashtable to cache decodings. We compared the performance of - // hashtable_t to std::map.find, std::map.lower_bound, std::tr1::unordered_map, - // and c++11 std::unordered_map (including tuning its load factor, initial size, - // and hash function), and hashtable_t outperformed the others (i#2056). - // Update: that measurement was when we did a hashtable lookup on every - // instruction pc. Now that we use block_summary_t and only look up each block, - // the hashtable performance matters much less. + class block_hashtable_t { + // We use a hashtable to cache decodings. We compared the performance of + // hashtable_t to std::map.find, std::map.lower_bound, std::tr1::unordered_map, + // and c++11 std::unordered_map (including tuning its load factor, initial size, + // and hash function), and hashtable_t outperformed the others (i#2056). + // Update: that measurement was when we did a hashtable lookup on every + // instruction pc. Now that we use block_summary_t and only look up each block, + // the hashtable performance matters much less. + // Plus, for 32-bit we cannot fit our modidx:modoffs key in the 32-bit-limited + // hashtable_t key, so we now use std::unordered_map there. + public: + explicit block_hashtable_t(int worker_count) + { +#ifdef X64 + // We go ahead and start with a reasonably large capacity. + // We do not want the built-in mutex: this is per-worker so it can be + // lockless. + hashtable_init_ex(&table, 16, HASH_INTPTR, false, false, free_payload, + nullptr, nullptr); + // We pay a little memory to get a lower load factor, unless we have + // many duplicated tables. + hashtable_config_t config = { sizeof(config), true, + worker_count <= 8 + ? 40U + : (worker_count <= 16 ? 50U : 60U) }; + hashtable_configure(&table, &config); +#endif + } +#ifdef X64 + ~block_hashtable_t() + { + hashtable_delete(&table); + } +#endif + block_summary_t * + lookup(uint64 modidx, uint64 modoffs) + { +#ifdef X64 + return static_cast(hashtable_lookup( + &table, reinterpret_cast(hash_key(modidx, modoffs)))); +#else + return table[hash_key(modidx, modoffs)].get(); +#endif + } + // Takes ownership of "block". + void + add(uint64 modidx, uint64 modoffs, block_summary_t *block) + { +#ifdef X64 + hashtable_add(&table, reinterpret_cast(hash_key(modidx, modoffs)), + block); +#else + table[hash_key(modidx, modoffs)].reset(block); +#endif + } + + private: + static void + free_payload(void *ptr) + { + delete (static_cast(ptr)); + } + static inline uint64 + hash_key(uint64 modidx, uint64 modoffs) + { + return (modidx << PC_MODOFFS_BITS) | modoffs; + } + +#ifdef X64 + hashtable_t table; +#else + std::unordered_map> table; +#endif + }; + // We use a per-worker cache to avoid locks. - static inline void * - decode_cache_key(uint64 modidx, uint64 modoffs) - { - return reinterpret_cast((modidx << PC_MODOFFS_BITS) | modoffs); - } - std::vector decode_cache_; + std::vector decode_cache_; // Store optional parameters for the module_mapper_t until we need to construct it. const char *(*user_parse_)(const char *src, OUT void **data) = nullptr; From 0a04bdd7dd253305aceb43f58ce131fabb6ad016 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Fri, 19 Aug 2022 01:18:02 -0400 Subject: [PATCH 4/7] Work around Visual Studio known problem with unique_ptr in STL containers --- clients/drcachesim/tracer/raw2trace.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/clients/drcachesim/tracer/raw2trace.h b/clients/drcachesim/tracer/raw2trace.h index 5123b44c9cd..0baebc8b265 100644 --- a/clients/drcachesim/tracer/raw2trace.h +++ b/clients/drcachesim/tracer/raw2trace.h @@ -1919,6 +1919,15 @@ class raw2trace_t : public trace_converter_t { { hashtable_delete(&table); } +#else + // Work around a known Visual Studio issue where it complains about deleted copy + // constructors for unique_ptr by deleting our copies and defaulting our moves. + block_hashtable_t(const block_hashtable_t &) = delete; + block_hashtable_t & + operator=(const block_hashtable_t &) = delete; + block_hashtable_t(block_hashtable_t &&) = default; + block_hashtable_t & + operator=(block_hashtable_t &&) = default; #endif block_summary_t * lookup(uint64 modidx, uint64 modoffs) From ba6dc25bd2faaa7787f6b101b0c900102d2afb69 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Fri, 19 Aug 2022 10:53:55 -0400 Subject: [PATCH 5/7] Fix 32-bit incorrect sign extension --- clients/drcachesim/tracer/raw2trace.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clients/drcachesim/tracer/raw2trace.h b/clients/drcachesim/tracer/raw2trace.h index 0baebc8b265..75f12847ab4 100644 --- a/clients/drcachesim/tracer/raw2trace.h +++ b/clients/drcachesim/tracer/raw2trace.h @@ -1149,8 +1149,9 @@ template class trace_converter_t { impl()->get_file_type(tls)); bool is_instr_only_trace = TESTANY(OFFLINE_FILE_TYPE_INSTRUCTION_ONLY, impl()->get_file_type(tls)); - uint64_t cur_pc = reinterpret_cast( - modmap_().get_orig_pc(in_entry->pc.modidx, in_entry->pc.modoffs)); + // Cast to unsigned pointer-sized int first to avoid sign-extending. + uint64_t cur_pc = static_cast(reinterpret_cast( + modmap_().get_orig_pc(in_entry->pc.modidx, in_entry->pc.modoffs))); // Legacy traces need the offset, not the pc. uint64_t cur_offs = in_entry->pc.modoffs; std::unordered_map reg_vals; From 0c1523f6faca475a3d3be8ec2f4e8d5fbd5991ee Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Fri, 19 Aug 2022 14:42:05 -0400 Subject: [PATCH 6/7] Fix encoding file reading checking map size instead of file size --- clients/drcachesim/tracer/raw2trace.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clients/drcachesim/tracer/raw2trace.cpp b/clients/drcachesim/tracer/raw2trace.cpp index f28192cbe87..0d6bc3c9de9 100644 --- a/clients/drcachesim/tracer/raw2trace.cpp +++ b/clients/drcachesim/tracer/raw2trace.cpp @@ -329,9 +329,11 @@ module_mapper_t::do_encoding_parsing() if (*reinterpret_cast(map_start) != ENCODING_FILE_VERSION) return "Encoding file has invalid version"; size_t offs = sizeof(uint64_t); - while (offs < map_size) { + while (offs < file_size) { encoding_entry_t *entry = reinterpret_cast(map_start + offs); - if (offs + entry->length > map_size) + if (entry->length < sizeof(encoding_entry_t)) + return "Encoding file is corrupted"; + if (offs + entry->length > file_size) return "Encoding file is truncated"; encodings_[entry->id] = entry; offs += entry->length; From a2cfa3fd00e36b96955189226b98b461fd748156 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Fri, 19 Aug 2022 16:31:05 -0400 Subject: [PATCH 7/7] Review requests: extra comment on sentinel; split class comments --- clients/drcachesim/tests/raw2trace_unit_tests.cpp | 5 +++-- clients/drcachesim/tracer/raw2trace.cpp | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/clients/drcachesim/tests/raw2trace_unit_tests.cpp b/clients/drcachesim/tests/raw2trace_unit_tests.cpp index 95ac5af027a..a7e8b773420 100644 --- a/clients/drcachesim/tests/raw2trace_unit_tests.cpp +++ b/clients/drcachesim/tests/raw2trace_unit_tests.cpp @@ -65,8 +65,8 @@ # error Unsupported arch #endif -// Subclasses raw2trace_t and module_mapper_t and replaces the module loading with -// a buffer of encoded instr_t. +// Subclasses module_mapper_t and replaces the module loading with a +// buffer of encoded instr_t. class module_mapper_test_t : public module_mapper_t { public: module_mapper_test_t(instrlist_t &instrs, void *drcontext) @@ -89,6 +89,7 @@ class module_mapper_test_t : public module_mapper_t { byte decode_buf_[MAX_DECODE_SIZE]; }; +// Subclasses raw2trace_t and replaces the module_mapper_t with our own version. class raw2trace_test_t : public raw2trace_t { public: raw2trace_test_t(const std::vector &input, diff --git a/clients/drcachesim/tracer/raw2trace.cpp b/clients/drcachesim/tracer/raw2trace.cpp index 0d6bc3c9de9..609794e2bad 100644 --- a/clients/drcachesim/tracer/raw2trace.cpp +++ b/clients/drcachesim/tracer/raw2trace.cpp @@ -772,6 +772,7 @@ raw2trace_t::lookup_block_summary(void *tls, uint64 modidx, uint64 modoffs, app_pc block_start) { auto tdata = reinterpret_cast(tls); + // There is no sentinel available for modidx+modoffs so we use block_start for that. if (block_start == tdata->last_decode_block_start && modidx == tdata->last_decode_modidx && modoffs == tdata->last_decode_modoffs) { VPRINT(5, "Using last block summary " PFX " for " PFX "\n",