On 20:00 Thu 26 May , Peter Maydell wrote:
> In two places in gdbstub.c we look at gdbserver_state.init to decide
> whether we're going to do a semihosting syscall via the gdb remote
> protocol:
> * when setting up, if the user didn't explicitly select either
> native semihosting or gdb semihosting, we autoselect, with the
> intended behaviour "use gdb if gdb is connected"
> * when the semihosting layer attempts to do a syscall via gdb, we
> silently ignore it if the gdbstub wasn't actually set up
>
> However, if the user's commandline sets up the gdbstub but tells QEMU
> to start rather than waiting for a GDB to connect (eg using '-s' but
> not '-S'), then we will have gdbserver_state.init true but no actual
> connection; an attempt to use gdb syscalls will then crash because we
> try to use gdbserver_state.c_cpu when it hasn't been set up:
>
> #0 0x00007ffff6803ba8 in qemu_cpu_kick (cpu=0x0) at ../../softmmu/cpus.c:457
> #1 0x00007ffff6c03913 in gdb_do_syscallv (cb=0x7ffff6c19944 <common_semi_cb>,
> fmt=0x7ffff7573b7e "", va=0x7ffff56294c0) at ../../gdbstub.c:2946
> #2 0x00007ffff6c19c3a in common_semi_gdb_syscall (cs=0x7ffff83fe060,
> cb=0x7ffff6c19944 <common_semi_cb>, fmt=0x7ffff7573b75 "isatty,%x")
> at ../../semihosting/arm-compat-semi.c:494
> #3 0x00007ffff6c1a064 in gdb_isattyfn (cs=0x7ffff83fe060, gf=0x7ffff86a3690)
> at ../../semihosting/arm-compat-semi.c:636
> #4 0x00007ffff6c1b20f in do_common_semihosting (cs=0x7ffff83fe060)
> at ../../semihosting/arm-compat-semi.c:967
> #5 0x00007ffff693a037 in handle_semihosting (cs=0x7ffff83fe060)
> at ../../target/arm/helper.c:10316
>
> You can probably also get into this state via some odd
> corner cases involving connecting a GDB and then telling it
> to detach from all the vCPUs.
>
> Abstract out the test into a new gdb_attached() function
> which returns true only if there's actually a GDB connected
> to the debug stub and attached to at least one vCPU.
>
> Reported-by: Liviu Ionescu <i...@livius.net>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
Reviewed-by: Luc Michel <l...@lmichel.fr>
> ---
> Silently doing nothing in gdb_do_syscallv(), never calling the
> callback function, is kind of dodgy. But it's what the code is doing
> already, and besides it's not clear what we should do if the user
> specifically says "semihosting calls via the gdb stub" and then
> doesn't connect gdb...
> ---
> gdbstub.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index a3ff8702cef..88a34c8f522 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -443,6 +443,15 @@ static int get_char(void)
> }
> #endif
>
> +/*
> + * Return true if there is a GDB currently connected to the stub
> + * and attached to a CPU
> + */
> +static bool gdb_attached(void)
> +{
> + return gdbserver_state.init && gdbserver_state.c_cpu;
> +}
> +
> static enum {
> GDB_SYS_UNKNOWN,
> GDB_SYS_ENABLED,
> @@ -464,8 +473,7 @@ int use_gdb_syscalls(void)
> /* -semihosting-config target=auto */
> /* On the first call check if gdb is connected and remember. */
> if (gdb_syscall_mode == GDB_SYS_UNKNOWN) {
> - gdb_syscall_mode = gdbserver_state.init ?
> - GDB_SYS_ENABLED : GDB_SYS_DISABLED;
> + gdb_syscall_mode = gdb_attached() ? GDB_SYS_ENABLED :
> GDB_SYS_DISABLED;
> }
> return gdb_syscall_mode == GDB_SYS_ENABLED;
> }
> @@ -2886,7 +2894,7 @@ void gdb_do_syscallv(gdb_syscall_complete_cb cb, const
> char *fmt, va_list va)
> target_ulong addr;
> uint64_t i64;
>
> - if (!gdbserver_state.init) {
> + if (!gdb_attached()) {
> return;
> }
>
> --
> 2.25.1
>
>
--
+---------------------+--------------------------+
| Luc MICHEL | TIMA Lab / SLS Team |
| | Phone: +33 4 76 57 43 34 |
+---------------------+--------------------------+
