Re: [PATCH] sim: riscv: Fix PC at gdb breakpoints

[Andrew Burgess <aburgess@redhat.com>]

Bernd Edlinger <bernd.edlinger@hotmail.de> writes:

> The uncompressed EBREAK instruction does not work
> correctly this way, and the comment saying that
> GDB expects us to step over EBREAK is just wrong.
> The PC was always 4 bytes too high, which skips one
> instruction at break and step over commands, and
> causes complete chaos.  The compressed EBREAK was
> already implemented correctly.
>
> Tested by using gdb's "target sim" and single-stepping.

Thanks for fixing this.

For the record, in v1.12 of the RISC-V privileged architecture
specification, section 3.3.1: Environment Call and Breakpoint documents
that the $pc value should be the address of the EBREAK instruction, not
the address of the following instruction.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

> ---
>  sim/riscv/sim-main.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
> index 0876d455570..66b99e40314 100644
> --- a/sim/riscv/sim-main.c
> +++ b/sim/riscv/sim-main.c
> @@ -624,9 +624,7 @@ execute_i (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
>        break;
>      case MATCH_EBREAK:
>        TRACE_INSN (cpu, "ebreak;");
> -      /* GDB expects us to step over EBREAK.  */
> -      sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc + 4, sim_stopped,
> -		       SIM_SIGTRAP);
> +      sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_stopped, SIM_SIGTRAP);
>        break;
>      case MATCH_ECALL:
>        TRACE_INSN (cpu, "ecall;");
> -- 
> 2.25.1