i#7224 skipped header: Get filetype from stream in opcode_mix

Fixes the opcode_mix tool to get the filetype from the memtrace_stream_t interface instead
of the TRACE_MARKER_TYPE_FILETYPE marker memref which may not be seen if -skip_instrs is
applied by the user.

Adds a new unit test that runs the opcode_mix tool with -skip_instrs, which would fail
because of an unseen filetype prior to this fix.

Issue: #7224, #7113

clients/drcachesim/scheduler/scheduler.h

@@ -267,6 +267,10 @@ template <typename RecordType, typename ReaderType> class scheduler_tmpl_t {
          * with no timeout but do not include a corresponding
          * #TRACE_MARKER_TYPE_SYSCALL_SCHEDULE for wakeup, an input could remain
          * unscheduled.
+         *
+         * Also beware that this can skip over trace header entries (like
+         * #TRACE_MARKER_TYPE_FILETYPE), which should be ideally be obtained from the
+         * #dynamorio::drmemtrace::memtrace_stream_t API instead.
          */
         std::vector<range_t> regions_of_interest;
     };

clients/drcachesim/tools/opcode_mix.cpp

@@ -85,6 +85,17 @@ bool
 opcode_mix_t::init_decode_cache(shard_data_t *shard, void *dcontext,
                                 offline_file_type_t filetype)
 {
+    // XXX: Perhaps this should be moved into decode_cache_t::init().
+    // We remove OFFLINE_FILE_TYPE_ARCH_REGDEPS from this check since DR_ISA_REGDEPS
+    // is not a real ISA and can coexist with any real architecture.
+    if (TESTANY(OFFLINE_FILE_TYPE_ARCH_ALL & ~OFFLINE_FILE_TYPE_ARCH_REGDEPS, filetype) &&
+        !TESTANY(build_target_arch_type(), filetype)) {
+        shard->error = std::string("Architecture mismatch: trace recorded on ") +
+            trace_arch_string(static_cast<offline_file_type_t>(filetype)) +
+            " but tool built for " + trace_arch_string(build_target_arch_type());
+        return false;
+    }
+
     shard->decode_cache =
         std::unique_ptr<decode_cache_t<opcode_data_t>>(new decode_cache_t<opcode_data_t>(
             dcontext,
@@ -100,9 +111,12 @@ opcode_mix_t::init_decode_cache(shard_data_t *shard, void *dcontext,
 }
 
 std::string
-opcode_mix_t::initialize()
+opcode_mix_t::initialize_stream(dynamorio::drmemtrace::memtrace_stream_t *serial_stream)
 {
     dcontext_.dcontext = dr_standalone_init();
+    if (serial_stream != nullptr) {
+        serial_shard_.stream = serial_stream;
+    }
     return "";
 }
 
@@ -125,6 +139,7 @@ opcode_mix_t::parallel_shard_init_stream(
     dynamorio::drmemtrace::memtrace_stream_t *shard_stream)
 {
     auto shard = new shard_data_t();
+    shard->stream = shard_stream;
     std::lock_guard<std::mutex> guard(shard_map_mutex_);
     shard_map_[shard_index] = shard;
     return reinterpret_cast<void *>(shard);
@@ -145,22 +160,7 @@ opcode_mix_t::parallel_shard_memref(void *shard_data, const memref_t &memref)
 {
     shard_data_t *shard = reinterpret_cast<shard_data_t *>(shard_data);
     if (memref.marker.type == TRACE_TYPE_MARKER &&
-        memref.marker.marker_type == TRACE_MARKER_TYPE_FILETYPE) {
-        shard->filetype = static_cast<offline_file_type_t>(memref.marker.marker_value);
-        /* We remove OFFLINE_FILE_TYPE_ARCH_REGDEPS from this check since DR_ISA_REGDEPS
-         * is not a real ISA and can coexist with any real architecture.
-         */
-        if (TESTANY(OFFLINE_FILE_TYPE_ARCH_ALL & ~OFFLINE_FILE_TYPE_ARCH_REGDEPS,
-                    memref.marker.marker_value) &&
-            !TESTANY(build_target_arch_type(), memref.marker.marker_value)) {
-            shard->error = std::string("Architecture mismatch: trace recorded on ") +
-                trace_arch_string(static_cast<offline_file_type_t>(
-                    memref.marker.marker_value)) +
-                " but tool built for " + trace_arch_string(build_target_arch_type());
-            return false;
-        }
-    } else if (memref.marker.type == TRACE_TYPE_MARKER &&
-               memref.marker.marker_type == TRACE_MARKER_TYPE_VECTOR_LENGTH) {
+        memref.marker.marker_type == TRACE_MARKER_TYPE_VECTOR_LENGTH) {
 #ifdef AARCH64
         const int new_vl_bits = memref.marker.marker_value * 8;
         if (dr_get_vector_length() != new_vl_bits) {
@@ -177,18 +177,19 @@ opcode_mix_t::parallel_shard_memref(void *shard_data, const memref_t &memref)
     }
 
     /* At this point we start processing memref instructions, so we're past the header of
-     * the trace, which contains the trace filetype. If we didn't encounter a
-     * TRACE_MARKER_TYPE_FILETYPE we return an error and stop processing the trace.
-     * We expected the value of TRACE_MARKER_TYPE_FILETYPE to contain at least one of the
+     * the trace, which contains the trace filetype. If -skip_instrs was used, we may not
+     * have seen TRACE_MARKER_TYPE_FILETYPE itself but the memtrace_stream_t should be
+     * able to provide it to us.
+     */
+    shard->filetype = static_cast<offline_file_type_t>(shard->stream->get_filetype());
+
+    /* We expected the value of TRACE_MARKER_TYPE_FILETYPE to contain at least one of the
      * enum values of offline_file_type_t (e.g., OFFLINE_FILE_TYPE_ARCH_), so
      * OFFLINE_FILE_TYPE_DEFAULT (== 0) should never be present alone and can be used as
      * uninitialized value of filetype for an error check.
      * XXX i#6812: we could allow traces that have some shards with no filetype, as long
      * as there is at least one shard with it, by caching the filetype from shards that
      * have it and using that one. We can do this using memtrace_stream_t::get_filetype(),
-     * which requires updating opcode_mix to use parallel_shard_init_stream() rather than
-     * the current (and deprecated) parallel_shard_init(). However, we should first decide
-     * whether we want to allow traces that have some shards without a filetype.
      */
     if (shard->filetype == OFFLINE_FILE_TYPE_DEFAULT) {
         shard->error = "No file type found in this shard";

clients/drcachesim/tools/opcode_mix.h

@@ -62,7 +62,7 @@ class opcode_mix_t : public analysis_tool_t {
                  const std::string &alt_module_dir = "");
     virtual ~opcode_mix_t();
     std::string
-    initialize() override;
+    initialize_stream(dynamorio::drmemtrace::memtrace_stream_t *serial_stream) override;
     bool
     process_memref(const memref_t &memref) override;
     bool
@@ -142,20 +142,17 @@ class opcode_mix_t : public analysis_tool_t {
 
     struct shard_data_t {
         shard_data_t()
-            : instr_count(0)
-            , last_trace_module_start(nullptr)
-            , last_trace_module_size(0)
-            , last_mapped_module_start(nullptr)
         {
         }
 
-        int64_t instr_count;
+        int64_t instr_count = 0;
         std::unordered_map<int, int64_t> opcode_counts;
         std::unordered_map<uint, int64_t> category_counts;
         std::string error;
-        app_pc last_trace_module_start;
-        size_t last_trace_module_size;
-        app_pc last_mapped_module_start;
+        app_pc last_trace_module_start = nullptr;
+        size_t last_trace_module_size = 0;
+        app_pc last_mapped_module_start = nullptr;
+        dynamorio::drmemtrace::memtrace_stream_t *stream = nullptr;
         std::unique_ptr<decode_cache_t<opcode_data_t>> decode_cache;
         offline_file_type_t filetype = OFFLINE_FILE_TYPE_DEFAULT;
     };

suite/tests/CMakeLists.txt

@@ -4452,6 +4452,7 @@ if (BUILD_CLIENTS)
       # TODO i#3544: Port tests to RISC-V 64
       torunonly_drcacheoff(opcode_mix ${ci_shared_app} ""
         "@-tool@opcode_mix" "")
+
       # Ensure the tool works without the raw/ subdir.
       set(tool.drcacheoff.opcode_mix_postcmd
         "firstglob@${drraw2trace_path}@-indir@${dir_prefix}.*.dir")
@@ -4460,6 +4461,9 @@ if (BUILD_CLIENTS)
       set(tool.drcacheoff.opcode_mix_postcmd3
         "firstglob@${drcachesim_path}@-indir@${dir_prefix}.*.dir@-tool@opcode_mix")
 
+      torunonly_drcacheoff(opcode_mix.skip ${ci_shared_app} ""
+        "@-tool@opcode_mix@-skip_instrs@1" "")
+
       torunonly_drcacheoff(view ${ci_shared_app} ""
         "@-tool@view@-sim_refs@16384" "")
       unset(tool.drcacheoff.view_rawtemp) # Use preprocessor