fix(mir-importer): preserve non-terminating drops

cuda-oxide/crates/mir-importer/src/translator/terminator/drop_glue.rs

@@ -8,8 +8,8 @@
 //! This module provides two capabilities:
 //!
 //! 1. **No-op analysis** ([`drop_glue_is_noop`]): a conservative static proof
-//!    that dropping a value does nothing observable, allowing the `Drop`
-//!    terminator to be lowered as a plain branch (fast path).
+//!    that dropping a value does nothing observable and returns, allowing
+//!    the `Drop` terminator to be lowered as a plain branch (fast path).
 //!
 //! 2. **Drop glue emission** (`emit_drop_glue`): when the no-op proof fails,
 //!    emits a device-side call to the monomorphized `drop_in_place::<T>`
@@ -42,6 +42,11 @@
 //! Anything else (writes through pointers, asserts, inline asm, calls we
 //! cannot resolve or whose body we cannot see) fails the proof.
 //!
+//! The proof also requires the glue to return. A reachable control-flow
+//! cycle, or a call back into an instance that is already under proof,
+//! fails it even when every statement is harmless: erasing a call that
+//! may never return would let the code after the drop execute.
+//!
 //! The walk follows constant `switchInt` discriminants, so branches that
 //! the compiler has already folded shut (for example checks behind the
 //! `UbChecks` flag, which is off for device builds) do not have to be
@@ -320,16 +325,17 @@ fn compute_drop_place_address(
 }
 
 // ============================================================================
-// No-op analysis (unchanged from original)
+// Conservative no-op and termination analysis
 // ============================================================================
 
-/// Proves that calling `instance` does nothing observable.
+/// Proves that erasing a call to `instance` preserves observable behavior,
+/// including whether control can return to its caller.
 ///
 /// `in_progress` holds the mangled names of instances currently being
-/// proven further up the call stack. If we meet one of them again we
-/// treat the cycle as harmless: a cycle cannot *introduce* an
-/// observable effect, every effect would already have failed the proof
-/// on some statement or terminator along the way.
+/// proven further up the call stack. Meeting one again means termination
+/// is not proven: even recursion with no externally visible writes can
+/// diverge, and erasing such a call would make unreachable code execute.
+/// Recursion therefore fails this conservative proof.
 fn instance_is_noop(instance: &Instance, in_progress: &mut Vec<String>) -> bool {
     // Fast path: a type with no drop glue at all resolves to an "empty"
     // drop shim that exists only to fill vtable slots. rustc_public
@@ -344,7 +350,7 @@ fn instance_is_noop(instance: &Instance, in_progress: &mut Vec<String>) -> bool
 
     let name = instance.mangled_name();
     if in_progress.contains(&name) {
-        return true;
+        return false;
     }
 
     // No body means we cannot see what the call does (an intrinsic, a
@@ -359,102 +365,136 @@ fn instance_is_noop(instance: &Instance, in_progress: &mut Vec<String>) -> bool
     result
 }
 
-/// Walks every block of `body` reachable from the entry block and
-/// checks that nothing observable happens on the way to `return`.
+/// Walks every block of `body` reachable from the entry block and proves
+/// that execution is effect-free and contains no reachable control-flow cycle.
+///
+/// A cycle fails the proof even when every statement in it is locally harmless:
+/// removing a call that can diverge would change whether its continuation runs.
 fn body_is_noop(body: &mir::Body, in_progress: &mut Vec<String>) -> bool {
-    let mut visited = vec![false; body.blocks.len()];
-    let mut worklist: Vec<mir::BasicBlockIdx> = vec![0];
+    #[derive(Clone, Copy, PartialEq, Eq)]
+    enum BlockState {
+        Unseen,
+        InProgress,
+        Proven,
+    }
 
-    while let Some(idx) = worklist.pop() {
-        if std::mem::replace(&mut visited[idx], true) {
-            continue;
-        }
-        let block = &body.blocks[idx];
+    #[derive(Clone, Copy)]
+    enum WorkItem {
+        Enter(mir::BasicBlockIdx),
+        Finish(mir::BasicBlockIdx),
+    }
 
-        for stmt in &block.statements {
-            if !statement_is_noop(&stmt.kind) {
-                return false;
-            }
-        }
+    let mut states = vec![BlockState::Unseen; body.blocks.len()];
+    let mut worklist = vec![WorkItem::Enter(0)];
 
-        match &block.terminator.kind {
-            // Reaching `return` with only no-op work behind us is the
-            // success case. `unreachable` cannot execute in a valid
-            // program, so it cannot contribute an effect either.
-            mir::TerminatorKind::Return | mir::TerminatorKind::Unreachable => {}
-
-            mir::TerminatorKind::Goto { target } => worklist.push(*target),
-
-            mir::TerminatorKind::SwitchInt { discr, targets } => {
-                match const_operand_bits(discr) {
-                    // Known discriminant: only the matching branch can
-                    // run, so only that branch needs to be a no-op.
-                    // This is what skips the dead "really drop the
-                    // elements" branch in `IntoIter`'s destructor.
-                    Some(value) => {
-                        let target = targets
-                            .branches()
-                            .find(|(branch_value, _)| *branch_value == value)
-                            .map(|(_, target)| target)
-                            .unwrap_or_else(|| targets.otherwise());
-                        worklist.push(target);
-                    }
-                    // Unknown discriminant: every branch must be a
-                    // no-op.
-                    None => worklist.extend(targets.all_targets()),
-                }
+    while let Some(item) = worklist.pop() {
+        match item {
+            WorkItem::Finish(idx) => {
+                debug_assert!(matches!(states[idx], BlockState::InProgress));
+                states[idx] = BlockState::Proven;
+                continue;
             }
 
-            // A nested drop is fine when the dropped value's own glue
-            // passes this same proof.
-            mir::TerminatorKind::Drop { place, target, .. } => {
-                let Ok(place_ty) = place.ty(body.locals()) else {
-                    return false;
-                };
-                let nested = Instance::resolve_drop_in_place(place_ty);
-                if !instance_is_noop(&nested, in_progress) {
-                    return false;
+            WorkItem::Enter(idx) => {
+                match states[idx] {
+                    // A completed block reached through another predecessor is
+                    // ordinary CFG reconvergence.
+                    BlockState::Proven => continue,
+
+                    // A block already on the active DFS path is a reachable CFG
+                    // cycle. Effect-freedom does not prove that it terminates.
+                    BlockState::InProgress => return false,
+
+                    BlockState::Unseen => {}
                 }
-                worklist.push(*target);
-            }
 
-            // A call is fine when we can resolve exactly which function
-            // runs and that function passes this same proof. The
-            // `drop_in_place` shim for a type with an `impl Drop` is a
-            // single such call to `<T as Drop>::drop`.
-            mir::TerminatorKind::Call {
-                func,
-                destination,
-                target: Some(target),
-                ..
-            } => {
-                if place_writes_through_pointer(destination) {
-                    return false;
+                states[idx] = BlockState::InProgress;
+                let block = &body.blocks[idx];
+
+                for stmt in &block.statements {
+                    if !statement_is_noop(&stmt.kind) {
+                        return false;
+                    }
                 }
-                let Ok(func_ty) = func.ty(body.locals()) else {
-                    return false;
-                };
-                let TyKind::RigidTy(RigidTy::FnDef(def, args)) = func_ty.kind() else {
-                    // A function pointer or other indirect callee: we
-                    // do not know what runs.
-                    return false;
-                };
-                let Ok(callee) = Instance::resolve(def, &args) else {
-                    return false;
-                };
-                if !instance_is_noop(&callee, in_progress) {
-                    return false;
+
+                // Only mark this block Proven after every runtime-reachable
+                // successor has itself been proven.
+                worklist.push(WorkItem::Finish(idx));
+
+                match &block.terminator.kind {
+                    mir::TerminatorKind::Return | mir::TerminatorKind::Unreachable => {}
+
+                    mir::TerminatorKind::Goto { target } => {
+                        worklist.push(WorkItem::Enter(*target));
+                    }
+
+                    mir::TerminatorKind::SwitchInt { discr, targets } => {
+                        match const_operand_bits(discr) {
+                            Some(value) => {
+                                let target = targets
+                                    .branches()
+                                    .find(|(branch_value, _)| *branch_value == value)
+                                    .map(|(_, target)| target)
+                                    .unwrap_or_else(|| targets.otherwise());
+
+                                worklist.push(WorkItem::Enter(target));
+                            }
+
+                            None => {
+                                for target in targets.all_targets() {
+                                    worklist.push(WorkItem::Enter(target));
+                                }
+                            }
+                        }
+                    }
+
+                    mir::TerminatorKind::Drop { place, target, .. } => {
+                        let Ok(place_ty) = place.ty(body.locals()) else {
+                            return false;
+                        };
+
+                        let nested = Instance::resolve_drop_in_place(place_ty);
+                        if !instance_is_noop(&nested, in_progress) {
+                            return false;
+                        }
+
+                        worklist.push(WorkItem::Enter(*target));
+                    }
+
+                    mir::TerminatorKind::Call {
+                        func,
+                        destination,
+                        target: Some(target),
+                        ..
+                    } => {
+                        if place_writes_through_pointer(destination) {
+                            return false;
+                        }
+
+                        let Ok(func_ty) = func.ty(body.locals()) else {
+                            return false;
+                        };
+
+                        let TyKind::RigidTy(RigidTy::FnDef(def, args)) = func_ty.kind() else {
+                            return false;
+                        };
+
+                        let Ok(callee) = Instance::resolve(def, &args) else {
+                            return false;
+                        };
+
+                        if !instance_is_noop(&callee, in_progress) {
+                            return false;
+                        }
+
+                        worklist.push(WorkItem::Enter(*target));
+                    }
+
+                    // Asserts, diverging calls, inline asm, resume/abort, and
+                    // unknown terminators do not prove normal termination.
+                    _ => return false,
                 }
-                worklist.push(*target);
             }
-
-            // Everything else (asserts, diverging calls, inline asm,
-            // resume/abort) either has an effect or might not return.
-            // Asserts fail the proof deliberately: following only the
-            // success edge would silently delete a would-be device trap.
-            // A failed proof now emits the real drop_in_place call, so
-            // there is no pressure to widen the proof here.
-            _ => return false,
         }
     }
 

cuda-oxide/crates/rustc-codegen-cuda/STATUS.md

@@ -39,9 +39,10 @@ other device function; see the `drop_glue` example. This covers direct
 `impl Drop` types reachable from kernels. The shim and every
 `Drop::drop` body it reaches go through the same device pipeline, so
 drop glue whose MIR uses constructs the pipeline cannot yet translate
-(e.g. slice/`Vec` element drops, panic formatting) still fails
+(e.g. `Vec` element drops, panic formatting) still fails
 compilation with a diagnostic. Destructors are never silently skipped:
-a drop is either proven unobservable or its call is emitted.
+a drop is either proven unobservable and terminating, or its call is
+emitted.
 
 The remaining enum storage fixtures deliberately fail closed. Pointer and
 integer payloads cannot share one lowered slot without erasing LLVM pointer

cuda-oxide/crates/rustc-codegen-cuda/examples/drop_glue/README.md

@@ -42,6 +42,34 @@ distinguish a destructor that ran from one that was correctly suppressed.
 | 5 | `MaybeUninit::write` + `assume_init_drop` | `0xDEADBEEF` |
 | 6 | `MaybeUninit::assume_init_read` | `0xDEADBEEF` |
 
+## Compile-only drop proof probes
+
+Three additional kernels guard the proof that lets the compiler erase a
+destructor call: two against erasing a non-terminating destructor (#1360),
+one against rejecting a destructor the proof should accept.
+
+- `diverging_cfg_drop_probe`: the destructor contains `loop {}`.
+- `diverging_recursive_drop_probe`: the destructor calls an unconditionally
+  recursive helper.
+- `reconverging_drop_probe`: the destructor is an effect-free `if`/`else`
+  whose branches join back together. The proof must still accept it, so
+  the drop lowers to a plain branch.
+
+Each probe writes a sentinel before the drop and a different value after
+it. `verify-code-shape.sh` first asserts the importer's decision in the
+pre-opt `drop_glue.ll`: the two diverging probes keep a call to their drop
+glue in the kernel body, and the reconverging probe has none. It then
+checks the optimized PTX. For the diverging probes the pre-drop store
+remains and is immediately followed by an unconditional self-loop, while
+the unreachable post-drop store is absent. For the reconverging probe both
+stores remain. The PTX adjacency holds because `opt -O2` inlines the
+retained glue; `#[inline(never)]` is not propagated as `noinline`.
+
+These kernels are compiled but **never launched by `main`**. Do not launch
+the diverging probes manually: their destructors intentionally never
+return. The existing runtime checks above continue to exercise terminating
+destructors only.
+
 ## Usage
 
 Optimized:
@@ -56,6 +84,21 @@ Low MIR optimization:
 CUDA_OXIDE_NO_OPT=1 cargo oxide run drop_glue
 ```
 
+Optimized compile-only code checks, from the cuda-oxide repository root:
+
+```bash
+cargo oxide build drop_glue
+bash crates/rustc-codegen-cuda/examples/drop_glue/verify-code-shape.sh
+```
+
+The shape check targets optimized output; rebuild without
+`CUDA_OXIDE_NO_OPT=1` before running it. The existing smoketest runner also
+discovers and runs this script automatically:
+
+```bash
+scripts/smoketest.sh -o '^drop_glue$'
+```
+
 ## Expected output
 
 ```text