The shared CFG creates multiple ControlFlowNodes per AST node in
conditional contexts (e.g. afterTrue/afterFalse for boolean conditions,
empty/non-empty for for-loops, matched/unmatched for match cases).
These splits matter for control-flow analysis, but for dataflow — where
we ask 'what is the value of this expression?' — we need exactly one
representative per AST or we double-count calls, arguments, and store
steps.
This adds Cfg::isCanonicalAstNodeRepresentative as a purely structural
pick: for split ASTs it selects the 'positive' outcome variant; for
non-split ASTs it selects the unique variant. The picker is implemented
via genuine-outcome helpers that work around the shared CFG's
cross-kind isAfterValue fallback (ControlFlowGraph.qll:870-892), see
the doc on isGenuineAfterTrue for details.
The TCfgNode-family newtypes in DataFlowPublic, TNormalCall and
TPotentialLibraryCall in DataFlowDispatch, and the SSA-projected
use-use/def-use steps in DataFlowPrivate are all routed through the
canonical filter. DataFlowConsistency and the test UnresolvedCalls
helper qualify their CallNode casts with Cfg:: to keep working.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes the test failures that arose from making `ExtractedArgumentNode`
local.
For the consistency checks, we now explicitly exclude the
`ExtractedArgumentNode`s (now much more plentiful due to the
overapproximation) that don't have a corresponding `getCallArg` tuple.
For various queries/tests using `instanceof ArgumentNode`, we instead us
`isArgumentNode`, which explicitly filters out the ones for which
`isArgumentOf` doesn't hold (which, again, is the case for most of the
nodes in the overapproximation).
When looking things over a bit more, we could actually exclude the steps
that would never be used instead. A much more involved solution, but
more performance oriented and clear in terms of what is supported (at
least until we start supporting type-tracking with more than depth 1
access-path, if that ever happens)
This would have found the problem in
https://github.com/github/codeql/pull/15755.
As highlighted in the comment in the code, it's not a perfect solution
since we don't have an automatic way to ensure we don't introduce a new
PhaseDependentFlow use with a new step relation and forget to add it to
this consistency check... but I think this consistency check still adds
value!
These are the labmda self references. This is similar to
how `BlockParameterArgumentNode` is excluded for Ruby.
It is important that we restrict `call` in this logic.
Otherwise, we get a cartesian product and the consistency
check runs for a very long time...
