diff options
10 files changed, 199 insertions, 15 deletions
diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index 0e7ffcdffc9..550c7a44c41 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -119,11 +119,20 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } Drop { place, target, unwind } => { + let frame = self.frame(); + let ty = place.ty(&frame.body.local_decls, *self.tcx).ty; + let ty = self.subst_from_frame_and_normalize_erasing_regions(frame, ty)?; + let instance = Instance::resolve_drop_in_place(*self.tcx, ty); + if let ty::InstanceDef::DropGlue(_, None) = instance.def { + // This is the branch we enter if and only if the dropped type has no drop glue + // whatsoever. This can happen as a result of monomorphizing a drop of a + // generic. In order to make sure that generic and non-generic code behaves + // roughly the same (and in keeping with Mir semantics) we do nothing here. + self.go_to_block(target); + return Ok(()); + } let place = self.eval_place(place)?; - let ty = place.layout.ty; trace!("TerminatorKind::drop: {:?}, type {}", place, ty); - - let instance = Instance::resolve_drop_in_place(*self.tcx, ty); self.drop_in_place(&place, instance, target, unwind)?; } diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index bb03359b138..6b4489026d3 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -564,14 +564,13 @@ pub enum TerminatorKind<'tcx> { Unreachable, /// The behavior of this statement differs significantly before and after drop elaboration. - /// After drop elaboration, `Drop` executes the drop glue for the specified place, after which - /// it continues execution/unwinds at the given basic blocks. It is possible that executing drop - /// glue is special - this would be part of Rust's memory model. (**FIXME**: due we have an - /// issue tracking if drop glue has any interesting semantics in addition to those of a function - /// call?) - /// - /// `Drop` before drop elaboration is a *conditional* execution of the drop glue. Specifically, the - /// `Drop` will be executed if... + /// + /// After drop elaboration: `Drop` terminators are a complete nop for types that have no drop + /// glue. For other types, `Drop` terminators behave exactly like a call to + /// `core::mem::drop_in_place` with a pointer to the given place. + /// + /// `Drop` before drop elaboration is a *conditional* execution of the drop glue. Specifically, + /// the `Drop` will be executed if... /// /// **Needs clarification**: End of that sentence. This in effect should document the exact /// behavior of drop elaboration. The following sounds vaguely right, but I'm not quite sure: diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index f8b55c86287..dace540fa29 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -174,9 +174,36 @@ fn build_drop_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, ty: Option<Ty<'tcx>>) let mut body = new_body(source, blocks, local_decls_for_sig(&sig, span), sig.inputs().len(), span); + // The first argument (index 0), but add 1 for the return value. + let mut dropee_ptr = Place::from(Local::new(1 + 0)); + if tcx.sess.opts.unstable_opts.mir_emit_retag { + // We want to treat the function argument as if it was passed by `&mut`. As such, we + // generate + // ``` + // temp = &mut *arg; + // Retag(temp, FnEntry) + // ``` + // It's important that we do this first, before anything that depends on `dropee_ptr` + // has been put into the body. + let reborrow = Rvalue::Ref( + tcx.lifetimes.re_erased, + BorrowKind::Mut { allow_two_phase_borrow: false }, + tcx.mk_place_deref(dropee_ptr), + ); + let ref_ty = reborrow.ty(body.local_decls(), tcx); + dropee_ptr = body.local_decls.push(LocalDecl::new(ref_ty, span)).into(); + let new_statements = [ + StatementKind::Assign(Box::new((dropee_ptr, reborrow))), + StatementKind::Retag(RetagKind::FnEntry, Box::new(dropee_ptr)), + ]; + for s in new_statements { + body.basic_blocks_mut()[START_BLOCK] + .statements + .push(Statement { source_info, kind: s }); + } + } + if ty.is_some() { - // The first argument (index 0), but add 1 for the return value. - let dropee_ptr = Place::from(Local::new(1 + 0)); let patch = { let param_env = tcx.param_env_reveal_all_normalized(def_id); let mut elaborator = diff --git a/src/test/mir-opt/retag.core.ptr-drop_in_place.Test.SimplifyCfg-make_shim.after.mir b/src/test/mir-opt/retag.core.ptr-drop_in_place.Test.SimplifyCfg-make_shim.after.mir index 14f297e948b..f495f147be3 100644 --- a/src/test/mir-opt/retag.core.ptr-drop_in_place.Test.SimplifyCfg-make_shim.after.mir +++ b/src/test/mir-opt/retag.core.ptr-drop_in_place.Test.SimplifyCfg-make_shim.after.mir @@ -3,11 +3,14 @@ fn std::ptr::drop_in_place(_1: *mut Test) -> () { let mut _0: (); // return place in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 let mut _2: &mut Test; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - let mut _3: (); // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + let mut _3: &mut Test; // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + let mut _4: (); // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 bb0: { _2 = &mut (*_1); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 - _3 = <Test as Drop>::drop(move _2) -> bb1; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + Retag([fn entry] _2); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + _3 = &mut (*_2); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 + _4 = <Test as Drop>::drop(move _3) -> bb1; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 // mir::Constant // + span: $SRC_DIR/core/src/ptr/mod.rs:LL:COL // + literal: Const { ty: for<'a> fn(&'a mut Test) {<Test as Drop>::drop}, val: Value(<ZST>) } diff --git a/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_protector.rs b/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_protector.rs new file mode 100644 index 00000000000..8cf63ee700b --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_protector.rs @@ -0,0 +1,27 @@ +//! Test that drop_in_place retags the entire place, +//! invalidating all aliases to it. + +// A zero-sized drop type -- the retagging of `fn drop` itself won't +// do anything (since it is zero-sized); we are entirely relying on the retagging +// in `drop_in_place` here. +#[repr(transparent)] +struct HasDrop; +impl Drop for HasDrop { + fn drop(&mut self) { + unsafe { + let _val = *P; + //~^ ERROR: /not granting access .* because that would remove .* which is strongly protected/ + } + } +} + +static mut P: *mut u8 = core::ptr::null_mut(); + +fn main() { + unsafe { + let mut x = (HasDrop, 0u8); + let x = core::ptr::addr_of_mut!(x); + P = x.cast(); + core::ptr::drop_in_place(x); + } +} diff --git a/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_protector.stderr b/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_protector.stderr new file mode 100644 index 00000000000..bd51a6645a6 --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_protector.stderr @@ -0,0 +1,33 @@ +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID + --> $DIR/drop_in_place_protector.rs:LL:CC + | +LL | let _val = *P; + | ^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID + | + = 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> was created by a SharedReadWrite retag at offsets [0x0..0x1] + --> $DIR/drop_in_place_protector.rs:LL:CC + | +LL | let x = core::ptr::addr_of_mut!(x); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: <TAG> is this argument + --> $DIR/drop_in_place_protector.rs:LL:CC + | +LL | core::ptr::drop_in_place(x); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: BACKTRACE: + = note: inside `<HasDrop as std::ops::Drop>::drop` at $DIR/drop_in_place_protector.rs:LL:CC + = note: inside `std::ptr::drop_in_place::<HasDrop> - shim(Some(HasDrop))` at RUSTLIB/core/src/ptr/mod.rs:LL:CC + = note: inside `std::ptr::drop_in_place::<(HasDrop, u8)> - shim(Some((HasDrop, u8)))` at RUSTLIB/core/src/ptr/mod.rs:LL:CC +note: inside `main` + --> $DIR/drop_in_place_protector.rs:LL:CC + | +LL | core::ptr::drop_in_place(x); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the macro `core::ptr::addr_of_mut` (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 + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_retag.rs b/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_retag.rs new file mode 100644 index 00000000000..8180e2f03a7 --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_retag.rs @@ -0,0 +1,12 @@ +//! Test that drop_in_place mutably retags the entire place, even for a type that does not need +//! dropping, ensuring among other things that it is writeable + +//@error-pattern: /retag .* for Unique permission .* only grants SharedReadOnly permission/ + +fn main() { + unsafe { + let x = 0u8; + let x = core::ptr::addr_of!(x); + core::ptr::drop_in_place(x.cast_mut()); + } +} diff --git a/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_retag.stderr b/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_retag.stderr new file mode 100644 index 00000000000..3f9e6708bda --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_retag.stderr @@ -0,0 +1,29 @@ +error: Undefined Behavior: trying to retag from <TAG> for Unique permission at ALLOC[0x0], but that tag only grants SharedReadOnly permission for this location + --> RUSTLIB/core/src/ptr/mod.rs:LL:CC + | +LL | pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | trying to retag from <TAG> for Unique permission at ALLOC[0x0], but that tag only grants SharedReadOnly permission for this location + | this error occurs as part of retag at ALLOC[0x0..0x1] + | + = 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> was created by a SharedReadOnly retag at offsets [0x0..0x1] + --> $DIR/drop_in_place_retag.rs:LL:CC + | +LL | let x = core::ptr::addr_of!(x); + | ^^^^^^^^^^^^^^^^^^^^^^ + = note: BACKTRACE: + = note: inside `std::ptr::drop_in_place::<u8> - shim(None)` at RUSTLIB/core/src/ptr/mod.rs:LL:CC +note: inside `main` + --> $DIR/drop_in_place_retag.rs:LL:CC + | +LL | core::ptr::drop_in_place(x.cast_mut()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the macro `core::ptr::addr_of` (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 + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/unaligned_pointers/drop_in_place.rs b/src/tools/miri/tests/fail/unaligned_pointers/drop_in_place.rs new file mode 100644 index 00000000000..cf3a558bb99 --- /dev/null +++ b/src/tools/miri/tests/fail/unaligned_pointers/drop_in_place.rs @@ -0,0 +1,25 @@ +#[repr(transparent)] +struct HasDrop(u8); + +impl Drop for HasDrop { + fn drop(&mut self) {} +} + +#[repr(C, align(2))] +struct PartialDrop { + a: HasDrop, + b: u8, +} + +//@error-pattern: /alignment 2 is required/ +fn main() { + unsafe { + // Create an unaligned pointer + let mut x = [0_u16; 2]; + let p = core::ptr::addr_of_mut!(x).cast::<u8>(); + let p = p.add(1); + let p = p.cast::<PartialDrop>(); + + core::ptr::drop_in_place(p); + } +} diff --git a/src/tools/miri/tests/fail/unaligned_pointers/drop_in_place.stderr b/src/tools/miri/tests/fail/unaligned_pointers/drop_in_place.stderr new file mode 100644 index 00000000000..ef20b43c118 --- /dev/null +++ b/src/tools/miri/tests/fail/unaligned_pointers/drop_in_place.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: accessing memory with alignment ALIGN, but alignment ALIGN is required + --> RUSTLIB/core/src/ptr/mod.rs:LL:CC + | +LL | pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ accessing memory with alignment ALIGN, but alignment ALIGN is required + | + = 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 `std::ptr::drop_in_place::<PartialDrop> - shim(Some(PartialDrop))` at RUSTLIB/core/src/ptr/mod.rs:LL:CC +note: inside `main` + --> $DIR/drop_in_place.rs:LL:CC + | +LL | core::ptr::drop_in_place(p); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + |