From 05ddb8a44931db1c51a611b8c5afaad09bfb2da2 Mon Sep 17 00:00:00 2001 From: Kai Date: Tue, 22 Oct 2024 10:36:16 -0700 Subject: [PATCH 1/3] i#7050: reset expected read and write record counts after a kernel transfer. When an instruction is preempted by a kernel transfer, the instruction is not retired. The trace might not have captured all the read and write records. To avoid false positive, the invariant checker should reset the expected read and write record counters. Fixes #7050 --- .../tests/invariant_checker_test.cpp | 27 +++++++++++++++++++ .../drcachesim/tools/invariant_checker.cpp | 4 +++ 2 files changed, 31 insertions(+) diff --git a/clients/drcachesim/tests/invariant_checker_test.cpp b/clients/drcachesim/tests/invariant_checker_test.cpp index 3d37b88fbda..6c27864c02b 100644 --- a/clients/drcachesim/tests/invariant_checker_test.cpp +++ b/clients/drcachesim/tests/invariant_checker_test.cpp @@ -2794,6 +2794,33 @@ check_read_write_records_match_operands() "Fail to catch missing write records")) return false; } + // Correct: skip read and write records check when the previous instruction was + // preempted. + { + 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_instr_vec = { + { gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS), + 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), store }, + { gen_marker(TID_A, TRACE_MARKER_TYPE_KERNEL_EVENT, 2), nop }, + { gen_exit(TID_A), 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; + } + } #if defined(X86_64) || defined(X86_32) // Correct: number of read and write records matches the operand. { diff --git a/clients/drcachesim/tools/invariant_checker.cpp b/clients/drcachesim/tools/invariant_checker.cpp index a3bb8663d6d..486c98ee3ee 100644 --- a/clients/drcachesim/tools/invariant_checker.cpp +++ b/clients/drcachesim/tools/invariant_checker.cpp @@ -1068,6 +1068,10 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem shard->prev_entry_.marker.marker_type != TRACE_MARKER_TYPE_RSEQ_ABORT) { shard->retaddr_stack_.push(0); } + // Reset read/write record count when the previous instruction was + // preempted. + shard->expected_read_records_ = 0; + shard->expected_write_records_ = 0; } #ifdef UNIX report_if_false(shard, memref.marker.marker_value != 0, From 1ede6827ccb3e01b830dff5ca996bc65cd4c008d Mon Sep 17 00:00:00 2001 From: Kai Date: Mon, 28 Oct 2024 14:59:59 -0700 Subject: [PATCH 2/3] Adding debug to confirm aarchxx results. --- clients/drcachesim/tracer/raw2trace.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clients/drcachesim/tracer/raw2trace.cpp b/clients/drcachesim/tracer/raw2trace.cpp index 7908ba47904..37358d82e33 100644 --- a/clients/drcachesim/tracer/raw2trace.cpp +++ b/clients/drcachesim/tracer/raw2trace.cpp @@ -2114,6 +2114,8 @@ raw2trace_t::append_bb_entries(raw2trace_thread_data_t *tdata, buf->addr = (addr_t)orig_pc; ++buf; log(4, "Appended instr fetch for original %p\n", orig_pc); + } else { + log(0, "Skipped preempted instruction.\n"); } decode_pc = pc; if (tdata->rseq_past_end_) { From 10c8705db9809d33020a084de60182680268e056 Mon Sep 17 00:00:00 2001 From: Kai Date: Mon, 28 Oct 2024 16:05:15 -0700 Subject: [PATCH 3/3] Update signal_invariants.c to ignore preempted instr and memref. --- clients/drcachesim/tests/signal_invariants.c | 24 +++++++++++++------- clients/drcachesim/tracer/raw2trace.cpp | 2 -- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/clients/drcachesim/tests/signal_invariants.c b/clients/drcachesim/tests/signal_invariants.c index a0044b0623f..f72371d7a4b 100644 --- a/clients/drcachesim/tests/signal_invariants.c +++ b/clients/drcachesim/tests/signal_invariants.c @@ -190,8 +190,10 @@ GLOBAL_LABEL(FUNCNAME:) #define FUNCNAME test_signal_midbb DECLARE_FUNC(FUNCNAME) GLOBAL_LABEL(FUNCNAME:) - /* prefetcht2's address is the instr count until a signal */ - prefetcht2 [3] + /* prefetcht2's address is the instr count until a signal not counting + * the preempted instr. + */ + prefetcht2 [2] nop nop ud2 @@ -205,8 +207,10 @@ GLOBAL_LABEL(FUNCNAME:) #define FUNCNAME test_signal_startbb DECLARE_FUNC(FUNCNAME) GLOBAL_LABEL(FUNCNAME:) - /* prefetcht2's address is the instr count until a signal */ - prefetcht2 [2] + /* prefetcht2's address is the instr count until a signal not counting + * the preempted instr. + */ + prefetcht2 [1] jmp new_bb new_bb: ud2 @@ -221,10 +225,14 @@ GLOBAL_LABEL(FUNCNAME:) * XXX i#3958: Today the 2nd movs memref is incorrectly included *before* * the fault. */ - /* prefetcht2's address is the instr count until a signal */ - prefetcht2 [5] - /* prefetcht1's address is the memref count until a signal */ - prefetcht1 [3] + /* prefetcht2's address is the instr count until a signal not counting + * the preempted instr. + */ + prefetcht2 [4] + /* prefetcht1's address is the memref count until a signal not counting + * the preempted memref. + */ + prefetcht1 [1] mov REG_XSI, HEX(42) mov REG_XDI, REG_XSP push REG_XAX diff --git a/clients/drcachesim/tracer/raw2trace.cpp b/clients/drcachesim/tracer/raw2trace.cpp index 37358d82e33..7908ba47904 100644 --- a/clients/drcachesim/tracer/raw2trace.cpp +++ b/clients/drcachesim/tracer/raw2trace.cpp @@ -2114,8 +2114,6 @@ raw2trace_t::append_bb_entries(raw2trace_thread_data_t *tdata, buf->addr = (addr_t)orig_pc; ++buf; log(4, "Appended instr fetch for original %p\n", orig_pc); - } else { - log(0, "Skipped preempted instruction.\n"); } decode_pc = pc; if (tdata->rseq_past_end_) {