Re: [PATCH for-6.1 v6 11/17] hw/core: Introduce CPUClass.gdb_adjust_breakpoint

[Peter Maydell]

On Tue, 20 Jul 2021 at 20:54, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This will allow a breakpoint hack to move out of AVR's translator.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> diff --git a/cpu.c b/cpu.c
> index 83059537d7..91d9e38acb 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -267,8 +267,13 @@ static void breakpoint_invalidate(CPUState *cpu, 
> target_ulong pc)
>  int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags,
>                            CPUBreakpoint **breakpoint)
>  {
> +    CPUClass *cc = CPU_GET_CLASS(cpu);
>      CPUBreakpoint *bp;
>
> +    if (cc->gdb_adjust_breakpoint) {
> +        pc = cc->gdb_adjust_breakpoint(cpu, pc);
> +    }
> +
>      bp = g_malloc(sizeof(*bp));
>
>      bp->pc = pc;
> @@ -294,8 +299,13 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int 
> flags,
>  /* Remove a specific breakpoint.  */
>  int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int flags)
>  {
> +    CPUClass *cc = CPU_GET_CLASS(cpu);
>      CPUBreakpoint *bp;
>
> +    if (cc->gdb_adjust_breakpoint) {
> +        pc = cc->gdb_adjust_breakpoint(cpu, pc);
> +    }
> +
>      QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
>          if (bp->pc == pc && bp->flags == flags) {
>              cpu_breakpoint_remove_by_ref(cpu, bp);
> --

So previously for AVR we would have considered the bp at 0x100
and the one at 0x800100 as distinct (in the sense that the only way
the gdb remote protocol distinguishes breakpoints is by "what address",
and these have different addresses). After this change, they won't
be distinct, because if you set a bp at 0x100 and 0x800100 and then
try to remove the one at 0x100 we might remove the 0x800100 one,
because we're storing only the adjusted-address, not the one gdb used.

This might not matter in practice...

-- PMM