i#7644: Allow specifying a default syscall trace in template file

clients/drcachesim/common/trace_entry.h

@@ -1086,15 +1086,13 @@ typedef enum {
      * system call execution. Each system call trace should end with an indirect
      * branch instruction (e.g., iret/sysret/sysexit on x86, or eret on AArch64) which
      * must be preceded by a #TRACE_MARKER_TYPE_BRANCH_TARGET marker with any value;
-     * the marker's value will be appropriately set to point to the fallthrough pc of
-     * the prior syscall instruction when the trace template is injected. Note: the
-     * marker value will not be the actual next pc in the trace in some cases (i#7496):
-     * - if a #TRACE_MARKER_TYPE_KERNEL_EVENT immediately follows the syscall trace,
-     *   it indicates interruption of the syscall by a signal; in this case, the next
-     *   pc after the signal is the #TRACE_MARKER_TYPE_KERNEL_EVENT marker value,
-     *   which for auto-restart syscalls would be the same as the syscall instr pc.
-     * - for the sigreturn syscall, the next pc in the trace is what was specified
-     *   in the prior #TRACE_MARKER_TYPE_KERNEL_EVENT marker.
+     * the marker's value will be appropriately set to point to the next instr in the
+     * thread's trace when the trace template is injected.
+     *
+     * The file may also include a "default" trace that can be used for system calls that
+     * do not have any trace specified in this file. The default trace sets the
+     * sysnum as #DEFAULT_SYSCALL_TRACE_TEMPLATE_NUM in the various markers.
+     *
      * 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.
      *
@@ -1521,6 +1519,16 @@ typedef struct _pt_data_buf_t pt_data_buf_t;
 
 #endif // defined(BUILD_PT_TRACER) || defined(BUILD_PT_POST_PROCESSOR)
 
+/**
+ * Value used by the system call trace template files (having the type
+ * #OFFLINE_FILE_TYPE_KERNEL_SYSCALL_TRACE_TEMPLATES) in the
+ * #TRACE_MARKER_TYPE_SYSCALL_TRACE_START and
+ * #TRACE_MARKER_TYPE_SYSCALL_TRACE_END markers to denote a trace to
+ * be used when no other trace is available for some syscall in the
+ * template file.
+ */
+constexpr int DEFAULT_SYSCALL_TRACE_TEMPLATE_NUM = 0xffff;
+
 /**
  * The name of the file in -offline mode where module data is written.
  * Its creation can be customized using drmemtrace_custom_module_data()

clients/drcachesim/scheduler/scheduler_impl.cpp

@@ -1597,16 +1597,38 @@ scheduler_impl_tmpl_t<RecordType, ReaderType>::read_switch_sequences()
                                  TRACE_MARKER_TYPE_CONTEXT_SWITCH_END, "context switch");
 }
 
+template <typename RecordType, typename ReaderType>
+typename scheduler_impl_tmpl_t<RecordType, ReaderType>::trace_sequence_t *
+scheduler_impl_tmpl_t<RecordType, ReaderType>::get_syscall_sequence(int syscall_num)
+{
+    if (syscall_sequence_.find(syscall_num) != syscall_sequence_.end()) {
+        return &syscall_sequence_[syscall_num];
+    }
+    if (default_syscall_sequence_.first_pc_valid) {
+        return &default_syscall_sequence_;
+    }
+    return nullptr;
+}
+
 template <typename RecordType, typename ReaderType>
 typename scheduler_tmpl_t<RecordType, ReaderType>::scheduler_status_t
 scheduler_impl_tmpl_t<RecordType, ReaderType>::read_syscall_sequences()
 {
-
-    return read_kernel_sequences(syscall_sequence_, options_.kernel_syscall_trace_path,
-                                 std::move(options_.kernel_syscall_reader),
-                                 std::move(options_.kernel_syscall_reader_end),
-                                 TRACE_MARKER_TYPE_SYSCALL_TRACE_START,
-                                 TRACE_MARKER_TYPE_SYSCALL_TRACE_END, "system call");
+    scheduler_status_t status =
+        read_kernel_sequences(syscall_sequence_, options_.kernel_syscall_trace_path,
+                              std::move(options_.kernel_syscall_reader),
+                              std::move(options_.kernel_syscall_reader_end),
+                              TRACE_MARKER_TYPE_SYSCALL_TRACE_START,
+                              TRACE_MARKER_TYPE_SYSCALL_TRACE_END, "system call");
+    if (status != sched_type_t::STATUS_SUCCESS) {
+        return status;
+    }
+    auto default_seq_it = syscall_sequence_.find(DEFAULT_SYSCALL_TRACE_TEMPLATE_NUM);
+    if (default_seq_it != syscall_sequence_.end()) {
+        default_syscall_sequence_ = std::move(default_seq_it->second);
+        syscall_sequence_.erase(default_seq_it);
+    }
+    return status;
 }
 
 template <typename RecordType, typename ReaderType>
@@ -1885,17 +1907,17 @@ scheduler_impl_tmpl_t<RecordType, ReaderType>::inject_pending_syscall_sequence(
         // records from the reader.
         input->queue.emplace_front(record);
     }
-    int syscall_num = input->to_inject_syscall;
-    input->to_inject_syscall = input_info_t::INJECT_NONE;
-    assert(syscall_sequence_.find(syscall_num) != syscall_sequence_.end());
-    stream_status_t res = inject_kernel_sequence(syscall_sequence_[syscall_num], input);
+    trace_sequence_t *to_inject_sequence = get_syscall_sequence(input->to_inject_syscall);
+    assert(to_inject_sequence != nullptr);
+    stream_status_t res = inject_kernel_sequence(*to_inject_sequence, input);
     if (res != stream_status_t::STATUS_OK)
         return res;
     ++outputs_[output]
           .stats[memtrace_stream_t::SCHED_STAT_KERNEL_SYSCALL_SEQUENCE_INJECTIONS];
     VPRINT(this, 3, "Inserted %zu syscall records for syscall %d to %d.%d\n",
-           syscall_sequence_[syscall_num].records.size(), syscall_num, input->workload,
+           to_inject_sequence->records.size(), input->to_inject_syscall, input->workload,
            input->index);
+    input->to_inject_syscall = input_info_t::INJECT_NONE;
 
     // Return the first injected record.
     assert(!input->queue.empty());
@@ -1972,7 +1994,7 @@ scheduler_impl_tmpl_t<RecordType, ReaderType>::inject_kernel_sequence(
     assert(!sequence.records.empty());
     bool saw_any_instr = false;
     bool set_branch_target_marker = false;
-    trace_marker_type_t marker_type;
+    trace_marker_type_t marker_type = TRACE_MARKER_TYPE_RESERVED_END;
     uintptr_t marker_value = 0;
 
     // We walk the to-be-injected trace backwards, so next_trace_pc starts at the
@@ -1992,6 +2014,15 @@ scheduler_impl_tmpl_t<RecordType, ReaderType>::inject_kernel_sequence(
     const addr_t next_input_trace_pc = static_cast<addr_t>(next_trace_pc);
     for (int i = static_cast<int>(sequence.records.size()) - 1; i >= 0; --i) {
         RecordType record = sequence.records[i];
+        // If we're injecting the default syscall trace, we need to adjust the
+        // values of the syscall_trace_start and syscall_trace_end markers
+        // to the actual syscall num, instead of the sentinel.
+        if (record_type_is_marker(record, marker_type, marker_value) &&
+            (marker_type == TRACE_MARKER_TYPE_SYSCALL_TRACE_START ||
+             marker_type == TRACE_MARKER_TYPE_SYSCALL_TRACE_END) &&
+            marker_value == DEFAULT_SYSCALL_TRACE_TEMPLATE_NUM) {
+            record_type_set_marker_value(record, input->to_inject_syscall);
+        }
         // TODO i#7495: Add invariant checks that ensure these are equal to the
         // context-switched-to thread when the switch sequence is injected into a
         // trace.
@@ -3244,17 +3275,18 @@ scheduler_impl_tmpl_t<RecordType, ReaderType>::finalize_next_record(
     bool is_marker = record_type_is_marker(record, marker_type, marker_value);
     // Good to queue the injected records at this point, because we now surely will
     // be done with TRACE_MARKER_TYPE_SYSCALL.
-    if (is_marker && marker_type == TRACE_MARKER_TYPE_SYSCALL &&
-        syscall_sequence_.find(static_cast<int>(marker_value)) !=
-            syscall_sequence_.end()) {
+    if (is_marker && marker_type == TRACE_MARKER_TYPE_SYSCALL) {
         assert(!input->in_syscall_injection);
-        // The actual injection of the syscall trace happens later at the intended
-        // point between the syscall function tracing markers.
-        input->to_inject_syscall = static_cast<int>(marker_value);
-        assert(syscall_sequence_[input->to_inject_syscall].first_pc_valid);
-        input->to_inject_syscall_first_pc =
-            syscall_sequence_[input->to_inject_syscall].first_pc;
-        input->saw_first_func_id_marker_after_syscall = false;
+        int syscall_num = static_cast<int>(marker_value);
+        trace_sequence_t *to_inject_sequence = get_syscall_sequence(syscall_num);
+        if (to_inject_sequence != nullptr) {
+            // The actual injection of the syscall trace happens later at the intended
+            // point between the syscall function tracing markers.
+            input->to_inject_syscall = syscall_num;
+            assert(to_inject_sequence->first_pc_valid);
+            input->to_inject_syscall_first_pc = to_inject_sequence->first_pc;
+            input->saw_first_func_id_marker_after_syscall = false;
+        }
     } else if (record_type_is_instr(record, &instr_pc, &instr_size)) {
         input->last_pc_fallthrough = instr_pc + instr_size;
     }

clients/drcachesim/scheduler/scheduler_impl.h

@@ -828,6 +828,9 @@ template <typename RecordType, typename ReaderType> class scheduler_impl_tmpl_t
     scheduler_status_t
     read_switch_sequences();
 
+    trace_sequence_t *
+    get_syscall_sequence(int syscall_num);
+
     scheduler_status_t
     read_syscall_sequences();
 
@@ -1116,6 +1119,7 @@ template <typename RecordType, typename ReaderType> class scheduler_impl_tmpl_t
     // We specify a custom hash function only to make it easier to generalize with
     // switch_sequence_ defined above.
     std::unordered_map<int, trace_sequence_t, custom_hash_t<int>> syscall_sequence_;
+    trace_sequence_t default_syscall_sequence_;
     // For single_lockstep_output.
     std::unique_ptr<stream_t> global_stream_;
     // For online where we currently have to map dynamically observed thread ids

clients/drcachesim/tests/invariant_checker_test.cpp

@@ -3832,7 +3832,8 @@ check_kernel_syscall_trace(void)
     }
 #    ifdef LINUX
     // Signal return immediately after sigreturn syscall trace. The syscall-trace-end
-    // branch target marker holds the pc of the signal resumption point.
+    // branch target marker holds the pc of the signal resumption point, which is an
+    // instr in this test.
     {
         std::vector<memref_with_IR_t> memref_setup = {
             { gen_marker(TID_A, TRACE_MARKER_TYPE_VERSION,
@@ -3863,6 +3864,90 @@ check_kernel_syscall_trace(void)
         if (!run_checker(memrefs, false))
             res = false;
     }
+    // Signal return immediately after sigreturn syscall trace. The syscall-trace-end
+    // branch target marker holds the pc of the signal resumption point, which is a
+    // kernel_event marker from another signal in this test. The kernel_event marker is
+    // the first thing in its signal context (since the first signal context was just
+    // ended by the kernel_xfer), so regular pc continuity checks would be disabled, but
+    // we want to ensure proper operation of syscall-trace-end-branch-target vs next-pc
+    // equality checks.
+    {
+        std::vector<memref_with_IR_t> memref_setup = {
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_VERSION,
+                         TRACE_ENTRY_VERSION_BRANCH_INFO),
+              nullptr },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE, FILE_TYPE_FULL_SYSCALL_TRACE),
+              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), sys },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL, SYS_rt_sigreturn), nullptr },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYS_rt_sigreturn),
+              nullptr },
+            { gen_instr(TID_A), move },
+            { gen_instr(TID_A), load },
+            { gen_data(TID_A, /*load=*/true, /*addr=*/0x1234, /*size=*/4), nullptr },
+            // add_encodings_to_memrefs removes this from the memref list and adds it
+            // to memref_t.instr.indirect_branch_target instead for the following instr.
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_BRANCH_TARGET, 0), nop },
+            { gen_instr_type(TRACE_TYPE_INSTR_INDIRECT_JUMP, TID_A), sys_return },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYS_rt_sigreturn),
+              nullptr },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_KERNEL_XFER, 42), nullptr },
+            // Ensure the syscall-trace-end branch_target marker vs next-pc check
+            // happens at this kernel_event marker.
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_KERNEL_EVENT, 0), nop },
+            { gen_instr(TID_A), load },
+            { gen_data(TID_A, /*load=*/true, /*addr=*/0x1234, /*size=*/4), nullptr },
+            { gen_exit(TID_A), nullptr }
+        };
+        auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR);
+        if (!run_checker(memrefs, false))
+            res = false;
+    }
+    // Signal return immediately after sigreturn syscall trace. The syscall-trace-end
+    // branch target marker holds incorrect pc of the signal resumption point, which is a
+    // kernel_event marker from another signal in this test.
+    // Similar to the test above, but verifies the case where the check fails.
+    {
+        std::vector<memref_with_IR_t> memref_setup = {
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_VERSION,
+                         TRACE_ENTRY_VERSION_BRANCH_INFO),
+              nullptr },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE, FILE_TYPE_FULL_SYSCALL_TRACE),
+              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), sys },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL, SYS_rt_sigreturn), nullptr },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYS_rt_sigreturn),
+              nullptr },
+            { gen_instr(TID_A), move },
+            { gen_instr(TID_A), load },
+            { gen_data(TID_A, /*load=*/true, /*addr=*/0x1234, /*size=*/4), nullptr },
+            // add_encodings_to_memrefs removes this from the memref list and adds it
+            // to memref_t.instr.indirect_branch_target instead for the following instr.
+            // Incorrectly points to the load instruction, when the resumption point
+            // specified by the later kernel_event marker is the nop instruction.
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_BRANCH_TARGET, 0), load },
+            { gen_instr_type(TRACE_TYPE_INSTR_INDIRECT_JUMP, TID_A), sys_return },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYS_rt_sigreturn),
+              nullptr },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_KERNEL_XFER, 42), nullptr },
+            { gen_marker(TID_A, TRACE_MARKER_TYPE_KERNEL_EVENT, 0), nop },
+            { gen_exit(TID_A), nullptr }
+        };
+        auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR);
+        if (!run_checker(memrefs, true,
+                         { "Syscall trace-end branch marker incorrect "
+                           "@ context-initial kernel_event marker",
+                           /*tid=*/TID_A,
+                           /*ref_ordinal=*/14, /*last_timestamp=*/0,
+                           /*instrs_since_last_timestamp=*/4 },
+                         "Failed to detect PC discontinuity after syscall-trace at "
+                         "context-initial kernel_event marker"))
+            res = false;
+    }
 #    endif
     // No version marker so branch target marker is not expected.
     // This test is also a baseline for later test cases where we simplify test setup

clients/drcachesim/tests/scheduler_unit_tests.cpp

@@ -490,6 +490,13 @@ get_mock_syscall_sequence(int syscall_base, addr_t syscall_pc_start = 0xfeed101)
             test_util::make_instr(syscall_pc_start + 12, TRACE_TYPE_INSTR_INDIRECT_JUMP),
             test_util::make_marker(
                 TRACE_MARKER_TYPE_SYSCALL_TRACE_END, syscall_base + 1),
+            test_util::make_marker(
+                TRACE_MARKER_TYPE_SYSCALL_TRACE_START,
+                DEFAULT_SYSCALL_TRACE_TEMPLATE_NUM),
+            test_util::make_marker(TRACE_MARKER_TYPE_BRANCH_TARGET, DONT_CARE),
+            test_util::make_instr(syscall_pc_start + 20, TRACE_TYPE_INSTR_INDIRECT_JUMP),
+            test_util::make_marker(
+                TRACE_MARKER_TYPE_SYSCALL_TRACE_END, DEFAULT_SYSCALL_TRACE_TEMPLATE_NUM),
             test_util::make_exit(TID_IN_SYSCALLS),
             test_util::make_footer(),
         /* clang-format on */
@@ -7633,7 +7640,10 @@ test_kernel_syscall_sequences()
                     bool add_post_timestamp = true;
                     inputs.push_back(test_util::make_timestamp(TIMESTAMP + instr_idx));
                     inputs.push_back(test_util::make_marker(
-                        TRACE_MARKER_TYPE_SYSCALL, SYSCALL_BASE + (instr_idx / 2) % 2));
+                        // We specify syscall_num = SYSCALL_BASE + {0, 1, or 2}.
+                        // SYSCALL_BASE+2 does not have a trace specified by our test
+                        // template file, so will use the default one.
+                        TRACE_MARKER_TYPE_SYSCALL, SYSCALL_BASE + (instr_idx / 2) % 3));
                     // Every other syscall is a blocking syscall.
                     if (instr_idx % 4 == 0) {
                         inputs.push_back(test_util::make_marker(
@@ -7719,10 +7729,10 @@ test_kernel_syscall_sequences()
         // quantum, but no context switch happens in the middle of the syscall seq.
         assert(sched_as_string[0] ==
                "Avf0i0SsFF1ii1kk,Cvf0i0SsFF1ii1FFs0,Aii0S2iii20,Cii0S2iii20,"
-               "Aii0Ss1ii10,Cii0Ss1ii10,Aii0S2iii20,Cii0S2iii20,Aii0Ss1ii10,Cii0Ss1ii10");
+               "Aii0Ss3i30,Cii0Ss3i30,Aii0S1ii10,Cii0S1ii10,Aii0Ss2iii20,Cii0Ss2iii20");
         assert(sched_as_string[1] ==
-               "Bvf0i0SsFF1ii1k0ii0S2iii20ii0Ss1ii10ii0S2iii20ii0Ss1ii10______________"
-               "____________________________________________");
+               "Bvf0i0SsFF1ii1k0ii0S2iii20ii0Ss3i30ii0S1ii10ii0Ss2iii20______________"
+               "___________________________________________");
         // Zoom in and check the first few syscall sequences on the first output record
         // by record with value checks.
         int idx = 0;

clients/drcachesim/tests/syscall_insertion.templatex

@@ -2,8 +2,8 @@ Schedule stats tool results:
 Total counts:
            4 cores
 .*
-      650604 instructions
+      651070 instructions
 .*
            0 context switch sequence injections
-          14 system call sequence injections
+         480 system call sequence injections
 .*

clients/drcachesim/tests/syscall_insertion_core_sharded.templatex

@@ -2,8 +2,8 @@ Schedule stats tool results:
 Total counts:
            4 cores
 .*
-      650604 instructions
+      651070 instructions
 .*
            0 context switch sequence injections
-          14 system call sequence injections
+         480 system call sequence injections
 .*