Skip to content

i#7470: Fix order of unreading split trace entries #7469

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
abhinav92003 merged 8 commits into from
Apr 29, 2025
Merged

i#7470: Fix order of unreading split trace entries #7469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
efe150c
Fix order of unreading split trace entries
abhinav92003 Apr 28, 2025
19dc0bf
Add unit test
abhinav92003 Apr 28, 2025
73238d9
Add todo for other bug and fix for x86-32
abhinav92003 Apr 28, 2025
7030482
Revert iostream diff
abhinav92003 Apr 28, 2025
4c42f0e
Use jump_reg which is more available than jump_mem
abhinav92003 Apr 28, 2025
9f354b4
use symb const
abhinav92003 Apr 28, 2025
0cf4f85
Fix build issue
abhinav92003 Apr 28, 2025
95f21ab
Fix issue#
abhinav92003 Apr 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion clients/drcachesim/tests/raw2trace_unit_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* **********************************************************
* Copyright (c) 2021-2023 Google, Inc. All rights reserved.
* Copyright (c) 2021-2025 Google, Inc. All rights reserved.
* **********************************************************/

/*
Expand Down Expand Up @@ -441,15 +441,27 @@ test_branch_delays(void *drcontext)
XINST_CREATE_move(drcontext, opnd_create_reg(REG1), opnd_create_reg(REG2));
instr_t *jmp = XINST_CREATE_jump(drcontext, opnd_create_instr(move));
instr_t *jcc = XINST_CREATE_jump_cond(drcontext, DR_PRED_EQ, opnd_create_instr(jmp));
instr_t *jump_reg = XINST_CREATE_jump_reg(drcontext, opnd_create_reg(REG1));
instr_t *move2 =
XINST_CREATE_move(drcontext, opnd_create_reg(REG1), opnd_create_reg(REG2));
instrlist_append(ilist, nop);
instrlist_append(ilist, jcc);
instrlist_append(ilist, jmp);
instrlist_append(ilist, move);
instrlist_append(ilist, jump_reg);
instrlist_append(ilist, move2);
size_t offs_nop = 0;
size_t offs_jz = offs_nop + instr_length(drcontext, nop);
size_t offs_jmp = offs_jz + instr_length(drcontext, jcc);
size_t offs_mov = offs_jmp + instr_length(drcontext, jmp);
size_t offs_jump_reg = offs_mov + instr_length(drcontext, move);
size_t offs_mov2 = offs_jump_reg + instr_length(drcontext, jump_reg);

#ifdef X64
constexpr uint64_t kernel_event_value = 0x9abc12345678;
#else
constexpr uint32_t kernel_event_value = 0x12345678;
#endif
// Now we synthesize our raw trace itself, including a valid header sequence.
std::vector<offline_entry_t> raw;
raw.push_back(make_header());
Expand All @@ -461,6 +473,13 @@ test_branch_delays(void *drcontext)
raw.push_back(make_core());
raw.push_back(make_block(offs_jmp, 1));
raw.push_back(make_block(offs_mov, 1));
raw.push_back(make_block(offs_jump_reg, 1));
#ifdef X64
raw.push_back(make_marker(TRACE_MARKER_TYPE_SPLIT_VALUE, (kernel_event_value >> 32)));
#endif
raw.push_back(
make_marker(TRACE_MARKER_TYPE_KERNEL_EVENT, (kernel_event_value & 0xffffffff)));
raw.push_back(make_block(offs_mov2, 1));
raw.push_back(make_exit());

std::vector<trace_entry_t> entries;
Expand Down Expand Up @@ -493,6 +512,23 @@ test_branch_delays(void *drcontext)
check_entry(entries, idx, TRACE_TYPE_INSTR_DIRECT_JUMP, -1) &&
check_entry(entries, idx, TRACE_TYPE_ENCODING, -1) &&
check_entry(entries, idx, TRACE_TYPE_INSTR, -1) &&
check_entry(entries, idx, TRACE_TYPE_ENCODING, -1) &&
#ifdef X64
// TODO i#7470: This is a bug. When looking for the kernel_event marker,
// raw2trace doesn't look for the ones preceded by the split_value marker
// because the address was too large for a single marker. Some other marker
// related logic also needs to expect the split_value marker.
check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_BRANCH_TARGET,
offs_mov2) &&
#else
check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_BRANCH_TARGET,
kernel_event_value) &&
#endif
check_entry(entries, idx, TRACE_TYPE_INSTR_INDIRECT_JUMP, -1) &&
check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_KERNEL_EVENT,
kernel_event_value) &&
check_entry(entries, idx, TRACE_TYPE_ENCODING, -1) &&
check_entry(entries, idx, TRACE_TYPE_INSTR, -1) &&
check_entry(entries, idx, TRACE_TYPE_THREAD_EXIT, -1) &&
check_entry(entries, idx, TRACE_TYPE_FOOTER, -1));
}
Expand Down
2 changes: 1 addition & 1 deletion clients/drcachesim/tracer/raw2trace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2794,12 +2794,12 @@ void
raw2trace_t::unread_last_entry(raw2trace_thread_data_t *tdata)
{
VPRINT(5, "Unreading last entry\n");
tdata->pre_read.push_front(tdata->last_entry);
Comment thread
abhinav92003 marked this conversation as resolved.
if (tdata->last_entry_is_split) {
VPRINT(4, "Unreading both parts of split entry at once\n");
tdata->pre_read.push_front(tdata->last_split_first_entry);
tdata->last_entry_is_split = false;
}
tdata->pre_read.push_front(tdata->last_entry);
}

void
Expand Down
Loading