Add defensive checks to the flow graph builder.
authorkmillikin@chromium.org <kmillikin@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 10 Mar 2010 17:02:25 +0000 (17:02 +0000)
committerkmillikin@chromium.org <kmillikin@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 10 Mar 2010 17:02:25 +0000 (17:02 +0000)
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
src/data-flow.cc
src/data-flow.h

index ebb62f11c2df42a4aabeac97632df72463091b50..11fff1abb8c7f38fccea117a8f202a258530e4bd 100755 (executable)
@@ -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
index f8af2c16e699eb5790e8d4953deca5cb8dbb22ec..70c90a12004ad7031ce5d31165d51fab2322c79e 100644 (file)
@@ -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());
 }
index 2dc2d73275c8a8ab02ea0bdbdd075f66d7e8b712..8f58283e0e6b15e2b72cce3eb2554464475d288c 100644 (file)
@@ -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) {