From 717c64e18884c20a11603f181b207ff280b13f9a Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:25 -0700 Subject: [PATCH 01/31] Silence "skipping const checks" if outside a const context --- src/librustc_mir/transform/qualify_consts.rs | 4 +++- .../miri_unleashed/mutable_references.rs | 4 +--- .../miri_unleashed/mutable_references.stderr | 20 +------------------ .../miri_unleashed/mutable_references_ice.rs | 2 +- .../mutable_references_ice.stderr | 6 ------ 5 files changed, 6 insertions(+), 30 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 7423a668f94d2..cce2c9f5ee9c4 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -678,7 +678,9 @@ struct Checker<'a, 'tcx> { macro_rules! unleash_miri { ($this:expr) => {{ if $this.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you { - $this.tcx.sess.span_warn($this.span, "skipping const checks"); + if $this.mode.requires_const_checking() { + $this.tcx.sess.span_warn($this.span, "skipping const checks"); + } return; } }} diff --git a/src/test/ui/consts/miri_unleashed/mutable_references.rs b/src/test/ui/consts/miri_unleashed/mutable_references.rs index 5f9888053a196..f81c78a8221a1 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_references.rs +++ b/src/test/ui/consts/miri_unleashed/mutable_references.rs @@ -27,9 +27,7 @@ static OH_YES: &mut i32 = &mut 42; fn main() { unsafe { - *MEH.x.get() = 99; //~ WARN skipping const checks - //~^ WARN skipping const checks + *MEH.x.get() = 99; } *OH_YES = 99; //~ ERROR cannot assign to `*OH_YES`, as `OH_YES` is an immutable static item - //~^ WARN skipping const checks } diff --git a/src/test/ui/consts/miri_unleashed/mutable_references.stderr b/src/test/ui/consts/miri_unleashed/mutable_references.stderr index b870aca640a0a..12840e00df9b3 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_references.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_references.stderr @@ -1,23 +1,5 @@ -warning: skipping const checks - --> $DIR/mutable_references.rs:30:10 - | -LL | *MEH.x.get() = 99; - | ^^^^^ - -warning: skipping const checks - --> $DIR/mutable_references.rs:30:9 - | -LL | *MEH.x.get() = 99; - | ^^^^^^^^^^^^^^^^^ - -warning: skipping const checks - --> $DIR/mutable_references.rs:33:5 - | -LL | *OH_YES = 99; - | ^^^^^^^^^^^^ - error[E0594]: cannot assign to `*OH_YES`, as `OH_YES` is an immutable static item - --> $DIR/mutable_references.rs:33:5 + --> $DIR/mutable_references.rs:32:5 | LL | *OH_YES = 99; | ^^^^^^^^^^^^ cannot assign diff --git a/src/test/ui/consts/miri_unleashed/mutable_references_ice.rs b/src/test/ui/consts/miri_unleashed/mutable_references_ice.rs index 4fcd89a74db61..b051f6b59c46e 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_references_ice.rs +++ b/src/test/ui/consts/miri_unleashed/mutable_references_ice.rs @@ -24,6 +24,6 @@ const MUH: Meh = Meh { fn main() { unsafe { - *MUH.x.get() = 99; //~ WARN skipping const checks + *MUH.x.get() = 99; } } diff --git a/src/test/ui/consts/miri_unleashed/mutable_references_ice.stderr b/src/test/ui/consts/miri_unleashed/mutable_references_ice.stderr index 28cf3537d605a..24c9de5ee2eca 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_references_ice.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_references_ice.stderr @@ -1,9 +1,3 @@ -warning: skipping const checks - --> $DIR/mutable_references_ice.rs:27:9 - | -LL | *MUH.x.get() = 99; - | ^^^^^^^^^^^^^^^^^ - thread 'rustc' panicked at 'assertion failed: `(left != right)` left: `Const`, right: `Const`: UnsafeCells are not allowed behind references in constants. This should have been prevented statically by const qualification. If this were allowed one would be able to change a constant at one use site and other use sites could observe that mutation.', src/librustc_mir/interpret/intern.rs:LL:CC From 457c3aa6722cd8d2599c7f78347b2f8f586f3527 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:26 -0700 Subject: [PATCH 02/31] Add additional `const` tests --- src/test/ui/consts/const-if.rs | 5 +++++ src/test/ui/consts/const-if.stderr | 15 +++++++++++++++ src/test/ui/consts/const-multi-ref.rs | 11 +++++++++++ src/test/ui/consts/const-multi-ref.stderr | 15 +++++++++++++++ 4 files changed, 46 insertions(+) create mode 100644 src/test/ui/consts/const-if.rs create mode 100644 src/test/ui/consts/const-if.stderr create mode 100644 src/test/ui/consts/const-multi-ref.rs create mode 100644 src/test/ui/consts/const-multi-ref.stderr diff --git a/src/test/ui/consts/const-if.rs b/src/test/ui/consts/const-if.rs new file mode 100644 index 0000000000000..9bb5bcc499e61 --- /dev/null +++ b/src/test/ui/consts/const-if.rs @@ -0,0 +1,5 @@ +const _X: i32 = if true { 5 } else { 6 }; +//~^ ERROR constant contains unimplemented expression type +//~| ERROR constant contains unimplemented expression type + +fn main() {} diff --git a/src/test/ui/consts/const-if.stderr b/src/test/ui/consts/const-if.stderr new file mode 100644 index 0000000000000..655fcdae58748 --- /dev/null +++ b/src/test/ui/consts/const-if.stderr @@ -0,0 +1,15 @@ +error[E0019]: constant contains unimplemented expression type + --> $DIR/const-if.rs:1:20 + | +LL | const _X: i32 = if true { 5 } else { 6 }; + | ^^^^ + +error[E0019]: constant contains unimplemented expression type + --> $DIR/const-if.rs:1:17 + | +LL | const _X: i32 = if true { 5 } else { 6 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0019`. diff --git a/src/test/ui/consts/const-multi-ref.rs b/src/test/ui/consts/const-multi-ref.rs new file mode 100644 index 0000000000000..498e99e668b43 --- /dev/null +++ b/src/test/ui/consts/const-multi-ref.rs @@ -0,0 +1,11 @@ +const _X: i32 = { + let mut a = 5; + let p = &mut a; //~ ERROR references in constants may only refer to immutable values + + let reborrow = {p}; //~ ERROR references in constants may only refer to immutable values + let pp = &reborrow; + let ppp = &pp; + ***ppp +}; + +fn main() {} diff --git a/src/test/ui/consts/const-multi-ref.stderr b/src/test/ui/consts/const-multi-ref.stderr new file mode 100644 index 0000000000000..9e525ef9aac75 --- /dev/null +++ b/src/test/ui/consts/const-multi-ref.stderr @@ -0,0 +1,15 @@ +error[E0017]: references in constants may only refer to immutable values + --> $DIR/const-multi-ref.rs:3:13 + | +LL | let p = &mut a; + | ^^^^^^ constants require immutable values + +error[E0017]: references in constants may only refer to immutable values + --> $DIR/const-multi-ref.rs:5:21 + | +LL | let reborrow = {p}; + | ^ constants require immutable values + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0017`. From e81297d990632d29a3a82ecbfc78dbc4e6017994 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:30 -0700 Subject: [PATCH 03/31] Add analysis to determine if a local is indirectly mutable This adds a dataflow analysis that determines if a reference to a given `Local` or part of a `Local` that would allow mutation exists before a point in the CFG. If no such reference exists, we know for sure that that `Local` cannot have been mutated via an indirect assignment or function call. --- .../dataflow/impls/indirect_mutation.rs | 152 ++++++++++++++++++ src/librustc_mir/dataflow/impls/mod.rs | 8 +- src/librustc_mir/dataflow/mod.rs | 1 + 3 files changed, 157 insertions(+), 4 deletions(-) create mode 100644 src/librustc_mir/dataflow/impls/indirect_mutation.rs diff --git a/src/librustc_mir/dataflow/impls/indirect_mutation.rs b/src/librustc_mir/dataflow/impls/indirect_mutation.rs new file mode 100644 index 0000000000000..f45fdb2146356 --- /dev/null +++ b/src/librustc_mir/dataflow/impls/indirect_mutation.rs @@ -0,0 +1,152 @@ +use rustc::mir::visit::Visitor; +use rustc::mir::{self, Local, Location}; +use rustc::ty::{self, TyCtxt}; +use rustc_data_structures::bit_set::BitSet; +use syntax_pos::DUMMY_SP; + +use crate::dataflow::{self, GenKillSet}; + +/// Whether a borrow to a `Local` has been created that could allow that `Local` to be mutated +/// indirectly. This could either be a mutable reference (`&mut`) or a shared borrow if the type of +/// that `Local` allows interior mutability. +/// +/// If this returns `false` for a `Local` at a given `Location`, the user can assume that `Local` +/// has not been mutated as a result of an indirect assignment (`*p = x`) or as a side-effect of a +/// function call or drop terminator. +#[derive(Copy, Clone)] +pub struct IndirectlyMutableLocals<'mir, 'tcx> { + body: &'mir mir::Body<'tcx>, + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, +} + +impl<'mir, 'tcx> IndirectlyMutableLocals<'mir, 'tcx> { + pub fn new( + tcx: TyCtxt<'tcx>, + body: &'mir mir::Body<'tcx>, + param_env: ty::ParamEnv<'tcx>, + ) -> Self { + IndirectlyMutableLocals { body, tcx, param_env } + } + + fn transfer_function<'a>( + &self, + trans: &'a mut GenKillSet, + ) -> TransferFunction<'a, 'mir, 'tcx> { + TransferFunction { + body: self.body, + tcx: self.tcx, + param_env: self.param_env, + trans + } + } +} + +impl<'mir, 'tcx> dataflow::BitDenotation<'tcx> for IndirectlyMutableLocals<'mir, 'tcx> { + type Idx = Local; + + fn name() -> &'static str { "mut_borrowed_locals" } + + fn bits_per_block(&self) -> usize { + self.body.local_decls.len() + } + + fn start_block_effect(&self, _entry_set: &mut BitSet) { + // Nothing is borrowed on function entry + } + + fn statement_effect( + &self, + trans: &mut GenKillSet, + loc: Location, + ) { + let stmt = &self.body[loc.block].statements[loc.statement_index]; + self.transfer_function(trans).visit_statement(stmt, loc); + } + + fn terminator_effect( + &self, + trans: &mut GenKillSet, + loc: Location, + ) { + let terminator = self.body[loc.block].terminator(); + self.transfer_function(trans).visit_terminator(terminator, loc); + } + + fn propagate_call_return( + &self, + _in_out: &mut BitSet, + _call_bb: mir::BasicBlock, + _dest_bb: mir::BasicBlock, + _dest_place: &mir::Place<'tcx>, + ) { + // Nothing to do when a call returns successfully + } +} + +impl<'mir, 'tcx> dataflow::BottomValue for IndirectlyMutableLocals<'mir, 'tcx> { + // bottom = unborrowed + const BOTTOM_VALUE: bool = false; +} + +/// A `Visitor` that defines the transfer function for `IndirectlyMutableLocals`. +struct TransferFunction<'a, 'mir, 'tcx> { + trans: &'a mut GenKillSet, + body: &'mir mir::Body<'tcx>, + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, +} + +impl<'tcx> Visitor<'tcx> for TransferFunction<'_, '_, 'tcx> { + fn visit_rvalue( + &mut self, + rvalue: &mir::Rvalue<'tcx>, + location: Location, + ) { + if let mir::Rvalue::Ref(_, kind, ref borrowed_place) = *rvalue { + let is_mut = match kind { + mir::BorrowKind::Mut { .. } => true, + + | mir::BorrowKind::Shared + | mir::BorrowKind::Shallow + | mir::BorrowKind::Unique + => { + !borrowed_place + .ty(self.body, self.tcx) + .ty + .is_freeze(self.tcx, self.param_env, DUMMY_SP) + } + }; + + if is_mut { + match borrowed_place.base { + mir::PlaceBase::Local(borrowed_local) if !borrowed_place.is_indirect() + => self.trans.gen(borrowed_local), + + _ => (), + } + } + } + + self.super_rvalue(rvalue, location); + } + + + fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) { + self.super_terminator(terminator, location); + + match &terminator.kind { + // Drop terminators borrow the location + mir::TerminatorKind::Drop { location: dropped_place, .. } | + mir::TerminatorKind::DropAndReplace { location: dropped_place, .. } => { + match dropped_place.base { + mir::PlaceBase::Local(dropped_local) if !dropped_place.is_indirect() + => self.trans.gen(dropped_local), + + _ => (), + } + } + _ => (), + } + } +} diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs index 69bbe08792140..d669c786c09df 100644 --- a/src/librustc_mir/dataflow/impls/mod.rs +++ b/src/librustc_mir/dataflow/impls/mod.rs @@ -18,13 +18,13 @@ use super::drop_flag_effects_for_function_entry; use super::drop_flag_effects_for_location; use super::on_lookup_result_bits; -mod storage_liveness; - -pub use self::storage_liveness::*; - mod borrowed_locals; +mod indirect_mutation; +mod storage_liveness; pub use self::borrowed_locals::*; +pub use self::indirect_mutation::IndirectlyMutableLocals; +pub use self::storage_liveness::*; pub(super) mod borrows; diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index 5ab4e25b683cb..47eb47cf664de 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -23,6 +23,7 @@ pub use self::impls::DefinitelyInitializedPlaces; pub use self::impls::EverInitializedPlaces; pub use self::impls::borrows::Borrows; pub use self::impls::HaveBeenBorrowedLocals; +pub use self::impls::IndirectlyMutableLocals; pub use self::at_location::{FlowAtLocation, FlowsAtLocation}; pub(crate) use self::drop_flag_effects::*; From 83a3e04ed0ba59a1cdff333d4017019817cbf911 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:31 -0700 Subject: [PATCH 04/31] Don't treat locals as mutably borrowed after they're dropped --- .../dataflow/impls/indirect_mutation.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/librustc_mir/dataflow/impls/indirect_mutation.rs b/src/librustc_mir/dataflow/impls/indirect_mutation.rs index f45fdb2146356..934b843cc40b8 100644 --- a/src/librustc_mir/dataflow/impls/indirect_mutation.rs +++ b/src/librustc_mir/dataflow/impls/indirect_mutation.rs @@ -135,18 +135,11 @@ impl<'tcx> Visitor<'tcx> for TransferFunction<'_, '_, 'tcx> { fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) { self.super_terminator(terminator, location); - match &terminator.kind { - // Drop terminators borrow the location - mir::TerminatorKind::Drop { location: dropped_place, .. } | - mir::TerminatorKind::DropAndReplace { location: dropped_place, .. } => { - match dropped_place.base { - mir::PlaceBase::Local(dropped_local) if !dropped_place.is_indirect() - => self.trans.gen(dropped_local), - - _ => (), - } - } - _ => (), + if let mir::TerminatorKind::Drop { location: _, .. } + | mir::TerminatorKind::DropAndReplace { location: _, .. } = &terminator.kind + { + // Although drop terminators mutably borrow the location being dropped, that borrow + // cannot live beyond the drop terminator because the dropped location is invalidated. } } } From eec93ca08c5116e1e7f4517bd851670ffc6c33f5 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:32 -0700 Subject: [PATCH 05/31] Copy `Qualif` to start work on new const validator This is an exact copy of the `Qualif` trait from `qualify_consts.rs` and its first two implementers, `HasMutInterior` and `NeedsDrop`. --- .../transform/check_consts/qualifs.rs | 271 ++++++++++++++++++ 1 file changed, 271 insertions(+) create mode 100644 src/librustc_mir/transform/check_consts/qualifs.rs diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs new file mode 100644 index 0000000000000..0050a56624bbd --- /dev/null +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -0,0 +1,271 @@ +/// A "qualif"(-ication) is a way to look for something "bad" in the MIR that would disqualify some +/// code for promotion or prevent it from evaluating at compile time. So `return true` means +/// "I found something bad, no reason to go on searching". `false` is only returned if we +/// definitely cannot find anything bad anywhere. +/// +/// The default implementations proceed structurally. +trait Qualif { + const IDX: usize; + + /// Return the qualification that is (conservatively) correct for any value + /// of the type, or `None` if the qualification is not value/type-based. + fn in_any_value_of_ty(_cx: &ConstCx<'_, 'tcx>, _ty: Ty<'tcx>) -> Option { + None + } + + /// Return a mask for the qualification, given a type. This is `false` iff + /// no value of that type can have the qualification. + fn mask_for_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> bool { + Self::in_any_value_of_ty(cx, ty).unwrap_or(true) + } + + fn in_local(cx: &ConstCx<'_, '_>, local: Local) -> bool { + cx.per_local.0[Self::IDX].contains(local) + } + + fn in_static(_cx: &ConstCx<'_, 'tcx>, _static: &Static<'tcx>) -> bool { + // FIXME(eddyb) should we do anything here for value properties? + false + } + + fn in_projection_structurally( + cx: &ConstCx<'_, 'tcx>, + place: PlaceRef<'_, 'tcx>, + ) -> bool { + if let [proj_base @ .., elem] = place.projection { + let base_qualif = Self::in_place(cx, PlaceRef { + base: place.base, + projection: proj_base, + }); + let qualif = base_qualif && Self::mask_for_ty( + cx, + Place::ty_from(place.base, proj_base, cx.body, cx.tcx) + .projection_ty(cx.tcx, elem) + .ty, + ); + match elem { + ProjectionElem::Deref | + ProjectionElem::Subslice { .. } | + ProjectionElem::Field(..) | + ProjectionElem::ConstantIndex { .. } | + ProjectionElem::Downcast(..) => qualif, + + ProjectionElem::Index(local) => qualif || Self::in_local(cx, *local), + } + } else { + bug!("This should be called if projection is not empty"); + } + } + + fn in_projection( + cx: &ConstCx<'_, 'tcx>, + place: PlaceRef<'_, 'tcx>, + ) -> bool { + Self::in_projection_structurally(cx, place) + } + + fn in_place(cx: &ConstCx<'_, 'tcx>, place: PlaceRef<'_, 'tcx>) -> bool { + match place { + PlaceRef { + base: PlaceBase::Local(local), + projection: [], + } => Self::in_local(cx, *local), + PlaceRef { + base: PlaceBase::Static(box Static { + kind: StaticKind::Promoted(..), + .. + }), + projection: [], + } => bug!("qualifying already promoted MIR"), + PlaceRef { + base: PlaceBase::Static(static_), + projection: [], + } => { + Self::in_static(cx, static_) + }, + PlaceRef { + base: _, + projection: [.., _], + } => Self::in_projection(cx, place), + } + } + + fn in_operand(cx: &ConstCx<'_, 'tcx>, operand: &Operand<'tcx>) -> bool { + match *operand { + Operand::Copy(ref place) | + Operand::Move(ref place) => Self::in_place(cx, place.as_ref()), + + Operand::Constant(ref constant) => { + if let ConstValue::Unevaluated(def_id, _) = constant.literal.val { + // Don't peek inside trait associated constants. + if cx.tcx.trait_of_item(def_id).is_some() { + Self::in_any_value_of_ty(cx, constant.literal.ty).unwrap_or(false) + } else { + let (bits, _) = cx.tcx.at(constant.span).mir_const_qualif(def_id); + + let qualif = PerQualif::decode_from_bits(bits).0[Self::IDX]; + + // Just in case the type is more specific than + // the definition, e.g., impl associated const + // with type parameters, take it into account. + qualif && Self::mask_for_ty(cx, constant.literal.ty) + } + } else { + false + } + } + } + } + + fn in_rvalue_structurally(cx: &ConstCx<'_, 'tcx>, rvalue: &Rvalue<'tcx>) -> bool { + match *rvalue { + Rvalue::NullaryOp(..) => false, + + Rvalue::Discriminant(ref place) | + Rvalue::Len(ref place) => Self::in_place(cx, place.as_ref()), + + Rvalue::Use(ref operand) | + Rvalue::Repeat(ref operand, _) | + Rvalue::UnaryOp(_, ref operand) | + Rvalue::Cast(_, ref operand, _) => Self::in_operand(cx, operand), + + Rvalue::BinaryOp(_, ref lhs, ref rhs) | + Rvalue::CheckedBinaryOp(_, ref lhs, ref rhs) => { + Self::in_operand(cx, lhs) || Self::in_operand(cx, rhs) + } + + Rvalue::Ref(_, _, ref place) => { + // Special-case reborrows to be more like a copy of the reference. + if let box [proj_base @ .., elem] = &place.projection { + if ProjectionElem::Deref == *elem { + let base_ty = Place::ty_from(&place.base, proj_base, cx.body, cx.tcx).ty; + if let ty::Ref(..) = base_ty.sty { + return Self::in_place(cx, PlaceRef { + base: &place.base, + projection: proj_base, + }); + } + } + } + + Self::in_place(cx, place.as_ref()) + } + + Rvalue::Aggregate(_, ref operands) => { + operands.iter().any(|o| Self::in_operand(cx, o)) + } + } + } + + fn in_rvalue(cx: &ConstCx<'_, 'tcx>, rvalue: &Rvalue<'tcx>) -> bool { + Self::in_rvalue_structurally(cx, rvalue) + } + + fn in_call( + cx: &ConstCx<'_, 'tcx>, + _callee: &Operand<'tcx>, + _args: &[Operand<'tcx>], + return_ty: Ty<'tcx>, + ) -> bool { + // Be conservative about the returned value of a const fn. + Self::in_any_value_of_ty(cx, return_ty).unwrap_or(false) + } + + fn in_value(cx: &ConstCx<'_, 'tcx>, source: ValueSource<'_, 'tcx>) -> bool { + match source { + ValueSource::Rvalue(rvalue) => Self::in_rvalue(cx, rvalue), + ValueSource::DropAndReplace(source) => Self::in_operand(cx, source), + ValueSource::Call { callee, args, return_ty } => { + Self::in_call(cx, callee, args, return_ty) + } + } + } +} + +/// Constant containing interior mutability (`UnsafeCell`). +/// This must be ruled out to make sure that evaluating the constant at compile-time +/// and at *any point* during the run-time would produce the same result. In particular, +/// promotion of temporaries must not change program behavior; if the promoted could be +/// written to, that would be a problem. +struct HasMutInterior; + +impl Qualif for HasMutInterior { + const IDX: usize = 0; + + fn in_any_value_of_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> Option { + Some(!ty.is_freeze(cx.tcx, cx.param_env, DUMMY_SP)) + } + + fn in_rvalue(cx: &ConstCx<'_, 'tcx>, rvalue: &Rvalue<'tcx>) -> bool { + match *rvalue { + // Returning `true` for `Rvalue::Ref` indicates the borrow isn't + // allowed in constants (and the `Checker` will error), and/or it + // won't be promoted, due to `&mut ...` or interior mutability. + Rvalue::Ref(_, kind, ref place) => { + let ty = place.ty(cx.body, cx.tcx).ty; + + if let BorrowKind::Mut { .. } = kind { + // In theory, any zero-sized value could be borrowed + // mutably without consequences. However, only &mut [] + // is allowed right now, and only in functions. + if cx.mode == Mode::StaticMut { + // Inside a `static mut`, &mut [...] is also allowed. + match ty.sty { + ty::Array(..) | ty::Slice(_) => {} + _ => return true, + } + } else if let ty::Array(_, len) = ty.sty { + // FIXME(eddyb) the `cx.mode == Mode::NonConstFn` condition + // seems unnecessary, given that this is merely a ZST. + match len.try_eval_usize(cx.tcx, cx.param_env) { + Some(0) if cx.mode == Mode::NonConstFn => {}, + _ => return true, + } + } else { + return true; + } + } + } + + Rvalue::Aggregate(ref kind, _) => { + if let AggregateKind::Adt(def, ..) = **kind { + if Some(def.did) == cx.tcx.lang_items().unsafe_cell_type() { + let ty = rvalue.ty(cx.body, cx.tcx); + assert_eq!(Self::in_any_value_of_ty(cx, ty), Some(true)); + return true; + } + } + } + + _ => {} + } + + Self::in_rvalue_structurally(cx, rvalue) + } +} + +/// Constant containing an ADT that implements `Drop`. +/// This must be ruled out (a) because we cannot run `Drop` during compile-time +/// as that might not be a `const fn`, and (b) because implicit promotion would +/// remove side-effects that occur as part of dropping that value. +struct NeedsDrop; + +impl Qualif for NeedsDrop { + const IDX: usize = 1; + + fn in_any_value_of_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> Option { + Some(ty.needs_drop(cx.tcx, cx.param_env)) + } + + fn in_rvalue(cx: &ConstCx<'_, 'tcx>, rvalue: &Rvalue<'tcx>) -> bool { + if let Rvalue::Aggregate(ref kind, _) = *rvalue { + if let AggregateKind::Adt(def, ..) = **kind { + if def.has_dtor(cx.tcx) { + return true; + } + } + } + + Self::in_rvalue_structurally(cx, rvalue) + } +} From 3a5442a89100b6d7adaf5cb82ee6665a0aa049eb Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:33 -0700 Subject: [PATCH 06/31] Remove unnecessary method --- src/librustc_mir/transform/check_consts/qualifs.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index 0050a56624bbd..33034cb8880ab 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -170,16 +170,6 @@ trait Qualif { // Be conservative about the returned value of a const fn. Self::in_any_value_of_ty(cx, return_ty).unwrap_or(false) } - - fn in_value(cx: &ConstCx<'_, 'tcx>, source: ValueSource<'_, 'tcx>) -> bool { - match source { - ValueSource::Rvalue(rvalue) => Self::in_rvalue(cx, rvalue), - ValueSource::DropAndReplace(source) => Self::in_operand(cx, source), - ValueSource::Call { callee, args, return_ty } => { - Self::in_call(cx, callee, args, return_ty) - } - } - } } /// Constant containing interior mutability (`UnsafeCell`). From c2e121da081afeada5542774d481f54c9148d838 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:34 -0700 Subject: [PATCH 07/31] Make new qualifs public --- src/librustc_mir/transform/check_consts/qualifs.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index 33034cb8880ab..914c1575f930c 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -4,7 +4,7 @@ /// definitely cannot find anything bad anywhere. /// /// The default implementations proceed structurally. -trait Qualif { +pub trait Qualif { const IDX: usize; /// Return the qualification that is (conservatively) correct for any value @@ -177,7 +177,7 @@ trait Qualif { /// and at *any point* during the run-time would produce the same result. In particular, /// promotion of temporaries must not change program behavior; if the promoted could be /// written to, that would be a problem. -struct HasMutInterior; +pub struct HasMutInterior; impl Qualif for HasMutInterior { const IDX: usize = 0; @@ -238,7 +238,7 @@ impl Qualif for HasMutInterior { /// This must be ruled out (a) because we cannot run `Drop` during compile-time /// as that might not be a `const fn`, and (b) because implicit promotion would /// remove side-effects that occur as part of dropping that value. -struct NeedsDrop; +pub struct NeedsDrop; impl Qualif for NeedsDrop { const IDX: usize = 1; From 48d3843be637af64247851eb84e9fe8381d1fc33 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:35 -0700 Subject: [PATCH 08/31] Add requisite imports and bitset to hold qualifs --- .../transform/check_consts/qualifs.rs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index 914c1575f930c..742e5d196284e 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -1,3 +1,21 @@ +use rustc::mir::*; +use rustc::mir::interpret::ConstValue; +use rustc::ty::{self, Ty}; +use rustc_data_structures::bit_set::BitSet; +use syntax_pos::DUMMY_SP; + +use super::Item as ConstCx; +use super::validation::Mode; + +#[derive(Clone, Copy)] +pub struct QualifSet(u8); + +impl QualifSet { + fn contains(self) -> bool { + self.0 & (1 << Q::IDX) != 0 + } +} + /// A "qualif"(-ication) is a way to look for something "bad" in the MIR that would disqualify some /// code for promotion or prevent it from evaluating at compile time. So `return true` means /// "I found something bad, no reason to go on searching". `false` is only returned if we @@ -103,7 +121,7 @@ pub trait Qualif { } else { let (bits, _) = cx.tcx.at(constant.span).mir_const_qualif(def_id); - let qualif = PerQualif::decode_from_bits(bits).0[Self::IDX]; + let qualif = QualifSet(bits).contains::(); // Just in case the type is more specific than // the definition, e.g., impl associated const From 3758e383a66fb74067d575615f85a5bf14eb0d32 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:37 -0700 Subject: [PATCH 09/31] Remove reference to `Mode::NonConstFn` in qualifs This should have no effect on behavior since the validator is never run in const contexts. --- .../transform/check_consts/qualifs.rs | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index 742e5d196284e..39a1a2e7d2915 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -214,23 +214,19 @@ impl Qualif for HasMutInterior { if let BorrowKind::Mut { .. } = kind { // In theory, any zero-sized value could be borrowed - // mutably without consequences. However, only &mut [] - // is allowed right now, and only in functions. - if cx.mode == Mode::StaticMut { + // mutably without consequences. + match ty.sty { // Inside a `static mut`, &mut [...] is also allowed. - match ty.sty { - ty::Array(..) | ty::Slice(_) => {} - _ => return true, - } - } else if let ty::Array(_, len) = ty.sty { - // FIXME(eddyb) the `cx.mode == Mode::NonConstFn` condition - // seems unnecessary, given that this is merely a ZST. - match len.try_eval_usize(cx.tcx, cx.param_env) { - Some(0) if cx.mode == Mode::NonConstFn => {}, - _ => return true, - } - } else { - return true; + ty::Array(..) | ty::Slice(_) if cx.mode == Mode::StaticMut => {}, + + // FIXME(ecstaticmorse): uncomment the following match arm to stop marking + // `&mut []` as `HasMutInterior`. + /* + ty::Array(_, len) if len.try_eval_usize(cx.tcx, cx.param_env) == Some(0) + => {}, + */ + + _ => return true, } } } From 908dcb80b7e828ad7ecb312388533b9996281293 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:38 -0700 Subject: [PATCH 10/31] Control whether a `Qualif` is cleared on move --- src/librustc_mir/transform/check_consts/qualifs.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index 39a1a2e7d2915..54f5a07a4a3b1 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -25,6 +25,9 @@ impl QualifSet { pub trait Qualif { const IDX: usize; + /// Whether this `Qualif` is cleared when a local is moved from. + const IS_CLEARED_ON_MOVE: bool = false; + /// Return the qualification that is (conservatively) correct for any value /// of the type, or `None` if the qualification is not value/type-based. fn in_any_value_of_ty(_cx: &ConstCx<'_, 'tcx>, _ty: Ty<'tcx>) -> Option { @@ -256,6 +259,7 @@ pub struct NeedsDrop; impl Qualif for NeedsDrop { const IDX: usize = 1; + const IS_CLEARED_ON_MOVE: bool = true; fn in_any_value_of_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> Option { Some(ty.needs_drop(cx.tcx, cx.param_env)) From 3698d04fefaa72a1418986aede9cc1d05b6b899b Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:39 -0700 Subject: [PATCH 11/31] Pass current qualification state in a separate parameter --- .../transform/check_consts/qualifs.rs | 61 +++++++++++-------- 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index 54f5a07a4a3b1..232a5401d73fa 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -40,10 +40,6 @@ pub trait Qualif { Self::in_any_value_of_ty(cx, ty).unwrap_or(true) } - fn in_local(cx: &ConstCx<'_, '_>, local: Local) -> bool { - cx.per_local.0[Self::IDX].contains(local) - } - fn in_static(_cx: &ConstCx<'_, 'tcx>, _static: &Static<'tcx>) -> bool { // FIXME(eddyb) should we do anything here for value properties? false @@ -51,10 +47,11 @@ pub trait Qualif { fn in_projection_structurally( cx: &ConstCx<'_, 'tcx>, + per_local: &BitSet, place: PlaceRef<'_, 'tcx>, ) -> bool { if let [proj_base @ .., elem] = place.projection { - let base_qualif = Self::in_place(cx, PlaceRef { + let base_qualif = Self::in_place(cx, per_local, PlaceRef { base: place.base, projection: proj_base, }); @@ -71,7 +68,7 @@ pub trait Qualif { ProjectionElem::ConstantIndex { .. } | ProjectionElem::Downcast(..) => qualif, - ProjectionElem::Index(local) => qualif || Self::in_local(cx, *local), + ProjectionElem::Index(local) => qualif || per_local.contains(*local), } } else { bug!("This should be called if projection is not empty"); @@ -80,17 +77,22 @@ pub trait Qualif { fn in_projection( cx: &ConstCx<'_, 'tcx>, + per_local: &BitSet, place: PlaceRef<'_, 'tcx>, ) -> bool { - Self::in_projection_structurally(cx, place) + Self::in_projection_structurally(cx, per_local, place) } - fn in_place(cx: &ConstCx<'_, 'tcx>, place: PlaceRef<'_, 'tcx>) -> bool { + fn in_place( + cx: &ConstCx<'_, 'tcx>, + per_local: &BitSet, + place: PlaceRef<'_, 'tcx>, + ) -> bool { match place { PlaceRef { base: PlaceBase::Local(local), projection: [], - } => Self::in_local(cx, *local), + } => per_local.contains(*local), PlaceRef { base: PlaceBase::Static(box Static { kind: StaticKind::Promoted(..), @@ -107,14 +109,18 @@ pub trait Qualif { PlaceRef { base: _, projection: [.., _], - } => Self::in_projection(cx, place), + } => Self::in_projection(cx, per_local, place), } } - fn in_operand(cx: &ConstCx<'_, 'tcx>, operand: &Operand<'tcx>) -> bool { + fn in_operand( + cx: &ConstCx<'_, 'tcx>, + per_local: &BitSet, + operand: &Operand<'tcx>, + ) -> bool { match *operand { Operand::Copy(ref place) | - Operand::Move(ref place) => Self::in_place(cx, place.as_ref()), + Operand::Move(ref place) => Self::in_place(cx, per_local, place.as_ref()), Operand::Constant(ref constant) => { if let ConstValue::Unevaluated(def_id, _) = constant.literal.val { @@ -138,21 +144,25 @@ pub trait Qualif { } } - fn in_rvalue_structurally(cx: &ConstCx<'_, 'tcx>, rvalue: &Rvalue<'tcx>) -> bool { + fn in_rvalue_structurally( + cx: &ConstCx<'_, 'tcx>, + per_local: &BitSet, + rvalue: &Rvalue<'tcx>, + ) -> bool { match *rvalue { Rvalue::NullaryOp(..) => false, Rvalue::Discriminant(ref place) | - Rvalue::Len(ref place) => Self::in_place(cx, place.as_ref()), + Rvalue::Len(ref place) => Self::in_place(cx, per_local, place.as_ref()), Rvalue::Use(ref operand) | Rvalue::Repeat(ref operand, _) | Rvalue::UnaryOp(_, ref operand) | - Rvalue::Cast(_, ref operand, _) => Self::in_operand(cx, operand), + Rvalue::Cast(_, ref operand, _) => Self::in_operand(cx, per_local, operand), Rvalue::BinaryOp(_, ref lhs, ref rhs) | Rvalue::CheckedBinaryOp(_, ref lhs, ref rhs) => { - Self::in_operand(cx, lhs) || Self::in_operand(cx, rhs) + Self::in_operand(cx, per_local, lhs) || Self::in_operand(cx, per_local, rhs) } Rvalue::Ref(_, _, ref place) => { @@ -161,7 +171,7 @@ pub trait Qualif { if ProjectionElem::Deref == *elem { let base_ty = Place::ty_from(&place.base, proj_base, cx.body, cx.tcx).ty; if let ty::Ref(..) = base_ty.sty { - return Self::in_place(cx, PlaceRef { + return Self::in_place(cx, per_local, PlaceRef { base: &place.base, projection: proj_base, }); @@ -169,21 +179,22 @@ pub trait Qualif { } } - Self::in_place(cx, place.as_ref()) + Self::in_place(cx, per_local, place.as_ref()) } Rvalue::Aggregate(_, ref operands) => { - operands.iter().any(|o| Self::in_operand(cx, o)) + operands.iter().any(|o| Self::in_operand(cx, per_local, o)) } } } - fn in_rvalue(cx: &ConstCx<'_, 'tcx>, rvalue: &Rvalue<'tcx>) -> bool { - Self::in_rvalue_structurally(cx, rvalue) + fn in_rvalue(cx: &ConstCx<'_, 'tcx>, per_local: &BitSet, rvalue: &Rvalue<'tcx>) -> bool { + Self::in_rvalue_structurally(cx, per_local, rvalue) } fn in_call( cx: &ConstCx<'_, 'tcx>, + _per_local: &BitSet, _callee: &Operand<'tcx>, _args: &[Operand<'tcx>], return_ty: Ty<'tcx>, @@ -207,7 +218,7 @@ impl Qualif for HasMutInterior { Some(!ty.is_freeze(cx.tcx, cx.param_env, DUMMY_SP)) } - fn in_rvalue(cx: &ConstCx<'_, 'tcx>, rvalue: &Rvalue<'tcx>) -> bool { + fn in_rvalue(cx: &ConstCx<'_, 'tcx>, per_local: &BitSet, rvalue: &Rvalue<'tcx>) -> bool { match *rvalue { // Returning `true` for `Rvalue::Ref` indicates the borrow isn't // allowed in constants (and the `Checker` will error), and/or it @@ -247,7 +258,7 @@ impl Qualif for HasMutInterior { _ => {} } - Self::in_rvalue_structurally(cx, rvalue) + Self::in_rvalue_structurally(cx, per_local, rvalue) } } @@ -265,7 +276,7 @@ impl Qualif for NeedsDrop { Some(ty.needs_drop(cx.tcx, cx.param_env)) } - fn in_rvalue(cx: &ConstCx<'_, 'tcx>, rvalue: &Rvalue<'tcx>) -> bool { + fn in_rvalue(cx: &ConstCx<'_, 'tcx>, per_local: &BitSet, rvalue: &Rvalue<'tcx>) -> bool { if let Rvalue::Aggregate(ref kind, _) = *rvalue { if let AggregateKind::Adt(def, ..) = **kind { if def.has_dtor(cx.tcx) { @@ -274,6 +285,6 @@ impl Qualif for NeedsDrop { } } - Self::in_rvalue_structurally(cx, rvalue) + Self::in_rvalue_structurally(cx, per_local, rvalue) } } From fc92d3b8202874c52328fbc3b7816f999586045a Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:40 -0700 Subject: [PATCH 12/31] Add dataflow-based const validation --- .../transform/check_consts/mod.rs | 45 + .../transform/check_consts/resolver.rs | 327 +++++++ .../transform/check_consts/validation.rs | 919 ++++++++++++++++++ src/librustc_mir/transform/mod.rs | 1 + 4 files changed, 1292 insertions(+) create mode 100644 src/librustc_mir/transform/check_consts/mod.rs create mode 100644 src/librustc_mir/transform/check_consts/resolver.rs create mode 100644 src/librustc_mir/transform/check_consts/validation.rs diff --git a/src/librustc_mir/transform/check_consts/mod.rs b/src/librustc_mir/transform/check_consts/mod.rs new file mode 100644 index 0000000000000..ce26af64741b8 --- /dev/null +++ b/src/librustc_mir/transform/check_consts/mod.rs @@ -0,0 +1,45 @@ +use rustc::hir::def_id::DefId; +use rustc::mir; +use rustc::ty::{self, TyCtxt}; + +pub use self::qualifs::Qualif; + +mod resolver; +mod qualifs; +pub mod validation; + +/// Information about the item currently being validated, as well as a reference to the global +/// context. +pub struct Item<'mir, 'tcx> { + body: &'mir mir::Body<'tcx>, + tcx: TyCtxt<'tcx>, + def_id: DefId, + param_env: ty::ParamEnv<'tcx>, + mode: validation::Mode, +} + +impl Item<'mir, 'tcx> { + pub fn new( + tcx: TyCtxt<'tcx>, + def_id: DefId, + body: &'mir mir::Body<'tcx>, + ) -> Self { + let param_env = tcx.param_env(def_id); + let mode = validation::Mode::for_item(tcx, def_id) + .expect("const validation must only be run inside a const context"); + + Item { + body, + tcx, + def_id, + param_env, + mode, + } + } +} + + +fn is_lang_panic_fn(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool { + Some(def_id) == tcx.lang_items().panic_fn() || + Some(def_id) == tcx.lang_items().begin_panic_fn() +} diff --git a/src/librustc_mir/transform/check_consts/resolver.rs b/src/librustc_mir/transform/check_consts/resolver.rs new file mode 100644 index 0000000000000..4fa00bf098be4 --- /dev/null +++ b/src/librustc_mir/transform/check_consts/resolver.rs @@ -0,0 +1,327 @@ +use rustc::mir::visit::Visitor; +use rustc::mir::{self, BasicBlock, Local, Location}; +use rustc_data_structures::bit_set::BitSet; + +use std::cell::RefCell; +use std::marker::PhantomData; +use std::rc::Rc; + +use crate::dataflow::{self as old_dataflow, generic as dataflow}; +use super::{Item, Qualif}; +use self::old_dataflow::IndirectlyMutableLocals; + +/// A `Visitor` that propagates qualifs between locals. This defines the transfer function of +/// `FlowSensitiveAnalysis` as well as the logic underlying `TempPromotionResolver`. +/// +/// This transfer does nothing when encountering an indirect assignment. Consumers should rely on +/// the `IndirectlyMutableLocals` dataflow pass to see if a `Local` may have become qualified via +/// an indirect assignment or function call. +struct TransferFunction<'a, 'mir, 'tcx, Q> { + item: &'a Item<'mir, 'tcx>, + qualifs_per_local: &'a mut BitSet, + + _qualif: PhantomData, +} + +impl TransferFunction<'a, 'mir, 'tcx, Q> +where + Q: Qualif, +{ + fn new( + item: &'a Item<'mir, 'tcx>, + qualifs_per_local: &'a mut BitSet, + ) -> Self { + TransferFunction { + item, + qualifs_per_local, + _qualif: PhantomData, + } + } + + fn initialize_state(&mut self) { + self.qualifs_per_local.clear(); + + for arg in self.item.body.args_iter() { + let arg_ty = self.item.body.local_decls[arg].ty; + if Q::in_any_value_of_ty(self.item, arg_ty).unwrap() { + self.qualifs_per_local.insert(arg); + } + } + } + + fn assign_qualif_direct(&mut self, place: &mir::Place<'tcx>, value: bool) { + debug_assert!(!place.is_indirect()); + + match (value, place) { + (true, mir::Place { base: mir::PlaceBase::Local(local), .. }) => { + self.qualifs_per_local.insert(*local); + } + + // For now, we do not clear the qualif if a local is overwritten in full by + // an unqualified rvalue (e.g. `y = 5`). This is to be consistent + // with aggregates where we overwrite all fields with assignments, which would not + // get this feature. + (false, mir::Place { base: mir::PlaceBase::Local(_local), projection: box [] }) => { + // self.qualifs_per_local.remove(*local); + } + + _ => {} + } + } + + fn apply_call_return_effect( + &mut self, + _block: BasicBlock, + func: &mir::Operand<'tcx>, + args: &[mir::Operand<'tcx>], + return_place: &mir::Place<'tcx>, + ) { + let return_ty = return_place.ty(self.item.body, self.item.tcx).ty; + let qualif = Q::in_call(self.item, &mut self.qualifs_per_local, func, args, return_ty); + if !return_place.is_indirect() { + self.assign_qualif_direct(return_place, qualif); + } + } +} + +impl Visitor<'tcx> for TransferFunction<'_, '_, 'tcx, Q> +where + Q: Qualif, +{ + fn visit_operand(&mut self, operand: &mir::Operand<'tcx>, location: Location) { + self.super_operand(operand, location); + + if !Q::IS_CLEARED_ON_MOVE { + return; + } + + // If a local with no projections is moved from (e.g. `x` in `y = x`), record that + // it no longer needs to be dropped. + if let mir::Operand::Move(mir::Place { + base: mir::PlaceBase::Local(local), + projection: box [], + }) = *operand { + self.qualifs_per_local.remove(local); + } + } + + fn visit_assign( + &mut self, + place: &mir::Place<'tcx>, + rvalue: &mir::Rvalue<'tcx>, + location: Location, + ) { + let qualif = Q::in_rvalue(self.item, self.qualifs_per_local, rvalue); + if !place.is_indirect() { + self.assign_qualif_direct(place, qualif); + } + + // We need to assign qualifs to the left-hand side before visiting `rvalue` since + // qualifs can be cleared on move. + self.super_assign(place, rvalue, location); + } + + fn visit_terminator_kind(&mut self, kind: &mir::TerminatorKind<'tcx>, location: Location) { + // The effect of assignment to the return place in `TerminatorKind::Call` is not applied + // here; that occurs in `apply_call_return_effect`. + + if let mir::TerminatorKind::DropAndReplace { value, location: dest, .. } = kind { + let qualif = Q::in_operand(self.item, self.qualifs_per_local, value); + if !dest.is_indirect() { + self.assign_qualif_direct(dest, qualif); + } + } + + // We need to assign qualifs to the dropped location before visiting the operand that + // replaces it since qualifs can be cleared on move. + self.super_terminator_kind(kind, location); + } +} + +/// Types that can compute the qualifs of each local at each location in a `mir::Body`. +/// +/// Code that wishes to use a `QualifResolver` must call `visit_{statement,terminator}` for each +/// statement or terminator, processing blocks in reverse post-order beginning from the +/// `START_BLOCK`. Calling code may optionally call `get` after visiting each statement or +/// terminator to query the qualification state immediately before that statement or terminator. +/// +/// These conditions are much more restrictive than woud be required by `FlowSensitiveResolver` +/// alone. This is to allow a linear, on-demand `TempPromotionResolver` that can operate +/// efficiently on simple CFGs. +pub trait QualifResolver { + /// Get the qualifs of each local at the last location visited. + /// + /// This takes `&mut self` so qualifs can be computed lazily. + fn get(&mut self) -> &BitSet; + + /// A convenience method for `self.get().contains(local)`. + fn contains(&mut self, local: Local) -> bool { + self.get().contains(local) + } + + /// Resets the resolver to the `START_BLOCK`. This allows a resolver to be reused + /// for multiple passes over a `mir::Body`. + fn reset(&mut self); +} + +type IndirectlyMutableResults<'mir, 'tcx> = + old_dataflow::DataflowResultsCursor<'mir, 'tcx, IndirectlyMutableLocals<'mir, 'tcx>>; + +/// A resolver for qualifs that works on arbitrarily complex CFGs. +/// +/// As soon as a `Local` becomes writable through a reference (as determined by the +/// `IndirectlyMutableLocals` dataflow pass), we must assume that it takes on all other qualifs +/// possible for its type. This is because no effort is made to track qualifs across indirect +/// assignments (e.g. `*p = x` or calls to opaque functions). +/// +/// It is possible to be more precise here by waiting until an indirect assignment actually occurs +/// before marking a borrowed `Local` as qualified. +pub struct FlowSensitiveResolver<'a, 'mir, 'tcx, Q> +where + Q: Qualif, +{ + location: Location, + indirectly_mutable_locals: Rc>>, + cursor: dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'a, 'mir, 'tcx, Q>>, + qualifs_per_local: BitSet, +} + +impl FlowSensitiveResolver<'a, 'mir, 'tcx, Q> +where + Q: Qualif, +{ + pub fn new( + _: Q, + item: &'a Item<'mir, 'tcx>, + indirectly_mutable_locals: Rc>>, + dead_unwinds: &BitSet, + ) -> Self { + let analysis = FlowSensitiveAnalysis { + item, + _qualif: PhantomData, + }; + let results = + dataflow::Engine::new(item.body, dead_unwinds, analysis).iterate_to_fixpoint(); + let cursor = dataflow::ResultsCursor::new(item.body, results); + + FlowSensitiveResolver { + cursor, + indirectly_mutable_locals, + qualifs_per_local: BitSet::new_empty(item.body.local_decls.len()), + location: Location { block: mir::START_BLOCK, statement_index: 0 }, + } + } +} + +impl Visitor<'tcx> for FlowSensitiveResolver<'_, '_, 'tcx, Q> +where + Q: Qualif +{ + fn visit_statement(&mut self, _: &mir::Statement<'tcx>, location: Location) { + self.location = location; + } + + fn visit_terminator(&mut self, _: &mir::Terminator<'tcx>, location: Location) { + self.location = location; + } +} + +impl QualifResolver for FlowSensitiveResolver<'_, '_, '_, Q> +where + Q: Qualif +{ + fn get(&mut self) -> &BitSet { + let mut indirectly_mutable_locals = self.indirectly_mutable_locals.borrow_mut(); + + indirectly_mutable_locals.seek(self.location); + self.cursor.seek_before(self.location); + + self.qualifs_per_local.overwrite(indirectly_mutable_locals.get()); + self.qualifs_per_local.union(self.cursor.get()); + &self.qualifs_per_local + } + + fn contains(&mut self, local: Local) -> bool { + self.cursor.seek_before(self.location); + if self.cursor.get().contains(local) { + return true; + } + + + let mut indirectly_mutable_locals = self.indirectly_mutable_locals.borrow_mut(); + indirectly_mutable_locals.seek(self.location); + indirectly_mutable_locals.get().contains(local) + } + + fn reset(&mut self) { + self.location = Location { block: mir::START_BLOCK, statement_index: 0 }; + } +} + +/// The dataflow analysis used to propagate qualifs on arbitrary CFGs. +pub(super) struct FlowSensitiveAnalysis<'a, 'mir, 'tcx, Q> { + item: &'a Item<'mir, 'tcx>, + _qualif: PhantomData, +} + +impl<'a, 'mir, 'tcx, Q> FlowSensitiveAnalysis<'a, 'mir, 'tcx, Q> +where + Q: Qualif, +{ + fn transfer_function( + &self, + state: &'a mut BitSet, + ) -> TransferFunction<'a, 'mir, 'tcx, Q> { + TransferFunction::::new(self.item, state) + } +} + +impl old_dataflow::BottomValue for FlowSensitiveAnalysis<'_, '_, '_, Q> { + const BOTTOM_VALUE: bool = false; +} + +impl dataflow::Analysis<'tcx> for FlowSensitiveAnalysis<'_, '_, 'tcx, Q> +where + Q: Qualif, +{ + type Idx = Local; + + const NAME: &'static str = "flow_sensitive_qualif"; + + fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize { + body.local_decls.len() + } + + fn initialize_start_block(&self, _body: &mir::Body<'tcx>, state: &mut BitSet) { + self.transfer_function(state).initialize_state(); + } + + fn apply_statement_effect( + &self, + state: &mut BitSet, + statement: &mir::Statement<'tcx>, + location: Location, + ) { + self.transfer_function(state).visit_statement(statement, location); + } + + fn apply_terminator_effect( + &self, + state: &mut BitSet, + terminator: &mir::Terminator<'tcx>, + location: Location, + ) { + self.transfer_function(state).visit_terminator(terminator, location); + } + + fn apply_call_return_effect( + &self, + state: &mut BitSet, + block: BasicBlock, + func: &mir::Operand<'tcx>, + args: &[mir::Operand<'tcx>], + return_place: &mir::Place<'tcx>, + ) { + self.transfer_function(state).apply_call_return_effect(block, func, args, return_place) + } +} diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs new file mode 100644 index 0000000000000..eee8f4856fd8d --- /dev/null +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -0,0 +1,919 @@ +use rustc::hir::{self, def_id::DefId}; +use rustc::mir::visit::{PlaceContext, Visitor, MutatingUseContext, NonMutatingUseContext}; +use rustc::mir::*; +use rustc::session::config::nightly_options; +use rustc::ty::cast::CastTy; +use rustc::ty::{self, TyCtxt}; +use rustc_data_structures::bit_set::BitSet; +use rustc_target::spec::abi::Abi; +use syntax::feature_gate::{emit_feature_err, GateIssue}; +use syntax::symbol::sym; +use syntax_pos::{Span, Symbol}; + +use std::cell::RefCell; +use std::fmt; +use std::ops::Deref; +use std::rc::Rc; + +use crate::dataflow as old_dataflow; +use super::{Item, Qualif, is_lang_panic_fn}; +use super::resolver::{QualifResolver, FlowSensitiveResolver}; +use super::qualifs::{HasMutInterior, NeedsDrop}; + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum CheckOpResult { + Forbidden, + Unleashed, + Allowed, +} + +/// What kind of item we are in. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum Mode { + /// A `static` item. + Static, + /// A `static mut` item. + StaticMut, + /// A `const fn` item. + ConstFn, + /// A `const` item or an anonymous constant (e.g. in array lengths). + Const, +} + +impl Mode { + /// Returns the validation mode for the item with the given `DefId`, or `None` if this item + /// does not require validation (e.g. a non-const `fn`). + pub fn for_item(tcx: TyCtxt<'tcx>, def_id: DefId) -> Option { + use hir::BodyOwnerKind as HirKind; + + let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap(); + + let mode = match tcx.hir().body_owner_kind(hir_id) { + HirKind::Closure => return None, + + HirKind::Fn if tcx.is_const_fn(def_id) => Mode::ConstFn, + HirKind::Fn => return None, + + HirKind::Const => Mode::Const, + + HirKind::Static(hir::MutImmutable) => Mode::Static, + HirKind::Static(hir::MutMutable) => Mode::StaticMut, + }; + + Some(mode) + } + + pub fn is_static(self) -> bool { + match self { + Mode::Static | Mode::StaticMut => true, + Mode::ConstFn | Mode::Const => false, + } + } + + /// Returns `true` if the value returned by this item must be `Sync`. + /// + /// This returns false for `StaticMut` since all accesses to one are `unsafe` anyway. + pub fn requires_sync(self) -> bool { + match self { + Mode::Static => true, + Mode::ConstFn | Mode::Const | Mode::StaticMut => false, + } + } +} + +impl fmt::Display for Mode { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match *self { + Mode::Const => write!(f, "constant"), + Mode::Static | Mode::StaticMut => write!(f, "static"), + Mode::ConstFn => write!(f, "constant function"), + } + } +} + +/// An operation that is not *always* allowed in a const context. +pub trait NonConstOp { + /// Whether this operation can be evaluated by miri. + const IS_SUPPORTED_IN_MIRI: bool = true; + + /// Returns a boolean indicating whether the feature gate that would allow this operation is + /// enabled, or `None` if such a feature gate does not exist. + fn feature_gate(_tcx: TyCtxt<'tcx>) -> Option { + None + } + + /// Returns `true` if this operation is allowed in the given item. + /// + /// This check should assume that we are not in a non-const `fn`, where all operations are + /// legal. + fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { + Self::feature_gate(item.tcx).unwrap_or(false) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let mut err = struct_span_err!( + item.tcx.sess, + span, + E0019, + "{} contains unimplemented expression type", + item.mode + ); + if item.tcx.sess.teach(&err.get_code().unwrap()) { + err.note("A function call isn't allowed in the const's initialization expression \ + because the expression's value must be known at compile-time."); + err.note("Remember: you can't use a function call inside a const's initialization \ + expression! However, you can use it anywhere else."); + } + err.emit(); + } +} + +pub struct Qualifs<'a, 'mir, 'tcx> { + has_mut_interior: FlowSensitiveResolver<'a, 'mir, 'tcx, HasMutInterior>, + needs_drop: FlowSensitiveResolver<'a, 'mir, 'tcx, NeedsDrop>, +} + +pub struct Validator<'a, 'mir, 'tcx> { + item: &'a Item<'mir, 'tcx>, + qualifs: Qualifs<'a, 'mir, 'tcx>, + + /// The span of the current statement. + span: Span, + + /// True if the local was assigned the result of an illegal borrow (`ops::MutBorrow`). + /// + /// This is used to hide errors from {re,}borrowing the newly-assigned local, instead pointing + /// the user to the place where the illegal borrow occurred. This set is only populated once an + /// error has been emitted, so it will never cause an erroneous `mir::Body` to pass validation. + /// + /// FIXME(ecstaticmorse): assert at the end of checking that if `tcx.has_errors() == false`, + /// this set is empty. Note that if we start removing locals from + /// `derived_from_illegal_borrow`, just checking at the end won't be enough. + derived_from_illegal_borrow: BitSet, + + errors: Vec<(Span, String)>, + + /// Whether to actually emit errors or just store them in `errors`. + pub(crate) suppress_errors: bool, +} + +impl Deref for Validator<'_, 'mir, 'tcx> { + type Target = Item<'mir, 'tcx>; + + fn deref(&self) -> &Self::Target { + &self.item + } +} + +impl Validator<'a, 'mir, 'tcx> { + pub fn new(item: &'a Item<'mir, 'tcx>) -> Self { + let dead_unwinds = BitSet::new_empty(item.body.basic_blocks().len()); + + let indirectly_mutable_locals = old_dataflow::do_dataflow( + item.tcx, + item.body, + item.def_id, + &[], + &dead_unwinds, + old_dataflow::IndirectlyMutableLocals::new(item.tcx, item.body, item.param_env), + |_, local| old_dataflow::DebugFormatted::new(&local), + ); + + let indirectly_mutable_locals = old_dataflow::DataflowResultsCursor::new( + indirectly_mutable_locals, + item.body, + ); + let indirectly_mutable_locals = Rc::new(RefCell::new(indirectly_mutable_locals)); + + let needs_drop = FlowSensitiveResolver::new( + NeedsDrop, + item, + indirectly_mutable_locals.clone(), + &dead_unwinds, + ); + + let has_mut_interior = FlowSensitiveResolver::new( + HasMutInterior, + item, + indirectly_mutable_locals.clone(), + &dead_unwinds, + ); + + let qualifs = Qualifs { + needs_drop, + has_mut_interior, + }; + + Validator { + span: item.body.span, + item, + qualifs, + errors: vec![], + derived_from_illegal_borrow: BitSet::new_empty(item.body.local_decls.len()), + suppress_errors: false, + } + } + + /// Resets the `QualifResolver`s used by this `Validator` and returns them so they can be + /// reused. + pub fn into_qualifs(mut self) -> Qualifs<'a, 'mir, 'tcx> { + self.qualifs.needs_drop.reset(); + self.qualifs.has_mut_interior.reset(); + self.qualifs + } + + pub fn take_errors(&mut self) -> Vec<(Span, String)> { + std::mem::replace(&mut self.errors, vec![]) + } + + /// Emits an error at the given `span` if an expression cannot be evaluated in the current + /// context. Returns `Forbidden` if an error was emitted. + pub fn check_op_spanned(&mut self, op: O, span: Span) -> CheckOpResult + where + O: NonConstOp + fmt::Debug + { + trace!("check_op: op={:?}", op); + + if op.is_allowed_in_item(self) { + return CheckOpResult::Allowed; + } + + // If an operation is supported in miri (and is not already controlled by a feature gate) it + // can be turned on with `-Zunleash-the-miri-inside-of-you`. + let is_unleashable = O::IS_SUPPORTED_IN_MIRI + && O::feature_gate(self.tcx).is_none(); + + if is_unleashable && self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you { + self.tcx.sess.span_warn(span, "skipping const checks"); + return CheckOpResult::Unleashed; + } + + if !self.suppress_errors { + op.emit_error(self, span); + } + + self.errors.push((span, format!("{:?}", op))); + CheckOpResult::Forbidden + } + + /// Emits an error if an expression cannot be evaluated in the current context. + pub fn check_op(&mut self, op: impl NonConstOp + fmt::Debug) -> CheckOpResult { + let span = self.span; + self.check_op_spanned(op, span) + } +} + +impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { + fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) { + trace!("visit_rvalue: rvalue={:?} location={:?}", rvalue, location); + + // Check nested operands and places. + if let Rvalue::Ref(_, kind, ref place) = *rvalue { + // Special-case reborrows to be more like a copy of a reference. + let mut reborrow_place = None; + if let box [proj_base @ .., elem] = &place.projection { + if *elem == ProjectionElem::Deref { + let base_ty = Place::ty_from(&place.base, proj_base, self.body, self.tcx).ty; + if let ty::Ref(..) = base_ty.sty { + reborrow_place = Some(proj_base); + } + } + } + + if let Some(proj) = reborrow_place { + let ctx = match kind { + BorrowKind::Shared => PlaceContext::NonMutatingUse( + NonMutatingUseContext::SharedBorrow, + ), + BorrowKind::Shallow => PlaceContext::NonMutatingUse( + NonMutatingUseContext::ShallowBorrow, + ), + BorrowKind::Unique => PlaceContext::NonMutatingUse( + NonMutatingUseContext::UniqueBorrow, + ), + BorrowKind::Mut { .. } => PlaceContext::MutatingUse( + MutatingUseContext::Borrow, + ), + }; + self.visit_place_base(&place.base, ctx, location); + self.visit_projection(&place.base, proj, ctx, location); + } else { + self.super_rvalue(rvalue, location); + } + } else { + self.super_rvalue(rvalue, location); + } + + match *rvalue { + Rvalue::Use(_) | + Rvalue::Repeat(..) | + Rvalue::UnaryOp(UnOp::Neg, _) | + Rvalue::UnaryOp(UnOp::Not, _) | + Rvalue::NullaryOp(NullOp::SizeOf, _) | + Rvalue::CheckedBinaryOp(..) | + Rvalue::Cast(CastKind::Pointer(_), ..) | + Rvalue::Discriminant(..) | + Rvalue::Len(_) | + Rvalue::Ref(..) | + Rvalue::Aggregate(..) => {} + + Rvalue::Cast(CastKind::Misc, ref operand, cast_ty) => { + let operand_ty = operand.ty(self.body, self.tcx); + let cast_in = CastTy::from_ty(operand_ty).expect("bad input type for cast"); + let cast_out = CastTy::from_ty(cast_ty).expect("bad output type for cast"); + + if let (CastTy::Ptr(_), CastTy::Int(_)) + | (CastTy::FnPtr, CastTy::Int(_)) = (cast_in, cast_out) { + self.check_op(ops::RawPtrToIntCast); + } + } + + Rvalue::BinaryOp(op, ref lhs, _) => { + if let ty::RawPtr(_) | ty::FnPtr(..) = lhs.ty(self.body, self.tcx).sty { + assert!(op == BinOp::Eq || op == BinOp::Ne || + op == BinOp::Le || op == BinOp::Lt || + op == BinOp::Ge || op == BinOp::Gt || + op == BinOp::Offset); + + + self.check_op(ops::RawPtrComparison); + } + } + + Rvalue::NullaryOp(NullOp::Box, _) => { + self.check_op(ops::HeapAllocation); + } + } + } + + fn visit_place_base( + &mut self, + place_base: &PlaceBase<'tcx>, + context: PlaceContext, + location: Location, + ) { + trace!( + "visit_place_base: place_base={:?} context={:?} location={:?}", + place_base, + context, + location, + ); + self.super_place_base(place_base, context, location); + + match place_base { + PlaceBase::Local(_) => {} + PlaceBase::Static(box Static{ kind: StaticKind::Promoted(_, _), .. }) => { + bug!("Promotion must be run after const validation"); + } + + PlaceBase::Static(box Static{ kind: StaticKind::Static, def_id, .. }) => { + let is_thread_local = self.tcx.has_attr(*def_id, sym::thread_local); + if is_thread_local { + self.check_op(ops::ThreadLocalAccess); + } else if self.mode == Mode::Static && context.is_mutating_use() { + // this is not strictly necessary as miri will also bail out + // For interior mutability we can't really catch this statically as that + // goes through raw pointers and intermediate temporaries, so miri has + // to catch this anyway + + self.tcx.sess.span_err( + self.span, + "cannot mutate statics in the initializer of another static", + ); + } else { + self.check_op(ops::StaticAccess); + } + } + } + } + + fn visit_assign(&mut self, dest: &Place<'tcx>, rvalue: &Rvalue<'tcx>, location: Location) { + trace!("visit_assign: dest={:?} rvalue={:?} location={:?}", dest, rvalue, location); + + // Error on mutable borrows or shared borrows of values with interior mutability. + // + // This replicates the logic at the start of `assign` in the old const checker. Note that + // it depends on `HasMutInterior` being set for mutable borrows as well as values with + // interior mutability. + if let Rvalue::Ref(_, kind, ref borrowed_place) = *rvalue { + let rvalue_has_mut_interior = HasMutInterior::in_rvalue( + &self.item, + self.qualifs.has_mut_interior.get(), + rvalue, + ); + + if rvalue_has_mut_interior { + let is_derived_from_illegal_borrow = match *borrowed_place { + // If an unprojected local was borrowed and its value was the result of an + // illegal borrow, suppress this error and mark the result of this borrow as + // illegal as well. + Place { base: PlaceBase::Local(borrowed_local), projection: box [] } + if self.derived_from_illegal_borrow.contains(borrowed_local) => true, + + // Otherwise proceed normally: check the legality of a mutable borrow in this + // context. + _ => self.check_op(ops::MutBorrow(kind)) == CheckOpResult::Forbidden, + }; + + // When the target of the assignment is a local with no projections, mark it as + // derived from an illegal borrow if necessary. + // + // FIXME: should we also clear `derived_from_illegal_borrow` when a local is + // assigned a new value? + if is_derived_from_illegal_borrow { + if let Place { base: PlaceBase::Local(dest), projection: box [] } = *dest { + self.derived_from_illegal_borrow.insert(dest); + } + } + } + } + + self.super_assign(dest, rvalue, location); + } + + fn visit_projection( + &mut self, + place_base: &PlaceBase<'tcx>, + proj: &[PlaceElem<'tcx>], + context: PlaceContext, + location: Location, + ) { + trace!( + "visit_place_projection: proj={:?} context={:?} location={:?}", + proj, + context, + location, + ); + self.super_projection(place_base, proj, context, location); + + let (elem, proj_base) = match proj.split_last() { + Some(x) => x, + None => return, + }; + + match elem { + ProjectionElem::Deref => { + if context.is_mutating_use() { + self.check_op(ops::MutDeref); + } + + let base_ty = Place::ty_from(place_base, proj_base, self.body, self.tcx).ty; + if let ty::RawPtr(_) = base_ty.sty { + self.check_op(ops::RawPtrDeref); + } + } + + ProjectionElem::ConstantIndex {..} | + ProjectionElem::Subslice {..} | + ProjectionElem::Field(..) | + ProjectionElem::Index(_) => { + let base_ty = Place::ty_from(place_base, proj_base, self.body, self.tcx).ty; + match base_ty.ty_adt_def() { + Some(def) if def.is_union() => { + self.check_op(ops::UnionAccess); + } + + _ => {} + } + } + + ProjectionElem::Downcast(..) => { + self.check_op(ops::Downcast); + } + } + } + + + fn visit_source_info(&mut self, source_info: &SourceInfo) { + trace!("visit_source_info: source_info={:?}", source_info); + self.span = source_info.span; + } + + fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { + trace!("visit_statement: statement={:?} location={:?}", statement, location); + + self.qualifs.needs_drop.visit_statement(statement, location); + self.qualifs.has_mut_interior.visit_statement(statement, location); + debug!("needs_drop: {:?}", self.qualifs.needs_drop.get()); + debug!("has_mut_interior: {:?}", self.qualifs.has_mut_interior.get()); + + match statement.kind { + StatementKind::Assign(..) => { + self.super_statement(statement, location); + } + StatementKind::FakeRead(FakeReadCause::ForMatchedPlace, _) => { + self.check_op(ops::IfOrMatch); + } + // FIXME(eddyb) should these really do nothing? + StatementKind::FakeRead(..) | + StatementKind::SetDiscriminant { .. } | + StatementKind::StorageLive(_) | + StatementKind::StorageDead(_) | + StatementKind::InlineAsm {..} | + StatementKind::Retag { .. } | + StatementKind::AscribeUserType(..) | + StatementKind::Nop => {} + } + } + + fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { + trace!("visit_terminator: terminator={:?} location={:?}", terminator, location); + + self.qualifs.needs_drop.visit_terminator(terminator, location); + self.qualifs.has_mut_interior.visit_terminator(terminator, location); + debug!("needs_drop: {:?}", self.qualifs.needs_drop.get()); + debug!("has_mut_interior: {:?}", self.qualifs.has_mut_interior.get()); + + self.super_terminator(terminator, location); + } + + fn visit_terminator_kind(&mut self, kind: &TerminatorKind<'tcx>, location: Location) { + trace!("visit_terminator_kind: kind={:?} location={:?}", kind, location); + self.super_terminator_kind(kind, location); + + match kind { + TerminatorKind::Call { func, .. } => { + let fn_ty = func.ty(self.body, self.tcx); + + let def_id = match fn_ty.sty { + ty::FnDef(def_id, _) => def_id, + + ty::FnPtr(_) => { + self.check_op(ops::FnCallIndirect); + return; + } + _ => { + self.check_op(ops::FnCallOther); + return; + } + }; + + // At this point, we are calling a function whose `DefId` is known... + + if let Abi::RustIntrinsic | Abi::PlatformIntrinsic = self.tcx.fn_sig(def_id).abi() { + assert!(!self.tcx.is_const_fn(def_id)); + + if self.tcx.item_name(def_id) == sym::transmute { + self.check_op(ops::Transmute); + return; + } + + // To preserve the current semantics, we return early, allowing all + // intrinsics (except `transmute`) to pass unchecked to miri. + // + // FIXME: We should keep a whitelist of allowed intrinsics (or at least a + // blacklist of unimplemented ones) and fail here instead. + return; + } + + if self.tcx.is_const_fn(def_id) { + return; + } + + if is_lang_panic_fn(self.tcx, def_id) { + self.check_op(ops::Panic); + } else if let Some(feature) = self.tcx.is_unstable_const_fn(def_id) { + // Exempt unstable const fns inside of macros with + // `#[allow_internal_unstable]`. + if !self.span.allows_unstable(feature) { + self.check_op(ops::FnCallUnstable(def_id, feature)); + } + } else { + self.check_op(ops::FnCallNonConst(def_id)); + } + + } + + // Forbid all `Drop` terminators unless the place being dropped is a local with no + // projections that cannot be `NeedsDrop`. + | TerminatorKind::Drop { location: dropped_place, .. } + | TerminatorKind::DropAndReplace { location: dropped_place, .. } + => { + let mut err_span = self.span; + + // Check to see if the type of this place can ever have a drop impl. If not, this + // `Drop` terminator is frivolous. + let ty_needs_drop = dropped_place + .ty(self.body, self.tcx) + .ty + .needs_drop(self.tcx, self.param_env); + + if !ty_needs_drop { + return; + } + + let needs_drop = if let Place { + base: PlaceBase::Local(local), + projection: box [], + } = *dropped_place { + // Use the span where the local was declared as the span of the drop error. + err_span = self.body.local_decls[local].source_info.span; + self.qualifs.needs_drop.contains(local) + } else { + true + }; + + if needs_drop { + self.check_op_spanned(ops::LiveDrop, err_span); + } + } + + _ => {} + } + } +} + +/// All implementers of `NonConstOp`. +pub mod ops { + use super::*; + + /// A `Downcast` projection. + #[derive(Debug)] + pub struct Downcast; + impl NonConstOp for Downcast {} + + /// A function call where the callee is a pointer. + #[derive(Debug)] + pub struct FnCallIndirect; + impl NonConstOp for FnCallIndirect { + const IS_SUPPORTED_IN_MIRI: bool = false; + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let mut err = item.tcx.sess.struct_span_err( + span, + &format!("function pointers are not allowed in const fn")); + err.emit(); + } + } + + /// A function call where the callee is not marked as `const`. + #[derive(Debug)] + pub struct FnCallNonConst(pub DefId); + impl NonConstOp for FnCallNonConst { + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let mut err = struct_span_err!( + item.tcx.sess, + span, + E0015, + "calls in {}s are limited to constant functions, \ + tuple structs and tuple variants", + item.mode, + ); + err.emit(); + } + } + + /// A function call where the callee is not a function definition or function pointer, e.g. a + /// closure. + /// + /// This can be subdivided in the future to produce a better error message. + #[derive(Debug)] + pub struct FnCallOther; + impl NonConstOp for FnCallOther { + const IS_SUPPORTED_IN_MIRI: bool = false; + } + + /// A call to a `#[unstable]` const fn or `#[rustc_const_unstable]` function. + /// + /// Contains the name of the feature that would allow the use of this function. + #[derive(Debug)] + pub struct FnCallUnstable(pub DefId, pub Symbol); + impl NonConstOp for FnCallUnstable { + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let FnCallUnstable(def_id, feature) = *self; + + let mut err = item.tcx.sess.struct_span_err(span, + &format!("`{}` is not yet stable as a const fn", + item.tcx.def_path_str(def_id))); + if nightly_options::is_nightly_build() { + help!(&mut err, + "add `#![feature({})]` to the \ + crate attributes to enable", + feature); + } + err.emit(); + } + } + + #[derive(Debug)] + pub struct HeapAllocation; + impl NonConstOp for HeapAllocation { + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let mut err = struct_span_err!(item.tcx.sess, span, E0010, + "allocations are not allowed in {}s", item.mode); + err.span_label(span, format!("allocation not allowed in {}s", item.mode)); + if item.tcx.sess.teach(&err.get_code().unwrap()) { + err.note( + "The value of statics and constants must be known at compile time, \ + and they live for the entire lifetime of a program. Creating a boxed \ + value allocates memory on the heap at runtime, and therefore cannot \ + be done at compile time." + ); + } + err.emit(); + } + } + + #[derive(Debug)] + pub struct IfOrMatch; + impl NonConstOp for IfOrMatch {} + + #[derive(Debug)] + pub struct LiveDrop; + impl NonConstOp for LiveDrop { + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + struct_span_err!(item.tcx.sess, span, E0493, + "destructors cannot be evaluated at compile-time") + .span_label(span, format!("{}s cannot evaluate destructors", + item.mode)) + .emit(); + } + } + + #[derive(Debug)] + pub struct Loop; + impl NonConstOp for Loop {} + + #[derive(Debug)] + pub struct MutBorrow(pub BorrowKind); + impl NonConstOp for MutBorrow { + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let kind = self.0; + if let BorrowKind::Mut { .. } = kind { + let mut err = struct_span_err!(item.tcx.sess, span, E0017, + "references in {}s may only refer \ + to immutable values", item.mode); + err.span_label(span, format!("{}s require immutable values", + item.mode)); + if item.tcx.sess.teach(&err.get_code().unwrap()) { + err.note("References in statics and constants may only refer \ + to immutable values.\n\n\ + Statics are shared everywhere, and if they refer to \ + mutable data one might violate memory safety since \ + holding multiple mutable references to shared data \ + is not allowed.\n\n\ + If you really want global mutable state, try using \ + static mut or a global UnsafeCell."); + } + err.emit(); + } else { + span_err!(item.tcx.sess, span, E0492, + "cannot borrow a constant which may contain \ + interior mutability, create a static instead"); + } + } + } + + #[derive(Debug)] + pub struct MutDeref; + impl NonConstOp for MutDeref {} + + #[derive(Debug)] + pub struct Panic; + impl NonConstOp for Panic { + fn feature_gate(tcx: TyCtxt<'_>) -> Option { + Some(tcx.features().const_panic) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + emit_feature_err( + &item.tcx.sess.parse_sess, + sym::const_panic, + span, + GateIssue::Language, + &format!("panicking in {}s is unstable", item.mode), + ); + } + } + + #[derive(Debug)] + pub struct RawPtrComparison; + impl NonConstOp for RawPtrComparison { + fn feature_gate(tcx: TyCtxt<'_>) -> Option { + Some(tcx.features().const_compare_raw_pointers) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + emit_feature_err( + &item.tcx.sess.parse_sess, + sym::const_compare_raw_pointers, + span, + GateIssue::Language, + &format!("comparing raw pointers inside {}", item.mode), + ); + } + } + + #[derive(Debug)] + pub struct RawPtrDeref; + impl NonConstOp for RawPtrDeref { + fn feature_gate(tcx: TyCtxt<'_>) -> Option { + Some(tcx.features().const_raw_ptr_deref) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + emit_feature_err( + &item.tcx.sess.parse_sess, sym::const_raw_ptr_deref, + span, GateIssue::Language, + &format!( + "dereferencing raw pointers in {}s is unstable", + item.mode, + ), + ); + } + } + + #[derive(Debug)] + pub struct RawPtrToIntCast; + impl NonConstOp for RawPtrToIntCast { + fn feature_gate(tcx: TyCtxt<'_>) -> Option { + Some(tcx.features().const_raw_ptr_to_usize_cast) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + emit_feature_err( + &item.tcx.sess.parse_sess, sym::const_raw_ptr_to_usize_cast, + span, GateIssue::Language, + &format!( + "casting pointers to integers in {}s is unstable", + item.mode, + ), + ); + } + } + + /// An access to a (non-thread-local) `static`. + #[derive(Debug)] + pub struct StaticAccess; + impl NonConstOp for StaticAccess { + fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { + item.mode.is_static() + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let mut err = struct_span_err!(item.tcx.sess, span, E0013, + "{}s cannot refer to statics, use \ + a constant instead", item.mode); + if item.tcx.sess.teach(&err.get_code().unwrap()) { + err.note( + "Static and const variables can refer to other const variables. \ + But a const variable cannot refer to a static variable." + ); + err.help( + "To fix this, the value can be extracted as a const and then used." + ); + } + err.emit(); + } + } + + /// An access to a thread-local `static`. + #[derive(Debug)] + pub struct ThreadLocalAccess; + impl NonConstOp for ThreadLocalAccess { + const IS_SUPPORTED_IN_MIRI: bool = false; + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + span_err!(item.tcx.sess, span, E0625, + "thread-local statics cannot be \ + accessed at compile-time"); + } + } + + #[derive(Debug)] + pub struct Transmute; + impl NonConstOp for Transmute { + fn feature_gate(tcx: TyCtxt<'_>) -> Option { + Some(tcx.features().const_transmute) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + emit_feature_err( + &item.tcx.sess.parse_sess, sym::const_transmute, + span, GateIssue::Language, + &format!("The use of std::mem::transmute() \ + is gated in {}s", item.mode)); + } + } + + #[derive(Debug)] + pub struct UnionAccess; + impl NonConstOp for UnionAccess { + fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { + // Union accesses are stable in all contexts except `const fn`. + item.mode != Mode::ConstFn || Self::feature_gate(item.tcx).unwrap() + } + + fn feature_gate(tcx: TyCtxt<'_>) -> Option { + Some(tcx.features().const_fn_union) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + emit_feature_err( + &item.tcx.sess.parse_sess, sym::const_fn_union, + span, GateIssue::Language, + "unions in const fn are unstable", + ); + } + } +} diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 0da1f3a1affd1..5037c791cb688 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -15,6 +15,7 @@ use syntax_pos::Span; pub mod add_retag; pub mod add_moves_for_packed_drops; pub mod cleanup_post_borrowck; +pub mod check_consts; pub mod check_unsafety; pub mod simplify_branches; pub mod simplify; From c990243922ac37789d6b49a8ea8eea7da0a28993 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:41 -0700 Subject: [PATCH 13/31] Run new validator in compare mode --- src/librustc_mir/transform/qualify_consts.rs | 125 ++++++++++++++----- 1 file changed, 97 insertions(+), 28 deletions(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index cce2c9f5ee9c4..54cc63c929cc7 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -34,6 +34,7 @@ use std::usize; use rustc::hir::HirId; use crate::transform::{MirPass, MirSource}; use super::promote_consts::{self, Candidate, TempState}; +use crate::transform::check_consts::validation::{ops, NonConstOp}; /// What kind of item we are in. #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -673,12 +674,14 @@ struct Checker<'a, 'tcx> { temp_promotion_state: IndexVec, promotion_candidates: Vec, + errors: Vec<(Span, String)>, + suppress_errors: bool, } macro_rules! unleash_miri { ($this:expr) => {{ if $this.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you { - if $this.mode.requires_const_checking() { + if $this.mode.requires_const_checking() && !$this.suppress_errors { $this.tcx.sess.span_warn($this.span, "skipping const checks"); } return; @@ -736,16 +739,19 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { def_id, rpo, temp_promotion_state: temps, - promotion_candidates: vec![] + promotion_candidates: vec![], + errors: vec![], + suppress_errors: false, } } // FIXME(eddyb) we could split the errors into meaningful // categories, but enabling full miri would make that // slightly pointless (even with feature-gating). - fn not_const(&mut self) { + fn not_const(&mut self, op: impl NonConstOp + fmt::Debug) { unleash_miri!(self); - if self.mode.requires_const_checking() { + if self.mode.requires_const_checking() && !self.suppress_errors { + self.record_error(op); let mut err = struct_span_err!( self.tcx.sess, self.span, @@ -763,6 +769,14 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { } } + fn record_error(&mut self, op: impl NonConstOp + fmt::Debug) { + self.record_error_spanned(op, self.span); + } + + fn record_error_spanned(&mut self, op: impl NonConstOp + fmt::Debug, span: Span) { + self.errors.push((span, format!("{:?}", op))); + } + /// Assigns an rvalue/call qualification to the given destination. fn assign(&mut self, dest: &Place<'tcx>, source: ValueSource<'_, 'tcx>, location: Location) { trace!("assign: {:?} <- {:?}", dest, source); @@ -781,8 +795,10 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { qualifs[HasMutInterior] = false; qualifs[IsNotPromotable] = true; - if self.mode.requires_const_checking() { + debug!("suppress_errors: {}", self.suppress_errors); + if self.mode.requires_const_checking() && !self.suppress_errors { if !self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you { + self.record_error(ops::MutBorrow(kind)); if let BorrowKind::Mut { .. } = kind { let mut err = struct_span_err!(self.tcx.sess, self.span, E0017, "references in {}s may only refer \ @@ -927,8 +943,23 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { /// Check a whole const, static initializer or const fn. fn check_const(&mut self) -> (u8, &'tcx BitSet) { + use crate::transform::check_consts as new_checker; + debug!("const-checking {} {:?}", self.mode, self.def_id); + // FIXME: Also use the new validator when features that require it (e.g. `const_if`) are + // enabled. + let use_new_validator = self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you; + if use_new_validator { + debug!("Using dataflow-based const validator"); + } + + let item = new_checker::Item::new(self.tcx, self.def_id, self.body); + let mut validator = new_checker::validation::Validator::new(&item); + + validator.suppress_errors = !use_new_validator; + self.suppress_errors = use_new_validator; + let body = self.body; let mut seen_blocks = BitSet::new_empty(body.basic_blocks().len()); @@ -937,6 +968,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { seen_blocks.insert(bb.index()); self.visit_basic_block_data(bb, &body[bb]); + validator.visit_basic_block_data(bb, &body[bb]); let target = match body[bb].terminator().kind { TerminatorKind::Goto { target } | @@ -972,12 +1004,27 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { bb = target; } _ => { - self.not_const(); + self.not_const(ops::Loop); + validator.check_op(ops::Loop); break; } } } + // The new validation pass should agree with the old when running on simple const bodies + // (e.g. no `if` or `loop`). + if !use_new_validator { + let mut new_errors = validator.take_errors(); + + // FIXME: each checker sometimes emits the same error with the same span twice in a row. + self.errors.dedup(); + new_errors.dedup(); + if self.errors != new_errors { + error!("old validator: {:?}", self.errors); + error!("new validator: {:?}", new_errors); + panic!("disagreement between validators:"); + } + } // Collect all the temps we need to promote. let mut promoted_temps = BitSet::new_empty(self.temp_promotion_state.len()); @@ -1043,7 +1090,8 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { .get_attrs(*def_id) .iter() .any(|attr| attr.check_name(sym::thread_local)) { - if self.mode.requires_const_checking() { + if self.mode.requires_const_checking() && !self.suppress_errors { + self.record_error(ops::ThreadLocalAccess); span_err!(self.tcx.sess, self.span, E0625, "thread-local statics cannot be \ accessed at compile-time"); @@ -1053,7 +1101,10 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { // Only allow statics (not consts) to refer to other statics. if self.mode == Mode::Static || self.mode == Mode::StaticMut { - if self.mode == Mode::Static && context.is_mutating_use() { + if self.mode == Mode::Static + && context.is_mutating_use() + && !self.suppress_errors + { // this is not strictly necessary as miri will also bail out // For interior mutability we can't really catch this statically as that // goes through raw pointers and intermediate temporaries, so miri has @@ -1067,7 +1118,8 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { } unleash_miri!(self); - if self.mode.requires_const_checking() { + if self.mode.requires_const_checking() && !self.suppress_errors { + self.record_error(ops::StaticAccess); let mut err = struct_span_err!(self.tcx.sess, self.span, E0013, "{}s cannot refer to statics, use \ a constant instead", self.mode); @@ -1104,14 +1156,16 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { ProjectionElem::Deref => { if context.is_mutating_use() { // `not_const` errors out in const contexts - self.not_const() + self.not_const(ops::MutDeref) } let base_ty = Place::ty_from(place_base, proj_base, self.body, self.tcx).ty; match self.mode { - Mode::NonConstFn => {}, + Mode::NonConstFn => {} + _ if self.suppress_errors => {} _ => { if let ty::RawPtr(_) = base_ty.kind { if !self.tcx.features().const_raw_ptr_deref { + self.record_error(ops::RawPtrDeref); emit_feature_err( &self.tcx.sess.parse_sess, sym::const_raw_ptr_deref, self.span, GateIssue::Language, @@ -1135,7 +1189,10 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { if def.is_union() { match self.mode { Mode::ConstFn => { - if !self.tcx.features().const_fn_union { + if !self.tcx.features().const_fn_union + && !self.suppress_errors + { + self.record_error(ops::UnionAccess); emit_feature_err( &self.tcx.sess.parse_sess, sym::const_fn_union, self.span, GateIssue::Language, @@ -1155,7 +1212,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { } ProjectionElem::Downcast(..) => { - self.not_const() + self.not_const(ops::Downcast) } } } @@ -1241,9 +1298,12 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { (CastTy::Ptr(_), CastTy::Int(_)) | (CastTy::FnPtr, CastTy::Int(_)) if self.mode != Mode::NonConstFn => { unleash_miri!(self); - if !self.tcx.features().const_raw_ptr_to_usize_cast { + if !self.tcx.features().const_raw_ptr_to_usize_cast + && !self.suppress_errors + { // in const fn and constants require the feature gate // FIXME: make it unsafe inside const fn and constants + self.record_error(ops::RawPtrToIntCast); emit_feature_err( &self.tcx.sess.parse_sess, sym::const_raw_ptr_to_usize_cast, self.span, GateIssue::Language, @@ -1267,8 +1327,10 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { unleash_miri!(self); if self.mode.requires_const_checking() && - !self.tcx.features().const_compare_raw_pointers + !self.tcx.features().const_compare_raw_pointers && + !self.suppress_errors { + self.record_error(ops::RawPtrComparison); // require the feature gate inside constants and const fn // FIXME: make it unsafe to use these operations emit_feature_err( @@ -1284,7 +1346,8 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { Rvalue::NullaryOp(NullOp::Box, _) => { unleash_miri!(self); - if self.mode.requires_const_checking() { + if self.mode.requires_const_checking() && !self.suppress_errors { + self.record_error(ops::HeapAllocation); let mut err = struct_span_err!(self.tcx.sess, self.span, E0010, "allocations are not allowed in {}s", self.mode); err.span_label(self.span, format!("allocation not allowed in {}s", self.mode)); @@ -1329,9 +1392,12 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { // special intrinsic that can be called diretly without an intrinsic // feature gate needs a language feature gate "transmute" => { - if self.mode.requires_const_checking() { + if self.mode.requires_const_checking() + && !self.suppress_errors + { // const eval transmute calls only with the feature gate if !self.tcx.features().const_transmute { + self.record_error(ops::Transmute); emit_feature_err( &self.tcx.sess.parse_sess, sym::const_transmute, self.span, GateIssue::Language, @@ -1359,7 +1425,10 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { .opts .debugging_opts .unleash_the_miri_inside_of_you; - if self.tcx.is_const_fn(def_id) || unleash_miri { + if self.tcx.is_const_fn(def_id) + || unleash_miri + || self.suppress_errors + { // stable const fns or unstable const fns // with their feature gate active // FIXME(eddyb) move stability checks from `is_const_fn` here. @@ -1370,6 +1439,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { // since the macro is marked with the attribute. if !self.tcx.features().const_panic { // Don't allow panics in constants without the feature gate. + self.record_error(ops::Panic); emit_feature_err( &self.tcx.sess.parse_sess, sym::const_panic, @@ -1384,6 +1454,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { // functions without the feature gate active in this crate in // order to report a better error message than the one below. if !self.span.allows_unstable(feature) { + self.record_error(ops::FnCallUnstable(def_id, feature)); let mut err = self.tcx.sess.struct_span_err(self.span, &format!("`{}` is not yet stable as a const fn", self.tcx.def_path_str(def_id))); @@ -1396,6 +1467,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { err.emit(); } } else { + self.record_error(ops::FnCallNonConst(def_id)); let mut err = struct_span_err!( self.tcx.sess, self.span, @@ -1411,13 +1483,9 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { } } ty::FnPtr(_) => { - let unleash_miri = self - .tcx - .sess - .opts - .debugging_opts - .unleash_the_miri_inside_of_you; - if self.mode.requires_const_checking() && !unleash_miri { + unleash_miri!(self); + if self.mode.requires_const_checking() && !self.suppress_errors { + self.record_error(ops::FnCallIndirect); let mut err = self.tcx.sess.struct_span_err( self.span, "function pointers are not allowed in const fn" @@ -1426,7 +1494,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { } } _ => { - self.not_const(); + self.not_const(ops::FnCallOther); } } @@ -1484,7 +1552,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { } // Deny *any* live drops anywhere other than functions. - if self.mode.requires_const_checking() { + if self.mode.requires_const_checking() && !self.suppress_errors { unleash_miri!(self); // HACK(eddyb): emulate a bit of dataflow analysis, // conservatively, that drop elaboration will do. @@ -1505,6 +1573,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { // Double-check the type being dropped, to minimize false positives. let ty = place.ty(self.body, self.tcx).ty; if ty.needs_drop(self.tcx, self.param_env) { + self.record_error_spanned(ops::LiveDrop, span); struct_span_err!(self.tcx.sess, span, E0493, "destructors cannot be evaluated at compile-time") .span_label(span, format!("{}s cannot evaluate destructors", @@ -1549,7 +1618,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { self.super_statement(statement, location); } StatementKind::FakeRead(FakeReadCause::ForMatchedPlace, _) => { - self.not_const(); + self.not_const(ops::IfOrMatch); } // FIXME(eddyb) should these really do nothing? StatementKind::FakeRead(..) | From 670c84dde3ad25f113b00436066a65a0dfb808cd Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 17 Sep 2019 16:25:42 -0700 Subject: [PATCH 14/31] Fix tests broken by more consistent miri unleashed warnings --- .../ui/consts/const-eval/const_fn_ptr.stderr | 144 ++---------------- .../ui/consts/const-eval/const_fn_ptr_fail.rs | 1 + .../const-eval/const_fn_ptr_fail.stderr | 6 + .../consts/const-eval/const_fn_ptr_fail2.rs | 8 +- .../const-eval/const_fn_ptr_fail2.stderr | 50 +----- .../consts/miri_unleashed/assoc_const.stderr | 4 +- .../miri_unleashed/enum_discriminants.stderr | 48 ------ .../ui/consts/miri_unleashed/mutable_const.rs | 1 + .../miri_unleashed/mutable_const.stderr | 10 +- .../miri_unleashed/mutable_references.rs | 6 + .../miri_unleashed/mutable_references.stderr | 38 ++++- .../miri_unleashed/mutable_references_ice.rs | 2 +- .../mutable_references_ice.stderr | 6 + 13 files changed, 82 insertions(+), 242 deletions(-) create mode 100644 src/test/ui/consts/const-eval/const_fn_ptr_fail.stderr diff --git a/src/test/ui/consts/const-eval/const_fn_ptr.stderr b/src/test/ui/consts/const-eval/const_fn_ptr.stderr index 41452ee59eb94..2fbb19322442b 100644 --- a/src/test/ui/consts/const-eval/const_fn_ptr.stderr +++ b/src/test/ui/consts/const-eval/const_fn_ptr.stderr @@ -1,146 +1,20 @@ warning: skipping const checks - --> $DIR/const_fn_ptr.rs:25:5 + --> $DIR/const_fn_ptr.rs:12:5 | -LL | assert_eq!(Y, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:25:5 - | -LL | assert_eq!(Y, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:25:5 - | -LL | assert_eq!(Y, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:27:5 - | -LL | assert_eq!(y, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:27:5 - | -LL | assert_eq!(y, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:27:5 - | -LL | assert_eq!(y, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:29:5 - | -LL | assert_eq!(y, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:29:5 - | -LL | assert_eq!(y, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) +LL | X(x) + | ^^^^ warning: skipping const checks - --> $DIR/const_fn_ptr.rs:29:5 - | -LL | assert_eq!(y, 4); - | ^^^^^^^^^^^^^^^^^ + --> $DIR/const_fn_ptr.rs:16:5 | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) +LL | X_const(x) + | ^^^^^^^^^^ warning: skipping const checks - --> $DIR/const_fn_ptr.rs:32:5 - | -LL | assert_eq!(Z, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:32:5 - | -LL | assert_eq!(Z, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:32:5 - | -LL | assert_eq!(Z, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:34:5 - | -LL | assert_eq!(z, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:34:5 - | -LL | assert_eq!(z, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:34:5 - | -LL | assert_eq!(z, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:36:5 - | -LL | assert_eq!(z, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:36:5 - | -LL | assert_eq!(z, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr.rs:36:5 - | -LL | assert_eq!(z, 4); - | ^^^^^^^^^^^^^^^^^ + --> $DIR/const_fn_ptr.rs:20:5 | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) +LL | x(y) + | ^^^^ warning: constant `X_const` should have an upper case name --> $DIR/const_fn_ptr.rs:9:7 diff --git a/src/test/ui/consts/const-eval/const_fn_ptr_fail.rs b/src/test/ui/consts/const-eval/const_fn_ptr_fail.rs index 14bd6558e7f89..90d3cba07a598 100644 --- a/src/test/ui/consts/const-eval/const_fn_ptr_fail.rs +++ b/src/test/ui/consts/const-eval/const_fn_ptr_fail.rs @@ -8,6 +8,7 @@ const X: fn(usize) -> usize = double; const fn bar(x: usize) -> usize { X(x) // FIXME: this should error someday + //~^ WARN: skipping const checks } fn main() {} diff --git a/src/test/ui/consts/const-eval/const_fn_ptr_fail.stderr b/src/test/ui/consts/const-eval/const_fn_ptr_fail.stderr new file mode 100644 index 0000000000000..e80f363ff8be4 --- /dev/null +++ b/src/test/ui/consts/const-eval/const_fn_ptr_fail.stderr @@ -0,0 +1,6 @@ +warning: skipping const checks + --> $DIR/const_fn_ptr_fail.rs:10:5 + | +LL | X(x) // FIXME: this should error someday + | ^^^^ + diff --git a/src/test/ui/consts/const-eval/const_fn_ptr_fail2.rs b/src/test/ui/consts/const-eval/const_fn_ptr_fail2.rs index 74c60f9a2a58d..b300119509ce4 100644 --- a/src/test/ui/consts/const-eval/const_fn_ptr_fail2.rs +++ b/src/test/ui/consts/const-eval/const_fn_ptr_fail2.rs @@ -6,7 +6,7 @@ fn double(x: usize) -> usize { x * 2 } const X: fn(usize) -> usize = double; const fn bar(x: fn(usize) -> usize, y: usize) -> usize { - x(y) + x(y) //~ WARN skipping const checks } const Y: usize = bar(X, 2); // FIXME: should fail to typeck someday @@ -15,12 +15,6 @@ const Z: usize = bar(double, 2); // FIXME: should fail to typeck someday fn main() { assert_eq!(Y, 4); //~^ ERROR evaluation of constant expression failed - //~^^ WARN skipping const checks - //~^^^ WARN skipping const checks - //~^^^^ WARN skipping const checks assert_eq!(Z, 4); //~^ ERROR evaluation of constant expression failed - //~^^ WARN skipping const checks - //~^^^ WARN skipping const checks - //~^^^^ WARN skipping const checks } diff --git a/src/test/ui/consts/const-eval/const_fn_ptr_fail2.stderr b/src/test/ui/consts/const-eval/const_fn_ptr_fail2.stderr index 611cc5313c057..9d74d3b0bf229 100644 --- a/src/test/ui/consts/const-eval/const_fn_ptr_fail2.stderr +++ b/src/test/ui/consts/const-eval/const_fn_ptr_fail2.stderr @@ -1,50 +1,8 @@ warning: skipping const checks - --> $DIR/const_fn_ptr_fail2.rs:16:5 - | -LL | assert_eq!(Y, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr_fail2.rs:16:5 - | -LL | assert_eq!(Y, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr_fail2.rs:16:5 - | -LL | assert_eq!(Y, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr_fail2.rs:21:5 - | -LL | assert_eq!(Z, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr_fail2.rs:21:5 - | -LL | assert_eq!(Z, 4); - | ^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/const_fn_ptr_fail2.rs:21:5 - | -LL | assert_eq!(Z, 4); - | ^^^^^^^^^^^^^^^^^ + --> $DIR/const_fn_ptr_fail2.rs:9:5 | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) +LL | x(y) + | ^^^^ error[E0080]: evaluation of constant expression failed --> $DIR/const_fn_ptr_fail2.rs:16:5 @@ -57,7 +15,7 @@ LL | assert_eq!(Y, 4); = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error[E0080]: evaluation of constant expression failed - --> $DIR/const_fn_ptr_fail2.rs:21:5 + --> $DIR/const_fn_ptr_fail2.rs:18:5 | LL | assert_eq!(Z, 4); | ^^^^^^^^^^^-^^^^^ diff --git a/src/test/ui/consts/miri_unleashed/assoc_const.stderr b/src/test/ui/consts/miri_unleashed/assoc_const.stderr index e814303923e18..6a6cb343f17d5 100644 --- a/src/test/ui/consts/miri_unleashed/assoc_const.stderr +++ b/src/test/ui/consts/miri_unleashed/assoc_const.stderr @@ -1,8 +1,8 @@ warning: skipping const checks - --> $DIR/assoc_const.rs:12:31 + --> $DIR/assoc_const.rs:12:20 | LL | const F: u32 = (U::X, 42).1; - | ^ + | ^^^^^^^^^^ error[E0080]: erroneous constant used --> $DIR/assoc_const.rs:29:13 diff --git a/src/test/ui/consts/miri_unleashed/enum_discriminants.stderr b/src/test/ui/consts/miri_unleashed/enum_discriminants.stderr index 8ca81ad22b72b..df366ba22e4d1 100644 --- a/src/test/ui/consts/miri_unleashed/enum_discriminants.stderr +++ b/src/test/ui/consts/miri_unleashed/enum_discriminants.stderr @@ -22,51 +22,3 @@ warning: skipping const checks LL | if let E1::V2 { .. } = (E1::V1 { f: true }) { | ^^^^^^^^^^^^^ -warning: skipping const checks - --> $DIR/enum_discriminants.rs:108:5 - | -LL | assert_eq!(OVERFLOW, 0); - | ^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/enum_discriminants.rs:108:5 - | -LL | assert_eq!(OVERFLOW, 0); - | ^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/enum_discriminants.rs:108:5 - | -LL | assert_eq!(OVERFLOW, 0); - | ^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/enum_discriminants.rs:109:5 - | -LL | assert_eq!(MORE_OVERFLOW, 0); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/enum_discriminants.rs:109:5 - | -LL | assert_eq!(MORE_OVERFLOW, 0); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -warning: skipping const checks - --> $DIR/enum_discriminants.rs:109:5 - | -LL | assert_eq!(MORE_OVERFLOW, 0); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - diff --git a/src/test/ui/consts/miri_unleashed/mutable_const.rs b/src/test/ui/consts/miri_unleashed/mutable_const.rs index b476e04529a52..44b408494679c 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_const.rs +++ b/src/test/ui/consts/miri_unleashed/mutable_const.rs @@ -7,6 +7,7 @@ use std::cell::UnsafeCell; // make sure we do not just intern this as mutable const MUTABLE_BEHIND_RAW: *mut i32 = &UnsafeCell::new(42) as *const _ as *mut _; +//~^ WARN: skipping const checks const MUTATING_BEHIND_RAW: () = { // Test that `MUTABLE_BEHIND_RAW` is actually immutable, by doing this at const time. diff --git a/src/test/ui/consts/miri_unleashed/mutable_const.stderr b/src/test/ui/consts/miri_unleashed/mutable_const.stderr index 507d4823a111d..757f0ffb59ff7 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_const.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_const.stderr @@ -1,11 +1,17 @@ warning: skipping const checks - --> $DIR/mutable_const.rs:14:9 + --> $DIR/mutable_const.rs:9:38 + | +LL | const MUTABLE_BEHIND_RAW: *mut i32 = &UnsafeCell::new(42) as *const _ as *mut _; + | ^^^^^^^^^^^^^^^^^^^^ + +warning: skipping const checks + --> $DIR/mutable_const.rs:15:9 | LL | *MUTABLE_BEHIND_RAW = 99 | ^^^^^^^^^^^^^^^^^^^^^^^^ error: any use of this value will cause an error - --> $DIR/mutable_const.rs:14:9 + --> $DIR/mutable_const.rs:15:9 | LL | / const MUTATING_BEHIND_RAW: () = { LL | | // Test that `MUTABLE_BEHIND_RAW` is actually immutable, by doing this at const time. diff --git a/src/test/ui/consts/miri_unleashed/mutable_references.rs b/src/test/ui/consts/miri_unleashed/mutable_references.rs index f81c78a8221a1..b2f3e0cdb287f 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_references.rs +++ b/src/test/ui/consts/miri_unleashed/mutable_references.rs @@ -6,12 +6,16 @@ use std::cell::UnsafeCell; // a test demonstrating what things we could allow with a smarter const qualification static FOO: &&mut u32 = &&mut 42; +//~^ WARN: skipping const checks +//~| WARN: skipping const checks static BAR: &mut () = &mut (); +//~^ WARN: skipping const checks struct Foo(T); static BOO: &mut Foo<()> = &mut Foo(()); +//~^ WARN: skipping const checks struct Meh { x: &'static UnsafeCell, @@ -21,9 +25,11 @@ unsafe impl Sync for Meh {} static MEH: Meh = Meh { x: &UnsafeCell::new(42), + //~^ WARN: skipping const checks }; static OH_YES: &mut i32 = &mut 42; +//~^ WARN: skipping const checks fn main() { unsafe { diff --git a/src/test/ui/consts/miri_unleashed/mutable_references.stderr b/src/test/ui/consts/miri_unleashed/mutable_references.stderr index 12840e00df9b3..ba01d15572501 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_references.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_references.stderr @@ -1,5 +1,41 @@ +warning: skipping const checks + --> $DIR/mutable_references.rs:8:26 + | +LL | static FOO: &&mut u32 = &&mut 42; + | ^^^^^^^ + +warning: skipping const checks + --> $DIR/mutable_references.rs:8:25 + | +LL | static FOO: &&mut u32 = &&mut 42; + | ^^^^^^^^ + +warning: skipping const checks + --> $DIR/mutable_references.rs:12:23 + | +LL | static BAR: &mut () = &mut (); + | ^^^^^^^ + +warning: skipping const checks + --> $DIR/mutable_references.rs:17:28 + | +LL | static BOO: &mut Foo<()> = &mut Foo(()); + | ^^^^^^^^^^^^ + +warning: skipping const checks + --> $DIR/mutable_references.rs:27:8 + | +LL | x: &UnsafeCell::new(42), + | ^^^^^^^^^^^^^^^^^^^^ + +warning: skipping const checks + --> $DIR/mutable_references.rs:31:27 + | +LL | static OH_YES: &mut i32 = &mut 42; + | ^^^^^^^ + error[E0594]: cannot assign to `*OH_YES`, as `OH_YES` is an immutable static item - --> $DIR/mutable_references.rs:32:5 + --> $DIR/mutable_references.rs:38:5 | LL | *OH_YES = 99; | ^^^^^^^^^^^^ cannot assign diff --git a/src/test/ui/consts/miri_unleashed/mutable_references_ice.rs b/src/test/ui/consts/miri_unleashed/mutable_references_ice.rs index b051f6b59c46e..635cad81c9798 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_references_ice.rs +++ b/src/test/ui/consts/miri_unleashed/mutable_references_ice.rs @@ -19,7 +19,7 @@ unsafe impl Sync for Meh {} // the following will never be ok! const MUH: Meh = Meh { - x: &UnsafeCell::new(42), + x: &UnsafeCell::new(42), //~ WARN: skipping const checks }; fn main() { diff --git a/src/test/ui/consts/miri_unleashed/mutable_references_ice.stderr b/src/test/ui/consts/miri_unleashed/mutable_references_ice.stderr index 24c9de5ee2eca..c148842bcbc66 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_references_ice.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_references_ice.stderr @@ -1,3 +1,9 @@ +warning: skipping const checks + --> $DIR/mutable_references_ice.rs:22:8 + | +LL | x: &UnsafeCell::new(42), + | ^^^^^^^^^^^^^^^^^^^^ + thread 'rustc' panicked at 'assertion failed: `(left != right)` left: `Const`, right: `Const`: UnsafeCells are not allowed behind references in constants. This should have been prevented statically by const qualification. If this were allowed one would be able to change a constant at one use site and other use sites could observe that mutation.', src/librustc_mir/interpret/intern.rs:LL:CC From 27bd84916c857f84d1562874e660cbb8f2d4aa2c Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 20 Sep 2019 08:48:47 -0700 Subject: [PATCH 15/31] Correct list of miri-supported operations Heap allocations are out, indirect `fn` calls are in! --- src/librustc_mir/transform/check_consts/validation.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index eee8f4856fd8d..6a8ac3f30854e 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -636,8 +636,6 @@ pub mod ops { #[derive(Debug)] pub struct FnCallIndirect; impl NonConstOp for FnCallIndirect { - const IS_SUPPORTED_IN_MIRI: bool = false; - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { let mut err = item.tcx.sess.struct_span_err( span, @@ -698,6 +696,8 @@ pub mod ops { #[derive(Debug)] pub struct HeapAllocation; impl NonConstOp for HeapAllocation { + const IS_SUPPORTED_IN_MIRI: bool = false; + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { let mut err = struct_span_err!(item.tcx.sess, span, E0010, "allocations are not allowed in {}s", item.mode); From f2ff425622183ff403e406fd472460cf4af142de Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 20 Sep 2019 08:49:46 -0700 Subject: [PATCH 16/31] Add rationale for `suppress_errors` flag --- src/librustc_mir/transform/qualify_consts.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 54cc63c929cc7..bb9b11656ca2d 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -674,8 +674,10 @@ struct Checker<'a, 'tcx> { temp_promotion_state: IndexVec, promotion_candidates: Vec, - errors: Vec<(Span, String)>, + + /// If `true`, do not emit errors to the user, merely collect them in `errors`. suppress_errors: bool, + errors: Vec<(Span, String)>, } macro_rules! unleash_miri { From e296436ee0ff782491993892a85fc261baeef769 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 20 Sep 2019 08:50:39 -0700 Subject: [PATCH 17/31] Remember to replace ICE with some form of warning --- src/librustc_mir/transform/qualify_consts.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index bb9b11656ca2d..1a09a2ddb3457 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -1021,10 +1021,12 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { // FIXME: each checker sometimes emits the same error with the same span twice in a row. self.errors.dedup(); new_errors.dedup(); + + // FIXME: Downgrade this to a warning. if self.errors != new_errors { error!("old validator: {:?}", self.errors); error!("new validator: {:?}", new_errors); - panic!("disagreement between validators:"); + panic!("disagreement between validators."); } } From b3e59bb291a0c0de8df0671c7acf87920c168aa5 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 20 Sep 2019 09:00:18 -0700 Subject: [PATCH 18/31] Move non-const ops into their own module --- .../transform/check_consts/mod.rs | 3 +- .../transform/check_consts/ops.rs | 337 ++++++++++++++++++ .../transform/check_consts/validation.rs | 337 +----------------- src/librustc_mir/transform/qualify_consts.rs | 2 +- 4 files changed, 342 insertions(+), 337 deletions(-) create mode 100644 src/librustc_mir/transform/check_consts/ops.rs diff --git a/src/librustc_mir/transform/check_consts/mod.rs b/src/librustc_mir/transform/check_consts/mod.rs index ce26af64741b8..3f26b4f0f45cc 100644 --- a/src/librustc_mir/transform/check_consts/mod.rs +++ b/src/librustc_mir/transform/check_consts/mod.rs @@ -4,8 +4,9 @@ use rustc::ty::{self, TyCtxt}; pub use self::qualifs::Qualif; -mod resolver; +pub mod ops; mod qualifs; +mod resolver; pub mod validation; /// Information about the item currently being validated, as well as a reference to the global diff --git a/src/librustc_mir/transform/check_consts/ops.rs b/src/librustc_mir/transform/check_consts/ops.rs new file mode 100644 index 0000000000000..31e981d2b5a9b --- /dev/null +++ b/src/librustc_mir/transform/check_consts/ops.rs @@ -0,0 +1,337 @@ +use rustc::hir::def_id::DefId; +use rustc::mir::BorrowKind; +use rustc::session::config::nightly_options; +use rustc::ty::TyCtxt; +use syntax::feature_gate::{emit_feature_err, GateIssue}; +use syntax::symbol::sym; +use syntax_pos::{Span, Symbol}; + +use super::Item; +use super::validation::Mode; + +/// An operation that is not *always* allowed in a const context. +pub trait NonConstOp { + /// Whether this operation can be evaluated by miri. + const IS_SUPPORTED_IN_MIRI: bool = true; + + /// Returns a boolean indicating whether the feature gate that would allow this operation is + /// enabled, or `None` if such a feature gate does not exist. + fn feature_gate(_tcx: TyCtxt<'tcx>) -> Option { + None + } + + /// Returns `true` if this operation is allowed in the given item. + /// + /// This check should assume that we are not in a non-const `fn`, where all operations are + /// legal. + fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { + Self::feature_gate(item.tcx).unwrap_or(false) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let mut err = struct_span_err!( + item.tcx.sess, + span, + E0019, + "{} contains unimplemented expression type", + item.mode + ); + if item.tcx.sess.teach(&err.get_code().unwrap()) { + err.note("A function call isn't allowed in the const's initialization expression \ + because the expression's value must be known at compile-time."); + err.note("Remember: you can't use a function call inside a const's initialization \ + expression! However, you can use it anywhere else."); + } + err.emit(); + } +} + +/// A `Downcast` projection. +#[derive(Debug)] +pub struct Downcast; +impl NonConstOp for Downcast {} + +/// A function call where the callee is a pointer. +#[derive(Debug)] +pub struct FnCallIndirect; +impl NonConstOp for FnCallIndirect { + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let mut err = item.tcx.sess.struct_span_err( + span, + &format!("function pointers are not allowed in const fn")); + err.emit(); + } +} + +/// A function call where the callee is not marked as `const`. +#[derive(Debug)] +pub struct FnCallNonConst(pub DefId); +impl NonConstOp for FnCallNonConst { + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let mut err = struct_span_err!( + item.tcx.sess, + span, + E0015, + "calls in {}s are limited to constant functions, \ + tuple structs and tuple variants", + item.mode, + ); + err.emit(); + } +} + +/// A function call where the callee is not a function definition or function pointer, e.g. a +/// closure. +/// +/// This can be subdivided in the future to produce a better error message. +#[derive(Debug)] +pub struct FnCallOther; +impl NonConstOp for FnCallOther { + const IS_SUPPORTED_IN_MIRI: bool = false; +} + +/// A call to a `#[unstable]` const fn or `#[rustc_const_unstable]` function. +/// +/// Contains the name of the feature that would allow the use of this function. +#[derive(Debug)] +pub struct FnCallUnstable(pub DefId, pub Symbol); +impl NonConstOp for FnCallUnstable { + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let FnCallUnstable(def_id, feature) = *self; + + let mut err = item.tcx.sess.struct_span_err(span, + &format!("`{}` is not yet stable as a const fn", + item.tcx.def_path_str(def_id))); + if nightly_options::is_nightly_build() { + help!(&mut err, + "add `#![feature({})]` to the \ + crate attributes to enable", + feature); + } + err.emit(); + } +} + +#[derive(Debug)] +pub struct HeapAllocation; +impl NonConstOp for HeapAllocation { + const IS_SUPPORTED_IN_MIRI: bool = false; + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let mut err = struct_span_err!(item.tcx.sess, span, E0010, + "allocations are not allowed in {}s", item.mode); + err.span_label(span, format!("allocation not allowed in {}s", item.mode)); + if item.tcx.sess.teach(&err.get_code().unwrap()) { + err.note( + "The value of statics and constants must be known at compile time, \ + and they live for the entire lifetime of a program. Creating a boxed \ + value allocates memory on the heap at runtime, and therefore cannot \ + be done at compile time." + ); + } + err.emit(); + } +} + +#[derive(Debug)] +pub struct IfOrMatch; +impl NonConstOp for IfOrMatch {} + +#[derive(Debug)] +pub struct LiveDrop; +impl NonConstOp for LiveDrop { + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + struct_span_err!(item.tcx.sess, span, E0493, + "destructors cannot be evaluated at compile-time") + .span_label(span, format!("{}s cannot evaluate destructors", + item.mode)) + .emit(); + } +} + +#[derive(Debug)] +pub struct Loop; +impl NonConstOp for Loop {} + +#[derive(Debug)] +pub struct MutBorrow(pub BorrowKind); +impl NonConstOp for MutBorrow { + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let kind = self.0; + if let BorrowKind::Mut { .. } = kind { + let mut err = struct_span_err!(item.tcx.sess, span, E0017, + "references in {}s may only refer \ + to immutable values", item.mode); + err.span_label(span, format!("{}s require immutable values", + item.mode)); + if item.tcx.sess.teach(&err.get_code().unwrap()) { + err.note("References in statics and constants may only refer \ + to immutable values.\n\n\ + Statics are shared everywhere, and if they refer to \ + mutable data one might violate memory safety since \ + holding multiple mutable references to shared data \ + is not allowed.\n\n\ + If you really want global mutable state, try using \ + static mut or a global UnsafeCell."); + } + err.emit(); + } else { + span_err!(item.tcx.sess, span, E0492, + "cannot borrow a constant which may contain \ + interior mutability, create a static instead"); + } + } +} + +#[derive(Debug)] +pub struct MutDeref; +impl NonConstOp for MutDeref {} + +#[derive(Debug)] +pub struct Panic; +impl NonConstOp for Panic { + fn feature_gate(tcx: TyCtxt<'_>) -> Option { + Some(tcx.features().const_panic) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + emit_feature_err( + &item.tcx.sess.parse_sess, + sym::const_panic, + span, + GateIssue::Language, + &format!("panicking in {}s is unstable", item.mode), + ); + } +} + +#[derive(Debug)] +pub struct RawPtrComparison; +impl NonConstOp for RawPtrComparison { + fn feature_gate(tcx: TyCtxt<'_>) -> Option { + Some(tcx.features().const_compare_raw_pointers) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + emit_feature_err( + &item.tcx.sess.parse_sess, + sym::const_compare_raw_pointers, + span, + GateIssue::Language, + &format!("comparing raw pointers inside {}", item.mode), + ); + } +} + +#[derive(Debug)] +pub struct RawPtrDeref; +impl NonConstOp for RawPtrDeref { + fn feature_gate(tcx: TyCtxt<'_>) -> Option { + Some(tcx.features().const_raw_ptr_deref) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + emit_feature_err( + &item.tcx.sess.parse_sess, sym::const_raw_ptr_deref, + span, GateIssue::Language, + &format!( + "dereferencing raw pointers in {}s is unstable", + item.mode, + ), + ); + } +} + +#[derive(Debug)] +pub struct RawPtrToIntCast; +impl NonConstOp for RawPtrToIntCast { + fn feature_gate(tcx: TyCtxt<'_>) -> Option { + Some(tcx.features().const_raw_ptr_to_usize_cast) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + emit_feature_err( + &item.tcx.sess.parse_sess, sym::const_raw_ptr_to_usize_cast, + span, GateIssue::Language, + &format!( + "casting pointers to integers in {}s is unstable", + item.mode, + ), + ); + } +} + +/// An access to a (non-thread-local) `static`. +#[derive(Debug)] +pub struct StaticAccess; +impl NonConstOp for StaticAccess { + fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { + item.mode.is_static() + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + let mut err = struct_span_err!(item.tcx.sess, span, E0013, + "{}s cannot refer to statics, use \ + a constant instead", item.mode); + if item.tcx.sess.teach(&err.get_code().unwrap()) { + err.note( + "Static and const variables can refer to other const variables. \ + But a const variable cannot refer to a static variable." + ); + err.help( + "To fix this, the value can be extracted as a const and then used." + ); + } + err.emit(); + } +} + +/// An access to a thread-local `static`. +#[derive(Debug)] +pub struct ThreadLocalAccess; +impl NonConstOp for ThreadLocalAccess { + const IS_SUPPORTED_IN_MIRI: bool = false; + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + span_err!(item.tcx.sess, span, E0625, + "thread-local statics cannot be \ + accessed at compile-time"); + } +} + +#[derive(Debug)] +pub struct Transmute; +impl NonConstOp for Transmute { + fn feature_gate(tcx: TyCtxt<'_>) -> Option { + Some(tcx.features().const_transmute) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + emit_feature_err( + &item.tcx.sess.parse_sess, sym::const_transmute, + span, GateIssue::Language, + &format!("The use of std::mem::transmute() \ + is gated in {}s", item.mode)); + } +} + +#[derive(Debug)] +pub struct UnionAccess; +impl NonConstOp for UnionAccess { + fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { + // Union accesses are stable in all contexts except `const fn`. + item.mode != Mode::ConstFn || Self::feature_gate(item.tcx).unwrap() + } + + fn feature_gate(tcx: TyCtxt<'_>) -> Option { + Some(tcx.features().const_fn_union) + } + + fn emit_error(&self, item: &Item<'_, '_>, span: Span) { + emit_feature_err( + &item.tcx.sess.parse_sess, sym::const_fn_union, + span, GateIssue::Language, + "unions in const fn are unstable", + ); + } +} diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 6a8ac3f30854e..c61fdf5b970e1 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -1,14 +1,12 @@ use rustc::hir::{self, def_id::DefId}; use rustc::mir::visit::{PlaceContext, Visitor, MutatingUseContext, NonMutatingUseContext}; use rustc::mir::*; -use rustc::session::config::nightly_options; use rustc::ty::cast::CastTy; use rustc::ty::{self, TyCtxt}; use rustc_data_structures::bit_set::BitSet; use rustc_target::spec::abi::Abi; -use syntax::feature_gate::{emit_feature_err, GateIssue}; use syntax::symbol::sym; -use syntax_pos::{Span, Symbol}; +use syntax_pos::Span; use std::cell::RefCell; use std::fmt; @@ -19,6 +17,7 @@ use crate::dataflow as old_dataflow; use super::{Item, Qualif, is_lang_panic_fn}; use super::resolver::{QualifResolver, FlowSensitiveResolver}; use super::qualifs::{HasMutInterior, NeedsDrop}; +use super::ops::{self, NonConstOp}; #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum CheckOpResult { @@ -91,43 +90,6 @@ impl fmt::Display for Mode { } } -/// An operation that is not *always* allowed in a const context. -pub trait NonConstOp { - /// Whether this operation can be evaluated by miri. - const IS_SUPPORTED_IN_MIRI: bool = true; - - /// Returns a boolean indicating whether the feature gate that would allow this operation is - /// enabled, or `None` if such a feature gate does not exist. - fn feature_gate(_tcx: TyCtxt<'tcx>) -> Option { - None - } - - /// Returns `true` if this operation is allowed in the given item. - /// - /// This check should assume that we are not in a non-const `fn`, where all operations are - /// legal. - fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { - Self::feature_gate(item.tcx).unwrap_or(false) - } - - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - let mut err = struct_span_err!( - item.tcx.sess, - span, - E0019, - "{} contains unimplemented expression type", - item.mode - ); - if item.tcx.sess.teach(&err.get_code().unwrap()) { - err.note("A function call isn't allowed in the const's initialization expression \ - because the expression's value must be known at compile-time."); - err.note("Remember: you can't use a function call inside a const's initialization \ - expression! However, you can use it anywhere else."); - } - err.emit(); - } -} - pub struct Qualifs<'a, 'mir, 'tcx> { has_mut_interior: FlowSensitiveResolver<'a, 'mir, 'tcx, HasMutInterior>, needs_drop: FlowSensitiveResolver<'a, 'mir, 'tcx, NeedsDrop>, @@ -622,298 +584,3 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { } } } - -/// All implementers of `NonConstOp`. -pub mod ops { - use super::*; - - /// A `Downcast` projection. - #[derive(Debug)] - pub struct Downcast; - impl NonConstOp for Downcast {} - - /// A function call where the callee is a pointer. - #[derive(Debug)] - pub struct FnCallIndirect; - impl NonConstOp for FnCallIndirect { - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - let mut err = item.tcx.sess.struct_span_err( - span, - &format!("function pointers are not allowed in const fn")); - err.emit(); - } - } - - /// A function call where the callee is not marked as `const`. - #[derive(Debug)] - pub struct FnCallNonConst(pub DefId); - impl NonConstOp for FnCallNonConst { - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - let mut err = struct_span_err!( - item.tcx.sess, - span, - E0015, - "calls in {}s are limited to constant functions, \ - tuple structs and tuple variants", - item.mode, - ); - err.emit(); - } - } - - /// A function call where the callee is not a function definition or function pointer, e.g. a - /// closure. - /// - /// This can be subdivided in the future to produce a better error message. - #[derive(Debug)] - pub struct FnCallOther; - impl NonConstOp for FnCallOther { - const IS_SUPPORTED_IN_MIRI: bool = false; - } - - /// A call to a `#[unstable]` const fn or `#[rustc_const_unstable]` function. - /// - /// Contains the name of the feature that would allow the use of this function. - #[derive(Debug)] - pub struct FnCallUnstable(pub DefId, pub Symbol); - impl NonConstOp for FnCallUnstable { - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - let FnCallUnstable(def_id, feature) = *self; - - let mut err = item.tcx.sess.struct_span_err(span, - &format!("`{}` is not yet stable as a const fn", - item.tcx.def_path_str(def_id))); - if nightly_options::is_nightly_build() { - help!(&mut err, - "add `#![feature({})]` to the \ - crate attributes to enable", - feature); - } - err.emit(); - } - } - - #[derive(Debug)] - pub struct HeapAllocation; - impl NonConstOp for HeapAllocation { - const IS_SUPPORTED_IN_MIRI: bool = false; - - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - let mut err = struct_span_err!(item.tcx.sess, span, E0010, - "allocations are not allowed in {}s", item.mode); - err.span_label(span, format!("allocation not allowed in {}s", item.mode)); - if item.tcx.sess.teach(&err.get_code().unwrap()) { - err.note( - "The value of statics and constants must be known at compile time, \ - and they live for the entire lifetime of a program. Creating a boxed \ - value allocates memory on the heap at runtime, and therefore cannot \ - be done at compile time." - ); - } - err.emit(); - } - } - - #[derive(Debug)] - pub struct IfOrMatch; - impl NonConstOp for IfOrMatch {} - - #[derive(Debug)] - pub struct LiveDrop; - impl NonConstOp for LiveDrop { - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - struct_span_err!(item.tcx.sess, span, E0493, - "destructors cannot be evaluated at compile-time") - .span_label(span, format!("{}s cannot evaluate destructors", - item.mode)) - .emit(); - } - } - - #[derive(Debug)] - pub struct Loop; - impl NonConstOp for Loop {} - - #[derive(Debug)] - pub struct MutBorrow(pub BorrowKind); - impl NonConstOp for MutBorrow { - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - let kind = self.0; - if let BorrowKind::Mut { .. } = kind { - let mut err = struct_span_err!(item.tcx.sess, span, E0017, - "references in {}s may only refer \ - to immutable values", item.mode); - err.span_label(span, format!("{}s require immutable values", - item.mode)); - if item.tcx.sess.teach(&err.get_code().unwrap()) { - err.note("References in statics and constants may only refer \ - to immutable values.\n\n\ - Statics are shared everywhere, and if they refer to \ - mutable data one might violate memory safety since \ - holding multiple mutable references to shared data \ - is not allowed.\n\n\ - If you really want global mutable state, try using \ - static mut or a global UnsafeCell."); - } - err.emit(); - } else { - span_err!(item.tcx.sess, span, E0492, - "cannot borrow a constant which may contain \ - interior mutability, create a static instead"); - } - } - } - - #[derive(Debug)] - pub struct MutDeref; - impl NonConstOp for MutDeref {} - - #[derive(Debug)] - pub struct Panic; - impl NonConstOp for Panic { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_panic) - } - - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - emit_feature_err( - &item.tcx.sess.parse_sess, - sym::const_panic, - span, - GateIssue::Language, - &format!("panicking in {}s is unstable", item.mode), - ); - } - } - - #[derive(Debug)] - pub struct RawPtrComparison; - impl NonConstOp for RawPtrComparison { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_compare_raw_pointers) - } - - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - emit_feature_err( - &item.tcx.sess.parse_sess, - sym::const_compare_raw_pointers, - span, - GateIssue::Language, - &format!("comparing raw pointers inside {}", item.mode), - ); - } - } - - #[derive(Debug)] - pub struct RawPtrDeref; - impl NonConstOp for RawPtrDeref { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_raw_ptr_deref) - } - - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - emit_feature_err( - &item.tcx.sess.parse_sess, sym::const_raw_ptr_deref, - span, GateIssue::Language, - &format!( - "dereferencing raw pointers in {}s is unstable", - item.mode, - ), - ); - } - } - - #[derive(Debug)] - pub struct RawPtrToIntCast; - impl NonConstOp for RawPtrToIntCast { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_raw_ptr_to_usize_cast) - } - - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - emit_feature_err( - &item.tcx.sess.parse_sess, sym::const_raw_ptr_to_usize_cast, - span, GateIssue::Language, - &format!( - "casting pointers to integers in {}s is unstable", - item.mode, - ), - ); - } - } - - /// An access to a (non-thread-local) `static`. - #[derive(Debug)] - pub struct StaticAccess; - impl NonConstOp for StaticAccess { - fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { - item.mode.is_static() - } - - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - let mut err = struct_span_err!(item.tcx.sess, span, E0013, - "{}s cannot refer to statics, use \ - a constant instead", item.mode); - if item.tcx.sess.teach(&err.get_code().unwrap()) { - err.note( - "Static and const variables can refer to other const variables. \ - But a const variable cannot refer to a static variable." - ); - err.help( - "To fix this, the value can be extracted as a const and then used." - ); - } - err.emit(); - } - } - - /// An access to a thread-local `static`. - #[derive(Debug)] - pub struct ThreadLocalAccess; - impl NonConstOp for ThreadLocalAccess { - const IS_SUPPORTED_IN_MIRI: bool = false; - - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - span_err!(item.tcx.sess, span, E0625, - "thread-local statics cannot be \ - accessed at compile-time"); - } - } - - #[derive(Debug)] - pub struct Transmute; - impl NonConstOp for Transmute { - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_transmute) - } - - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - emit_feature_err( - &item.tcx.sess.parse_sess, sym::const_transmute, - span, GateIssue::Language, - &format!("The use of std::mem::transmute() \ - is gated in {}s", item.mode)); - } - } - - #[derive(Debug)] - pub struct UnionAccess; - impl NonConstOp for UnionAccess { - fn is_allowed_in_item(&self, item: &Item<'_, '_>) -> bool { - // Union accesses are stable in all contexts except `const fn`. - item.mode != Mode::ConstFn || Self::feature_gate(item.tcx).unwrap() - } - - fn feature_gate(tcx: TyCtxt<'_>) -> Option { - Some(tcx.features().const_fn_union) - } - - fn emit_error(&self, item: &Item<'_, '_>, span: Span) { - emit_feature_err( - &item.tcx.sess.parse_sess, sym::const_fn_union, - span, GateIssue::Language, - "unions in const fn are unstable", - ); - } - } -} diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 1a09a2ddb3457..d9854237eeb8c 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -34,7 +34,7 @@ use std::usize; use rustc::hir::HirId; use crate::transform::{MirPass, MirSource}; use super::promote_consts::{self, Candidate, TempState}; -use crate::transform::check_consts::validation::{ops, NonConstOp}; +use crate::transform::check_consts::ops::{self, NonConstOp}; /// What kind of item we are in. #[derive(Copy, Clone, Debug, PartialEq, Eq)] From 93ee7791b60937b3dd163509c5ed631e1698e99b Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 20 Sep 2019 09:31:44 -0700 Subject: [PATCH 19/31] Explain why `visit_terminator` does nothing for `IndirectlyMutableLocals` --- src/librustc_mir/dataflow/impls/indirect_mutation.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc_mir/dataflow/impls/indirect_mutation.rs b/src/librustc_mir/dataflow/impls/indirect_mutation.rs index 934b843cc40b8..852bd316da3a9 100644 --- a/src/librustc_mir/dataflow/impls/indirect_mutation.rs +++ b/src/librustc_mir/dataflow/impls/indirect_mutation.rs @@ -133,6 +133,9 @@ impl<'tcx> Visitor<'tcx> for TransferFunction<'_, '_, 'tcx> { fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) { + // This method purposely does nothing except call `super_terminator`. It exists solely to + // document the subtleties around drop terminators. + self.super_terminator(terminator, location); if let mir::TerminatorKind::Drop { location: _, .. } From bc7928a5076616b88b2325b26d91d2547b9a4ac3 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 23 Sep 2019 21:33:58 -0700 Subject: [PATCH 20/31] Trigger ICE on nightly if validators disagree Also adds an unstable flag to disable the ICE (`-Zsuppress-const-validation-back-compat-ice`) so that nightly users do not have to revert to a previous nightly if their code causes disagreement between the validators. --- src/librustc/session/config.rs | 2 ++ src/librustc_mir/transform/qualify_consts.rs | 21 ++++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index cbb22f1e44830..7c97fd11af2a5 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1359,6 +1359,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "describes how to render the `rendered` field of json diagnostics"), unleash_the_miri_inside_of_you: bool = (false, parse_bool, [TRACKED], "take the breaks off const evaluation. NOTE: this is unsound"), + suppress_const_validation_back_compat_ice: bool = (false, parse_bool, [TRACKED], + "silence ICE triggered when the new const validator disagrees with the old"), osx_rpath_install_name: bool = (false, parse_bool, [TRACKED], "pass `-install_name @rpath/...` to the macOS linker"), sanitizer: Option = (None, parse_sanitizer, [TRACKED], diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index d9854237eeb8c..fef918b3298e0 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -1022,11 +1022,24 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { self.errors.dedup(); new_errors.dedup(); - // FIXME: Downgrade this to a warning. if self.errors != new_errors { error!("old validator: {:?}", self.errors); error!("new validator: {:?}", new_errors); - panic!("disagreement between validators."); + + // ICE on nightly if the validators do not emit exactly the same errors. + // Users can supress this panic with an unstable compiler flag (hopefully after + // filing an issue). + let opts = &self.tcx.sess.opts; + let trigger_ice = opts.unstable_features.is_nightly_build() + && !opts.debugging_opts.suppress_const_validation_back_compat_ice; + + if trigger_ice { + span_bug!( + body.span, + "{}", + VALIDATOR_MISMATCH_ERR, + ); + } } } @@ -1850,3 +1863,7 @@ fn args_required_const(tcx: TyCtxt<'_>, def_id: DefId) -> Option Date: Wed, 25 Sep 2019 11:47:56 -0700 Subject: [PATCH 21/31] Give usage instructions `IndirectlyMutableLocals` docs --- src/librustc_mir/dataflow/impls/indirect_mutation.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/dataflow/impls/indirect_mutation.rs b/src/librustc_mir/dataflow/impls/indirect_mutation.rs index 852bd316da3a9..353892820e3f4 100644 --- a/src/librustc_mir/dataflow/impls/indirect_mutation.rs +++ b/src/librustc_mir/dataflow/impls/indirect_mutation.rs @@ -10,9 +10,13 @@ use crate::dataflow::{self, GenKillSet}; /// indirectly. This could either be a mutable reference (`&mut`) or a shared borrow if the type of /// that `Local` allows interior mutability. /// -/// If this returns `false` for a `Local` at a given `Location`, the user can assume that `Local` -/// has not been mutated as a result of an indirect assignment (`*p = x`) or as a side-effect of a -/// function call or drop terminator. +/// If this returns false for a `Local` at the location of an indirect assignment, that `Local` +/// cannot be mutated by that assignment. For example, if the dataflow state at a statement like +/// `*p = 42` contained locals `x` and `z` but not `y`, we know that while `x` or `z` may be the +/// target of that assignment, `y` cannot be. +/// +/// Assignments through a pointer are not the only place where a `Local` can be mutated indirectly: +/// Function calls, drop terminators and inline assembly can all mutate `Local`s as a side-effect. #[derive(Copy, Clone)] pub struct IndirectlyMutableLocals<'mir, 'tcx> { body: &'mir mir::Body<'tcx>, From 2f5ea633d46abb9aedda22c96bc91889687a99f5 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 25 Sep 2019 11:52:12 -0700 Subject: [PATCH 22/31] Return a `bool` from `in_any_value_of_ty` The `Option` was only used for the promotion qualifiers, so we can use a simpler API for validation. --- .../transform/check_consts/qualifs.rs | 30 +++++++------------ .../transform/check_consts/resolver.rs | 2 +- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index 232a5401d73fa..ea25d6ff5b8d3 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -29,16 +29,8 @@ pub trait Qualif { const IS_CLEARED_ON_MOVE: bool = false; /// Return the qualification that is (conservatively) correct for any value - /// of the type, or `None` if the qualification is not value/type-based. - fn in_any_value_of_ty(_cx: &ConstCx<'_, 'tcx>, _ty: Ty<'tcx>) -> Option { - None - } - - /// Return a mask for the qualification, given a type. This is `false` iff - /// no value of that type can have the qualification. - fn mask_for_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> bool { - Self::in_any_value_of_ty(cx, ty).unwrap_or(true) - } + /// of the type. + fn in_any_value_of_ty(_cx: &ConstCx<'_, 'tcx>, _ty: Ty<'tcx>) -> bool; fn in_static(_cx: &ConstCx<'_, 'tcx>, _static: &Static<'tcx>) -> bool { // FIXME(eddyb) should we do anything here for value properties? @@ -55,7 +47,7 @@ pub trait Qualif { base: place.base, projection: proj_base, }); - let qualif = base_qualif && Self::mask_for_ty( + let qualif = base_qualif && Self::in_any_value_of_ty( cx, Place::ty_from(place.base, proj_base, cx.body, cx.tcx) .projection_ty(cx.tcx, elem) @@ -126,7 +118,7 @@ pub trait Qualif { if let ConstValue::Unevaluated(def_id, _) = constant.literal.val { // Don't peek inside trait associated constants. if cx.tcx.trait_of_item(def_id).is_some() { - Self::in_any_value_of_ty(cx, constant.literal.ty).unwrap_or(false) + Self::in_any_value_of_ty(cx, constant.literal.ty) } else { let (bits, _) = cx.tcx.at(constant.span).mir_const_qualif(def_id); @@ -135,7 +127,7 @@ pub trait Qualif { // Just in case the type is more specific than // the definition, e.g., impl associated const // with type parameters, take it into account. - qualif && Self::mask_for_ty(cx, constant.literal.ty) + qualif && Self::in_any_value_of_ty(cx, constant.literal.ty) } } else { false @@ -200,7 +192,7 @@ pub trait Qualif { return_ty: Ty<'tcx>, ) -> bool { // Be conservative about the returned value of a const fn. - Self::in_any_value_of_ty(cx, return_ty).unwrap_or(false) + Self::in_any_value_of_ty(cx, return_ty) } } @@ -214,8 +206,8 @@ pub struct HasMutInterior; impl Qualif for HasMutInterior { const IDX: usize = 0; - fn in_any_value_of_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> Option { - Some(!ty.is_freeze(cx.tcx, cx.param_env, DUMMY_SP)) + fn in_any_value_of_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> bool { + !ty.is_freeze(cx.tcx, cx.param_env, DUMMY_SP) } fn in_rvalue(cx: &ConstCx<'_, 'tcx>, per_local: &BitSet, rvalue: &Rvalue<'tcx>) -> bool { @@ -249,7 +241,7 @@ impl Qualif for HasMutInterior { if let AggregateKind::Adt(def, ..) = **kind { if Some(def.did) == cx.tcx.lang_items().unsafe_cell_type() { let ty = rvalue.ty(cx.body, cx.tcx); - assert_eq!(Self::in_any_value_of_ty(cx, ty), Some(true)); + assert_eq!(Self::in_any_value_of_ty(cx, ty), true); return true; } } @@ -272,8 +264,8 @@ impl Qualif for NeedsDrop { const IDX: usize = 1; const IS_CLEARED_ON_MOVE: bool = true; - fn in_any_value_of_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> Option { - Some(ty.needs_drop(cx.tcx, cx.param_env)) + fn in_any_value_of_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> bool { + ty.needs_drop(cx.tcx, cx.param_env) } fn in_rvalue(cx: &ConstCx<'_, 'tcx>, per_local: &BitSet, rvalue: &Rvalue<'tcx>) -> bool { diff --git a/src/librustc_mir/transform/check_consts/resolver.rs b/src/librustc_mir/transform/check_consts/resolver.rs index 4fa00bf098be4..2350382e66390 100644 --- a/src/librustc_mir/transform/check_consts/resolver.rs +++ b/src/librustc_mir/transform/check_consts/resolver.rs @@ -43,7 +43,7 @@ where for arg in self.item.body.args_iter() { let arg_ty = self.item.body.local_decls[arg].ty; - if Q::in_any_value_of_ty(self.item, arg_ty).unwrap() { + if Q::in_any_value_of_ty(self.item, arg_ty) { self.qualifs_per_local.insert(arg); } } From dcecefcb712c0fd714013fcdf8499f4d65f220b8 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 25 Sep 2019 11:56:31 -0700 Subject: [PATCH 23/31] Use conservative, type-based qualifcation for statics --- src/librustc_mir/transform/check_consts/qualifs.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index ea25d6ff5b8d3..8e0b49d157e09 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -32,9 +32,8 @@ pub trait Qualif { /// of the type. fn in_any_value_of_ty(_cx: &ConstCx<'_, 'tcx>, _ty: Ty<'tcx>) -> bool; - fn in_static(_cx: &ConstCx<'_, 'tcx>, _static: &Static<'tcx>) -> bool { - // FIXME(eddyb) should we do anything here for value properties? - false + fn in_static(cx: &ConstCx<'_, 'tcx>, statik: &Static<'tcx>) -> bool { + Self::in_any_value_of_ty(cx, statik.ty) } fn in_projection_structurally( From 1a14d17c4d7f8a5cc6a068355230c811f0fef19b Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 25 Sep 2019 11:58:12 -0700 Subject: [PATCH 24/31] Require `fmt::Debug` to implement `NonConstOp` --- src/librustc_mir/transform/check_consts/ops.rs | 2 +- src/librustc_mir/transform/qualify_consts.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/ops.rs b/src/librustc_mir/transform/check_consts/ops.rs index 31e981d2b5a9b..a05a13dc123da 100644 --- a/src/librustc_mir/transform/check_consts/ops.rs +++ b/src/librustc_mir/transform/check_consts/ops.rs @@ -10,7 +10,7 @@ use super::Item; use super::validation::Mode; /// An operation that is not *always* allowed in a const context. -pub trait NonConstOp { +pub trait NonConstOp: std::fmt::Debug { /// Whether this operation can be evaluated by miri. const IS_SUPPORTED_IN_MIRI: bool = true; diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index fef918b3298e0..5add7a64aabf6 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -750,7 +750,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { // FIXME(eddyb) we could split the errors into meaningful // categories, but enabling full miri would make that // slightly pointless (even with feature-gating). - fn not_const(&mut self, op: impl NonConstOp + fmt::Debug) { + fn not_const(&mut self, op: impl NonConstOp) { unleash_miri!(self); if self.mode.requires_const_checking() && !self.suppress_errors { self.record_error(op); @@ -771,11 +771,11 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { } } - fn record_error(&mut self, op: impl NonConstOp + fmt::Debug) { + fn record_error(&mut self, op: impl NonConstOp) { self.record_error_spanned(op, self.span); } - fn record_error_spanned(&mut self, op: impl NonConstOp + fmt::Debug, span: Span) { + fn record_error_spanned(&mut self, op: impl NonConstOp, span: Span) { self.errors.push((span, format!("{:?}", op))); } From 713ec152fcbe19dc3d75e41d7878b227477d46ba Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 25 Sep 2019 12:02:56 -0700 Subject: [PATCH 25/31] Share `IndirectlyMutableLocals` results via reference --- .../transform/check_consts/resolver.rs | 7 ++- .../transform/check_consts/validation.rs | 51 +++++++++++-------- src/librustc_mir/transform/qualify_consts.rs | 3 +- 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/resolver.rs b/src/librustc_mir/transform/check_consts/resolver.rs index 2350382e66390..52c471ad691e4 100644 --- a/src/librustc_mir/transform/check_consts/resolver.rs +++ b/src/librustc_mir/transform/check_consts/resolver.rs @@ -4,7 +4,6 @@ use rustc_data_structures::bit_set::BitSet; use std::cell::RefCell; use std::marker::PhantomData; -use std::rc::Rc; use crate::dataflow::{self as old_dataflow, generic as dataflow}; use super::{Item, Qualif}; @@ -164,7 +163,7 @@ pub trait QualifResolver { fn reset(&mut self); } -type IndirectlyMutableResults<'mir, 'tcx> = +pub type IndirectlyMutableResults<'mir, 'tcx> = old_dataflow::DataflowResultsCursor<'mir, 'tcx, IndirectlyMutableLocals<'mir, 'tcx>>; /// A resolver for qualifs that works on arbitrarily complex CFGs. @@ -181,7 +180,7 @@ where Q: Qualif, { location: Location, - indirectly_mutable_locals: Rc>>, + indirectly_mutable_locals: &'a RefCell>, cursor: dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'a, 'mir, 'tcx, Q>>, qualifs_per_local: BitSet, } @@ -193,7 +192,7 @@ where pub fn new( _: Q, item: &'a Item<'mir, 'tcx>, - indirectly_mutable_locals: Rc>>, + indirectly_mutable_locals: &'a RefCell>, dead_unwinds: &BitSet, ) -> Self { let analysis = FlowSensitiveAnalysis { diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index c61fdf5b970e1..74bf70f05e687 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -11,11 +11,10 @@ use syntax_pos::Span; use std::cell::RefCell; use std::fmt; use std::ops::Deref; -use std::rc::Rc; use crate::dataflow as old_dataflow; use super::{Item, Qualif, is_lang_panic_fn}; -use super::resolver::{QualifResolver, FlowSensitiveResolver}; +use super::resolver::{FlowSensitiveResolver, IndirectlyMutableResults, QualifResolver}; use super::qualifs::{HasMutInterior, NeedsDrop}; use super::ops::{self, NonConstOp}; @@ -127,37 +126,47 @@ impl Deref for Validator<'_, 'mir, 'tcx> { } } +pub fn compute_indirectly_mutable_locals<'mir, 'tcx>( + item: &Item<'mir, 'tcx>, +) -> RefCell> { + let dead_unwinds = BitSet::new_empty(item.body.basic_blocks().len()); + + let indirectly_mutable_locals = old_dataflow::do_dataflow( + item.tcx, + item.body, + item.def_id, + &[], + &dead_unwinds, + old_dataflow::IndirectlyMutableLocals::new(item.tcx, item.body, item.param_env), + |_, local| old_dataflow::DebugFormatted::new(&local), + ); + + let indirectly_mutable_locals = old_dataflow::DataflowResultsCursor::new( + indirectly_mutable_locals, + item.body, + ); + + RefCell::new(indirectly_mutable_locals) +} + impl Validator<'a, 'mir, 'tcx> { - pub fn new(item: &'a Item<'mir, 'tcx>) -> Self { + pub fn new( + item: &'a Item<'mir, 'tcx>, + indirectly_mutable_locals: &'a RefCell>, + ) -> Self { let dead_unwinds = BitSet::new_empty(item.body.basic_blocks().len()); - let indirectly_mutable_locals = old_dataflow::do_dataflow( - item.tcx, - item.body, - item.def_id, - &[], - &dead_unwinds, - old_dataflow::IndirectlyMutableLocals::new(item.tcx, item.body, item.param_env), - |_, local| old_dataflow::DebugFormatted::new(&local), - ); - - let indirectly_mutable_locals = old_dataflow::DataflowResultsCursor::new( - indirectly_mutable_locals, - item.body, - ); - let indirectly_mutable_locals = Rc::new(RefCell::new(indirectly_mutable_locals)); - let needs_drop = FlowSensitiveResolver::new( NeedsDrop, item, - indirectly_mutable_locals.clone(), + indirectly_mutable_locals, &dead_unwinds, ); let has_mut_interior = FlowSensitiveResolver::new( HasMutInterior, item, - indirectly_mutable_locals.clone(), + indirectly_mutable_locals, &dead_unwinds, ); diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 5add7a64aabf6..387540655f07b 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -957,7 +957,8 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { } let item = new_checker::Item::new(self.tcx, self.def_id, self.body); - let mut validator = new_checker::validation::Validator::new(&item); + let mut_borrowed_locals = new_checker::validation::compute_indirectly_mutable_locals(&item); + let mut validator = new_checker::validation::Validator::new(&item, &mut_borrowed_locals); validator.suppress_errors = !use_new_validator; self.suppress_errors = use_new_validator; From ff6faabda7840db86b90663f239bac909bae3a18 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 25 Sep 2019 12:30:25 -0700 Subject: [PATCH 26/31] Add description for every module in `check_consts` --- src/librustc_mir/transform/check_consts/mod.rs | 6 ++++++ src/librustc_mir/transform/check_consts/ops.rs | 2 ++ src/librustc_mir/transform/check_consts/qualifs.rs | 2 ++ src/librustc_mir/transform/check_consts/resolver.rs | 5 +++++ src/librustc_mir/transform/check_consts/validation.rs | 2 ++ 5 files changed, 17 insertions(+) diff --git a/src/librustc_mir/transform/check_consts/mod.rs b/src/librustc_mir/transform/check_consts/mod.rs index 3f26b4f0f45cc..3a959a86edd83 100644 --- a/src/librustc_mir/transform/check_consts/mod.rs +++ b/src/librustc_mir/transform/check_consts/mod.rs @@ -1,3 +1,9 @@ +//! Check the bodies of `const`s, `static`s and `const fn`s for illegal operations. +//! +//! This module will eventually replace the parts of `qualify_consts.rs` that check whether a local +//! has interior mutability or needs to be dropped, as well as the visitor that emits errors when +//! it finds operations that are invalid in a certain context. + use rustc::hir::def_id::DefId; use rustc::mir; use rustc::ty::{self, TyCtxt}; diff --git a/src/librustc_mir/transform/check_consts/ops.rs b/src/librustc_mir/transform/check_consts/ops.rs index a05a13dc123da..f457b739949c1 100644 --- a/src/librustc_mir/transform/check_consts/ops.rs +++ b/src/librustc_mir/transform/check_consts/ops.rs @@ -1,3 +1,5 @@ +//! Concrete error types for all operations which may be invalid in a certain const context. + use rustc::hir::def_id::DefId; use rustc::mir::BorrowKind; use rustc::session::config::nightly_options; diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index 8e0b49d157e09..5902ee0cc3f98 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -1,3 +1,5 @@ +//! A copy of the `Qualif` trait in `qualify_consts.rs` that is suitable for the new validator. + use rustc::mir::*; use rustc::mir::interpret::ConstValue; use rustc::ty::{self, Ty}; diff --git a/src/librustc_mir/transform/check_consts/resolver.rs b/src/librustc_mir/transform/check_consts/resolver.rs index 52c471ad691e4..c23a7e98efcbc 100644 --- a/src/librustc_mir/transform/check_consts/resolver.rs +++ b/src/librustc_mir/transform/check_consts/resolver.rs @@ -1,3 +1,8 @@ +//! Propagate `Qualif`s between locals and query the results. +//! +//! This also contains the dataflow analysis used to track `Qualif`s on complex control-flow +//! graphs. + use rustc::mir::visit::Visitor; use rustc::mir::{self, BasicBlock, Local, Location}; use rustc_data_structures::bit_set::BitSet; diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 74bf70f05e687..11a93225d3631 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -1,3 +1,5 @@ +//! The `Visitor` responsible for actually checking a `mir::Body` for invalid operations. + use rustc::hir::{self, def_id::DefId}; use rustc::mir::visit::{PlaceContext, Visitor, MutatingUseContext, NonMutatingUseContext}; use rustc::mir::*; From a302055caac70f51641cdad1dfa87f134090496a Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 25 Sep 2019 12:53:28 -0700 Subject: [PATCH 27/31] Mask results from flow-sensitive resolver with `in_any_value_of_ty` We relied previously on the caller (e.g. `Q::in_operand`) to ignore `Local`s that were indirectly mutable (and thus assumed to be qualified). However, it's much clearer (and more efficient) to do this in the resolver itself. This does not yet remove the masking done in `Q::in_operand` and others for safety's sake, although I believe that should now be possible. --- .../transform/check_consts/resolver.rs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/check_consts/resolver.rs b/src/librustc_mir/transform/check_consts/resolver.rs index c23a7e98efcbc..2789693ecb6ec 100644 --- a/src/librustc_mir/transform/check_consts/resolver.rs +++ b/src/librustc_mir/transform/check_consts/resolver.rs @@ -188,6 +188,9 @@ where indirectly_mutable_locals: &'a RefCell>, cursor: dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'a, 'mir, 'tcx, Q>>, qualifs_per_local: BitSet, + + /// The value of `Q::in_any_value_of_ty` for each local. + qualifs_in_any_value_of_ty: BitSet, } impl FlowSensitiveResolver<'a, 'mir, 'tcx, Q> @@ -208,10 +211,18 @@ where dataflow::Engine::new(item.body, dead_unwinds, analysis).iterate_to_fixpoint(); let cursor = dataflow::ResultsCursor::new(item.body, results); + let mut qualifs_in_any_value_of_ty = BitSet::new_empty(item.body.local_decls.len()); + for (local, decl) in item.body.local_decls.iter_enumerated() { + if Q::in_any_value_of_ty(item, decl.ty) { + qualifs_in_any_value_of_ty.insert(local); + } + } + FlowSensitiveResolver { cursor, indirectly_mutable_locals, qualifs_per_local: BitSet::new_empty(item.body.local_decls.len()), + qualifs_in_any_value_of_ty, location: Location { block: mir::START_BLOCK, statement_index: 0 }, } } @@ -242,16 +253,23 @@ where self.qualifs_per_local.overwrite(indirectly_mutable_locals.get()); self.qualifs_per_local.union(self.cursor.get()); + self.qualifs_per_local.intersect(&self.qualifs_in_any_value_of_ty); &self.qualifs_per_local } fn contains(&mut self, local: Local) -> bool { + // No need to update the cursor if we know that `Local` cannot possibly be qualified. + if !self.qualifs_in_any_value_of_ty.contains(local) { + return false; + } + + // Otherwise, return `true` if this local is qualified or was indirectly mutable at any + // point before this statement. self.cursor.seek_before(self.location); if self.cursor.get().contains(local) { return true; } - let mut indirectly_mutable_locals = self.indirectly_mutable_locals.borrow_mut(); indirectly_mutable_locals.seek(self.location); indirectly_mutable_locals.get().contains(local) From 8bfe82bfad881c580b02fe61b699a770790e18e2 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 25 Sep 2019 13:25:06 -0700 Subject: [PATCH 28/31] Correct `IndirectlyMutableLocals` docs --- src/librustc_mir/dataflow/impls/indirect_mutation.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/dataflow/impls/indirect_mutation.rs b/src/librustc_mir/dataflow/impls/indirect_mutation.rs index 353892820e3f4..7d20248ebd1f5 100644 --- a/src/librustc_mir/dataflow/impls/indirect_mutation.rs +++ b/src/librustc_mir/dataflow/impls/indirect_mutation.rs @@ -8,15 +8,11 @@ use crate::dataflow::{self, GenKillSet}; /// Whether a borrow to a `Local` has been created that could allow that `Local` to be mutated /// indirectly. This could either be a mutable reference (`&mut`) or a shared borrow if the type of -/// that `Local` allows interior mutability. +/// that `Local` allows interior mutability. Operations that can mutate local's indirectly include: +/// assignments through a pointer (`*p = 42`), function calls, drop terminators and inline assembly. /// -/// If this returns false for a `Local` at the location of an indirect assignment, that `Local` -/// cannot be mutated by that assignment. For example, if the dataflow state at a statement like -/// `*p = 42` contained locals `x` and `z` but not `y`, we know that while `x` or `z` may be the -/// target of that assignment, `y` cannot be. -/// -/// Assignments through a pointer are not the only place where a `Local` can be mutated indirectly: -/// Function calls, drop terminators and inline assembly can all mutate `Local`s as a side-effect. +/// If this returns false for a `Local` at a given statement (or terminator), that `Local` could +/// not possibly have been mutated indirectly prior to that statement. #[derive(Copy, Clone)] pub struct IndirectlyMutableLocals<'mir, 'tcx> { body: &'mir mir::Body<'tcx>, From f2e7faf15327082a1e699ecdeadfd5f2cef2b8ee Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 25 Sep 2019 14:56:50 -0700 Subject: [PATCH 29/31] Revert "Use conservative, type-based qualifcation for statics" This reverts commit ac7a343cef8287427a98b9210cdb1a772486be10. --- src/librustc_mir/transform/check_consts/qualifs.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index 5902ee0cc3f98..dd2ea914402cc 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -34,8 +34,9 @@ pub trait Qualif { /// of the type. fn in_any_value_of_ty(_cx: &ConstCx<'_, 'tcx>, _ty: Ty<'tcx>) -> bool; - fn in_static(cx: &ConstCx<'_, 'tcx>, statik: &Static<'tcx>) -> bool { - Self::in_any_value_of_ty(cx, statik.ty) + fn in_static(_cx: &ConstCx<'_, 'tcx>, _static: &Static<'tcx>) -> bool { + // FIXME(eddyb) should we do anything here for value properties? + false } fn in_projection_structurally( From ff4158abf2a66a6a3f246fef8178e179201fcae0 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 25 Sep 2019 15:18:26 -0700 Subject: [PATCH 30/31] Bless miri unleashed output --- .../consts/miri_unleashed/mutable_references.rs | 1 - .../miri_unleashed/mutable_references.stderr | 16 +++++----------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/test/ui/consts/miri_unleashed/mutable_references.rs b/src/test/ui/consts/miri_unleashed/mutable_references.rs index b2f3e0cdb287f..59dafcbf4d50c 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_references.rs +++ b/src/test/ui/consts/miri_unleashed/mutable_references.rs @@ -7,7 +7,6 @@ use std::cell::UnsafeCell; static FOO: &&mut u32 = &&mut 42; //~^ WARN: skipping const checks -//~| WARN: skipping const checks static BAR: &mut () = &mut (); //~^ WARN: skipping const checks diff --git a/src/test/ui/consts/miri_unleashed/mutable_references.stderr b/src/test/ui/consts/miri_unleashed/mutable_references.stderr index ba01d15572501..e8a867307ce19 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_references.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_references.stderr @@ -5,37 +5,31 @@ LL | static FOO: &&mut u32 = &&mut 42; | ^^^^^^^ warning: skipping const checks - --> $DIR/mutable_references.rs:8:25 - | -LL | static FOO: &&mut u32 = &&mut 42; - | ^^^^^^^^ - -warning: skipping const checks - --> $DIR/mutable_references.rs:12:23 + --> $DIR/mutable_references.rs:11:23 | LL | static BAR: &mut () = &mut (); | ^^^^^^^ warning: skipping const checks - --> $DIR/mutable_references.rs:17:28 + --> $DIR/mutable_references.rs:16:28 | LL | static BOO: &mut Foo<()> = &mut Foo(()); | ^^^^^^^^^^^^ warning: skipping const checks - --> $DIR/mutable_references.rs:27:8 + --> $DIR/mutable_references.rs:26:8 | LL | x: &UnsafeCell::new(42), | ^^^^^^^^^^^^^^^^^^^^ warning: skipping const checks - --> $DIR/mutable_references.rs:31:27 + --> $DIR/mutable_references.rs:30:27 | LL | static OH_YES: &mut i32 = &mut 42; | ^^^^^^^ error[E0594]: cannot assign to `*OH_YES`, as `OH_YES` is an immutable static item - --> $DIR/mutable_references.rs:38:5 + --> $DIR/mutable_references.rs:37:5 | LL | *OH_YES = 99; | ^^^^^^^^^^^^ cannot assign From 0bf1a80b322f285efb9ea45b62e7dc764ebe1954 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sat, 28 Sep 2019 07:20:06 -0700 Subject: [PATCH 31/31] Rename `sty` to `kind` Picks up changes made in #64513 --- src/librustc_mir/transform/check_consts/qualifs.rs | 4 ++-- src/librustc_mir/transform/check_consts/validation.rs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index dd2ea914402cc..d9d0ce1855573 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -164,7 +164,7 @@ pub trait Qualif { if let box [proj_base @ .., elem] = &place.projection { if ProjectionElem::Deref == *elem { let base_ty = Place::ty_from(&place.base, proj_base, cx.body, cx.tcx).ty; - if let ty::Ref(..) = base_ty.sty { + if let ty::Ref(..) = base_ty.kind { return Self::in_place(cx, per_local, PlaceRef { base: &place.base, projection: proj_base, @@ -223,7 +223,7 @@ impl Qualif for HasMutInterior { if let BorrowKind::Mut { .. } = kind { // In theory, any zero-sized value could be borrowed // mutably without consequences. - match ty.sty { + match ty.kind { // Inside a `static mut`, &mut [...] is also allowed. ty::Array(..) | ty::Slice(_) if cx.mode == Mode::StaticMut => {}, diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 11a93225d3631..7e876dd1d9980 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -247,7 +247,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { if let box [proj_base @ .., elem] = &place.projection { if *elem == ProjectionElem::Deref { let base_ty = Place::ty_from(&place.base, proj_base, self.body, self.tcx).ty; - if let ty::Ref(..) = base_ty.sty { + if let ty::Ref(..) = base_ty.kind { reborrow_place = Some(proj_base); } } @@ -302,7 +302,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { } Rvalue::BinaryOp(op, ref lhs, _) => { - if let ty::RawPtr(_) | ty::FnPtr(..) = lhs.ty(self.body, self.tcx).sty { + if let ty::RawPtr(_) | ty::FnPtr(..) = lhs.ty(self.body, self.tcx).kind { assert!(op == BinOp::Eq || op == BinOp::Ne || op == BinOp::Le || op == BinOp::Lt || op == BinOp::Ge || op == BinOp::Gt || @@ -431,7 +431,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { } let base_ty = Place::ty_from(place_base, proj_base, self.body, self.tcx).ty; - if let ty::RawPtr(_) = base_ty.sty { + if let ty::RawPtr(_) = base_ty.kind { self.check_op(ops::RawPtrDeref); } } @@ -508,7 +508,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { TerminatorKind::Call { func, .. } => { let fn_ty = func.ty(self.body, self.tcx); - let def_id = match fn_ty.sty { + let def_id = match fn_ty.kind { ty::FnDef(def_id, _) => def_id, ty::FnPtr(_) => {