rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
94.82k stars 12.22k forks source link

`-Zoom=panic` double panic due to panicking allocating #126683

Open alecmocatta opened 2 weeks ago

alecmocatta commented 2 weeks ago

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.

https://github.com/rust-lang/rust/blob/a07f3eb43acc5df851e15176c7081a900a30a4d7/library/std/src/panicking.rs#L593-L615

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:

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

```text // target = wasm32-unknown-unknown std::panicking::rust_panic_with_hook::he5ab6644a2777339 at std::panicking::begin_panic_handler::{{closure}}::hbc8949711eb93f07 at std::sys_common::backtrace::__rust_end_short_backtrace::h43c6e6030cfac424 at src/sys_common/backtrace.rs rust_begin_unwind at core::panicking::panic_fmt::hd059f920eb86b234 at src/panicking.rs std::alloc::default_alloc_error_hook::hf29909cb78062dee at __rg_oom at src/alloc.rs __rust_alloc_error_handler at 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::reserve::do_reserve_and_handle::h010686df1ee1f672 at ::write_str::h78dc7b49e79e749d.23038 at lib/rustlib/src/rust/library/alloc/src/string.rs core::fmt::write::hebf5ad877658ea5c at ::get::h8680c05bc1165a6c at std::panicking::rust_panic_with_hook::he5ab6644a2777339 at std::panicking::begin_panic_handler::{{closure}}::hbc8949711eb93f07 at std::sys_common::backtrace::__rust_end_short_backtrace::h43c6e6030cfac424 at src/sys_common/backtrace.rs rust_begin_unwind at core::panicking::panic_fmt::hd059f920eb86b234 at src/panicking.rs std::alloc::default_alloc_error_hook::hf29909cb78062dee at __rg_oom at src/alloc.rs __rust_alloc_error_handler at alloc::alloc::handle_alloc_error::h77f60f5703455990 at src/alloc.rs as core::clone::Clone>::clone::h5d7cc23d7389cd6e at ```

veera-sivarajan commented 2 weeks ago

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

bjorn3 commented 2 weeks ago

Changing https://github.com/rust-lang/rust/blob/a07f3eb43acc5df851e15176c7081a900a30a4d7/library/std/src/alloc.rs#L354 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 https://github.com/rust-lang/rust/pull/112331 will do), this still has to be put in a Box when you start unwinding.

the8472 commented 2 weeks ago

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.