diff --git a/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs b/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs index 38576230883cd..fcce829eba412 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs @@ -94,11 +94,10 @@ where } } - fn address_of_allows_mutation(&self, mt: mir::Mutability, place: mir::Place<'tcx>) -> bool { - match mt { - mir::Mutability::Mut => true, - mir::Mutability::Not => self.shared_borrow_allows_mutation(place), - } + fn address_of_allows_mutation(&self, _mt: mir::Mutability, _place: mir::Place<'tcx>) -> bool { + // Exact set of permissions granted by AddressOf is undecided. Conservatively assume that + // it might allow mutation until resolution of #56604. + true } fn ref_allows_mutation(&self, kind: mir::BorrowKind, place: mir::Place<'tcx>) -> bool { @@ -110,6 +109,15 @@ where } } + /// `&` only allow mutation if the borrowed place is `!Freeze`. + /// + /// This assumes that it is UB to take the address of a struct field whose type is + /// `Freeze`, then use pointer arithmetic to derive a pointer to a *different* field of + /// that same struct whose type is `!Freeze`. If we decide that this is not UB, we will + /// have to check the type of the borrowed **local** instead of the borrowed **place** + /// below. See [rust-lang/unsafe-code-guidelines#134]. + /// + /// [rust-lang/unsafe-code-guidelines#134]: https://github.com/rust-lang/unsafe-code-guidelines/issues/134 fn shared_borrow_allows_mutation(&self, place: mir::Place<'tcx>) -> bool { !place .ty(self.ccx.body, self.ccx.tcx) diff --git a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs index 158ba1b942528..d38b567a95849 100644 --- a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs +++ b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs @@ -3,25 +3,14 @@ use super::*; use crate::{AnalysisDomain, GenKill, GenKillAnalysis}; use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::*; -use rustc_middle::ty::{ParamEnv, TyCtxt}; -use rustc_span::DUMMY_SP; - -pub type MaybeMutBorrowedLocals<'mir, 'tcx> = MaybeBorrowedLocals>; /// A dataflow analysis that tracks whether a pointer or reference could possibly exist that points /// to a given local. /// -/// The `K` parameter determines what kind of borrows are tracked. By default, -/// `MaybeBorrowedLocals` looks for *any* borrow of a local. If you are only interested in borrows -/// that might allow mutation, use the `MaybeMutBorrowedLocals` type alias instead. -/// /// At present, this is used as a very limited form of alias analysis. For example, /// `MaybeBorrowedLocals` is used to compute which locals are live during a yield expression for -/// immovable generators. `MaybeMutBorrowedLocals` is used during const checking to prove that a -/// local has not been mutated via indirect assignment (e.g., `*p = 42`), the side-effects of a -/// function call or inline assembly. -pub struct MaybeBorrowedLocals { - kind: K, +/// immovable generators. +pub struct MaybeBorrowedLocals { ignore_borrow_on_drop: bool, } @@ -29,29 +18,11 @@ impl MaybeBorrowedLocals { /// A dataflow analysis that records whether a pointer or reference exists that may alias the /// given local. pub fn all_borrows() -> Self { - MaybeBorrowedLocals { kind: AnyBorrow, ignore_borrow_on_drop: false } - } -} - -impl MaybeMutBorrowedLocals<'mir, 'tcx> { - /// A dataflow analysis that records whether a pointer or reference exists that may *mutably* - /// alias the given local. - /// - /// This includes `&mut` and pointers derived from an `&mut`, as well as shared borrows of - /// types with interior mutability. - pub fn mut_borrows_only( - tcx: TyCtxt<'tcx>, - body: &'mir mir::Body<'tcx>, - param_env: ParamEnv<'tcx>, - ) -> Self { - MaybeBorrowedLocals { - kind: MutBorrow { body, tcx, param_env }, - ignore_borrow_on_drop: false, - } + MaybeBorrowedLocals { ignore_borrow_on_drop: false } } } -impl MaybeBorrowedLocals { +impl MaybeBorrowedLocals { /// During dataflow analysis, ignore the borrow that may occur when a place is dropped. /// /// Drop terminators may call custom drop glue (`Drop::drop`), which takes `&mut self` as a @@ -69,21 +40,14 @@ impl MaybeBorrowedLocals { MaybeBorrowedLocals { ignore_borrow_on_drop: true, ..self } } - fn transfer_function<'a, T>(&'a self, trans: &'a mut T) -> TransferFunction<'a, T, K> { - TransferFunction { - kind: &self.kind, - trans, - ignore_borrow_on_drop: self.ignore_borrow_on_drop, - } + fn transfer_function<'a, T>(&'a self, trans: &'a mut T) -> TransferFunction<'a, T> { + TransferFunction { trans, ignore_borrow_on_drop: self.ignore_borrow_on_drop } } } -impl AnalysisDomain<'tcx> for MaybeBorrowedLocals -where - K: BorrowAnalysisKind<'tcx>, -{ +impl AnalysisDomain<'tcx> for MaybeBorrowedLocals { type Domain = BitSet; - const NAME: &'static str = K::ANALYSIS_NAME; + const NAME: &'static str = "maybe_borrowed_locals"; fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain { // bottom = unborrowed @@ -95,10 +59,7 @@ where } } -impl GenKillAnalysis<'tcx> for MaybeBorrowedLocals -where - K: BorrowAnalysisKind<'tcx>, -{ +impl GenKillAnalysis<'tcx> for MaybeBorrowedLocals { type Idx = Local; fn statement_effect( @@ -131,16 +92,14 @@ where } /// A `Visitor` that defines the transfer function for `MaybeBorrowedLocals`. -struct TransferFunction<'a, T, K> { +struct TransferFunction<'a, T> { trans: &'a mut T, - kind: &'a K, ignore_borrow_on_drop: bool, } -impl Visitor<'tcx> for TransferFunction<'a, T, K> +impl Visitor<'tcx> for TransferFunction<'a, T> where T: GenKill, - K: BorrowAnalysisKind<'tcx>, { fn visit_statement(&mut self, stmt: &Statement<'tcx>, location: Location) { self.super_statement(stmt, location); @@ -156,14 +115,14 @@ where self.super_rvalue(rvalue, location); match rvalue { - mir::Rvalue::AddressOf(mt, borrowed_place) => { - if !borrowed_place.is_indirect() && self.kind.in_address_of(*mt, *borrowed_place) { + mir::Rvalue::AddressOf(_mt, borrowed_place) => { + if !borrowed_place.is_indirect() { self.trans.gen(borrowed_place.local); } } - mir::Rvalue::Ref(_, kind, borrowed_place) => { - if !borrowed_place.is_indirect() && self.kind.in_ref(*kind, *borrowed_place) { + mir::Rvalue::Ref(_, _kind, borrowed_place) => { + if !borrowed_place.is_indirect() { self.trans.gen(borrowed_place.local); } } @@ -211,64 +170,3 @@ where } } } - -pub struct AnyBorrow; - -pub struct MutBorrow<'mir, 'tcx> { - tcx: TyCtxt<'tcx>, - body: &'mir Body<'tcx>, - param_env: ParamEnv<'tcx>, -} - -impl MutBorrow<'mir, 'tcx> { - /// `&` and `&raw` only allow mutation if the borrowed place is `!Freeze`. - /// - /// This assumes that it is UB to take the address of a struct field whose type is - /// `Freeze`, then use pointer arithmetic to derive a pointer to a *different* field of - /// that same struct whose type is `!Freeze`. If we decide that this is not UB, we will - /// have to check the type of the borrowed **local** instead of the borrowed **place** - /// below. See [rust-lang/unsafe-code-guidelines#134]. - /// - /// [rust-lang/unsafe-code-guidelines#134]: https://github.com/rust-lang/unsafe-code-guidelines/issues/134 - fn shared_borrow_allows_mutation(&self, place: Place<'tcx>) -> bool { - !place.ty(self.body, self.tcx).ty.is_freeze(self.tcx.at(DUMMY_SP), self.param_env) - } -} - -pub trait BorrowAnalysisKind<'tcx> { - const ANALYSIS_NAME: &'static str; - - fn in_address_of(&self, mt: Mutability, place: Place<'tcx>) -> bool; - fn in_ref(&self, kind: mir::BorrowKind, place: Place<'tcx>) -> bool; -} - -impl BorrowAnalysisKind<'tcx> for AnyBorrow { - const ANALYSIS_NAME: &'static str = "maybe_borrowed_locals"; - - fn in_ref(&self, _: mir::BorrowKind, _: Place<'_>) -> bool { - true - } - fn in_address_of(&self, _: Mutability, _: Place<'_>) -> bool { - true - } -} - -impl BorrowAnalysisKind<'tcx> for MutBorrow<'mir, 'tcx> { - const ANALYSIS_NAME: &'static str = "maybe_mut_borrowed_locals"; - - fn in_ref(&self, kind: mir::BorrowKind, place: Place<'tcx>) -> bool { - match kind { - mir::BorrowKind::Mut { .. } => true, - mir::BorrowKind::Shared | mir::BorrowKind::Shallow | mir::BorrowKind::Unique => { - self.shared_borrow_allows_mutation(place) - } - } - } - - fn in_address_of(&self, mt: Mutability, place: Place<'tcx>) -> bool { - match mt { - Mutability::Mut => true, - Mutability::Not => self.shared_borrow_allows_mutation(place), - } - } -} diff --git a/compiler/rustc_mir_dataflow/src/impls/mod.rs b/compiler/rustc_mir_dataflow/src/impls/mod.rs index 474f4f2a79b2a..91dddc6cd55c5 100644 --- a/compiler/rustc_mir_dataflow/src/impls/mod.rs +++ b/compiler/rustc_mir_dataflow/src/impls/mod.rs @@ -22,7 +22,7 @@ mod init_locals; mod liveness; mod storage_liveness; -pub use self::borrowed_locals::{MaybeBorrowedLocals, MaybeMutBorrowedLocals}; +pub use self::borrowed_locals::MaybeBorrowedLocals; pub use self::init_locals::MaybeInitializedLocals; pub use self::liveness::MaybeLiveLocals; pub use self::storage_liveness::{MaybeRequiresStorage, MaybeStorageLive}; diff --git a/compiler/rustc_mir_dataflow/src/rustc_peek.rs b/compiler/rustc_mir_dataflow/src/rustc_peek.rs index 2d27d085b4893..28e5d76783aa4 100644 --- a/compiler/rustc_mir_dataflow/src/rustc_peek.rs +++ b/compiler/rustc_mir_dataflow/src/rustc_peek.rs @@ -11,8 +11,7 @@ use rustc_middle::mir::{self, Body, Local, Location}; use rustc_middle::ty::{self, Ty, TyCtxt}; use crate::impls::{ - DefinitelyInitializedPlaces, MaybeInitializedPlaces, MaybeLiveLocals, MaybeMutBorrowedLocals, - MaybeUninitializedPlaces, + DefinitelyInitializedPlaces, MaybeInitializedPlaces, MaybeLiveLocals, MaybeUninitializedPlaces, }; use crate::move_paths::{HasMoveData, MoveData}; use crate::move_paths::{LookupResult, MovePathIndex}; @@ -62,14 +61,6 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { sanity_check_via_rustc_peek(tcx, body, &attributes, &flow_def_inits); } - if has_rustc_mir_with(sess, &attributes, sym::rustc_peek_indirectly_mutable).is_some() { - let flow_mut_borrowed = MaybeMutBorrowedLocals::mut_borrows_only(tcx, body, param_env) - .into_engine(tcx, body) - .iterate_to_fixpoint(); - - sanity_check_via_rustc_peek(tcx, body, &attributes, &flow_mut_borrowed); - } - if has_rustc_mir_with(sess, &attributes, sym::rustc_peek_liveness).is_some() { let flow_liveness = MaybeLiveLocals.into_engine(tcx, body).iterate_to_fixpoint(); @@ -281,26 +272,6 @@ where } } -impl<'tcx> RustcPeekAt<'tcx> for MaybeMutBorrowedLocals<'_, 'tcx> { - fn peek_at( - &self, - tcx: TyCtxt<'tcx>, - place: mir::Place<'tcx>, - flow_state: &BitSet, - call: PeekCall, - ) { - info!(?place, "peek_at"); - let Some(local) = place.as_local() else { - tcx.sess.span_err(call.span, "rustc_peek: argument was not a local"); - return; - }; - - if !flow_state.contains(local) { - tcx.sess.span_err(call.span, "rustc_peek: bit not set"); - } - } -} - impl<'tcx> RustcPeekAt<'tcx> for MaybeLiveLocals { fn peek_at( &self, diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 5f71e955e2ad7..52e2a8f48e23b 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1128,7 +1128,6 @@ symbols! { rustc_partition_reused, rustc_peek, rustc_peek_definite_init, - rustc_peek_indirectly_mutable, rustc_peek_liveness, rustc_peek_maybe_init, rustc_peek_maybe_uninit, diff --git a/src/test/ui/consts/qualif-indirect-mutation-fail.rs b/src/test/ui/consts/qualif-indirect-mutation-fail.rs index cedead00fec97..f74a25a346fda 100644 --- a/src/test/ui/consts/qualif-indirect-mutation-fail.rs +++ b/src/test/ui/consts/qualif-indirect-mutation-fail.rs @@ -2,6 +2,7 @@ #![feature(const_mut_refs)] #![feature(const_precise_live_drops)] #![feature(const_swap)] +#![feature(raw_ref_op)] // Mutable borrow of a field with drop impl. pub const fn f() { @@ -42,3 +43,22 @@ pub const fn g2() { let _ = x.is_some(); let _y = x; //~ ERROR destructors cannot be evaluated } + +// Mutable raw reference to a Drop type. +pub const fn address_of_mut() { + let mut x: Option = None; //~ ERROR destructors cannot be evaluated + &raw mut x; + + let mut y: Option = None; //~ ERROR destructors cannot be evaluated + std::ptr::addr_of_mut!(y); +} + +// Const raw reference to a Drop type. Conservatively assumed to allow mutation +// until resolution of https://github.com/rust-lang/rust/issues/56604. +pub const fn address_of_const() { + let x: Option = None; //~ ERROR destructors cannot be evaluated + &raw const x; + + let y: Option = None; //~ ERROR destructors cannot be evaluated + std::ptr::addr_of!(y); +} diff --git a/src/test/ui/consts/qualif-indirect-mutation-fail.stderr b/src/test/ui/consts/qualif-indirect-mutation-fail.stderr index aa6ed465594e3..713df12b7a55f 100644 --- a/src/test/ui/consts/qualif-indirect-mutation-fail.stderr +++ b/src/test/ui/consts/qualif-indirect-mutation-fail.stderr @@ -1,33 +1,57 @@ error[E0493]: destructors cannot be evaluated at compile-time - --> $DIR/qualif-indirect-mutation-fail.rs:8:9 + --> $DIR/qualif-indirect-mutation-fail.rs:9:9 | LL | let mut a: (u32, Option) = (0, None); | ^^^^^ constant functions cannot evaluate destructors error[E0493]: destructors cannot be evaluated at compile-time - --> $DIR/qualif-indirect-mutation-fail.rs:14:9 + --> $DIR/qualif-indirect-mutation-fail.rs:15:9 | LL | let mut x = None; | ^^^^^ constants cannot evaluate destructors error[E0493]: destructors cannot be evaluated at compile-time - --> $DIR/qualif-indirect-mutation-fail.rs:30:9 + --> $DIR/qualif-indirect-mutation-fail.rs:31:9 | LL | let _z = x; | ^^ constants cannot evaluate destructors error[E0493]: destructors cannot be evaluated at compile-time - --> $DIR/qualif-indirect-mutation-fail.rs:35:9 + --> $DIR/qualif-indirect-mutation-fail.rs:36:9 | LL | let x: Option = None; | ^ constant functions cannot evaluate destructors error[E0493]: destructors cannot be evaluated at compile-time - --> $DIR/qualif-indirect-mutation-fail.rs:43:9 + --> $DIR/qualif-indirect-mutation-fail.rs:44:9 | LL | let _y = x; | ^^ constant functions cannot evaluate destructors -error: aborting due to 5 previous errors +error[E0493]: destructors cannot be evaluated at compile-time + --> $DIR/qualif-indirect-mutation-fail.rs:52:9 + | +LL | let mut y: Option = None; + | ^^^^^ constant functions cannot evaluate destructors + +error[E0493]: destructors cannot be evaluated at compile-time + --> $DIR/qualif-indirect-mutation-fail.rs:49:9 + | +LL | let mut x: Option = None; + | ^^^^^ constant functions cannot evaluate destructors + +error[E0493]: destructors cannot be evaluated at compile-time + --> $DIR/qualif-indirect-mutation-fail.rs:62:9 + | +LL | let y: Option = None; + | ^ constant functions cannot evaluate destructors + +error[E0493]: destructors cannot be evaluated at compile-time + --> $DIR/qualif-indirect-mutation-fail.rs:59:9 + | +LL | let x: Option = None; + | ^ constant functions cannot evaluate destructors + +error: aborting due to 9 previous errors For more information about this error, try `rustc --explain E0493`. diff --git a/src/test/ui/consts/qualif-indirect-mutation-pass.rs b/src/test/ui/consts/qualif-indirect-mutation-pass.rs index 35a9b70a5f6f1..06af6a03b8f60 100644 --- a/src/test/ui/consts/qualif-indirect-mutation-pass.rs +++ b/src/test/ui/consts/qualif-indirect-mutation-pass.rs @@ -3,6 +3,7 @@ #![feature(const_mut_refs)] #![feature(const_precise_live_drops)] +// Mutable reference allows only mutation of !Drop place. pub const fn f() { let mut x: (Option, u32) = (None, 0); let mut a = 10; @@ -10,7 +11,14 @@ pub const fn f() { x.1 = a; } +// Mutable reference allows only mutation of !Drop place. pub const fn g() { let mut a: (u32, Option) = (0, None); let _ = &mut a.0; } + +// Shared reference does not allow for mutation. +pub const fn h() { + let x: Option = None; + let _ = &x; +} diff --git a/src/test/ui/mir-dataflow/indirect-mutation-offset.rs b/src/test/ui/mir-dataflow/indirect-mutation-offset.rs deleted file mode 100644 index 374a9f75a134b..0000000000000 --- a/src/test/ui/mir-dataflow/indirect-mutation-offset.rs +++ /dev/null @@ -1,48 +0,0 @@ -// compile-flags: -Zunleash-the-miri-inside-of-you - -// This test demonstrates a shortcoming of the `MaybeMutBorrowedLocals` analysis. It does not -// handle code that takes a reference to one field of a struct, then use pointer arithmetic to -// transform it to another field of that same struct that may have interior mutability. For now, -// this is UB, but this may change in the future. See [rust-lang/unsafe-code-guidelines#134]. -// -// [rust-lang/unsafe-code-guidelines#134]: https://github.com/rust-lang/unsafe-code-guidelines/issues/134 - -#![feature(core_intrinsics, rustc_attrs, const_raw_ptr_deref)] - -use std::cell::UnsafeCell; -use std::intrinsics::rustc_peek; - -#[repr(C)] -struct PartialInteriorMut { - zst: [i32; 0], - cell: UnsafeCell, -} - -#[rustc_mir(rustc_peek_indirectly_mutable,stop_after_dataflow)] -const BOO: i32 = { - let x = PartialInteriorMut { - zst: [], - cell: UnsafeCell::new(0), - }; - - let p_zst: *const _ = &x.zst ; // Doesn't cause `x` to get marked as indirectly mutable. - - let rmut_cell = unsafe { - // Take advantage of the fact that `zst` and `cell` are at the same location in memory. - // This trick would work with any size type if miri implemented `ptr::offset`. - let p_cell = p_zst as *const UnsafeCell; - - let pmut_cell = (*p_cell).get(); - &mut *pmut_cell - }; - - *rmut_cell = 42; // Mutates `x` indirectly even though `x` is not marked indirectly mutable!!! - let val = *rmut_cell; - rustc_peek(x); //~ ERROR rustc_peek: bit not set - - val -}; - -fn main() { - println!("{}", BOO); -} diff --git a/src/test/ui/mir-dataflow/indirect-mutation-offset.stderr b/src/test/ui/mir-dataflow/indirect-mutation-offset.stderr deleted file mode 100644 index 1d5287c15ab79..0000000000000 --- a/src/test/ui/mir-dataflow/indirect-mutation-offset.stderr +++ /dev/null @@ -1,10 +0,0 @@ -error: rustc_peek: bit not set - --> $DIR/indirect-mutation-offset.rs:41:5 - | -LL | rustc_peek(x); - | ^^^^^^^^^^^^^ - -error: stop_after_dataflow ended compilation - -error: aborting due to 2 previous errors -