diff --git a/compiler/rustc_const_eval/src/interpret/call.rs b/compiler/rustc_const_eval/src/interpret/call.rs index 46720328ea4..99f0ac702c5 100644 --- a/compiler/rustc_const_eval/src/interpret/call.rs +++ b/compiler/rustc_const_eval/src/interpret/call.rs @@ -883,19 +883,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { .local_to_op(mir::RETURN_PLACE, None) .expect("return place should always be live"); let dest = self.frame().return_place.clone(); - let res = if self.stack().len() == 1 { - // The initializer of constants and statics will get validated separately - // after the constant has been fully evaluated. While we could fall back to the default - // code path, that will cause -Zenforce-validity to cycle on static initializers. - // Reading from a static's memory is not allowed during its evaluation, and will always - // trigger a cycle error. Validation must read from the memory of the current item. - // For Miri this means we do not validate the root frame return value, - // but Miri anyway calls `read_target_isize` on that so separate validation - // is not needed. - self.copy_op_no_dest_validation(&op, &dest) - } else { - self.copy_op_allow_transmute(&op, &dest) - }; + let res = self.copy_op_allow_transmute(&op, &dest); trace!("return value: {:?}", self.dump_place(&dest.into())); // We delay actually short-circuiting on this error until *after* the stack frame is // popped, since we want this error to be attributed to the caller, whose type defines diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index 04346af41fc..0664a882c1d 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -425,7 +425,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { self.write_scalar(result, dest)?; } sym::typed_swap_nonoverlapping => { - self.typed_swap_intrinsic(&args[0], &args[1])?; + self.typed_swap_nonoverlapping_intrinsic(&args[0], &args[1])?; } sym::vtable_size => { @@ -638,19 +638,35 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { } /// Does a *typed* swap of `*left` and `*right`. - fn typed_swap_intrinsic( + fn typed_swap_nonoverlapping_intrinsic( &mut self, left: &OpTy<'tcx, >::Provenance>, right: &OpTy<'tcx, >::Provenance>, ) -> InterpResult<'tcx> { let left = self.deref_pointer(left)?; let right = self.deref_pointer(right)?; - debug_assert_eq!(left.layout, right.layout); + assert_eq!(left.layout, right.layout); + assert!(left.layout.is_sized()); let kind = MemoryKind::Stack; let temp = self.allocate(left.layout, kind)?; - self.copy_op(&left, &temp)?; - self.copy_op(&right, &left)?; // this checks that they are non-overlapping - self.copy_op(&temp, &right)?; + self.copy_op(&left, &temp)?; // checks alignment of `left` + + // We want to always enforce non-overlapping, even if this is a scalar type. + // Therefore we directly use the underlying `mem_copy` here. + self.mem_copy(right.ptr(), left.ptr(), left.layout.size, /*nonoverlapping*/ true)?; + // This means we also need to do the validation of the value that used to be in `right` + // ourselves. This value is now in `left.` The one that started out in `left` already got + // validated by the copy above. + if M::enforce_validity(self, left.layout) { + self.validate_operand( + &left.clone().into(), + M::enforce_validity_recursively(self, left.layout), + /*reset_provenance_and_padding*/ true, + )?; + } + + self.copy_op(&temp, &right)?; // checks alignment of `right` + self.deallocate_ptr(temp.ptr(), None, kind)?; interp_ok(()) } diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 810e9356b26..0d974071619 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -773,22 +773,6 @@ where interp_ok(()) } - /// Copies the data from an operand to a place. - /// The layouts of the `src` and `dest` may disagree. - /// Does not perform validation of the destination. - /// The only known use case for this function is checking the return - /// value of a static during stack frame popping. - #[inline(always)] - pub(super) fn copy_op_no_dest_validation( - &mut self, - src: &impl Projectable<'tcx, M::Provenance>, - dest: &impl Writeable<'tcx, M::Provenance>, - ) -> InterpResult<'tcx> { - self.copy_op_inner( - src, dest, /* allow_transmute */ true, /* validate_dest */ false, - ) - } - /// Copies the data from an operand to a place. /// The layouts of the `src` and `dest` may disagree. #[inline(always)] @@ -797,9 +781,7 @@ where src: &impl Projectable<'tcx, M::Provenance>, dest: &impl Writeable<'tcx, M::Provenance>, ) -> InterpResult<'tcx> { - self.copy_op_inner( - src, dest, /* allow_transmute */ true, /* validate_dest */ true, - ) + self.copy_op_inner(src, dest, /* allow_transmute */ true) } /// Copies the data from an operand to a place. @@ -810,9 +792,7 @@ where src: &impl Projectable<'tcx, M::Provenance>, dest: &impl Writeable<'tcx, M::Provenance>, ) -> InterpResult<'tcx> { - self.copy_op_inner( - src, dest, /* allow_transmute */ false, /* validate_dest */ true, - ) + self.copy_op_inner(src, dest, /* allow_transmute */ false) } /// Copies the data from an operand to a place. @@ -824,22 +804,21 @@ where src: &impl Projectable<'tcx, M::Provenance>, dest: &impl Writeable<'tcx, M::Provenance>, allow_transmute: bool, - validate_dest: bool, ) -> InterpResult<'tcx> { // These are technically *two* typed copies: `src` is a not-yet-loaded value, - // so we're going a typed copy at `src` type from there to some intermediate storage. + // so we're doing a typed copy at `src` type from there to some intermediate storage. // And then we're doing a second typed copy from that intermediate storage to `dest`. // But as an optimization, we only make a single direct copy here. // Do the actual copy. self.copy_op_no_validate(src, dest, allow_transmute)?; - if validate_dest && M::enforce_validity(self, dest.layout()) { + if M::enforce_validity(self, dest.layout()) { let dest = dest.to_place(); // Given that there were two typed copies, we have to ensure this is valid at both types, // and we have to ensure this loses provenance and padding according to both types. // But if the types are identical, we only do one pass. - if allow_transmute && src.layout().ty != dest.layout().ty { + if src.layout().ty != dest.layout().ty { self.validate_operand( &dest.transmute(src.layout(), self)?, M::enforce_validity_recursively(self, src.layout()), diff --git a/src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-scalar.stderr b/src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-scalar.left.stderr similarity index 100% rename from src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-scalar.stderr rename to src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-scalar.left.stderr diff --git a/src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-scalar.right.stderr b/src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-scalar.right.stderr new file mode 100644 index 00000000000..54b21f155ca --- /dev/null +++ b/src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-scalar.right.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: constructing invalid value: encountered 0x03, but expected a boolean + --> tests/fail/intrinsics/typed-swap-invalid-scalar.rs:LL:CC + | +LL | typed_swap_nonoverlapping(a, b); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0x03, but expected a boolean + | + = 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 + = note: BACKTRACE: + = note: inside `invalid_scalar` at tests/fail/intrinsics/typed-swap-invalid-scalar.rs:LL:CC +note: inside `main` + --> tests/fail/intrinsics/typed-swap-invalid-scalar.rs:LL:CC + | +LL | invalid_scalar(); + | ^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-scalar.rs b/src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-scalar.rs index 3cc96e79fec..d5a72ea8612 100644 --- a/src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-scalar.rs +++ b/src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-scalar.rs @@ -1,3 +1,4 @@ +//@revisions: left right #![feature(core_intrinsics)] #![feature(rustc_attrs)] @@ -5,8 +6,9 @@ use std::intrinsics::typed_swap_nonoverlapping; use std::ptr::addr_of_mut; fn invalid_scalar() { - let mut a = 1_u8; - let mut b = 2_u8; + // We run the test twice, with either the left or the right side being invalid. + let mut a = if cfg!(left) { 2_u8} else { 1_u8 }; + let mut b = if cfg!(right) { 3_u8} else { 1_u8 }; unsafe { let a = addr_of_mut!(a).cast::(); let b = addr_of_mut!(b).cast::(); diff --git a/src/tools/miri/tests/fail/intrinsics/typed-swap-overlap.rs b/src/tools/miri/tests/fail/intrinsics/typed-swap-overlap.rs index 7b1be4abb15..e643091a02a 100644 --- a/src/tools/miri/tests/fail/intrinsics/typed-swap-overlap.rs +++ b/src/tools/miri/tests/fail/intrinsics/typed-swap-overlap.rs @@ -5,7 +5,7 @@ use std::intrinsics::typed_swap_nonoverlapping; use std::ptr::addr_of_mut; fn main() { - let mut a = [0_u8; 100]; + let mut a = 0_u8; unsafe { let a = addr_of_mut!(a); typed_swap_nonoverlapping(a, a); //~ERROR: called on overlapping ranges