Skip to content

Commit a69eaf6

Browse files
michaelwoeristermichaelwoerister
committed
Improve validation of TypeckTables keys.
1 parent 1f54df1 commit a69eaf6

File tree

12 files changed

+55
-38
lines changed

12 files changed

+55
-38
lines changed

‎src/librustc/infer/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ impl<'a, 'gcx, 'tcx> InferCtxtBuilder<'a, 'gcx, 'tcx> {
359359
/// Used only by `rustc_typeck` during body type-checking/inference,
360360
/// will initialize `in_progress_tables` with fresh `TypeckTables`.
361361
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))));
363363
self
364364
}
365365

‎src/librustc/lint/context.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ use syntax::ast;
4343
use syntax_pos::{MultiSpan, Span};
4444
use errors::DiagnosticBuilder;
4545
use hir;
46-
use hir::def_id::{DefId, LOCAL_CRATE};
46+
use hir::def_id::LOCAL_CRATE;
4747
use hir::intravisit as hir_visit;
4848
use syntax::visit as ast_visit;
4949

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

987987
let mut cx = LateContext {
988988
tcx,
989-
tables: &ty::TypeckTables::empty(DefId::invalid()),
989+
tables: &ty::TypeckTables::empty(None),
990990
param_env: ty::ParamEnv::empty(Reveal::UserFacing),
991991
access_levels,
992992
lint_sess: LintSession::new(&tcx.sess.lint_store),

‎src/librustc/middle/dead.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ fn find_live<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
427427
let mut symbol_visitor = MarkSymbolVisitor {
428428
worklist,
429429
tcx,
430-
tables: &ty::TypeckTables::empty(DefId::invalid()),
430+
tables: &ty::TypeckTables::empty(None),
431431
live_symbols: box FxHashSet(),
432432
struct_has_extern_repr: false,
433433
ignore_non_const_paths: false,

‎src/librustc/middle/effect.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ use syntax::ast;
1919
use syntax_pos::Span;
2020
use hir::{self, PatKind};
2121
use hir::def::Def;
22-
use hir::def_id::DefId;
2322
use hir::intravisit::{self, FnKind, Visitor, NestedVisitorMap};
2423

2524
#[derive(Copy, Clone)]
@@ -263,7 +262,7 @@ impl<'a, 'tcx> Visitor<'tcx> for EffectCheckVisitor<'a, 'tcx> {
263262
pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
264263
let mut visitor = EffectCheckVisitor {
265264
tcx,
266-
tables: &ty::TypeckTables::empty(DefId::invalid()),
265+
tables: &ty::TypeckTables::empty(None),
267266
body_id: hir::BodyId { node_id: ast::CRATE_NODE_ID },
268267
unsafe_context: UnsafeContext::new(SafeContext),
269268
};

‎src/librustc/middle/reachable.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ fn reachable_set<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, crate_num: CrateNum) ->
375375
});
376376
let mut reachable_context = ReachableContext {
377377
tcx,
378-
tables: &ty::TypeckTables::empty(DefId::invalid()),
378+
tables: &ty::TypeckTables::empty(None),
379379
reachable_symbols: NodeSet(),
380380
worklist: Vec::new(),
381381
any_library,

‎src/librustc/ty/context.rs

+33-19
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ pub struct CommonTypes<'tcx> {
212212
}
213213

214214
pub struct LocalTableInContext<'a, V: 'a> {
215-
local_id_root: DefId,
215+
local_id_root: Option<DefId>,
216216
data: &'a ItemLocalMap<V>
217217
}
218218

@@ -223,11 +223,13 @@ pub struct LocalTableInContext<'a, V: 'a> {
223223
/// would be in a different frame of reference and using its `local_id`
224224
/// would result in lookup errors, or worse, in silently wrong data being
225225
/// stored/returned.
226-
fn validate_hir_id_for_typeck_tables(table_id_root: DefId, hir_id: hir::HirId) {
226+
fn validate_hir_id_for_typeck_tables(local_id_root: Option<DefId>,
227+
hir_id: hir::HirId,
228+
mut_access: bool) {
227229
#[cfg(debug_assertions)]
228230
{
229-
if table_id_root.is_local() {
230-
if hir_id.owner != table_id_root.index {
231+
if let Some(local_id_root) = local_id_root {
232+
if hir_id.owner != local_id_root.index {
231233
ty::tls::with(|tcx| {
232234
let node_id = tcx.hir
233235
.definitions()
@@ -237,21 +239,30 @@ fn validate_hir_id_for_typeck_tables(table_id_root: DefId, hir_id: hir::HirId) {
237239
TypeckTables with local_id_root {:?}",
238240
tcx.hir.node_to_string(node_id),
239241
DefId::local(hir_id.owner),
240-
table_id_root)
242+
local_id_root)
241243
});
242244
}
245+
} else {
246+
// We use "Null Object" TypeckTables in some of the analysis passes.
247+
// These are just expected to be empty and their `local_id_root` is
248+
// `None`. Therefore we cannot verify whether a given `HirId` would
249+
// be a valid key for the given table. Instead we make sure that
250+
// nobody tries to write to such a Null Object table.
251+
if mut_access {
252+
bug!("access to invalid TypeckTables")
253+
}
243254
}
244255
}
245256
}
246257

247258
impl<'a, V> LocalTableInContext<'a, V> {
248259
pub fn contains_key(&self, id: hir::HirId) -> bool {
249-
validate_hir_id_for_typeck_tables(self.local_id_root, id);
260+
validate_hir_id_for_typeck_tables(self.local_id_root, id, false);
250261
self.data.contains_key(&id.local_id)
251262
}
252263

253264
pub fn get(&self, id: hir::HirId) -> Option<&V> {
254-
validate_hir_id_for_typeck_tables(self.local_id_root, id);
265+
validate_hir_id_for_typeck_tables(self.local_id_root, id, false);
255266
self.data.get(&id.local_id)
256267
}
257268

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

271282
pub struct LocalTableInContextMut<'a, V: 'a> {
272-
local_id_root: DefId,
283+
local_id_root: Option<DefId>,
273284
data: &'a mut ItemLocalMap<V>
274285
}
275286

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

278289
pub fn get_mut(&mut self, id: hir::HirId) -> Option<&mut V> {
279-
validate_hir_id_for_typeck_tables(self.local_id_root, id);
290+
validate_hir_id_for_typeck_tables(self.local_id_root, id, true);
280291
self.data.get_mut(&id.local_id)
281292
}
282293

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

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

293304
pub fn remove(&mut self, id: hir::HirId) -> Option<V> {
294-
validate_hir_id_for_typeck_tables(self.local_id_root, id);
305+
validate_hir_id_for_typeck_tables(self.local_id_root, id, true);
295306
self.data.remove(&id.local_id)
296307
}
297308
}
298309

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

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

365376
impl<'tcx> TypeckTables<'tcx> {
366-
pub fn empty(local_id_root: DefId) -> TypeckTables<'tcx> {
377+
pub fn empty(local_id_root: Option<DefId>) -> TypeckTables<'tcx> {
367378
TypeckTables {
368379
local_id_root,
369380
type_dependent_defs: ItemLocalMap(),
@@ -388,7 +399,7 @@ impl<'tcx> TypeckTables<'tcx> {
388399
match *qpath {
389400
hir::QPath::Resolved(_, ref path) => path.def,
390401
hir::QPath::TypeRelative(..) => {
391-
validate_hir_id_for_typeck_tables(self.local_id_root, id);
402+
validate_hir_id_for_typeck_tables(self.local_id_root, id, false);
392403
self.type_dependent_defs.get(&id.local_id).cloned().unwrap_or(Def::Err)
393404
}
394405
}
@@ -436,7 +447,7 @@ impl<'tcx> TypeckTables<'tcx> {
436447
}
437448

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

@@ -448,12 +459,12 @@ impl<'tcx> TypeckTables<'tcx> {
448459
}
449460

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

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

@@ -502,7 +513,7 @@ impl<'tcx> TypeckTables<'tcx> {
502513

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

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

677+
let local_id_root =
678+
local_id_root.expect("trying to hash invalid TypeckTables");
679+
666680
let var_def_id = DefId {
667681
krate: local_id_root.krate,
668682
index: var_id,

‎src/librustc_driver/pretty.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ use std::option;
4545
use std::path::Path;
4646
use std::str::FromStr;
4747

48-
use rustc::hir::def_id::DefId;
4948
use rustc::hir::map as hir_map;
5049
use rustc::hir::map::blocks;
5150
use rustc::hir;
@@ -233,7 +232,7 @@ impl PpSourceMode {
233232
arenas,
234233
id,
235234
|tcx, _, _, _| {
236-
let empty_tables = ty::TypeckTables::empty(DefId::invalid());
235+
let empty_tables = ty::TypeckTables::empty(None);
237236
let annotation = TypedAnnotation {
238237
tcx: tcx,
239238
tables: Cell::new(&empty_tables)

‎src/librustc_passes/consts.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ fn check_adjustments<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>, e: &hir::Exp
471471
pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
472472
tcx.hir.krate().visit_all_item_likes(&mut CheckCrateVisitor {
473473
tcx: tcx,
474-
tables: &ty::TypeckTables::empty(DefId::invalid()),
474+
tables: &ty::TypeckTables::empty(None),
475475
in_fn: false,
476476
promotable: false,
477477
mut_rvalue_borrows: NodeSet(),

‎src/librustc_privacy/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1656,7 +1656,7 @@ fn privacy_access_levels<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
16561656

16571657
let krate = tcx.hir.krate();
16581658

1659-
let empty_tables = ty::TypeckTables::empty(DefId::invalid());
1659+
let empty_tables = ty::TypeckTables::empty(None);
16601660

16611661

16621662
// Check privacy of names not checked in previous compilation stages.

‎src/librustc_save_analysis/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,7 @@ pub fn process_crate<'l, 'tcx, H: SaveHandler>(tcx: TyCtxt<'l, 'tcx, 'tcx>,
977977

978978
let save_ctxt = SaveContext {
979979
tcx: tcx,
980-
tables: &ty::TypeckTables::empty(DefId::invalid()),
980+
tables: &ty::TypeckTables::empty(None),
981981
analysis: analysis,
982982
span_utils: SpanUtils::new(&tcx.sess),
983983
config: find_config(config),

‎src/librustc_typeck/check/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -901,7 +901,7 @@ fn typeck_tables_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
901901
// Consistency check our TypeckTables instance can hold all ItemLocalIds
902902
// it will need to hold.
903903
assert_eq!(tables.local_id_root,
904-
DefId::local(tcx.hir.definitions().node_to_hir_id(id).owner));
904+
Some(DefId::local(tcx.hir.definitions().node_to_hir_id(id).owner)));
905905
tables
906906
}
907907

‎src/librustc_typeck/check/writeback.rs

+11-6
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
8181

8282
WritebackCx {
8383
fcx: fcx,
84-
tables: ty::TypeckTables::empty(DefId::local(owner.owner)),
84+
tables: ty::TypeckTables::empty(Some(DefId::local(owner.owner))),
8585
body: body
8686
}
8787
}
@@ -229,10 +229,11 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
229229
fn visit_closures(&mut self) {
230230
let fcx_tables = self.fcx.tables.borrow();
231231
debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root);
232+
let common_local_id_root = fcx_tables.local_id_root.unwrap();
232233

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

242243
for (&id, &closure_kind) in fcx_tables.closure_kinds().iter() {
243244
let hir_id = hir::HirId {
244-
owner: fcx_tables.local_id_root.index,
245+
owner: common_local_id_root.index,
245246
local_id: id,
246247
};
247248
self.tables.closure_kinds_mut().insert(hir_id, closure_kind);
@@ -251,11 +252,13 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
251252
fn visit_cast_types(&mut self) {
252253
let fcx_tables = self.fcx.tables.borrow();
253254
let fcx_cast_kinds = fcx_tables.cast_kinds();
255+
debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root);
254256
let mut self_cast_kinds = self.tables.cast_kinds_mut();
257+
let common_local_id_root = fcx_tables.local_id_root.unwrap();
255258

256259
for (&local_id, &cast_kind) in fcx_cast_kinds.iter() {
257260
let hir_id = hir::HirId {
258-
owner: fcx_tables.local_id_root.index,
261+
owner: common_local_id_root.index,
259262
local_id,
260263
};
261264
self_cast_kinds.insert(hir_id, cast_kind);
@@ -357,10 +360,11 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
357360
fn visit_liberated_fn_sigs(&mut self) {
358361
let fcx_tables = self.fcx.tables.borrow();
359362
debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root);
363+
let common_local_id_root = fcx_tables.local_id_root.unwrap();
360364

361365
for (&local_id, fn_sig) in fcx_tables.liberated_fn_sigs().iter() {
362366
let hir_id = hir::HirId {
363-
owner: fcx_tables.local_id_root.index,
367+
owner: common_local_id_root.index,
364368
local_id,
365369
};
366370
let fn_sig = self.resolve(fn_sig, &hir_id);
@@ -371,10 +375,11 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
371375
fn visit_fru_field_types(&mut self) {
372376
let fcx_tables = self.fcx.tables.borrow();
373377
debug_assert_eq!(fcx_tables.local_id_root, self.tables.local_id_root);
378+
let common_local_id_root = fcx_tables.local_id_root.unwrap();
374379

375380
for (&local_id, ftys) in fcx_tables.fru_field_types().iter() {
376381
let hir_id = hir::HirId {
377-
owner: fcx_tables.local_id_root.index,
382+
owner: common_local_id_root.index,
378383
local_id,
379384
};
380385
let ftys = self.resolve(ftys, &hir_id);

0 commit comments

Comments
 (0)