Skip to content

Commit

Permalink
Auto merge of rust-lang#135344 - scottmcm:safe-dangling, r=<try>
Browse files Browse the repository at this point in the history
Less unsafe in `dangling`/`without_provenance`

This PR was inspired by the new `NonNull::without_provenance` (cc rust-lang#135243 (comment)) since it made me realize that we could write `NonNull::dangling` in completely-safe code using other existing things.

Then doing that led me to a few more places that could be simplified, like now that GVN will optimize Transmute-then-PtrToPtr, we can just implement `ptr::without_provenance` by calling `ptr::without_provenance_mut` since the shipped rlib of `core` ends up with the same single statement as the implementation (thanks to GVN merging the steps) and thus there's no need to duplicate the `transmute` -- and more importantly, no need to repeat a long safety comment.

There did end up being a couple of other changes needed to avoid exploding certain bits of MIR, though -- like `<Box<[i32]>>::default()`'s MIR originally got way worse as certain things didn't inline, or had a bunch of extraneous UbChecks -- so there's a couple of other changes to solve that.
  • Loading branch information
bors committed Jan 14, 2025
2 parents a48e7b0 + c0d5988 commit f97ac68
Show file tree
Hide file tree
Showing 18 changed files with 272 additions and 457 deletions.
12 changes: 9 additions & 3 deletions library/core/src/ptr/alignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ impl Alignment {
/// but in an `Alignment` instead of a `usize`.
#[unstable(feature = "ptr_alignment_type", issue = "102070")]
#[inline]
#[must_use]
pub const fn of<T>() -> Self {
// SAFETY: rustc ensures that type alignment is always a power of two.
unsafe { Alignment::new_unchecked(mem::align_of::<T>()) }
// This can't actually panic since type alignment is always a power of two.
const { Alignment::new(mem::align_of::<T>()).unwrap() }
}

/// Creates an `Alignment` from a `usize`, or returns `None` if it's
Expand Down Expand Up @@ -95,8 +96,13 @@ impl Alignment {
#[unstable(feature = "ptr_alignment_type", issue = "102070")]
#[inline]
pub const fn as_nonzero(self) -> NonZero<usize> {
// This transmutes directly to avoid the UbCheck in `NonZero::new_unchecked`
// since there's no way for the user to trip that check anyway -- the
// validity invariant of the type would have to have been broken earlier --
// and emitting it in an otherwise simple method is bad for compile time.

// SAFETY: All the discriminants are non-zero.
unsafe { NonZero::new_unchecked(self.as_usize()) }
unsafe { mem::transmute::<Alignment, NonZero<usize>>(self) }
}

/// Returns the base-2 logarithm of the alignment.
Expand Down
11 changes: 3 additions & 8 deletions library/core/src/ptr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -596,12 +596,7 @@ pub const fn null_mut<T: ?Sized + Thin>() -> *mut T {
#[stable(feature = "strict_provenance", since = "1.84.0")]
#[rustc_const_stable(feature = "strict_provenance", since = "1.84.0")]
pub const fn without_provenance<T>(addr: usize) -> *const T {
// An int-to-pointer transmute currently has exactly the intended semantics: it creates a
// pointer without provenance. Note that this is *not* a stable guarantee about transmute
// semantics, it relies on sysroot crates having special status.
// SAFETY: every valid integer is also a valid pointer (as long as you don't dereference that
// pointer).
unsafe { mem::transmute(addr) }
without_provenance_mut(addr)
}

/// Creates a new pointer that is dangling, but non-null and well-aligned.
Expand All @@ -618,7 +613,7 @@ pub const fn without_provenance<T>(addr: usize) -> *const T {
#[stable(feature = "strict_provenance", since = "1.84.0")]
#[rustc_const_stable(feature = "strict_provenance", since = "1.84.0")]
pub const fn dangling<T>() -> *const T {
without_provenance(mem::align_of::<T>())
dangling_mut()
}

/// Creates a pointer with the given address and no [provenance][crate::ptr#provenance].
Expand Down Expand Up @@ -661,7 +656,7 @@ pub const fn without_provenance_mut<T>(addr: usize) -> *mut T {
#[stable(feature = "strict_provenance", since = "1.84.0")]
#[rustc_const_stable(feature = "strict_provenance", since = "1.84.0")]
pub const fn dangling_mut<T>() -> *mut T {
without_provenance_mut(mem::align_of::<T>())
NonNull::dangling().as_ptr()
}

/// Converts an address back to a pointer, picking up some previously 'exposed'
Expand Down
16 changes: 7 additions & 9 deletions library/core/src/ptr/non_null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,12 @@ impl<T: Sized> NonNull<T> {
///
/// This is a [Strict Provenance][crate::ptr#strict-provenance] API.
#[unstable(feature = "nonnull_provenance", issue = "135243")]
#[must_use]
#[inline]
pub const fn without_provenance(addr: NonZero<usize>) -> Self {
let pointer = crate::ptr::without_provenance(addr.get());
// SAFETY: we know `addr` is non-zero.
unsafe {
let ptr = crate::ptr::without_provenance_mut(addr.get());
NonNull::new_unchecked(ptr)
}
unsafe { NonNull { pointer } }
}

/// Creates a new `NonNull` that is dangling, but well-aligned.
Expand All @@ -123,11 +123,8 @@ impl<T: Sized> NonNull<T> {
#[must_use]
#[inline]
pub const fn dangling() -> Self {
// SAFETY: ptr::dangling_mut() returns a non-null well-aligned pointer.
unsafe {
let ptr = crate::ptr::dangling_mut::<T>();
NonNull::new_unchecked(ptr)
}
let align = crate::ptr::Alignment::of::<T>();
NonNull::without_provenance(align.as_nonzero())
}

/// Converts an address back to a mutable pointer, picking up some previously 'exposed'
Expand All @@ -137,6 +134,7 @@ impl<T: Sized> NonNull<T> {
///
/// This is an [Exposed Provenance][crate::ptr#exposed-provenance] API.
#[unstable(feature = "nonnull_provenance", issue = "135243")]
#[inline]
pub fn with_exposed_provenance(addr: NonZero<usize>) -> Self {
// SAFETY: we know `addr` is non-zero.
unsafe {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,23 @@
scope 4 (inlined Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let _6: *mut [bool; 0];
let mut _6: std::num::NonZero<usize>;
scope 6 {
scope 10 (inlined NonNull::<[bool; 0]>::new_unchecked) {
let mut _8: bool;
let _9: ();
let mut _10: *mut ();
let mut _11: *const [bool; 0];
scope 11 (inlined core::ub_checks::check_language_ub) {
scope 12 (inlined core::ub_checks::check_language_ub::runtime) {
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _7: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
}
scope 12 (inlined without_provenance::<[bool; 0]>) {
scope 13 (inlined without_provenance_mut::<[bool; 0]>) {
}
}
}
}
scope 7 (inlined dangling_mut::<[bool; 0]>) {
let mut _7: usize;
scope 8 (inlined align_of::<[bool; 0]>) {
}
scope 9 (inlined without_provenance_mut::<[bool; 0]>) {
}
scope 7 (inlined std::ptr::Alignment::of::<[bool; 0]>) {
}
}
}
Expand All @@ -44,54 +42,31 @@
StorageLive(_1);
StorageLive(_2);
StorageLive(_3);
StorageLive(_9);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
_6 = const NonZero::<usize>(core::num::nonzero::private::NonZeroUsizeInner(1_usize));
StorageLive(_7);
_7 = const 1_usize;
_6 = const {0x1 as *mut [bool; 0]};
StorageLive(_11);
StorageLive(_8);
_8 = UbChecks();
switchInt(move _8) -> [0: bb4, otherwise: bb2];
}

bb1: {
StorageDead(_1);
return;
}

bb2: {
StorageLive(_10);
_10 = const {0x1 as *mut ()};
_9 = NonNull::<T>::new_unchecked::precondition_check(const {0x1 as *mut ()}) -> [return: bb3, unwind unreachable];
}

bb3: {
StorageDead(_10);
goto -> bb4;
}

bb4: {
StorageDead(_8);
_11 = const {0x1 as *const [bool; 0]};
_7 = const {0x1 as *const [bool; 0]};
_5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_11);
StorageDead(_7);
StorageDead(_6);
_4 = const Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
_3 = const Unique::<[bool]> {{ pointer: NonNull::<[bool]> {{ pointer: Indirect { alloc_id: ALLOC0, offset: Size(0 bytes) }: *const [bool] }}, _marker: PhantomData::<[bool]> }};
StorageDead(_4);
_2 = const Box::<[bool]>(Unique::<[bool]> {{ pointer: NonNull::<[bool]> {{ pointer: Indirect { alloc_id: ALLOC1, offset: Size(0 bytes) }: *const [bool] }}, _marker: PhantomData::<[bool]> }}, std::alloc::Global);
StorageDead(_9);
StorageDead(_3);
_1 = const A {{ foo: Box::<[bool]>(Unique::<[bool]> {{ pointer: NonNull::<[bool]> {{ pointer: Indirect { alloc_id: ALLOC2, offset: Size(0 bytes) }: *const [bool] }}, _marker: PhantomData::<[bool]> }}, std::alloc::Global) }};
StorageDead(_2);
_0 = const ();
drop(_1) -> [return: bb1, unwind unreachable];
}

bb1: {
StorageDead(_1);
return;
}
}

ALLOC2 (size: 8, align: 4) { .. }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,23 @@
scope 4 (inlined Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let _6: *mut [bool; 0];
let mut _6: std::num::NonZero<usize>;
scope 6 {
scope 10 (inlined NonNull::<[bool; 0]>::new_unchecked) {
let mut _8: bool;
let _9: ();
let mut _10: *mut ();
let mut _11: *const [bool; 0];
scope 11 (inlined core::ub_checks::check_language_ub) {
scope 12 (inlined core::ub_checks::check_language_ub::runtime) {
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _7: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
}
scope 12 (inlined without_provenance::<[bool; 0]>) {
scope 13 (inlined without_provenance_mut::<[bool; 0]>) {
}
}
}
}
scope 7 (inlined dangling_mut::<[bool; 0]>) {
let mut _7: usize;
scope 8 (inlined align_of::<[bool; 0]>) {
}
scope 9 (inlined without_provenance_mut::<[bool; 0]>) {
}
scope 7 (inlined std::ptr::Alignment::of::<[bool; 0]>) {
}
}
}
Expand All @@ -44,58 +42,35 @@
StorageLive(_1);
StorageLive(_2);
StorageLive(_3);
StorageLive(_9);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
_6 = const NonZero::<usize>(core::num::nonzero::private::NonZeroUsizeInner(1_usize));
StorageLive(_7);
_7 = const 1_usize;
_6 = const {0x1 as *mut [bool; 0]};
StorageLive(_11);
StorageLive(_8);
_8 = UbChecks();
switchInt(move _8) -> [0: bb5, otherwise: bb3];
}

bb1: {
StorageDead(_1);
return;
}

bb2 (cleanup): {
resume;
}

bb3: {
StorageLive(_10);
_10 = const {0x1 as *mut ()};
_9 = NonNull::<T>::new_unchecked::precondition_check(const {0x1 as *mut ()}) -> [return: bb4, unwind unreachable];
}

bb4: {
StorageDead(_10);
goto -> bb5;
}

bb5: {
StorageDead(_8);
_11 = const {0x1 as *const [bool; 0]};
_7 = const {0x1 as *const [bool; 0]};
_5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_11);
StorageDead(_7);
StorageDead(_6);
_4 = const Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
_3 = const Unique::<[bool]> {{ pointer: NonNull::<[bool]> {{ pointer: Indirect { alloc_id: ALLOC0, offset: Size(0 bytes) }: *const [bool] }}, _marker: PhantomData::<[bool]> }};
StorageDead(_4);
_2 = const Box::<[bool]>(Unique::<[bool]> {{ pointer: NonNull::<[bool]> {{ pointer: Indirect { alloc_id: ALLOC1, offset: Size(0 bytes) }: *const [bool] }}, _marker: PhantomData::<[bool]> }}, std::alloc::Global);
StorageDead(_9);
StorageDead(_3);
_1 = const A {{ foo: Box::<[bool]>(Unique::<[bool]> {{ pointer: NonNull::<[bool]> {{ pointer: Indirect { alloc_id: ALLOC2, offset: Size(0 bytes) }: *const [bool] }}, _marker: PhantomData::<[bool]> }}, std::alloc::Global) }};
StorageDead(_2);
_0 = const ();
drop(_1) -> [return: bb1, unwind: bb2];
}

bb1: {
StorageDead(_1);
return;
}

bb2 (cleanup): {
resume;
}
}

ALLOC2 (size: 8, align: 4) { .. }
Expand Down
Loading

0 comments on commit f97ac68

Please sign in to comment.