Skip to content

Commit

Permalink
Rollup merge of rust-lang#66587 - matthewjasper:handle-static-as-cons…
Browse files Browse the repository at this point in the history
…t, r=oli-obk

Handle statics in MIR as const pointers

This is the first PR towards the goal of removing `PlaceBase::Static`. In this PR:

* Statics are lowered to dereferencing a const pointer.
* The temporaries holding such pointers are tracked in MIR, for the most part this is only used for diagnostics. There are two exceptions:
    * The borrow checker has some checks for thread-locals that directly use this data.
    * Const checking will suppress "cannot dereference raw pointer" diagnostics for pointers to `static mut`/`extern static`. This is to maintain the current behaviour (12 tests fail otherwise).

The following are left to future PRs (I think that @spastorino will be working on the first 3):

* Applying the same treatments to promoted statics.
* Removing `PlaceBase::Static`.
* Replacing `PlaceBase` with `Local`.
* Moving the ever growing collection of metadata that we have for diagnostics in MIR passes somewhere more appropriate.

r? @oli-obk
  • Loading branch information
Centril authored Nov 22, 2019
2 parents c66b508 + bccc59a commit 3031720
Show file tree
Hide file tree
Showing 39 changed files with 488 additions and 397 deletions.
105 changes: 82 additions & 23 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{GlobalAlloc, PanicInfo, Scalar};
use crate::mir::visit::MirVisitable;
use crate::ty::adjustment::PointerCast;
use crate::ty::fold::{TypeFoldable, TypeFolder, TypeVisitor};
Expand Down Expand Up @@ -292,7 +292,7 @@ impl<'tcx> Body<'tcx> {
pub fn temps_iter<'a>(&'a self) -> impl Iterator<Item = Local> + '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)
Expand All @@ -305,7 +305,7 @@ impl<'tcx> Body<'tcx> {
pub fn vars_iter<'a>(&'a self) -> impl Iterator<Item = Local> + '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
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -689,14 +689,8 @@ 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`.
pub is_user_variable: Option<ClearCrossCrate<BindingForm<'tcx>>>,
// FIXME(matthewjasper) Don't store in this in `Body`
pub local_info: LocalInfo<'tcx>,

/// `true` if this is an internal local.
///
Expand All @@ -721,6 +715,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<BlockTailInfo>,

/// The type of this local.
Expand All @@ -730,6 +725,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.
Expand Down Expand Up @@ -824,6 +820,21 @@ 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
///
/// 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<BindingForm<'tcx>>),
/// 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
Expand All @@ -832,15 +843,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,
}
Expand All @@ -850,26 +863,54 @@ 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,
}
}

/// Returns `true` if this is a reference to a variable bound in a `match`
/// 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,
}
}
Expand Down Expand Up @@ -918,7 +959,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,
}
}
Expand All @@ -937,7 +978,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,
}
}
}
Expand Down Expand Up @@ -2341,6 +2382,24 @@ pub struct Constant<'tcx> {
pub literal: &'tcx ty::Const<'tcx>,
}

impl Constant<'tcx> {
pub fn check_static_ptr(&self, tcx: TyCtxt<'_>) -> Option<DefId> {
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 => {
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
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ macro_rules! make_mir_visitor {
source_info,
visibility_scope,
internal: _,
is_user_variable: _,
local_info: _,
is_block_tail: _,
} = local_decl;

Expand Down
27 changes: 25 additions & 2 deletions src/librustc_codegen_ssa/mir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,40 @@ 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<OperandRef<'tcx, Bx::Value>, ErrorHandled> {
match constant.literal.val {
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);
// 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>,
) -> 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();
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_codegen_ssa/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/borrow_check/borrow_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
35 changes: 23 additions & 12 deletions src/librustc_mir/borrow_check/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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())
{
Expand All @@ -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:
//
Expand Down Expand Up @@ -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),
Expand Down
Loading

0 comments on commit 3031720

Please sign in to comment.