Improve validation of TypeckTables keys.

michaelwoerister · michaelwoerister · commit a69eaf62c5e3 · 2017-08-11T12:17:07.000+02:00
diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs
@@ -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
     }
 
diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs
@@ -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;
 
@@ -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),
diff --git a/src/librustc/middle/dead.rs b/src/librustc/middle/dead.rs
@@ -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,
diff --git a/src/librustc/middle/effect.rs b/src/librustc/middle/effect.rs
@@ -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)]
@@ -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),
     };
diff --git a/src/librustc/middle/reachable.rs b/src/librustc/middle/reachable.rs
@@ -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,
diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs
@@ -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>
 }
 
@@ -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()
@@ -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)
     }
 
@@ -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.
@@ -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(),
@@ -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)
             }
         }
@@ -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()
     }
 
@@ -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()
     }
 
@@ -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[..])
     }
 
@@ -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,
diff --git a/src/librustc_driver/pretty.rs b/src/librustc_driver/pretty.rs
@@ -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;
@@ -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)
diff --git a/src/librustc_passes/consts.rs b/src/librustc_passes/consts.rs
@@ -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(),
diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs
@@ -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.
diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs
@@ -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),
diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs
@@ -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
 }
 
diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs
@@ -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
         }
     }
@@ -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);
@@ -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);
@@ -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);
@@ -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);
@@ -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);

Original file line number	Diff line number	Diff line change
`@@ -359,7 +359,7 @@ impl<'a, 'gcx, 'tcx> InferCtxtBuilder<'a, 'gcx, 'tcx> {`
`359`	`359`	/// Used only by `rustc_typeck` during body type-checking/inference,
`360`	`360`	/// will initialize `in_progress_tables` with fresh `TypeckTables`.
`361`	`361`	`pub fn with_fresh_in_progress_tables(mut self, table_owner: DefId) -> Self {`
`362`		`- self.fresh_tables = Some(RefCell::new(ty::TypeckTables::empty(table_owner)));`
	`362`	`+ self.fresh_tables = Some(RefCell::new(ty::TypeckTables::empty(Some(table_owner))));`
`363`	`363`	`self`
`364`	`364`	`}`
`365`	`365`