i#6940: Do not always load binaries in view tool (#6941)

The view tool was blindly loading binaries even for traces that have
encodings.  This leads to fatal errors when binaries have changed, even
when the change has no impact on viewing a trace.  We fix that here by
reading the filtype at init time.

Work around the #6942 crash by always setting the disasm syntax to DR
style for REGDEPS traces.
    
This change actually sets the disasm syntax to AT&T by default if no
module path is passed in; which is what it is supposed to do: but it was not
doing that and this breaks 3 tests comparing DR-style output. We put in a
quick fix to request DR style for those tests.

Tested locally where the view tool asserts without this fix.

Issue: #6940, #6942
Fixes #6940

clients/drcachesim/tools/view.cpp

@@ -97,6 +97,30 @@ view_t::initialize_stream(memtrace_stream_t *serial_stream)
     serial_stream_ = serial_stream;
     print_header();
     dcontext_.dcontext = dr_standalone_init();
+
+    dr_disasm_flags_t flags = IF_X86_ELSE(
+        DR_DISASM_ATT,
+        IF_AARCH64_ELSE(DR_DISASM_DR, IF_RISCV64_ELSE(DR_DISASM_RISCV, DR_DISASM_ARM)));
+    if (knob_syntax_ == "intel") {
+        flags = DR_DISASM_INTEL;
+    } else if (knob_syntax_ == "dr") {
+        flags = DR_DISASM_DR;
+    } else if (knob_syntax_ == "arm") {
+        flags = DR_DISASM_ARM;
+    } else if (knob_syntax_ == "riscv") {
+        flags = DR_DISASM_RISCV;
+    }
+    disassemble_set_syntax(flags);
+
+    // Get the filetype up front if available.
+    // We leave setting and processing filetype_ to when we see the marker.
+    if (TESTANY(OFFLINE_FILE_TYPE_ENCODINGS, serial_stream->get_filetype())) {
+        // We do not need to try to find and load the binaries, so don't (as trying
+        // can result in errors if those are not present or have changed).
+        has_modules_ = false;
+        return "";
+    }
+
     if (module_file_path_.empty()) {
         has_modules_ = false;
     } else {
@@ -118,19 +142,6 @@ view_t::initialize_stream(memtrace_stream_t *serial_stream)
     std::string error = module_mapper_->get_last_error();
     if (!error.empty())
         return "Failed to load binaries: " + error;
-    dr_disasm_flags_t flags = IF_X86_ELSE(
-        DR_DISASM_ATT,
-        IF_AARCH64_ELSE(DR_DISASM_DR, IF_RISCV64_ELSE(DR_DISASM_RISCV, DR_DISASM_ARM)));
-    if (knob_syntax_ == "intel") {
-        flags = DR_DISASM_INTEL;
-    } else if (knob_syntax_ == "dr") {
-        flags = DR_DISASM_DR;
-    } else if (knob_syntax_ == "arm") {
-        flags = DR_DISASM_ARM;
-    } else if (knob_syntax_ == "riscv") {
-        flags = DR_DISASM_RISCV;
-    }
-    disassemble_set_syntax(flags);
     return "";
 }
 
@@ -236,6 +247,10 @@ view_t::parallel_shard_memref(void *shard_data, const memref_t &memref)
              */
             if (TESTANY(OFFLINE_FILE_TYPE_ARCH_REGDEPS, filetype_)) {
                 dr_set_isa_mode(dcontext_.dcontext, DR_ISA_REGDEPS, nullptr);
+                // Ignore the requested syntax: we only support DR style.
+                // XXX i#6942: Should we return an error if the users asks for
+                // another syntax?  Should DR's libraries return an error?
+                disassemble_set_syntax(DR_DISASM_DR);
             }
             return true; // Do not count toward -sim_refs yet b/c we don't have tid.
         case TRACE_MARKER_TYPE_TIMESTAMP:

suite/tests/CMakeLists.txt

@@ -4276,14 +4276,14 @@ if (BUILD_CLIENTS)
       torunonly_api(tool.drcacheoff.skip "${drcachesim_path}" "offline-skip" ""
         # Test invariants with global headers after skipping instrs.
         # Here the skip does not find real headers and we expect synthetic.
-        "-tool;view;-infile;${zip_path};${test_mode_flag};-skip_instrs;62;-sim_refs;10"
+        "-tool;view;-view_syntax;dr;-infile;${zip_path};${test_mode_flag};-skip_instrs;62;-sim_refs;10"
         OFF OFF)
       set(tool.drcacheoff.skip_basedir "${PROJECT_SOURCE_DIR}/clients/drcachesim/tests")
       set(tool.drcacheoff.skip_rawtemp ON) # no preprocessor
       torunonly_api(tool.drcacheoff.skip2 "${drcachesim_path}" "offline-skip2" ""
         # Test invariants with global headers after skipping instrs.
         # Here the skip finds real headers and we expect no synthetic.
-        "-tool;view;-infile;${zip_path};${test_mode_flag};-skip_instrs;63;-sim_refs;10"
+        "-tool;view;-view_syntax;dr;-infile;${zip_path};${test_mode_flag};-skip_instrs;63;-sim_refs;10"
         OFF OFF)
       set(tool.drcacheoff.skip2_basedir "${PROJECT_SOURCE_DIR}/clients/drcachesim/tests")
       set(tool.drcacheoff.skip2_rawtemp ON) # no preprocessor
@@ -4813,7 +4813,7 @@ if (BUILD_CLIENTS)
       file(MAKE_DIRECTORY ${srcdir})
       file(COPY ${zip_path} DESTINATION ${srcdir})
       torunonly_api(tool.drcacheoff.trim "${drcachesim_path}" "offline-trim" ""
-        "-tool;view;-indir;${outdir};${test_mode_flag}"
+        "-tool;view;-view_syntax;dr;-indir;${outdir};${test_mode_flag}"
         OFF OFF)
       set(tool.drcacheoff.trim_runcmp "${CMAKE_CURRENT_SOURCE_DIR}/runmulti.cmake")
       # The filter overwrites any existing file in the dir from a prior run.