From a1d04cc1d87d990c12da5c816dee412704b38d7f Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 24 Oct 2019 17:20:18 +0200 Subject: [PATCH 1/8] Remove statics from HAIR by lowering them to a pointer constant --- src/librustc_mir/build/expr/as_place.rs | 7 ------ src/librustc_mir/build/expr/as_rvalue.rs | 1 - src/librustc_mir/build/expr/category.rs | 1 - src/librustc_mir/build/expr/into.rs | 1 - src/librustc_mir/hair/cx/expr.rs | 28 ++++++++++++++++++++++-- src/librustc_mir/hair/mod.rs | 3 --- 6 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/librustc_mir/build/expr/as_place.rs b/src/librustc_mir/build/expr/as_place.rs index d3e013acc9e3a..aed4759322cb9 100644 --- a/src/librustc_mir/build/expr/as_place.rs +++ b/src/librustc_mir/build/expr/as_place.rs @@ -197,13 +197,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }; block.and(place_builder) } - ExprKind::StaticRef { id } => block.and(PlaceBuilder::from( - PlaceBase::Static(Box::new(Static { - ty: expr.ty, - kind: StaticKind::Static, - def_id: id, - })) - )), ExprKind::PlaceTypeAscription { source, user_ty } => { let source = this.hir.mirror(source); diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index f9b77a4b5dd2a..3bbd8093d3bae 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -288,7 +288,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::Continue { .. } | ExprKind::Return { .. } | ExprKind::InlineAsm { .. } - | ExprKind::StaticRef { .. } | ExprKind::PlaceTypeAscription { .. } | ExprKind::ValueTypeAscription { .. } => { // these do not have corresponding `Rvalue` variants, diff --git a/src/librustc_mir/build/expr/category.rs b/src/librustc_mir/build/expr/category.rs index ae5289986e77c..e7b68acc2ef97 100644 --- a/src/librustc_mir/build/expr/category.rs +++ b/src/librustc_mir/build/expr/category.rs @@ -40,7 +40,6 @@ impl Category { | ExprKind::Index { .. } | ExprKind::SelfRef | ExprKind::VarRef { .. } - | ExprKind::StaticRef { .. } | ExprKind::PlaceTypeAscription { .. } | ExprKind::ValueTypeAscription { .. } => Some(Category::Place), diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 404ca3204e6c0..1a19878a1f18e 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -384,7 +384,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Avoid creating a temporary ExprKind::VarRef { .. } | ExprKind::SelfRef | - ExprKind::StaticRef { .. } | ExprKind::PlaceTypeAscription { .. } | ExprKind::ValueTypeAscription { .. } => { debug_assert!(Category::of(&expr.kind) == Some(Category::Place)); diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index f25e4b0ae8639..4bae4bf9052d6 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -5,7 +5,7 @@ use crate::hair::cx::to_ref::ToRef; use crate::hair::util::UserAnnotatedTyHelpers; use rustc_index::vec::Idx; use rustc::hir::def::{CtorOf, Res, DefKind, CtorKind}; -use rustc::mir::interpret::{GlobalId, ErrorHandled}; +use rustc::mir::interpret::{GlobalId, ErrorHandled, ConstValue, Scalar}; use rustc::ty::{self, AdtKind, Ty}; use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, AutoBorrowMutability, PointerCast}; use rustc::ty::subst::{InternalSubsts, SubstsRef}; @@ -961,7 +961,31 @@ fn convert_path_expr<'a, 'tcx>( } } - Res::Def(DefKind::Static, id) => ExprKind::StaticRef { id }, + // We encode uses of statics as a `*&STATIC` where the `&STATIC` part is + // a constant reference (or constant raw pointer for `static mut`) in MIR + Res::Def(DefKind::Static, id) => { + let ty = cx.tcx.type_of(id); + let ty = if cx.tcx.is_mutable_static(id) { + cx.tcx.mk_mut_ptr(ty) + } else if cx.tcx.is_foreign_item(id) { + cx.tcx.mk_imm_ptr(ty) + } else { + cx.tcx.mk_imm_ref(cx.tcx.lifetimes.re_static, ty) + }; + let ptr = cx.tcx.alloc_map.lock().create_static_alloc(id); + let temp_lifetime = cx.region_scope_tree.temporary_scope(expr.hir_id.local_id); + ExprKind::Deref { arg: Expr { + ty, + temp_lifetime, + span: expr.span, + kind: ExprKind::Literal { + literal: cx.tcx.mk_const(ty::Const { + ty, val: ConstValue::Scalar(Scalar::Ptr(ptr.into())), + }), + user_ty: None, + } + }.to_ref() } + }, Res::Local(var_hir_id) => convert_var(cx, expr, var_hir_id), diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index 78e3a17d76632..28794859c564d 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -208,9 +208,6 @@ pub enum ExprKind<'tcx> { }, /// first argument, used for self in a closure SelfRef, - StaticRef { - id: DefId, - }, Borrow { borrow_kind: BorrowKind, arg: ExprRef<'tcx>, From ae9677c82f592a7d6d25a5e3451f1f9cda55e754 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 11 Nov 2019 12:15:38 +0100 Subject: [PATCH 2/8] Readjust const qualification to detect statics again --- src/librustc/mir/mod.rs | 20 ++++++++++++++++++- .../transform/check_consts/qualifs.rs | 18 ++++++----------- .../transform/check_consts/validation.rs | 15 +++++++++++--- .../transform/qualify_min_const_fn.rs | 13 +++++------- .../consts/const-fn-not-safe-for-const.stderr | 4 ++-- .../consts/min_const_fn/min_const_fn.stderr | 4 ++-- 6 files changed, 46 insertions(+), 28 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index b7d0f538db5dc..dd27a59200446 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -7,7 +7,7 @@ use crate::hir::def::{CtorKind, Namespace}; use crate::hir::def_id::DefId; use crate::hir; -use crate::mir::interpret::{PanicInfo, Scalar}; +use crate::mir::interpret::{ConstValue, GlobalAlloc, PanicInfo, Scalar}; use crate::mir::visit::MirVisitable; use crate::ty::adjustment::PointerCast; use crate::ty::fold::{TypeFoldable, TypeFolder, TypeVisitor}; @@ -2341,6 +2341,24 @@ pub struct Constant<'tcx> { pub literal: &'tcx ty::Const<'tcx>, } +impl Constant<'tcx> { + pub fn check_static_ptr(&self, tcx: TyCtxt<'_>) -> Option { + match self.literal.val { + ConstValue::Scalar(Scalar::Ptr(ptr)) => match tcx.alloc_map.lock().get(ptr.alloc_id) { + Some(GlobalAlloc::Static(def_id)) => Some(def_id), + Some(_) => None, + None => { + tcx.sess.delay_span_bug( + DUMMY_SP, "MIR cannot contain dangling const pointers", + ); + None + }, + }, + _ => None, + } + } +} + /// A collection of projections into user types. /// /// They are projections because a binding can occur a part of a diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index aad14299c1d94..9ed1ca740b8e7 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -2,6 +2,7 @@ use rustc::mir::*; use rustc::ty::{self, Ty}; +use rustc::hir::def_id::DefId; use syntax_pos::DUMMY_SP; use super::{ConstKind, Item as ConstCx}; @@ -32,7 +33,7 @@ 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 { + fn in_static(_cx: &ConstCx<'_, 'tcx>, _def_id: DefId) -> bool { // FIXME(eddyb) should we do anything here for value properties? false } @@ -86,18 +87,9 @@ pub trait Qualif { projection: [], } => per_local(*local), PlaceRef { - base: PlaceBase::Static(box Static { - kind: StaticKind::Promoted(..), - .. - }), + base: PlaceBase::Static(_), projection: [], } => bug!("qualifying already promoted MIR"), - PlaceRef { - base: PlaceBase::Static(static_), - projection: [], - } => { - Self::in_static(cx, static_) - }, PlaceRef { base: _, projection: [.., _], @@ -115,7 +107,9 @@ pub trait Qualif { Operand::Move(ref place) => Self::in_place(cx, per_local, place.as_ref()), Operand::Constant(ref constant) => { - if let ty::ConstKind::Unevaluated(def_id, _) = constant.literal.val { + if let Some(static_) = constant.check_static_ptr(cx.tcx) { + Self::in_static(cx, static_) + } else if let ty::ConstKind::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) diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 21e7c9ce565f0..61de6b185e32d 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -408,12 +408,21 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { match place_base { PlaceBase::Local(_) => {} - PlaceBase::Static(box Static{ kind: StaticKind::Promoted(_, _), .. }) => { + PlaceBase::Static(_) => { 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); + fn visit_operand( + &mut self, + op: &Operand<'tcx>, + location: Location, + ) { + self.super_operand(op, location); + if let Operand::Constant(c) = op { + if let Some(def_id) = c.check_static_ptr(self.tcx) { + 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.const_kind() != ConstKind::Static || !context.is_mutating_use() { diff --git a/src/librustc_mir/transform/qualify_min_const_fn.rs b/src/librustc_mir/transform/qualify_min_const_fn.rs index 83bde5ed34eae..65fc7cd20439d 100644 --- a/src/librustc_mir/transform/qualify_min_const_fn.rs +++ b/src/librustc_mir/transform/qualify_min_const_fn.rs @@ -249,7 +249,10 @@ fn check_operand( Operand::Move(place) | Operand::Copy(place) => { check_place(tcx, place, span, def_id, body) } - Operand::Constant(_) => Ok(()), + Operand::Constant(c) => match c.check_static_ptr(tcx) { + Some(_) => Err((span, "cannot access `static` items in const fn".into())), + None => Ok(()), + }, } } @@ -285,13 +288,7 @@ fn check_place( } } - match place.base { - PlaceBase::Static(box Static { kind: StaticKind::Static, .. }) => { - Err((span, "cannot access `static` items in const fn".into())) - } - PlaceBase::Local(_) - | PlaceBase::Static(box Static { kind: StaticKind::Promoted(_, _), .. }) => Ok(()), - } + Ok(()) } /// Returns whether `allow_internal_unstable(..., , ...)` is present. diff --git a/src/test/ui/consts/const-fn-not-safe-for-const.stderr b/src/test/ui/consts/const-fn-not-safe-for-const.stderr index ba5d58a51d2dd..2d4175ea8eb7d 100644 --- a/src/test/ui/consts/const-fn-not-safe-for-const.stderr +++ b/src/test/ui/consts/const-fn-not-safe-for-const.stderr @@ -11,10 +11,10 @@ LL | Y | ^ error[E0013]: constant functions cannot refer to statics, use a constant instead - --> $DIR/const-fn-not-safe-for-const.rs:25:5 + --> $DIR/const-fn-not-safe-for-const.rs:25:6 | LL | &Y - | ^^ + | ^ error: aborting due to 3 previous errors diff --git a/src/test/ui/consts/min_const_fn/min_const_fn.stderr b/src/test/ui/consts/min_const_fn/min_const_fn.stderr index 5ce21e378cd1e..cb1663ed22f95 100644 --- a/src/test/ui/consts/min_const_fn/min_const_fn.stderr +++ b/src/test/ui/consts/min_const_fn/min_const_fn.stderr @@ -116,10 +116,10 @@ LL | const fn foo25() -> u32 { BAR } = help: add `#![feature(const_fn)]` to the crate attributes to enable error[E0723]: cannot access `static` items in const fn - --> $DIR/min_const_fn.rs:91:36 + --> $DIR/min_const_fn.rs:91:37 | LL | const fn foo26() -> &'static u32 { &BAR } - | ^^^^ + | ^^^ | = note: for more information, see issue https://github.com/rust-lang/rust/issues/57563 = help: add `#![feature(const_fn)]` to the crate attributes to enable From 36006955e7c8e120f7ae9651ae23a3cd98a715e8 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 11 Nov 2019 12:49:21 +0100 Subject: [PATCH 3/8] Simplify pattern --- src/librustc_codegen_ssa/mir/constant.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_codegen_ssa/mir/constant.rs b/src/librustc_codegen_ssa/mir/constant.rs index 181787e398546..1cc091d962063 100644 --- a/src/librustc_codegen_ssa/mir/constant.rs +++ b/src/librustc_codegen_ssa/mir/constant.rs @@ -14,8 +14,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { constant: &mir::Constant<'tcx>, ) -> Result<&'tcx ty::Const<'tcx>, ErrorHandled> { match constant.literal.val { - ty::ConstKind::Unevaluated(def_id, ref substs) => { - let substs = self.monomorphize(substs); + ty::ConstKind::Unevaluated(def_id, substs) => { + let substs = self.monomorphize(&substs); let instance = ty::Instance::resolve( self.cx.tcx(), ty::ParamEnv::reveal_all(), def_id, substs, ).unwrap(); From 47a3294a1cc2deab2f55795a6ef9b7d4ce51adf9 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 11 Nov 2019 12:53:31 +0100 Subject: [PATCH 4/8] Readjust constant evaluation for operands --- src/librustc_codegen_ssa/mir/constant.rs | 23 +++++++++++++++++++++++ src/librustc_codegen_ssa/mir/operand.rs | 3 +-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/librustc_codegen_ssa/mir/constant.rs b/src/librustc_codegen_ssa/mir/constant.rs index 1cc091d962063..41d2baaab8035 100644 --- a/src/librustc_codegen_ssa/mir/constant.rs +++ b/src/librustc_codegen_ssa/mir/constant.rs @@ -5,10 +5,33 @@ use rustc::ty::{self, Ty}; use rustc::ty::layout::{self, HasTyCtxt}; use syntax::source_map::Span; use crate::traits::*; +use crate::mir::operand::OperandRef; use super::FunctionCx; impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { + pub fn eval_mir_constant_to_operand( + &mut self, + bx: &mut Bx, + constant: &mir::Constant<'tcx>, + ) -> Result, ErrorHandled> { + match constant.literal.val { + mir::interpret::ConstValue::Unevaluated(def_id, substs) + if self.cx.tcx().is_static(def_id) => { + assert!(substs.is_empty(), "we don't support generic statics yet"); + let static_ = bx.get_static(def_id); + // we treat operands referring to statics as if they were `&STATIC` instead + let ptr_ty = self.cx.tcx().mk_mut_ptr(self.monomorphize(&constant.literal.ty)); + let layout = bx.layout_of(ptr_ty); + Ok(OperandRef::from_immediate_or_packed_pair(bx, static_, layout)) + } + _ => { + let val = self.eval_mir_constant(constant)?; + Ok(OperandRef::from_const(bx, val)) + } + } + } + pub fn eval_mir_constant( &mut self, constant: &mir::Constant<'tcx>, diff --git a/src/librustc_codegen_ssa/mir/operand.rs b/src/librustc_codegen_ssa/mir/operand.rs index 78d09f834c68c..de0edf82d1793 100644 --- a/src/librustc_codegen_ssa/mir/operand.rs +++ b/src/librustc_codegen_ssa/mir/operand.rs @@ -465,8 +465,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } mir::Operand::Constant(ref constant) => { - self.eval_mir_constant(constant) - .map(|c| OperandRef::from_const(bx, c)) + self.eval_mir_constant_to_operand(bx, constant) .unwrap_or_else(|err| { match err { // errored or at least linted From c6d97dfd835b5d9a740249570e008eb94f1a9e85 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 18 Nov 2019 22:15:33 +0000 Subject: [PATCH 5/8] Fix rebase --- src/librustc/mir/mod.rs | 6 +++--- src/librustc_codegen_ssa/mir/constant.rs | 2 +- src/librustc_mir/hair/cx/expr.rs | 10 ++++------ src/librustc_mir/transform/check_consts/validation.rs | 2 +- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index dd27a59200446..c0cd74ecf3a45 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -7,7 +7,7 @@ use crate::hir::def::{CtorKind, Namespace}; use crate::hir::def_id::DefId; use crate::hir; -use crate::mir::interpret::{ConstValue, GlobalAlloc, PanicInfo, Scalar}; +use crate::mir::interpret::{GlobalAlloc, PanicInfo, Scalar}; use crate::mir::visit::MirVisitable; use crate::ty::adjustment::PointerCast; use crate::ty::fold::{TypeFoldable, TypeFolder, TypeVisitor}; @@ -2343,8 +2343,8 @@ pub struct Constant<'tcx> { impl Constant<'tcx> { pub fn check_static_ptr(&self, tcx: TyCtxt<'_>) -> Option { - match self.literal.val { - ConstValue::Scalar(Scalar::Ptr(ptr)) => match tcx.alloc_map.lock().get(ptr.alloc_id) { + match self.literal.val.try_to_scalar() { + Some(Scalar::Ptr(ptr)) => match tcx.alloc_map.lock().get(ptr.alloc_id) { Some(GlobalAlloc::Static(def_id)) => Some(def_id), Some(_) => None, None => { diff --git a/src/librustc_codegen_ssa/mir/constant.rs b/src/librustc_codegen_ssa/mir/constant.rs index 41d2baaab8035..27891be6b82c5 100644 --- a/src/librustc_codegen_ssa/mir/constant.rs +++ b/src/librustc_codegen_ssa/mir/constant.rs @@ -16,7 +16,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { constant: &mir::Constant<'tcx>, ) -> Result, ErrorHandled> { match constant.literal.val { - mir::interpret::ConstValue::Unevaluated(def_id, substs) + ty::ConstKind::Unevaluated(def_id, substs) if self.cx.tcx().is_static(def_id) => { assert!(substs.is_empty(), "we don't support generic statics yet"); let static_ = bx.get_static(def_id); diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index 4bae4bf9052d6..de50755616bb0 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -5,7 +5,7 @@ use crate::hair::cx::to_ref::ToRef; use crate::hair::util::UserAnnotatedTyHelpers; use rustc_index::vec::Idx; use rustc::hir::def::{CtorOf, Res, DefKind, CtorKind}; -use rustc::mir::interpret::{GlobalId, ErrorHandled, ConstValue, Scalar}; +use rustc::mir::interpret::{GlobalId, ErrorHandled, Scalar}; use rustc::ty::{self, AdtKind, Ty}; use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, AutoBorrowMutability, PointerCast}; use rustc::ty::subst::{InternalSubsts, SubstsRef}; @@ -978,11 +978,9 @@ fn convert_path_expr<'a, 'tcx>( ty, temp_lifetime, span: expr.span, - kind: ExprKind::Literal { - literal: cx.tcx.mk_const(ty::Const { - ty, val: ConstValue::Scalar(Scalar::Ptr(ptr.into())), - }), - user_ty: None, + kind: ExprKind::StaticRef { + literal: ty::Const::from_scalar(cx.tcx, Scalar::Ptr(ptr.into()), ty), + def_id: id, } }.to_ref() } }, diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 61de6b185e32d..772f27fb7e14f 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -425,7 +425,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { 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.const_kind() != ConstKind::Static || !context.is_mutating_use() { + } else { self.check_op(ops::StaticAccess); } } From 9abc34ed9d34873066a186ac5551d5aad9e783b6 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 18 Nov 2019 23:04:06 +0000 Subject: [PATCH 6/8] Track pointers to statics in MIR --- src/librustc/mir/mod.rs | 74 ++++++++--- src/librustc/mir/visit.rs | 2 +- src/librustc_mir/borrow_check/borrow_set.rs | 4 +- .../borrow_check/conflict_errors.rs | 35 +++-- .../borrow_check/error_reporting.rs | 78 ++++------- src/librustc_mir/borrow_check/mod.rs | 35 +++-- src/librustc_mir/borrow_check/move_errors.rs | 27 ++-- .../borrow_check/mutability_errors.rs | 124 ++++++++---------- .../borrow_check/nll/type_check/mod.rs | 4 +- src/librustc_mir/borrow_check/place_ext.rs | 63 +++++---- src/librustc_mir/build/expr/as_constant.rs | 7 + src/librustc_mir/build/expr/as_place.rs | 1 + src/librustc_mir/build/expr/as_rvalue.rs | 1 + src/librustc_mir/build/expr/as_temp.rs | 6 + src/librustc_mir/build/expr/category.rs | 3 +- src/librustc_mir/build/expr/into.rs | 3 +- src/librustc_mir/build/matches/mod.rs | 28 ++-- src/librustc_mir/build/mod.rs | 22 ++-- src/librustc_mir/dataflow/impls/borrows.rs | 4 +- src/librustc_mir/hair/mod.rs | 5 + src/librustc_mir/shim.rs | 2 +- .../transform/check_consts/validation.rs | 36 +++-- src/librustc_mir/transform/check_unsafety.rs | 43 +++--- src/librustc_mir/transform/generator.rs | 6 +- src/librustc_mir/transform/promote_consts.rs | 45 +++---- 25 files changed, 359 insertions(+), 299 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index c0cd74ecf3a45..2928a8ad9bcc5 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -292,7 +292,7 @@ impl<'tcx> Body<'tcx> { pub fn temps_iter<'a>(&'a self) -> impl Iterator + 'a { (self.arg_count + 1..self.local_decls.len()).filter_map(move |index| { let local = Local::new(index); - if self.local_decls[local].is_user_variable.is_some() { + if self.local_decls[local].is_user_variable() { None } else { Some(local) @@ -305,7 +305,7 @@ impl<'tcx> Body<'tcx> { pub fn vars_iter<'a>(&'a self) -> impl Iterator + 'a { (self.arg_count + 1..self.local_decls.len()).filter_map(move |index| { let local = Local::new(index); - if self.local_decls[local].is_user_variable.is_some() { + if self.local_decls[local].is_user_variable() { Some(local) } else { None @@ -319,7 +319,7 @@ impl<'tcx> Body<'tcx> { (self.arg_count + 1..self.local_decls.len()).filter_map(move |index| { let local = Local::new(index); let decl = &self.local_decls[local]; - if decl.is_user_variable.is_some() && decl.mutability == Mutability::Mut { + if decl.is_user_variable() && decl.mutability == Mutability::Mut { Some(local) } else { None @@ -333,7 +333,7 @@ impl<'tcx> Body<'tcx> { (1..self.local_decls.len()).filter_map(move |index| { let local = Local::new(index); let decl = &self.local_decls[local]; - if (decl.is_user_variable.is_some() || index < self.arg_count + 1) + if (decl.is_user_variable() || index < self.arg_count + 1) && decl.mutability == Mutability::Mut { Some(local) @@ -696,7 +696,8 @@ pub struct LocalDecl<'tcx> { /// therefore it need not be visible across crates. pnkfelix /// currently hypothesized we *need* to wrap this in a /// `ClearCrossCrate` as long as it carries as `HirId`. - pub is_user_variable: Option>>, + // FIXME(matthewjasper) Don't store in this in `Body` + pub local_info: LocalInfo<'tcx>, /// `true` if this is an internal local. /// @@ -721,6 +722,7 @@ pub struct LocalDecl<'tcx> { /// then it is a temporary created for evaluation of some /// subexpression of some block's tail expression (with no /// intervening statement context). + // FIXME(matthewjasper) Don't store in this in `Body` pub is_block_tail: Option, /// The type of this local. @@ -730,6 +732,7 @@ pub struct LocalDecl<'tcx> { /// e.g., via `let x: T`, then we carry that type here. The MIR /// borrow checker needs this information since it can affect /// region inference. + // FIXME(matthewjasper) Don't store in this in `Body` pub user_ty: UserTypeProjections, /// The name of the local, used in debuginfo and pretty-printing. @@ -824,6 +827,17 @@ pub struct LocalDecl<'tcx> { pub visibility_scope: SourceScope, } +/// Extra information about a local that's used for diagnostics. +#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)] +pub enum LocalInfo<'tcx> { + /// A user-defined local variable or function parameter + User(ClearCrossCrate>), + /// A temporary created that references the static with the given `DefId`. + StaticRef { def_id: DefId, is_thread_local: bool }, + /// Any other temporary, the return place, or an anonymous function parameter. + Other, +} + impl<'tcx> LocalDecl<'tcx> { /// Returns `true` only if local is a binding that can itself be /// made mutable via the addition of the `mut` keyword, namely @@ -832,15 +846,17 @@ impl<'tcx> LocalDecl<'tcx> { /// - `let x = ...`, /// - or `match ... { C(x) => ... }` pub fn can_be_made_mutable(&self) -> bool { - match self.is_user_variable { - Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { + match self.local_info { + LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { binding_mode: ty::BindingMode::BindByValue(_), opt_ty_info: _, opt_match_place: _, pat_span: _, }))) => true, - Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf(ImplicitSelfKind::Imm))) => true, + LocalInfo::User( + ClearCrossCrate::Set(BindingForm::ImplicitSelf(ImplicitSelfKind::Imm)), + ) => true, _ => false, } @@ -850,16 +866,26 @@ impl<'tcx> LocalDecl<'tcx> { /// `ref mut ident` binding. (Such bindings cannot be made into /// mutable bindings, but the inverse does not necessarily hold). pub fn is_nonref_binding(&self) -> bool { - match self.is_user_variable { - Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { + match self.local_info { + LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { binding_mode: ty::BindingMode::BindByValue(_), opt_ty_info: _, opt_match_place: _, pat_span: _, }))) => true, - Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf(_))) => true, + LocalInfo::User(ClearCrossCrate::Set(BindingForm::ImplicitSelf(_))) => true, + + _ => false, + } + } + /// Returns `true` if this variable is a named variable or function + /// parameter declared by the user. + #[inline] + pub fn is_user_variable(&self) -> bool { + match self.local_info { + LocalInfo::User(_) => true, _ => false, } } @@ -868,8 +894,26 @@ impl<'tcx> LocalDecl<'tcx> { /// expression that is used to access said variable for the guard of the /// match arm. pub fn is_ref_for_guard(&self) -> bool { - match self.is_user_variable { - Some(ClearCrossCrate::Set(BindingForm::RefForGuard)) => true, + match self.local_info { + LocalInfo::User(ClearCrossCrate::Set(BindingForm::RefForGuard)) => true, + _ => false, + } + } + + /// Returns `Some` if this is a reference to a static item that is used to + /// access that static + pub fn is_ref_to_static(&self) -> bool { + match self.local_info { + LocalInfo::StaticRef { .. } => true, + _ => false, + } + } + + /// Returns `Some` if this is a reference to a static item that is used to + /// access that static + pub fn is_ref_to_thread_local(&self) -> bool { + match self.local_info { + LocalInfo::StaticRef { is_thread_local, .. } => is_thread_local, _ => false, } } @@ -918,7 +962,7 @@ impl<'tcx> LocalDecl<'tcx> { source_info: SourceInfo { span, scope: OUTERMOST_SOURCE_SCOPE }, visibility_scope: OUTERMOST_SOURCE_SCOPE, internal, - is_user_variable: None, + local_info: LocalInfo::Other, is_block_tail: None, } } @@ -937,7 +981,7 @@ impl<'tcx> LocalDecl<'tcx> { internal: false, is_block_tail: None, name: None, // FIXME maybe we do want some name here? - is_user_variable: None, + local_info: LocalInfo::Other, } } } diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 6a41b843e5794..fc0e77aab43a4 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -691,7 +691,7 @@ macro_rules! make_mir_visitor { source_info, visibility_scope, internal: _, - is_user_variable: _, + local_info: _, is_block_tail: _, } = local_decl; diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index 98641031c1787..943234319906a 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -189,8 +189,8 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> { location: mir::Location, ) { if let mir::Rvalue::Ref(region, kind, ref borrowed_place) = *rvalue { - if borrowed_place.ignore_borrow( - self.tcx, self.body, &self.locals_state_at_exit) { + if borrowed_place.ignore_borrow(self.tcx, self.body, &self.locals_state_at_exit) { + debug!("ignoring_borrow of {:?}", borrowed_place); return; } diff --git a/src/librustc_mir/borrow_check/conflict_errors.rs b/src/librustc_mir/borrow_check/conflict_errors.rs index ebc25138a0619..3595312f3f41b 100644 --- a/src/librustc_mir/borrow_check/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/conflict_errors.rs @@ -3,8 +3,8 @@ use rustc::hir::def_id::DefId; use rustc::hir::{AsyncGeneratorKind, GeneratorKind}; use rustc::mir::{ self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, ConstraintCategory, - FakeReadCause, Local, LocalDecl, LocalKind, Location, Operand, Place, PlaceBase, PlaceRef, - ProjectionElem, Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm, + FakeReadCause, Local, LocalDecl, LocalInfo, LocalKind, Location, Operand, Place, PlaceBase, + PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm, }; use rustc::ty::{self, Ty}; use rustc_data_structures::fx::FxHashSet; @@ -744,6 +744,17 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { projection: root_place_projection, }, borrow_span)); + if let PlaceBase::Local(local) = borrow.borrowed_place.base { + if self.body.local_decls[local].is_ref_to_thread_local() { + let err = self.report_thread_local_value_does_not_live_long_enough( + drop_span, + borrow_span, + ); + err.buffer(&mut self.errors_buffer); + return; + } + }; + if let StorageDeadOrDrop::Destructor(dropped_ty) = self.classify_drop_access_kind(borrow.borrowed_place.as_ref()) { @@ -770,9 +781,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { explanation ); let err = match (place_desc, explanation) { - (Some(_), _) if self.is_place_thread_local(root_place) => { - self.report_thread_local_value_does_not_live_long_enough(drop_span, borrow_span) - } // If the outlives constraint comes from inside the closure, // for example: // @@ -1509,19 +1517,22 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // place being assigned later. let (place_description, assigned_span) = match local_decl { Some(LocalDecl { - is_user_variable: Some(ClearCrossCrate::Clear), + local_info: LocalInfo::User(ClearCrossCrate::Clear), .. }) | Some(LocalDecl { - is_user_variable: - Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { - opt_match_place: None, - .. - }))), + local_info: LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { + opt_match_place: None, + .. + }))), + .. + }) + | Some(LocalDecl { + local_info: LocalInfo::StaticRef { .. }, .. }) | Some(LocalDecl { - is_user_variable: None, + local_info: LocalInfo::Other, .. }) | None => (self.describe_place(place.as_ref()), assigned_span), diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 4036e9db33b34..3835503b0ef35 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -3,7 +3,7 @@ use rustc::hir::def::Namespace; use rustc::hir::def_id::DefId; use rustc::hir::GeneratorKind; use rustc::mir::{ - AggregateKind, Constant, Field, Local, LocalKind, Location, Operand, + AggregateKind, Constant, Field, Local, LocalInfo, LocalKind, Location, Operand, Place, PlaceBase, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Static, StaticKind, Terminator, TerminatorKind, }; @@ -12,7 +12,6 @@ use rustc::ty::layout::VariantIdx; use rustc::ty::print::Print; use rustc_errors::DiagnosticBuilder; use syntax_pos::Span; -use syntax::symbol::sym; use super::borrow_set::BorrowData; use super::MirBorrowckCtxt; @@ -178,6 +177,31 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } => { buf.push_str(&self.infcx.tcx.item_name(*def_id).to_string()); } + PlaceRef { + base: &PlaceBase::Local(local), + projection: [ProjectionElem::Deref] + } if self.body.local_decls[local].is_ref_for_guard() => { + self.append_place_to_string( + PlaceRef { + base: &PlaceBase::Local(local), + projection: &[], + }, + buf, + autoderef, + &including_downcast, + )?; + }, + PlaceRef { + base: &PlaceBase::Local(local), + projection: [ProjectionElem::Deref] + } if self.body.local_decls[local].is_ref_to_static() => { + let local_info = &self.body.local_decls[local].local_info; + if let LocalInfo::StaticRef { def_id, .. } = *local_info { + buf.push_str(&self.infcx.tcx.item_name(def_id).as_str()); + } else { + unreachable!(); + } + }, PlaceRef { base, projection: [proj_base @ .., elem], @@ -208,32 +232,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { )?; } else { match (proj_base, base) { - ([], PlaceBase::Local(local)) => { - if self.body.local_decls[*local].is_ref_for_guard() { - self.append_place_to_string( - PlaceRef { - base, - projection: proj_base, - }, - buf, - autoderef, - &including_downcast, - )?; - } else { - // FIXME deduplicate this and the _ => body below - buf.push_str(&"*"); - self.append_place_to_string( - PlaceRef { - base, - projection: proj_base, - }, - buf, - autoderef, - &including_downcast, - )?; - } - } - _ => { buf.push_str(&"*"); self.append_place_to_string( @@ -440,30 +438,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - /// Checks if a place is a thread-local static. - pub fn is_place_thread_local(&self, place_ref: PlaceRef<'cx, 'tcx>) -> bool { - if let PlaceRef { - base: PlaceBase::Static(box Static { - kind: StaticKind::Static, - def_id, - .. - }), - projection: [], - } = place_ref { - let attrs = self.infcx.tcx.get_attrs(*def_id); - let is_thread_local = attrs.iter().any(|attr| attr.check_name(sym::thread_local)); - - debug!( - "is_place_thread_local: attrs={:?} is_thread_local={:?}", - attrs, is_thread_local - ); - is_thread_local - } else { - debug!("is_place_thread_local: no"); - false - } - } - /// Add a note that a type does not implement `Copy` pub(super) fn note_type_does_not_implement_copy( &self, diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index b8b4a1053e5b0..8e34eb6f413d8 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -308,7 +308,7 @@ fn do_mir_borrowck<'a, 'tcx>( // would have a chance of erroneously adding non-user-defined mutable vars // to the set. let temporary_used_locals: FxHashSet = mbcx.used_mut.iter() - .filter(|&local| mbcx.body.local_decls[*local].is_user_variable.is_none()) + .filter(|&local| !mbcx.body.local_decls[*local].is_user_variable()) .cloned() .collect(); // For the remaining unused locals that are marked as mutable, we avoid linting any that @@ -1287,7 +1287,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { match *operand { Operand::Move(ref place) | Operand::Copy(ref place) => { match place.as_local() { - Some(local) if self.body.local_decls[local].is_user_variable.is_none() => { + Some(local) if !self.body.local_decls[local].is_user_variable() => { if self.body.local_decls[local].ty.is_mutable_ptr() { // The variable will be marked as mutable by the borrow. return; @@ -1399,7 +1399,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ) { debug!("check_for_invalidation_at_exit({:?})", borrow); let place = &borrow.borrowed_place; - let root_place = self.prefixes(place.as_ref(), PrefixSet::All).last().unwrap(); + let deref = [ProjectionElem::Deref]; + let mut root_place = PlaceRef { base: &place.base, projection: &[] }; // FIXME(nll-rfc#40): do more precise destructor tracking here. For now // we just know that all locals are dropped at function exit (otherwise @@ -1407,26 +1408,20 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // // FIXME: allow thread-locals to borrow other thread locals? - assert!(root_place.projection.is_empty()); let (might_be_alive, will_be_dropped) = match root_place.base { - PlaceBase::Static(box Static { - kind: StaticKind::Promoted(..), - .. - }) => { + PlaceBase::Static(_) => { (true, false) } - PlaceBase::Static(box Static { - kind: StaticKind::Static, - .. - }) => { - // Thread-locals might be dropped after the function exits, but - // "true" statics will never be. - (true, self.is_place_thread_local(root_place)) - } - PlaceBase::Local(_) => { - // Locals are always dropped at function exit, and if they - // have a destructor it would've been called already. - (false, self.locals_are_invalidated_at_exit) + PlaceBase::Local(local) => { + if self.body.local_decls[*local].is_ref_to_thread_local() { + // Thread-locals might be dropped after the function exits + // We have to dereference the outer reference because + // borrows don't conflict behind shared references. + root_place.projection = &deref; + (true, true) + } else { + (false, self.locals_are_invalidated_at_exit) + } } }; diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs index d9e958d945001..b1f63d729ba9b 100644 --- a/src/librustc_mir/borrow_check/move_errors.rs +++ b/src/librustc_mir/borrow_check/move_errors.rs @@ -104,13 +104,14 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { // // opt_match_place is None for let [mut] x = ... statements, // whether or not the right-hand side is a place expression - if let Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { - opt_match_place: Some((ref opt_match_place, match_span)), - binding_mode: _, - opt_ty_info: _, - pat_span: _, - }))) = local_decl.is_user_variable - { + if let LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var( + VarBindingForm { + opt_match_place: Some((ref opt_match_place, match_span)), + binding_mode: _, + opt_ty_info: _, + pat_span: _, + }, + ))) = local_decl.local_info { let stmt_source_info = self.body.source_info(location); self.append_binding_error( grouped_errors, @@ -242,7 +243,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { ( match kind { IllegalMoveOriginKind::Static => { - self.report_cannot_move_from_static(original_path, span) + unreachable!(); } IllegalMoveOriginKind::BorrowedContent { target_place } => { self.report_cannot_move_from_borrowed_content( @@ -272,12 +273,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { place: &Place<'tcx>, span: Span ) -> DiagnosticBuilder<'a> { - let description = if place.projection.is_empty() { + let description = if place.projection.len() == 1 { format!("static item `{}`", self.describe_place(place.as_ref()).unwrap()) } else { let base_static = PlaceRef { base: &place.base, - projection: &place.projection[..1], + projection: &[ProjectionElem::Deref], }; format!( @@ -327,6 +328,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { "variables bound in patterns cannot be moved from \ until after the end of the pattern guard"); return err; + } else if decl.is_ref_to_static() { + return self.report_cannot_move_from_static(move_place, span); } } @@ -508,12 +511,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { let mut suggestions: Vec<(Span, &str, String)> = Vec::new(); for local in binds_to { let bind_to = &self.body.local_decls[*local]; - if let Some( + if let LocalInfo::User( ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { pat_span, .. })) - ) = bind_to.is_user_variable { + ) = bind_to.local_info { if let Ok(pat_snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(pat_span) { if pat_snippet.starts_with('&') { diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index 11e89de810e5b..404684c07a09c 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -1,9 +1,7 @@ use rustc::hir; use rustc::hir::Node; -use rustc::mir::{self, BindingForm, ClearCrossCrate, Local, Location, Body}; -use rustc::mir::{ - Mutability, Place, PlaceRef, PlaceBase, ProjectionElem, Static, StaticKind -}; +use rustc::mir::{self, Body, ClearCrossCrate, Local, LocalInfo, Location}; +use rustc::mir::{Mutability, Place, PlaceRef, PlaceBase, ProjectionElem}; use rustc::ty::{self, Ty, TyCtxt}; use rustc_index::vec::Idx; use syntax_pos::Span; @@ -76,6 +74,31 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } } + PlaceRef { + base: &PlaceBase::Local(local), + projection: [ProjectionElem::Deref], + } if self.body.local_decls[local].is_ref_for_guard() => { + item_msg = format!("`{}`", access_place_desc.unwrap()); + reason = ", as it is immutable for the pattern guard".to_string(); + } + PlaceRef { + base: &PlaceBase::Local(local), + projection: [ProjectionElem::Deref], + } if self.body.local_decls[local].is_ref_to_static() => { + if access_place.projection.len() == 1 { + item_msg = format!("immutable static item `{}`", access_place_desc.unwrap()); + reason = String::new(); + } else { + item_msg = format!("`{}`", access_place_desc.unwrap()); + let local_info = &self.body.local_decls[local].local_info; + if let LocalInfo::StaticRef { def_id, .. } = *local_info { + let static_name = &self.infcx.tcx.item_name(def_id); + reason = format!(", as `{}` is an immutable static item", static_name); + } else { + bug!("is_ref_to_static return true, but not ref to static?"); + } + } + } PlaceRef { base: _, projection: [proj_base @ .., ProjectionElem::Deref], @@ -101,15 +124,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } else { ", as `Fn` closures cannot mutate their captured variables".to_string() } - } else if { - if let (PlaceBase::Local(local), []) = (&the_place_err.base, proj_base) { - self.body.local_decls[*local].is_ref_for_guard() - } else { - false - } - } { - item_msg = format!("`{}`", access_place_desc.unwrap()); - reason = ", as it is immutable for the pattern guard".to_string(); } else { let source = self.borrowed_content_source(PlaceRef { base: the_place_err.base, @@ -133,37 +147,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } PlaceRef { - base: - PlaceBase::Static(box Static { - kind: StaticKind::Promoted(..), - .. - }), - projection: [], - } => unreachable!(), - - PlaceRef { - base: - PlaceBase::Static(box Static { - kind: StaticKind::Static, - def_id, - .. - }), - projection: [], - } => { - if let PlaceRef { - base: &PlaceBase::Static(_), - projection: &[], - } = access_place.as_ref() { - item_msg = format!("immutable static item `{}`", access_place_desc.unwrap()); - reason = String::new(); - } else { - item_msg = format!("`{}`", access_place_desc.unwrap()); - let static_name = &self.infcx.tcx.item_name(*def_id); - reason = format!(", as `{}` is an immutable static item", static_name); - } + base: PlaceBase::Static(_), + .. } - - PlaceRef { + | PlaceRef { base: _, projection: [.., ProjectionElem::Index(_)], } @@ -257,15 +244,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { projection: [], } if { self.body.local_decls.get(*local).map(|local_decl| { - if let ClearCrossCrate::Set( + if let LocalInfo::User(ClearCrossCrate::Set( mir::BindingForm::ImplicitSelf(kind) - ) = local_decl.is_user_variable.as_ref().unwrap() { + )) = local_decl.local_info { // Check if the user variable is a `&mut self` and we can therefore // suggest removing the `&mut`. // // Deliberately fall into this case for all implicit self types, // so that we don't fall in to the next case with them. - *kind == mir::ImplicitSelfKind::MutRef + kind == mir::ImplicitSelfKind::MutRef } else if Some(kw::SelfLower) == local_decl.name { // Otherwise, check if the name is the self kewyord - in which case // we have an explicit self. Do the same thing in this case and check @@ -360,16 +347,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { PlaceRef { base: PlaceBase::Local(local), projection: [ProjectionElem::Deref], - } if { - if let Some(ClearCrossCrate::Set(BindingForm::RefForGuard)) = - self.body.local_decls[*local].is_user_variable - { - true - } else { - false - } - } => - { + } if self.body.local_decls[*local].is_ref_for_guard() => { err.span_label(span, format!("cannot {ACT}", ACT = act)); err.note( "variables bound in patterns are immutable until the end of the pattern guard", @@ -384,38 +362,42 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { PlaceRef { base: PlaceBase::Local(local), projection: [ProjectionElem::Deref], - } if self.body.local_decls[*local].is_user_variable.is_some() => + } if self.body.local_decls[*local].is_user_variable() => { let local_decl = &self.body.local_decls[*local]; - let suggestion = match local_decl.is_user_variable.as_ref().unwrap() { - ClearCrossCrate::Set(mir::BindingForm::ImplicitSelf(_)) => { + let suggestion = match local_decl.local_info { + LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::ImplicitSelf(_))) => { Some(suggest_ampmut_self(self.infcx.tcx, local_decl)) } - ClearCrossCrate::Set(mir::BindingForm::Var(mir::VarBindingForm { - binding_mode: ty::BindingMode::BindByValue(_), - opt_ty_info, - .. - })) => Some(suggest_ampmut( + LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var( + mir::VarBindingForm { + binding_mode: ty::BindingMode::BindByValue(_), + opt_ty_info, + .. + }, + ))) => Some(suggest_ampmut( self.infcx.tcx, self.body, *local, local_decl, - *opt_ty_info, + opt_ty_info, )), - ClearCrossCrate::Set(mir::BindingForm::Var(mir::VarBindingForm { - binding_mode: ty::BindingMode::BindByReference(_), - .. - })) => { + LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var( + mir::VarBindingForm { + binding_mode: ty::BindingMode::BindByReference(_), + .. + }, + ))) => { let pattern_span = local_decl.source_info.span; suggest_ref_mut(self.infcx.tcx, pattern_span) .map(|replacement| (pattern_span, replacement)) } - ClearCrossCrate::Set(mir::BindingForm::RefForGuard) => unreachable!(), + LocalInfo::User(ClearCrossCrate::Clear) => bug!("saw cleared local state"), - ClearCrossCrate::Clear => bug!("saw cleared local state"), + _ => unreachable!(), }; let (pointer_sigil, pointer_desc) = if local_decl.ty.is_region_ptr() { diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index 9f20a24a183c9..99bcfa9bc2599 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -1387,7 +1387,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { } else { ConstraintCategory::Return }, - Some(l) if !body.local_decls[l].is_user_variable.is_some() => { + Some(l) if !body.local_decls[l].is_user_variable() => { ConstraintCategory::Boring } _ => ConstraintCategory::Assignment, @@ -1693,7 +1693,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ConstraintCategory::Return } } - Some(l) if !body.local_decls[l].is_user_variable.is_some() => { + Some(l) if !body.local_decls[l].is_user_variable() => { ConstraintCategory::Boring } _ => ConstraintCategory::Assignment, diff --git a/src/librustc_mir/borrow_check/place_ext.rs b/src/librustc_mir/borrow_check/place_ext.rs index f0d2927ba45e7..c62de2af55f44 100644 --- a/src/librustc_mir/borrow_check/place_ext.rs +++ b/src/librustc_mir/borrow_check/place_ext.rs @@ -1,6 +1,6 @@ use rustc::hir; use rustc::mir::ProjectionElem; -use rustc::mir::{Body, Place, PlaceBase, Mutability, Static, StaticKind}; +use rustc::mir::{Body, Place, PlaceBase, Mutability}; use rustc::ty::{self, TyCtxt}; use crate::borrow_check::borrow_set::LocalsStateAtExit; @@ -25,7 +25,7 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> { body: &Body<'tcx>, locals_state_at_exit: &LocalsStateAtExit, ) -> bool { - let ignore = match self.base { + let local = match self.base { // If a local variable is immutable, then we only need to track borrows to guard // against two kinds of errors: // * The variable being dropped while still borrowed (e.g., because the fn returns @@ -34,22 +34,22 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> { // // In particular, the variable cannot be mutated -- the "access checks" will fail -- // so we don't have to worry about mutation while borrowed. - PlaceBase::Local(index) => { + PlaceBase::Local(local) => { match locals_state_at_exit { - LocalsStateAtExit::AllAreInvalidated => false, + LocalsStateAtExit::AllAreInvalidated => local, LocalsStateAtExit::SomeAreInvalidated { has_storage_dead_or_moved } => { - let ignore = !has_storage_dead_or_moved.contains(index) && - body.local_decls[index].mutability == Mutability::Not; - debug!("ignore_borrow: local {:?} => {:?}", index, ignore); - ignore + let ignore = !has_storage_dead_or_moved.contains(local) && + body.local_decls[local].mutability == Mutability::Not; + debug!("ignore_borrow: local {:?} => {:?}", local, ignore); + if ignore { + return true; + } else { + local + } } } } - PlaceBase::Static(box Static{ kind: StaticKind::Promoted(_, _), .. }) => - false, - PlaceBase::Static(box Static{ kind: StaticKind::Static, def_id, .. }) => { - tcx.is_mutable_static(def_id) - } + PlaceBase::Static(_) => return true, }; for (i, elem) in self.projection.iter().enumerate() { @@ -57,22 +57,33 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> { if *elem == ProjectionElem::Deref { let ty = Place::ty_from(&self.base, proj_base, body, tcx).ty; - if let ty::RawPtr(..) | ty::Ref(_, _, hir::Mutability::Immutable) = ty.kind { - // For both derefs of raw pointers and `&T` - // references, the original path is `Copy` and - // therefore not significant. In particular, - // there is nothing the user can do to the - // original path that would invalidate the - // newly created reference -- and if there - // were, then the user could have copied the - // original path into a new variable and - // borrowed *that* one, leaving the original - // path unborrowed. - return true; + match ty.kind { + ty::Ref(_, _, hir::Mutability::Immutable) if i == 0 => { + // For references to thread-local statics, we do need + // to track the borrow. + if body.local_decls[local].is_ref_to_thread_local() { + continue; + } + return true; + } + ty::RawPtr(..) | ty::Ref(_, _, hir::Mutability::Immutable) => { + // For both derefs of raw pointers and `&T` + // references, the original path is `Copy` and + // therefore not significant. In particular, + // there is nothing the user can do to the + // original path that would invalidate the + // newly created reference -- and if there + // were, then the user could have copied the + // original path into a new variable and + // borrowed *that* one, leaving the original + // path unborrowed. + return true; + } + _ => {} } } } - ignore + false } } diff --git a/src/librustc_mir/build/expr/as_constant.rs b/src/librustc_mir/build/expr/as_constant.rs index 39bdc871d83c6..6db7ec65096ec 100644 --- a/src/librustc_mir/build/expr/as_constant.rs +++ b/src/librustc_mir/build/expr/as_constant.rs @@ -45,6 +45,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { literal, } }, + ExprKind::StaticRef { literal, .. } => { + Constant { + span, + user_ty: None, + literal, + } + } _ => span_bug!(span, "expression is not a valid constant {:?}", kind), } } diff --git a/src/librustc_mir/build/expr/as_place.rs b/src/librustc_mir/build/expr/as_place.rs index aed4759322cb9..f66f1cb73666a 100644 --- a/src/librustc_mir/build/expr/as_place.rs +++ b/src/librustc_mir/build/expr/as_place.rs @@ -285,6 +285,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::Continue { .. } | ExprKind::Return { .. } | ExprKind::Literal { .. } + | ExprKind::StaticRef { .. } | ExprKind::InlineAsm { .. } | ExprKind::Yield { .. } | ExprKind::Call { .. } => { diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index 3bbd8093d3bae..37eb0cc9d961e 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -270,6 +270,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { resume.and(this.unit_rvalue()) } ExprKind::Literal { .. } + | ExprKind::StaticRef { .. } | ExprKind::Block { .. } | ExprKind::Match { .. } | ExprKind::NeverToAny { .. } diff --git a/src/librustc_mir/build/expr/as_temp.rs b/src/librustc_mir/build/expr/as_temp.rs index 18332ed68f8bd..864b449c29c9e 100644 --- a/src/librustc_mir/build/expr/as_temp.rs +++ b/src/librustc_mir/build/expr/as_temp.rs @@ -6,6 +6,7 @@ use crate::hair::*; use rustc::hir; use rustc::middle::region; use rustc::mir::*; +use syntax_pos::symbol::sym; impl<'a, 'tcx> Builder<'a, 'tcx> { /// Compile `expr` into a fresh temporary. This is used when building @@ -63,6 +64,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { if let Some(tail_info) = this.block_context.currently_in_block_tail() { local_decl = local_decl.block_tail(tail_info); } + if let ExprKind::StaticRef { def_id, .. } = expr.kind { + let attrs = this.hir.tcx().get_attrs(def_id); + let is_thread_local = attrs.iter().any(|attr| attr.check_name(sym::thread_local)); + local_decl.local_info = LocalInfo::StaticRef {def_id, is_thread_local }; + } this.local_decls.push(local_decl) }; let temp_place = &Place::from(temp); diff --git a/src/librustc_mir/build/expr/category.rs b/src/librustc_mir/build/expr/category.rs index e7b68acc2ef97..270a1a6447435 100644 --- a/src/librustc_mir/build/expr/category.rs +++ b/src/librustc_mir/build/expr/category.rs @@ -65,7 +65,8 @@ impl Category { | ExprKind::Yield { .. } | ExprKind::InlineAsm { .. } => Some(Category::Rvalue(RvalueFunc::AsRvalue)), - ExprKind::Literal { .. } => Some(Category::Constant), + ExprKind::Literal { .. } + | ExprKind::StaticRef { .. } => Some(Category::Constant), ExprKind::Loop { .. } | ExprKind::Block { .. } diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 1a19878a1f18e..e991181189f41 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -231,7 +231,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { source_info, visibility_scope: source_info.scope, internal: true, - is_user_variable: None, + local_info: LocalInfo::Other, is_block_tail: None, }); let ptr_temp = Place::from(ptr_temp); @@ -425,6 +425,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::Tuple { .. } | ExprKind::Closure { .. } | ExprKind::Literal { .. } + | ExprKind::StaticRef { .. } | ExprKind::Yield { .. } => { debug_assert!(match Category::of(&expr.kind).unwrap() { // should be handled above diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index 667b37bbd80c8..ada547aa39c9e 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -458,10 +458,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { for binding in &candidate.bindings { let local = self.var_local_id(binding.var_id, OutsideGuard); - if let Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { + if let LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { opt_match_place: Some((ref mut match_place, _)), .. - }))) = self.local_decls[local].is_user_variable + }))) = self.local_decls[local].local_info { *match_place = Some(initializer.clone()); } else { @@ -1734,16 +1734,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { visibility_scope, internal: false, is_block_tail: None, - is_user_variable: Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { - binding_mode, - // hypothetically, `visit_bindings` could try to unzip - // an outermost hir::Ty as we descend, matching up - // idents in pat; but complex w/ unclear UI payoff. - // Instead, just abandon providing diagnostic info. - opt_ty_info: None, - opt_match_place, - pat_span, - }))), + local_info: LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var( + VarBindingForm { + binding_mode, + // hypothetically, `visit_bindings` could try to unzip + // an outermost hir::Ty as we descend, matching up + // idents in pat; but complex w/ unclear UI payoff. + // Instead, just abandon providing diagnostic info. + opt_ty_info: None, + opt_match_place, + pat_span, + }, + ))), }; let for_arm_body = self.local_decls.push(local); let locals = if has_guard.0 { @@ -1758,7 +1760,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { visibility_scope, internal: false, is_block_tail: None, - is_user_variable: Some(ClearCrossCrate::Set(BindingForm::RefForGuard)), + local_info: LocalInfo::User(ClearCrossCrate::Set(BindingForm::RefForGuard)), }); LocalsForNode::ForGuard { ref_for_guard, diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index fb605bb2b557b..6b458cc244c9e 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -820,7 +820,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { visibility_scope: source_info.scope, name, internal: false, - is_user_variable: None, + local_info: LocalInfo::Other, is_block_tail: None, }); } @@ -855,17 +855,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } => { self.local_decls[local].mutability = mutability; self.local_decls[local].source_info.scope = self.source_scope; - self.local_decls[local].is_user_variable = + self.local_decls[local].local_info = if let Some(kind) = self_binding { - Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf(*kind))) + LocalInfo::User(ClearCrossCrate::Set( + BindingForm::ImplicitSelf(*kind), + )) } else { let binding_mode = ty::BindingMode::BindByValue(mutability.into()); - Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { - binding_mode, - opt_ty_info, - opt_match_place: Some((Some(place.clone()), span)), - pat_span: span, - }))) + LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var( + VarBindingForm { + binding_mode, + opt_ty_info, + opt_match_place: Some((Some(place.clone()), span)), + pat_span: span, + }, + ))) }; self.var_indices.insert(var, LocalsForNode::One(local)); } diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index 5e64144df2cd1..402e5aeacbf24 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -209,7 +209,9 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { // local must conflict. This is purely an optimization so we don't have to call // `places_conflict` for every borrow. if place.projection.is_empty() { - trans.kill_all(other_borrows_of_local); + if !self.body.local_decls[local].is_ref_to_static() { + trans.kill_all(other_borrows_of_local); + } return; } diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index 28794859c564d..f47c92cbd542d 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -264,6 +264,11 @@ pub enum ExprKind<'tcx> { literal: &'tcx Const<'tcx>, user_ty: Option>>, }, + /// A literal containing the address of a `static` + StaticRef { + literal: &'tcx Const<'tcx>, + def_id: DefId, + }, InlineAsm { asm: &'tcx hir::InlineAsmInner, outputs: Vec>, diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 2913d6e59eb3f..17f5e3d4e47a9 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -152,7 +152,7 @@ fn temp_decl(mutability: Mutability, ty: Ty<'_>, span: Span) -> LocalDecl<'_> { source_info, visibility_scope: source_info.scope, internal: false, - is_user_variable: None, + local_info: LocalInfo::Other, is_block_tail: None, } } diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 772f27fb7e14f..bee37f69a5ec5 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -1,6 +1,6 @@ //! The `Visitor` responsible for actually checking a `mir::Body` for invalid operations. -use rustc::hir::HirId; +use rustc::hir::{HirId, def_id::DefId}; use rustc::middle::lang_items; use rustc::mir::visit::{PlaceContext, Visitor, MutatingUseContext, NonMutatingUseContext}; use rustc::mir::*; @@ -288,6 +288,15 @@ impl Validator<'a, 'mir, 'tcx> { let span = self.span; self.check_op_spanned(op, span) } + + fn check_static(&mut self, def_id: DefId, span: Span) -> CheckOpResult { + let is_thread_local = self.tcx.has_attr(def_id, sym::thread_local); + if is_thread_local { + self.check_op_spanned(ops::ThreadLocalAccess, span) + } else { + self.check_op_spanned(ops::StaticAccess, span) + } + } } impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { @@ -422,12 +431,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { self.super_operand(op, location); if let Operand::Constant(c) = op { if let Some(def_id) = c.check_static_ptr(self.tcx) { - let is_thread_local = self.tcx.has_attr(def_id, sym::thread_local); - if is_thread_local { - self.check_op(ops::ThreadLocalAccess); - } else { - self.check_op(ops::StaticAccess); - } + self.check_static(def_id, self.span); } } } @@ -506,14 +510,24 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { 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.kind { + if proj_base.is_empty() { + if let (PlaceBase::Local(local), []) = (place_base, proj_base) { + let decl = &self.body.local_decls[*local]; + if let LocalInfo::StaticRef { def_id, .. } = decl.local_info { + let span = decl.source_info.span; + self.check_static(def_id, span); + return; + } + } + } self.check_op(ops::RawPtrDeref); } + + if context.is_mutating_use() { + self.check_op(ops::MutDeref); + } } ProjectionElem::ConstantIndex {..} | diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index 9374109c82e41..b7cc4e9fcf66c 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -206,25 +206,10 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { // Locals are safe. } PlaceBase::Static(box Static { kind: StaticKind::Promoted(_, _), .. }) => { - bug!("unsafety checking should happen before promotion") + bug!("unsafety checking should happen before promotion"); } - PlaceBase::Static(box Static { kind: StaticKind::Static, def_id, .. }) => { - if self.tcx.is_mutable_static(def_id) { - self.require_unsafe( - "use of mutable static", - "mutable statics can be mutated by multiple threads: aliasing \ - violations or data races will cause undefined behavior", - UnsafetyViolationKind::General, - ); - } else if self.tcx.is_foreign_item(def_id) { - self.require_unsafe( - "use of extern static", - "extern statics are not controlled by the Rust type system: \ - invalid data, aliasing violations or data races will cause \ - undefined behavior", - UnsafetyViolationKind::General, - ); - } + PlaceBase::Static(box Static { kind: StaticKind::Static, .. }) => { + bug!("StaticKind::Static should not exist"); } } @@ -264,11 +249,31 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { } let old_source_info = self.source_info; if let (PlaceBase::Local(local), []) = (&place.base, proj_base) { - if self.body.local_decls[*local].internal { + let decl = &self.body.local_decls[*local]; + if decl.internal { // Internal locals are used in the `move_val_init` desugaring. // We want to check unsafety against the source info of the // desugaring, rather than the source info of the RHS. self.source_info = self.body.local_decls[*local].source_info; + } else if let LocalInfo::StaticRef { def_id, .. } = decl.local_info { + if self.tcx.is_mutable_static(def_id) { + self.require_unsafe( + "use of mutable static", + "mutable statics can be mutated by multiple threads: aliasing \ + violations or data races will cause undefined behavior", + UnsafetyViolationKind::General, + ); + return; + } else if self.tcx.is_foreign_item(def_id) { + self.require_unsafe( + "use of extern static", + "extern statics are not controlled by the Rust type system: \ + invalid data, aliasing violations or data races will cause \ + undefined behavior", + UnsafetyViolationKind::General, + ); + return; + } } } let base_ty = Place::ty_from(&place.base, proj_base, self.body, self.tcx).ty; diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 37c239001a505..524b6b087908c 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -432,7 +432,7 @@ fn replace_result_variable<'tcx>( visibility_scope: source_info.scope, internal: false, is_block_tail: None, - is_user_variable: None, + local_info: LocalInfo::Other }; let new_ret_local = Local::new(body.local_decls.len()); body.local_decls.push(new_ret); @@ -967,7 +967,7 @@ fn create_generator_drop_shim<'tcx>( visibility_scope: source_info.scope, internal: false, is_block_tail: None, - is_user_variable: None, + local_info: LocalInfo::Other }; make_generator_state_argument_indirect(tcx, def_id, &mut body); @@ -985,7 +985,7 @@ fn create_generator_drop_shim<'tcx>( visibility_scope: source_info.scope, internal: false, is_block_tail: None, - is_user_variable: None, + local_info: LocalInfo::Other }; if tcx.sess.opts.debugging_opts.mir_emit_retag { // Alias tracking must know we changed the type diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index c79d382a37480..86ecfbb4fbea5 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -512,34 +512,9 @@ impl<'tcx> Validator<'_, 'tcx> { projection: [], } => self.validate_local(*local), PlaceRef { - base: PlaceBase::Static(box Static { - kind: StaticKind::Promoted { .. }, - .. - }), + base: PlaceBase::Static(_), projection: [], } => bug!("qualifying already promoted MIR"), - PlaceRef { - base: PlaceBase::Static(box Static { - kind: StaticKind::Static, - def_id, - .. - }), - projection: [], - } => { - // Only allow statics (not consts) to refer to other statics. - // FIXME(eddyb) does this matter at all for promotion? - let is_static = self.const_kind.map_or(false, |k| k.is_static()); - if !is_static { - return Err(Unpromotable); - } - - let is_thread_local = self.tcx.has_attr(*def_id, sym::thread_local); - if is_thread_local { - return Err(Unpromotable); - } - - Ok(()) - } PlaceRef { base: _, projection: [proj_base @ .., elem], @@ -584,7 +559,23 @@ impl<'tcx> Validator<'_, 'tcx> { // The qualifs for a constant (e.g. `HasMutInterior`) are checked in // `validate_rvalue` upon access. - Operand::Constant(_) => Ok(()), + Operand::Constant(c) => { + if let Some(def_id) = c.check_static_ptr(self.tcx) { + // Only allow statics (not consts) to refer to other statics. + // FIXME(eddyb) does this matter at all for promotion? + let is_static = self.const_kind.map_or(false, |k| k.is_static()); + if !is_static { + return Err(Unpromotable); + } + + let is_thread_local = self.tcx.has_attr(def_id, sym::thread_local); + if is_thread_local { + return Err(Unpromotable); + } + } + + Ok(()) + }, } } From 025630d189c66a83b6f334e715cec6690cb85adf Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Tue, 19 Nov 2019 23:16:58 +0000 Subject: [PATCH 7/8] Bless remaining test output --- .../const_prop/read_immutable_static.rs | 12 ++++--- src/test/ui/consts/projection_qualif.stderr | 10 +++--- .../issue-17718-const-bad-values.stderr | 10 +++--- .../ui/issues/issue-17718-references.stderr | 4 +-- src/test/ui/issues/issue-18118-2.stderr | 4 +-- src/test/ui/thread-local-in-ctfe.rs | 2 -- src/test/ui/thread-local-in-ctfe.stderr | 31 +++++-------------- 7 files changed, 28 insertions(+), 45 deletions(-) diff --git a/src/test/mir-opt/const_prop/read_immutable_static.rs b/src/test/mir-opt/const_prop/read_immutable_static.rs index c2902dbd7c129..d14ec0397166f 100644 --- a/src/test/mir-opt/const_prop/read_immutable_static.rs +++ b/src/test/mir-opt/const_prop/read_immutable_static.rs @@ -10,10 +10,12 @@ fn main() { // START rustc.main.ConstProp.before.mir // bb0: { // ... -// _2 = (FOO: u8); +// _3 = const Scalar(AllocId(0).0x0) : &u8; +// _2 = (*_3); // ... -// _3 = (FOO: u8); -// _1 = Add(move _2, move _3); +// _5 = const Scalar(AllocId(0).0x0) : &u8; +// _4 = (*_5); +// _1 = Add(move _2, move _4); // ... // } // END rustc.main.ConstProp.before.mir @@ -22,8 +24,8 @@ fn main() { // ... // _2 = const 2u8; // ... -// _3 = const 2u8; -// _1 = Add(move _2, move _3); +// _4 = const 2u8; +// _1 = Add(move _2, move _4); // ... // } // END rustc.main.ConstProp.after.mir diff --git a/src/test/ui/consts/projection_qualif.stderr b/src/test/ui/consts/projection_qualif.stderr index 0c09f7ec52fa2..0efb6bfd10a17 100644 --- a/src/test/ui/consts/projection_qualif.stderr +++ b/src/test/ui/consts/projection_qualif.stderr @@ -4,20 +4,20 @@ error[E0017]: references in constants may only refer to immutable values LL | let b: *mut u32 = &mut a; | ^^^^^^ constants require immutable values -error[E0019]: constant contains unimplemented expression type +error[E0658]: dereferencing raw pointers in constants is unstable --> $DIR/projection_qualif.rs:7:18 | LL | unsafe { *b = 5; } | ^^^^^^ + | + = note: for more information, see https://github.com/rust-lang/rust/issues/51911 + = help: add `#![feature(const_raw_ptr_deref)]` to the crate attributes to enable -error[E0658]: dereferencing raw pointers in constants is unstable +error[E0019]: constant contains unimplemented expression type --> $DIR/projection_qualif.rs:7:18 | LL | unsafe { *b = 5; } | ^^^^^^ - | - = note: for more information, see https://github.com/rust-lang/rust/issues/51911 - = help: add `#![feature(const_raw_ptr_deref)]` to the crate attributes to enable error: aborting due to 3 previous errors diff --git a/src/test/ui/issues/issue-17718-const-bad-values.stderr b/src/test/ui/issues/issue-17718-const-bad-values.stderr index 7a49e89a1af70..8d875d37d85f9 100644 --- a/src/test/ui/issues/issue-17718-const-bad-values.stderr +++ b/src/test/ui/issues/issue-17718-const-bad-values.stderr @@ -4,17 +4,17 @@ error[E0017]: references in constants may only refer to immutable values LL | const C1: &'static mut [usize] = &mut []; | ^^^^^^^ constants require immutable values -error[E0017]: references in constants may only refer to immutable values - --> $DIR/issue-17718-const-bad-values.rs:5:41 +error[E0013]: constants cannot refer to statics, use a constant instead + --> $DIR/issue-17718-const-bad-values.rs:5:46 | LL | const C2: &'static mut usize = unsafe { &mut S }; - | ^^^^^^ constants require immutable values + | ^ -error[E0013]: constants cannot refer to statics, use a constant instead +error[E0017]: references in constants may only refer to immutable values --> $DIR/issue-17718-const-bad-values.rs:5:41 | LL | const C2: &'static mut usize = unsafe { &mut S }; - | ^^^^^^ + | ^^^^^^ constants require immutable values error: aborting due to 3 previous errors diff --git a/src/test/ui/issues/issue-17718-references.stderr b/src/test/ui/issues/issue-17718-references.stderr index 15c3e67f7a1a6..27aad9c03cebe 100644 --- a/src/test/ui/issues/issue-17718-references.stderr +++ b/src/test/ui/issues/issue-17718-references.stderr @@ -1,8 +1,8 @@ error[E0013]: constants cannot refer to statics, use a constant instead - --> $DIR/issue-17718-references.rs:9:28 + --> $DIR/issue-17718-references.rs:9:29 | LL | const T2: &'static usize = &S; - | ^^ + | ^ error[E0013]: constants cannot refer to statics, use a constant instead --> $DIR/issue-17718-references.rs:14:19 diff --git a/src/test/ui/issues/issue-18118-2.stderr b/src/test/ui/issues/issue-18118-2.stderr index 4e848c261be33..d58822f16eb35 100644 --- a/src/test/ui/issues/issue-18118-2.stderr +++ b/src/test/ui/issues/issue-18118-2.stderr @@ -1,8 +1,8 @@ error[E0013]: constants cannot refer to statics, use a constant instead - --> $DIR/issue-18118-2.rs:4:9 + --> $DIR/issue-18118-2.rs:4:10 | LL | &p - | ^^ + | ^ error: aborting due to previous error diff --git a/src/test/ui/thread-local-in-ctfe.rs b/src/test/ui/thread-local-in-ctfe.rs index 722c3883fdda4..313d39de36b6a 100644 --- a/src/test/ui/thread-local-in-ctfe.rs +++ b/src/test/ui/thread-local-in-ctfe.rs @@ -8,14 +8,12 @@ static B: u32 = A; static C: &u32 = &A; //~^ ERROR thread-local statics cannot be accessed at compile-time -//~| ERROR thread-local variable borrowed past end of function const D: u32 = A; //~^ ERROR thread-local statics cannot be accessed at compile-time const E: &u32 = &A; //~^ ERROR thread-local statics cannot be accessed at compile-time -//~| ERROR thread-local variable borrowed past end of function const fn f() -> u32 { A diff --git a/src/test/ui/thread-local-in-ctfe.stderr b/src/test/ui/thread-local-in-ctfe.stderr index 2983ac3f60cf2..9890597b7bd5b 100644 --- a/src/test/ui/thread-local-in-ctfe.stderr +++ b/src/test/ui/thread-local-in-ctfe.stderr @@ -5,45 +5,28 @@ LL | static B: u32 = A; | ^ error[E0625]: thread-local statics cannot be accessed at compile-time - --> $DIR/thread-local-in-ctfe.rs:9:18 + --> $DIR/thread-local-in-ctfe.rs:9:19 | LL | static C: &u32 = &A; - | ^^ - -error[E0712]: thread-local variable borrowed past end of function - --> $DIR/thread-local-in-ctfe.rs:9:18 - | -LL | static C: &u32 = &A; - | ^^- end of enclosing function is here - | | - | thread-local variables cannot be borrowed beyond the end of the function + | ^ error[E0625]: thread-local statics cannot be accessed at compile-time - --> $DIR/thread-local-in-ctfe.rs:13:16 + --> $DIR/thread-local-in-ctfe.rs:12:16 | LL | const D: u32 = A; | ^ error[E0625]: thread-local statics cannot be accessed at compile-time - --> $DIR/thread-local-in-ctfe.rs:16:17 - | -LL | const E: &u32 = &A; - | ^^ - -error[E0712]: thread-local variable borrowed past end of function - --> $DIR/thread-local-in-ctfe.rs:16:17 + --> $DIR/thread-local-in-ctfe.rs:15:18 | LL | const E: &u32 = &A; - | ^^- end of enclosing function is here - | | - | thread-local variables cannot be borrowed beyond the end of the function + | ^ error[E0625]: thread-local statics cannot be accessed at compile-time - --> $DIR/thread-local-in-ctfe.rs:21:5 + --> $DIR/thread-local-in-ctfe.rs:19:5 | LL | A | ^ -error: aborting due to 7 previous errors +error: aborting due to 5 previous errors -For more information about this error, try `rustc --explain E0712`. From bccc59a8e6e25c2b63308e77f853c6f23782e631 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 21 Nov 2019 21:20:47 +0000 Subject: [PATCH 8/8] Address review comments --- src/librustc/mir/mod.rs | 11 ++++------- src/librustc_mir/build/expr/as_temp.rs | 3 +-- src/librustc_mir/hair/mod.rs | 5 ++++- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 2928a8ad9bcc5..67681dd2da44a 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -689,13 +689,6 @@ pub struct LocalDecl<'tcx> { /// Temporaries and the return place are always mutable. pub mutability: Mutability, - /// `Some(binding_mode)` if this corresponds to a user-declared local variable. - /// - /// This is solely used for local diagnostics when generating - /// warnings/errors when compiling the current crate, and - /// therefore it need not be visible across crates. pnkfelix - /// currently hypothesized we *need* to wrap this in a - /// `ClearCrossCrate` as long as it carries as `HirId`. // FIXME(matthewjasper) Don't store in this in `Body` pub local_info: LocalInfo<'tcx>, @@ -831,6 +824,10 @@ pub struct LocalDecl<'tcx> { #[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)] pub enum LocalInfo<'tcx> { /// A user-defined local variable or function parameter + /// + /// The `BindingForm` is solely used for local diagnostics when generating + /// warnings/errors when compiling the current crate, and therefore it need + /// not be visible across crates. User(ClearCrossCrate>), /// A temporary created that references the static with the given `DefId`. StaticRef { def_id: DefId, is_thread_local: bool }, diff --git a/src/librustc_mir/build/expr/as_temp.rs b/src/librustc_mir/build/expr/as_temp.rs index 864b449c29c9e..4dad9ab498f63 100644 --- a/src/librustc_mir/build/expr/as_temp.rs +++ b/src/librustc_mir/build/expr/as_temp.rs @@ -65,8 +65,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { local_decl = local_decl.block_tail(tail_info); } if let ExprKind::StaticRef { def_id, .. } = expr.kind { - let attrs = this.hir.tcx().get_attrs(def_id); - let is_thread_local = attrs.iter().any(|attr| attr.check_name(sym::thread_local)); + let is_thread_local = this.hir.tcx().has_attr(def_id, sym::thread_local); local_decl.local_info = LocalInfo::StaticRef {def_id, is_thread_local }; } this.local_decls.push(local_decl) diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index f47c92cbd542d..47644d9ba8372 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -264,7 +264,10 @@ pub enum ExprKind<'tcx> { literal: &'tcx Const<'tcx>, user_ty: Option>>, }, - /// A literal containing the address of a `static` + /// A literal containing the address of a `static`. + /// + /// This is only distinguished from `Literal` so that we can register some + /// info for diagnostics. StaticRef { literal: &'tcx Const<'tcx>, def_id: DefId,