[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2 03/15] target/arm/arm-semi: Correct comment about
From: |
Peter Maydell |
Subject: |
[Qemu-devel] [PATCH v2 03/15] target/arm/arm-semi: Correct comment about gdb syscall races |
Date: |
Mon, 16 Sep 2019 15:15:32 +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>
---
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(-)
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
- [Qemu-devel] [PATCH v2 00/15] target/arm: Implement semihosting v2.0, Peter Maydell, 2019/09/16
- [Qemu-devel] [PATCH v2 01/15] target/arm/arm-semi: Capture errno in softmmu version of set_swi_errno(), Peter Maydell, 2019/09/16
- [Qemu-devel] [PATCH v2 02/15] target/arm/arm-semi: Always set some kind of errno for failed calls, Peter Maydell, 2019/09/16
- [Qemu-devel] [PATCH v2 03/15] target/arm/arm-semi: Correct comment about gdb syscall races,
Peter Maydell <=
- [Qemu-devel] [PATCH v2 06/15] target/arm/arm-semi: Use set_swi_errno() in gdbstub callback functions, Peter Maydell, 2019/09/16
- [Qemu-devel] [PATCH v2 04/15] target/arm/arm-semi: Make semihosting code hand out its own file descriptors, Peter Maydell, 2019/09/16
- [Qemu-devel] [PATCH v2 07/15] target/arm/arm-semi: Factor out implementation of SYS_CLOSE, Peter Maydell, 2019/09/16
- [Qemu-devel] [PATCH v2 05/15] target/arm/arm-semi: Restrict use of TaskState*, Peter Maydell, 2019/09/16
- [Qemu-devel] [PATCH v2 09/15] target/arm/arm-semi: Factor out implementation of SYS_READ, Peter Maydell, 2019/09/16
- [Qemu-devel] [PATCH v2 08/15] target/arm/arm-semi: Factor out implementation of SYS_WRITE, Peter Maydell, 2019/09/16
- [Qemu-devel] [PATCH v2 12/15] target/arm/arm-semi: Factor out implementation of SYS_FLEN, Peter Maydell, 2019/09/16
- [Qemu-devel] [PATCH v2 10/15] target/arm/arm-semi: Factor out implementation of SYS_ISTTY, Peter Maydell, 2019/09/16
- [Qemu-devel] [PATCH v2 11/15] target/arm/arm-semi: Factor out implementation of SYS_SEEK, Peter Maydell, 2019/09/16
- [Qemu-devel] [PATCH v2 14/15] target/arm/arm-semi: Implement SH_EXT_EXIT_EXTENDED extension, Peter Maydell, 2019/09/16