i#5975: Add invariant check that read/write records match operands

api/docs/release.dox

@@ -139,6 +139,9 @@ Further non-compatibility-affecting changes include:
    -core_serial) and control the schedule
    (-sched_quantum, -sched_time, sched_order_time, -record_file,
    -replay_file, -cpu_schedule_file).
+ - Added type_is_read() API that returns true if a trace type reads from memory.
+ - Added instr_num_memory_read_access() and instr_num_memory_write_access() that return
+   the number of memory read and write accesses of an instruction respectively.
 
 **************************************************
 <hr>

clients/drcachesim/common/trace_entry.h

@@ -634,6 +634,15 @@ type_is_data(const trace_type_t type)
         type == TRACE_TYPE_DATA_FLUSH_END;
 }
 
+/**
+ * Returns whether the type represents a memory read access.
+ */
+static inline bool
+type_is_read(const trace_type_t type)
+{
+    return type_is_prefetch(type) || type == TRACE_TYPE_READ;
+}
+
 static inline bool
 marker_type_is_function_marker(const trace_marker_type_t mark)
 {

clients/drcachesim/tests/invariant_checker_test.cpp

@@ -277,22 +277,27 @@ check_sane_control_flow()
 
     // Negative test: branches with encodings which do not go to their targets.
     {
-        std::vector<memref_t> memrefs = {
-            gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
-#    if defined(X86_64) || defined(X86_32)
-            // 0x74 is "je" with the 2nd byte the offset.
-            gen_branch_encoded(TID, 0x71019dbc, { 0x74, 0x32 }),
-            gen_instr_encoded(0x71019ded, { 0x01 }, TID),
-#    elif defined(ARM_64)
-            // 71019dbc:   540001a1        b.ne    71019df0
-            // <__executable_start+0x19df0>
-            gen_branch_encoded(TID, 0x71019dbc, 0x540001a1),
-            gen_instr_encoded(0x71019ded, 0x01, TID),
-#    else
-        // TODO i#5871: Add AArch32 (and RISC-V) encodings.
-#    endif
-        };
+        instr_t *move1 = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
+                                           opnd_create_reg(REG2));
+        instr_t *move2 = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
+                                           opnd_create_reg(REG2));
+        instr_t *cond_jmp =
+            XINST_CREATE_jump_cond(GLOBAL_DCONTEXT, DR_PRED_EQ, opnd_create_instr(move1));
+
+        instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
+        instrlist_append(ilist, cond_jmp);
+        instrlist_append(ilist, move1);
+        instrlist_append(ilist, move2);
 
+        std::vector<memref_with_IR_t> memref_instr_vec = {
+            { gen_marker(1, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
+              nullptr },
+            { gen_branch(1), cond_jmp },
+            { gen_instr(1), move2 }
+        };
+        static constexpr addr_t BASE_ADDR = 0xeba4ad4;
+        auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
+        instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
         if (!run_checker(memrefs, true,
                          { "Branch does not go to the correct target", TID,
                            /*ref_ordinal=*/3, /*last_timestamp=*/0,
@@ -303,21 +308,26 @@ check_sane_control_flow()
     }
     // Positive test: branches with encodings which go to their targets.
     {
-        std::vector<memref_t> memrefs = {
-            gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
-#    if defined(X86_64) || defined(X86_32)
-            // 0x74 is "je" with the 2nd byte the offset.
-            gen_branch_encoded(TID, 0x71019dbc, { 0x74, 0x32 }),
-#    elif defined(ARM_64)
-            // 71019dbc:   540001a1        b.ne    71019df0
-            // <__executable_start+0x19df0>
-            gen_branch_encoded(TID, 0x71019dbc, 0x540001a1),
-#    else
-        // TODO i#5871: Add AArch32 (and RISC-V) encodings.
-#    endif
-            gen_instr(TID, 0x71019df0),
-        };
+        instr_t *move1 = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
+                                           opnd_create_reg(REG2));
+        instr_t *move2 = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
+                                           opnd_create_reg(REG2));
+        instr_t *cond_jmp =
+            XINST_CREATE_jump_cond(GLOBAL_DCONTEXT, DR_PRED_EQ, opnd_create_instr(move1));
 
+        instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
+        instrlist_append(ilist, cond_jmp);
+        instrlist_append(ilist, move1);
+        instrlist_append(ilist, move2);
+
+        std::vector<memref_with_IR_t> memref_instr_vec = {
+            { gen_marker(1, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
+              nullptr },
+            { gen_branch(1), cond_jmp },
+            { gen_instr(1), move1 }
+        };
+        static constexpr addr_t BASE_ADDR = 0xeba4ad4;
+        auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
         if (!run_checker(memrefs, false)) {
             return false;
         }
@@ -1872,6 +1882,201 @@ check_timestamps_increase_monotonically(void)
     return true;
 }
 
+bool
+check_read_write_records_match_operands()
+{
+    // Only the number of memory read and write records is checked against the
+    // operands. Address and size are not used.
+    std::cerr << "Testing number of memory read/write records matching operands\n";
+    constexpr memref_tid_t TID = 1;
+
+    // Correct: number of read records matches operand.
+    {
+        instr_t *load = XINST_CREATE_load(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
+                                          OPND_CREATE_MEMPTR(REG1, /*disp=*/0));
+        instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
+        instrlist_append(ilist, load);
+
+        std::vector<memref_with_IR_t> memref_instr_vec = {
+            { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
+              nullptr },
+            { gen_instr(TID), load },
+            { gen_data(TID, /*load=*/true, /*addr=*/0, /*size=*/0), nullptr }
+        };
+        static constexpr addr_t BASE_ADDR = 0xeba4ad4;
+        auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
+        instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
+        if (!run_checker(memrefs, false)) {
+            return false;
+        }
+    }
+    // Incorrect: too many read records.
+    {
+        instr_t *load = XINST_CREATE_load(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
+                                          OPND_CREATE_MEMPTR(REG1, /*disp=*/0));
+        instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
+        instrlist_append(ilist, load);
+
+        std::vector<memref_with_IR_t> memref_instr_vec = {
+            { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
+              nullptr },
+            { gen_instr(TID), load },
+            { gen_data(TID, /*load=*/true, /*addr=*/0, /*size=*/0), nullptr },
+            { gen_data(TID, /*load=*/true, /*addr=*/0, /*size=*/0), nullptr }
+        };
+        static constexpr addr_t BASE_ADDR = 0xeba4ad4;
+        auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
+        instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
+        if (!run_checker(memrefs, true,
+                         { "Too many read records", TID, /*ref_ordinal=*/4,
+                           /*last_timestamp=*/0, /*instrs_since_last_timestamp=*/1 },
+                         "Failed to catch too many read records"))
+            return false;
+    }
+    // Incorrect: missing read records.
+    {
+        instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT);
+        instr_t *load = XINST_CREATE_load(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
+                                          OPND_CREATE_MEMPTR(REG1, /*disp=*/0));
+        instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
+        instrlist_append(ilist, load);
+        instrlist_append(ilist, nop);
+
+        std::vector<memref_with_IR_t> memref_instr_vec = {
+            { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
+              nullptr },
+            { gen_instr(TID), load },
+            { gen_instr(TID), nop },
+        };
+        static constexpr addr_t BASE_ADDR = 0xeba4ad4;
+        auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
+        instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
+        if (!run_checker(memrefs, true,
+                         { "Missing read records", TID, /*ref_ordinal=*/3,
+                           /*last_timestamp=*/0, /*instrs_since_last_timestamp=*/2 },
+                         "Failed to catch missing read records"))
+            return false;
+    }
+    // Correct: number of write records match operand.
+    {
+        instr_t *store = XINST_CREATE_store(
+            GLOBAL_DCONTEXT, OPND_CREATE_MEMPTR(REG2, /*disp=*/0), opnd_create_reg(REG1));
+        instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
+        instrlist_append(ilist, store);
+
+        std::vector<memref_with_IR_t> memref_instr_vec = {
+            { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
+              nullptr },
+            { gen_instr(TID), store },
+            { gen_data(TID, /*load=*/false, /*addr=*/0, /*size=*/0), nullptr }
+        };
+
+        static constexpr addr_t BASE_ADDR = 0xeba4ad4;
+        auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
+        instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
+        if (!run_checker(memrefs, false)) {
+            return false;
+        }
+    }
+    // Incorrect: too many write records.
+    {
+        instr_t *store = XINST_CREATE_store(
+            GLOBAL_DCONTEXT, OPND_CREATE_MEMPTR(REG2, /*disp=*/0), opnd_create_reg(REG1));
+        instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
+        instrlist_append(ilist, store);
+
+        std::vector<memref_with_IR_t> memref_instr_vec = {
+            { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
+              nullptr },
+            { gen_instr(TID), store },
+            { gen_data(TID, /*load=*/false, /*addr=*/0, /*size=*/0), nullptr },
+            { gen_data(TID, /*load=*/false, /*addr=*/0, /*size=*/0), nullptr }
+        };
+
+        static constexpr addr_t BASE_ADDR = 0xeba4ad4;
+        auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
+        instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
+        if (!run_checker(memrefs, true,
+                         { "Too many write records", TID, /*ref_ordinal=*/4,
+                           /*last_timestamp=*/0,
+                           /*instrs_since_last_timestamp=*/1 },
+                         "Failed to catch too many write records"))
+            return false;
+    }
+    // Incorrect: missing write records.
+    {
+        instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT);
+        instr_t *store = XINST_CREATE_store(
+            GLOBAL_DCONTEXT, OPND_CREATE_MEMPTR(REG2, /*disp=*/0), opnd_create_reg(REG1));
+        instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
+        instrlist_append(ilist, store);
+        instrlist_append(ilist, nop);
+
+        std::vector<memref_with_IR_t> memref_instr_vec = {
+            { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
+              nullptr },
+            { gen_instr(TID), store },
+            { gen_instr(TID), nop },
+        };
+
+        static constexpr addr_t BASE_ADDR = 0xeba4ad4;
+        auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
+        instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
+        if (!run_checker(memrefs, true,
+                         { "Missing write records", TID, /*ref_ordinal=*/3,
+                           /*last_timestamp=*/0,
+                           /*instrs_since_last_timestamp=*/2 },
+                         "Fail to catch missing write records"))
+            return false;
+    }
+#if defined(X86_64) || defined(X86_32)
+    // Correct: ignore predicated operands which may not have memory access.
+    {
+        instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT);
+        instr_t *rep_movs = INSTR_CREATE_rep_movs_1(GLOBAL_DCONTEXT);
+        instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
+        instrlist_append(ilist, rep_movs);
+        instrlist_append(ilist, nop);
+        static constexpr addr_t BASE_ADDR = 0xeba4ad4;
+        std::vector<memref_with_IR_t> memref_setup = {
+            { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
+              nullptr },
+            { gen_instr(TID), rep_movs },
+            { gen_instr(TID), nop },
+        };
+        auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR);
+        instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
+        if (!run_checker(memrefs, false)) {
+            return false;
+        }
+    }
+    // Correct: ignore operands with opcode which do not have real memory
+    // access.
+    {
+        instr_t *lea =
+            INSTR_CREATE_lea(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
+                             opnd_create_base_disp(REG1, REG_NULL, 0, 1, OPSZ_lea));
+        instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT);
+        instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
+        instrlist_append(ilist, lea);
+        instrlist_append(ilist, nop);
+        static constexpr addr_t BASE_ADDR = 0xeba4ad4;
+        std::vector<memref_with_IR_t> memref_setup = {
+            { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
+              nullptr },
+            { gen_instr(TID), lea },
+            { gen_instr(TID), nop },
+        };
+        auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR);
+        instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
+        if (!run_checker(memrefs, false)) {
+            return false;
+        }
+    }
+#endif
+    return true;
+}
+
 int
 test_main(int argc, const char *argv[])
 {
@@ -1880,7 +2085,8 @@ test_main(int argc, const char *argv[])
         check_duplicate_syscall_with_same_pc() && check_false_syscalls() &&
         check_rseq_side_exit_discontinuity() && check_schedule_file() &&
         check_branch_decoration() && check_filter_endpoint() &&
-        check_timestamps_increase_monotonically()) {
+        check_timestamps_increase_monotonically() &&
+        check_read_write_records_match_operands()) {
         std::cerr << "invariant_checker_test passed\n";
         return 0;
     }

clients/drcachesim/tools/invariant_checker.cpp

@@ -131,6 +131,14 @@ invariant_checker_t::parallel_shard_init(int shard_index, void *worker_data)
 bool
 invariant_checker_t::parallel_shard_exit(void *shard_data)
 {
+    per_shard_t *shard = reinterpret_cast<per_shard_t *>(shard_data);
+    if (!TESTANY(OFFLINE_FILE_TYPE_FILTERED | OFFLINE_FILE_TYPE_DFILTERED,
+                 shard->file_type_)) {
+        report_if_false(shard, shard->expected_read_records_ == 0,
+                        "Missing read records");
+        report_if_false(shard, shard->expected_write_records_ == 0,
+                        "Missing write records");
+    }
     return true;
 }
 
@@ -512,6 +520,23 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem
             if (next_pc == nullptr) {
                 cur_instr_decoded.reset(nullptr);
             }
+            if (cur_instr_decoded != nullptr && cur_instr_decoded->data != nullptr) {
+                instr_t *instr = cur_instr_decoded->data;
+                if (!instr_is_predicated(instr) &&
+                    !TESTANY(OFFLINE_FILE_TYPE_FILTERED | OFFLINE_FILE_TYPE_DFILTERED,
+                             shard->file_type_)) {
+                    // Verify the number of read/write records matches the last
+                    // operand. Skip D-filtered traces which don't have every load or
+                    // store records.
+                    report_if_false(shard, shard->expected_read_records_ == 0,
+                                    "Missing read records");
+                    report_if_false(shard, shard->expected_write_records_ == 0,
+                                    "Missing write records");
+
+                    shard->expected_read_records_ = instr_num_memory_read_access(instr);
+                    shard->expected_write_records_ = instr_num_memory_write_access(instr);
+                }
+            }
         }
         if (knob_verbose_ >= 3) {
             std::cerr << "::" << memref.data.pid << ":" << memref.data.tid << ":: "
@@ -836,6 +861,30 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem
     shard->prev_entry_ = memref;
     if (type_is_instr_branch(shard->prev_entry_.instr.type))
         shard->last_branch_ = shard->prev_entry_;
+
+    if (type_is_data(memref.data.type) && shard->prev_instr_decoded_ != nullptr) {
+        // If the instruction is predicated, the check is skipped since we do
+        // not have the data to determine how many memory accesses to expect.
+        if (!instr_is_predicated(shard->prev_instr_decoded_->data) &&
+            !TESTANY(OFFLINE_FILE_TYPE_FILTERED | OFFLINE_FILE_TYPE_DFILTERED,
+                     shard->file_type_)) {
+            if (type_is_read(memref.data.type)) {
+                // Skip D-filtered traces which don't have every load or store records.
+                report_if_false(shard, shard->expected_read_records_ > 0,
+                                "Too many read records");
+                if (shard->expected_read_records_ > 0) {
+                    shard->expected_read_records_--;
+                }
+            } else {
+                // Skip D-filtered traces which don't have every load or store records.
+                report_if_false(shard, shard->expected_write_records_ > 0,
+                                "Too many write records");
+                if (shard->expected_write_records_ > 0) {
+                    shard->expected_write_records_--;
+                }
+            }
+        }
+    }
     return true;
 }