diff --git a/src/libstd/sys/unix/stack_overflow.rs b/src/libstd/sys/unix/stack_overflow.rs index c7614db329959..510c4df4e2eb2 100644 --- a/src/libstd/sys/unix/stack_overflow.rs +++ b/src/libstd/sys/unix/stack_overflow.rs @@ -83,16 +83,19 @@ mod imp { // were originally supposed to do. // // This handler currently exists purely to print an informative message - // whenever a thread overflows its stack. When run the handler always - // un-registers itself after running and then returns (to allow the original - // signal to be delivered again). By returning we're ensuring that segfaults - // do indeed look like segfaults. + // whenever a thread overflows its stack. We then abort to exit and + // indicate a crash, but to avoid a misleading SIGSEGV that might lead + // users to believe that unsafe code has accessed an invalid pointer; the + // SIGSEGV encountered when overflowing the stack is expected and + // well-defined. // - // Returning from this kind of signal handler is technically not defined to - // work when reading the POSIX spec strictly, but in practice it turns out - // many large systems and all implementations allow returning from a signal - // handler to work. For a more detailed explanation see the comments on - // #26458. + // If this is not a stack overflow, the handler un-registers itself and + // then returns (to allow the original signal to be delivered again). + // Returning from this kind of signal handler is technically not defined + // to work when reading the POSIX spec strictly, but in practice it turns + // out many large systems and all implementations allow returning from a + // signal handler to work. For a more detailed explanation see the + // comments on #26458. unsafe extern fn signal_handler(signum: libc::c_int, info: *mut libc::siginfo_t, _data: *mut libc::c_void) { @@ -102,17 +105,18 @@ mod imp { let addr = siginfo_si_addr(info); // If the faulting address is within the guard page, then we print a - // message saying so. + // message saying so and abort. if guard != 0 && guard - PAGE_SIZE <= addr && addr < guard { report_overflow(); + rtabort!("stack overflow"); + } else { + // Unregister ourselves by reverting back to the default behavior. + let mut action: sigaction = mem::zeroed(); + action.sa_sigaction = SIG_DFL; + sigaction(signum, &action, ptr::null_mut()); + + // See comment above for why this function returns. } - - // Unregister ourselves by reverting back to the default behavior. - let mut action: sigaction = mem::zeroed(); - action.sa_sigaction = SIG_DFL; - sigaction(signum, &action, ptr::null_mut()); - - // See comment above for why this function returns. } static mut MAIN_ALTSTACK: *mut libc::c_void = ptr::null_mut(); diff --git a/src/rt/rust_test_helpers.c b/src/rt/rust_test_helpers.c index 320cd3dbd851a..00bfa63e6fea8 100644 --- a/src/rt/rust_test_helpers.c +++ b/src/rt/rust_test_helpers.c @@ -158,6 +158,11 @@ rust_get_test_int() { return 1; } +char * +rust_get_null_ptr() { + return 0; +} + /* Debug helpers strictly to verify ABI conformance. * * FIXME (#2665): move these into a testcase when the testsuite diff --git a/src/test/run-pass/out-of-stack.rs b/src/test/run-pass/out-of-stack.rs index 149de10ce0242..4401861a1631c 100644 --- a/src/test/run-pass/out-of-stack.rs +++ b/src/test/run-pass/out-of-stack.rs @@ -12,6 +12,10 @@ // ignore-musl #![feature(asm)] +#![feature(libc)] + +#[cfg(unix)] +extern crate libc; use std::env; use std::process::Command; @@ -35,6 +39,24 @@ fn loud_recurse() { black_box(()); // don't optimize this into a tail call. please. } +#[cfg(unix)] +fn check_status(status: std::process::ExitStatus) +{ + use libc; + use std::os::unix::process::ExitStatusExt; + + assert!(!status.success()); + assert!(status.signal() != Some(libc::SIGSEGV) + && status.signal() != Some(libc::SIGBUS)); +} + +#[cfg(not(unix))] +fn check_status(status: std::process::ExitStatus) +{ + assert!(!status.success()); +} + + fn main() { let args: Vec = env::args().collect(); if args.len() > 1 && args[1] == "silent" { @@ -62,7 +84,9 @@ fn main() { println!("testing: {}", mode); let silent = Command::new(&args[0]).arg(mode).output().unwrap(); - assert!(!silent.status.success()); + + check_status(silent.status); + let error = String::from_utf8_lossy(&silent.stderr); assert!(error.contains("has overflowed its stack"), "missing overflow message: {}", error); diff --git a/src/test/run-pass/segfault-no-out-of-stack.rs b/src/test/run-pass/segfault-no-out-of-stack.rs index 6f5fc8e6b4e4e..0158c4282dae7 100644 --- a/src/test/run-pass/segfault-no-out-of-stack.rs +++ b/src/test/run-pass/segfault-no-out-of-stack.rs @@ -15,10 +15,31 @@ extern crate libc; use std::process::{Command, ExitStatus}; use std::env; +#[link(name = "rust_test_helpers")] +extern { + fn rust_get_null_ptr() -> *mut ::libc::c_char; +} + +#[cfg(unix)] +fn check_status(status: std::process::ExitStatus) +{ + use libc; + use std::os::unix::process::ExitStatusExt; + + assert!(status.signal() == Some(libc::SIGSEGV) + || status.signal() == Some(libc::SIGBUS)); +} + +#[cfg(not(unix))] +fn check_status(status: std::process::ExitStatus) +{ + assert!(!status.success()); +} + fn main() { let args: Vec = env::args().collect(); if args.len() > 1 && args[1] == "segfault" { - unsafe { *(0 as *mut isize) = 1 }; // trigger a segfault + unsafe { *rust_get_null_ptr() = 1; }; // trigger a segfault } else { let segfault = Command::new(&args[0]).arg("segfault").output().unwrap(); let stderr = String::from_utf8_lossy(&segfault.stderr); @@ -26,7 +47,7 @@ fn main() { println!("stdout: {}", stdout); println!("stderr: {}", stderr); println!("status: {}", segfault.status); - assert!(!segfault.status.success()); + check_status(segfault.status); assert!(!stderr.contains("has overflowed its stack")); } }