Auto merge of #117035 - scottmcm:slice-index-intrinsic, r=<try>

Use MIR slice indexing for get(_unchecked)(_mut) on slices

Rather than having the MIR also need to drop down to pointers and `offset`.

Inspired by #116915

r? `@ghost`

Author: bors

compiler/rustc_hir_analysis/src/check/intrinsic.rs

@@ -216,6 +216,7 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
 
             sym::type_name => (1, Vec::new(), Ty::new_static_str(tcx)),
             sym::type_id => (1, Vec::new(), tcx.types.u128),
+            sym::slice_index => (2, vec![param(0), tcx.types.usize], param(1)),
             sym::offset => (2, vec![param(0), param(1)], param(0)),
             sym::arith_offset => (
                 1,

compiler/rustc_mir_transform/src/lower_intrinsics.rs

@@ -234,6 +234,43 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
                             terminator.kind = TerminatorKind::Goto { target };
                         }
                     }
+                    sym::slice_index => {
+                        let target = target.unwrap();
+
+                        let elem =
+                            if let Some(slice_ptr) = args[0].place()
+                            && let Some(index_place) = args[1].place()
+                            && let Some(index) = index_place.as_local()
+                            {
+                                let slice = tcx.mk_place_deref(slice_ptr);
+                                tcx.mk_place_index(slice, index)
+                            } else {
+                                span_bug!(
+                                    terminator.source_info.span,
+                                    "Invalid call to slice_index"
+                                );
+                            };
+
+                        let elem_ptr =
+                            match generic_args.type_at(1).kind() {
+                                ty::Ref(region, _, Mutability::Mut) =>
+                                    Rvalue::Ref(*region, BorrowKind::Mut { kind: MutBorrowKind::Default }, elem),
+                                ty::Ref(region, _, Mutability::Not) =>
+                                    Rvalue::Ref(*region, BorrowKind::Shared, elem),
+                                ty::RawPtr(ty_and_mut) =>
+                                    Rvalue::AddressOf(ty_and_mut.mutbl, elem),
+                                x => bug!("Invalid return type for slice_index: {x:?}"),
+                            };
+
+                        block.statements.push(Statement {
+                            source_info: terminator.source_info,
+                            kind: StatementKind::Assign(Box::new((
+                                *destination,
+                                elem_ptr,
+                            ))),
+                        });
+                        terminator.kind = TerminatorKind::Goto { target };
+                    }
                     sym::offset => {
                         let target = target.unwrap();
                         let Ok([ptr, delta]) = <[_; 2]>::try_from(std::mem::take(args)) else {

compiler/rustc_span/src/symbol.rs

@@ -1528,6 +1528,7 @@ symbols! {
         slice,
         slice_from_raw_parts,
         slice_from_raw_parts_mut,
+        slice_index,
         slice_len_fn,
         slice_patterns,
         slicing_syntax,

library/core/src/intrinsics.rs

@@ -74,6 +74,7 @@ pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) {
     unsafe { crate::ptr::drop_in_place(to_drop) }
 }
 
+#[allow(private_bounds)]
 extern "rust-intrinsic" {
     // N.B., these intrinsics take raw pointers because they mutate aliased
     // memory, which is not valid for either `&` or `&mut`.
@@ -1410,6 +1411,39 @@ extern "rust-intrinsic" {
     #[rustc_nounwind]
     pub fn needs_drop<T: ?Sized>() -> bool;
 
+    /// Indexes into a slice using `ProjectionElem::Index`
+    ///
+    /// Depending on the return type, this lowers to one of the following in MIR:
+    /// - `&raw const (*slice)[index]`, for `*const _`
+    /// - `&raw mut (*slice)[index]`, for `*mut _`
+    /// - `&(*slice)[index]`, for `&_`
+    /// - `&mut (*slice)[index]`, for `&mut _`
+    ///
+    /// This is intended for use in implementing `[T]::get_unchecked_mut`,
+    /// `[T]::get`, and friends. It should not be used elsewhere; ICEs from
+    /// misuse in callers of intrinsics are *not* considered compiler bugs.
+    ///
+    /// While semantically unnecessary, as it could be done via `offset`, this
+    /// reduces and simplifies the MIR produced for common operations.
+    ///
+    /// # Safety
+    ///
+    /// Requires `index < slice.len()` and that `slice` points to a valid slice.
+    ///
+    /// If the return type is a reference (mutable or shared), then you must
+    /// ensure that its lifetime is no longer than the input's.
+    #[cfg(not(bootstrap))]
+    #[must_use = "returns a new pointer rather than modifying its argument"]
+    #[rustc_const_stable(feature = "const_ptr_offset", since = "1.61.0")]
+    #[rustc_nounwind]
+    pub fn slice_index<
+        Slice: PointerOrReference<To = [Item::To]>,
+        Item: PointerOrReference<To: Sized>,
+    >(
+        slice: Slice,
+        index: usize,
+    ) -> Item;
+
     /// Calculates the offset from a pointer.
     ///
     /// This is implemented as an intrinsic to avoid converting to and from an
@@ -1430,7 +1464,7 @@ extern "rust-intrinsic" {
     #[must_use = "returns a new pointer rather than modifying its argument"]
     #[rustc_const_stable(feature = "const_ptr_offset", since = "1.61.0")]
     #[rustc_nounwind]
-    pub fn offset<Ptr, Delta>(dst: Ptr, offset: Delta) -> Ptr;
+    pub fn offset<Ptr: PointerOrReference<To: Sized>, Delta>(dst: Ptr, offset: Delta) -> Ptr;
 
     /// Calculates the offset from a pointer, potentially wrapping.
     ///
@@ -2855,3 +2889,20 @@ pub const unsafe fn write_bytes<T>(dst: *mut T, val: u8, count: usize) {
         write_bytes(dst, val, count)
     }
 }
+
+/// No semantic meaning; just to help reduce mistakes with generic intrinsics
+trait PointerOrReference {
+    type To: ?Sized;
+}
+impl<T: ?Sized> PointerOrReference for &T {
+    type To = T;
+}
+impl<T: ?Sized> PointerOrReference for &mut T {
+    type To = T;
+}
+impl<T: ?Sized> PointerOrReference for *const T {
+    type To = T;
+}
+impl<T: ?Sized> PointerOrReference for *mut T {
+    type To = T;
+}

library/core/src/slice/index.rs

@@ -2,6 +2,8 @@
 
 use crate::intrinsics::assert_unsafe_precondition;
 use crate::intrinsics::const_eval_select;
+#[cfg(not(bootstrap))]
+use crate::intrinsics::slice_index;
 use crate::intrinsics::unchecked_sub;
 use crate::ops;
 use crate::ptr;
@@ -213,14 +215,38 @@ unsafe impl<T> SliceIndex<[T]> for usize {
 
     #[inline]
     fn get(self, slice: &[T]) -> Option<&T> {
-        // SAFETY: `self` is checked to be in bounds.
-        if self < slice.len() { unsafe { Some(&*self.get_unchecked(slice)) } } else { None }
+        if self < slice.len() {
+            #[cfg(bootstrap)]
+            // SAFETY: `self` is checked to be in bounds.
+            unsafe {
+                Some(&*self.get_unchecked(slice))
+            }
+            #[cfg(not(bootstrap))]
+            // SAFETY: `self` is checked to be in bounds.
+            unsafe {
+                Some(slice_index(slice, self))
+            }
+        } else {
+            None
+        }
     }
 
     #[inline]
     fn get_mut(self, slice: &mut [T]) -> Option<&mut T> {
-        // SAFETY: `self` is checked to be in bounds.
-        if self < slice.len() { unsafe { Some(&mut *self.get_unchecked_mut(slice)) } } else { None }
+        if self < slice.len() {
+            #[cfg(bootstrap)]
+            // SAFETY: `self` is checked to be in bounds.
+            unsafe {
+                Some(&mut *self.get_unchecked_mut(slice))
+            }
+            #[cfg(not(bootstrap))]
+            // SAFETY: `self` is checked to be in bounds.
+            unsafe {
+                Some(slice_index(slice, self))
+            }
+        } else {
+            None
+        }
     }
 
     #[inline]
@@ -231,11 +257,18 @@ unsafe impl<T> SliceIndex<[T]> for usize {
         // `self` is in bounds of `slice` so `self` cannot overflow an `isize`,
         // so the call to `add` is safe.
         unsafe {
-            assert_unsafe_precondition!(
-                "slice::get_unchecked requires that the index is within the slice",
-                [T](this: usize, slice: *const [T]) => this < slice.len()
-            );
-            slice.as_ptr().add(self)
+            #[cfg(bootstrap)]
+            {
+                assert_unsafe_precondition!(
+                    "slice::get_unchecked requires that the index is within the slice",
+                    [T](this: usize, slice: *const [T]) => this < slice.len()
+                );
+                slice.as_ptr().add(self)
+            }
+            #[cfg(not(bootstrap))]
+            {
+                slice_index(slice, this)
+            }
         }
     }
 
@@ -244,11 +277,18 @@ unsafe impl<T> SliceIndex<[T]> for usize {
         let this = self;
         // SAFETY: see comments for `get_unchecked` above.
         unsafe {
-            assert_unsafe_precondition!(
-                "slice::get_unchecked_mut requires that the index is within the slice",
-                [T](this: usize, slice: *mut [T]) => this < slice.len()
-            );
-            slice.as_mut_ptr().add(self)
+            #[cfg(bootstrap)]
+            {
+                assert_unsafe_precondition!(
+                    "slice::get_unchecked_mut requires that the index is within the slice",
+                    [T](this: usize, slice: *mut [T]) => this < slice.len()
+                );
+                slice.as_mut_ptr().add(self)
+            }
+            #[cfg(not(bootstrap))]
+            {
+                slice_index(slice, this)
+            }
         }
     }
 

src/tools/miri/tests/fail/stacked_borrows/zst_slice.rs

@@ -1,11 +1,10 @@
 //@compile-flags: -Zmiri-strict-provenance
-//@error-in-other-file: /retag .* tag does not exist in the borrow stack/
 
 fn main() {
     unsafe {
         let a = [1, 2, 3];
         let s = &a[0..0];
         assert_eq!(s.len(), 0);
-        assert_eq!(*s.get_unchecked(1), 2);
+        assert_eq!(*s.as_ptr().add(1), 2); //~ ERROR: /retag .* tag does not exist in the borrow stack/
     }
 }

src/tools/miri/tests/fail/stacked_borrows/zst_slice.stderr

@@ -1,26 +1,22 @@
 error: Undefined Behavior: trying to retag from <TAG> for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location
-  --> RUSTLIB/core/src/slice/mod.rs:LL:CC
+  --> $DIR/zst_slice.rs:LL:CC
    |
-LL |         unsafe { &*index.get_unchecked(self) }
-   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |                  |
-   |                  trying to retag from <TAG> for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location
-   |                  this error occurs as part of retag at ALLOC[0x4..0x8]
+LL |         assert_eq!(*s.as_ptr().add(1), 2);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         |
+   |         trying to retag from <TAG> for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location
+   |         this error occurs as part of retag at ALLOC[0x4..0x8]
    |
    = 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
 help: <TAG> would have been created here, but this is a zero-size retag ([0x0..0x0]) so the tag in question does not exist anywhere
   --> $DIR/zst_slice.rs:LL:CC
    |
-LL |         assert_eq!(*s.get_unchecked(1), 2);
-   |                     ^^^^^^^^^^^^^^^^^^
+LL |         assert_eq!(*s.as_ptr().add(1), 2);
+   |                     ^^^^^^^^^^
    = note: BACKTRACE (of the first span):
-   = note: inside `core::slice::<impl [i32]>::get_unchecked::<usize>` at RUSTLIB/core/src/slice/mod.rs:LL:CC
-note: inside `main`
-  --> $DIR/zst_slice.rs:LL:CC
-   |
-LL |         assert_eq!(*s.get_unchecked(1), 2);
-   |                     ^^^^^^^^^^^^^^^^^^
+   = note: inside `main` at RUSTLIB/core/src/macros/mod.rs:LL:CC
+   = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
 

src/tools/miri/tests/fail/uninit_byte_read.rs

@@ -1,7 +1,7 @@
 //@compile-flags: -Zmiri-disable-stacked-borrows
 fn main() {
     let v: Vec<u8> = Vec::with_capacity(10);
-    let undef = unsafe { *v.get_unchecked(5) }; //~ ERROR: uninitialized
+    let undef = unsafe { *v.as_ptr().add(5) }; //~ ERROR: uninitialized
     let x = undef + 1;
     panic!("this should never print: {}", x);
 }

src/tools/miri/tests/fail/uninit_byte_read.stderr

@@ -1,8 +1,8 @@
 error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
   --> $DIR/uninit_byte_read.rs:LL:CC
    |
-LL |     let undef = unsafe { *v.get_unchecked(5) };
-   |                          ^^^^^^^^^^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
+LL |     let undef = unsafe { *v.as_ptr().add(5) };
+   |                          ^^^^^^^^^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information