| Message ID | 20240307182623.1450717-3-gustavo.romero@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/5] gdbstub: Rename back gdb_handlesig | expand |
On 3/7/24 08:26, Gustavo Romero wrote: > Save target's siginfo into gdbserver_state so it can be used later, for > example, in any stub that requires the target's si_signo and si_code. > > This change affects only linux-user mode. > > Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > --- > gdbstub/internals.h | 3 +++ > gdbstub/user-target.c | 3 ++- > gdbstub/user.c | 14 ++++++++++---- > include/gdbstub/user.h | 6 +++++- > linux-user/main.c | 2 +- > linux-user/signal.c | 5 ++++- > 6 files changed, 25 insertions(+), 8 deletions(-) > > diff --git a/gdbstub/internals.h b/gdbstub/internals.h > index 56b7c13b75..a7cc69dab3 100644 > --- a/gdbstub/internals.h > +++ b/gdbstub/internals.h > @@ -58,6 +58,9 @@ typedef struct GDBState { > int line_csum; /* checksum at the end of the packet */ > GByteArray *last_packet; > int signal; > +#ifdef CONFIG_USER_ONLY > + uint8_t siginfo[MAX_SIGINFO_LENGTH]; > +#endif If we this in GDBUserState in user.c -- no need for ifdefs then. > --- a/gdbstub/user-target.c > +++ b/gdbstub/user-target.c > @@ -10,11 +10,12 @@ > #include "qemu/osdep.h" > #include "exec/gdbstub.h" > #include "qemu.h" > -#include "internals.h" > #ifdef CONFIG_LINUX > #include "linux-user/loader.h" > #include "linux-user/qemu.h" > +#include "gdbstub/user.h" > #endif > +#include "internals.h" > > /* > * Map target signal numbers to GDB protocol signal numbers and vice Why are any changes required here? Perhaps this is improper patch split from one of the others? > @@ -140,6 +141,11 @@ int gdb_handlesig(CPUState *cpu, int sig, const char *reason) > return sig; > } > > + if (siginfo) { > + /* Save target-specific siginfo. */ > + memcpy(gdbserver_state.siginfo, siginfo, siginfo_len); > + } A comment here about asserting the size at compile-time elsewhere would be welcome for future code browsers. Need to record siginfo_len for later use -- you don't want to expose all 128 bytes if the actual structure is smaller. > @@ -510,7 +516,7 @@ void gdb_breakpoint_remove_all(CPUState *cs) > void gdb_syscall_handling(const char *syscall_packet) > { > gdb_put_packet(syscall_packet); > - gdb_handlesig(gdbserver_state.c_cpu, 0, NULL); > + gdb_handlesig(gdbserver_state.c_cpu, 0, NULL, NULL, 0); > } > > static bool should_catch_syscall(int num) > @@ -528,7 +534,7 @@ void gdb_syscall_entry(CPUState *cs, int num) > { > if (should_catch_syscall(num)) { > g_autofree char *reason = g_strdup_printf("syscall_entry:%x;", num); > - gdb_handlesig(cs, gdb_target_sigtrap(), reason); > + gdb_handlesig(cs, gdb_target_sigtrap(), reason, NULL, 0); > } > } > > @@ -536,7 +542,7 @@ void gdb_syscall_return(CPUState *cs, int num) > { > if (should_catch_syscall(num)) { > g_autofree char *reason = g_strdup_printf("syscall_return:%x;", num); > - gdb_handlesig(cs, gdb_target_sigtrap(), reason); > + gdb_handlesig(cs, gdb_target_sigtrap(), reason, NULL, 0); > } All of this makes me wonder if we should provide a different interface for syscalls, even if it uses the same code paths internally. Do we want to zero the gdbserver siginfo to indicate that the contents are no longer valid? I know it's not a real signal delivered to the process, but might we need to construct a simple siginfo struct to match the sigtrap? r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 3/7/24 08:26, Gustavo Romero wrote: >> Save target's siginfo into gdbserver_state so it can be used later, for >> example, in any stub that requires the target's si_signo and si_code. >> This change affects only linux-user mode. >> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> >> Suggested-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> gdbstub/internals.h | 3 +++ >> gdbstub/user-target.c | 3 ++- >> gdbstub/user.c | 14 ++++++++++---- >> include/gdbstub/user.h | 6 +++++- >> linux-user/main.c | 2 +- >> linux-user/signal.c | 5 ++++- >> 6 files changed, 25 insertions(+), 8 deletions(-) >> diff --git a/gdbstub/internals.h b/gdbstub/internals.h >> index 56b7c13b75..a7cc69dab3 100644 >> --- a/gdbstub/internals.h >> +++ b/gdbstub/internals.h >> @@ -58,6 +58,9 @@ typedef struct GDBState { >> int line_csum; /* checksum at the end of the packet */ >> GByteArray *last_packet; >> int signal; >> +#ifdef CONFIG_USER_ONLY >> + uint8_t siginfo[MAX_SIGINFO_LENGTH]; >> +#endif > > If we this in GDBUserState in user.c -- no need for ifdefs then. Although it does break on FreeBSD's user target: FAILED: libqemu-arm-bsd-user.fa.p/gdbstub_user-target.c.o cc -m64 -mcx16 -Ilibqemu-arm-bsd-user.fa.p -I. -I.. -Itarget/arm -I../target/arm -I../common-user/host/x86_64 -I../bsd-user/include -Ibsd-user/freebsd -I../bsd-user/freebsd -I../bsd-user/host/x86_64 -Ibsd-user -I../bsd-user -I../bsd-user/arm -Iqapi -Itrace -Iui -Iui/shader -I/usr/local/include/capstone -I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include -I/usr/local/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -Wempty-body -Wendif-labels -Wexpansion-to-defined -Wformat-security -Wformat-y2k -Wignored-qualifiers -Winit-self -Wmissing-format-attribute -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wredundant-decls -Wstrict-prototypes -Wtype-limits -Wundef -Wvla -Wwrite-strings -Wno-gnu-variable-sized-type-not-at-end -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-psabi -Wno-shift-negative-value -Wno-string-plus-int -Wno-tautological-type-limit-compare -Wno-typedef-redefinition -Wthread-safety -iquote . -iquote /tmp/cirrus-ci-build -iquote /tmp/cirrus-ci-build/include -iquote /tmp/cirrus-ci-build/host/include/x86_64 -iquote /tmp/cirrus-ci-build/host/include/generic -iquote /tmp/cirrus-ci-build/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -ftrivial-auto-var-init=zero -fPIE -DNEED_CPU_H '-DCONFIG_TARGET="arm-bsd-user-config-target.h"' '-DCONFIG_DEVICES="arm-bsd-user-config-devices.h"' -MD -MQ libqemu-arm-bsd-user.fa.p/gdbstub_user-target.c.o -MF libqemu-arm-bsd-user.fa.p/gdbstub_user-target.c.o.d -o libqemu-arm-bsd-user.fa.p/gdbstub_user-target.c.o -c ../gdbstub/user-target.c In file included from ../gdbstub/user-target.c:18: ../gdbstub/internals.h:62:21: error: use of undeclared identifier 'MAX_SIGINFO_LENGTH' 62 | uint8_t siginfo[MAX_SIGINFO_LENGTH]; | ^ 1 error generated. [2084/6731] Compiling C object libqemu-arm See: https://gitlab.com/stsquad/qemu/-/jobs/6345829419 <snip>
Hi Alex, On 3/7/24 7:33 PM, Alex Bennée wrote: > Richard Henderson <richard.henderson@linaro.org> writes: > >> On 3/7/24 08:26, Gustavo Romero wrote: >>> Save target's siginfo into gdbserver_state so it can be used later, for >>> example, in any stub that requires the target's si_signo and si_code. >>> This change affects only linux-user mode. >>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> >>> Suggested-by: Richard Henderson <richard.henderson@linaro.org> >>> --- >>> gdbstub/internals.h | 3 +++ >>> gdbstub/user-target.c | 3 ++- >>> gdbstub/user.c | 14 ++++++++++---- >>> include/gdbstub/user.h | 6 +++++- >>> linux-user/main.c | 2 +- >>> linux-user/signal.c | 5 ++++- >>> 6 files changed, 25 insertions(+), 8 deletions(-) >>> diff --git a/gdbstub/internals.h b/gdbstub/internals.h >>> index 56b7c13b75..a7cc69dab3 100644 >>> --- a/gdbstub/internals.h >>> +++ b/gdbstub/internals.h >>> @@ -58,6 +58,9 @@ typedef struct GDBState { >>> int line_csum; /* checksum at the end of the packet */ >>> GByteArray *last_packet; >>> int signal; >>> +#ifdef CONFIG_USER_ONLY >>> + uint8_t siginfo[MAX_SIGINFO_LENGTH]; >>> +#endif >> >> If we this in GDBUserState in user.c -- no need for ifdefs then. > > Although it does break on FreeBSD's user target: > > FAILED: libqemu-arm-bsd-user.fa.p/gdbstub_user-target.c.o > cc -m64 -mcx16 -Ilibqemu-arm-bsd-user.fa.p -I. -I.. -Itarget/arm -I../target/arm -I../common-user/host/x86_64 -I../bsd-user/include -Ibsd-user/freebsd -I../bsd-user/freebsd -I../bsd-user/host/x86_64 -Ibsd-user -I../bsd-user -I../bsd-user/arm -Iqapi -Itrace -Iui -Iui/shader -I/usr/local/include/capstone -I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include -I/usr/local/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -Wempty-body -Wendif-labels -Wexpansion-to-defined -Wformat-security -Wformat-y2k -Wignored-qualifiers -Winit-self -Wmissing-format-attribute -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wredundant-decls -Wstrict-prototypes -Wtype-limits -Wundef -Wvla -Wwrite-strings -Wno-gnu-variable-sized-type-not-at-end -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-psabi -Wno-shift-negative-value -Wno-string-plus-int -Wno-tautological-type-limit-compare -Wno-typedef-redefinition -Wthread-safety -iquote . -iquote /tmp/cirrus-ci-build -iquote /tmp/cirrus-ci-build/include -iquote /tmp/cirrus-ci-build/host/include/x86_64 -iquote /tmp/cirrus-ci-build/host/include/generic -iquote /tmp/cirrus-ci-build/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -ftrivial-auto-var-init=zero -fPIE -DNEED_CPU_H '-DCONFIG_TARGET="arm-bsd-user-config-target.h"' '-DCONFIG_DEVICES="arm-bsd-user-config-devices.h"' -MD -MQ libqemu-arm-bsd-user.fa.p/gdbstub_user-target.c.o -MF libqemu-arm-bsd-user.fa.p/gdbstub_user-target.c.o.d -o libqemu-arm-bsd-user.fa.p/gdbstub_user-target.c.o -c ../gdbstub/user-target.c > In file included from ../gdbstub/user-target.c:18: > ../gdbstub/internals.h:62:21: error: use of undeclared identifier 'MAX_SIGINFO_LENGTH' > 62 | uint8_t siginfo[MAX_SIGINFO_LENGTH]; > | ^ > 1 error generated. > [2084/6731] Compiling C object libqemu-arm > > See: https://gitlab.com/stsquad/qemu/-/jobs/6345829419 argh, I've tested all targets for linux-user, but missed bsd-user. I've tried once to build it but that requires a BSD-like host, which I don't have at the moment, then I forgot about it... Let me setup one and review the change in the light of the comments from you and Richard. Thanks! Cheers, Gustavo
Gustavo Romero <gustavo.romero@linaro.org> writes: > Hi Alex, > > On 3/7/24 7:33 PM, Alex Bennée wrote: >> Richard Henderson <richard.henderson@linaro.org> writes: >> >>> On 3/7/24 08:26, Gustavo Romero wrote: >>>> Save target's siginfo into gdbserver_state so it can be used later, for >>>> example, in any stub that requires the target's si_signo and si_code. >>>> This change affects only linux-user mode. >>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> >>>> Suggested-by: Richard Henderson <richard.henderson@linaro.org> >>>> --- >>>> gdbstub/internals.h | 3 +++ >>>> gdbstub/user-target.c | 3 ++- >>>> gdbstub/user.c | 14 ++++++++++---- >>>> include/gdbstub/user.h | 6 +++++- >>>> linux-user/main.c | 2 +- >>>> linux-user/signal.c | 5 ++++- >>>> 6 files changed, 25 insertions(+), 8 deletions(-) >>>> diff --git a/gdbstub/internals.h b/gdbstub/internals.h >>>> index 56b7c13b75..a7cc69dab3 100644 >>>> --- a/gdbstub/internals.h >>>> +++ b/gdbstub/internals.h >>>> @@ -58,6 +58,9 @@ typedef struct GDBState { >>>> int line_csum; /* checksum at the end of the packet */ >>>> GByteArray *last_packet; >>>> int signal; >>>> +#ifdef CONFIG_USER_ONLY >>>> + uint8_t siginfo[MAX_SIGINFO_LENGTH]; >>>> +#endif >>> >>> If we this in GDBUserState in user.c -- no need for ifdefs then. >> Although it does break on FreeBSD's user target: >> FAILED: libqemu-arm-bsd-user.fa.p/gdbstub_user-target.c.o >> cc -m64 -mcx16 -Ilibqemu-arm-bsd-user.fa.p -I. -I.. -Itarget/arm -I../target/arm -I../common-user/host/x86_64 -I../bsd-user/include -Ibsd-user/freebsd -I../bsd-user/freebsd -I../bsd-user/host/x86_64 -Ibsd-user -I../bsd-user -I../bsd-user/arm -Iqapi -Itrace -Iui -Iui/shader -I/usr/local/include/capstone -I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include -I/usr/local/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -Wempty-body -Wendif-labels -Wexpansion-to-defined -Wformat-security -Wformat-y2k -Wignored-qualifiers -Winit-self -Wmissing-format-attribute -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wredundant-decls -Wstrict-prototypes -Wtype-limits -Wundef -Wvla -Wwrite-strings -Wno-gnu-variable-sized-type-not-at-end -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-psabi -Wno-shift-negative-value -Wno-string-plus-int -Wno-tautological-type-limit-compare -Wno-typedef-redefinition -Wthread-safety -iquote . -iquote /tmp/cirrus-ci-build -iquote /tmp/cirrus-ci-build/include -iquote /tmp/cirrus-ci-build/host/include/x86_64 -iquote /tmp/cirrus-ci-build/host/include/generic -iquote /tmp/cirrus-ci-build/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -ftrivial-auto-var-init=zero -fPIE -DNEED_CPU_H '-DCONFIG_TARGET="arm-bsd-user-config-target.h"' '-DCONFIG_DEVICES="arm-bsd-user-config-devices.h"' -MD -MQ libqemu-arm-bsd-user.fa.p/gdbstub_user-target.c.o -MF libqemu-arm-bsd-user.fa.p/gdbstub_user-target.c.o.d -o libqemu-arm-bsd-user.fa.p/gdbstub_user-target.c.o -c ../gdbstub/user-target.c >> In file included from ../gdbstub/user-target.c:18: >> ../gdbstub/internals.h:62:21: error: use of undeclared identifier 'MAX_SIGINFO_LENGTH' >> 62 | uint8_t siginfo[MAX_SIGINFO_LENGTH]; >> | ^ >> 1 error generated. >> [2084/6731] Compiling C object libqemu-arm >> See: https://gitlab.com/stsquad/qemu/-/jobs/6345829419 > > argh, I've tested all targets for linux-user, but missed bsd-user. I've tried > once to build it but that requires a BSD-like host, which I don't have at the > moment, then I forgot about it... Let me setup one and review the change in > the light of the comments from you and Richard. make vm-build-[open|net|free]bsd see make vm-help for details. > > Thanks! > > > Cheers, > Gustavo
Hi Richard, On 3/7/24 6:09 PM, Richard Henderson wrote: > On 3/7/24 08:26, Gustavo Romero wrote: >> Save target's siginfo into gdbserver_state so it can be used later, for >> example, in any stub that requires the target's si_signo and si_code. >> >> This change affects only linux-user mode. >> >> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> >> Suggested-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> gdbstub/internals.h | 3 +++ >> gdbstub/user-target.c | 3 ++- >> gdbstub/user.c | 14 ++++++++++---- >> include/gdbstub/user.h | 6 +++++- >> linux-user/main.c | 2 +- >> linux-user/signal.c | 5 ++++- >> 6 files changed, 25 insertions(+), 8 deletions(-) >> >> diff --git a/gdbstub/internals.h b/gdbstub/internals.h >> index 56b7c13b75..a7cc69dab3 100644 >> --- a/gdbstub/internals.h >> +++ b/gdbstub/internals.h >> @@ -58,6 +58,9 @@ typedef struct GDBState { >> int line_csum; /* checksum at the end of the packet */ >> GByteArray *last_packet; >> int signal; >> +#ifdef CONFIG_USER_ONLY >> + uint8_t siginfo[MAX_SIGINFO_LENGTH]; >> +#endif > > If we this in GDBUserState in user.c -- no need for ifdefs then. Thanks, I've moved it to user.c. >> --- a/gdbstub/user-target.c >> +++ b/gdbstub/user-target.c >> @@ -10,11 +10,12 @@ >> #include "qemu/osdep.h" >> #include "exec/gdbstub.h" >> #include "qemu.h" >> -#include "internals.h" >> #ifdef CONFIG_LINUX >> #include "linux-user/loader.h" >> #include "linux-user/qemu.h" >> +#include "gdbstub/user.h" >> #endif >> +#include "internals.h" >> /* >> * Map target signal numbers to GDB protocol signal numbers and vice > > Why are any changes required here? > Perhaps this is improper patch split from one of the others? This was intentional. Because I declared siginfo[MAX_SIGINFO_LENGTH] in GDBState struct, which is in internals.h and MAX_SIGINFO_LENGTH is defined in gdbstub/user.h I had to move internals.h after user.h was included so MAX_SIGINFO_LENGTH could be found. I'm reverting it. >> @@ -140,6 +141,11 @@ int gdb_handlesig(CPUState *cpu, int sig, const char *reason) >> return sig; >> } >> + if (siginfo) { >> + /* Save target-specific siginfo. */ >> + memcpy(gdbserver_state.siginfo, siginfo, siginfo_len); >> + } > > A comment here about asserting the size at compile-time elsewhere would be welcome for future code browsers. Done. > Need to record siginfo_len for later use -- you don't want to expose all 128 bytes if the actual structure is smaller. In the stub, the full size is only used to check if the requested offset+len is valid. So the only size that matters for reading data from siginfo and assembling the reply is the length in the query, not siginfo_len. But I agree it's better, even more now that I moved the stub from user-target.c to user.c. >> @@ -510,7 +516,7 @@ void gdb_breakpoint_remove_all(CPUState *cs) >> void gdb_syscall_handling(const char *syscall_packet) >> { >> gdb_put_packet(syscall_packet); >> - gdb_handlesig(gdbserver_state.c_cpu, 0, NULL); >> + gdb_handlesig(gdbserver_state.c_cpu, 0, NULL, NULL, 0); >> } >> static bool should_catch_syscall(int num) >> @@ -528,7 +534,7 @@ void gdb_syscall_entry(CPUState *cs, int num) >> { >> if (should_catch_syscall(num)) { >> g_autofree char *reason = g_strdup_printf("syscall_entry:%x;", num); >> - gdb_handlesig(cs, gdb_target_sigtrap(), reason); >> + gdb_handlesig(cs, gdb_target_sigtrap(), reason, NULL, 0); >> } >> } >> @@ -536,7 +542,7 @@ void gdb_syscall_return(CPUState *cs, int num) >> { >> if (should_catch_syscall(num)) { >> g_autofree char *reason = g_strdup_printf("syscall_return:%x;", num); >> - gdb_handlesig(cs, gdb_target_sigtrap(), reason); >> + gdb_handlesig(cs, gdb_target_sigtrap(), reason, NULL, 0); >> } > > All of this makes me wonder if we should provide a different interface for syscalls, even if it uses the same code paths internally. Should I address it in this series? I'm sure how that interface would be. > Do we want to zero the gdbserver siginfo to indicate that the contents are no longer valid? I know it's not a real signal delivered to the process, but might we need to construct a simple siginfo struct to match the sigtrap? In gdb_handlesig we always copy the full size of siginfo to gdbserver_user_state siginfo, which is passed in siginfo_len and now recorded gdbserver siginfo_len too for later use. Isn't that copy guaranteeing that gdbserver siginfo has always no stale data? Cheers, Gustavo
On 3/8/24 09:25, Alex Bennée wrote: > make vm-build-[open|net|free]bsd > > see make vm-help for details. That won't build freebsd user. Something I've mentioned to you before... r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 3/8/24 09:25, Alex Bennée wrote: >> make vm-build-[open|net|free]bsd >> see make vm-help for details. > > That won't build freebsd user. > Something I've mentioned to you before... make vm-build-freebsd BUILD_TARGET=all I guess the default "check" target doesn't work due to the missing cross compilers meaning we have no check-tcg target in the default check list. > > > r~
diff --git a/gdbstub/internals.h b/gdbstub/internals.h index 56b7c13b75..a7cc69dab3 100644 --- a/gdbstub/internals.h +++ b/gdbstub/internals.h @@ -58,6 +58,9 @@ typedef struct GDBState { int line_csum; /* checksum at the end of the packet */ GByteArray *last_packet; int signal; +#ifdef CONFIG_USER_ONLY + uint8_t siginfo[MAX_SIGINFO_LENGTH]; +#endif bool multiprocess; GDBProcess *processes; int process_num; diff --git a/gdbstub/user-target.c b/gdbstub/user-target.c index b7d4c37cd8..215bf33ab3 100644 --- a/gdbstub/user-target.c +++ b/gdbstub/user-target.c @@ -10,11 +10,12 @@ #include "qemu/osdep.h" #include "exec/gdbstub.h" #include "qemu.h" -#include "internals.h" #ifdef CONFIG_LINUX #include "linux-user/loader.h" #include "linux-user/qemu.h" +#include "gdbstub/user.h" #endif +#include "internals.h" /* * Map target signal numbers to GDB protocol signal numbers and vice diff --git a/gdbstub/user.c b/gdbstub/user.c index a157e67f95..777fa78ef4 100644 --- a/gdbstub/user.c +++ b/gdbstub/user.c @@ -131,7 +131,8 @@ void gdb_qemu_exit(int code) exit(code); } -int gdb_handlesig(CPUState *cpu, int sig, const char *reason) +int gdb_handlesig(CPUState *cpu, int sig, const char *reason, void *siginfo, + int siginfo_len) { char buf[256]; int n; @@ -140,6 +141,11 @@ int gdb_handlesig(CPUState *cpu, int sig, const char *reason) return sig; } + if (siginfo) { + /* Save target-specific siginfo. */ + memcpy(gdbserver_state.siginfo, siginfo, siginfo_len); + } + /* disable single step if it was enabled */ cpu_single_step(cpu, 0); tb_flush(cpu); @@ -510,7 +516,7 @@ void gdb_breakpoint_remove_all(CPUState *cs) void gdb_syscall_handling(const char *syscall_packet) { gdb_put_packet(syscall_packet); - gdb_handlesig(gdbserver_state.c_cpu, 0, NULL); + gdb_handlesig(gdbserver_state.c_cpu, 0, NULL, NULL, 0); } static bool should_catch_syscall(int num) @@ -528,7 +534,7 @@ void gdb_syscall_entry(CPUState *cs, int num) { if (should_catch_syscall(num)) { g_autofree char *reason = g_strdup_printf("syscall_entry:%x;", num); - gdb_handlesig(cs, gdb_target_sigtrap(), reason); + gdb_handlesig(cs, gdb_target_sigtrap(), reason, NULL, 0); } } @@ -536,7 +542,7 @@ void gdb_syscall_return(CPUState *cs, int num) { if (should_catch_syscall(num)) { g_autofree char *reason = g_strdup_printf("syscall_return:%x;", num); - gdb_handlesig(cs, gdb_target_sigtrap(), reason); + gdb_handlesig(cs, gdb_target_sigtrap(), reason, NULL, 0); } } diff --git a/include/gdbstub/user.h b/include/gdbstub/user.h index 6647af2123..0ec9a7e596 100644 --- a/include/gdbstub/user.h +++ b/include/gdbstub/user.h @@ -9,11 +9,15 @@ #ifndef GDBSTUB_USER_H #define GDBSTUB_USER_H +#define MAX_SIGINFO_LENGTH 128 + /** * gdb_handlesig() - 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 + * @siginfo: target-specific siginfo struct + * @siginfo_len: target-specific siginfo struct length * * 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 @@ -25,7 +29,7 @@ * or 0 if no signal should be delivered, ie the signal that caused * us to stop should be ignored. */ -int gdb_handlesig(CPUState *, int, const char *); +int gdb_handlesig(CPUState *, int, const char *, void *, int); /** * gdb_signalled() - inform remote gdb of sig exit diff --git a/linux-user/main.c b/linux-user/main.c index 049fd85a2a..3187be48d6 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -1014,7 +1014,7 @@ int main(int argc, char **argv, char **envp) gdbstub); exit(EXIT_FAILURE); } - gdb_handlesig(cpu, 0, NULL); + gdb_handlesig(cpu, 0, NULL, NULL, 0); } #ifdef CONFIG_SEMIHOSTING diff --git a/linux-user/signal.c b/linux-user/signal.c index 7a4c8e416e..98d1eacffe 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -34,6 +34,9 @@ #include "user/safe-syscall.h" #include "tcg/tcg.h" +/* target_siginfo_t must fit in gdbstub's siginfo save area. */ +QEMU_BUILD_BUG_ON(sizeof(target_siginfo_t) > MAX_SIGINFO_LENGTH); + static struct target_sigaction sigact_table[TARGET_NSIG]; static void host_signal_handler(int host_signum, siginfo_t *info, @@ -1186,7 +1189,7 @@ static void handle_pending_signal(CPUArchState *cpu_env, int sig, */ tswap_siginfo(&k->info, &k->info); - sig = gdb_handlesig(cpu, sig, NULL); + sig = gdb_handlesig(cpu, sig, NULL, &k->info, sizeof(k->info)); if (!sig) { sa = NULL; handler = TARGET_SIG_IGN;
Save target's siginfo into gdbserver_state so it can be used later, for example, in any stub that requires the target's si_signo and si_code. This change affects only linux-user mode. Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> Suggested-by: Richard Henderson <richard.henderson@linaro.org> --- gdbstub/internals.h | 3 +++ gdbstub/user-target.c | 3 ++- gdbstub/user.c | 14 ++++++++++---- include/gdbstub/user.h | 6 +++++- linux-user/main.c | 2 +- linux-user/signal.c | 5 ++++- 6 files changed, 25 insertions(+), 8 deletions(-)