Skip to content

Commit

Permalink
Make trapped_instruction_at() explicitly x86-specific
Browse files Browse the repository at this point in the history 
There were some code paths where trapped_instruction_at() was
callable on non-x86 platforms; as a result it may have been possible
to reach x86-specific handling of trapped instructions in cases
where the instruction bytes happened to match. Fix it by adding
an architecture check to said code paths, renaming the function to
x86_trapped_instruction_at() (likewise for trapped_instruction_len() and
TrappedInstruction) and adding an assertion that checks the architecture.
  • Loading branch information
@pcc
pcc committed Oct 11, 2024
1 parent deeb983 commit 1692d8a
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 50 deletions.
8 changes: 4 additions & 4 deletions src/DiversionSession.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,10 @@ DiversionSession::DiversionResult DiversionSession::diversion_step(
!result.break_status.singlestep_complete) {
result.break_status.signal = unique_ptr<siginfo_t>(new siginfo_t(t->get_siginfo()));
result.break_status.signal->si_signo = t->stop_sig();
} else if (t->stop_sig() == SIGSEGV) {
auto trapped_instruction = trapped_instruction_at(t, t->ip());
if (trapped_instruction == TrappedInstruction::RDTSC) {
size_t len = trapped_instruction_len(trapped_instruction);
} else if (t->stop_sig() == SIGSEGV && is_x86ish(t->arch())) {
auto trapped_instruction = x86_trapped_instruction_at(t, t->ip());
if (trapped_instruction == X86TrappedInstruction::RDTSC) {
size_t len = x86_trapped_instruction_len(trapped_instruction);
uint64_t rdtsc_value = next_rdtsc_value();
LOG(debug) << "Faking RDTSC instruction with value " << rdtsc_value;
Registers r = t->regs();
Expand Down
4 changes: 2 additions & 2 deletions src/ReplaySession.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ bool ReplaySession::handle_unrecorded_cpuid_fault(
ReplayTask* t, const StepConstraints& constraints) {
if (t->stop_sig() != SIGSEGV || !has_cpuid_faulting() ||
trace_in.uses_cpuid_faulting() ||
trapped_instruction_at(t, t->ip()) != TrappedInstruction::CPUID) {
x86_trapped_instruction_at(t, t->ip()) != X86TrappedInstruction::CPUID) {
return false;
}
// OK, this is a case where we did not record using CPUID faulting but we are
Expand All @@ -522,7 +522,7 @@ bool ReplaySession::handle_unrecorded_cpuid_fault(
<< " CX=" << HEX(r.cx()) << "; defaulting to 0/0/0/0";
r.set_cpuid_output(0, 0, 0, 0);
}
r.set_ip(r.ip() + trapped_instruction_len(TrappedInstruction::CPUID));
r.set_ip(r.ip() + x86_trapped_instruction_len(X86TrappedInstruction::CPUID));
t->set_regs(r);
// Clear SIGSEGV status since we're handling it
t->set_status(constraints.is_singlestep() ? WaitStatus::for_stop_sig(SIGTRAP)
Expand Down
28 changes: 14 additions & 14 deletions src/Task.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1366,18 +1366,18 @@ TrapReasons Task::compute_trap_reasons() {
// step over those instructions so we need to detect that here.
reasons.singlestep = true;
} else {
TrappedInstruction ti =
trapped_instruction_at(this, address_of_last_execution_resume);
if (ti == TrappedInstruction::CPUID &&
X86TrappedInstruction ti =
x86_trapped_instruction_at(this, address_of_last_execution_resume);
if (ti == X86TrappedInstruction::CPUID &&
ip() == address_of_last_execution_resume +
trapped_instruction_len(TrappedInstruction::CPUID)) {
x86_trapped_instruction_len(X86TrappedInstruction::CPUID)) {
// Likewise we emulate CPUID instructions and must forcibly detect that
// here.
reasons.singlestep = true;
// This also takes care of the did_set_breakpoint_after_cpuid workaround case
} else if (ti == TrappedInstruction::INT3 &&
} else if (ti == X86TrappedInstruction::INT3 &&
ip() == address_of_last_execution_resume +
trapped_instruction_len(TrappedInstruction::INT3)) {
x86_trapped_instruction_len(X86TrappedInstruction::INT3)) {
// INT3 instructions should also be turned into a singlestep here.
reasons.singlestep = true;
}
Expand Down Expand Up @@ -1562,12 +1562,12 @@ bool Task::resume_execution(ResumeRequest how, WaitRequest wait_how,
if (is_singlestep_resume(how)) {
work_around_KNL_string_singlestep_bug();
if (is_x86ish(arch())) {
singlestepping_instruction = trapped_instruction_at(this, ip());
if (singlestepping_instruction == TrappedInstruction::CPUID) {
singlestepping_instruction = x86_trapped_instruction_at(this, ip());
if (singlestepping_instruction == X86TrappedInstruction::CPUID) {
// In KVM virtual machines (and maybe others), singlestepping over CPUID
// executes the following instruction as well. Work around that.
did_set_breakpoint_after_cpuid =
vm()->add_breakpoint(ip() + trapped_instruction_len(singlestepping_instruction), BKPT_INTERNAL);
vm()->add_breakpoint(ip() + x86_trapped_instruction_len(singlestepping_instruction), BKPT_INTERNAL);
}
} else if (arch() == aarch64 && is_singlestep_resume(how_last_execution_resumed)) {
// On aarch64, if the last execution was any sort of single step, then
Expand Down Expand Up @@ -2429,7 +2429,7 @@ bool Task::did_waitpid(WaitStatus status) {

if (did_set_breakpoint_after_cpuid) {
remote_code_ptr bkpt_addr =
address_of_last_execution_resume + trapped_instruction_len(singlestepping_instruction);
address_of_last_execution_resume + x86_trapped_instruction_len(singlestepping_instruction);
if (ip().undo_executed_bkpt(arch()) == bkpt_addr) {
Registers r = regs();
r.set_ip(bkpt_addr);
Expand All @@ -2438,18 +2438,18 @@ bool Task::did_waitpid(WaitStatus status) {
vm()->remove_breakpoint(bkpt_addr, BKPT_INTERNAL);
did_set_breakpoint_after_cpuid = false;
}
if ((singlestepping_instruction == TrappedInstruction::PUSHF ||
singlestepping_instruction == TrappedInstruction::PUSHF16) &&
if ((singlestepping_instruction == X86TrappedInstruction::PUSHF ||
singlestepping_instruction == X86TrappedInstruction::PUSHF16) &&
ip() == address_of_last_execution_resume +
trapped_instruction_len(singlestepping_instruction)) {
x86_trapped_instruction_len(singlestepping_instruction)) {
// We singlestepped through a pushf. Clear TF bit on stack.
auto sp = regs().sp().cast<uint16_t>();
// If this address is invalid then we should have segfaulted instead of
// retiring the instruction!
uint16_t val = read_mem(sp);
write_mem(sp, (uint16_t)(val & ~X86_TF_FLAG));
}
singlestepping_instruction = TrappedInstruction::NONE;
singlestepping_instruction = X86TrappedInstruction::NONE;

// We might have singlestepped at the resumption address and just exited
// the kernel without executing the breakpoint at that address.
Expand Down
2 changes: 1 addition & 1 deletion src/Task.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1317,7 +1317,7 @@ class Task {
// Task.cc
uint64_t last_resume_orig_cx;
// The instruction type we're singlestepping through.
TrappedInstruction singlestepping_instruction;
X86TrappedInstruction singlestepping_instruction;
// True if we set a breakpoint after a singlestepped CPUID instruction.
// We need this in addition to `singlestepping_instruction` because that
// might be CPUID but we failed to set the breakpoint.
Expand Down
21 changes: 12 additions & 9 deletions src/record_signal.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,18 @@ static void restore_signal_state(RecordTask* t, int sig,
static bool try_handle_trapped_instruction(RecordTask* t, siginfo_t* si) {
ASSERT(t, si->si_signo == SIGSEGV);

auto trapped_instruction = trapped_instruction_at(t, t->ip());
if (!is_x86ish(t->arch()))
return false;

auto trapped_instruction = x86_trapped_instruction_at(t, t->ip());
switch (trapped_instruction) {
case TrappedInstruction::RDTSC:
case TrappedInstruction::RDTSCP:
case X86TrappedInstruction::RDTSC:
case X86TrappedInstruction::RDTSCP:
if (t->tsc_mode == PR_TSC_SIGSEGV) {
return false;
}
break;
case TrappedInstruction::CPUID:
case X86TrappedInstruction::CPUID:
if (t->cpuid_mode == 0) {
return false;
}
Expand All @@ -95,13 +98,13 @@ static bool try_handle_trapped_instruction(RecordTask* t, siginfo_t* si) {
return false;
}

size_t len = trapped_instruction_len(trapped_instruction);
size_t len = x86_trapped_instruction_len(trapped_instruction);
ASSERT(t, len > 0);

Registers r = t->regs();
if (trapped_instruction == TrappedInstruction::RDTSC ||
trapped_instruction == TrappedInstruction::RDTSCP) {
if (trapped_instruction == TrappedInstruction::RDTSC &&
if (trapped_instruction == X86TrappedInstruction::RDTSC ||
trapped_instruction == X86TrappedInstruction::RDTSCP) {
if (trapped_instruction == X86TrappedInstruction::RDTSC &&
t->vm()->monkeypatcher().try_patch_trapping_instruction(t, len, true)) {
Event ev = Event::patch_syscall();
ev.PatchSyscall().patch_trapping_instruction = true;
Expand All @@ -114,7 +117,7 @@ static bool try_handle_trapped_instruction(RecordTask* t, siginfo_t* si) {
r.set_rdtsc_output(current_time);

LOG(debug) << " trapped for rdtsc: returning " << current_time;
} else if (trapped_instruction == TrappedInstruction::CPUID) {
} else if (trapped_instruction == X86TrappedInstruction::CPUID) {
auto eax = r.syscallno();
auto ecx = r.cx();
auto cpuid_data = cpuid(eax, ecx);
Expand Down
33 changes: 17 additions & 16 deletions src/util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1944,53 +1944,54 @@ static const uint8_t pushf_insn[] = { 0x9c };
static const uint8_t pushf16_insn[] = { 0x66, 0x9c };

// XXX this probably needs to be extended to decode ignored prefixes
TrappedInstruction trapped_instruction_at(Task* t, remote_code_ptr ip) {
X86TrappedInstruction x86_trapped_instruction_at(Task* t, remote_code_ptr ip) {
DEBUG_ASSERT(is_x86ish(t->arch()));
uint8_t insn[sizeof(rdtscp_insn)];
ssize_t ret =
t->read_bytes_fallible(ip.to_data_ptr<uint8_t>(), sizeof(insn), insn);
if (ret < 0) {
return TrappedInstruction::NONE;
return X86TrappedInstruction::NONE;
}
size_t len = ret;
if (len >= sizeof(rdtsc_insn) &&
!memcmp(insn, rdtsc_insn, sizeof(rdtsc_insn))) {
return TrappedInstruction::RDTSC;
return X86TrappedInstruction::RDTSC;
}
if (len >= sizeof(rdtscp_insn) &&
!memcmp(insn, rdtscp_insn, sizeof(rdtscp_insn))) {
return TrappedInstruction::RDTSCP;
return X86TrappedInstruction::RDTSCP;
}
if (len >= sizeof(cpuid_insn) &&
!memcmp(insn, cpuid_insn, sizeof(cpuid_insn))) {
return TrappedInstruction::CPUID;
return X86TrappedInstruction::CPUID;
}
if (len >= sizeof(int3_insn) &&
!memcmp(insn, int3_insn, sizeof(int3_insn))) {
return TrappedInstruction::INT3;
return X86TrappedInstruction::INT3;
}
if (len >= sizeof(pushf_insn) &&
!memcmp(insn, pushf_insn, sizeof(pushf_insn))) {
return TrappedInstruction::PUSHF;
return X86TrappedInstruction::PUSHF;
}
if (len >= sizeof(pushf16_insn) &&
!memcmp(insn, pushf16_insn, sizeof(pushf16_insn))) {
return TrappedInstruction::PUSHF16;
return X86TrappedInstruction::PUSHF16;
}
return TrappedInstruction::NONE;
return X86TrappedInstruction::NONE;
}

size_t trapped_instruction_len(TrappedInstruction insn) {
if (insn == TrappedInstruction::RDTSC) {
size_t x86_trapped_instruction_len(X86TrappedInstruction insn) {
if (insn == X86TrappedInstruction::RDTSC) {
return sizeof(rdtsc_insn);
} else if (insn == TrappedInstruction::RDTSCP) {
} else if (insn == X86TrappedInstruction::RDTSCP) {
return sizeof(rdtscp_insn);
} else if (insn == TrappedInstruction::CPUID) {
} else if (insn == X86TrappedInstruction::CPUID) {
return sizeof(cpuid_insn);
} else if (insn == TrappedInstruction::INT3) {
} else if (insn == X86TrappedInstruction::INT3) {
return sizeof(int3_insn);
} else if (insn == TrappedInstruction::PUSHF) {
} else if (insn == X86TrappedInstruction::PUSHF) {
return sizeof(pushf_insn);
} else if (insn == TrappedInstruction::PUSHF16) {
} else if (insn == X86TrappedInstruction::PUSHF16) {
return sizeof(pushf16_insn);
} else {
return 0;
Expand Down
8 changes: 4 additions & 4 deletions src/util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ std::vector<std::string> current_env();
*/
int get_num_cpus();

enum class TrappedInstruction {
enum class X86TrappedInstruction {
NONE = 0,
RDTSC = 1,
RDTSCP = 2,
Expand All @@ -497,12 +497,12 @@ enum class TrappedInstruction {
};

/* If |t->ip()| points at a decoded instruction, return the instruction */
TrappedInstruction trapped_instruction_at(Task* t, remote_code_ptr ip);
X86TrappedInstruction x86_trapped_instruction_at(Task* t, remote_code_ptr ip);

extern const uint8_t rdtsc_insn[2];

/* Return the length of the TrappedInstruction */
size_t trapped_instruction_len(TrappedInstruction insn);
/* Return the length of the X86TrappedInstruction */
size_t x86_trapped_instruction_len(X86TrappedInstruction insn);

/**
* Certain instructions generate deterministic signals but also advance pc.
Expand Down

0 comments on commit 1692d8a

Please sign in to comment.