Cleanup

api/docs/release.dox

@@ -143,9 +143,9 @@ Further non-compatibility-affecting changes include:
    encodings are obtained from the application binaries instead.
  - Added a new drmemtrace analyzer flag -sched_syscall_file to allow specifying the
    system call trace template file to be used for dynamic injection of system call trace
-   templates. Added similar options for the drmemtrace scheduler
-   #dynamorio::drmemtrace::scheduler_tmpl_t::scheduler_options_t::kernel_syscall_trace_path,
-   and #dynamorio::drmemtrace::scheduler_tmpl_t::scheduler_options_t::kernel_syscall_reader.
+   templates. Added similar options for the drmemtrace scheduler: #dynamorio::drmemtrace::
+   scheduler_tmpl_t::scheduler_options_t::kernel_syscall_trace_path, and #dynamorio::
+   drmemtrace::scheduler_tmpl_t::scheduler_options_t::kernel_syscall_reader.
 
 **************************************************
 <hr>

clients/drcachesim/common/options.cpp

@@ -1041,17 +1041,19 @@ droption_t<std::string> op_sched_switch_file(
     DROPTION_SCOPE_FRONTEND, "sched_switch_file", "",
     "Path to file holding kernel context switch sequences",
     "Applies to -core_sharded and -core_serial.  Path to file holding context switch "
-    "sequences.  The file can contain multiple sequences each with regular trace headers "
-    "and the sequence proper bracketed by TRACE_MARKER_TYPE_CONTEXT_SWITCH_START and "
-    "TRACE_MARKER_TYPE_CONTEXT_SWITCH_END markers.");
+    "sequences.  The file can contain multiple sequences each with regular trace "
+    "headers and the sequence proper bracketed by "
+    "TRACE_MARKER_TYPE_CONTEXT_SWITCH_START and TRACE_MARKER_TYPE_CONTEXT_SWITCH_END "
+    "markers.");
 
 droption_t<std::string> op_sched_syscall_file(
     DROPTION_SCOPE_FRONTEND, "sched_syscall_file", "",
     "Path to file holding kernel system call sequences",
     "Applies to -core_sharded and -core_serial.  Path to file holding system call "
-    "sequences.  The file can contain multiple sequences each with regular trace headers "
-    "and the sequence proper bracketed by TRACE_MARKER_TYPE_SYSCALL_TRACE_START and "
-    "TRACE_MARKER_TYPE_SYSCALL_TRACE_END markers.");
+    "sequences.  The file can contain multiple sequences each with regular trace "
+    "headers and the sequence proper bracketed by "
+    "TRACE_MARKER_TYPE_SYSCALL_TRACE_START and TRACE_MARKER_TYPE_SYSCALL_TRACE_END "
+    "markers.");
 
 droption_t<bool> op_sched_randomize(
     DROPTION_SCOPE_FRONTEND, "sched_randomize", false,

clients/drcachesim/scheduler/scheduler.h

@@ -837,8 +837,8 @@ template <typename RecordType, typename ReaderType> class scheduler_tmpl_t {
          * indicating the system call it corresponds to. Sequences for
          * multiple system calls are concatenated into a single file.
          * Each sequence should be in the regular offline drmemtrace format.
-         * The sequence is inserted into the output stream on the
-         * #TRACE_MARKER_TYPE_SYSCALL marker with the indicated value.
+         * The sequence is inserted into the output stream after the
+         * #TRACE_MARKER_TYPE_SYSCALL markers with the indicated value.
          * The same file (or reader) must be passed when replaying as this kernel
          * code is not stored when recording.
          * An alternative to passing the file path is to pass #kernel_syscall_reader

clients/drcachesim/scheduler/scheduler_impl.cpp

@@ -1457,8 +1457,8 @@ scheduler_impl_tmpl_t<RecordType, ReaderType>::read_kernel_sequences(
         // Only remember the records between the markers.
         trace_marker_type_t marker_type = TRACE_MARKER_TYPE_RESERVED_END;
         uintptr_t marker_value = 0;
-        if (record_type_is_marker(record, marker_type, marker_value) &&
-            marker_type == start_marker) {
+        bool is_marker = record_type_is_marker(record, marker_type, marker_value);
+        if (is_marker && marker_type == start_marker) {
             sequence_key = static_cast<SequenceKey>(marker_value);
             in_sequence = true;
             if (!sequence[sequence_key].empty()) {
@@ -1468,18 +1468,17 @@ scheduler_impl_tmpl_t<RecordType, ReaderType>::read_kernel_sequences(
         }
         if (in_sequence) {
             sequence[sequence_key].push_back(record);
-            if (record_type_is_marker(record, marker_type, marker_value) &&
-                marker_type == end_marker) {
-                if (static_cast<SequenceKey>(marker_value) != sequence_key) {
-                    error_string_ += sequence_type + " marker values mismatched";
-                    return sched_type_t::STATUS_ERROR_INVALID_PARAMETER;
-                }
-                VPRINT(this, 1, "Read %zu kernel %s records for key %d\n",
-                       sequence[sequence_key].size(), sequence_type.c_str(),
-                       sequence_key);
-                in_sequence = false;
+        }
+        if (is_marker && marker_type == end_marker) {
+            if (!in_sequence || static_cast<SequenceKey>(marker_value) != sequence_key) {
+                error_string_ += sequence_type + " marker values mismatched";
+                return sched_type_t::STATUS_ERROR_INVALID_PARAMETER;
             }
+            VPRINT(this, 1, "Read %zu kernel %s records for key %d\n",
+                   sequence[sequence_key].size(), sequence_type.c_str(), sequence_key);
+            in_sequence = false;
         }
+
         ++(*reader);
     }
     return sched_type_t::STATUS_SUCCESS;
@@ -2759,7 +2758,9 @@ scheduler_impl_tmpl_t<RecordType, ReaderType>::next_record(output_ordinal_t outp
     record_type_has_pid(record, input->pid);
 
     trace_marker_type_t marker_type;
-    uintptr_t marker_value = 0;
+    uintptr_t marker_value;
+    // Good to queue the injected records at this point, because we now surely will
+    // be done with TRACE_MARKER_TYPE_SYSCALL.
     if (record_type_is_marker(record, marker_type, marker_value) &&
         marker_type == TRACE_MARKER_TYPE_SYSCALL &&
         syscall_sequence_.find(static_cast<int>(marker_value)) !=

clients/drcachesim/scheduler/scheduler_impl.h

@@ -546,6 +546,7 @@ template <typename RecordType, typename ReaderType> class scheduler_impl_tmpl_t
         uint64_t timestamp;
     };
 
+    // Custom hash function used for switch_type_t and syscall num (int).
     template <typename IntCastable> struct custom_hash_t {
         std::size_t
         operator()(const IntCastable &st) const
@@ -1007,7 +1008,7 @@ template <typename RecordType, typename ReaderType> class scheduler_impl_tmpl_t
     std::unordered_map<switch_type_t, std::vector<RecordType>,
                        custom_hash_t<switch_type_t>>
         switch_sequence_;
-    // We specify a custom hash function only to make it generalize with
+    // We specify a custom hash function only to make it easier to generalize with
     // switch_sequence_ defined above.
     std::unordered_map<int, std::vector<RecordType>, custom_hash_t<int>>
         syscall_sequence_;

clients/drcachesim/tests/scheduler_unit_tests.cpp

@@ -6193,6 +6193,8 @@ test_kernel_syscall_sequences()
         for (int i = 0; i < NUM_OUTPUTS; i++) {
             std::cerr << "cpu #" << i << " schedule: " << sched_as_string[i] << "\n";
         }
+        // The instrs in the injected syscall sequence count towards the #instr
+        // quantum, but no context switch happens in the middle of the syscall seq.
         assert(sched_as_string[0] ==
                "Av0is1ii1,Cv0is1ii1,Aiis2iii2,Ciis2iii2,Aiis1ii1,Ciis1ii1,Aiis2iii2,"
                "Ciis2iii2,Aiis1ii1,Ciis1ii1");
@@ -6212,6 +6214,7 @@ test_kernel_syscall_sequences()
             check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) &&
             check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER,
                       TRACE_MARKER_TYPE_SYSCALL, SYSCALL_NUM_1) &&
+
             // Syscall_1 trace on first thread.
             check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER,
                       TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1) &&
@@ -6227,13 +6230,15 @@ test_kernel_syscall_sequences()
             check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) &&
             check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER,
                       TRACE_MARKER_TYPE_SYSCALL, SYSCALL_NUM_1) &&
+
             // Syscall_1 trace on second thread.
             check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER,
                       TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1) &&
             check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) &&
             check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) &&
             check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER,
                       TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1) &&
+
             check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) &&
             check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) &&
             check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER,