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 0x76803ba8 in qemu_cpu_kick (cpu=0x0) at ../../softmmu/cpus.c:457
> #1 0x76c03913 in gdb_do_syscallv (cb=0x76c19944 ,
> fmt=0x77573b7e "", va=0x756294c0) at ../../gdbstub.c:2946
> #2 0x76c19c3a in common_semi_gdb_syscall (cs=0x783fe060,
> cb=0x76c19944 , fmt=0x77573b75 "isatty,%x")
> at ../../semihosting/arm-compat-semi.c:494
> #3 0x76c1a064 in gdb_isattyfn (cs=0x783fe060, gf=0x786a3690)
> at ../../semihosting/arm-compat-semi.c:636
> #4 0x76c1b20f in do_common_semihosting (cs=0x783fe060)
> at ../../semihosting/arm-compat-semi.c:967
> #5 0x7693a037 in handle_semihosting (cs=0x783fe060)
> 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
> Signed-off-by: Peter Maydell
Reviewed-by: Luc Michel
> ---
> 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 |
+-+--+