Skip to content

Commit

Permalink
i#4575: Track threads and nesting in trace_invariants test
Browse files Browse the repository at this point in the history 
Improves the robustness of the trace_invariants test by shifting the
various tracked state to be per-thread and per-nested-signal, avoiding
incorrect results from thread interleavings or nested signals.
This presumably explains past flakiness like #4575.

Fixes #4575
  • Loading branch information
@derekbruening
derekbruening committed Jun 28, 2021
1 parent 2b86bc0 commit 59c6e33
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 63 deletions.
126 changes: 70 additions & 56 deletions clients/drcachesim/tests/trace_invariants.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,8 @@ trace_invariants_t::trace_invariants_t(bool offline, unsigned int verbose,
: knob_offline_(offline)
, knob_verbose_(verbose)
, knob_test_name_(test_name)
, instrs_until_interrupt_(-1)
, memrefs_until_interrupt_(-1)
, app_handler_pc_(0)
{
memset(&prev_instr_, 0, sizeof(prev_instr_));
memset(&pre_signal_instr_, 0, sizeof(pre_signal_instr_));
memset(&prev_xfer_marker_, 0, sizeof(prev_xfer_marker_));
memset(&prev_entry_, 0, sizeof(prev_entry_));
memset(&prev_prev_entry_, 0, sizeof(prev_prev_entry_));
}

trace_invariants_t::~trace_invariants_t()
Expand All @@ -58,31 +51,43 @@ trace_invariants_t::~trace_invariants_t()
bool
trace_invariants_t::process_memref(const memref_t &memref)
{
if (prev_entry_.find(memref.data.tid) == prev_entry_.end()) {
instrs_until_interrupt_[memref.data.tid] = -1;
memrefs_until_interrupt_[memref.data.tid] = -1;
prev_instr_[memref.data.tid] = {};
prev_xfer_marker_[memref.data.tid] = {};
prev_entry_[memref.data.tid] = {};
prev_prev_entry_[memref.data.tid] = {};
}
// Check conditions specific to the signal_invariants app, where it
// has annotations in prefetch instructions telling us how many instrs
// and/or memrefs until a signal should arrive.
if ((instrs_until_interrupt_ == 0 && memrefs_until_interrupt_ == -1) ||
(instrs_until_interrupt_ == -1 && memrefs_until_interrupt_ == 0) ||
(instrs_until_interrupt_ == 0 && memrefs_until_interrupt_ == 0)) {
if ((instrs_until_interrupt_[memref.data.tid] == 0 &&
memrefs_until_interrupt_[memref.data.tid] == -1) ||
(instrs_until_interrupt_[memref.data.tid] == -1 &&
memrefs_until_interrupt_[memref.data.tid] == 0) ||
(instrs_until_interrupt_[memref.data.tid] == 0 &&
memrefs_until_interrupt_[memref.data.tid] == 0)) {
assert((memref.marker.type == TRACE_TYPE_MARKER &&
memref.marker.marker_type == TRACE_MARKER_TYPE_KERNEL_EVENT) ||
// TODO i#3937: Online instr bundles currently violate this.
!knob_offline_);
instrs_until_interrupt_ = -1;
memrefs_until_interrupt_ = -1;
instrs_until_interrupt_[memref.data.tid] = -1;
memrefs_until_interrupt_[memref.data.tid] = -1;
}
if (memrefs_until_interrupt_ >= 0 &&
if (memrefs_until_interrupt_[memref.data.tid] >= 0 &&
(memref.data.type == TRACE_TYPE_READ || memref.data.type == TRACE_TYPE_WRITE)) {
assert(memrefs_until_interrupt_ != 0);
--memrefs_until_interrupt_;
assert(memrefs_until_interrupt_[memref.data.tid] != 0);
--memrefs_until_interrupt_[memref.data.tid];
}
// Check that the signal delivery marker is immediately followed by the
// app's signal handler, which we have annotated with "prefetcht0 [1]".
if (memref.data.type == TRACE_TYPE_PREFETCHT0 && memref.data.addr == 1) {
assert(type_is_instr(prev_entry_.instr.type) &&
prev_prev_entry_.marker.type == TRACE_TYPE_MARKER &&
prev_xfer_marker_.marker.marker_type == TRACE_MARKER_TYPE_KERNEL_EVENT);
app_handler_pc_ = prev_entry_.instr.addr;
assert(type_is_instr(prev_entry_[memref.data.tid].instr.type) &&
prev_prev_entry_[memref.data.tid].marker.type == TRACE_TYPE_MARKER &&
prev_xfer_marker_[memref.data.tid].marker.marker_type ==
TRACE_MARKER_TYPE_KERNEL_EVENT);
app_handler_pc_ = prev_entry_[memref.data.tid].instr.addr;
}

if (memref.marker.type == TRACE_TYPE_MARKER &&
Expand Down Expand Up @@ -123,67 +128,75 @@ trace_invariants_t::process_memref(const memref_t &memref)
: "")
<< " instr x" << memref.instr.size << "\n";
}
assert(instrs_until_interrupt_ != 0);
if (instrs_until_interrupt_ > 0)
--instrs_until_interrupt_;
assert(instrs_until_interrupt_[memref.data.tid] != 0);
if (instrs_until_interrupt_[memref.data.tid] > 0)
--instrs_until_interrupt_[memref.data.tid];
// Invariant: offline traces guarantee that a branch target must immediately
// follow the branch w/ no intervening trace switch.
if (knob_offline_ && type_is_instr_branch(prev_instr_.instr.type)) {
assert(
prev_instr_.instr.tid == memref.instr.tid ||
// For limited-window traces a thread might exit after a branch.
thread_exited_[prev_instr_.instr.tid] ||
// The invariant is relaxed for a signal.
(prev_xfer_marker_.instr.tid == prev_instr_.instr.tid &&
prev_xfer_marker_.marker.marker_type == TRACE_MARKER_TYPE_KERNEL_EVENT));
// follow the branch w/ no intervening thread switch.
if (knob_offline_ &&
type_is_instr_branch(prev_instr_[memref.data.tid].instr.type)) {
assert(prev_instr_[memref.data.tid].instr.tid == memref.instr.tid ||
// For limited-window traces a thread might exit after a branch.
thread_exited_[prev_instr_[memref.data.tid].instr.tid] ||
// The invariant is relaxed for a signal.
(prev_xfer_marker_[memref.data.tid].instr.tid ==
prev_instr_[memref.data.tid].instr.tid &&
prev_xfer_marker_[memref.data.tid].marker.marker_type ==
TRACE_MARKER_TYPE_KERNEL_EVENT));
}
// Invariant: non-explicit control flow (i.e., kernel-mediated) is indicated
// by markers.
if (prev_instr_.instr.addr != 0 /*first*/ &&
prev_instr_.instr.tid == memref.instr.tid &&
!type_is_instr_branch(prev_instr_.instr.type)) {
if (prev_instr_[memref.data.tid].instr.addr != 0 /*first*/ &&
!type_is_instr_branch(prev_instr_[memref.data.tid].instr.type)) {
assert( // Filtered.
TESTALL(OFFLINE_FILE_TYPE_FILTERED, file_type_) ||
// Regular fall-through.
(prev_instr_.instr.addr + prev_instr_.instr.size == memref.instr.addr) ||
(prev_instr_[memref.data.tid].instr.addr +
prev_instr_[memref.data.tid].instr.size ==
memref.instr.addr) ||
// String loop.
(prev_instr_.instr.addr == memref.instr.addr &&
(prev_instr_[memref.data.tid].instr.addr == memref.instr.addr &&
memref.instr.type == TRACE_TYPE_INSTR_NO_FETCH) ||
// Kernel-mediated, but we can't tell if we had a thread swap.
(prev_xfer_marker_.instr.tid != 0 &&
(prev_xfer_marker_.instr.tid != memref.instr.tid ||
prev_xfer_marker_.marker.marker_type ==
(prev_xfer_marker_[memref.data.tid].instr.tid != 0 &&
(prev_xfer_marker_[memref.data.tid].instr.tid != memref.instr.tid ||
prev_xfer_marker_[memref.data.tid].marker.marker_type ==
TRACE_MARKER_TYPE_KERNEL_EVENT ||
prev_xfer_marker_.marker.marker_type ==
prev_xfer_marker_[memref.data.tid].marker.marker_type ==
TRACE_MARKER_TYPE_KERNEL_XFER)) ||
prev_instr_.instr.type == TRACE_TYPE_INSTR_SYSENTER);
prev_instr_[memref.data.tid].instr.type == TRACE_TYPE_INSTR_SYSENTER);
// XXX: If we had instr decoding we could check direct branch targets
// and look for gaps after branches.
}
#ifdef UNIX
// Ensure signal handlers return to the interruption point.
if (prev_xfer_marker_.marker.marker_type == TRACE_MARKER_TYPE_KERNEL_XFER) {
assert(memref.instr.tid != pre_signal_instr_.instr.tid ||
memref.instr.addr == pre_signal_instr_.instr.addr ||
if (prev_xfer_marker_[memref.data.tid].marker.marker_type ==
TRACE_MARKER_TYPE_KERNEL_XFER) {
assert(memref.instr.addr ==
pre_signal_instr_[memref.data.tid].top().instr.addr ||
// Asynch will go to the subsequent instr.
memref.instr.addr ==
pre_signal_instr_.instr.addr + pre_signal_instr_.instr.size ||
pre_signal_instr_[memref.data.tid].top().instr.addr +
pre_signal_instr_[memref.data.tid].top().instr.size ||
// Nested signal. XXX: This only works for our annotated test
// signal_invariants.
memref.instr.addr == app_handler_pc_ ||
// Marker for rseq abort handler. Not as unique as a prefetch, but
// we need an instruction and not a data type.
memref.instr.type == TRACE_TYPE_INSTR_DIRECT_JUMP ||
// Too hard to figure out branch targets.
type_is_instr_branch(pre_signal_instr_.instr.type) ||
pre_signal_instr_.instr.type == TRACE_TYPE_INSTR_SYSENTER);
type_is_instr_branch(
pre_signal_instr_[memref.data.tid].top().instr.type) ||
pre_signal_instr_[memref.data.tid].top().instr.type ==
TRACE_TYPE_INSTR_SYSENTER);
pre_signal_instr_[memref.data.tid].pop();
}
#endif
prev_instr_ = memref;
prev_instr_[memref.data.tid] = memref;
// Clear prev_xfer_marker_ on an instr (not a memref which could come between an
// instr and a kernel-mediated far-away instr) to ensure it's *immediately*
// prior (i#3937).
memset(&prev_xfer_marker_, 0, sizeof(prev_xfer_marker_));
prev_xfer_marker_[memref.data.tid] = {};
}
if (memref.marker.type == TRACE_TYPE_MARKER &&
// Ignore timestamp, etc. markers which show up a signal delivery boundaries
Expand All @@ -197,22 +210,23 @@ trace_invariants_t::process_memref(const memref_t &memref)
}
if (memref.marker.marker_type == TRACE_MARKER_TYPE_KERNEL_EVENT &&
// Give up on back-to-back signals.
prev_xfer_marker_.marker.marker_type != TRACE_MARKER_TYPE_KERNEL_XFER)
pre_signal_instr_ = prev_instr_;
prev_xfer_marker_ = memref;
prev_xfer_marker_[memref.data.tid].marker.marker_type !=
TRACE_MARKER_TYPE_KERNEL_XFER)
pre_signal_instr_[memref.data.tid].push(prev_instr_[memref.data.tid]);
prev_xfer_marker_[memref.data.tid] = memref;
}

// Look for annotations where signal_invariants.c and rseq.c pass info to us on what
// to check for. We assume the app does not have prefetch instrs w/ low addresses.
if (memref.data.type == TRACE_TYPE_PREFETCHT2 && memref.data.addr < 1024) {
instrs_until_interrupt_ = static_cast<int>(memref.data.addr);
instrs_until_interrupt_[memref.data.tid] = static_cast<int>(memref.data.addr);
}
if (memref.data.type == TRACE_TYPE_PREFETCHT1 && memref.data.addr < 1024) {
memrefs_until_interrupt_ = static_cast<int>(memref.data.addr);
memrefs_until_interrupt_[memref.data.tid] = static_cast<int>(memref.data.addr);
}

prev_prev_entry_ = prev_entry_;
prev_entry_ = memref;
prev_prev_entry_[memref.data.tid] = prev_entry_[memref.data.tid];
prev_entry_[memref.data.tid] = memref;
return true;
}

Expand Down
15 changes: 8 additions & 7 deletions clients/drcachesim/tests/trace_invariants.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

#include "../analysis_tool.h"
#include "../common/memref.h"
#include <stack>
#include <unordered_map>

class trace_invariants_t : public analysis_tool_t {
Expand All @@ -54,13 +55,13 @@ class trace_invariants_t : public analysis_tool_t {
bool knob_offline_;
unsigned int knob_verbose_;
std::string knob_test_name_;
memref_t prev_instr_;
memref_t prev_xfer_marker_;
memref_t prev_entry_;
memref_t prev_prev_entry_;
memref_t pre_signal_instr_;
int instrs_until_interrupt_;
int memrefs_until_interrupt_;
std::unordered_map<memref_tid_t, memref_t> prev_instr_;
std::unordered_map<memref_tid_t, memref_t> prev_xfer_marker_;
std::unordered_map<memref_tid_t, memref_t> prev_entry_;
std::unordered_map<memref_tid_t, memref_t> prev_prev_entry_;
std::unordered_map<memref_tid_t, std::stack<memref_t>> pre_signal_instr_;
std::unordered_map<memref_tid_t, int> instrs_until_interrupt_;
std::unordered_map<memref_tid_t, int> memrefs_until_interrupt_;
addr_t app_handler_pc_;
offline_file_type_t file_type_ = OFFLINE_FILE_TYPE_DEFAULT;
std::unordered_map<memref_tid_t, bool> thread_exited_;
Expand Down

0 comments on commit 59c6e33

Please sign in to comment.