Auto merge of #111517 - lukas-code:addr-of-mutate, r=tmiasko

allow mutating function args through `&raw const`

Fixes #111502 by "turning off the sketchy optimization while we figure out if this is ok", like `@JakobDegen` said.

The first commit in this PR removes some suspicious looking logic from the same method, but should have no functional changes, since it doesn't modify the `context` outside of the method. Best reviewed commit by commit.

r? opsem

Author: bors

compiler/rustc_mir_transform/src/deduce_param_attrs.rs

@@ -8,7 +8,7 @@
 use rustc_hir::def_id::LocalDefId;
 use rustc_index::bit_set::BitSet;
 use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor};
-use rustc_middle::mir::{Body, Local, Location, Operand, Terminator, TerminatorKind, RETURN_PLACE};
+use rustc_middle::mir::{Body, Location, Operand, Place, Terminator, TerminatorKind, RETURN_PLACE};
 use rustc_middle::ty::{self, DeducedParamAttrs, Ty, TyCtxt};
 use rustc_session::config::OptLevel;
 
@@ -29,31 +29,31 @@ impl DeduceReadOnly {
 }
 
 impl<'tcx> Visitor<'tcx> for DeduceReadOnly {
-    fn visit_local(&mut self, local: Local, mut context: PlaceContext, _: Location) {
+    fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _location: Location) {
         // We're only interested in arguments.
-        if local == RETURN_PLACE || local.index() > self.mutable_args.domain_size() {
+        if place.local == RETURN_PLACE || place.local.index() > self.mutable_args.domain_size() {
             return;
         }
 
-        // Replace place contexts that are moves with copies. This is safe in all cases except
-        // function argument position, which we already handled in `visit_terminator()` by using the
-        // ArgumentChecker. See the comment in that method for more details.
-        //
-        // In the future, we might want to move this out into a separate pass, but for now let's
-        // just do it on the fly because that's faster.
-        if matches!(context, PlaceContext::NonMutatingUse(NonMutatingUseContext::Move)) {
-            context = PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy);
-        }
-
-        match context {
-            PlaceContext::MutatingUse(..)
-            | PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) => {
+        let mark_as_mutable = match context {
+            PlaceContext::MutatingUse(..) => {
                 // This is a mutation, so mark it as such.
-                self.mutable_args.insert(local.index() - 1);
+                true
+            }
+            PlaceContext::NonMutatingUse(NonMutatingUseContext::AddressOf) => {
+                // Whether mutating though a `&raw const` is allowed is still undecided, so we
+                // disable any sketchy `readonly` optimizations for now.
+                // But we only need to do this if the pointer would point into the argument.
+                !place.is_indirect()
             }
             PlaceContext::NonMutatingUse(..) | PlaceContext::NonUse(..) => {
                 // Not mutating, so it's fine.
+                false
             }
+        };
+
+        if mark_as_mutable {
+            self.mutable_args.insert(place.local.index() - 1);
         }
     }
 

tests/codegen/addr-of-mutate.rs

@@ -0,0 +1,34 @@
+// compile-flags: -C opt-level=3 -C no-prepopulate-passes
+// min-llvm-version: 15.0 (for opaque pointers)
+
+#![crate_type = "lib"]
+
+// Test for the absence of `readonly` on the argument when it is mutated via `&raw const`.
+// See <https://github.com/rust-lang/rust/issues/111502>.
+
+// CHECK: i8 @foo(ptr noalias nocapture noundef dereferenceable(128) %x)
+#[no_mangle]
+pub fn foo(x: [u8; 128]) -> u8 {
+    let ptr = core::ptr::addr_of!(x).cast_mut();
+    unsafe {
+        (*ptr)[0] = 1;
+    }
+    x[0]
+}
+
+// CHECK: i1 @second(ptr noalias nocapture noundef dereferenceable({{[0-9]+}}) %a_ptr_and_b)
+#[no_mangle]
+pub unsafe fn second(a_ptr_and_b: (*mut (i32, bool), (i64, bool))) -> bool {
+    let b_bool_ptr = core::ptr::addr_of!(a_ptr_and_b.1.1).cast_mut();
+    (*b_bool_ptr) = true;
+    a_ptr_and_b.1.1
+}
+
+// If going through a deref (and there are no other mutating accesses), then `readonly` is fine.
+// CHECK: i1 @third(ptr noalias nocapture noundef readonly dereferenceable({{[0-9]+}}) %a_ptr_and_b)
+#[no_mangle]
+pub unsafe fn third(a_ptr_and_b: (*mut (i32, bool), (i64, bool))) -> bool {
+    let b_bool_ptr = core::ptr::addr_of!((*a_ptr_and_b.0).1).cast_mut();
+    (*b_bool_ptr) = true;
+    a_ptr_and_b.1.1
+}