From a0b75c30e0937c379ed64629abba0fb3bda86833 Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Tue, 15 Oct 2024 12:31:28 -0700 Subject: [PATCH] Make existing special instruction handling x86-specific 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. Moreover we plan to introduce ARM-specific special instruction handling as part of the fix for #3740. The ARM special instructions take a register operand unlike the x86 instructions so this will need to return more than just an enum. And as pointed out, TrappedInstruction is used not only for instructions that we trap on but also instructions that we handle specially such as PUSHF. Therefore, add an architecture check to trapped_instruction_at() (now special_instruction_at()), make it return a struct (for now just containing the opcode enum), rename the existing enumerators to be prefixed with X86_ and, while we're here, replace "trapped" with "special". --- src/DiversionSession.cc | 6 ++-- src/ReplaySession.cc | 4 +-- src/Task.cc | 28 +++++++-------- src/Task.h | 2 +- src/record_signal.cc | 20 +++++------ src/util.cc | 80 +++++++++++++++++++++-------------------- src/util.h | 24 +++++++------ 7 files changed, 85 insertions(+), 79 deletions(-) diff --git a/src/DiversionSession.cc b/src/DiversionSession.cc index 99f29b434ab..1bf3ac0fbbf 100644 --- a/src/DiversionSession.cc +++ b/src/DiversionSession.cc @@ -227,9 +227,9 @@ DiversionSession::DiversionResult DiversionSession::diversion_step( result.break_status.signal = unique_ptr(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); + auto special_instruction = special_instruction_at(t, t->ip()); + if (special_instruction.opcode == SpecialInstOpcode::X86_RDTSC) { + size_t len = special_instruction_len(special_instruction.opcode); uint64_t rdtsc_value = next_rdtsc_value(); LOG(debug) << "Faking RDTSC instruction with value " << rdtsc_value; Registers r = t->regs(); diff --git a/src/ReplaySession.cc b/src/ReplaySession.cc index cffac2efea1..effff2b5260 100644 --- a/src/ReplaySession.cc +++ b/src/ReplaySession.cc @@ -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) { + special_instruction_at(t, t->ip()).opcode != SpecialInstOpcode::X86_CPUID) { return false; } // OK, this is a case where we did not record using CPUID faulting but we are @@ -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() + special_instruction_len(SpecialInstOpcode::X86_CPUID)); t->set_regs(r); // Clear SIGSEGV status since we're handling it t->set_status(constraints.is_singlestep() ? WaitStatus::for_stop_sig(SIGTRAP) diff --git a/src/Task.cc b/src/Task.cc index 12c4135c834..fb14eb7cff3 100644 --- a/src/Task.cc +++ b/src/Task.cc @@ -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 && + SpecialInst si = + special_instruction_at(this, address_of_last_execution_resume); + if (si.opcode == SpecialInstOpcode::X86_CPUID && ip() == address_of_last_execution_resume + - trapped_instruction_len(TrappedInstruction::CPUID)) { + special_instruction_len(SpecialInstOpcode::X86_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 (si.opcode == SpecialInstOpcode::X86_INT3 && ip() == address_of_last_execution_resume + - trapped_instruction_len(TrappedInstruction::INT3)) { + special_instruction_len(SpecialInstOpcode::X86_INT3)) { // INT3 instructions should also be turned into a singlestep here. reasons.singlestep = true; } @@ -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 = special_instruction_at(this, ip()); + if (singlestepping_instruction.opcode == SpecialInstOpcode::X86_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() + special_instruction_len(singlestepping_instruction.opcode), 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 @@ -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 + special_instruction_len(singlestepping_instruction.opcode); if (ip().undo_executed_bkpt(arch()) == bkpt_addr) { Registers r = regs(); r.set_ip(bkpt_addr); @@ -2438,10 +2438,10 @@ 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.opcode == SpecialInstOpcode::X86_PUSHF || + singlestepping_instruction.opcode == SpecialInstOpcode::X86_PUSHF16) && ip() == address_of_last_execution_resume + - trapped_instruction_len(singlestepping_instruction)) { + special_instruction_len(singlestepping_instruction.opcode)) { // We singlestepped through a pushf. Clear TF bit on stack. auto sp = regs().sp().cast(); // If this address is invalid then we should have segfaulted instead of @@ -2449,7 +2449,7 @@ bool Task::did_waitpid(WaitStatus status) { uint16_t val = read_mem(sp); write_mem(sp, (uint16_t)(val & ~X86_TF_FLAG)); } - singlestepping_instruction = TrappedInstruction::NONE; + singlestepping_instruction.opcode = SpecialInstOpcode::NONE; // We might have singlestepped at the resumption address and just exited // the kernel without executing the breakpoint at that address. diff --git a/src/Task.h b/src/Task.h index 256c4d803f8..a3b90e35945 100644 --- a/src/Task.h +++ b/src/Task.h @@ -1317,7 +1317,7 @@ class Task { // Task.cc uint64_t last_resume_orig_cx; // The instruction type we're singlestepping through. - TrappedInstruction singlestepping_instruction; + SpecialInst 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. diff --git a/src/record_signal.cc b/src/record_signal.cc index 5ef873fa3dd..0aef790dd3b 100644 --- a/src/record_signal.cc +++ b/src/record_signal.cc @@ -78,15 +78,15 @@ 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()); - switch (trapped_instruction) { - case TrappedInstruction::RDTSC: - case TrappedInstruction::RDTSCP: + auto special_instruction = special_instruction_at(t, t->ip()); + switch (special_instruction.opcode) { + case SpecialInstOpcode::X86_RDTSC: + case SpecialInstOpcode::X86_RDTSCP: if (t->tsc_mode == PR_TSC_SIGSEGV) { return false; } break; - case TrappedInstruction::CPUID: + case SpecialInstOpcode::X86_CPUID: if (t->cpuid_mode == 0) { return false; } @@ -95,13 +95,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 = special_instruction_len(special_instruction.opcode); ASSERT(t, len > 0); Registers r = t->regs(); - if (trapped_instruction == TrappedInstruction::RDTSC || - trapped_instruction == TrappedInstruction::RDTSCP) { - if (trapped_instruction == TrappedInstruction::RDTSC && + if (special_instruction.opcode == SpecialInstOpcode::X86_RDTSC || + special_instruction.opcode == SpecialInstOpcode::X86_RDTSCP) { + if (special_instruction.opcode == SpecialInstOpcode::X86_RDTSC && t->vm()->monkeypatcher().try_patch_trapping_instruction(t, len, true)) { Event ev = Event::patch_syscall(); ev.PatchSyscall().patch_trapping_instruction = true; @@ -114,7 +114,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 (special_instruction.opcode == SpecialInstOpcode::X86_CPUID) { auto eax = r.syscallno(); auto ecx = r.cx(); auto cpuid_data = cpuid(eax, ecx); diff --git a/src/util.cc b/src/util.cc index e691b183f74..1ddcc709217 100644 --- a/src/util.cc +++ b/src/util.cc @@ -1947,53 +1947,55 @@ 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) { - uint8_t insn[sizeof(rdtscp_insn)]; - ssize_t ret = - t->read_bytes_fallible(ip.to_data_ptr(), sizeof(insn), insn); - if (ret < 0) { - return TrappedInstruction::NONE; - } - size_t len = ret; - if (len >= sizeof(rdtsc_insn) && - !memcmp(insn, rdtsc_insn, sizeof(rdtsc_insn))) { - return TrappedInstruction::RDTSC; - } - if (len >= sizeof(rdtscp_insn) && - !memcmp(insn, rdtscp_insn, sizeof(rdtscp_insn))) { - return TrappedInstruction::RDTSCP; - } - if (len >= sizeof(cpuid_insn) && - !memcmp(insn, cpuid_insn, sizeof(cpuid_insn))) { - return TrappedInstruction::CPUID; - } - if (len >= sizeof(int3_insn) && - !memcmp(insn, int3_insn, sizeof(int3_insn))) { - return TrappedInstruction::INT3; - } - if (len >= sizeof(pushf_insn) && - !memcmp(insn, pushf_insn, sizeof(pushf_insn))) { - return TrappedInstruction::PUSHF; - } - if (len >= sizeof(pushf16_insn) && - !memcmp(insn, pushf16_insn, sizeof(pushf16_insn))) { - return TrappedInstruction::PUSHF16; +SpecialInst special_instruction_at(Task* t, remote_code_ptr ip) { + if (is_x86ish(t->arch())) { + uint8_t insn[sizeof(rdtscp_insn)]; + ssize_t ret = + t->read_bytes_fallible(ip.to_data_ptr(), sizeof(insn), insn); + if (ret < 0) { + return {SpecialInstOpcode::NONE}; + } + size_t len = ret; + if (len >= sizeof(rdtsc_insn) && + !memcmp(insn, rdtsc_insn, sizeof(rdtsc_insn))) { + return {SpecialInstOpcode::X86_RDTSC}; + } + if (len >= sizeof(rdtscp_insn) && + !memcmp(insn, rdtscp_insn, sizeof(rdtscp_insn))) { + return {SpecialInstOpcode::X86_RDTSCP}; + } + if (len >= sizeof(cpuid_insn) && + !memcmp(insn, cpuid_insn, sizeof(cpuid_insn))) { + return {SpecialInstOpcode::X86_CPUID}; + } + if (len >= sizeof(int3_insn) && + !memcmp(insn, int3_insn, sizeof(int3_insn))) { + return {SpecialInstOpcode::X86_INT3}; + } + if (len >= sizeof(pushf_insn) && + !memcmp(insn, pushf_insn, sizeof(pushf_insn))) { + return {SpecialInstOpcode::X86_PUSHF}; + } + if (len >= sizeof(pushf16_insn) && + !memcmp(insn, pushf16_insn, sizeof(pushf16_insn))) { + return {SpecialInstOpcode::X86_PUSHF16}; + } } - return TrappedInstruction::NONE; + return {SpecialInstOpcode::NONE}; } -size_t trapped_instruction_len(TrappedInstruction insn) { - if (insn == TrappedInstruction::RDTSC) { +size_t special_instruction_len(SpecialInstOpcode insn) { + if (insn == SpecialInstOpcode::X86_RDTSC) { return sizeof(rdtsc_insn); - } else if (insn == TrappedInstruction::RDTSCP) { + } else if (insn == SpecialInstOpcode::X86_RDTSCP) { return sizeof(rdtscp_insn); - } else if (insn == TrappedInstruction::CPUID) { + } else if (insn == SpecialInstOpcode::X86_CPUID) { return sizeof(cpuid_insn); - } else if (insn == TrappedInstruction::INT3) { + } else if (insn == SpecialInstOpcode::X86_INT3) { return sizeof(int3_insn); - } else if (insn == TrappedInstruction::PUSHF) { + } else if (insn == SpecialInstOpcode::X86_PUSHF) { return sizeof(pushf_insn); - } else if (insn == TrappedInstruction::PUSHF16) { + } else if (insn == SpecialInstOpcode::X86_PUSHF16) { return sizeof(pushf16_insn); } else { return 0; diff --git a/src/util.h b/src/util.h index 9a8459dd549..85ac73cf643 100644 --- a/src/util.h +++ b/src/util.h @@ -486,23 +486,27 @@ std::vector current_env(); */ int get_num_cpus(); -enum class TrappedInstruction { - NONE = 0, - RDTSC = 1, - RDTSCP = 2, - CPUID = 3, - INT3 = 4, - PUSHF = 5, - PUSHF16 = 6, +enum class SpecialInstOpcode { + NONE, + X86_RDTSC, + X86_RDTSCP, + X86_CPUID, + X86_INT3, + X86_PUSHF, + X86_PUSHF16, +}; + +struct SpecialInst { + SpecialInstOpcode opcode; }; /* If |t->ip()| points at a decoded instruction, return the instruction */ -TrappedInstruction trapped_instruction_at(Task* t, remote_code_ptr ip); +SpecialInst special_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); +size_t special_instruction_len(SpecialInstOpcode insn); /** * Certain instructions generate deterministic signals but also advance pc.