Skip to content

Commit

Permalink
std: Avoid allocating panic message unless needed
Browse files Browse the repository at this point in the history
This commit removes allocation of the panic message in instances like
`panic!("foo: {}", "bar")` if we don't actually end up needing the message. We
don't need it in the case of wasm32 right now, and in general it's not needed
for panic=abort instances that use the default panic hook.

For now this commit only solves the wasm use case where with LTO the allocation
is entirely removed, but the panic=abort use case can be implemented at a later
date if needed.
  • Loading branch information
alexcrichton committed Apr 13, 2018
1 parent 2bb5b5c commit 46d16b6
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 50 deletions.
14 changes: 10 additions & 4 deletions src/libcore/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,17 @@ impl<'a> PanicInfo<'a> {
and related macros",
issue = "0")]
#[doc(hidden)]
pub fn internal_constructor(payload: &'a (Any + Send),
message: Option<&'a fmt::Arguments<'a>>,
#[inline]
pub fn internal_constructor(message: Option<&'a fmt::Arguments<'a>>,
location: Location<'a>)
-> Self {
PanicInfo { payload, location, message }
PanicInfo { payload: &(), location, message }
}

#[doc(hidden)]
#[inline]
pub fn set_payload(&mut self, info: &'a (Any + Send)) {
self.payload = info;
}

/// Returns the payload associated with the panic.
Expand Down Expand Up @@ -259,5 +265,5 @@ impl<'a> fmt::Display for Location<'a> {
#[doc(hidden)]
pub unsafe trait BoxMeUp {
fn box_me_up(&mut self) -> *mut (Any + Send);
fn get(&self) -> &(Any + Send);
fn get(&mut self) -> &(Any + Send);
}
118 changes: 73 additions & 45 deletions src/libstd/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,6 @@ fn default_hook(info: &PanicInfo) {
#[cfg(feature = "backtrace")]
use sys_common::backtrace;

// Some platforms know that printing to stderr won't ever actually print
// anything, and if that's the case we can skip everything below.
if stderr_prints_nothing() {
return
}

// If this is a double panic, make sure that we print a backtrace
// for this panic. Otherwise only print it if logging is enabled.
#[cfg(feature = "backtrace")]
Expand All @@ -185,9 +179,6 @@ fn default_hook(info: &PanicInfo) {
};

let location = info.location().unwrap(); // The current implementation always returns Some
let file = location.file();
let line = location.line();
let col = location.column();

let msg = match info.payload().downcast_ref::<&'static str>() {
Some(s) => *s,
Expand All @@ -201,8 +192,8 @@ fn default_hook(info: &PanicInfo) {
let name = thread.as_ref().and_then(|t| t.name()).unwrap_or("<unnamed>");

let write = |err: &mut ::io::Write| {
let _ = writeln!(err, "thread '{}' panicked at '{}', {}:{}:{}",
name, msg, file, line, col);
let _ = writeln!(err, "thread '{}' panicked at '{}', {}",
name, msg, location);

#[cfg(feature = "backtrace")]
{
Expand Down Expand Up @@ -350,9 +341,38 @@ pub fn begin_panic_fmt(msg: &fmt::Arguments,
// panic + OOM properly anyway (see comment in begin_panic
// below).

let mut s = String::new();
let _ = s.write_fmt(*msg);
rust_panic_with_hook(&mut PanicPayload::new(s), Some(msg), file_line_col)
rust_panic_with_hook(&mut PanicPayload::new(msg), Some(msg), file_line_col);

struct PanicPayload<'a> {
inner: &'a fmt::Arguments<'a>,
string: Option<String>,
}

impl<'a> PanicPayload<'a> {
fn new(inner: &'a fmt::Arguments<'a>) -> PanicPayload<'a> {
PanicPayload { inner, string: None }
}

fn fill(&mut self) -> &mut String {
let inner = self.inner;
self.string.get_or_insert_with(|| {
let mut s = String::new();
drop(s.write_fmt(*inner));
s
})
}
}

unsafe impl<'a> BoxMeUp for PanicPayload<'a> {
fn box_me_up(&mut self) -> *mut (Any + Send) {
let contents = mem::replace(self.fill(), String::new());
Box::into_raw(Box::new(contents))
}

fn get(&mut self) -> &(Any + Send) {
self.fill()
}
}
}

/// This is the entry point of panicking for panic!() and assert!().
Expand All @@ -368,42 +388,41 @@ pub fn begin_panic<M: Any + Send>(msg: M, file_line_col: &(&'static str, u32, u3
// be performed in the parent of this thread instead of the thread that's
// panicking.

rust_panic_with_hook(&mut PanicPayload::new(msg), None, file_line_col)
}

struct PanicPayload<A> {
inner: Option<A>,
}
rust_panic_with_hook(&mut PanicPayload::new(msg), None, file_line_col);

impl<A: Send + 'static> PanicPayload<A> {
fn new(inner: A) -> PanicPayload<A> {
PanicPayload { inner: Some(inner) }
struct PanicPayload<A> {
inner: Option<A>,
}
}

unsafe impl<A: Send + 'static> BoxMeUp for PanicPayload<A> {
fn box_me_up(&mut self) -> *mut (Any + Send) {
let data = match self.inner.take() {
Some(a) => Box::new(a) as Box<Any + Send>,
None => Box::new(()),
};
Box::into_raw(data)
impl<A: Send + 'static> PanicPayload<A> {
fn new(inner: A) -> PanicPayload<A> {
PanicPayload { inner: Some(inner) }
}
}

fn get(&self) -> &(Any + Send) {
match self.inner {
Some(ref a) => a,
None => &(),
unsafe impl<A: Send + 'static> BoxMeUp for PanicPayload<A> {
fn box_me_up(&mut self) -> *mut (Any + Send) {
let data = match self.inner.take() {
Some(a) => Box::new(a) as Box<Any + Send>,
None => Box::new(()),
};
Box::into_raw(data)
}

fn get(&mut self) -> &(Any + Send) {
match self.inner {
Some(ref a) => a,
None => &(),
}
}
}
}

/// Executes the primary logic for a panic, including checking for recursive
/// panics and panic hooks.
/// Central point for dispatching panics.
///
/// This is the entry point or panics from libcore, formatted panics, and
/// `Box<Any>` panics. Here we'll verify that we're not panicking recursively,
/// run panic hooks, and then delegate to the actual implementation of panics.
/// Executes the primary logic for a panic, including checking for recursive
/// panics, panic hooks, and finally dispatching to the panic runtime to either
/// abort or unwind.
fn rust_panic_with_hook(payload: &mut BoxMeUp,
message: Option<&fmt::Arguments>,
file_line_col: &(&'static str, u32, u32)) -> ! {
Expand All @@ -423,15 +442,24 @@ fn rust_panic_with_hook(payload: &mut BoxMeUp,
}

unsafe {
let info = PanicInfo::internal_constructor(
payload.get(),
let mut info = PanicInfo::internal_constructor(
message,
Location::internal_constructor(file, line, col),
);
HOOK_LOCK.read();
match HOOK {
Hook::Default => default_hook(&info),
Hook::Custom(ptr) => (*ptr)(&info),
// Some platforms know that printing to stderr won't ever actually
// print anything, and if that's the case we can skip the default
// hook.
Hook::Default if stderr_prints_nothing() => {}
Hook::Default => {
info.set_payload(payload.get());
default_hook(&info);
}
Hook::Custom(ptr) => {
info.set_payload(payload.get());
(*ptr)(&info);
}
}
HOOK_LOCK.read_unlock();
}
Expand Down Expand Up @@ -460,7 +488,7 @@ pub fn update_count_then_panic(msg: Box<Any + Send>) -> ! {
Box::into_raw(mem::replace(&mut self.0, Box::new(())))
}

fn get(&self) -> &(Any + Send) {
fn get(&mut self) -> &(Any + Send) {
&*self.0
}
}
Expand Down
8 changes: 7 additions & 1 deletion src/test/run-make/wasm-panic-small/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@

ifeq ($(TARGET),wasm32-unknown-unknown)
all:
$(RUSTC) foo.rs -C lto -O --target wasm32-unknown-unknown
$(RUSTC) foo.rs -C lto -O --target wasm32-unknown-unknown --cfg a
wc -c < $(TMPDIR)/foo.wasm
[ "`wc -c < $(TMPDIR)/foo.wasm`" -lt "1024" ]
$(RUSTC) foo.rs -C lto -O --target wasm32-unknown-unknown --cfg b
wc -c < $(TMPDIR)/foo.wasm
[ "`wc -c < $(TMPDIR)/foo.wasm`" -lt "5120" ]
$(RUSTC) foo.rs -C lto -O --target wasm32-unknown-unknown --cfg c
wc -c < $(TMPDIR)/foo.wasm
[ "`wc -c < $(TMPDIR)/foo.wasm`" -lt "5120" ]
else
all:
endif
Expand Down
13 changes: 13 additions & 0 deletions src/test/run-make/wasm-panic-small/foo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,19 @@
#![crate_type = "cdylib"]

#[no_mangle]
#[cfg(a)]
pub fn foo() {
panic!("test");
}

#[no_mangle]
#[cfg(b)]
pub fn foo() {
panic!("{}", 1);
}

#[no_mangle]
#[cfg(c)]
pub fn foo() {
panic!("{}", "a");
}

0 comments on commit 46d16b6

Please sign in to comment.