Skip to content

Commit

Permalink
Rollup merge of #91737 - Manishearth:panic-immediate-stdlib, r=joshtr…
Browse files Browse the repository at this point in the history
…iplett

Make certain panicky stdlib functions behave better under panic_immediate_abort

The stdlib has a `panic_immediate_abort` feature that turns panics into immediate aborts, without any formatting/display logic. This feature was [introduced](#55011) primarily for codesize-constrained situations.

Unfortunately, this win doesn't quite propagate to `Result::expect()` and `Result::unwrap()`, while the formatting machinery is reduced, `expect()` and `unwrap()` both call `unwrap_failed("msg", &err)` which has a signature of `fn unwrap_failed(msg: &str, error: &dyn fmt::Debug)` and is `#[inline(never)]`. This means that `unwrap_failed` will unconditionally construct a `dyn Debug` trait object even though the object is never used in the function.

Constructing a trait object (even if you never call a method on it!) forces rust to include the vtable and any dependencies. This means that in `panic_immediate_abort` mode, calling expect/unwrap on a Result will pull in a whole bunch of formatting code for the error type even if it's completely unused.

This PR swaps out the function with one that won't require a trait object such that it won't force the inclusion of vtables in the code. It also gates off `#[inline(never)]` in a bunch of other places where allowing the inlining of an abort may be useful (this kind of thing is already done elsewhere in the stdlib).

I don't know how to write a test for this; we don't really seem to have any tests for `panic_immediate_abort` anyway so perhaps it's fine as is.
  • Loading branch information
matthiaskrgr authored Dec 11, 2021
2 parents 443ed7c + 917dafc commit 90eb610
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 6 deletions.
3 changes: 2 additions & 1 deletion library/core/src/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1671,7 +1671,8 @@ impl<T, E> Option<Result<T, E>> {
}

// This is a separate function to reduce the code size of .expect() itself.
#[inline(never)]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[cold]
#[track_caller]
const fn expect_failed(msg: &str) -> ! {
Expand Down
13 changes: 13 additions & 0 deletions library/core/src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1653,13 +1653,26 @@ impl<T> Result<T, T> {
}

// This is a separate function to reduce the code size of the methods
#[cfg(not(feature = "panic_immediate_abort"))]
#[inline(never)]
#[cold]
#[track_caller]
fn unwrap_failed(msg: &str, error: &dyn fmt::Debug) -> ! {
panic!("{}: {:?}", msg, error)
}

// This is a separate function to avoid constructing a `dyn Debug`
// that gets immediately thrown away, since vtables don't get cleaned up
// by dead code elimination if a trait object is constructed even if it goes
// unused
#[cfg(feature = "panic_immediate_abort")]
#[inline]
#[cold]
#[track_caller]
fn unwrap_failed<T>(_msg: &str, _error: &T) -> ! {
panic!()
}

/////////////////////////////////////////////////////////////////////////////
// Trait implementations
/////////////////////////////////////////////////////////////////////////////
Expand Down
15 changes: 10 additions & 5 deletions library/core/src/slice/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,35 +27,40 @@ where
}
}

#[inline(never)]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[cold]
#[track_caller]
fn slice_start_index_len_fail(index: usize, len: usize) -> ! {
panic!("range start index {} out of range for slice of length {}", index, len);
}

#[inline(never)]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[cold]
#[track_caller]
fn slice_end_index_len_fail(index: usize, len: usize) -> ! {
panic!("range end index {} out of range for slice of length {}", index, len);
}

#[inline(never)]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[cold]
#[track_caller]
fn slice_index_order_fail(index: usize, end: usize) -> ! {
panic!("slice index starts at {} but ends at {}", index, end);
}

#[inline(never)]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[cold]
#[track_caller]
fn slice_start_index_overflow_fail() -> ! {
panic!("attempted to index slice from after maximum usize");
}

#[inline(never)]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[cold]
#[track_caller]
fn slice_end_index_overflow_fail() -> ! {
Expand Down

0 comments on commit 90eb610

Please sign in to comment.