Richard Henderson <richard.hender...@linaro.org> writes: > On 10/23/20 2:49 PM, Keith Packard via wrote: >> static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a) >> { >> - generate_exception(ctx, RISCV_EXCP_BREAKPOINT); >> + uint32_t pre = opcode_at(&ctx->base, ctx->base.pc_next - 4); >> + uint32_t ebreak = opcode_at(&ctx->base, ctx->base.pc_next); >> + uint32_t post = opcode_at(&ctx->base, ctx->base.pc_next + 4); > > Alistair asked if this approach is ok. I think it is. There are other places > in which we scan forward (usually only forward, not backward, but this is a > special nop, so it doesn't matter). > > However: > > (1) No need to re-read the current ebreak insn. That is how we arrived here, > after all.
The semihosting spec requires that we land here using a 32-bit ebreak instruction, not the compressed version, so I think we still need to check for this. > (2) You need to check for page boundaries before reading pre and post. > Otherwise you could wind up with SIGSEGV (or the equivalent internal qemu > exception) when you shouldn't. Right, in that case the ebreak instruction should just raise a regular break point exception. I've added a check to make sure the address of the three functions match when and-ed with TARGET_PAGE_MASK. I've finished re-working this patch into a series which first modifies the ARM semihosting support to be architecture-independent, then adds the changes necessary to support RISC-V. Expect to see that series on the list shortly. -- -keith
signature.asc
Description: PGP signature