Skip to content

Miri: let machine canonicalize AllocIDs #69408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 2, 2020
Merged

Miri: let machine canonicalize AllocIDs #69408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5da3939
miri/machine: add canonical_alloc_id hook to replace find_foreign_static
RalfJung Feb 23, 2020
01d9329
canonicalize alloc ID before calling tag_static_base_pointer
RalfJung Feb 23, 2020
9b62d60
fmt
RalfJung Feb 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions src/librustc_mir/const_eval/machine.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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