lib/59401: libc: thr_sigsetmask definition is incoherent

[campbell+netbsd%mumble.net@localhost]

>Number:         59401
>Category:       lib
>Synopsis:       libc: thr_sigsetmask definition is incoherent
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue May 06 22:45:00 +0000 2025
>Originator:     Taylor R Campbell
>Release:        
>Organization:
The NetBSD _Reentrancy
>Environment:
>Description:
If _REENTRANT is defined (default), thr_sigsetmask gets you either:

- a wrapper for sigprocmask(2), in single-threaded applications; or
- a per-thread version of sigprocmask(2), in multi-threaded applications linked against libpthread.

But if _REENTRANT is _not_ defined, thr_sigsetmask is a macro that expands to nothing.

I recently wrote some code (for softfloat SIGFPE delivery, https://mail-index.netbsd.org/source-changes/2025/04/27/msg156650.html) assuming that it would block signals for the current thread -- not realizing it does nothing in non-_REENTRANT builds.

This thr_sigsetmask definition appears to date back to the import of Sun TI-RPC code back in 2000:

https://mail-index.netbsd.org/source-changes/2000/06/02/msg077344.html

(At the time, _REENTRANT was called __REENT; it was renamed with the nathanw_sa merge.)

And, the TI-RPC code seems to _rely_ on thr_sigsetmask being a noop when !_REENTRANT:

    105 #ifdef _REENTRANT
    106 #define __rpc_lock_value __isthreaded;
    107 static cond_t	*dg_cv;
    108 #define	release_fd_lock(fd, mask) {		\
    109 	mutex_lock(&clnt_fd_lock);	\
    110 	dg_fd_locks[fd] = 0;		\
    111 	mutex_unlock(&clnt_fd_lock);	\
    112 	thr_sigsetmask(SIG_SETMASK, &(mask), NULL);	\
    113 	cond_signal(&dg_cv[fd]);	\
    114 }
    115 #else
    116 #define release_fd_lock(fd,mask)
    117 #define __rpc_lock_value 0
    118 #endif

https://nxr.netbsd.org/xref/src/lib/libc/rpc/clnt_dg.c?r=1.33#105

    328 #ifdef _REENTRANT
    329 	sigset_t mask, *maskp = &mask;
    330 #else
    331 	sigset_t *maskp = NULL;
    332 #endif
...
    342 	__clnt_sigfillset(&newmask);
    343 	thr_sigsetmask(SIG_SETMASK, &newmask, &mask);
...
    412 		n = pollts(&cu->cu_pfdp, 1, &ts, maskp);
...
    501 out:
    502 	release_fd_lock(cu->cu_fd, mask);

https://nxr.netbsd.org/xref/src/lib/libc/rpc/clnt_dg.c?r=1.33#328

This strikes me as likely to be incoherent: I bet this code really does expect to have signals masked, but it got lost in translation somewhere along the decades and I can't find the original TI-RPC source that fvdl@ derived the lib/libc/rpc/ code from.  However, we also don't have automatic tests for any signal business in lib/libc/rpc/ code, so I'm reluctant to make changes to it.
>How-To-Repeat:
1. code inspection
2. build libc without _REENTRANT
>Fix:
Yes, please!

Either:

(a) sprinkle more _REENTRANT conditionals (ugh) to use thr_sigsetmask if _REENTRANT and sigprocmask if not (ugh); or
(b) find some way to test changes to signal masking in lib/libc/rpc/, change reentrant.h to do #define thr_sigsetmask(f, n, o) (sigprocmask(f, n, o) ? errno : 0), and change lib/libc/rpc/ to do #define release_fd_lock(fd, lock) thr_sigsetmask(SIG_SETMASK, &(mask), NULL)

(Also: the name `reentrant' is wrong, but it's baked into some historic feature test macros like for lgamma_r(3), so, bleh.)