also do not add noalias on not-Unpin Box

Author: RalfJung

compiler/rustc_abi/src/lib.rs

@@ -1443,8 +1443,8 @@ pub enum PointerKind {
     SharedRef { frozen: bool },
     /// Mutable reference. `unpin` indicates the absence of any pinned data.
     MutableRef { unpin: bool },
-    /// Box.
-    Box,
+    /// Box. `unpin` indicates the absence of any pinned data.
+    Box { unpin: bool },
 }
 
 /// Note that this information is advisory only, and backends are free to ignore it.

compiler/rustc_middle/src/ty/layout.rs

@@ -818,110 +818,114 @@ where
         let tcx = cx.tcx();
         let param_env = cx.param_env();
 
-        let pointee_info =
-            match *this.ty.kind() {
-                ty::RawPtr(mt) if offset.bytes() == 0 => {
-                    tcx.layout_of(param_env.and(mt.ty)).ok().map(|layout| PointeeInfo {
-                        size: layout.size,
-                        align: layout.align.abi,
-                        safe: None,
-                    })
-                }
-                ty::FnPtr(fn_sig) if offset.bytes() == 0 => {
-                    tcx.layout_of(param_env.and(tcx.mk_fn_ptr(fn_sig))).ok().map(|layout| {
-                        PointeeInfo { size: layout.size, align: layout.align.abi, safe: None }
-                    })
-                }
-                ty::Ref(_, ty, mt) if offset.bytes() == 0 => {
-                    // 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).
-                    let optimize = tcx.sess.opts.optimize != OptLevel::No;
-                    let kind = match mt {
-                        hir::Mutability::Not => PointerKind::SharedRef {
-                            frozen: optimize && ty.is_freeze(tcx, cx.param_env()),
-                        },
-                        hir::Mutability::Mut => PointerKind::MutableRef {
-                            unpin: optimize && ty.is_unpin(tcx, cx.param_env()),
-                        },
-                    };
+        let pointee_info = match *this.ty.kind() {
+            ty::RawPtr(mt) if offset.bytes() == 0 => {
+                tcx.layout_of(param_env.and(mt.ty)).ok().map(|layout| PointeeInfo {
+                    size: layout.size,
+                    align: layout.align.abi,
+                    safe: None,
+                })
+            }
+            ty::FnPtr(fn_sig) if offset.bytes() == 0 => {
+                tcx.layout_of(param_env.and(tcx.mk_fn_ptr(fn_sig))).ok().map(|layout| PointeeInfo {
+                    size: layout.size,
+                    align: layout.align.abi,
+                    safe: None,
+                })
+            }
+            ty::Ref(_, ty, mt) if offset.bytes() == 0 => {
+                // 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).
+                let optimize = tcx.sess.opts.optimize != OptLevel::No;
+                let kind = match mt {
+                    hir::Mutability::Not => PointerKind::SharedRef {
+                        frozen: optimize && ty.is_freeze(tcx, cx.param_env()),
+                    },
+                    hir::Mutability::Mut => PointerKind::MutableRef {
+                        unpin: optimize && ty.is_unpin(tcx, cx.param_env()),
+                    },
+                };
 
-                    tcx.layout_of(param_env.and(ty)).ok().map(|layout| PointeeInfo {
-                        size: layout.size,
-                        align: layout.align.abi,
-                        safe: Some(kind),
-                    })
-                }
+                tcx.layout_of(param_env.and(ty)).ok().map(|layout| PointeeInfo {
+                    size: layout.size,
+                    align: layout.align.abi,
+                    safe: Some(kind),
+                })
+            }
 
-                _ => {
-                    let mut data_variant = match this.variants {
-                        // Within the discriminant field, only the niche itself is
-                        // always initialized, so we only check for a pointer at its
-                        // offset.
-                        //
-                        // If the niche is a pointer, it's either valid (according
-                        // to its type), or null (which the niche field's scalar
-                        // validity range encodes). This allows using
-                        // `dereferenceable_or_null` for e.g., `Option<&T>`, and
-                        // this will continue to work as long as we don't start
-                        // using more niches than just null (e.g., the first page of
-                        // the address space, or unaligned pointers).
-                        Variants::Multiple {
-                            tag_encoding: TagEncoding::Niche { untagged_variant, .. },
-                            tag_field,
-                            ..
-                        } if this.fields.offset(tag_field) == offset => {
-                            Some(this.for_variant(cx, untagged_variant))
-                        }
-                        _ => Some(this),
-                    };
+            _ => {
+                let mut data_variant = match this.variants {
+                    // Within the discriminant field, only the niche itself is
+                    // always initialized, so we only check for a pointer at its
+                    // offset.
+                    //
+                    // If the niche is a pointer, it's either valid (according
+                    // to its type), or null (which the niche field's scalar
+                    // validity range encodes). This allows using
+                    // `dereferenceable_or_null` for e.g., `Option<&T>`, and
+                    // this will continue to work as long as we don't start
+                    // using more niches than just null (e.g., the first page of
+                    // the address space, or unaligned pointers).
+                    Variants::Multiple {
+                        tag_encoding: TagEncoding::Niche { untagged_variant, .. },
+                        tag_field,
+                        ..
+                    } if this.fields.offset(tag_field) == offset => {
+                        Some(this.for_variant(cx, untagged_variant))
+                    }
+                    _ => Some(this),
+                };
 
-                    if let Some(variant) = data_variant {
-                        // We're not interested in any unions.
-                        if let FieldsShape::Union(_) = variant.fields {
-                            data_variant = None;
-                        }
+                if let Some(variant) = data_variant {
+                    // We're not interested in any unions.
+                    if let FieldsShape::Union(_) = variant.fields {
+                        data_variant = None;
                     }
+                }
 
-                    let mut result = None;
-
-                    if let Some(variant) = data_variant {
-                        // FIXME(erikdesjardins): handle non-default addrspace ptr sizes
-                        // (requires passing in the expected address space from the caller)
-                        let ptr_end = offset + Pointer(AddressSpace::DATA).size(cx);
-                        for i in 0..variant.fields.count() {
-                            let field_start = variant.fields.offset(i);
-                            if field_start <= offset {
-                                let field = variant.field(cx, i);
-                                result = field.to_result().ok().and_then(|field| {
-                                    if ptr_end <= field_start + field.size {
-                                        // We found the right field, look inside it.
-                                        let field_info =
-                                            field.pointee_info_at(cx, offset - field_start);
-                                        field_info
-                                    } else {
-                                        None
-                                    }
-                                });
-                                if result.is_some() {
-                                    break;
+                let mut result = None;
+
+                if let Some(variant) = data_variant {
+                    // FIXME(erikdesjardins): handle non-default addrspace ptr sizes
+                    // (requires passing in the expected address space from the caller)
+                    let ptr_end = offset + Pointer(AddressSpace::DATA).size(cx);
+                    for i in 0..variant.fields.count() {
+                        let field_start = variant.fields.offset(i);
+                        if field_start <= offset {
+                            let field = variant.field(cx, i);
+                            result = field.to_result().ok().and_then(|field| {
+                                if ptr_end <= field_start + field.size {
+                                    // We found the right field, look inside it.
+                                    let field_info =
+                                        field.pointee_info_at(cx, offset - field_start);
+                                    field_info
+                                } else {
+                                    None
                                 }
+                            });
+                            if result.is_some() {
+                                break;
                             }
                         }
                     }
+                }
 
-                    // FIXME(eddyb) This should be for `ptr::Unique<T>`, not `Box<T>`.
-                    if let Some(ref mut pointee) = result {
-                        if let ty::Adt(def, _) = this.ty.kind() {
-                            if def.is_box() && offset.bytes() == 0 {
-                                pointee.safe = Some(PointerKind::Box);
-                            }
+                // FIXME(eddyb) This should be for `ptr::Unique<T>`, not `Box<T>`.
+                if let Some(ref mut pointee) = result {
+                    if let ty::Adt(def, _) = this.ty.kind() {
+                        if def.is_box() && offset.bytes() == 0 {
+                            let optimize = tcx.sess.opts.optimize != OptLevel::No;
+                            pointee.safe = Some(PointerKind::Box {
+                                unpin: optimize && this.ty.boxed_ty().is_unpin(tcx, cx.param_env()),
+                            });
                         }
                     }
-
-                    result
                 }
-            };
+
+                result
+            }
+        };
 
         debug!(
             "pointee_info_at (offset={:?}, type kind: {:?}) => {:?}",

compiler/rustc_ty_utils/src/abi.rs

@@ -261,7 +261,7 @@ fn adjust_for_rust_scalar<'tcx>(
             // <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::Box { .. }
                 | PointerKind::SharedRef { frozen: false }
                 | PointerKind::MutableRef { unpin: false } => Size::ZERO,
                 PointerKind::SharedRef { frozen: true }
@@ -278,17 +278,16 @@ fn adjust_for_rust_scalar<'tcx>(
             // versions at all anymore. We still support turning it off using -Zmutable-noalias.
             let noalias_mut_ref = cx.tcx.sess.opts.unstable_opts.mutable_noalias;
 
-            // `&mut` pointer parameters never alias other parameters,
-            // or mutable global data
-            //
             // `&T` where `T` contains no `UnsafeCell<U>` is immutable, and can be marked as both
             // `readonly` and `noalias`, as LLVM's definition of `noalias` is based solely on memory
             // dependencies rather than pointer equality. However this only applies to arguments,
             // not return values.
+            //
+            // `&mut T` and `Box<T>` where `T: Unpin` are unique and hence `noalias`.
             let no_alias = match kind {
                 PointerKind::SharedRef { frozen } => frozen,
                 PointerKind::MutableRef { unpin } => unpin && noalias_mut_ref,
-                PointerKind::Box => noalias_for_box,
+                PointerKind::Box { unpin } => unpin && noalias_for_box,
             };
             // We can never add `noalias` in return position; that LLVM attribute has some very surprising semantics
             // (see <https://github.com/rust-lang/unsafe-code-guidelines/issues/385#issuecomment-1368055745>).

src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs

@@ -135,6 +135,32 @@ impl NewPermission {
         }
     }
 
+    fn from_box_ty<'tcx>(
+        ty: Ty<'tcx>,
+        kind: RetagKind,
+        cx: &crate::MiriInterpCx<'_, 'tcx>,
+    ) -> Self {
+        // `ty` is not the `Box` but the field of the Box with this pointer (due to allocator handling).
+        let pointee = ty.builtin_deref(true).unwrap().ty;
+        if pointee.is_unpin(*cx.tcx, cx.param_env()) {
+            // A regular box. On `FnEntry` this is `noalias`, but not `dereferenceable` (hence only
+            // a weak protector).
+            NewPermission::Uniform {
+                perm: Permission::Unique,
+                access: Some(AccessKind::Write),
+                protector: (kind == RetagKind::FnEntry)
+                    .then_some(ProtectorKind::WeakProtector),
+            }
+        } else {
+            // `!Unpin` boxes do not get `noalias` nor `dereferenceable`.
+            NewPermission::Uniform {
+                perm: Permission::SharedReadWrite,
+                access: None,
+                protector: None,
+            }
+        }
+    }
+
     fn protector(&self) -> Option<ProtectorKind> {
         match self {
             NewPermission::Uniform { protector, .. } => *protector,
@@ -914,12 +940,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
             fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> {
                 // Boxes get a weak protectors, since they may be deallocated.
-                let new_perm = NewPermission::Uniform {
-                    perm: Permission::Unique,
-                    access: Some(AccessKind::Write),
-                    protector: (self.kind == RetagKind::FnEntry)
-                        .then_some(ProtectorKind::WeakProtector),
-                };
+                let new_perm = NewPermission::from_box_ty(place.layout.ty, self.kind, self.ecx);
                 self.retag_ptr_inplace(place, new_perm, self.retag_cause)
             }
 

src/tools/miri/tests/pass/stacked-borrows/future-self-referential.rs

@@ -26,6 +26,19 @@ impl Future for Delay {
     }
 }
 
+fn mk_waker() -> Waker {
+    use std::sync::Arc;
+
+    struct MyWaker;
+    impl Wake for MyWaker {
+        fn wake(self: Arc<Self>) {
+            unimplemented!()
+        }
+    }
+
+    Waker::from(Arc::new(MyWaker))
+}
+
 async fn do_stuff() {
     (&mut Delay::new(1)).await;
 }
@@ -73,16 +86,7 @@ impl Future for DoStuff {
 }
 
 fn run_fut<T>(fut: impl Future<Output = T>) -> T {
-    use std::sync::Arc;
-
-    struct MyWaker;
-    impl Wake for MyWaker {
-        fn wake(self: Arc<Self>) {
-            unimplemented!()
-        }
-    }
-
-    let waker = Waker::from(Arc::new(MyWaker));
+    let waker = mk_waker();
     let mut context = Context::from_waker(&waker);
 
     let mut pinned = pin!(fut);
@@ -94,7 +98,37 @@ fn run_fut<T>(fut: impl Future<Output = T>) -> T {
     }
 }
 
+fn self_referential_box() {
+    let waker = mk_waker();
+    let cx = &mut Context::from_waker(&waker);
+
+    async fn my_fut() -> i32 {
+        let val = 10;
+        let val_ref = &val;
+
+        let _ = Delay::new(1).await;
+
+        *val_ref
+    }
+
+    fn box_poll<F: Future>(
+        mut f: Pin<Box<F>>,
+        cx: &mut Context<'_>,
+    ) -> (Pin<Box<F>>, Poll<F::Output>) {
+        let p = f.as_mut().poll(cx);
+        (f, p)
+    }
+
+    let my_fut = Box::pin(my_fut());
+    let (my_fut, p1) = box_poll(my_fut, cx);
+    assert!(p1.is_pending());
+    let (my_fut, p2) = box_poll(my_fut, cx);
+    assert!(p2.is_ready());
+    drop(my_fut);
+}
+
 fn main() {
     run_fut(do_stuff());
     run_fut(DoStuff::new());
+    self_referential_box();
 }

tests/codegen/function-arguments.rs

@@ -181,6 +181,12 @@ pub fn _box(x: Box<i32>) -> Box<i32> {
   x
 }
 
+// CHECK: noundef nonnull align 4 {{i32\*|ptr}} @notunpin_box({{i32\*|ptr}} noundef nonnull align 4 %x)
+#[no_mangle]
+pub fn notunpin_box(x: Box<NotUnpin>) -> Box<NotUnpin> {
+  x
+}
+
 // CHECK: @struct_return({{%S\*|ptr}} noalias nocapture noundef sret(%S) dereferenceable(32){{( %0)?}})
 #[no_mangle]
 pub fn struct_return() -> S {
@@ -247,12 +253,12 @@ pub fn trait_raw(_: *const dyn Drop) {
 
 // CHECK: @trait_box({{\{\}\*|ptr}} noalias noundef nonnull align 1{{( %0)?}}, {{.+}} noalias noundef readonly align {{.*}} dereferenceable({{.*}}){{( %1)?}})
 #[no_mangle]
-pub fn trait_box(_: Box<dyn Drop>) {
+pub fn trait_box(_: Box<dyn Drop + Unpin>) {
 }
 
 // CHECK: { {{i8\*|ptr}}, {{i8\*|ptr}} } @trait_option({{i8\*|ptr}} noalias noundef align 1 %x.0, {{i8\*|ptr}} %x.1)
 #[no_mangle]
-pub fn trait_option(x: Option<Box<dyn Drop>>) -> Option<Box<dyn Drop>> {
+pub fn trait_option(x: Option<Box<dyn Drop + Unpin>>) -> Option<Box<dyn Drop + Unpin>> {
   x
 }