From 62087439a46f09b6a6716fc25b4a032f3d76eb71 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 14 Mar 2020 11:12:32 +0100 Subject: [PATCH 1/5] add Scalar::from methods for signed integers --- src/librustc/mir/interpret/value.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 9dc0b24cd2f..1e630c96dd4 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -272,11 +272,13 @@ impl<'tcx, Tag> Scalar { #[inline] pub fn from_bool(b: bool) -> Self { + // Guaranteed to be truncated and does not need sign extension. Scalar::Raw { data: b as u128, size: 1 } } #[inline] pub fn from_char(c: char) -> Self { + // Guaranteed to be truncated and does not need sign extension. Scalar::Raw { data: c as u128, size: 4 } } @@ -299,21 +301,25 @@ impl<'tcx, Tag> Scalar { #[inline] pub fn from_u8(i: u8) -> Self { + // Guaranteed to be truncated and does not need sign extension. Scalar::Raw { data: i as u128, size: 1 } } #[inline] pub fn from_u16(i: u16) -> Self { + // Guaranteed to be truncated and does not need sign extension. Scalar::Raw { data: i as u128, size: 2 } } #[inline] pub fn from_u32(i: u32) -> Self { + // Guaranteed to be truncated and does not need sign extension. Scalar::Raw { data: i as u128, size: 4 } } #[inline] pub fn from_u64(i: u64) -> Self { + // Guaranteed to be truncated and does not need sign extension. Scalar::Raw { data: i as u128, size: 8 } } @@ -341,6 +347,26 @@ impl<'tcx, Tag> Scalar { .unwrap_or_else(|| bug!("Signed value {:#x} does not fit in {} bits", i, size.bits())) } + #[inline] + pub fn from_i8(i: i8) -> Self { + Self::from_int(i, Size::from_bits(8)) + } + + #[inline] + pub fn from_i16(i: i16) -> Self { + Self::from_int(i, Size::from_bits(16)) + } + + #[inline] + pub fn from_i32(i: i32) -> Self { + Self::from_int(i, Size::from_bits(32)) + } + + #[inline] + pub fn from_i64(i: i64) -> Self { + Self::from_int(i, Size::from_bits(64)) + } + #[inline] pub fn from_machine_isize(i: i64, cx: &impl HasDataLayout) -> Self { Self::from_int(i, cx.data_layout().pointer_size) From 9c5d8e9b520c12044c818dd3d48f02bcea075ec3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 14 Mar 2020 11:23:39 +0100 Subject: [PATCH 2/5] adjust Miri interaction with panic runtime --- src/libcore/intrinsics.rs | 8 +++++--- src/libpanic_unwind/lib.rs | 35 ++++++++++++++++++++++------------- src/libpanic_unwind/miri.rs | 20 ++++++++++++++++++++ 3 files changed, 47 insertions(+), 16 deletions(-) create mode 100644 src/libpanic_unwind/miri.rs diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index c2f13047e54..d722406b82b 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -1892,10 +1892,12 @@ extern "rust-intrinsic" { pub fn ptr_offset_from(ptr: *const T, base: *const T) -> isize; /// Internal hook used by Miri to implement unwinding. - /// Compiles to a NOP during non-Miri codegen. + /// ICEs when encountered during non-Miri codegen. /// - /// Perma-unstable: do not use - pub fn miri_start_panic(data: *mut (dyn crate::any::Any + crate::marker::Send)) -> (); + /// The `payload` ptr here will be exactly the one `do_catch` gets passed by `try`. + /// + /// Perma-unstable: do not use. + pub fn miri_start_panic(payload: *mut u8) -> !; } // Some functions are defined here because they accidentally got made diff --git a/src/libpanic_unwind/lib.rs b/src/libpanic_unwind/lib.rs index d6c33666938..c213b19d062 100644 --- a/src/libpanic_unwind/lib.rs +++ b/src/libpanic_unwind/lib.rs @@ -31,6 +31,9 @@ #![panic_runtime] #![feature(panic_runtime)] +// `real_imp` is unused with Miri, so silence warnings. +#![cfg_attr(miri, allow(dead_code))] + use alloc::boxed::Box; use core::any::Any; use core::panic::BoxMeUp; @@ -38,25 +41,38 @@ use core::panic::BoxMeUp; cfg_if::cfg_if! { if #[cfg(target_os = "emscripten")] { #[path = "emcc.rs"] - mod imp; + mod real_imp; } else if #[cfg(target_arch = "wasm32")] { #[path = "dummy.rs"] - mod imp; + mod real_imp; } else if #[cfg(target_os = "hermit")] { #[path = "hermit.rs"] - mod imp; + mod real_imp; } else if #[cfg(all(target_env = "msvc", target_arch = "aarch64"))] { #[path = "dummy.rs"] - mod imp; + mod real_imp; } else if #[cfg(target_env = "msvc")] { #[path = "seh.rs"] - mod imp; + mod real_imp; } else { // Rust runtime's startup objects depend on these symbols, so make them public. #[cfg(all(target_os="windows", target_arch = "x86", target_env="gnu"))] - pub use imp::eh_frame_registry::*; + pub use real_imp::eh_frame_registry::*; #[path = "gcc.rs"] + mod real_imp; + } +} + +cfg_if::cfg_if! { + if #[cfg(miri)] { + // Use the Miri runtime. + // We still need to also load the normal runtime above, as rustc expects certain lang + // items from there to be defined. + #[path = "miri.rs"] mod imp; + } else { + // Use the real runtime. + use real_imp as imp; } } @@ -81,12 +97,5 @@ pub unsafe extern "C" fn __rust_start_panic(payload: usize) -> u32 { let payload = payload as *mut &mut dyn BoxMeUp; let payload = (*payload).take_box(); - // Miri panic support: cfg'd out of normal builds just to be sure. - // When going through normal codegen, `miri_start_panic` is a NOP, so the - // Miri-enabled sysroot still supports normal unwinding. But when executed in - // Miri, this line initiates unwinding. - #[cfg(miri)] - core::intrinsics::miri_start_panic(payload); - imp::panic(Box::from_raw(payload)) } diff --git a/src/libpanic_unwind/miri.rs b/src/libpanic_unwind/miri.rs new file mode 100644 index 00000000000..5f44b98566f --- /dev/null +++ b/src/libpanic_unwind/miri.rs @@ -0,0 +1,20 @@ +//! Unwinding panics for Miri. +use core::any::Any; +use alloc::boxed::Box; + +// The type of the payload that the Miri engine propagates through unwinding for us. +// Must be pointer-sized. +type Payload = Box>; + +pub unsafe fn panic(payload: Box) -> u32 { + // The payload we pass to `miri_start_panic` will be exactly the argument we get + // in `cleanup` below. So we just box it up once, to get something pointer-sized. + let payload_box: Payload = Box::new(payload); + core::intrinsics::miri_start_panic(Box::into_raw(payload_box) as *mut u8) +} + +pub unsafe fn cleanup(payload_box: *mut u8) -> Box { + // Recover the underlying `Box`. + let payload_box: Payload = Box::from_raw(payload_box as *mut _); + *payload_box +} From 445284372082c1e760bc8cc0fac1d69644f27606 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 14 Mar 2020 11:36:40 +0100 Subject: [PATCH 3/5] update panicking comments in libstd --- src/libstd/panicking.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 0be71b52d9e..7bc86650a73 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -251,21 +251,20 @@ pub unsafe fn r#try R>(f: F) -> Result> // // We go through a transition where: // - // * First, we set the data to be the closure that we're going to call. + // * First, we set the data field `f` to be the argumentless closure that we're going to call. // * When we make the function call, the `do_call` function below, we take - // ownership of the function pointer. At this point the `Data` union is + // ownership of the function pointer. At this point the `data` union is // entirely uninitialized. // * If the closure successfully returns, we write the return value into the - // data's return slot. Note that `ptr::write` is used as it's overwriting - // uninitialized data. + // data's return slot (field `r`). + // * If the closure panics (`do_catch` below), we write the panic payload into field `p`. // * Finally, when we come back out of the `try` intrinsic we're // in one of two states: // // 1. The closure didn't panic, in which case the return value was - // filled in. We move it out of `data` and return it. - // 2. The closure panicked, in which case the return value wasn't - // filled in. In this case the entire `data` union is invalid, so - // there is no need to drop anything. + // filled in. We move it out of `data.r` and return it. + // 2. The closure panicked, in which case the panic payload was + // filled in. We move it out of `data.p` and return it. // // Once we stack all that together we should have the "most efficient' // method of calling a catch panic whilst juggling ownership. From b5938adb4d432bc6706f9b1f6ec0a85fc49bf11d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 14 Mar 2020 11:51:27 +0100 Subject: [PATCH 4/5] adjust Miri to needs of changed unwinding strategy --- src/librustc_mir/interpret/eval_context.rs | 31 +++++++++------------- src/librustc_mir/interpret/machine.rs | 12 ++++----- src/librustc_mir/interpret/mod.rs | 2 +- 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 9b28b7a20c0..0d43e4759f7 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -21,7 +21,7 @@ use rustc_span::source_map::{self, Span, DUMMY_SP}; use super::{ Immediate, MPlaceTy, Machine, MemPlace, MemPlaceMeta, Memory, OpTy, Operand, Place, PlaceTy, - ScalarMaybeUndef, StackPopInfo, + ScalarMaybeUndef, StackPopJump, }; pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> { @@ -623,23 +623,15 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ::log_settings::settings().indentation -= 1; let frame = self.stack.pop().expect("tried to pop a stack frame, but there were none"); - let stack_pop_info = M::stack_pop(self, frame.extra, unwinding)?; - if let (false, StackPopInfo::StopUnwinding) = (unwinding, stack_pop_info) { - bug!("Attempted to stop unwinding while there is no unwinding!"); - } // Now where do we jump next? - // Determine if we leave this function normally or via unwinding. - let cur_unwinding = - if let StackPopInfo::StopUnwinding = stack_pop_info { false } else { unwinding }; - // Usually we want to clean up (deallocate locals), but in a few rare cases we don't. // In that case, we return early. We also avoid validation in that case, // because this is CTFE and the final value will be thoroughly validated anyway. let (cleanup, next_block) = match frame.return_to_block { StackPopCleanup::Goto { ret, unwind } => { - (true, Some(if cur_unwinding { unwind } else { ret })) + (true, Some(if unwinding { unwind } else { ret })) } StackPopCleanup::None { cleanup, .. } => (cleanup, None), }; @@ -647,7 +639,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { if !cleanup { assert!(self.stack.is_empty(), "only the topmost frame should ever be leaked"); assert!(next_block.is_none(), "tried to skip cleanup when we have a next block!"); - // Leak the locals, skip validation. + assert!(!unwinding, "tried to skip cleanup during unwinding"); + // Leak the locals, skip validation, skip machine hook. return Ok(()); } @@ -656,13 +649,13 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.deallocate_local(local.value)?; } - trace!( - "StackPopCleanup: {:?} StackPopInfo: {:?} cur_unwinding = {:?}", - frame.return_to_block, - stack_pop_info, - cur_unwinding - ); - if cur_unwinding { + if M::stack_pop(self, frame.extra, unwinding)? == StackPopJump::NoJump { + // The hook already did everything. + // We want to skip the `trace!` below, hence early return. + return Ok(()); + } + // Normal return. + if unwinding { // Follow the unwind edge. let unwind = next_block.expect("Encountered StackPopCleanup::None when unwinding!"); self.unwind_to_block(unwind); @@ -697,7 +690,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { "CONTINUING({}) {} (unwinding = {})", self.cur_frame(), self.frame().instance, - cur_unwinding + unwinding ); } diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 366de6e5561..0e70e54ad85 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -17,16 +17,16 @@ use super::{ /// Data returned by Machine::stack_pop, /// to provide further control over the popping of the stack frame #[derive(Eq, PartialEq, Debug, Copy, Clone)] -pub enum StackPopInfo { +pub enum StackPopJump { /// Indicates that no special handling should be /// done - we'll either return normally or unwind /// based on the terminator for the function /// we're leaving. Normal, - /// Indicates that we should stop unwinding, - /// as we've reached a catch frame - StopUnwinding, + /// Indicates that we should *not* jump to the return/unwind address, as the callback already + /// took care of everything. + NoJump, } /// Whether this kind of memory is allowed to leak @@ -276,9 +276,9 @@ pub trait Machine<'mir, 'tcx>: Sized { _ecx: &mut InterpCx<'mir, 'tcx, Self>, _extra: Self::FrameExtra, _unwinding: bool, - ) -> InterpResult<'tcx, StackPopInfo> { + ) -> InterpResult<'tcx, StackPopJump> { // By default, we do not support unwinding from panics - Ok(StackPopInfo::Normal) + Ok(StackPopJump::Normal) } fn int_to_ptr( diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index a519f38e712..c3fd9682765 100644 --- a/src/librustc_mir/interpret/mod.rs +++ b/src/librustc_mir/interpret/mod.rs @@ -24,7 +24,7 @@ pub use self::place::{MPlaceTy, MemPlace, MemPlaceMeta, Place, PlaceTy}; pub use self::memory::{AllocCheck, FnVal, Memory, MemoryKind}; -pub use self::machine::{AllocMap, Machine, MayLeak, StackPopInfo}; +pub use self::machine::{AllocMap, Machine, MayLeak, StackPopJump}; pub use self::operand::{ImmTy, Immediate, OpTy, Operand, ScalarMaybeUndef}; From b450e1baf4c35ad4812fba9cb1946ea20d405ad8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 14 Mar 2020 12:10:00 +0100 Subject: [PATCH 5/5] fix comment, rustfmt --- src/libpanic_unwind/lib.rs | 1 - src/libpanic_unwind/miri.rs | 2 +- src/librustc_mir/interpret/eval_context.rs | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libpanic_unwind/lib.rs b/src/libpanic_unwind/lib.rs index c213b19d062..0a2a0e9e045 100644 --- a/src/libpanic_unwind/lib.rs +++ b/src/libpanic_unwind/lib.rs @@ -30,7 +30,6 @@ #![feature(raw)] #![panic_runtime] #![feature(panic_runtime)] - // `real_imp` is unused with Miri, so silence warnings. #![cfg_attr(miri, allow(dead_code))] diff --git a/src/libpanic_unwind/miri.rs b/src/libpanic_unwind/miri.rs index 5f44b98566f..9d92b2b2f32 100644 --- a/src/libpanic_unwind/miri.rs +++ b/src/libpanic_unwind/miri.rs @@ -1,6 +1,6 @@ //! Unwinding panics for Miri. -use core::any::Any; use alloc::boxed::Box; +use core::any::Any; // The type of the payload that the Miri engine propagates through unwinding for us. // Must be pointer-sized. diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 0d43e4759f7..482c143a73e 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -651,7 +651,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { if M::stack_pop(self, frame.extra, unwinding)? == StackPopJump::NoJump { // The hook already did everything. - // We want to skip the `trace!` below, hence early return. + // We want to skip the `info!` below, hence early return. return Ok(()); } // Normal return.