diff --git a/src/librustc/dep_graph/README.md b/src/librustc/dep_graph/README.md index f16a9b386bb..48f5b7ea259 100644 --- a/src/librustc/dep_graph/README.md +++ b/src/librustc/dep_graph/README.md @@ -341,6 +341,8 @@ path is found (as demonstrated above). ### Debugging the dependency graph +#### Dumping the graph + The compiler is also capable of dumping the dependency graph for your debugging pleasure. To do so, pass the `-Z dump-dep-graph` flag. The graph will be dumped to `dep_graph.{txt,dot}` in the current @@ -392,6 +394,35 @@ This will dump out all the nodes that lead from `Hir(foo)` to `TypeckItemBody(bar)`, from which you can (hopefully) see the source of the erroneous edge. +#### Tracking down incorrect edges + +Sometimes, after you dump the dependency graph, you will find some +path that should not exist, but you will not be quite sure how it came +to be. **When the compiler is built with debug assertions,** it can +help you track that down. Simply set the `RUST_FORBID_DEP_GRAPH_EDGE` +environment variable to a filter. Every edge created in the dep-graph +will be tested against that filter -- if it matches, a `bug!` is +reported, so you can easily see the backtrace (`RUST_BACKTRACE=1`). + +The syntax for these filters is the same as described in the previous +section. However, note that this filter is applied to every **edge** +and doesn't handle longer paths in the graph, unlike the previous +section. + +Example: + +You find that there is a path from the `Hir` of `foo` to the type +check of `bar` and you don't think there should be. You dump the +dep-graph as described in the previous section and open `dep-graph.txt` +to see something like: + + Hir(foo) -> Collect(bar) + Collect(bar) -> TypeckItemBody(bar) + +That first edge looks suspicious to you. So you set +`RUST_FORBID_DEP_GRAPH_EDGE` to `Hir&foo -> Collect&bar`, re-run, and +then observe the backtrace. Voila, bug fixed! + ### Inlining of HIR nodes For the time being, at least, we still sometimes "inline" HIR nodes diff --git a/src/librustc/dep_graph/debug.rs b/src/librustc/dep_graph/debug.rs index 15b0380374c..5b15c5e6717 100644 --- a/src/librustc/dep_graph/debug.rs +++ b/src/librustc/dep_graph/debug.rs @@ -66,4 +66,11 @@ impl EdgeFilter { }) } } + + pub fn test(&self, + source: &DepNode, + target: &DepNode) + -> bool { + self.source.test(source) && self.target.test(target) + } } diff --git a/src/librustc/dep_graph/mod.rs b/src/librustc/dep_graph/mod.rs index a499cb10f23..9c00e95c17e 100644 --- a/src/librustc/dep_graph/mod.rs +++ b/src/librustc/dep_graph/mod.rs @@ -15,6 +15,7 @@ mod edges; mod graph; mod query; mod raii; +mod shadow; mod thread; mod visit; diff --git a/src/librustc/dep_graph/shadow.rs b/src/librustc/dep_graph/shadow.rs new file mode 100644 index 00000000000..e038997e0ad --- /dev/null +++ b/src/librustc/dep_graph/shadow.rs @@ -0,0 +1,123 @@ +//! The "Shadow Graph" is maintained on the main thread and which +//! tracks each message relating to the dep-graph and applies some +//! sanity checks as they go by. If an error results, it means you get +//! a nice stack-trace telling you precisely what caused the error. +//! +//! NOTE: This is a debugging facility which can potentially have non-trivial +//! runtime impact. Therefore, it is largely compiled out if +//! debug-assertions are not enabled. +//! +//! The basic sanity check, always enabled, is that there is always a +//! task (or ignore) on the stack when you do read/write. +//! +//! Optionally, if you specify RUST_FORBID_DEP_GRAPH_EDGE, you can +//! specify an edge filter to be applied to each edge as it is +//! created. See `./README.md` for details. + +use hir::def_id::DefId; +use std::cell::{BorrowState, RefCell}; +use std::env; + +use super::DepNode; +use super::thread::DepMessage; +use super::debug::EdgeFilter; + +pub struct ShadowGraph { + // if you push None onto the stack, that corresponds to an Ignore + stack: RefCell>>>, + forbidden_edge: Option, +} + +const ENABLED: bool = cfg!(debug_assertions); + +impl ShadowGraph { + pub fn new() -> Self { + let forbidden_edge = if !ENABLED { + None + } else { + match env::var("RUST_FORBID_DEP_GRAPH_EDGE") { + Ok(s) => { + match EdgeFilter::new(&s) { + Ok(f) => Some(f), + Err(err) => bug!("RUST_FORBID_DEP_GRAPH_EDGE invalid: {}", err), + } + } + Err(_) => None, + } + }; + + ShadowGraph { + stack: RefCell::new(vec![]), + forbidden_edge: forbidden_edge, + } + } + + pub fn enqueue(&self, message: &DepMessage) { + if ENABLED { + match self.stack.borrow_state() { + BorrowState::Unused => {} + _ => { + // When we apply edge filters, that invokes the + // Debug trait on DefIds, which in turn reads from + // various bits of state and creates reads! Ignore + // those recursive reads. + return; + } + } + + let mut stack = self.stack.borrow_mut(); + match *message { + DepMessage::Read(ref n) => self.check_edge(Some(Some(n)), top(&stack)), + DepMessage::Write(ref n) => self.check_edge(top(&stack), Some(Some(n))), + DepMessage::PushTask(ref n) => stack.push(Some(n.clone())), + DepMessage::PushIgnore => stack.push(None), + DepMessage::PopTask(_) | + DepMessage::PopIgnore => { + // we could easily check that the stack is + // well-formed here, but since we use closures and + // RAII accessors, this bug basically never + // happens, so it seems not worth the overhead + stack.pop(); + } + DepMessage::Query => (), + } + } + } + + fn check_edge(&self, + source: Option>>, + target: Option>>) { + assert!(ENABLED); + match (source, target) { + // cannot happen, one side is always Some(Some(_)) + (None, None) => unreachable!(), + + // nothing on top of the stack + (None, Some(n)) | (Some(n), None) => bug!("read/write of {:?} but no current task", n), + + // this corresponds to an Ignore being top of the stack + (Some(None), _) | (_, Some(None)) => (), + + // a task is on top of the stack + (Some(Some(source)), Some(Some(target))) => { + if let Some(ref forbidden_edge) = self.forbidden_edge { + if forbidden_edge.test(source, target) { + bug!("forbidden edge {:?} -> {:?} created", source, target) + } + } + } + } + } +} + +// Do a little juggling: we get back a reference to an option at the +// top of the stack, convert it to an optional reference. +fn top<'s>(stack: &'s Vec>>) -> Option>> { + stack.last() + .map(|n: &'s Option>| -> Option<&'s DepNode> { + // (*) + // (*) type annotation just there to clarify what would + // otherwise be some *really* obscure code + n.as_ref() + }) +} diff --git a/src/librustc/dep_graph/thread.rs b/src/librustc/dep_graph/thread.rs index 4e16fae1870..90c42d66b7a 100644 --- a/src/librustc/dep_graph/thread.rs +++ b/src/librustc/dep_graph/thread.rs @@ -20,13 +20,13 @@ use hir::def_id::DefId; use rustc_data_structures::veccell::VecCell; -use std::cell::Cell; use std::sync::mpsc::{self, Sender, Receiver}; use std::thread; use super::DepGraphQuery; use super::DepNode; use super::edges::DepGraphEdges; +use super::shadow::ShadowGraph; #[derive(Debug)] pub enum DepMessage { @@ -42,12 +42,16 @@ pub enum DepMessage { pub struct DepGraphThreadData { enabled: bool, - // Local counter that just tracks how many tasks are pushed onto the - // stack, so that we still get an error in the case where one is - // missing. If dep-graph construction is enabled, we'd get the same - // error when processing tasks later on, but that's annoying because - // it lacks precision about the source of the error. - tasks_pushed: Cell, + // The "shadow graph" is a debugging aid. We give it each message + // in real time as it arrives and it checks for various errors + // (for example, a read/write when there is no current task; it + // can also apply user-defined filters; see `shadow` module for + // details). This only occurs if debug-assertions are enabled. + // + // Note that in some cases the same errors will occur when the + // data is processed off the main thread, but that's annoying + // because it lacks precision about the source of the error. + shadow_graph: ShadowGraph, // current buffer, where we accumulate messages messages: VecCell, @@ -76,7 +80,7 @@ impl DepGraphThreadData { DepGraphThreadData { enabled: enabled, - tasks_pushed: Cell::new(0), + shadow_graph: ShadowGraph::new(), messages: VecCell::with_capacity(INITIAL_CAPACITY), swap_in: rx2, swap_out: tx1, @@ -118,21 +122,7 @@ impl DepGraphThreadData { /// the buffer is full, this may swap.) #[inline] pub fn enqueue(&self, message: DepMessage) { - // Regardless of whether dep graph construction is enabled, we - // still want to check that we always have a valid task on the - // stack when a read/write/etc event occurs. - match message { - DepMessage::Read(_) | DepMessage::Write(_) => - if self.tasks_pushed.get() == 0 { - self.invalid_message("read/write but no current task") - }, - DepMessage::PushTask(_) | DepMessage::PushIgnore => - self.tasks_pushed.set(self.tasks_pushed.get() + 1), - DepMessage::PopTask(_) | DepMessage::PopIgnore => - self.tasks_pushed.set(self.tasks_pushed.get() - 1), - DepMessage::Query => - (), - } + self.shadow_graph.enqueue(&message); if self.enabled { self.enqueue_enabled(message); @@ -147,11 +137,6 @@ impl DepGraphThreadData { self.swap(); } } - - // Outline this too. - fn invalid_message(&self, string: &str) { - bug!("{}; see src/librustc/dep_graph/README.md for more information", string) - } } /// Definition of the depgraph thread. diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index f70349d0ee0..d2a2f8a972d 100644 --- a/src/librustc/lib.rs +++ b/src/librustc/lib.rs @@ -24,6 +24,7 @@ #![cfg_attr(not(stage0), deny(warnings))] #![feature(associated_consts)] +#![feature(borrow_state)] #![feature(box_patterns)] #![feature(box_syntax)] #![feature(collections)]