[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 27/68] target/arm/arm-semi: Correct comment about gdb syscall race
From: |
Peter Maydell |
Subject: |
[PULL 27/68] target/arm/arm-semi: Correct comment about gdb syscall races |
Date: |
Mon, 14 Oct 2019 17:03:23 +0100 |
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 <address@hidden>
Reviewed-by: Richard Henderson <address@hidden>
Message-id: address@hidden
---
target/arm/arm-semi.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
index 51b55816faf..302529f2278 100644
--- a/target/arm/arm-semi.c
+++ b/target/arm/arm-semi.c
@@ -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];
--
2.20.1
- [PULL 17/68] hw/timer/exynos4210_pwm.c: Switch to transaction-based ptimer API, (continued)
- [PULL 17/68] hw/timer/exynos4210_pwm.c: Switch to transaction-based ptimer API, Peter Maydell, 2019/10/14
- [PULL 18/68] hw/timer/exynos4210_rtc.c: Switch 1Hz ptimer to transaction-based API, Peter Maydell, 2019/10/14
- [PULL 19/68] hw/timer/exynos4210_rtc.c: Switch main ptimer to transaction-based API, Peter Maydell, 2019/10/14
- [PULL 20/68] hw/timer/imx_epit.c: Switch to transaction-based ptimer API, Peter Maydell, 2019/10/14
- [PULL 21/68] hw/timer/imx_gpt.c: Switch to transaction-based ptimer API, Peter Maydell, 2019/10/14
- [PULL 22/68] hw/timer/mss-timerc: Switch to transaction-based ptimer API, Peter Maydell, 2019/10/14
- [PULL 23/68] hw/watchdog/cmsdk-apb-watchdog.c: Switch to transaction-based ptimer API, Peter Maydell, 2019/10/14
- [PULL 24/68] hw/net/lan9118.c: Switch to transaction-based ptimer API, Peter Maydell, 2019/10/14
- [PULL 25/68] target/arm/arm-semi: Capture errno in softmmu version of set_swi_errno(), Peter Maydell, 2019/10/14
- [PULL 26/68] target/arm/arm-semi: Always set some kind of errno for failed calls, Peter Maydell, 2019/10/14
- [PULL 27/68] target/arm/arm-semi: Correct comment about gdb syscall races,
Peter Maydell <=
- [PULL 30/68] target/arm/arm-semi: Use set_swi_errno() in gdbstub callback functions, Peter Maydell, 2019/10/14
- [PULL 28/68] target/arm/arm-semi: Make semihosting code hand out its own file descriptors, Peter Maydell, 2019/10/14
- [PULL 29/68] target/arm/arm-semi: Restrict use of TaskState*, Peter Maydell, 2019/10/14
- [PULL 31/68] target/arm/arm-semi: Factor out implementation of SYS_CLOSE, Peter Maydell, 2019/10/14
- [PULL 32/68] target/arm/arm-semi: Factor out implementation of SYS_WRITE, Peter Maydell, 2019/10/14
- [PULL 33/68] target/arm/arm-semi: Factor out implementation of SYS_READ, Peter Maydell, 2019/10/14
- [PULL 34/68] target/arm/arm-semi: Factor out implementation of SYS_ISTTY, Peter Maydell, 2019/10/14
- [PULL 35/68] target/arm/arm-semi: Factor out implementation of SYS_SEEK, Peter Maydell, 2019/10/14
- [PULL 36/68] target/arm/arm-semi: Factor out implementation of SYS_FLEN, Peter Maydell, 2019/10/14
- [PULL 37/68] target/arm/arm-semi: Implement support for semihosting feature detection, Peter Maydell, 2019/10/14