diff options
| author | bors <bors@rust-lang.org> | 2015-06-22 16:11:38 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2015-06-22 16:11:38 +0000 |
| commit | 4e2a898afcd890d44a45bd229dee8b9db8d330cc (patch) | |
| tree | 8ac7c508782bf1b880c6e7a624025f52e5413e60 /src/libstd/sys/unix/stack_overflow.rs | |
| parent | 2287b4b628cb6f4d66578e1298cd9f34e9ef77db (diff) | |
| parent | a8dbb92b471cae1d3f8225857f5553311dd8aeb3 (diff) | |
| download | rust-4e2a898afcd890d44a45bd229dee8b9db8d330cc.tar.gz rust-4e2a898afcd890d44a45bd229dee8b9db8d330cc.zip | |
Auto merge of #25784 - geofft:subprocess-signal-masks, r=alexcrichton
UNIX specifies that signal dispositions and masks get inherited to child processes, but in general, programs are not very robust to being started with non-default signal dispositions or to signals being blocked. For example, libstd sets `SIGPIPE` to be ignored, on the grounds that Rust code using libstd will get the `EPIPE` errno and handle it correctly. But shell pipelines are built around the assumption that `SIGPIPE` will have its default behavior of killing the process, so that things like `head` work:
```
geofft@titan:/tmp$ for i in `seq 1 20`; do echo "$i"; done | head -1
1
geofft@titan:/tmp$ cat bash.rs
fn main() {
std::process::Command::new("bash").status();
}
geofft@titan:/tmp$ ./bash
geofft@titan:/tmp$ for i in `seq 1 20`; do echo "$i"; done | head -1
1
bash: echo: write error: Broken pipe
bash: echo: write error: Broken pipe
bash: echo: write error: Broken pipe
bash: echo: write error: Broken pipe
bash: echo: write error: Broken pipe
[...]
```
Here, `head` is supposed to terminate the input process quietly, but the bash subshell has inherited the ignored disposition of `SIGPIPE` from its Rust grandparent process. So it gets a bunch of `EPIPE`s that it doesn't know what to do with, and treats it as a generic, transient error. You can see similar behavior with `find / | head`, `yes | head`, etc.
This PR resets Rust's `SIGPIPE` handler, as well as any signal mask that may have been set, before spawning a child. Setting a signal mask, and then using a dedicated thread or something like `signalfd` to dequeue signals, is one of two reasonable ways for a library to process signals. See carllerche/mio#16 for more discussion about this approach to signal handling and why it needs a change to `std::process`. The other approach is for the library to set a signal-handling function (`signal()` / `sigaction()`): in that case, dispositions are reset to the default behavior on exec (since the function pointer isn't valid across exec), so we don't have to care about that here.
As part of this PR, I noticed that we had two somewhat-overlapping sets of bindings to signal functionality in `libstd`. One dated to old-IO and probably the old runtime, and was mostly unused. The other is currently used by `stack_overflow.rs`. I consolidated the two bindings into one set, and double-checked them by hand against all supported platforms' headers. This probably means it's safe to enable `stack_overflow.rs` on more targets, but I'm not including such a change in this PR.
r? @alexcrichton
cc @Zoxc for changes to `stack_overflow.rs`
Diffstat (limited to 'src/libstd/sys/unix/stack_overflow.rs')
| -rw-r--r-- | src/libstd/sys/unix/stack_overflow.rs | 141 |
1 files changed, 5 insertions, 136 deletions
diff --git a/src/libstd/sys/unix/stack_overflow.rs b/src/libstd/sys/unix/stack_overflow.rs index 2bc280d1274..52494a17b9d 100644 --- a/src/libstd/sys/unix/stack_overflow.rs +++ b/src/libstd/sys/unix/stack_overflow.rs @@ -44,11 +44,12 @@ mod imp { use mem; use ptr; use intrinsics; - use self::signal::{siginfo, sigaction, SIGBUS, SIG_DFL, - SA_SIGINFO, SA_ONSTACK, sigaltstack, - SIGSTKSZ}; + use sys::c::{siginfo, sigaction, SIGBUS, SIG_DFL, + SA_SIGINFO, SA_ONSTACK, sigaltstack, + SIGSTKSZ, sighandler_t, raise}; use libc; use libc::funcs::posix88::mman::{mmap, munmap}; + use libc::funcs::posix01::signal::signal; use libc::consts::os::posix88::{SIGSEGV, PROT_READ, PROT_WRITE, @@ -120,7 +121,7 @@ mod imp { pub unsafe fn make_handler() -> Handler { let alt_stack = mmap(ptr::null_mut(), - signal::SIGSTKSZ, + SIGSTKSZ, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, @@ -143,138 +144,6 @@ mod imp { pub unsafe fn drop_handler(handler: &mut Handler) { munmap(handler._data, SIGSTKSZ); } - - pub type sighandler_t = *mut libc::c_void; - - #[cfg(any(all(target_os = "linux", target_arch = "x86"), // may not match - all(target_os = "linux", target_arch = "x86_64"), - all(target_os = "linux", target_arch = "arm"), // may not match - all(target_os = "linux", target_arch = "aarch64"), - all(target_os = "linux", target_arch = "mips"), // may not match - all(target_os = "linux", target_arch = "mipsel"), // may not match - all(target_os = "linux", target_arch = "powerpc"), // may not match - target_os = "android"))] // may not match - mod signal { - use libc; - pub use super::sighandler_t; - - pub static SA_ONSTACK: libc::c_int = 0x08000000; - pub static SA_SIGINFO: libc::c_int = 0x00000004; - pub static SIGBUS: libc::c_int = 7; - - pub static SIGSTKSZ: libc::size_t = 8192; - - pub const SIG_DFL: sighandler_t = 0 as sighandler_t; - - // This definition is not as accurate as it could be, {si_addr} is - // actually a giant union. Currently we're only interested in that field, - // however. - #[repr(C)] - pub struct siginfo { - si_signo: libc::c_int, - si_errno: libc::c_int, - si_code: libc::c_int, - pub si_addr: *mut libc::c_void - } - - #[repr(C)] - pub struct sigaction { - pub sa_sigaction: sighandler_t, - pub sa_mask: sigset_t, - pub sa_flags: libc::c_int, - sa_restorer: *mut libc::c_void, - } - - #[cfg(target_pointer_width = "32")] - #[repr(C)] - pub struct sigset_t { - __val: [libc::c_ulong; 32], - } - #[cfg(target_pointer_width = "64")] - #[repr(C)] - pub struct sigset_t { - __val: [libc::c_ulong; 16], - } - - #[repr(C)] - pub struct sigaltstack { - pub ss_sp: *mut libc::c_void, - pub ss_flags: libc::c_int, - pub ss_size: libc::size_t - } - - } - - #[cfg(any(target_os = "macos", - target_os = "bitrig", - target_os = "openbsd"))] - mod signal { - use libc; - pub use super::sighandler_t; - - pub const SA_ONSTACK: libc::c_int = 0x0001; - pub const SA_SIGINFO: libc::c_int = 0x0040; - pub const SIGBUS: libc::c_int = 10; - - #[cfg(target_os = "macos")] - pub const SIGSTKSZ: libc::size_t = 131072; - #[cfg(any(target_os = "bitrig", target_os = "openbsd"))] - pub const SIGSTKSZ: libc::size_t = 40960; - - pub const SIG_DFL: sighandler_t = 0 as sighandler_t; - - pub type sigset_t = u32; - - // This structure has more fields, but we're not all that interested in - // them. - #[cfg(target_os = "macos")] - #[repr(C)] - pub struct siginfo { - pub si_signo: libc::c_int, - pub si_errno: libc::c_int, - pub si_code: libc::c_int, - pub pid: libc::pid_t, - pub uid: libc::uid_t, - pub status: libc::c_int, - pub si_addr: *mut libc::c_void - } - - #[cfg(any(target_os = "bitrig", target_os = "openbsd"))] - #[repr(C)] - pub struct siginfo { - pub si_signo: libc::c_int, - pub si_code: libc::c_int, - pub si_errno: libc::c_int, - //union - pub si_addr: *mut libc::c_void - } - - #[repr(C)] - pub struct sigaltstack { - pub ss_sp: *mut libc::c_void, - pub ss_size: libc::size_t, - pub ss_flags: libc::c_int - } - - #[repr(C)] - pub struct sigaction { - pub sa_sigaction: sighandler_t, - pub sa_mask: sigset_t, - pub sa_flags: libc::c_int, - } - } - - extern { - pub fn signal(signum: libc::c_int, handler: sighandler_t) -> sighandler_t; - pub fn raise(signum: libc::c_int) -> libc::c_int; - - pub fn sigaction(signum: libc::c_int, - act: *const sigaction, - oldact: *mut sigaction) -> libc::c_int; - - pub fn sigaltstack(ss: *const sigaltstack, - oss: *mut sigaltstack) -> libc::c_int; - } } #[cfg(not(any(target_os = "linux", |
