From cc491d60fef5dca4544a59706d08d3264f4c4627 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 21:41:49 +0000 Subject: [PATCH] Rewrite slice index uses past the bounds check's target block Slice index reconstruction replaced the indexed place only in the bounds check's target block, but MIR can use it again in a later block of the same expression: the inner bounds check of `grid[i][j]` re-reads `grid[i]` in its own target, and with overflow checks `s[i] += 1` writes `s[i]` in the target of the overflow assert. The untouched `(*slice)[index]` place then reached `Path::from` or `elaborate_place_for_borrow` and panicked, and the read-only first use selected `Index::index` even though the expression writes. Follow the blocks entered only from the target, up to the next bounds check on the same slice and index, rewrite the indexed place in all of them, and select `IndexMut::index_mut` when any of those uses mutates. A bounds check whose length is read through the indexed place (the inner check of `grid[i][i]`) does not end the expression. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019sxU2nknAtpn8CJu7xZeQt --- src/analyze/reconstruct_slice_indexing.rs | 139 ++++++++++++++---- ...slice_index_add_assign_overflow_checked.rs | 14 ++ tests/ui/fail/slice_index_nested.rs | 14 ++ ...slice_index_add_assign_overflow_checked.rs | 14 ++ tests/ui/pass/slice_index_nested.rs | 14 ++ 5 files changed, 165 insertions(+), 30 deletions(-) create mode 100644 tests/ui/fail/slice_index_add_assign_overflow_checked.rs create mode 100644 tests/ui/fail/slice_index_nested.rs create mode 100644 tests/ui/pass/slice_index_add_assign_overflow_checked.rs create mode 100644 tests/ui/pass/slice_index_nested.rs diff --git a/src/analyze/reconstruct_slice_indexing.rs b/src/analyze/reconstruct_slice_indexing.rs index dfe226df..02b3690c 100644 --- a/src/analyze/reconstruct_slice_indexing.rs +++ b/src/analyze/reconstruct_slice_indexing.rs @@ -24,8 +24,10 @@ struct BoundsCheck<'tcx> { source_info: mir::SourceInfo, } -/// The slice access in the target of a [`BoundsCheck`]. +/// The slice access following a [`BoundsCheck`]. struct SliceAccess<'tcx> { + /// The blocks that use the indexed place, starting from the bounds check's target. + blocks: Vec, indexed_place: mir::Place<'tcx>, receiver: mir::Place<'tcx>, receiver_mutability: mir_ty::Mutability, @@ -48,9 +50,6 @@ impl<'tcx> mir::visit::Visitor<'tcx> for IndexedPlaceFinder<'_, 'tcx> { context: mir::visit::PlaceContext, _location: mir::Location, ) { - if self.found.is_some() { - return; - } let Some(index_pos) = place .projection .iter() @@ -70,11 +69,16 @@ impl<'tcx> mir::visit::Visitor<'tcx> for IndexedPlaceFinder<'_, 'tcx> { .tcx .mk_place_elems(&place.projection.as_slice()[..index_pos]), }; - if matches!( + if !matches!( base.ty(&self.body.local_decls, self.tcx).ty.kind(), mir_ty::TyKind::Slice(_) ) { - self.found = Some((indexed, context.is_mutating_use())); + return; + } + match &mut self.found { + None => self.found = Some((indexed, context.is_mutating_use())), + Some((found, mutable)) if *found == indexed => *mutable |= context.is_mutating_use(), + Some(_) => {} } } } @@ -204,22 +208,101 @@ fn find_bounds_check<'tcx>(body: &Body<'tcx>, block: BasicBlock) -> Option( +/// Finds the first `(*slice)[index]` place used in `blocks`, and whether any of its uses there +/// is mutating. +fn find_indexed_place<'tcx>( tcx: TyCtxt<'tcx>, body: &Body<'tcx>, - bounds_check: &BoundsCheck<'tcx>, -) -> Option> { + index: Local, + blocks: &[BasicBlock], +) -> Option<(mir::Place<'tcx>, bool)> { use mir::visit::Visitor as _; let mut finder = IndexedPlaceFinder { body, tcx, - index: bounds_check.index_local, + index, found: None, }; - finder.visit_basic_block_data(bounds_check.target, &body.basic_blocks[bounds_check.target]); - let (indexed_place, mutable) = finder.found?; + for &block in blocks { + finder.visit_basic_block_data(block, &body.basic_blocks[block]); + } + finder.found +} + +/// Whether `bounds_check` checks the length of a slice reached through `indexed_place`, as the +/// inner bounds check of `slice[i][i]` does. +fn checks_slice_within<'tcx>( + body: &Body<'tcx>, + bounds_check: &BoundsCheck<'tcx>, + indexed_place: mir::Place<'tcx>, +) -> bool { + let statements = &body.basic_blocks[bounds_check.block].statements; + let assigned_rvalue = |local: Local| { + statements.iter().find_map(|statement| { + let (lhs, rvalue) = statement.kind.as_assign()?; + (lhs.as_local() == Some(local)).then_some(rvalue) + }) + }; + let Some(len) = bounds_check.len.place().and_then(|place| place.as_local()) else { + return false; + }; + let Some(Rvalue::UnaryOp(mir::UnOp::PtrMetadata, operand)) = assigned_rvalue(len) else { + return false; + }; + let Some(mut measured) = operand.place() else { + return false; + }; + if let Some(Rvalue::RawPtr(_, place)) = measured.as_local().and_then(assigned_rvalue) { + measured = *place; + } + measured.local == indexed_place.local + && measured + .projection + .as_slice() + .starts_with(indexed_place.projection.as_slice()) +} + +/// The blocks an indexing expression uses `indexed_place` in after its bounds check. +/// +/// Besides the bounds check's target, these are the blocks entered only from one of them, such +/// as the target of the overflow check in `slice[index] += 1` or of the inner bounds check in +/// `slice[i][j]`. Another bounds check on the same slice and index starts another indexing +/// expression. +fn blocks_using_indexed_place<'tcx>( + body: &Body<'tcx>, + bounds_check: &BoundsCheck<'tcx>, + indexed_place: mir::Place<'tcx>, +) -> Vec { + let mut blocks = vec![bounds_check.target]; + let mut visited = 0; + while let Some(&block) = blocks.get(visited) { + visited += 1; + if find_bounds_check(body, block).is_some_and(|next| { + next.index_local == bounds_check.index_local + && !checks_slice_within(body, &next, indexed_place) + }) { + continue; + } + for successor in body.basic_blocks[block].terminator().successors() { + if body.basic_blocks.predecessors()[successor].as_slice() == [block] { + blocks.push(successor); + } + } + } + blocks +} + +/// Finds the matching `(*slice)[index]` place in the blocks following the bounds check. +fn find_slice_access<'tcx>( + tcx: TyCtxt<'tcx>, + body: &Body<'tcx>, + bounds_check: &BoundsCheck<'tcx>, +) -> Option> { + let index = bounds_check.index_local; + let (indexed_place, _) = find_indexed_place(tcx, body, index, &[bounds_check.target])?; + let blocks = blocks_using_indexed_place(body, bounds_check, indexed_place); + let (indexed_place, mutable) = find_indexed_place(tcx, body, index, &blocks)?; // The indexed place has the shape `receiver.*[index]`; strip the dereference and index to // recover the reference passed to `Index::index` or `IndexMut::index_mut`. @@ -246,6 +329,7 @@ fn find_slice_access<'tcx>( } Some(SliceAccess { + blocks, indexed_place, receiver, receiver_mutability: *receiver_mutability, @@ -276,13 +360,7 @@ fn reconstruct_access<'tcx>( .local_decls .push(mir::LocalDecl::new(result_ty, bounds_check.source_info.span).immutable()); - replace_indexed_place( - body, - tcx, - bounds_check.target, - access.indexed_place, - result_local, - ); + replace_indexed_place(body, tcx, &access, result_local); let receiver = receiver_operand(tcx, body, &bounds_check, &access, region); let (lang_item, method_name) = if access.mutable { @@ -318,23 +396,24 @@ fn reconstruct_access<'tcx>( remove_bounds_check_setup(body, &bounds_check); } -/// Replaces every use of the indexed place in the target block with `*result_local`. +/// Replaces every use of the indexed place in the access's blocks with `*result_local`. fn replace_indexed_place<'tcx>( body: &mut Body<'tcx>, tcx: TyCtxt<'tcx>, - target: BasicBlock, - indexed_place: mir::Place<'tcx>, + access: &SliceAccess<'tcx>, result_local: Local, ) { let replacement = tcx.mk_place_deref(result_local.into()); let mut replacer = - analyze::ReplacePlacesVisitor::with_replacement(tcx, indexed_place, replacement); - let target_data = &mut body.basic_blocks.as_mut()[target]; - for statement in &mut target_data.statements { - replacer.visit_statement(statement); - } - if let Some(terminator) = &mut target_data.terminator { - replacer.visit_terminator(terminator); + analyze::ReplacePlacesVisitor::with_replacement(tcx, access.indexed_place, replacement); + for &block in &access.blocks { + let data = &mut body.basic_blocks.as_mut()[block]; + for statement in &mut data.statements { + replacer.visit_statement(statement); + } + if let Some(terminator) = &mut data.terminator { + replacer.visit_terminator(terminator); + } } } diff --git a/tests/ui/fail/slice_index_add_assign_overflow_checked.rs b/tests/ui/fail/slice_index_add_assign_overflow_checked.rs new file mode 100644 index 00000000..df31df72 --- /dev/null +++ b/tests/ui/fail/slice_index_add_assign_overflow_checked.rs @@ -0,0 +1,14 @@ +//@error-in-other-file: Unsat +//@compile-flags: -C overflow-checks=on +//@rustc-env: THRUST_SOLVER=tests/thrust-pcsat-wrapper + +fn incr(s: &mut [i64], i: usize) { + s[i] += 1; +} + +fn main() { + let mut a = [1, 2]; + incr(&mut a, 1); + let s: &[i64] = &a; + assert!(s[1] == 2); +} diff --git a/tests/ui/fail/slice_index_nested.rs b/tests/ui/fail/slice_index_nested.rs new file mode 100644 index 00000000..18ddf95a --- /dev/null +++ b/tests/ui/fail/slice_index_nested.rs @@ -0,0 +1,14 @@ +//@error-in-other-file: Unsat +//@compile-flags: -C debug-assertions=off +//@rustc-env: THRUST_SOLVER=tests/thrust-pcsat-wrapper + +fn diagonal(grid: &[&[i64]], i: usize) -> i64 { + grid[i][i] +} + +fn main() { + let row0 = [1, 2]; + let row1 = [3, 4]; + let rows: [&[i64]; 2] = [&row0, &row1]; + assert!(diagonal(&rows, 1) == 3); +} diff --git a/tests/ui/pass/slice_index_add_assign_overflow_checked.rs b/tests/ui/pass/slice_index_add_assign_overflow_checked.rs new file mode 100644 index 00000000..1f2c129f --- /dev/null +++ b/tests/ui/pass/slice_index_add_assign_overflow_checked.rs @@ -0,0 +1,14 @@ +//@check-pass +//@compile-flags: -C overflow-checks=on +//@rustc-env: THRUST_SOLVER=tests/thrust-pcsat-wrapper + +fn incr(s: &mut [i64], i: usize) { + s[i] += 1; +} + +fn main() { + let mut a = [1, 2]; + incr(&mut a, 1); + let s: &[i64] = &a; + assert!(s[1] == 3); +} diff --git a/tests/ui/pass/slice_index_nested.rs b/tests/ui/pass/slice_index_nested.rs new file mode 100644 index 00000000..ff088695 --- /dev/null +++ b/tests/ui/pass/slice_index_nested.rs @@ -0,0 +1,14 @@ +//@check-pass +//@compile-flags: -C debug-assertions=off +//@rustc-env: THRUST_SOLVER=tests/thrust-pcsat-wrapper + +fn diagonal(grid: &[&[i64]], i: usize) -> i64 { + grid[i][i] +} + +fn main() { + let row0 = [1, 2]; + let row1 = [3, 4]; + let rows: [&[i64]; 2] = [&row0, &row1]; + assert!(diagonal(&rows, 1) == 4); +}