From 112d3c080171b7b44d9d59904915dbb17e24a769 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Fri, 12 Sep 2025 14:40:57 -0700 Subject: [PATCH 1/2] Add a `BlockId` module that is language agnostic, plus cpp impl. --- README.md | 14 + cpp/src/qtil/Cpp.qll | 1 + cpp/src/qtil/cpp/cfg/BlockId.qll | 41 +++ cpp/test/qtil/cpp/cfg/BlockIdTest.expected | 43 +++ cpp/test/qtil/cpp/cfg/BlockIdTest.ql | 5 + cpp/test/qtil/cpp/cfg/test.cpp | 154 ++++++++ src/qtil/cfg/BlockId.qll | 391 +++++++++++++++++++++ test/qtil/cfg/BlockIdTest.expected | 1 + test/qtil/cfg/BlockIdTest.ql | 339 ++++++++++++++++++ 9 files changed, 989 insertions(+) create mode 100644 cpp/src/qtil/cpp/cfg/BlockId.qll create mode 100644 cpp/test/qtil/cpp/cfg/BlockIdTest.expected create mode 100644 cpp/test/qtil/cpp/cfg/BlockIdTest.ql create mode 100644 cpp/test/qtil/cpp/cfg/test.cpp create mode 100644 src/qtil/cfg/BlockId.qll create mode 100644 test/qtil/cfg/BlockIdTest.expected create mode 100644 test/qtil/cfg/BlockIdTest.ql diff --git a/README.md b/README.md index 7f94683..6c5c928 100644 --- a/README.md +++ b/README.md @@ -238,6 +238,20 @@ predicate intPlusConstantOld(BinaryExpr e) { } ``` +### Control flow: + +_(qtil-cpp only)_ **BlockId**: This module assigns IDs to blocks or control flow nodes. It currently +assigns unique IDs to each block/node, except it can produce duplicates IDs in the presence of +switch/case statements. This module has a language-agnostic implementation but it is currently only +implemented for C/C++. + +```ql +import qtil.cpp.BlockId + +from BasicBlock b +select b, blockId(b) +``` + ### Query Formatting **QlFormat** offers a way of formatting CodeQL query messages in a consistent way, with varying diff --git a/cpp/src/qtil/Cpp.qll b/cpp/src/qtil/Cpp.qll index e0c5228..0d47009 100644 --- a/cpp/src/qtil/Cpp.qll +++ b/cpp/src/qtil/Cpp.qll @@ -3,6 +3,7 @@ module Qtil { // Importing qtil.Cpp should import all of Qtil. import Common::Qtil import qtil.cpp.ast.TwoOperands + import qtil.cpp.cfg.BlockId import qtil.cpp.format.QlFormat import qtil.cpp.graph.CustomPathProblem } diff --git a/cpp/src/qtil/cpp/cfg/BlockId.qll b/cpp/src/qtil/cpp/cfg/BlockId.qll new file mode 100644 index 0000000..ed435c4 --- /dev/null +++ b/cpp/src/qtil/cpp/cfg/BlockId.qll @@ -0,0 +1,41 @@ +private import cpp as cpp +private import qtil.cfg.BlockId +private import qtil.inheritance.Instance + +/** + * Get an ID for a given block, which is typically(*) unique. + * + * (*) Current implementation of algorithm that assigns blocks will not give unique for results + * inside switch/case statements. + */ +int blockId(cpp::BasicBlock bb) { result = CppBlockId::blockId(bb) } + +/** + * Get an ID for a given control flow node, which is typically(*) unique. + * + * (*) Current implementation of algorithm that assigns blocks will not give unique for results + * inside switch/case statements. + */ +int controlFlowNodeId(cpp::ControlFlowNode node) { result = CppBlockId::controlFlowNodeId(node) } + +private module CppBlockIdConfig implements BlockIdConfigSig { + class BasicBlock extends Instance::Type { + BasicBlock getTrueSuccessor() { result = inst().getATrueSuccessor() } + + BasicBlock getFalseSuccessor() { result = inst().getAFalseSuccessor() } + + BasicBlock getASuccessor() { result = inst().getASuccessor() } + + ControlFlowNode getNode(int index) { result = inst().getNode(index) } + } + + class EntryBasicBlock extends BasicBlock { + EntryBasicBlock() { this instanceof cpp::EntryBasicBlock } + } + + class ControlFlowNode extends Instance::Type { + BasicBlock getBasicBlock() { result = inst().getBasicBlock() } + } +} + +private import BlockId as CppBlockId diff --git a/cpp/test/qtil/cpp/cfg/BlockIdTest.expected b/cpp/test/qtil/cpp/cfg/BlockIdTest.expected new file mode 100644 index 0000000..52650de --- /dev/null +++ b/cpp/test/qtil/cpp/cfg/BlockIdTest.expected @@ -0,0 +1,43 @@ +| test.cpp:2:1:1:7 | { ... } | 1 | +| test.cpp:7:1:9:10 | { ... } | 1 | +| test.cpp:10:5:11:10 | { ... } | 2 | +| test.cpp:14:5:15:10 | { ... } | 3 | +| test.cpp:18:5:6:7 | ExprStmt | 4 | +| test.cpp:22:1:27:5 | { ... } | 1 | +| test.cpp:24:12:24:13 | p1 | 2 | +| test.cpp:25:5:26:10 | { ... } | 3 | +| test.cpp:29:5:21:7 | ExprStmt | 4 | +| test.cpp:32:6:32:7 | f4 | 4 | +| test.cpp:33:1:35:10 | { ... } | 1 | +| test.cpp:36:5:38:15 | { ... } | 2 | +| test.cpp:41:5:42:10 | ExprStmt | 3 | +| test.cpp:43:5:45:15 | { ... } | 5 | +| test.cpp:48:5:49:1 | ExprStmt | 6 | +| test.cpp:52:1:54:10 | { ... } | 1 | +| test.cpp:55:5:57:14 | { ... } | 2 | +| test.cpp:58:9:59:14 | { ... } | 4 | +| test.cpp:62:9:63:14 | { ... } | 5 | +| test.cpp:67:5:68:10 | { ... } | 3 | +| test.cpp:71:5:51:7 | ExprStmt | 6 | +| test.cpp:75:1:77:12 | { ... } | 1 | +| test.cpp:78:5:79:17 | { ... } | 2 | +| test.cpp:84:9:85:14 | { ... } | 3 | +| test.cpp:93:5:74:7 | ExprStmt | 4 | +| test.cpp:97:1:99:18 | { ... } | 1 | +| test.cpp:99:21:99:26 | i | 2 | +| test.cpp:100:5:102:22 | { ... } | 3 | +| test.cpp:103:9:104:14 | { ... } | 5 | +| test.cpp:107:9:108:14 | { ... } | 6 | +| test.cpp:111:9:99:31 | ExprStmt | 7 | +| test.cpp:114:5:96:7 | ExprStmt | 4 | +| test.cpp:118:1:120:12 | { ... } | 1 | +| test.cpp:121:5:123:22 | { ... } | 2 | +| test.cpp:123:25:123:29 | i | 3 | +| test.cpp:124:9:123:34 | { ... } | 4 | +| test.cpp:128:9:128:10 | ExprStmt | 5 | +| test.cpp:135:5:117:7 | ExprStmt | 6 | +| test.cpp:139:1:151:5 | { ... } | 1 | +| test.cpp:141:12:141:15 | 1 | 2 | +| test.cpp:142:5:144:16 | { ... } | 3 | +| test.cpp:145:9:147:18 | { ... } | 4 | +| test.cpp:151:5:138:7 | label ...: | 5 | diff --git a/cpp/test/qtil/cpp/cfg/BlockIdTest.ql b/cpp/test/qtil/cpp/cfg/BlockIdTest.ql new file mode 100644 index 0000000..fb49e5c --- /dev/null +++ b/cpp/test/qtil/cpp/cfg/BlockIdTest.ql @@ -0,0 +1,5 @@ +import cpp +import qtil.cpp.cfg.BlockId + +from BasicBlock b +select b, blockId(b) diff --git a/cpp/test/qtil/cpp/cfg/test.cpp b/cpp/test/qtil/cpp/cfg/test.cpp new file mode 100644 index 0000000..7e5eeee --- /dev/null +++ b/cpp/test/qtil/cpp/cfg/test.cpp @@ -0,0 +1,154 @@ +void f1() +{ // B1 + // Single basic block function. +} + +void f2(bool p1) +{ // B1 + f1(); + if (p1) + { // B2 + f1(); + } + else + { // B3 + f1(); + } + // B4 + f1(); +} + +void f3(bool p1) +{ // B1 + f1(); + while (p1) // B2 + { // B3 + f1(); + } + // B4 + f1(); +} + +void f4(bool p1) // B4 -- Not sure why this is assigned block 4 +{ // B1 + f1(); + if (p1) + { // B2 + f1(); + return; + } + // B3 + f1(); + if (p1) + { // B5 + f1(); + return; + } + // B6 + f1(); +} + +void f5(bool p1) +{ // B1 + f1(); + if (p1) + { // B2 + f1(); + if (p1) + { // B4 + f1(); + } + else + { // B5 + f1(); + } + } + else + { // B3 + f1(); + } + // B6 + f1(); +} + +void f6() +{ // B1 + f1(); + if (true) + { // B2 + if (false) + { // Unreachable, no ID. + f1(); + } + else + { // B3 + f1(); + } + } + else + { // Unreachable, no ID. + f1(); + } + // B4 + f1(); +} + +void f7() +{ // B1 + f1(); + for (int i = 0; i < 10; i++) // B2 + { // B3 + f1(); + if (i % 2 == 0) + { // B5 + f1(); + } + else + { // B6 + f1(); + } + // B7 + f1(); + } + // B4 + f1(); +} + +void f8() +{ // B1 + f1(); + if (true) + { // B2 + f1(); + for (int i = 0; i < 5; i++) // B3 + { // B4 + f1(); + } + // B5 + f1(); + } + else + { // unreachable + f1(); + } + // B6 + f1(); +} + +void f9() +{ // B1 + f1(); + while (true) // B2 + { // B3 + f1(); + if (true) + { // B4 + f1(); + break; + } + // B5 + f1(); + } + // Unreachable, no block id + f1(); +} \ No newline at end of file diff --git a/src/qtil/cfg/BlockId.qll b/src/qtil/cfg/BlockId.qll new file mode 100644 index 0000000..1887eb1 --- /dev/null +++ b/src/qtil/cfg/BlockId.qll @@ -0,0 +1,391 @@ +/** + * A module that orders and assigns a unique ID to each block in a control flow graph, without + * requiring location information, and ignoring cycles. + * + * In CodeQL we cannot impose an arbitrary order on objects, but only order them by integer or + * string properties. We cannot order blocks by location information, because blocks inside macros + * do not have unique locations. We also cannot perform a typical recursive algorithm with a set of + * visited nodes in the graph, both because CodeQL does not have a `Set` type, and also because + * checking `not f(x)` in predicate `f(Block b)` introduces non-monotonic recursion. + * + * Instead we perform a multi step process to build a cycle-free binary tree over + * `getTrueSuccessor()`/`getFalseSuccessor()`, via a new edge type `BlockEdge`, such that every node + * in the tree has exactly one parent, which we can use to trivially assign a unique ID to each + * block via a depth-first traversal of the control flow graph edges. + * + * The overall process to assign a unique ID to each block is as follows: + * - First, we count the number of children from each root block, giving us an upper limit of + * recursive steps that may be needed in later steps, avoiding infinite recursion. + * - Second, we compute the depth from which each block can be reached from its root, up to the + * maximum possible depth of the graph, the node count. This step may produce more than one result + * for each block involved in a cycle. However, it is monotonic, because adding a new depth for a + * block does not invalidate any previous depths. + * - Third, we take the minimum of the depths computed at step two for each block. + * - Fourth, for each node we find the "primary" and "secondary" deeper successors, which are the + * true and false successors that have a higher minimum depth than the current block, if they + * exist. The true successor is considered the primary and the false successor is considered the + * secondary, assuming both exist. + * - Fourth, we define a new edge type `BlockEdge` that represents the edges in the tree formed by + * the primary and secondary deeper successors. Each edge points from a parent edge to a child + * block. This way, the data structure encodes the full path of how a block was reached, which is + * unique, and means each edge has exactly one parent, making our binary tree more or less a well + * formed and well behaved tree. + * - Fifth, we can perform a simple depth-first traversal of the tree formed by `BlockEdge` to + * assign a unique ID to each edge. Our goal is that primary successors are assigned their parent + * ID + 1, and secondary successors are assigned the highest ID from the primary edge's subtree, + * plus one. We do not do this as a recursive algorithm with n steps, because in CodeQL that will + * introduce a join for each of the n steps, leading to O(n^2) performance. Instead, we separately + * count the number of edges in a subtree in log(n) steps, and then use that to infer what the + * maximum child Id of a primary edge will be. With this we can assign an ID to each node in log n + * steps since we recurse without backtracking. + * - Finally, we can use `[rank](x)` to assign a unique ID to each block without gaps, in a + * breadth-first ordering, by ordering the blocks by their minimum temporary ID and their minimum + * depth. Their rank in that ordering is their final unique ID. + * + * If this process seems unnecessarily complex, consider an alternative approach that gets stuck in + * infinite recursion, to perhaps justify this design. + * + * Assume we follow the fifth step above, but instead of using a `BlockEdge` type to represent the + * edges in the tree, we simply use the predicates `getPrimarySuccessor()` and + * `getSecondarySuccessor()` to recursively traverse the tree. Step five states that the id of the + * primary successor is the parent ID + 1, and the id of the secondary successor is the highest ID + * under the primary successor + 1. However, a block can have multiple parents. Therefore, there may + * be a block reached via (primary, primary) and also via (secondary, primary). Under this process, + * the block reached via (primary, secondary) should have an ID that is greater than the IDs of all + * of the blocks reachable from (primary, ...). This is impossible, as the block's ID therefore + * would need to be greater than itself. In CodeQL this manifests as giving this block an infinite + * number of possible IDs, and therefore the query does not terminate. + * + * To avoid this, we fundamentally need to know how we got to assign the ID to a block, which is + * what the `BlockEdge` type provides. + */ + +import qtil.parameterization.Finalize + +/** + * Signature to add support for the `BlockId` module for a new language. + * + * This module is not necessarily intended to be used directly, check for a language-specific + * instantiation of this module in the language-specific qtil pack. + */ +signature module BlockIdConfigSig { + /** + * A block specific to the given language's CFG. + */ + class BasicBlock { + /** + * A block that is reached immediately after this block when a given conditional value returns + * a true value. + * + * This predicate should only ever return a single result. + */ + BasicBlock getTrueSuccessor(); + + /** + * A block that is reached immediately after this block when a given conditional value returns + * a false value. + * + * This predicate should only ever return a single result. + */ + BasicBlock getFalseSuccessor(); + + /** + * Get a block that conditionally follows this block -- perhaps unconditionally. + * + * This predicate is allowed to return: + * - The true successor and/or the false successor. These are not treated as unconditional. + * - An unconditional successor IF no true or false successor exists. + * - Multiple conditional non-true, non-false successors (for example, switch/case) is not + * fully supported, and will result in duplicated block IDs. + */ + BasicBlock getASuccessor(); + + /** + * Get the nth control flow node in this block. + */ + ControlFlowNode getNode(int index); + + string toString(); + } + + /** + * A basic block representing an entry point into a control flow graph. + */ + class EntryBasicBlock extends BasicBlock; + + /** + * A control flow node, which is a part of a basic block. + */ + class ControlFlowNode { + string toString(); + + /** + * Get the basic block that controls this node. + */ + BasicBlock getBasicBlock(); + } +} + +/** + * A module that orders and assigns a unique ID to each block in a control flow graph, without + * requiring location information, and ignoring cycles via predicates `blockId(b)`, and + * `controlFlowNodeId(n)`. + * + * This module is not necessarily intended to be used directly, check for a language-specific + * instantiation of this module in the language-specific qtil pack. + * + * Supporting assignment of a blockID without location information is important for certain + * languages and use cases -- for instance, C++ macro expansions do not have location info. + */ +module BlockId { + import Config + + /** + * A contiguous unique ID for each basic block in the control flow graph, assigned in a + * breadth-first manner. + */ + cached + int blockId(BasicBlock b) { + exists(RootBlock root | + b = root.getAChild() and + b = + rank[result](BasicBlock test | + test = root.getAChild() + | + test order by getMinBlockDepth(root, test), depthFirstBlockId(test) + ) + ) + } + + /** + * A contiguous unique ID for each control flow node in the control flow graph, assigned in a + * breadth-first manner. + */ + cached + int controlFlowNodeId(ControlFlowNode n) { + exists(RootBlock root | + n.getBasicBlock() = root.getAChild() and + n = + rank[result](ControlFlowNode test, BasicBlock bb, int idx | + bb = root.getAChild() and + bb = test.getBasicBlock() and + test = bb.getNode(idx) + | + test order by blockId(bb), idx + ) + ) + } + + /** + * An extension of `EntryBasicBlock` that we use to find all reachable children. + */ + private class RootBlock extends Final::Type { + BasicBlock getAChild() { + result = this + or + exists(BasicBlock pred | pred = getAChild() and result = pred.getASuccessor()) + } + + int getChildCount() { result = count(BasicBlock b | b = getAChild()) } + } + + /** + * Recursively assigns parent depth + 1 to each child block, starting from the root block at depth + * zero. + * + * In the presence of cycles, this will assign multiple depths to cyclic blocks, and would not + * terminate without an upper limit on the depth, for which we can use `getChildCount()` from the + * root block. + */ + private predicate getABlockDepth(RootBlock root, BasicBlock b, int depth) { + depth = 0 and b = root + or + // We must carefully consider loops in the CFG. This process would never complete without an + // upper limit in the presence of cycles. This predicate still can hold for multiple depths for + // a given block, but we can aggregate those in a second stage. + depth <= root.getChildCount() and + exists(BasicBlock parent | + getABlockDepth(root, parent, depth - 1) and + parent.getASuccessor() = b + ) + } + + /** + * Finds the minimum number of steps from the root block to the given basic block, properly handling + * cycles in the control flow graph. + * + * Note that this pairing of `getABlockDepth()` and `getMinBlockDepth()` is a CodeQL friendly way of + * getting this data without introducing non-monotonic recursion. + */ + private int getMinBlockDepth(RootBlock root, BasicBlock b) { + result = min(int depth | getABlockDepth(root, b, depth)) + } + + /** + * Returns the true successor of the given basic block if it has a higher minimum depth. + */ + private BasicBlock getDeeperTrueSuccessor(BasicBlock b) { + result = b.getTrueSuccessor() and + exists(RootBlock root | getMinBlockDepth(root, result) = getMinBlockDepth(root, b) + 1) + } + + /** + * Returns the false successor of the given basic block if it has a higher minimum depth. + */ + private BasicBlock getDeeperFalseSuccessor(BasicBlock b) { + result = b.getFalseSuccessor() and + exists(RootBlock root | getMinBlockDepth(root, result) = getMinBlockDepth(root, b) + 1) + } + + /** + * Some blocks do not have a true or false successor, but an unconditional successor. This function + * returns that successor if it has a higher minimum depth. + */ + private BasicBlock getDeeperUnconditionalSuccessor(BasicBlock b) { + // TODO: Add support for multiple ordered successors, to support, for instance, switch/case. + result = b.getASuccessor() and + not result = b.getTrueSuccessor() and + not result = b.getFalseSuccessor() and + //exists(RootBlock root | getMinBlockDepth(root, result) > getMinBlockDepth(root, b)) + exists(RootBlock root | getMinBlockDepth(root, result) = getMinBlockDepth(root, b) + 1) + } + + /** + * Returns the "first" deeper successor of the given basic block: this is the deeper true successor + * or deeper unconditional successor if either exists, or the deeper false successor, if it exists. + * A block may not have a deeper successor at all, in which case this function returns nothing. + */ + private BasicBlock getPrimarySuccessor(BasicBlock b) { + if exists(getDeeperUnconditionalSuccessor(b)) + then result = getDeeperUnconditionalSuccessor(b) + else + if exists(getDeeperTrueSuccessor(b)) + then result = getDeeperTrueSuccessor(b) + else result = getDeeperFalseSuccessor(b) + } + + /** + * Returns the "second" deeper successor of the given basic block. This can only be the deeper + * false successor, if it exists and is not the primary successor; otherwise returns nothing. + */ + private BasicBlock getSecondarySuccessor(BasicBlock b) { + exists(getDeeperTrueSuccessor(b)) and + result = getDeeperFalseSuccessor(b) + } + + /** + * A new type that represents edges in the tree formed by primary and secondary deeper successors. + * + * This type will produce a mostly well-formed binary tree, where each node has exactly one parent. + * To accomplish this, we say that the parent is a prior edge rather than a block. Blocks that have + * multiple incoming edges will therefore produce multiple `BlockEdge` instances, each uniquely + * identifiable with a single, different parent edge. + * + * Constructing this without infinite recursion requires that we use the `getPrimarySuccessor()` and + * `getSecondarySuccessor()` functions to find the child blocks, which is a cycle-free graph. + */ + private newtype TBlockEdge = + TStartEdge(RootBlock root) or + TPrimaryEdge(BlockEdge parentEdge, BasicBlock child) { + child = getPrimarySuccessor(parentEdge.getEnd()) + } or + TSecondaryEdge(BlockEdge parentEdge, BasicBlock child) { + child = getSecondarySuccessor(parentEdge.getEnd()) + } + + private class BlockEdge extends TBlockEdge { + /** + * Get the block this edge points to. + */ + BasicBlock getEnd() { + this = TStartEdge(result) + or + this = TPrimaryEdge(_, result) + or + this = TSecondaryEdge(_, result) + } + + /** + * Get the parent edge. + * + * Edges don't point from a block to a block, but rather from an edge to a block, to create a more + * well-formed tree structure. + */ + BlockEdge getStart() { + this = TPrimaryEdge(result, _) or + this = TSecondaryEdge(result, _) + } + + /** + * Whether the edge represents a primary or secondary successor. + */ + predicate isPrimaryEdge() { this = TPrimaryEdge(_, _) } + + /** + * Whether the edge represents a primary or secondary successor. + */ + predicate isSecondaryEdge() { this = TSecondaryEdge(_, _) } + + /** + * Gets the child edge in the graph which is a primary edge. + */ + BlockEdge getPrimaryEdge() { result = TPrimaryEdge(this, _) } + + /** + * Gets the child edge in the graph which is a secondary edge. + */ + BlockEdge getSecondaryEdge() { result = TSecondaryEdge(this, _) } + + /** + * Counts the number of edges in this graph rooted at this edge, including this edge. + */ + int countEdges() { + result = getPrimaryEdge().countEdges() + getSecondaryEdge().countEdges() + 1 + or + not exists(getSecondaryEdge()) and result = getPrimaryEdge().countEdges() + 1 + or + not exists(getPrimaryEdge()) and result = 1 + } + + /** + * Computes the ID of this edge in a depth-first traversal of the tree formed by `BlockEdge` + * instances. + * + * The ID is of a primary edge is equal to its parent's ID + 1. The ID of a secondary edge should + * equal the highest ID of any edge in the primary edge's subtree + 1. + * + * The ID assignment process could recurse down the primary nodes, and backtrack to recurse on the + * secondary nodes, continually incrementing a counter as it goes. However, this performs poorly + * in CodeQL, as it takes `n` steps, and for each `n` we join the previous delta against the `n` + * nodes that may require an update, leading to O(n^2) performance. + * + * Instead, we first count the number of edges in each subtree, which takes log(n) steps. Then, we + * can compute what the maximum ID in the primary subtree is, without recursion, as the parent ID + * + the number of edges in the primary subtree. Then we can recurse down the tree as before, but + * without needing to backtrack, completing in O(n log n) steps. + */ + int getEdgeId() { + this = TStartEdge(_) and result = 0 + or + ( + isPrimaryEdge() and result = getStart().getEdgeId() + 1 + or + isSecondaryEdge() and + result = getStart().getEdgeId() + getStart().getPrimaryEdge().countEdges() + 1 + ) + } + + string toString() { result = "Edge" } + } + + /** + * Gets the depth-first ID of the given basic block, based on the depth-first traversal of the tree + * formed by `BlockEdge` instances. + * + * Note that this ID is sparse, as multiple edges can point to the same block. We only take the + * minimum of these IDs, which leaves gaps in the numbering. + */ + private int depthFirstBlockId(BasicBlock b) { + result = min(BlockEdge edge | edge.getEnd() = b | edge.getEdgeId()) + } +} diff --git a/test/qtil/cfg/BlockIdTest.expected b/test/qtil/cfg/BlockIdTest.expected new file mode 100644 index 0000000..5ce1bf8 --- /dev/null +++ b/test/qtil/cfg/BlockIdTest.expected @@ -0,0 +1 @@ +| All 11 tests passed. | diff --git a/test/qtil/cfg/BlockIdTest.ql b/test/qtil/cfg/BlockIdTest.ql new file mode 100644 index 0000000..8b165df --- /dev/null +++ b/test/qtil/cfg/BlockIdTest.ql @@ -0,0 +1,339 @@ +import qtil.testing.Qnit +import qtil.cfg.BlockId + +/** + * Mock control flow nodes for testing BlockId functionality + */ +class TestNode extends string { + TestNode() { + this = + [ + "entry", "if", "if2", "then", "then2", "else", "else2", "merge", "loop", "exit", + "early_exit" + ] + } + + string toString() { result = this } + + TestBlock getBasicBlock() { + this = "entry" and result = "Entry" + or + this = "if" and result = "If" + or + this = "if2" and result = "If2" + or + this = "then" and result = "Then" + or + this = "then2" and result = "Then2" + or + this = "else" and result = "Else" + or + this = "else2" and result = "Else2" + or + this = "merge" and result = "Merge" + or + this = "loop" and result = "Loop" + or + this = "exit" and result = "Exit" + or + this = "early_exit" and result = "EarlyExit" + } +} + +/** + * Mock basic blocks for testing different control flow patterns + */ +class TestBlock extends string { + TestBlock() { + this = + ["Entry", "If", "Then", "Else", "If2", "Then2", "Else2", "Merge", "Loop", "Exit", "EarlyExit"] + } + + string toString() { result = this } + + // Test diverging control flow (if-then-else) + TestBlock getTrueSuccessor() { + this = "If" and result = "Then" + or + this = "If2" and result = "Then2" + or + this = "Loop" and result = "Merge" // Loop condition true -> continue + } + + TestBlock getFalseSuccessor() { + this = "If" and result = "Else" + or + this = "If2" and result = "Else2" + or + this = "Loop" and result = "Exit" // Loop condition false -> exit + } + + TestBlock getASuccessor() { + // Diverging paths + result = this.getTrueSuccessor() + or + result = this.getFalseSuccessor() + or + // Unconditional successors + this = "Entry" and result = "If" + or + this = "Then" and result = "If2" // Non-exiting if-branch + or + this = "Else" and result = "EarlyExit" // Converging control flow + or + this = "Then2" and result = "Merge" // Converging control flow + or + this = "Else2" and result = "Merge" // Converging control flow + or + this = "Merge" and result = "Loop" // Cycle creation + } + + TestNode getNode(int index) { + index = 0 and + ( + this = "Entry" and result = "entry" + or + this = "If" and result = "if" + or + this = "Then" and result = "then" + or + this = "Else" and result = "else" + or + this = "Merge" and result = "merge" + or + this = "Loop" and result = "loop" + or + this = "Exit" and result = "exit" + or + this = "EarlyExit" and result = "early_exit" + ) + } +} + +class TestEntryBlock extends TestBlock { + TestEntryBlock() { this = "Entry" } +} + +/** + * Configuration for testing BlockId with our mock control flow graph + */ +module TestBlockIdConfig implements BlockIdConfigSig { + class BasicBlock = TestBlock; + + class EntryBasicBlock = TestEntryBlock; + + class ControlFlowNode = TestNode; +} + +module TestBlockId = BlockId; + +/** + * Test that all blocks receive unique IDs + */ +class TestUniqueBlockIds extends Test, Case { + override predicate run(Qnit test) { + if + forall(TestBlock b1, TestBlock b2 | + b1 != b2 and exists(TestBlockId::blockId(b1)) and exists(TestBlockId::blockId(b2)) + | + TestBlockId::blockId(b1) != TestBlockId::blockId(b2) + ) + then test.pass("All blocks have unique IDs") + else test.fail("Some blocks have duplicate IDs") + } +} + +/** + * Test that all control flow nodes receive unique IDs + */ +class TestUniqueNodeIds extends Test, Case { + override predicate run(Qnit test) { + if + forall(TestNode n1, TestNode n2 | + n1 != n2 and + exists(TestBlockId::controlFlowNodeId(n1)) and + exists(TestBlockId::controlFlowNodeId(n2)) + | + TestBlockId::controlFlowNodeId(n1) != TestBlockId::controlFlowNodeId(n2) + ) + then test.pass("All nodes have unique IDs") + else test.fail("Some nodes have duplicate IDs") + } +} + +/** + * Test that block IDs are contiguous (no gaps) + */ +class TestContiguousBlockIds extends Test, Case { + override predicate run(Qnit test) { + exists(int maxId | + maxId = max(TestBlock b | exists(TestBlockId::blockId(b)) | TestBlockId::blockId(b)) and + forall(int i | i in [1 .. maxId] | exists(TestBlock b | TestBlockId::blockId(b) = i)) + ) and + test.pass("Block IDs are contiguous") + or + not exists(int maxId | + maxId = max(TestBlock b | exists(TestBlockId::blockId(b)) | TestBlockId::blockId(b)) and + forall(int i | i in [1 .. maxId] | exists(TestBlock b | TestBlockId::blockId(b) = i)) + ) and + test.fail("Block IDs have gaps") + } +} + +/** + * Test that node IDs are contiguous (no gaps) + */ +class TestContiguousNodeIds extends Test, Case { + override predicate run(Qnit test) { + exists(int maxId | + maxId = + max(TestNode n | + exists(TestBlockId::controlFlowNodeId(n)) + | + TestBlockId::controlFlowNodeId(n) + ) and + forall(int i | i in [1 .. maxId] | exists(TestNode n | TestBlockId::controlFlowNodeId(n) = i)) + ) and + test.pass("Node IDs are contiguous") + or + not exists(int maxId | + maxId = + max(TestNode n | + exists(TestBlockId::controlFlowNodeId(n)) + | + TestBlockId::controlFlowNodeId(n) + ) and + forall(int i | i in [1 .. maxId] | exists(TestNode n | TestBlockId::controlFlowNodeId(n) = i)) + ) and + test.fail("Node IDs have gaps") + } +} + +/** + * Test that entry block has the lowest ID + */ +class TestEntryBlockFirstId extends Test, Case { + override predicate run(Qnit test) { + if + exists(TestEntryBlock entry | + TestBlockId::blockId(entry) = 1 or + TestBlockId::blockId(entry) = + min(TestBlock b | exists(TestBlockId::blockId(b)) | TestBlockId::blockId(b)) + ) + then test.pass("Entry block has the first ID") + else test.fail("Entry block does not have the first ID") + } +} + +/** + * Test convergence: blocks that converge should have higher IDs than their predecessors + */ +class TestConvergenceOrdering extends Test, Case { + override predicate run(Qnit test) { + if + forall(TestBlock pred, TestBlock succ | + succ = [pred.getFalseSuccessor(), succ.getTrueSuccessor(), succ.getASuccessor()] + | + TestBlockId::blockId(pred) < TestBlockId::blockId(succ) + ) + then test.pass("Convergence points have higher IDs than predecessors") + else test.fail("Some convergence points have lower IDs than predecessors") + } +} + +/** + * Test that cyclic control flow is handled properly (no infinite recursion) + */ +class TestCyclicControlFlow extends Test, Case { + override predicate run(Qnit test) { + if + exists(TestBlock loop | + loop.toString() = "Loop" and + exists(TestBlockId::blockId(loop)) + ) and + exists(TestBlock merge | + merge.toString() = "Merge" and + exists(TestBlockId::blockId(merge)) + ) + then test.pass("Cyclic control flow is handled without infinite recursion") + else test.fail("Cyclic control flow causes issues") + } +} + +/** + * Test that early exit paths are properly handled + */ +class TestEarlyExitPaths extends Test, Case { + override predicate run(Qnit test) { + if + exists(TestBlock earlyExit | + earlyExit.toString() = "EarlyExit" and + exists(TestBlockId::blockId(earlyExit)) + ) + then test.pass("Early exit paths are assigned IDs") + else test.fail("Early exit paths are not properly handled") + } +} + +/** + * Test diverging control flow: conditional successors should have different IDs + */ +class TestDivergingControlFlow extends Test, Case { + override predicate run(Qnit test) { + if + exists(TestBlock ifBlock, TestBlock thenBlock, TestBlock elseBlock | + ifBlock.toString() = "If" and + thenBlock = ifBlock.getTrueSuccessor() and + elseBlock = ifBlock.getFalseSuccessor() and + exists(TestBlockId::blockId(thenBlock)) and + exists(TestBlockId::blockId(elseBlock)) and + TestBlockId::blockId(thenBlock) != TestBlockId::blockId(elseBlock) + ) + then test.pass("Diverging control flow creates different branch IDs") + else test.fail("Diverging control flow does not create different branch IDs") + } +} + +/** + * Test that all reachable blocks from entry are assigned IDs + */ +class TestAllReachableBlocksHaveIds extends Test, Case { + override predicate run(Qnit test) { + if + forall(TestBlock b | + // Define reachability transitively from entry + exists(TestEntryBlock entry | + b = entry + or + exists(TestBlock pred | + (pred = entry or pred.getASuccessor*() = b) and + pred.getASuccessor() = b + ) + ) + | + exists(TestBlockId::blockId(b)) + ) + then test.pass("All reachable blocks have IDs") + else test.fail("Some reachable blocks do not have IDs") + } +} + +/** + * Test that nodes within blocks maintain relative ordering + */ +class TestNodeOrderingWithinBlocks extends Test, Case { + override predicate run(Qnit test) { + if + forall(TestNode n1, TestNode n2 | + n1.getBasicBlock() = n2.getBasicBlock() and + exists(TestBlockId::controlFlowNodeId(n1)) and + exists(TestBlockId::controlFlowNodeId(n2)) and + n1 != n2 + | + // Since we only have one node per block in our test, this should be trivially true + TestBlockId::controlFlowNodeId(n1) != TestBlockId::controlFlowNodeId(n2) + ) + then test.pass("Nodes within blocks maintain proper ordering") + else test.fail("Node ordering within blocks is incorrect") + } +} From 9264fcbd2301dec595650eb8835a86a6e8f507d5 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Fri, 12 Sep 2025 20:15:44 -0700 Subject: [PATCH 2/2] Fix BlockIdTest pred/succ, and other test improvements. --- test/qtil/cfg/BlockIdTest.ql | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/test/qtil/cfg/BlockIdTest.ql b/test/qtil/cfg/BlockIdTest.ql index 8b165df..dec9529 100644 --- a/test/qtil/cfg/BlockIdTest.ql +++ b/test/qtil/cfg/BlockIdTest.ql @@ -226,18 +226,19 @@ class TestEntryBlockFirstId extends Test, Case { } /** - * Test convergence: blocks that converge should have higher IDs than their predecessors + * Test ordering: blocks that don't loop should have higher IDs than their predecessors */ -class TestConvergenceOrdering extends Test, Case { +class TestBlockIdOrdering extends Test, Case { override predicate run(Qnit test) { if forall(TestBlock pred, TestBlock succ | - succ = [pred.getFalseSuccessor(), succ.getTrueSuccessor(), succ.getASuccessor()] + succ = [pred.getFalseSuccessor(), pred.getTrueSuccessor(), pred.getASuccessor()] and + not succ.getASuccessor+() = pred | TestBlockId::blockId(pred) < TestBlockId::blockId(succ) ) - then test.pass("Convergence points have higher IDs than predecessors") - else test.fail("Some convergence points have lower IDs than predecessors") + then test.pass("Non-looping blocks have higher IDs than predecessors") + else test.fail("Some non-looping blocks have lower IDs than predecessors") } } @@ -249,11 +250,9 @@ class TestCyclicControlFlow extends Test, Case { if exists(TestBlock loop | loop.toString() = "Loop" and - exists(TestBlockId::blockId(loop)) - ) and - exists(TestBlock merge | - merge.toString() = "Merge" and - exists(TestBlockId::blockId(merge)) + loop.getASuccessor+() = loop and + exists(TestBlockId::blockId(loop)) and + forall(TestBlock succ | succ = loop.getASuccessor() | exists(TestBlockId::blockId(succ))) ) then test.pass("Cyclic control flow is handled without infinite recursion") else test.fail("Cyclic control flow causes issues")