-Zoom=panic double panic due to panicking allocating

[alecmocatta]

We had an abort in production from fn begin_panic_handler, specifically FormatStringPayload, allocating during an OoM panic, triggering a second OoM panic and thus abort.

rust/library/std/src/panicking.rs

Lines 593 to 615 in a07f3eb

	pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
	struct FormatStringPayload<'a> {
	inner: &'a fmt::Arguments<'a>,
	string: Option<String>,
	}
	impl<'a> FormatStringPayload<'a> {
	fn new(inner: &'a fmt::Arguments<'a>) -> Self {
	Self { inner, string: None }
	}
	fn fill(&mut self) -> &mut String {
	use crate::fmt::Write;
	let inner = self.inner;
	// Lazily, the first time this gets called, run the actual string formatting.
	self.string.get_or_insert_with(|| {
	let mut s = String::new();
	let _err = s.write_fmt(*inner);
	s
	})
	}
	}

From that file it looks like there may be other allocations also. Perhaps it's possible to avoid them on the OoM panic path.

cc:

• Tracking issue for oom=panic (RFC 2116) #43596
• Implement -Z oom=panic #88098
• Split core's PanicInfo and std's PanicInfo #115974
• Who is responsible for preventing reentrancy issues through the allocator? unsafe-code-guidelines#506

Meta

rustc --version --verbose:

rustc 1.79.0-nightly (a07f3eb43 2024-04-11)
binary: rustc
commit-hash: a07f3eb43acc5df851e15176c7081a900a30a4d7
commit-date: 2024-04-11
host: aarch64-apple-darwin
release: 1.79.0-nightly
LLVM version: 18.1.3

Backtrace

// target = wasm32-unknown-unknown

std::panicking::rust_panic_with_hook::he5ab6644a2777339
        at <unknown>
std::panicking::begin_panic_handler::{{closure}}::hbc8949711eb93f07
        at <unknown>
std::sys_common::backtrace::__rust_end_short_backtrace::h43c6e6030cfac424
        at src/sys_common/backtrace.rs
rust_begin_unwind
        at <unknown>
core::panicking::panic_fmt::hd059f920eb86b234
        at src/panicking.rs
std::alloc::default_alloc_error_hook::hf29909cb78062dee
        at <unknown>
__rg_oom
        at src/alloc.rs
__rust_alloc_error_handler
        at <unknown>
alloc::alloc::handle_alloc_error::h77f60f5703455990
        at src/alloc.rs
alloc::raw_vec::handle_error::h2651e03894002f2d
        at src/raw_vec.rs
alloc::raw_vec::RawVec<T,A>::reserve::do_reserve_and_handle::h010686df1ee1f672
        at <unknown>
<alloc::string::String as core::fmt::Write>::write_str::h78dc7b49e79e749d.23038
        at lib/rustlib/src/rust/library/alloc/src/string.rs
core::fmt::write::hebf5ad877658ea5c
        at <unknown>
<std::panicking::begin_panic_handler::FormatStringPayload as core::panic::PanicPayload>::get::h8680c05bc1165a6c
        at <unknown>
std::panicking::rust_panic_with_hook::he5ab6644a2777339
        at <unknown>
std::panicking::begin_panic_handler::{{closure}}::hbc8949711eb93f07
        at <unknown>
std::sys_common::backtrace::__rust_end_short_backtrace::h43c6e6030cfac424
        at src/sys_common/backtrace.rs
rust_begin_unwind
        at <unknown>
core::panicking::panic_fmt::hd059f920eb86b234
        at src/panicking.rs
std::alloc::default_alloc_error_hook::hf29909cb78062dee
        at <unknown>
__rg_oom
        at src/alloc.rs
__rust_alloc_error_handler
        at <unknown>
alloc::alloc::handle_alloc_error::h77f60f5703455990
        at src/alloc.rs
<generic_btree::map::binding::Binding<K,V> as core::clone::Clone>::clone::h5d7cc23d7389cd6e
        at <unknown>

[veera-sivarajan]

@rustbot label -needs-triage +T-compiler +A-panic

[bjorn3]

Changing

rust/library/std/src/alloc.rs

Line 354 in a07f3eb

panic!("memory allocation of {} bytes failed", layout.size());

to no longer include the allocation size should fix the double panic assuming, but that would be a usability regression. On nightly you could set your own alloc error hook which panics with a constant string yourself though. If you pass any extra argument to panic!() besides the format string itself, it will have to allocate a String to set as payload for the panic info passed to the panic hook. This is unavoidable. Even if you use a separate payload type for allocation errors (as #112331 will do), this still has to be put in a Box when you start unwinding.

[the8472]

If we had allocator hierarchies then the panic boxing could use an emergency allocator that is a child of the global allocator but keeps a reserve.