[PATCH] linux-user/arm: Deliver SIGTRAP for UDF patterns used as breakpoints

Peter Maydell posted 1 patch 3 years, 5 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20201117155634.6924-1-peter.maydell@linaro.org
Maintainers: Laurent Vivier <laurent@vivier.eu>
linux-user/arm/cpu_loop.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
[PATCH] linux-user/arm: Deliver SIGTRAP for UDF patterns used as breakpoints
Posted by Peter Maydell 3 years, 5 months ago
The Linux kernel doesn't use the official bkpt insn for breakpoints;
instead it uses three instructions in the guaranteed-to-UNDEF space,
and generates SIGTRAP for these rather than the SIGILL that most
UNDEF insns generate:

https://elixir.bootlin.com/linux/v5.9.8/source/arch/arm/kernel/ptrace.c#L197

Make QEMU treat these insns specially too.  The main benefit of this
is that if you're running a debugger on a guest program that runs
into a GCC __builtin_trap() or LLVM "trap because execution should
never reach here" then you'll get the expected signal rather than a
SIGILL.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This is not a "fix" for LP:1904210, but it does bring QEMU's
behaviour into line with that of the real kernel on that binary.

---
 linux-user/arm/cpu_loop.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index 13629ee1f6a..3d272b56ef0 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -205,6 +205,24 @@ do_kernel_trap(CPUARMState *env)
     return 0;
 }
 
+static bool insn_is_linux_bkpt(uint32_t opcode, bool is_thumb)
+{
+    /*
+     * Return true if this insn is one of the three magic UDF insns
+     * which the kernel treats as breakpoint insns.
+     */
+    if (!is_thumb) {
+        return (opcode & 0x0fffffff) == 0x07f001f0;
+    } else {
+        /*
+         * Note that we get the two halves of the 32-bit T32 insn
+         * in the opposite order to the value the kernel uses in
+         * its undef_hook struct.
+         */
+        return ((opcode & 0xffff) == 0xde01) || (opcode == 0xa000f7f0);
+    }
+}
+
 void cpu_loop(CPUARMState *env)
 {
     CPUState *cs = env_cpu(env);
@@ -234,6 +252,16 @@ void cpu_loop(CPUARMState *env)
                 /* FIXME - what to do if get_user() fails? */
                 get_user_code_u32(opcode, env->regs[15], env);
 
+                /*
+                 * The Linux kernel treats some UDF patterns specially
+                 * to use as breakpoints (instead of the architectural
+                 * bkpt insn). These should trigger a SIGTRAP rather
+                 * than SIGILL.
+                 */
+                if (insn_is_linux_bkpt(opcode, env->thumb)) {
+                    goto excp_debug;
+                }
+
                 rc = EmulateAll(opcode, &ts->fpa, env);
                 if (rc == 0) { /* illegal instruction */
                     info.si_signo = TARGET_SIGILL;
-- 
2.20.1


Re: [PATCH] linux-user/arm: Deliver SIGTRAP for UDF patterns used as breakpoints
Posted by Richard Henderson 3 years, 4 months ago
On 11/17/20 7:56 AM, Peter Maydell wrote:
> The Linux kernel doesn't use the official bkpt insn for breakpoints;
> instead it uses three instructions in the guaranteed-to-UNDEF space,
> and generates SIGTRAP for these rather than the SIGILL that most
> UNDEF insns generate:
> 
> https://elixir.bootlin.com/linux/v5.9.8/source/arch/arm/kernel/ptrace.c#L197
> 
> Make QEMU treat these insns specially too.  The main benefit of this
> is that if you're running a debugger on a guest program that runs
> into a GCC __builtin_trap() or LLVM "trap because execution should
> never reach here" then you'll get the expected signal rather than a
> SIGILL.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is not a "fix" for LP:1904210, but it does bring QEMU's
> behaviour into line with that of the real kernel on that binary.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~