From d92cbf831c8395b6cf9a3d01a60fbf2a1767e3ea Mon Sep 17 00:00:00 2001 From: David Anderson Date: Thu, 24 May 2018 22:59:44 -0700 Subject: [PATCH 1/5] Fix a bug where not all blocks were enqueued for traversal --- vm/graph-builder.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/vm/graph-builder.cpp b/vm/graph-builder.cpp index f2bd00c1e..aa6f1cc64 100644 --- a/vm/graph-builder.cpp +++ b/vm/graph-builder.cpp @@ -128,6 +128,7 @@ GraphBuilder::scanFlow() -> FlowState RefPtr block = getOrAddBlock(cip_); current_->endWithJump(cip_, block); current_ = nullptr; + enqueueBlock(block); return FlowState::Ended; } From f8309f9636e793defb7a946b0c2a13e4b09e510d Mon Sep 17 00:00:00 2001 From: David Anderson Date: Thu, 24 May 2018 22:59:24 -0700 Subject: [PATCH 2/5] Add reverse/postordering to the control flow graph. --- third_party/amtl | 2 +- vm/control-flow.cpp | 95 +++++++++++++++++++++++++++++++++++++++++--- vm/control-flow.h | 36 ++++++++++++++++- vm/graph-builder.cpp | 3 +- 4 files changed, 127 insertions(+), 9 deletions(-) diff --git a/third_party/amtl b/third_party/amtl index 6d010aa92..c3a4526e2 160000 --- a/third_party/amtl +++ b/third_party/amtl @@ -1 +1 @@ -Subproject commit 6d010aa9275492bc7df18d47338c7b8dbfa2a2fc +Subproject commit c3a4526e2f678b7c2b5a3f9797b3ff1f023c40b8 diff --git a/vm/control-flow.cpp b/vm/control-flow.cpp index 8cd41ac81..d13c7587e 100644 --- a/vm/control-flow.cpp +++ b/vm/control-flow.cpp @@ -28,22 +28,106 @@ ControlFlowGraph::ControlFlowGraph(PluginRuntime* rt, const uint8_t* start_offse ControlFlowGraph::~ControlFlowGraph() { // This is necessary because blocks contain cycles between each other. - for (const auto& block : blocks_) - block->unlink(); + auto iter = blocks_.begin(); + while (iter != blocks_.end()) { + Block* block = *iter; + iter = blocks_.erase(iter); + block->Release(); + } } -ke::RefPtr +RefPtr ControlFlowGraph::newBlock(const uint8_t* start) { - ke::RefPtr block = new Block(*this, start); + Block* block = new Block(*this, start); + block->AddRef(); blocks_.append(block); return block; } +// By default, we want to store blocks in reverse postorder, which is the +// easiest form for forward flow analysis. +void +ControlFlowGraph::computeOrdering() +{ + // Note, the following graphs might look similar but they are *not* + // ambiguous: + // + // A -> B -> D -> E + // A -> C + // B -> C + // + // And its mirror, + // + // A -> C -> E + // A -> B -> D + // B -> C + // + // In both cases the correct traversal order is A, B, C, D, E. The only way + // to get this is to compute a post-order traversal, left-to-right and + // reverse it. This will guarantee the critical condition that a predecessor + // always appears before its successor in the reverse postordering, unless + // there is a backedge. (In fact, this will be our definition of a backedge). + struct Entry { + // All blocks will stay alive during this time, so we don't store RefPtrs. + Block* block; + size_t index; + }; + Vector work; + +#if !defined(NDEBUG) + size_t block_count = blocks_.length(); +#endif + + // Clear the old block list. + auto iter = blocks_.begin(); + while (iter != blocks_.end()) + iter = blocks_.erase(iter); + + Vector postordering; + + // Compute the postorder traversal. + newEpoch(); + work.append(Entry{entry_, 0}); + while (!work.empty()) { + Block* block = work.back().block; + size_t successor_index = work.back().index; + + if (successor_index >= block->successors().length()) { + postordering.append(block); + work.pop(); + continue; + } + work.back().index++; + + Block* child = block->successors()[successor_index]; + if (child->visited()) + continue; + child->setVisited(); + + if (!child->successors().empty()) + work.append(Entry{child, 0}); + else + postordering.append(child); + } + assert(postordering.length() == block_count); + + // Add and number everything in RPO. + uint32_t id = 1; + for (size_t i = postordering.length() - 1; i < postordering.length(); i--) { + Block* block = postordering[i]; + + assert(!block->id()); + block->setId(id++); + blocks_.append(block); + } +} + void ControlFlowGraph::dump(FILE* fp) { - for (const auto& block : blocks_) { + for (RpoIterator iter = rpoBegin(); iter != rpoEnd(); iter++) { + RefPtr block = *iter; fprintf(fp, "Block %p:\n", block.get()); for (const auto& pred : block->predecessors()) fprintf(fp, " predecessor: %p\n", pred.get()); @@ -71,6 +155,7 @@ Block::Block(ControlFlowGraph& graph, const uint8_t* start) start_(start), end_(nullptr), end_type_(BlockEnd::Unknown), + id_(0), epoch_(0) { } diff --git a/vm/control-flow.h b/vm/control-flow.h index 06e228367..103e7f151 100644 --- a/vm/control-flow.h +++ b/vm/control-flow.h @@ -13,6 +13,7 @@ #ifndef _include_sourcepawn_vm_control_flow_h_ #define _include_sourcepawn_vm_control_flow_h_ +#include #include #include #include @@ -28,7 +29,9 @@ enum class BlockEnd { Jump }; -class Block : public ke::Refcounted +class Block : + public ke::Refcounted, + public ke::InlineListNode { friend class ControlFlowGraph; @@ -54,6 +57,12 @@ class Block : public ke::Refcounted const ke::Vector>& successors() const { return successors_; } + uint32_t id() const { + return id_; + } + void setId(uint32_t id) { + id_ = id; + } ~Block() { assert(!predecessors_.length()); @@ -82,10 +91,16 @@ class Block : public ke::Refcounted const uint8_t* end_; BlockEnd end_type_; + // Reverse post-order index. + uint32_t id_; + // Counter for fast already-visited testing. uint32_t epoch_; }; +typedef ke::InlineList::iterator RpoIterator; +typedef ke::InlineList::reverse_iterator PostorderIterator; + class ControlFlowGraph : public ke::Refcounted { public: @@ -98,6 +113,23 @@ class ControlFlowGraph : public ke::Refcounted ke::RefPtr newBlock(const uint8_t* start); + // Compute reverse/postorder traversal of the graph. This re-orders blocks. + void computeOrdering(); + + // Iterators for blocks - reverse postorder, and postorder. + RpoIterator rpoBegin() { + return blocks_.begin(); + } + RpoIterator rpoEnd() { + return blocks_.end(); + } + PostorderIterator poBegin() { + return blocks_.rbegin(); + } + PostorderIterator poEnd() { + return blocks_.rend(); + } + // Increase the epoch number, effectively resetting which blcoks have been // visited. void newEpoch() { @@ -112,7 +144,7 @@ class ControlFlowGraph : public ke::Refcounted private: PluginRuntime* rt_; ke::RefPtr entry_; - ke::Vector> blocks_; + ke::InlineList blocks_; uint32_t epoch_; }; diff --git a/vm/graph-builder.cpp b/vm/graph-builder.cpp index aa6f1cc64..874de9a51 100644 --- a/vm/graph-builder.cpp +++ b/vm/graph-builder.cpp @@ -39,8 +39,9 @@ GraphBuilder::build() if (!scan()) return nullptr; - assert(graph_); assert(!error_code_); + + graph_->computeOrdering(); return graph_; } From 5dacfa9aeea580d25cc9971ca67cf097761110cd Mon Sep 17 00:00:00 2001 From: David Anderson Date: Sat, 26 May 2018 01:07:42 -0700 Subject: [PATCH 3/5] Add a control-flow pass to immediate dominators. --- vm/control-flow.cpp | 106 +++++++++++++++++++++++++++++++++++++++++++- vm/control-flow.h | 20 +++++++-- 2 files changed, 120 insertions(+), 6 deletions(-) diff --git a/vm/control-flow.cpp b/vm/control-flow.cpp index d13c7587e..f2b3da8f1 100644 --- a/vm/control-flow.cpp +++ b/vm/control-flow.cpp @@ -13,6 +13,7 @@ #include "control-flow.h" #include "opcodes.h" +#include namespace sp { @@ -123,12 +124,83 @@ ControlFlowGraph::computeOrdering() } } +// "A Simple, Fast Dominance Algorithm" by Keith D. Cooper, Timothy J. HArvey, +// and Ken Kennedy. +// +// Iterative solution with a two-finger intersection algorithm. Note the paper +// uses postordered numbers, whereas we use RPO. The id comparisons are +// therefore inverted. +static Block* +IntersectDominators(Block* block1, Block* block2) +{ + Block* finger1 = block1; + Block* finger2 = block2; + while (finger1 != finger2) { + while (finger1->id() > finger2->id()) + finger1 = finger1->idom(); + while (finger2->id() > finger1->id()) + finger2 = finger2->idom(); + } + return finger1; +} + +void +ControlFlowGraph::computeDominators() +{ + // The entry block dominates itself. + entry_->setImmediateDominator(entry_); + + // Compute immediate dominators. This is technically an O(n^2) algorithm, + // with a worst case being that every block is a join point. In practice + // it is nowhere near that bad. In addition, since we traverse the graph + // in RPO, the maximum number of iterations is 2. + // + // This is not technically true, if the graph is irreducible. Irreducible + // graphs will fail to validate in the computeLoopHeaders pass. + bool changed = false; + do { + changed = false; + + for (auto iter = rpoBegin(); iter != rpoEnd(); iter++) { + Block* block = *iter; + + if (block->predecessors().empty()) { + // There is only one entry block. + assert(block == entry_); + continue; + } + + // Pick a candidate for this node's dominator. + Block* idom = nullptr; + for (size_t i = 0; i < block->predecessors().length(); i++) { + Block* pred = block->predecessors()[i]; + if (!pred->idom()) + continue; + if (idom) + idom = IntersectDominators(idom, pred); + else + idom = pred; + assert(idom); + } + + // We should always get one candidate dominator, since RPO order + // guarantees we've processed at least one predecessor. + assert(idom); + + if (idom != block->idom()) { + block->setImmediateDominator(idom); + changed = true; + } + } + } while (changed); +} + void ControlFlowGraph::dump(FILE* fp) { for (RpoIterator iter = rpoBegin(); iter != rpoEnd(); iter++) { - RefPtr block = *iter; - fprintf(fp, "Block %p:\n", block.get()); + Block* block = *iter; + fprintf(fp, "Block %p (%d):\n", block, block->id()); for (const auto& pred : block->predecessors()) fprintf(fp, " predecessor: %p\n", pred.get()); for (const auto& child : block->successors()) @@ -150,6 +222,29 @@ ControlFlowGraph::dump(FILE* fp) } } +static AString +MakeDotBlockname(Block* block) +{ + AString name; + name.format("block%d_%p", block->id(), block); + return name; +} + +void +ControlFlowGraph::dumpDot(FILE* fp) +{ + fprintf(fp, "digraph cfg {\n"); + for (RpoIterator iter = rpoBegin(); iter != rpoEnd(); iter++) { + Block* block = *iter; + for (const auto& successor : block->successors()) { + fprintf(fp, " %s -> %s;\n", + MakeDotBlockname(block).chars(), + MakeDotBlockname(successor).chars()); + } + } + fprintf(fp, "}\n"); +} + Block::Block(ControlFlowGraph& graph, const uint8_t* start) : graph_(graph), start_(start), @@ -194,11 +289,18 @@ Block::setVisited() epoch_ = graph_.epoch(); } +void +Block::setImmediateDominator(Block* block) +{ + idom_ = block; +} + void Block::unlink() { predecessors_.clear(); successors_.clear(); + idom_ = nullptr; } } // namespace sp diff --git a/vm/control-flow.h b/vm/control-flow.h index 103e7f151..ff8eb9893 100644 --- a/vm/control-flow.h +++ b/vm/control-flow.h @@ -39,6 +39,11 @@ class Block : Block(ControlFlowGraph& graph, const uint8_t* start); public: + ~Block() { + assert(!predecessors_.length()); + assert(!successors_.length()); + } + const uint8_t* start() const { return start_; } @@ -63,16 +68,16 @@ class Block : void setId(uint32_t id) { id_ = id; } - - ~Block() { - assert(!predecessors_.length()); - assert(!successors_.length()); + Block* idom() const { + return idom_.get(); } void addTarget(Block* target); void endWithJump(const uint8_t* cip, Block* target); void end(const uint8_t* end_at, BlockEnd end_type); + void setImmediateDominator(Block* block); + bool visited() const; void setVisited(); @@ -94,6 +99,9 @@ class Block : // Reverse post-order index. uint32_t id_; + // Immediate dominator. + ke::RefPtr idom_; + // Counter for fast already-visited testing. uint32_t epoch_; }; @@ -116,6 +124,9 @@ class ControlFlowGraph : public ke::Refcounted // Compute reverse/postorder traversal of the graph. This re-orders blocks. void computeOrdering(); + // Compute dominance. This should be called after the graph is finalized. + void computeDominators(); + // Iterators for blocks - reverse postorder, and postorder. RpoIterator rpoBegin() { return blocks_.begin(); @@ -140,6 +151,7 @@ class ControlFlowGraph : public ke::Refcounted } void dump(FILE* fp); + void dumpDot(FILE* fp); private: PluginRuntime* rt_; From f03cbd1cf888331b4a6035239e01004c7eed97d3 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Sat, 26 May 2018 17:43:44 -0700 Subject: [PATCH 4/5] Compute a dominator tree so we can perform dominance tests. --- third_party/amtl | 2 +- vm/control-flow.cpp | 71 +++++++++++++++++++++++++++++++++++++++++++- vm/control-flow.h | 31 +++++++++++++++++-- vm/graph-builder.cpp | 1 + 4 files changed, 101 insertions(+), 4 deletions(-) diff --git a/third_party/amtl b/third_party/amtl index c3a4526e2..c54b14bc6 160000 --- a/third_party/amtl +++ b/third_party/amtl @@ -1 +1 @@ -Subproject commit c3a4526e2f678b7c2b5a3f9797b3ff1f023c40b8 +Subproject commit c54b14bc6781765a7096ab321eaccd5edfb15068 diff --git a/vm/control-flow.cpp b/vm/control-flow.cpp index f2b3da8f1..4bb6b1423 100644 --- a/vm/control-flow.cpp +++ b/vm/control-flow.cpp @@ -145,7 +145,7 @@ IntersectDominators(Block* block1, Block* block2) } void -ControlFlowGraph::computeDominators() +ControlFlowGraph::computeDominance() { // The entry block dominates itself. entry_->setImmediateDominator(entry_); @@ -193,6 +193,45 @@ ControlFlowGraph::computeDominators() } } } while (changed); + + // Build the dominator tree, walking the graph bottom-up. + for (auto iter = poBegin(); iter != poEnd(); iter++) { + Block* block = *iter; + Block* idom = block->idom(); + + // Only the entry should have itself as an immediate dominator. + if (block == idom) { + assert(block == entry_); + continue; + } + + idom->addImmediatelyDominated(block); + } + assert(entry_->numDominated() == blocks_.length()); + + // Process the dominator tree. Note that it is acyclic, so we do not need a + // visited marker. We walk the tree and assign a pre-order index to each + // dominator. + Vector work; + work.append(entry_); + + uint32_t id = 0; + while (!work.empty()) { + Block* block = work.popCopy(); + + // We should never visit the same block twice. + assert(!block->domTreeId()); + + block->setDomTreeId(id++); + for (const auto& child : block->immediatelyDominated()) + work.append(child); + } + +#if !defined(NDEBUG) + // Every node should have an index in the dominator tree. + for (auto iter = rpoBegin(); iter != rpoEnd(); iter++) + assert(iter->domTreeId() || *iter == entry_); +#endif } void @@ -245,12 +284,34 @@ ControlFlowGraph::dumpDot(FILE* fp) fprintf(fp, "}\n"); } +void +ControlFlowGraph::dumpDomTreeDot(FILE* fp) +{ + fprintf(fp, "digraph domtree {\n"); + + Vector work; + work.append(entry_); + while (!work.empty()) { + Block* block = work.popCopy(); + for (const auto& child : block->immediatelyDominated()) { + fprintf(fp, " %s -> %s;\n", + MakeDotBlockname(block).chars(), + MakeDotBlockname(child).chars()); + work.append(child); + } + } + + fprintf(fp, "}\n"); +} + Block::Block(ControlFlowGraph& graph, const uint8_t* start) : graph_(graph), start_(start), end_(nullptr), end_type_(BlockEnd::Unknown), id_(0), + domtree_id_(0), + num_dominated_(1), epoch_(0) { } @@ -295,12 +356,20 @@ Block::setImmediateDominator(Block* block) idom_ = block; } +void +Block::addImmediatelyDominated(Block* child) +{ + immediately_dominated_.append(child); + num_dominated_ += child->numDominated(); +} + void Block::unlink() { predecessors_.clear(); successors_.clear(); idom_ = nullptr; + immediately_dominated_.clear(); } } // namespace sp diff --git a/vm/control-flow.h b/vm/control-flow.h index ff8eb9893..85ffe2c79 100644 --- a/vm/control-flow.h +++ b/vm/control-flow.h @@ -71,12 +71,33 @@ class Block : Block* idom() const { return idom_.get(); } + uint32_t domTreeId() const { + return domtree_id_; + } + void setDomTreeId(uint32_t id) { + domtree_id_ = id; + } + uint32_t numDominated() const { + return num_dominated_; + } + const ke::Vector>& immediatelyDominated() const { + return immediately_dominated_; + } + bool dominates(const Block* other) const { + // The dominator tree is numbered in pre-order, and num_dominated_ is total + // number of children in our position in the tree. Therefore, we can check + // if we dominate a node by seeing if its index is within the range of + // indices under this node. + return other->domtree_id_ >= domtree_id_ && + other->domtree_id_ < (domtree_id_ + num_dominated_); + } void addTarget(Block* target); void endWithJump(const uint8_t* cip, Block* target); void end(const uint8_t* end_at, BlockEnd end_type); void setImmediateDominator(Block* block); + void addImmediatelyDominated(Block* block); bool visited() const; void setVisited(); @@ -99,8 +120,13 @@ class Block : // Reverse post-order index. uint32_t id_; - // Immediate dominator. + // Immediate dominator, and list of dominated blocks. ke::RefPtr idom_; + ke::Vector> immediately_dominated_; + // Dominator tree index. + uint32_t domtree_id_; + // The number of nodes this block dominates, including itself. + uint32_t num_dominated_; // Counter for fast already-visited testing. uint32_t epoch_; @@ -125,7 +151,7 @@ class ControlFlowGraph : public ke::Refcounted void computeOrdering(); // Compute dominance. This should be called after the graph is finalized. - void computeDominators(); + void computeDominance(); // Iterators for blocks - reverse postorder, and postorder. RpoIterator rpoBegin() { @@ -152,6 +178,7 @@ class ControlFlowGraph : public ke::Refcounted void dump(FILE* fp); void dumpDot(FILE* fp); + void dumpDomTreeDot(FILE* fp); private: PluginRuntime* rt_; diff --git a/vm/graph-builder.cpp b/vm/graph-builder.cpp index 874de9a51..b81657840 100644 --- a/vm/graph-builder.cpp +++ b/vm/graph-builder.cpp @@ -42,6 +42,7 @@ GraphBuilder::build() assert(!error_code_); graph_->computeOrdering(); + graph_->computeDominance(); return graph_; } From 31dd66068a645d59b4eadb4193ffba8a3e207c00 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Sat, 26 May 2018 21:19:27 -0700 Subject: [PATCH 5/5] Mark loop headers and validate reducibility. --- vm/control-flow.cpp | 22 ++++++++++++++++++++++ vm/control-flow.h | 12 ++++++++++++ vm/graph-builder.cpp | 4 ++++ 3 files changed, 38 insertions(+) diff --git a/vm/control-flow.cpp b/vm/control-flow.cpp index 4bb6b1423..301cdce5c 100644 --- a/vm/control-flow.cpp +++ b/vm/control-flow.cpp @@ -234,6 +234,28 @@ ControlFlowGraph::computeDominance() #endif } +bool +ControlFlowGraph::computeLoopHeaders() +{ + // This algorithm is O(E), where E is the number of edges in the graph. + for (auto iter = rpoBegin(); iter != rpoEnd(); iter++) { + Block* block = *iter; + for (const auto& child : iter->successors()) { + // Since we number blocks in RPO, any backedge will precede this block + // in the graph. + if (child->id() <= block->id()) { + child->setIsLoopHeader(); + + // If the loop header does not dominate the contained block, then + // the graph is not reducible. We want this to be invalid for now. + if (!child->dominates(block)) + return false; + } + } + } + return true; +} + void ControlFlowGraph::dump(FILE* fp) { diff --git a/vm/control-flow.h b/vm/control-flow.h index 85ffe2c79..88cebc177 100644 --- a/vm/control-flow.h +++ b/vm/control-flow.h @@ -91,6 +91,12 @@ class Block : return other->domtree_id_ >= domtree_id_ && other->domtree_id_ < (domtree_id_ + num_dominated_); } + void setIsLoopHeader() { + is_loop_header_ = true; + } + bool isLoopHeader() const { + return is_loop_header_; + } void addTarget(Block* target); void endWithJump(const uint8_t* cip, Block* target); @@ -128,6 +134,9 @@ class Block : // The number of nodes this block dominates, including itself. uint32_t num_dominated_; + // Set to true if this is a loop header. + bool is_loop_header_; + // Counter for fast already-visited testing. uint32_t epoch_; }; @@ -153,6 +162,9 @@ class ControlFlowGraph : public ke::Refcounted // Compute dominance. This should be called after the graph is finalized. void computeDominance(); + // Compute validity of loops. + bool computeLoopHeaders(); + // Iterators for blocks - reverse postorder, and postorder. RpoIterator rpoBegin() { return blocks_.begin(); diff --git a/vm/graph-builder.cpp b/vm/graph-builder.cpp index b81657840..45c1678cc 100644 --- a/vm/graph-builder.cpp +++ b/vm/graph-builder.cpp @@ -43,6 +43,10 @@ GraphBuilder::build() graph_->computeOrdering(); graph_->computeDominance(); + if (!graph_->computeLoopHeaders()) { + error(SP_ERROR_INVALID_INSTRUCTION); + return nullptr; + } return graph_; }