Skip to content
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.

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

i#5975: Add invariant check that read/write records match operands #6283

Merged
merged 14 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions api/docs/release.dox
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ Further non-compatibility-affecting changes include:
-replay_file, -cpu_schedule_file).
ivankyluk marked this conversation as resolved.
Show resolved Hide resolved
- Added additional timestamps to drmemtrace traces: at the end of each buffer,
and before and after each system call.
- Added type_is_read() API that returns true if a trace type reads from memory.
- Added instr_num_memory_read_access() and instr_num_memory_write_access() that return
the number of memory read and write accesses of an instruction respectively.

**************************************************
<hr>
Expand Down
11 changes: 11 additions & 0 deletions clients/drcachesim/common/trace_entry.h
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,17 @@ type_is_data(const trace_type_t type)
type == TRACE_TYPE_DATA_FLUSH_END;
ivankyluk marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Returns whether the type represents a memory read access.
*/
static inline bool
type_is_read(const trace_type_t type)
{
return type_is_prefetch(type) || type == TRACE_TYPE_READ ||
type == TRACE_TYPE_INSTR_FLUSH || type == TRACE_TYPE_INSTR_FLUSH_END ||
type == TRACE_TYPE_DATA_FLUSH || type == TRACE_TYPE_DATA_FLUSH_END;
}

static inline bool
marker_type_is_function_marker(const trace_marker_type_t mark)
{
Expand Down
307 changes: 277 additions & 30 deletions clients/drcachesim/tests/invariant_checker_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,22 +277,27 @@ check_sane_control_flow()

// Negative test: branches with encodings which do not go to their targets.
{
std::vector<memref_t> memrefs = {
gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
# if defined(X86_64) || defined(X86_32)
// 0x74 is "je" with the 2nd byte the offset.
gen_branch_encoded(TID, 0x71019dbc, { 0x74, 0x32 }),
gen_instr_encoded(0x71019ded, { 0x01 }, TID),
# elif defined(ARM_64)
// 71019dbc: 540001a1 b.ne 71019df0
// <__executable_start+0x19df0>
gen_branch_encoded(TID, 0x71019dbc, 0x540001a1),
gen_instr_encoded(0x71019ded, 0x01, TID),
# else
// TODO i#5871: Add AArch32 (and RISC-V) encodings.
# endif
};
instr_t *move1 = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
opnd_create_reg(REG2));
instr_t *move2 = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
opnd_create_reg(REG2));
instr_t *cond_jmp =
XINST_CREATE_jump_cond(GLOBAL_DCONTEXT, DR_PRED_EQ, opnd_create_instr(move1));

instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
instrlist_append(ilist, cond_jmp);
instrlist_append(ilist, move1);
instrlist_append(ilist, move2);

std::vector<memref_with_IR_t> memref_instr_vec = {
{ gen_marker(1, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
nullptr },
{ gen_branch(1), cond_jmp },
{ gen_instr(1), move2 }
};
static constexpr addr_t BASE_ADDR = 0xeba4ad4;
auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
if (!run_checker(memrefs, true,
{ "Branch does not go to the correct target", TID,
/*ref_ordinal=*/3, /*last_timestamp=*/0,
Expand All @@ -303,21 +308,26 @@ check_sane_control_flow()
}
// Positive test: branches with encodings which go to their targets.
{
std::vector<memref_t> memrefs = {
gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
# if defined(X86_64) || defined(X86_32)
// 0x74 is "je" with the 2nd byte the offset.
gen_branch_encoded(TID, 0x71019dbc, { 0x74, 0x32 }),
# elif defined(ARM_64)
// 71019dbc: 540001a1 b.ne 71019df0
// <__executable_start+0x19df0>
gen_branch_encoded(TID, 0x71019dbc, 0x540001a1),
# else
// TODO i#5871: Add AArch32 (and RISC-V) encodings.
# endif
gen_instr(TID, 0x71019df0),
};
instr_t *move1 = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
opnd_create_reg(REG2));
instr_t *move2 = XINST_CREATE_move(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
opnd_create_reg(REG2));
instr_t *cond_jmp =
XINST_CREATE_jump_cond(GLOBAL_DCONTEXT, DR_PRED_EQ, opnd_create_instr(move1));

instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
instrlist_append(ilist, cond_jmp);
instrlist_append(ilist, move1);
instrlist_append(ilist, move2);

std::vector<memref_with_IR_t> memref_instr_vec = {
{ gen_marker(1, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
nullptr },
{ gen_branch(1), cond_jmp },
{ gen_instr(1), move1 }
};
static constexpr addr_t BASE_ADDR = 0xeba4ad4;
auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
if (!run_checker(memrefs, false)) {
return false;
}
Expand Down Expand Up @@ -1955,6 +1965,242 @@ check_timestamps_increase_monotonically(void)
return true;
}

bool
check_read_write_records_match_operands()
{
// Only the number of memory read and write records is checked against the
// operands. Address and size are not used.
std::cerr << "Testing number of memory read/write records matching operands\n";
constexpr memref_tid_t TID = 1;

// Correct: number of read records matches the operand.
{
instr_t *load = XINST_CREATE_load(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
OPND_CREATE_MEMPTR(REG1, /*disp=*/0));
instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
instrlist_append(ilist, load);

std::vector<memref_with_IR_t> memref_instr_vec = {
{ gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
nullptr },
{ gen_instr(TID), load },
{ gen_data(TID, /*load=*/true, /*addr=*/0, /*size=*/0), nullptr }
};
static constexpr addr_t BASE_ADDR = 0xeba4ad4;
auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
if (!run_checker(memrefs, false)) {
return false;
}
}
// Incorrect: too many read records.
{
instr_t *load = XINST_CREATE_load(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
OPND_CREATE_MEMPTR(REG1, /*disp=*/0));
instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
instrlist_append(ilist, load);

std::vector<memref_with_IR_t> memref_instr_vec = {
{ gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
nullptr },
{ gen_instr(TID), load },
{ gen_data(TID, /*load=*/true, /*addr=*/0, /*size=*/0), nullptr },
{ gen_data(TID, /*load=*/true, /*addr=*/0, /*size=*/0), nullptr }
};
static constexpr addr_t BASE_ADDR = 0xeba4ad4;
auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
if (!run_checker(memrefs, true,
{ "Too many read records", TID, /*ref_ordinal=*/4,
/*last_timestamp=*/0, /*instrs_since_last_timestamp=*/1 },
"Failed to catch too many read records"))
return false;
}
// Incorrect: missing read records.
{
instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT);
instr_t *load = XINST_CREATE_load(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
OPND_CREATE_MEMPTR(REG1, /*disp=*/0));
instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
instrlist_append(ilist, load);
instrlist_append(ilist, nop);

std::vector<memref_with_IR_t> memref_instr_vec = {
{ gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
nullptr },
{ gen_instr(TID), load },
{ gen_instr(TID), nop },
};
static constexpr addr_t BASE_ADDR = 0xeba4ad4;
auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
if (!run_checker(memrefs, true,
{ "Missing read records", TID, /*ref_ordinal=*/3,
/*last_timestamp=*/0, /*instrs_since_last_timestamp=*/2 },
"Failed to catch missing read records"))
return false;
}
// Correct: number of write records matches the operand.
{
instr_t *store = XINST_CREATE_store(
GLOBAL_DCONTEXT, OPND_CREATE_MEMPTR(REG2, /*disp=*/0), opnd_create_reg(REG1));
instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
instrlist_append(ilist, store);

std::vector<memref_with_IR_t> memref_instr_vec = {
{ gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
nullptr },
{ gen_instr(TID), store },
{ gen_data(TID, /*load=*/false, /*addr=*/0, /*size=*/0), nullptr }
};

static constexpr addr_t BASE_ADDR = 0xeba4ad4;
auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
if (!run_checker(memrefs, false)) {
return false;
}
}
// Incorrect: too many write records.
{
instr_t *store = XINST_CREATE_store(
GLOBAL_DCONTEXT, OPND_CREATE_MEMPTR(REG2, /*disp=*/0), opnd_create_reg(REG1));
instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
instrlist_append(ilist, store);

std::vector<memref_with_IR_t> memref_instr_vec = {
{ gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
nullptr },
{ gen_instr(TID), store },
{ gen_data(TID, /*load=*/false, /*addr=*/0, /*size=*/0), nullptr },
{ gen_data(TID, /*load=*/false, /*addr=*/0, /*size=*/0), nullptr }
};

static constexpr addr_t BASE_ADDR = 0xeba4ad4;
auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
if (!run_checker(memrefs, true,
{ "Too many write records", TID, /*ref_ordinal=*/4,
/*last_timestamp=*/0,
/*instrs_since_last_timestamp=*/1 },
"Failed to catch too many write records"))
return false;
}
// Incorrect: missing write records.
{
instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT);
instr_t *store = XINST_CREATE_store(
GLOBAL_DCONTEXT, OPND_CREATE_MEMPTR(REG2, /*disp=*/0), opnd_create_reg(REG1));
instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
instrlist_append(ilist, store);
instrlist_append(ilist, nop);

std::vector<memref_with_IR_t> memref_instr_vec = {
{ gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
nullptr },
{ gen_instr(TID), store },
{ gen_instr(TID), nop },
};

static constexpr addr_t BASE_ADDR = 0xeba4ad4;
auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
if (!run_checker(memrefs, true,
{ "Missing write records", TID, /*ref_ordinal=*/3,
/*last_timestamp=*/0,
/*instrs_since_last_timestamp=*/2 },
"Fail to catch missing write records"))
return false;
}
#if defined(X86_64) || defined(X86_32)
// Correct: number of read and write records matches the operand.
{
instr_t *movs = INSTR_CREATE_movs_1(GLOBAL_DCONTEXT);
instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
instrlist_append(ilist, movs);

std::vector<memref_with_IR_t> memref_instr_vec = {
{ gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
nullptr },
{ gen_instr(TID), movs },
{ gen_data(TID, /*load=*/true, /*addr=*/0, /*size=*/0), nullptr },
{ gen_data(TID, /*load=*/false, /*addr=*/0, /*size=*/0), nullptr }
};

static constexpr addr_t BASE_ADDR = 0xeba4ad4;
auto memrefs = add_encodings_to_memrefs(ilist, memref_instr_vec, BASE_ADDR);
instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
if (!run_checker(memrefs, false)) {
return false;
}
}
// Correct: handle cache flush operand correctly.
{
instr_t *clflush = INSTR_CREATE_clflush(
GLOBAL_DCONTEXT, OPND_CREATE_MEM_clflush(REG1, REG_NULL, 0, 0));
instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
instrlist_append(ilist, clflush);
static constexpr addr_t BASE_ADDR = 0xeba4ad4;
std::vector<memref_with_IR_t> memref_setup = {
{ gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
nullptr },
{ gen_instr(TID), clflush },
{ gen_addr(TID, /*type=*/TRACE_TYPE_DATA_FLUSH, /*addr=*/0, /*size=*/0),
nullptr },
};
auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR);
instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
if (!run_checker(memrefs, false)) {
return false;
}
}
// Correct: ignore predicated operands which may not have memory access.
{
instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT);
instr_t *rep_movs = INSTR_CREATE_rep_movs_1(GLOBAL_DCONTEXT);
instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
instrlist_append(ilist, rep_movs);
instrlist_append(ilist, nop);
static constexpr addr_t BASE_ADDR = 0xeba4ad4;
std::vector<memref_with_IR_t> memref_setup = {
{ gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
nullptr },
{ gen_instr(TID), rep_movs },
{ gen_instr(TID), nop },
};
auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR);
instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
if (!run_checker(memrefs, false)) {
return false;
}
}
// Correct: ignore operands with opcode which do not have real memory
// access.
{
instr_t *lea =
INSTR_CREATE_lea(GLOBAL_DCONTEXT, opnd_create_reg(REG1),
opnd_create_base_disp(REG1, REG_NULL, 0, 1, OPSZ_lea));
instr_t *nop = XINST_CREATE_nop(GLOBAL_DCONTEXT);
instrlist_t *ilist = instrlist_create(GLOBAL_DCONTEXT);
instrlist_append(ilist, lea);
instrlist_append(ilist, nop);
static constexpr addr_t BASE_ADDR = 0xeba4ad4;
std::vector<memref_with_IR_t> memref_setup = {
{ gen_marker(TID, TRACE_MARKER_TYPE_FILETYPE, OFFLINE_FILE_TYPE_ENCODINGS),
nullptr },
{ gen_instr(TID), lea },
{ gen_instr(TID), nop },
};
auto memrefs = add_encodings_to_memrefs(ilist, memref_setup, BASE_ADDR);
instrlist_clear_and_destroy(GLOBAL_DCONTEXT, ilist);
if (!run_checker(memrefs, false)) {
return false;
}
}
ivankyluk marked this conversation as resolved.
Show resolved Hide resolved
ivankyluk marked this conversation as resolved.
Show resolved Hide resolved
#endif
return true;
}

int
test_main(int argc, const char *argv[])
{
Expand All @@ -1963,7 +2209,8 @@ test_main(int argc, const char *argv[])
check_duplicate_syscall_with_same_pc() && check_syscalls() &&
check_rseq_side_exit_discontinuity() && check_schedule_file() &&
check_branch_decoration() && check_filter_endpoint() &&
check_timestamps_increase_monotonically()) {
check_timestamps_increase_monotonically() &&
check_read_write_records_match_operands()) {
std::cerr << "invariant_checker_test passed\n";
return 0;
}
Expand Down
11 changes: 11 additions & 0 deletions clients/drcachesim/tests/memref_gen.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@ struct memref_with_IR_t {
// for instrs created as such.
};

inline memref_t
gen_addr(memref_tid_t tid, trace_type_t type, addr_t addr, size_t size = 1)
{
memref_t memref = {};
memref.instr.type = type;
memref.instr.tid = tid;
memref.instr.addr = addr;
memref.instr.size = size;
return memref;
}

inline memref_t
gen_data(memref_tid_t tid, bool load, addr_t addr, size_t size)
{
Expand Down
Loading