Skip to content

Commit

Permalink
std: Tweak stack overflow printing for robustness
Browse files Browse the repository at this point in the history
The printing of the error message on stack overflow had two sometimes false
assumptions previously. The first is that a local task was always available (it
called Local::take) and the second is that it used println! instead of
manually writing.

The first assumption isn't necessarily true because while stack overflow will
likely only be detected in situations that a local task is available, it's not
guaranteed to always be in TLS. For example, during a println! call a task
may be blocking, causing it to be unavailable. By using Local::try_take(), we
can be resilient against these occurrences.

The second assumption could lead to odd behavior because the stdout logger can
be overwritten to run arbitrary code. Currently this should be possible, but the
utility is much diminished because a stack overflow translates to an abort()
instead of a failure.
  • Loading branch information
alexcrichton committed Feb 26, 2014
1 parent 3c2650b commit 4f4d43b
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 5 deletions.
15 changes: 10 additions & 5 deletions src/libstd/rt/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub static RED_ZONE: uint = 20 * 1024;
// irrelevant for documentation purposes.
#[cfg(not(test))] // in testing, use the original libstd's version
pub extern "C" fn rust_stack_exhausted() {
use option::None;
use option::{Option, None, Some};
use rt::local::Local;
use rt::task::Task;
use str::Str;
Expand Down Expand Up @@ -85,16 +85,21 @@ pub extern "C" fn rust_stack_exhausted() {
// #9854 - unwinding on windows through __morestack has never worked
// #2361 - possible implementation of not using landing pads

let mut task = Local::borrow(None::<Task>);
let n = task.get().name.as_ref()
.map(|n| n.as_slice()).unwrap_or("<unnamed>");
let task: Option<~Task> = Local::try_take();
let name = match task {
Some(ref task) => {
task.name.as_ref().map(|n| n.as_slice())
}
None => None
};
let name = name.unwrap_or("<unknown>");

// See the message below for why this is not emitted to the
// task's logger. This has the additional conundrum of the
// logger may not be initialized just yet, meaning that an FFI
// call would happen to initialized it (calling out to libuv),
// and the FFI call needs 2MB of stack when we just ran out.
println!("task '{}' has overflowed its stack", n);
rterrln!("task '{}' has overflowed its stack", name);

intrinsics::abort();
}
Expand Down
50 changes: 50 additions & 0 deletions src/test/run-pass/out-of-stack.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// ignore-fast

#[feature(asm)];

use std::io::Process;
use std::os;
use std::str;

// lifted from the test module
pub fn black_box<T>(dummy: T) { unsafe { asm!("" : : "r"(&dummy)) } }

fn silent_recurse() {
let buf = [0, ..1000];
black_box(buf);
silent_recurse();
}

fn loud_recurse() {
println!("hello!");
loud_recurse();
}

fn main() {
let args = os::args();
if args.len() > 1 && args[1].as_slice() == "silent" {
silent_recurse();
} else if args.len() > 1 && args[1].as_slice() == "loud" {
loud_recurse();
} else {
let silent = Process::output(args[0], [~"silent"]).unwrap();
assert!(!silent.status.success());
let error = str::from_utf8_lossy(silent.error);
assert!(error.as_slice().contains("has overflowed its stack"));

let loud = Process::output(args[0], [~"loud"]).unwrap();
assert!(!loud.status.success());
let error = str::from_utf8_lossy(silent.error);
assert!(error.as_slice().contains("has overflowed its stack"));
}
}

5 comments on commit 4f4d43b

@bors
Copy link
Contributor

@bors bors commented on 4f4d43b Feb 26, 2014

Choose a reason for hiding this comment

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

saw approval from brson
at alexcrichton@4f4d43b

@bors
Copy link
Contributor

@bors bors commented on 4f4d43b Feb 26, 2014

Choose a reason for hiding this comment

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

merging alexcrichton/rust/fix-stack-overflow = 4f4d43b into auto

@bors
Copy link
Contributor

@bors bors commented on 4f4d43b Feb 26, 2014

Choose a reason for hiding this comment

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

alexcrichton/rust/fix-stack-overflow = 4f4d43b merged ok, testing candidate = eb86913

@bors
Copy link
Contributor

@bors bors commented on 4f4d43b Feb 26, 2014

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

@bors bors commented on 4f4d43b Feb 26, 2014

Choose a reason for hiding this comment

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

fast-forwarding master to auto = eb86913

Please sign in to comment.