On Thu, Oct 24, 2024 at 2:00 PM Ilya Leoshkevich <i...@linux.ibm.com> wrote:
> Attaching to the gdbstub of a running process requires stopping its
> threads. For threads that run on a CPU, cpu_exit() is enough, but the
> only way to grab attention of a thread that is stuck in a long-running
> syscall is to interrupt it with a signal.
>
> Reserve a host realtime signal for this, just like it's already done
> for TARGET_SIGABRT on Linux. This may reduce the number of available
> guest realtime signals by one, but this is acceptable, since there are
> quite a lot of them, and it's unlikely that there are apps that need
> them all.
>
> Set signal_pending for the safe_sycall machinery to prevent invoking
> the syscall. This is a lie, since we don't queue a guest signal, but
> process_pending_signals() can handle the absence of pending signals.
> The syscall returns with QEMU_ERESTARTSYS errno, which arranges for
> the automatic restart. This is important, because it helps avoiding
> disturbing poorly written guests.
>
> Signed-off-by: Ilya Leoshkevich <i...@linux.ibm.com>
> ---
> bsd-user/signal.c | 12 ++++++++++++
> include/user/signal.h | 2 ++
> linux-user/signal.c | 11 +++++++++++
> 3 files changed, 25 insertions(+)
>
> diff --git a/bsd-user/signal.c b/bsd-user/signal.c
> index a2b11a97131..992736df5c5 100644
> --- a/bsd-user/signal.c
> +++ b/bsd-user/signal.c
> @@ -49,6 +49,8 @@ static inline int sas_ss_flags(TaskState *ts, unsigned
> long sp)
> on_sig_stack(ts, sp) ? SS_ONSTACK : 0;
> }
>
> +int host_interrupt_signal = SIGRTMAX;
> +
>
I'd be tempted to use SIGRTMAX + 1 or even TARGET_NSIG. 127 or 128 would
work and not overflow any arrays (or hit any bounds tests) I'd likely use
SIGRTMAX + 1,
though, since it avoids any edge-cases from sig == NSIG that might be in
the code
unnoticed.
Now, having said that, I don't think that there's too many (any?) programs
we need
to run as bsd-user that have real-time signals, much less one that uses
SIGRTMAX,
but stranger things have happened. But it is a little wiggle room just in
case.
Other than that:
Reviewed-by: Warner Losh <i...@bsdimp.com>
> /*
> * The BSD ABIs use the same signal numbers across all the CPU
> architectures, so
> * (unlike Linux) these functions are just the identity mapping. This
> might not
> @@ -489,6 +491,12 @@ static void host_signal_handler(int host_sig,
> siginfo_t *info, void *puc)
> uintptr_t pc = 0;
> bool sync_sig = false;
>
> + if (host_sig == host_interrupt_signal) {
> + ts->signal_pending = 1;
> + cpu_exit(thread_cpu);
> + return;
> + }
> +
> /*
> * Non-spoofed SIGSEGV and SIGBUS are synchronous, and need special
> * handling wrt signal blocking and unwinding.
> @@ -852,6 +860,9 @@ void signal_init(void)
>
> for (i = 1; i <= TARGET_NSIG; i++) {
> host_sig = target_to_host_signal(i);
> + if (host_sig == host_interrupt_signal) {
> + continue;
> + }
> sigaction(host_sig, NULL, &oact);
> if (oact.sa_sigaction == (void *)SIG_IGN) {
> sigact_table[i - 1]._sa_handler = TARGET_SIG_IGN;
> @@ -870,6 +881,7 @@ void signal_init(void)
> sigaction(host_sig, &act, NULL);
> }
> }
> + sigaction(host_interrupt_signal, &act, NULL);
> }
>
> static void handle_pending_signal(CPUArchState *env, int sig,
> diff --git a/include/user/signal.h b/include/user/signal.h
> index 19b6b9e5ddc..7fa33b05d91 100644
> --- a/include/user/signal.h
> +++ b/include/user/signal.h
> @@ -20,4 +20,6 @@
> */
> int target_to_host_signal(int sig);
>
> +extern int host_interrupt_signal;
> +
> #endif
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 84bb8a34808..f0bcbd367d5 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -514,6 +514,8 @@ static int core_dump_signal(int sig)
> }
> }
>
> +int host_interrupt_signal;
> +
> static void signal_table_init(void)
> {
> int hsig, tsig, count;
> @@ -540,6 +542,7 @@ static void signal_table_init(void)
> hsig = SIGRTMIN;
> host_to_target_signal_table[SIGABRT] = 0;
> host_to_target_signal_table[hsig++] = TARGET_SIGABRT;
> + host_interrupt_signal = hsig++;
>
> for (tsig = TARGET_SIGRTMIN;
> hsig <= SIGRTMAX && tsig <= TARGET_NSIG;
> @@ -619,6 +622,8 @@ void signal_init(void)
> }
> sigact_table[tsig - 1]._sa_handler = thand;
> }
> +
> + sigaction(host_interrupt_signal, &act, NULL);
> }
>
> /* Force a synchronously taken signal. The kernel force_sig() function
> @@ -966,6 +971,12 @@ static void host_signal_handler(int host_sig,
> siginfo_t *info, void *puc)
> bool sync_sig = false;
> void *sigmask;
>
> + if (host_sig == host_interrupt_signal) {
> + ts->signal_pending = 1;
> + cpu_exit(thread_cpu);
> + return;
> + }
> +
> /*
> * Non-spoofed SIGSEGV and SIGBUS are synchronous, and need special
> * handling wrt signal blocking and unwinding. Non-spoofed SIGILL,
> --
> 2.47.0
>
>
