Skip to content

Commit 4627bff

Browse files
committed
Show backtrace on allocation failures when possible
And if an allocation while printing the backtrace fails, don't try to print another backtrace as that will never succeed.
1 parent 42ec52b commit 4627bff

File tree

6 files changed

+136
-23
lines changed

6 files changed

+136
-23
lines changed

library/std/src/alloc.rs

Lines changed: 70 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
#![stable(feature = "alloc_module", since = "1.28.0")]
5858

5959
use core::ptr::NonNull;
60-
use core::sync::atomic::{Atomic, AtomicPtr, Ordering};
60+
use core::sync::atomic::{AtomicBool, AtomicPtr, Ordering};
6161
use core::{hint, mem, ptr};
6262

6363
#[stable(feature = "alloc_module", since = "1.28.0")]
@@ -287,7 +287,7 @@ unsafe impl Allocator for System {
287287
}
288288
}
289289

290-
static HOOK: Atomic<*mut ()> = AtomicPtr::new(ptr::null_mut());
290+
static HOOK: AtomicPtr<()> = AtomicPtr::new(ptr::null_mut());
291291

292292
/// Registers a custom allocation error hook, replacing any that was previously registered.
293293
///
@@ -344,7 +344,12 @@ pub fn take_alloc_error_hook() -> fn(Layout) {
344344
if hook.is_null() { default_alloc_error_hook } else { unsafe { mem::transmute(hook) } }
345345
}
346346

347+
#[optimize(size)]
347348
fn default_alloc_error_hook(layout: Layout) {
349+
if cfg!(panic = "immediate-abort") {
350+
return;
351+
}
352+
348353
unsafe extern "Rust" {
349354
// This symbol is emitted by rustc next to __rust_alloc_error_handler.
350355
// Its value depends on the -Zoom={panic,abort} compiler option.
@@ -354,28 +359,79 @@ fn default_alloc_error_hook(layout: Layout) {
354359

355360
if unsafe { __rust_alloc_error_handler_should_panic_v2() != 0 } {
356361
panic!("memory allocation of {} bytes failed", layout.size());
362+
}
363+
364+
// This is the default path taken on OOM, and the only path taken on stable with std.
365+
// Crucially, it does *not* call any user-defined code, and therefore users do not have to
366+
// worry about allocation failure causing reentrancy issues. That makes it different from
367+
// the default `__rdl_alloc_error_handler` defined in alloc (i.e., the default alloc error
368+
// handler that is called when there is no `#[alloc_error_handler]`), which triggers a
369+
// regular panic and thus can invoke a user-defined panic hook, executing arbitrary
370+
// user-defined code.
371+
372+
static PREV_ALLOC_FAILURE: AtomicBool = AtomicBool::new(false);
373+
if PREV_ALLOC_FAILURE.swap(true, Ordering::Relaxed) {
374+
// Don't try to print a backtrace if a previous alloc error happened. This likely means
375+
// there is not enough memory to print a backtrace, although it could also mean that two
376+
// threads concurrently run out of memory.
377+
rtprintpanic!(
378+
"memory allocation of {} bytes failed\nskipping backtrace printing to avoid potential recursion\n",
379+
layout.size()
380+
);
381+
return;
357382
} else {
358-
// This is the default path taken on OOM, and the only path taken on stable with std.
359-
// Crucially, it does *not* call any user-defined code, and therefore users do not have to
360-
// worry about allocation failure causing reentrancy issues. That makes it different from
361-
// the default `__rdl_alloc_error_handler` defined in alloc (i.e., the default alloc error
362-
// handler that is called when there is no `#[alloc_error_handler]`), which triggers a
363-
// regular panic and thus can invoke a user-defined panic hook, executing arbitrary
364-
// user-defined code.
365383
rtprintpanic!("memory allocation of {} bytes failed\n", layout.size());
366384
}
385+
386+
let Some(mut out) = crate::sys::stdio::panic_output() else {
387+
return;
388+
};
389+
390+
// Use a lock to prevent mixed output in multithreading context.
391+
// Some platforms also require it when printing a backtrace, like `SymFromAddr` on Windows.
392+
// Make sure to not take this lock until after checking PREV_ALLOC_FAILURE to avoid deadlocks
393+
// when there is too little memory to print a backtrace.
394+
let mut lock = crate::sys::backtrace::lock();
395+
396+
match crate::panic::get_backtrace_style() {
397+
Some(crate::panic::BacktraceStyle::Short) => {
398+
drop(lock.print(&mut out, crate::backtrace_rs::PrintFmt::Short))
399+
}
400+
Some(crate::panic::BacktraceStyle::Full) => {
401+
drop(lock.print(&mut out, crate::backtrace_rs::PrintFmt::Full))
402+
}
403+
Some(crate::panic::BacktraceStyle::Off) => {
404+
use crate::io::Write;
405+
let _ = writeln!(
406+
out,
407+
"note: run with `RUST_BACKTRACE=1` environment variable to display a \
408+
backtrace"
409+
);
410+
if cfg!(miri) {
411+
let _ = writeln!(
412+
out,
413+
"note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` \
414+
for the environment variable to have an effect"
415+
);
416+
}
417+
}
418+
// If backtraces aren't supported or are forced-off, do nothing.
419+
None => {}
420+
}
367421
}
368422

369423
#[cfg(not(test))]
370424
#[doc(hidden)]
371425
#[alloc_error_handler]
372426
#[unstable(feature = "alloc_internals", issue = "none")]
373427
pub fn rust_oom(layout: Layout) -> ! {
374-
let hook = HOOK.load(Ordering::Acquire);
375-
let hook: fn(Layout) =
376-
if hook.is_null() { default_alloc_error_hook } else { unsafe { mem::transmute(hook) } };
377-
hook(layout);
378-
crate::process::abort()
428+
crate::sys::backtrace::__rust_end_short_backtrace(|| {
429+
let hook = HOOK.load(Ordering::Acquire);
430+
let hook: fn(Layout) =
431+
if hook.is_null() { default_alloc_error_hook } else { unsafe { mem::transmute(hook) } };
432+
hook(layout);
433+
crate::process::abort()
434+
})
379435
}
380436

381437
#[cfg(not(test))]

library/std/src/panicking.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,6 @@ fn default_hook(info: &PanicHookInfo<'_>) {
285285
static FIRST_PANIC: Atomic<bool> = AtomicBool::new(true);
286286

287287
match backtrace {
288-
// SAFETY: we took out a lock just a second ago.
289288
Some(BacktraceStyle::Short) => {
290289
drop(lock.print(err, crate::backtrace_rs::PrintFmt::Short))
291290
}

library/std/src/sys/backtrace.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ pub(crate) fn lock<'a>() -> BacktraceLock<'a> {
2020

2121
impl BacktraceLock<'_> {
2222
/// Prints the current backtrace.
23-
///
24-
/// NOTE: this function is not Sync. The caller must hold a mutex lock, or there must be only one thread in the program.
2523
pub(crate) fn print(&mut self, w: &mut dyn Write, format: PrintFmt) -> io::Result<()> {
2624
// There are issues currently linking libbacktrace into tests, and in
2725
// general during std's own unit tests we're not testing this path. In
@@ -36,13 +34,17 @@ impl BacktraceLock<'_> {
3634
}
3735
impl fmt::Display for DisplayBacktrace {
3836
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
37+
// SAFETY: the backtrace lock is held
3938
unsafe { _print_fmt(fmt, self.format) }
4039
}
4140
}
4241
write!(w, "{}", DisplayBacktrace { format })
4342
}
4443
}
4544

45+
/// # Safety
46+
///
47+
/// This function is not Sync. The caller must hold a mutex lock, or there must be only one thread in the program.
4648
unsafe fn _print_fmt(fmt: &mut fmt::Formatter<'_>, print_fmt: PrintFmt) -> fmt::Result {
4749
// Always 'fail' to get the cwd when running under Miri -
4850
// this allows Miri to display backtraces in isolation mode

src/tools/miri/tests/fail/alloc/alloc_error_handler.stderr

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
memory allocation of 4 bytes failed
2+
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
3+
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
24
error: abnormal termination: the program aborted execution
35
--> RUSTLIB/std/src/alloc.rs:LL:CC
46
|
5-
LL | crate::process::abort()
6-
| ^^^^^^^^^^^^^^^^^^^^^^^ abnormal termination occurred here
7+
LL | crate::process::abort()
8+
| ^^^^^^^^^^^^^^^^^^^^^^^ abnormal termination occurred here
79
|
810
= note: BACKTRACE:
11+
= note: inside closure at RUSTLIB/std/src/alloc.rs:LL:CC
12+
= note: inside `std::sys::backtrace::__rust_end_short_backtrace::<{closure@std::alloc::rust_oom::{closure#0}}, !>` at RUSTLIB/std/src/sys/backtrace.rs:LL:CC
913
= note: inside `std::alloc::rust_oom` at RUSTLIB/std/src/alloc.rs:LL:CC
1014
= note: inside `std::alloc::_::__rust_alloc_error_handler` at RUSTLIB/std/src/alloc.rs:LL:CC
1115
= note: inside `std::alloc::handle_alloc_error::rt_error` at RUSTLIB/alloc/src/alloc.rs:LL:CC
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//@ run-pass
2+
// We disable tail merging here because it can't preserve debuginfo and thus
3+
// potentially breaks the backtraces. Also, subtle changes can decide whether
4+
// tail merging succeeds, so the test might work today but fail tomorrow due to a
5+
// seemingly completely unrelated change.
6+
// Unfortunately, LLVM has no "disable" option for this, so we have to set
7+
// "enable" to 0 instead.
8+
9+
//@ compile-flags:-g -Copt-level=0 -Cllvm-args=-enable-tail-merge=0
10+
//@ compile-flags:-Cforce-frame-pointers=yes
11+
//@ compile-flags:-Cstrip=none
12+
//@ ignore-android FIXME #17520
13+
//@ needs-subprocess
14+
//@ ignore-fuchsia Backtrace not symbolized, trace different line alignment
15+
//@ ignore-ios needs the `.dSYM` files to be moved to the device
16+
//@ ignore-tvos needs the `.dSYM` files to be moved to the device
17+
//@ ignore-watchos needs the `.dSYM` files to be moved to the device
18+
//@ ignore-visionos needs the `.dSYM` files to be moved to the device
19+
20+
// FIXME(#117097): backtrace (possibly unwinding mechanism) seems to be different on at least
21+
// `i686-mingw` (32-bit windows-gnu)? cc #128911.
22+
//@ ignore-windows-gnu
23+
//@ ignore-backends: gcc
24+
//@ ignore-msvc see #62897 and `backtrace-debuginfo.rs` test
25+
26+
use std::alloc::{Layout, handle_alloc_error};
27+
use std::process::Command;
28+
use std::{env, str};
29+
30+
fn main() {
31+
if env::args().len() > 1 {
32+
handle_alloc_error(Layout::new::<[u8; 42]>())
33+
}
34+
35+
let me = env::current_exe().unwrap();
36+
let output = Command::new(&me).env("RUST_BACKTRACE", "1").arg("next").output().unwrap();
37+
assert!(!output.status.success(), "{:?} is a success", output.status);
38+
39+
let mut stderr = str::from_utf8(&output.stderr).unwrap();
40+
41+
// When running inside QEMU user-mode emulation, there will be an extra message printed by QEMU
42+
// in the stderr whenever a core dump happens. Remove it before the check.
43+
stderr = stderr
44+
.strip_suffix("qemu: uncaught target signal 6 (Aborted) - core dumped\n")
45+
.unwrap_or(stderr);
46+
47+
assert!(stderr.contains("memory allocation of 42 bytes failed"), "{}", stderr);
48+
assert!(stderr.contains("alloc_error_backtrace::main"), "{}", stderr);
49+
}

tests/ui/alloc-error/default-alloc-error-hook.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,16 @@
22
//@ needs-subprocess
33

44
use std::alloc::{Layout, handle_alloc_error};
5-
use std::env;
65
use std::process::Command;
7-
use std::str;
6+
use std::{env, str};
87

98
fn main() {
109
if env::args().len() > 1 {
1110
handle_alloc_error(Layout::new::<[u8; 42]>())
1211
}
1312

1413
let me = env::current_exe().unwrap();
15-
let output = Command::new(&me).arg("next").output().unwrap();
14+
let output = Command::new(&me).env("RUST_BACKTRACE", "0").arg("next").output().unwrap();
1615
assert!(!output.status.success(), "{:?} is a success", output.status);
1716

1817
let mut stderr = str::from_utf8(&output.stderr).unwrap();
@@ -23,5 +22,9 @@ fn main() {
2322
.strip_suffix("qemu: uncaught target signal 6 (Aborted) - core dumped\n")
2423
.unwrap_or(stderr);
2524

26-
assert_eq!(stderr, "memory allocation of 42 bytes failed\n");
25+
assert_eq!(
26+
stderr,
27+
"memory allocation of 42 bytes failed\n\
28+
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\n"
29+
);
2730
}

0 commit comments

Comments
 (0)