GDB supports stopping on syscall entry and exit using the "catch
syscall" command. It relies on 3 packets, which are currently not
supported by QEMU:
* qSupported:QCatchSyscalls+ [1]
* QCatchSyscalls: [2]
* T05syscall_entry: and T05syscall_return: [3]
Implement generation and handling of these packets.
[1] https://sourceware.org/gdb/current/onlinedocs/gdb.html/General-Query-Packets.html#qSupported
[2] https://sourceware.org/gdb/current/onlinedocs/gdb.html/General-Query-Packets.html#QCatchSyscalls
[3] https://sourceware.org/gdb/current/onlinedocs/gdb.html/Stop-Reply-Packets.html
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
gdbstub/gdbstub.c | 9 +++
gdbstub/internals.h | 2 +
gdbstub/user-target.c | 5 ++
gdbstub/user.c | 104 ++++++++++++++++++++++++++++++++++-
include/gdbstub/user.h | 29 +++++++++-
include/user/syscall-trace.h | 7 ++-
6 files changed, 151 insertions(+), 5 deletions(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 46d752bbc2c..7e73e916bdc 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1617,6 +1617,7 @@ static void handle_query_supported(GArray *params, void *user_ctx)
if (gdbserver_state.c_cpu->opaque) {
g_string_append(gdbserver_state.str_buf, ";qXfer:auxv:read+");
}
+ g_string_append(gdbserver_state.str_buf, ";QCatchSyscalls+");
#endif
g_string_append(gdbserver_state.str_buf, ";qXfer:exec-file:read+");
#endif
@@ -1810,6 +1811,14 @@ static const GdbCmdParseEntry gdb_gen_set_table[] = {
.schema = "l0"
},
#endif
+#if defined(CONFIG_USER_ONLY)
+ {
+ .handler = gdb_handle_set_catch_syscalls,
+ .cmd = "CatchSyscalls:",
+ .cmd_startswith = 1,
+ .schema = "s0",
+ },
+#endif
};
static void handle_gen_query(GArray *params, void *user_ctx)
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 5c0c725e54c..56b7c13b750 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -136,6 +136,7 @@ void gdb_append_thread_id(CPUState *cpu, GString *buf);
int gdb_get_cpu_index(CPUState *cpu);
unsigned int gdb_get_max_cpus(void); /* both */
bool gdb_can_reverse(void); /* softmmu, stub for user */
+int gdb_target_sigtrap(void); /* user */
void gdb_create_default_process(GDBState *s);
@@ -194,6 +195,7 @@ void gdb_handle_v_file_close(GArray *params, void *user_ctx); /* user */
void gdb_handle_v_file_pread(GArray *params, void *user_ctx); /* user */
void gdb_handle_v_file_readlink(GArray *params, void *user_ctx); /* user */
void gdb_handle_query_xfer_exec_file(GArray *params, void *user_ctx); /* user */
+void gdb_handle_set_catch_syscalls(GArray *params, void *user_ctx); /* user */
void gdb_handle_query_attached(GArray *params, void *user_ctx); /* both */
diff --git a/gdbstub/user-target.c b/gdbstub/user-target.c
index c4bba4c72c7..b7d4c37cd81 100644
--- a/gdbstub/user-target.c
+++ b/gdbstub/user-target.c
@@ -418,3 +418,8 @@ void gdb_handle_query_xfer_exec_file(GArray *params, void *user_ctx)
ts->bprm->filename + offset);
gdb_put_strbuf();
}
+
+int gdb_target_sigtrap(void)
+{
+ return TARGET_SIGTRAP;
+}
diff --git a/gdbstub/user.c b/gdbstub/user.c
index dbe1d9b8875..01dd7169258 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -10,6 +10,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/bitops.h"
#include "qemu/cutils.h"
#include "qemu/sockets.h"
#include "exec/hwaddr.h"
@@ -21,11 +22,25 @@
#include "trace.h"
#include "internals.h"
+enum GDBCatchSyscallsState {
+ GDB_CATCH_SYSCALLS_NONE,
+ GDB_CATCH_SYSCALLS_ALL,
+ GDB_CATCH_SYSCALLS_SELECTED,
+};
+#define GDB_NR_SYSCALLS 1024
+typedef unsigned long GDBSyscallsMask[BITS_TO_LONGS(GDB_NR_SYSCALLS)];
+
/* User-mode specific state */
typedef struct {
int fd;
char *socket_path;
int running_state;
+ /*
+ * Store syscalls mask without memory allocation in order to avoid
+ * implementing synchronization.
+ */
+ enum GDBCatchSyscallsState catch_syscalls_state;
+ GDBSyscallsMask catch_syscalls_mask;
} GDBUserState;
static GDBUserState gdbserver_user_state;
@@ -121,7 +136,7 @@ void gdb_qemu_exit(int code)
exit(code);
}
-int gdb_handlesig(CPUState *cpu, int sig)
+int gdb_handlesig_reason(CPUState *cpu, int sig, const char *reason)
{
char buf[256];
int n;
@@ -141,6 +156,9 @@ int gdb_handlesig(CPUState *cpu, int sig)
"T%02xthread:", gdb_target_signal_to_gdb(sig));
gdb_append_thread_id(cpu, gdbserver_state.str_buf);
g_string_append_c(gdbserver_state.str_buf, ';');
+ if (reason) {
+ g_string_append(gdbserver_state.str_buf, reason);
+ }
gdb_put_strbuf();
gdbserver_state.allow_stop_reply = false;
}
@@ -499,3 +517,87 @@ void gdb_syscall_handling(const char *syscall_packet)
gdb_put_packet(syscall_packet);
gdb_handlesig(gdbserver_state.c_cpu, 0);
}
+
+static bool should_catch_syscall(int num)
+{
+ switch (gdbserver_user_state.catch_syscalls_state) {
+ case GDB_CATCH_SYSCALLS_NONE:
+ return false;
+ case GDB_CATCH_SYSCALLS_ALL:
+ return true;
+ case GDB_CATCH_SYSCALLS_SELECTED:
+ if (num < 0 || num >= GDB_NR_SYSCALLS) {
+ return false;
+ } else {
+ return test_bit(num, gdbserver_user_state.catch_syscalls_mask);
+ }
+ default:
+ g_assert_not_reached();
+ }
+}
+
+void gdb_syscall_entry(CPUState *cs, int num)
+{
+ char reason[32];
+
+ if (should_catch_syscall(num)) {
+ snprintf(reason, sizeof(reason), "syscall_entry:%x;", num);
+ gdb_handlesig_reason(cs, gdb_target_sigtrap(), reason);
+ }
+}
+
+void gdb_syscall_return(CPUState *cs, int num)
+{
+ char reason[32];
+
+ if (should_catch_syscall(num)) {
+ snprintf(reason, sizeof(reason), "syscall_return:%x;", num);
+ gdb_handlesig_reason(cs, gdb_target_sigtrap(), reason);
+ }
+}
+
+void gdb_handle_set_catch_syscalls(GArray *params, void *user_ctx)
+{
+ enum GDBCatchSyscallsState catch_syscalls_state;
+ const char *param = get_param(params, 0)->data;
+ GDBSyscallsMask catch_syscalls_mask;
+ bool catch_syscalls_none;
+ unsigned int num;
+ const char *p;
+
+ catch_syscalls_none = strcmp(param, "0") == 0;
+ if (catch_syscalls_none || strcmp(param, "1") == 0) {
+ gdbserver_user_state.catch_syscalls_state =
+ catch_syscalls_none ? GDB_CATCH_SYSCALLS_NONE :
+ GDB_CATCH_SYSCALLS_ALL;
+ gdb_put_packet("OK");
+ return;
+ }
+
+ if (param[0] == '1' && param[1] == ';') {
+ catch_syscalls_state = GDB_CATCH_SYSCALLS_SELECTED;
+ memset(catch_syscalls_mask, 0, sizeof(catch_syscalls_mask));
+ for (p = ¶m[2];; p++) {
+ if (qemu_strtoui(p, &p, 16, &num) || (*p && *p != ';')) {
+ goto err;
+ }
+ if (num >= GDB_NR_SYSCALLS) {
+ /* Fall back to reporting all syscalls. */
+ catch_syscalls_state = GDB_CATCH_SYSCALLS_ALL;
+ } else {
+ set_bit(num, catch_syscalls_mask);
+ }
+ if (!*p) {
+ break;
+ }
+ }
+ gdbserver_user_state.catch_syscalls_state = catch_syscalls_state;
+ memcpy(gdbserver_user_state.catch_syscalls_mask, catch_syscalls_mask,
+ sizeof(catch_syscalls_mask));
+ gdb_put_packet("OK");
+ return;
+ }
+
+err:
+ gdb_put_packet("E00");
+}
diff --git a/include/gdbstub/user.h b/include/gdbstub/user.h
index d392e510c59..68b6534130c 100644
--- a/include/gdbstub/user.h
+++ b/include/gdbstub/user.h
@@ -10,9 +10,10 @@
#define GDBSTUB_USER_H
/**
- * gdb_handlesig() - yield control to gdb
+ * gdb_handlesig_reason() - yield control to gdb
* @cpu: CPU
* @sig: if non-zero, the signal number which caused us to stop
+ * @reason: stop reason for stop reply packet or NULL
*
* This function yields control to gdb, when a user-mode-only target
* needs to stop execution. If @sig is non-zero, then we will send a
@@ -24,7 +25,18 @@
* or 0 if no signal should be delivered, ie the signal that caused
* us to stop should be ignored.
*/
-int gdb_handlesig(CPUState *, int);
+int gdb_handlesig_reason(CPUState *, int, const char *);
+
+/**
+ * gdb_handlesig() - yield control to gdb
+ * @cpu CPU
+ * @sig: if non-zero, the signal number which caused us to stop
+ * @see gdb_handlesig_reason()
+ */
+static inline int gdb_handlesig(CPUState *cpu, int sig)
+{
+ return gdb_handlesig_reason(cpu, sig, NULL);
+}
/**
* gdb_signalled() - inform remote gdb of sig exit
@@ -39,5 +51,18 @@ void gdb_signalled(CPUArchState *as, int sig);
*/
void gdbserver_fork(CPUState *cs);
+/**
+ * gdb_syscall_entry() - inform gdb of syscall entry and yield control to it
+ * @cs: CPU
+ * @num: syscall number
+ */
+void gdb_syscall_entry(CPUState *cs, int num);
+
+/**
+ * gdb_syscall_entry() - inform gdb of syscall return and yield control to it
+ * @cs: CPU
+ * @num: syscall number
+ */
+void gdb_syscall_return(CPUState *cs, int num);
#endif /* GDBSTUB_USER_H */
diff --git a/include/user/syscall-trace.h b/include/user/syscall-trace.h
index 557f881a79b..b48b2b2d0ae 100644
--- a/include/user/syscall-trace.h
+++ b/include/user/syscall-trace.h
@@ -11,6 +11,7 @@
#define SYSCALL_TRACE_H
#include "exec/user/abitypes.h"
+#include "gdbstub/user.h"
#include "qemu/plugin.h"
#include "trace/trace-root.h"
@@ -20,7 +21,7 @@
* could potentially unify the -strace code here as well.
*/
-static inline void record_syscall_start(void *cpu, int num,
+static inline void record_syscall_start(CPUState *cpu, int num,
abi_long arg1, abi_long arg2,
abi_long arg3, abi_long arg4,
abi_long arg5, abi_long arg6,
@@ -29,11 +30,13 @@ static inline void record_syscall_start(void *cpu, int num,
qemu_plugin_vcpu_syscall(cpu, num,
arg1, arg2, arg3, arg4,
arg5, arg6, arg7, arg8);
+ gdb_syscall_entry(cpu, num);
}
-static inline void record_syscall_return(void *cpu, int num, abi_long ret)
+static inline void record_syscall_return(CPUState *cpu, int num, abi_long ret)
{
qemu_plugin_vcpu_syscall_ret(cpu, num, ret);
+ gdb_syscall_return(cpu, num);
}
--
2.43.0Ilya Leoshkevich <iii@linux.ibm.com> writes:
> GDB supports stopping on syscall entry and exit using the "catch
> syscall" command. It relies on 3 packets, which are currently not
> supported by QEMU:
>
> * qSupported:QCatchSyscalls+ [1]
> * QCatchSyscalls: [2]
> * T05syscall_entry: and T05syscall_return: [3]
>
> Implement generation and handling of these packets.
>
> [1] https://sourceware.org/gdb/current/onlinedocs/gdb.html/General-Query-Packets.html#qSupported
> [2] https://sourceware.org/gdb/current/onlinedocs/gdb.html/General-Query-Packets.html#QCatchSyscalls
> [3] https://sourceware.org/gdb/current/onlinedocs/gdb.html/Stop-Reply-Packets.html
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> gdbstub/gdbstub.c | 9 +++
> gdbstub/internals.h | 2 +
> gdbstub/user-target.c | 5 ++
> gdbstub/user.c | 104 ++++++++++++++++++++++++++++++++++-
> include/gdbstub/user.h | 29 +++++++++-
> include/user/syscall-trace.h | 7 ++-
> 6 files changed, 151 insertions(+), 5 deletions(-)
>
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index 46d752bbc2c..7e73e916bdc 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -1617,6 +1617,7 @@ static void handle_query_supported(GArray *params, void *user_ctx)
> if (gdbserver_state.c_cpu->opaque) {
> g_string_append(gdbserver_state.str_buf, ";qXfer:auxv:read+");
> }
> + g_string_append(gdbserver_state.str_buf, ";QCatchSyscalls+");
> #endif
> g_string_append(gdbserver_state.str_buf, ";qXfer:exec-file:read+");
> #endif
> @@ -1810,6 +1811,14 @@ static const GdbCmdParseEntry gdb_gen_set_table[] = {
> .schema = "l0"
> },
> #endif
> +#if defined(CONFIG_USER_ONLY)
> + {
> + .handler = gdb_handle_set_catch_syscalls,
> + .cmd = "CatchSyscalls:",
> + .cmd_startswith = 1,
> + .schema = "s0",
> + },
> +#endif
> };
>
> static void handle_gen_query(GArray *params, void *user_ctx)
> diff --git a/gdbstub/internals.h b/gdbstub/internals.h
> index 5c0c725e54c..56b7c13b750 100644
> --- a/gdbstub/internals.h
> +++ b/gdbstub/internals.h
> @@ -136,6 +136,7 @@ void gdb_append_thread_id(CPUState *cpu, GString *buf);
> int gdb_get_cpu_index(CPUState *cpu);
> unsigned int gdb_get_max_cpus(void); /* both */
> bool gdb_can_reverse(void); /* softmmu, stub for user */
> +int gdb_target_sigtrap(void); /* user */
>
> void gdb_create_default_process(GDBState *s);
>
> @@ -194,6 +195,7 @@ void gdb_handle_v_file_close(GArray *params, void *user_ctx); /* user */
> void gdb_handle_v_file_pread(GArray *params, void *user_ctx); /* user */
> void gdb_handle_v_file_readlink(GArray *params, void *user_ctx); /* user */
> void gdb_handle_query_xfer_exec_file(GArray *params, void *user_ctx); /* user */
> +void gdb_handle_set_catch_syscalls(GArray *params, void *user_ctx); /* user */
>
> void gdb_handle_query_attached(GArray *params, void *user_ctx); /* both */
>
> diff --git a/gdbstub/user-target.c b/gdbstub/user-target.c
> index c4bba4c72c7..b7d4c37cd81 100644
> --- a/gdbstub/user-target.c
> +++ b/gdbstub/user-target.c
> @@ -418,3 +418,8 @@ void gdb_handle_query_xfer_exec_file(GArray *params, void *user_ctx)
> ts->bprm->filename + offset);
> gdb_put_strbuf();
> }
> +
> +int gdb_target_sigtrap(void)
> +{
> + return TARGET_SIGTRAP;
> +}
> diff --git a/gdbstub/user.c b/gdbstub/user.c
> index dbe1d9b8875..01dd7169258 100644
> --- a/gdbstub/user.c
> +++ b/gdbstub/user.c
> @@ -10,6 +10,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/bitops.h"
> #include "qemu/cutils.h"
> #include "qemu/sockets.h"
> #include "exec/hwaddr.h"
> @@ -21,11 +22,25 @@
> #include "trace.h"
> #include "internals.h"
>
> +enum GDBCatchSyscallsState {
> + GDB_CATCH_SYSCALLS_NONE,
> + GDB_CATCH_SYSCALLS_ALL,
> + GDB_CATCH_SYSCALLS_SELECTED,
> +};
> +#define GDB_NR_SYSCALLS 1024
> +typedef unsigned long GDBSyscallsMask[BITS_TO_LONGS(GDB_NR_SYSCALLS)];
> +
> /* User-mode specific state */
> typedef struct {
> int fd;
> char *socket_path;
> int running_state;
> + /*
> + * Store syscalls mask without memory allocation in order to avoid
> + * implementing synchronization.
> + */
> + enum GDBCatchSyscallsState catch_syscalls_state;
> + GDBSyscallsMask catch_syscalls_mask;
> } GDBUserState;
>
> static GDBUserState gdbserver_user_state;
> @@ -121,7 +136,7 @@ void gdb_qemu_exit(int code)
> exit(code);
> }
>
> -int gdb_handlesig(CPUState *cpu, int sig)
> +int gdb_handlesig_reason(CPUState *cpu, int sig, const char *reason)
> {
> char buf[256];
> int n;
> @@ -141,6 +156,9 @@ int gdb_handlesig(CPUState *cpu, int sig)
> "T%02xthread:", gdb_target_signal_to_gdb(sig));
> gdb_append_thread_id(cpu, gdbserver_state.str_buf);
> g_string_append_c(gdbserver_state.str_buf, ';');
> + if (reason) {
> + g_string_append(gdbserver_state.str_buf, reason);
> + }
> gdb_put_strbuf();
> gdbserver_state.allow_stop_reply = false;
> }
> @@ -499,3 +517,87 @@ void gdb_syscall_handling(const char *syscall_packet)
> gdb_put_packet(syscall_packet);
> gdb_handlesig(gdbserver_state.c_cpu, 0);
> }
> +
> +static bool should_catch_syscall(int num)
> +{
> + switch (gdbserver_user_state.catch_syscalls_state) {
> + case GDB_CATCH_SYSCALLS_NONE:
> + return false;
> + case GDB_CATCH_SYSCALLS_ALL:
> + return true;
> + case GDB_CATCH_SYSCALLS_SELECTED:
> + if (num < 0 || num >= GDB_NR_SYSCALLS) {
> + return false;
> + } else {
> + return test_bit(num, gdbserver_user_state.catch_syscalls_mask);
> + }
> + default:
> + g_assert_not_reached();
> + }
> +}
> +
> +void gdb_syscall_entry(CPUState *cs, int num)
> +{
> + char reason[32];
> +
> + if (should_catch_syscall(num)) {
> + snprintf(reason, sizeof(reason), "syscall_entry:%x;", num);
> + gdb_handlesig_reason(cs, gdb_target_sigtrap(), reason);
> + }
> +}
> +
> +void gdb_syscall_return(CPUState *cs, int num)
> +{
> + char reason[32];
> +
> + if (should_catch_syscall(num)) {
> + snprintf(reason, sizeof(reason), "syscall_return:%x;", num);
> + gdb_handlesig_reason(cs, gdb_target_sigtrap(), reason);
> + }
> +}
I'm not super keen on re-introducing snprintf's as we've been slowly
eradicating them from the code base. How about:
g_autoptr(GString) reason = g_string_printf("syscall_return:%x;", num);
gdb_handlesig_reason(cs, gdb_target_sigtrap(), reason);
> +
> +void gdb_handle_set_catch_syscalls(GArray *params, void *user_ctx)
> +{
> + enum GDBCatchSyscallsState catch_syscalls_state;
> + const char *param = get_param(params, 0)->data;
> + GDBSyscallsMask catch_syscalls_mask;
> + bool catch_syscalls_none;
> + unsigned int num;
> + const char *p;
> +
Perhaps a little comment:
/* terminating with 0/1 to disable/enable all */
> + catch_syscalls_none = strcmp(param, "0") == 0;
> + if (catch_syscalls_none || strcmp(param, "1") == 0) {
> + gdbserver_user_state.catch_syscalls_state =
> + catch_syscalls_none ? GDB_CATCH_SYSCALLS_NONE :
> + GDB_CATCH_SYSCALLS_ALL;
> + gdb_put_packet("OK");
> + return;
> + }
/* otherwise decode the following list of syscalls... */
?
> + if (param[0] == '1' && param[1] == ';') {
> + catch_syscalls_state = GDB_CATCH_SYSCALLS_SELECTED;
> + memset(catch_syscalls_mask, 0, sizeof(catch_syscalls_mask));
> + for (p = ¶m[2];; p++) {
> + if (qemu_strtoui(p, &p, 16, &num) || (*p && *p != ';')) {
> + goto err;
> + }
> + if (num >= GDB_NR_SYSCALLS) {
> + /* Fall back to reporting all syscalls. */
> + catch_syscalls_state = GDB_CATCH_SYSCALLS_ALL;
Is this the right thing or maybe we should error because gdb sent us
something strange? In fact you could do:
if (qemu_strtoui(p, &p, 16, &num) ||
(*p && *p != ';') ||
num >= GDB_NR_SYSCALLS) {
gdb_put_packet("E00");
return;
}
and skip the goto err
> + } else {
> + set_bit(num, catch_syscalls_mask);
> + }
> + if (!*p) {
> + break;
> + }
Could this be in the for loop?
for(p = ¶m[2]; *p; p++)
> + }
> + gdbserver_user_state.catch_syscalls_state = catch_syscalls_state;
> + memcpy(gdbserver_user_state.catch_syscalls_mask, catch_syscalls_mask,
> + sizeof(catch_syscalls_mask));
> + gdb_put_packet("OK");
> + return;
> + }
> +
> +err:
> + gdb_put_packet("E00");
> +}
> diff --git a/include/gdbstub/user.h b/include/gdbstub/user.h
> index d392e510c59..68b6534130c 100644
> --- a/include/gdbstub/user.h
> +++ b/include/gdbstub/user.h
> @@ -10,9 +10,10 @@
> #define GDBSTUB_USER_H
>
> /**
> - * gdb_handlesig() - yield control to gdb
> + * gdb_handlesig_reason() - yield control to gdb
> * @cpu: CPU
> * @sig: if non-zero, the signal number which caused us to stop
> + * @reason: stop reason for stop reply packet or NULL
> *
> * This function yields control to gdb, when a user-mode-only target
> * needs to stop execution. If @sig is non-zero, then we will send a
> @@ -24,7 +25,18 @@
> * or 0 if no signal should be delivered, ie the signal that caused
> * us to stop should be ignored.
> */
> -int gdb_handlesig(CPUState *, int);
> +int gdb_handlesig_reason(CPUState *, int, const char *);
> +
> +/**
> + * gdb_handlesig() - yield control to gdb
> + * @cpu CPU
> + * @sig: if non-zero, the signal number which caused us to stop
> + * @see gdb_handlesig_reason()
> + */
> +static inline int gdb_handlesig(CPUState *cpu, int sig)
> +{
> + return gdb_handlesig_reason(cpu, sig, NULL);
> +}
>
> /**
> * gdb_signalled() - inform remote gdb of sig exit
> @@ -39,5 +51,18 @@ void gdb_signalled(CPUArchState *as, int sig);
> */
> void gdbserver_fork(CPUState *cs);
>
> +/**
> + * gdb_syscall_entry() - inform gdb of syscall entry and yield control to it
> + * @cs: CPU
> + * @num: syscall number
> + */
> +void gdb_syscall_entry(CPUState *cs, int num);
> +
> +/**
> + * gdb_syscall_entry() - inform gdb of syscall return and yield control to it
> + * @cs: CPU
> + * @num: syscall number
> + */
> +void gdb_syscall_return(CPUState *cs, int num);
>
> #endif /* GDBSTUB_USER_H */
> diff --git a/include/user/syscall-trace.h b/include/user/syscall-trace.h
> index 557f881a79b..b48b2b2d0ae 100644
> --- a/include/user/syscall-trace.h
> +++ b/include/user/syscall-trace.h
> @@ -11,6 +11,7 @@
> #define SYSCALL_TRACE_H
>
> #include "exec/user/abitypes.h"
> +#include "gdbstub/user.h"
> #include "qemu/plugin.h"
> #include "trace/trace-root.h"
>
> @@ -20,7 +21,7 @@
> * could potentially unify the -strace code here as well.
> */
>
> -static inline void record_syscall_start(void *cpu, int num,
> +static inline void record_syscall_start(CPUState *cpu, int num,
> abi_long arg1, abi_long arg2,
> abi_long arg3, abi_long arg4,
> abi_long arg5, abi_long arg6,
> @@ -29,11 +30,13 @@ static inline void record_syscall_start(void *cpu, int num,
> qemu_plugin_vcpu_syscall(cpu, num,
> arg1, arg2, arg3, arg4,
> arg5, arg6, arg7, arg8);
> + gdb_syscall_entry(cpu, num);
> }
>
> -static inline void record_syscall_return(void *cpu, int num, abi_long ret)
> +static inline void record_syscall_return(CPUState *cpu, int num, abi_long ret)
> {
> qemu_plugin_vcpu_syscall_ret(cpu, num, ret);
> + gdb_syscall_return(cpu, num);
This clean-up belongs in a separate patch.
> }
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
On Thu, 2024-02-01 at 17:15 +0000, Alex Bennée wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
>
> > GDB supports stopping on syscall entry and exit using the "catch
> > syscall" command. It relies on 3 packets, which are currently not
> > supported by QEMU:
> >
> > * qSupported:QCatchSyscalls+ [1]
> > * QCatchSyscalls: [2]
> > * T05syscall_entry: and T05syscall_return: [3]
> >
> > Implement generation and handling of these packets.
> >
> > [1]
> > https://sourceware.org/gdb/current/onlinedocs/gdb.html/General-Query-Packets.html#qSupported
> > [2]
> > https://sourceware.org/gdb/current/onlinedocs/gdb.html/General-Query-Packets.html#QCatchSyscalls
> > [3]
> > https://sourceware.org/gdb/current/onlinedocs/gdb.html/Stop-Reply-Packets.html
> >
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> > gdbstub/gdbstub.c | 9 +++
> > gdbstub/internals.h | 2 +
> > gdbstub/user-target.c | 5 ++
> > gdbstub/user.c | 104
> > ++++++++++++++++++++++++++++++++++-
> > include/gdbstub/user.h | 29 +++++++++-
> > include/user/syscall-trace.h | 7 ++-
> > 6 files changed, 151 insertions(+), 5 deletions(-)
[...]
> > @@ -499,3 +517,87 @@ void gdb_syscall_handling(const char
> > *syscall_packet)
> > gdb_put_packet(syscall_packet);
> > gdb_handlesig(gdbserver_state.c_cpu, 0);
> > }
> > +
> > +static bool should_catch_syscall(int num)
> > +{
> > + switch (gdbserver_user_state.catch_syscalls_state) {
> > + case GDB_CATCH_SYSCALLS_NONE:
> > + return false;
> > + case GDB_CATCH_SYSCALLS_ALL:
> > + return true;
> > + case GDB_CATCH_SYSCALLS_SELECTED:
> > + if (num < 0 || num >= GDB_NR_SYSCALLS) {
> > + return false;
> > + } else {
> > + return test_bit(num,
> > gdbserver_user_state.catch_syscalls_mask);
> > + }
> > + default:
> > + g_assert_not_reached();
> > + }
> > +}
> > +
> > +void gdb_syscall_entry(CPUState *cs, int num)
> > +{
> > + char reason[32];
> > +
> > + if (should_catch_syscall(num)) {
> > + snprintf(reason, sizeof(reason), "syscall_entry:%x;",
> > num);
> > + gdb_handlesig_reason(cs, gdb_target_sigtrap(), reason);
> > + }
> > +}
> > +
> > +void gdb_syscall_return(CPUState *cs, int num)
> > +{
> > + char reason[32];
> > +
> > + if (should_catch_syscall(num)) {
> > + snprintf(reason, sizeof(reason), "syscall_return:%x;",
> > num);
> > + gdb_handlesig_reason(cs, gdb_target_sigtrap(), reason);
> > + }
> > +}
>
> I'm not super keen on re-introducing snprintf's as we've been slowly
> eradicating them from the code base. How about:
>
> g_autoptr(GString) reason = g_string_printf("syscall_return:%x;",
> num);
> gdb_handlesig_reason(cs, gdb_target_sigtrap(), reason);
Ok.
> > +
> > +void gdb_handle_set_catch_syscalls(GArray *params, void *user_ctx)
> > +{
> > + enum GDBCatchSyscallsState catch_syscalls_state;
> > + const char *param = get_param(params, 0)->data;
> > + GDBSyscallsMask catch_syscalls_mask;
> > + bool catch_syscalls_none;
> > + unsigned int num;
> > + const char *p;
> > +
>
> Perhaps a little comment:
>
> /* terminating with 0/1 to disable/enable all */
>
> > + catch_syscalls_none = strcmp(param, "0") == 0;
> > + if (catch_syscalls_none || strcmp(param, "1") == 0) {
> > + gdbserver_user_state.catch_syscalls_state =
> > + catch_syscalls_none ? GDB_CATCH_SYSCALLS_NONE :
> > + GDB_CATCH_SYSCALLS_ALL;
> > + gdb_put_packet("OK");
> > + return;
> > + }
>
> /* otherwise decode the following list of syscalls... */
>
> ?
Ok.
>
> > + if (param[0] == '1' && param[1] == ';') {
> > + catch_syscalls_state = GDB_CATCH_SYSCALLS_SELECTED;
> > + memset(catch_syscalls_mask, 0,
> > sizeof(catch_syscalls_mask));
> > + for (p = ¶m[2];; p++) {
> > + if (qemu_strtoui(p, &p, 16, &num) || (*p && *p !=
> > ';')) {
> > + goto err;
> > + }
> > + if (num >= GDB_NR_SYSCALLS) {
> > + /* Fall back to reporting all syscalls. */
> > + catch_syscalls_state = GDB_CATCH_SYSCALLS_ALL;
>
> Is this the right thing or maybe we should error because gdb sent us
> something strange? In fact you could do:
I would keep this, because the size of GDBSyscallsMask is chosen
somewhat arbitrarily, and it's nice to have a fallback. The spec allows
this:
Note that if a syscall not in the list is reported, GDB will still
filter the event according to its own list from all corresponding
catch syscall commands. However, it is more efficient to only
report the requested syscalls.
> if (qemu_strtoui(p, &p, 16, &num) ||
> (*p && *p != ';') ||
> num >= GDB_NR_SYSCALLS) {
> gdb_put_packet("E00");
> return;
> }
>
> and skip the goto err
>
> > + } else {
> > + set_bit(num, catch_syscalls_mask);
> > + }
> > + if (!*p) {
> > + break;
> > + }
>
> Could this be in the for loop?
>
> for(p = ¶m[2]; *p; p++)
The idea behind this is to catch the empty syscall list, which I
believe is not a valid syntax. I will add a comment.
> > + }
> > + gdbserver_user_state.catch_syscalls_state =
> > catch_syscalls_state;
> > + memcpy(gdbserver_user_state.catch_syscalls_mask,
> > catch_syscalls_mask,
> > + sizeof(catch_syscalls_mask));
> > + gdb_put_packet("OK");
> > + return;
> > + }
> > +
> > +err:
> > + gdb_put_packet("E00");
> > +}
> > diff --git a/include/gdbstub/user.h b/include/gdbstub/user.h
> > index d392e510c59..68b6534130c 100644
> > --- a/include/gdbstub/user.h
> > +++ b/include/gdbstub/user.h
> > @@ -10,9 +10,10 @@
> > #define GDBSTUB_USER_H
> >
> > /**
> > - * gdb_handlesig() - yield control to gdb
> > + * gdb_handlesig_reason() - yield control to gdb
> > * @cpu: CPU
> > * @sig: if non-zero, the signal number which caused us to stop
> > + * @reason: stop reason for stop reply packet or NULL
> > *
> > * This function yields control to gdb, when a user-mode-only
> > target
> > * needs to stop execution. If @sig is non-zero, then we will send
> > a
> > @@ -24,7 +25,18 @@
> > * or 0 if no signal should be delivered, ie the signal that
> > caused
> > * us to stop should be ignored.
> > */
> > -int gdb_handlesig(CPUState *, int);
> > +int gdb_handlesig_reason(CPUState *, int, const char *);
> > +
> > +/**
> > + * gdb_handlesig() - yield control to gdb
> > + * @cpu CPU
> > + * @sig: if non-zero, the signal number which caused us to stop
> > + * @see gdb_handlesig_reason()
> > + */
> > +static inline int gdb_handlesig(CPUState *cpu, int sig)
> > +{
> > + return gdb_handlesig_reason(cpu, sig, NULL);
> > +}
> >
> > /**
> > * gdb_signalled() - inform remote gdb of sig exit
> > @@ -39,5 +51,18 @@ void gdb_signalled(CPUArchState *as, int sig);
> > */
> > void gdbserver_fork(CPUState *cs);
> >
> > +/**
> > + * gdb_syscall_entry() - inform gdb of syscall entry and yield
> > control to it
> > + * @cs: CPU
> > + * @num: syscall number
> > + */
> > +void gdb_syscall_entry(CPUState *cs, int num);
> > +
> > +/**
> > + * gdb_syscall_entry() - inform gdb of syscall return and yield
> > control to it
> > + * @cs: CPU
> > + * @num: syscall number
> > + */
> > +void gdb_syscall_return(CPUState *cs, int num);
> >
> > #endif /* GDBSTUB_USER_H */
> > diff --git a/include/user/syscall-trace.h b/include/user/syscall-
> > trace.h
> > index 557f881a79b..b48b2b2d0ae 100644
> > --- a/include/user/syscall-trace.h
> > +++ b/include/user/syscall-trace.h
> > @@ -11,6 +11,7 @@
> > #define SYSCALL_TRACE_H
> >
> > #include "exec/user/abitypes.h"
> > +#include "gdbstub/user.h"
> > #include "qemu/plugin.h"
> > #include "trace/trace-root.h"
> >
> > @@ -20,7 +21,7 @@
> > * could potentially unify the -strace code here as well.
> > */
> >
> > -static inline void record_syscall_start(void *cpu, int num,
> > +static inline void record_syscall_start(CPUState *cpu, int num,
> > abi_long arg1, abi_long
> > arg2,
> > abi_long arg3, abi_long
> > arg4,
> > abi_long arg5, abi_long
> > arg6,
> > @@ -29,11 +30,13 @@ static inline void record_syscall_start(void
> > *cpu, int num,
> > qemu_plugin_vcpu_syscall(cpu, num,
> > arg1, arg2, arg3, arg4,
> > arg5, arg6, arg7, arg8);
> > + gdb_syscall_entry(cpu, num);
> > }
> >
> > -static inline void record_syscall_return(void *cpu, int num,
> > abi_long ret)
> > +static inline void record_syscall_return(CPUState *cpu, int num,
> > abi_long ret)
> > {
> > qemu_plugin_vcpu_syscall_ret(cpu, num, ret);
> > + gdb_syscall_return(cpu, num);
>
> This clean-up belongs in a separate patch.
Ok.
>
> > }
>
© 2016 - 2024 Red Hat, Inc.