Don't crash on cyclic references between rule bindings.
authorNico Weber <nicolasweber@gmx.de>
Wed, 18 Mar 2015 20:54:16 +0000 (16:54 -0400)
committerNico Weber <nicolasweber@gmx.de>
Sat, 21 Mar 2015 15:15:41 +0000 (08:15 -0700)
Fixes #902.

This dynamically detects cycles.  I like this approach less than
detecting them statically when parsing rules [1], but it has the
advantage that it doesn't break existing ninja manifest files.

It has the disadvantage that it slows down manifest_parser_perftest by
3.9%.

1: https://github.com/martine/ninja/commit/cc6f54d6d436047

src/graph.cc

index b19dc85..41055ec 100644 (file)
@@ -226,6 +226,7 @@ struct EdgeEnv : public Env {
                       vector<Node*>::iterator end,
                       char sep);
 
+  vector<string> lookups_;
   Edge* edge_;
   EscapeKind escape_in_out_;
 };
@@ -243,8 +244,19 @@ string EdgeEnv::LookupVariable(const string& var) {
                         ' ');
   }
 
+  vector<string>::const_iterator it;
+  if ((it = find(lookups_.begin(), lookups_.end(), var)) != lookups_.end()) {
+    string cycle;
+    for (; it != lookups_.end(); ++it)
+      cycle.append(*it + " -> ");
+    cycle.append(var);
+    Fatal(("cycle in rule variables: " + cycle).c_str());
+  }
+
   // See notes on BindingEnv::LookupWithFallback.
   const EvalString* eval = edge_->rule_->GetBinding(var);
+  if (eval)
+    lookups_.push_back(var);
   return edge_->env_->LookupWithFallback(var, eval, this);
 }