Re: [PATCH 21/30] bsd-user/signal.c: force_sig
On Thu, Jan 13, 2022 at 4:04 PM Kyle Evans wrote: > On Thu, Jan 13, 2022 at 2:53 PM Peter Maydell > wrote: > > > > On Thu, 13 Jan 2022 at 20:29, Peter Maydell > wrote: > > > > > > On Sun, 9 Jan 2022 at 16:44, Warner Losh wrote: > > > > > > > > Force delivering a signal and generating a core file. > > > > > > +/* Abort execution with signal. */ > > > > +void QEMU_NORETURN force_sig(int target_sig) > > > > > > In linux-user we call this dump_core_and_abort(), which is > > > a name that better describes what it's actually doing. > > > > > > (Today's linux-user's force_sig() does what the Linux kernel's > > > function of that name does -- it's a wrapper around > > > queue_signal() which delivers a signal to the guest with > > > .si_code = SI_KERNEL , si_pid = si_uid = 0. > > > Whether you want one of those or not depends on what BSD > > > kernels do in that kind of "we have to kill this process" > > > situation.) > > > > It looks like the FreeBSD kernel uses sigexit() as its equivalent > > function to Linux's force_sig(), incidentally. Not sure if > > you/we would prefer the bsd-user code to follow the naming that > > FreeBSD's kernel uses or the naming linux-user takes from > > the Linux kernel. > > > > My $.02: let's go with linux-inherited linux-user names and drop in a > comment with the FreeBSD name, if they're functionally similar enough > (in general, not just for this specific case). My gut feeling is that > it'll be more useful in the long run if we can more quickly identify > parallels between the two, so changes affecting linux-user that may > benefit bsd-user are more easily identified and exchanged (and > vice-versa). > OK. I've updated the patches to do the renaming. There was one bad call to force_sig (do_sigreturn should just return EFAULT instead of generating a core). I've also fixed the SIGILL on BSD as well. The rest all look like they should be renamed. None look like they were copied from linux-user and should be the linux version :). Warner
Re: [PATCH 21/30] bsd-user/signal.c: force_sig
On Thu, Jan 13, 2022 at 2:53 PM Peter Maydell wrote: > > On Thu, 13 Jan 2022 at 20:29, Peter Maydell wrote: > > > > On Sun, 9 Jan 2022 at 16:44, Warner Losh wrote: > > > > > > Force delivering a signal and generating a core file. > > > > +/* Abort execution with signal. */ > > > +void QEMU_NORETURN force_sig(int target_sig) > > > > In linux-user we call this dump_core_and_abort(), which is > > a name that better describes what it's actually doing. > > > > (Today's linux-user's force_sig() does what the Linux kernel's > > function of that name does -- it's a wrapper around > > queue_signal() which delivers a signal to the guest with > > .si_code = SI_KERNEL , si_pid = si_uid = 0. > > Whether you want one of those or not depends on what BSD > > kernels do in that kind of "we have to kill this process" > > situation.) > > It looks like the FreeBSD kernel uses sigexit() as its equivalent > function to Linux's force_sig(), incidentally. Not sure if > you/we would prefer the bsd-user code to follow the naming that > FreeBSD's kernel uses or the naming linux-user takes from > the Linux kernel. > My $.02: let's go with linux-inherited linux-user names and drop in a comment with the FreeBSD name, if they're functionally similar enough (in general, not just for this specific case). My gut feeling is that it'll be more useful in the long run if we can more quickly identify parallels between the two, so changes affecting linux-user that may benefit bsd-user are more easily identified and exchanged (and vice-versa). Thanks, Kyle Evans
Re: [PATCH 21/30] bsd-user/signal.c: force_sig
On Thu, 13 Jan 2022 at 20:29, Peter Maydell wrote: > > On Sun, 9 Jan 2022 at 16:44, Warner Losh wrote: > > > > Force delivering a signal and generating a core file. > > +/* Abort execution with signal. */ > > +void QEMU_NORETURN force_sig(int target_sig) > > In linux-user we call this dump_core_and_abort(), which is > a name that better describes what it's actually doing. > > (Today's linux-user's force_sig() does what the Linux kernel's > function of that name does -- it's a wrapper around > queue_signal() which delivers a signal to the guest with > .si_code = SI_KERNEL , si_pid = si_uid = 0. > Whether you want one of those or not depends on what BSD > kernels do in that kind of "we have to kill this process" > situation.) It looks like the FreeBSD kernel uses sigexit() as its equivalent function to Linux's force_sig(), incidentally. Not sure if you/we would prefer the bsd-user code to follow the naming that FreeBSD's kernel uses or the naming linux-user takes from the Linux kernel. -- PMM
Re: [PATCH 21/30] bsd-user/signal.c: force_sig
On Sun, 9 Jan 2022 at 16:44, Warner Losh wrote:
>
> Force delivering a signal and generating a core file.
>
> Signed-off-by: Stacey Son
> Signed-off-by: Kyle Evans
> Signed-off-by: Warner Losh
> ---
> bsd-user/qemu.h | 1 +
> bsd-user/signal.c | 59 +
> bsd-user/syscall_defs.h | 1 +
> 3 files changed, 61 insertions(+)
>
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index 7c54a933eb8..e12617f5d69 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -223,6 +223,7 @@ void queue_signal(CPUArchState *env, int sig,
> target_siginfo_t *info);
> abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, abi_ulong
> sp);
> int target_to_host_signal(int sig);
> int host_to_target_signal(int sig);
> +void QEMU_NORETURN force_sig(int target_sig);
>
> /* mmap.c */
> int target_mprotect(abi_ulong start, abi_ulong len, int prot);
> diff --git a/bsd-user/signal.c b/bsd-user/signal.c
> index 824535be8b8..97f42f9c45e 100644
> --- a/bsd-user/signal.c
> +++ b/bsd-user/signal.c
> @@ -109,6 +109,65 @@ static int core_dump_signal(int sig)
> }
> }
>
> +/* Abort execution with signal. */
> +void QEMU_NORETURN force_sig(int target_sig)
In linux-user we call this dump_core_and_abort(), which is
a name that better describes what it's actually doing.
(Today's linux-user's force_sig() does what the Linux kernel's
function of that name does -- it's a wrapper around
queue_signal() which delivers a signal to the guest with
.si_code = SI_KERNEL , si_pid = si_uid = 0.
Whether you want one of those or not depends on what BSD
kernels do in that kind of "we have to kill this process"
situation.)
> +{
> +CPUArchState *env = thread_cpu->env_ptr;
> +CPUState *cpu = env_cpu(env);
> +TaskState *ts = cpu->opaque;
> +int core_dumped = 0;
> +int host_sig;
> +struct sigaction act;
> +
> +host_sig = target_to_host_signal(target_sig);
> +gdb_signalled(env, target_sig);
> +
> +/* Dump core if supported by target binary format */
> +if (core_dump_signal(target_sig) && (ts->bprm->core_dump != NULL)) {
> +stop_all_tasks();
> +core_dumped =
> +((*ts->bprm->core_dump)(target_sig, env) == 0);
> +}
> +if (core_dumped) {
> +struct rlimit nodump;
> +
> +/*
> + * We already dumped the core of target process, we don't want
> + * a coredump of qemu itself.
> + */
> + getrlimit(RLIMIT_CORE, &nodump);
> + nodump.rlim_cur = 0;
> + setrlimit(RLIMIT_CORE, &nodump);
> + (void) fprintf(stderr, "qemu: uncaught target signal %d (%s) "
> + "- %s\n", target_sig, strsignal(host_sig), "core dumped");
> +}
> +
> +/*
> + * The proper exit code for dying from an uncaught signal is
> + * -. The kernel doesn't allow exit() or _exit() to pass
> + * a negative value. To get the proper exit code we need to
> + * actually die from an uncaught signal. Here the default signal
> + * handler is installed, we send ourself a signal and we wait for
> + * it to arrive.
> + */
> +memset(&act, 0, sizeof(act));
> +sigfillset(&act.sa_mask);
> +act.sa_handler = SIG_DFL;
> +sigaction(host_sig, &act, NULL);
> +
> +kill(getpid(), host_sig);
> +
> +/*
> + * Make sure the signal isn't masked (just reuse the mask inside
> + * of act).
> + */
> +sigdelset(&act.sa_mask, host_sig);
> +sigsuspend(&act.sa_mask);
> +
> +/* unreachable */
> +abort();
> +}
> +
> /*
> * Queue a signal so that it will be send to the virtual CPU as soon as
> * possible.
> diff --git a/bsd-user/syscall_defs.h b/bsd-user/syscall_defs.h
> index 04a1a886d7b..62b472b990b 100644
> --- a/bsd-user/syscall_defs.h
> +++ b/bsd-user/syscall_defs.h
> @@ -21,6 +21,7 @@
> #define _SYSCALL_DEFS_H_
>
> #include
> +#include
>
> #include "errno_defs.h"
>
-- PMM
