From dd8a7e1bc639a0690529bf89ae9f8b0e80186817 Mon Sep 17 00:00:00 2001 From: "kmillikin@chromium.org" Date: Wed, 10 Mar 2010 17:02:25 +0000 Subject: [PATCH] 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 | 4 +- src/data-flow.cc | 102 ++++++++++++++++---------------- src/data-flow.h | 148 +++++++++++++++++++++-------------------------- 3 files changed, 117 insertions(+), 137 deletions(-) diff --git a/src/compiler.cc b/src/compiler.cc index ebb62f11c2..11fff1abb8 100755 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -84,7 +84,7 @@ static Handle MakeCode(Handle 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 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 diff --git a/src/data-flow.cc b/src/data-flow.cc index f8af2c16e6..70c90a1200 100644 --- a/src/data-flow.cc +++ b/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* preorder, - ZoneList* 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* preorder, ZoneList* 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()); } diff --git a/src/data-flow.h b/src/data-flow.h index 2dc2d73275..8f58283e0e 100644 --- a/src/data-flow.h +++ b/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* 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* preorder, - ZoneList* 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* 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* 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) {