From 364f9fd74c19303511868616ef7f7b64643c3e3d Mon Sep 17 00:00:00 2001 From: beepster4096 <19316085+beepster4096@users.noreply.github.com> Date: Fri, 7 Aug 2026 20:29:47 -0700 Subject: [PATCH 1/2] fix borrowck not considering sibling index and constantindex projections to be overlapping in some cases --- .../rustc_borrowck/src/diagnostics/mod.rs | 4 +- .../src/diagnostics/move_errors.rs | 4 +- compiler/rustc_borrowck/src/lib.rs | 47 +++++++++---------- .../src/polonius/legacy/accesses.rs | 2 +- .../src/drop_flag_effects.rs | 4 +- .../rustc_mir_dataflow/src/move_paths/mod.rs | 25 ++++++---- compiler/rustc_mir_dataflow/src/rustc_peek.rs | 2 +- .../src/elaborate_drops.rs | 8 ++-- .../src/lint_tail_expr_drop_order.rs | 2 +- .../ui/borrowck/index-after-constantindex.rs | 8 ++++ .../borrowck/index-after-constantindex.stderr | 18 +++++++ 11 files changed, 78 insertions(+), 46 deletions(-) create mode 100644 tests/ui/borrowck/index-after-constantindex.rs create mode 100644 tests/ui/borrowck/index-after-constantindex.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index bde2529d855cf..6a2cbf507bad2 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -557,7 +557,7 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> { // we'll use this to check whether it was originally from an overloaded // operator. match self.move_data.rev_lookup.find(deref_base) { - LookupResult::Exact(mpi) | LookupResult::Parent(Some(mpi)) => { + LookupResult::Exact(mpi) | LookupResult::Parent { mpi, .. } => { debug!("borrowed_content_source: mpi={:?}", mpi); for i in &self.move_data.init_path_map[mpi] { @@ -594,7 +594,7 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> { } } // Base is a `static` so won't be from an overloaded operator - _ => (), + LookupResult::None => (), }; // If we didn't find an overloaded deref or index, then assume it's a diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs index 9fac00016eac2..b70b4aa1fc4a1 100644 --- a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs @@ -189,7 +189,7 @@ impl<'diag, 'tcx> MirBorrowckCtxt<'_, 'diag, 'tcx> { match self.move_data.rev_lookup.find(match_place.as_ref()) { // Error with the match place - LookupResult::Parent(_) => { + LookupResult::Parent { .. } | LookupResult::None => { for ge in &mut *grouped_errors { if let GroupedMoveError::MovesFromPlace { span, binds_to, .. } = ge && match_span == *span @@ -219,7 +219,7 @@ impl<'diag, 'tcx> MirBorrowckCtxt<'_, 'diag, 'tcx> { } // Error with the pattern LookupResult::Exact(_) => { - let LookupResult::Parent(Some(mpi)) = + let LookupResult::Parent { mpi, .. } = self.move_data.rev_lookup.find(move_from.as_ref()) else { // move_from should be a projection from match_place. diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index d990d72e3fb42..6001a99a918e9 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -2072,15 +2072,33 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { // This code covers scenarios 1, 2, and 3. debug!("check_if_full_path_is_moved place: {:?}", place_span.0); - let (prefix, mpi) = self.move_path_closest_to(place_span.0); - if maybe_uninits.contains(mpi) { + + let uninit_mpi = match self.move_data.rev_lookup.find(place_span.0) { + // Index projections arbitrarily overlap sibling move paths, so we need to check all descendents of the parent + // Subslice and ConstantIndex projections of slices also overlap siblings, + // but the parent slice will never have a move path + // Subslice projections of arrays are specifically checked in `check_if_subslice_element_is_moved` + LookupResult::Parent { mpi, next_elem: PlaceElem::Index(..) } => self + .move_data + .find_in_move_path_or_its_descendants(mpi, |mpi| maybe_uninits.contains(mpi)), + + LookupResult::Exact(mpi) | LookupResult::Parent { mpi, next_elem: _ } => { + maybe_uninits.contains(mpi).then_some(mpi) + } + + LookupResult::None => bug!("should have move path for every Local"), + }; + + if let Some(mpi) = uninit_mpi { self.report_use_of_moved_or_uninitialized( location, desired_action, - (prefix, place_span.0, place_span.1), + (self.move_data.move_paths[mpi].place.as_ref(), place_span.0, place_span.1), mpi, ); - } // Only query longest prefix with a MovePath, not further + } + + // Only query longest prefix with a MovePath, not further // ancestors; dataflow recurs on children when parents // move (to support partial (re)inits). // @@ -2202,32 +2220,13 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { } } - /// Currently MoveData does not store entries for all places in - /// the input MIR. For example it will currently filter out - /// places that are Copy; thus we do not track places of shared - /// reference type. This routine will walk up a place along its - /// prefixes, searching for a foundational place that *is* - /// tracked in the MoveData. - /// - /// An Err result includes a tag indicated why the search failed. - /// Currently this can only occur if the place is built off of a - /// static variable, as we do not track those in the MoveData. - fn move_path_closest_to(&mut self, place: PlaceRef<'tcx>) -> (PlaceRef<'tcx>, MovePathIndex) { - match self.move_data.rev_lookup.find(place) { - LookupResult::Parent(Some(mpi)) | LookupResult::Exact(mpi) => { - (self.move_data.move_paths[mpi].place.as_ref(), mpi) - } - LookupResult::Parent(None) => panic!("should have move path for every Local"), - } - } - fn move_path_for_place(&mut self, place: PlaceRef<'tcx>) -> Option { // If returns None, then there is no move path corresponding // to a direct owner of `place` (which means there is nothing // that borrowck tracks for its analysis). match self.move_data.rev_lookup.find(place) { - LookupResult::Parent(_) => None, + LookupResult::Parent { .. } | LookupResult::None => None, LookupResult::Exact(mpi) => Some(mpi), } } diff --git a/compiler/rustc_borrowck/src/polonius/legacy/accesses.rs b/compiler/rustc_borrowck/src/polonius/legacy/accesses.rs index dc174775af2e5..49858232b3563 100644 --- a/compiler/rustc_borrowck/src/polonius/legacy/accesses.rs +++ b/compiler/rustc_borrowck/src/polonius/legacy/accesses.rs @@ -67,7 +67,7 @@ impl<'a, 'tcx> Visitor<'tcx> for AccessFactsExtractor<'a, 'tcx> { match context { PlaceContext::NonMutatingUse(_) | PlaceContext::MutatingUse(MutatingUseContext::Borrow) => { - let (LookupResult::Exact(path) | LookupResult::Parent(Some(path))) = + let (LookupResult::Exact(path) | LookupResult::Parent { mpi: path, .. }) = self.move_data.rev_lookup.find(place.as_ref()) else { // There's no path access to emit. diff --git a/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs b/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs index b6fc1219a8503..bfebc552ceed9 100644 --- a/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs +++ b/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs @@ -49,13 +49,13 @@ where pub fn on_lookup_result_bits<'tcx, F>( move_data: &MoveData<'tcx>, - lookup_result: LookupResult, + lookup_result: LookupResult<'tcx>, each_child: F, ) where F: FnMut(MovePathIndex), { match lookup_result { - LookupResult::Parent(..) => { + LookupResult::Parent { .. } | LookupResult::None => { // access to untracked value - do not touch children } LookupResult::Exact(e) => on_all_children_bits(move_data, e, each_child), diff --git a/compiler/rustc_mir_dataflow/src/move_paths/mod.rs b/compiler/rustc_mir_dataflow/src/move_paths/mod.rs index 83d40a5a2f284..a9b444e18c75e 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/mod.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/mod.rs @@ -340,15 +340,22 @@ pub struct MovePathLookup<'tcx> { mod builder; #[derive(Copy, Clone, Debug)] -pub enum LookupResult { +pub enum LookupResult<'tcx> { /// This exact thing has a move path. E.g. we looked up `x` or `x.m` and it has been moved. Exact(MovePathIndex), - /// - If the field is `None`, neither the exact thing nor any ancestor of it has a move path. - /// E.g. we looked up `x.m` and neither it nor `x` have a move path. - /// - If the field is `Some`, the exact thing has no move path, but an ancestor does. E.g. we - /// looked up `x.m` which has no move path but `x` has one. Not possible for locals. - Parent(Option), + /// The exact thing has no move path, but an ancestor does. + /// E.g. we looked up `x.m` which has no move path but `x` has one. Not possible for locals. + Parent { + mpi: MovePathIndex, + + /// The PlaceElem in the place immediately projecting from the parent move path. + next_elem: PlaceElem<'tcx>, + }, + + /// Neither the exact thing nor any ancestor of it has a move path. + /// E.g. we looked up `x.m` and neither it nor `x` have a move path. + None, } impl<'tcx> MovePathLookup<'tcx> { @@ -356,10 +363,10 @@ impl<'tcx> MovePathLookup<'tcx> { // alternative will *not* create a MovePath on the fly for an // unknown place, but will rather return the nearest available // parent. - pub fn find(&self, place: PlaceRef<'tcx>) -> LookupResult { + pub fn find(&self, place: PlaceRef<'tcx>) -> LookupResult<'tcx> { // Look first in the locals (roots). let Some(mut result) = self.find_local(place.local) else { - return LookupResult::Parent(None); + return LookupResult::None; }; // Look for a projection through the found local. @@ -372,7 +379,7 @@ impl<'tcx> MovePathLookup<'tcx> { }; let Some(&subpath) = subpath else { - return LookupResult::Parent(Some(result)); + return LookupResult::Parent { mpi: result, next_elem: elem }; }; result = subpath; } diff --git a/compiler/rustc_mir_dataflow/src/rustc_peek.rs b/compiler/rustc_mir_dataflow/src/rustc_peek.rs index 35c601f09acd4..9f6ae5013d18a 100644 --- a/compiler/rustc_mir_dataflow/src/rustc_peek.rs +++ b/compiler/rustc_mir_dataflow/src/rustc_peek.rs @@ -224,7 +224,7 @@ where } } - LookupResult::Parent(..) => { + LookupResult::Parent { .. } | LookupResult::None => { tcx.dcx().emit_err(PeekArgumentUntracked { span: call.span }); } } diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index 84c9c044ae6a6..8133274604182 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -314,8 +314,8 @@ impl<'a, 'tcx> ElaborateDropsCtxt<'a, 'tcx> { } }); } - LookupResult::Parent(None) => {} - LookupResult::Parent(Some(parent)) => { + LookupResult::None => {} + LookupResult::Parent { mpi: parent, .. } => { if self.body.local_decls[place.local].is_deref_temp() { continue; } @@ -387,8 +387,8 @@ impl<'a, 'tcx> ElaborateDropsCtxt<'a, 'tcx> { drop, ) } - LookupResult::Parent(None) => {} - LookupResult::Parent(Some(_)) => { + LookupResult::None => {} + LookupResult::Parent { .. } => { if !replace { self.tcx.dcx().span_bug( terminator.source_info.span, diff --git a/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs b/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs index fe09594d7c5f3..330855d66da5f 100644 --- a/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs +++ b/compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs @@ -76,7 +76,7 @@ impl<'a, 'mir, 'tcx> DropsReachable<'a, 'mir, 'tcx> { } MovePathIndexAtBlock::Unknown => { if let TerminatorKind::Drop { place, .. } = &terminator.kind - && let LookupResult::Exact(idx) | LookupResult::Parent(Some(idx)) = + && let LookupResult::Exact(idx) | LookupResult::Parent { mpi: idx, .. } = self.move_data.rev_lookup.find(place.as_ref()) { // Since we are working with MIRs at a very early stage, observing a `drop` diff --git a/tests/ui/borrowck/index-after-constantindex.rs b/tests/ui/borrowck/index-after-constantindex.rs new file mode 100644 index 0000000000000..5d78db4e411f5 --- /dev/null +++ b/tests/ui/borrowck/index-after-constantindex.rs @@ -0,0 +1,8 @@ +// test that an Index projection fails after a sibling ConstantIndex projection is moved out of +// regression test for #160525 + +fn main() { + let mut arr = [[Box::new(42)]]; + let alias = &mut arr[0][{ let [row] = arr; drop(row); 0 }]; //~ ERROR + println!("{}", **alias); // use-after-free of arr's dead stack slot +} diff --git a/tests/ui/borrowck/index-after-constantindex.stderr b/tests/ui/borrowck/index-after-constantindex.stderr new file mode 100644 index 0000000000000..d31a57df20a01 --- /dev/null +++ b/tests/ui/borrowck/index-after-constantindex.stderr @@ -0,0 +1,18 @@ +error[E0382]: borrow of moved value: `arr[..]` + --> $DIR/index-after-constantindex.rs:6:17 + | +LL | let alias = &mut arr[0][{ let [row] = arr; drop(row); 0 }]; + | ^^^^^^^^^^^^^^^^^^^---^^^^^^^^^^^^^^^^^^^^^^^^ + | | | + | | value moved here + | value borrowed here after move + | + = note: move occurs because `arr[..]` has type `[Box; 1]`, which does not implement the `Copy` trait +help: borrow this binding in the pattern to avoid moving the value + | +LL | let alias = &mut arr[0][{ let [ref row] = arr; drop(row); 0 }]; + | +++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0382`. From e2ab0373fa07a4f6b82893767f9a23849d3d31f8 Mon Sep 17 00:00:00 2001 From: beepster4096 <19316085+beepster4096@users.noreply.github.com> Date: Mon, 10 Aug 2026 17:06:55 -0700 Subject: [PATCH 2/2] use ProjectionKind instead of PlaceElem for LookupResult and match exhaustively on it --- compiler/rustc_borrowck/src/lib.rs | 17 +++++++++++++---- .../rustc_mir_dataflow/src/drop_flag_effects.rs | 2 +- .../rustc_mir_dataflow/src/move_paths/mod.rs | 10 +++++----- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 6001a99a918e9..bb9e42db5c3ca 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -2078,13 +2078,22 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { // Subslice and ConstantIndex projections of slices also overlap siblings, // but the parent slice will never have a move path // Subslice projections of arrays are specifically checked in `check_if_subslice_element_is_moved` - LookupResult::Parent { mpi, next_elem: PlaceElem::Index(..) } => self + LookupResult::Parent { mpi, next_elem: ProjectionKind::Index(..) } => self .move_data .find_in_move_path_or_its_descendants(mpi, |mpi| maybe_uninits.contains(mpi)), - LookupResult::Exact(mpi) | LookupResult::Parent { mpi, next_elem: _ } => { - maybe_uninits.contains(mpi).then_some(mpi) - } + LookupResult::Exact(mpi) + | LookupResult::Parent { + mpi, + next_elem: + ProjectionKind::Deref + | ProjectionKind::Field(..) + | ProjectionKind::ConstantIndex { .. } + | ProjectionKind::Subslice { .. } + | ProjectionKind::Downcast(..) + | ProjectionKind::OpaqueCast(..) + | ProjectionKind::UnwrapUnsafeBinder(..), + } => maybe_uninits.contains(mpi).then_some(mpi), LookupResult::None => bug!("should have move path for every Local"), }; diff --git a/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs b/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs index bfebc552ceed9..f4768e6f2569f 100644 --- a/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs +++ b/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs @@ -49,7 +49,7 @@ where pub fn on_lookup_result_bits<'tcx, F>( move_data: &MoveData<'tcx>, - lookup_result: LookupResult<'tcx>, + lookup_result: LookupResult, each_child: F, ) where F: FnMut(MovePathIndex), diff --git a/compiler/rustc_mir_dataflow/src/move_paths/mod.rs b/compiler/rustc_mir_dataflow/src/move_paths/mod.rs index a9b444e18c75e..c26fd8564e526 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/mod.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/mod.rs @@ -340,7 +340,7 @@ pub struct MovePathLookup<'tcx> { mod builder; #[derive(Copy, Clone, Debug)] -pub enum LookupResult<'tcx> { +pub enum LookupResult { /// This exact thing has a move path. E.g. we looked up `x` or `x.m` and it has been moved. Exact(MovePathIndex), @@ -349,8 +349,8 @@ pub enum LookupResult<'tcx> { Parent { mpi: MovePathIndex, - /// The PlaceElem in the place immediately projecting from the parent move path. - next_elem: PlaceElem<'tcx>, + /// The projection in the place immediately projecting from the parent move path. + next_elem: ProjectionKind, }, /// Neither the exact thing nor any ancestor of it has a move path. @@ -363,7 +363,7 @@ impl<'tcx> MovePathLookup<'tcx> { // alternative will *not* create a MovePath on the fly for an // unknown place, but will rather return the nearest available // parent. - pub fn find(&self, place: PlaceRef<'tcx>) -> LookupResult<'tcx> { + pub fn find(&self, place: PlaceRef<'tcx>) -> LookupResult { // Look first in the locals (roots). let Some(mut result) = self.find_local(place.local) else { return LookupResult::None; @@ -379,7 +379,7 @@ impl<'tcx> MovePathLookup<'tcx> { }; let Some(&subpath) = subpath else { - return LookupResult::Parent { mpi: result, next_elem: elem }; + return LookupResult::Parent { mpi: result, next_elem: elem.kind() }; }; result = subpath; }