Add defensive checks to the flow graph builder.

Visitor stack overflow is used to signal an unsupported construct in
the flow graph.  Check for it in more places.  Make the utility
functions for appending to graphs handle more cases if they can be
handled correctly.

Remove the entry node in favor of a block with a NULL predecessor as
single entry.  Represent the empty flow graph as a single empty block.
Add empty blocks lazily where needed to preserve edge-split form.

Review URL: http://codereview.chromium.org/804003

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4086 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/compiler.cc

@@ -84,7 +84,7 @@ static Handle<Code> MakeCode(Handle<Context> context, CompilationInfo* info) {
     builder.Build(function);
 
 #ifdef DEBUG
-    if (FLAG_print_graph_text) {
+    if (FLAG_print_graph_text && !builder.HasStackOverflow()) {
       builder.graph()->PrintText(builder.postorder());
     }
 #endif
@@ -468,7 +468,7 @@ Handle<JSFunction> Compiler::BuildBoilerplate(FunctionLiteral* literal,
       builder.Build(literal);
 
 #ifdef DEBUG
-      if (FLAG_print_graph_text) {
+      if (FLAG_print_graph_text && !builder.HasStackOverflow()) {
         builder.graph()->PrintText(builder.postorder());
       }
 #endif

src/data-flow.cc

@@ -34,67 +34,69 @@ namespace internal {
 
 
 void FlowGraph::AppendInstruction(AstNode* instruction) {
+  // Add a (non-null) AstNode to the end of the graph fragment.
   ASSERT(instruction != NULL);
-  if (is_empty() || !exit()->IsBlockNode()) {
-    AppendNode(new BlockNode());
-  }
+  if (exit()->IsExitNode()) return;
+  if (!exit()->IsBlockNode()) AppendNode(new BlockNode());
   BlockNode::cast(exit())->AddInstruction(instruction);
 }
 
 
 void FlowGraph::AppendNode(Node* node) {
+  // Add a node to the end of the graph.  An empty block is added to
+  // maintain edge-split form (that no join nodes or exit nodes as
+  // successors to branch nodes).
   ASSERT(node != NULL);
-  if (is_empty()) {
-    entry_ = exit_ = node;
-  } else {
-    exit()->AddSuccessor(node);
-    node->AddPredecessor(exit());
-    exit_ = node;
+  if (exit()->IsExitNode()) return;
+  if (exit()->IsBranchNode() && (node->IsJoinNode() || node->IsExitNode())) {
+    AppendNode(new BlockNode());
   }
+  exit()->AddSuccessor(node);
+  node->AddPredecessor(exit());
+  exit_ = node;
 }
 
 
 void FlowGraph::AppendGraph(FlowGraph* graph) {
-  ASSERT(!graph->is_empty());
-  if (is_empty()) {
-    entry_ = graph->entry();
-    exit_ = graph->exit();
-  } else {
-    exit()->AddSuccessor(graph->entry());
-    graph->entry()->AddPredecessor(exit());
-    exit_ = graph->exit();
+  // Add a flow graph fragment to the end of this one.  An empty block is
+  // added to maintain edge-split form (that no join nodes or exit nodes as
+  // successors to branch nodes).
+  ASSERT(graph != NULL);
+  if (exit()->IsExitNode()) return;
+  Node* node = graph->entry();
+  if (exit()->IsBranchNode() && (node->IsJoinNode() || node->IsExitNode())) {
+    AppendNode(new BlockNode());
   }
+  exit()->AddSuccessor(node);
+  node->AddPredecessor(exit());
+  exit_ = graph->exit();
 }
 
 
 void FlowGraph::Split(BranchNode* branch,
                       FlowGraph* left,
                       FlowGraph* right,
-                      JoinNode* merge) {
-  // Graphs are in edge split form.  Add empty blocks if necessary.
-  if (left->is_empty()) left->AppendNode(new BlockNode());
-  if (right->is_empty()) right->AppendNode(new BlockNode());
-
-  // Add the branch, left flowgraph and merge.
+                      JoinNode* join) {
+  // Add the branch node, left flowgraph, join node.
   AppendNode(branch);
   AppendGraph(left);
-  AppendNode(merge);
+  AppendNode(join);
 
   // Splice in the right flowgraph.
-  right->AppendNode(merge);
+  right->AppendNode(join);
   branch->AddSuccessor(right->entry());
   right->entry()->AddPredecessor(branch);
 }
 
 
-void FlowGraph::Loop(JoinNode* merge,
+void FlowGraph::Loop(JoinNode* join,
                      FlowGraph* condition,
                      BranchNode* branch,
                      FlowGraph* body) {
-  // Add the merge, condition and branch.  Add merge's predecessors in
+  // Add the join, condition and branch.  Add join's predecessors in
   // left-to-right order.
-  AppendNode(merge);
-  body->AppendNode(merge);
+  AppendNode(join);
+  body->AppendNode(join);
   AppendGraph(condition);
   AppendNode(branch);
 
@@ -104,19 +106,6 @@ void FlowGraph::Loop(JoinNode* merge,
 }
 
 
-void EntryNode::Traverse(bool mark,
-                         ZoneList<Node*>* preorder,
-                         ZoneList<Node*>* postorder) {
-  ASSERT(successor_ != NULL);
-  preorder->Add(this);
-  if (!successor_->IsMarkedWith(mark)) {
-    successor_->MarkWith(mark);
-    successor_->Traverse(mark, preorder, postorder);
-  }
-  postorder->Add(this);
-}
-
-
 void ExitNode::Traverse(bool mark,
                         ZoneList<Node*>* preorder,
                         ZoneList<Node*>* postorder) {
@@ -169,16 +158,15 @@ void JoinNode::Traverse(bool mark,
 
 
 void FlowGraphBuilder::Build(FunctionLiteral* lit) {
-  graph_ = FlowGraph::Empty();
-  graph_.AppendNode(new EntryNode());
   global_exit_ = new ExitNode();
   VisitStatements(lit->body());
 
-  if (HasStackOverflow()) {
-    graph_ = FlowGraph::Empty();
-    return;
-  }
+  if (HasStackOverflow()) return;
 
+  // The graph can end with a branch node (if the function ended with a
+  // loop).  Maintain edge-split form (no join nodes or exit nodes as
+  // successors to branch nodes).
+  if (graph_.exit()->IsBranchNode()) graph_.AppendNode(new BlockNode());
   graph_.AppendNode(global_exit_);
 
   // Build preorder and postorder traversal orders.  All the nodes in
@@ -222,6 +210,7 @@ void FlowGraphBuilder::VisitIfStatement(IfStatement* stmt) {
   graph_ = FlowGraph::Empty();
   Visit(stmt->else_statement());
 
+  if (HasStackOverflow()) return;
   JoinNode* join = new JoinNode();
   original.Split(branch, &left, &graph_, join);
   graph_ = original;
@@ -283,6 +272,7 @@ void FlowGraphBuilder::VisitForStatement(ForStatement* stmt) {
 
   if (stmt->next() != NULL) Visit(stmt->next());
 
+  if (HasStackOverflow()) return;
   original.Loop(join, &condition, branch, &graph_);
   graph_ = original;
 }
@@ -374,6 +364,8 @@ void FlowGraphBuilder::VisitAssignment(Assignment* expr) {
     if (!prop->key()->IsPropertyName()) Visit(prop->key());
     Visit(expr->value());
   }
+
+  if (HasStackOverflow()) return;
   graph_.AppendInstruction(expr);
 }
 
@@ -386,6 +378,8 @@ void FlowGraphBuilder::VisitThrow(Throw* expr) {
 void FlowGraphBuilder::VisitProperty(Property* expr) {
   Visit(expr->obj());
   if (!expr->key()->IsPropertyName()) Visit(expr->key());
+
+  if (HasStackOverflow()) return;
   graph_.AppendInstruction(expr);
 }
 
@@ -396,6 +390,8 @@ void FlowGraphBuilder::VisitCall(Call* expr) {
   for (int i = 0, len = arguments->length(); i < len; i++) {
     Visit(arguments->at(i));
   }
+
+  if (HasStackOverflow()) return;
   graph_.AppendInstruction(expr);
 }
 
@@ -423,6 +419,7 @@ void FlowGraphBuilder::VisitUnaryOperation(UnaryOperation* expr) {
     case Token::ADD:
     case Token::SUB:
       Visit(expr->expression());
+      if (HasStackOverflow()) return;
       graph_.AppendInstruction(expr);
       break;
 
@@ -438,6 +435,8 @@ void FlowGraphBuilder::VisitCountOperation(CountOperation* expr) {
   if (var != NULL && var->IsStackAllocated()) {
     definitions_.Add(expr);
   }
+
+  if (HasStackOverflow()) return;
   graph_.AppendInstruction(expr);
 }
 
@@ -463,6 +462,7 @@ void FlowGraphBuilder::VisitBinaryOperation(BinaryOperation* expr) {
     case Token::SAR:
       Visit(expr->left());
       Visit(expr->right());
+      if (HasStackOverflow()) return;
       graph_.AppendInstruction(expr);
       break;
 
@@ -489,6 +489,7 @@ void FlowGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
     case Token::GTE:
       Visit(expr->left());
       Visit(expr->right());
+      if (HasStackOverflow()) return;
       graph_.AppendInstruction(expr);
       break;
 
@@ -1341,11 +1342,6 @@ void BlockNode::AssignNumbers() {
 }
 
 
-void EntryNode::PrintText() {
-  PrintF("L%d: Entry\n", number());
-  PrintF("goto L%d\n\n", successor_->number());
-}
-
 void ExitNode::PrintText() {
   PrintF("L%d: Exit\n\n", number());
 }

src/data-flow.h

@@ -122,59 +122,6 @@ class BitVector: public ZoneObject {
 };
 
 
-// Forward declarations of Node types.
-class Node;
-class BranchNode;
-class JoinNode;
-
-// Flow graphs have a single entry and single exit.  The empty flowgraph is
-// represented by both entry and exit being NULL.
-class FlowGraph BASE_EMBEDDED {
- public:
-  FlowGraph() : entry_(NULL), exit_(NULL) {}
-
-  static FlowGraph Empty() { return FlowGraph(); }
-
-  bool is_empty() const { return entry_ == NULL; }
-  Node* entry() const { return entry_; }
-  Node* exit() const { return exit_; }
-
-  // Add a single instruction to the end of this flowgraph.
-  void AppendInstruction(AstNode* instruction);
-
-  // Add a single node to the end of this flow graph.
-  void AppendNode(Node* node);
-
-  // Add a flow graph fragment to the end of this one.
-  void AppendGraph(FlowGraph* graph);
-
-  // Concatenate an if-then-else flow-graph to this one.  Control is split
-  // and merged, so the graph remains single-entry, single-exit.
-  void Split(BranchNode* branch,
-             FlowGraph* left,
-             FlowGraph* right,
-             JoinNode* merge);
-
-  // Concatenate a forward loop (e.g., while or for loop) flow-graph to this
-  // one.  Control is split by the condition and merged back from the back
-  // edge at end of the body to the beginning of the condition.  The single
-  // (free) exit of the result graph is the right (false) arm of the branch
-  // node.
-  void Loop(JoinNode* merge,
-            FlowGraph* condition,
-            BranchNode* branch,
-            FlowGraph* body);
-
-#ifdef DEBUG
-  void PrintText(ZoneList<Node*>* postorder);
-#endif
-
- private:
-  Node* entry_;
-  Node* exit_;
-};
-
-
 // Flow-graph nodes.
 class Node: public ZoneObject {
  public:
@@ -182,7 +129,9 @@ class Node: public ZoneObject {
 
   virtual ~Node() {}
 
+  virtual bool IsExitNode() { return false; }
   virtual bool IsBlockNode() { return false; }
+  virtual bool IsBranchNode() { return false; }
   virtual bool IsJoinNode() { return false; }
 
   virtual void AddPredecessor(Node* predecessor) = 0;
@@ -213,44 +162,19 @@ class Node: public ZoneObject {
 };
 
 
-// An entry node has no predecessors and a single successor.
-class EntryNode: public Node {
- public:
-  EntryNode() : successor_(NULL) {}
-
-  void AddPredecessor(Node* predecessor) { UNREACHABLE(); }
-
-  void AddSuccessor(Node* successor) {
-    ASSERT(successor_ == NULL && successor != NULL);
-    successor_ = successor;
-  }
-
-  void Traverse(bool mark,
-                ZoneList<Node*>* preorder,
-                ZoneList<Node*>* postorder);
-
-#ifdef DEBUG
-  void PrintText();
-#endif
-
- private:
-  Node* successor_;
-
-  DISALLOW_COPY_AND_ASSIGN(EntryNode);
-};
-
-
 // An exit node has a arbitrarily many predecessors and no successors.
 class ExitNode: public Node {
  public:
   ExitNode() : predecessors_(4) {}
 
+  bool IsExitNode() { return true; }
+
   void AddPredecessor(Node* predecessor) {
     ASSERT(predecessor != NULL);
     predecessors_.Add(predecessor);
   }
 
-  void AddSuccessor(Node* successor) { /* Do nothing. */ }
+  void AddSuccessor(Node* successor) { UNREACHABLE(); }
 
   void Traverse(bool mark,
                 ZoneList<Node*>* preorder,
@@ -280,6 +204,8 @@ class BlockNode: public Node {
 
   bool IsBlockNode() { return true; }
 
+  bool is_empty() { return instructions_.is_empty(); }
+
   void AddPredecessor(Node* predecessor) {
     ASSERT(predecessor_ == NULL && predecessor != NULL);
     predecessor_ = predecessor;
@@ -317,6 +243,8 @@ class BranchNode: public Node {
  public:
   BranchNode() : predecessor_(NULL), successor0_(NULL), successor1_(NULL) {}
 
+  bool IsBranchNode() { return true; }
+
   void AddPredecessor(Node* predecessor) {
     ASSERT(predecessor_ == NULL && predecessor != NULL);
     predecessor_ = predecessor;
@@ -386,12 +314,68 @@ class JoinNode: public Node {
 };
 
 
+// Flow graphs have a single entry and single exit.  The empty flowgraph is
+// represented by both entry and exit being NULL.
+class FlowGraph BASE_EMBEDDED {
+ public:
+  static FlowGraph Empty() {
+    FlowGraph graph;
+    graph.entry_ = new BlockNode();
+    graph.exit_ = graph.entry_;
+    return graph;
+  }
+
+  bool is_empty() const {
+    return entry_ == exit_ && BlockNode::cast(entry_)->is_empty();
+  }
+  Node* entry() const { return entry_; }
+  Node* exit() const { return exit_; }
+
+  // Add a single instruction to the end of this flowgraph.
+  void AppendInstruction(AstNode* instruction);
+
+  // Add a single node to the end of this flow graph.
+  void AppendNode(Node* node);
+
+  // Add a flow graph fragment to the end of this one.
+  void AppendGraph(FlowGraph* graph);
+
+  // Concatenate an if-then-else flow-graph to this one.  Control is split
+  // and merged, so the graph remains single-entry, single-exit.
+  void Split(BranchNode* branch,
+             FlowGraph* left,
+             FlowGraph* right,
+             JoinNode* merge);
+
+  // Concatenate a forward loop (e.g., while or for loop) flow-graph to this
+  // one.  Control is split by the condition and merged back from the back
+  // edge at end of the body to the beginning of the condition.  The single
+  // (free) exit of the result graph is the right (false) arm of the branch
+  // node.
+  void Loop(JoinNode* merge,
+            FlowGraph* condition,
+            BranchNode* branch,
+            FlowGraph* body);
+
+#ifdef DEBUG
+  void PrintText(ZoneList<Node*>* postorder);
+#endif
+
+ private:
+  FlowGraph() : entry_(NULL), exit_(NULL) {}
+
+  Node* entry_;
+  Node* exit_;
+};
+
+
 // Construct a flow graph from a function literal.  Build pre- and postorder
 // traversal orders as a byproduct.
 class FlowGraphBuilder: public AstVisitor {
  public:
   FlowGraphBuilder()
-      : global_exit_(NULL),
+      : graph_(FlowGraph::Empty()),
+        global_exit_(NULL),
         preorder_(4),
         postorder_(4),
         definitions_(4) {