Repository navigation
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
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:
- 2-D slices (
&[&[T]],&mut [&mut [T]]):grid[i][j]re-readsgrid[i]in the target of the inner bounds check. - Compound assignment with overflow checks:
s[i] += 1readss[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 choosesIndex::index, notIndexMut::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_mutwhen 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