From e4e0c75f0f8adcbd44ce85d4316fb7ef818e2e60 Mon Sep 17 00:00:00 2001 From: Sergey Fedorov Date: Fri, 16 Feb 2018 14:54:32 -0500 Subject: [PATCH] target-arm: Fix CPU breakpoint handling A QEMU breakpoint match is not definitely an architectural breakpoint match. If an exception is generated unconditionally during translation, it is hardly possible to ignore it in the debug exception handler. Generate a call to a helper to check CPU breakpoints and raise an exception only if any breakpoint matches architecturally. Backports commit 5d98bf8f38c17a348ab6e8af196088cd4953acd0 from qemu --- qemu/aarch64.h | 1 + qemu/aarch64eb.h | 1 + qemu/arm.h | 1 + qemu/armeb.h | 1 + qemu/header_gen.py | 1 + qemu/m68k.h | 1 + qemu/mips.h | 1 + qemu/mips64.h | 1 + qemu/mips64el.h | 1 + qemu/mipsel.h | 1 + qemu/powerpc.h | 1 + qemu/sparc.h | 1 + qemu/sparc64.h | 1 + qemu/target-arm/helper.h | 2 ++ qemu/target-arm/op_helper.c | 40 ++++++++++++++++++++------------- qemu/target-arm/translate-a64.c | 17 +++++++++----- qemu/target-arm/translate.c | 19 +++++++++++----- qemu/x86_64.h | 1 + 18 files changed, 66 insertions(+), 26 deletions(-) diff --git a/qemu/aarch64.h b/qemu/aarch64.h index 3b70b8cb..ebb9789d 100644 --- a/qemu/aarch64.h +++ b/qemu/aarch64.h @@ -667,6 +667,7 @@ #define gen_helper_access_check_cp_reg gen_helper_access_check_cp_reg_aarch64 #define gen_helper_add_saturate gen_helper_add_saturate_aarch64 #define gen_helper_add_setq gen_helper_add_setq_aarch64 +#define gen_helper_check_breakpoints gen_helper_check_breakpoints_aarch64 #define gen_helper_clear_pstate_ss gen_helper_clear_pstate_ss_aarch64 #define gen_helper_clz32 gen_helper_clz32_aarch64 #define gen_helper_clz64 gen_helper_clz64_aarch64 diff --git a/qemu/aarch64eb.h b/qemu/aarch64eb.h index 47329860..35823c82 100644 --- a/qemu/aarch64eb.h +++ b/qemu/aarch64eb.h @@ -667,6 +667,7 @@ #define gen_helper_access_check_cp_reg gen_helper_access_check_cp_reg_aarch64eb #define gen_helper_add_saturate gen_helper_add_saturate_aarch64eb #define gen_helper_add_setq gen_helper_add_setq_aarch64eb +#define gen_helper_check_breakpoints gen_helper_check_breakpoints_aarch64eb #define gen_helper_clear_pstate_ss gen_helper_clear_pstate_ss_aarch64eb #define gen_helper_clz32 gen_helper_clz32_aarch64eb #define gen_helper_clz64 gen_helper_clz64_aarch64eb diff --git a/qemu/arm.h b/qemu/arm.h index b72a3a49..7e8e35ca 100644 --- a/qemu/arm.h +++ b/qemu/arm.h @@ -667,6 +667,7 @@ #define gen_helper_access_check_cp_reg gen_helper_access_check_cp_reg_arm #define gen_helper_add_saturate gen_helper_add_saturate_arm #define gen_helper_add_setq gen_helper_add_setq_arm +#define gen_helper_check_breakpoints gen_helper_check_breakpoints_arm #define gen_helper_clear_pstate_ss gen_helper_clear_pstate_ss_arm #define gen_helper_clz32 gen_helper_clz32_arm #define gen_helper_clz64 gen_helper_clz64_arm diff --git a/qemu/armeb.h b/qemu/armeb.h index 24abe150..28884079 100644 --- a/qemu/armeb.h +++ b/qemu/armeb.h @@ -667,6 +667,7 @@ #define gen_helper_access_check_cp_reg gen_helper_access_check_cp_reg_armeb #define gen_helper_add_saturate gen_helper_add_saturate_armeb #define gen_helper_add_setq gen_helper_add_setq_armeb +#define gen_helper_check_breakpoints gen_helper_check_breakpoints_armeb #define gen_helper_clear_pstate_ss gen_helper_clear_pstate_ss_armeb #define gen_helper_clz32 gen_helper_clz32_armeb #define gen_helper_clz64 gen_helper_clz64_armeb diff --git a/qemu/header_gen.py b/qemu/header_gen.py index 28a788b1..27444464 100644 --- a/qemu/header_gen.py +++ b/qemu/header_gen.py @@ -673,6 +673,7 @@ symbols = ( 'gen_helper_access_check_cp_reg', 'gen_helper_add_saturate', 'gen_helper_add_setq', + 'gen_helper_check_breakpoints', 'gen_helper_clear_pstate_ss', 'gen_helper_clz32', 'gen_helper_clz64', diff --git a/qemu/m68k.h b/qemu/m68k.h index 1e349af6..0a5c6ce7 100644 --- a/qemu/m68k.h +++ b/qemu/m68k.h @@ -667,6 +667,7 @@ #define gen_helper_access_check_cp_reg gen_helper_access_check_cp_reg_m68k #define gen_helper_add_saturate gen_helper_add_saturate_m68k #define gen_helper_add_setq gen_helper_add_setq_m68k +#define gen_helper_check_breakpoints gen_helper_check_breakpoints_m68k #define gen_helper_clear_pstate_ss gen_helper_clear_pstate_ss_m68k #define gen_helper_clz32 gen_helper_clz32_m68k #define gen_helper_clz64 gen_helper_clz64_m68k diff --git a/qemu/mips.h b/qemu/mips.h index 6f818175..bfb03cdd 100644 --- a/qemu/mips.h +++ b/qemu/mips.h @@ -667,6 +667,7 @@ #define gen_helper_access_check_cp_reg gen_helper_access_check_cp_reg_mips #define gen_helper_add_saturate gen_helper_add_saturate_mips #define gen_helper_add_setq gen_helper_add_setq_mips +#define gen_helper_check_breakpoints gen_helper_check_breakpoints_mips #define gen_helper_clear_pstate_ss gen_helper_clear_pstate_ss_mips #define gen_helper_clz32 gen_helper_clz32_mips #define gen_helper_clz64 gen_helper_clz64_mips diff --git a/qemu/mips64.h b/qemu/mips64.h index 24903223..298f847b 100644 --- a/qemu/mips64.h +++ b/qemu/mips64.h @@ -667,6 +667,7 @@ #define gen_helper_access_check_cp_reg gen_helper_access_check_cp_reg_mips64 #define gen_helper_add_saturate gen_helper_add_saturate_mips64 #define gen_helper_add_setq gen_helper_add_setq_mips64 +#define gen_helper_check_breakpoints gen_helper_check_breakpoints_mips64 #define gen_helper_clear_pstate_ss gen_helper_clear_pstate_ss_mips64 #define gen_helper_clz32 gen_helper_clz32_mips64 #define gen_helper_clz64 gen_helper_clz64_mips64 diff --git a/qemu/mips64el.h b/qemu/mips64el.h index 11c7d5a1..f05e9ff7 100644 --- a/qemu/mips64el.h +++ b/qemu/mips64el.h @@ -667,6 +667,7 @@ #define gen_helper_access_check_cp_reg gen_helper_access_check_cp_reg_mips64el #define gen_helper_add_saturate gen_helper_add_saturate_mips64el #define gen_helper_add_setq gen_helper_add_setq_mips64el +#define gen_helper_check_breakpoints gen_helper_check_breakpoints_mips64el #define gen_helper_clear_pstate_ss gen_helper_clear_pstate_ss_mips64el #define gen_helper_clz32 gen_helper_clz32_mips64el #define gen_helper_clz64 gen_helper_clz64_mips64el diff --git a/qemu/mipsel.h b/qemu/mipsel.h index a3d75736..2c065d55 100644 --- a/qemu/mipsel.h +++ b/qemu/mipsel.h @@ -667,6 +667,7 @@ #define gen_helper_access_check_cp_reg gen_helper_access_check_cp_reg_mipsel #define gen_helper_add_saturate gen_helper_add_saturate_mipsel #define gen_helper_add_setq gen_helper_add_setq_mipsel +#define gen_helper_check_breakpoints gen_helper_check_breakpoints_mipsel #define gen_helper_clear_pstate_ss gen_helper_clear_pstate_ss_mipsel #define gen_helper_clz32 gen_helper_clz32_mipsel #define gen_helper_clz64 gen_helper_clz64_mipsel diff --git a/qemu/powerpc.h b/qemu/powerpc.h index fbed930a..ae98adce 100644 --- a/qemu/powerpc.h +++ b/qemu/powerpc.h @@ -667,6 +667,7 @@ #define gen_helper_access_check_cp_reg gen_helper_access_check_cp_reg_powerpc #define gen_helper_add_saturate gen_helper_add_saturate_powerpc #define gen_helper_add_setq gen_helper_add_setq_powerpc +#define gen_helper_check_breakpoints gen_helper_check_breakpoints_powerpc #define gen_helper_clear_pstate_ss gen_helper_clear_pstate_ss_powerpc #define gen_helper_clz32 gen_helper_clz32_powerpc #define gen_helper_clz64 gen_helper_clz64_powerpc diff --git a/qemu/sparc.h b/qemu/sparc.h index 372cf063..05e04750 100644 --- a/qemu/sparc.h +++ b/qemu/sparc.h @@ -667,6 +667,7 @@ #define gen_helper_access_check_cp_reg gen_helper_access_check_cp_reg_sparc #define gen_helper_add_saturate gen_helper_add_saturate_sparc #define gen_helper_add_setq gen_helper_add_setq_sparc +#define gen_helper_check_breakpoints gen_helper_check_breakpoints_sparc #define gen_helper_clear_pstate_ss gen_helper_clear_pstate_ss_sparc #define gen_helper_clz32 gen_helper_clz32_sparc #define gen_helper_clz64 gen_helper_clz64_sparc diff --git a/qemu/sparc64.h b/qemu/sparc64.h index 371afd00..219fc17a 100644 --- a/qemu/sparc64.h +++ b/qemu/sparc64.h @@ -667,6 +667,7 @@ #define gen_helper_access_check_cp_reg gen_helper_access_check_cp_reg_sparc64 #define gen_helper_add_saturate gen_helper_add_saturate_sparc64 #define gen_helper_add_setq gen_helper_add_setq_sparc64 +#define gen_helper_check_breakpoints gen_helper_check_breakpoints_sparc64 #define gen_helper_clear_pstate_ss gen_helper_clear_pstate_ss_sparc64 #define gen_helper_clz32 gen_helper_clz32_sparc64 #define gen_helper_clz64 gen_helper_clz64_sparc64 diff --git a/qemu/target-arm/helper.h b/qemu/target-arm/helper.h index 082bf980..ef0dec43 100644 --- a/qemu/target-arm/helper.h +++ b/qemu/target-arm/helper.h @@ -57,6 +57,8 @@ DEF_HELPER_1(yield, void, env) DEF_HELPER_1(pre_hvc, void, env) DEF_HELPER_2(pre_smc, void, env, i32) +DEF_HELPER_1(check_breakpoints, void, env) + DEF_HELPER_3(cpsr_write, void, env, i32, i32) DEF_HELPER_1(cpsr_read, i32, env) diff --git a/qemu/target-arm/op_helper.c b/qemu/target-arm/op_helper.c index 298a6925..204ffe37 100644 --- a/qemu/target-arm/op_helper.c +++ b/qemu/target-arm/op_helper.c @@ -864,6 +864,15 @@ static bool check_breakpoints(ARMCPU *cpu) return false; } +void HELPER(check_breakpoints)(CPUARMState *env) +{ + ARMCPU *cpu = arm_env_get_cpu(env); + + if (check_breakpoints(cpu)) { + HELPER(exception_internal(env, EXCP_DEBUG)); + } +} + void arm_debug_excp_handler(CPUState *cs) { /* Called by core code when a watchpoint or breakpoint fires; @@ -894,25 +903,24 @@ void arm_debug_excp_handler(CPUState *cs) } } } else { - /* Unicorn: commented out - uint64_t pc = is_a64(env) ? env->pc : env->regs[15]; + // Unicorn: commented out + //uint64_t pc = is_a64(env) ? env->pc : env->regs[15]; + bool same_el = (arm_debug_target_el(env) == arm_current_el(env)); - if (cpu_breakpoint_test(cs, pc, BP_GDB)) { - return; - }*/ + // Unicorn: commented out + //if (cpu_breakpoint_test(cs, pc, BP_GDB)) { + // return; + //} - if (check_breakpoints(cpu)) { - bool same_el = (arm_debug_target_el(env) == arm_current_el(env)); - if (extended_addresses_enabled(env)) { - env->exception.fsr = (1 << 9) | 0x22; - } else { - env->exception.fsr = 0x2; - } - /* FAR is UNKNOWN, so doesn't need setting */ - raise_exception(env, EXCP_PREFETCH_ABORT, - syn_breakpoint(same_el), - arm_debug_target_el(env)); + if (extended_addresses_enabled(env)) { + env->exception.fsr = (1 << 9) | 0x22; + } else { + env->exception.fsr = 0x2; } + /* FAR is UNKNOWN, so doesn't need setting */ + raise_exception(env, EXCP_PREFETCH_ABORT, + syn_breakpoint(same_el), + arm_debug_target_el(env)); } } diff --git a/qemu/target-arm/translate-a64.c b/qemu/target-arm/translate-a64.c index 11adefa2..93f1a66a 100644 --- a/qemu/target-arm/translate-a64.c +++ b/qemu/target-arm/translate-a64.c @@ -11319,11 +11319,18 @@ void gen_intermediate_code_a64(ARMCPU *cpu, TranslationBlock *tb) if (unlikely(!QTAILQ_EMPTY(&cs->breakpoints))) { QTAILQ_FOREACH(bp, &cs->breakpoints, entry) { if (bp->pc == dc->pc) { - gen_exception_internal_insn(dc, 0, EXCP_DEBUG); - /* Advance PC so that clearing the breakpoint will - invalidate this TB. */ - dc->pc += 2; - goto done_generating; + if (bp->flags & BP_CPU) { + gen_helper_check_breakpoints(tcg_ctx, tcg_ctx->cpu_env); + /* End the TB early; it likely won't be executed */ + dc->is_jmp = DISAS_UPDATE; + } else { + gen_exception_internal_insn(dc, 0, EXCP_DEBUG); + /* Advance PC so that clearing the breakpoint will + invalidate this TB. */ + dc->pc += 4; + goto done_generating; + } + break; } } } diff --git a/qemu/target-arm/translate.c b/qemu/target-arm/translate.c index f64d81fe..2d027ebb 100644 --- a/qemu/target-arm/translate.c +++ b/qemu/target-arm/translate.c @@ -11534,11 +11534,20 @@ void gen_intermediate_code(CPUARMState *env, TranslationBlock *tb) if (unlikely(!QTAILQ_EMPTY(&cs->breakpoints))) { QTAILQ_FOREACH(bp, &cs->breakpoints, entry) { if (bp->pc == dc->pc) { - gen_exception_internal_insn(dc, 0, EXCP_DEBUG); - /* Advance PC so that clearing the breakpoint will - invalidate this TB. */ - dc->pc += 2; - goto done_generating; + if (bp->flags & BP_CPU) { + gen_helper_check_breakpoints(tcg_ctx, tcg_ctx->cpu_env); + /* End the TB early; it's likely not going to be executed */ + dc->is_jmp = DISAS_UPDATE; + } else { + gen_exception_internal_insn(dc, 0, EXCP_DEBUG); + /* Advance PC so that clearing the breakpoint will + invalidate this TB. */ + /* TODO: Advance PC by correct instruction length to + * avoid disassembler error messages */ + dc->pc += 2; + goto done_generating; + } + break; } } } diff --git a/qemu/x86_64.h b/qemu/x86_64.h index c4e82b42..f19572dc 100644 --- a/qemu/x86_64.h +++ b/qemu/x86_64.h @@ -667,6 +667,7 @@ #define gen_helper_access_check_cp_reg gen_helper_access_check_cp_reg_x86_64 #define gen_helper_add_saturate gen_helper_add_saturate_x86_64 #define gen_helper_add_setq gen_helper_add_setq_x86_64 +#define gen_helper_check_breakpoints gen_helper_check_breakpoints_x86_64 #define gen_helper_clear_pstate_ss gen_helper_clear_pstate_ss_x86_64 #define gen_helper_clz32 gen_helper_clz32_x86_64 #define gen_helper_clz64 gen_helper_clz64_x86_64