Auto merge of #98017 - RalfJung:dereferenceable, r=nikic

bors · bors · commit 848090dcd185 · 2022-07-22T21:20:35.000Z
do not mark interior mutable shared refs as dereferenceable My proposed solution to #55005.
diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs
@@ -2618,14 +2618,14 @@ where
                     // Use conservative pointer kind if not optimizing. This saves us the
                     // Freeze/Unpin queries, and can save time in the codegen backend (noalias
                     // attributes in LLVM have compile-time cost even in unoptimized builds).
-                    PointerKind::Shared
+                    PointerKind::SharedMutable
                 } else {
                     match mt {
                         hir::Mutability::Not => {
                             if ty.is_freeze(tcx.at(DUMMY_SP), cx.param_env()) {
                                 PointerKind::Frozen
                             } else {
-                                PointerKind::Shared
+                                PointerKind::SharedMutable
                             }
                         }
                         hir::Mutability::Mut => {
@@ -2636,7 +2636,7 @@ where
                             if ty.is_unpin(tcx.at(DUMMY_SP), cx.param_env()) {
                                 PointerKind::UniqueBorrowed
                             } else {
-                                PointerKind::Shared
+                                PointerKind::UniqueBorrowedPinned
                             }
                         }
                     }
@@ -3255,10 +3255,13 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
 
                     // `Box` (`UniqueBorrowed`) are not necessarily dereferenceable
                     // for the entire duration of the function as they can be deallocated
-                    // at any time. Set their valid size to 0.
+                    // at any time. Same for shared mutable references. If LLVM had a
+                    // way to say "dereferenceable on entry" we could use it here.
                     attrs.pointee_size = match kind {
-                        PointerKind::UniqueOwned => Size::ZERO,
-                        _ => pointee.size,
+                        PointerKind::UniqueBorrowed
+                        | PointerKind::UniqueBorrowedPinned
+                        | PointerKind::Frozen => pointee.size,
+                        PointerKind::SharedMutable | PointerKind::UniqueOwned => Size::ZERO,
                     };
 
                     // `Box`, `&T`, and `&mut T` cannot be undef.
@@ -3285,7 +3288,9 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
                     // or not to actually emit the attribute. It can also be controlled with the
                     // `-Zmutable-noalias` debugging option.
                     let no_alias = match kind {
-                        PointerKind::Shared | PointerKind::UniqueBorrowed => false,
+                        PointerKind::SharedMutable
+                        | PointerKind::UniqueBorrowed
+                        | PointerKind::UniqueBorrowedPinned => false,
                         PointerKind::UniqueOwned => noalias_for_box,
                         PointerKind::Frozen => !is_return,
                     };
diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs
@@ -1350,15 +1350,19 @@ impl<'a, Ty> Deref for TyAndLayout<'a, Ty> {
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub enum PointerKind {
     /// Most general case, we know no restrictions to tell LLVM.
-    Shared,
+    SharedMutable,
 
-    /// `&T` where `T` contains no `UnsafeCell`, is `noalias` and `readonly`.
+    /// `&T` where `T` contains no `UnsafeCell`, is `dereferenceable`, `noalias` and `readonly`.
     Frozen,
 
-    /// `&mut T` which is `noalias` but not `readonly`.
+    /// `&mut T` which is `dereferenceable` and `noalias` but not `readonly`.
     UniqueBorrowed,
 
-    /// `Box<T>`, unlike `UniqueBorrowed`, it also has `noalias` on returns.
+    /// `&mut !Unpin`, which is `dereferenceable` but neither `noalias` nor `readonly`.
+    UniqueBorrowedPinned,
+
+    /// `Box<T>`, which is `noalias` (even on return types, unlike the above) but neither `readonly`
+    /// nor `dereferenceable`.
     UniqueOwned,
 }
 
diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs
@@ -1766,15 +1766,24 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> {
 ///
 /// The precise Rust aliasing rules are somewhat in flux, but the main points are not contentious:
 ///
-/// - If you create a safe reference with lifetime `'a` (either a `&T` or `&mut T`
-/// reference) that is accessible by safe code (for example, because you returned it),
-/// then you must not access the data in any way that contradicts that reference for the
-/// remainder of `'a`. For example, this means that if you take the `*mut T` from an
-/// `UnsafeCell<T>` and cast it to an `&T`, then the data in `T` must remain immutable
-/// (modulo any `UnsafeCell` data found within `T`, of course) until that reference's
-/// lifetime expires. Similarly, if you create a `&mut T` reference that is released to
-/// safe code, then you must not access the data within the `UnsafeCell` until that
-/// reference expires.
+/// - If you create a safe reference with lifetime `'a` (either a `&T` or `&mut T` reference), then
+/// you must not access the data in any way that contradicts that reference for the remainder of
+/// `'a`. For example, this means that if you take the `*mut T` from an `UnsafeCell<T>` and cast it
+/// to an `&T`, then the data in `T` must remain immutable (modulo any `UnsafeCell` data found
+/// within `T`, of course) until that reference's lifetime expires. Similarly, if you create a `&mut
+/// T` reference that is released to safe code, then you must not access the data within the
+/// `UnsafeCell` until that reference expires.
+///
+/// - For both `&T` without `UnsafeCell<_>` and `&mut T`, you must also not deallocate the data
+/// until the reference expires. As a special exception, given an `&T`, any part of it that is
+/// inside an `UnsafeCell<_>` may be deallocated during the lifetime of the reference, after the
+/// last time the reference is used (dereferenced or reborrowed). Since you cannot deallocate a part
+/// of what a reference points to, this means the memory an `&T` points to can be deallocted only if
+/// *every part of it* (including padding) is inside an `UnsafeCell`.
+///
+///     However, whenever a `&UnsafeCell<T>` is constructed or dereferenced, it must still point to
+/// live memory and the compiler is allowed to insert spurious reads if it can prove that this
+/// memory has not yet been deallocated.
 ///
 /// - At all times, you must avoid data races. If multiple threads have access to
 /// the same `UnsafeCell`, then any writes must have a proper happens-before relation to all other
diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs
@@ -5,6 +5,7 @@
 
 use std::mem::MaybeUninit;
 use std::num::NonZeroU64;
+use std::marker::PhantomPinned;
 
 pub struct S {
   _field: [i32; 8],
@@ -14,6 +15,11 @@ pub struct UnsafeInner {
   _field: std::cell::UnsafeCell<i16>,
 }
 
+pub struct NotUnpin {
+  _field: i32,
+  _marker: PhantomPinned,
+}
+
 pub enum MyBool {
   True,
   False,
@@ -91,7 +97,7 @@ pub fn static_borrow(_: &'static i32) {
 pub fn named_borrow<'r>(_: &'r i32) {
 }
 
-// CHECK: @unsafe_borrow({{i16\*|ptr}} noundef align 2 dereferenceable(2) %_1)
+// CHECK: @unsafe_borrow({{i16\*|ptr}} noundef nonnull align 2 %_1)
 // unsafe interior means this isn't actually readonly and there may be aliases ...
 #[no_mangle]
 pub fn unsafe_borrow(_: &UnsafeInner) {
@@ -109,6 +115,18 @@ pub fn mutable_unsafe_borrow(_: &mut UnsafeInner) {
 pub fn mutable_borrow(_: &mut i32) {
 }
 
+#[no_mangle]
+// CHECK: @mutable_notunpin_borrow({{i32\*|ptr}} noundef align 4 dereferenceable(4) %_1)
+// This one is *not* `noalias` because it might be self-referential.
+pub fn mutable_notunpin_borrow(_: &mut NotUnpin) {
+}
+
+// CHECK: @notunpin_borrow({{i32\*|ptr}} noalias noundef readonly align 4 dereferenceable(4) %_1)
+// But `&NotUnpin` behaves perfectly normal.
+#[no_mangle]
+pub fn notunpin_borrow(_: &NotUnpin) {
+}
+
 // CHECK: @indirect_struct({{%S\*|ptr}} noalias nocapture noundef dereferenceable(32) %_1)
 #[no_mangle]
 pub fn indirect_struct(_: S) {
