From a7490046bd451d40d52422e9b5a744c305e03e05 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Thu, 12 Oct 2023 18:53:58 -0400 Subject: [PATCH] i#6354: Do not output zero-instruction (unfiltered) threads (#6356) 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 --- .../drcachesim/tests/scheduler_launcher.cpp | 16 ++++++ .../drcachesim/tools/invariant_checker.cpp | 5 ++ clients/drcachesim/tracer/output.cpp | 56 +++++++++++++------ 3 files changed, 59 insertions(+), 18 deletions(-) diff --git a/clients/drcachesim/tests/scheduler_launcher.cpp b/clients/drcachesim/tests/scheduler_launcher.cpp index cedea03c401..bfebb7e5861 100644 --- a/clients/drcachesim/tests/scheduler_launcher.cpp +++ b/clients/drcachesim/tests/scheduler_launcher.cpp @@ -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 } } diff --git a/clients/drcachesim/tools/invariant_checker.cpp b/clients/drcachesim/tools/invariant_checker.cpp index d97cd15a079..90250def034 100644 --- a/clients/drcachesim/tools/invariant_checker.cpp +++ b/clients/drcachesim/tools/invariant_checker.cpp @@ -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) { diff --git a/clients/drcachesim/tracer/output.cpp b/clients/drcachesim/tracer/output.cpp index 6f28e14e0f8..ea4927c9b92 100644 --- a/clients/drcachesim/tracer/output.cpp +++ b/clients/drcachesim/tracer/output.cpp @@ -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(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) @@ -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(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,