diff --git a/compiler/rustc_mir/src/transform/check_unsafety.rs b/compiler/rustc_mir/src/transform/check_unsafety.rs index e64955c4986ce..0732d2568ad1d 100644 --- a/compiler/rustc_mir/src/transform/check_unsafety.rs +++ b/compiler/rustc_mir/src/transform/check_unsafety.rs @@ -223,13 +223,6 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { // Check for raw pointer `Deref`. for (base, proj) in place.iter_projections() { if proj == ProjectionElem::Deref { - let source_info = self.source_info; // Backup source_info so we can restore it later. - if base.projection.is_empty() && decl.internal { - // Internal locals are used in the `move_val_init` desugaring. - // We want to check unsafety against the source info of the - // desugaring, rather than the source info of the RHS. - self.source_info = self.body.local_decls[place.local].source_info; - } let base_ty = base.ty(self.body, self.tcx).ty; if base_ty.is_unsafe_ptr() { self.require_unsafe( @@ -237,7 +230,6 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { UnsafetyViolationDetails::DerefOfRawPointer, ) } - self.source_info = source_info; // Restore backed-up source_info. } } diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 9777a97b57ee0..639f2bbc6e05f 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -10,9 +10,7 @@ use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_hir as hir; use rustc_middle::middle::region; use rustc_middle::mir::*; -use rustc_middle::ty::{self, CanonicalUserTypeAnnotation}; -use rustc_span::symbol::sym; -use rustc_target::spec::abi::Abi; +use rustc_middle::ty::{CanonicalUserTypeAnnotation}; use std::slice; @@ -219,79 +217,41 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }, ) } - ExprKind::Call { ty, fun, args, from_hir_call, fn_span } => { - let intrinsic = match *ty.kind() { - ty::FnDef(def_id, _) => { - let f = ty.fn_sig(this.hir.tcx()); - if f.abi() == Abi::RustIntrinsic || f.abi() == Abi::PlatformIntrinsic { - Some(this.hir.tcx().item_name(def_id)) - } else { - None - } - } - _ => None, - }; + ExprKind::Call { ty: _, fun, args, from_hir_call, fn_span } => { let fun = unpack!(block = this.as_local_operand(block, fun)); - if let Some(sym::move_val_init) = intrinsic { - // `move_val_init` has "magic" semantics - the second argument is - // always evaluated "directly" into the first one. - - let mut args = args.into_iter(); - let ptr = args.next().expect("0 arguments to `move_val_init`"); - let val = args.next().expect("1 argument to `move_val_init`"); - assert!(args.next().is_none(), ">2 arguments to `move_val_init`"); - - let ptr = this.hir.mirror(ptr); - let ptr_ty = ptr.ty; - // Create an *internal* temp for the pointer, so that unsafety - // checking won't complain about the raw pointer assignment. - let ptr_temp = this - .local_decls - .push(LocalDecl::with_source_info(ptr_ty, source_info).internal()); - let ptr_temp = Place::from(ptr_temp); - // No need for a scope, ptr_temp doesn't need drop - let block = unpack!(this.into(ptr_temp, None, block, ptr)); - // Maybe we should provide a scope here so that - // `move_val_init` wouldn't leak on panic even with an - // arbitrary `val` expression, but `schedule_drop`, - // borrowck and drop elaboration all prevent us from - // dropping `ptr_temp.deref()`. - this.into(this.hir.tcx().mk_place_deref(ptr_temp), None, block, val) - } else { - let args: Vec<_> = args - .into_iter() - .map(|arg| unpack!(block = this.as_local_call_operand(block, arg))) - .collect(); - - let success = this.cfg.start_new_block(); - - this.record_operands_moved(&args); - - debug!("into_expr: fn_span={:?}", fn_span); - - this.cfg.terminate( - block, - source_info, - TerminatorKind::Call { - func: fun, - args, - cleanup: None, - // FIXME(varkor): replace this with an uninhabitedness-based check. - // This requires getting access to the current module to call - // `tcx.is_ty_uninhabited_from`, which is currently tricky to do. - destination: if expr.ty.is_never() { - None - } else { - Some((destination, success)) - }, - from_hir_call, - fn_span, + let args: Vec<_> = args + .into_iter() + .map(|arg| unpack!(block = this.as_local_call_operand(block, arg))) + .collect(); + + let success = this.cfg.start_new_block(); + + this.record_operands_moved(&args); + + debug!("into_expr: fn_span={:?}", fn_span); + + this.cfg.terminate( + block, + source_info, + TerminatorKind::Call { + func: fun, + args, + cleanup: None, + // FIXME(varkor): replace this with an uninhabitedness-based check. + // This requires getting access to the current module to call + // `tcx.is_ty_uninhabited_from`, which is currently tricky to do. + destination: if expr.ty.is_never() { + None + } else { + Some((destination, success)) }, - ); - this.diverge_from(block); - schedule_drop(this); - success.unit() - } + from_hir_call, + fn_span, + }, + ); + this.diverge_from(block); + schedule_drop(this); + success.unit() } ExprKind::Use { source } => this.into(destination, scope, block, source), ExprKind::Borrow { arg, borrow_kind } => { diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index b6cf584d875f1..63f95a3908434 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -716,7 +716,6 @@ symbols! { more_struct_aliases, movbe_target_feature, move_ref_pattern, - move_val_init, mul, mul_assign, mul_with_overflow, diff --git a/compiler/rustc_typeck/src/check/intrinsic.rs b/compiler/rustc_typeck/src/check/intrinsic.rs index e4e6cf73c7e23..e99db7a247c46 100644 --- a/compiler/rustc_typeck/src/check/intrinsic.rs +++ b/compiler/rustc_typeck/src/check/intrinsic.rs @@ -157,7 +157,6 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) { } sym::forget => (1, vec![param(0)], tcx.mk_unit()), sym::transmute => (2, vec![param(0)], param(1)), - sym::move_val_init => (1, vec![tcx.mk_mut_ptr(param(0)), param(0)], tcx.mk_unit()), sym::prefetch_read_data | sym::prefetch_write_data | sym::prefetch_read_instruction diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 0130586b43042..fa5f0d5040047 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -768,13 +768,6 @@ extern "rust-intrinsic" { #[rustc_const_stable(feature = "const_size_of", since = "1.40.0")] pub fn size_of() -> usize; - /// Moves a value to an uninitialized memory location. - /// - /// Drop glue is not run on the destination. - /// - /// The stabilized version of this intrinsic is [`core::ptr::write`](crate::ptr::write). - pub fn move_val_init(dst: *mut T, src: T); - /// The minimum alignment of a type. /// /// The stabilized version of this intrinsic is [`core::mem::align_of`](crate::mem::align_of). diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index 807f114ea466c..d2e1bac58f402 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -883,12 +883,19 @@ pub const unsafe fn read_unaligned(src: *const T) -> T { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub unsafe fn write(dst: *mut T, src: T) { - if cfg!(debug_assertions) && !is_aligned_and_not_null(dst) { - // Not panicking to keep codegen impact smaller. - abort(); + // 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" { + fn copy_nonoverlapping(src: *const T, dst: *mut T, count: usize); + } + + // SAFETY: the caller must guarantee that `dst` is valid for writes. + // `dst` cannot overlap `src` because the caller has mutable access + // to `dst` while `src` is owned by this function. + unsafe { + copy_nonoverlapping(&src as *const T, dst, 1); + intrinsics::forget(src); } - // SAFETY: the caller must uphold the safety contract for `move_val_init`. - unsafe { intrinsics::move_val_init(&mut *dst, src) } } /// Overwrites a memory location with the given value without reading or diff --git a/src/test/codegen/intrinsics/move-val-init.rs b/src/test/codegen/intrinsics/move-val-init.rs deleted file mode 100644 index 6222536b50600..0000000000000 --- a/src/test/codegen/intrinsics/move-val-init.rs +++ /dev/null @@ -1,19 +0,0 @@ -// compile-flags: -C no-prepopulate-passes - -#![feature(core_intrinsics)] -#![crate_type = "lib"] - -// test that `move_val_init` actually avoids big allocas - -use std::intrinsics::move_val_init; - -pub struct Big { - pub data: [u8; 65536] -} - -// CHECK-LABEL: @test_mvi -#[no_mangle] -pub unsafe fn test_mvi(target: *mut Big, make_big: fn() -> Big) { - // CHECK: call void %make_big(%Big*{{[^%]*}} %target) - move_val_init(target, make_big()); -} diff --git a/src/test/ui/intrinsics/intrinsic-move-val-cleanups.rs b/src/test/ui/intrinsics/intrinsic-move-val-cleanups.rs deleted file mode 100644 index 9804c421db081..0000000000000 --- a/src/test/ui/intrinsics/intrinsic-move-val-cleanups.rs +++ /dev/null @@ -1,191 +0,0 @@ -// run-pass -#![allow(unused_braces)] -#![allow(unused_unsafe)] -#![allow(unreachable_code)] -// ignore-emscripten no threads support -#![allow(stable_features)] - -// This test is checking that the move_val_init intrinsic is -// respecting cleanups for both of its argument expressions. -// -// In other words, if either DEST or SOURCE in -// -// `intrinsics::move_val_init(DEST, SOURCE) -// -// introduce temporaries that require cleanup, and SOURCE panics, then -// make sure the cleanups still occur. - -#![feature(core_intrinsics, sync_poison)] - -use std::cell::RefCell; -use std::intrinsics; -use std::sync::{Arc, LockResult, Mutex, MutexGuard}; -use std::thread; - -type LogEntry = (&'static str, i32); -type Guarded = RefCell>; -#[derive(Clone)] -struct Log(Arc>); -struct Acquired<'a>(MutexGuard<'a, Guarded>); -type LogState = (MutexWas, &'static [LogEntry]); - -#[derive(Copy, Clone, PartialEq, Eq, Debug)] -enum MutexWas { Poisoned, NotPoisoned } - -impl Log { - fn lock(&self) -> LockResult>>> { self.0.lock() } - fn acquire(&self) -> Acquired { Acquired(self.0.lock().unwrap()) } -} - -impl<'a> Acquired<'a> { - fn log(&self, s: &'static str, i: i32) { self.0.borrow_mut().push((s, i)); } -} - -const TEST1_EXPECT: LogState = (MutexWas::NotPoisoned, - &[("double-check non-poisoning path", 1) - ]); - -fn test1(log: Log) { - { - let acq = log.acquire(); - acq.log("double-check non-poisoning path", 1); - } - panic!("every test ends in a panic"); -} - -const TEST2_EXPECT: LogState = (MutexWas::Poisoned, - &[("double-check poisoning path", 1), - ("and multiple log entries", 2), - ]); -fn test2(log: Log) { - let acq = log.acquire(); - acq.log("double-check poisoning path", 1); - acq.log("and multiple log entries", 2); - panic!("every test ends in a panic"); -} - -struct LogOnDrop<'a>(&'a Acquired<'a>, &'static str, i32); -impl<'a> Drop for LogOnDrop<'a> { - fn drop(&mut self) { - self.0.log(self.1, self.2); - } -} - -const TEST3_EXPECT: LogState = (MutexWas::Poisoned, - &[("double-check destructors can log", 1), - ("drop d2", 2), - ("drop d1", 3), - ]); -fn test3(log: Log) { - let acq = log.acquire(); - acq.log("double-check destructors can log", 1); - let _d1 = LogOnDrop(&acq, "drop d1", 3); - let _d2 = LogOnDrop(&acq, "drop d2", 2); - panic!("every test ends in a panic"); -} - -// The *real* tests of panic-handling for move_val_init intrinsic -// start here. - -const TEST4_EXPECT: LogState = (MutexWas::Poisoned, - &[("neither arg panics", 1), - ("drop temp LOD", 2), - ("drop temp LOD", 3), - ("drop dest_b", 4), - ("drop dest_a", 5), - ]); -fn test4(log: Log) { - let acq = log.acquire(); - acq.log("neither arg panics", 1); - let mut dest_a = LogOnDrop(&acq, "a will be overwritten, not dropped", 0); - let mut dest_b = LogOnDrop(&acq, "b will be overwritten, not dropped", 0); - unsafe { - intrinsics::move_val_init({ LogOnDrop(&acq, "drop temp LOD", 2); &mut dest_a }, - LogOnDrop(&acq, "drop dest_a", 5)); - intrinsics::move_val_init(&mut dest_b, { LogOnDrop(&acq, "drop temp LOD", 3); - LogOnDrop(&acq, "drop dest_b", 4) }); - } - panic!("every test ends in a panic"); -} - - -// Check that move_val_init(PANIC, SOURCE_EXPR) never evaluates SOURCE_EXPR -const TEST5_EXPECT: LogState = (MutexWas::Poisoned, - &[("first arg panics", 1), - ("drop orig dest_a", 2), - ]); -fn test5(log: Log) { - let acq = log.acquire(); - acq.log("first arg panics", 1); - let mut _dest_a = LogOnDrop(&acq, "drop orig dest_a", 2); - unsafe { - intrinsics::move_val_init({ panic!("every test ends in a panic") }, - LogOnDrop(&acq, "we never get here", 0)); - } -} - -// Check that move_val_init(DEST_EXPR, PANIC) cleans up temps from DEST_EXPR. -const TEST6_EXPECT: LogState = (MutexWas::Poisoned, - &[("second arg panics", 1), - ("drop temp LOD", 2), - ("drop orig dest_a", 3), - ]); -fn test6(log: Log) { - let acq = log.acquire(); - acq.log("second arg panics", 1); - let mut dest_a = LogOnDrop(&acq, "drop orig dest_a", 3); - unsafe { - intrinsics::move_val_init({ LogOnDrop(&acq, "drop temp LOD", 2); &mut dest_a }, - { panic!("every test ends in a panic"); }); - } -} - -// Check that move_val_init(DEST_EXPR, COMPLEX_PANIC) cleans up temps from COMPLEX_PANIC. -const TEST7_EXPECT: LogState = (MutexWas::Poisoned, - &[("second arg panics", 1), - ("drop temp LOD", 2), - ("drop temp LOD", 3), - ("drop orig dest_a", 4), - ]); -fn test7(log: Log) { - let acq = log.acquire(); - acq.log("second arg panics", 1); - let mut dest_a = LogOnDrop(&acq, "drop orig dest_a", 4); - unsafe { - intrinsics::move_val_init({ LogOnDrop(&acq, "drop temp LOD", 2); &mut dest_a }, - { LogOnDrop(&acq, "drop temp LOD", 3); - panic!("every test ends in a panic"); }); - } -} - -const TEST_SUITE: &'static [(&'static str, fn (Log), LogState)] = - &[("test1", test1, TEST1_EXPECT), - ("test2", test2, TEST2_EXPECT), - ("test3", test3, TEST3_EXPECT), - ("test4", test4, TEST4_EXPECT), - ("test5", test5, TEST5_EXPECT), - ("test6", test6, TEST6_EXPECT), - ("test7", test7, TEST7_EXPECT), - ]; - -fn main() { - for &(name, test, expect) in TEST_SUITE { - let log = Log(Arc::new(Mutex::new(RefCell::new(Vec::new())))); - let ret = { let log = log.clone(); thread::spawn(move || test(log)).join() }; - assert!(ret.is_err(), "{} must end with panic", name); - { - let l = log.lock(); - match l { - Ok(acq) => { - assert_eq!((MutexWas::NotPoisoned, &acq.borrow()[..]), expect); - println!("{} (unpoisoned) log: {:?}", name, *acq); - } - Err(e) => { - let acq = e.into_inner(); - assert_eq!((MutexWas::Poisoned, &acq.borrow()[..]), expect); - println!("{} (poisoned) log: {:?}", name, *acq); - } - } - } - } -} diff --git a/src/test/ui/intrinsics/intrinsic-move-val.rs b/src/test/ui/intrinsics/intrinsic-move-val.rs deleted file mode 100644 index b672f1ed26e8d..0000000000000 --- a/src/test/ui/intrinsics/intrinsic-move-val.rs +++ /dev/null @@ -1,81 +0,0 @@ -// run-pass - -#![feature(box_syntax)] -#![feature(intrinsics)] - -mod rusti { - extern "rust-intrinsic" { - pub fn move_val_init(dst: *mut T, src: T); - } -} - -pub fn main() { - unsafe { - // sanity check - check_drops_state(0, None); - - let mut x: Option> = Some(box D(1)); - assert_eq!(x.as_ref().unwrap().0, 1); - - // A normal overwrite, to demonstrate `check_drops_state`. - x = Some(box D(2)); - - // At this point, one destructor has run, because the - // overwrite of `x` drops its initial value. - check_drops_state(1, Some(1)); - - let mut y: Option> = std::mem::zeroed(); - - // An initial binding does not overwrite anything. - check_drops_state(1, Some(1)); - - // Since `y` has been initialized via the `init` intrinsic, it - // would be unsound to directly overwrite its value via normal - // assignment. - // - // The code currently generated by the compiler is overly - // accepting, however, in that it will check if `y` is itself - // null and thus avoid the unsound action of attempting to - // free null. In other words, if we were to do a normal - // assignment like `y = box D(4);` here, it probably would not - // crash today. But the plan is that it may well crash in the - // future, (I believe). - - // `x` is moved here; the manner in which this is tracked by the - // compiler is hidden. - rusti::move_val_init(&mut y, x); - - // But what we *can* observe is how many times the destructor - // for `D` is invoked, and what the last value we saw was - // during such a destructor call. We do so after the end of - // this scope. - - assert_eq!(y.as_ref().unwrap().0, 2); - y.as_mut().unwrap().0 = 3; - assert_eq!(y.as_ref().unwrap().0, 3); - - check_drops_state(1, Some(1)); - } - - check_drops_state(2, Some(3)); -} - -static mut NUM_DROPS: i32 = 0; -static mut LAST_DROPPED: Option = None; - -fn check_drops_state(num_drops: i32, last_dropped: Option) { - unsafe { - assert_eq!(NUM_DROPS, num_drops); - assert_eq!(LAST_DROPPED, last_dropped); - } -} - -struct D(i32); -impl Drop for D { - fn drop(&mut self) { - unsafe { - NUM_DROPS += 1; - LAST_DROPPED = Some(self.0); - } - } -} diff --git a/src/test/ui/unsafe/unsafe-move-val-init.rs b/src/test/ui/unsafe/unsafe-move-val-init.rs deleted file mode 100644 index 24249a7a813ec..0000000000000 --- a/src/test/ui/unsafe/unsafe-move-val-init.rs +++ /dev/null @@ -1,10 +0,0 @@ -#![feature(core_intrinsics)] - -use std::intrinsics; - -// `move_val_init` has an odd desugaring, check that it is still treated -// as unsafe. -fn main() { - intrinsics::move_val_init(1 as *mut u32, 1); - //~^ ERROR dereference of raw pointer is unsafe -} diff --git a/src/test/ui/unsafe/unsafe-move-val-init.stderr b/src/test/ui/unsafe/unsafe-move-val-init.stderr deleted file mode 100644 index 44c2aae7cf4f4..0000000000000 --- a/src/test/ui/unsafe/unsafe-move-val-init.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error[E0133]: dereference of raw pointer is unsafe and requires unsafe function or block - --> $DIR/unsafe-move-val-init.rs:8:5 - | -LL | intrinsics::move_val_init(1 as *mut u32, 1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dereference of raw pointer - | - = note: raw pointers may be NULL, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0133`.