From 14a8777c5b8bdad1323c4f73f182445ed66d8b37 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Wed, 9 Oct 2024 01:00:02 -0400 Subject: [PATCH 1/7] i#5505 PT tracing: Add burst PT test with interrupted futex Adds test where one of the threads is waiting on a futex when detach occurs. Such futex PT traces have been observed to fail in libipt decode. We also do not want such PT traces because they do not include real app behavior. This test is to ensure we do not include such syscalls in the final trace. Issue: #5505 --- clients/drcachesim/CMakeLists.txt | 11 + clients/drcachesim/tests/burst_syscall_pt.cpp | 240 ++++++++++++++++++ .../offline-burst_syscall_pt_SUDO.template | 8 + .../offline-burst_syscall_pt_SUDO.templatex | 8 + clients/drcachesim/tracer/tracer.cpp | 2 +- suite/tests/CMakeLists.txt | 16 +- 6 files changed, 280 insertions(+), 5 deletions(-) create mode 100644 clients/drcachesim/tests/burst_syscall_pt.cpp create mode 100644 clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.template create mode 100644 clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.templatex diff --git a/clients/drcachesim/CMakeLists.txt b/clients/drcachesim/CMakeLists.txt index 81e54281312..6d6ad8e344b 100644 --- a/clients/drcachesim/CMakeLists.txt +++ b/clients/drcachesim/CMakeLists.txt @@ -1086,6 +1086,17 @@ if (BUILD_TESTS) use_DynamoRIO_drmemtrace_tracer(tool.drcacheoff.burst_syscall_inject) endif () + if (LINUX AND BUILD_PT_POST_PROCESSOR AND BUILD_PT_TRACER) + add_executable(tool.drcacheoff.burst_syscall_pt_SUDO tests/burst_syscall_pt.cpp) + configure_DynamoRIO_static(tool.drcacheoff.burst_syscall_pt_SUDO) + use_DynamoRIO_static_client(tool.drcacheoff.burst_syscall_pt_SUDO drmemtrace_static) + target_link_libraries(tool.drcacheoff.burst_syscall_pt_SUDO drmemtrace_raw2trace + drmemtrace_analyzer test_helpers drmemtrace_basic_counts) + add_win32_flags(tool.drcacheoff.burst_syscall_pt_SUDO) + use_DynamoRIO_drmemtrace_tracer(tool.drcacheoff.burst_syscall_pt_SUDO) + link_with_pthread(tool.drcacheoff.burst_syscall_pt_SUDO) + endif () + if (UNIX) if (X86 AND NOT APPLE) # This test is x86-specific. # uses ptrace and looks for linux-specific syscalls diff --git a/clients/drcachesim/tests/burst_syscall_pt.cpp b/clients/drcachesim/tests/burst_syscall_pt.cpp new file mode 100644 index 00000000000..41769d98635 --- /dev/null +++ b/clients/drcachesim/tests/burst_syscall_pt.cpp @@ -0,0 +1,240 @@ +/* ********************************************************** + * Copyright (c) 2016-2024 Google, Inc. All rights reserved. + * **********************************************************/ + +/* + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * * Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * * Neither the name of Google, Inc. nor the names of its contributors may be + * used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL GOOGLE, INC. OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH + * DAMAGE. + */ + +// This application links in drmemtrace_static and acquires a trace during +// a "burst" of execution that includes some system call traces collected +// using Intel-PT. + +// This is set globally in CMake for other tests so easier to undef here. +#undef DR_REG_ENUM_COMPATIBILITY + +#include "analyzer.h" +#include "tools/basic_counts.h" +#include "dr_api.h" +#include "drmemtrace/drmemtrace.h" +#include "drmemtrace/raw2trace.h" +#include "mock_reader.h" +#include "raw2trace_directory.h" +#include "scheduler.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace dynamorio { +namespace drmemtrace { + +#define FATAL_ERROR(msg, ...) \ + do { \ + fprintf(stderr, "ERROR: " msg "\n", ##__VA_ARGS__); \ + fflush(stderr); \ + exit(1); \ + } while (0) + +/* The futex the child waits at initially. */ +static uint32_t futex_var = 0xf00d; +/* The futex the child is transferred to. */ +static uint32_t futex_var_other = 0x8bad; + +static void * +child_futex_wait(void *) +{ + long res = syscall(SYS_futex, &futex_var, FUTEX_WAIT, /*#val=*/0xf00d, + /*timeout=*/nullptr, /*uaddr2=*/nullptr, /*val3=*/0); + assert(res == 0); + std::cerr << "Child released from futex\n"; + return NULL; +} + +static void +parent_futex_wake() +{ + /* The child would be waiting at the other futex by now. + * iX: Note that the child thread undergoes detach while it is waiting + * on futex_var_other. There is a bug at this point due to a possible + * transparency violation. When the child thread restarts futex after + * being interrupted by the detach signal, it seems like it resumes + * waiting at the original futex_var instead of futex_var_other. + * If we modify this code to do detach after this call, then the child + * is found to be waiting at futex_var_other, as expected. + */ + uint32_t *child_waiting_at_futex = &futex_var; + long res = syscall(SYS_futex, child_waiting_at_futex, FUTEX_WAKE, /*#wakeup=*/1, + /*timeout=*/nullptr, /*uaddr2=*/nullptr, /*val3=*/0); + assert(res == 1); +} + +static void +parent_futex_reque() +{ + long res; + do { + /* Repeat until the child is surely waiting at the futex. We'll know this + * when the following call returns a 1, which means the child was + * transferred to the other futex var. + */ + res = syscall(SYS_futex, &futex_var, FUTEX_CMP_REQUEUE, /*#wakeup_max=*/0, + /*#requeue_max=*/1, /*uaddr2=*/&futex_var_other, /*val3=*/0xf00d); + assert(res == 0 || res == 1); + } while (res == 0); +} + +static int +do_some_syscalls() +{ + getpid(); + gettid(); + return 1; +} + +static std::string +postprocess(void *dr_context) +{ + std::cerr << "Post-processing the trace\n"; + // Get path to write the final trace to. + const char *raw_dir; + drmemtrace_status_t mem_res = drmemtrace_get_output_path(&raw_dir); + assert(mem_res == DRMEMTRACE_SUCCESS); + std::string outdir = std::string(raw_dir) + DIRSEP + "post_processed"; + + const char *kcore_path; + drmemtrace_status_t kcore_res = drmemtrace_get_kcore_path(&kcore_path); + assert(kcore_res == DRMEMTRACE_SUCCESS); + + raw2trace_directory_t dir; + if (!dr_create_dir(outdir.c_str())) + FATAL_ERROR("Failed to create output dir."); + std::string dir_err = dir.initialize(raw_dir, outdir, DEFAULT_TRACE_COMPRESSION_TYPE, + /*syscall_template_file=*/""); + assert(dir_err.empty()); + raw2trace_t raw2trace(dir.modfile_bytes_, dir.in_files_, dir.out_files_, + dir.out_archives_, dir.encoding_file_, + dir.serial_schedule_file_, dir.cpu_schedule_file_, dr_context, + /*verbosity=*/0, /*worker_count=*/-1, + /*alt_module_dir=*/"", + /*chunk_instr_count=*/10 * 1000 * 1000, dir.in_kfiles_map_, + dir.kcoredir_, /*kallsyms_path=*/"", + /*syscall_template_file=*/nullptr, + // We want to fail if any error is encountered. + /*pt2ir_best_effort=*/false); + std::string error = raw2trace.do_conversion(); + if (!error.empty()) + FATAL_ERROR("raw2trace failed: %s\n", error.c_str()); + uint64 decoded_syscall_count = + raw2trace.get_statistic(RAW2TRACE_STAT_SYSCALL_TRACES_CONVERTED); + if (decoded_syscall_count <= 2) { + std::cerr << "Incorrect decoded syscall count (found: " << decoded_syscall_count + << " vs expected > 2)\n"; + } + return outdir; +} + +static basic_counts_t::counters_t +get_basic_counts(const std::string &trace_dir) +{ + auto basic_counts_tool = + std::unique_ptr(new basic_counts_t(/*verbose=*/0)); + std::vector tools; + tools.push_back(basic_counts_tool.get()); + analyzer_t analyzer(trace_dir, &tools[0], static_cast(tools.size())); + if (!analyzer) { + FATAL_ERROR("failed to initialize analyzer: %s", + analyzer.get_error_string().c_str()); + } + if (!analyzer.run()) { + FATAL_ERROR("failed to run analyzer: %s", analyzer.get_error_string().c_str()); + } + return basic_counts_tool->get_total_counts(); +} + +static void +gather_trace() +{ + if (setenv("DYNAMORIO_OPTIONS", + "-stderr_mask 0xc -client_lib ';;-offline -enable_kernel_tracing", + 1 /*override*/) != 0) + std::cerr << "failed to set env var!\n"; + dr_app_setup(); + assert(!dr_app_running_under_dynamorio()); + dr_app_start(); + + pthread_t child_thread; + int res = pthread_create(&child_thread, NULL, child_futex_wait, NULL); + assert(res == 0); + + /* Ensure that the child is waiting at a futex. */ + parent_futex_reque(); + + do_some_syscalls(); + + dr_app_stop_and_cleanup(); + + /* Wake up the child finally. */ + parent_futex_wake(); + + pthread_join(child_thread, NULL); + + return; +} + +static int +test_pt_trace(void *dr_context) +{ + std::string trace_dir = postprocess(dr_context); + basic_counts_t::counters_t final_trace_counts = get_basic_counts(trace_dir); + if (final_trace_counts.kernel_instrs == 0) { + std::cerr << "Unexpected kernel instr count in the final trace (" + << final_trace_counts.kernel_instrs << ")\n"; + return 1; + } + return 0; +} + +int +test_main(int argc, const char *argv[]) +{ + gather_trace(); + void *dr_context = dr_standalone_init(); + if (test_pt_trace(dr_context)) { + return 1; + } + dr_standalone_exit(); + return 0; +} + +} // namespace drmemtrace +} // namespace dynamorio diff --git a/clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.template b/clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.template new file mode 100644 index 00000000000..66c97fa07f2 --- /dev/null +++ b/clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.template @@ -0,0 +1,8 @@ +ERROR: PT tracing for the last syscall .* of thread .* was found active at detach. +Child released from futex +Post-processing the trace +Syscall mix tool results: + syscall count : syscall_num +.* + syscall trace count : syscall_num +.* diff --git a/clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.templatex b/clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.templatex new file mode 100644 index 00000000000..66c97fa07f2 --- /dev/null +++ b/clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.templatex @@ -0,0 +1,8 @@ +ERROR: PT tracing for the last syscall .* of thread .* was found active at detach. +Child released from futex +Post-processing the trace +Syscall mix tool results: + syscall count : syscall_num +.* + syscall trace count : syscall_num +.* diff --git a/clients/drcachesim/tracer/tracer.cpp b/clients/drcachesim/tracer/tracer.cpp index 6d8f6cab7d1..4bee64dc2f6 100644 --- a/clients/drcachesim/tracer/tracer.cpp +++ b/clients/drcachesim/tracer/tracer.cpp @@ -1861,7 +1861,7 @@ event_thread_exit(void *drcontext) int cur_recording_sysnum = data->syscall_pt_trace.get_cur_recording_sysnum(); if (cur_recording_sysnum != INVALID_SYSNUM) { NOTIFY(0, - "ERROR: PT tracing for the last syscall %d of thread T%d was " + "ERROR: PT tracing for the last syscall %d of thread T%d was " "found active at detach.\n", cur_recording_sysnum, dr_get_thread_id(drcontext)); // Ignore return value and try to continue in release build. diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 577fe842833..f1a1ffdbfac 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -1402,8 +1402,9 @@ function(torun test key source native standalone_dr dr_ops exe_ops added_out pas string(REGEX REPLACE "/drrun;" "/drrun;-32;" rundr "${rundr}") endif () + set(cmd_prefix "") if (DEFINED ${key}_sudo) - set(rundr "sudo;${rundr}") + set(cmd_prefix "sudo") endif () if (is_runall) @@ -1491,7 +1492,7 @@ function(torun test key source native standalone_dr dr_ops exe_ops added_out pas # Clear client from dr command and run native. string(REGEX REPLACE ";-quiet;(.*);--" ";-no_inject;--;" rundr "${rundr}") endif () - set(cmd_with_at ${rundr} ${app_path} ${exe_ops}) + set(cmd_with_at ${cmd_prefix} ${rundr} ${app_path} ${exe_ops}) # we pass intra-arg spaces via @@ and inter-arg via @ and ; via ! # to get around the pain of trying to quote everything just right: # much simpler this way. @@ -1516,9 +1517,9 @@ function(torun test key source native standalone_dr dr_ops exe_ops added_out pas # doing its built-in regex cmp so we use a separate script. # We also use this approach for tests that need custom success tests. if (DEFINED ${key}_nodr) - set(cmd_with_at ${exepath} ${exe_ops}) + set(cmd_with_at ${cmd_prefix} ${exepath} ${exe_ops}) else () - set(cmd_with_at ${rundr} ${exepath} ${exe_ops}) + set(cmd_with_at ${cmd_prefix} ${rundr} ${exepath} ${exe_ops}) endif () # we pass intra-arg spaces via @@ and inter-arg via @ and ; via ! # to get around the pain of trying to quote everything just right: @@ -5040,6 +5041,13 @@ if (BUILD_CLIENTS) # kernel tracing code is not being intentionally tested. torunonly_drcachesim(kernel-skip-kcore_SUDO ${ci_shared_app} "-offline -enable_kernel_tracing -skip_kcore_dump" "") + + if (LINUX) + set(tool.drcacheoff.burst_syscall_pt_SUDO_nodr ON) + set(tool.drcacheoff.burst_syscall_pt_SUDO_sudo ON) + torunonly_drcacheoff(burst_syscall_pt_SUDO tool.drcacheoff.burst_syscall_pt_SUDO + "" "@-tool@syscall_mix" "") + endif () endif (BUILD_PT_TRACER AND BUILD_PT_POST_PROCESSOR) endif (proc_supports_pt) From f5e59e4940a824dc2f9e117759fc3b076d9651be Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Wed, 9 Oct 2024 01:07:22 -0400 Subject: [PATCH 2/7] Remove extraneous file --- .../tests/offline-burst_syscall_pt_SUDO.template | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.template diff --git a/clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.template b/clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.template deleted file mode 100644 index 66c97fa07f2..00000000000 --- a/clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.template +++ /dev/null @@ -1,8 +0,0 @@ -ERROR: PT tracing for the last syscall .* of thread .* was found active at detach. -Child released from futex -Post-processing the trace -Syscall mix tool results: - syscall count : syscall_num -.* - syscall trace count : syscall_num -.* From a8e7195726f23ef4086a6db25a9038c9ec57cb62 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Wed, 9 Oct 2024 01:46:45 -0400 Subject: [PATCH 3/7] fix other sudo tests --- suite/tests/CMakeLists.txt | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index f1a1ffdbfac..590c4ccf0f3 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -1402,9 +1402,12 @@ function(torun test key source native standalone_dr dr_ops exe_ops added_out pas string(REGEX REPLACE "/drrun;" "/drrun;-32;" rundr "${rundr}") endif () - set(cmd_prefix "") if (DEFINED ${key}_sudo) - set(cmd_prefix "sudo") + if (DEFINED ${key}_nodr) + set(exepath "sudo;${exepath}") + else () + set(rundr "sudo;${rundr}") + endif () endif () if (is_runall) @@ -1492,7 +1495,7 @@ function(torun test key source native standalone_dr dr_ops exe_ops added_out pas # Clear client from dr command and run native. string(REGEX REPLACE ";-quiet;(.*);--" ";-no_inject;--;" rundr "${rundr}") endif () - set(cmd_with_at ${cmd_prefix} ${rundr} ${app_path} ${exe_ops}) + set(cmd_with_at ${rundr} ${app_path} ${exe_ops}) # we pass intra-arg spaces via @@ and inter-arg via @ and ; via ! # to get around the pain of trying to quote everything just right: # much simpler this way. @@ -1517,9 +1520,9 @@ function(torun test key source native standalone_dr dr_ops exe_ops added_out pas # doing its built-in regex cmp so we use a separate script. # We also use this approach for tests that need custom success tests. if (DEFINED ${key}_nodr) - set(cmd_with_at ${cmd_prefix} ${exepath} ${exe_ops}) + set(cmd_with_at ${exepath} ${exe_ops}) else () - set(cmd_with_at ${cmd_prefix} ${rundr} ${exepath} ${exe_ops}) + set(cmd_with_at ${rundr} ${exepath} ${exe_ops}) endif () # we pass intra-arg spaces via @@ and inter-arg via @ and ; via ! # to get around the pain of trying to quote everything just right: From e8ac6c436c6ad76a0cbbe1fa687c81a982c90b4b Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Wed, 9 Oct 2024 10:39:13 -0400 Subject: [PATCH 4/7] Add custom test analysis tool to look for needed patterns --- clients/drcachesim/tests/burst_syscall_pt.cpp | 126 ++++++++++++++++-- .../offline-burst_syscall_pt_SUDO.templatex | 1 + 2 files changed, 116 insertions(+), 11 deletions(-) diff --git a/clients/drcachesim/tests/burst_syscall_pt.cpp b/clients/drcachesim/tests/burst_syscall_pt.cpp index 41769d98635..cc7d9681c9e 100644 --- a/clients/drcachesim/tests/burst_syscall_pt.cpp +++ b/clients/drcachesim/tests/burst_syscall_pt.cpp @@ -163,13 +163,118 @@ postprocess(void *dr_context) return outdir; } -static basic_counts_t::counters_t -get_basic_counts(const std::string &trace_dir) +// Trace analysis tool that allows us to verify properties of the generated PT trace. +class pt_analysis_tool_t : public analysis_tool_t { +public: + pt_analysis_tool_t() + { + } + bool + process_memref(const memref_t &memref) override + { + FATAL_ERROR("Expected to use sharded mode"); + return true; + } + bool + parallel_shard_supported() override + { + return true; + } + void * + parallel_shard_init(int shard_index, void *worker_data) override + { + auto per_shard = new per_shard_t; + return reinterpret_cast(per_shard); + } + bool + parallel_shard_exit(void *shard_data) override + { + std::lock_guard guard(shard_exit_mutex_); + per_shard_t *shard = reinterpret_cast(shard_data); + if (shard->syscall_count == 0) + return true; + if (shard->syscall_count > 1 && !shard->any_syscall_had_trace) { + std::cerr << "No syscall had a trace\n"; + } + // iX: Invert the second condition below after #7027 is merged. + if (shard->prev_was_futex_marker && shard->prev_syscall_had_trace) { + found_final_futex_without_trace_ = true; + } + if (shard->kernel_instr_count > 0) { + found_some_kernel_instrs_ = true; + } + return true; + } + bool + parallel_shard_memref(void *shard_data, const memref_t &memref) override + { + per_shard_t *shard = reinterpret_cast(shard_data); + if (memref.marker.type == TRACE_TYPE_MARKER) { + switch (memref.marker.marker_type) { + case TRACE_MARKER_TYPE_SYSCALL_TRACE_START: + shard->in_syscall_trace = true; + break; + case TRACE_MARKER_TYPE_SYSCALL_TRACE_END: + shard->in_syscall_trace = false; + shard->prev_syscall_had_trace = true; + shard->any_syscall_had_trace = true; + break; + case TRACE_MARKER_TYPE_SYSCALL: + ++shard->syscall_count; + shard->prev_syscall_had_trace = false; + if (memref.marker.marker_value == SYS_futex) { + shard->prev_was_futex_marker = true; + } + break; + } + if (shard->in_syscall_trace) { + ++shard->kernel_instr_count; + return true; + } + if (type_is_instr(memref.data.type)) { + shard->prev_was_futex_marker = false; + shard->prev_syscall_had_trace = false; + } + } + return true; + } + bool + print_results() override + { + if (!found_final_futex_without_trace_) { + std::cerr + << "Did not find any thread trace with final futex without PT trace\n"; + } else { + std::cerr << "Found matching signature in a thread\n"; + } + if (!found_some_kernel_instrs_) { + std::cerr << "Did not find any kernel instrs\n"; + } + return true; + } + +private: + // Data tracked per shard. + struct per_shard_t { + bool prev_was_futex_marker = false; + bool prev_syscall_had_trace = false; + bool any_syscall_had_trace = false; + int syscall_count = 0; + bool in_syscall_trace = false; + int kernel_instr_count = 0; + }; + + bool found_final_futex_without_trace_ = false; + bool found_some_kernel_instrs_ = false; + std::mutex shard_exit_mutex_; +}; + +static bool +run_pt_analysis(const std::string &trace_dir) { - auto basic_counts_tool = - std::unique_ptr(new basic_counts_t(/*verbose=*/0)); + auto pt_analysis_tool = std::unique_ptr(new pt_analysis_tool_t()); std::vector tools; - tools.push_back(basic_counts_tool.get()); + tools.push_back(pt_analysis_tool.get()); analyzer_t analyzer(trace_dir, &tools[0], static_cast(tools.size())); if (!analyzer) { FATAL_ERROR("failed to initialize analyzer: %s", @@ -178,7 +283,10 @@ get_basic_counts(const std::string &trace_dir) if (!analyzer.run()) { FATAL_ERROR("failed to run analyzer: %s", analyzer.get_error_string().c_str()); } - return basic_counts_tool->get_total_counts(); + if (!analyzer.print_stats()) { + FATAL_ERROR("failed to print stats: %s", analyzer.get_error_string().c_str()); + } + return true; } static void @@ -215,12 +323,8 @@ static int test_pt_trace(void *dr_context) { std::string trace_dir = postprocess(dr_context); - basic_counts_t::counters_t final_trace_counts = get_basic_counts(trace_dir); - if (final_trace_counts.kernel_instrs == 0) { - std::cerr << "Unexpected kernel instr count in the final trace (" - << final_trace_counts.kernel_instrs << ")\n"; + if (!run_pt_analysis(trace_dir)) return 1; - } return 0; } diff --git a/clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.templatex b/clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.templatex index 66c97fa07f2..840000756c6 100644 --- a/clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.templatex +++ b/clients/drcachesim/tests/offline-burst_syscall_pt_SUDO.templatex @@ -1,6 +1,7 @@ ERROR: PT tracing for the last syscall .* of thread .* was found active at detach. Child released from futex Post-processing the trace +Found matching signature in a thread Syscall mix tool results: syscall count : syscall_num .* From fa048d0c5c8fa2392dcf71c14fcf35689848f90b Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Wed, 9 Oct 2024 11:37:37 -0400 Subject: [PATCH 5/7] Fix test logic --- clients/drcachesim/tests/burst_syscall_pt.cpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/clients/drcachesim/tests/burst_syscall_pt.cpp b/clients/drcachesim/tests/burst_syscall_pt.cpp index cc7d9681c9e..ac51382bdfa 100644 --- a/clients/drcachesim/tests/burst_syscall_pt.cpp +++ b/clients/drcachesim/tests/burst_syscall_pt.cpp @@ -196,8 +196,7 @@ class pt_analysis_tool_t : public analysis_tool_t { if (shard->syscall_count > 1 && !shard->any_syscall_had_trace) { std::cerr << "No syscall had a trace\n"; } - // iX: Invert the second condition below after #7027 is merged. - if (shard->prev_was_futex_marker && shard->prev_syscall_had_trace) { + if (shard->prev_was_futex_marker && !shard->prev_syscall_had_trace) { found_final_futex_without_trace_ = true; } if (shard->kernel_instr_count > 0) { @@ -227,15 +226,15 @@ class pt_analysis_tool_t : public analysis_tool_t { } break; } - if (shard->in_syscall_trace) { - ++shard->kernel_instr_count; - return true; - } - if (type_is_instr(memref.data.type)) { - shard->prev_was_futex_marker = false; - shard->prev_syscall_had_trace = false; - } } + if (!type_is_instr(memref.data.type)) + return true; + if (shard->in_syscall_trace) { + ++shard->kernel_instr_count; + return true; + } + shard->prev_was_futex_marker = false; + shard->prev_syscall_had_trace = false; return true; } bool From ae943feb6f2807f0a0f24646fee4cf32f3526884 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Wed, 9 Oct 2024 19:48:51 -0400 Subject: [PATCH 6/7] Add issue# to comment --- clients/drcachesim/tests/burst_syscall_pt.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/drcachesim/tests/burst_syscall_pt.cpp b/clients/drcachesim/tests/burst_syscall_pt.cpp index ac51382bdfa..37149957fcd 100644 --- a/clients/drcachesim/tests/burst_syscall_pt.cpp +++ b/clients/drcachesim/tests/burst_syscall_pt.cpp @@ -84,7 +84,7 @@ static void parent_futex_wake() { /* The child would be waiting at the other futex by now. - * iX: Note that the child thread undergoes detach while it is waiting + * i#7034: Note that the child thread undergoes detach while it is waiting * on futex_var_other. There is a bug at this point due to a possible * transparency violation. When the child thread restarts futex after * being interrupted by the detach signal, it seems like it resumes From ddb4abd7901a72315b6060c51fb1188e6b5f63df Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Thu, 10 Oct 2024 10:18:13 -0400 Subject: [PATCH 7/7] Comment clarifications --- clients/drcachesim/tests/burst_syscall_pt.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/clients/drcachesim/tests/burst_syscall_pt.cpp b/clients/drcachesim/tests/burst_syscall_pt.cpp index 37149957fcd..f58dccaf9e1 100644 --- a/clients/drcachesim/tests/burst_syscall_pt.cpp +++ b/clients/drcachesim/tests/burst_syscall_pt.cpp @@ -86,11 +86,11 @@ parent_futex_wake() /* The child would be waiting at the other futex by now. * i#7034: Note that the child thread undergoes detach while it is waiting * on futex_var_other. There is a bug at this point due to a possible - * transparency violation. When the child thread restarts futex after - * being interrupted by the detach signal, it seems like it resumes + * transparency violation in DR. When the child thread restarts futex after + * being interrupted by DR's detach signal, it is found to resume * waiting at the original futex_var instead of futex_var_other. - * If we modify this code to do detach after this call, then the child - * is found to be waiting at futex_var_other, as expected. + * If we modify this app to do detach after parent_futex_wake returns, then + * the child is found to be waiting at futex_var_other as expected. */ uint32_t *child_waiting_at_futex = &futex_var; long res = syscall(SYS_futex, child_waiting_at_futex, FUTEX_WAKE, /*#wakeup=*/1, @@ -105,7 +105,8 @@ parent_futex_reque() do { /* Repeat until the child is surely waiting at the futex. We'll know this * when the following call returns a 1, which means the child was - * transferred to the other futex var. + * transferred to futex_var_other. This is to ensure that the child thread + * is inside the futex syscall when DR detaches. */ res = syscall(SYS_futex, &futex_var, FUTEX_CMP_REQUEUE, /*#wakeup_max=*/0, /*#requeue_max=*/1, /*uaddr2=*/&futex_var_other, /*val3=*/0xf00d); @@ -156,6 +157,7 @@ postprocess(void *dr_context) FATAL_ERROR("raw2trace failed: %s\n", error.c_str()); uint64 decoded_syscall_count = raw2trace.get_statistic(RAW2TRACE_STAT_SYSCALL_TRACES_CONVERTED); + // We should see atleast the getpid, gettid, and futex syscalls made by the parent. if (decoded_syscall_count <= 2) { std::cerr << "Incorrect decoded syscall count (found: " << decoded_syscall_count << " vs expected > 2)\n"; @@ -193,6 +195,8 @@ class pt_analysis_tool_t : public analysis_tool_t { per_shard_t *shard = reinterpret_cast(shard_data); if (shard->syscall_count == 0) return true; + // In case the child has just the one futex syscall which was skipped + // from the trace. if (shard->syscall_count > 1 && !shard->any_syscall_had_trace) { std::cerr << "No syscall had a trace\n"; }