Re: [PATCH 1/2] gdbstub: Don't use GDB syscalls if no GDB is attached
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 |
+-+--+
Re: [PATCH 1/2] gdbstub: Don't use GDB syscalls if no GDB is attached
On 26/5/22 21:00, 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 --- 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... When using a JTAG probe as GDB monitor, you get an error the probe can't get initialized. If the probe gets disconnected while running then the semihosting calls are executed but don't do anything (or return default errno). Reviewed-by: Philippe Mathieu-Daudé
[PATCH 1/2] gdbstub: Don't use GDB syscalls if no GDB is attached
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
---
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
