i#6495: Handle invariant errors in x86 QEMU syscall templates

api/docs/release.dox

@@ -212,6 +212,11 @@ Further non-compatibility-affecting changes include:
    #dynamorio::drmemtrace::analysis_tool_t to allow the tool to make holistic
    adjustments to the interval snapshots after all have been generated, and before
    they are used for merging across shards (potentially), and printing the results.
+ - Added -abort_on_invariant_error flag that instructs the invariant checker drmemtrace
+   analysis tool to abort trace analysis when a trace invariant error is found. This
+   is set to true by default to match the existing behavior of the invariant checker.
+ - Added a new instr API instr_is_xrstor() that tells whether an instruction is any
+   variant of the x86 xrstor opcode.
 
 **************************************************
 <hr>

clients/drcachesim/analyzer_multi.cpp

@@ -175,7 +175,8 @@ analyzer_multi_t::create_invariant_checker()
     }
     return new invariant_checker_t(op_offline.get_value(), op_verbose.get_value(),
                                    op_test_mode_name.get_value(),
-                                   serial_schedule_file_.get(), cpu_schedule_file_.get());
+                                   serial_schedule_file_.get(), cpu_schedule_file_.get(),
+                                   op_abort_on_invariant_error.get_value());
 }
 
 template <>

clients/drcachesim/common/options.cpp

@@ -931,7 +931,8 @@ droption_t<std::string> op_syscall_template_file(
     "Path to the file that contains system call trace templates.",
     "Path to the file that contains system call trace templates. "
     "If set, system call traces will be injected from the file "
-    "into the resulting trace.");
+    "into the resulting trace. This is still experimental so the template file "
+    "format may change without backward compatibility.");
 
 // Record filter options.
 droption_t<uint64_t> op_filter_stop_timestamp(
@@ -976,5 +977,13 @@ droption_t<uint64_t> op_trim_after_timestamp(
     "Removes all records from the first TRACE_MARKER_TYPE_TIMESTAMP marker with "
     "timestamp larger than the specified value.");
 
+droption_t<bool> op_abort_on_invariant_error(
+    DROPTION_SCOPE_ALL, "abort_on_invariant_error", true,
+    "Abort invariant checker when a trace invariant error is found.",
+    "When set to true, the trace invariant checker analysis tool aborts when a trace "
+    "invariant error is found. Otherwise it prints the error and continues. Also, the "
+    "total invariant error count is printed at the end; a non-zero error count does not "
+    "affect the exit code of the analyzer.");
+
 } // namespace drmemtrace
 } // namespace dynamorio

clients/drcachesim/common/options.h

@@ -215,6 +215,7 @@ extern dynamorio::droption::droption_t<std::string> op_filter_trace_types;
 extern dynamorio::droption::droption_t<std::string> op_filter_marker_types;
 extern dynamorio::droption::droption_t<uint64_t> op_trim_before_timestamp;
 extern dynamorio::droption::droption_t<uint64_t> op_trim_after_timestamp;
+extern dynamorio::droption::droption_t<bool> op_abort_on_invariant_error;
 
 } // namespace drmemtrace
 } // namespace dynamorio

clients/drcachesim/common/trace_entry.h

@@ -940,9 +940,10 @@ typedef enum {
     OFFLINE_FILE_TYPE_BIMODAL_FILTERED_WARMUP = 0x2000,
     /**
      * Indicates an offline trace that contains trace templates for some system calls.
-     * The individual traces are separated by a #TRACE_MARKER_TYPE_SYSCALL marker which
-     * also specifies what system call the following trace belongs to. This file can be
-     * used with -syscall_template_file to raw2trace to create a
+     * The individual traces are enclosed within a pair of
+     * #TRACE_MARKER_TYPE_SYSCALL_TRACE_START and #TRACE_MARKER_TYPE_SYSCALL_TRACE_END
+     * markers which also specify what system call the contained trace belongs to. This
+     * file can be used with -syscall_template_file to raw2trace to create an
      * #OFFLINE_FILE_TYPE_KERNEL_SYSCALLS trace. See the sample file written by the
      * burst_syscall_inject.cpp test for more details on the expected format for the
      * system call template file.

clients/drcachesim/tests/burst_syscall_inject.cpp

@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2016-2023 Google, Inc.  All rights reserved.
+ * Copyright (c) 2016-2024 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /*
@@ -141,16 +141,22 @@ write_system_call_template(void *dr_context)
         trace_entry_t to_write = *reinterpret_cast<trace_entry_t *>(buf_at);
         write_trace_entry(writer, to_write);
     }
+    write_trace_entry(writer, make_marker(TRACE_MARKER_TYPE_CACHE_LINE_SIZE, 64));
+    write_trace_entry(writer, make_marker(TRACE_MARKER_TYPE_PAGE_SIZE, 4096));
 
     // Write the trace template for SYS_getpid.
-    write_trace_entry(writer, make_marker(TRACE_MARKER_TYPE_SYSCALL, SYS_getpid));
+    write_trace_entry(writer,
+                      make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYS_getpid));
     // Just a random instruction.
     instr_in_getpid = XINST_CREATE_nop(dr_context);
     write_instr_entry(dr_context, writer, instr_in_getpid,
                       reinterpret_cast<app_pc>(PC_SYSCALL_GETPID));
+    write_trace_entry(writer,
+                      make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYS_getpid));
 
     // Write the trace template for SYS_gettid.
-    write_trace_entry(writer, make_marker(TRACE_MARKER_TYPE_SYSCALL, SYS_gettid));
+    write_trace_entry(writer,
+                      make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYS_gettid));
     // Just a random instruction.
 #ifdef X86
 #    define TEST_REG DR_REG_XDX
@@ -167,7 +173,14 @@ write_system_call_template(void *dr_context)
     write_trace_entry(
         writer,
         make_memref(READ_MEMADDR_GETTID, TRACE_TYPE_READ, opnd_size_in_bytes(OPSZ_PTR)));
+    write_trace_entry(writer,
+                      make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYS_gettid));
 
+    // Write footer.
+    dynamorio::drmemtrace::trace_entry_t thread_exit = {
+        dynamorio::drmemtrace::TRACE_TYPE_THREAD_EXIT, 0, { /*tid=*/1 }
+    };
+    write_trace_entry(writer, thread_exit);
     write_trace_entry(writer, make_footer());
     std::cerr << "Done writing system call trace template\n";
     return syscall_trace_template_file;
@@ -406,7 +419,8 @@ test_main(int argc, const char *argv[])
     std::cerr << "Getting basic counts for system call trace template\n";
     basic_counts_t::counters_t template_counts = get_basic_counts(syscall_trace_template);
     if (!(template_counts.instrs == 2 && template_counts.encodings == 2 &&
-          template_counts.syscall_number_markers == 2)) {
+          // We only have trace start and end markers, no syscall number markers.
+          template_counts.syscall_number_markers == 0)) {
         std::cerr << "Unexpected counts in system call trace template: "
                   << syscall_trace_template << ": #instrs: " << template_counts.instrs
                   << ", #encodings: " << template_counts.encodings

clients/drcachesim/tests/invariant_checker_test.cpp

@@ -3165,6 +3165,160 @@ check_kernel_syscall_trace(void)
                          "Failed to catch nested syscall traces"))
             res = false;
     }
+#    ifdef X86
+    // TODO i#6495: Adapt this test to AArch64-equivalent scenarios.
+    {
+        instr_t *move1 = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
+                                           opnd_create_reg(REG2));
+        instr_t *iret = INSTR_CREATE_iret(GLOBAL_DCONTEXT);
+        instr_t *sti = INSTR_CREATE_sti(GLOBAL_DCONTEXT);
+        instr_t *nop1 = XINST_CREATE_nop(GLOBAL_DCONTEXT);
+        instr_t *nop2 = XINST_CREATE_nop(GLOBAL_DCONTEXT);
+        instr_t *xrstors = INSTR_CREATE_xrstors64(
+            GLOBAL_DCONTEXT,
+            opnd_create_base_disp(DR_REG_XCX, DR_REG_NULL, 0, 0, OPSZ_xsave));
+        instr_t *xsaves = INSTR_CREATE_xsaves64(
+            GLOBAL_DCONTEXT,
+            opnd_create_base_disp(DR_REG_XCX, DR_REG_NULL, 0, 0, OPSZ_xsave));
+        instr_t *hlt = INSTR_CREATE_hlt(GLOBAL_DCONTEXT);
+        instr_t *nop3 = XINST_CREATE_nop(GLOBAL_DCONTEXT);
+        instr_t *prefetch = INSTR_CREATE_prefetchnta(
+            GLOBAL_DCONTEXT,
+            opnd_create_base_disp(DR_REG_XCX, DR_REG_NULL, 0, 0, OPSZ_1));
+        instr_t *sysret = INSTR_CREATE_sysret(GLOBAL_DCONTEXT);
+        instr_t *nop4 = XINST_CREATE_nop(GLOBAL_DCONTEXT);
+        instr_t *sys1 = instr_clone(GLOBAL_DCONTEXT, sys);
+        instr_t *nop5 = XINST_CREATE_nop(GLOBAL_DCONTEXT);
+        instr_t *sys2 = instr_clone(GLOBAL_DCONTEXT, sys);
+        instr_t *nop6 = XINST_CREATE_nop(GLOBAL_DCONTEXT);
+        instrlist_t *ilist2 = instrlist_create(GLOBAL_DCONTEXT);
+        instrlist_append(ilist2, move1);
+        instrlist_append(ilist2, iret);
+        instrlist_append(ilist2, sti);
+        instrlist_append(ilist2, nop1);
+        instrlist_append(ilist2, nop2);
+        instrlist_append(ilist2, xrstors);
+        instrlist_append(ilist2, xsaves);
+        instrlist_append(ilist2, hlt);
+        instrlist_append(ilist2, nop3);
+        instrlist_append(ilist2, prefetch);
+        instrlist_append(ilist2, sysret);
+        instrlist_append(ilist2, nop4);
+        instrlist_append(ilist2, sys1);
+        instrlist_append(ilist2, nop5);
+        instrlist_append(ilist2, sys2);
+        instrlist_append(ilist2, nop6);
+        {
+            std::vector<memref_with_IR_t> memref_instr_vec = {
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE,
+                             OFFLINE_FILE_TYPE_ENCODINGS |
+                                 OFFLINE_FILE_TYPE_SYSCALL_NUMBERS |
+                                 OFFLINE_FILE_TYPE_KERNEL_SYSCALLS),
+                  nullptr },
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_CACHE_LINE_SIZE, 64), nullptr },
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_PAGE_SIZE, 4096), nullptr },
+                { gen_instr(TID_A), sys1 },
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL, 42), nullptr },
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_START, 42), nullptr },
+                { gen_instr(TID_A), move1 },
+                { gen_instr(TID_A), iret },
+                // Multiple reads. Acceptable because of the prior iret.
+                { gen_data(TID_A, true, 42, 8), nullptr },
+                { gen_data(TID_A, true, 42, 8), nullptr },
+                { gen_data(TID_A, true, 42, 8), nullptr },
+                { gen_data(TID_A, true, 42, 8), nullptr },
+                { gen_instr(TID_A), sti },
+                { gen_instr(TID_A), nop1 },
+                // Missing nop2. Acceptable because of the recent sti.
+                { gen_instr(TID_A), xrstors },
+                // Multiple reads. Acceptable because of the prior xrstors.
+                { gen_data(TID_A, true, 42, 8), nullptr },
+                { gen_data(TID_A, true, 42, 8), nullptr },
+                { gen_data(TID_A, true, 42, 8), nullptr },
+                { gen_data(TID_A, true, 42, 8), nullptr },
+                { gen_instr(TID_A), xsaves },
+                // Multiple writes. Acceptable because of the prior xsaves.
+                { gen_data(TID_A, false, 42, 8), nullptr },
+                { gen_data(TID_A, false, 42, 8), nullptr },
+                { gen_data(TID_A, false, 42, 8), nullptr },
+                { gen_data(TID_A, false, 42, 8), nullptr },
+                { gen_instr(TID_A), hlt },
+                // Missing nop3. Acceptable because of the prior hlt.
+                { gen_instr(TID_A), prefetch },
+                // Missing reads. Acceptable because of the prior prefetch.
+                { gen_instr(TID_A), sysret },
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, 42), nullptr },
+                // Continues after sys1.
+                { gen_instr(TID_A), nop5 },
+                { gen_instr(TID_A), sys2 },
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL, 41), nullptr },
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_START, 41), nullptr },
+                { gen_instr(TID_A), move1 },
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, 41), nullptr },
+                // Continues after sys2.
+                { gen_instr(TID_A), nop6 },
+                { gen_exit(TID_A), nullptr }
+            };
+            auto memrefs = add_encodings_to_memrefs(ilist2, memref_instr_vec, BASE_ADDR);
+            if (!run_checker(memrefs, false))
+                res = false;
+        }
+        {
+            std::vector<memref_with_IR_t> memref_instr_vec = {
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE,
+                             OFFLINE_FILE_TYPE_ENCODINGS |
+                                 OFFLINE_FILE_TYPE_SYSCALL_NUMBERS |
+                                 OFFLINE_FILE_TYPE_KERNEL_SYSCALLS),
+                  nullptr },
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_CACHE_LINE_SIZE, 64), nullptr },
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_PAGE_SIZE, 4096), nullptr },
+                { gen_instr(TID_A), sys1 },
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL, 42), nullptr },
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_START, 42), nullptr },
+                { gen_instr(TID_A), move1 },
+                { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, 42), nullptr },
+                // Missing instrs.
+                { gen_instr(TID_A), nop6 },
+                { gen_exit(TID_A), nullptr }
+            };
+            auto memrefs = add_encodings_to_memrefs(ilist2, memref_instr_vec, BASE_ADDR);
+            if (!run_checker(memrefs, true,
+                             { "Non-explicit control flow has no marker",
+                               /*tid=*/TID_A,
+                               /*ref_ordinal=*/9, /*last_timestamp=*/0,
+                               /*instrs_since_last_timestamp=*/3 },
+                             "Failed to catch discontinuity on return from syscall"))
+                res = false;
+        }
+        std::vector<memref_with_IR_t> memref_instr_vec = {
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE,
+                         OFFLINE_FILE_TYPE_ENCODINGS | OFFLINE_FILE_TYPE_SYSCALL_NUMBERS |
+                             OFFLINE_FILE_TYPE_KERNEL_SYSCALLS),
+              nullptr },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_CACHE_LINE_SIZE, 64), nullptr },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_PAGE_SIZE, 4096), nullptr },
+            { gen_instr(TID_A), sys1 },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL, 42), nullptr },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_START, 42), nullptr },
+            { gen_instr(TID_A), move1 },
+            // Missing instrs.
+            { gen_instr(TID_A), sti },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, 42), nullptr },
+            { gen_instr(TID_A), nop5 },
+            { gen_exit(TID_A), nullptr }
+        };
+        {
+            auto memrefs = add_encodings_to_memrefs(ilist2, memref_instr_vec, BASE_ADDR);
+            if (!run_checker(memrefs, true,
+                             { "Non-explicit control flow has no marker",
+                               /*tid=*/TID_A,
+                               /*ref_ordinal=*/8, /*last_timestamp=*/0,
+                               /*instrs_since_last_timestamp=*/3 },
+                             "Failed to catch discontinuity inside syscall trace"))
+                res = false;
+        }
+    }
+#    endif
     return res;
 #endif
 }