From 7348d2aa6e06aa49e2f142ec0fb8353c634da779 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 12:19:58 +0200 Subject: [PATCH 1/4] Machine: make self-like parameters come first --- src/librustc_mir/const_eval.rs | 8 ++++---- src/librustc_mir/interpret/machine.rs | 12 ++++++------ src/librustc_mir/interpret/memory.rs | 12 ++++++------ 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 887ef4b520ea3..b6ce6a2a1eae5 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -11,7 +11,7 @@ use rustc::hir::def::DefKind; use rustc::hir::def_id::DefId; use rustc::mir::interpret::{ConstEvalErr, ErrorHandled, ScalarMaybeUndef}; use rustc::mir; -use rustc::ty::{self, TyCtxt, query::TyCtxtAt}; +use rustc::ty::{self, TyCtxt}; use rustc::ty::layout::{self, LayoutOf, VariantIdx}; use rustc::ty::subst::Subst; use rustc::traits::Reveal; @@ -398,18 +398,18 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, } fn find_foreign_static( + _tcx: TyCtxt<'tcx>, _def_id: DefId, - _tcx: TyCtxtAt<'tcx>, ) -> InterpResult<'tcx, Cow<'tcx, Allocation>> { err!(ReadForeignStatic) } #[inline(always)] fn tag_allocation<'b>( + _memory: &Memory<'mir, 'tcx, Self>, _id: AllocId, alloc: Cow<'b, Allocation>, _kind: Option>, - _memory: &Memory<'mir, 'tcx, Self>, ) -> (Cow<'b, Allocation>, Self::PointerTag) { // We do not use a tag so we can just cheaply forward the allocation (alloc, ()) @@ -417,8 +417,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, #[inline(always)] fn tag_static_base_pointer( - _id: AllocId, _memory: &Memory<'mir, 'tcx, Self>, + _id: AllocId, ) -> Self::PointerTag { () } diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 4eb95f20d9354..a9c3d7ea21711 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -7,7 +7,7 @@ use std::hash::Hash; use rustc::hir::def_id::DefId; use rustc::mir; -use rustc::ty::{self, query::TyCtxtAt}; +use rustc::ty::{self, TyCtxt}; use super::{ Allocation, AllocId, InterpResult, Scalar, AllocationExtra, @@ -136,8 +136,8 @@ pub trait Machine<'mir, 'tcx>: Sized { /// /// This allocation will then be fed to `tag_allocation` to initialize the "extra" state. fn find_foreign_static( + tcx: TyCtxt<'tcx>, def_id: DefId, - tcx: TyCtxtAt<'tcx>, ) -> InterpResult<'tcx, Cow<'tcx, Allocation>>; /// Called for all binary operations on integer(-like) types when one operand is a pointer @@ -174,10 +174,10 @@ pub trait Machine<'mir, 'tcx>: Sized { /// For static allocations, the tag returned must be the same as the one returned by /// `tag_static_base_pointer`. fn tag_allocation<'b>( + memory: &Memory<'mir, 'tcx, Self>, id: AllocId, alloc: Cow<'b, Allocation>, kind: Option>, - memory: &Memory<'mir, 'tcx, Self>, ) -> (Cow<'b, Allocation>, Self::PointerTag); /// Return the "base" tag for the given static allocation: the one that is used for direct @@ -186,8 +186,8 @@ pub trait Machine<'mir, 'tcx>: Sized { /// Be aware that requesting the `Allocation` for that `id` will lead to cycles /// for cyclic statics! fn tag_static_base_pointer( - id: AllocId, memory: &Memory<'mir, 'tcx, Self>, + id: AllocId, ) -> Self::PointerTag; /// Executes a retagging operation @@ -210,8 +210,8 @@ pub trait Machine<'mir, 'tcx>: Sized { ) -> InterpResult<'tcx>; fn int_to_ptr( - int: u64, _mem: &Memory<'mir, 'tcx, Self>, + int: u64, ) -> InterpResult<'tcx, Pointer> { if int == 0 { err!(InvalidNullPointerUsage) @@ -221,8 +221,8 @@ pub trait Machine<'mir, 'tcx>: Sized { } fn ptr_to_int( - _ptr: Pointer, _mem: &Memory<'mir, 'tcx, Self>, + _ptr: Pointer, ) -> InterpResult<'tcx, u64> { err!(ReadPointerAsBytes) } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index c3eec677a4850..c42bddd9ad1ef 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -117,7 +117,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { #[inline] pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer { - ptr.with_tag(M::tag_static_base_pointer(ptr.alloc_id, &self)) + ptr.with_tag(M::tag_static_base_pointer(&self, ptr.alloc_id)) } pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer { @@ -150,7 +150,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { kind: MemoryKind, ) -> Pointer { let id = self.tcx.alloc_map.lock().reserve(); - let (alloc, tag) = M::tag_allocation(id, Cow::Owned(alloc), Some(kind), &self); + let (alloc, tag) = M::tag_allocation(&self, id, Cow::Owned(alloc), Some(kind)); self.alloc_map.insert(id, (kind, alloc.into_owned())); Pointer::from(id).with_tag(tag) } @@ -381,7 +381,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // We got a "lazy" static that has not been computed yet. if tcx.is_foreign_item(def_id) { trace!("static_alloc: foreign item {:?}", def_id); - M::find_foreign_static(def_id, tcx)? + M::find_foreign_static(tcx.tcx, def_id)? } else { trace!("static_alloc: Need to compute {:?}", def_id); let instance = Instance::mono(tcx.tcx, def_id); @@ -411,10 +411,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // We got tcx memory. Let the machine figure out whether and how to // turn that into memory with the right pointer tag. Ok(M::tag_allocation( + memory, id, // always use the ID we got as input, not the "hidden" one. alloc, M::STATIC_KIND.map(MemoryKind::Machine), - memory ).0) } @@ -887,7 +887,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { ) -> InterpResult<'tcx, Pointer> { match scalar { Scalar::Ptr(ptr) => Ok(ptr), - _ => M::int_to_ptr(scalar.to_usize(self)?, self) + _ => M::int_to_ptr(&self, scalar.to_usize(self)?) } } @@ -898,7 +898,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { ) -> InterpResult<'tcx, u128> { match scalar.to_bits_or_ptr(size, self) { Ok(bits) => Ok(bits), - Err(ptr) => Ok(M::ptr_to_int(ptr, self)? as u128) + Err(ptr) => Ok(M::ptr_to_int(&self, ptr)? as u128) } } } From da3dd712a3a7ff368c5740b8ca0afbf9ab889709 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 12:31:17 +0200 Subject: [PATCH 2/4] Go back to just passing MemoryExtra to the machine-level allocation hooks This is needed to avoid doing unnecessary global alloc_map lookups --- src/librustc_mir/const_eval.rs | 6 +++--- src/librustc_mir/interpret/machine.rs | 4 ++-- src/librustc_mir/interpret/memory.rs | 18 +++++++++--------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index b6ce6a2a1eae5..36c7c5b57721c 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -23,7 +23,7 @@ use crate::interpret::{self, PlaceTy, MPlaceTy, OpTy, ImmTy, Immediate, Scalar, RawConst, ConstValue, InterpResult, InterpErrorInfo, InterpError, GlobalId, InterpretCx, StackPopCleanup, - Allocation, AllocId, MemoryKind, Memory, + Allocation, AllocId, MemoryKind, snapshot, RefTracking, intern_const_alloc_recursive, }; @@ -406,7 +406,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, #[inline(always)] fn tag_allocation<'b>( - _memory: &Memory<'mir, 'tcx, Self>, + _memory_extra: &(), _id: AllocId, alloc: Cow<'b, Allocation>, _kind: Option>, @@ -417,7 +417,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, #[inline(always)] fn tag_static_base_pointer( - _memory: &Memory<'mir, 'tcx, Self>, + _memory_extra: &(), _id: AllocId, ) -> Self::PointerTag { () diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index a9c3d7ea21711..adf6339724f87 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -174,7 +174,7 @@ pub trait Machine<'mir, 'tcx>: Sized { /// For static allocations, the tag returned must be the same as the one returned by /// `tag_static_base_pointer`. fn tag_allocation<'b>( - memory: &Memory<'mir, 'tcx, Self>, + memory_extra: &Self::MemoryExtra, id: AllocId, alloc: Cow<'b, Allocation>, kind: Option>, @@ -186,7 +186,7 @@ pub trait Machine<'mir, 'tcx>: Sized { /// Be aware that requesting the `Allocation` for that `id` will lead to cycles /// for cyclic statics! fn tag_static_base_pointer( - memory: &Memory<'mir, 'tcx, Self>, + memory_extra: &Self::MemoryExtra, id: AllocId, ) -> Self::PointerTag; diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index c42bddd9ad1ef..0e7328addd2bd 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -117,7 +117,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { #[inline] pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer { - ptr.with_tag(M::tag_static_base_pointer(&self, ptr.alloc_id)) + ptr.with_tag(M::tag_static_base_pointer(&self.extra, ptr.alloc_id)) } pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer { @@ -150,7 +150,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { kind: MemoryKind, ) -> Pointer { let id = self.tcx.alloc_map.lock().reserve(); - let (alloc, tag) = M::tag_allocation(&self, id, Cow::Owned(alloc), Some(kind)); + let (alloc, tag) = M::tag_allocation(&self.extra, id, Cow::Owned(alloc), Some(kind)); self.alloc_map.insert(id, (kind, alloc.into_owned())); Pointer::from(id).with_tag(tag) } @@ -365,9 +365,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { /// contains a reference to memory that was created during its evaluation (i.e., not to /// another static), those inner references only exist in "resolved" form. fn get_static_alloc( - id: AllocId, + memory_extra: &M::MemoryExtra, tcx: TyCtxtAt<'tcx>, - memory: &Memory<'mir, 'tcx, M>, + id: AllocId, ) -> InterpResult<'tcx, Cow<'tcx, Allocation>> { let alloc = tcx.alloc_map.lock().get(id); let alloc = match alloc { @@ -391,7 +391,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { }; // use the raw query here to break validation cycles. Later uses of the static // will call the full query anyway - let raw_const = tcx.const_eval_raw(ty::ParamEnv::reveal_all().and(gid)) + let raw_const = tcx.tcx.const_eval_raw(ty::ParamEnv::reveal_all().and(gid)) .map_err(|err| { // no need to report anything, the const_eval call takes care of that // for statics @@ -411,7 +411,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // We got tcx memory. Let the machine figure out whether and how to // turn that into memory with the right pointer tag. Ok(M::tag_allocation( - memory, + memory_extra, id, // always use the ID we got as input, not the "hidden" one. alloc, M::STATIC_KIND.map(MemoryKind::Machine), @@ -427,7 +427,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // `get_static_alloc` that we can actually use directly without inserting anything anywhere. // So the error type is `InterpResult<'tcx, &Allocation>`. let a = self.alloc_map.get_or(id, || { - let alloc = Self::get_static_alloc(id, self.tcx, &self).map_err(Err)?; + let alloc = Self::get_static_alloc(&self.extra, self.tcx, id).map_err(Err)?; match alloc { Cow::Borrowed(alloc) => { // We got a ref, cheaply return that as an "error" so that the @@ -456,11 +456,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { id: AllocId, ) -> InterpResult<'tcx, &mut Allocation> { let tcx = self.tcx; - let alloc = Self::get_static_alloc(id, tcx, &self); + let memory_extra = &self.extra; let a = self.alloc_map.get_mut_or(id, || { // Need to make a copy, even if `get_static_alloc` is able // to give us a cheap reference. - let alloc = alloc?; + let alloc = Self::get_static_alloc(memory_extra, tcx, id)?; if alloc.mutability == Mutability::Immutable { return err!(ModifiedConstantMemory); } From 3408713f4fb23edc96b1cb53113cb4e3b3295ade Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 10:48:51 +0200 Subject: [PATCH 3/4] more inlining --- src/librustc_mir/interpret/machine.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index adf6339724f87..85674d48d6d05 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -10,7 +10,7 @@ use rustc::mir; use rustc::ty::{self, TyCtxt}; use super::{ - Allocation, AllocId, InterpResult, Scalar, AllocationExtra, + Allocation, AllocId, InterpResult, InterpError, Scalar, AllocationExtra, InterpretCx, PlaceTy, OpTy, ImmTy, MemoryKind, Pointer, Memory }; @@ -209,17 +209,19 @@ pub trait Machine<'mir, 'tcx>: Sized { extra: Self::FrameExtra, ) -> InterpResult<'tcx>; + #[inline(always)] fn int_to_ptr( _mem: &Memory<'mir, 'tcx, Self>, int: u64, ) -> InterpResult<'tcx, Pointer> { - if int == 0 { - err!(InvalidNullPointerUsage) + Err((if int == 0 { + InterpError::InvalidNullPointerUsage } else { - err!(ReadBytesAsPointer) - } + InterpError::ReadBytesAsPointer + }).into()) } + #[inline(always)] fn ptr_to_int( _mem: &Memory<'mir, 'tcx, Self>, _ptr: Pointer, From ffc6670234fc9ddda92efaf371e6f82c2016bb7e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 10:49:02 +0200 Subject: [PATCH 4/4] organize methods a bit better --- src/librustc_mir/interpret/eval_context.rs | 74 +++++++++++----------- 1 file changed, 38 insertions(+), 36 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index c6e762bddd4d9..f998ae16d6f21 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -217,6 +217,23 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpretCx<'mir, 'tcx, M> { &mut self.memory } + #[inline(always)] + pub fn force_ptr( + &self, + scalar: Scalar, + ) -> InterpResult<'tcx, Pointer> { + self.memory.force_ptr(scalar) + } + + #[inline(always)] + pub fn force_bits( + &self, + scalar: Scalar, + size: Size + ) -> InterpResult<'tcx, u128> { + self.memory.force_bits(scalar, size) + } + #[inline(always)] pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer { self.memory.tag_static_base_pointer(ptr) @@ -248,6 +265,27 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpretCx<'mir, 'tcx, M> { self.frame().body } + #[inline(always)] + pub fn sign_extend(&self, value: u128, ty: TyLayout<'_>) -> u128 { + assert!(ty.abi.is_signed()); + sign_extend(value, ty.size) + } + + #[inline(always)] + pub fn truncate(&self, value: u128, ty: TyLayout<'_>) -> u128 { + truncate(value, ty.size) + } + + #[inline] + pub fn type_is_sized(&self, ty: Ty<'tcx>) -> bool { + ty.is_sized(self.tcx, self.param_env) + } + + #[inline] + pub fn type_is_freeze(&self, ty: Ty<'tcx>) -> bool { + ty.is_freeze(*self.tcx, self.param_env, DUMMY_SP) + } + pub(super) fn subst_and_normalize_erasing_regions>( &self, substs: T, @@ -283,14 +321,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpretCx<'mir, 'tcx, M> { ).ok_or_else(|| InterpError::TooGeneric.into()) } - pub fn type_is_sized(&self, ty: Ty<'tcx>) -> bool { - ty.is_sized(self.tcx, self.param_env) - } - - pub fn type_is_freeze(&self, ty: Ty<'tcx>) -> bool { - ty.is_freeze(*self.tcx, self.param_env, DUMMY_SP) - } - pub fn load_mir( &self, instance: ty::InstanceDef<'tcx>, @@ -761,32 +791,4 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpretCx<'mir, 'tcx, M> { trace!("generate stacktrace: {:#?}, {:?}", frames, explicit_span); frames } - - #[inline(always)] - pub fn sign_extend(&self, value: u128, ty: TyLayout<'_>) -> u128 { - assert!(ty.abi.is_signed()); - sign_extend(value, ty.size) - } - - #[inline(always)] - pub fn truncate(&self, value: u128, ty: TyLayout<'_>) -> u128 { - truncate(value, ty.size) - } - - #[inline(always)] - pub fn force_ptr( - &self, - scalar: Scalar, - ) -> InterpResult<'tcx, Pointer> { - self.memory.force_ptr(scalar) - } - - #[inline(always)] - pub fn force_bits( - &self, - scalar: Scalar, - size: Size - ) -> InterpResult<'tcx, u128> { - self.memory.force_bits(scalar, size) - } }