const-eval: always do mem-to-mem copies if there might be padding involved

Author: RalfJung

compiler/rustc_const_eval/src/interpret/place.rs

@@ -876,10 +876,29 @@ where
                 dest.layout().ty,
             );
         }
+        // If the source has padding, we want to always do the mem-to-mem copy to ensure consistent
+        // padding in the target independent of layout choices.
+        let src_has_padding = match src.layout().backend_repr {
+            BackendRepr::Scalar(_) => false,
+            BackendRepr::ScalarPair(left, right) => {
+                let left_size = left.size(self);
+                let right_size = right.size(self);
+                // We have padding if the sizes don't add up to the total.
+                left_size + right_size != src.layout().size
+            }
+            // Everything else can only exist in memory anyway.
+            _ => true,
+        };
 
-        // Let us see if the layout is simple so we take a shortcut,
-        // avoid force_allocation.
-        let src = match self.read_immediate_raw(src)? {
+        let src_val = if src_has_padding {
+            // Do our best to get an mplace. If there's no mplace, then this is stored as an
+            // "optimized" local, so its padding is definitely uninitialized and we are fine.
+            src.to_op(self)?.as_mplace_or_imm()
+        } else {
+            // Do our best to get an immediate, to avoid having to force_allocate the destination.
+            self.read_immediate_raw(src)?
+        };
+        let src = match src_val {
             Right(src_val) => {
                 assert!(!src.layout().is_unsized());
                 assert!(!dest.layout().is_unsized());

compiler/rustc_const_eval/src/interpret/projection.rs

@@ -97,7 +97,7 @@ pub trait Projectable<'tcx, Prov: Provenance>: Sized + std::fmt::Debug {
     }
 
     /// Convert this to an `OpTy`. This might be an irreversible transformation, but is useful for
-    /// reading from this thing.
+    /// reading from this thing. This will never actually do a read from memory!
     fn to_op<M: Machine<'tcx, Provenance = Prov>>(
         &self,
         ecx: &InterpCx<'tcx, M>,

tests/ui/consts/const-eval/ptr_fragments_in_final.rs

@@ -1,5 +1,4 @@
 //! Test that we properly error when there is a pointer fragment in the final value.
-//@ ignore-test: disabled due to <https://github.com/rust-lang/rust/issues/146291>
 
 use std::{mem::{self, MaybeUninit}, ptr};
 
@@ -13,14 +12,50 @@ const unsafe fn memcpy(dst: *mut Byte, src: *const Byte, n: usize) {
     }
 }
 
-const MEMCPY_RET: MaybeUninit<*const i32> = unsafe { //~ERROR: partial pointer in final value
+const MEMCPY_RET: MaybeUninit<*const i32> = unsafe { //~vvv ERROR: unable to read parts of a pointer
     let ptr = &42;
     let mut ptr2 = MaybeUninit::new(ptr::null::<i32>());
     memcpy(&mut ptr2 as *mut _ as *mut _, &ptr as *const _ as *const _, mem::size_of::<&i32>() / 2);
     // Return in a MaybeUninit so it does not get treated as a scalar.
     ptr2
 };
 
+/// This has pointer bytes in the padding of the memory that the final value is read from.
+/// To ensure consistent behavior, we want to *always* copy that padding, even if the value
+/// could be represented as a more efficient ScalarPair. Hence this must fail to compile.
+fn fragment_in_padding() -> impl Copy {
+    #[cfg(target_pointer_width = "64")]
+    type TwoUsize = u128;
+    #[cfg(target_pointer_width = "32")]
+    type TwoUsize = u64;
+
+    #[repr(C, align(16))]
+    #[derive(Clone, Copy)]
+    struct Thing {
+        x: TwoUsize,
+        y: usize,
+        // one pointer worth of padding
+    }
+    const _: () = assert!(mem::size_of::<Thing>() == 4 * mem::size_of::<usize>());
+    #[derive(Clone, Copy)]
+    union PreservePad {
+        thing: Thing,
+        bytes: [u8; mem::size_of::<Thing>()],
+    }
+
+    const A: Thing = unsafe { //~vvvvvv ERROR: unable to read parts of a pointer
+        let mut buffer = [PreservePad { bytes: [0u8; mem::size_of::<Thing>()] }; 2];
+        // The offset half-way through the padding, so that copying one `Thing` copies exactly
+        // half the pointer.
+        let offset = mem::size_of::<Thing>() - mem::size_of::<usize>()/2;
+        (&raw mut buffer).cast::<&i32>().byte_add(offset).write_unaligned(&1);
+        buffer[0].thing
+    };
+
+    A
+}
+
 fn main() {
     assert_eq!(unsafe { MEMCPY_RET.assume_init().read() }, 42);
+    fragment_in_padding();
 }

tests/ui/consts/const-eval/ptr_fragments_in_final.stderr

@@ -1,10 +1,26 @@
-error: encountered partial pointer in final value of constant
-  --> $DIR/ptr_fragments_in_final.rs:15:1
+error[E0080]: unable to read parts of a pointer from memory at ALLOC0
+  --> $DIR/ptr_fragments_in_final.rs:18:5
    |
-LL | const MEMCPY_RET: MaybeUninit<*const i32> = unsafe {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     memcpy(&mut ptr2 as *mut _ as *mut _, &ptr as *const _ as *const _, mem::size_of::<&i32>() / 2);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ evaluation of `MEMCPY_RET` failed inside this call
    |
-   = note: while pointers can be broken apart into individual bytes during const-evaluation, only complete pointers (with all their bytes in the right order) are supported in the final value
+   = help: this code performed an operation that depends on the underlying bytes representing a pointer
+   = help: the absolute address of a pointer is not known at compile-time, so such operations are not supported
+note: inside `memcpy`
+  --> $DIR/ptr_fragments_in_final.rs:10:26
+   |
+LL |         dst.add(i).write(src.add(i).read());
+   |                          ^^^^^^^^^^^^^^^^^ the failure occurred here
+
+error[E0080]: unable to read parts of a pointer from memory at ALLOC1
+  --> $DIR/ptr_fragments_in_final.rs:52:9
+   |
+LL |         buffer[0].thing
+   |         ^^^^^^^^^^^^^^^ evaluation of `fragment_in_padding::A` failed here
+   |
+   = help: this code performed an operation that depends on the underlying bytes representing a pointer
+   = help: the absolute address of a pointer is not known at compile-time, so such operations are not supported
 
-error: aborting due to 1 previous error
+error: aborting due to 2 previous errors
 
+For more information about this error, try `rustc --explain E0080`.