array index accesses are stable places

rust-lang · bors · Nov 15, 2018 · Nov 6, 2018 · Nov 6, 2018 · Nov 6, 2018
commit f174099885fed58c8053cf02bf353b6822897b75
diff --git a/src/librustc_mir/build/expr/as_place.rs b/src/librustc_mir/build/expr/as_place.rs
@@ -86,6 +86,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 // region_scope=None so place indexes live forever. They are scalars so they
                 // do not need storage annotations, and they are often copied between
                 // places.
+                // Making this a *fresh* temporary also means we do not have to worry about
+                // the index changing later: Nothing will ever change this temporary.
+                // The "retagging" transformation (for Stacked Borrows) relies on this.
                 let idx = unpack!(block = this.as_temp(block, None, index, Mutability::Mut));
 
                 // bounds check:

diff --git a/src/librustc_mir/transform/add_retag.rs b/src/librustc_mir/transform/add_retag.rs
@@ -38,17 +38,19 @@ fn is_stable<'tcx>(
         // Recurse for projections
         Projection(ref proj) => {
             match proj.elem {
-                ProjectionElem::Deref |
-                ProjectionElem::Index(_) =>
-                    // Which place these point to depends on external circumstances
-                    // (a local storing the array index, the current value of
-                    // the projection base), so we stop tracking here.
+                // Which place this evaluates to can change with any memory write,
+                // so cannot assume this to be stable.
+                ProjectionElem::Deref =>
                     false,
+                // Array indices are intersting, but MIR building generates a *fresh*
+                // temporary for every array access, so the index cannot be changed as
+                // a side-effect.
+                ProjectionElem::Index { .. } |
+                // The rest is completely boring, they just offset by a constant.
                 ProjectionElem::Field { .. } |
                 ProjectionElem::ConstantIndex { .. } |
                 ProjectionElem::Subslice { .. } |
                 ProjectionElem::Downcast { .. } =>
-                    // These just offset by a constant, entirely independent of everything else.
                     is_stable(&proj.base),
             }
         }

diff --git a/src/test/mir-opt/array-index-is-temporary.rs b/src/test/mir-opt/array-index-is-temporary.rs
@@ -0,0 +1,43 @@
+// Retagging (from Stacked Borrows) relies on the array index being a fresh
+// temporary, so that side-effects cannot change it.
+// Test that this is indeed the case.
+
+unsafe fn foo(z: *mut usize) -> u32 {
+    *z = 2;
+    99
+}
+
+fn main() {
+    let mut x = [42, 43, 44];
+    let mut y = 1;
+    let z: *mut usize = &mut y;
+    x[y] = unsafe { foo(z) };
+}
+
+// END RUST SOURCE
+// START rustc.main.EraseRegions.after.mir
+//     bb0: {
+//         ...
+//         _6 = &mut _2;
+//         _5 = &mut (*_6);
+//         _4 = move _5 as *mut usize (Misc);
+//         _3 = move _4;
+//         ...
+//         _8 = _3;
+//         _7 = const foo(move _8) -> bb1;
+//     }
+//
+//     bb1: {
+//         ...
+//         _9 = _2;
+//         _10 = Len(_1);
+//         _11 = Lt(_9, _10);
+//         assert(move _11, "index out of bounds: the len is move _10 but the index is _9") -> bb2;
+//     }
+//
+//     bb2: {
+//         _1[_9] = move _7;
+//         ...
+//         return;
+//     }
+// END rustc.main.EraseRegions.after.mir