Skip to content

Commit

Permalink
Improve validation of TypeckTables keys.
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelwoerister committed Aug 11, 2017
1 parent 1f54df1 commit a69eaf6
Show file tree
Hide file tree
Showing 12 changed files with 55 additions and 38 deletions.
2 changes: 1 addition & 1 deletion src/librustc/infer/mod.rs
Expand Up @@ -359,7 +359,7 @@ impl<'a, 'gcx, 'tcx> InferCtxtBuilder<'a, 'gcx, 'tcx> {
/// Used only by `rustc_typeck` during body type-checking/inference,
/// will initialize `in_progress_tables` with fresh `TypeckTables`.
pub fn with_fresh_in_progress_tables(mut self, table_owner: DefId) -> Self {
self.fresh_tables = Some(RefCell::new(ty::TypeckTables::empty(table_owner)));
self.fresh_tables = Some(RefCell::new(ty::TypeckTables::empty(Some(table_owner))));
self
}

Expand Down
4 changes: 2 additions & 2 deletions src/librustc/lint/context.rs
Expand Up @@ -43,7 +43,7 @@ use syntax::ast;
use syntax_pos::{MultiSpan, Span};
use errors::DiagnosticBuilder;
use hir;
use hir::def_id::{DefId, LOCAL_CRATE};
use hir::def_id::LOCAL_CRATE;
use hir::intravisit as hir_visit;
use syntax::visit as ast_visit;

Expand Down Expand Up @@ -986,7 +986,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {

let mut cx = LateContext {
tcx,
tables: &ty::TypeckTables::empty(DefId::invalid()),
tables: &ty::TypeckTables::empty(None),
param_env: ty::ParamEnv::empty(Reveal::UserFacing),
access_levels,
lint_sess: LintSession::new(&tcx.sess.lint_store),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/dead.rs
Expand Up @@ -427,7 +427,7 @@ fn find_live<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let mut symbol_visitor = MarkSymbolVisitor {
worklist,
tcx,
tables: &ty::TypeckTables::empty(DefId::invalid()),
tables: &ty::TypeckTables::empty(None),
live_symbols: box FxHashSet(),
struct_has_extern_repr: false,
ignore_non_const_paths: false,
Expand Down
3 changes: 1 addition & 2 deletions src/librustc/middle/effect.rs
Expand Up @@ -19,7 +19,6 @@ use syntax::ast;
use syntax_pos::Span;
use hir::{self, PatKind};
use hir::def::Def;
use hir::def_id::DefId;
use hir::intravisit::{self, FnKind, Visitor, NestedVisitorMap};

#[derive(Copy, Clone)]
Expand Down Expand Up @@ -263,7 +262,7 @@ impl<'a, 'tcx> Visitor<'tcx> for EffectCheckVisitor<'a, 'tcx> {
pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
let mut visitor = EffectCheckVisitor {
tcx,
tables: &ty::TypeckTables::empty(DefId::invalid()),
tables: &ty::TypeckTables::empty(None),
body_id: hir::BodyId { node_id: ast::CRATE_NODE_ID },
unsafe_context: UnsafeContext::new(SafeContext),
};
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/reachable.rs
Expand Up @@ -375,7 +375,7 @@ fn reachable_set<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, crate_num: CrateNum) ->
});
let mut reachable_context = ReachableContext {
tcx,
tables: &ty::TypeckTables::empty(DefId::invalid()),
tables: &ty::TypeckTables::empty(None),
reachable_symbols: NodeSet(),
worklist: Vec::new(),
any_library,
Expand Down
52 changes: 33 additions & 19 deletions src/librustc/ty/context.rs
Expand Up @@ -212,7 +212,7 @@ pub struct CommonTypes<'tcx> {
}

pub struct LocalTableInContext<'a, V: 'a> {
local_id_root: DefId,
local_id_root: Option<DefId>,
data: &'a ItemLocalMap<V>
}

Expand All @@ -223,11 +223,13 @@ pub struct LocalTableInContext<'a, V: 'a> {
/// would be in a different frame of reference and using its `local_id`
/// would result in lookup errors, or worse, in silently wrong data being
/// stored/returned.
fn validate_hir_id_for_typeck_tables(table_id_root: DefId, hir_id: hir::HirId) {
fn validate_hir_id_for_typeck_tables(local_id_root: Option<DefId>,
hir_id: hir::HirId,
mut_access: bool) {
#[cfg(debug_assertions)]
{
if table_id_root.is_local() {
if hir_id.owner != table_id_root.index {
if let Some(local_id_root) = local_id_root {
if hir_id.owner != local_id_root.index {
ty::tls::with(|tcx| {
let node_id = tcx.hir
.definitions()
Expand All @@ -237,21 +239,30 @@ fn validate_hir_id_for_typeck_tables(table_id_root: DefId, hir_id: hir::HirId) {
TypeckTables with local_id_root {:?}",
tcx.hir.node_to_string(node_id),
DefId::local(hir_id.owner),
table_id_root)
local_id_root)
});
}
} else {
// We use "Null Object" TypeckTables in some of the analysis passes.
// These are just expected to be empty and their `local_id_root` is
// `None`. Therefore we cannot verify whether a given `HirId` would
// be a valid key for the given table. Instead we make sure that
// nobody tries to write to such a Null Object table.
if mut_access {
bug!("access to invalid TypeckTables")
}
}
}
}

impl<'a, V> LocalTableInContext<'a, V> {
pub fn contains_key(&self, id: hir::HirId) -> bool {
validate_hir_id_for_typeck_tables(self.local_id_root, id);
validate_hir_id_for_typeck_tables(self.local_id_root, id, false);
self.data.contains_key(&id.local_id)
}

pub fn get(&self, id: hir::HirId) -> Option<&V> {
validate_hir_id_for_typeck_tables(self.local_id_root, id);
validate_hir_id_for_typeck_tables(self.local_id_root, id, false);
self.data.get(&id.local_id)
}

Expand All @@ -269,37 +280,37 @@ impl<'a, V> ::std::ops::Index<hir::HirId> for LocalTableInContext<'a, V> {
}

pub struct LocalTableInContextMut<'a, V: 'a> {
local_id_root: DefId,
local_id_root: Option<DefId>,
data: &'a mut ItemLocalMap<V>
}

impl<'a, V> LocalTableInContextMut<'a, V> {

pub fn get_mut(&mut self, id: hir::HirId) -> Option<&mut V> {
validate_hir_id_for_typeck_tables(self.local_id_root, id);
validate_hir_id_for_typeck_tables(self.local_id_root, id, true);
self.data.get_mut(&id.local_id)
}

pub fn entry(&mut self, id: hir::HirId) -> Entry<hir::ItemLocalId, V> {
validate_hir_id_for_typeck_tables(self.local_id_root, id);
validate_hir_id_for_typeck_tables(self.local_id_root, id, true);
self.data.entry(id.local_id)
}

pub fn insert(&mut self, id: hir::HirId, val: V) -> Option<V> {
validate_hir_id_for_typeck_tables(self.local_id_root, id);
validate_hir_id_for_typeck_tables(self.local_id_root, id, true);
self.data.insert(id.local_id, val)
}

pub fn remove(&mut self, id: hir::HirId) -> Option<V> {
validate_hir_id_for_typeck_tables(self.local_id_root, id);
validate_hir_id_for_typeck_tables(self.local_id_root, id, true);
self.data.remove(&id.local_id)
}
}

#[derive(RustcEncodable, RustcDecodable)]
pub struct TypeckTables<'tcx> {
/// The HirId::owner all ItemLocalIds in this table are relative to.
pub local_id_root: DefId,
pub local_id_root: Option<DefId>,

/// Resolved definitions for `<T>::X` associated paths and
/// method calls, including those of overloaded operators.
Expand Down Expand Up @@ -363,7 +374,7 @@ pub struct TypeckTables<'tcx> {
}

impl<'tcx> TypeckTables<'tcx> {
pub fn empty(local_id_root: DefId) -> TypeckTables<'tcx> {
pub fn empty(local_id_root: Option<DefId>) -> TypeckTables<'tcx> {
TypeckTables {
local_id_root,
type_dependent_defs: ItemLocalMap(),
Expand All @@ -388,7 +399,7 @@ impl<'tcx> TypeckTables<'tcx> {
match *qpath {
hir::QPath::Resolved(_, ref path) => path.def,
hir::QPath::TypeRelative(..) => {
validate_hir_id_for_typeck_tables(self.local_id_root, id);
validate_hir_id_for_typeck_tables(self.local_id_root, id, false);
self.type_dependent_defs.get(&id.local_id).cloned().unwrap_or(Def::Err)
}
}
Expand Down Expand Up @@ -436,7 +447,7 @@ impl<'tcx> TypeckTables<'tcx> {
}

pub fn node_id_to_type_opt(&self, id: hir::HirId) -> Option<Ty<'tcx>> {
validate_hir_id_for_typeck_tables(self.local_id_root, id);
validate_hir_id_for_typeck_tables(self.local_id_root, id, false);
self.node_types.get(&id.local_id).cloned()
}

Expand All @@ -448,12 +459,12 @@ impl<'tcx> TypeckTables<'tcx> {
}

pub fn node_substs(&self, id: hir::HirId) -> &'tcx Substs<'tcx> {
validate_hir_id_for_typeck_tables(self.local_id_root, id);
validate_hir_id_for_typeck_tables(self.local_id_root, id, false);
self.node_substs.get(&id.local_id).cloned().unwrap_or(Substs::empty())
}

pub fn node_substs_opt(&self, id: hir::HirId) -> Option<&'tcx Substs<'tcx>> {
validate_hir_id_for_typeck_tables(self.local_id_root, id);
validate_hir_id_for_typeck_tables(self.local_id_root, id, false);
self.node_substs.get(&id.local_id).cloned()
}

Expand Down Expand Up @@ -502,7 +513,7 @@ impl<'tcx> TypeckTables<'tcx> {

pub fn expr_adjustments(&self, expr: &hir::Expr)
-> &[ty::adjustment::Adjustment<'tcx>] {
validate_hir_id_for_typeck_tables(self.local_id_root, expr.hir_id);
validate_hir_id_for_typeck_tables(self.local_id_root, expr.hir_id, false);
self.adjustments.get(&expr.hir_id.local_id).map_or(&[], |a| &a[..])
}

Expand Down Expand Up @@ -663,6 +674,9 @@ impl<'a, 'gcx, 'tcx> HashStable<StableHashingContext<'a, 'gcx, 'tcx>> for Typeck
closure_expr_id
} = *up_var_id;

let local_id_root =
local_id_root.expect("trying to hash invalid TypeckTables");

let var_def_id = DefId {
krate: local_id_root.krate,
index: var_id,
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_driver/pretty.rs
Expand Up @@ -45,7 +45,6 @@ use std::option;
use std::path::Path;
use std::str::FromStr;

use rustc::hir::def_id::DefId;
use rustc::hir::map as hir_map;
use rustc::hir::map::blocks;
use rustc::hir;
Expand Down Expand Up @@ -233,7 +232,7 @@ impl PpSourceMode {
arenas,
id,
|tcx, _, _, _| {
let empty_tables = ty::TypeckTables::empty(DefId::invalid());
let empty_tables = ty::TypeckTables::empty(None);
let annotation = TypedAnnotation {
tcx: tcx,
tables: Cell::new(&empty_tables)
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_passes/consts.rs
Expand Up @@ -471,7 +471,7 @@ fn check_adjustments<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>, e: &hir::Exp
pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
tcx.hir.krate().visit_all_item_likes(&mut CheckCrateVisitor {
tcx: tcx,
tables: &ty::TypeckTables::empty(DefId::invalid()),
tables: &ty::TypeckTables::empty(None),
in_fn: false,
promotable: false,
mut_rvalue_borrows: NodeSet(),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_privacy/lib.rs
Expand Up @@ -1656,7 +1656,7 @@ fn privacy_access_levels<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

let krate = tcx.hir.krate();

let empty_tables = ty::TypeckTables::empty(DefId::invalid());
let empty_tables = ty::TypeckTables::empty(None);


// Check privacy of names not checked in previous compilation stages.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_save_analysis/lib.rs
Expand Up @@ -977,7 +977,7 @@ pub fn process_crate<'l, 'tcx, H: SaveHandler>(tcx: TyCtxt<'l, 'tcx, 'tcx>,

let save_ctxt = SaveContext {
tcx: tcx,
tables: &ty::TypeckTables::empty(DefId::invalid()),
tables: &ty::TypeckTables::empty(None),
analysis: analysis,
span_utils: SpanUtils::new(&tcx.sess),
config: find_config(config),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/mod.rs
Expand Up @@ -901,7 +901,7 @@ fn typeck_tables_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
// Consistency check our TypeckTables instance can hold all ItemLocalIds
// it will need to hold.
assert_eq!(tables.local_id_root,
DefId::local(tcx.hir.definitions().node_to_hir_id(id).owner));
Some(DefId::local(tcx.hir.definitions().node_to_hir_id(id).owner)));
tables
}

Expand Down
17 changes: 11 additions & 6 deletions src/librustc_typeck/check/writeback.rs
Expand Up @@ -81,7 +81,7 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {

WritebackCx {
fcx: fcx,
tables: ty::TypeckTables::empty(DefId::local(owner.owner)),
tables: ty::TypeckTables::empty(Some(DefId::local(owner.owner))),
body: body
}
}
Expand Down Expand Up @@ -229,10 +229,11 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
fn visit_closures(&mut self) {
let fcx_tables = self.fcx.tables.borrow();
debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root);
let common_local_id_root = fcx_tables.local_id_root.unwrap();

for (&id, closure_ty) in fcx_tables.closure_tys().iter() {
let hir_id = hir::HirId {
owner: fcx_tables.local_id_root.index,
owner: common_local_id_root.index,
local_id: id,
};
let closure_ty = self.resolve(closure_ty, &hir_id);
Expand All @@ -241,7 +242,7 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {

for (&id, &closure_kind) in fcx_tables.closure_kinds().iter() {
let hir_id = hir::HirId {
owner: fcx_tables.local_id_root.index,
owner: common_local_id_root.index,
local_id: id,
};
self.tables.closure_kinds_mut().insert(hir_id, closure_kind);
Expand All @@ -251,11 +252,13 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
fn visit_cast_types(&mut self) {
let fcx_tables = self.fcx.tables.borrow();
let fcx_cast_kinds = fcx_tables.cast_kinds();
debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root);
let mut self_cast_kinds = self.tables.cast_kinds_mut();
let common_local_id_root = fcx_tables.local_id_root.unwrap();

for (&local_id, &cast_kind) in fcx_cast_kinds.iter() {
let hir_id = hir::HirId {
owner: fcx_tables.local_id_root.index,
owner: common_local_id_root.index,
local_id,
};
self_cast_kinds.insert(hir_id, cast_kind);
Expand Down Expand Up @@ -357,10 +360,11 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
fn visit_liberated_fn_sigs(&mut self) {
let fcx_tables = self.fcx.tables.borrow();
debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root);
let common_local_id_root = fcx_tables.local_id_root.unwrap();

for (&local_id, fn_sig) in fcx_tables.liberated_fn_sigs().iter() {
let hir_id = hir::HirId {
owner: fcx_tables.local_id_root.index,
owner: common_local_id_root.index,
local_id,
};
let fn_sig = self.resolve(fn_sig, &hir_id);
Expand All @@ -371,10 +375,11 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
fn visit_fru_field_types(&mut self) {
let fcx_tables = self.fcx.tables.borrow();
debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root);
let common_local_id_root = fcx_tables.local_id_root.unwrap();

for (&local_id, ftys) in fcx_tables.fru_field_types().iter() {
let hir_id = hir::HirId {
owner: fcx_tables.local_id_root.index,
owner: common_local_id_root.index,
local_id,
};
let ftys = self.resolve(ftys, &hir_id);
Expand Down

0 comments on commit a69eaf6

Please sign in to comment.