stack_overflow: Enable re-raising SIGSEGV / SIGBUS in their own handlers

By default, UNIX signals are blocked during their own handlers' execution, so `raise(signum)` from inside a handler was not actually getting delivered. Therefore the following instruction, `intrinsics::abort()`, was getting executed. This led to segfaulting Rust programs incorrectly having a termination status indicating SIGILL from the `abort()`, instead of SIGSEGV (or SIGBUS). We can use the `SA_NODEFER` flag to prevent this behavior. We can also use `SA_RESETHAND` to have the system reset the default signal handler for us upon entry, instead of doing that ourselves.
geofft · Jun 20, 2015 · 5b8b6a5 · 5b8b6a5
1 parent 2a93dca
commit 5b8b6a5
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 18 deletions.
diff --git a/src/libstd/sys/unix/c.rs b/src/libstd/sys/unix/c.rs
@@ -25,7 +25,8 @@
 #![allow(non_camel_case_types)]
 
 pub use self::signal_os::{sigaction, siginfo, sigset_t, sigaltstack};
-pub use self::signal_os::{SA_ONSTACK, SA_SIGINFO, SIGBUS, SIGSTKSZ, SIG_SETMASK};
+pub use self::signal_os::{SA_ONSTACK, SA_SIGINFO, SA_NODEFER, SA_RESETHAND};
+pub use self::signal_os::{SIGBUS, SIGSTKSZ, SIG_SETMASK};
 
 use libc;
 
@@ -168,8 +169,8 @@ pub unsafe fn sigemptyset(set: *mut sigset_t) -> libc::c_int {
 #[cfg(any(target_os = "linux",
           target_os = "android"))]
 mod signal_os {
-    pub use self::arch::{SA_ONSTACK, SA_SIGINFO, SIGBUS, SIG_SETMASK,
-                         sigaction, sigaltstack};
+    pub use self::arch::{SA_ONSTACK, SA_SIGINFO, SA_NODEFER, SA_RESETHAND,
+                         SIGBUS, SIG_SETMASK, sigaction, sigaltstack};
     use libc;
 
     #[cfg(any(target_arch = "x86",
@@ -226,6 +227,8 @@ mod signal_os {
 
         pub const SA_ONSTACK: libc::c_ulong = 0x08000000;
         pub const SA_SIGINFO: libc::c_ulong = 0x00000004;
+        pub const SA_NODEFER: libc::c_ulong = 0x40000000;
+        pub const SA_RESETHAND: libc::c_ulong = 0x80000000;
 
         pub const SIGBUS: libc::c_int = 7;
 
@@ -275,6 +278,8 @@ mod signal_os {
 
         pub const SA_ONSTACK: libc::c_ulong = 0x08000000;
         pub const SA_SIGINFO: libc::c_ulong = 0x00000008;
+        pub const SA_NODEFER: libc::c_ulong = 0x40000000;
+        pub const SA_RESETHAND: libc::c_ulong = 0x80000000;
 
         pub const SIGBUS: libc::c_int = 10;
 
@@ -328,6 +333,8 @@ mod signal_os {
 
     pub const SA_ONSTACK: libc::c_int = 0x0001;
     pub const SA_SIGINFO: libc::c_int = 0x0040;
+    pub const SA_NODEFER: libc::c_ulong = 0x0010;
+    pub const SA_RESETHAND: libc::c_int = 0x0004;
 
     pub const SIGBUS: libc::c_int = 10;
 

diff --git a/src/libstd/sys/unix/stack_overflow.rs b/src/libstd/sys/unix/stack_overflow.rs
@@ -44,8 +44,8 @@ mod imp {
     use mem;
     use ptr;
     use intrinsics;
-    use sys::c::{siginfo, sigaction, SIGBUS, SIG_DFL,
-                 SA_SIGINFO, SA_ONSTACK, sigaltstack,
+    use sys::c::{siginfo, sigaction, sigaltstack, SIGBUS,
+                 SA_SIGINFO, SA_ONSTACK, SA_NODEFER, SA_RESETHAND,
                  SIGSTKSZ, sighandler_t, raise};
     use libc;
     use libc::funcs::posix88::mman::{mmap, munmap};
@@ -71,26 +71,18 @@ mod imp {
         // We can not return from a SIGSEGV or SIGBUS signal.
         // See: https://www.gnu.org/software/libc/manual/html_node/Handler-Returns.html
 
-        unsafe fn term(signum: libc::c_int) -> ! {
-            use core::mem::transmute;
-
-            signal(signum, transmute(SIG_DFL));
-            raise(signum);
-            intrinsics::abort();
-        }
-
         // We're calling into functions with stack checks
         stack::record_sp_limit(0);
 
         let guard = thread_info::stack_guard().unwrap_or(0);
         let addr = (*info).si_addr as usize;
 
         if guard == 0 || addr < guard - PAGE_SIZE || addr >= guard {
-            term(signum);
+            raise(signum);
+        } else {
+            report_overflow();
         }
 
-        report_overflow();
-
         intrinsics::abort()
     }
 
@@ -105,7 +97,7 @@ mod imp {
         PAGE_SIZE = psize as usize;
 
         let mut action: sigaction = mem::zeroed();
-        action.sa_flags = SA_SIGINFO | SA_ONSTACK;
+        action.sa_flags = SA_SIGINFO | SA_ONSTACK | SA_NODEFER | SA_RESETHAND;
         action.sa_sigaction = signal_handler as sighandler_t;
         sigaction(SIGSEGV, &action, ptr::null_mut());
         sigaction(SIGBUS, &action, ptr::null_mut());

diff --git a/src/test/run-pass/segfault-no-out-of-stack.rs b/src/test/run-pass/segfault-no-out-of-stack.rs
@@ -8,17 +8,20 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+#![feature(libc)]
+extern crate libc;
 
 use std::process::Command;
 use std::env;
+use libc::consts::os::posix88::SIGSEGV;
 
 fn main() {
     let args: Vec<String> = env::args().collect();
     if args.len() > 1 && args[1] == "segfault" {
         unsafe { *(0 as *mut isize) = 1 }; // trigger a segfault
     } else {
         let segfault = Command::new(&args[0]).arg("segfault").output().unwrap();
-        assert!(!segfault.status.success());
+        assert!(segfault.status.signal() == Some(SIGSEGV));
         let error = String::from_utf8_lossy(&segfault.stderr);
         assert!(!error.contains("has overflowed its stack"));
     }