From c3a5d6b130e27d7d7587f56581247d5b08c38594 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 29 Mar 2018 14:59:13 -0700 Subject: [PATCH 1/4] std: Minimize size of panicking on wasm This commit applies a few code size optimizations for the wasm target to the standard library, namely around panics. We notably know that in most configurations it's impossible for us to print anything in wasm32-unknown-unknown so we can skip larger portions of panicking that are otherwise simply informative. This allows us to get quite a nice size reduction. Finally we can also tweak where the allocation happens for the `Box` that we panic with. By only allocating once unwinding starts we can reduce the size of a panicking wasm module from 44k to 350 bytes. --- src/ci/docker/wasm32-unknown/Dockerfile | 6 ++ src/libcore/panic.rs | 10 +++ src/libpanic_abort/lib.rs | 2 +- src/libpanic_unwind/lib.rs | 12 +-- src/libstd/lib.rs | 1 + src/libstd/panicking.rs | 88 ++++++++++++++++----- src/libstd/sys/cloudabi/stdio.rs | 4 + src/libstd/sys/redox/stdio.rs | 4 + src/libstd/sys/unix/stdio.rs | 4 + src/libstd/sys/wasm/rwlock.rs | 4 +- src/libstd/sys/wasm/stdio.rs | 4 + src/libstd/sys/windows/stdio.rs | 4 + src/libstd/sys_common/backtrace.rs | 13 ++- src/libstd/sys_common/mod.rs | 14 +++- src/libstd/sys_common/thread_local.rs | 4 +- src/libstd/sys_common/util.rs | 5 +- src/libstd/thread/local.rs | 41 +++++++++- src/libstd/thread/mod.rs | 3 + src/test/run-make/wasm-panic-small/Makefile | 11 +++ src/test/run-make/wasm-panic-small/foo.rs | 16 ++++ 20 files changed, 205 insertions(+), 45 deletions(-) create mode 100644 src/test/run-make/wasm-panic-small/Makefile create mode 100644 src/test/run-make/wasm-panic-small/foo.rs diff --git a/src/ci/docker/wasm32-unknown/Dockerfile b/src/ci/docker/wasm32-unknown/Dockerfile index 853923ad947cd..56eda5480715b 100644 --- a/src/ci/docker/wasm32-unknown/Dockerfile +++ b/src/ci/docker/wasm32-unknown/Dockerfile @@ -25,6 +25,12 @@ ENV RUST_CONFIGURE_ARGS \ --set build.nodejs=/node-v9.2.0-linux-x64/bin/node \ --set rust.lld +# Some run-make tests have assertions about code size, and enabling debug +# assertions in libstd causes the binary to be much bigger than it would +# otherwise normally be. We already test libstd with debug assertions in lots of +# other contexts as well +ENV NO_DEBUG_ASSERTIONS=1 + ENV SCRIPT python2.7 /checkout/x.py test --target $TARGETS \ src/test/run-make \ src/test/ui \ diff --git a/src/libcore/panic.rs b/src/libcore/panic.rs index 1720c9d8c6079..37c79f2fafe97 100644 --- a/src/libcore/panic.rs +++ b/src/libcore/panic.rs @@ -251,3 +251,13 @@ impl<'a> fmt::Display for Location<'a> { write!(formatter, "{}:{}:{}", self.file, self.line, self.col) } } + +/// An internal trait used by libstd to pass data from libstd to `panic_unwind` +/// and other panic runtimes. Not intended to be stabilized any time soon, do +/// not use. +#[unstable(feature = "std_internals", issue = "0")] +#[doc(hidden)] +pub unsafe trait BoxMeUp { + fn box_me_up(&mut self) -> *mut (Any + Send); + fn get(&self) -> &(Any + Send); +} diff --git a/src/libpanic_abort/lib.rs b/src/libpanic_abort/lib.rs index 43c5bbbc618c2..392bf17968fbd 100644 --- a/src/libpanic_abort/lib.rs +++ b/src/libpanic_abort/lib.rs @@ -52,7 +52,7 @@ pub unsafe extern fn __rust_maybe_catch_panic(f: fn(*mut u8), // now hopefully. #[no_mangle] #[rustc_std_internal_symbol] -pub unsafe extern fn __rust_start_panic(_data: usize, _vtable: usize) -> u32 { +pub unsafe extern fn __rust_start_panic(_payload: usize) -> u32 { abort(); #[cfg(any(unix, target_os = "cloudabi"))] diff --git a/src/libpanic_unwind/lib.rs b/src/libpanic_unwind/lib.rs index 9321d6917d156..6c52c0fa10cc0 100644 --- a/src/libpanic_unwind/lib.rs +++ b/src/libpanic_unwind/lib.rs @@ -29,6 +29,7 @@ html_root_url = "https://doc.rust-lang.org/nightly/", issue_tracker_base_url = "https://github.com/rust-lang/rust/issues/")] +#![feature(allocator_api)] #![feature(alloc)] #![feature(core_intrinsics)] #![feature(lang_items)] @@ -36,6 +37,7 @@ #![feature(panic_unwind)] #![feature(raw)] #![feature(staged_api)] +#![feature(std_internals)] #![feature(unwind_attributes)] #![cfg_attr(target_env = "msvc", feature(raw))] @@ -47,9 +49,11 @@ extern crate libc; #[cfg(not(any(target_env = "msvc", all(windows, target_arch = "x86_64", target_env = "gnu"))))] extern crate unwind; +use alloc::boxed::Box; use core::intrinsics; use core::mem; use core::raw; +use core::panic::BoxMeUp; // Rust runtime's startup objects depend on these symbols, so make them public. #[cfg(all(target_os="windows", target_arch = "x86", target_env="gnu"))] @@ -112,9 +116,7 @@ pub unsafe extern "C" fn __rust_maybe_catch_panic(f: fn(*mut u8), // implementation. #[no_mangle] #[unwind(allowed)] -pub unsafe extern "C" fn __rust_start_panic(data: usize, vtable: usize) -> u32 { - imp::panic(mem::transmute(raw::TraitObject { - data: data as *mut (), - vtable: vtable as *mut (), - })) +pub unsafe extern "C" fn __rust_start_panic(payload: usize) -> u32 { + let payload = payload as *mut &mut BoxMeUp; + imp::panic(Box::from_raw((*payload).box_me_up())) } diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index a34fcb5a7f98b..dd96c57538c79 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -292,6 +292,7 @@ #![feature(rand)] #![feature(raw)] #![feature(rustc_attrs)] +#![feature(std_internals)] #![feature(stdsimd)] #![feature(shrink_to)] #![feature(slice_bytes)] diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index fba3269204e90..715fd45e490da 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -17,6 +17,8 @@ //! * Executing a panic up to doing the actual implementation //! * Shims around "try" +use core::panic::BoxMeUp; + use io::prelude::*; use any::Any; @@ -27,7 +29,7 @@ use intrinsics; use mem; use ptr; use raw; -use sys::stdio::Stderr; +use sys::stdio::{Stderr, stderr_prints_nothing}; use sys_common::rwlock::RWLock; use sys_common::thread_info; use sys_common::util; @@ -56,7 +58,7 @@ extern { data_ptr: *mut usize, vtable_ptr: *mut usize) -> u32; #[unwind(allowed)] - fn __rust_start_panic(data: usize, vtable: usize) -> u32; + fn __rust_start_panic(payload: usize) -> u32; } #[derive(Copy, Clone)] @@ -163,6 +165,12 @@ 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")] @@ -212,15 +220,15 @@ fn default_hook(info: &PanicInfo) { let prev = LOCAL_STDERR.with(|s| s.borrow_mut().take()); match (prev, err.as_mut()) { - (Some(mut stderr), _) => { - write(&mut *stderr); - let mut s = Some(stderr); - LOCAL_STDERR.with(|slot| { - *slot.borrow_mut() = s.take(); - }); - } - (None, Some(ref mut err)) => { write(err) } - _ => {} + (Some(mut stderr), _) => { + write(&mut *stderr); + let mut s = Some(stderr); + LOCAL_STDERR.with(|slot| { + *slot.borrow_mut() = s.take(); + }); + } + (None, Some(ref mut err)) => { write(err) } + _ => {} } } @@ -344,7 +352,7 @@ pub fn begin_panic_fmt(msg: &fmt::Arguments, let mut s = String::new(); let _ = s.write_fmt(*msg); - rust_panic_with_hook(Box::new(s), Some(msg), file_line_col) + rust_panic_with_hook(&mut PanicPayload::new(s), Some(msg), file_line_col) } /// This is the entry point of panicking for panic!() and assert!(). @@ -360,7 +368,34 @@ pub fn begin_panic(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(Box::new(msg), None, file_line_col) + rust_panic_with_hook(&mut PanicPayload::new(msg), None, file_line_col) +} + +struct PanicPayload { + inner: Option, +} + +impl PanicPayload { + fn new(inner: A) -> PanicPayload { + PanicPayload { inner: Some(inner) } + } +} + +unsafe impl BoxMeUp for PanicPayload { + fn box_me_up(&mut self) -> *mut (Any + Send) { + let data = match self.inner.take() { + Some(a) => Box::new(a) as Box, + None => Box::new(()), + }; + Box::into_raw(data) + } + + fn get(&self) -> &(Any + Send) { + match self.inner { + Some(ref a) => a, + None => &(), + } + } } /// Executes the primary logic for a panic, including checking for recursive @@ -369,9 +404,7 @@ pub fn begin_panic(msg: M, file_line_col: &(&'static str, u32, u3 /// This is the entry point or panics from libcore, formatted panics, and /// `Box` panics. Here we'll verify that we're not panicking recursively, /// run panic hooks, and then delegate to the actual implementation of panics. -#[inline(never)] -#[cold] -fn rust_panic_with_hook(payload: Box, +fn rust_panic_with_hook(payload: &mut BoxMeUp, message: Option<&fmt::Arguments>, file_line_col: &(&'static str, u32, u32)) -> ! { let (file, line, col) = *file_line_col; @@ -391,7 +424,7 @@ fn rust_panic_with_hook(payload: Box, unsafe { let info = PanicInfo::internal_constructor( - &*payload, + payload.get(), message, Location::internal_constructor(file, line, col), ); @@ -419,16 +452,29 @@ fn rust_panic_with_hook(payload: Box, /// Shim around rust_panic. Called by resume_unwind. pub fn update_count_then_panic(msg: Box) -> ! { update_panic_count(1); - rust_panic(msg) + + struct RewrapBox(Box); + + unsafe impl BoxMeUp for RewrapBox { + fn box_me_up(&mut self) -> *mut (Any + Send) { + Box::into_raw(mem::replace(&mut self.0, Box::new(()))) + } + + fn get(&self) -> &(Any + Send) { + &*self.0 + } + } + + rust_panic(&mut RewrapBox(msg)) } /// A private no-mangle function on which to slap yer breakpoints. #[no_mangle] #[allow(private_no_mangle_fns)] // yes we get it, but we like breakpoints -pub fn rust_panic(msg: Box) -> ! { +pub fn rust_panic(mut msg: &mut BoxMeUp) -> ! { let code = unsafe { - let obj = mem::transmute::<_, raw::TraitObject>(msg); - __rust_start_panic(obj.data as usize, obj.vtable as usize) + let obj = &mut msg as *mut &mut BoxMeUp; + __rust_start_panic(obj as usize) }; rtabort!("failed to initiate panic, error {}", code) } diff --git a/src/libstd/sys/cloudabi/stdio.rs b/src/libstd/sys/cloudabi/stdio.rs index 9519a92647108..1d7344f921c9d 100644 --- a/src/libstd/sys/cloudabi/stdio.rs +++ b/src/libstd/sys/cloudabi/stdio.rs @@ -77,3 +77,7 @@ pub fn is_ebadf(err: &io::Error) -> bool { } pub const STDIN_BUF_SIZE: usize = ::sys_common::io::DEFAULT_BUF_SIZE; + +pub fn stderr_prints_nothing() -> bool { + false +} diff --git a/src/libstd/sys/redox/stdio.rs b/src/libstd/sys/redox/stdio.rs index 3abb094ac34e3..7a4d11b0ecb9a 100644 --- a/src/libstd/sys/redox/stdio.rs +++ b/src/libstd/sys/redox/stdio.rs @@ -75,3 +75,7 @@ pub fn is_ebadf(err: &io::Error) -> bool { } pub const STDIN_BUF_SIZE: usize = ::sys_common::io::DEFAULT_BUF_SIZE; + +pub fn stderr_prints_nothing() -> bool { + false +} diff --git a/src/libstd/sys/unix/stdio.rs b/src/libstd/sys/unix/stdio.rs index e9b3d4affc7dd..87ba2aef4f1d3 100644 --- a/src/libstd/sys/unix/stdio.rs +++ b/src/libstd/sys/unix/stdio.rs @@ -75,3 +75,7 @@ pub fn is_ebadf(err: &io::Error) -> bool { } pub const STDIN_BUF_SIZE: usize = ::sys_common::io::DEFAULT_BUF_SIZE; + +pub fn stderr_prints_nothing() -> bool { + false +} diff --git a/src/libstd/sys/wasm/rwlock.rs b/src/libstd/sys/wasm/rwlock.rs index 8b06f54167487..6516010af4759 100644 --- a/src/libstd/sys/wasm/rwlock.rs +++ b/src/libstd/sys/wasm/rwlock.rs @@ -30,7 +30,7 @@ impl RWLock { if *mode >= 0 { *mode += 1; } else { - panic!("rwlock locked for writing"); + rtabort!("rwlock locked for writing"); } } @@ -51,7 +51,7 @@ impl RWLock { if *mode == 0 { *mode = -1; } else { - panic!("rwlock locked for reading") + rtabort!("rwlock locked for reading") } } diff --git a/src/libstd/sys/wasm/stdio.rs b/src/libstd/sys/wasm/stdio.rs index beb19c0ed2c1f..023f29576a27d 100644 --- a/src/libstd/sys/wasm/stdio.rs +++ b/src/libstd/sys/wasm/stdio.rs @@ -69,3 +69,7 @@ pub const STDIN_BUF_SIZE: usize = 0; pub fn is_ebadf(_err: &io::Error) -> bool { true } + +pub fn stderr_prints_nothing() -> bool { + !cfg!(feature = "wasm_syscall") +} diff --git a/src/libstd/sys/windows/stdio.rs b/src/libstd/sys/windows/stdio.rs index b43df20bddd08..81b89da21d3c6 100644 --- a/src/libstd/sys/windows/stdio.rs +++ b/src/libstd/sys/windows/stdio.rs @@ -227,3 +227,7 @@ pub fn is_ebadf(err: &io::Error) -> bool { // idea is that on windows we use a slightly smaller buffer that's // been seen to be acceptable. pub const STDIN_BUF_SIZE: usize = 8 * 1024; + +pub fn stderr_prints_nothing() -> bool { + false +} diff --git a/src/libstd/sys_common/backtrace.rs b/src/libstd/sys_common/backtrace.rs index 1955f3ec9a28f..20109d2d0d5ac 100644 --- a/src/libstd/sys_common/backtrace.rs +++ b/src/libstd/sys_common/backtrace.rs @@ -139,10 +139,10 @@ pub fn __rust_begin_short_backtrace(f: F) -> T /// Controls how the backtrace should be formatted. #[derive(Debug, Copy, Clone, Eq, PartialEq)] pub enum PrintFormat { - /// Show all the frames with absolute path for files. - Full = 2, /// Show only relevant data from the backtrace. - Short = 3, + Short = 2, + /// Show all the frames with absolute path for files. + Full = 3, } // For now logging is turned off by default, and this function checks to see @@ -150,11 +150,10 @@ pub enum PrintFormat { pub fn log_enabled() -> Option { static ENABLED: atomic::AtomicIsize = atomic::AtomicIsize::new(0); match ENABLED.load(Ordering::SeqCst) { - 0 => {}, + 0 => {} 1 => return None, - 2 => return Some(PrintFormat::Full), - 3 => return Some(PrintFormat::Short), - _ => unreachable!(), + 2 => return Some(PrintFormat::Short), + _ => return Some(PrintFormat::Full), } let val = match env::var_os("RUST_BACKTRACE") { diff --git a/src/libstd/sys_common/mod.rs b/src/libstd/sys_common/mod.rs index 27504d374ddbf..d0c4d6a773746 100644 --- a/src/libstd/sys_common/mod.rs +++ b/src/libstd/sys_common/mod.rs @@ -28,6 +28,16 @@ use sync::Once; use sys; +macro_rules! rtabort { + ($($t:tt)*) => (::sys_common::util::abort(format_args!($($t)*))) +} + +macro_rules! rtassert { + ($e:expr) => (if !$e { + rtabort!(concat!("assertion failed: ", stringify!($e))); + }) +} + pub mod at_exit_imp; #[cfg(feature = "backtrace")] pub mod backtrace; @@ -101,10 +111,6 @@ pub fn at_exit(f: F) -> Result<(), ()> { if at_exit_imp::push(Box::new(f)) {Ok(())} else {Err(())} } -macro_rules! rtabort { - ($($t:tt)*) => (::sys_common::util::abort(format_args!($($t)*))) -} - /// One-time runtime cleanup. pub fn cleanup() { static CLEANUP: Once = Once::new(); diff --git a/src/libstd/sys_common/thread_local.rs b/src/libstd/sys_common/thread_local.rs index a4aa3d96d25c0..d0d6224de0a15 100644 --- a/src/libstd/sys_common/thread_local.rs +++ b/src/libstd/sys_common/thread_local.rs @@ -169,7 +169,7 @@ impl StaticKey { self.key.store(key, Ordering::SeqCst); } INIT_LOCK.unlock(); - assert!(key != 0); + rtassert!(key != 0); return key } @@ -190,7 +190,7 @@ impl StaticKey { imp::destroy(key1); key2 }; - assert!(key != 0); + rtassert!(key != 0); match self.key.compare_and_swap(0, key as usize, Ordering::SeqCst) { // The CAS succeeded, so we've created the actual key 0 => key as usize, diff --git a/src/libstd/sys_common/util.rs b/src/libstd/sys_common/util.rs index a391c7cc6ef0c..a373e980b970d 100644 --- a/src/libstd/sys_common/util.rs +++ b/src/libstd/sys_common/util.rs @@ -10,10 +10,13 @@ use fmt; use io::prelude::*; -use sys::stdio::Stderr; +use sys::stdio::{Stderr, stderr_prints_nothing}; use thread; pub fn dumb_print(args: fmt::Arguments) { + if stderr_prints_nothing() { + return + } let _ = Stderr::new().map(|mut stderr| stderr.write_fmt(args)); } diff --git a/src/libstd/thread/local.rs b/src/libstd/thread/local.rs index 99479bc56eff3..40d3280baa687 100644 --- a/src/libstd/thread/local.rs +++ b/src/libstd/thread/local.rs @@ -172,12 +172,16 @@ macro_rules! __thread_local_inner { &'static $crate::cell::UnsafeCell< $crate::option::Option<$t>>> { + #[cfg(target_arch = "wasm32")] + static __KEY: $crate::thread::__StaticLocalKeyInner<$t> = + $crate::thread::__StaticLocalKeyInner::new(); + #[thread_local] - #[cfg(target_thread_local)] + #[cfg(all(target_thread_local, not(target_arch = "wasm32")))] static __KEY: $crate::thread::__FastLocalKeyInner<$t> = $crate::thread::__FastLocalKeyInner::new(); - #[cfg(not(target_thread_local))] + #[cfg(all(not(target_thread_local), not(target_arch = "wasm32")))] static __KEY: $crate::thread::__OsLocalKeyInner<$t> = $crate::thread::__OsLocalKeyInner::new(); @@ -295,6 +299,39 @@ impl LocalKey { } } +/// On some platforms like wasm32 there's no threads, so no need to generate +/// thread locals and we can instead just use plain statics! +#[doc(hidden)] +#[cfg(target_arch = "wasm32")] +pub mod statik { + use cell::UnsafeCell; + use fmt; + + pub struct Key { + inner: UnsafeCell>, + } + + unsafe impl ::marker::Sync for Key { } + + impl fmt::Debug for Key { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.pad("Key { .. }") + } + } + + impl Key { + pub const fn new() -> Key { + Key { + inner: UnsafeCell::new(None), + } + } + + pub unsafe fn get(&self) -> Option<&'static UnsafeCell>> { + Some(&*(&self.inner as *const _)) + } + } +} + #[doc(hidden)] #[cfg(target_thread_local)] pub mod fast { diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index 71aee673cfe3e..1b976b79b4c98 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -202,6 +202,9 @@ pub use self::local::{LocalKey, AccessError}; // where fast TLS was not available; end-user code is compiled with fast TLS // where available, but both are needed. +#[unstable(feature = "libstd_thread_internals", issue = "0")] +#[cfg(target_arch = "wasm32")] +#[doc(hidden)] pub use self::local::statik::Key as __StaticLocalKeyInner; #[unstable(feature = "libstd_thread_internals", issue = "0")] #[cfg(target_thread_local)] #[doc(hidden)] pub use self::local::fast::Key as __FastLocalKeyInner; diff --git a/src/test/run-make/wasm-panic-small/Makefile b/src/test/run-make/wasm-panic-small/Makefile new file mode 100644 index 0000000000000..a11fba235957b --- /dev/null +++ b/src/test/run-make/wasm-panic-small/Makefile @@ -0,0 +1,11 @@ +-include ../../run-make-fulldeps/tools.mk + +ifeq ($(TARGET),wasm32-unknown-unknown) +all: + $(RUSTC) foo.rs -C lto -O --target wasm32-unknown-unknown + wc -c < $(TMPDIR)/foo.wasm + [ "`wc -c < $(TMPDIR)/foo.wasm`" -lt "1024" ] +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 new file mode 100644 index 0000000000000..9654d5f7c0991 --- /dev/null +++ b/src/test/run-make/wasm-panic-small/foo.rs @@ -0,0 +1,16 @@ +// Copyright 2018 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![crate_type = "cdylib"] + +#[no_mangle] +pub fn foo() { + panic!("test"); +} From 66c5e3ffb2b7a0804ceb989b9dc9138a7758bfd6 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 6 Apr 2018 13:46:10 -0700 Subject: [PATCH 2/4] Reduce the size of panics in RawVec Create one canonical location which panics with "capacity overflow" instead of having many. This reduces the size of a `panic!("{}", 1)` binary on wasm from 34k to 17k. --- src/liballoc/raw_vec.rs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/liballoc/raw_vec.rs b/src/liballoc/raw_vec.rs index 214cc7d7d0cd9..ae933f937c462 100644 --- a/src/liballoc/raw_vec.rs +++ b/src/liballoc/raw_vec.rs @@ -85,8 +85,8 @@ impl RawVec { unsafe { let elem_size = mem::size_of::(); - let alloc_size = cap.checked_mul(elem_size).expect("capacity overflow"); - alloc_guard(alloc_size).expect("capacity overflow"); + let alloc_size = cap.checked_mul(elem_size).unwrap_or_else(|| capacity_overflow()); + alloc_guard(alloc_size).unwrap_or_else(|_| capacity_overflow()); // handles ZSTs and `cap = 0` alike let ptr = if alloc_size == 0 { @@ -309,7 +309,7 @@ impl RawVec { // `from_size_align_unchecked`. let new_cap = 2 * self.cap; let new_size = new_cap * elem_size; - alloc_guard(new_size).expect("capacity overflow"); + alloc_guard(new_size).unwrap_or_else(|_| capacity_overflow()); let ptr_res = self.a.realloc(NonNull::from(self.ptr).as_opaque(), cur, new_size); @@ -368,7 +368,7 @@ impl RawVec { // overflow and the alignment is sufficiently small. let new_cap = 2 * self.cap; let new_size = new_cap * elem_size; - alloc_guard(new_size).expect("capacity overflow"); + alloc_guard(new_size).unwrap_or_else(|_| capacity_overflow()); match self.a.grow_in_place(NonNull::from(self.ptr).as_opaque(), old_layout, new_size) { Ok(_) => { // We can't directly divide `size`. @@ -440,7 +440,7 @@ impl RawVec { pub fn reserve_exact(&mut self, used_cap: usize, needed_extra_cap: usize) { match self.try_reserve_exact(used_cap, needed_extra_cap) { - Err(CapacityOverflow) => panic!("capacity overflow"), + Err(CapacityOverflow) => capacity_overflow(), Err(AllocErr) => self.a.oom(), Ok(()) => { /* yay */ } } @@ -550,7 +550,7 @@ impl RawVec { /// The same as try_reserve, but errors are lowered to a call to oom(). pub fn reserve(&mut self, used_cap: usize, needed_extra_cap: usize) { match self.try_reserve(used_cap, needed_extra_cap) { - Err(CapacityOverflow) => panic!("capacity overflow"), + Err(CapacityOverflow) => capacity_overflow(), Err(AllocErr) => self.a.oom(), Ok(()) => { /* yay */ } } @@ -591,7 +591,7 @@ impl RawVec { } let new_cap = self.amortized_new_size(used_cap, needed_extra_cap) - .expect("capacity overflow"); + .unwrap_or_else(|_| capacity_overflow()); // Here, `cap < used_cap + needed_extra_cap <= new_cap` // (regardless of whether `self.cap - used_cap` wrapped). @@ -599,7 +599,7 @@ impl RawVec { let new_layout = Layout::new::().repeat(new_cap).unwrap().0; // FIXME: may crash and burn on over-reserve - alloc_guard(new_layout.size()).expect("capacity overflow"); + alloc_guard(new_layout.size()).unwrap_or_else(|_| capacity_overflow()); match self.a.grow_in_place( NonNull::from(self.ptr).as_opaque(), old_layout, new_layout.size(), ) { @@ -731,6 +731,13 @@ fn alloc_guard(alloc_size: usize) -> Result<(), CollectionAllocErr> { } } +// One central function responsible for reporting capacity overflows. This'll +// ensure that the code generation related to these panics is minimal as there's +// only one location which panics rather than a bunch throughout the module. +fn capacity_overflow() -> ! { + panic!("capacity overflow") +} + #[cfg(test)] mod tests { use super::*; From 2bb5b5c07c5cd015c567ce86eae07e92db07bb8a Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 6 Apr 2018 14:20:22 -0700 Subject: [PATCH 3/4] core: Remove an implicit panic from Formatter::pad The expression `&s[..i]` in general can panic if `i` is out of bounds or not on a character boundary for a string, and this caused the codegen for `Formatter::pad` to be a bit larger than it otherwise needed to be. This commit replaces this with `s.get(..i).unwrap_or(&s)` which while having different behavior if `i` is out of bounds has a much smaller code footprint and otherwise avoids the need for `unsafe` code. --- src/libcore/fmt/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs index d55219d7226e6..277bef2bf661a 100644 --- a/src/libcore/fmt/mod.rs +++ b/src/libcore/fmt/mod.rs @@ -1212,7 +1212,11 @@ impl<'a> Formatter<'a> { // truncation. However other flags like `fill`, `width` and `align` // must act as always. if let Some((i, _)) = s.char_indices().skip(max).next() { - &s[..i] + // LLVM here can't prove that `..i` won't panic `&s[..i]`, but + // we know that it can't panic. Use `get` + `unwrap_or` to avoid + // `unsafe` and otherwise don't emit any panic-related code + // here. + s.get(..i).unwrap_or(&s) } else { &s } From 46d16b66e0b017430eb50b247926ea447c60ef07 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 6 Apr 2018 14:22:13 -0700 Subject: [PATCH 4/4] 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. --- src/libcore/panic.rs | 14 ++- src/libstd/panicking.rs | 118 ++++++++++++-------- src/test/run-make/wasm-panic-small/Makefile | 8 +- src/test/run-make/wasm-panic-small/foo.rs | 13 +++ 4 files changed, 103 insertions(+), 50 deletions(-) diff --git a/src/libcore/panic.rs b/src/libcore/panic.rs index 37c79f2fafe97..27ec4aaac75de 100644 --- 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 @@ 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); } diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 715fd45e490da..24eae6a4c821e 100644 --- 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(""); 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, + } + + 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(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 { - inner: Option, -} + rust_panic_with_hook(&mut PanicPayload::new(msg), None, file_line_col); -impl PanicPayload { - fn new(inner: A) -> PanicPayload { - PanicPayload { inner: Some(inner) } + struct PanicPayload { + inner: Option, } -} -unsafe impl BoxMeUp for PanicPayload { - fn box_me_up(&mut self) -> *mut (Any + Send) { - let data = match self.inner.take() { - Some(a) => Box::new(a) as Box, - None => Box::new(()), - }; - Box::into_raw(data) + impl PanicPayload { + fn new(inner: A) -> PanicPayload { + PanicPayload { inner: Some(inner) } + } } - fn get(&self) -> &(Any + Send) { - match self.inner { - Some(ref a) => a, - None => &(), + unsafe impl BoxMeUp for PanicPayload { + fn box_me_up(&mut self) -> *mut (Any + Send) { + let data = match self.inner.take() { + Some(a) => Box::new(a) as Box, + 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` 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 @@ pub fn update_count_then_panic(msg: Box) -> ! { 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 a11fba235957b..330ae300c445e 100644 --- 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 9654d5f7c0991..1ea724ca94d47 100644 --- 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"); +}