Skip to content

Commit

Permalink
Revert "Report allocation errors as panics"
Browse files Browse the repository at this point in the history
This reverts commit c9a6e41.
  • Loading branch information
matthiaskrgr committed Apr 24, 2023
1 parent f54dbe6 commit 23a3638
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 107 deletions.
3 changes: 0 additions & 3 deletions library/alloc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,3 @@ compiler-builtins-mem = ['compiler_builtins/mem']
compiler-builtins-c = ["compiler_builtins/c"]
compiler-builtins-no-asm = ["compiler_builtins/no-asm"]
compiler-builtins-mangled-names = ["compiler_builtins/mangled-names"]

# Make panics and failed asserts immediately abort without formatting any message
panic_immediate_abort = ["core/panic_immediate_abort"]
84 changes: 9 additions & 75 deletions library/alloc/src/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ use core::ptr::{self, NonNull};
#[doc(inline)]
pub use core::alloc::*;

#[cfg(not(no_global_oom_handling))]
use core::any::Any;
#[cfg(not(no_global_oom_handling))]
use core::panic::BoxMeUp;

#[cfg(test)]
mod tests;

Expand Down Expand Up @@ -348,77 +343,14 @@ pub(crate) unsafe fn box_free<T: ?Sized, A: Allocator>(ptr: Unique<T>, alloc: A)
}
}

/// Payload passed to the panic handler when `handle_alloc_error` is called.
#[unstable(feature = "panic_oom_payload", issue = "none")]
#[derive(Debug)]
pub struct AllocErrorPanicPayload {
layout: Layout,
}

impl AllocErrorPanicPayload {
/// Internal function for the standard library to clone a payload.
#[unstable(feature = "std_internals", issue = "none")]
#[doc(hidden)]
pub fn internal_clone(&self) -> Self {
AllocErrorPanicPayload { layout: self.layout }
}

/// Returns the [`Layout`] of the allocation attempt that caused the error.
#[unstable(feature = "panic_oom_payload", issue = "none")]
pub fn layout(&self) -> Layout {
self.layout
}
}

#[unstable(feature = "std_internals", issue = "none")]
#[cfg(not(no_global_oom_handling))]
unsafe impl BoxMeUp for AllocErrorPanicPayload {
fn take_box(&mut self) -> *mut (dyn Any + Send) {
use crate::boxed::Box;
Box::into_raw(Box::new(self.internal_clone()))
}

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

// # Allocation error handler

#[cfg(all(not(no_global_oom_handling), not(test)))]
fn rust_oom(layout: Layout) -> ! {
if cfg!(feature = "panic_immediate_abort") {
core::intrinsics::abort()
}

extern "Rust" {
// NOTE This function never crosses the FFI boundary; it's a Rust-to-Rust call
// that gets resolved to the `#[panic_handler]` function.
#[lang = "panic_impl"]
fn panic_impl(pi: &core::panic::PanicInfo<'_>) -> !;

// This symbol is emitted by rustc next to __rust_alloc_error_handler.
// Its value depends on the -Zoom={panic,abort} compiler option.
static __rust_alloc_error_handler_should_panic: u8;
}

// Hack to work around issues with the lifetime of Arguments.
match format_args!("memory allocation of {} bytes failed", layout.size()) {
fmt => {
// Create a PanicInfo with a custom payload for the panic handler.
let can_unwind = unsafe { __rust_alloc_error_handler_should_panic != 0 };
let mut pi = core::panic::PanicInfo::internal_constructor(
Some(&fmt),
core::panic::Location::caller(),
can_unwind,
);
let payload = AllocErrorPanicPayload { layout };
pi.set_payload(&payload);

// SAFETY: `panic_impl` is defined in safe Rust code and thus is safe to call.
unsafe { panic_impl(&pi) }
}
}
#[cfg(not(no_global_oom_handling))]
extern "Rust" {
// This is the magic symbol to call the global alloc error handler. rustc generates
// it to call `__rg_oom` if there is a `#[alloc_error_handler]`, or to call the
// default implementations below (`__rdl_oom`) otherwise.
fn __rust_alloc_error_handler(size: usize, align: usize) -> !;
}

/// Abort on memory allocation error or failure.
Expand All @@ -443,7 +375,9 @@ pub const fn handle_alloc_error(layout: Layout) -> ! {
}

fn rt_error(layout: Layout) -> ! {
rust_oom(layout);
unsafe {
__rust_alloc_error_handler(layout.size(), layout.align());
}
}

unsafe { core::intrinsics::const_eval_select((layout,), ct_error, rt_error) }
Expand Down
1 change: 0 additions & 1 deletion library/alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@
#![feature(maybe_uninit_slice)]
#![feature(maybe_uninit_uninit_array)]
#![feature(maybe_uninit_uninit_array_transpose)]
#![feature(panic_internals)]
#![feature(pattern)]
#![feature(pointer_byte_offsets)]
#![feature(provide_any)]
Expand Down
2 changes: 1 addition & 1 deletion library/std/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ llvm-libunwind = ["unwind/llvm-libunwind"]
system-llvm-libunwind = ["unwind/system-llvm-libunwind"]

# Make panics and failed asserts immediately abort without formatting any message
panic_immediate_abort = ["alloc/panic_immediate_abort"]
panic_immediate_abort = ["core/panic_immediate_abort"]

# Enable std_detect default features for stdarch/crates/std_detect:
# https://github.com/rust-lang/stdarch/blob/master/crates/std_detect/Cargo.toml
Expand Down
1 change: 0 additions & 1 deletion library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,6 @@
#![feature(get_mut_unchecked)]
#![feature(map_try_insert)]
#![feature(new_uninit)]
#![feature(panic_oom_payload)]
#![feature(slice_concat_trait)]
#![feature(thin_box)]
#![feature(try_reserve_kind)]
Expand Down
43 changes: 17 additions & 26 deletions library/std/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,24 +245,19 @@ fn default_hook(info: &PanicInfo<'_>) {

// The current implementation always returns `Some`.
let location = info.location().unwrap();

let msg = match info.payload().downcast_ref::<&'static str>() {
Some(s) => *s,
None => match info.payload().downcast_ref::<String>() {
Some(s) => &s[..],
None => "Box<dyn Any>",
},
};
let thread = thread_info::current_thread();
let name = thread.as_ref().and_then(|t| t.name()).unwrap_or("<unnamed>");

let write = |err: &mut dyn crate::io::Write| {
// Use the panic message directly if available, otherwise take it from
// the payload.
if let Some(msg) = info.message() {
let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}");
} else {
let msg = if let Some(s) = info.payload().downcast_ref::<&'static str>() {
*s
} else if let Some(s) = info.payload().downcast_ref::<String>() {
&s[..]
} else {
"Box<dyn Any>"
};
let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}");
}
let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}");

static FIRST_PANIC: AtomicBool = AtomicBool::new(true);

Expand Down Expand Up @@ -529,8 +524,6 @@ pub fn panicking() -> bool {
#[cfg(not(test))]
#[panic_handler]
pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
use alloc::alloc::AllocErrorPanicPayload;

struct PanicPayload<'a> {
inner: &'a fmt::Arguments<'a>,
string: Option<String>,
Expand All @@ -557,7 +550,8 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
unsafe impl<'a> BoxMeUp for PanicPayload<'a> {
fn take_box(&mut self) -> *mut (dyn Any + Send) {
// We do two allocations here, unfortunately. But (a) they're required with the current
// scheme, and (b) OOM uses its own separate payload type which doesn't allocate.
// scheme, and (b) we don't handle panic + OOM properly anyway (see comment in
// begin_panic below).
let contents = mem::take(self.fill());
Box::into_raw(Box::new(contents))
}
Expand All @@ -582,14 +576,7 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
let loc = info.location().unwrap(); // The current implementation always returns Some
let msg = info.message().unwrap(); // The current implementation always returns Some
crate::sys_common::backtrace::__rust_end_short_backtrace(move || {
if let Some(payload) = info.payload().downcast_ref::<AllocErrorPanicPayload>() {
rust_panic_with_hook(
&mut payload.internal_clone(),
info.message(),
loc,
info.can_unwind(),
);
} else if let Some(msg) = msg.as_str() {
if let Some(msg) = msg.as_str() {
rust_panic_with_hook(&mut StrPanicPayload(msg), info.message(), loc, info.can_unwind());
} else {
rust_panic_with_hook(
Expand Down Expand Up @@ -636,7 +623,11 @@ pub const fn begin_panic<M: Any + Send>(msg: M) -> ! {

unsafe impl<A: Send + 'static> BoxMeUp for PanicPayload<A> {
fn take_box(&mut self) -> *mut (dyn Any + Send) {
// Note that this should be the only allocation performed in this code path.
// Note that this should be the only allocation performed in this code path. Currently
// this means that panic!() on OOM will invoke this code path, but then again we're not
// really ready for panic on OOM anyway. If we do start doing this, then we should
// propagate this allocation to be performed in the parent of this thread instead of the
// thread that's panicking.
let data = match self.inner.take() {
Some(a) => Box::new(a) as Box<dyn Any + Send>,
None => process::abort(),
Expand Down

0 comments on commit 23a3638

Please sign in to comment.