Skip to content

Commit

Permalink
Rollup merge of rust-lang#95620 - RalfJung:memory-no-extras, r=oli-obk
Browse files Browse the repository at this point in the history
interpret: remove MemoryExtra in favor of giving access to the Machine

The Miri PR for this is upcoming.

r? `@oli-obk`
  • Loading branch information
Dylan-DPC authored Apr 4, 2022
2 parents b3dd3b7 + 84a343d commit db44e8d
Show file tree
Hide file tree
Showing 15 changed files with 201 additions and 253 deletions.
8 changes: 3 additions & 5 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{CompileTimeEvalContext, CompileTimeInterpreter, ConstEvalErr, MemoryExtra};
use super::{CompileTimeEvalContext, CompileTimeInterpreter, ConstEvalErr};
use crate::interpret::eval_nullary_intrinsic;
use crate::interpret::{
intern_const_alloc_recursive, Allocation, ConstAlloc, ConstValue, CtfeValidationMode, GlobalId,
Expand Down Expand Up @@ -100,8 +100,7 @@ pub(super) fn mk_eval_cx<'mir, 'tcx>(
tcx,
root_span,
param_env,
CompileTimeInterpreter::new(tcx.const_eval_limit()),
MemoryExtra { can_access_statics },
CompileTimeInterpreter::new(tcx.const_eval_limit(), can_access_statics),
)
}

Expand Down Expand Up @@ -285,10 +284,9 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
tcx,
tcx.def_span(def.did),
key.param_env,
CompileTimeInterpreter::new(tcx.const_eval_limit()),
// Statics (and promoteds inside statics) may access other statics, because unlike consts
// they do not have to behave "as if" they were evaluated at runtime.
MemoryExtra { can_access_statics: is_static },
CompileTimeInterpreter::new(tcx.const_eval_limit(), /*can_access_statics:*/ is_static),
);

let res = ecx.load_mir(cid.instance.def, cid.promoted);
Expand Down
23 changes: 11 additions & 12 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,7 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {

/// The virtual call stack.
pub(crate) stack: Vec<Frame<'mir, 'tcx, AllocId, ()>>,
}

#[derive(Copy, Clone, Debug)]
pub struct MemoryExtra {
/// We need to make sure consts never point to anything mutable, even recursively. That is
/// relied on for pattern matching on consts with references.
/// To achieve this, two pieces have to work together:
Expand All @@ -107,8 +104,12 @@ pub struct MemoryExtra {
}

impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {
pub(super) fn new(const_eval_limit: Limit) -> Self {
CompileTimeInterpreter { steps_remaining: const_eval_limit.0, stack: Vec::new() }
pub(super) fn new(const_eval_limit: Limit, can_access_statics: bool) -> Self {
CompileTimeInterpreter {
steps_remaining: const_eval_limit.0,
stack: Vec::new(),
can_access_statics,
}
}
}

Expand Down Expand Up @@ -233,8 +234,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,

type MemoryKind = MemoryKind;

type MemoryExtra = MemoryExtra;

const PANIC_ON_ALLOC_FAIL: bool = false; // will be raised as a proper error

fn load_mir(
Expand Down Expand Up @@ -345,7 +344,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
Err(err) => throw_ub_format!("align has to be a power of 2, {}", err),
};

let ptr = ecx.memory.allocate(
let ptr = ecx.allocate_ptr(
Size::from_bytes(size as u64),
align,
interpret::MemoryKind::Machine(MemoryKind::Heap),
Expand All @@ -365,14 +364,14 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,

// If an allocation is created in an another const,
// we don't deallocate it.
let (alloc_id, _, _) = ecx.memory.ptr_get_alloc(ptr)?;
let (alloc_id, _, _) = ecx.ptr_get_alloc_id(ptr)?;
let is_allocated_in_another_const = matches!(
ecx.tcx.get_global_alloc(alloc_id),
Some(interpret::GlobalAlloc::Memory(_))
);

if !is_allocated_in_another_const {
ecx.memory.deallocate(
ecx.deallocate_ptr(
ptr,
Some((size, align)),
interpret::MemoryKind::Machine(MemoryKind::Heap),
Expand Down Expand Up @@ -472,7 +471,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
}

fn before_access_global(
memory_extra: &MemoryExtra,
machine: &Self,
alloc_id: AllocId,
alloc: ConstAllocation<'tcx>,
static_def_id: Option<DefId>,
Expand All @@ -488,7 +487,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
}
} else {
// Read access. These are usually allowed, with some exceptions.
if memory_extra.can_access_statics {
if machine.can_access_statics {
// Machine configuration allows us read from anything (e.g., `static` initializer).
Ok(())
} else if static_def_id.is_some() {
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_const_eval/src/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
)
.ok_or_else(|| err_inval!(TooGeneric))?;

let fn_ptr = self.memory.create_fn_alloc(FnVal::Instance(instance));
let fn_ptr = self.create_fn_alloc_ptr(FnVal::Instance(instance));
self.write_pointer(fn_ptr, dest)?;
}
_ => span_bug!(self.cur_span(), "reify fn pointer on {:?}", src.layout.ty),
Expand Down Expand Up @@ -87,7 +87,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
substs,
ty::ClosureKind::FnOnce,
);
let fn_ptr = self.memory.create_fn_alloc(FnVal::Instance(instance));
let fn_ptr = self.create_fn_alloc_ptr(FnVal::Instance(instance));
self.write_pointer(fn_ptr, dest)?;
}
_ => span_bug!(self.cur_span(), "closure fn pointer on {:?}", src.layout.ty),
Expand Down Expand Up @@ -153,8 +153,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
return Ok(**src);
} else {
// Casting the metadata away from a fat ptr.
assert_eq!(src.layout.size, 2 * self.memory.pointer_size());
assert_eq!(dest_layout.size, self.memory.pointer_size());
assert_eq!(src.layout.size, 2 * self.pointer_size());
assert_eq!(dest_layout.size, self.pointer_size());
assert!(src.layout.ty.is_unsafe_ptr());
return match **src {
Immediate::ScalarPair(data, _) => Ok(data.into()),
Expand Down
60 changes: 8 additions & 52 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ use rustc_span::{Pos, Span};
use rustc_target::abi::{call::FnAbi, Align, HasDataLayout, Size, TargetDataLayout};

use super::{
AllocCheck, AllocId, GlobalId, Immediate, InterpErrorInfo, InterpResult, MPlaceTy, Machine,
MemPlace, MemPlaceMeta, Memory, MemoryKind, Operand, Place, PlaceTy, Pointer,
PointerArithmetic, Provenance, Scalar, ScalarMaybeUninit, StackPopJump,
AllocId, GlobalId, Immediate, InterpErrorInfo, InterpResult, MPlaceTy, Machine, MemPlace,
MemPlaceMeta, Memory, MemoryKind, Operand, Place, PlaceTy, PointerArithmetic, Provenance,
Scalar, ScalarMaybeUninit, StackPopJump,
};
use crate::transform::validate::equal_up_to_regions;

Expand Down Expand Up @@ -413,13 +413,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
root_span: Span,
param_env: ty::ParamEnv<'tcx>,
machine: M,
memory_extra: M::MemoryExtra,
) -> Self {
InterpCx {
machine,
tcx: tcx.at(root_span),
param_env,
memory: Memory::new(tcx, memory_extra),
memory: Memory::new(),
recursion_limit: tcx.recursion_limit(),
}
}
Expand All @@ -433,49 +432,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
.map_or(self.tcx.span, |f| f.current_span())
}

#[inline(always)]
pub fn scalar_to_ptr(&self, scalar: Scalar<M::PointerTag>) -> Pointer<Option<M::PointerTag>> {
self.memory.scalar_to_ptr(scalar)
}

/// Test if this value might be null.
/// If the machine does not support ptr-to-int casts, this is conservative.
pub fn scalar_may_be_null(&self, scalar: Scalar<M::PointerTag>) -> bool {
match scalar.try_to_int() {
Ok(int) => int.is_null(),
Err(_) => {
// Can only happen during CTFE.
let ptr = self.scalar_to_ptr(scalar);
match self.memory.ptr_try_get_alloc(ptr) {
Ok((alloc_id, offset, _)) => {
let (size, _align) = self
.memory
.get_size_and_align(alloc_id, AllocCheck::MaybeDead)
.expect("alloc info with MaybeDead cannot fail");
// If the pointer is out-of-bounds, it may be null.
// Note that one-past-the-end (offset == size) is still inbounds, and never null.
offset > size
}
Err(_offset) => bug!("a non-int scalar is always a pointer"),
}
}
}
}

/// Call this to turn untagged "global" pointers (obtained via `tcx`) into
/// the machine pointer to the allocation. Must never be used
/// for any other pointers, nor for TLS statics.
///
/// Using the resulting pointer represents a *direct* access to that memory
/// (e.g. by directly using a `static`),
/// as opposed to access through a pointer that was created by the program.
///
/// This function can fail only if `ptr` points to an `extern static`.
#[inline(always)]
pub fn global_base_pointer(&self, ptr: Pointer) -> InterpResult<'tcx, Pointer<M::PointerTag>> {
self.memory.global_base_pointer(ptr)
}

#[inline(always)]
pub(crate) fn stack(&self) -> &[Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>] {
M::stack(self)
Expand Down Expand Up @@ -949,9 +905,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
trace!(
"deallocating local {:?}: {:?}",
local,
self.memory.dump_alloc(ptr.provenance.unwrap().get_alloc_id())
self.dump_alloc(ptr.provenance.unwrap().get_alloc_id())
);
self.memory.deallocate(ptr, None, MemoryKind::Stack)?;
self.deallocate_ptr(ptr, None, MemoryKind::Stack)?;
};
Ok(())
}
Expand Down Expand Up @@ -1057,15 +1013,15 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> std::fmt::Debug
}
}

write!(fmt, ": {:?}", self.ecx.memory.dump_allocs(allocs))
write!(fmt, ": {:?}", self.ecx.dump_allocs(allocs))
}
Place::Ptr(mplace) => match mplace.ptr.provenance.map(Provenance::get_alloc_id) {
Some(alloc_id) => write!(
fmt,
"by align({}) ref {:?}: {:?}",
mplace.align.bytes(),
mplace.ptr,
self.ecx.memory.dump_alloc(alloc_id)
self.ecx.dump_alloc(alloc_id)
),
ptr => write!(fmt, " integral by ref: {:?}", ptr),
},
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// exception from the exception.)
// This is the dual to the special exception for offset-by-0
// in the inbounds pointer offset operation (see `ptr_offset_inbounds` below).
match (self.memory.ptr_try_get_alloc(a), self.memory.ptr_try_get_alloc(b)) {
match (self.ptr_try_get_alloc_id(a), self.ptr_try_get_alloc_id(b)) {
(Err(a), Err(b)) if a == b && a != 0 => {
// Both are the same non-null integer.
self.write_scalar(Scalar::from_machine_isize(0, self), dest)?;
Expand All @@ -335,13 +335,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
);
}
// And they must both be valid for zero-sized accesses ("in-bounds or one past the end").
self.memory.check_ptr_access_align(
self.check_ptr_access_align(
a,
Size::ZERO,
Align::ONE,
CheckInAllocMsg::OffsetFromTest,
)?;
self.memory.check_ptr_access_align(
self.check_ptr_access_align(
b,
Size::ZERO,
Align::ONE,
Expand Down Expand Up @@ -545,7 +545,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let min_ptr = if offset_bytes >= 0 { ptr } else { offset_ptr };
let size = offset_bytes.unsigned_abs();
// This call handles checking for integer/null pointers.
self.memory.check_ptr_access_align(
self.check_ptr_access_align(
min_ptr,
Size::from_bytes(size),
Align::ONE,
Expand Down Expand Up @@ -577,7 +577,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let src = self.read_pointer(&src)?;
let dst = self.read_pointer(&dst)?;

self.memory.copy(src, align, dst, align, size, nonoverlapping)
self.mem_copy(src, align, dst, align, size, nonoverlapping)
}

pub(crate) fn write_bytes_intrinsic(
Expand All @@ -600,7 +600,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
.ok_or_else(|| err_ub_format!("overflow computing total size of `write_bytes`"))?;

let bytes = std::iter::repeat(byte).take(len.bytes_usize());
self.memory.write_bytes(dst, bytes)
self.write_bytes_ptr(dst, bytes)
}

pub(crate) fn raw_eq_intrinsic(
Expand All @@ -613,8 +613,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

let lhs = self.read_pointer(lhs)?;
let rhs = self.read_pointer(rhs)?;
let lhs_bytes = self.memory.read_bytes(lhs, layout.size)?;
let rhs_bytes = self.memory.read_bytes(rhs, layout.size)?;
let lhs_bytes = self.read_bytes_ptr(lhs, layout.size)?;
let rhs_bytes = self.read_bytes_ptr(rhs, layout.size)?;
Ok(Scalar::from_bool(lhs_bytes == rhs_bytes))
}
}
Loading

0 comments on commit db44e8d

Please sign in to comment.