Date: Mon, 18 Oct 2021 14:08:14 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 81f2e9063d64 - main - signal: Add SIG_FOREACH and refactor issignal() Message-ID: <202110181408.19IE8EQq066477@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=81f2e9063d64cc976b47e7ee1e9c35692cda7cb4 commit 81f2e9063d64cc976b47e7ee1e9c35692cda7cb4 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2021-10-16 13:44:40 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2021-10-18 13:56:58 +0000 signal: Add SIG_FOREACH and refactor issignal() Add a SIG_FOREACH macro that can be used to iterate over a signal set. This is a bit cleaner and more efficient than calling sig_ffs() in a loop. The implementation is based on BIT_FOREACH_ISSET(), except that the bitset limbs are always 32 bits wide, and signal sets are 1-indexed rather than 0-indexed like bitset(9) sets. issignal() cannot really be modified to use SIG_FOREACH() directly. Take this opportunity to split the function into two explicit loops. I've always found this function hard to read and think that this change is an improvement. Remove sig_ffs(), nothing uses it now. Reviewed by: kib MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D32473 --- sys/kern/kern_sig.c | 386 +++++++++++++++++++++++++++++----------------------- sys/sys/signalvar.h | 1 - 2 files changed, 217 insertions(+), 170 deletions(-) diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index b7155074aa5a..d6826e8dc507 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -249,6 +249,29 @@ static int sigproptbl[NSIG] = { [SIGUSR2] = SIGPROP_KILL, }; +#define _SIG_FOREACH_ADVANCE(i, set) ({ \ + int __found; \ + for (;;) { \ + if (__bits != 0) { \ + int __sig = ffs(__bits); \ + __bits &= ~(1u << (__sig - 1)); \ + sig = __i * sizeof((set)->__bits[0]) * NBBY + __sig; \ + __found = 1; \ + break; \ + } \ + if (++__i == _SIG_WORDS) { \ + __found = 0; \ + break; \ + } \ + __bits = (set)->__bits[__i]; \ + } \ + __found != 0; \ +}) + +#define SIG_FOREACH(i, set) \ + for (int32_t __i = -1, __bits = 0; \ + _SIG_FOREACH_ADVANCE(i, set); ) \ + sigset_t fastblock_mask; static void @@ -660,17 +683,6 @@ sigprop(int sig) return (0); } -int -sig_ffs(sigset_t *set) -{ - int i; - - for (i = 0; i < _SIG_WORDS; i++) - if (set->__bits[i]) - return (ffs(set->__bits[i]) + (i * 32)); - return (0); -} - static bool sigact_flag_test(const struct sigaction *act, int flag) { @@ -2761,8 +2773,7 @@ reschedule_signals(struct proc *p, sigset_t block, int flags) return; SIGSETAND(block, p->p_siglist); fastblk = (flags & SIGPROCMASK_FASTBLK) != 0; - while ((sig = sig_ffs(&block)) != 0) { - SIGDELSET(block, sig); + SIG_FOREACH(sig, &block) { td = sigtd(p, sig, fastblk); /* @@ -2894,13 +2905,188 @@ sigallowstop_impl(int prev) } } +enum sigstatus { + SIGSTATUS_HANDLE, + SIGSTATUS_HANDLED, + SIGSTATUS_IGNORE, + SIGSTATUS_SBDRY_STOP, +}; + +/* + * The thread has signal "sig" pending. Figure out what to do with it: + * + * _HANDLE -> the caller should handle the signal + * _HANDLED -> handled internally, reload pending signal set + * _IGNORE -> ignored, remove from the set of pending signals and try the + * next pending signal + * _SBDRY_STOP -> the signal should stop the thread but this is not + * permitted in the current context + */ +static enum sigstatus +sigprocess(struct thread *td, int sig) +{ + struct proc *p; + struct sigacts *ps; + struct sigqueue *queue; + ksiginfo_t ksi; + int prop; + + KASSERT(_SIG_VALID(sig), ("%s: invalid signal %d", __func__, sig)); + + p = td->td_proc; + ps = p->p_sigacts; + mtx_assert(&ps->ps_mtx, MA_OWNED); + PROC_LOCK_ASSERT(p, MA_OWNED); + + /* + * We should allow pending but ignored signals below + * only if there is sigwait() active, or P_TRACED was + * on when they were posted. + */ + if (SIGISMEMBER(ps->ps_sigignore, sig) && + (p->p_flag & P_TRACED) == 0 && + (td->td_flags & TDF_SIGWAIT) == 0) { + return (SIGSTATUS_IGNORE); + } + + if ((p->p_flag & (P_TRACED | P_PPTRACE)) == P_TRACED) { + /* + * If traced, always stop. + * Remove old signal from queue before the stop. + * XXX shrug off debugger, it causes siginfo to + * be thrown away. + */ + queue = &td->td_sigqueue; + ksiginfo_init(&ksi); + if (sigqueue_get(queue, sig, &ksi) == 0) { + queue = &p->p_sigqueue; + sigqueue_get(queue, sig, &ksi); + } + td->td_si = ksi.ksi_info; + + mtx_unlock(&ps->ps_mtx); + sig = ptracestop(td, sig, &ksi); + mtx_lock(&ps->ps_mtx); + + td->td_si.si_signo = 0; + + /* + * Keep looking if the debugger discarded or + * replaced the signal. + */ + if (sig == 0) + return (SIGSTATUS_HANDLED); + + /* + * If the signal became masked, re-queue it. + */ + if (SIGISMEMBER(td->td_sigmask, sig)) { + ksi.ksi_flags |= KSI_HEAD; + sigqueue_add(&p->p_sigqueue, sig, &ksi); + return (SIGSTATUS_HANDLED); + } + + /* + * If the traced bit got turned off, requeue the signal and + * reload the set of pending signals. This ensures that p_sig* + * and p_sigact are consistent. + */ + if ((p->p_flag & P_TRACED) == 0) { + ksi.ksi_flags |= KSI_HEAD; + sigqueue_add(queue, sig, &ksi); + return (SIGSTATUS_HANDLED); + } + } + + /* + * Decide whether the signal should be returned. + * Return the signal's number, or fall through + * to clear it from the pending mask. + */ + switch ((intptr_t)p->p_sigacts->ps_sigact[_SIG_IDX(sig)]) { + case (intptr_t)SIG_DFL: + /* + * Don't take default actions on system processes. + */ + if (p->p_pid <= 1) { +#ifdef DIAGNOSTIC + /* + * Are you sure you want to ignore SIGSEGV + * in init? XXX + */ + printf("Process (pid %lu) got signal %d\n", + (u_long)p->p_pid, sig); +#endif + return (SIGSTATUS_IGNORE); + } + + /* + * If there is a pending stop signal to process with + * default action, stop here, then clear the signal. + * Traced or exiting processes should ignore stops. + * Additionally, a member of an orphaned process group + * should ignore tty stops. + */ + prop = sigprop(sig); + if (prop & SIGPROP_STOP) { + mtx_unlock(&ps->ps_mtx); + if ((p->p_flag & (P_TRACED | P_WEXIT | + P_SINGLE_EXIT)) != 0 || ((p->p_pgrp-> + pg_flags & PGRP_ORPHANED) != 0 && + (prop & SIGPROP_TTYSTOP) != 0)) { + mtx_lock(&ps->ps_mtx); + return (SIGSTATUS_IGNORE); + } + if (TD_SBDRY_INTR(td)) { + KASSERT((td->td_flags & TDF_SBDRY) != 0, + ("lost TDF_SBDRY")); + mtx_lock(&ps->ps_mtx); + return (SIGSTATUS_SBDRY_STOP); + } + WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, + &p->p_mtx.lock_object, "Catching SIGSTOP"); + sigqueue_delete(&td->td_sigqueue, sig); + sigqueue_delete(&p->p_sigqueue, sig); + p->p_flag |= P_STOPPED_SIG; + p->p_xsig = sig; + PROC_SLOCK(p); + sig_suspend_threads(td, p, 0); + thread_suspend_switch(td, p); + PROC_SUNLOCK(p); + mtx_lock(&ps->ps_mtx); + return (SIGSTATUS_HANDLED); + } else if ((prop & SIGPROP_IGNORE) != 0 && + (td->td_flags & TDF_SIGWAIT) == 0) { + /* + * Default action is to ignore; drop it if + * not in kern_sigtimedwait(). + */ + return (SIGSTATUS_IGNORE); + } else { + return (SIGSTATUS_HANDLE); + } + + case (intptr_t)SIG_IGN: + if ((td->td_flags & TDF_SIGWAIT) == 0) + return (SIGSTATUS_IGNORE); + else + return (SIGSTATUS_HANDLE); + + default: + /* + * This signal has an action, let postsig() process it. + */ + return (SIGSTATUS_HANDLE); + } +} + /* * If the current process has received a signal (should be caught or cause * termination, should interrupt current syscall), return the signal number. * Stop signals with default action are processed immediately, then cleared; * they aren't returned. This is checked after each entry to the system for - * a syscall or trap (though this can usually be done without calling issignal - * by checking the pending signal masks in cursig.) The normal call + * a syscall or trap (though this can usually be done without calling + * issignal by checking the pending signal masks in cursig.) The normal call * sequence is * * while (sig = cursig(curthread)) @@ -2910,16 +3096,12 @@ static int issignal(struct thread *td) { struct proc *p; - struct sigacts *ps; - struct sigqueue *queue; sigset_t sigpending; - ksiginfo_t ksi; - int prop, sig; + int sig; p = td->td_proc; - ps = p->p_sigacts; - mtx_assert(&ps->ps_mtx, MA_OWNED); PROC_LOCK_ASSERT(p, MA_OWNED); + for (;;) { sigpending = td->td_sigqueue.sq_signals; SIGSETOR(sigpending, p->p_sigqueue.sq_signals); @@ -2957,160 +3139,27 @@ issignal(struct thread *td) * execute the debugger attach ritual in * order. */ - sig = SIGSTOP; td->td_dbgflags |= TDB_FSTP; - } else { - sig = sig_ffs(&sigpending); + SIGEMPTYSET(sigpending); + SIGADDSET(sigpending, SIGSTOP); } - /* - * We should allow pending but ignored signals below - * only if there is sigwait() active, or P_TRACED was - * on when they were posted. - */ - if (SIGISMEMBER(ps->ps_sigignore, sig) && - (p->p_flag & P_TRACED) == 0 && - (td->td_flags & TDF_SIGWAIT) == 0) { - sigqueue_delete(&td->td_sigqueue, sig); - sigqueue_delete(&p->p_sigqueue, sig); - continue; - } - if ((p->p_flag & (P_TRACED | P_PPTRACE)) == P_TRACED) { - /* - * If traced, always stop. - * Remove old signal from queue before the stop. - * XXX shrug off debugger, it causes siginfo to - * be thrown away. - */ - queue = &td->td_sigqueue; - ksiginfo_init(&ksi); - if (sigqueue_get(queue, sig, &ksi) == 0) { - queue = &p->p_sigqueue; - sigqueue_get(queue, sig, &ksi); - } - td->td_si = ksi.ksi_info; - - mtx_unlock(&ps->ps_mtx); - sig = ptracestop(td, sig, &ksi); - mtx_lock(&ps->ps_mtx); - - td->td_si.si_signo = 0; - - /* - * Keep looking if the debugger discarded or - * replaced the signal. - */ - if (sig == 0) - continue; - - /* - * If the signal became masked, re-queue it. - */ - if (SIGISMEMBER(td->td_sigmask, sig)) { - ksi.ksi_flags |= KSI_HEAD; - sigqueue_add(&p->p_sigqueue, sig, &ksi); - continue; - } - - /* - * If the traced bit got turned off, requeue - * the signal and go back up to the top to - * rescan signals. This ensures that p_sig* - * and p_sigact are consistent. - */ - if ((p->p_flag & P_TRACED) == 0) { - ksi.ksi_flags |= KSI_HEAD; - sigqueue_add(queue, sig, &ksi); - continue; - } - } - - prop = sigprop(sig); - - /* - * Decide whether the signal should be returned. - * Return the signal's number, or fall through - * to clear it from the pending mask. - */ - switch ((intptr_t)p->p_sigacts->ps_sigact[_SIG_IDX(sig)]) { - case (intptr_t)SIG_DFL: - /* - * Don't take default actions on system processes. - */ - if (p->p_pid <= 1) { -#ifdef DIAGNOSTIC - /* - * Are you sure you want to ignore SIGSEGV - * in init? XXX - */ - printf("Process (pid %lu) got signal %d\n", - (u_long)p->p_pid, sig); -#endif - break; /* == ignore */ - } - /* - * If there is a pending stop signal to process with - * default action, stop here, then clear the signal. - * Traced or exiting processes should ignore stops. - * Additionally, a member of an orphaned process group - * should ignore tty stops. - */ - if (prop & SIGPROP_STOP) { - mtx_unlock(&ps->ps_mtx); - if ((p->p_flag & (P_TRACED | P_WEXIT | - P_SINGLE_EXIT)) != 0 || ((p->p_pgrp-> - pg_flags & PGRP_ORPHANED) != 0 && - (prop & SIGPROP_TTYSTOP) != 0)) { - mtx_lock(&ps->ps_mtx); - break; /* == ignore */ - } - if (TD_SBDRY_INTR(td)) { - KASSERT((td->td_flags & TDF_SBDRY) != 0, - ("lost TDF_SBDRY")); - mtx_lock(&ps->ps_mtx); - return (-1); - } - WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, - &p->p_mtx.lock_object, "Catching SIGSTOP"); + SIG_FOREACH(sig, &sigpending) { + switch (sigprocess(td, sig)) { + case SIGSTATUS_HANDLE: + return (sig); + case SIGSTATUS_HANDLED: + goto next; + case SIGSTATUS_IGNORE: sigqueue_delete(&td->td_sigqueue, sig); sigqueue_delete(&p->p_sigqueue, sig); - p->p_flag |= P_STOPPED_SIG; - p->p_xsig = sig; - PROC_SLOCK(p); - sig_suspend_threads(td, p, 0); - thread_suspend_switch(td, p); - PROC_SUNLOCK(p); - mtx_lock(&ps->ps_mtx); - goto next; - } else if ((prop & SIGPROP_IGNORE) != 0 && - (td->td_flags & TDF_SIGWAIT) == 0) { - /* - * Default action is to ignore; drop it if - * not in kern_sigtimedwait(). - */ - break; /* == ignore */ - } else - return (sig); - /*NOTREACHED*/ - - case (intptr_t)SIG_IGN: - if ((td->td_flags & TDF_SIGWAIT) == 0) - break; /* == ignore */ - else - return (sig); - - default: - /* - * This signal has an action, let - * postsig() process it. - */ - return (sig); + break; + case SIGSTATUS_SBDRY_STOP: + return (-1); + } } - sigqueue_delete(&td->td_sigqueue, sig); /* take the signal! */ - sigqueue_delete(&p->p_sigqueue, sig); next:; } - /* NOTREACHED */ } void @@ -4119,8 +4168,7 @@ sig_drop_caught(struct proc *p) ps = p->p_sigacts; PROC_LOCK_ASSERT(p, MA_OWNED); mtx_assert(&ps->ps_mtx, MA_OWNED); - while (SIGNOTEMPTY(ps->ps_sigcatch)) { - sig = sig_ffs(&ps->ps_sigcatch); + SIG_FOREACH(sig, &ps->ps_sigcatch) { sigdflt(ps, sig); if ((sigprop(sig) & SIGPROP_IGNORE) != 0) sigqueue_delete_proc(p, sig); diff --git a/sys/sys/signalvar.h b/sys/sys/signalvar.h index a7fe174b40bf..4e86f54856f6 100644 --- a/sys/sys/signalvar.h +++ b/sys/sys/signalvar.h @@ -404,7 +404,6 @@ int sig_ast_needsigchk(struct thread *td); void sig_drop_caught(struct proc *p); void sigexit(struct thread *td, int sig) __dead2; int sigev_findtd(struct proc *p, struct sigevent *sigev, struct thread **); -int sig_ffs(sigset_t *set); void sigfastblock_clear(struct thread *td); void sigfastblock_fetch(struct thread *td); void sigfastblock_setpend(struct thread *td, bool resched);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202110181408.19IE8EQq066477>