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

@@ -227,9 +227,9 @@ DiversionSession::DiversionResult DiversionSession::diversion_step(
         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);
+        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();

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)

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,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.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<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.opcode = SpecialInstOpcode::NONE;
 
     // We might have singlestepped at the resumption address and just exited
     // the kernel without executing the breakpoint at that address.

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.

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);

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<uint8_t>(), 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<uint8_t>(), 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;

src/util.h

@@ -486,23 +486,27 @@ std::vector<std::string> 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.