On Mon, Apr 20, 2020 at 10:22:03PM +0100, Peter Maydell wrote:
> 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: <omerg...@gmail.com>
> Fixes: https://bugs.launchpad.net/qemu/+bug/1873898
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> ---
> linux-user/arm/cpu_loop.c | 30 ++++++++----------------------
> 1 file changed, 8 insertions(+), 22 deletions(-)
>
> diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
> index cf618daa1ca..82d0dd3c312 100644
> --- 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;
> --
> 2.20.1
>
>
