On 6/6/24 2:50 PM, Warner Losh wrote:
The branch main has been updated by imp:
URL:
https://cgit.FreeBSD.org/src/commit/?id=7cae020b9c977c11881363d726b13d1cd2feec5e
commit 7cae020b9c977c11881363d726b13d1cd2feec5e
Author: Dapeng Gao <dg...@cam.ac.uk>
AuthorDate: 2024-06-03 18:18:13 +0000
Commit: Warner Losh <i...@freebsd.org>
CommitDate: 2024-06-06 21:48:39 +0000
Simplify signal handling code in libthr by removing use of SYS_sigreturn
The use of SYS_sigreturn is unnecessary here.
If handle_signal is called when a signal is delivered, it can just
return normally back to sigcode which will call sigreturn anyway.
In case handle_signal is called by check_deferred_signal, using
setcontext is better than SYS_sigreturn because that is the correct
system call to pair with the earlier getcontext.
Reviewed by: imp, kib
Differential Revision: https://reviews.freebsd.org/D44893
---
lib/libthr/thread/thr_sig.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/lib/libthr/thread/thr_sig.c b/lib/libthr/thread/thr_sig.c
index ad291d106001..b953c430158c 100644
--- a/lib/libthr/thread/thr_sig.c
+++ b/lib/libthr/thread/thr_sig.c
@@ -247,7 +247,6 @@ static void
handle_signal(struct sigaction *actp, int sig, siginfo_t *info, ucontext_t
*ucp)
{
struct pthread *curthread = _get_curthread();
- ucontext_t uc2;
__siginfohandler_t *sigfunc;
int cancel_point;
int cancel_async;
@@ -307,13 +306,11 @@ handle_signal(struct sigaction *actp, int sig, siginfo_t
*info, ucontext_t *ucp)
curthread->cancel_point = cancel_point;
curthread->cancel_enable = cancel_enable;
- memcpy(&uc2, ucp, sizeof(uc2));
- SIGDELSET(uc2.uc_sigmask, SIGCANCEL);
+ SIGDELSET(ucp->uc_sigmask, SIGCANCEL);
/* reschedule cancellation */
- check_cancel(curthread, &uc2);
+ check_cancel(curthread, ucp);
errno = err;
- syscall(SYS_sigreturn, &uc2);
}
void
@@ -400,6 +397,7 @@ check_deferred_signal(struct pthread *curthread)
/* remove signal */
curthread->deferred_siginfo.si_signo = 0;
handle_signal(&act, info.si_signo, &info, uc);
+ syscall(SYS_sigreturn, uc);
The commit log implies this should be calling setcontext() instead of
syscall()? Was that a stale part of the commit log? I thought I
remember discussing this at one point. Maybe the issue was that
you couldn't pre-resolve the PLT for setcontext()?
--
John Baldwin
