linux-user/arm: BKPT should cause SIGTRAP, not be a syscall

In linux-user/arm/cpu-loop.c we incorrectly treat EXCP_BKPT similarly
to EXCP_SWI, which means that if the guest executes a BKPT insn then
QEMU will perform a syscall for it (which syscall depends on what
value happens to be in r7...). The correct behaviour is that the
guest process should take a SIGTRAP.

This code has been like this (more or less) since commit
06c949e62a098f in 2006 which added BKPT in the first place.  This is
probably because at the time the same code path was used to handle
both Linux syscalls and semihosting calls, and (on M profile) BKPT
with a suitable magic number is used for semihosting calls.  But
these days we've moved handling of semihosting out to an entirely
different codepath, so we can fix this bug by simply removing this
handling of EXCP_BKPT and instead making it deliver a SIGTRAP like
EXCP_DEBUG (as we do already on aarch64).

Reported-by: <omerg681@gmail.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20200420212206.12776-2-peter.maydell@linaro.org
Fixes: https://bugs.launchpad.net/qemu/+bug/1873898
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index cf618daa1ca5937237be43549aff79342c70e726..82d0dd3c312730f424495356dd96b572563e861e 100644 (file)
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -295,32 +295,17 @@ void cpu_loop(CPUARMState *env)
             }
             break;
         case EXCP_SWI:
-        case EXCP_BKPT:
             {
                 env->eabi = 1;
                 /* system call */
-                if (trapnr == EXCP_BKPT) {
-                    if (env->thumb) {
-                        /* FIXME - what to do if get_user() fails? */
-                        get_user_code_u16(insn, env->regs[15], env);
-                        n = insn & 0xff;
-                        env->regs[15] += 2;
-                    } else {
-                        /* FIXME - what to do if get_user() fails? */
-                        get_user_code_u32(insn, env->regs[15], env);
-                        n = (insn & 0xf) | ((insn >> 4) & 0xff0);
-                        env->regs[15] += 4;
-                    }
+                if (env->thumb) {
+                    /* FIXME - what to do if get_user() fails? */
+                    get_user_code_u16(insn, env->regs[15] - 2, env);
+                    n = insn & 0xff;
                 } else {
-                    if (env->thumb) {
-                        /* FIXME - what to do if get_user() fails? */
-                        get_user_code_u16(insn, env->regs[15] - 2, env);
-                        n = insn & 0xff;
-                    } else {
-                        /* FIXME - what to do if get_user() fails? */
-                        get_user_code_u32(insn, env->regs[15] - 4, env);
-                        n = insn & 0xffffff;
-                    }
+                    /* FIXME - what to do if get_user() fails? */
+                    get_user_code_u32(insn, env->regs[15] - 4, env);
+                    n = insn & 0xffffff;
                 }
 
                 if (n == ARM_NR_cacheflush) {
@@ -396,6 +381,7 @@ void cpu_loop(CPUARMState *env)
             }
             break;
         case EXCP_DEBUG:
+        case EXCP_BKPT:
         excp_debug:
             info.si_signo = TARGET_SIGTRAP;
             info.si_errno = 0;