Skip to content

Commit c9a6e41

Browse files
committed
Report allocation errors as panics
1 parent 2816486 commit c9a6e41

File tree

6 files changed

+107
-27
lines changed

6 files changed

+107
-27
lines changed

Diff for: library/alloc/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,6 @@ compiler-builtins-mem = ['compiler_builtins/mem']
3535
compiler-builtins-c = ["compiler_builtins/c"]
3636
compiler-builtins-no-asm = ["compiler_builtins/no-asm"]
3737
compiler-builtins-mangled-names = ["compiler_builtins/mangled-names"]
38+
39+
# Make panics and failed asserts immediately abort without formatting any message
40+
panic_immediate_abort = ["core/panic_immediate_abort"]

Diff for: library/alloc/src/alloc.rs

+75-9
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ use core::ptr::{self, NonNull};
1414
#[doc(inline)]
1515
pub use core::alloc::*;
1616

17+
#[cfg(not(no_global_oom_handling))]
18+
use core::any::Any;
19+
#[cfg(not(no_global_oom_handling))]
20+
use core::panic::BoxMeUp;
21+
1722
#[cfg(test)]
1823
mod tests;
1924

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

346-
// # Allocation error handler
351+
/// Payload passed to the panic handler when `handle_alloc_error` is called.
352+
#[unstable(feature = "panic_oom_payload", issue = "none")]
353+
#[derive(Debug)]
354+
pub struct AllocErrorPanicPayload {
355+
layout: Layout,
356+
}
357+
358+
impl AllocErrorPanicPayload {
359+
/// Internal function for the standard library to clone a payload.
360+
#[unstable(feature = "std_internals", issue = "none")]
361+
#[doc(hidden)]
362+
pub fn internal_clone(&self) -> Self {
363+
AllocErrorPanicPayload { layout: self.layout }
364+
}
347365

366+
/// Returns the [`Layout`] of the allocation attempt that caused the error.
367+
#[unstable(feature = "panic_oom_payload", issue = "none")]
368+
pub fn layout(&self) -> Layout {
369+
self.layout
370+
}
371+
}
372+
373+
#[unstable(feature = "std_internals", issue = "none")]
348374
#[cfg(not(no_global_oom_handling))]
349-
extern "Rust" {
350-
// This is the magic symbol to call the global alloc error handler. rustc generates
351-
// it to call `__rg_oom` if there is a `#[alloc_error_handler]`, or to call the
352-
// default implementations below (`__rdl_oom`) otherwise.
353-
fn __rust_alloc_error_handler(size: usize, align: usize) -> !;
375+
unsafe impl BoxMeUp for AllocErrorPanicPayload {
376+
fn take_box(&mut self) -> *mut (dyn Any + Send) {
377+
use crate::boxed::Box;
378+
Box::into_raw(Box::new(self.internal_clone()))
379+
}
380+
381+
fn get(&mut self) -> &(dyn Any + Send) {
382+
self
383+
}
384+
}
385+
386+
// # Allocation error handler
387+
388+
#[cfg(all(not(no_global_oom_handling), not(test)))]
389+
fn rust_oom(layout: Layout) -> ! {
390+
if cfg!(feature = "panic_immediate_abort") {
391+
core::intrinsics::abort()
392+
}
393+
394+
extern "Rust" {
395+
// NOTE This function never crosses the FFI boundary; it's a Rust-to-Rust call
396+
// that gets resolved to the `#[panic_handler]` function.
397+
#[lang = "panic_impl"]
398+
fn panic_impl(pi: &core::panic::PanicInfo<'_>) -> !;
399+
400+
// This symbol is emitted by rustc next to __rust_alloc_error_handler.
401+
// Its value depends on the -Zoom={panic,abort} compiler option.
402+
static __rust_alloc_error_handler_should_panic: u8;
403+
}
404+
405+
// Hack to work around issues with the lifetime of Arguments.
406+
match format_args!("memory allocation of {} bytes failed", layout.size()) {
407+
fmt => {
408+
// Create a PanicInfo with a custom payload for the panic handler.
409+
let can_unwind = unsafe { __rust_alloc_error_handler_should_panic != 0 };
410+
let mut pi = core::panic::PanicInfo::internal_constructor(
411+
Some(&fmt),
412+
core::panic::Location::caller(),
413+
can_unwind,
414+
);
415+
let payload = AllocErrorPanicPayload { layout };
416+
pi.set_payload(&payload);
417+
418+
// SAFETY: `panic_impl` is defined in safe Rust code and thus is safe to call.
419+
unsafe { panic_impl(&pi) }
420+
}
421+
}
354422
}
355423

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

377445
fn rt_error(layout: Layout) -> ! {
378-
unsafe {
379-
__rust_alloc_error_handler(layout.size(), layout.align());
380-
}
446+
rust_oom(layout);
381447
}
382448

383449
unsafe { core::intrinsics::const_eval_select((layout,), ct_error, rt_error) }

Diff for: library/alloc/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@
136136
#![feature(maybe_uninit_slice)]
137137
#![feature(maybe_uninit_uninit_array)]
138138
#![feature(maybe_uninit_uninit_array_transpose)]
139+
#![feature(panic_internals)]
139140
#![feature(pattern)]
140141
#![feature(pointer_byte_offsets)]
141142
#![feature(provide_any)]

Diff for: library/std/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ llvm-libunwind = ["unwind/llvm-libunwind"]
6767
system-llvm-libunwind = ["unwind/system-llvm-libunwind"]
6868

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

7272
# Enable std_detect default features for stdarch/crates/std_detect:
7373
# https://github.com/rust-lang/stdarch/blob/master/crates/std_detect/Cargo.toml

Diff for: library/std/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,7 @@
319319
#![feature(get_mut_unchecked)]
320320
#![feature(map_try_insert)]
321321
#![feature(new_uninit)]
322+
#![feature(panic_oom_payload)]
322323
#![feature(slice_concat_trait)]
323324
#![feature(thin_box)]
324325
#![feature(try_reserve_kind)]

Diff for: library/std/src/panicking.rs

+26-17
Original file line numberDiff line numberDiff line change
@@ -245,19 +245,24 @@ fn default_hook(info: &PanicInfo<'_>) {
245245

246246
// The current implementation always returns `Some`.
247247
let location = info.location().unwrap();
248-
249-
let msg = match info.payload().downcast_ref::<&'static str>() {
250-
Some(s) => *s,
251-
None => match info.payload().downcast_ref::<String>() {
252-
Some(s) => &s[..],
253-
None => "Box<dyn Any>",
254-
},
255-
};
256248
let thread = thread_info::current_thread();
257249
let name = thread.as_ref().and_then(|t| t.name()).unwrap_or("<unnamed>");
258250

259251
let write = |err: &mut dyn crate::io::Write| {
260-
let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}");
252+
// Use the panic message directly if available, otherwise take it from
253+
// the payload.
254+
if let Some(msg) = info.message() {
255+
let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}");
256+
} else {
257+
let msg = if let Some(s) = info.payload().downcast_ref::<&'static str>() {
258+
*s
259+
} else if let Some(s) = info.payload().downcast_ref::<String>() {
260+
&s[..]
261+
} else {
262+
"Box<dyn Any>"
263+
};
264+
let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}");
265+
}
261266

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

@@ -524,6 +529,8 @@ pub fn panicking() -> bool {
524529
#[cfg(not(test))]
525530
#[panic_handler]
526531
pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
532+
use alloc::alloc::AllocErrorPanicPayload;
533+
527534
struct PanicPayload<'a> {
528535
inner: &'a fmt::Arguments<'a>,
529536
string: Option<String>,
@@ -550,8 +557,7 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
550557
unsafe impl<'a> BoxMeUp for PanicPayload<'a> {
551558
fn take_box(&mut self) -> *mut (dyn Any + Send) {
552559
// We do two allocations here, unfortunately. But (a) they're required with the current
553-
// scheme, and (b) we don't handle panic + OOM properly anyway (see comment in
554-
// begin_panic below).
560+
// scheme, and (b) OOM uses its own separate payload type which doesn't allocate.
555561
let contents = mem::take(self.fill());
556562
Box::into_raw(Box::new(contents))
557563
}
@@ -576,7 +582,14 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
576582
let loc = info.location().unwrap(); // The current implementation always returns Some
577583
let msg = info.message().unwrap(); // The current implementation always returns Some
578584
crate::sys_common::backtrace::__rust_end_short_backtrace(move || {
579-
if let Some(msg) = msg.as_str() {
585+
if let Some(payload) = info.payload().downcast_ref::<AllocErrorPanicPayload>() {
586+
rust_panic_with_hook(
587+
&mut payload.internal_clone(),
588+
info.message(),
589+
loc,
590+
info.can_unwind(),
591+
);
592+
} else if let Some(msg) = msg.as_str() {
580593
rust_panic_with_hook(&mut StrPanicPayload(msg), info.message(), loc, info.can_unwind());
581594
} else {
582595
rust_panic_with_hook(
@@ -623,11 +636,7 @@ pub const fn begin_panic<M: Any + Send>(msg: M) -> ! {
623636

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

0 commit comments

Comments
 (0)