| Message ID | 20240116094411.216665-2-iii@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gdbstub: Implement catching syscalls | expand |
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(-) > > 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. > }
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. > > > } >
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); }
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(-)