Skip to content

Commit

Permalink
i#6354: Do not output zero-instruction (unfiltered) threads (#6356)
Browse files Browse the repository at this point in the history
Tightens the normal-mode output rule to discard a thread with no
instructions (keeping the no-data check for i-filtered modes) and
expands the empty-thread behavior to include regular exit runs and not
just detach.

Adds the scheduler_launcher CPU check from PR #6355.

It is not easy to create a test with a thread that reliably executes
zero instructions yet still outputs some data. I added an invariant
check for a zero-instruction thread and confirmed it fires on the trace
in PR #6355:

```
$ bin64/drrun -t drcachesim -simulator_type invariant_checker -indir ~/tmp/drmemtrace.schedtest.x64
Trace invariant failure in T85300 at ref # 12 (0 instrs since timestamp 13341391214912820): An unfiltered thread should have at least 1 instruction
```

Issue: #6354
  • Loading branch information
derekbruening authored Oct 12, 2023
1 parent ea47baf commit a749004
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 18 deletions.
16 changes: 16 additions & 0 deletions clients/drcachesim/tests/scheduler_launcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,22 @@ simulate_core(int ordinal, scheduler_t::stream_t *stream, const scheduler_t &sch
cur_segment_instrs = 0;
}
}
#ifdef HAS_ZIP
else if (record.marker.type == dynamorio::drmemtrace::TRACE_TYPE_MARKER) {
if (record.marker.marker_type ==
dynamorio::drmemtrace::TRACE_MARKER_TYPE_CPU_ID) {
if (!op_cpu_schedule_file.get_value().empty()) {
int cpu = (int)record.marker.marker_value;
int output_cpuid = stream->get_output_cpuid();
if (cpu != output_cpuid) {
FATAL_ERROR("CPU marker %d on core #%d differs from output "
"stream CPU ID %d\n",
cpu, ordinal, output_cpuid);
}
}
}
}
#endif
}
}

Expand Down
5 changes: 5 additions & 0 deletions clients/drcachesim/tools/invariant_checker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,11 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem
report_if_false(shard, shard->last_instr_count_marker_ == ASM_INSTR_COUNT,
"Incorrect instr count marker value");
}
if (!TESTANY(OFFLINE_FILE_TYPE_FILTERED | OFFLINE_FILE_TYPE_IFILTERED,
shard->file_type_)) {
report_if_false(shard, type_is_instr(shard->prev_instr_.memref.instr.type),
"An unfiltered thread should have at least 1 instruction");
}
}
if (shard->prev_entry_.marker.type == TRACE_TYPE_MARKER &&
shard->prev_entry_.marker.marker_type == TRACE_MARKER_TYPE_PHYSICAL_ADDRESS) {
Expand Down
56 changes: 38 additions & 18 deletions clients/drcachesim/tracer/output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,22 @@ local_instr_count_threshold(uint64 trace_for_instrs)
}
}

static bool
buffer_contains_nontrivial_data(per_thread_t *data)
{
if (op_L0I_filter.get_value()) {
return BUF_PTR(data->seg_base) - data->buf_base >
static_cast<ssize_t>(data->init_header_size + buf_hdr_slots_size);
}
byte *buf_ptr = BUF_PTR(data->seg_base);
for (byte *mem_ref = data->buf_base + buf_hdr_slots_size; mem_ref < buf_ptr;
mem_ref += instru->sizeof_entry()) {
if (instru->get_instr_count(mem_ref) > 0)
return true;
}
return false;
}

// Returns whether we've reached the end of this tracing window.
static bool
count_traced_instrs(void *drcontext, uintptr_t toadd, uint64 trace_for_instrs)
Expand Down Expand Up @@ -1362,24 +1378,28 @@ exit_thread_io(void *drcontext)
}
#endif

// Append a thread exit marker and output remaining records for this thread
// if we are still in a tracing mode or the thread has data from a prior window
// that it never wrote out.
if (is_in_tracing_mode(tracing_mode.load(std::memory_order_acquire)) ||
(has_tracing_windows() &&
// If non-split we always want to append a thread exit marker as there
// wouldn't be on otherwise (split has one at the end of each window file).
(!op_split_windows.get_value() ||
// If split, we only need to write if we have data from a prior window.
(get_local_window(data) < tracing_window.load(std::memory_order_acquire) &&
!is_new_window_buffer_empty(data)))) ||
// For attach we switch to BBDUP_MODE_NOP but still need to finalize
// each thread. However, we omit threads that did nothing the entire time
// we were attached.
(!has_tracing_windows() && align_attach_detach_endpoints() &&
(data->bytes_written > 0 ||
BUF_PTR(data->seg_base) - data->buf_base >
static_cast<ssize_t>(data->init_header_size + buf_hdr_slots_size)))) {
// Append a thread exit marker and output remaining records for this thread if it has
// data from a prior window that it never wrote out.
bool has_prior_window_data = has_tracing_windows() &&
// If non-split we always want to append a thread exit marker as there
// wouldn't be one otherwise (split has one at the end of each window file).
(!op_split_windows.get_value() ||
// If split, we only need to write if we have data from a prior window.
(get_local_window(data) < tracing_window.load(std::memory_order_acquire) &&
!is_new_window_buffer_empty(data)));

// Also append an exit for non-empty threads (those that wrote buffers out before,
// or have a current non-empty buffer). We completely omit empty threads.
bool is_not_empty =
(data->bytes_written > 0 || buffer_contains_nontrivial_data(data)) &&
// XXX: We may not need any of the conditions below? Should revisit
// whether a current-nop window-up-to-date needs to be excluded here.
(is_in_tracing_mode(tracing_mode.load(std::memory_order_acquire)) ||
// For attach we switch to BBDUP_MODE_NOP but still need to finalize
// each (non-empty) thread.
(!has_tracing_windows() && align_attach_detach_endpoints()));

if (has_prior_window_data || is_not_empty) {
BUF_PTR(data->seg_base) += instru->append_thread_exit(
BUF_PTR(data->seg_base), dr_get_thread_id(drcontext));
process_and_output_buffer(drcontext,
Expand Down

0 comments on commit a749004

Please sign in to comment.