From 3ab20b0e7fe99e997da9c1f5f642e7b36ad8ae9a Mon Sep 17 00:00:00 2001 From: edeiana Date: Wed, 9 Oct 2024 03:49:19 -0700 Subject: [PATCH 01/13] i#7031: invalidate CPU marker filter In some cases we don't want to expose the "as traced" CPU schedule in an offline trace because it might not be representative of the native execution of the traced program. To do so we implement `invalidate_cpu_filter_t`, a new filter that is used with `record_filter` as follows: ``` drrun -t drmemtrace -tool record_filter -filter_invalidate_cpu -indir path/to/input/trace -outdir path/to/output/trace ``` This filter set the value of TRACE_MARKER_TYPE_CPU_ID markers to (uintptr_t)-1, which representes an unknown CPU. We add a unit test `test_invalidate_cpu_filter()` and an end-to-end test `code_api|tool.record_filter_invalidate_cpu` which invokes the `invariant_checker` and `view` tool on the filtered trace. Fixes #7031 --- api/docs/release.dox | 4 + clients/drcachesim/CMakeLists.txt | 1 + clients/drcachesim/analyzer_multi.cpp | 3 +- clients/drcachesim/common/options.cpp | 5 + clients/drcachesim/common/options.h | 1 + .../record_filter_invalidate_cpu.templatex | 24 +++++ .../tests/record_filter_unit_tests.cpp | 67 +++++++++++++- .../tools/filter/invalidate_cpu_filter.h | 92 +++++++++++++++++++ .../drcachesim/tools/filter/record_filter.cpp | 8 +- .../tools/filter/record_filter_create.h | 4 +- .../tools/record_filter_launcher.cpp | 9 +- suite/tests/CMakeLists.txt | 8 ++ 12 files changed, 221 insertions(+), 5 deletions(-) create mode 100644 clients/drcachesim/tests/record_filter_invalidate_cpu.templatex create mode 100644 clients/drcachesim/tools/filter/invalidate_cpu_filter.h diff --git a/api/docs/release.dox b/api/docs/release.dox index f43af9d3274..a097f7d08b9 100644 --- a/api/docs/release.dox +++ b/api/docs/release.dox @@ -265,6 +265,10 @@ Further non-compatibility-affecting changes include: - Added -trace_instr_intervals_file option to the drmemtrace trace analysis tools framework. The file must be in CSV format containing a tracing interval per line where start and duration are expressed in number of instructions. + - Added invalidate_cpu_filter_t to #dynamorio::drmemtrace::record_filter_t to invalidate + the value of markers #dynamorio::drmemtrace::TRACE_MARKER_TYPE_CPU_ID. When + -filter_invalidate_cpu is used, the value of those markers is set to (uintptr_t)-1, + which means "undefined". **************************************************
diff --git a/clients/drcachesim/CMakeLists.txt b/clients/drcachesim/CMakeLists.txt index 81e54281312..a0921fa31bd 100644 --- a/clients/drcachesim/CMakeLists.txt +++ b/clients/drcachesim/CMakeLists.txt @@ -201,6 +201,7 @@ add_exported_library(drmemtrace_record_filter STATIC tools/filter/type_filter.h tools/filter/encodings2regdeps_filter.h tools/filter/func_id_filter.h + tools/filter/invalidate_cpu_filter.h tools/filter/null_filter.h) target_link_libraries(drmemtrace_record_filter drmemtrace_simulator drmemtrace_schedule_file) diff --git a/clients/drcachesim/analyzer_multi.cpp b/clients/drcachesim/analyzer_multi.cpp index c27f231831c..628e832369c 100644 --- a/clients/drcachesim/analyzer_multi.cpp +++ b/clients/drcachesim/analyzer_multi.cpp @@ -339,7 +339,8 @@ record_analyzer_multi_t::create_analysis_tool_from_options(const std::string &to op_filter_cache_size.get_value(), op_filter_trace_types.get_value(), op_filter_marker_types.get_value(), op_trim_before_timestamp.get_value(), op_trim_after_timestamp.get_value(), op_encodings2regdeps.get_value(), - op_filter_func_ids.get_value(), op_verbose.get_value()); + op_filter_func_ids.get_value(), op_invalidate_cpu.get_value(), + op_verbose.get_value()); } ERRMSG("Usage error: unsupported record analyzer type \"%s\". Only " RECORD_FILTER " is supported.\n", diff --git a/clients/drcachesim/common/options.cpp b/clients/drcachesim/common/options.cpp index 5166a8bcd63..f36c603820d 100644 --- a/clients/drcachesim/common/options.cpp +++ b/clients/drcachesim/common/options.cpp @@ -1112,6 +1112,11 @@ droption_t "for the listed function IDs and removes those belonging to " "unlisted function IDs."); +droption_t op_invalidate_cpu( + DROPTION_SCOPE_FRONTEND, "filter_invalidate_cpu", false, + "Invalidate TRACE_MARKER_TYPE_CPU_ID", + "Invalidate TRACE_MARKER_TYPE_CPU_ID by setting its value to (uintptr_t)-1."); + droption_t op_trim_before_timestamp( DROPTION_SCOPE_ALL, "trim_before_timestamp", 0, 0, (std::numeric_limits::max)(), diff --git a/clients/drcachesim/common/options.h b/clients/drcachesim/common/options.h index ef24824b4d0..761c8748fd9 100644 --- a/clients/drcachesim/common/options.h +++ b/clients/drcachesim/common/options.h @@ -230,6 +230,7 @@ extern dynamorio::droption::droption_t op_filter_trace_types; extern dynamorio::droption::droption_t op_filter_marker_types; extern dynamorio::droption::droption_t op_encodings2regdeps; extern dynamorio::droption::droption_t op_filter_func_ids; +extern dynamorio::droption::droption_t op_invalidate_cpu; extern dynamorio::droption::droption_t op_trim_before_timestamp; extern dynamorio::droption::droption_t op_trim_after_timestamp; extern dynamorio::droption::droption_t op_abort_on_invariant_error; diff --git a/clients/drcachesim/tests/record_filter_invalidate_cpu.templatex b/clients/drcachesim/tests/record_filter_invalidate_cpu.templatex new file mode 100644 index 00000000000..31eaf5eb655 --- /dev/null +++ b/clients/drcachesim/tests/record_filter_invalidate_cpu.templatex @@ -0,0 +1,24 @@ +Estimation of pi is 3.142425985001098 + +Trace invariant checks passed + +Output .* entries from .* entries. + +Output format: + +<--record#-> <--instr#->: <---tid---> + +------------------------------------------------------------ + + 1 0: +[0-9]+ + 2 0: +[0-9]+ + 3 0: +[0-9]+ + 4 0: +[0-9]+ + 5 0: +[0-9]+ + 6 0: +[0-9]+ +#ifdef X64 + 7 0: +[0-9]+ +#else + 7 0: +[0-9]+ +#endif +.* diff --git a/clients/drcachesim/tests/record_filter_unit_tests.cpp b/clients/drcachesim/tests/record_filter_unit_tests.cpp index fd6d6ccf0cf..f915477c4c6 100644 --- a/clients/drcachesim/tests/record_filter_unit_tests.cpp +++ b/clients/drcachesim/tests/record_filter_unit_tests.cpp @@ -44,6 +44,7 @@ #include "tools/filter/type_filter.h" #include "tools/filter/encodings2regdeps_filter.h" #include "tools/filter/func_id_filter.h" +#include "tools/filter/invalidate_cpu_filter.h" #include "trace_entry.h" #include "zipfile_ostream.h" @@ -600,6 +601,70 @@ test_func_id_filter() return true; } +static bool +test_invalidate_cpu_filter() +{ + constexpr addr_t PC = 0x7f6fdd3ec360; + constexpr addr_t ENCODING = 0xe78948; + std::vector entries = { + /* Trace shard header. + */ + { { TRACE_TYPE_HEADER, 0, { 0x1 } }, true, { true } }, + { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_VERSION, { 0x2 } }, true, { true } }, + { { TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_FILETYPE, + { OFFLINE_FILE_TYPE_ARCH_X86_64 | OFFLINE_FILE_TYPE_ENCODINGS | + OFFLINE_FILE_TYPE_SYSCALL_NUMBERS | OFFLINE_FILE_TYPE_BLOCKING_SYSCALLS } }, + true, + { true } }, + { { TRACE_TYPE_THREAD, 0, { 0x4 } }, true, { true } }, + { { TRACE_TYPE_PID, 0, { 0x5 } }, true, { true } }, + { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CACHE_LINE_SIZE, { 0x6 } }, + true, + { true } }, + + { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP, { 0x7 } }, true, { true } }, + { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CPU_ID, { 0x8 } }, true, { false } }, + // invalidate_cpu_filter overwrites the value of TRACE_MARKER_TYPE_CPU_ID with + // (uintptr_t)-1. + { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CPU_ID, { 0xffffffffffffffff } }, + false, + { true } }, + /* We need at least one instruction with encodings to make record_filter output + * the trace. + */ + { { TRACE_TYPE_ENCODING, 3, { ENCODING } }, true, { true } }, + { { TRACE_TYPE_INSTR, 3, { PC } }, true, { true } }, + + { { TRACE_TYPE_FOOTER, 0, { 0x0 } }, true, { true } }, + }; + + /* Construct invalidate_cpu_filter_t. + */ + std::vector> filters; + auto invalidate_cpu_filter = std::unique_ptr( + new dynamorio::drmemtrace::invalidate_cpu_filter_t()); + if (!invalidate_cpu_filter->get_error_string().empty()) { + fprintf(stderr, "Couldn't construct an invalidate_cpu_filter %s", + invalidate_cpu_filter->get_error_string().c_str()); + return false; + } + filters.push_back(std::move(invalidate_cpu_filter)); + + /* Construct record_filter_t. + */ + auto record_filter = std::unique_ptr( + new test_record_filter_t(std::move(filters), 0, /*write_archive=*/true)); + + /* Run the test. + */ + if (!process_entries_and_check_result(record_filter.get(), entries, 0)) + return false; + + fprintf(stderr, "test_invalidate_cpu_filter passed\n"); + return true; +} + static bool test_cache_and_type_filter() { @@ -1450,7 +1515,7 @@ test_main(int argc, const char *argv[]) dr_standalone_init(); if (!test_cache_and_type_filter() || !test_chunk_update() || !test_trim_filter() || !test_null_filter() || !test_wait_filter() || !test_encodings2regdeps_filter() || - !test_func_id_filter()) + !test_func_id_filter() || !test_invalidate_cpu_filter()) return 1; fprintf(stderr, "All done!\n"); dr_standalone_exit(); diff --git a/clients/drcachesim/tools/filter/invalidate_cpu_filter.h b/clients/drcachesim/tools/filter/invalidate_cpu_filter.h new file mode 100644 index 00000000000..a06934aece3 --- /dev/null +++ b/clients/drcachesim/tools/filter/invalidate_cpu_filter.h @@ -0,0 +1,92 @@ +/* ********************************************************** + * Copyright (c) 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 VMWARE, 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. + */ + +#ifndef _INVALIDATE_CPU_FILTER_H_ +#define _INVALIDATE_CPU_FILTER_H_ 1 + +#include "record_filter.h" +#include "trace_entry.h" + +#include + +#define INVALID_CPU_MARKER_VALUE ~((addr_t)0UL) + +namespace dynamorio { +namespace drmemtrace { + +/* This filter invalidates the value of TRACE_MARKER_TYPE_CPU_ID by setting its value to + * (uintptr_t)-1, which indicates that the CPU could not be determined. + */ +class invalidate_cpu_filter_t : public record_filter_t::record_filter_func_t { +public: + invalidate_cpu_filter_t() + { + } + + void * + parallel_shard_init(memtrace_stream_t *shard_stream, + bool partial_trace_filter) override + { + return nullptr; + } + + bool + parallel_shard_filter( + trace_entry_t &entry, void *shard_data, + record_filter_t::record_filter_info_t &record_filter_info) override + { + trace_type_t entry_type = static_cast(entry.type); + // Output any trace_entry_t that it's not a marker. + if (entry_type != TRACE_TYPE_MARKER) + return true; + + trace_marker_type_t marker_type = static_cast(entry.size); + // Output any trace_entry_t that it's not a CPU marker. + if (marker_type != TRACE_MARKER_TYPE_CPU_ID) + return true; + + // Invalidate CPU marker value. + entry.addr = INVALID_CPU_MARKER_VALUE; + + return true; + } + + bool + parallel_shard_exit(void *shard_data) override + { + return true; + } +}; + +} // namespace drmemtrace +} // namespace dynamorio +#endif /* _INCALIDATE_CPU_FILTER_H_ */ diff --git a/clients/drcachesim/tools/filter/record_filter.cpp b/clients/drcachesim/tools/filter/record_filter.cpp index 9d038583b61..8ce3ffab496 100644 --- a/clients/drcachesim/tools/filter/record_filter.cpp +++ b/clients/drcachesim/tools/filter/record_filter.cpp @@ -62,6 +62,7 @@ #include "type_filter.h" #include "encodings2regdeps_filter.h" #include "func_id_filter.h" +#include "invalidate_cpu_filter.h" #undef VPRINT #ifdef DEBUG @@ -119,7 +120,7 @@ record_filter_tool_create(const std::string &output_dir, uint64_t stop_timestamp const std::string &remove_marker_types, uint64_t trim_before_timestamp, uint64_t trim_after_timestamp, bool encodings2regdeps, const std::string &keep_func_ids, - unsigned int verbose) + bool invalidate_cpu, unsigned int verbose) { std::vector< std::unique_ptr> @@ -160,6 +161,11 @@ record_filter_tool_create(const std::string &output_dir, uint64_t stop_timestamp std::unique_ptr( new dynamorio::drmemtrace::func_id_filter_t(keep_func_ids_list))); } + if (invalidate_cpu) { + filter_funcs.emplace_back( + std::unique_ptr( + new dynamorio::drmemtrace::invalidate_cpu_filter_t())); + } // TODO i#5675: Add other filters. diff --git a/clients/drcachesim/tools/filter/record_filter_create.h b/clients/drcachesim/tools/filter/record_filter_create.h index 0516d7bb713..10edf952361 100644 --- a/clients/drcachesim/tools/filter/record_filter_create.h +++ b/clients/drcachesim/tools/filter/record_filter_create.h @@ -67,6 +67,8 @@ namespace drmemtrace { * @param[in] keep_func_ids A comma-separated list of integers representing the * function IDs related to #TRACE_MARKER_TYPE_FUNC_ID (and _ARG, _RETVAL, _RETADDR) * markers to preserve in the trace, while removing all other function markers. + * @param[in] invalidate_cpu Set value of TRACE_MARKER_TYPE_CPU_ID to (uintptr_t)-1, + * which represents CPU unknown. * @param[in] verbose Verbosity level for notifications. */ record_analysis_tool_t * @@ -75,7 +77,7 @@ record_filter_tool_create(const std::string &output_dir, uint64_t stop_timestamp const std::string &remove_marker_types, uint64_t trim_before_timestamp, uint64_t trim_after_timestamp, bool encodings2regdeps, const std::string &keep_func_ids, - unsigned int verbose); + bool invalidate_cpu, unsigned int verbose); } // namespace drmemtrace } // namespace dynamorio diff --git a/clients/drcachesim/tools/record_filter_launcher.cpp b/clients/drcachesim/tools/record_filter_launcher.cpp index e1a8bb7a481..2118ae27f06 100644 --- a/clients/drcachesim/tools/record_filter_launcher.cpp +++ b/clients/drcachesim/tools/record_filter_launcher.cpp @@ -138,6 +138,12 @@ droption_t "TRACE_MARKER_TYPE_FUNC_[ID | ARG | RETVAL | RETADDR] " "markers for the listed function IDs and removed those " "belonging to unlisted function IDs."); + +droption_t op_invalidate_cpu( + DROPTION_SCOPE_FRONTEND, "filter_invalidate_cpu", false, + "Invalidate TRACE_MARKER_TYPE_CPU_ID", + "Invalidate TRACE_MARKER_TYPE_CPU_ID by setting its value to (uintptr_t)-1."); + } // namespace int @@ -168,7 +174,8 @@ _tmain(int argc, const TCHAR *targv[]) op_cache_filter_size.get_value(), op_remove_trace_types.get_value(), op_remove_marker_types.get_value(), op_trim_before_timestamp.get_value(), op_trim_after_timestamp.get_value(), op_encodings2regdeps.get_value(), - op_filter_func_ids.get_value(), op_verbose.get_value())); + op_filter_func_ids.get_value(), op_invalidate_cpu.get_value(), + op_verbose.get_value())); std::vector tools; tools.push_back(record_filter.get()); diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 577fe842833..5c9ed83743b 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4790,6 +4790,14 @@ if (BUILD_CLIENTS) # We run basic_counts on the filtered trace to check that there are no function # related markers left. "basic_counts") + + set(testname "tool.record_filter_invalidate_cpu") + torun_record_filter("${testname}" ${kernel_xfer_app} + "record_filter_invalidate_cpu" + "${drcachesim_path}@-simulator_type@record_filter@-filter_invalidate_cpu@-indir@${testname}.${kernel_xfer_app}.*.dir/trace@-outdir@${testname}.filtered.dir" + # We run basic_counts on the filtered trace to check that there are no function + # related markers left. + "view") endif () if (X86 AND X64 AND UNIX AND NOT APPLE) From acdec3dac1cb7c86140e2dd78c3b18161ad1dbca Mon Sep 17 00:00:00 2001 From: edeiana Date: Wed, 9 Oct 2024 04:08:27 -0700 Subject: [PATCH 02/13] Improved test comment. --- suite/tests/CMakeLists.txt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 5c9ed83743b..991655b77ab 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4791,12 +4791,14 @@ if (BUILD_CLIENTS) # related markers left. "basic_counts") + # Run invalidate_cpu_filter to overwrite the values of TRACE_MARKER_TYPE_CPU_ID + # markers with (uintptr_t)-1, which represents CPU unknown. set(testname "tool.record_filter_invalidate_cpu") torun_record_filter("${testname}" ${kernel_xfer_app} "record_filter_invalidate_cpu" "${drcachesim_path}@-simulator_type@record_filter@-filter_invalidate_cpu@-indir@${testname}.${kernel_xfer_app}.*.dir/trace@-outdir@${testname}.filtered.dir" - # We run basic_counts on the filtered trace to check that there are no function - # related markers left. + # We run view on the filtered trace to check that the value of the first + # TRACE_MARKER_TYPE_CPU_ID is (uintptr_t)-1. "view") endif () From 4aac7ec041e7490ad18c7f450313ffff51d71dc1 Mon Sep 17 00:00:00 2001 From: edeiana Date: Wed, 9 Oct 2024 04:27:25 -0700 Subject: [PATCH 03/13] Added missing '.' to brief option description. --- clients/drcachesim/common/options.cpp | 2 +- clients/drcachesim/tools/record_filter_launcher.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clients/drcachesim/common/options.cpp b/clients/drcachesim/common/options.cpp index f36c603820d..8f96e4229ba 100644 --- a/clients/drcachesim/common/options.cpp +++ b/clients/drcachesim/common/options.cpp @@ -1114,7 +1114,7 @@ droption_t droption_t op_invalidate_cpu( DROPTION_SCOPE_FRONTEND, "filter_invalidate_cpu", false, - "Invalidate TRACE_MARKER_TYPE_CPU_ID", + "Invalidate TRACE_MARKER_TYPE_CPU_ID.", "Invalidate TRACE_MARKER_TYPE_CPU_ID by setting its value to (uintptr_t)-1."); droption_t op_trim_before_timestamp( diff --git a/clients/drcachesim/tools/record_filter_launcher.cpp b/clients/drcachesim/tools/record_filter_launcher.cpp index 2118ae27f06..8cf83e93011 100644 --- a/clients/drcachesim/tools/record_filter_launcher.cpp +++ b/clients/drcachesim/tools/record_filter_launcher.cpp @@ -141,7 +141,7 @@ droption_t droption_t op_invalidate_cpu( DROPTION_SCOPE_FRONTEND, "filter_invalidate_cpu", false, - "Invalidate TRACE_MARKER_TYPE_CPU_ID", + "Invalidate TRACE_MARKER_TYPE_CPU_ID.", "Invalidate TRACE_MARKER_TYPE_CPU_ID by setting its value to (uintptr_t)-1."); } // namespace From 679b8ab9d4ae48db170bc5948ee251b96e0cec7e Mon Sep 17 00:00:00 2001 From: edeiana Date: Wed, 9 Oct 2024 04:28:40 -0700 Subject: [PATCH 04/13] Improved unit test. --- clients/drcachesim/tests/record_filter_unit_tests.cpp | 2 +- clients/drcachesim/tools/filter/invalidate_cpu_filter.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clients/drcachesim/tests/record_filter_unit_tests.cpp b/clients/drcachesim/tests/record_filter_unit_tests.cpp index f915477c4c6..4b9cd55add7 100644 --- a/clients/drcachesim/tests/record_filter_unit_tests.cpp +++ b/clients/drcachesim/tests/record_filter_unit_tests.cpp @@ -627,7 +627,7 @@ test_invalidate_cpu_filter() { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CPU_ID, { 0x8 } }, true, { false } }, // invalidate_cpu_filter overwrites the value of TRACE_MARKER_TYPE_CPU_ID with // (uintptr_t)-1. - { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CPU_ID, { 0xffffffffffffffff } }, + { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CPU_ID, { INVALID_CPU_MARKER_VALUE } }, false, { true } }, /* We need at least one instruction with encodings to make record_filter output diff --git a/clients/drcachesim/tools/filter/invalidate_cpu_filter.h b/clients/drcachesim/tools/filter/invalidate_cpu_filter.h index a06934aece3..6f721cb5430 100644 --- a/clients/drcachesim/tools/filter/invalidate_cpu_filter.h +++ b/clients/drcachesim/tools/filter/invalidate_cpu_filter.h @@ -38,7 +38,7 @@ #include -#define INVALID_CPU_MARKER_VALUE ~((addr_t)0UL) +#define INVALID_CPU_MARKER_VALUE ((uintptr_t) - 1) namespace dynamorio { namespace drmemtrace { From e84abca9d27558cc26a698b362c8964b5a83d4af Mon Sep 17 00:00:00 2001 From: edeiana Date: Wed, 9 Oct 2024 04:32:04 -0700 Subject: [PATCH 05/13] clang-format. --- clients/drcachesim/tools/filter/invalidate_cpu_filter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/drcachesim/tools/filter/invalidate_cpu_filter.h b/clients/drcachesim/tools/filter/invalidate_cpu_filter.h index 6f721cb5430..8ca1e64735a 100644 --- a/clients/drcachesim/tools/filter/invalidate_cpu_filter.h +++ b/clients/drcachesim/tools/filter/invalidate_cpu_filter.h @@ -38,7 +38,7 @@ #include -#define INVALID_CPU_MARKER_VALUE ((uintptr_t) - 1) +#define INVALID_CPU_MARKER_VALUE (uintptr_t)-1 namespace dynamorio { namespace drmemtrace { From 8cd2ec0ae342e4068020ba459f5981866b70a7b1 Mon Sep 17 00:00:00 2001 From: edeiana Date: Wed, 9 Oct 2024 04:34:31 -0700 Subject: [PATCH 06/13] clang-format. --- clients/drcachesim/tools/filter/invalidate_cpu_filter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/drcachesim/tools/filter/invalidate_cpu_filter.h b/clients/drcachesim/tools/filter/invalidate_cpu_filter.h index 8ca1e64735a..e0d3ea31898 100644 --- a/clients/drcachesim/tools/filter/invalidate_cpu_filter.h +++ b/clients/drcachesim/tools/filter/invalidate_cpu_filter.h @@ -38,7 +38,7 @@ #include -#define INVALID_CPU_MARKER_VALUE (uintptr_t)-1 +#define INVALID_CPU_MARKER_VALUE (uintptr_t) - 1 namespace dynamorio { namespace drmemtrace { From 1c1694f110b280759ae3134c5f6425fd6d219fe6 Mon Sep 17 00:00:00 2001 From: edeiana Date: Tue, 15 Oct 2024 04:47:41 -0700 Subject: [PATCH 07/13] Addressed PR feedback. --- api/docs/release.dox | 8 +-- clients/drcachesim/CMakeLists.txt | 2 +- clients/drcachesim/analyzer_multi.cpp | 2 +- clients/drcachesim/common/options.cpp | 12 ++-- clients/drcachesim/common/options.h | 2 +- clients/drcachesim/common/trace_entry.h | 3 + ...cord_filter_modify_marker_value.templatex} | 8 +-- .../tests/record_filter_unit_tests.cpp | 64 +++++++++++-------- ..._filter.h => modify_marker_value_filter.h} | 45 +++++++++---- .../drcachesim/tools/filter/record_filter.cpp | 11 ++-- .../tools/filter/record_filter_create.h | 7 +- .../tools/record_filter_launcher.cpp | 14 ++-- clients/drcachesim/tools/view.cpp | 9 ++- suite/tests/CMakeLists.txt | 14 ++-- 14 files changed, 124 insertions(+), 77 deletions(-) rename clients/drcachesim/tests/{record_filter_invalidate_cpu.templatex => record_filter_modify_marker_value.templatex} (70%) rename clients/drcachesim/tools/filter/{invalidate_cpu_filter.h => modify_marker_value_filter.h} (60%) diff --git a/api/docs/release.dox b/api/docs/release.dox index a097f7d08b9..e4b8ee27826 100644 --- a/api/docs/release.dox +++ b/api/docs/release.dox @@ -265,10 +265,10 @@ Further non-compatibility-affecting changes include: - Added -trace_instr_intervals_file option to the drmemtrace trace analysis tools framework. The file must be in CSV format containing a tracing interval per line where start and duration are expressed in number of instructions. - - Added invalidate_cpu_filter_t to #dynamorio::drmemtrace::record_filter_t to invalidate - the value of markers #dynamorio::drmemtrace::TRACE_MARKER_TYPE_CPU_ID. When - -filter_invalidate_cpu is used, the value of those markers is set to (uintptr_t)-1, - which means "undefined". + - Added modify_marker_value_filter_t to #dynamorio::drmemtrace::record_filter_t to modify + the value of TRACE_MARKER_TYPE_ markers. This filter takes a list of + and changes every listed markers in the trace to its + corresponding new_value. **************************************************
diff --git a/clients/drcachesim/CMakeLists.txt b/clients/drcachesim/CMakeLists.txt index a0921fa31bd..4bd4a3f76e5 100644 --- a/clients/drcachesim/CMakeLists.txt +++ b/clients/drcachesim/CMakeLists.txt @@ -201,7 +201,7 @@ add_exported_library(drmemtrace_record_filter STATIC tools/filter/type_filter.h tools/filter/encodings2regdeps_filter.h tools/filter/func_id_filter.h - tools/filter/invalidate_cpu_filter.h + tools/filter/modify_marker_value_filter.h tools/filter/null_filter.h) target_link_libraries(drmemtrace_record_filter drmemtrace_simulator drmemtrace_schedule_file) diff --git a/clients/drcachesim/analyzer_multi.cpp b/clients/drcachesim/analyzer_multi.cpp index 628e832369c..47bc5ff0c58 100644 --- a/clients/drcachesim/analyzer_multi.cpp +++ b/clients/drcachesim/analyzer_multi.cpp @@ -339,7 +339,7 @@ record_analyzer_multi_t::create_analysis_tool_from_options(const std::string &to op_filter_cache_size.get_value(), op_filter_trace_types.get_value(), op_filter_marker_types.get_value(), op_trim_before_timestamp.get_value(), op_trim_after_timestamp.get_value(), op_encodings2regdeps.get_value(), - op_filter_func_ids.get_value(), op_invalidate_cpu.get_value(), + op_filter_func_ids.get_value(), op_modify_marker_value.get_value(), op_verbose.get_value()); } ERRMSG("Usage error: unsupported record analyzer type \"%s\". Only " RECORD_FILTER diff --git a/clients/drcachesim/common/options.cpp b/clients/drcachesim/common/options.cpp index 8f96e4229ba..8e98e08ed9d 100644 --- a/clients/drcachesim/common/options.cpp +++ b/clients/drcachesim/common/options.cpp @@ -1112,10 +1112,14 @@ droption_t "for the listed function IDs and removes those belonging to " "unlisted function IDs."); -droption_t op_invalidate_cpu( - DROPTION_SCOPE_FRONTEND, "filter_invalidate_cpu", false, - "Invalidate TRACE_MARKER_TYPE_CPU_ID.", - "Invalidate TRACE_MARKER_TYPE_CPU_ID by setting its value to (uintptr_t)-1."); +droption_t op_modify_marker_value( + DROPTION_SCOPE_FRONTEND, "filter_modify_marker_value", "", + "Comma-separated pairs of integers representing .", + "This option is for -tool " RECORD_FILTER ". It modifies the value of all listed " + "TRACE_MARKER_TYPE_ markers in the trace with their corresponding new_value. " + "The list must have an even size. Example: -filter_modify_marker_value 3,24,18,2048 " + "sets all TRACE_MARKER_TYPE_CPU_ID == 3 in the trace to core 24 and " + "TRACE_MARKER_TYPE_PAGE_SIZE == 18 to 2k."); droption_t op_trim_before_timestamp( DROPTION_SCOPE_ALL, "trim_before_timestamp", 0, 0, diff --git a/clients/drcachesim/common/options.h b/clients/drcachesim/common/options.h index 761c8748fd9..9e64dd920b3 100644 --- a/clients/drcachesim/common/options.h +++ b/clients/drcachesim/common/options.h @@ -230,7 +230,7 @@ extern dynamorio::droption::droption_t op_filter_trace_types; extern dynamorio::droption::droption_t op_filter_marker_types; extern dynamorio::droption::droption_t op_encodings2regdeps; extern dynamorio::droption::droption_t op_filter_func_ids; -extern dynamorio::droption::droption_t op_invalidate_cpu; +extern dynamorio::droption::droption_t op_modify_marker_value; extern dynamorio::droption::droption_t op_trim_before_timestamp; extern dynamorio::droption::droption_t op_trim_after_timestamp; extern dynamorio::droption::droption_t op_abort_on_invariant_error; diff --git a/clients/drcachesim/common/trace_entry.h b/clients/drcachesim/common/trace_entry.h index 1b97224485a..3273ee0038a 100644 --- a/clients/drcachesim/common/trace_entry.h +++ b/clients/drcachesim/common/trace_entry.h @@ -699,6 +699,9 @@ typedef enum { // Values below here are available for users to use for custom markers. } trace_marker_type_t; +// As documented in TRACE_MARKER_TYPE_CPU_ID, this value indicates an unknown CPU. +#define INVALID_CPU_MARKER_VALUE ((uintptr_t) - 1) + /** Constants related to function or system call parameter tracing. */ enum class func_trace_t : uint64_t { // VS2019 won't infer 64-bit with "enum {". /** diff --git a/clients/drcachesim/tests/record_filter_invalidate_cpu.templatex b/clients/drcachesim/tests/record_filter_modify_marker_value.templatex similarity index 70% rename from clients/drcachesim/tests/record_filter_invalidate_cpu.templatex rename to clients/drcachesim/tests/record_filter_modify_marker_value.templatex index 31eaf5eb655..e64c4881cc2 100644 --- a/clients/drcachesim/tests/record_filter_invalidate_cpu.templatex +++ b/clients/drcachesim/tests/record_filter_modify_marker_value.templatex @@ -14,11 +14,7 @@ Output format: 2 0: +[0-9]+ 3 0: +[0-9]+ 4 0: +[0-9]+ - 5 0: +[0-9]+ + 5 0: +[0-9]+ 6 0: +[0-9]+ -#ifdef X64 - 7 0: +[0-9]+ -#else - 7 0: +[0-9]+ -#endif + 7 0: +[0-9]+ .* diff --git a/clients/drcachesim/tests/record_filter_unit_tests.cpp b/clients/drcachesim/tests/record_filter_unit_tests.cpp index 4b9cd55add7..7bdab2bc5ad 100644 --- a/clients/drcachesim/tests/record_filter_unit_tests.cpp +++ b/clients/drcachesim/tests/record_filter_unit_tests.cpp @@ -44,7 +44,7 @@ #include "tools/filter/type_filter.h" #include "tools/filter/encodings2regdeps_filter.h" #include "tools/filter/func_id_filter.h" -#include "tools/filter/invalidate_cpu_filter.h" +#include "tools/filter/modify_marker_value_filter.h" #include "trace_entry.h" #include "zipfile_ostream.h" @@ -602,13 +602,13 @@ test_func_id_filter() } static bool -test_invalidate_cpu_filter() +test_modify_marker_value_filter() { constexpr addr_t PC = 0x7f6fdd3ec360; constexpr addr_t ENCODING = 0xe78948; + constexpr uint64_t NEW_PAGE_SIZE_MARKER_VALUE = 0x800; // 2k pages. std::vector entries = { - /* Trace shard header. - */ + // Trace shard header. { { TRACE_TYPE_HEADER, 0, { 0x1 } }, true, { true } }, { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_VERSION, { 0x2 } }, true, { true } }, { { TRACE_TYPE_MARKER, @@ -622,46 +622,56 @@ test_invalidate_cpu_filter() { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CACHE_LINE_SIZE, { 0x6 } }, true, { true } }, - + { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_PAGE_SIZE, { 0x1000 } }, // 4k pages. + true, + { false } }, + // Overwrite the value of TRACE_MARKER_TYPE_PAGE_SIZE with 0x800 == 2048 == 2k + // page size. + { { TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_PAGE_SIZE, + { NEW_PAGE_SIZE_MARKER_VALUE } }, + false, + { true } }, { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP, { 0x7 } }, true, { true } }, { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CPU_ID, { 0x8 } }, true, { false } }, - // invalidate_cpu_filter overwrites the value of TRACE_MARKER_TYPE_CPU_ID with - // (uintptr_t)-1. + // Overwrite the value of TRACE_MARKER_TYPE_CPU_ID with ((uintptr_t)-1). { { TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CPU_ID, { INVALID_CPU_MARKER_VALUE } }, false, { true } }, - /* We need at least one instruction with encodings to make record_filter output - * the trace. - */ + // We need at least one instruction with encodings to make record_filter output + // the trace. { { TRACE_TYPE_ENCODING, 3, { ENCODING } }, true, { true } }, { { TRACE_TYPE_INSTR, 3, { PC } }, true, { true } }, { { TRACE_TYPE_FOOTER, 0, { 0x0 } }, true, { true } }, }; - /* Construct invalidate_cpu_filter_t. - */ + // Construct modify_marker_value_filter_t. We change TRACE_MARKER_TYPE_CPU_ID values + // with INVALID_CPU_MARKER_VALUE == ((uintptr_t)-1) and TRACE_MARKER_TYPE_PAGE_SIZE + // with 2k. + std::vector modify_marker_value_pairs_list = { TRACE_MARKER_TYPE_CPU_ID, + INVALID_CPU_MARKER_VALUE, + TRACE_MARKER_TYPE_PAGE_SIZE, + NEW_PAGE_SIZE_MARKER_VALUE }; std::vector> filters; - auto invalidate_cpu_filter = std::unique_ptr( - new dynamorio::drmemtrace::invalidate_cpu_filter_t()); - if (!invalidate_cpu_filter->get_error_string().empty()) { - fprintf(stderr, "Couldn't construct an invalidate_cpu_filter %s", - invalidate_cpu_filter->get_error_string().c_str()); + auto modify_marker_value_filter = std::unique_ptr( + new dynamorio::drmemtrace::modify_marker_value_filter_t( + modify_marker_value_pairs_list)); + if (!modify_marker_value_filter->get_error_string().empty()) { + fprintf(stderr, "Couldn't construct a modify_marker_value_filter %s", + modify_marker_value_filter->get_error_string().c_str()); return false; } - filters.push_back(std::move(invalidate_cpu_filter)); + filters.push_back(std::move(modify_marker_value_filter)); - /* Construct record_filter_t. - */ - auto record_filter = std::unique_ptr( - new test_record_filter_t(std::move(filters), 0, /*write_archive=*/true)); + // Construct record_filter_t. + test_record_filter_t record_filter(std::move(filters), 0, /*write_archive=*/true); - /* Run the test. - */ - if (!process_entries_and_check_result(record_filter.get(), entries, 0)) + // Run the test. + if (!process_entries_and_check_result(&record_filter, entries, 0)) return false; - fprintf(stderr, "test_invalidate_cpu_filter passed\n"); + fprintf(stderr, "test_modify_marker_value_filter passed\n"); return true; } @@ -1515,7 +1525,7 @@ test_main(int argc, const char *argv[]) dr_standalone_init(); if (!test_cache_and_type_filter() || !test_chunk_update() || !test_trim_filter() || !test_null_filter() || !test_wait_filter() || !test_encodings2regdeps_filter() || - !test_func_id_filter() || !test_invalidate_cpu_filter()) + !test_func_id_filter() || !test_modify_marker_value_filter()) return 1; fprintf(stderr, "All done!\n"); dr_standalone_exit(); diff --git a/clients/drcachesim/tools/filter/invalidate_cpu_filter.h b/clients/drcachesim/tools/filter/modify_marker_value_filter.h similarity index 60% rename from clients/drcachesim/tools/filter/invalidate_cpu_filter.h rename to clients/drcachesim/tools/filter/modify_marker_value_filter.h index e0d3ea31898..283b7353c75 100644 --- a/clients/drcachesim/tools/filter/invalidate_cpu_filter.h +++ b/clients/drcachesim/tools/filter/modify_marker_value_filter.h @@ -30,26 +30,40 @@ * DAMAGE. */ -#ifndef _INVALIDATE_CPU_FILTER_H_ -#define _INVALIDATE_CPU_FILTER_H_ 1 +#ifndef _MODIFY_MARKER_VALUE_FILTER_H_ +#define _MODIFY_MARKER_VALUE_FILTER_H_ 1 #include "record_filter.h" #include "trace_entry.h" #include - -#define INVALID_CPU_MARKER_VALUE (uintptr_t) - 1 +#include namespace dynamorio { namespace drmemtrace { -/* This filter invalidates the value of TRACE_MARKER_TYPE_CPU_ID by setting its value to - * (uintptr_t)-1, which indicates that the CPU could not be determined. +/* This filter takes a list of pairs and modifies the value + * of all listed markers in the trace with the given new_value. */ -class invalidate_cpu_filter_t : public record_filter_t::record_filter_func_t { +class modify_marker_value_filter_t : public record_filter_t::record_filter_func_t { public: - invalidate_cpu_filter_t() + modify_marker_value_filter_t(std::vector modify_marker_value_pairs_list) { + int list_size = modify_marker_value_pairs_list.size(); + if (list_size == 0) { + error_string_ = "List of pairs is empty."; + } else if (list_size % 2 != 0) { + error_string_ = "List of pairs is missing " + "part of a pair as its size is not even"; + } else { + for (int i = 0; i < list_size; i += 2) { + trace_marker_type_t marker_type = + static_cast(modify_marker_value_pairs_list[i]); + uint64_t new_value = modify_marker_value_pairs_list[i + 1]; + // We ignore duplicate pairs and use the last pair in the list. + marker_to_value_map_[marker_type] = new_value; + } + } } void * @@ -69,13 +83,15 @@ class invalidate_cpu_filter_t : public record_filter_t::record_filter_func_t { if (entry_type != TRACE_TYPE_MARKER) return true; + // Check if the TRACE_TYPE_MARKER_ is in the list of markers for which we want to + // overwrite their value. If not, output the marker unchanged. trace_marker_type_t marker_type = static_cast(entry.size); - // Output any trace_entry_t that it's not a CPU marker. - if (marker_type != TRACE_MARKER_TYPE_CPU_ID) + const auto &it = marker_to_value_map_.find(marker_type); + if (it == marker_to_value_map_.end()) return true; - // Invalidate CPU marker value. - entry.addr = INVALID_CPU_MARKER_VALUE; + // Overwrite marker value. + entry.addr = it->second; return true; } @@ -85,8 +101,11 @@ class invalidate_cpu_filter_t : public record_filter_t::record_filter_func_t { { return true; } + +private: + std::unordered_map marker_to_value_map_; }; } // namespace drmemtrace } // namespace dynamorio -#endif /* _INCALIDATE_CPU_FILTER_H_ */ +#endif /* _MODIFY_MARKER_VALUE_FILTER_H_ */ diff --git a/clients/drcachesim/tools/filter/record_filter.cpp b/clients/drcachesim/tools/filter/record_filter.cpp index 8ce3ffab496..35edfb5718b 100644 --- a/clients/drcachesim/tools/filter/record_filter.cpp +++ b/clients/drcachesim/tools/filter/record_filter.cpp @@ -62,7 +62,7 @@ #include "type_filter.h" #include "encodings2regdeps_filter.h" #include "func_id_filter.h" -#include "invalidate_cpu_filter.h" +#include "modify_marker_value_filter.h" #undef VPRINT #ifdef DEBUG @@ -120,7 +120,7 @@ record_filter_tool_create(const std::string &output_dir, uint64_t stop_timestamp const std::string &remove_marker_types, uint64_t trim_before_timestamp, uint64_t trim_after_timestamp, bool encodings2regdeps, const std::string &keep_func_ids, - bool invalidate_cpu, unsigned int verbose) + const std::string &modify_marker_value, unsigned int verbose) { std::vector< std::unique_ptr> @@ -161,10 +161,13 @@ record_filter_tool_create(const std::string &output_dir, uint64_t stop_timestamp std::unique_ptr( new dynamorio::drmemtrace::func_id_filter_t(keep_func_ids_list))); } - if (invalidate_cpu) { + if (!modify_marker_value.empty()) { + std::vector modify_marker_value_pairs_list = + parse_string(modify_marker_value); filter_funcs.emplace_back( std::unique_ptr( - new dynamorio::drmemtrace::invalidate_cpu_filter_t())); + new dynamorio::drmemtrace::modify_marker_value_filter_t( + modify_marker_value_pairs_list))); } // TODO i#5675: Add other filters. diff --git a/clients/drcachesim/tools/filter/record_filter_create.h b/clients/drcachesim/tools/filter/record_filter_create.h index 10edf952361..d48513a4255 100644 --- a/clients/drcachesim/tools/filter/record_filter_create.h +++ b/clients/drcachesim/tools/filter/record_filter_create.h @@ -67,8 +67,9 @@ namespace drmemtrace { * @param[in] keep_func_ids A comma-separated list of integers representing the * function IDs related to #TRACE_MARKER_TYPE_FUNC_ID (and _ARG, _RETVAL, _RETADDR) * markers to preserve in the trace, while removing all other function markers. - * @param[in] invalidate_cpu Set value of TRACE_MARKER_TYPE_CPU_ID to (uintptr_t)-1, - * which represents CPU unknown. + * @param[in] modify_marker_value A list of comma-separated pairs of integers representing + * to modify the value of all listed TRACE_MARKER_TYPE_ + * in the trace with their corresponding new_value. * @param[in] verbose Verbosity level for notifications. */ record_analysis_tool_t * @@ -77,7 +78,7 @@ record_filter_tool_create(const std::string &output_dir, uint64_t stop_timestamp const std::string &remove_marker_types, uint64_t trim_before_timestamp, uint64_t trim_after_timestamp, bool encodings2regdeps, const std::string &keep_func_ids, - bool invalidate_cpu, unsigned int verbose); + const std::string &modify_marker_value, unsigned int verbose); } // namespace drmemtrace } // namespace dynamorio diff --git a/clients/drcachesim/tools/record_filter_launcher.cpp b/clients/drcachesim/tools/record_filter_launcher.cpp index 8cf83e93011..bcc9f2c1098 100644 --- a/clients/drcachesim/tools/record_filter_launcher.cpp +++ b/clients/drcachesim/tools/record_filter_launcher.cpp @@ -139,10 +139,14 @@ droption_t "markers for the listed function IDs and removed those " "belonging to unlisted function IDs."); -droption_t op_invalidate_cpu( - DROPTION_SCOPE_FRONTEND, "filter_invalidate_cpu", false, - "Invalidate TRACE_MARKER_TYPE_CPU_ID.", - "Invalidate TRACE_MARKER_TYPE_CPU_ID by setting its value to (uintptr_t)-1."); +droption_t op_modify_marker_value( + DROPTION_SCOPE_FRONTEND, "filter_modify_marker_value", "", + "Comma-separated pairs of integers representing .", + "This option is for -tool record_filter. It modifies the value of all listed " + "TRACE_MARKER_TYPE_ markers in the trace with their corresponding new_value. " + "The list must have an even size. Example: -filter_modify_marker_value 3,24,18,2048 " + "sets all TRACE_MARKER_TYPE_CPU_ID == 3 in the trace to core 24 and " + "TRACE_MARKER_TYPE_PAGE_SIZE == 18 to 2k."); } // namespace @@ -174,7 +178,7 @@ _tmain(int argc, const TCHAR *targv[]) op_cache_filter_size.get_value(), op_remove_trace_types.get_value(), op_remove_marker_types.get_value(), op_trim_before_timestamp.get_value(), op_trim_after_timestamp.get_value(), op_encodings2regdeps.get_value(), - op_filter_func_ids.get_value(), op_invalidate_cpu.get_value(), + op_filter_func_ids.get_value(), op_modify_marker_value.get_value(), op_verbose.get_value())); std::vector tools; tools.push_back(record_filter.get()); diff --git a/clients/drcachesim/tools/view.cpp b/clients/drcachesim/tools/view.cpp index 7817169c89d..702667db73c 100644 --- a/clients/drcachesim/tools/view.cpp +++ b/clients/drcachesim/tools/view.cpp @@ -335,8 +335,13 @@ view_t::parallel_shard_memref(void *shard_data, const memref_t &memref) // see a cpuid marker on a thread switch. To avoid that assumption // we would want to track the prior tid and print out a thread switch // message whenever it changes. - std::cerr << "\n"; + if (memref.marker.marker_value == INVALID_CPU_MARKER_VALUE) { + std::cerr << "\n"; + } else { + std::cerr << "\n"; + } break; case TRACE_MARKER_TYPE_KERNEL_EVENT: if (trace_version_ <= TRACE_ENTRY_VERSION_NO_KERNEL_PC) { diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 991655b77ab..2dce8d8ab12 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4791,14 +4791,16 @@ if (BUILD_CLIENTS) # related markers left. "basic_counts") - # Run invalidate_cpu_filter to overwrite the values of TRACE_MARKER_TYPE_CPU_ID - # markers with (uintptr_t)-1, which represents CPU unknown. - set(testname "tool.record_filter_invalidate_cpu") + # Run modify_marker_value_filter to overwrite the values of + # TRACE_MARKER_TYPE_CPU_ID markers with (uintptr_t)-1, which represents an unknown + # CPU, and TRACE_MARKER_TYPE_PAGE_SIZE to 2k. + set(testname "tool.record_filter_modify_marker_value") torun_record_filter("${testname}" ${kernel_xfer_app} - "record_filter_invalidate_cpu" - "${drcachesim_path}@-simulator_type@record_filter@-filter_invalidate_cpu@-indir@${testname}.${kernel_xfer_app}.*.dir/trace@-outdir@${testname}.filtered.dir" + "record_filter_modify_marker_value" + "${drcachesim_path}@-simulator_type@record_filter@-filter_modify_marker_value@3,18446744073709551615,18,2048@-indir@${testname}.${kernel_xfer_app}.*.dir/trace@-outdir@${testname}.filtered.dir" # We run view on the filtered trace to check that the value of the first - # TRACE_MARKER_TYPE_CPU_ID is (uintptr_t)-1. + # TRACE_MARKER_TYPE_CPU_ID is (uintptr_t)-1 and the value of + # TRACE_MARKER_TYPE_PAGE_SIZE is 2k. "view") endif () From 2bc0b60bedebbc7d887dde316f93c5475e7e3d45 Mon Sep 17 00:00:00 2001 From: edeiana Date: Tue, 15 Oct 2024 05:04:49 -0700 Subject: [PATCH 08/13] Handling hex. --- clients/drcachesim/tools/filter/record_filter.cpp | 4 +++- suite/tests/CMakeLists.txt | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/clients/drcachesim/tools/filter/record_filter.cpp b/clients/drcachesim/tools/filter/record_filter.cpp index 35edfb5718b..4a52c8b0ef8 100644 --- a/clients/drcachesim/tools/filter/record_filter.cpp +++ b/clients/drcachesim/tools/filter/record_filter.cpp @@ -96,7 +96,9 @@ parse_string(const std::string &s, char sep = ',') std::vector vec; do { pos = s.find(sep, at); - unsigned long long parsed_number = std::stoull(s.substr(at, pos)); + // base = 0 allows to handle both decimal and hex numbers. + unsigned long long parsed_number = + std::stoull(s.substr(at, pos), nullptr, /*base = */ 0); // XXX: parsed_number may be truncated if T is not large enough. // We could check that parsed_number is within the limits of T using // std::numeric_limits<>::min()/max(), but this returns 0 on T that are enums, diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 2dce8d8ab12..8e0eb76dc3e 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4797,7 +4797,7 @@ if (BUILD_CLIENTS) set(testname "tool.record_filter_modify_marker_value") torun_record_filter("${testname}" ${kernel_xfer_app} "record_filter_modify_marker_value" - "${drcachesim_path}@-simulator_type@record_filter@-filter_modify_marker_value@3,18446744073709551615,18,2048@-indir@${testname}.${kernel_xfer_app}.*.dir/trace@-outdir@${testname}.filtered.dir" + "${drcachesim_path}@-simulator_type@record_filter@-filter_modify_marker_value@3,0xffffffffffffffff,18,2048@-indir@${testname}.${kernel_xfer_app}.*.dir/trace@-outdir@${testname}.filtered.dir" # We run view on the filtered trace to check that the value of the first # TRACE_MARKER_TYPE_CPU_ID is (uintptr_t)-1 and the value of # TRACE_MARKER_TYPE_PAGE_SIZE is 2k. From 7efcd31a4c5cf10249d44a208ca5b98783fc45a3 Mon Sep 17 00:00:00 2001 From: edeiana Date: Tue, 15 Oct 2024 05:11:53 -0700 Subject: [PATCH 09/13] static_cast rather than C cast (). --- clients/drcachesim/common/trace_entry.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/drcachesim/common/trace_entry.h b/clients/drcachesim/common/trace_entry.h index 3273ee0038a..903374ddd00 100644 --- a/clients/drcachesim/common/trace_entry.h +++ b/clients/drcachesim/common/trace_entry.h @@ -700,7 +700,7 @@ typedef enum { } trace_marker_type_t; // As documented in TRACE_MARKER_TYPE_CPU_ID, this value indicates an unknown CPU. -#define INVALID_CPU_MARKER_VALUE ((uintptr_t) - 1) +#define INVALID_CPU_MARKER_VALUE static_cast(-1) /** Constants related to function or system call parameter tracing. */ enum class func_trace_t : uint64_t { // VS2019 won't infer 64-bit with "enum {". From ac6f68efbd69dc354b79e62bf64b67641f820c5e Mon Sep 17 00:00:00 2001 From: edeiana Date: Tue, 15 Oct 2024 05:17:42 -0700 Subject: [PATCH 10/13] Warning as error fix, windows build. --- clients/drcachesim/tools/filter/modify_marker_value_filter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/drcachesim/tools/filter/modify_marker_value_filter.h b/clients/drcachesim/tools/filter/modify_marker_value_filter.h index 283b7353c75..1832466269f 100644 --- a/clients/drcachesim/tools/filter/modify_marker_value_filter.h +++ b/clients/drcachesim/tools/filter/modify_marker_value_filter.h @@ -49,7 +49,7 @@ class modify_marker_value_filter_t : public record_filter_t::record_filter_func_ public: modify_marker_value_filter_t(std::vector modify_marker_value_pairs_list) { - int list_size = modify_marker_value_pairs_list.size(); + size_t list_size = modify_marker_value_pairs_list.size(); if (list_size == 0) { error_string_ = "List of pairs is empty."; } else if (list_size % 2 != 0) { From f458930bb31b1386cde8e43a303f9e3eae99f496 Mon Sep 17 00:00:00 2001 From: edeiana Date: Tue, 15 Oct 2024 05:22:47 -0700 Subject: [PATCH 11/13] Fixed comparison int-uint error. --- clients/drcachesim/tools/filter/modify_marker_value_filter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/drcachesim/tools/filter/modify_marker_value_filter.h b/clients/drcachesim/tools/filter/modify_marker_value_filter.h index 1832466269f..58296ee8b77 100644 --- a/clients/drcachesim/tools/filter/modify_marker_value_filter.h +++ b/clients/drcachesim/tools/filter/modify_marker_value_filter.h @@ -56,7 +56,7 @@ class modify_marker_value_filter_t : public record_filter_t::record_filter_func_ error_string_ = "List of pairs is missing " "part of a pair as its size is not even"; } else { - for (int i = 0; i < list_size; i += 2) { + for (size_t i = 0; i < list_size; i += 2) { trace_marker_type_t marker_type = static_cast(modify_marker_value_pairs_list[i]); uint64_t new_value = modify_marker_value_pairs_list[i + 1]; From bbbd9cd4085cbc949675ea74b0c6b6db77e187db Mon Sep 17 00:00:00 2001 From: edeiana Date: Tue, 15 Oct 2024 05:29:27 -0700 Subject: [PATCH 12/13] Fixed warning as error (due to implicit casting), windows build. --- clients/drcachesim/tools/filter/modify_marker_value_filter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/drcachesim/tools/filter/modify_marker_value_filter.h b/clients/drcachesim/tools/filter/modify_marker_value_filter.h index 58296ee8b77..7f906db05d9 100644 --- a/clients/drcachesim/tools/filter/modify_marker_value_filter.h +++ b/clients/drcachesim/tools/filter/modify_marker_value_filter.h @@ -91,7 +91,7 @@ class modify_marker_value_filter_t : public record_filter_t::record_filter_func_ return true; // Overwrite marker value. - entry.addr = it->second; + entry.addr = static_cast(it->second); return true; } From f49b91b46fb9332c82a9d157cdcadfc0fb29a616 Mon Sep 17 00:00:00 2001 From: edeiana Date: Tue, 15 Oct 2024 17:45:47 -0700 Subject: [PATCH 13/13] Fixed grammar. --- api/docs/release.dox | 2 +- clients/drcachesim/tools/filter/modify_marker_value_filter.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/docs/release.dox b/api/docs/release.dox index e4b8ee27826..9e5a6d06697 100644 --- a/api/docs/release.dox +++ b/api/docs/release.dox @@ -267,7 +267,7 @@ Further non-compatibility-affecting changes include: interval per line where start and duration are expressed in number of instructions. - Added modify_marker_value_filter_t to #dynamorio::drmemtrace::record_filter_t to modify the value of TRACE_MARKER_TYPE_ markers. This filter takes a list of - and changes every listed markers in the trace to its + and changes every listed marker in the trace to its corresponding new_value. ************************************************** diff --git a/clients/drcachesim/tools/filter/modify_marker_value_filter.h b/clients/drcachesim/tools/filter/modify_marker_value_filter.h index 7f906db05d9..7833ba76826 100644 --- a/clients/drcachesim/tools/filter/modify_marker_value_filter.h +++ b/clients/drcachesim/tools/filter/modify_marker_value_filter.h @@ -79,7 +79,7 @@ class modify_marker_value_filter_t : public record_filter_t::record_filter_func_ record_filter_t::record_filter_info_t &record_filter_info) override { trace_type_t entry_type = static_cast(entry.type); - // Output any trace_entry_t that it's not a marker. + // Output any trace_entry_t that's not a marker. if (entry_type != TRACE_TYPE_MARKER) return true;