From 732081829263fd02b6995ff815d4c001cce460cf Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 19 Oct 2019 21:00:21 +0100 Subject: [PATCH] Fix soundness issue with index bounds checks An expression like `x[1][{ x = y; 2}]` would perform the bounds check for the inner index operation before evaluating the outer index. This would allow out of bounds memory accesses. --- src/librustc/mir/mod.rs | 14 +- .../borrow_check/conflict_errors.rs | 154 ++++++--- src/librustc_mir/build/expr/as_place.rs | 303 +++++++++++++++--- src/librustc_mir/util/borrowck_errors.rs | 14 +- .../mir-opt/storage_live_dead_in_statics.rs | 4 +- .../mir-opt/uninhabited_enum_branching.rs | 18 +- .../slice-index-bounds-check-invalidation.rs | 82 +++++ ...ice-index-bounds-check-invalidation.stderr | 35 ++ 8 files changed, 500 insertions(+), 124 deletions(-) create mode 100644 src/test/ui/borrowck/slice-index-bounds-check-invalidation.rs create mode 100644 src/test/ui/borrowck/slice-index-bounds-check-invalidation.stderr diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index a3ddfec765f..fd2063e2da9 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1665,6 +1665,15 @@ pub enum FakeReadCause { /// Therefore, we insert a "fake read" here to ensure that we get /// appropriate errors. ForLet, + + /// If we have an index expression like + /// + /// (*x)[1][{ x = y; 4}] + /// + /// then the first bounds check is invalidated when we evaluate the second + /// index expression. Thus we create a fake borrow of `x` across the second + /// indexer, which will cause a borrow check error. + ForIndex, } #[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable)] @@ -1764,9 +1773,8 @@ pub enum StaticKind<'tcx> { def_id }); -#[derive( - Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable, HashStable, -)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(RustcEncodable, RustcDecodable, HashStable)] pub enum ProjectionElem { Deref, Field(Field, T), diff --git a/src/librustc_mir/borrow_check/conflict_errors.rs b/src/librustc_mir/borrow_check/conflict_errors.rs index 0913d743328..ebc25138a06 100644 --- a/src/librustc_mir/borrow_check/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/conflict_errors.rs @@ -2,9 +2,9 @@ use rustc::hir::def_id::DefId; use rustc::hir::{AsyncGeneratorKind, GeneratorKind}; use rustc::mir::{ - self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, ConstraintCategory, Local, - LocalDecl, LocalKind, Location, Operand, Place, PlaceBase, PlaceRef, ProjectionElem, Rvalue, - Statement, StatementKind, TerminatorKind, VarBindingForm, + self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, ConstraintCategory, + FakeReadCause, Local, LocalDecl, LocalKind, Location, Operand, Place, PlaceBase, PlaceRef, + ProjectionElem, Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm, }; use rustc::ty::{self, Ty}; use rustc_data_structures::fx::FxHashSet; @@ -380,42 +380,38 @@ pub(super) fn report_conflicting_borrow( let first_borrow_desc; let mut err = match ( gen_borrow_kind, - "immutable", - "mutable", issued_borrow.kind, - "immutable", - "mutable", ) { - (BorrowKind::Shared, lft, _, BorrowKind::Mut { .. }, _, rgt) => { + (BorrowKind::Shared, BorrowKind::Mut { .. }) => { first_borrow_desc = "mutable "; self.cannot_reborrow_already_borrowed( span, &desc_place, &msg_place, - lft, + "immutable", issued_span, "it", - rgt, + "mutable", &msg_borrow, None, ) } - (BorrowKind::Mut { .. }, _, lft, BorrowKind::Shared, rgt, _) => { + (BorrowKind::Mut { .. }, BorrowKind::Shared) => { first_borrow_desc = "immutable "; self.cannot_reborrow_already_borrowed( span, &desc_place, &msg_place, - lft, + "mutable", issued_span, "it", - rgt, + "immutable", &msg_borrow, None, ) } - (BorrowKind::Mut { .. }, _, _, BorrowKind::Mut { .. }, _, _) => { + (BorrowKind::Mut { .. }, BorrowKind::Mut { .. }) => { first_borrow_desc = "first "; self.cannot_mutably_borrow_multiply( span, @@ -427,7 +423,7 @@ pub(super) fn report_conflicting_borrow( ) } - (BorrowKind::Unique, _, _, BorrowKind::Unique, _, _) => { + (BorrowKind::Unique, BorrowKind::Unique) => { first_borrow_desc = "first "; self.cannot_uniquely_borrow_by_two_closures( span, @@ -437,25 +433,45 @@ pub(super) fn report_conflicting_borrow( ) } - (BorrowKind::Mut { .. }, _, _, BorrowKind::Shallow, _, _) - | (BorrowKind::Unique, _, _, BorrowKind::Shallow, _, _) => { - let mut err = self.cannot_mutate_in_match_guard( - span, - issued_span, - &desc_place, - "mutably borrow", - ); - borrow_spans.var_span_label( - &mut err, - format!( - "borrow occurs due to use of `{}`{}", desc_place, borrow_spans.describe() - ), - ); + (BorrowKind::Mut { .. }, BorrowKind::Shallow) + | (BorrowKind::Unique, BorrowKind::Shallow) => { + if let Some(immutable_section_description) = self.classify_immutable_section( + &issued_borrow.assigned_place, + ) { + let mut err = self.cannot_mutate_in_immutable_section( + span, + issued_span, + &desc_place, + immutable_section_description, + "mutably borrow", + ); + borrow_spans.var_span_label( + &mut err, + format!( + "borrow occurs due to use of `{}`{}", + desc_place, + borrow_spans.describe(), + ), + ); - return err; + return err; + } else { + first_borrow_desc = "immutable "; + self.cannot_reborrow_already_borrowed( + span, + &desc_place, + &msg_place, + "mutable", + issued_span, + "it", + "immutable", + &msg_borrow, + None, + ) + } } - (BorrowKind::Unique, _, _, _, _, _) => { + (BorrowKind::Unique, _) => { first_borrow_desc = "first "; self.cannot_uniquely_borrow_by_one_closure( span, @@ -469,14 +485,14 @@ pub(super) fn report_conflicting_borrow( ) }, - (BorrowKind::Shared, lft, _, BorrowKind::Unique, _, _) => { + (BorrowKind::Shared, BorrowKind::Unique) => { first_borrow_desc = "first "; self.cannot_reborrow_already_uniquely_borrowed( span, container_name, &desc_place, "", - lft, + "immutable", issued_span, "", None, @@ -484,14 +500,14 @@ pub(super) fn report_conflicting_borrow( ) } - (BorrowKind::Mut { .. }, _, lft, BorrowKind::Unique, _, _) => { + (BorrowKind::Mut { .. }, BorrowKind::Unique) => { first_borrow_desc = "first "; self.cannot_reborrow_already_uniquely_borrowed( span, container_name, &desc_place, "", - lft, + "mutable", issued_span, "", None, @@ -499,12 +515,12 @@ pub(super) fn report_conflicting_borrow( ) } - (BorrowKind::Shared, _, _, BorrowKind::Shared, _, _) - | (BorrowKind::Shared, _, _, BorrowKind::Shallow, _, _) - | (BorrowKind::Shallow, _, _, BorrowKind::Mut { .. }, _, _) - | (BorrowKind::Shallow, _, _, BorrowKind::Unique, _, _) - | (BorrowKind::Shallow, _, _, BorrowKind::Shared, _, _) - | (BorrowKind::Shallow, _, _, BorrowKind::Shallow, _, _) => unreachable!(), + (BorrowKind::Shared, BorrowKind::Shared) + | (BorrowKind::Shared, BorrowKind::Shallow) + | (BorrowKind::Shallow, BorrowKind::Mut { .. }) + | (BorrowKind::Shallow, BorrowKind::Unique) + | (BorrowKind::Shallow, BorrowKind::Shared) + | (BorrowKind::Shallow, BorrowKind::Shallow) => unreachable!(), }; if issued_spans == borrow_spans { @@ -1429,20 +1445,23 @@ pub(super) fn report_illegal_mutation_of_borrowed( let loan_span = loan_spans.args_or_use(); if loan.kind == BorrowKind::Shallow { - let mut err = self.cannot_mutate_in_match_guard( - span, - loan_span, - &self.describe_place(place.as_ref()).unwrap_or_else(|| "_".to_owned()), - "assign", - ); - loan_spans.var_span_label( - &mut err, - format!("borrow occurs due to use{}", loan_spans.describe()), - ); + if let Some(section) = self.classify_immutable_section(&loan.assigned_place) { + let mut err = self.cannot_mutate_in_immutable_section( + span, + loan_span, + &self.describe_place(place.as_ref()).unwrap_or_else(|| "_".to_owned()), + section, + "assign", + ); + loan_spans.var_span_label( + &mut err, + format!("borrow occurs due to use{}", loan_spans.describe()), + ); - err.buffer(&mut self.errors_buffer); + err.buffer(&mut self.errors_buffer); - return; + return; + } } let mut err = self.cannot_assign_to_borrowed( @@ -1593,6 +1612,35 @@ fn classify_drop_access_kind(&self, place: PlaceRef<'cx, 'tcx>) -> StorageDeadOr } } + /// Describe the reason for the fake borrow that was assigned to `place`. + fn classify_immutable_section(&self, place: &Place<'tcx>) -> Option<&'static str> { + use rustc::mir::visit::Visitor; + struct FakeReadCauseFinder<'a, 'tcx> { + place: &'a Place<'tcx>, + cause: Option, + } + impl<'tcx> Visitor<'tcx> for FakeReadCauseFinder<'_, 'tcx> { + fn visit_statement(&mut self, statement: &Statement<'tcx>, _: Location) { + match statement { + Statement { + kind: StatementKind::FakeRead(cause, box ref place), + .. + } if *place == *self.place => { + self.cause = Some(*cause); + } + _ => (), + } + } + } + let mut visitor = FakeReadCauseFinder { place, cause: None }; + visitor.visit_body(&self.body); + match visitor.cause { + Some(FakeReadCause::ForMatchGuard) => Some("match guard"), + Some(FakeReadCause::ForIndex) => Some("indexing expression"), + _ => None, + } + } + /// Annotate argument and return type of function and closure with (synthesized) lifetime for /// borrow of local value that does not live long enough. fn annotate_argument_and_return_for_borrow( diff --git a/src/librustc_mir/build/expr/as_place.rs b/src/librustc_mir/build/expr/as_place.rs index 8d2bef39bed..d3e013acc9e 100644 --- a/src/librustc_mir/build/expr/as_place.rs +++ b/src/librustc_mir/build/expr/as_place.rs @@ -4,9 +4,11 @@ use crate::build::ForGuard::{OutsideGuard, RefWithinGuard}; use crate::build::{BlockAnd, BlockAndExtension, Builder}; use crate::hair::*; +use rustc::middle::region; use rustc::mir::interpret::{PanicInfo::BoundsCheck}; use rustc::mir::*; -use rustc::ty::{CanonicalUserTypeAnnotation, Ty, TyCtxt, Variance}; +use rustc::ty::{self, CanonicalUserTypeAnnotation, Ty, TyCtxt, Variance}; +use syntax_pos::Span; use rustc_index::vec::Idx; @@ -68,6 +70,17 @@ fn from(base: PlaceBase<'tcx>) -> Self { impl<'a, 'tcx> Builder<'a, 'tcx> { /// Compile `expr`, yielding a place that we can move from etc. + /// + /// WARNING: Any user code might: + /// * Invalidate any slice bounds checks performed. + /// * Change the address that this `Place` refers to. + /// * Modify the memory that this place refers to. + /// * Invalidate the memory that this place refers to, this will be caught + /// by borrow checking. + /// + /// Extra care is needed if any user code is allowed to run between calling + /// this method and using it, as is the case for `match` and index + /// expressions. pub fn as_place(&mut self, mut block: BasicBlock, expr: M) -> BlockAnd> where M: Mirror<'tcx, Output = Expr<'tcx>>, @@ -83,7 +96,7 @@ fn as_place_builder(&mut self, block: BasicBlock, expr: M) -> BlockAnd>, { let expr = self.hir.mirror(expr); - self.expr_as_place(block, expr, Mutability::Mut) + self.expr_as_place(block, expr, Mutability::Mut, None) } /// Compile `expr`, yielding a place that we can move from etc. @@ -114,7 +127,7 @@ fn as_read_only_place_builder( M: Mirror<'tcx, Output = Expr<'tcx>>, { let expr = self.hir.mirror(expr); - self.expr_as_place(block, expr, Mutability::Not) + self.expr_as_place(block, expr, Mutability::Not, None) } fn expr_as_place( @@ -122,6 +135,7 @@ fn expr_as_place( mut block: BasicBlock, expr: Expr<'tcx>, mutability: Mutability, + fake_borrow_temps: Option<&mut Vec>, ) -> BlockAnd> { debug!( "expr_as_place(block={:?}, expr={:?}, mutability={:?})", @@ -137,63 +151,40 @@ fn expr_as_place( lint_level, value, } => this.in_scope((region_scope, source_info), lint_level, |this| { - if mutability == Mutability::Not { - this.as_read_only_place_builder(block, value) - } else { - this.as_place_builder(block, value) - } + let value = this.hir.mirror(value); + this.expr_as_place(block, value, mutability, fake_borrow_temps) }), ExprKind::Field { lhs, name } => { - let place_builder = unpack!(block = this.as_place_builder(block, lhs)); + let lhs = this.hir.mirror(lhs); + let place_builder = unpack!(block = this.expr_as_place( + block, + lhs, + mutability, + fake_borrow_temps, + )); block.and(place_builder.field(name, expr.ty)) } ExprKind::Deref { arg } => { - let place_builder = unpack!(block = this.as_place_builder(block, arg)); + let arg = this.hir.mirror(arg); + let place_builder = unpack!(block = this.expr_as_place( + block, + arg, + mutability, + fake_borrow_temps, + )); block.and(place_builder.deref()) } ExprKind::Index { lhs, index } => { - let (usize_ty, bool_ty) = (this.hir.usize_ty(), this.hir.bool_ty()); - - let place_builder = unpack!(block = this.as_place_builder(block, lhs)); - // 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( + this.lower_index_expression( block, - expr.temp_lifetime, + lhs, index, - Mutability::Not, - )); - - let slice = place_builder.clone().into_place(this.hir.tcx()); - // bounds check: - let (len, lt) = ( - this.temp(usize_ty.clone(), expr_span), - this.temp(bool_ty, expr_span), - ); - this.cfg.push_assign( - block, - source_info, // len = len(slice) - &len, - Rvalue::Len(slice), - ); - this.cfg.push_assign( - block, - source_info, // lt = idx < len - <, - Rvalue::BinaryOp( - BinOp::Lt, - Operand::Copy(Place::from(idx)), - Operand::Copy(len.clone()), - ), - ); - - let msg = BoundsCheck { - len: Operand::Move(len), - index: Operand::Copy(Place::from(idx)), - }; - let success = this.assert(block, Operand::Move(lt), true, msg, expr_span); - success.and(place_builder.index(idx)) + mutability, + fake_borrow_temps, + expr.temp_lifetime, + expr_span, + source_info, + ) } ExprKind::SelfRef => block.and(PlaceBuilder::from(Local::new(1))), ExprKind::VarRef { id } => { @@ -215,7 +206,13 @@ fn expr_as_place( )), ExprKind::PlaceTypeAscription { source, user_ty } => { - let place_builder = unpack!(block = this.as_place_builder(block, source)); + let source = this.hir.mirror(source); + let place_builder = unpack!(block = this.expr_as_place( + block, + source, + mutability, + fake_borrow_temps, + )); if let Some(user_ty) = user_ty { let annotation_index = this.canonical_user_type_annotations.push( CanonicalUserTypeAnnotation { @@ -309,4 +306,208 @@ fn expr_as_place( } } } + + /// Lower an index expression + /// + /// This has two complications; + /// + /// * We need to do a bounds check. + /// * We need to ensure that the bounds check can't be invalidated using an + /// expression like `x[1][{x = y; 2}]`. We use fake borrows here to ensure + /// that this is the case. + fn lower_index_expression( + &mut self, + mut block: BasicBlock, + base: ExprRef<'tcx>, + index: ExprRef<'tcx>, + mutability: Mutability, + fake_borrow_temps: Option<&mut Vec>, + temp_lifetime: Option, + expr_span: Span, + source_info: SourceInfo + ) -> BlockAnd> { + let lhs = self.hir.mirror(base); + + let base_fake_borrow_temps = &mut Vec::new(); + let is_outermost_index = fake_borrow_temps.is_none(); + let fake_borrow_temps = fake_borrow_temps.unwrap_or(base_fake_borrow_temps); + + let base_place = unpack!(block = self.expr_as_place( + block, + lhs, + mutability, + Some(fake_borrow_temps), + )); + + // Making this a *fresh* temporary 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 = self.as_temp( + block, + temp_lifetime, + index, + Mutability::Not, + )); + + block = self.bounds_check( + block, + base_place.clone().into_place(self.hir.tcx()), + idx, + expr_span, + source_info, + ); + + if is_outermost_index { + self.read_fake_borrows(block, fake_borrow_temps, source_info) + } else { + self.add_fake_borrows_of_base( + &base_place, + block, + fake_borrow_temps, + expr_span, + source_info, + ); + } + + block.and(base_place.index(idx)) + } + + fn bounds_check( + &mut self, + block: BasicBlock, + slice: Place<'tcx>, + index: Local, + expr_span: Span, + source_info: SourceInfo, + ) -> BasicBlock { + let usize_ty = self.hir.usize_ty(); + let bool_ty = self.hir.bool_ty(); + // bounds check: + let len = self.temp(usize_ty, expr_span); + let lt = self.temp(bool_ty, expr_span); + + // len = len(slice) + self.cfg.push_assign( + block, + source_info, + &len, + Rvalue::Len(slice), + ); + // lt = idx < len + self.cfg.push_assign( + block, + source_info, + <, + Rvalue::BinaryOp( + BinOp::Lt, + Operand::Copy(Place::from(index)), + Operand::Copy(len.clone()), + ), + ); + let msg = BoundsCheck { + len: Operand::Move(len), + index: Operand::Copy(Place::from(index)), + }; + // assert!(lt, "...") + self.assert(block, Operand::Move(lt), true, msg, expr_span) + } + + fn add_fake_borrows_of_base( + &mut self, + base_place: &PlaceBuilder<'tcx>, + block: BasicBlock, + fake_borrow_temps: &mut Vec, + expr_span: Span, + source_info: SourceInfo, + ) { + let tcx = self.hir.tcx(); + let place_ty = Place::ty_from( + &base_place.base, + &base_place.projection, + &self.local_decls, + tcx, + ); + if let ty::Slice(_) = place_ty.ty.kind { + // We need to create fake borrows to ensure that the bounds + // check that we just did stays valid. Since we can't assign to + // unsized values, we only need to ensure that none of the + // pointers in the base place are modified. + for (idx, elem) in base_place.projection.iter().enumerate().rev() { + match elem { + ProjectionElem::Deref => { + let fake_borrow_deref_ty = Place::ty_from( + &base_place.base, + &base_place.projection[..idx], + &self.local_decls, + tcx, + ).ty; + let fake_borrow_ty = tcx.mk_imm_ref( + tcx.lifetimes.re_erased, + fake_borrow_deref_ty, + ); + let fake_borrow_temp = self.local_decls.push( + LocalDecl::new_temp(fake_borrow_ty, expr_span) + ); + let projection = tcx.intern_place_elems(&base_place.projection[..idx]); + self.cfg.push_assign( + block, + source_info, + &fake_borrow_temp.into(), + Rvalue::Ref( + tcx.lifetimes.re_erased, + BorrowKind::Shallow, + Place { + base: base_place.base.clone(), + projection, + } + ), + ); + fake_borrow_temps.push(fake_borrow_temp); + } + ProjectionElem::Index(_) => { + let index_ty = Place::ty_from( + &base_place.base, + &base_place.projection[..idx], + &self.local_decls, + tcx, + ); + match index_ty.ty.kind { + // The previous index expression has already + // done any index expressions needed here. + ty::Slice(_) => break, + ty::Array(..) => (), + _ => bug!("unexpected index base"), + } + } + ProjectionElem::Field(..) + | ProjectionElem::Downcast(..) + | ProjectionElem::ConstantIndex { .. } + | ProjectionElem::Subslice { .. } => (), + } + } + } + } + + fn read_fake_borrows( + &mut self, + block: BasicBlock, + fake_borrow_temps: &mut Vec, + source_info: SourceInfo, + ) { + // All indexes have been evaluated now, read all of the + // fake borrows so that they are live across those index + // expressions. + for temp in fake_borrow_temps { + self.cfg.push( + block, + Statement { + source_info, + kind: StatementKind::FakeRead( + FakeReadCause::ForIndex, + Box::new(Place::from(*temp)), + ) + } + ); + } + } } diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs index 96ba8293582..bf01ad1a023 100644 --- a/src/librustc_mir/util/borrowck_errors.rs +++ b/src/librustc_mir/util/borrowck_errors.rs @@ -395,23 +395,25 @@ impl<'cx, 'tcx> crate::borrow_check::MirBorrowckCtxt<'cx, 'tcx> { ) } - crate fn cannot_mutate_in_match_guard( + crate fn cannot_mutate_in_immutable_section( &self, mutate_span: Span, - match_span: Span, - match_place: &str, + immutable_span: Span, + immutable_place: &str, + immutable_section: &str, action: &str, ) -> DiagnosticBuilder<'cx> { let mut err = struct_span_err!( self, mutate_span, E0510, - "cannot {} `{}` in match guard", + "cannot {} `{}` in {}", action, - match_place, + immutable_place, + immutable_section, ); err.span_label(mutate_span, format!("cannot {}", action)); - err.span_label(match_span, String::from("value is immutable in match guard")); + err.span_label(immutable_span, format!("value is immutable in {}", immutable_section)); err } diff --git a/src/test/mir-opt/storage_live_dead_in_statics.rs b/src/test/mir-opt/storage_live_dead_in_statics.rs index 2ed34ecfad2..5dc15286bab 100644 --- a/src/test/mir-opt/storage_live_dead_in_statics.rs +++ b/src/test/mir-opt/storage_live_dead_in_statics.rs @@ -36,11 +36,11 @@ fn main() { // END RUST SOURCE // START rustc.XXX.mir_map.0.mir // let mut _0: &'static Foo; -// let mut _1: &'static Foo; +// let _1: &'static Foo; // let _2: Foo; // let mut _3: &'static [(u32, u32)]; // let mut _4: &'static [(u32, u32); 42]; -// let mut _5: &'static [(u32, u32); 42]; +// let _5: &'static [(u32, u32); 42]; // let _6: [(u32, u32); 42]; // let mut _7: (u32, u32); // let mut _8: (u32, u32); diff --git a/src/test/mir-opt/uninhabited_enum_branching.rs b/src/test/mir-opt/uninhabited_enum_branching.rs index 1f37ff1498d..aa56918a9b8 100644 --- a/src/test/mir-opt/uninhabited_enum_branching.rs +++ b/src/test/mir-opt/uninhabited_enum_branching.rs @@ -34,12 +34,12 @@ fn main() { // let _1: &str; // let mut _2: Test1; // let mut _3: isize; -// let mut _4: &str; -// let mut _5: &str; +// let _4: &str; +// let _5: &str; // let _6: &str; // let mut _7: Test2; // let mut _8: isize; -// let mut _9: &str; +// let _9: &str; // bb0: { // StorageLive(_1); // StorageLive(_2); @@ -103,12 +103,12 @@ fn main() { // let _1: &str; // let mut _2: Test1; // let mut _3: isize; -// let mut _4: &str; -// let mut _5: &str; +// let _4: &str; +// let _5: &str; // let _6: &str; // let mut _7: Test2; // let mut _8: isize; -// let mut _9: &str; +// let _9: &str; // bb0: { // StorageLive(_1); // StorageLive(_2); @@ -172,12 +172,12 @@ fn main() { // let _1: &str; // let mut _2: Test1; // let mut _3: isize; -// let mut _4: &str; -// let mut _5: &str; +// let _4: &str; +// let _5: &str; // let _6: &str; // let mut _7: Test2; // let mut _8: isize; -// let mut _9: &str; +// let _9: &str; // bb0: { // StorageLive(_1); // StorageLive(_2); diff --git a/src/test/ui/borrowck/slice-index-bounds-check-invalidation.rs b/src/test/ui/borrowck/slice-index-bounds-check-invalidation.rs new file mode 100644 index 00000000000..0e0e3cda6e2 --- /dev/null +++ b/src/test/ui/borrowck/slice-index-bounds-check-invalidation.rs @@ -0,0 +1,82 @@ +// Test that we error if a slice is modified after it has been bounds checked +// and before we actually index it. + +fn modify_before_assert_slice_slice(x: &[&[i32]]) -> i32 { + let mut x = x; + let z: &[i32] = &[1, 2, 3]; + let y: &[&[i32]] = &[z]; + x[{ x = y; 0 }][2] // OK we haven't checked any bounds before we index `x`. +} + +fn modify_before_assert_array_slice(x: &[&[i32]; 3]) -> i32 { + let mut x = x; + let z: &[i32] = &[1, 2, 3]; + let y: &[&[i32]; 3] = &[z, z, z]; + x[{ x = y; 0 }][2] // OK we haven't checked any bounds before we index `x`. +} + +fn modify_before_assert_slice_array(x: &[&[i32; 3]]) -> i32 { + let mut x = x; + let z: &[i32; 3] = &[1, 2, 3]; + let y: &[&[i32; 3]] = &[z]; + x[{ x = y; 0 }][2] // OK we haven't checked any bounds before we index `x`. +} + +fn modify_before_assert_array_array(x: &[&[i32; 3]; 3]) -> i32 { + let mut x = x; + let z: &[i32; 3] = &[1, 2, 3]; + let y: &[&[i32; 3]; 3] = &[z, z, z]; + x[{ x = y; 0 }][2] // OK we haven't checked any bounds before we index `x`. +} + +fn modify_after_assert_slice_slice(x: &[&[i32]]) -> i32 { + let mut x = x; + let z: &[i32] = &[1, 2, 3]; + let y: &[&[i32]] = &[&z]; + x[1][{ x = y; 2}] //~ ERROR cannot assign `x` in indexing expression +} + +fn modify_after_assert_array_slice(x: &[&[i32]; 1]) -> i32 { + let mut x = x; + let z: &[i32] = &[1, 2, 3]; + let y: &[&[i32]; 1] = &[&z]; + x[0][{ x = y; 2}] // OK cannot invalidate a fixed-size array bounds check +} + +fn modify_after_assert_slice_array(x: &[&[i32; 3]]) -> i32 { + let mut x = x; + let z: &[i32; 3] = &[1, 2, 3]; + let y: &[&[i32; 3]] = &[&z]; + x[1][{ x = y; 2}] //~ ERROR cannot assign `x` in indexing expression +} + +fn modify_after_assert_array_array(x: &[&[i32; 3]; 1]) -> i32 { + let mut x = x; + let z: &[i32; 3] = &[1, 2, 3]; + let y: &[&[i32; 3]; 1] = &[&z]; + x[0][{ x = y; 2}] // OK cannot invalidate a fixed-size array bounds check +} + +fn modify_after_assert_slice_slice_array(x: &[&[[i32; 1]]]) -> i32 { + let mut x = x; + let z: &[[i32; 1]] = &[[1], [2], [3]]; + let y: &[&[[i32; 1]]] = &[&z]; + x[1][{ x = y; 2}][0] //~ ERROR cannot assign `x` in indexing expression +} + +fn modify_after_assert_slice_slice_slice(x: &[&[&[i32]]]) -> i32 { + let mut x = x; + let z: &[&[i32]] = &[&[1], &[2], &[3]]; + let y: &[&[&[i32]]] = &[z]; + x[1][{ x = y; 2}][0] //~ ERROR cannot assign `x` in indexing expression +} + + +fn main() { + println!("{}", modify_after_assert_slice_array(&[&[4, 5, 6], &[9, 10, 11]])); + println!("{}", modify_after_assert_slice_slice(&[&[4, 5, 6], &[9, 10, 11]])); + println!("{}", modify_after_assert_slice_slice_array(&[&[[4], [5], [6]], &[[9], [10], [11]]])); + println!("{}", modify_after_assert_slice_slice_slice( + &[&[&[4], &[5], &[6]], &[&[9], &[10], &[11]]]), + ); +} diff --git a/src/test/ui/borrowck/slice-index-bounds-check-invalidation.stderr b/src/test/ui/borrowck/slice-index-bounds-check-invalidation.stderr new file mode 100644 index 00000000000..f9ed16f19cd --- /dev/null +++ b/src/test/ui/borrowck/slice-index-bounds-check-invalidation.stderr @@ -0,0 +1,35 @@ +error[E0510]: cannot assign `x` in indexing expression + --> $DIR/slice-index-bounds-check-invalidation.rs:36:12 + | +LL | x[1][{ x = y; 2}] + | ---- ^^^^^ cannot assign + | | + | value is immutable in indexing expression + +error[E0510]: cannot assign `x` in indexing expression + --> $DIR/slice-index-bounds-check-invalidation.rs:50:12 + | +LL | x[1][{ x = y; 2}] + | ---- ^^^^^ cannot assign + | | + | value is immutable in indexing expression + +error[E0510]: cannot assign `x` in indexing expression + --> $DIR/slice-index-bounds-check-invalidation.rs:64:12 + | +LL | x[1][{ x = y; 2}][0] + | ---- ^^^^^ cannot assign + | | + | value is immutable in indexing expression + +error[E0510]: cannot assign `x` in indexing expression + --> $DIR/slice-index-bounds-check-invalidation.rs:71:12 + | +LL | x[1][{ x = y; 2}][0] + | ---- ^^^^^ cannot assign + | | + | value is immutable in indexing expression + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0510`.