[PATCH] linux-user/i386: report the real trap number in sigcontext.trapno

Nat Brown <[email protected]>
Newsgroups org.nongnu.qemu-devel
Message-ID <[email protected]>
setup_sigcontext() fills sigcontext.trapno from CPUState::exception_index, but
by the time a signal frame is built that field has already been reset to -1:
cpu_exec() copies the exception number into its return value and clears
exception_index before returning to cpu_loop(), which is where the signal is
queued.  Guests therefore see trapno == -1 for every cpu exception.

  #include <signal.h>
  #include <stdio.h>
  #include <ucontext.h>

  static void h(int sig, siginfo_t *si, void *uc_)
  {
      ucontext_t *uc = uc_;
      printf("sig=%d trapno=%d err=%#x\n", sig,
             (int)uc->uc_mcontext.gregs[REG_TRAPNO],
             (unsigned)uc->uc_mcontext.gregs[REG_ERR]);
      fflush(stdout);
      _exit(0);
  }

  int main(void)
  {
      struct sigaction sa = { .sa_sigaction = h, .sa_flags = SA_SIGINFO };
      sigaction(SIGSEGV, &sa, 0);
      *(volatile int *)0 = 1;
  }

Natively this prints trapno=14 err=0x6; under qemu it prints trapno=-1 err=0x6.
A SIGILL likewise reports -1 instead of 6, and SIGFPE -1 instead of 0.  Both
qemu-i386 and qemu-x86_64 are affected, and both report the expected 14/6/0
with this patch applied.  Only trapno is wrong; error_code lives in
CPUX86State and survives.

x86_cpu_record_sigsegv() already notes the coupling, observing that we cannot
let the caller clobber exception_index "short of inventing a new place to store
the trapno".  Invent it: record the exception in env->trap_nr, mirroring linux's
thread.trap_nr, which is exactly what the kernel reports in sigcontext.trapno
and which likewise persists beyond the exception that set it.  Only hardware
vectors are recorded; the EXCP_* values at 0x100 and above are emulation
internals and never appear in a signal frame.

This matters to wine, whose segv_handler() dispatches on the trap number and
cannot service a fault it sees as -1, so 32-bit module loading fails under
qemu-i386.  The equivalent 64-bit handler dispatches the same way, so the same
failure is expected under qemu-x86_64.  Discussed at
https://gitlab.winehq.org/wine/wine/-/merge_requests/11737 , where the
suggestion was that qemu is the right place to fix this.

Signed-off-by: Nat Brown <[email protected]>
---
 linux-user/i386/cpu_loop.c         |  9 +++++++++
 linux-user/i386/signal.c           |  6 ++----
 target/i386/cpu.h                  | 10 ++++++++++
 target/i386/tcg/user/excp_helper.c |  8 ++++----
 4 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
index fe922fceb5..24fdcc9b24 100644
--- a/linux-user/i386/cpu_loop.c
+++ b/linux-user/i386/cpu_loop.c
@@ -218,6 +218,15 @@ void cpu_loop(CPUX86State *env)
         cpu_exec_end(cs);
         qemu_process_cpu_events(cs);
 
+        /*
+         * Remember the exception for sigcontext.trapno, as linux does in
+         * thread.trap_nr.  Only hardware exception vectors qualify; the
+         * EXCP_* values at 0x100 and above are emulation internals.
+         */
+        if (trapnr >= EXCP00_DIVZ && trapnr <= EXCP12_MCHK) {
+            env->trap_nr = trapnr;
+        }
+
         switch(trapnr) {
         case 0x80:
 #ifndef TARGET_X86_64
diff --git a/linux-user/i386/signal.c b/linux-user/i386/signal.c
index b646fde431..9650fac940 100644
--- a/linux-user/i386/signal.c
+++ b/linux-user/i386/signal.c
@@ -367,8 +367,6 @@ static void setup_sigcontext(CPUX86State *env,
                              abi_ptr fxstate_addr,
                              abi_ptr fpend_addr)
 {
-    CPUState *cs = env_cpu(env);
-
 #ifndef TARGET_X86_64
     uint16_t magic;
 
@@ -385,7 +383,7 @@ static void setup_sigcontext(CPUX86State *env,
     __put_user(env->regs[R_EDX], &sc->edx);
     __put_user(env->regs[R_ECX], &sc->ecx);
     __put_user(env->regs[R_EAX], &sc->eax);
-    __put_user(cs->exception_index, &sc->trapno);
+    __put_user(env->trap_nr, &sc->trapno);
     __put_user(env->error_code, &sc->err);
     __put_user(env->eip, &sc->eip);
     __put_user(env->segs[R_CS].selector, (uint32_t *)&sc->cs);
@@ -416,7 +414,7 @@ static void setup_sigcontext(CPUX86State *env,
     __put_user(env->regs[14], &sc->r14);
     __put_user(env->regs[15], &sc->r15);
 
-    __put_user(cs->exception_index, &sc->trapno);
+    __put_user(env->trap_nr, &sc->trapno);
     __put_user(env->error_code, &sc->err);
     __put_user(env->eip, &sc->rip);
 
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 641f3ee5c2..cf727b2a56 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2188,6 +2188,16 @@ typedef struct CPUArchState {
 
     /* exception/interrupt handling */
     int error_code;
+#ifdef CONFIG_USER_ONLY
+    /*
+     * The number of the last cpu exception taken by this thread, mirroring
+     * linux's thread.trap_nr, which is what the kernel reports in
+     * sigcontext.trapno.  CPUState::exception_index cannot be used for this:
+     * cpu_exec() resets it before returning, long before the signal frame is
+     * built during delivery.
+     */
+    int trap_nr;
+#endif
     int exception_is_int;
     target_ulong exception_next_eip;
     target_ulong dr[8]; /* debug registers; note dr4 and dr5 are unused */
diff --git a/target/i386/tcg/user/excp_helper.c b/target/i386/tcg/user/excp_helper.c
index 0957ad2e9e..26a4c0ef92 100644
--- a/target/i386/tcg/user/excp_helper.c
+++ b/target/i386/tcg/user/excp_helper.c
@@ -31,10 +31,10 @@ void x86_cpu_record_sigsegv(CPUState *cs, vaddr addr,
 
     /*
      * The error_code that hw reports as part of the exception frame
-     * is copied to linux sigcontext.err.  The exception_index is
-     * copied to linux sigcontext.trapno.  Short of inventing a new
-     * place to store the trapno, we cannot let our caller raise the
-     * signal and set exception_index to EXCP_INTERRUPT.
+     * is copied to linux sigcontext.err.  The trapno reported in
+     * linux sigcontext.trapno is recorded separately in env->trap_nr
+     * by cpu_loop(), since cpu_exec() clears exception_index before
+     * the signal frame is built.
      */
     env->cr[2] = addr;
     env->error_code = (maperr ? 0 : PG_ERROR_P_MASK)
-- 
2.50.1 (Apple Git-155)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.