From 0546b56bab52a88c77656fa0afa7eedeabec76b7 Mon Sep 17 00:00:00 2001 From: edeiana Date: Wed, 18 Dec 2024 07:56:20 -0800 Subject: [PATCH 1/2] i#6662 public traces: add signal and uncompleted instr invariants Public traces should not have TRACE_MARKER_TYPE_SIGNAL_NUMBER and TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION markers. We add two invariants in invariant_checker to detect the presence of these markers in REGDEPS traces and raise an invariant error if necessary. Updated unit tests and end-to-end tests. Issue #6662 --- .../tests/invariant_checker_test.cpp | 42 +++++++++++++++++++ .../drcachesim/tools/invariant_checker.cpp | 10 +++++ suite/tests/CMakeLists.txt | 20 +++++---- 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/clients/drcachesim/tests/invariant_checker_test.cpp b/clients/drcachesim/tests/invariant_checker_test.cpp index 5b0b75624be..46a68944299 100644 --- a/clients/drcachesim/tests/invariant_checker_test.cpp +++ b/clients/drcachesim/tests/invariant_checker_test.cpp @@ -3581,6 +3581,48 @@ check_regdeps(void) { std::cerr << "Testing regdeps traces\n"; + // Incorrect: TRACE_MARKER_TYPE_SIGNAL_NUMBER not allowed. + { + std::vector memrefs = { + gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ARCH_REGDEPS), + gen_marker(TID_A, TRACE_MARKER_TYPE_CACHE_LINE_SIZE, 64), + gen_marker(TID_A, TRACE_MARKER_TYPE_PAGE_SIZE, 4096), + gen_marker(TID_A, TRACE_MARKER_TYPE_SIGNAL_NUMBER, 42), + gen_instr(TID_A), + gen_exit(TID_A), + }; + if (!run_checker( + memrefs, true, + { "OFFLINE_FILE_TYPE_ARCH_REGDEPS traces cannot have " + "TRACE_MARKER_TYPE_SIGNAL_NUMBER markers", + /*tid=*/TID_A, + /*ref_ordinal=*/4, /*last_timestamp=*/0, + /*instrs_since_last_timestamp=*/0 }, + "Failed to catch non-allowed TRACE_MARKER_TYPE_SIGNAL_NUMBER marker")) + return false; + } + + // Incorrect: TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION not allowed. + { + std::vector memrefs = { + gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ARCH_REGDEPS), + gen_marker(TID_A, TRACE_MARKER_TYPE_CACHE_LINE_SIZE, 64), + gen_marker(TID_A, TRACE_MARKER_TYPE_PAGE_SIZE, 4096), + gen_marker(TID_A, TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION, 42), + gen_instr(TID_A), + gen_exit(TID_A), + }; + if (!run_checker(memrefs, true, + { "OFFLINE_FILE_TYPE_ARCH_REGDEPS traces cannot have " + "TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION markers", + /*tid=*/TID_A, + /*ref_ordinal=*/4, /*last_timestamp=*/0, + /*instrs_since_last_timestamp=*/0 }, + "Failed to catch non-allowed " + "TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION marker")) + return false; + } + // Incorrect: OFFLINE_FILE_TYPE_ARCH_AARCH64 not allowed. { std::vector memrefs = { diff --git a/clients/drcachesim/tools/invariant_checker.cpp b/clients/drcachesim/tools/invariant_checker.cpp index 0c791129433..5df160bde60 100644 --- a/clients/drcachesim/tools/invariant_checker.cpp +++ b/clients/drcachesim/tools/invariant_checker.cpp @@ -1707,6 +1707,16 @@ invariant_checker_t::check_regdeps_invariants(per_shard_t *shard, const memref_t "was not built on a Linux platform.\n"; #endif } break; + case TRACE_MARKER_TYPE_SIGNAL_NUMBER: + report_if_false(shard, false, + "OFFLINE_FILE_TYPE_ARCH_REGDEPS traces cannot have " + "TRACE_MARKER_TYPE_SIGNAL_NUMBER markers"); + break; + case TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION: + report_if_false(shard, false, + "OFFLINE_FILE_TYPE_ARCH_REGDEPS traces cannot have " + "TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION markers"); + break; default: // All other markers are allowed. break; diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 3a509bb5adb..1c95ca65cbb 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4849,10 +4849,11 @@ if (BUILD_CLIENTS) # -filter_marker_types 19,25,27,28,30 (which correspond to # TRACE_MARKER_TYPE_SYSCALL_IDX, TRACE_MARKER_TYPE_SYSCALL, # TRACE_MARKER_TYPE_SYSCALL_TRACE_START, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, - # TRACE_MARKER_TYPE_SYSCALL_FAILED), and -filter_modify_marker_value 3,-1 - # (which changes the value of TRACE_MARKER_TYPE_CPU_ID == 3 to - # INVALID_CPU_MARKER_VALUE == (uintptr_t)-1). - "${drcachesim_path}@-simulator_type@record_filter@-filter_encodings2regdeps@-indir@${testname}.${ci_shared_app}.*.dir/trace@-outdir@${testname}.filtered.dir@-filter_marker_types@19,25,27,28,30@-filter_keep_func_ids@4294967498@-filter_modify_marker_value@3,-1" + # TRACE_MARKER_TYPE_SYSCALL_FAILED, TRACE_MARKER_TYPE_SIGNAL_NUMBER, + # TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION), and + # -filter_modify_marker_value 3,-1 (which changes the value of + # TRACE_MARKER_TYPE_CPU_ID == 3 to INVALID_CPU_MARKER_VALUE == (uintptr_t)-1). + "${drcachesim_path}@-simulator_type@record_filter@-filter_encodings2regdeps@-indir@${testname}.${ci_shared_app}.*.dir/trace@-outdir@${testname}.filtered.dir@-filter_marker_types@19,25,27,28,30,40,41@-filter_keep_func_ids@4294967498@-filter_modify_marker_value@3,-1" # We run the invariant_checker analyzer on the REGDEPS filtered trace. # We expect no invariant errors. "invariant_checker") @@ -4866,13 +4867,14 @@ if (BUILD_CLIENTS) # with -filter_encodings2regdeps to change instruction encodings, # -filter_keep_func_ids 4294967498 (which is SYS_futex, associated to the only # TRACE_MARKER_TYPE_FUNC_ markers we want to keep), - # -filter_marker_types 19,25,27,28,30 (which correspond to + # -filter_marker_types 19,25,27,28,30,40,41 (which correspond to # TRACE_MARKER_TYPE_SYSCALL_IDX, TRACE_MARKER_TYPE_SYSCALL, # TRACE_MARKER_TYPE_SYSCALL_TRACE_START, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, - # TRACE_MARKER_TYPE_SYSCALL_FAILED), and -filter_modify_marker_value 3,-1 - # (which changes the value of TRACE_MARKER_TYPE_CPU_ID == 3 to - # INVALID_CPU_MARKER_VALUE == (uintptr_t)-1). - "${drcachesim_path}@-simulator_type@record_filter@-filter_encodings2regdeps@-indir@${testname}.${kernel_xfer_app}.*.dir/trace@-outdir@${testname}.filtered.dir@-filter_marker_types@19,25,27,28,30@-filter_keep_func_ids@4294967498@-filter_modify_marker_value@3,-1" + # TRACE_MARKER_TYPE_SYSCALL_FAILED, TRACE_MARKER_TYPE_SIGNAL_NUMBER, + # TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION), and + # -filter_modify_marker_value 3,-1 (which changes the value of + # TRACE_MARKER_TYPE_CPU_ID == 3 to INVALID_CPU_MARKER_VALUE == (uintptr_t)-1). + "${drcachesim_path}@-simulator_type@record_filter@-filter_encodings2regdeps@-indir@${testname}.${kernel_xfer_app}.*.dir/trace@-outdir@${testname}.filtered.dir@-filter_marker_types@19,25,27,28,30,40,41@-filter_keep_func_ids@4294967498@-filter_modify_marker_value@3,-1" # We run the invariant_checker analyzer on the REGDEPS filtered trace. # We expect no invariant errors. "invariant_checker") From 7e02a78558958faa2007b44cb78c1d6fa048982d Mon Sep 17 00:00:00 2001 From: edeiana Date: Wed, 18 Dec 2024 09:58:25 -0800 Subject: [PATCH 2/2] Addressed PR feedback. --- .../tests/invariant_checker_test.cpp | 1 + .../drcachesim/tools/invariant_checker.cpp | 1 + suite/tests/CMakeLists.txt | 42 ++++++------------- 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/clients/drcachesim/tests/invariant_checker_test.cpp b/clients/drcachesim/tests/invariant_checker_test.cpp index 46a68944299..f5cfabbb7a0 100644 --- a/clients/drcachesim/tests/invariant_checker_test.cpp +++ b/clients/drcachesim/tests/invariant_checker_test.cpp @@ -3603,6 +3603,7 @@ check_regdeps(void) } // Incorrect: TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION not allowed. + // XXX i#7155: Allow these markers once we update their values in record_filter. { std::vector memrefs = { gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ARCH_REGDEPS), diff --git a/clients/drcachesim/tools/invariant_checker.cpp b/clients/drcachesim/tools/invariant_checker.cpp index 5df160bde60..5759a52498c 100644 --- a/clients/drcachesim/tools/invariant_checker.cpp +++ b/clients/drcachesim/tools/invariant_checker.cpp @@ -1712,6 +1712,7 @@ invariant_checker_t::check_regdeps_invariants(per_shard_t *shard, const memref_t "OFFLINE_FILE_TYPE_ARCH_REGDEPS traces cannot have " "TRACE_MARKER_TYPE_SIGNAL_NUMBER markers"); break; + // XXX i#7155: Allow these markers once we update their values in record_filter. case TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION: report_if_false(shard, false, "OFFLINE_FILE_TYPE_ARCH_REGDEPS traces cannot have " diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 1c95ca65cbb..6ba6d409f91 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4838,45 +4838,29 @@ if (BUILD_CLIENTS) if (X86 AND X64 AND UNIX AND NOT APPLE) # Run the invariant_checker on an OFFLINE_FILE_TYPE_ARCH_REGDEPS trace of - # ci_shared_app. + # ci_shared_app and kernel_xfer_app. We expect no invariant errors. + # Generate an OFFLINE_FILE_TYPE_ARCH_REGDEPS trace by running record_filter + # with: 1) -filter_encodings2regdeps to change instruction encodings; 2) + # -filter_keep_func_ids 4294967498 (which is SYS_futex, associated to the only + # TRACE_MARKER_TYPE_FUNC_* markers we want to keep), + # -filter_marker_types 19,25,27,28,30,40,41 (which correspond to + # TRACE_MARKER_TYPE_SYSCALL_IDX, TRACE_MARKER_TYPE_SYSCALL, + # TRACE_MARKER_TYPE_SYSCALL_TRACE_START, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, + # TRACE_MARKER_TYPE_SYSCALL_FAILED, TRACE_MARKER_TYPE_SIGNAL_NUMBER, + # TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION), note that the removal of + # TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION is only temporary (xref i#7155); + # 3) -filter_modify_marker_value 3,-1 (which changes the value of + # TRACE_MARKER_TYPE_CPU_ID == 3 to INVALID_CPU_MARKER_VALUE == (uintptr_t)-1). set(testname "tool.invariant_checker_on_regdeps_trace_ci_shared_app") torun_record_filter("${testname}" ${ci_shared_app} "invariant_checker_on_regdeps_trace_ci_shared_app" - # Generate an OFFLINE_FILE_TYPE_ARCH_REGDEPS trace by running record_filter - # with -filter_encodings2regdeps to change instruction encodings, - # -filter_keep_func_ids 4294967498 (which is SYS_futex, associated to the only - # TRACE_MARKER_TYPE_FUNC_ markers we want to keep), - # -filter_marker_types 19,25,27,28,30 (which correspond to - # TRACE_MARKER_TYPE_SYSCALL_IDX, TRACE_MARKER_TYPE_SYSCALL, - # TRACE_MARKER_TYPE_SYSCALL_TRACE_START, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, - # TRACE_MARKER_TYPE_SYSCALL_FAILED, TRACE_MARKER_TYPE_SIGNAL_NUMBER, - # TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION), and - # -filter_modify_marker_value 3,-1 (which changes the value of - # TRACE_MARKER_TYPE_CPU_ID == 3 to INVALID_CPU_MARKER_VALUE == (uintptr_t)-1). "${drcachesim_path}@-simulator_type@record_filter@-filter_encodings2regdeps@-indir@${testname}.${ci_shared_app}.*.dir/trace@-outdir@${testname}.filtered.dir@-filter_marker_types@19,25,27,28,30,40,41@-filter_keep_func_ids@4294967498@-filter_modify_marker_value@3,-1" - # We run the invariant_checker analyzer on the REGDEPS filtered trace. - # We expect no invariant errors. "invariant_checker") - # Run the invariant_checker on an OFFLINE_FILE_TYPE_ARCH_REGDEPS trace of - # kernel_xfer_app. set(testname "tool.invariant_checker_on_regdeps_trace_kernel_xfer_app") torun_record_filter("${testname}" ${kernel_xfer_app} "invariant_checker_on_regdeps_trace_kernel_xfer_app" - # Generate an OFFLINE_FILE_TYPE_ARCH_REGDEPS trace by running record_filter - # with -filter_encodings2regdeps to change instruction encodings, - # -filter_keep_func_ids 4294967498 (which is SYS_futex, associated to the only - # TRACE_MARKER_TYPE_FUNC_ markers we want to keep), - # -filter_marker_types 19,25,27,28,30,40,41 (which correspond to - # TRACE_MARKER_TYPE_SYSCALL_IDX, TRACE_MARKER_TYPE_SYSCALL, - # TRACE_MARKER_TYPE_SYSCALL_TRACE_START, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, - # TRACE_MARKER_TYPE_SYSCALL_FAILED, TRACE_MARKER_TYPE_SIGNAL_NUMBER, - # TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION), and - # -filter_modify_marker_value 3,-1 (which changes the value of - # TRACE_MARKER_TYPE_CPU_ID == 3 to INVALID_CPU_MARKER_VALUE == (uintptr_t)-1). "${drcachesim_path}@-simulator_type@record_filter@-filter_encodings2regdeps@-indir@${testname}.${kernel_xfer_app}.*.dir/trace@-outdir@${testname}.filtered.dir@-filter_marker_types@19,25,27,28,30,40,41@-filter_keep_func_ids@4294967498@-filter_modify_marker_value@3,-1" - # We run the invariant_checker analyzer on the REGDEPS filtered trace. - # We expect no invariant errors. "invariant_checker") endif ()