Auto merge of #90413 - tmiasko:addr-of-mutable, r=RalfJung,oli-obk

`addr_of!` grants mutable access, maybe?

The exact set of permissions granted when forming a raw reference is
currently undecided https://github.com/rust-lang/rust/issues/56604.

To avoid presupposing any particular outcome, adjust the const
qualification to be compatible with decision where raw reference
constructed from `addr_of!` grants mutable access.

Additionally, to avoid keeping `MaybeMutBorrowedLocals` in sync with
const qualification, remove it. It's no longer used.

`@rust-lang/wg-const-eval`

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 {
+    fn address_of_allows_mutation(&self, _mt: mir::Mutability, _place: mir::Place<'tcx>) -> bool {
-        match mt {
+        // Exact set of permissions granted by AddressOf is undecided. Conservatively assume that
-            mir::Mutability::Mut => true,
+        // it might allow mutation until resolution of #56604.
-            mir::Mutability::Not => self.shared_borrow_allows_mutation(place),
+        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)

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<MutBorrow<'mir, 'tcx>>;
 
 /// 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
+/// immovable generators.
-/// local has not been mutated via indirect assignment (e.g., `*p = 42`), the side-effects of a
+pub struct MaybeBorrowedLocals {
-/// function call or inline assembly.
-pub struct MaybeBorrowedLocals<K = AnyBorrow> {
-    kind: K,
     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 }
+        MaybeBorrowedLocals { ignore_borrow_on_drop: false }
     }
 }
 
-impl MaybeMutBorrowedLocals<'mir, 'tcx> {
+impl MaybeBorrowedLocals {
-    /// 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,
-        }
-    }
-}
-
-impl<K> MaybeBorrowedLocals<K> {
     /// 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<K> MaybeBorrowedLocals<K> {
         MaybeBorrowedLocals { ignore_borrow_on_drop: true, ..self }
     }
 
-    fn transfer_function<'a, T>(&'a self, trans: &'a mut T) -> TransferFunction<'a, T, K> {
+    fn transfer_function<'a, T>(&'a self, trans: &'a mut T) -> TransferFunction<'a, T> {
-        TransferFunction {
+        TransferFunction { trans, ignore_borrow_on_drop: self.ignore_borrow_on_drop }
-            kind: &self.kind,
-            trans,
-            ignore_borrow_on_drop: self.ignore_borrow_on_drop,
-        }
     }
 }
 
-impl<K> AnalysisDomain<'tcx> for MaybeBorrowedLocals<K>
+impl AnalysisDomain<'tcx> for MaybeBorrowedLocals {
-where
-    K: BorrowAnalysisKind<'tcx>,
-{
     type Domain = BitSet<Local>;
-    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<K> GenKillAnalysis<'tcx> for MaybeBorrowedLocals<K>
+impl GenKillAnalysis<'tcx> for MaybeBorrowedLocals {
-where
-    K: BorrowAnalysisKind<'tcx>,
-{
     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<T, K> Visitor<'tcx> for TransferFunction<'a, T, K>
+impl<T> Visitor<'tcx> for TransferFunction<'a, T>
 where
     T: GenKill<Local>,
-    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) => {
+            mir::Rvalue::AddressOf(_mt, borrowed_place) => {
-                if !borrowed_place.is_indirect() && self.kind.in_address_of(*mt, *borrowed_place) {
+                if !borrowed_place.is_indirect() {
                     self.trans.gen(borrowed_place.local);
                 }
             }
 
-            mir::Rvalue::Ref(_, kind, borrowed_place) => {
+            mir::Rvalue::Ref(_, _kind, borrowed_place) => {
-                if !borrowed_place.is_indirect() && self.kind.in_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),
-        }
-    }
-}

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};

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,
+    DefinitelyInitializedPlaces, MaybeInitializedPlaces, MaybeLiveLocals, MaybeUninitializedPlaces,
-    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<Local>,
-        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,

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,

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<T>() {
     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<String> = None; //~ ERROR destructors cannot be evaluated
+    &raw mut x;
+
+    let mut y: Option<String> = 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<String> = None; //~ ERROR destructors cannot be evaluated
+    &raw const x;
+
+    let y: Option<String> = None; //~ ERROR destructors cannot be evaluated
+    std::ptr::addr_of!(y);
+}

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<String>) = (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<T> = 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<String> = 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<String> = 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<String> = 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<String> = None;
+   |         ^ constant functions cannot evaluate destructors
+
+error: aborting due to 9 previous errors
 
 For more information about this error, try `rustc --explain E0493`.

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<String>, 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<String>) = (0, None);
     let _ = &mut a.0;
 }
+
+// Shared reference does not allow for mutation.
+pub const fn h() {
+    let x: Option<String> = None;
+    let _ = &x;
+}

src/test/ui/mir-dataflow/indirect-mutation-offset.rs

@@ -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<i32>,
-}
-
-#[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<i32>;
-
-        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);
-}

src/test/ui/mir-dataflow/indirect-mutation-offset.stderr

@@ -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
-