Skip to content

Commit

Permalink
i#6495 syscall templs: More invariant adaptation (#6768)
Browse files Browse the repository at this point in the history 
Adapts more invariant checks to the kernel system call trace templates
collected on QEMU.

Skips tracking the retaddr_stack_ for calls and rets inside the system
call trace part, since this is used only to verify the function marker
return address which is user-space only.

Allows reads following xsave which apparently also reads the header of
the xsave area
(https://www.felixcloutier.com/x86/xsave).

Relaxes the check that verifies that the instr preceding the syscall
trace is a system call, for the case when there are no encodings in the
trace. This is particularly useful for some unit test setup.

Issue: #6495
  • Loading branch information
@abhinav92003
abhinav92003 authored Apr 11, 2024
1 parent 7d1a9ef commit e75ab23
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 50 deletions.
82 changes: 70 additions & 12 deletions clients/drcachesim/tests/invariant_checker_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3279,13 +3279,20 @@ check_kernel_syscall_trace(void)
instr_t *xsaves = INSTR_CREATE_xsaves64(
GLOBAL_DCONTEXT,
opnd_create_base_disp(DR_REG_XCX, DR_REG_NULL, 0, 0, OPSZ_xsave));
instr_t *xsaveopt = INSTR_CREATE_xsaveopt64(
GLOBAL_DCONTEXT,
opnd_create_base_disp(DR_REG_XCX, DR_REG_NULL, 0, 0, OPSZ_xsave));

# else
instr_t *xrstors = INSTR_CREATE_xrstors32(
GLOBAL_DCONTEXT,
opnd_create_base_disp(DR_REG_XCX, DR_REG_NULL, 0, 0, OPSZ_xsave));
instr_t *xsaves = INSTR_CREATE_xsaves32(
GLOBAL_DCONTEXT,
opnd_create_base_disp(DR_REG_XCX, DR_REG_NULL, 0, 0, OPSZ_xsave));
instr_t *xsaveopt = INSTR_CREATE_xsaveopt32(
GLOBAL_DCONTEXT,
opnd_create_base_disp(DR_REG_XCX, DR_REG_NULL, 0, 0, OPSZ_xsave));
# endif
instr_t *hlt = INSTR_CREATE_hlt(GLOBAL_DCONTEXT);
instr_t *nop3 = XINST_CREATE_nop(GLOBAL_DCONTEXT);
Expand All @@ -3306,6 +3313,7 @@ check_kernel_syscall_trace(void)
instrlist_append(ilist2, nop2);
instrlist_append(ilist2, xrstors);
instrlist_append(ilist2, xsaves);
instrlist_append(ilist2, xsaveopt);
instrlist_append(ilist2, hlt);
instrlist_append(ilist2, nop3);
instrlist_append(ilist2, prefetch);
Expand All @@ -3330,25 +3338,31 @@ check_kernel_syscall_trace(void)
{ gen_instr(TID_A), move1 },
{ gen_instr(TID_A), iret },
// Multiple reads. Acceptable because of the prior iret.
{ gen_data(TID_A, true, 42, 8), nullptr },
{ gen_data(TID_A, true, 42, 8), nullptr },
{ gen_data(TID_A, true, 42, 8), nullptr },
{ gen_data(TID_A, true, 42, 8), nullptr },
{ gen_data(TID_A, true, 4000, 8), nullptr },
{ gen_data(TID_A, true, 4008, 8), nullptr },
{ gen_data(TID_A, true, 4016, 8), nullptr },
{ gen_data(TID_A, true, 4024, 8), nullptr },
{ gen_instr(TID_A), sti },
{ gen_instr(TID_A), nop1 },
// Missing nop2. Acceptable because of the recent sti.
{ gen_instr(TID_A), xrstors },
// Multiple reads. Acceptable because of the prior xrstors.
{ gen_data(TID_A, true, 42, 8), nullptr },
{ gen_data(TID_A, true, 42, 8), nullptr },
{ gen_data(TID_A, true, 42, 8), nullptr },
{ gen_data(TID_A, true, 42, 8), nullptr },
{ gen_data(TID_A, true, 4032, 8), nullptr },
{ gen_data(TID_A, true, 4040, 8), nullptr },
{ gen_data(TID_A, true, 4048, 8), nullptr },
{ gen_data(TID_A, true, 4056, 8), nullptr },
{ gen_instr(TID_A), xsaves },
// Multiple writes. Acceptable because of the prior xsaves.
{ gen_data(TID_A, false, 42, 8), nullptr },
{ gen_data(TID_A, false, 42, 8), nullptr },
{ gen_data(TID_A, false, 42, 8), nullptr },
{ gen_data(TID_A, false, 42, 8), nullptr },
{ gen_data(TID_A, false, 4064, 8), nullptr },
{ gen_data(TID_A, false, 4072, 8), nullptr },
{ gen_data(TID_A, false, 4080, 8), nullptr },
{ gen_data(TID_A, false, 4088, 8), nullptr },
{ gen_instr(TID_A), xsaveopt },
// Multiple writes and a read. Acceptable because of the prior xsaveopt.
{ gen_data(TID_A, false, 4096, 8), nullptr },
{ gen_data(TID_A, false, 4104, 8), nullptr },
{ gen_data(TID_A, true, 4112, 8), nullptr },
{ gen_data(TID_A, false, 4120, 8), nullptr },
{ gen_instr(TID_A), hlt },
// Missing nop3. Acceptable because of the prior hlt.
{ gen_instr(TID_A), prefetch },
Expand All @@ -3370,6 +3384,50 @@ check_kernel_syscall_trace(void)
if (!run_checker(memrefs, false))
res = false;
}
{
std::vector<memref_with_IR_t> memref_instr_vec = {
{ gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE,
OFFLINE_FILE_TYPE_ENCODINGS |
OFFLINE_FILE_TYPE_SYSCALL_NUMBERS |
OFFLINE_FILE_TYPE_KERNEL_SYSCALLS),
nullptr },
{ gen_marker(TID_A, TRACE_MARKER_TYPE_CACHE_LINE_SIZE, 64), nullptr },
{ gen_marker(TID_A, TRACE_MARKER_TYPE_PAGE_SIZE, 4096), nullptr },
{ gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL, 42), nullptr },
{ gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_START, 42), nullptr },
{ gen_instr(TID_A), move1 },
{ gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, 42), nullptr },
{ gen_exit(TID_A), nullptr }
};
auto memrefs = add_encodings_to_memrefs(ilist2, memref_instr_vec, BASE_ADDR);
if (!run_checker(
memrefs, true,
{ "prev_instr at syscall trace start is not a syscall",
/*tid=*/TID_A,
/*ref_ordinal=*/5, /*last_timestamp=*/0,
/*instrs_since_last_timestamp=*/0 },
"Failed to catch missing syscall instr before syscall trace"))
res = false;
}
{
std::vector<memref_t> memrefs = {
gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE,
OFFLINE_FILE_TYPE_SYSCALL_NUMBERS |
OFFLINE_FILE_TYPE_KERNEL_SYSCALLS),
gen_marker(TID_A, TRACE_MARKER_TYPE_CACHE_LINE_SIZE, 64),
gen_marker(TID_A, TRACE_MARKER_TYPE_PAGE_SIZE, 4096),
// Since the file type does not indicate presence of encodings, we do
// not need this instr to be a system call.
gen_instr(TID_A),
gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL, 42),
gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_START, 42),
gen_instr(TID_A),
gen_marker(TID_A, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, 42),
gen_exit(TID_A),
};
if (!run_checker(memrefs, false))
return false;
}
{
std::vector<memref_with_IR_t> memref_instr_vec = {
{ gen_marker(TID_A, TRACE_MARKER_TYPE_FILETYPE,
Expand Down
104 changes: 66 additions & 38 deletions clients/drcachesim/tools/invariant_checker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,54 @@ invariant_checker_t::is_a_unit_test(per_shard_t *shard)
return shard->stream == nullptr || shard->stream->get_input_interface() == nullptr;
}

#ifdef X86
bool
invariant_checker_t::relax_expected_write_count_check_for_kernel(per_shard_t *shard)
{
if (!shard->between_kernel_syscall_trace_markers_ &&
!shard->between_kernel_context_switch_markers_)
return false;
bool relax =
// Xsave does multiple writes. Write count cannot be determined using the
// decoder. System call trace templates collected on QEMU would show all writes.
// XXX: Perhaps we can query the expected size using cpuid and add it as a
// marker in the system call trace template, and then adjust it according to the
// cpuid value for the trace being injected into.
shard->prev_instr_.decoding.is_xsave;
return relax;
}

bool
invariant_checker_t::relax_expected_read_count_check_for_kernel(per_shard_t *shard)
{
if (!shard->between_kernel_syscall_trace_markers_ &&
!shard->between_kernel_context_switch_markers_)
return false;
bool relax =
// iret pops bunch of data from the stack at interrupt returns. System call
// trace templates collected on QEMU would show all reads.
// TODO i#6742: iret has different behavior in user vs protected mode. This
// difference in iret behavior can be detected in the decoder and perhaps be
// accounted for here in a way better than relying on
// between_kernel_syscall_trace_markers_.
shard->prev_instr_.decoding.opcode == OP_iret

// Xrstor does multiple reads. Read count cannot be determined using the
// decoder. System call trace templates collected on QEMU would show all reads.
// XXX: Perhaps we can query the expected size using cpuid and add it as a
// marker in the system call trace template, and then adjust it according to the
// cpuid value for the trace being injected into.
|| shard->prev_instr_.decoding.is_xrstor

// xsave variants also read some fields from the xsave header
// (https://www.felixcloutier.com/x86/xsaveopt).
// XXX, i#6769: Same as above, can we store any metadata in the trace to allow
// us to adapt the decoder to expect this?
|| shard->prev_instr_.decoding.is_xsave;
return relax;
}
#endif

bool
invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &memref)
{
Expand Down Expand Up @@ -506,7 +554,9 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem
shard->last_syscall_marker_value_ ==
static_cast<int>(memref.marker.marker_value),
"Mismatching syscall num in trace start and syscall marker");
report_if_false(shard, shard->prev_instr_.decoding.is_syscall,
report_if_false(shard,
!TESTANY(OFFLINE_FILE_TYPE_ENCODINGS, shard->file_type_) ||
shard->prev_instr_.decoding.is_syscall,
"prev_instr at syscall trace start is not a syscall");
}
shard->pre_syscall_trace_instr_ = shard->prev_instr_;
Expand Down Expand Up @@ -790,13 +840,19 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem
if (shard->instrs_until_interrupt_ > 0)
--shard->instrs_until_interrupt_;
#endif
if (memref.instr.type == TRACE_TYPE_INSTR_DIRECT_CALL ||
memref.instr.type == TRACE_TYPE_INSTR_INDIRECT_CALL) {
shard->retaddr_stack_.push(memref.instr.addr + memref.instr.size);
}
if (memref.instr.type == TRACE_TYPE_INSTR_RETURN) {
if (!shard->retaddr_stack_.empty()) {
shard->retaddr_stack_.pop();

// retaddr_stack_ is used for verifying invariants related to the function
// return markers; these are only user-space.
if (!(shard->between_kernel_context_switch_markers_ ||
shard->between_kernel_syscall_trace_markers_)) {
if (memref.instr.type == TRACE_TYPE_INSTR_DIRECT_CALL ||
memref.instr.type == TRACE_TYPE_INSTR_INDIRECT_CALL) {
shard->retaddr_stack_.push(memref.instr.addr + memref.instr.size);
}
if (memref.instr.type == TRACE_TYPE_INSTR_RETURN) {
if (!shard->retaddr_stack_.empty()) {
shard->retaddr_stack_.pop();
}
}
}
// Invariant: offline traces guarantee that a branch target must immediately
Expand Down Expand Up @@ -1092,26 +1148,7 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem
report_if_false(
shard,
shard->expected_read_records_ >
0 IF_X86(
// iret pops bunch of data from the stack at interrupt
// returns. System call trace templates collected on
// QEMU would show all reads.
// TODO i#6742: iret has different behavior in user vs
// protected mode. This difference in iret behavior can
// be detected in the decoder and perhaps be accounted
// for here in a way better than relying on
// between_kernel_syscall_trace_markers_.
|| (shard->between_kernel_syscall_trace_markers_ &&
shard->prev_instr_.decoding.opcode == OP_iret)
// Xrstor does multiple reads. Read count cannot be
// determined using the decoder. System call trace
// templates collected on QEMU would show all reads.
// XXX: Perhaps we can query the expected size using
// cpuid and add it as a marker in the system call
// trace template, and then adjust it according to the
// cpuid value for the trace being injected into.
|| (shard->between_kernel_syscall_trace_markers_ &&
shard->prev_instr_.decoding.is_xrstor)),
0 IF_X86(|| relax_expected_read_count_check_for_kernel(shard)),
"Too many read records");
if (shard->expected_read_records_ > 0) {
shard->expected_read_records_--;
Expand All @@ -1121,16 +1158,7 @@ invariant_checker_t::parallel_shard_memref(void *shard_data, const memref_t &mem
report_if_false(
shard,
shard->expected_write_records_ >
0 IF_X86(
// Xsave does multiple writes. Write count cannot be
// determined using the decoder. System call trace
// templates collected on QEMU would show all writes.
// XXX: Perhaps we can query the expected size using
// cpuid and add it as a marker in the system call
// trace template, and then adjust it according to the
// cpuid value for the trace being injected into.
|| (shard->between_kernel_syscall_trace_markers_ &&
shard->prev_instr_.decoding.is_xsave)),
0 IF_X86(|| relax_expected_write_count_check_for_kernel(shard)),
"Too many write records");
if (shard->expected_write_records_ > 0) {
shard->expected_write_records_--;
Expand Down
12 changes: 12 additions & 0 deletions clients/drcachesim/tools/invariant_checker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,18 @@ class invariant_checker_t : public analysis_tool_t {
const per_shard_t::instr_info_t &cur_memref_info,
bool expect_encoding, bool at_kernel_event);

#ifdef X86
// Whether the expected write entry count check should be relaxed for the kernel
// part of the trace.
bool
relax_expected_write_count_check_for_kernel(per_shard_t *shard);

// Whether the expected read entry count check should be relaxed for the kernel
// part of the trace.
bool
relax_expected_read_count_check_for_kernel(per_shard_t *shard);
#endif

void *drcontext_ = dr_standalone_init();
std::unordered_map<int, std::unique_ptr<per_shard_t>> shard_map_;
// This mutex is only needed in parallel_shard_init. In all other accesses to
Expand Down

0 comments on commit e75ab23

Please sign in to comment.