fix(libc): guard hash_page sigprocmask use behind _NO_SIGPROCMASK#12
Open
esaurez wants to merge 1 commit into
Open
fix(libc): guard hash_page sigprocmask use behind _NO_SIGPROCMASK#12esaurez wants to merge 1 commit into
esaurez wants to merge 1 commit into
Conversation
newlib/libc/search/hash_page.c::open_temp() unconditionally calls sigprocmask() and sigfillset() around a mkstemp + unlink window. Targets whose newlib build does not provide sigprocmask (e.g. nanvix) therefore fail to link when libc.a is pulled in with --whole-archive: undefined references to sigprocmask and sigfillset. Introduce _NO_SIGPROCMASK as a target opt-out, matching the existing _NO_SIGSET / _NO_POPEN / _NO_POSIX_SPAWN family already used by newlib/configure.host. The default code path is unchanged; targets that lack sigprocmask add -D_NO_SIGPROCMASK to newlib_cflags and the masking step is compiled out. This mirrors the in-function HAVE_FCNTL guard around fcntl(F_SETFD, ...) on the next line: an optional libc API the target may or may not provide is gated by a capability macro defined per target in configure.host. Effect of the opt-out: the mkstemp + unlink window is no longer protected by a blocked signal mask. On a target with no asynchronous signal delivery, no handler can run inside that window, so the protection was a no-op in practice. configure.host is updated only for *-nanvix*; other targets see no behavior change. Related, not fixed in this PR: newlib/libc/posix/posix_spawn.c also references sigprocmask unconditionally; same root cause, separate fix. Verification: preprocessor + unifdef gating test under ghcr.io/nanvix/toolchain-gcc:sha-34a3641 confirms that with -D_NO_SIGPROCMASK the sigprocmask/sigfillset calls are elided, and that without the macro they remain present (default behavior preserved for all other targets). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Guard the
sigprocmask/sigfillsetcalls innewlib/libc/search/hash_page.c::open_temp()behind a new_NO_SIGPROCMASKtarget opt-out macro, and enable that opt-out for*-nanvix*innewlib/configure.host.Why
open_temp()brackets amkstemp+unlinkwindow in a signal-maskblock:
The
sigprocmaskandsigfillsetcalls are unconditional. The nanvixnewlib port does not provide a
sigprocmaskimplementation, so linkinglibc.awith--whole-archive(as cpython and several other consumersdo) fails with undefined references to
sigprocmaskandsigfillset.The original
<sys/signal.h>declarations and theSIG_BLOCK/SIG_SETMASKmacros are all gated together by#if __POSIX_VISIBLE,which is the default visibility level — so simply testing the macros
is not a reliable proxy for "the symbol is linkable on this target."
How
Introduce
_NO_SIGPROCMASKas a target opt-out macro, matching theexisting family already established in
newlib/configure.host:_NO_SIGSET— used bynewlib/libc/unix/sigset.c_NO_POPEN— same opt-out style_NO_POSIX_SPAWN— same opt-out style_NO_WORDEXP,_NO_GETPWENT,_NO_GETLOGIN,_NO_GETUT,_NO_GETPASSFor nanvix,
-D_NO_SIGPROCMASKis added tonewlib_cflagsin the*-nanvix*section ofconfigure.host. The default code path isunchanged for every other target.
This mirrors the existing in-function precedent in the same
open_temp()body: the very next line guards an optional libc APIthe target may not provide:
Same idiom — an optional API gated by a target-provided capability
macro.
Re-enabling the protection
A target that wishes to re-enable the original signal-mask protection
needs both:
-D_NO_SIGPROCMASKfrom itsnewlib_cflagsentry innewlib/configure.host.sigprocmask(andsigfillset) implementationfor the target.
For nanvix specifically that means implementing real signal-mask
support in libposix (or the kernel) and then dropping the opt-out from
configure.host.Effect of the opt-out
When
_NO_SIGPROCMASKis defined, themkstemp+unlinkwindow isno longer protected by a blocked signal mask. On a target with no
asynchronous signal delivery, no handler can run inside that window,
so the protection was a no-op in practice. On a target with partial
signal support, a signal interrupting between
mkstempandunlinkcould leave the temp file behind — which is exactly why such targets
opt out explicitly.
Related, out of scope
newlib/libc/posix/posix_spawn.calso referencessigprocmaskunconditionally; same root cause but a different translation unit.
Will be addressed in a follow-up PR.
The same bug is present in upstream sourceware newlib; landing in the
nanvix fork first.
Verification
Validated with a small
Dockerfile.localpatch-hash-pageagainstghcr.io/nanvix/toolchain-gcc:sha-34a3641usingunifdefto simulatethe opt-out and the default builds:
sigprocmask(presentsigfillset(present-D_NO_SIGPROCMASKCo-authored-by: Copilot 223556219+Copilot@users.noreply.github.com