Skip to content

Commit

Permalink
Unrolled build for rust-lang#135976
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#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 rust-lang#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.
  • Loading branch information
rust-timer authored Jan 25, 2025
2 parents 1e9b017 + 9b82f20 commit f66ac11
Show file tree
Hide file tree
Showing 13 changed files with 28 additions and 20 deletions.
9 changes: 9 additions & 0 deletions compiler/rustc_mir_build/src/builder/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
6 changes: 0 additions & 6 deletions tests/crashes/128097.rs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@
bb6: {
+ _8 = const false;
StorageDead(_4);
StorageDead(_3);
drop(_2) -> [return: bb7, unwind: bb12];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@
bb6: {
+ _8 = const false;
StorageDead(_4);
StorageDead(_3);
- drop(_2) -> [return: bb7, unwind continue];
+ drop(_2) -> [return: bb7, unwind: bb12];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ fn f() -> () {

bb6: {
StorageDead(_4);
StorageDead(_3);
drop(_2) -> [return: bb7, unwind: bb17];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ fn f() -> () {

bb6: {
StorageDead(_4);
StorageDead(_3);
drop(_2) -> [return: bb7, unwind: bb17];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@
bb8: {
+ _12 = const false;
StorageDead(_6);
StorageDead(_5);
drop(_4) -> [return: bb9, unwind: bb16];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@
bb8: {
+ _12 = const false;
StorageDead(_6);
StorageDead(_5);
drop(_4) -> [return: bb9, unwind: bb16];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ fn f_with_arg(_1: String, _2: String) -> () {

bb8: {
StorageDead(_6);
StorageDead(_5);
drop(_4) -> [return: bb9, unwind: bb23];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ fn f_with_arg(_1: String, _2: String) -> () {

bb8: {
StorageDead(_6);
StorageDead(_5);
drop(_4) -> [return: bb9, unwind: bb23];
}

Expand Down
7 changes: 3 additions & 4 deletions tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 14 additions & 0 deletions tests/ui/explicit-tail-calls/two-phase.rs
Original file line number Diff line number Diff line change
@@ -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() {}

0 comments on commit f66ac11

Please sign in to comment.