Restrict amount of ignored locals.

Author: cjgillot

compiler/rustc_middle/src/mir/mod.rs

@@ -902,6 +902,8 @@ pub enum LocalInfo<'tcx> {
     AggregateTemp,
     /// A temporary created during the pass `Derefer` to avoid it's retagging
     DerefTemp,
+    /// A temporary created for borrow checking.
+    FakeBorrow,
 }
 
 impl<'tcx> LocalDecl<'tcx> {

compiler/rustc_middle/src/mir/query.rs

@@ -140,8 +140,8 @@ pub struct GeneratorSavedTy<'tcx> {
     pub ty: Ty<'tcx>,
     /// Source info corresponding to the local in the original MIR body.
     pub source_info: SourceInfo,
-    /// Whether the local was introduced as a raw pointer to a static.
-    pub is_static_ptr: bool,
+    /// Whether the local should be ignored for trait bound computations.
+    pub ignore_for_traits: bool,
 }
 
 /// The layout of generator state.

compiler/rustc_middle/src/ty/util.rs

@@ -625,7 +625,7 @@ impl<'tcx> TyCtxt<'tcx> {
         generator_layout
             .field_tys
             .iter()
-            .filter(|decl| !decl.is_static_ptr)
+            .filter(|decl| !decl.ignore_for_traits)
             .map(|decl| ty::EarlyBinder(decl.ty))
     }
 

compiler/rustc_mir_build/src/build/matches/mod.rs

@@ -1749,6 +1749,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let fake_borrow_ty = tcx.mk_imm_ref(tcx.lifetimes.re_erased, fake_borrow_deref_ty);
                 let mut fake_borrow_temp = LocalDecl::new(fake_borrow_ty, temp_span);
                 fake_borrow_temp.internal = self.local_decls[matched_place.local].internal;
+                fake_borrow_temp.local_info = Some(Box::new(LocalInfo::FakeBorrow));
                 let fake_borrow_temp = self.local_decls.push(fake_borrow_temp);
 
                 (matched_place, fake_borrow_temp)

compiler/rustc_mir_transform/src/generator.rs

@@ -879,7 +879,7 @@ fn sanitize_witness<'tcx>(
 
     let mut mismatches = Vec::new();
     for fty in &layout.field_tys {
-        if fty.is_static_ptr {
+        if fty.ignore_for_traits {
             continue;
         }
         let decl_ty = tcx.normalize_erasing_regions(param_env, fty.ty);
@@ -904,6 +904,7 @@ fn sanitize_witness<'tcx>(
 }
 
 fn compute_layout<'tcx>(
+    tcx: TyCtxt<'tcx>,
     liveness: LivenessInfo,
     body: &Body<'tcx>,
 ) -> (
@@ -923,15 +924,33 @@ fn compute_layout<'tcx>(
     let mut locals = IndexVec::<GeneratorSavedLocal, _>::new();
     let mut tys = IndexVec::<GeneratorSavedLocal, _>::new();
     for (saved_local, local) in saved_locals.iter_enumerated() {
+        debug!("generator saved local {:?} => {:?}", saved_local, local);
+
         locals.push(local);
         let decl = &body.local_decls[local];
-        let decl = GeneratorSavedTy {
-            ty: decl.ty,
-            source_info: decl.source_info,
-            is_static_ptr: decl.internal,
+        debug!(?decl);
+
+        let ignore_for_traits = if tcx.sess.opts.unstable_opts.drop_tracking_mir {
+            match decl.local_info {
+                // Do not include raw pointers created from accessing `static` items, as those could
+                // well be re-created by another access to the same static.
+                Some(box LocalInfo::StaticRef { is_thread_local, .. }) => !is_thread_local,
+                // Fake borrows are only read by fake reads, so do not have any reality in
+                // post-analysis MIR.
+                Some(box LocalInfo::FakeBorrow) => true,
+                _ => false,
+            }
+        } else {
+            // FIXME(#105084) HIR-based drop tracking does not account for all the temporaries that
+            // MIR building may introduce. This leads to wrongly ignored types, but this is
+            // necessary for internal consistency and to avoid ICEs.
+            decl.internal
         };
+        let decl =
+            GeneratorSavedTy { ty: decl.ty, source_info: decl.source_info, ignore_for_traits };
+        debug!(?decl);
+
         tys.push(decl);
-        debug!("generator saved local {:?} => {:?}", saved_local, local);
     }
 
     // Leave empty variants for the UNRESUMED, RETURNED, and POISONED states.
@@ -1401,7 +1420,7 @@ pub(crate) fn mir_generator_witnesses<'tcx>(
     // Extract locals which are live across suspension point into `layout`
     // `remap` gives a mapping from local indices onto generator struct indices
     // `storage_liveness` tells us which locals have live storage at suspension points
-    let (_, generator_layout, _) = compute_layout(liveness_info, body);
+    let (_, generator_layout, _) = compute_layout(tcx, liveness_info, body);
 
     if tcx.sess.opts.unstable_opts.drop_tracking_mir {
         check_suspend_tys(tcx, &generator_layout, &body);
@@ -1503,7 +1522,7 @@ impl<'tcx> MirPass<'tcx> for StateTransform {
         // Extract locals which are live across suspension point into `layout`
         // `remap` gives a mapping from local indices onto generator struct indices
         // `storage_liveness` tells us which locals have live storage at suspension points
-        let (remap, layout, storage_liveness) = compute_layout(liveness_info, body);
+        let (remap, layout, storage_liveness) = compute_layout(tcx, liveness_info, body);
 
         let can_return = can_return(tcx, body, tcx.param_env(body.source.def_id()));
 
@@ -1700,7 +1719,7 @@ fn check_suspend_tys<'tcx>(tcx: TyCtxt<'tcx>, layout: &GeneratorLayout<'tcx>, bo
             let decl = &layout.field_tys[local];
             debug!(?decl);
 
-            if !decl.is_static_ptr && linted_tys.insert(decl.ty) {
+            if !decl.ignore_for_traits && linted_tys.insert(decl.ty) {
                 let Some(hir_id) = decl.source_info.scope.lint_root(&body.source_scopes) else { continue };
 
                 check_must_not_suspend_ty(

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -2389,7 +2389,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                 for &local in variant {
                     let decl = &generator_info.field_tys[local];
                     debug!(?decl);
-                    if ty_matches(ty::Binder::dummy(decl.ty)) && !decl.is_static_ptr {
+                    if ty_matches(ty::Binder::dummy(decl.ty)) && !decl.ignore_for_traits {
                         interior_or_upvar_span = Some(GeneratorInteriorOrUpvar::Interior(
                             decl.source_info.span,
                             Some((None, source_info.span, None, from_awaited_ty)),

tests/ui/generator/issue-105084.drop_tracking_mir.stderr

@@ -0,0 +1,51 @@
+error[E0382]: borrow of moved value: `g`
+  --> $DIR/issue-105084.rs:44:14
+   |
+LL |     let mut g = || {
+   |         ----- move occurs because `g` has type `[generator@$DIR/issue-105084.rs:22:17: 22:19]`, which does not implement the `Copy` trait
+...
+LL |     let mut h = copy(g);
+   |                      - value moved here
+...
+LL |     Pin::new(&mut g).resume(());
+   |              ^^^^^^ value borrowed here after move
+   |
+note: consider changing this parameter type in function `copy` to borrow instead if owning the value isn't necessary
+  --> $DIR/issue-105084.rs:17:21
+   |
+LL | fn copy<T: Copy>(x: T) -> T {
+   |    ----             ^ this parameter takes ownership of the value
+   |    |
+   |    in this function
+help: consider cloning the value if the performance cost is acceptable
+   |
+LL |     let mut h = copy(g.clone());
+   |                       ++++++++
+
+error[E0277]: the trait bound `Box<(i32, ())>: Copy` is not satisfied in `[generator@$DIR/issue-105084.rs:22:17: 22:19]`
+  --> $DIR/issue-105084.rs:38:17
+   |
+LL |     let mut g = || {
+   |                 -- within this `[generator@$DIR/issue-105084.rs:22:17: 22:19]`
+...
+LL |     let mut h = copy(g);
+   |                 ^^^^ within `[generator@$DIR/issue-105084.rs:22:17: 22:19]`, the trait `Copy` is not implemented for `Box<(i32, ())>`
+   |
+note: generator does not implement `Copy` as this value is used across a yield
+  --> $DIR/issue-105084.rs:28:25
+   |
+LL |         let t = box (5, yield);
+   |                 --------^^^^^-
+   |                 |       |
+   |                 |       yield occurs here, with `box (5, yield)` maybe used later
+   |                 has type `Box<(i32, ())>` which does not implement `Copy`
+note: required by a bound in `copy`
+  --> $DIR/issue-105084.rs:17:12
+   |
+LL | fn copy<T: Copy>(x: T) -> T {
+   |            ^^^^ required by this bound in `copy`
+
+error: aborting due to 2 previous errors
+
+Some errors have detailed explanations: E0277, E0382.
+For more information about an error, try `rustc --explain E0277`.

tests/ui/generator/issue-105084.rs

@@ -0,0 +1,49 @@
+// revisions: no_drop_tracking drop_tracking drop_tracking_mir
+// [drop_tracking] compile-flags: -Zdrop-tracking
+// [drop_tracking_mir] compile-flags: -Zdrop-tracking-mir
+// [no_drop_tracking] known-bug: #105084
+// [no_drop_tracking] check-pass
+// [drop_tracking] known-bug: #105084
+// [drop_tracking] check-pass
+
+#![feature(generators)]
+#![feature(generator_clone)]
+#![feature(generator_trait)]
+#![feature(box_syntax)]
+
+use std::ops::Generator;
+use std::pin::Pin;
+
+fn copy<T: Copy>(x: T) -> T {
+    x
+}
+
+fn main() {
+    let mut g = || {
+        // This is desuraged as 4 stages:
+        // - allocate a `*mut u8` with `exchange_malloc`;
+        // - create a Box that is ignored for trait computations;
+        // - compute fields (and yields);
+        // - assign to `t`.
+        let t = box (5, yield);
+        drop(t);
+    };
+
+    // Allocate the temporary box.
+    Pin::new(&mut g).resume(());
+
+    // The temporary box is in generator locals.
+    // As it is not taken into account for trait computation,
+    // the generator is `Copy`.
+    let mut h = copy(g);
+    //[drop_tracking_mir]~^ ERROR the trait bound `Box<(i32, ())>: Copy` is not satisfied in
+
+    // We now have 2 boxes with the same backing allocation:
+    // one inside `g` and one inside `h`.
+    // Proceed and drop `t` in `g`.
+    Pin::new(&mut g).resume(());
+    //[drop_tracking_mir]~^ ERROR borrow of moved value: `g`
+
+    // Proceed and drop `t` in `h` -> double free!
+    Pin::new(&mut h).resume(());
+}