@@ -217,10 +217,21 @@ static target_ulong arm_gdb_syscall(ARMCPU *cpu, gdb_syscall_complete_cb cb,
gdb_do_syscallv(cb, fmt, va);
va_end(va);
- /* FIXME: we are implicitly relying on the syscall completing
- * before this point, which is not guaranteed. We should
- * put in an explicit synchronization between this and
- * the callback function.
+ /*
+ * FIXME: in softmmu mode, the gdbstub will schedule our callback
+ * to occur, but will not actually call it to complete the syscall
+ * until after this function has returned and we are back in the
+ * CPU main loop. Therefore callers to this function must not
+ * do anything with its return value, because it is not necessarily
+ * the result of the syscall, but could just be the old value of X0.
+ * The only thing safe to do with this is that the callers of
+ * do_arm_semihosting() will write it straight back into X0.
+ * (In linux-user mode, the callback will have happened before
+ * gdb_do_syscallv() returns.)
+ *
+ * We should tidy this up so neither this function nor
+ * do_arm_semihosting() return a value, so the mistake of
+ * doing something with the return value is not possible to make.
*/
return is_a64(env) ? env->xregs[0] : env->regs[0];
In arm_gdb_syscall() we have a comment suggesting a race because the syscall completion callback might not happen before the gdb_do_syscallv() call returns. The comment is correct that the callback may not happen but incorrect about the effects. Correct it and note the important caveat that callers must never do any work of any kind after return from arm_gdb_syscall() that depends on its return value. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- I'll come back to do the cleanup later, but I preferred not to tangle it up with the rest of the refactoring in this series; I also think it's probably easier done afterwards rather than before. --- target/arm/arm-semi.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) -- 2.20.1