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

Remove const eval loop detector #70087

Merged
merged 4 commits into from
Mar 24, 2020
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
14 changes: 7 additions & 7 deletions src/librustc/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,8 +565,10 @@ impl fmt::Debug for UnsupportedOpInfo<'tcx> {
pub enum ResourceExhaustionInfo {
/// The stack grew too big.
StackFrameLimitReached,
/// The program ran into an infinite loop.
InfiniteLoop,
/// The program ran for too long.
///
/// The exact limit is set by the `const_eval_limit` attribute.
StepLimitReached,
}

impl fmt::Debug for ResourceExhaustionInfo {
Expand All @@ -576,11 +578,9 @@ impl fmt::Debug for ResourceExhaustionInfo {
StackFrameLimitReached => {
write!(f, "reached the configured maximum number of stack frames")
}
InfiniteLoop => write!(
f,
"duplicate interpreter state observed here, const evaluation will never \
terminate"
),
StepLimitReached => {
write!(f, "exceeded interpreter step limit (see `#[const_eval_limit]`)")
}
}
}
}
Expand Down
67 changes: 20 additions & 47 deletions src/librustc_mir/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use rustc::ty::layout::HasTyCtxt;
use rustc::ty::{self, Ty};
use std::borrow::{Borrow, Cow};
use std::collections::hash_map::Entry;
use std::convert::TryFrom;
use std::hash::Hash;

use rustc_data_structures::fx::FxHashMap;
Expand All @@ -13,13 +12,13 @@ use rustc_span::source_map::Span;
use rustc_span::symbol::Symbol;

use crate::interpret::{
self, snapshot, AllocId, Allocation, GlobalId, ImmTy, InterpCx, InterpResult, Memory,
MemoryKind, OpTy, PlaceTy, Pointer, Scalar,
self, AllocId, Allocation, GlobalId, ImmTy, InterpCx, InterpResult, Memory, MemoryKind, OpTy,
PlaceTy, Pointer, Scalar,
};

use super::error::*;

impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>> {
impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter> {
/// Evaluate a const function where all arguments (if any) are zero-sized types.
/// The evaluation is memoized thanks to the query system.
///
Expand Down Expand Up @@ -86,22 +85,13 @@ impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>> {
}
}

/// The number of steps between loop detector snapshots.
/// Should be a power of two for performance reasons.
const DETECTOR_SNAPSHOT_PERIOD: isize = 256;

// Extra machine state for CTFE, and the Machine instance
pub struct CompileTimeInterpreter<'mir, 'tcx> {
/// When this value is negative, it indicates the number of interpreter
/// steps *until* the loop detector is enabled. When it is positive, it is
/// the number of steps after the detector has been enabled modulo the loop
/// detector period.
pub(super) steps_since_detector_enabled: isize,

pub(super) is_detector_enabled: bool,

/// Extra state to detect loops.
pub(super) loop_detector: snapshot::InfiniteLoopDetector<'mir, 'tcx>,
/// Extra machine state for CTFE, and the Machine instance
pub struct CompileTimeInterpreter {
/// For now, the number of terminators that can be evaluated before we throw a resource
/// exhuastion error.
///
/// Setting this to `0` disables the limit and allows the interpreter to run forever.
pub steps_remaining: usize,
}

#[derive(Copy, Clone, Debug)]
Expand All @@ -110,16 +100,9 @@ pub struct MemoryExtra {
pub(super) can_access_statics: bool,
}

impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {
impl CompileTimeInterpreter {
pub(super) fn new(const_eval_limit: usize) -> Self {
let steps_until_detector_enabled =
isize::try_from(const_eval_limit).unwrap_or(std::isize::MAX);

CompileTimeInterpreter {
loop_detector: Default::default(),
steps_since_detector_enabled: -steps_until_detector_enabled,
is_detector_enabled: const_eval_limit != 0,
}
CompileTimeInterpreter { steps_remaining: const_eval_limit }
}
}

Expand Down Expand Up @@ -173,8 +156,7 @@ impl<K: Hash + Eq, V> interpret::AllocMap<K, V> for FxHashMap<K, V> {
}
}

crate type CompileTimeEvalContext<'mir, 'tcx> =
InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>;
crate type CompileTimeEvalContext<'mir, 'tcx> = InterpCx<'mir, 'tcx, CompileTimeInterpreter>;

impl interpret::MayLeak for ! {
#[inline(always)]
Expand All @@ -184,7 +166,7 @@ impl interpret::MayLeak for ! {
}
}

impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, 'tcx> {
impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter {
type MemoryKinds = !;
type PointerTag = ();
type ExtraFnVal = !;
Expand Down Expand Up @@ -346,26 +328,17 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
}

fn before_terminator(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
if !ecx.machine.is_detector_enabled {
// The step limit has already been hit in a previous call to `before_terminator`.
if ecx.machine.steps_remaining == 0 {
return Ok(());
}

{
let steps = &mut ecx.machine.steps_since_detector_enabled;

*steps += 1;
if *steps < 0 {
return Ok(());
}

*steps %= DETECTOR_SNAPSHOT_PERIOD;
if *steps != 0 {
return Ok(());
}
ecx.machine.steps_remaining -= 1;
if ecx.machine.steps_remaining == 0 {
throw_exhaust!(StepLimitReached)
}

let span = ecx.frame().span;
ecx.machine.loop_detector.observe_and_analyze(*ecx.tcx, span, &ecx.memory, &ecx.stack[..])
Ok(())
}

#[inline(always)]
Expand Down
19 changes: 0 additions & 19 deletions src/librustc_mir/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,25 +112,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout for Memory<'mir, 'tcx, M>
}
}

// FIXME: Really we shouldn't clone memory, ever. Snapshot machinery should instead
// carefully copy only the reachable parts.
impl<'mir, 'tcx, M> Clone for Memory<'mir, 'tcx, M>
where
M: Machine<'mir, 'tcx, PointerTag = (), AllocExtra = ()>,
M::MemoryExtra: Copy,
M::MemoryMap: AllocMap<AllocId, (MemoryKind<M::MemoryKinds>, Allocation)>,
{
fn clone(&self) -> Self {
Memory {
alloc_map: self.alloc_map.clone(),
extra_fn_ptr_map: self.extra_fn_ptr_map.clone(),
dead_alloc_map: self.dead_alloc_map.clone(),
extra: self.extra,
tcx: self.tcx,
}
}
}

impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
pub fn new(tcx: TyCtxtAt<'tcx>, extra: M::MemoryExtra) -> Self {
Memory {
Expand Down
1 change: 0 additions & 1 deletion src/librustc_mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ mod memory;
mod operand;
mod operator;
mod place;
pub(crate) mod snapshot; // for const_eval
mod step;
mod terminator;
mod traits;
Expand Down
Loading