From 9f7fe81d53b421e946c1b9ab35d43920428deb94 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 11 Jan 2025 10:35:00 +0100 Subject: [PATCH 1/2] use a single large catch_unwind in lang_start --- library/std/src/rt.rs | 36 +++++++++++-------- .../tests/fail/tail_calls/cc-mismatch.stderr | 10 ++++-- .../pass/backtrace/backtrace-api-v1.stderr | 6 +++- .../backtrace/backtrace-global-alloc.stderr | 14 ++++++-- .../tests/pass/backtrace/backtrace-std.stderr | 14 ++++++-- 5 files changed, 56 insertions(+), 24 deletions(-) diff --git a/library/std/src/rt.rs b/library/std/src/rt.rs index b2492238bd37b..334a57d372e0b 100644 --- a/library/std/src/rt.rs +++ b/library/std/src/rt.rs @@ -67,7 +67,7 @@ macro_rules! rtunwrap { }; } -fn handle_rt_panic(e: Box) { +fn handle_rt_panic(e: Box) -> T { mem::forget(e); rtabort!("initialization or cleanup bug"); } @@ -168,19 +168,27 @@ fn lang_start_internal( // panic is a std implementation bug. A quite likely one too, as there isn't any way to // prevent std from accidentally introducing a panic to these functions. Another is from // user code from `main` or, more nefariously, as described in e.g. issue #86030. - // SAFETY: Only called once during runtime initialization. - panic::catch_unwind(move || unsafe { init(argc, argv, sigpipe) }) - .unwrap_or_else(handle_rt_panic); - let ret_code = panic::catch_unwind(move || panic::catch_unwind(main).unwrap_or(101) as isize) - .map_err(move |e| { - mem::forget(e); - rtabort!("drop of the panic payload panicked"); - }); - panic::catch_unwind(cleanup).unwrap_or_else(handle_rt_panic); - // Guard against multiple threads calling `libc::exit` concurrently. - // See the documentation for `unique_thread_exit` for more information. - panic::catch_unwind(crate::sys::exit_guard::unique_thread_exit).unwrap_or_else(handle_rt_panic); - ret_code + // + // We use `catch_unwind` with `handle_rt_panic` instead of `abort_unwind` to make the error in + // case of a panic a bit nicer. + panic::catch_unwind(move || { + // SAFETY: Only called once during runtime initialization. + unsafe { init(argc, argv, sigpipe) }; + let ret_code = + panic::catch_unwind(move || panic::catch_unwind(main).unwrap_or(101) as isize).map_err( + move |e| { + // Print a specific error when we abort due to a panicing payload destructor. + mem::forget(e); + rtabort!("drop of the panic payload panicked"); + }, + ); + cleanup(); + // Guard against multiple threads calling `libc::exit` concurrently. + // See the documentation for `unique_thread_exit` for more information. + crate::sys::exit_guard::unique_thread_exit(); + ret_code + }) + .unwrap_or_else(handle_rt_panic) } #[cfg(not(any(test, doctest)))] diff --git a/src/tools/miri/tests/fail/tail_calls/cc-mismatch.stderr b/src/tools/miri/tests/fail/tail_calls/cc-mismatch.stderr index b416f0eb68976..4a0886cf897c9 100644 --- a/src/tools/miri/tests/fail/tail_calls/cc-mismatch.stderr +++ b/src/tools/miri/tests/fail/tail_calls/cc-mismatch.stderr @@ -15,9 +15,13 @@ LL | extern "rust-call" fn call_once(self, args: Args) -> Self::Output; = note: inside `std::panicking::r#try:: i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at RUSTLIB/std/src/panicking.rs:LL:CC = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at RUSTLIB/std/src/panic.rs:LL:CC = note: inside closure at RUSTLIB/std/src/rt.rs:LL:CC - = note: inside `std::panicking::r#try::do_call::<{closure@std::rt::lang_start_internal::{closure#1}}, isize>` at RUSTLIB/std/src/panicking.rs:LL:CC - = note: inside `std::panicking::r#try::` at RUSTLIB/std/src/panicking.rs:LL:CC - = note: inside `std::panic::catch_unwind::<{closure@std::rt::lang_start_internal::{closure#1}}, isize>` at RUSTLIB/std/src/panic.rs:LL:CC + = note: inside `std::panicking::r#try::do_call::<{closure@std::rt::lang_start_internal::{closure#0}::{closure#0}}, isize>` at RUSTLIB/std/src/panicking.rs:LL:CC + = note: inside `std::panicking::r#try::` at RUSTLIB/std/src/panicking.rs:LL:CC + = note: inside `std::panic::catch_unwind::<{closure@std::rt::lang_start_internal::{closure#0}::{closure#0}}, isize>` at RUSTLIB/std/src/panic.rs:LL:CC + = note: inside closure at RUSTLIB/std/src/rt.rs:LL:CC + = note: inside `std::panicking::r#try::do_call::<{closure@std::rt::lang_start_internal::{closure#0}}, std::result::Result>` at RUSTLIB/std/src/panicking.rs:LL:CC + = note: inside `std::panicking::r#try::, {closure@std::rt::lang_start_internal::{closure#0}}>` at RUSTLIB/std/src/panicking.rs:LL:CC + = note: inside `std::panic::catch_unwind::<{closure@std::rt::lang_start_internal::{closure#0}}, std::result::Result>` at RUSTLIB/std/src/panic.rs:LL:CC = note: inside `std::rt::lang_start_internal` at RUSTLIB/std/src/rt.rs:LL:CC = note: inside `std::rt::lang_start::<()>` at RUSTLIB/std/src/rt.rs:LL:CC diff --git a/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.stderr b/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.stderr index 48d9649c9201f..826645ba3df90 100644 --- a/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.stderr +++ b/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.stderr @@ -10,7 +10,11 @@ RUSTLIB/core/src/ops/function.rs:LL:CC (std::ops::function::impls::call_once) RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try::do_call) RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try) RUSTLIB/std/src/panic.rs:LL:CC (std::panic::catch_unwind) -RUSTLIB/std/src/rt.rs:LL:CC (std::rt::lang_start_internal::{closure#1}) +RUSTLIB/std/src/rt.rs:LL:CC (std::rt::lang_start_internal::{closure#0}::{closure#0}) +RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try::do_call) +RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try) +RUSTLIB/std/src/panic.rs:LL:CC (std::panic::catch_unwind) +RUSTLIB/std/src/rt.rs:LL:CC (std::rt::lang_start_internal::{closure#0}) RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try::do_call) RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try) RUSTLIB/std/src/panic.rs:LL:CC (std::panic::catch_unwind) diff --git a/src/tools/miri/tests/pass/backtrace/backtrace-global-alloc.stderr b/src/tools/miri/tests/pass/backtrace/backtrace-global-alloc.stderr index 667ee04e62413..30643bc6708a0 100644 --- a/src/tools/miri/tests/pass/backtrace/backtrace-global-alloc.stderr +++ b/src/tools/miri/tests/pass/backtrace/backtrace-global-alloc.stderr @@ -14,7 +14,7 @@ at RUSTLIB/std/src/panicking.rs:LL:CC 7: std::panic::catch_unwind at RUSTLIB/std/src/panic.rs:LL:CC - 8: std::rt::lang_start_internal::{closure#1} + 8: std::rt::lang_start_internal::{closure#0}::{closure#0} at RUSTLIB/std/src/rt.rs:LL:CC 9: std::panicking::r#try::do_call at RUSTLIB/std/src/panicking.rs:LL:CC @@ -22,7 +22,15 @@ at RUSTLIB/std/src/panicking.rs:LL:CC 11: std::panic::catch_unwind at RUSTLIB/std/src/panic.rs:LL:CC - 12: std::rt::lang_start_internal + 12: std::rt::lang_start_internal::{closure#0} at RUSTLIB/std/src/rt.rs:LL:CC - 13: std::rt::lang_start + 13: std::panicking::r#try::do_call + at RUSTLIB/std/src/panicking.rs:LL:CC + 14: std::panicking::r#try + at RUSTLIB/std/src/panicking.rs:LL:CC + 15: std::panic::catch_unwind + at RUSTLIB/std/src/panic.rs:LL:CC + 16: std::rt::lang_start_internal + at RUSTLIB/std/src/rt.rs:LL:CC + 17: std::rt::lang_start at RUSTLIB/std/src/rt.rs:LL:CC diff --git a/src/tools/miri/tests/pass/backtrace/backtrace-std.stderr b/src/tools/miri/tests/pass/backtrace/backtrace-std.stderr index b3a3a9d654a78..e04d2e561d7a6 100644 --- a/src/tools/miri/tests/pass/backtrace/backtrace-std.stderr +++ b/src/tools/miri/tests/pass/backtrace/backtrace-std.stderr @@ -22,7 +22,7 @@ at RUSTLIB/std/src/panicking.rs:LL:CC 11: std::panic::catch_unwind at RUSTLIB/std/src/panic.rs:LL:CC - 12: std::rt::lang_start_internal::{closure#1} + 12: std::rt::lang_start_internal::{closure#0}::{closure#0} at RUSTLIB/std/src/rt.rs:LL:CC 13: std::panicking::r#try::do_call at RUSTLIB/std/src/panicking.rs:LL:CC @@ -30,7 +30,15 @@ at RUSTLIB/std/src/panicking.rs:LL:CC 15: std::panic::catch_unwind at RUSTLIB/std/src/panic.rs:LL:CC - 16: std::rt::lang_start_internal + 16: std::rt::lang_start_internal::{closure#0} at RUSTLIB/std/src/rt.rs:LL:CC - 17: std::rt::lang_start + 17: std::panicking::r#try::do_call + at RUSTLIB/std/src/panicking.rs:LL:CC + 18: std::panicking::r#try + at RUSTLIB/std/src/panicking.rs:LL:CC + 19: std::panic::catch_unwind + at RUSTLIB/std/src/panic.rs:LL:CC + 20: std::rt::lang_start_internal + at RUSTLIB/std/src/rt.rs:LL:CC + 21: std::rt::lang_start at RUSTLIB/std/src/rt.rs:LL:CC From 471d8301066815613e4c468b9fbbf7286fb4cfe6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 11 Jan 2025 15:43:43 +0100 Subject: [PATCH 2/2] avoid nesting the user-defined main so deeply on the stack --- library/std/src/rt.rs | 29 +++++++++++-------- .../tests/fail/tail_calls/cc-mismatch.stderr | 10 ++----- .../pass/backtrace/backtrace-api-v1.stderr | 4 --- .../backtrace/backtrace-global-alloc.stderr | 14 ++------- .../tests/pass/backtrace/backtrace-std.stderr | 14 ++------- 5 files changed, 26 insertions(+), 45 deletions(-) diff --git a/library/std/src/rt.rs b/library/std/src/rt.rs index 334a57d372e0b..24a362072ab61 100644 --- a/library/std/src/rt.rs +++ b/library/std/src/rt.rs @@ -157,7 +157,7 @@ fn lang_start_internal( argc: isize, argv: *const *const u8, sigpipe: u8, -) -> Result { +) -> isize { // Guard against the code called by this function from unwinding outside of the Rust-controlled // code, which is UB. This is a requirement imposed by a combination of how the // `#[lang="start"]` attribute is implemented as well as by the implementation of the panicking @@ -174,18 +174,24 @@ fn lang_start_internal( panic::catch_unwind(move || { // SAFETY: Only called once during runtime initialization. unsafe { init(argc, argv, sigpipe) }; - let ret_code = - panic::catch_unwind(move || panic::catch_unwind(main).unwrap_or(101) as isize).map_err( - move |e| { - // Print a specific error when we abort due to a panicing payload destructor. - mem::forget(e); - rtabort!("drop of the panic payload panicked"); - }, - ); + + let ret_code = panic::catch_unwind(main).unwrap_or_else(move |payload| { + // Carefully dispose of the panic payload. + let payload = panic::AssertUnwindSafe(payload); + panic::catch_unwind(move || drop({ payload }.0)).unwrap_or_else(move |e| { + mem::forget(e); // do *not* drop the 2nd payload + rtabort!("drop of the panic payload panicked"); + }); + // Return error code for panicking programs. + 101 + }); + let ret_code = ret_code as isize; + cleanup(); // Guard against multiple threads calling `libc::exit` concurrently. // See the documentation for `unique_thread_exit` for more information. crate::sys::exit_guard::unique_thread_exit(); + ret_code }) .unwrap_or_else(handle_rt_panic) @@ -199,11 +205,10 @@ fn lang_start( argv: *const *const u8, sigpipe: u8, ) -> isize { - let Ok(v) = lang_start_internal( + lang_start_internal( &move || crate::sys::backtrace::__rust_begin_short_backtrace(main).report().to_i32(), argc, argv, sigpipe, - ); - v + ) } diff --git a/src/tools/miri/tests/fail/tail_calls/cc-mismatch.stderr b/src/tools/miri/tests/fail/tail_calls/cc-mismatch.stderr index 4a0886cf897c9..5061c9e8dc3f0 100644 --- a/src/tools/miri/tests/fail/tail_calls/cc-mismatch.stderr +++ b/src/tools/miri/tests/fail/tail_calls/cc-mismatch.stderr @@ -15,13 +15,9 @@ LL | extern "rust-call" fn call_once(self, args: Args) -> Self::Output; = note: inside `std::panicking::r#try:: i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at RUSTLIB/std/src/panicking.rs:LL:CC = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at RUSTLIB/std/src/panic.rs:LL:CC = note: inside closure at RUSTLIB/std/src/rt.rs:LL:CC - = note: inside `std::panicking::r#try::do_call::<{closure@std::rt::lang_start_internal::{closure#0}::{closure#0}}, isize>` at RUSTLIB/std/src/panicking.rs:LL:CC - = note: inside `std::panicking::r#try::` at RUSTLIB/std/src/panicking.rs:LL:CC - = note: inside `std::panic::catch_unwind::<{closure@std::rt::lang_start_internal::{closure#0}::{closure#0}}, isize>` at RUSTLIB/std/src/panic.rs:LL:CC - = note: inside closure at RUSTLIB/std/src/rt.rs:LL:CC - = note: inside `std::panicking::r#try::do_call::<{closure@std::rt::lang_start_internal::{closure#0}}, std::result::Result>` at RUSTLIB/std/src/panicking.rs:LL:CC - = note: inside `std::panicking::r#try::, {closure@std::rt::lang_start_internal::{closure#0}}>` at RUSTLIB/std/src/panicking.rs:LL:CC - = note: inside `std::panic::catch_unwind::<{closure@std::rt::lang_start_internal::{closure#0}}, std::result::Result>` at RUSTLIB/std/src/panic.rs:LL:CC + = note: inside `std::panicking::r#try::do_call::<{closure@std::rt::lang_start_internal::{closure#0}}, isize>` at RUSTLIB/std/src/panicking.rs:LL:CC + = note: inside `std::panicking::r#try::` at RUSTLIB/std/src/panicking.rs:LL:CC + = note: inside `std::panic::catch_unwind::<{closure@std::rt::lang_start_internal::{closure#0}}, isize>` at RUSTLIB/std/src/panic.rs:LL:CC = note: inside `std::rt::lang_start_internal` at RUSTLIB/std/src/rt.rs:LL:CC = note: inside `std::rt::lang_start::<()>` at RUSTLIB/std/src/rt.rs:LL:CC diff --git a/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.stderr b/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.stderr index 826645ba3df90..1ee5298f17d63 100644 --- a/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.stderr +++ b/src/tools/miri/tests/pass/backtrace/backtrace-api-v1.stderr @@ -10,10 +10,6 @@ RUSTLIB/core/src/ops/function.rs:LL:CC (std::ops::function::impls::call_once) RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try::do_call) RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try) RUSTLIB/std/src/panic.rs:LL:CC (std::panic::catch_unwind) -RUSTLIB/std/src/rt.rs:LL:CC (std::rt::lang_start_internal::{closure#0}::{closure#0}) -RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try::do_call) -RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try) -RUSTLIB/std/src/panic.rs:LL:CC (std::panic::catch_unwind) RUSTLIB/std/src/rt.rs:LL:CC (std::rt::lang_start_internal::{closure#0}) RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try::do_call) RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try) diff --git a/src/tools/miri/tests/pass/backtrace/backtrace-global-alloc.stderr b/src/tools/miri/tests/pass/backtrace/backtrace-global-alloc.stderr index 30643bc6708a0..26cdee18e3c23 100644 --- a/src/tools/miri/tests/pass/backtrace/backtrace-global-alloc.stderr +++ b/src/tools/miri/tests/pass/backtrace/backtrace-global-alloc.stderr @@ -14,7 +14,7 @@ at RUSTLIB/std/src/panicking.rs:LL:CC 7: std::panic::catch_unwind at RUSTLIB/std/src/panic.rs:LL:CC - 8: std::rt::lang_start_internal::{closure#0}::{closure#0} + 8: std::rt::lang_start_internal::{closure#0} at RUSTLIB/std/src/rt.rs:LL:CC 9: std::panicking::r#try::do_call at RUSTLIB/std/src/panicking.rs:LL:CC @@ -22,15 +22,7 @@ at RUSTLIB/std/src/panicking.rs:LL:CC 11: std::panic::catch_unwind at RUSTLIB/std/src/panic.rs:LL:CC - 12: std::rt::lang_start_internal::{closure#0} + 12: std::rt::lang_start_internal at RUSTLIB/std/src/rt.rs:LL:CC - 13: std::panicking::r#try::do_call - at RUSTLIB/std/src/panicking.rs:LL:CC - 14: std::panicking::r#try - at RUSTLIB/std/src/panicking.rs:LL:CC - 15: std::panic::catch_unwind - at RUSTLIB/std/src/panic.rs:LL:CC - 16: std::rt::lang_start_internal - at RUSTLIB/std/src/rt.rs:LL:CC - 17: std::rt::lang_start + 13: std::rt::lang_start at RUSTLIB/std/src/rt.rs:LL:CC diff --git a/src/tools/miri/tests/pass/backtrace/backtrace-std.stderr b/src/tools/miri/tests/pass/backtrace/backtrace-std.stderr index e04d2e561d7a6..d89ae3837b981 100644 --- a/src/tools/miri/tests/pass/backtrace/backtrace-std.stderr +++ b/src/tools/miri/tests/pass/backtrace/backtrace-std.stderr @@ -22,7 +22,7 @@ at RUSTLIB/std/src/panicking.rs:LL:CC 11: std::panic::catch_unwind at RUSTLIB/std/src/panic.rs:LL:CC - 12: std::rt::lang_start_internal::{closure#0}::{closure#0} + 12: std::rt::lang_start_internal::{closure#0} at RUSTLIB/std/src/rt.rs:LL:CC 13: std::panicking::r#try::do_call at RUSTLIB/std/src/panicking.rs:LL:CC @@ -30,15 +30,7 @@ at RUSTLIB/std/src/panicking.rs:LL:CC 15: std::panic::catch_unwind at RUSTLIB/std/src/panic.rs:LL:CC - 16: std::rt::lang_start_internal::{closure#0} + 16: std::rt::lang_start_internal at RUSTLIB/std/src/rt.rs:LL:CC - 17: std::panicking::r#try::do_call - at RUSTLIB/std/src/panicking.rs:LL:CC - 18: std::panicking::r#try - at RUSTLIB/std/src/panicking.rs:LL:CC - 19: std::panic::catch_unwind - at RUSTLIB/std/src/panic.rs:LL:CC - 20: std::rt::lang_start_internal - at RUSTLIB/std/src/rt.rs:LL:CC - 21: std::rt::lang_start + 17: std::rt::lang_start at RUSTLIB/std/src/rt.rs:LL:CC