From 307e80c1a64e189c5711892b4b03f0454fcefb79 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 11 Jun 2022 23:20:00 -0700 Subject: [PATCH 1/3] =?UTF-8?q?rename=20PointerKind::Shared=20=E2=86=92=20?= =?UTF-8?q?SharedMutable=20to=20indicate=20this=20is=20NOT=20the=20usual?= =?UTF-8?q?=20shared=20reference?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- compiler/rustc_middle/src/ty/layout.rs | 8 ++++---- compiler/rustc_target/src/abi/mod.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index ab76ad5098413..3c27e36795e1d 100644 --- 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::SharedMutable } } } @@ -3285,7 +3285,7 @@ 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 => 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 index 6f4d073d70486..b8398daadf147 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -1350,7 +1350,7 @@ 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`. Frozen, From 5b7197af7fb379a84895084625b1eed47aa5c74f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 20 Jun 2022 20:51:15 -0700 Subject: [PATCH 2/3] do not mark interior mutable shared refs as dereferenceable --- compiler/rustc_middle/src/ty/layout.rs | 15 ++++++++++----- compiler/rustc_target/src/abi/mod.rs | 10 +++++++--- src/test/codegen/function-arguments.rs | 20 +++++++++++++++++++- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 3c27e36795e1d..dde55dd96554b 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2636,7 +2636,7 @@ where if ty.is_unpin(tcx.at(DUMMY_SP), cx.param_env()) { PointerKind::UniqueBorrowed } else { - PointerKind::SharedMutable + 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::SharedMutable | 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 index b8398daadf147..b35502d9ee42b 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -1352,13 +1352,17 @@ pub enum PointerKind { /// Most general case, we know no restrictions to tell LLVM. 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`, unlike `UniqueBorrowed`, it also has `noalias` on returns. + /// `&mut !Unpin`, which is `dereferenceable` but neither `noalias` nor `readonly`. + UniqueBorrowedPinned, + + /// `Box`, which is `noalias` (even on return types, unlike the above) but neither `readonly` + /// nor `dereferenceable`. UniqueOwned, } diff --git a/src/test/codegen/function-arguments.rs b/src/test/codegen/function-arguments.rs index ae6abe7a184c6..dda139be6fcae 100644 --- 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, } +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) { From 35c6dec921d3bac70fb488db193580176f603aa1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 22 Jun 2022 14:36:30 -0700 Subject: [PATCH 3/3] adjust UnsafeCell documentation --- library/core/src/cell.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index 8a37fadc56f4c..fb4454c94cb33 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -1766,15 +1766,24 @@ impl 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` 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` 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` 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