diff --git a/go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll b/go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll index 488a8f9c0e0..0ec3579d28d 100644 --- a/go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll +++ b/go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll @@ -212,13 +212,19 @@ module GoCfg { } class ForeachStmt extends LoopStmt { - ForeachStmt() { none() } + ForeachStmt() { this instanceof Go::RangeStmt } + // Go's `range` statement binds its key and value by destructuring the + // current element rather than by evaluating ordinary target expressions, + // so it opts in to the synthesized element node (see + // `foreachHasElementNode`) and does not use the shared variable routing. Expr getVariable() { none() } - Expr getCollection() { none() } + Expr getCollection() { result = this.(Go::RangeStmt).getDomain() } } + predicate foreachHasElementNode(ForeachStmt foreach) { any() } + class BreakStmt = Go::BreakStmt; class ContinueStmt = Go::ContinueStmt; @@ -566,9 +572,6 @@ module GoCfg { tag = "result-zero-init:" + i.toString() ) or - // Next node for range statements - n instanceof Go::RangeStmt and tag = "next" - or // Send node n instanceof Go::SendStmt and not n = any(Go::CommClause cc).getComm() and @@ -965,7 +968,7 @@ module GoCfg { predicate overridesCallableBodyExit(Ast::Callable c) { funcHasDefer(c.(Go::FuncDef)) } predicate step(PreControlFlowNode n1, PreControlFlowNode n2) { - rangeLoop(n1, n2) or + rangeStmtStep(n1, n2) or selectStmt(n1, n2) or deferStmt(n1, n2) or goStmtStep(n1, n2) or @@ -1424,17 +1427,15 @@ module GoCfg { ) } - private predicate rangeLoop(PreControlFlowNode n1, PreControlFlowNode n2) { + private predicate rangeStmtStep(PreControlFlowNode n1, PreControlFlowNode n2) { exists(Go::RangeStmt s | - // Use the shared library's auto-created `[LoopHeader]` additional node - // (created for every `LoopStmt`) as the join/branch point of the range loop. - n1.isBefore(s) and n2.isBefore(s.getDomain()) - or - n1.isAfter(s.getDomain()) and n2.isAdditional(s, "[LoopHeader]") - or - n1.isAdditional(s, "[LoopHeader]") and n2.isAdditional(s, "next") - or - n1.isAdditional(s, "next") and + // The shared `ForeachStmt` model owns the loop skeleton (testing the + // domain for emptiness, the `[LoopHeader]` join/branch point, and the + // loop exit) and routes control flow into the synthesized + // `[ForeachElement]` node (see `foreachHasElementNode`). From there we + // destructure the current element into the key/value variables using + // the shared extract/assign epilogue machinery, then enter the body. + n1.isAdditional(s, "[ForeachElement]") and ( exists(getFirstEpilogueTag(s)) and n2.isAdditional(s, getFirstEpilogueTag(s)) or @@ -1448,10 +1449,6 @@ module GoCfg { ) or n1.isAdditional(s, getLastEpilogueTag(s)) and n2.isBefore(s.getBody()) - or - n1.isAfter(s.getBody()) and n2.isAdditional(s, "[LoopHeader]") - or - n1.isAdditional(s, "[LoopHeader]") and n2.isAfter(s) ) } diff --git a/go/ql/lib/semmle/go/controlflow/IR.qll b/go/ql/lib/semmle/go/controlflow/IR.qll index dc4048b46bf..f1dda341d63 100644 --- a/go/ql/lib/semmle/go/controlflow/IR.qll +++ b/go/ql/lib/semmle/go/controlflow/IR.qll @@ -790,7 +790,7 @@ module IR { result = evalExprInstruction(baseExpr) ) or - result.(GetNextEntryInstruction).isAdditional(s, "next") + result.(GetNextEntryInstruction).isAdditional(s, "[ForeachElement]") or result = evalExprInstruction(s.(ReturnStmt).getExpr()) or @@ -1148,7 +1148,7 @@ module IR { class GetNextEntryInstruction extends Instruction { RangeStmt rs; - GetNextEntryInstruction() { this.isAdditional(rs, "next") } + GetNextEntryInstruction() { this.isAdditional(rs, "[ForeachElement]") } /** * Gets the instruction computing the value whose key-value pairs this instruction reads. diff --git a/go/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected b/go/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected index 0edb9a5426a..f82c68778ae 100644 --- a/go/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected +++ b/go/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected @@ -3981,15 +3981,17 @@ | stmts.go:145:23:159:1 | block statement | stmts.go:145:23:159:1 | arg:0 block statement | | stmts.go:145:23:159:1 | param-init:0 block statement | stmts.go:146:2:151:2 | range statement | | stmts.go:146:2:151:2 | After range statement | stmts.go:153:2:155:2 | range statement | +| stmts.go:146:2:151:2 | [ForeachElement] range statement | stmts.go:146:2:151:2 | extract:0 range statement | | stmts.go:146:2:151:2 | [LoopHeader] range statement | stmts.go:146:2:151:2 | After range statement | -| stmts.go:146:2:151:2 | [LoopHeader] range statement | stmts.go:146:2:151:2 | next range statement | +| stmts.go:146:2:151:2 | [LoopHeader] range statement | stmts.go:146:2:151:2 | [ForeachElement] range statement | | stmts.go:146:2:151:2 | assign:0 range statement | stmts.go:146:20:151:2 | block statement | | stmts.go:146:2:151:2 | extract:0 range statement | stmts.go:146:2:151:2 | assign:0 range statement | -| stmts.go:146:2:151:2 | next range statement | stmts.go:146:2:151:2 | extract:0 range statement | | stmts.go:146:2:151:2 | range statement | stmts.go:146:17:146:18 | Before xs | -| stmts.go:146:17:146:18 | After xs | stmts.go:146:2:151:2 | [LoopHeader] range statement | +| stmts.go:146:17:146:18 | After xs [empty] | stmts.go:146:2:151:2 | After range statement | +| stmts.go:146:17:146:18 | After xs [non-empty] | stmts.go:146:2:151:2 | [ForeachElement] range statement | | stmts.go:146:17:146:18 | Before xs | stmts.go:146:17:146:18 | xs | -| stmts.go:146:17:146:18 | xs | stmts.go:146:17:146:18 | After xs | +| stmts.go:146:17:146:18 | xs | stmts.go:146:17:146:18 | After xs [empty] | +| stmts.go:146:17:146:18 | xs | stmts.go:146:17:146:18 | After xs [non-empty] | | stmts.go:146:20:151:2 | After block statement | stmts.go:146:2:151:2 | [LoopHeader] range statement | | stmts.go:146:20:151:2 | block statement | stmts.go:147:3:149:3 | if statement | | stmts.go:147:3:149:3 | After if statement | stmts.go:150:3:150:14 | expression statement | @@ -4021,17 +4023,19 @@ | stmts.go:150:13:150:13 | Before x | stmts.go:150:13:150:13 | x | | stmts.go:150:13:150:13 | x | stmts.go:150:13:150:13 | After x | | stmts.go:153:2:155:2 | After range statement | stmts.go:157:2:158:2 | range statement | +| stmts.go:153:2:155:2 | [ForeachElement] range statement | stmts.go:153:2:155:2 | extract:0 range statement | | stmts.go:153:2:155:2 | [LoopHeader] range statement | stmts.go:153:2:155:2 | After range statement | -| stmts.go:153:2:155:2 | [LoopHeader] range statement | stmts.go:153:2:155:2 | next range statement | +| stmts.go:153:2:155:2 | [LoopHeader] range statement | stmts.go:153:2:155:2 | [ForeachElement] range statement | | stmts.go:153:2:155:2 | assign:0 range statement | stmts.go:153:2:155:2 | extract:1 range statement | | stmts.go:153:2:155:2 | assign:1 range statement | stmts.go:153:23:155:2 | block statement | | stmts.go:153:2:155:2 | extract:0 range statement | stmts.go:153:2:155:2 | assign:0 range statement | | stmts.go:153:2:155:2 | extract:1 range statement | stmts.go:153:2:155:2 | assign:1 range statement | -| stmts.go:153:2:155:2 | next range statement | stmts.go:153:2:155:2 | extract:0 range statement | | stmts.go:153:2:155:2 | range statement | stmts.go:153:20:153:21 | Before xs | -| stmts.go:153:20:153:21 | After xs | stmts.go:153:2:155:2 | [LoopHeader] range statement | +| stmts.go:153:20:153:21 | After xs [empty] | stmts.go:153:2:155:2 | After range statement | +| stmts.go:153:20:153:21 | After xs [non-empty] | stmts.go:153:2:155:2 | [ForeachElement] range statement | | stmts.go:153:20:153:21 | Before xs | stmts.go:153:20:153:21 | xs | -| stmts.go:153:20:153:21 | xs | stmts.go:153:20:153:21 | After xs | +| stmts.go:153:20:153:21 | xs | stmts.go:153:20:153:21 | After xs [empty] | +| stmts.go:153:20:153:21 | xs | stmts.go:153:20:153:21 | After xs [non-empty] | | stmts.go:153:23:155:2 | After block statement | stmts.go:153:2:155:2 | [LoopHeader] range statement | | stmts.go:153:23:155:2 | block statement | stmts.go:154:3:154:17 | expression statement | | stmts.go:154:3:154:11 | After selection of Print | stmts.go:154:13:154:13 | Before i | @@ -4050,13 +4054,15 @@ | stmts.go:154:16:154:16 | Before v | stmts.go:154:16:154:16 | v | | stmts.go:154:16:154:16 | v | stmts.go:154:16:154:16 | After v | | stmts.go:157:2:158:2 | After range statement | stmts.go:145:23:159:1 | After block statement | +| stmts.go:157:2:158:2 | [ForeachElement] range statement | stmts.go:157:15:158:2 | block statement | | stmts.go:157:2:158:2 | [LoopHeader] range statement | stmts.go:157:2:158:2 | After range statement | -| stmts.go:157:2:158:2 | [LoopHeader] range statement | stmts.go:157:2:158:2 | next range statement | -| stmts.go:157:2:158:2 | next range statement | stmts.go:157:15:158:2 | block statement | +| stmts.go:157:2:158:2 | [LoopHeader] range statement | stmts.go:157:2:158:2 | [ForeachElement] range statement | | stmts.go:157:2:158:2 | range statement | stmts.go:157:12:157:13 | Before xs | -| stmts.go:157:12:157:13 | After xs | stmts.go:157:2:158:2 | [LoopHeader] range statement | +| stmts.go:157:12:157:13 | After xs [empty] | stmts.go:157:2:158:2 | After range statement | +| stmts.go:157:12:157:13 | After xs [non-empty] | stmts.go:157:2:158:2 | [ForeachElement] range statement | | stmts.go:157:12:157:13 | Before xs | stmts.go:157:12:157:13 | xs | -| stmts.go:157:12:157:13 | xs | stmts.go:157:12:157:13 | After xs | +| stmts.go:157:12:157:13 | xs | stmts.go:157:12:157:13 | After xs [empty] | +| stmts.go:157:12:157:13 | xs | stmts.go:157:12:157:13 | After xs [non-empty] | | stmts.go:157:15:158:2 | block statement | stmts.go:157:2:158:2 | [LoopHeader] range statement | | tst.go:0:0:0:0 | After tst.go | tst.go:0:0:0:0 | Normal Exit | | tst.go:0:0:0:0 | Entry | tst.go:0:0:0:0 | tst.go |