Address reviewer comments.

DynamoRIO · Nov 25, 2023 · cecad9d · cecad9d
1 parent 045fd47
commit cecad9d
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 4 deletions.
diff --git a/api/docs/release.dox b/api/docs/release.dox
@@ -143,8 +143,9 @@ changes:
 
 Further non-compatibility-affecting changes include:
  - Added a new scheme for the modoffs field in the PC trace entry which allows L0
-   filtering of non-module code. Also added file type entry to the header of encoding
-   files.
+   filtering of non-module code; see
+   #dynamorio::drmemtrace::ENCODING_FILE_TYPE_SEPARATE_NON_MOD_INSTRS. Also added
+   file type entry to the header of encoding files.
  - Fixed a bug in the AArch64 codec with the way that SVE scalar+immediate predicated
    contiguous load and store instructions represented the immediate offset in the IR.
    In 10.0.0 the memory operand in these instruction used the immediate value from the

diff --git a/clients/drcachesim/common/trace_entry.h b/clients/drcachesim/common/trace_entry.h
@@ -993,7 +993,7 @@ typedef enum {
      * entries in the trace correspond to an individual instr. The modoffs field is
      * interpreted as the cumulative encoding length of all instrs written to the
      * encoding file before the recorded instr. If this file type is not set, then
-     * non-module PC entries are interpreted as the non-mod block's idx.
+     * the PC entries' modoffs fields are interpreted as the non-mod block's idx.
      */
     ENCODING_FILE_TYPE_SEPARATE_NON_MOD_INSTRS = 0x1,
 } encoding_file_type_t;

diff --git a/clients/drcachesim/tests/offline-gencode_filtered_until.templatex b/clients/drcachesim/tests/offline-gencode_filtered_until.templatex
@@ -0,0 +1,6 @@
+pre-DR init
+pre-DR start
+pre-DR detach
+all done
+Opcode mix tool results:
+.*
diff --git a/clients/drcachesim/tracer/raw2trace.cpp b/clients/drcachesim/tracer/raw2trace.cpp
@@ -356,6 +356,8 @@ module_mapper_t::do_encoding_parsing()
     if (encoding_file_version > ENCODING_FILE_VERSION)
         return "Encoding file has invalid version";
     if (encoding_file_version >= ENCODING_FILE_VERSION_HAS_FILE_TYPE) {
+        if (map_at + sizeof(uint64_t) > map_end)
+            return "Encoding file header is truncated";
         uint64_t encoding_file_type = *reinterpret_cast<uint64_t *>(map_at);
         map_at += sizeof(uint64_t);
         separate_non_mod_instrs_ =

diff --git a/clients/drcachesim/tracer/tracer.cpp b/clients/drcachesim/tracer/tracer.cpp
@@ -2319,6 +2319,8 @@ drmemtrace_client_main(client_id_t id, int argc, const char *argv[])
         /* we use placement new for better isolation */
         DR_ASSERT(MAX_INSTRU_SIZE >= sizeof(offline_instru_t));
         placement = dr_global_alloc(MAX_INSTRU_SIZE);
+        // TODO i#6474, i#2062: Also handle op_L0_filter_until_instrs here when
+        // i#6474 is resolved.
         instru = new (placement) offline_instru_t(
             insert_load_buf_ptr, &scratch_reserve_vec, file_ops_func.write_file,
             module_file, encoding_file, op_disable_optimizations.get_value(),

diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt
@@ -4260,7 +4260,11 @@ if (BUILD_CLIENTS)
 
       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")
+        "" "@-simulator_type@opcode_mix"
+        "-L0_filter -subdir_prefix drmemtrace.tool.drcacheoff.gencode_filtered")
+
+      # TODO i#6474, i#2062: Add a similar gencode test with
+      # -L0_filter_until_instrs when i#6474 is fixed.
     endif ()
 
     # XXX i#1551: startstop API is NYI on ARM