Auto merge of rust-lang#87827 - eddyb:wrapperless-mem-replace, r=m-ou-se

Avoid using the `copy_nonoverlapping` wrapper through `mem::replace`. This is a much simpler way to achieve the pre-rust-lang#86003 behavior of `mem::replace` not needing dynamically-sized `memcpy`s (at least before inlining), than re-doing rust-lang#81238 (which needs rust-lang#86699 or something similar). I didn't notice it until recently, but `ptr::write` already explicitly avoided using the wrapper, while `ptr::read` just called the wrapper (and was the reason for us observing any behavior change from rust-lang#86003 in Rust-GPU). <hr/> The codegen test I've added fails without the change to `core::ptr::read` like this (ignore the `v0` mangling, I was using a worktree with it turned on by default, for this): ```llvm 13: ; core::intrinsics::copy_nonoverlapping::<u8> 14: ; Function Attrs: inlinehint nonlazybind uwtable 15: define internal void `@_RINvNtCscK5tvALCJol_4core10intrinsics19copy_nonoverlappinghECsaS4X3EinRE8_25mem_replace_direct_memcpy(i8*` %src, i8* %dst, i64 %count) unnamed_addr #0 { 16: start: 17: %0 = mul i64 %count, 1 18: call void `@llvm.memcpy.p0i8.p0i8.i64(i8*` align 1 %dst, i8* align 1 %src, i64 %0, i1 false) not:17 !~~~~~~~~~~~~~~~~~~~~~ error: no match expected 19: ret void 20: } ``` With the `core::ptr::read` change, `core::intrinsics::copy_nonoverlapping` doesn't get instantiated and the test passes. <hr/> r? `@m-ou-se` cc `@nagisa` (codegen test) `@oli-obk` / `@RalfJung` (miri diagnostic changes)
rust-lang-ci · Aug 8, 2021 · 4e886d6 · 4e886d6
2 parents e8c25f2 + a1d014b
commit 4e886d6
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 30 deletions.
diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs
@@ -685,6 +685,13 @@ pub const unsafe fn replace<T>(dst: *mut T, mut src: T) -> T {
 #[stable(feature = "rust1", since = "1.0.0")]
 #[rustc_const_unstable(feature = "const_ptr_read", issue = "80377")]
 pub const unsafe fn read<T>(src: *const T) -> T {
+    // We are calling the intrinsics directly to avoid function calls in the generated code
+    // as `intrinsics::copy_nonoverlapping` is a wrapper function.
+    extern "rust-intrinsic" {
+        #[rustc_const_unstable(feature = "const_intrinsic_copy", issue = "80697")]
+        fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize);
+    }
+
     let mut tmp = MaybeUninit::<T>::uninit();
     // SAFETY: the caller must guarantee that `src` is valid for reads.
     // `src` cannot overlap `tmp` because `tmp` was just allocated on

diff --git a/src/test/codegen/mem-replace-direct-memcpy.rs b/src/test/codegen/mem-replace-direct-memcpy.rs
@@ -0,0 +1,25 @@
+// This test ensures that `mem::replace::<T>` only ever calls `@llvm.memcpy`
+// with `size_of::<T>()` as the size, and never goes through any wrapper that
+// may e.g. multiply `size_of::<T>()` with a variable "count" (which is only
+// known to be `1` after inlining).
+
+// compile-flags: -C no-prepopulate-passes
+
+#![crate_type = "lib"]
+
+pub fn replace_byte(dst: &mut u8, src: u8) -> u8 {
+    std::mem::replace(dst, src)
+}
+
+// NOTE(eddyb) the `CHECK-NOT`s ensure that the only calls of `@llvm.memcpy` in
+// the entire output, are the two direct calls we want, from `ptr::{read,write}`.
+
+// CHECK-NOT: call void @llvm.memcpy
+// CHECK: ; core::ptr::read
+// CHECK-NOT: call void @llvm.memcpy
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i{{.*}}(i8* align 1 %{{.*}}, i8* align 1 %src, i{{.*}} 1, i1 false)
+// CHECK-NOT: call void @llvm.memcpy
+// CHECK: ; core::ptr::write
+// CHECK-NOT: call void @llvm.memcpy
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i{{.*}}(i8* align 1 %dst, i8* align 1 %src, i{{.*}} 1, i1 false)
+// CHECK-NOT: call void @llvm.memcpy
diff --git a/src/test/ui/const-ptr/out_of_bounds_read.stderr b/src/test/ui/const-ptr/out_of_bounds_read.stderr
@@ -1,35 +1,25 @@
 error[E0080]: evaluation of constant value failed
-  --> $SRC_DIR/core/src/intrinsics.rs:LL:COL
-   |
-LL |     unsafe { copy_nonoverlapping(src, dst, count) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |              |
-   |              memory access failed: alloc7 has size 4, so pointer to 4 bytes starting at offset 4 is out-of-bounds
-   |              inside `copy_nonoverlapping::<u32>` at $SRC_DIR/core/src/intrinsics.rs:LL:COL
-   |
-  ::: $SRC_DIR/core/src/ptr/mod.rs:LL:COL
+  --> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
    |
 LL |         copy_nonoverlapping(src, tmp.as_mut_ptr(), 1);
-   |         --------------------------------------------- inside `std::ptr::read::<u32>` at $SRC_DIR/core/src/ptr/mod.rs:LL:COL
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         |
+   |         memory access failed: alloc7 has size 4, so pointer to 4 bytes starting at offset 4 is out-of-bounds
+   |         inside `std::ptr::read::<u32>` at $SRC_DIR/core/src/ptr/mod.rs:LL:COL
    |
   ::: $DIR/out_of_bounds_read.rs:13:33
    |
 LL |     const _READ: u32 = unsafe { ptr::read(PAST_END_PTR) };
    |                                 ----------------------- inside `_READ` at $DIR/out_of_bounds_read.rs:13:33
 
 error[E0080]: evaluation of constant value failed
-  --> $SRC_DIR/core/src/intrinsics.rs:LL:COL
-   |
-LL |     unsafe { copy_nonoverlapping(src, dst, count) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |              |
-   |              memory access failed: alloc7 has size 4, so pointer to 4 bytes starting at offset 4 is out-of-bounds
-   |              inside `copy_nonoverlapping::<u32>` at $SRC_DIR/core/src/intrinsics.rs:LL:COL
-   |
-  ::: $SRC_DIR/core/src/ptr/mod.rs:LL:COL
+  --> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
    |
 LL |         copy_nonoverlapping(src, tmp.as_mut_ptr(), 1);
-   |         --------------------------------------------- inside `std::ptr::read::<u32>` at $SRC_DIR/core/src/ptr/mod.rs:LL:COL
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         |
+   |         memory access failed: alloc7 has size 4, so pointer to 4 bytes starting at offset 4 is out-of-bounds
+   |         inside `std::ptr::read::<u32>` at $SRC_DIR/core/src/ptr/mod.rs:LL:COL
    |
   ::: $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
    |
@@ -42,18 +32,13 @@ LL |     const _CONST_READ: u32 = unsafe { PAST_END_PTR.read() };
    |                                       ------------------- inside `_CONST_READ` at $DIR/out_of_bounds_read.rs:14:39
 
 error[E0080]: evaluation of constant value failed
-  --> $SRC_DIR/core/src/intrinsics.rs:LL:COL
-   |
-LL |     unsafe { copy_nonoverlapping(src, dst, count) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |              |
-   |              memory access failed: alloc7 has size 4, so pointer to 4 bytes starting at offset 4 is out-of-bounds
-   |              inside `copy_nonoverlapping::<u32>` at $SRC_DIR/core/src/intrinsics.rs:LL:COL
-   |
-  ::: $SRC_DIR/core/src/ptr/mod.rs:LL:COL
+  --> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
    |
 LL |         copy_nonoverlapping(src, tmp.as_mut_ptr(), 1);
-   |         --------------------------------------------- inside `std::ptr::read::<u32>` at $SRC_DIR/core/src/ptr/mod.rs:LL:COL
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         |
+   |         memory access failed: alloc7 has size 4, so pointer to 4 bytes starting at offset 4 is out-of-bounds
+   |         inside `std::ptr::read::<u32>` at $SRC_DIR/core/src/ptr/mod.rs:LL:COL
    |
   ::: $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL
    |