std: Avoid allocating panic message unless needed

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.

diff --git a/src/libcore/panic.rs b/src/libcore/panic.rs
index 37c79f2fafe9777e0f7ea5dd276221eba4ee0e9b..27ec4aaac75dec3a9ed09cc4f268a880f97f3896 100644 (file)
--- a/src/libcore/panic.rs
+++ b/src/libcore/panic.rs
@@ -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.
@@ -259,5 +265,5 @@ fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
 #[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);
 }

diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs
index 715fd45e490da8b2cecbbcf32398c6dd13020120..24eae6a4c821e1db50005be285519f85510a2eb3 100644 (file)
--- a/src/libstd/panicking.rs
+++ b/src/libstd/panicking.rs
@@ -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")]
@@ -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,
@@ -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")]
         {
@@ -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!().
@@ -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)) -> ! {
@@ -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();
     }
@@ -460,7 +488,7 @@ fn box_me_up(&mut self) -> *mut (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
         }
     }

diff --git a/src/test/run-make/wasm-panic-small/Makefile b/src/test/run-make/wasm-panic-small/Makefile
index a11fba235957b4dde171ffe04bf4b4386957a8a5..330ae300c445ee4334e67ed60e88ad53fe383243 100644 (file)
--- a/src/test/run-make/wasm-panic-small/Makefile
+++ b/src/test/run-make/wasm-panic-small/Makefile
@@ -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

diff --git a/src/test/run-make/wasm-panic-small/foo.rs b/src/test/run-make/wasm-panic-small/foo.rs
index 9654d5f7c0991af1ec32d6aee34dd42eb85059b3..1ea724ca94d477951407704d3b4ae957d43d0429 100644 (file)
--- a/src/test/run-make/wasm-panic-small/foo.rs
+++ b/src/test/run-make/wasm-panic-small/foo.rs
@@ -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");
+}