Auto merge of rust-lang#129582 - nbdd0121:unwind, r=nnethercote

Make destructors on `extern "C"` frames to be executed

This would make the example in rust-lang#123231 print "Noisy Drop". I didn't mark this as fixing the issue because the behaviour is yet to be spec'ed.

Tracking:

- rust-lang#74990

Author: bors

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);
             }

tests/mir-opt/asm_unwind_panic_abort.main.AbortUnwindingCalls.after.mir

@@ -7,12 +7,16 @@ 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: {
         StorageDead(_1);
         _0 = const ();
         return;
     }
+
+    bb2 (cleanup): {
+        terminate(abi);
+    }
 }

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));
     }

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() {}

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);
+    }
+}

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");
 }
 

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: