Skip to content
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.

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

Miri: refactor new allocation tagging #59986

Merged
merged 1 commit into from
Apr 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
73 changes: 20 additions & 53 deletions src/librustc/mir/interpret/allocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,10 @@ pub struct Allocation<Tag=(),Extra=()> {
}


pub trait AllocationExtra<Tag, MemoryExtra>: ::std::fmt::Debug + Clone {
/// Hook to initialize the extra data when an allocation gets created.
fn memory_allocated(
_size: Size,
_memory_extra: &MemoryExtra
) -> Self;
pub trait AllocationExtra<Tag>: ::std::fmt::Debug + Clone {
// There is no constructor in here because the constructor's type depends
// on `MemoryKind`, and making things sufficiently generic leads to painful
// inference failure.

/// Hook for performing extra checks on a memory read access.
///
Expand Down Expand Up @@ -88,15 +86,8 @@ pub trait AllocationExtra<Tag, MemoryExtra>: ::std::fmt::Debug + Clone {
}
}

impl AllocationExtra<(), ()> for () {
#[inline(always)]
fn memory_allocated(
_size: Size,
_memory_extra: &()
) -> Self {
()
}
}
// For Tag=() and no extra state, we have is a trivial implementation.
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
impl AllocationExtra<()> for () { }

impl<Tag, Extra> Allocation<Tag, Extra> {
/// Creates a read-only allocation initialized by the given bytes
Expand Down Expand Up @@ -159,23 +150,21 @@ impl<'tcx, Tag, Extra> Allocation<Tag, Extra> {
}

/// Byte accessors
impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
/// The last argument controls whether we error out when there are undefined
/// or pointer bytes. You should never call this, call `get_bytes` or
/// `get_bytes_with_undef_and_ptr` instead,
///
/// This function also guarantees that the resulting pointer will remain stable
/// even when new allocations are pushed to the `HashMap`. `copy_repeatedly` relies
/// on that.
fn get_bytes_internal<MemoryExtra>(
fn get_bytes_internal(
&self,
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
size: Size,
check_defined_and_ptr: bool,
) -> EvalResult<'tcx, &[u8]>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
self.check_bounds(cx, ptr, size)?;

Expand All @@ -196,43 +185,37 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
}

#[inline]
pub fn get_bytes<MemoryExtra>(
pub fn get_bytes(
&self,
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
size: Size,
) -> EvalResult<'tcx, &[u8]>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
self.get_bytes_internal(cx, ptr, size, true)
}

/// It is the caller's responsibility to handle undefined and pointer bytes.
/// However, this still checks that there are no relocations on the *edges*.
#[inline]
pub fn get_bytes_with_undef_and_ptr<MemoryExtra>(
pub fn get_bytes_with_undef_and_ptr(
&self,
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
size: Size,
) -> EvalResult<'tcx, &[u8]>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
self.get_bytes_internal(cx, ptr, size, false)
}

/// Just calling this already marks everything as defined and removes relocations,
/// so be sure to actually put data there!
pub fn get_bytes_mut<MemoryExtra>(
pub fn get_bytes_mut(
&mut self,
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
size: Size,
) -> EvalResult<'tcx, &mut [u8]>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`");
self.check_bounds(cx, ptr, size)?;
Expand All @@ -250,16 +233,14 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
}

/// Reading and writing
impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
/// Reads bytes until a `0` is encountered. Will error if the end of the allocation is reached
/// before a `0` is found.
pub fn read_c_str<MemoryExtra>(
pub fn read_c_str(
&self,
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
) -> EvalResult<'tcx, &[u8]>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
assert_eq!(ptr.offset.bytes() as usize as u64, ptr.offset.bytes());
let offset = ptr.offset.bytes() as usize;
Expand All @@ -278,15 +259,13 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
/// Validates that `ptr.offset` and `ptr.offset + size` do not point to the middle of a
/// relocation. If `allow_ptr_and_undef` is `false`, also enforces that the memory in the
/// given range contains neither relocations nor undef bytes.
pub fn check_bytes<MemoryExtra>(
pub fn check_bytes(
&self,
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
size: Size,
allow_ptr_and_undef: bool,
) -> EvalResult<'tcx>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
// Check bounds and relocations on the edges
self.get_bytes_with_undef_and_ptr(cx, ptr, size)?;
Expand All @@ -301,30 +280,26 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
/// Writes `src` to the memory starting at `ptr.offset`.
///
/// Will do bounds checks on the allocation.
pub fn write_bytes<MemoryExtra>(
pub fn write_bytes(
&mut self,
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
src: &[u8],
) -> EvalResult<'tcx>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
let bytes = self.get_bytes_mut(cx, ptr, Size::from_bytes(src.len() as u64))?;
bytes.clone_from_slice(src);
Ok(())
}

/// Sets `count` bytes starting at `ptr.offset` with `val`. Basically `memset`.
pub fn write_repeat<MemoryExtra>(
pub fn write_repeat(
&mut self,
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
val: u8,
count: Size
) -> EvalResult<'tcx>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
let bytes = self.get_bytes_mut(cx, ptr, count)?;
for b in bytes {
Expand All @@ -341,14 +316,12 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
/// being valid for ZSTs
///
/// Note: This function does not do *any* alignment checks, you need to do these before calling
pub fn read_scalar<MemoryExtra>(
pub fn read_scalar(
&self,
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
size: Size
) -> EvalResult<'tcx, ScalarMaybeUndef<Tag>>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
// get_bytes_unchecked tests relocation edges
let bytes = self.get_bytes_with_undef_and_ptr(cx, ptr, size)?;
Expand Down Expand Up @@ -379,13 +352,11 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
}

/// Note: This function does not do *any* alignment checks, you need to do these before calling
pub fn read_ptr_sized<MemoryExtra>(
pub fn read_ptr_sized(
&self,
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
) -> EvalResult<'tcx, ScalarMaybeUndef<Tag>>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
self.read_scalar(cx, ptr, cx.data_layout().pointer_size)
}
Expand All @@ -398,15 +369,13 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
/// being valid for ZSTs
///
/// Note: This function does not do *any* alignment checks, you need to do these before calling
pub fn write_scalar<MemoryExtra>(
pub fn write_scalar(
&mut self,
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
val: ScalarMaybeUndef<Tag>,
type_size: Size,
) -> EvalResult<'tcx>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
let val = match val {
ScalarMaybeUndef::Scalar(scalar) => scalar,
Expand Down Expand Up @@ -446,14 +415,12 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
}

/// Note: This function does not do *any* alignment checks, you need to do these before calling
pub fn write_ptr_sized<MemoryExtra>(
pub fn write_ptr_sized(
&mut self,
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
val: ScalarMaybeUndef<Tag>
) -> EvalResult<'tcx>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
let ptr_size = cx.data_layout().pointer_size;
self.write_scalar(cx, ptr.into(), val, ptr_size)
Expand Down
8 changes: 7 additions & 1 deletion src/librustc/mir/interpret/pointer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,17 @@ impl<'tcx> Pointer<()> {
Pointer { alloc_id, offset, tag: () }
}

#[inline(always)]
pub fn with_tag<Tag>(self, tag: Tag) -> Pointer<Tag>
{
Pointer::new_with_tag(self.alloc_id, self.offset, tag)
}

#[inline(always)]
pub fn with_default_tag<Tag>(self) -> Pointer<Tag>
where Tag: Default
{
Pointer::new_with_tag(self.alloc_id, self.offset, Default::default())
self.with_tag(Tag::default())
}
}

Expand Down
34 changes: 18 additions & 16 deletions src/librustc/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,19 @@ impl<Tag> fmt::Display for Scalar<Tag> {

impl<'tcx> Scalar<()> {
#[inline]
pub fn with_default_tag<Tag>(self) -> Scalar<Tag>
where Tag: Default
{
pub fn with_tag<Tag>(self, new_tag: Tag) -> Scalar<Tag> {
match self {
Scalar::Ptr(ptr) => Scalar::Ptr(ptr.with_default_tag()),
Scalar::Ptr(ptr) => Scalar::Ptr(ptr.with_tag(new_tag)),
Scalar::Bits { bits, size } => Scalar::Bits { bits, size },
}
}

#[inline(always)]
pub fn with_default_tag<Tag>(self) -> Scalar<Tag>
where Tag: Default
{
self.with_tag(Tag::default())
}
}

impl<'tcx, Tag> Scalar<Tag> {
Expand All @@ -138,14 +143,6 @@ impl<'tcx, Tag> Scalar<Tag> {
}
}

#[inline]
pub fn with_tag(self, new_tag: Tag) -> Self {
match self {
Scalar::Ptr(ptr) => Scalar::Ptr(Pointer { tag: new_tag, ..ptr }),
Scalar::Bits { bits, size } => Scalar::Bits { bits, size },
}
}

#[inline]
pub fn ptr_null(cx: &impl HasDataLayout) -> Self {
Scalar::Bits {
Expand Down Expand Up @@ -434,14 +431,19 @@ impl<Tag> fmt::Display for ScalarMaybeUndef<Tag> {

impl<'tcx> ScalarMaybeUndef<()> {
#[inline]
pub fn with_default_tag<Tag>(self) -> ScalarMaybeUndef<Tag>
where Tag: Default
{
pub fn with_tag<Tag>(self, new_tag: Tag) -> ScalarMaybeUndef<Tag> {
match self {
ScalarMaybeUndef::Scalar(s) => ScalarMaybeUndef::Scalar(s.with_default_tag()),
ScalarMaybeUndef::Scalar(s) => ScalarMaybeUndef::Scalar(s.with_tag(new_tag)),
ScalarMaybeUndef::Undef => ScalarMaybeUndef::Undef,
}
}

#[inline(always)]
pub fn with_default_tag<Tag>(self) -> ScalarMaybeUndef<Tag>
where Tag: Default
{
self.with_tag(Tag::default())
}
}

impl<'tcx, Tag> ScalarMaybeUndef<Tag> {
Expand Down
22 changes: 11 additions & 11 deletions src/librustc_mir/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc::hir::def::Def;
use rustc::mir::interpret::{ConstEvalErr, ErrorHandled};
use rustc::mir;
use rustc::ty::{self, TyCtxt, query::TyCtxtAt};
use rustc::ty::layout::{self, LayoutOf, VariantIdx};
use rustc::ty::layout::{self, LayoutOf, VariantIdx, Size};
use rustc::ty::subst::Subst;
use rustc::traits::Reveal;
use rustc::util::common::ErrorReported;
Expand All @@ -21,7 +21,7 @@ use syntax::ast::Mutability;
use syntax::source_map::{Span, DUMMY_SP};

use crate::interpret::{self,
PlaceTy, MPlaceTy, MemPlace, OpTy, ImmTy, Immediate, Scalar, Pointer,
PlaceTy, MPlaceTy, MemPlace, OpTy, ImmTy, Immediate, Scalar,
RawConst, ConstValue,
EvalResult, EvalError, InterpError, GlobalId, InterpretCx, StackPopCleanup,
Allocation, AllocId, MemoryKind,
Expand Down Expand Up @@ -406,6 +406,15 @@ impl<'a, 'mir, 'tcx> interpret::Machine<'a, 'mir, 'tcx>
Cow::Borrowed(alloc)
}

#[inline(always)]
fn new_allocation(
_size: Size,
_extra: &Self::MemoryExtra,
_kind: MemoryKind<!>,
) -> (Self::AllocExtra, Self::PointerTag) {
((), ())
}

fn box_alloc(
_ecx: &mut InterpretCx<'a, 'mir, 'tcx, Self>,
_dest: PlaceTy<'tcx>,
Expand Down Expand Up @@ -439,15 +448,6 @@ impl<'a, 'mir, 'tcx> interpret::Machine<'a, 'mir, 'tcx>
)
}

#[inline(always)]
fn tag_new_allocation(
_ecx: &mut InterpretCx<'a, 'mir, 'tcx, Self>,
ptr: Pointer,
_kind: MemoryKind<Self::MemoryKinds>,
) -> Pointer {
ptr
}

#[inline(always)]
fn stack_push(
_ecx: &mut InterpretCx<'a, 'mir, 'tcx, Self>,
Expand Down
Loading