i#7113 decode cache: Add analyzer library for decode_cache_t #7114
Conversation
Adds a new library to cache information about decoded instructions. This can be used by analysis tools that need to decode the instr encodings in the trace. The library allows the tools to specify what information they need to cache. Refactors the invariant checker tool to use this library. Issue: #7113
abhinav92003
changed the title
|
Decided to try out an alternate way to support module-mapper-decoding in instr_decode_cache_t that came out of offline discussion. Okay to hold off on the re-review until then (Cannot undo re-request review) |
…upport to instr_decode_cache_t
abhinav92003
changed the title
|
Almost all concerns from the review and offline discussions have been addressed, so this is ready for a re-review. Added a TODO for a couple items: dr_set_isa_mode for regdeps, and the Windows test-only i#5960. PTAL. |
|
Location for test_decode_cache_t and providing raw instr encoding for view_t need more discussion; resolved other threads. Ready for re-review. Though no rush because of the holidays. |
| } | ||
| dr_close_file(modfile); | ||
| module_mapper_ = | ||
| module_mapper_t::create(modfile_bytes_, nullptr, nullptr, nullptr, nullptr, |
There was a problem hiding this comment.
We have to override/replace this code internally: currently we do so in an opcode_mix subclass. This will thus break internal use without corresponding internal changes: make sure you have a valid and tested path forward there.
There was a problem hiding this comment.
Still trying to address this, so not requesting a re-review just yet.
|
|
||
| ++shard->instr_count; | ||
| opcode_data_t *opcode_data; | ||
| shard->error = shard->decode_cache->add_decode_info(memref.instr, opcode_data); |
There was a problem hiding this comment.
May be worth profiling an opcode_mix analyzer run on a medium-sized trace on local disk just to confirm there's no caching/lookup bottleneck we don't expect.
| if (shard_map_.empty()) { | ||
| total = serial_shard_; | ||
| // No default copy constructor for shard_data_t because of the | ||
| // std::unique_ptr, so we resort to keeping a pointer to it. |
There was a problem hiding this comment.
Will likely need a similar internal change: please test and prepare ahead of time.
| - Added the #dynamorio::drmemtrace::decode_cache_t library to make it easier and more | ||
| efficient for drmemtrace analysis tools to obtain decoded information about | ||
| instructions in the trace. This works for traces that have embedded instruction | ||
| encodings in them, and also for traces without embedded encodings where the |
There was a problem hiding this comment.
s/traces/legacy traces/
| @@ -142,6 +142,13 @@ copies of the binaries and reading the raw bytes for each instruction | |||
| in order to obtain the opcode and full operand information. | |||
| See also \ref sec_drcachesim_core. | |||
|
|
|||
| drmemtrace analysis tools may use the | |||
| #dynamorio::drmemtrace::decode_cache_t library, which handles the | |||
| heavy-lifting of decoding the trace instructions using either the embedded | |||
There was a problem hiding this comment.
nit: I think this should just be heavy lifting
| #dynamorio::drmemtrace::decode_cache_t library, which handles the | ||
| heavy-lifting of decoding the trace instructions using either the embedded | ||
| encodings in the trace or the encodings from the app binaries, and managing | ||
| their cache (including invalidating stale encodings based on the |
There was a problem hiding this comment.
nit: "managing their cache" w/o prior reference to a cache sounds off to me: how about "and manages caching the encodings" or sthg.
| err = check_decode_caching(drcontext, /*persist_instrs=*/true, | ||
| /*use_module_mapper=*/true); | ||
| err = check_decode_caching(drcontext, /*use_module_mapper=*/true, | ||
| /*include_decoded_instr=*/true, |
There was a problem hiding this comment.
Is it illegal to have include=false persist=true?
There was a problem hiding this comment.
Yes it is illegal. It's included in the doc, and there's also an assert.
Adds a new drmemtrace_decode_cache library to cache information about decoded instructions using decode_cache_t. This can be used by analysis tools that need to decode the instr encodings in the trace, to avoid overhead of redundant decodings which can get expensive.
The library allows the tools to specify what information they need to cache. Also, it uses instr_noalloc_t when possible to reduce heap usage and allocation/deallocation overhead.
If the trace does not include embedded encodings or if the user wants to get encodings from the app binaries using module_mapper_t instead, they can provide the module file path to the init API on the decode_cache_t object. decode_cache_t keeps a single initialized module_mapper_t at any time, which is shared between all decode_cache_t objects (even the ones of different template types); this is done by tracking the count of active objects using the module mapper.
decode_cache_t provides the clear_cache() API which can be used in parallel_shard_exit() to keep memory consumption in check by free-ing up cached decoding info that may not be needed for result computation in later print_results() which has to wait until all shards are done.
Refactors the invariant checker and opcode mix tools to use this library.
Modifies add_encodings_to_memrefs to support a mode where encodings are not set in the generated test memref but only the instr addr and size fields are set.
Makes the opcode cache in opcode_mix_t per-shard instead of per-worker. Decodings must not be cached per-worker as that may cause stale encodings for non-first shards processed by the worker. This means the worker init and worker exit APIs can be removed now from opcode_mix_t.
Adds decode_cache_test and opcode_mix_test unit tests that verify operation of the decode_cache_t.
Issue: #7113