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.

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

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

Merged
merged 14 commits into from
Sep 13, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
cd10f6b
i#5975: Add an invariant check for the number of memory read/write re…
ivankyluk Aug 28, 2023
45e5a52
i#5975: Add an invariant check for the number of memory read/write re…
ivankyluk Aug 28, 2023
516cb6f
Fix arm64 tests.
ivankyluk Aug 28, 2023
551568b
Remove changes related to x86 rep instrurctions which were submiited …
ivankyluk Sep 5, 2023
dcd0064
Merge branch 'master' into i5975-read-write-records-match-operands-check
ivankyluk Sep 5, 2023
3872028
Merge branch 'master' into i5975-read-write-records-match-operands-check
ivankyluk Sep 6, 2023
ac2e60d
Updated the code to address review comments.
ivankyluk Sep 7, 2023
c15d815
Merge branch 'master' into i5975-read-write-records-match-operands-check
ivankyluk Sep 7, 2023
da18c97
Reverted change in gen_instr_encoded(), and updated change log.
ivankyluk Sep 7, 2023
d333045
Corrected the format of gen_instr_encoded().
ivankyluk Sep 7, 2023
ee70bfc
Reverted the #include changes in instr_shared.c.
ivankyluk Sep 7, 2023
3a21224
Merge branch 'master' into i5975-read-write-records-match-operands-check
ivankyluk Sep 7, 2023
02ca885
Addressed review comments and changed the code to handle cache flush …
ivankyluk Sep 12, 2023
43bec75
Merge branch 'master' into i5975-read-write-records-match-operands-check
ivankyluk Sep 12, 2023
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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