On 5 November 2015 at 12:26, Sergey Fedorov <serge.f...@gmail.com> wrote:
> Do not raise a CPU exception if no CPU breakpoint has fired, since
> singlestep is also done by generating a debug internal exception. This
> fixes a bug with singlestepping in gdbstub.
>
> Signed-off-by: Sergey Fedorov <serge.f...@gmail.com>
> ---
> This is a v2 of 'target-arm: Fix arm_debug_excp_handler() for singlestep
> enabled.'
>
> Changes in v2:
> * Commit subject and body changed
> * Instead of checking for singlestep enabled, CPU breakpoint match checked
Isn't this fixing singlestep, not non-CPU breakpoints?
(GDB breakpoints were already being checked for and early-returned.)
I'll tweak the commit subject line and apply to target-arm.next.
Incidentally, this only affects gdbstub singlestep for aarch64
in practice, because gdb for 32-bit ARM uses set-breakpoint-and-continue
rather than the singlestep gdbstub protocol command.
> target-arm/op_helper.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index b5db345..6cd54c8 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -917,7 +917,13 @@ void arm_debug_excp_handler(CPUState *cs)
> 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)) {
> + /* (1) GDB breakpoints should be handled first.
> + * (2) Do not raise a CPU exception if no CPU breakpoint has fired,
> + * since singlestep is also done by generating a debug internal
> + * exception.
> + */
> + if (cpu_breakpoint_test(cs, pc, BP_GDB)
> + || !cpu_breakpoint_test(cs, pc, BP_CPU)) {
> return;
> }
>
> --
> 1.9.1
>
thanks
-- PMM
