From 1598aa4f83d249bbef1c8593274eb09b1d58f6ba Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Sun, 25 Aug 2024 16:54:46 +0100 Subject: [PATCH 1/3] Make destructors on `extern "C"` frames to be executed --- .../src/abort_unwinding_calls.rs | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs b/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs index d889bc90c9d74..84c8a91b08252 100644 --- a/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs +++ b/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs @@ -51,11 +51,20 @@ impl<'tcx> crate::MirPass<'tcx> for AbortUnwindingCalls { // This will filter to functions with `extern "C-unwind"` ABIs, for // example. for block in body.basic_blocks.as_mut() { + let Some(terminator) = &mut block.terminator else { continue }; + let span = terminator.source_info.span; + + // If we see an `UnwindResume` terminator inside a function that cannot unwind, we need + // to replace it with `UnwindTerminate`. + if let TerminatorKind::UnwindResume = &terminator.kind + && !body_can_unwind + { + terminator.kind = TerminatorKind::UnwindTerminate(UnwindTerminateReason::Abi); + } + if block.is_cleanup { continue; } - let Some(terminator) = &block.terminator else { continue }; - let span = terminator.source_info.span; let call_can_unwind = match &terminator.kind { TerminatorKind::Call { func, .. } => { @@ -87,14 +96,18 @@ impl<'tcx> crate::MirPass<'tcx> for AbortUnwindingCalls { if !call_can_unwind { // If this function call can't unwind, then there's no need for it // to have a landing pad. This means that we can remove any cleanup - // registered for it. + // registered for it (and turn it into `UnwindAction::Unreachable`). let cleanup = block.terminator_mut().unwind_mut().unwrap(); *cleanup = UnwindAction::Unreachable; - } else if !body_can_unwind { + } else if !body_can_unwind + && matches!(terminator.unwind(), Some(UnwindAction::Continue)) + { // Otherwise if this function can unwind, then if the outer function // can also unwind there's nothing to do. If the outer function - // can't unwind, however, we need to change the landing pad for this - // function call to one that aborts. + // can't unwind, however, we need to ensure that any `UnwindAction::Continue` + // is replaced with terminate. For those with `UnwindAction::Cleanup`, + // cleanup will still happen, and terminate will happen afterwards handled by + // the `UnwindResume` -> `UnwindTerminate` terminator replacement. let cleanup = block.terminator_mut().unwind_mut().unwrap(); *cleanup = UnwindAction::Terminate(UnwindTerminateReason::Abi); } From 8f63b6a7454c05f512515946a0e9ca7e3b904144 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Wed, 28 Aug 2024 18:00:21 +0100 Subject: [PATCH 2/3] Add and adapt tests --- ...c_abort.main.AbortUnwindingCalls.after.mir | 6 +++- tests/mir-opt/asm_unwind_panic_abort.rs | 4 ++- tests/mir-opt/c_unwind_terminate.rs | 25 +++++++++++++ ...rminate.test.AbortUnwindingCalls.after.mir | 36 +++++++++++++++++++ tests/ui/panics/panic-in-ffi.rs | 9 +++++ tests/ui/panics/panic-in-ffi.run.stderr | 3 +- 6 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 tests/mir-opt/c_unwind_terminate.rs create mode 100644 tests/mir-opt/c_unwind_terminate.test.AbortUnwindingCalls.after.mir diff --git a/tests/mir-opt/asm_unwind_panic_abort.main.AbortUnwindingCalls.after.mir b/tests/mir-opt/asm_unwind_panic_abort.main.AbortUnwindingCalls.after.mir index 005b3ee3b249b..0ea9937e2a243 100644 --- a/tests/mir-opt/asm_unwind_panic_abort.main.AbortUnwindingCalls.after.mir +++ b/tests/mir-opt/asm_unwind_panic_abort.main.AbortUnwindingCalls.after.mir @@ -7,7 +7,7 @@ fn main() -> () { bb0: { StorageLive(_1); _1 = const (); - asm!("", options(MAY_UNWIND)) -> [return: bb1, unwind terminate(abi)]; + asm!("", options(MAY_UNWIND)) -> [return: bb1, unwind: bb2]; } bb1: { @@ -15,4 +15,8 @@ fn main() -> () { _0 = const (); return; } + + bb2 (cleanup): { + terminate(abi); + } } diff --git a/tests/mir-opt/asm_unwind_panic_abort.rs b/tests/mir-opt/asm_unwind_panic_abort.rs index fff6094212449..4ae76cbd16fe7 100644 --- a/tests/mir-opt/asm_unwind_panic_abort.rs +++ b/tests/mir-opt/asm_unwind_panic_abort.rs @@ -10,7 +10,9 @@ fn main() { // CHECK-LABEL: fn main( // CHECK: asm!( - // CHECK-SAME: unwind terminate(abi) + // CHECK-SAME: unwind: [[unwind:bb.*]]] + // CHECK: [[unwind]] (cleanup) + // CHECK-NEXT: terminate(abi) unsafe { std::arch::asm!("", options(may_unwind)); } diff --git a/tests/mir-opt/c_unwind_terminate.rs b/tests/mir-opt/c_unwind_terminate.rs new file mode 100644 index 0000000000000..64524e74d28e5 --- /dev/null +++ b/tests/mir-opt/c_unwind_terminate.rs @@ -0,0 +1,25 @@ +//@ needs-unwind + +struct Noise; +impl Drop for Noise { + fn drop(&mut self) { + eprintln!("Noisy Drop"); + } +} + +fn panic() { + panic!(); +} + +// EMIT_MIR c_unwind_terminate.test.AbortUnwindingCalls.after.mir +extern "C" fn test() { + // CHECK-LABEL: fn test( + // CHECK: drop + // CHECK-SAME: unwind: [[unwind:bb.*]]] + // CHECK: [[unwind]] (cleanup) + // CHECK-NEXT: terminate(abi) + let _val = Noise; + panic(); +} + +fn main() {} diff --git a/tests/mir-opt/c_unwind_terminate.test.AbortUnwindingCalls.after.mir b/tests/mir-opt/c_unwind_terminate.test.AbortUnwindingCalls.after.mir new file mode 100644 index 0000000000000..dd792d743cc5a --- /dev/null +++ b/tests/mir-opt/c_unwind_terminate.test.AbortUnwindingCalls.after.mir @@ -0,0 +1,36 @@ +// MIR for `test` after AbortUnwindingCalls + +fn test() -> () { + let mut _0: (); + let _1: Noise; + let _2: (); + scope 1 { + debug _val => _1; + } + + bb0: { + StorageLive(_1); + _1 = Noise; + StorageLive(_2); + _2 = panic() -> [return: bb1, unwind: bb3]; + } + + bb1: { + StorageDead(_2); + _0 = const (); + drop(_1) -> [return: bb2, unwind: bb4]; + } + + bb2: { + StorageDead(_1); + return; + } + + bb3 (cleanup): { + drop(_1) -> [return: bb4, unwind terminate(cleanup)]; + } + + bb4 (cleanup): { + terminate(abi); + } +} diff --git a/tests/ui/panics/panic-in-ffi.rs b/tests/ui/panics/panic-in-ffi.rs index 88f45f9a871d7..c0ae1899f4c28 100644 --- a/tests/ui/panics/panic-in-ffi.rs +++ b/tests/ui/panics/panic-in-ffi.rs @@ -2,13 +2,22 @@ //@ exec-env:RUST_BACKTRACE=0 //@ check-run-results //@ error-pattern: panic in a function that cannot unwind +//@ error-pattern: Noisy Drop //@ normalize-stderr-test: "\n +[0-9]+:[^\n]+" -> "" //@ normalize-stderr-test: "\n +at [^\n]+" -> "" //@ normalize-stderr-test: "(core/src/panicking\.rs):[0-9]+:[0-9]+" -> "$1:$$LINE:$$COL" //@ needs-unwind //@ ignore-emscripten "RuntimeError" junk in output +struct Noise; +impl Drop for Noise { + fn drop(&mut self) { + eprintln!("Noisy Drop"); + } +} + extern "C" fn panic_in_ffi() { + let _val = Noise; panic!("Test"); } diff --git a/tests/ui/panics/panic-in-ffi.run.stderr b/tests/ui/panics/panic-in-ffi.run.stderr index fc70847ad9a3a..58f5187f0daf5 100644 --- a/tests/ui/panics/panic-in-ffi.run.stderr +++ b/tests/ui/panics/panic-in-ffi.run.stderr @@ -1,6 +1,7 @@ -thread 'main' panicked at $DIR/panic-in-ffi.rs:12:5: +thread 'main' panicked at $DIR/panic-in-ffi.rs:21:5: Test note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace +Noisy Drop thread 'main' panicked at core/src/panicking.rs:$LINE:$COL: panic in a function that cannot unwind stack backtrace: From bb531083cc0301d2152dc59746eb30472564948d Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Thu, 10 Oct 2024 15:02:47 +0100 Subject: [PATCH 3/3] Fix longjmp-across-rust test Destructor are removed from stack because it's considered UB. --- tests/run-make/longjmp-across-rust/main.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/run-make/longjmp-across-rust/main.rs b/tests/run-make/longjmp-across-rust/main.rs index cc1d5b126dd35..0ebf11ac03c9d 100644 --- a/tests/run-make/longjmp-across-rust/main.rs +++ b/tests/run-make/longjmp-across-rust/main.rs @@ -10,19 +10,11 @@ fn main() { } } -struct A; - -impl Drop for A { - fn drop(&mut self) {} -} - extern "C" fn test_middle() { - let _a = A; foo(); } fn foo() { - let _a = A; unsafe { test_end(); }