Skip to content

Commit

Permalink
i#5975: Add invariant check that read/write records match operands (#…
Browse files Browse the repository at this point in the history 
…6283)

Add an invariant check to verify the number of memory read/write records
is matching the operands.

Update check_sane_control_flow() to use instrlist_t and memref_with_IR_t
to avoid hardcoded encodings. Current tests fail the new check since
they have memory accesses.

Fixes #5975
  • Loading branch information
@ivankyluk
ivankyluk authored Sep 13, 2023
1 parent d6cceb0 commit 15da83a
Show file tree
Hide file tree
Showing 8 changed files with 408 additions and 30 deletions.
3 changes: 3 additions & 0 deletions api/docs/release.dox
View file
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).
- 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
View file
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;
}

/**
* 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
View file
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;
}
}
#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
View file
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

0 comments on commit 15da83a

Please sign in to comment.