Skip to content

Slice index reconstruction rewrites only the bounds check's target block, so grid[i][j] and s[i] += 1 under overflow checks panic on the leftover (*slice)[index] place #322

Description

@coord-e

Summary

reconstruct_slice_indexing assumes every use of the indexed place (*slice)[index] sits in the bounds check's target block: find_slice_access searches only that block, and replace_indexed_place rewrites only that block. MIR at the default opt-level=0 regularly uses the same indexed place again in a later block of the same expression. That leftover Index projection then reaches the analyzer and panics. Two common shapes are affected:

  1. 2-D slices (&[&[T]], &mut [&mut [T]]): grid[i][j] re-reads grid[i] in the target of the inner bounds check.
  2. Compound assignment with overflow checks: s[i] += 1 reads s[i] in the bounds check's target, but writes it in the target of the overflow assert. Also, because the only use the pass sees is the read, it chooses Index::index, not IndexMut::index_mut. That would be wrong even if the write were rewritten too.

Neither is a missing feature. Both are ordinary slice indexing, which the pass exists to support. They fail only because of how the pass picks the blocks to rewrite. This is the same kind of problem as #319 (fixed in #320), where the pass removed locals that other code still used.

Reproduction

// thrust-rustc -C debug-assertions=off  (THRUST_SOLVER = PCSat wrapper)
fn diagonal(grid: &[&[i64]], i: usize, j: usize) -> i64 {
    grid[i][j]
}
fn main() {}
thread 'rustc' panicked at src/refine/env.rs:1087:22:
not implemented            // Path::from on ProjectionElem::Index
fn clear(grid: &mut [&mut [i64]], i: usize, j: usize) { grid[i][j] = 0; }
thread 'rustc' panicked at src/refine/env.rs:1041:55   // locate_place: "deref unbound var"
// thrust-rustc -C overflow-checks=on
fn incr(s: &mut [i64], i: usize) { s[i] += 1; }
thread 'rustc' panicked at src/analyze/basic_block.rs:1236:9:
assertion failed: place.projection.last() == Some(&mir::ProjectionElem::Deref)   // elaborate_place_for_borrow

(Without overflow-checks, s[i] += 1 is a single (*_1)[_3] = Add(copy (*_1)[_3], 1) statement in the target block, which is why the existing tests pass.)

Root cause

The MIR for grid[i][j] (after unelaborate_derefs folds the deref_copy temps away):

bb0: { _5 = PtrMetadata(copy _1); _6 = Lt(copy _4, copy _5);
       assert(move _6, BoundsCheck { len: _5, index: _4 }) -> bb1 }
bb1: { _10 = &raw const (fake) (*((*_1)[_4]));        // inner length, read through grid[i]
       _11 = PtrMetadata(move _10); _12 = Lt(copy _9, copy _11);
       assert(move _12, BoundsCheck { len: _11, index: _9 }) -> bb2 }
bb2: { _0 = copy (*((*_1)[_4]))[_9]; return; }          // grid[i] used again here

For the outer check (bb0 → bb1) the pass inserts _r = Index::index(_1, _4) and rewrites (*_1)[_4] to *_r in bb1 only. The (*((*_1)[_4]))[_9] in bb2 keeps its Index(_4). When the inner check (bb1 → bb2) is reconstructed, its receiver becomes (*_1)[_4], so the call it inserts passes an operand that still holds an Index projection, and Path::from hits its unimplemented!().

For s[i] += 1 with overflow checks:

bb0: { ... assert(move _6, BoundsCheck { len: _5, index: _3 }) -> bb1 }
bb1: { _7 = AddWithOverflow(copy (*_1)[_3], const 1_i32);
       assert(!move (_7.1), Overflow(Add, copy (*_1)[_3], const 1_i32)) -> bb2 }
bb2: { (*_1)[_3] = move (_7.0: i32); ... }

IndexedPlaceFinder sees only the read in bb1, so the pass inserts Index::index (a shared reference). bb2's write (*_1)[_3] = … is never rewritten.

A fix has to do two things:

  • rewrite every use that belongs to the same indexing expression, not just the ones in the target block;
  • choose index_mut when any of those uses mutates.

It must still stop at the next bounds check on the same slice and index, which starts a separate indexing expression. That check matters at opt-level >= 1: there GVN reuses the index local across let a = s[i]; s[i] = a + 1;, and the two expressions must keep separate borrows. The inner check of grid[i][i] uses the same index local but checks a different slice (its length is read through grid[i]), so it must not count as a stopping point.

Impact

Any function that indexes a slice of slices (grids, matrices, adjacency lists as &[&[usize]], rows passed as &mut [&mut [T]]) cannot be analyzed at all. Neither can any s[i] op= x once overflow checks are on, which is the default for cargo build / debug profiles and is now verified since #280.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions