On 7/16/20 3:04 AM, Luc Michel wrote: > When single-stepping with a debugger attached to QEMU, and when an > exception is raised, the debugger misses the first instruction after the > exception: > > $ qemu-system-aarch64 -M virt -display none -cpu cortex-a53 -s -S > > $ aarch64-linux-gnu-gdb > GNU gdb (GDB) 9.2 > [...] > (gdb) tar rem :1234 > Remote debugging using :1234 > warning: No executable has been specified and target does not support > determining executable automatically. Try using the "file" command. > 0x0000000000000000 in ?? () > (gdb) # writing nop insns to 0x200 and 0x204 > (gdb) set *0x200 = 0xd503201f > (gdb) set *0x204 = 0xd503201f > (gdb) # 0x0 address contains 0 which is an invalid opcode. > (gdb) # The CPU should raise an exception and jump to 0x200 > (gdb) si > 0x0000000000000204 in ?? () > > With this commit, the same run steps correctly on the first instruction > of the exception vector: > > (gdb) si > 0x0000000000000200 in ?? () > > Signed-off-by: Luc Michel <luc.mic...@greensocs.com> > --- > > RFC because I'm really not sure this is the good place to do that since > EXCP_DEBUG are usually raised in each target translate.c. It could also > have implications with record/replay I'm not aware of. > > --- > accel/tcg/cpu-exec.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c > index d95c4848a4..e85fab5d40 100644 > --- a/accel/tcg/cpu-exec.c > +++ b/accel/tcg/cpu-exec.c > @@ -502,10 +502,21 @@ static inline bool cpu_handle_exception(CPUState *cpu, > int *ret) > CPUClass *cc = CPU_GET_CLASS(cpu); > qemu_mutex_lock_iothread(); > cc->do_interrupt(cpu); > qemu_mutex_unlock_iothread(); > cpu->exception_index = -1; > + > + if (unlikely(cpu->singlestep_enabled)) { > + /* > + * After processing the exception, ensure an EXCP_DEBUG is > + * raised when single-stepping so that GDB doesn't miss the > + * next instruction. > + */ > + cpu->exception_index = EXCP_DEBUG; > + return cpu_handle_exception(cpu, ret);
Plausible. Although recursion on an inline function is going to defeat the inline, in general. We could expand the recursion by hand with if (unlikely(cpu->singlestep_enabled)) { *ret = EXCP_DEBUG; cpu_handle_debug_exception(cpu); return true; } which might even be clearer. r~ > + } > + > } else if (!replay_has_interrupt()) { > /* give a chance to iothread in replay mode */ > *ret = EXCP_INTERRUPT; > return true; > } >