| Message ID | 20241024200031.80327-5-iii@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | gdbstub: Allow late attachment | expand |
On 10/24/24 20:59, Ilya Leoshkevich 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<iii@linux.ibm.com> > --- > bsd-user/signal.c | 12 ++++++++++++ > include/user/signal.h | 2 ++ > linux-user/signal.c | 11 +++++++++++ > 3 files changed, 25 insertions(+) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On Thu, Oct 24, 2024 at 2:00 PM Ilya Leoshkevich <iii@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 <iii@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 <imp@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 > >
On Tue, 2024-11-05 at 08:39 -0700, Warner Losh wrote: > On Thu, Oct 24, 2024 at 2:00 PM Ilya Leoshkevich <iii@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 <iii@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 <imp@bsdimp.com> Thanks for the suggestion, I'll use SIGRTMAX + 1 in v2.
On 11/5/24 15:50, Ilya Leoshkevich wrote: > On Tue, 2024-11-05 at 08:39 -0700, Warner Losh wrote: >> On Thu, Oct 24, 2024 at 2:00 PM Ilya Leoshkevich <iii@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 <iii@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 <imp@bsdimp.com> > > Thanks for the suggestion, I'll use SIGRTMAX + 1 in v2. That can't be right -- SIGRTMAX+1 is not a valid signal. r~
On Tue, 2024-11-05 at 22:30 +0000, Richard Henderson wrote: > On 11/5/24 15:50, Ilya Leoshkevich wrote: > > On Tue, 2024-11-05 at 08:39 -0700, Warner Losh wrote: > > > On Thu, Oct 24, 2024 at 2:00 PM Ilya Leoshkevich > > > <iii@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 <iii@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 <imp@bsdimp.com> > > > > Thanks for the suggestion, I'll use SIGRTMAX + 1 in v2. > > > That can't be right -- SIGRTMAX+1 is not a valid signal. > > > r~ I have to admit I didn't look into this too deeply, but I ran the following experiment on a FreeBSD 14.1 box: /usr/include $ grep -R SIGRTMAX . ./sys/signal.h:#define SIGRTMAX 126 $ sleep 100 & $ kill -126 %1 [1] Unknown signal: 126 sleep 100 $ sleep 100 & $ kill -127 %1 [1] + Unknown signal: 0 sleep 100 Clearly, something is wrong - at least with the shell - but at the same time the signal delivery seems to have occurred. Warner, does the above look normal to you?
On Tue, Nov 5, 2024 at 3:49 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > On Tue, 2024-11-05 at 22:30 +0000, Richard Henderson wrote: > > On 11/5/24 15:50, Ilya Leoshkevich wrote: > > > On Tue, 2024-11-05 at 08:39 -0700, Warner Losh wrote: > > > > On Thu, Oct 24, 2024 at 2:00 PM Ilya Leoshkevich > > > > <iii@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 <iii@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 <imp@bsdimp.com> > > > > > > Thanks for the suggestion, I'll use SIGRTMAX + 1 in v2. > > > > > > That can't be right -- SIGRTMAX+1 is not a valid signal. > > > > > > r~ > > I have to admit I didn't look into this too deeply, but I ran the > following experiment on a FreeBSD 14.1 box: > > /usr/include $ grep -R SIGRTMAX . > ./sys/signal.h:#define SIGRTMAX 126 > > $ sleep 100 & > $ kill -126 %1 > [1] Unknown signal: 126 sleep 100 > > $ sleep 100 & > $ kill -127 %1 > [1] + Unknown signal: 0 sleep 100 > > Clearly, something is wrong - at least with the shell - but at the > same time the signal delivery seems to have occurred. > > Warner, does the above look normal to you? > Oh! I understand.... I thought there was a gap above SIGRTMAX. It sure looks like there is. However, 0177 (127) is used to signal that the process is STOPPED, so can't be used. This is why SIGRTMAX is 126 and not 127. There's room in sigset_t, but that's not sufficient. And it has to be an actual signal we send, not just a flag. So forget I said anything. This was a silly idea. If we find any real thing that's using SIGRTMAX, we'll cope. Warner
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; + /* * 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,
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 <iii@linux.ibm.com> --- bsd-user/signal.c | 12 ++++++++++++ include/user/signal.h | 2 ++ linux-user/signal.c | 11 +++++++++++ 3 files changed, 25 insertions(+)