Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 55 additions & 6 deletions shared/controlflow/codeql/controlflow/ControlFlowGraph.qll
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,39 @@ signature module AstSig<LocationSig Location> {

/** A for-loop that iterates over the elements of a collection. */
class ForeachStmt extends LoopStmt {
/** Gets the variable declaration of this `foreach` loop. */
/**
* Gets the variable declaration of this `foreach` loop, if any.
*
* A `foreach` loop that binds more than one variable per iteration (for
* example when destructuring is used, as in `for k, v := range m` in Go, or
* `for (a, b) in ...` in Rust/Python/Swift) should instead opt in to the
* synthesized per-iteration element node (see `foreachHasElementNode`) and
* destructure that element into its variables, in which case this predicate
* need not have a result.
*/
Expr getVariable();

/** Gets the collection expression of this `foreach` loop. */
Expr getCollection();
}

/**
* Holds if `foreach` has a synthesized per-iteration "element" node, that is,
* an additional control flow node representing the element produced by each
* iteration of the loop.
*
* When this holds, the shared library routes control flow from the loop
* header (and from the non-empty collection) into the element node, but the
* language is then responsible for wiring control flow out of the element
* node and on to the loop body (typically destructuring the element into the
* loop variables). In this mode the shared `getVariable` routing is not used.
*
* This is useful for languages whose loop variables are bound by extracting
* components from an implicit "current element" value (as opposed to being
* evaluated as ordinary target expressions).
*/
default predicate foreachHasElementNode(ForeachStmt foreach) { none() }

/**
* A `break` statement.
*
Expand Down Expand Up @@ -684,6 +710,8 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {

private string patternMatchTrueTag() { result = "[MatchTrue]" }

private string foreachElementTag() { result = "[ForeachElement]" }

/**
* Holds if an additional node tagged with `tag` should be created for
* `n`. Edges targeting such nodes are labeled with `t` and therefore `t`
Expand All @@ -703,6 +731,10 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
tag = loopHeaderTag() and
t instanceof DirectSuccessor
or
foreachHasElementNode(n) and
tag = foreachElementTag() and
t instanceof DirectSuccessor
Comment thread
owen-mc marked this conversation as resolved.
or
n instanceof PatternMatchExpr and
tag = patternMatchTrueTag() and
t.(BooleanSuccessor).getValue() = true
Expand Down Expand Up @@ -1640,7 +1672,21 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
n2.isAfter(loopstmt)
)
or
exists(ForeachStmt foreachstmt |
exists(ForeachStmt foreachstmt, PreControlFlowNode iterentry |
// `iterentry` is where each iteration begins, after the loop header
// (or after the collection is found to be non-empty): the element
// node if the language opts in, otherwise the loop variable, or the
// body directly if there is no variable.
foreachHasElementNode(foreachstmt) and
iterentry.isAdditional(foreachstmt, foreachElementTag())
or
not foreachHasElementNode(foreachstmt) and
(
iterentry.isBefore(foreachstmt.getVariable())
or
not exists(foreachstmt.getVariable()) and iterentry.isBefore(foreachstmt.getBody())
)
|
n1.isBefore(foreachstmt) and
n2.isBefore(foreachstmt.getCollection())
or
Expand All @@ -1654,8 +1700,14 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
or
n1.isAfterValue(foreachstmt.getCollection(),
any(EmptinessSuccessor t | t.getValue() = false)) and
n2.isBefore(foreachstmt.getVariable())
n2 = iterentry
or
n1.isAdditional(foreachstmt, loopHeaderTag()) and
n2 = iterentry
or
// After the loop variable, enter the body. When the language opts in
// to the element node, it is instead responsible for wiring the
// element node through to the body itself.
n1.isAfter(foreachstmt.getVariable()) and
n2.isBefore(foreachstmt.getBody())
or
Expand All @@ -1668,9 +1720,6 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
or
not exists(getLoopElse(foreachstmt)) and n2.isAfter(foreachstmt)
)
or
n1.isAdditional(foreachstmt, loopHeaderTag()) and
n2.isBefore(foreachstmt.getVariable())
)
or
exists(ForStmt forstmt, PreControlFlowNode condentry |
Expand Down
Loading