Skip to content

Commit

Permalink
i#5538 memtrace seek, part 4: Check chunk boundaries
Browse files Browse the repository at this point in the history
Adds an invariant check that chunk boundaries contain the proper
number of instructions.

Fixes off-by-one errors in chunk counts due to not checking at
end-of-loop, found by visual inspection (with a local view tool that
prints instr counts; that will be committed later) and confirmed to
break this new check without the fix in the
tool.drcacheoff.invariant_checker test.

Issue: #5538
  • Loading branch information
derekbruening committed Oct 28, 2022
1 parent 196e320 commit a73ea0a
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 0 deletions.
15 changes: 15 additions & 0 deletions clients/drcachesim/tools/invariant_checker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem
++shard->ref_count;
if (shard->tid == -1 && memref.data.tid != 0)
shard->tid = memref.data.tid;
// XXX i#5538: Have the infrastructure provide a common instr and record count.
if (type_is_instr(memref.instr.type))
++shard->instr_count_;
#ifdef UNIX
if (has_annotations_) {
// Check conditions specific to the signal_invariants app, where it
Expand Down Expand Up @@ -191,6 +194,18 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem
shard->found_page_size_marker_ = true;
}

if (memref.marker.type == TRACE_TYPE_MARKER &&
memref.marker.marker_type == TRACE_MARKER_TYPE_CHUNK_INSTR_COUNT) {
shard->chunk_instr_count_ = memref.marker.marker_value;
}
if (memref.marker.type == TRACE_TYPE_MARKER &&
memref.marker.marker_type == TRACE_MARKER_TYPE_CHUNK_FOOTER) {
report_if_false(shard,
shard->chunk_instr_count_ != 0 &&
shard->instr_count_ % shard->chunk_instr_count_ == 0,
"Chunk instruction counts are inconsistent");
}

// Invariant: a function marker should not appear between an instruction and its
// memrefs or in the middle of a block (we assume elision is turned off and so a
// callee entry will always be the top of a block). (We don't check for other types
Expand Down
2 changes: 2 additions & 0 deletions clients/drcachesim/tools/invariant_checker.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ class invariant_checker_t : public analysis_tool_t {
offline_file_type_t file_type_ = OFFLINE_FILE_TYPE_DEFAULT;
uintptr_t last_window_ = 0;
bool window_transition_ = false;
uint64_t chunk_instr_count_ = 0;
uint64_t instr_count_ = 0;
};

// We provide this for subclasses to run these invariants with custom
Expand Down
14 changes: 14 additions & 0 deletions clients/drcachesim/tracer/raw2trace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1102,6 +1102,12 @@ raw2trace_t::append_delayed_branch(void *tls)
return "Failed to write to output file";
}
tdata->delayed_branch.clear();
if (tdata->cur_chunk_instr_count >= chunk_instr_count_) {
DEBUG_ASSERT(tdata->cur_chunk_instr_count == chunk_instr_count_);
std::string error = open_new_chunk(tdata);
if (!error.empty())
return error;
}
return "";
}

Expand Down Expand Up @@ -1225,6 +1231,14 @@ raw2trace_t::write(void *tls, const trace_entry_t *start, const trace_entry_t *e
reinterpret_cast<const char *>(end) -
reinterpret_cast<const char *>(start)))
return "Failed to write to output file";
// If we're at the end of a block (minus its delayed branch) we need
// to split now to avoid going too far by waiting for the next instr.
if (tdata->cur_chunk_instr_count >= chunk_instr_count_) {
DEBUG_ASSERT(tdata->cur_chunk_instr_count == chunk_instr_count_);
std::string error = open_new_chunk(tdata);
if (!error.empty())
return error;
}
return "";
}

Expand Down

0 comments on commit a73ea0a

Please sign in to comment.