about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2025-01-24 23:25:46 +0100
committerGitHub <noreply@github.com>2025-01-24 23:25:46 +0100
commit99e34a4ea0592d53496900ec9918018e16f67992 (patch)
treea1be987475e7a44f532b23e5de09558c17f8de13
parent8824ae6a6c14636a4937f89e9caee80da53a45dc (diff)
parent9b82f2026133642f884ff9f019edcf9eb270894b (diff)
downloadrust-99e34a4ea0592d53496900ec9918018e16f67992.tar.gz
rust-99e34a4ea0592d53496900ec9918018e16f67992.zip
Rollup merge of #135976 - WaffleLapkin:tailcall-nodrop, r=oli-obk
Don't drop types with no drop glue when building drops for tailcalls

this is required as otherwise drops of `&mut` refs count as a usage of a
'two-phase temporary' causing an ICE.

fixes #128097

The underlying issue is that the current code generates drops for `&mut` which are later counted as a second use of a two-phase temporary:

`bat t.rs -p`
```rust
#![expect(incomplete_features)]
#![feature(explicit_tail_calls)]

fn f(x: &mut ()) {
    let _y = String::new();
    become f(x);
}

fn main() {}
```
`rustc t.rs -Zdump_mir=f`
```text
error: internal compiler error: compiler/rustc_borrowck/src/borrow_set.rs:298:17: found two uses for 2-phase borrow temporary _4: bb2[1] and bb3[0]
 --> t.rs:6:5
  |
6 |     become f(x);
  |     ^^^^^^^^^^^

thread 'rustc' panicked at compiler/rustc_borrowck/src/borrow_set.rs:298:17:
Box<dyn Any>
stack backtrace:
[REDACTED]

error: aborting due to 1 previous error
```
`bat ./mir_dump/t.f.-------.renumber.0.mir -p -lrust`
```rust
// MIR for `f` 0 renumber

fn f(_1: &mut ()) -> () {
    debug x => _1;
    let mut _0: ();
    let mut _2: !;
    let _3: std::string::String;
    let mut _4: &mut ();
    scope 1 {
        debug _y => _3;
    }

    bb0: {
        StorageLive(_3);
        _3 = String::new() -> [return: bb1, unwind: bb4];
    }

    bb1: {
        FakeRead(ForLet(None), _3);
        StorageLive(_4);
        _4 = &mut (*_1);
        drop(_3) -> [return: bb2, unwind: bb3];
    }

    bb2: {
        StorageDead(_3);
        tailcall f(Spanned { node: move _4, span: t.rs:6:14: 6:15 (#0) });
    }

    bb3 (cleanup): {
        drop(_4) -> [return: bb4, unwind terminate(cleanup)];
    }

    bb4 (cleanup): {
        resume;
    }
}
```

Note how `_4 is moved into the tail call in `bb2` and dropped in `bb3`.

This PR adds a check that the locals we drop need dropping.

r? `@oli-obk` (feel free to reassign, I'm not sure who would be a good reviewer, but thought you might have an idea)
cc `@beepster4096,` since you wrote the original drop implementation.
-rw-r--r--compiler/rustc_mir_build/src/builder/scope.rs9
-rw-r--r--src/tools/miri/tests/fail/tail_calls/dangling-local-var.stderr4
-rw-r--r--tests/crashes/128097.rs6
-rw-r--r--tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-abort.diff1
-rw-r--r--tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-unwind.diff1
-rw-r--r--tests/mir-opt/tail_call_drops.f.built.after.panic-abort.mir1
-rw-r--r--tests/mir-opt/tail_call_drops.f.built.after.panic-unwind.mir1
-rw-r--r--tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-abort.diff1
-rw-r--r--tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-unwind.diff1
-rw-r--r--tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-abort.mir1
-rw-r--r--tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-unwind.mir1
-rw-r--r--tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.stderr7
-rw-r--r--tests/ui/explicit-tail-calls/two-phase.rs14
13 files changed, 28 insertions, 20 deletions
diff --git a/compiler/rustc_mir_build/src/builder/scope.rs b/compiler/rustc_mir_build/src/builder/scope.rs
index 20441530a47..945f02821d9 100644
--- a/compiler/rustc_mir_build/src/builder/scope.rs
+++ b/compiler/rustc_mir_build/src/builder/scope.rs
@@ -785,6 +785,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     let local =
                         place.as_local().unwrap_or_else(|| bug!("projection in tail call args"));
 
+                    if !self.local_decls[local].ty.needs_drop(self.tcx, self.typing_env()) {
+                        return None;
+                    }
+
                     Some(DropData { source_info, local, kind: DropKind::Value })
                 }
                 Operand::Constant(_) => None,
@@ -795,6 +799,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             self.scopes.scopes.iter().rev().nth(1).unwrap().region_scope,
             DUMMY_SP,
         );
+        let typing_env = self.typing_env();
         let unwind_drops = &mut self.scopes.unwind_drops;
 
         // the innermost scope contains only the destructors for the tail call arguments
@@ -805,6 +810,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let source_info = drop_data.source_info;
                 let local = drop_data.local;
 
+                if !self.local_decls[local].ty.needs_drop(self.tcx, typing_env) {
+                    continue;
+                }
+
                 match drop_data.kind {
                     DropKind::Value => {
                         // `unwind_to` should drop the value that we're about to
diff --git a/src/tools/miri/tests/fail/tail_calls/dangling-local-var.stderr b/src/tools/miri/tests/fail/tail_calls/dangling-local-var.stderr
index 6acd69ab3f8..33e1e53ea06 100644
--- a/src/tools/miri/tests/fail/tail_calls/dangling-local-var.stderr
+++ b/src/tools/miri/tests/fail/tail_calls/dangling-local-var.stderr
@@ -14,8 +14,8 @@ LL |     let local = 0;
 help: ALLOC was deallocated here:
   --> tests/fail/tail_calls/dangling-local-var.rs:LL:CC
    |
-LL | }
-   | ^
+LL |     become g(ptr)
+   |     ^^^^^^^^^^^^^
    = note: BACKTRACE (of the first span):
    = note: inside `g` at tests/fail/tail_calls/dangling-local-var.rs:LL:CC
 note: inside `main`
diff --git a/tests/crashes/128097.rs b/tests/crashes/128097.rs
deleted file mode 100644
index 6ffca640cbd..00000000000
--- a/tests/crashes/128097.rs
+++ /dev/null
@@ -1,6 +0,0 @@
-//@ known-bug: #128097
-#![feature(explicit_tail_calls)]
-fn f(x: &mut ()) {
-    let _y: String;
-    become f(x);
-}
diff --git a/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-abort.diff b/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-abort.diff
index 17c64d4baf0..9a4f27a497d 100644
--- a/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-abort.diff
+++ b/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-abort.diff
@@ -66,7 +66,6 @@
       bb6: {
 +         _8 = const false;
           StorageDead(_4);
-          StorageDead(_3);
           drop(_2) -> [return: bb7, unwind: bb12];
       }
   
diff --git a/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-unwind.diff b/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-unwind.diff
index 58d8a87986d..f13ee78aa36 100644
--- a/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-unwind.diff
+++ b/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-unwind.diff
@@ -66,7 +66,6 @@
       bb6: {
 +         _8 = const false;
           StorageDead(_4);
-          StorageDead(_3);
 -         drop(_2) -> [return: bb7, unwind continue];
 +         drop(_2) -> [return: bb7, unwind: bb12];
       }
diff --git a/tests/mir-opt/tail_call_drops.f.built.after.panic-abort.mir b/tests/mir-opt/tail_call_drops.f.built.after.panic-abort.mir
index 2c3d62491d7..e017424a4cc 100644
--- a/tests/mir-opt/tail_call_drops.f.built.after.panic-abort.mir
+++ b/tests/mir-opt/tail_call_drops.f.built.after.panic-abort.mir
@@ -63,7 +63,6 @@ fn f() -> () {
 
     bb6: {
         StorageDead(_4);
-        StorageDead(_3);
         drop(_2) -> [return: bb7, unwind: bb17];
     }
 
diff --git a/tests/mir-opt/tail_call_drops.f.built.after.panic-unwind.mir b/tests/mir-opt/tail_call_drops.f.built.after.panic-unwind.mir
index 2c3d62491d7..e017424a4cc 100644
--- a/tests/mir-opt/tail_call_drops.f.built.after.panic-unwind.mir
+++ b/tests/mir-opt/tail_call_drops.f.built.after.panic-unwind.mir
@@ -63,7 +63,6 @@ fn f() -> () {
 
     bb6: {
         StorageDead(_4);
-        StorageDead(_3);
         drop(_2) -> [return: bb7, unwind: bb17];
     }
 
diff --git a/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-abort.diff b/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-abort.diff
index 1a51601bc56..a8c57d2cfe0 100644
--- a/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-abort.diff
+++ b/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-abort.diff
@@ -80,7 +80,6 @@
       bb8: {
 +         _12 = const false;
           StorageDead(_6);
-          StorageDead(_5);
           drop(_4) -> [return: bb9, unwind: bb16];
       }
   
diff --git a/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-unwind.diff b/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-unwind.diff
index 1a51601bc56..a8c57d2cfe0 100644
--- a/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-unwind.diff
+++ b/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-unwind.diff
@@ -80,7 +80,6 @@
       bb8: {
 +         _12 = const false;
           StorageDead(_6);
-          StorageDead(_5);
           drop(_4) -> [return: bb9, unwind: bb16];
       }
   
diff --git a/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-abort.mir b/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-abort.mir
index 744f1989acc..f89b98a3205 100644
--- a/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-abort.mir
+++ b/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-abort.mir
@@ -77,7 +77,6 @@ fn f_with_arg(_1: String, _2: String) -> () {
 
     bb8: {
         StorageDead(_6);
-        StorageDead(_5);
         drop(_4) -> [return: bb9, unwind: bb23];
     }
 
diff --git a/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-unwind.mir b/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-unwind.mir
index 744f1989acc..f89b98a3205 100644
--- a/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-unwind.mir
+++ b/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-unwind.mir
@@ -77,7 +77,6 @@ fn f_with_arg(_1: String, _2: String) -> () {
 
     bb8: {
         StorageDead(_6);
-        StorageDead(_5);
         drop(_4) -> [return: bb9, unwind: bb23];
     }
 
diff --git a/tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.stderr b/tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.stderr
index 75fb13c378c..ece581dc626 100644
--- a/tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.stderr
+++ b/tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.stderr
@@ -4,10 +4,9 @@ error[E0597]: `local` does not live long enough
 LL |     let local = Type;
    |         ----- binding `local` declared here
 LL |     become takes_borrow(&local);
-   |                         ^^^^^^ borrowed value does not live long enough
-LL |
-LL | }
-   | - `local` dropped here while still borrowed
+   |                         ^^^^^^- `local` dropped here while still borrowed
+   |                         |
+   |                         borrowed value does not live long enough
 
 error: aborting due to 1 previous error
 
diff --git a/tests/ui/explicit-tail-calls/two-phase.rs b/tests/ui/explicit-tail-calls/two-phase.rs
new file mode 100644
index 00000000000..91365b5e739
--- /dev/null
+++ b/tests/ui/explicit-tail-calls/two-phase.rs
@@ -0,0 +1,14 @@
+// regression test for <https://github.com/rust-lang/rust/issues/112788>.
+// this test used to ICE because we tried to run drop glue of `x`
+// if dropping `_y` (happening at the `become` site) panicked and caused an unwind.
+//
+//@ check-pass
+#![expect(incomplete_features)]
+#![feature(explicit_tail_calls)]
+
+fn f(x: &mut ()) {
+    let _y = String::new();
+    become f(x);
+}
+
+fn main() {}