From 9697d76c2dc973ef475ea4174cf0c6f2c3de2fac Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 19 Dec 2023 21:33:13 +0000 Subject: [PATCH] Stratify `CFG::succ` to avoid recursion The first level doesn't deal with defer statements properly. The second level usees the first level to deal with them properly. --- .../go/controlflow/ControlFlowGraphImpl.qll | 102 ++++++++++-------- 1 file changed, 57 insertions(+), 45 deletions(-) diff --git a/go/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll b/go/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll index a1d14003031..d9dc296fcf5 100644 --- a/go/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll +++ b/go/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll @@ -623,13 +623,15 @@ module CFG { not cmpl.isNormal() } - predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { exists(int i | lastNode(this.getChildTreeRanked(i), pred, normalCompletion()) and firstNode(this.getChildTreeRanked(i + 1), succ) ) } + predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { this.succSimple(pred, succ) } + final ControlFlowTree getChildTreeRanked(int i) { exists(int j | result = this.getChildTree(j) and @@ -727,8 +729,8 @@ module CFG { last = this.getNode() and cmpl = this.getCompletion() } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { - super.succ(pred, succ) + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { + super.succSimple(pred, succ) or lastNode(this.getLastChildTree(), pred, normalCompletion()) and succ = this.getNode() @@ -750,8 +752,8 @@ module CFG { cmpl = Done() } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { - super.succ(pred, succ) + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { + super.succSimple(pred, succ) or pred = this.getNode() and firstNode(this.getFirstChildTree(), succ) @@ -853,8 +855,8 @@ module CFG { cmpl = Done() } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { - ControlFlowTree.super.succ(pred, succ) + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { + ControlFlowTree.super.succSimple(pred, succ) or exists(int i | lastNode(this.getLhs(i), pred, normalCompletion()) | firstNode(this.getLhs(i + 1), succ) @@ -978,7 +980,7 @@ module CFG { ) } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { exists(Completion lcmpl | lastNode(this.getLeftOperand(), pred, lcmpl) and succ = this.getGuard(lcmpl.getOutcome()) @@ -1028,11 +1030,11 @@ module CFG { not result instanceof TypeExpr } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { // interpose implicit argument destructuring nodes between last argument // and call itself; this is for cases like `f(g())` where `g` has multiple // results - exists(ControlFlow::Node mid | PostOrderTree.super.succ(pred, mid) | + exists(ControlFlow::Node mid | PostOrderTree.super.succSimple(pred, mid) | if mid = this.getNode() then succ = this.getEpilogueNode(0) else succ = mid ) or @@ -1102,8 +1104,8 @@ module CFG { lastNode(this.getStmt(this.getNumStmt() - 1), last, cmpl) } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { - ControlFlowTree.super.succ(pred, succ) + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { + ControlFlowTree.super.succSimple(pred, succ) or exists(int i | lastNode(this.getExpr(i), pred, normalCompletion()) and @@ -1172,7 +1174,7 @@ module CFG { cmpl = Done() } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { this.firstNode(pred) and succ = this.getElementStart(0) or @@ -1250,7 +1252,7 @@ module CFG { cmpl = Done() } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { lastNode(this.getInit(), pred, normalCompletion()) and firstNode(this.getCond(), succ) or @@ -1281,7 +1283,7 @@ module CFG { (cmpl = Done() or cmpl = Panic()) } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { lastNode(this.getBase(), pred, normalCompletion()) and ( succ = MkImplicitDeref(this.getBase()) @@ -1318,8 +1320,8 @@ module CFG { override predicate lastNode(ControlFlow::Node last, Completion cmpl) { none() } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { - ControlFlowTree.super.succ(pred, succ) + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { + ControlFlowTree.super.succSimple(pred, succ) or pred = MkEntryNode(this) and firstNode(this.getDecl(0), succ) @@ -1374,7 +1376,7 @@ module CFG { i = 5 and result = this.getBody() } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { exists(int i, ControlFlowTree predTree, Completion cmpl | predTree = this.getChildTreeRanked(i) and lastNode(predTree, pred, cmpl) and @@ -1440,13 +1442,13 @@ module CFG { // `defer` can be the first `defer` statement executed // there is always a predecessor node because the `defer`'s call is always // evaluated before the defer statement itself - MkDeferNode(defer) = succ(notDeferSucc*(this.getEntry())) + MkDeferNode(defer) = succSimple(notDeferSuccSimple*(this.getEntry())) ) } override predicate lastNode(ControlFlow::Node last, Completion cmpl) { none() } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { exists(int i | pred = this.getPrologueNode(i) and succ = this.getPrologueNode(i + 1) @@ -1460,10 +1462,19 @@ module CFG { firstNode(ls, succ) ) or + exists(int i | + pred = this.getEpilogueNode(i) and + succ = this.getEpilogueNode(i + 1) + ) + } + + override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + this.succSimple(pred, succ) + or exists(Completion cmpl | lastNode(this.getBody(), pred, cmpl) and // last node of function body can be reached without going through a `defer` statement - pred = notDeferSucc*(this.getEntry()) + pred = notDeferSuccSimple*(this.getEntry()) | // panic goes directly to exit, non-panic reads result variables first if cmpl = Panic() then succ = MkExitNode(this) else succ = this.getEpilogueNode(0) @@ -1473,7 +1484,7 @@ module CFG { exists(DeferStmt defer | defer = this.getADeferStmt() | succ = MkExprNode(defer.getCall()) and // the last `DeferStmt` executed before pred is this `defer` - pred = notDeferSucc*(MkDeferNode(defer)) + pred = notDeferSuccSimple*(MkDeferNode(defer)) ) or exists(DeferStmt predDefer, DeferStmt succDefer | @@ -1494,11 +1505,6 @@ module CFG { or succ = this.getEpilogueNode(0) ) - or - exists(int i | - pred = this.getEpilogueNode(i) and - succ = this.getEpilogueNode(i + 1) - ) } } @@ -1516,7 +1522,7 @@ module CFG { cmpl = Done() } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { lastNode(this.getOperand(), pred, normalCompletion()) and succ = MkImplicitOne(this) or @@ -1547,7 +1553,7 @@ module CFG { not cmpl.isNormal() } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { lastNode(this.getDomain(), pred, normalCompletion()) and succ = MkNextNode(this) or @@ -1622,7 +1628,7 @@ module CFG { override Completion getCompletion() { result = Return() } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { exists(int i | lastNode(this.getExpr(i), pred, normalCompletion()) and succ = this.complete(i) @@ -1678,8 +1684,8 @@ module CFG { ) } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { - ControlFlowTree.super.succ(pred, succ) + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { + ControlFlowTree.super.succSimple(pred, succ) or exists(CommClause cc, int i, Stmt comm | cc = this.getNonDefaultCommClause(i) and @@ -1777,7 +1783,7 @@ module CFG { cmpl = Done() } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { exists(int i | pred = this.getStepWithRank(i) and succ = this.getStepWithRank(i + 1)) } @@ -1814,8 +1820,8 @@ module CFG { (cmpl = Done() or cmpl = Panic()) } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { - ControlFlowTree.super.succ(pred, succ) + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { + ControlFlowTree.super.succSimple(pred, succ) or not this = any(CommClause cc).getComm() and lastNode(this.getValue(), pred, normalCompletion()) and @@ -1843,8 +1849,8 @@ module CFG { (cmpl = Done() or cmpl = Panic()) } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { - ControlFlowTree.super.succ(pred, succ) + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { + ControlFlowTree.super.succSimple(pred, succ) or lastNode(this.getBase(), pred, normalCompletion()) and ( @@ -1930,8 +1936,8 @@ module CFG { ) } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { - ControlFlowTree.super.succ(pred, succ) + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { + ControlFlowTree.super.succSimple(pred, succ) or lastNode(this.getInit(), pred, normalCompletion()) and ( @@ -2004,8 +2010,8 @@ module CFG { ) } - override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { - ControlFlowTree.super.succ(pred, succ) + override predicate succSimple(ControlFlow::Node pred, ControlFlow::Node succ) { + ControlFlowTree.super.succSimple(pred, succ) or not this = any(RecvStmt recv).getExpr() and lastNode(this.getOperand(), pred, normalCompletion()) and @@ -2033,9 +2039,9 @@ module CFG { } /** Gets a successor of `nd` that is not a `defer` node */ - private ControlFlow::Node notDeferSucc(ControlFlow::Node nd) { + private ControlFlow::Node notDeferSuccSimple(ControlFlow::Node nd) { not result = MkDeferNode(_) and - result = succ(nd) + result = succSimple(nd) } /** Gets `defer` statements that can be the first defer statement after `nd` in the CFG */ @@ -2043,9 +2049,9 @@ module CFG { nd = MkDeferNode(_) and result = MkDeferNode(_) and ( - result = succ(nd) + result = succSimple(nd) or - result = succ(notDeferSucc+(nd)) + result = succSimple(notDeferSuccSimple+(nd)) ) } @@ -2074,6 +2080,12 @@ module CFG { ) } + /** Gets a successor of `nd`, that is, a node that is executed after `nd`. */ + cached + ControlFlow::Node succSimple(ControlFlow::Node nd) { + any(ControlFlowTree tree).succSimple(nd, result) + } + /** Gets a successor of `nd`, that is, a node that is executed after `nd`. */ cached ControlFlow::Node succ(ControlFlow::Node nd) { any(ControlFlowTree tree).succ(nd, result) }