NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
lib/59401: libc: thr_sigsetmask definition is incoherent
>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.)
Home |
Main Index |
Thread Index |
Old Index