]> git.lizzy.rs Git - rust.git/commitdiff
make &mut !Unpin not dereferenceable
authorRalf Jung <post@ralfj.de>
Tue, 27 Dec 2022 11:46:08 +0000 (12:46 +0100)
committerRalf Jung <post@ralfj.de>
Mon, 6 Feb 2023 10:46:37 +0000 (11:46 +0100)
See https://github.com/rust-lang/unsafe-code-guidelines/issues/381 for discussion.

compiler/rustc_ty_utils/src/abi.rs
src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs
src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.rs [deleted file]
src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.stderr [deleted file]
src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs
tests/codegen/function-arguments.rs

index 884087987ce08c6e98123a696e70730887c5f703..4ee3202292afd91e0b6903195d3a9955a0887548 100644 (file)
@@ -256,13 +256,16 @@ fn adjust_for_rust_scalar<'tcx>(
 
             // `Box` are not necessarily dereferenceable for the entire duration of the function as
             // they can be deallocated at any time. Same for non-frozen shared references (see
-            // <https://github.com/rust-lang/rust/pull/98017>). If LLVM had a way to say
-            // "dereferenceable on entry" we could use it here.
+            // <https://github.com/rust-lang/rust/pull/98017>), and for mutable references to
+            // potentially self-referential types (see
+            // <https://github.com/rust-lang/unsafe-code-guidelines/issues/381>). If LLVM had a way
+            // to say "dereferenceable on entry" we could use it here.
             attrs.pointee_size = match kind {
-                PointerKind::Box | PointerKind::SharedRef { frozen: false } => Size::ZERO,
-                PointerKind::SharedRef { frozen: true } | PointerKind::MutableRef { .. } => {
-                    pointee.size
-                }
+                PointerKind::Box
+                | PointerKind::SharedRef { frozen: false }
+                | PointerKind::MutableRef { unpin: false } => Size::ZERO,
+                PointerKind::SharedRef { frozen: true }
+                | PointerKind::MutableRef { unpin: true } => pointee.size,
             };
 
             // The aliasing rules for `Box<T>` are still not decided, but currently we emit
index ec555ba2895c8468b82bcaae0008fcfb34b13ad0..3b3a41c2f03736a917c6877290d0d657f6d8623b 100644 (file)
@@ -81,21 +81,18 @@ fn from_ref_ty<'tcx>(
                         protector: None,
                     }
                 } else if pointee.is_unpin(*cx.tcx, cx.param_env()) {
-                    // A regular full mutable reference.
+                    // A regular full mutable reference. On `FnEntry` this is `noalias` and `dereferenceable`.
                     NewPermission::Uniform {
                         perm: Permission::Unique,
                         access: Some(AccessKind::Write),
                         protector,
                     }
                 } else {
+                    // `!Unpin` dereferences do not get `noalias` nor `dereferenceable`.
                     NewPermission::Uniform {
                         perm: Permission::SharedReadWrite,
-                        // FIXME: We emit `dereferenceable` for `!Unpin` mutable references, so we
-                        // should do fake accesses here. But then we run into
-                        // <https://github.com/rust-lang/unsafe-code-guidelines/issues/381>, so for now
-                        // we don't do that.
                         access: None,
-                        protector,
+                        protector: None,
                     }
                 }
             }
@@ -109,6 +106,7 @@ fn from_ref_ty<'tcx>(
                 }
             }
             ty::Ref(_, _pointee, Mutability::Not) => {
+                // Shared references. If frozen, these get `noalias` and `dereferenceable`; otherwise neither.
                 NewPermission::FreezeSensitive {
                     freeze_perm: Permission::SharedReadOnly,
                     freeze_access: Some(AccessKind::Read),
diff --git a/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.rs b/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.rs
deleted file mode 100644 (file)
index fd67dcc..0000000
+++ /dev/null
@@ -1,16 +0,0 @@
-//@error-pattern: /deallocating while item \[SharedReadWrite for .*\] is strongly protected/
-use std::marker::PhantomPinned;
-
-pub struct NotUnpin(i32, PhantomPinned);
-
-fn inner(x: &mut NotUnpin, f: fn(&mut NotUnpin)) {
-    // `f` may mutate, but it may not deallocate!
-    f(x)
-}
-
-fn main() {
-    inner(Box::leak(Box::new(NotUnpin(0, PhantomPinned))), |x| {
-        let raw = x as *mut _;
-        drop(unsafe { Box::from_raw(raw) });
-    });
-}
diff --git a/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.stderr b/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.stderr
deleted file mode 100644 (file)
index 47cfa0d..0000000
+++ /dev/null
@@ -1,38 +0,0 @@
-error: Undefined Behavior: deallocating while item [SharedReadWrite for <TAG>] is strongly protected by call ID
-  --> RUSTLIB/alloc/src/alloc.rs:LL:CC
-   |
-LL |     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ deallocating while item [SharedReadWrite for <TAG>] is strongly protected by call ID
-   |
-   = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
-   = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
-   = note: BACKTRACE:
-   = note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
-   = note: inside `<std::alloc::Global as std::alloc::Allocator>::deallocate` at RUSTLIB/alloc/src/alloc.rs:LL:CC
-   = note: inside `alloc::alloc::box_free::<NotUnpin, std::alloc::Global>` at RUSTLIB/alloc/src/alloc.rs:LL:CC
-   = note: inside `std::ptr::drop_in_place::<std::boxed::Box<NotUnpin>> - shim(Some(std::boxed::Box<NotUnpin>))` at RUSTLIB/core/src/ptr/mod.rs:LL:CC
-   = note: inside `std::mem::drop::<std::boxed::Box<NotUnpin>>` at RUSTLIB/core/src/mem/mod.rs:LL:CC
-note: inside closure
-  --> $DIR/deallocate_against_protector2.rs:LL:CC
-   |
-LL |         drop(unsafe { Box::from_raw(raw) });
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = note: inside `<[closure@$DIR/deallocate_against_protector2.rs:LL:CC] as std::ops::FnOnce<(&mut NotUnpin,)>>::call_once - shim` at RUSTLIB/core/src/ops/function.rs:LL:CC
-note: inside `inner`
-  --> $DIR/deallocate_against_protector2.rs:LL:CC
-   |
-LL |     f(x)
-   |     ^^^^
-note: inside `main`
-  --> $DIR/deallocate_against_protector2.rs:LL:CC
-   |
-LL | /     inner(Box::leak(Box::new(NotUnpin(0, PhantomPinned))), |x| {
-LL | |         let raw = x as *mut _;
-LL | |         drop(unsafe { Box::from_raw(raw) });
-LL | |     });
-   | |______^
-
-note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
-
-error: aborting due to previous error
-
index ef6eb346c17b12804cb9c92dd5caaa424d35dbec..8e78efa73c751578c9c3b7849553bb62fcd32e31 100644 (file)
@@ -19,6 +19,7 @@ fn main() {
     array_casts();
     mut_below_shr();
     wide_raw_ptr_in_tuple();
+    not_unpin_not_protected();
 }
 
 // Make sure that reading from an `&mut` does, like reborrowing to `&`,
@@ -219,3 +220,22 @@ fn wide_raw_ptr_in_tuple() {
     // Make sure the fn ptr part of the vtable is still fine.
     r.type_id();
 }
+
+fn not_unpin_not_protected() {
+    // `&mut !Unpin`, at least for now, does not get `noalias` nor `dereferenceable`, so we also
+    // don't add protectors. (We could, but until we have a better idea for where we want to go with
+    // the self-referntial-generator situation, it does not seem worth the potential trouble.)
+    use std::marker::PhantomPinned;
+
+    pub struct NotUnpin(i32, PhantomPinned);
+
+    fn inner(x: &mut NotUnpin, f: fn(&mut NotUnpin)) {
+        // `f` may mutate, but it may not deallocate!
+        f(x)
+    }
+
+    inner(Box::leak(Box::new(NotUnpin(0, PhantomPinned))), |x| {
+        let raw = x as *mut _;
+        drop(unsafe { Box::from_raw(raw) });
+    });
+}
index 1f979d7b90a70b08b7a5edd95028777218481772..0f4639086b885f9e8a15b59aaaf71193ee33ab64 100644 (file)
@@ -85,6 +85,12 @@ pub fn option_nonzero_int(x: Option<NonZeroU64>) -> Option<NonZeroU64> {
 pub fn readonly_borrow(_: &i32) {
 }
 
+// CHECK: noundef align 4 dereferenceable(4) {{i32\*|ptr}} @readonly_borrow_ret()
+#[no_mangle]
+pub fn readonly_borrow_ret() -> &'static i32 {
+  loop {}
+}
+
 // CHECK: @static_borrow({{i32\*|ptr}} noalias noundef readonly align 4 dereferenceable(4) %_1)
 // static borrow may be captured
 #[no_mangle]
@@ -115,9 +121,17 @@ pub fn mutable_unsafe_borrow(_: &mut UnsafeInner) {
 pub fn mutable_borrow(_: &mut i32) {
 }
 
+// CHECK: noundef align 4 dereferenceable(4) {{i32\*|ptr}} @mutable_borrow_ret()
+#[no_mangle]
+pub fn mutable_borrow_ret() -> &'static mut i32 {
+  loop {}
+}
+
 #[no_mangle]
-// CHECK: @mutable_notunpin_borrow({{i32\*|ptr}} noundef align 4 dereferenceable(4) %_1)
+// CHECK: @mutable_notunpin_borrow({{i32\*|ptr}} noundef nonnull align 4 %_1)
 // This one is *not* `noalias` because it might be self-referential.
+// It is also not `dereferenceable` due to
+// <https://github.com/rust-lang/unsafe-code-guidelines/issues/381>.
 pub fn mutable_notunpin_borrow(_: &mut NotUnpin) {
 }