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

Tidy CFTE/MIRI #53609

Merged
merged 9 commits into from
Aug 25, 2018
Merged
Show file tree
Hide file tree
Changes from 6 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
29 changes: 22 additions & 7 deletions src/librustc/mir/interpret/error.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave a newline after the header (here and elsewhere).


use std::{fmt, env};

use mir;
Expand Down Expand Up @@ -315,7 +325,8 @@ impl<'tcx, O> EvalErrorKind<'tcx, O> {
ReadForeignStatic =>
"tried to read from foreign (extern) static",
InvalidPointerMath =>
"attempted to do invalid arithmetic on pointers that would leak base addresses, e.g. comparing pointers into different allocations",
"attempted to do invalid arithmetic on pointers that would leak base addresses, \
e.g. comparing pointers into different allocations",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a backslash to not change the actual string being computed:

                "attempted to do invalid arithmetic on pointers that would leak base addresses, \
                e.g. comparing pointers into different allocations",

Here and elsewhere.

ReadUndefBytes =>
"attempted to read undefined bytes",
DeadLocal =>
Expand Down Expand Up @@ -369,11 +380,13 @@ impl<'tcx, O> EvalErrorKind<'tcx, O> {
Layout(_) =>
"rustc layout computation failed",
UnterminatedCString(_) =>
"attempted to get length of a null terminated string, but no null found before end of allocation",
"attempted to get length of a null terminated string, but no null found before end \
of allocation",
HeapAllocZeroBytes =>
"tried to re-, de- or allocate zero bytes on the heap",
HeapAllocNonPowerOfTwoAlignment(_) =>
"tried to re-, de-, or allocate heap memory with alignment that is not a power of two",
"tried to re-, de-, or allocate heap memory with alignment that is not a power of \
two",
Unreachable =>
"entered unreachable code",
Panic { .. } =>
Expand Down Expand Up @@ -435,8 +448,8 @@ impl<'tcx, O: fmt::Debug> fmt::Debug for EvalErrorKind<'tcx, O> {
kind, ptr, len, lock)
}
InvalidMemoryLockRelease { ptr, len, frame, ref lock } => {
write!(f, "frame {} tried to release memory write lock at {:?}, size {}, but cannot release lock {:?}",
frame, ptr, len, lock)
write!(f, "frame {} tried to release memory write lock at {:?}, size {}, but \
cannot release lock {:?}", frame, ptr, len, lock)
}
DeallocatedLockedMemory { ptr, ref lock } => {
write!(f, "tried to deallocate memory at {:?} in conflict with lock {:?}",
Expand All @@ -447,7 +460,8 @@ impl<'tcx, O: fmt::Debug> fmt::Debug for EvalErrorKind<'tcx, O> {
}
NoMirFor(ref func) => write!(f, "no mir for `{}`", func),
FunctionPointerTyMismatch(sig, got) =>
write!(f, "tried to call a function with sig {} through a function pointer of type {}", sig, got),
write!(f, "tried to call a function with sig {} through a \
function pointer of type {}", sig, got),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removes the space.

"a\
  b"

is the string "ab". See playground.

You should add a space in front of the backslash.

BoundsCheck { ref len, ref index } =>
write!(f, "index out of bounds: the len is {:?} but the index is {:?}", len, index),
ReallocatedWrongMemoryKind(ref old, ref new) =>
Expand All @@ -470,7 +484,8 @@ impl<'tcx, O: fmt::Debug> fmt::Debug for EvalErrorKind<'tcx, O> {
MachineError(ref inner) =>
write!(f, "{}", inner),
IncorrectAllocationInformation(size, size2, align, align2) =>
write!(f, "incorrect alloc info: expected size {} and align {}, got size {} and align {}", size.bytes(), align.abi(), size2.bytes(), align2.abi()),
write!(f, "incorrect alloc info: expected size {} and align {}, got size {} and \
align {}", size.bytes(), align.abi(), size2.bytes(), align2.abi()),
Panic { ref msg, line, col, ref file } =>
write!(f, "the evaluated program panicked at '{}', {}:{}:{}", msg, file, line, col),
_ => write!(f, "{}", self.description()),
Expand Down
13 changes: 12 additions & 1 deletion src/librustc/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//! An interpreter for MIR used in CTFE and by miri

#[macro_export]
Expand Down Expand Up @@ -40,7 +50,8 @@ use std::num::NonZeroU32;
pub enum Lock {
NoLock,
WriteLock(DynamicLifetime),
/// This should never be empty -- that would be a read lock held and nobody there to release it...
/// This should never be empty -- that would be a read lock held and nobody
/// there to release it...
ReadLock(Vec<DynamicLifetime>),
}

Expand Down
10 changes: 10 additions & 0 deletions src/librustc/mir/interpret/value.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![allow(unknown_lints)]

use ty::layout::{HasDataLayout, Size};
Expand Down
23 changes: 20 additions & 3 deletions src/librustc_mir/interpret/cast.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use rustc::ty::{self, Ty, TypeAndMut};
use rustc::ty::layout::{self, TyLayout, Size};
use syntax::ast::{FloatTy, IntTy, UintTy};
Expand Down Expand Up @@ -216,7 +226,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
Ok(Scalar::Bits { bits: v, size: 4 })
},

// No alignment check needed for raw pointers. But we have to truncate to target ptr size.
// No alignment check needed for raw pointers.
// But we have to truncate to target ptr size.
RawPtr(_) => {
Ok(Scalar::Bits {
bits: self.memory.truncate_to_ptr(v).0 as u128,
Expand All @@ -229,7 +240,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
}
}

fn cast_from_float(&self, bits: u128, fty: FloatTy, dest_ty: Ty<'tcx>) -> EvalResult<'tcx, Scalar> {
fn cast_from_float(
&self,
bits: u128,
fty: FloatTy,
dest_ty: Ty<'tcx>
) -> EvalResult<'tcx, Scalar> {
Copy link
Member

@RalfJung RalfJung Aug 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line with the arrow should not be indented:

    fn cast_from_float(
        &self,
        bits: u128,
        fty: FloatTy,
        dest_ty: Ty<'tcx>
    ) -> EvalResult<'tcx, Scalar> {
        // code goes here

That nicely groups the arguments.

use rustc::ty::TyKind::*;
use rustc_apfloat::FloatConvert;
match dest_ty.sty {
Expand Down Expand Up @@ -292,7 +308,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
fn cast_from_ptr(&self, ptr: Pointer, ty: Ty<'tcx>) -> EvalResult<'tcx, Scalar> {
use rustc::ty::TyKind::*;
match ty.sty {
// Casting to a reference or fn pointer is not permitted by rustc, no need to support it here.
// Casting to a reference or fn pointer is not permitted by rustc,
// no need to support it here.
RawPtr(_) |
Int(IntTy::Isize) |
Uint(UintTy::Usize) => Ok(ptr.into()),
Expand Down
10 changes: 10 additions & 0 deletions src/librustc_mir/interpret/const_eval.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::fmt;
use std::error::Error;

Expand Down
41 changes: 33 additions & 8 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::fmt::Write;
use std::hash::{Hash, Hasher};
use std::mem;
Expand Down Expand Up @@ -82,7 +92,8 @@ pub struct Frame<'mir, 'tcx: 'mir> {
pub return_place: Place,

/// The list of locals for this stack frame, stored in order as
/// `[return_ptr, arguments..., variables..., temporaries...]`. The locals are stored as `Option<Value>`s.
/// `[return_ptr, arguments..., variables..., temporaries...]`.
/// The locals are stored as `Option<Value>`s.
/// `None` represents a local that is currently dead, while a live local
/// can either directly contain `Scalar` or refer to some part of an `Allocation`.
pub locals: IndexVec<mir::Local, LocalValue>,
Expand Down Expand Up @@ -269,7 +280,9 @@ impl<'c, 'b, 'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout
}
}

impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> layout::HasTyCtxt<'tcx> for &'a EvalContext<'a, 'mir, 'tcx, M> {
impl<'a, 'mir, 'tcx, M> layout::HasTyCtxt<'tcx> for &'a EvalContext<'a, 'mir, 'tcx, M>
where M: Machine<'mir, 'tcx>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be indented? is this the best rearrangement of this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what our usual style is there, but yeah indentation makes sense.

{
#[inline]
fn tcx<'b>(&'b self) -> TyCtxt<'b, 'tcx, 'tcx> {
*self.tcx
Expand Down Expand Up @@ -329,7 +342,8 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M

pub(crate) fn with_fresh_body<F: FnOnce(&mut Self) -> R, R>(&mut self, f: F) -> R {
let stack = mem::replace(&mut self.stack, Vec::new());
let steps = mem::replace(&mut self.steps_since_detector_enabled, -STEPS_UNTIL_DETECTOR_ENABLED);
let steps = mem::replace(&mut self.steps_since_detector_enabled,
-STEPS_UNTIL_DETECTOR_ENABLED);
let r = f(self);
self.stack = stack;
self.steps_since_detector_enabled = steps;
Expand Down Expand Up @@ -378,7 +392,11 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
Ok(Value::new_slice(Scalar::Ptr(ptr), s.len() as u64, self.tcx.tcx))
}

pub(super) fn resolve(&self, def_id: DefId, substs: &'tcx Substs<'tcx>) -> EvalResult<'tcx, ty::Instance<'tcx>> {
pub(super) fn resolve(
&self,
def_id: DefId,
substs: &'tcx Substs<'tcx>
) -> EvalResult<'tcx, ty::Instance<'tcx>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you missed one :)

trace!("resolve: {:?}, {:#?}", def_id, substs);
trace!("substs: {:#?}", self.substs());
trace!("param_env: {:#?}", self.param_env);
Expand All @@ -405,7 +423,10 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
) -> EvalResult<'tcx, &'tcx mir::Mir<'tcx>> {
// do not continue if typeck errors occurred (can only occur in local crate)
let did = instance.def_id();
if did.is_local() && self.tcx.has_typeck_tables(did) && self.tcx.typeck_tables_of(did).tainted_by_errors {
if did.is_local()
&& self.tcx.has_typeck_tables(did)
&& self.tcx.typeck_tables_of(did).tainted_by_errors
{
return err!(TypeckError);
}
trace!("load mir {:?}", instance);
Expand Down Expand Up @@ -614,7 +635,8 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
match frame.return_to_block {
StackPopCleanup::MarkStatic(mutable) => {
if let Place::Ptr(MemPlace { ptr, .. }) = frame.return_place {
// FIXME: to_ptr()? might be too extreme here, static zsts might reach this under certain conditions
// FIXME: to_ptr()? might be too extreme here,
// static zsts might reach this under certain conditions
self.memory.mark_static_initialized(
ptr.to_ptr()?.alloc_id,
mutable,
Expand Down Expand Up @@ -651,7 +673,8 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
} else {
self.param_env
};
self.tcx.const_eval(param_env.and(gid)).map_err(|err| EvalErrorKind::ReferencedConstant(err).into())
self.tcx.const_eval(param_env.and(gid))
.map_err(|err| EvalErrorKind::ReferencedConstant(err).into())
}

#[inline(always)]
Expand Down Expand Up @@ -757,7 +780,9 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
} else {
last_span = Some(span);
}
let location = if self.tcx.def_key(instance.def_id()).disambiguated_data.data == DefPathData::ClosureExpr {
let location = if self.tcx.def_key(instance.def_id()).disambiguated_data.data
== DefPathData::ClosureExpr
{
"closure".to_owned()
} else {
instance.to_string()
Expand Down
10 changes: 10 additions & 0 deletions src/librustc_mir/interpret/machine.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//! This module contains everything needed to instantiate an interpreter.
//! This separation exists to ensure that no fancy miri features like
//! interpreting common C functions leak into CTFE.
Expand Down
Loading