Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DRAFT: Use a noop SIGPIPE handler instead of SIG_IGN #121578

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion library/std/src/sys/pal/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,24 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
UNIX_SIGPIPE_ATTR_SPECIFIED.store(true, crate::sync::atomic::Ordering::Relaxed);
}
if let Some(handler) = handler {
rtassert!(signal(libc::SIGPIPE, handler) != libc::SIG_ERR);
if handler == libc::SIG_IGN {
// Implements https://github.com/rust-lang/rust/issues/62569#issuecomment-1961586025
use crate::mem;
use crate::ptr;
let current = {
let mut current: libc::sigaction = mem::zeroed();
libc::sigaction(libc::SIGPIPE, ptr::null(), &mut current);
current.sa_sigaction
};
if current != libc::SIG_IGN {
let mut new: libc::sigaction = mem::zeroed();
new.sa_sigaction = _rustc_sigaction_noop as libc::sighandler_t;
new.sa_flags = libc::SA_RESTART;
libc::sigaction(libc::SIGPIPE, &new, ptr::null_mut());
}
} else {
rtassert!(signal(libc::SIGPIPE, handler) != libc::SIG_ERR);
}
#[cfg(target_os = "hurd")]
{
rtassert!(signal(libc::SIGLOST, handler) != libc::SIG_ERR);
Expand Down Expand Up @@ -227,6 +244,14 @@ pub(crate) fn unix_sigpipe_attr_specified() -> bool {
UNIX_SIGPIPE_ATTR_SPECIFIED.load(crate::sync::atomic::Ordering::Relaxed)
}

#[cfg(not(any(
target_os = "espidf",
target_os = "emscripten",
target_os = "fuchsia",
target_os = "horizon",
)))]
extern "C" fn _rustc_sigaction_noop(_: libc::c_int) {}
Copy link
Member

@bjorn3 bjorn3 Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe put this next to the sa_sigaction assignment?


// SAFETY: must be called only once during runtime cleanup.
// NOTE: this is not guaranteed to run, for example when the program aborts.
pub unsafe fn cleanup() {
Expand Down
15 changes: 15 additions & 0 deletions tests/run-make/sigpipe-in-child-processes/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# ignore-cross-compile because we run the compiled code
# only-unix

# See main.rs for a description of this test.

include ../tools.mk

all:
$(RUSTC) assert-sigpipe-disposition.rs -o $(TMPDIR)/assert-sigpipe-disposition;
for revision in default sig_dfl sig_ign inherit; do \
echo -n "Testing revision $$revision ... "; \
$(RUSTC) main.rs --cfg $$revision -o $(TMPDIR)/main.$$revision || exit 1; \
$(TMPDIR)/main.$$revision $(TMPDIR)/assert-sigpipe-disposition || exit 1; \
echo "ok"; \
done
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#![feature(start, rustc_private)]

extern crate libc;

// Use #[start] so we don't have a runtime that messes with SIGPIPE.
#[start]
fn start(argc: isize, argv: *const *const u8) -> isize {
assert_eq!(argc, 2, "Must pass SIG_IGN or SIG_DFL as first arg");
let expected =
match unsafe { std::ffi::CStr::from_ptr(*argv.offset(1) as *const i8) }.to_str().unwrap() {
"SIG_IGN" => libc::SIG_IGN,
"SIG_DFL" => libc::SIG_DFL,
arg => panic!("Must pass SIG_IGN or SIG_DFL as first arg. Got: {}", arg),
};

let actual = unsafe {
let mut actual: libc::sigaction = std::mem::zeroed();
libc::sigaction(libc::SIGPIPE, std::ptr::null(), &mut actual);
actual.sa_sigaction
};

assert_eq!(actual, expected, "actual and expected SIGPIPE disposition differs");

return 0;
}
33 changes: 33 additions & 0 deletions tests/run-make/sigpipe-in-child-processes/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Checks the signal disposition of `SIGPIPE` in child processes. Without any
// `unix_sigpipe` attribute, `SIG_IGN` is the default. But there is a
// difference in how `SIGPIPE` is treated in child processes with and without
// the attribute. Search for `unix_sigpipe_attr_specified()` in the code base to
// learn more.

// Note that tests for the attribute that does not involve child processes are
// in tests/ui/attributes/unix_sigpipe.

#![cfg_attr(any(sig_dfl, sig_ign), feature(unix_sigpipe))]

#[cfg_attr(sig_dfl, unix_sigpipe = "sig_dfl")]
#[cfg_attr(sig_ign, unix_sigpipe = "sig_ign")]
fn main() {
// By default, we get SIG_IGN but the child gets SIG_DFL.
#[cfg(default)]
let expected = "SIG_DFL";

// With #[unix_sigpipe = "sig_dfl"] we get SIG_DFL and the child does too.
#[cfg(sig_dfl)]
let expected = "SIG_DFL";

// With #[unix_sigpipe = "sig_ign"] we get SIG_IGN and the child does too.
#[cfg(sig_ign)]
let expected = "SIG_IGN";

// With #[unix_sigpipe = "inherit"] we get SIG_DFL and the child does too.
#[cfg(inherit)]
let expected = "SIG_DFL";

let child_program = std::env::args().nth(1).unwrap();
assert!(std::process::Command::new(child_program).arg(expected).status().unwrap().success());
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub fn assert_sigpipe_handler(expected_handler: SignalHandler) {
SignalHandler::Ignore => libc::SIG_IGN,
SignalHandler::Default => libc::SIG_DFL,
};
assert_eq!(prev, expected, "expected sigpipe value matches actual value");
assert_eq!(prev, expected, "FIXME: How do we know if SIGPIPE is ignored here?");

// Unlikely to matter, but restore the old value anyway
unsafe {
Expand Down
Loading