Re: git: 7cae020b9c97 - main - Simplify signal handling code in libthr by removing use of SYS_sigreturn

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 <dg612@cam.ac.uk>
> AuthorDate: 2024-06-03 18:18:13 +0000
> Commit:     Warner Losh <imp@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