diff --git a/clients/drcachesim/common/trace_entry.h b/clients/drcachesim/common/trace_entry.h index 031deb1f286..cb76b24e6fc 100644 --- a/clients/drcachesim/common/trace_entry.h +++ b/clients/drcachesim/common/trace_entry.h @@ -83,7 +83,7 @@ typedef enum { * conditional branches use either #TRACE_TYPE_INSTR_TAKEN_JUMP or * #TRACE_TYPE_INSTR_UNTAKEN_JUMP and that the target of indirect branches is in a * marker of type #TRACE_MARKER_TYPE_BRANCH_TARGET prior to the indirect branch - * instrution entry itself. This only applies to offline traces whose instructions + * instruction entry itself. This only applies to offline traces whose instructions * are not filtered; online traces, and i-filtered offline traces, even at this * version, do not contain this information. */ @@ -525,7 +525,7 @@ typedef enum { /** * This marker is present just before each indirect branch instruction in offline - * un-instruction-filtered traces. The marker value holds the actual target of the + * non-i-filtered traces. The marker value holds the actual target of the * branch. */ TRACE_MARKER_TYPE_BRANCH_TARGET, diff --git a/clients/drcachesim/docs/drcachesim.dox.in b/clients/drcachesim/docs/drcachesim.dox.in index d73497d1876..aca521168b5 100644 --- a/clients/drcachesim/docs/drcachesim.dox.in +++ b/clients/drcachesim/docs/drcachesim.dox.in @@ -1384,17 +1384,6 @@ behavior for rseq aborts): 2212820 1648447: 1249326 ifetch 5 byte(s) @ 0x00007f34066a8b37 4c 8b 54 24 48 mov 0x48(%rsp), %r10 ``` -Offline traces further guarantee that a branch target instruction -entry in a trace must immediately follow the branch instruction with -no intervening thread switch. This allows a core simulator to -identify the target of a branch by looking at the subsequent trace -entry in most cases, though as mentioned the target should be -available from the taken/untaken type plus the decoding and indirect -branch marker. This guarantee does not hold when a kernel event such -as a signal is delivered immediately after a branch; however, each -marker indicating such a kernel transfer includes the interrupted PC, -explicitly providing the branch target, for all but rseq aborts. - Filtered traces (filtered via -L0_filter) include the dynamic (pre-filtered) per-thread instruction count in a #dynamorio::drmemtrace::TRACE_MARKER_TYPE_INSTRUCTION_COUNT marker at diff --git a/clients/drcachesim/tests/invariant_checker_test.cpp b/clients/drcachesim/tests/invariant_checker_test.cpp index 9641f650e1e..b0ff9d010d6 100644 --- a/clients/drcachesim/tests/invariant_checker_test.cpp +++ b/clients/drcachesim/tests/invariant_checker_test.cpp @@ -940,30 +940,30 @@ bool check_branch_decoration() { std::cerr << "Testing branch decoration\n"; + static constexpr memref_tid_t TID = 1; // Indirect branch target: correct. { std::vector memrefs = { - gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), - gen_instr(/*tid=*/1, /*pc=*/1), - gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_BRANCH_TARGET, 32), - gen_instr_type(TRACE_TYPE_INSTR_INDIRECT_CALL, /*tid=*/1, /*pc=*/2), - gen_instr(/*tid=*/1, /*pc=*/32), + gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), + gen_instr(TID, /*pc=*/1), + gen_marker(TID, TRACE_MARKER_TYPE_BRANCH_TARGET, 32), + gen_instr_type(TRACE_TYPE_INSTR_INDIRECT_CALL, TID, /*pc=*/2), + gen_instr(TID, /*pc=*/32), }; if (!run_checker(memrefs, false)) return false; } #ifdef UNIX // Indirect branch target with kernel event: correct. + // We ensure the next PC is obtained from the kernel event interruption. { std::vector memrefs = { - gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), - gen_instr(/*tid=*/1, /*pc=*/1), - gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_BRANCH_TARGET, 32), - gen_instr_type(TRACE_TYPE_INSTR_INDIRECT_CALL, /*tid=*/1, /*pc=*/2), - gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_KERNEL_EVENT, 32), - gen_instr(/*tid=*/1, /*pc=*/999), + gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), + gen_instr(TID, /*pc=*/1), + gen_marker(TID, TRACE_MARKER_TYPE_BRANCH_TARGET, 32), + gen_instr_type(TRACE_TYPE_INSTR_INDIRECT_CALL, TID, /*pc=*/2), + gen_marker(TID, TRACE_MARKER_TYPE_KERNEL_EVENT, 32), + gen_instr(TID, /*pc=*/999), }; if (!run_checker(memrefs, false)) return false; @@ -972,11 +972,10 @@ check_branch_decoration() // Indirect branch target: no marker. { std::vector memrefs = { - gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), - gen_instr(/*tid=*/1, /*pc=*/1), - gen_instr_type(TRACE_TYPE_INSTR_INDIRECT_CALL, /*tid=*/1, /*pc=*/2), - gen_instr(/*tid=*/1, /*pc=*/32), + gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), + gen_instr(TID, /*pc=*/1), + gen_instr_type(TRACE_TYPE_INSTR_INDIRECT_CALL, TID, /*pc=*/2), + gen_instr(TID, /*pc=*/32), }; if (!run_checker(memrefs, true, 1, 3, "Indirect branches must be preceded by their targets", @@ -986,12 +985,11 @@ check_branch_decoration() // Indirect branch target: marker value incorrect. { std::vector memrefs = { - gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), - gen_instr(/*tid=*/1, /*pc=*/1), - gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_BRANCH_TARGET, 32), - gen_instr_type(TRACE_TYPE_INSTR_INDIRECT_CALL, /*tid=*/1, /*pc=*/2), - gen_instr(/*tid=*/1, /*pc=*/33), + gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), + gen_instr(TID, /*pc=*/1), + gen_marker(TID, TRACE_MARKER_TYPE_BRANCH_TARGET, 32), + gen_instr_type(TRACE_TYPE_INSTR_INDIRECT_CALL, TID, /*pc=*/2), + gen_instr(TID, /*pc=*/33), }; if (!run_checker(memrefs, true, 1, 5, "Branch does not go to the correct target", "Failed to catch bad indirect branch target marker")) @@ -1001,13 +999,12 @@ check_branch_decoration() // Indirect branch target with kernel event: marker value incorrect. { std::vector memrefs = { - gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), - gen_instr(/*tid=*/1, /*pc=*/1), - gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_BRANCH_TARGET, 32), - gen_instr_type(TRACE_TYPE_INSTR_INDIRECT_CALL, /*tid=*/1, /*pc=*/2), - gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_KERNEL_EVENT, 999), - gen_instr(/*tid=*/1, /*pc=*/32), + gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), + gen_instr(TID, /*pc=*/1), + gen_marker(TID, TRACE_MARKER_TYPE_BRANCH_TARGET, 32), + gen_instr_type(TRACE_TYPE_INSTR_INDIRECT_CALL, TID, /*pc=*/2), + gen_marker(TID, TRACE_MARKER_TYPE_KERNEL_EVENT, 999), + gen_instr(TID, /*pc=*/32), }; if (!run_checker(memrefs, true, 1, 5, "Branch does not go to the correct target", "Failed to catch bad indirect branch target marker")) @@ -1017,10 +1014,9 @@ check_branch_decoration() // Deprecated branch type. { std::vector memrefs = { - gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), - gen_instr(/*tid=*/1, /*pc=*/1), - gen_instr_type(TRACE_TYPE_INSTR_CONDITIONAL_JUMP, /*tid=*/1, /*pc=*/2), + gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), + gen_instr(TID, /*pc=*/1), + gen_instr_type(TRACE_TYPE_INSTR_CONDITIONAL_JUMP, TID, /*pc=*/2), }; if (!run_checker(memrefs, true, 1, 3, "The CONDITIONAL_JUMP type is deprecated and should not appear", @@ -1031,23 +1027,21 @@ check_branch_decoration() { instr_t *move = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1), opnd_create_reg(REG2)); - instr_t *cbr = + instr_t *cbr_to_move = XINST_CREATE_jump_cond(GLOBAL_DCONTEXT, DR_PRED_EQ, opnd_create_instr(move)); instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT); instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT); - instrlist_append(ilist, cbr); + instrlist_append(ilist, cbr_to_move); instrlist_append(ilist, nop); instrlist_append(ilist, move); static constexpr addr_t BASE_ADDR = 0x123450; std::vector memref_setup = { - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), + { gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), nullptr }, - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_FILETYPE, - OFFLINE_FILE_TYPE_ENCODINGS), + { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS), nullptr }, - { gen_instr_type(TRACE_TYPE_INSTR_TAKEN_JUMP, /*tid=*/1), cbr }, - { gen_instr(/*tid=*/1), move }, + { gen_instr_type(TRACE_TYPE_INSTR_TAKEN_JUMP, TID), cbr_to_move }, + { gen_instr(TID), move }, }; auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR); instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist); @@ -1059,24 +1053,22 @@ check_branch_decoration() { instr_t *move = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1), opnd_create_reg(REG2)); - instr_t *cbr = + instr_t *cbr_to_move = XINST_CREATE_jump_cond(GLOBAL_DCONTEXT, DR_PRED_EQ, opnd_create_instr(move)); instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT); instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT); - instrlist_append(ilist, cbr); + instrlist_append(ilist, cbr_to_move); instrlist_append(ilist, nop); instrlist_append(ilist, move); static constexpr addr_t BASE_ADDR = 0x123450; std::vector memref_setup = { - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), + { gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), nullptr }, - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_FILETYPE, - OFFLINE_FILE_TYPE_ENCODINGS), + { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS), nullptr }, - { gen_instr_type(TRACE_TYPE_INSTR_TAKEN_JUMP, /*tid=*/1), cbr }, - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_KERNEL_EVENT, 0), move }, - { gen_instr(/*tid=*/1), nop }, + { gen_instr_type(TRACE_TYPE_INSTR_TAKEN_JUMP, TID), cbr_to_move }, + { gen_marker(TID, TRACE_MARKER_TYPE_KERNEL_EVENT, 0), move }, + { gen_instr(TID), nop }, }; auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR); instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist); @@ -1088,23 +1080,21 @@ check_branch_decoration() { instr_t *move = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1), opnd_create_reg(REG2)); - instr_t *cbr = + instr_t *cbr_to_move = XINST_CREATE_jump_cond(GLOBAL_DCONTEXT, DR_PRED_EQ, opnd_create_instr(move)); instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT); instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT); - instrlist_append(ilist, cbr); + instrlist_append(ilist, cbr_to_move); instrlist_append(ilist, nop); instrlist_append(ilist, move); static constexpr addr_t BASE_ADDR = 0x123450; std::vector memref_setup = { - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), + { gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), nullptr }, - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_FILETYPE, - OFFLINE_FILE_TYPE_ENCODINGS), + { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS), nullptr }, - { gen_instr_type(TRACE_TYPE_INSTR_TAKEN_JUMP, /*tid=*/1), cbr }, - { gen_instr(/*tid=*/1), nop }, + { gen_instr_type(TRACE_TYPE_INSTR_TAKEN_JUMP, TID), cbr_to_move }, + { gen_instr(TID), nop }, }; auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR); instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist); @@ -1117,24 +1107,22 @@ check_branch_decoration() { instr_t *move = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1), opnd_create_reg(REG2)); - instr_t *cbr = + instr_t *cbr_to_move = XINST_CREATE_jump_cond(GLOBAL_DCONTEXT, DR_PRED_EQ, opnd_create_instr(move)); instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT); instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT); - instrlist_append(ilist, cbr); + instrlist_append(ilist, cbr_to_move); instrlist_append(ilist, nop); instrlist_append(ilist, move); static constexpr addr_t BASE_ADDR = 0x123450; std::vector memref_setup = { - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), + { gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), nullptr }, - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_FILETYPE, - OFFLINE_FILE_TYPE_ENCODINGS), + { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS), nullptr }, - { gen_instr_type(TRACE_TYPE_INSTR_TAKEN_JUMP, /*tid=*/1), cbr }, - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_KERNEL_EVENT, 0), nop }, - { gen_instr(/*tid=*/1), move }, + { gen_instr_type(TRACE_TYPE_INSTR_TAKEN_JUMP, TID), cbr_to_move }, + { gen_marker(TID, TRACE_MARKER_TYPE_KERNEL_EVENT, 0), nop }, + { gen_instr(TID), move }, }; auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR); instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist); @@ -1147,23 +1135,21 @@ check_branch_decoration() { instr_t *move = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1), opnd_create_reg(REG2)); - instr_t *cbr = + instr_t *cbr_to_move = XINST_CREATE_jump_cond(GLOBAL_DCONTEXT, DR_PRED_EQ, opnd_create_instr(move)); instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT); instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT); - instrlist_append(ilist, cbr); + instrlist_append(ilist, cbr_to_move); instrlist_append(ilist, nop); instrlist_append(ilist, move); static constexpr addr_t BASE_ADDR = 0x123450; std::vector memref_setup = { - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), + { gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), nullptr }, - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_FILETYPE, - OFFLINE_FILE_TYPE_ENCODINGS), + { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS), nullptr }, - { gen_instr_type(TRACE_TYPE_INSTR_UNTAKEN_JUMP, /*tid=*/1), cbr }, - { gen_instr(/*tid=*/1), nop }, + { gen_instr_type(TRACE_TYPE_INSTR_UNTAKEN_JUMP, TID), cbr_to_move }, + { gen_instr(TID), nop }, }; auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR); instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist); @@ -1175,24 +1161,22 @@ check_branch_decoration() { instr_t *move = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1), opnd_create_reg(REG2)); - instr_t *cbr = + instr_t *cbr_to_move = XINST_CREATE_jump_cond(GLOBAL_DCONTEXT, DR_PRED_EQ, opnd_create_instr(move)); instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT); instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT); - instrlist_append(ilist, cbr); + instrlist_append(ilist, cbr_to_move); instrlist_append(ilist, nop); instrlist_append(ilist, move); static constexpr addr_t BASE_ADDR = 0x123450; std::vector memref_setup = { - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), + { gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), nullptr }, - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_FILETYPE, - OFFLINE_FILE_TYPE_ENCODINGS), + { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS), nullptr }, - { gen_instr_type(TRACE_TYPE_INSTR_UNTAKEN_JUMP, /*tid=*/1), cbr }, - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_KERNEL_EVENT, 0), nop }, - { gen_instr(/*tid=*/1), move }, + { gen_instr_type(TRACE_TYPE_INSTR_UNTAKEN_JUMP, TID), cbr_to_move }, + { gen_marker(TID, TRACE_MARKER_TYPE_KERNEL_EVENT, 0), nop }, + { gen_instr(TID), move }, }; auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR); instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist); @@ -1204,23 +1188,21 @@ check_branch_decoration() { instr_t *move = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1), opnd_create_reg(REG2)); - instr_t *cbr = + instr_t *cbr_to_move = XINST_CREATE_jump_cond(GLOBAL_DCONTEXT, DR_PRED_EQ, opnd_create_instr(move)); instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT); instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT); - instrlist_append(ilist, cbr); + instrlist_append(ilist, cbr_to_move); instrlist_append(ilist, nop); instrlist_append(ilist, move); static constexpr addr_t BASE_ADDR = 0x123450; std::vector memref_setup = { - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), + { gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), nullptr }, - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_FILETYPE, - OFFLINE_FILE_TYPE_ENCODINGS), + { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS), nullptr }, - { gen_instr_type(TRACE_TYPE_INSTR_UNTAKEN_JUMP, /*tid=*/1), cbr }, - { gen_instr(/*tid=*/1), move }, + { gen_instr_type(TRACE_TYPE_INSTR_UNTAKEN_JUMP, TID), cbr_to_move }, + { gen_instr(TID), move }, }; auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR); instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist); @@ -1233,24 +1215,22 @@ check_branch_decoration() { instr_t *move = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1), opnd_create_reg(REG2)); - instr_t *cbr = + instr_t *cbr_to_move = XINST_CREATE_jump_cond(GLOBAL_DCONTEXT, DR_PRED_EQ, opnd_create_instr(move)); instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT); instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT); - instrlist_append(ilist, cbr); + instrlist_append(ilist, cbr_to_move); instrlist_append(ilist, nop); instrlist_append(ilist, move); static constexpr addr_t BASE_ADDR = 0x123450; std::vector memref_setup = { - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_VERSION, - TRACE_ENTRY_VERSION_BRANCH_INFO), + { gen_marker(TID, TRACE_MARKER_TYPE_VERSION, TRACE_ENTRY_VERSION_BRANCH_INFO), nullptr }, - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_FILETYPE, - OFFLINE_FILE_TYPE_ENCODINGS), + { gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS), nullptr }, - { gen_instr_type(TRACE_TYPE_INSTR_UNTAKEN_JUMP, /*tid=*/1), cbr }, - { gen_marker(/*tid=*/1, TRACE_MARKER_TYPE_KERNEL_EVENT, 0), move }, - { gen_instr(/*tid=*/1), nop }, + { gen_instr_type(TRACE_TYPE_INSTR_UNTAKEN_JUMP, TID), cbr_to_move }, + { gen_marker(TID, TRACE_MARKER_TYPE_KERNEL_EVENT, 0), move }, + { gen_instr(TID), nop }, }; auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR); instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist); diff --git a/clients/drcachesim/tests/raw2trace_unit_tests.cpp b/clients/drcachesim/tests/raw2trace_unit_tests.cpp index 8d799571771..26e63368af8 100644 --- a/clients/drcachesim/tests/raw2trace_unit_tests.cpp +++ b/clients/drcachesim/tests/raw2trace_unit_tests.cpp @@ -1983,7 +1983,7 @@ test_branch_decoration(void *drcontext) // We focus on signals, rseq rollbacks to branches, and terminal branches here. bool res = true; { - // Branch before signal. + // Taken branch before signal. instrlist_t *ilist = instrlist_create(drcontext); instr_t *nop1 = XINST_CREATE_nop(drcontext); // Avoid offset of 0. instr_t *move = @@ -2033,6 +2033,56 @@ test_branch_decoration(void *drcontext) check_entry(entries, idx, TRACE_TYPE_THREAD_EXIT, -1) && check_entry(entries, idx, TRACE_TYPE_FOOTER, -1); } + { + // Untaken branch before signal. + instrlist_t *ilist = instrlist_create(drcontext); + instr_t *nop1 = XINST_CREATE_nop(drcontext); // Avoid offset of 0. + instr_t *move = + XINST_CREATE_move(drcontext, opnd_create_reg(REG1), opnd_create_reg(REG2)); + instr_t *jcc = + XINST_CREATE_jump_cond(drcontext, DR_PRED_EQ, opnd_create_instr(move)); + instr_t *nop2 = XINST_CREATE_nop(drcontext); + instrlist_append(ilist, nop1); + instrlist_append(ilist, jcc); + instrlist_append(ilist, nop2); + instrlist_append(ilist, move); + size_t offs_nop1 = 0; + size_t offs_jcc = offs_nop1 + instr_length(drcontext, nop1); + size_t offs_nop2 = offs_jcc + instr_length(drcontext, jcc); + + std::vector raw; + raw.push_back(make_header()); + raw.push_back(make_tid()); + raw.push_back(make_pid()); + raw.push_back(make_line_size()); + raw.push_back(make_block(offs_jcc, 1)); + raw.push_back(make_marker(TRACE_MARKER_TYPE_KERNEL_EVENT, offs_nop2)); + raw.push_back(make_exit()); + + std::vector entries; + if (!run_raw2trace(drcontext, raw, ilist, entries)) + return false; + int idx = 0; + res = res && check_entry(entries, idx, TRACE_TYPE_HEADER, -1) && + check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_VERSION) && + check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_FILETYPE) && + check_entry(entries, idx, TRACE_TYPE_THREAD, -1) && + check_entry(entries, idx, TRACE_TYPE_PID, -1) && + check_entry(entries, idx, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_CACHE_LINE_SIZE) && + check_entry(entries, idx, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_CHUNK_INSTR_COUNT) && + check_entry(entries, idx, TRACE_TYPE_ENCODING, -1) && +#ifdef X86_32 + // An extra encoding entry is needed. + check_entry(entries, idx, TRACE_TYPE_ENCODING, -1) && +#endif + check_entry(entries, idx, TRACE_TYPE_INSTR_UNTAKEN_JUMP, -1, offs_jcc) && + check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_KERNEL_EVENT, + offs_nop2) && + check_entry(entries, idx, TRACE_TYPE_THREAD_EXIT, -1) && + check_entry(entries, idx, TRACE_TYPE_FOOTER, -1); + } { // Untaken branch at and of rseq rollback. instrlist_t *ilist = instrlist_create(drcontext); diff --git a/clients/drcachesim/tools/invariant_checker.cpp b/clients/drcachesim/tools/invariant_checker.cpp index a1739e6c5e0..b57bb98fd9f 100644 --- a/clients/drcachesim/tools/invariant_checker.cpp +++ b/clients/drcachesim/tools/invariant_checker.cpp @@ -295,7 +295,7 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem } if (memref.marker.type == TRACE_TYPE_MARKER && memref.marker.marker_type == TRACE_MARKER_TYPE_VERSION) { - shard->version_ = memref.marker.marker_value; + shard->trace_version_ = memref.marker.marker_value; report_if_false(shard, shard->stream == nullptr || memref.marker.marker_value == shard->stream->get_version(), @@ -502,10 +502,14 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem "Branch target not immediately after branch"); } // Invariant: non-explicit control flow (i.e., kernel-mediated) is indicated - // by markers. + // by markers. Checks are relaxed if a kernel event occurred but we pass + // last_instr_in_cur_context_ to avoid any confusion. + // XXX i#5912: For adding checks across every part of a signal handler should + // we change this to last_instr_in_cur_context_ and remove/adjust the + // check relaxations inside check_for_pc_discontinuity()? const std::string non_explicit_flow_violation_msg = check_for_pc_discontinuity( - shard, memref, shard->prev_instr_, memref.instr.addr, cur_instr_decoded, - expect_encoding, /*at_kernel_event=*/false); + shard, memref, shard->last_instr_in_cur_context_, memref.instr.addr, + cur_instr_decoded, expect_encoding, /*at_kernel_event=*/false); report_if_false(shard, non_explicit_flow_violation_msg.empty(), non_explicit_flow_violation_msg); @@ -696,21 +700,27 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem memref.marker.marker_type == TRACE_MARKER_TYPE_BRANCH_TARGET) { shard->last_branch_marker_value_ = memref.marker.marker_value; } - if (knob_offline_ && shard->version_ >= TRACE_ENTRY_VERSION_BRANCH_INFO && - type_is_instr_branch(memref.instr.type) && - // I-filtered traces don't mark branch targets. - !TESTANY(OFFLINE_FILE_TYPE_FILTERED | OFFLINE_FILE_TYPE_IFILTERED, - shard->file_type_)) { - report_if_false(shard, memref.instr.type != TRACE_TYPE_INSTR_CONDITIONAL_JUMP, - "The CONDITIONAL_JUMP type is deprecated and should not appear"); - if (!type_is_instr_direct_branch(memref.instr.type)) { - shard->last_indirect_target_ = shard->last_branch_marker_value_; - report_if_false(shard, - shard->last_indirect_target_ != 0 && - shard->prev_entry_.marker.type == TRACE_TYPE_MARKER && - shard->prev_entry_.marker.marker_type == - TRACE_MARKER_TYPE_BRANCH_TARGET, - "Indirect branches must be preceded by their targets"); + if (knob_offline_ && shard->trace_version_ >= TRACE_ENTRY_VERSION_BRANCH_INFO) { + if (type_is_instr_branch(memref.instr.type) && + // I-filtered traces don't mark branch targets. + !TESTANY(OFFLINE_FILE_TYPE_FILTERED | OFFLINE_FILE_TYPE_IFILTERED, + shard->file_type_)) { + report_if_false( + shard, memref.instr.type != TRACE_TYPE_INSTR_CONDITIONAL_JUMP, + "The CONDITIONAL_JUMP type is deprecated and should not appear"); + if (!type_is_instr_direct_branch(memref.instr.type)) { + shard->last_indirect_target_ = shard->last_branch_marker_value_; + report_if_false(shard, + shard->last_indirect_target_ != 0 && + shard->prev_entry_.marker.type == TRACE_TYPE_MARKER && + shard->prev_entry_.marker.marker_type == + TRACE_MARKER_TYPE_BRANCH_TARGET, + "Indirect branches must be preceded by their targets"); + } + } + if (!type_is_instr_branch(memref.instr.type) || + type_is_instr_direct_branch(memref.instr.type)) { + shard->last_indirect_target_ = 0; } } @@ -917,7 +927,7 @@ invariant_checker_t::check_for_pc_discontinuity( // Check for all valid transitions except taken branches. We consider taken // branches later so that we can provide a different message for those // invariant violations. - bool fall_through_ok = !type_is_instr_branch(prev_instr.instr.type) || + bool fall_through_allowed = !type_is_instr_branch(prev_instr.instr.type) || prev_instr.instr.type == TRACE_TYPE_INSTR_CONDITIONAL_JUMP || prev_instr.instr.type == TRACE_TYPE_INSTR_UNTAKEN_JUMP; const bool valid_nonbranch_flow = @@ -925,7 +935,7 @@ invariant_checker_t::check_for_pc_discontinuity( TESTANY(OFFLINE_FILE_TYPE_FILTERED | OFFLINE_FILE_TYPE_IFILTERED, shard->file_type_) || // Regular fall-through. - (fall_through_ok && prev_instr_trace_pc + prev_instr.instr.size == cur_pc) || + (fall_through_allowed && prev_instr_trace_pc + prev_instr.instr.size == cur_pc) || // String loop. (prev_instr_trace_pc == cur_pc && (memref.instr.type == TRACE_TYPE_INSTR_NO_FETCH || @@ -938,6 +948,9 @@ invariant_checker_t::check_for_pc_discontinuity( // same instruction. (prev_instr_trace_pc == cur_pc && at_kernel_event) || // Kernel-mediated, but we can't tell if we had a thread swap. + // TODO i#5912: Isn't this relaxation too loose since we really only want + // to relax if a kernel event happened immediately prior, while prev_xfer_marker_ + // could be many records back? (shard->prev_xfer_marker_.instr.tid != 0 && !at_kernel_event && (shard->prev_xfer_marker_.marker.marker_type == TRACE_MARKER_TYPE_KERNEL_EVENT || shard->prev_xfer_marker_.marker.marker_type == TRACE_MARKER_TYPE_KERNEL_XFER || @@ -949,7 +962,8 @@ invariant_checker_t::check_for_pc_discontinuity( // Check if the type is a branch instruction and there is a branch target // mismatch. if (type_is_instr_branch(prev_instr.instr.type)) { - if (knob_offline_ && shard->version_ >= TRACE_ENTRY_VERSION_BRANCH_INFO) { + if (knob_offline_ && + shard->trace_version_ >= TRACE_ENTRY_VERSION_BRANCH_INFO) { // We have precise branch target info. if (prev_instr.instr.type == TRACE_TYPE_INSTR_UNTAKEN_JUMP) { branch_target = prev_instr_trace_pc + prev_instr.instr.size; diff --git a/clients/drcachesim/tools/invariant_checker.h b/clients/drcachesim/tools/invariant_checker.h index 2f02c553f11..00b3942bf2f 100644 --- a/clients/drcachesim/tools/invariant_checker.h +++ b/clients/drcachesim/tools/invariant_checker.h @@ -127,9 +127,9 @@ class invariant_checker_t : public analysis_tool_t { // We treat 0 as a sentinel; thus we do not support a trace deliberately // jumping to 0 and handling the fault. addr_t last_indirect_target_ = 0; - // We need an "on-deck" value to handle consecutive indirect branches. + // We need a dedicated variable to handle consecutive indirect branches. addr_t last_branch_marker_value_ = 0; - uintptr_t version_ = 0; + uintptr_t trace_version_ = 0; #ifdef UNIX // We keep track of some state per nested signal depth. struct signal_context { diff --git a/clients/drcachesim/tracer/raw2trace.cpp b/clients/drcachesim/tracer/raw2trace.cpp index 9c3cf0d79bf..991becc1f83 100644 --- a/clients/drcachesim/tracer/raw2trace.cpp +++ b/clients/drcachesim/tracer/raw2trace.cpp @@ -1031,6 +1031,9 @@ raw2trace_t::process_next_thread_buffer(raw2trace_thread_data_t *tdata, (entry.extended.valueB == TRACE_MARKER_TYPE_WINDOW_ID && entry.extended.valueA != tdata->last_window))))) { app_pc next_pc = nullptr; + // Get the next instr's pc from the interruption value in the marker + // (a record for the next instr itself won't appear until the signal + // returns, if that happens). if (entry.extended.ext == OFFLINE_EXT_TYPE_MARKER && entry.extended.valueB == TRACE_MARKER_TYPE_KERNEL_EVENT) { uintptr_t marker_val = 0; @@ -2651,97 +2654,108 @@ raw2trace_t::thread_file_at_eof(raw2trace_thread_data_t *tdata) std::string raw2trace_t::append_delayed_branch(raw2trace_thread_data_t *tdata, app_pc next_pc) { + // While we no longer document a guarantee that branches are delayed to make them + // adjacent to their targets now that we have TRACE_ENTRY_VERSION_BRANCH_INFO, we + // still use the delayed branch mechanism as it was already in place and is the + // easiest way to implement TRACE_ENTRY_VERSION_BRANCH_INFO. If we decide to + // use a different implementation we should perhaps wait for all users to + // update their clients. if (tdata->delayed_branch_empty_) return ""; - // We need to transform TRACE_TYPE_INSTR_CONDITIONAL_JUMP into - // TRACE_TYPE_INSTR_{TAKEN,UNTAKEN}_JUMP based on which side of the branch - // was taken. We also need to add indirect branch target markers. - int instr_count = 0; - for (size_t i = 0; i < tdata->delayed_branch.size(); ++i) { - const auto &entry = tdata->delayed_branch[i]; - if (type_is_instr(static_cast(entry.type))) - ++instr_count; - } - int instr_index = instr_count - 1; - // Walk backward so we have the next pc for stacked branches. - app_pc next_instr_pc = next_pc; - for (int i = static_cast(tdata->delayed_branch.size()) - 1; i >= 0; --i) { - auto &entry = tdata->delayed_branch[i]; - // We can't infer branch targets for filtered instructions. - if (TESTANY(OFFLINE_FILE_TYPE_FILTERED | OFFLINE_FILE_TYPE_IFILTERED, - get_file_type(tdata))) - continue; - if (type_is_instr(static_cast(entry.type))) { - if (tdata->delayed_branch_target_pcs.size() <= - static_cast(instr_index)) { - return "Delayed branch target vector mis-sized"; - } - app_pc target = tdata->delayed_branch_target_pcs[instr_index]; - // Cache entry fields before we insert any markers at entry's position. - app_pc branch_addr = reinterpret_cast(entry.addr); - trace_type_t branch_type = static_cast(entry.type); - if (next_instr_pc == nullptr && - (target == nullptr || entry.type == TRACE_TYPE_INSTR_CONDITIONAL_JUMP)) { - // This is a trace-final or window-final branch but we do not have - // its taken/target without a subsequent instr: just delete it. - DEBUG_ASSERT(instr_index == instr_count - 1); - int erase_from = i; - while (erase_from > 0 && - tdata->delayed_branch[erase_from - 1].type == - TRACE_TYPE_ENCODING) { - log(4, "Erasing cached encoding for %p\n", - tdata->delayed_branch_decode_pcs[instr_index]); - tdata->encoding_emitted.erase( - tdata->delayed_branch_decode_pcs[instr_index]); - --erase_from; + // We can't infer branch targets for filtered instructions. + if (!TESTANY(OFFLINE_FILE_TYPE_FILTERED | OFFLINE_FILE_TYPE_IFILTERED, + get_file_type(tdata))) { + // We need to transform TRACE_TYPE_INSTR_CONDITIONAL_JUMP into + // TRACE_TYPE_INSTR_{TAKEN,UNTAKEN}_JUMP based on which side of the branch + // was taken. We also need to add indirect branch target markers. + int instr_count = 0; + for (size_t i = 0; i < tdata->delayed_branch.size(); ++i) { + const auto &entry = tdata->delayed_branch[i]; + if (type_is_instr(static_cast(entry.type))) + ++instr_count; + } + int instr_index = instr_count - 1; + // Walk backward so we have the next pc for stacked branches. + app_pc next_instr_pc = next_pc; + for (int i = static_cast(tdata->delayed_branch.size()) - 1; i >= 0; --i) { + auto &entry = tdata->delayed_branch[i]; + if (type_is_instr(static_cast(entry.type))) { + DEBUG_ASSERT(type_is_instr_branch(static_cast(entry.type))); + if (tdata->delayed_branch_target_pcs.size() <= + static_cast(instr_index)) { + return "Delayed branch target vector mis-sized"; } - VPRINT( - 4, - "Discarded %zd entries for final branch without subsequent instr\n", - tdata->delayed_branch.size() - erase_from); - tdata->delayed_branch.erase(tdata->delayed_branch.begin() + erase_from, - tdata->delayed_branch.end()); - tdata->delayed_branch_decode_pcs.pop_back(); - tdata->delayed_branch_target_pcs.pop_back(); - break; - } else { - if (target == nullptr) { - DEBUG_ASSERT(!type_is_instr_direct_branch( - static_cast(entry.type))); - trace_entry_t local[3]; - int size = trace_metadata_writer_t::write_marker( - reinterpret_cast(local), TRACE_MARKER_TYPE_BRANCH_TARGET, - reinterpret_cast(next_instr_pc)); - DEBUG_ASSERT(static_cast(size) <= sizeof(local)); - for (int local_idx = 0; - local_idx < size / static_cast(sizeof(local[0])); - ++local_idx) { - tdata->delayed_branch.insert(tdata->delayed_branch.begin() + i, - local[local_idx]); + app_pc target = tdata->delayed_branch_target_pcs[instr_index]; + // Cache entry fields before we insert any markers at entry's position. + app_pc branch_addr = reinterpret_cast(entry.addr); + trace_type_t branch_type = static_cast(entry.type); + if (next_instr_pc == nullptr && + (target == nullptr || + entry.type == TRACE_TYPE_INSTR_CONDITIONAL_JUMP)) { + // This is a trace-final or window-final branch but we do not have + // its taken/target without a subsequent instr: just delete it. + DEBUG_ASSERT(instr_index == instr_count - 1); + int erase_from = i; + while (erase_from > 0 && + tdata->delayed_branch[erase_from - 1].type == + TRACE_TYPE_ENCODING) { + log(4, "Erasing cached encoding for %p\n", + tdata->delayed_branch_decode_pcs[instr_index]); + tdata->encoding_emitted.erase( + tdata->delayed_branch_decode_pcs[instr_index]); + --erase_from; } - VPRINT(4, "Inserted indirect branch target %p\n", next_instr_pc); - } else if (entry.type == TRACE_TYPE_INSTR_CONDITIONAL_JUMP) { - if (target == next_instr_pc) { - branch_type = TRACE_TYPE_INSTR_TAKEN_JUMP; - } else { - branch_type = TRACE_TYPE_INSTR_UNTAKEN_JUMP; + VPRINT(4, + "Discarded %zd entries for final branch without subsequent " + "instr\n", + tdata->delayed_branch.size() - erase_from); + tdata->delayed_branch.erase(tdata->delayed_branch.begin() + + erase_from, + tdata->delayed_branch.end()); + tdata->delayed_branch_decode_pcs.pop_back(); + tdata->delayed_branch_target_pcs.pop_back(); + break; + } else { + if (target == nullptr) { + DEBUG_ASSERT(!type_is_instr_direct_branch( + static_cast(entry.type))); + trace_entry_t local[3]; + int size = trace_metadata_writer_t::write_marker( + reinterpret_cast(local), + TRACE_MARKER_TYPE_BRANCH_TARGET, + reinterpret_cast(next_instr_pc)); + DEBUG_ASSERT(static_cast(size) <= sizeof(local)); + for (int local_idx = 0; + local_idx < size / static_cast(sizeof(local[0])); + ++local_idx) { + tdata->delayed_branch.insert( + tdata->delayed_branch.begin() + i, local[local_idx]); + } + VPRINT(4, "Inserted indirect branch target %p\n", next_instr_pc); + } else if (entry.type == TRACE_TYPE_INSTR_CONDITIONAL_JUMP) { + if (target == next_instr_pc) { + branch_type = TRACE_TYPE_INSTR_TAKEN_JUMP; + } else { + branch_type = TRACE_TYPE_INSTR_UNTAKEN_JUMP; + } + entry.type = static_cast(branch_type); } - entry.type = static_cast(branch_type); + VPRINT( + 4, + "Appending delayed branch type=%d pc=%p decode=%p target=%p for " + "thread %d\n", + branch_type, branch_addr, + tdata->delayed_branch_decode_pcs[instr_index], target, + tdata->index); } + next_instr_pc = branch_addr; + --instr_index; + } else { VPRINT(4, - "Appending delayed branch type=%d pc=%p decode=%p target=%p for " - "thread %d\n", - branch_type, branch_addr, - tdata->delayed_branch_decode_pcs[instr_index], target, - tdata->index); + "Appending delayed branch tagalong entry type %s (%d) for thread " + "%d\n", + trace_type_names[entry.type], entry.type, tdata->index); } - next_instr_pc = branch_addr; - --instr_index; - } else { - VPRINT(4, - "Appending delayed branch tagalong entry type %s (%d) for thread " - "%d\n", - trace_type_names[entry.type], entry.type, tdata->index); } } if (!tdata->delayed_branch.empty()) {