Fix coroutine MIR saved local remapping by dingxiangfei2009 · Pull Request #158655 · rust-lang/rust

dingxiangfei2009 · 2026-07-01T12:32:15Z

The unsound analysis was found while I was working on the async-drop code. Apparently the counter variable in the array drop glue is not correctly identified as a saved local.

Also we did not patch up indices in ProjectionElem::Index when they are saved locals.

The reproduction is in the older commit with the expected output.

rustbot · 2026-07-01T12:32:18Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

rustbot · 2026-07-01T12:32:21Z

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 73 candidates
Random selection from 15 candidates

dingxiangfei2009 · 2026-07-01T12:32:47Z

r? cjgillot

who probably has more context

dingxiangfei2009 · 2026-07-03T16:15:45Z

@rustbot ready

cjgillot · 2026-07-03T21:22:16Z

+    fn visit_body(&mut self, body: &mut Body<'tcx>) {
+        self.super_body(body);
+    }
+


This is not useful any more

View changes since the review

cjgillot · 2026-07-03T22:19:22Z

+ 
+     bb33: {
+         StorageLive(_6);
+         _6 = async_drop_in_place::<[AsyncInt; 2]>(copy (_21.0: &mut [AsyncInt; 2])) -> [return: bb32, unwind: bb16];


The indexing does not appear in this file. Should we output the mir for StateTransform of async_drop_in_place::<[AsyncInt; 2]>?

View changes since the review

cjgillot · 2026-07-03T22:20:27Z

+            PlaceElem::Field(field, ty) => {
+                let mut new_ty = ty;
+                self.visit_ty(&mut new_ty, TyContext::Location(location));
+                if ty != new_ty { Some(PlaceElem::Field(field, new_ty)) } else { None }
+            }
+            PlaceElem::OpaqueCast(ty) => {
+                let mut new_ty = ty;
+                self.visit_ty(&mut new_ty, TyContext::Location(location));
+                if ty != new_ty { Some(PlaceElem::OpaqueCast(new_ty)) } else { None }
+            }
+            PlaceElem::UnwrapUnsafeBinder(ty) => {
+                let mut new_ty = ty;
+                self.visit_ty(&mut new_ty, TyContext::Location(location));
+                if ty != new_ty { Some(PlaceElem::UnwrapUnsafeBinder(new_ty)) } else { None }
+            }
+            PlaceElem::Deref


Suggested change

PlaceElem::Field(field, ty) => {

let mut new_ty = ty;

self.visit_ty(&mut new_ty, TyContext::Location(location));

if ty != new_ty { Some(PlaceElem::Field(field, new_ty)) } else { None }

}

PlaceElem::OpaqueCast(ty) => {

let mut new_ty = ty;

self.visit_ty(&mut new_ty, TyContext::Location(location));

if ty != new_ty { Some(PlaceElem::OpaqueCast(new_ty)) } else { None }

}

PlaceElem::UnwrapUnsafeBinder(ty) => {

let mut new_ty = ty;

self.visit_ty(&mut new_ty, TyContext::Location(location));

if ty != new_ty { Some(PlaceElem::UnwrapUnsafeBinder(new_ty)) } else { None }

}

PlaceElem::Deref

PlaceElem::Field(..)

| PlaceElem::OpaqueCast(..)

| PlaceElem::UnwrapUnsafeBinder(..)

| PlaceElem::Deref

View changes since the review

cjgillot · 2026-07-03T22:22:00Z

+                    None
+                } else {
+                    let mut new_local = local;
+                    self.visit_local(
+                        &mut new_local,
+                        PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy),
+                        location,
+                    );
+                    if new_local == local { None } else { Some(PlaceElem::Index(new_local)) }
+                }


Suggested change

None

} else {

let mut new_local = local;

self.visit_local(

&mut new_local,

PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy),

location,

);

if new_local == local { None } else { Some(PlaceElem::Index(new_local)) }

}

}

None

View changes since the review

cjgillot · 2026-07-03T22:22:23Z

+    ) -> Option<PlaceElem<'tcx>> {
+        match elem {
+            PlaceElem::Index(local) => {
+                if let Some(&Some((ty, variant, idx))) = self.remap.get(local) {


Do you mind adding a comment to explain what happens?

View changes since the review

cjgillot · 2026-07-03T22:24:03Z

+            | StatementKind::Intrinsic(..)
+            | StatementKind::BackwardIncompatibleDropHint { .. }
+            | StatementKind::StorageLive(..) => {}
+        }


A bit sad that we are duplicating code, but if #157943 lands, we will be able to use replace this file with its precise liveness analysis

View changes since the review

Signed-off-by: Xiangfei Ding <dingxiangfei2009@protonmail.ch>

Storage is still required even if the local is moved out and immediately moved in again. Signed-off-by: Xiangfei Ding <dingxiangfei2009@protonmail.ch>

rustbot · 2026-07-04T19:24:19Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

rust-log-analyzer · 2026-07-04T19:29:41Z

The job tidy failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[TIMING:end] tool::ToolBuild { build_compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu, forced_compiler: false }, target: x86_64-unknown-linux-gnu, tool: "tidy", path: "src/tools/tidy", mode: ToolBootstrap, source_type: InTree, extra_features: [], allow_features: "", cargo_args: [], artifact_kind: Binary } -- 60.326
[TIMING:end] tool::Tidy { compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu, forced_compiler: false }, target: x86_64-unknown-linux-gnu } -- 0.000
fmt check
Diff in /checkout/compiler/rustc_mir_transform/src/coroutine/mod.rs:68:
 use rustc_hir::{self as hir, CoroutineDesugaring, CoroutineKind};
 use rustc_index::bit_set::{BitMatrix, DenseBitSet, GrowableBitSet};
 use rustc_index::{Idx, IndexVec, indexvec};
-use rustc_middle::mir::visit::{
-    MutVisitor, MutatingUseContext, PlaceContext, Visitor,
-};
+use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
 use rustc_middle::mir::*;

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 1, 2026

rustbot assigned TaKO8Ki Jul 1, 2026

rustbot assigned cjgillot and unassigned TaKO8Ki Jul 1, 2026

dingxiangfei2009 requested a review from cjgillot July 1, 2026 12:33

cjgillot reviewed Jul 1, 2026

View reviewed changes

Comment thread compiler/rustc_mir_transform/src/coroutine/mod.rs Outdated

Comment thread compiler/rustc_mir_transform/src/coroutine/mod.rs Outdated

Comment thread tests/ui/async-await/async-drop/async-drop-initial.rs

cjgillot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 2, 2026

dingxiangfei2009 force-pushed the fix-remapping branch from f809a69 to fd23727 Compare July 3, 2026 07:55

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 3, 2026

cjgillot reviewed Jul 3, 2026

View reviewed changes

dingxiangfei2009 added 2 commits July 4, 2026 10:56

reproduction of miscompilation

9566dad

Signed-off-by: Xiangfei Ding <dingxiangfei2009@protonmail.ch>

fix remapping in indexing and self-assignments

aa485b2

Storage is still required even if the local is moved out and immediately moved in again. Signed-off-by: Xiangfei Ding <dingxiangfei2009@protonmail.ch>

dingxiangfei2009 force-pushed the fix-remapping branch from fd23727 to aa485b2 Compare July 4, 2026 19:24

Uh oh!

Uh oh!

Conversation

dingxiangfei2009 commented Jul 1, 2026

Uh oh!

rustbot commented Jul 1, 2026

Uh oh!

rustbot commented Jul 1, 2026

Uh oh!

dingxiangfei2009 commented Jul 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dingxiangfei2009 commented Jul 3, 2026

Uh oh!

cjgillot Jul 3, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjgillot Jul 3, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjgillot Jul 3, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjgillot Jul 3, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjgillot Jul 3, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjgillot Jul 3, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Jul 4, 2026

Uh oh!

rust-log-analyzer commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cjgillot Jul 3, 2026 •

edited by rustbot

Loading

cjgillot Jul 3, 2026 •

edited by rustbot

Loading

cjgillot Jul 3, 2026 •

edited by rustbot

Loading

cjgillot Jul 3, 2026 •

edited by rustbot

Loading

cjgillot Jul 3, 2026 •

edited by rustbot

Loading

cjgillot Jul 3, 2026 •

edited by rustbot

Loading