Skip to content

Commit

Permalink
Rollup merge of rust-lang#69408 - RalfJung:canonical-alloc-id, r=oli-obk
Browse files Browse the repository at this point in the history
Miri: let machine canonicalize AllocIDs

This implements the rustc side of the plan I laid out [here](rust-lang/miri#1147 (comment)).

Miri PR: rust-lang/miri#1190
  • Loading branch information
Dylan-DPC authored Feb 28, 2020
2 parents e5da563 + 9b62d60 commit 04f5aae
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 66 deletions.
10 changes: 1 addition & 9 deletions src/librustc_mir/const_eval/machine.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use rustc::mir;
use rustc::ty::layout::HasTyCtxt;
use rustc::ty::{self, Ty, TyCtxt};
use rustc_hir::def_id::DefId;
use rustc::ty::{self, Ty};
use std::borrow::{Borrow, Cow};
use std::collections::hash_map::Entry;
use std::hash::Hash;
Expand Down Expand Up @@ -320,13 +319,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
Err(ConstEvalErrKind::NeedsRfc("pointer arithmetic or comparison".to_string()).into())
}

fn find_foreign_static(
_tcx: TyCtxt<'tcx>,
_def_id: DefId,
) -> InterpResult<'tcx, Cow<'tcx, Allocation<Self::PointerTag>>> {
throw_unsup!(ReadForeignStatic)
}

#[inline(always)]
fn init_allocation_extra<'b>(
_memory_extra: &MemoryExtra,
Expand Down
43 changes: 24 additions & 19 deletions src/librustc_mir/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ use std::borrow::{Borrow, Cow};
use std::hash::Hash;

use rustc::mir;
use rustc::ty::{self, Ty, TyCtxt};
use rustc_hir::def_id::DefId;
use rustc::ty::{self, Ty};
use rustc_span::Span;

use super::{
Expand Down Expand Up @@ -123,10 +122,6 @@ pub trait Machine<'mir, 'tcx>: Sized {
/// Whether to enforce the validity invariant
fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;

/// Called before a basic block terminator is executed.
/// You can use this to detect endlessly running programs.
fn before_terminator(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx>;

/// Entry point to all function calls.
///
/// Returns either the mir to use for the call, or `None` if execution should
Expand Down Expand Up @@ -175,18 +170,6 @@ pub trait Machine<'mir, 'tcx>: Sized {
unwind: Option<mir::BasicBlock>,
) -> InterpResult<'tcx>;

/// Called for read access to a foreign static item.
///
/// This will only be called once per static and machine; the result is cached in
/// the machine memory. (This relies on `AllocMap::get_or` being able to add the
/// owned allocation to the map even when the map is shared.)
///
/// This allocation will then be fed to `tag_allocation` to initialize the "extra" state.
fn find_foreign_static(
tcx: TyCtxt<'tcx>,
def_id: DefId,
) -> InterpResult<'tcx, Cow<'tcx, Allocation>>;

/// Called for all binary operations where the LHS has pointer type.
///
/// Returns a (value, overflowed) pair if the operation succeeded
Expand All @@ -204,6 +187,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
) -> InterpResult<'tcx>;

/// Called to read the specified `local` from the `frame`.
#[inline]
fn access_local(
_ecx: &InterpCx<'mir, 'tcx, Self>,
frame: &Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>,
Expand All @@ -212,14 +196,33 @@ pub trait Machine<'mir, 'tcx>: Sized {
frame.locals[local].access()
}

/// Called before a basic block terminator is executed.
/// You can use this to detect endlessly running programs.
#[inline]
fn before_terminator(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
Ok(())
}

/// Called before a `Static` value is accessed.
#[inline]
fn before_access_static(
_memory_extra: &Self::MemoryExtra,
_allocation: &Allocation,
) -> InterpResult<'tcx> {
Ok(())
}

/// Called for *every* memory access to determine the real ID of the given allocation.
/// This provides a way for the machine to "redirect" certain allocations as it sees fit.
///
/// This is used by Miri to redirect extern statics to real allocations.
///
/// This function must be idempotent.
#[inline]
fn canonical_alloc_id(_mem: &Memory<'mir, 'tcx, Self>, id: AllocId) -> AllocId {
id
}

/// Called to initialize the "extra" state of an allocation and make the pointers
/// it contains (in relocations) tagged. The way we construct allocations is
/// to always first construct it without extra and then add the extra.
Expand Down Expand Up @@ -247,6 +250,8 @@ pub trait Machine<'mir, 'tcx>: Sized {
/// Return the "base" tag for the given *static* allocation: the one that is used for direct
/// accesses to this static/const/fn allocation. If `id` is not a static allocation,
/// this will return an unusable tag (i.e., accesses will be UB)!
///
/// Expects `id` to be already canonical, if needed.
fn tag_static_base_pointer(memory_extra: &Self::MemoryExtra, id: AllocId) -> Self::PointerTag;

/// Executes a retagging operation
Expand All @@ -259,7 +264,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
Ok(())
}

/// Called immediately before a new stack frame got pushed
/// Called immediately before a new stack frame got pushed.
fn stack_push(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx, Self::FrameExtra>;

/// Called immediately after a stack frame gets popped
Expand Down
59 changes: 33 additions & 26 deletions src/librustc_mir/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
/// through a pointer that was created by the program.
#[inline]
pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer<M::PointerTag> {
ptr.with_tag(M::tag_static_base_pointer(&self.extra, ptr.alloc_id))
let id = M::canonical_alloc_id(self, ptr.alloc_id);
ptr.with_tag(M::tag_static_base_pointer(&self.extra, id))
}

pub fn create_fn_alloc(
Expand Down Expand Up @@ -421,6 +422,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
/// The `GlobalAlloc::Memory` branch here is still reachable though; when a static
/// 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.
///
/// Assumes `id` is already canonical.
fn get_static_alloc(
memory_extra: &M::MemoryExtra,
tcx: TyCtxtAt<'tcx>,
Expand All @@ -434,31 +437,30 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
Some(GlobalAlloc::Static(def_id)) => {
// 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(tcx.tcx, def_id)?
} else {
trace!("static_alloc: Need to compute {:?}", def_id);
let instance = Instance::mono(tcx.tcx, def_id);
let gid = GlobalId { instance, promoted: None };
// 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)).map_err(|err| {
// no need to report anything, the const_eval call takes care of that
// for statics
assert!(tcx.is_static(def_id));
match err {
ErrorHandled::Reported => err_inval!(ReferencedConstant),
ErrorHandled::TooGeneric => err_inval!(TooGeneric),
}
})?;
// Make sure we use the ID of the resolved memory, not the lazy one!
let id = raw_const.alloc_id;
let allocation = tcx.alloc_map.lock().unwrap_memory(id);

M::before_access_static(memory_extra, allocation)?;
Cow::Borrowed(allocation)
trace!("get_static_alloc: foreign item {:?}", def_id);
throw_unsup!(ReadForeignStatic)
}
trace!("get_static_alloc: Need to compute {:?}", def_id);
let instance = Instance::mono(tcx.tcx, def_id);
let gid = GlobalId { instance, promoted: None };
// 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)).map_err(|err| {
// no need to report anything, the const_eval call takes care of that
// for statics
assert!(tcx.is_static(def_id));
match err {
ErrorHandled::Reported => err_inval!(ReferencedConstant),
ErrorHandled::TooGeneric => err_inval!(TooGeneric),
}
})?;
// Make sure we use the ID of the resolved memory, not the lazy one!
let id = raw_const.alloc_id;
let allocation = tcx.alloc_map.lock().unwrap_memory(id);

M::before_access_static(memory_extra, allocation)?;
Cow::Borrowed(allocation)
}
};
// We got tcx memory. Let the machine initialize its "extra" stuff.
Expand All @@ -478,6 +480,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
&self,
id: AllocId,
) -> InterpResult<'tcx, &Allocation<M::PointerTag, M::AllocExtra>> {
let id = M::canonical_alloc_id(self, id);
// The error type of the inner closure here is somewhat funny. We have two
// ways of "erroring": An actual error, or because we got a reference from
// `get_static_alloc` that we can actually use directly without inserting anything anywhere.
Expand Down Expand Up @@ -513,6 +516,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
&mut self,
id: AllocId,
) -> InterpResult<'tcx, &mut Allocation<M::PointerTag, M::AllocExtra>> {
let id = M::canonical_alloc_id(self, id);
let tcx = self.tcx;
let memory_extra = &self.extra;
let a = self.alloc_map.get_mut_or(id, || {
Expand Down Expand Up @@ -550,6 +554,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
id: AllocId,
liveness: AllocCheck,
) -> InterpResult<'static, (Size, Align)> {
let id = M::canonical_alloc_id(self, id);
// # Regular allocations
// Don't use `self.get_raw` here as that will
// a) cause cycles in case `id` refers to a static
Expand Down Expand Up @@ -602,6 +607,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
}
}

/// Assumes `id` is already canonical.
fn get_fn_alloc(&self, id: AllocId) -> Option<FnVal<'tcx, M::ExtraFnVal>> {
trace!("reading fn ptr: {}", id);
if let Some(extra) = self.extra_fn_ptr_map.get(&id) {
Expand All @@ -622,7 +628,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
if ptr.offset.bytes() != 0 {
throw_unsup!(InvalidFunctionPointer)
}
self.get_fn_alloc(ptr.alloc_id).ok_or_else(|| err_unsup!(ExecuteMemory).into())
let id = M::canonical_alloc_id(self, ptr.alloc_id);
self.get_fn_alloc(id).ok_or_else(|| err_unsup!(ExecuteMemory).into())
}

pub fn mark_immutable(&mut self, id: AllocId) -> InterpResult<'tcx> {
Expand Down
12 changes: 0 additions & 12 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use rustc::ty::subst::{InternalSubsts, Subst};
use rustc::ty::{self, ConstKind, Instance, ParamEnv, Ty, TyCtxt, TypeFoldable};
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_hir::HirId;
use rustc_index::vec::IndexVec;
use rustc_infer::traits;
Expand Down Expand Up @@ -222,13 +221,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine {
));
}

fn find_foreign_static(
_tcx: TyCtxt<'tcx>,
_def_id: DefId,
) -> InterpResult<'tcx, Cow<'tcx, Allocation<Self::PointerTag>>> {
throw_unsup!(ReadForeignStatic)
}

#[inline(always)]
fn init_allocation_extra<'b>(
_memory_extra: &(),
Expand Down Expand Up @@ -279,10 +271,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine {
Ok(())
}

fn before_terminator(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
Ok(())
}

#[inline(always)]
fn stack_push(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
Ok(())
Expand Down

0 comments on commit 04f5aae

Please sign in to comment.