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

[const prop] Fix "alloc id without corresponding allocation" ICE #66677

Merged
merged 4 commits into from
Nov 27, 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
2 changes: 1 addition & 1 deletion src/librustc/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ impl<Tag> From<Pointer<Tag>> for Scalar<Tag> {
}
}

#[derive(Clone, Copy, Eq, PartialEq, RustcEncodable, RustcDecodable, HashStable)]
#[derive(Clone, Copy, Eq, PartialEq, RustcEncodable, RustcDecodable, HashStable, Hash)]
pub enum ScalarMaybeUndef<Tag = (), Id = AllocId> {
Scalar(Scalar<Tag, Id>),
Undef,
Expand Down
51 changes: 30 additions & 21 deletions src/librustc_mir/interpret/intern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,34 @@
//! After a const evaluation has computed a value, before we destroy the const evaluator's session
//! memory, we need to extract all memory allocations to the global memory pool so they stay around.

use rustc::ty::{Ty, self};
use rustc::mir::interpret::{InterpResult, ErrorHandled};
use rustc::hir;
use super::validity::RefTracking;
use rustc_data_structures::fx::FxHashSet;
use rustc::hir;
use rustc::mir::interpret::{ErrorHandled, InterpResult};
use rustc::ty::{self, Ty};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};

use syntax::ast::Mutability;

use super::{
ValueVisitor, MemoryKind, AllocId, MPlaceTy, Scalar,
AllocId, Allocation, InterpCx, Machine, MemoryKind, MPlaceTy, Scalar, ValueVisitor,
};
use crate::const_eval::{CompileTimeInterpreter, CompileTimeEvalContext};

struct InternVisitor<'rt, 'mir, 'tcx> {
pub trait CompileTimeMachine<'mir, 'tcx> =
Machine<
'mir,
'tcx,
MemoryKinds = !,
PointerTag = (),
ExtraFnVal = !,
FrameExtra = (),
MemoryExtra = (),
AllocExtra = (),
MemoryMap = FxHashMap<AllocId, (MemoryKind<!>, Allocation)>,
>;

struct InternVisitor<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> {
/// The ectx from which we intern.
ecx: &'rt mut CompileTimeEvalContext<'mir, 'tcx>,
ecx: &'rt mut InterpCx<'mir, 'tcx, M>,
/// Previously encountered safe references.
ref_tracking: &'rt mut RefTracking<(MPlaceTy<'tcx>, Mutability, InternMode)>,
/// A list of all encountered allocations. After type-based interning, we traverse this list to
Expand Down Expand Up @@ -58,18 +70,15 @@ struct IsStaticOrFn;
/// `immutable` things might become mutable if `ty` is not frozen.
/// `ty` can be `None` if there is no potential interior mutability
/// to account for (e.g. for vtables).
fn intern_shallow<'rt, 'mir, 'tcx>(
ecx: &'rt mut CompileTimeEvalContext<'mir, 'tcx>,
fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>(
ecx: &'rt mut InterpCx<'mir, 'tcx, M>,
leftover_allocations: &'rt mut FxHashSet<AllocId>,
mode: InternMode,
alloc_id: AllocId,
mutability: Mutability,
ty: Option<Ty<'tcx>>,
) -> InterpResult<'tcx, Option<IsStaticOrFn>> {
trace!(
"InternVisitor::intern {:?} with {:?}",
alloc_id, mutability,
);
trace!("InternVisitor::intern {:?} with {:?}", alloc_id, mutability,);
// remove allocation
let tcx = ecx.tcx;
let (kind, mut alloc) = match ecx.memory.alloc_map.remove(&alloc_id) {
Expand Down Expand Up @@ -130,7 +139,7 @@ fn intern_shallow<'rt, 'mir, 'tcx>(
Ok(None)
}

impl<'rt, 'mir, 'tcx> InternVisitor<'rt, 'mir, 'tcx> {
impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> InternVisitor<'rt, 'mir, 'tcx, M> {
fn intern_shallow(
&mut self,
alloc_id: AllocId,
Expand All @@ -148,15 +157,15 @@ impl<'rt, 'mir, 'tcx> InternVisitor<'rt, 'mir, 'tcx> {
}
}

impl<'rt, 'mir, 'tcx>
ValueVisitor<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>
impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>
ValueVisitor<'mir, 'tcx, M>
for
InternVisitor<'rt, 'mir, 'tcx>
InternVisitor<'rt, 'mir, 'tcx, M>
{
type V = MPlaceTy<'tcx>;

#[inline(always)]
fn ecx(&self) -> &CompileTimeEvalContext<'mir, 'tcx> {
fn ecx(&self) -> &InterpCx<'mir, 'tcx, M> {
&self.ecx
}

Expand Down Expand Up @@ -265,8 +274,8 @@ for
}
}

pub fn intern_const_alloc_recursive(
ecx: &mut CompileTimeEvalContext<'mir, 'tcx>,
pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
ecx: &mut InterpCx<'mir, 'tcx, M>,
// The `mutability` of the place, ignoring the type.
place_mut: Option<hir::Mutability>,
ret: MPlaceTy<'tcx>,
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use rustc_macros::HashStable;
/// operations and fat pointers. This idea was taken from rustc's codegen.
/// In particular, thanks to `ScalarPair`, arithmetic operations and casts can be entirely
/// defined on `Immediate`, and do not have to work with a `Place`.
#[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable, Hash)]
pub enum Immediate<Tag=(), Id=AllocId> {
Scalar(ScalarMaybeUndef<Tag, Id>),
ScalarPair(ScalarMaybeUndef<Tag, Id>, ScalarMaybeUndef<Tag, Id>),
Expand Down Expand Up @@ -104,7 +104,7 @@ impl<'tcx, Tag> ::std::ops::Deref for ImmTy<'tcx, Tag> {
/// An `Operand` is the result of computing a `mir::Operand`. It can be immediate,
/// or still in memory. The latter is an optimization, to delay reading that chunk of
/// memory and to avoid having to store arbitrary-sized data here.
#[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable, Hash)]
pub enum Operand<Tag=(), Id=AllocId> {
Immediate(Immediate<Tag, Id>),
Indirect(MemPlace<Tag, Id>),
Expand Down Expand Up @@ -134,7 +134,7 @@ impl<Tag> Operand<Tag> {
}
}

#[derive(Copy, Clone, Debug, PartialEq)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub struct OpTy<'tcx, Tag=()> {
op: Operand<Tag>, // Keep this private, it helps enforce invariants
pub layout: TyLayout<'tcx>,
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment!
#![feature(range_is_empty)]
#![feature(stmt_expr_attributes)]
#![feature(bool_to_option)]
#![feature(trait_alias)]

#![recursion_limit="256"]

Expand Down
16 changes: 12 additions & 4 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use crate::interpret::{
self, InterpCx, ScalarMaybeUndef, Immediate, OpTy,
StackPopCleanup, LocalValue, LocalState, AllocId, Frame,
Allocation, MemoryKind, ImmTy, Pointer, Memory, PlaceTy,
Operand as InterpOperand,
Operand as InterpOperand, intern_const_alloc_recursive,
};
use crate::const_eval::error_to_const_error;
use crate::transform::{MirPass, MirSource};
Expand Down Expand Up @@ -649,9 +649,9 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}

fn should_const_prop(&mut self, op: OpTy<'tcx>) -> bool {
if self.tcx.sess.opts.debugging_opts.mir_opt_level >= 2 {
return true;
} else if self.tcx.sess.opts.debugging_opts.mir_opt_level == 0 {
let mir_opt_level = self.tcx.sess.opts.debugging_opts.mir_opt_level;

if mir_opt_level == 0 {
return false;
}

Expand All @@ -661,6 +661,14 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
interpret::Operand::Immediate(Immediate::ScalarPair(ScalarMaybeUndef::Scalar(l),
ScalarMaybeUndef::Scalar(r))) =>
l.is_bits() && r.is_bits(),
interpret::Operand::Indirect(_) if mir_opt_level >= 2 => {
intern_const_alloc_recursive(
&mut self.ecx,
None,
op.assert_mem_place()
).expect("failed to intern alloc");
true
},
_ => false
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/test/mir-opt/const_prop/const_prop_fails_gracefully.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ fn main() {
// START rustc.main.ConstProp.after.mir
// bb0: {
// ...
// _4 = const Scalar(AllocId(1).0x0) : &i32;
// _3 = const Scalar(AllocId(1).0x0) : &i32;
// _2 = const Value(Scalar(AllocId(1).0x0)) : *const i32;
// _4 = const main::FOO;
// _3 = _4;
// _2 = move _3 as *const i32 (Misc);
// ...
// _1 = move _2 as usize (Misc);
// ...
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/const_prop/ref_deref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn main() {
// START rustc.main.ConstProp.after.mir
// bb0: {
// ...
// _2 = const Scalar(AllocId(0).0x0) : &i32;
// _2 = &(promoted[0]: i32);
// _1 = const 4i32;
// ...
// }
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/const_prop/reify_fn_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fn main() {
// START rustc.main.ConstProp.after.mir
// bb0: {
// ...
// _3 = const main;
// _3 = const main as fn() (Pointer(ReifyFnPointer));
// _2 = move _3 as usize (Misc);
// ...
// _1 = move _2 as *const fn() (Misc);
Expand Down
4 changes: 2 additions & 2 deletions src/test/mir-opt/const_prop/slice_len.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ fn main() {
// START rustc.main.ConstProp.after.mir
// bb0: {
// ...
// _4 = const Scalar(AllocId(0).0x0) : &[u32; 3];
// _3 = const Scalar(AllocId(0).0x0) : &[u32; 3];
// _4 = &(promoted[0]: [u32; 3]);
// _3 = _4;
// _2 = move _3 as &[u32] (Pointer(Unsize));
// ...
// _6 = const 1usize;
Expand Down
24 changes: 24 additions & 0 deletions src/test/ui/consts/issue-66345.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// run-pass
// compile-flags: -Z mir-opt-level=3

// Checks that the compiler does not ICE when passing references to field of by-value struct
// with -Z mir-opt-level=3

fn do_nothing(_: &()) {}

pub struct Foo {
bar: (),
}

pub fn by_value_1(foo: Foo) {
do_nothing(&foo.bar);
}

pub fn by_value_2<T>(foo: Foo) {
do_nothing(&foo.bar);
}

fn main() {
by_value_1(Foo { bar: () });
by_value_2::<()>(Foo { bar: () });
}