C#: Replace CFG with the shared implementation by aschackmull · Pull Request #21565 · github/codeql

aschackmull · 2026-03-24T12:46:59Z

This PR migrates the C# libraries to use the new shared CFG library.

Several queries are impacted, but overall I'd say that it's generally increased precision.

The first few commits are preliminary setup tweaks. Then "C#: Replace CFG." is the big commit that deletes the old CFG and puts in the new instantiation of the shared libraries. And the several following commits then deal primarily with qltest fallout.

The additions are unintentional, but the fault lies with the shared SignAnalysis code. The removals are due to compile-time constant initializers no longer having CFG nodes.

…unctions.

…for performance.

Copilot

Pull request overview

Migrates the C# CodeQL libraries from the legacy C#-specific CFG to the shared control-flow graph implementation, updating affected queries and tests to the new node/basic-block APIs and semantics.

Changes:

Extend the shared CFG AST signature/support (e.g., goto, throw-as-expression, compound assignments, pattern matching) and align Java’s CFG wrapper to it.
Update C# libraries/queries to use shared ControlFlowNode/BasicBlock types and shared CFG utilities (incl. dispatch/SSA/dataflow integrations).
Update/refresh C# library tests and expected outputs to match the new CFG structure and precision changes.

Show a summary per file

File	Description
shared/controlflow/codeql/controlflow/ControlFlowGraph.qll	Extends shared CFG AST signature and CFG construction logic (goto/throw/assignments/pattern-match, dominance utilities, consistency helpers).
java/ql/lib/semmle/code/java/ControlFlowGraph.qll	Adapts Java CFG wrapper to the updated shared CFG AST signature.
csharp/ql/test/library-tests/standalone/controlflow/cfg.ql	Updates test query to use `ControlFlowNode` instead of legacy CFG node types.
csharp/ql/test/library-tests/standalone/controlflow/cfg.expected	Updates expected CFG edge output for the shared CFG structure.
csharp/ql/test/library-tests/security/dataflow/flowsources/StoredFlowSources.expected	Updates expected results reflecting CFG/dataflow precision changes.
csharp/ql/test/library-tests/obinit/ObInit.ql	Updates object-initializer CFG test to new node/successor APIs.
csharp/ql/test/library-tests/obinit/ObInit.expected	Updates expected output for shared CFG.
csharp/ql/test/library-tests/goto/Goto1.ql	Updates goto CFG test to use `getASuccessor(t)` on `ControlFlowNode`.
csharp/ql/test/library-tests/goto/Goto1.expected	Updates expected output for shared CFG and goto edges.
csharp/ql/test/library-tests/dataflow/ssa/SSAPhiRead.ql	Updates SSA phi-read tests to shared `BasicBlock` / `ControlFlowNode` types.
csharp/ql/test/library-tests/dataflow/ssa/SSAPhiRead.expected	Updates expected SSA outputs.
csharp/ql/test/library-tests/dataflow/ssa/SsaDefElement.expected	Updates expected SSA definition/phi placement outputs.
csharp/ql/test/library-tests/dataflow/ssa-large/countssa.ql	Updates SSA-large test to shared `BasicBlock`.
csharp/ql/test/library-tests/dataflow/ssa-large/countssa.expected	Updates expected counts after CFG/SSA changes.
csharp/ql/test/library-tests/dataflow/signanalysis/SignAnalysis.ql	Updates sign analysis test to `ControlFlowNodes::ExprNode` naming.
csharp/ql/test/library-tests/dataflow/signanalysis/SignAnalysis.expected	Updates expected sign analysis results.
csharp/ql/test/library-tests/dataflow/signanalysis/MissingSign.ql	Updates missing-sign test to new control-flow node classes.
csharp/ql/test/library-tests/dataflow/modulusanalysis/ModulusAnalysis.ql	Updates modulus analysis test to new control-flow node classes.
csharp/ql/test/library-tests/dataflow/modulusanalysis/ModulusAnalysis.expected	Updates expected modulus analysis output.
csharp/ql/test/library-tests/dataflow/defuse/useUseEquivalence.ql	Updates def-use equivalence tests to shared CFG/BasicBlock traversal APIs.
csharp/ql/test/library-tests/dataflow/defuse/parameterUseEquivalence.ql	Updates parameter reachability test to shared `ControlFlowNode`.
csharp/ql/test/library-tests/dataflow/defuse/defUseEquivalence.ql	Updates def-use reachability test to shared `ControlFlowNode`.
csharp/ql/test/library-tests/dataflow/call-sensitivity/CallSensitivityFlow.expected	Updates expected call-sensitivity output after CFG/dataflow changes.
csharp/ql/test/library-tests/csharp8/UsingControlFlow.ql	Updates CFG edge selection in using-declarations test to new successor API.
csharp/ql/test/library-tests/csharp8/UsingControlFlow.expected	Updates expected CFG edges for using-declarations.
csharp/ql/test/library-tests/csharp8/switchstmtctrlflow.ql	Updates switch-statement CFG edge test to new successor API.
csharp/ql/test/library-tests/csharp8/switchexprcontrolflow.ql	Updates switch-expression CFG edge test to new successor API.
csharp/ql/test/library-tests/csharp8/NullCoalescingControlFlow.ql	Updates null-coalescing CFG test to new successor API.
csharp/ql/test/library-tests/csharp8/NullCoalescingControlFlow.expected	Updates expected CFG edges for null-coalescing (incl. `??=` modeling).
csharp/ql/test/library-tests/csharp8/NullableRefTypes.ql	Updates nullable-ref CFG test to new node/successor API.
csharp/ql/test/library-tests/csharp8/NullableRefTypes.expected	Updates expected CFG output for nullable-ref tests.
csharp/ql/test/library-tests/csharp8/ispatternflow.ql	Updates pattern-flow CFG test to new successor API.
csharp/ql/test/library-tests/csharp7/LocalTaintFlow.expected	Updates expected local taint flow after CFG/dataflow changes.
csharp/ql/test/library-tests/csharp7/IsFlow.ql	Updates is-pattern/switch CFG edge test to new successor API.
csharp/ql/test/library-tests/csharp11/signAnalysis.ql	Updates C#11 sign analysis test to `ControlFlowNodes::ExprNode`.
csharp/ql/test/library-tests/controlflow/guards/Guards.cs	Adjusts guard test source comments to reflect new handling.
csharp/ql/test/library-tests/controlflow/guards/GuardedExpr.expected	Updates expected guarded-expression results.
csharp/ql/test/library-tests/controlflow/guards/GuardedControlFlowNode.expected	Updates expected guarded control-flow node results.
csharp/ql/test/library-tests/controlflow/guards/BooleanGuardedExpr.expected	Updates expected boolean-guard results.
csharp/ql/test/library-tests/controlflow/guards-large/GuardedExpr.expected	Updates expected results for stress/large guard suite.
csharp/ql/test/library-tests/controlflow/graph/Nodes.ql	Updates controlflow graph tests to shared CFG entrypoint/node mapping.
csharp/ql/test/library-tests/controlflow/graph/NodeGraph.ql	Switches test import to shared `ControlFlow::TestOutput`.
csharp/ql/test/library-tests/controlflow/graph/ExitElement.ql	Removes legacy exit-element test relying on old CFG internals.
csharp/ql/test/library-tests/controlflow/graph/EntryElement.ql	Re-implements “first node” selection using shared CFG traversal.
csharp/ql/test/library-tests/controlflow/graph/EnclosingCallable.ql	Updates basic-block enclosing callable accessor.
csharp/ql/test/library-tests/controlflow/graph/CONSISTENCY/CfgConsistency.expected	Removes legacy CFG consistency expected output.
csharp/ql/test/library-tests/controlflow/graph/Condition.ql	Updates condition-block and successor selection to shared CFG APIs.
csharp/ql/test/library-tests/controlflow/graph/Common.qll	Updates test helper classes to shared CFG node/basic-block types.
csharp/ql/src/Security Features/CWE-384/AbandonSession.ql	Updates control-flow stepping logic to shared node APIs (`asExpr`, typed successors).
csharp/ql/src/Performance/StringBuilderInLoop.ql	Updates loop-entry selection to shared CFG positioning (`isBefore`).
csharp/ql/src/Likely Bugs/UncheckedCastInEquals.ql	Reworks reachability logic to shared CFG nodes for parameter-access ordering.
csharp/ql/src/Likely Bugs/Statements/UseBraces.ql	Updates successor-statement search to shared node-to-stmt mapping (`asStmt`).
csharp/ql/src/Likely Bugs/NestedLoopsSameVariable.ql	Updates guard/exit-node selection using shared CFG node predicates.
csharp/ql/src/Concurrency/UnsynchronizedStaticAccess.ql	Updates “unlocked reachable” analysis to operate on shared `BasicBlock`s and node indices.
csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql	Updates constant-condition query input signature to shared `BasicBlock`.
csharp/ql/lib/semmle/code/csharp/security/dataflow/ConditionalBypassQuery.qll	Removes dependency on legacy BasicBlocks module.
csharp/ql/lib/semmle/code/csharp/exprs/Call.qll	Refines accessor-call static target handling (read vs write) for compound assignments.
csharp/ql/lib/semmle/code/csharp/ExprOrStmtParent.qll	Adjusts enclosing-body/callable parent relationships after CFG changes.
csharp/ql/lib/semmle/code/csharp/dispatch/Dispatch.qll	Adds control-flow-node access on dispatch calls; splits accessor calls into read/write variants.
csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll	Updates SSA definition/node mapping and BasicBlock typing; adjusts string formatting and element mapping.
csharp/ql/lib/semmle/code/csharp/dataflow/SignAnalysis.qll	Updates sign-analysis API signatures from legacy CFG nodes to shared `ControlFlowNode`.
csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll	Updates nullness analysis to shared node types and `asExpr()` usage.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPublic.qll	Replaces `*` closure with `fastTC` for local taint reachability (plus identity case).
csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll	Refactors SSA implementation to shared CFG (`Cfg`/`BasicBlock`/`ControlFlowNode`) and updated dispatch/call-graph integration.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/SsaUtils.qll	Updates range-analysis SSA utilities to `ControlFlowNodes::ExprNode`.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/SsaReadPositionSpecific.qll	Updates read-position ordering helpers to shared CFG constructs and new imports/aliases.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll	Updates range-analysis sign modeling to shared control-flow node classes.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/RangeUtils.qll	Updates range utilities to shared CFG types and node accessors.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/ModulusAnalysisSpecific.qll	Updates modulus-specific range modeling to shared node classes.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/ConstantUtils.qll	Updates constant utilities to shared `ControlFlowNodes::ExprNode`.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/BoundSpecific.qll	Updates bound modeling to shared `ControlFlowNodes::ExprNode`.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll	Updates public dataflow API to shared node types and switches closure to `fastTC` (+ identity).
csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll	Refactors dataflow-call control-flow anchoring and multi-body callable handling to shared CFG types.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll	Updates BaseSSA to shared `BasicBlock`/`Cfg` types.
csharp/ql/lib/semmle/code/csharp/controlflow/internal/Splitting.qll	Removes legacy CFG splitting implementation (now handled by shared CFG).
csharp/ql/lib/semmle/code/csharp/controlflow/internal/NonReturning.qll	Simplifies non-returning call/callable modeling after CFG changes.
csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll	Updates guard modeling to shared CFG node/basic-block types and successor kinds.
csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowReachability.qll	Updates reachability config input types to shared CFG nodes/basic blocks.
csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowElement.qll	Reworks element↔node mapping to shared CFG injection model and introduces `ControlFlowElementOrCallable`.
csharp/ql/lib/semmle/code/csharp/controlflow/BasicBlocks.qll	Removes legacy BasicBlocks wrapper module (replaced by shared CFG basic blocks).
csharp/ql/lib/semmle/code/csharp/Callable.qll	Updates callable base class and entry/exit point types to shared CFG nodes.
csharp/ql/lib/semmle/code/csharp/Caching.qll	Removes legacy ControlFlowStage caching that depended on old splitting.
csharp/ql/lib/semmle/code/csharp/Assignable.qll	Updates assignable read/definition APIs to shared node/basic-block types.
csharp/ql/lib/printCfg.ql	Updates print-CFG contextual query to use shared `ControlFlow::ViewCfgQuery` inputs and new CFG scopes.
csharp/ql/consistency-queries/VariableCaptureConsistency.ql	Removes legacy basic-block uniqueness wrapper tied to old CFG scope mapping.
csharp/ql/consistency-queries/DataFlowConsistency.ql	Removes legacy CFG-scope exclusions and adjusts exclusions for accessor calls that are both read/write.
csharp/ql/consistency-queries/CfgConsistency.ql	Switches to shared `ControlFlow::Consistency` module.
csharp/ql/campaigns/Solorigate/src/ModifiedFnvFunctionDetection.ql	Updates loop-exit control-flow anchoring to shared node predicates.

Copilot's findings

Files reviewed: 101/110 changed files
Comments generated: 0

hvitved

Very nice rewrite, this is a great improvement over the old CFG, and nice to see how little C# specific code needs to be written.

hvitved · 2026-04-17T11:42:18Z


    private string loopHeaderTag() { result = "[LoopHeader]" }

+    private string patternMatchTrueTag() { result = "[match-true]" }


Inconsistent casing compared to loopHeaderTag.

hvitved · 2026-04-17T11:44:43Z


+          /** Holds if `n` does not have a unique enclosing callable. */
+          query predicate nonUniqueEnclosingCallable(AstNode n, int callables) {
+            callables = strictcount(getEnclosingCallable(n)) and callables > 1


Should we also report missing enclosing callables, i.e. callables != 1 and then count instead of strictcount?

(alternatively using unique.)

Yes, fixing. But I like getting the actual count for understanding any failures, so I'll stick with count.

hvitved · 2026-04-17T11:48:43Z

+           * That is, all paths reaching `that` node from the callable entry
+           * node (`EntryNode`) must go through this node.
+           */
+          bindingset[this, that]


I wonder if it is good thing to have this bindingset, could one not imagine cases where you want to ask for all nodes dominated by a given node (or vice versa)?

I think that would be a pretty bad anti-pattern in terms of performance - it would be almost as bad as a cartesian product. So I don't want to support that out-of-the-box. If you really want it, then it's not too hard to roll your own using basic block dominance.

hvitved · 2026-04-17T12:02:30Z

@@ -23,290 +23,23 @@ import csharp
 private import semmle.code.csharp.commons.Assertions
 private import semmle.code.csharp.commons.Constants


hvitved · 2026-04-17T12:02:37Z

 private import semmle.code.csharp.frameworks.System
-private import ControlFlowGraphImpl
 private import NonReturning
 private import SuccessorType


hvitved · 2026-04-17T12:30:11Z

+        result = us.getExpr() and
+        i = 0
+        or
+        result = us.getVariableDeclExpr(i)


I believe it's mutually exclusive with getExpr, so it should be good. It was taken verbatim from the old CFG here:

codeql/csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll

Line 1201 in 9f310c2

result = us.getVariableDeclExpr(i)

Ah, yes, it is.

hvitved · 2026-04-17T12:42:03Z

+      rank[i + 1](Expr init, Location l |
+        staticMemberInitializer(staticCtor, init) and
+        l = init.getLocation()
+      |
+        init order by l.getStartLine(), l.getStartColumn(), l.getFile().getAbsolutePath()
+      )


Nit: I would prefer

rank[i + 1](Expr init, Location l, string filepath, int startline, int startcolumn | staticMemberInitializer(staticCtor, init) and l = init.getLocation() and l.hasLocationInfo(filepath, startline, startcolumn, _, _) | init order by startline, startcolumn, filepath )

hvitved · 2026-04-17T12:43:10Z

+  AssignExpr initializedInstanceMemberOrder(ObjectInitMethod obinit, CompilationExt comp, int i) {
+    obinit.initializes(result) and
+    result =
+      rank[i + 1](AssignExpr ae0, Location l |


hvitved · 2026-04-17T12:52:41Z

+        else
+          if exists(ctor.getInitializer())
+          then n2.isBefore(ctor.getInitializer())
+          else n2.isBefore(ctor.getBody())


For my understanding: If we instead did else none(), wouldn't it then be handled by the shared library?

That's correct.

hvitved · 2026-04-20T08:25:38Z

+    call.getControlFlowNode() = callBlock.getNode(j) and
+    callBlock = unlockedReachable(a) and
+    (
+      exists(int i | j <= i and firstLockingCallInBlock(callBlock, i))


I guess we don't want to consider the locking calls themselves? I.e. j < i instead.

I agree that sounds more reasonable, but I tried not to alter the query (and this was the preexisting condition).

github-actions bot added C# Java labels Mar 24, 2026

github-advanced-security AI found potential problems Mar 24, 2026

View reviewed changes

Comment thread csharp/ql/lib/semmle/code/csharp/commons/Constants.qll Fixed

Comment thread csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowGraph.qll Fixed

Comment thread csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/SsaReadPositionSpecific.qll Fixed

github-advanced-security AI found potential problems Mar 26, 2026

View reviewed changes

Comment thread csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll Dismissed

aschackmull force-pushed the csharp/cfg2 branch 3 times, most recently from 8dd66df to da7e652 Compare March 31, 2026 09:18

aschackmull force-pushed the csharp/cfg2 branch 6 times, most recently from 3db7a2a to ebd0412 Compare April 9, 2026 13:41

aschackmull added 17 commits April 10, 2026 15:47

Cfg: Support Throw expressions.

93a594e

Cfg: Support GotoStmt.

a53cffc

Cfg: Support short-circuiting compound assignments.

0b6c416

C#: Introduce ControlFlowElementOrCallable.

035b83c

Cfg/Java: Move InstanceOfExpr CFG into shared lib.

6ffed85

Cfg: Extend consistency checks.

88aaff8

C#: Rename ControlFlow::Node to ControlFlowNode.

61976e3

Cfg: Add dominance predicates to shared ControlFlowNode.

b85b02a

C#: Update some references in preparation for CFG swap.

03f6bdb

C#: Update some references to ControlFlow::Nodes.

b878ae3

C#: Rename remaining references to ControlFlow::Nodes.

13a4141

C#: Rename ControlFlow::BasicBlock to BasicBlock.

9cf9a36

C#: Replace CFG.

ff978d1

C#: Fix test.

b179033

C#: Fix UncheckedCastInEquals.

700d56f

C#: Bugfix in enclosing callable.

ac88b73

C#: Fix CFG position of property setter calls.

093eb57

aschackmull added 19 commits April 10, 2026 15:47

C#: Phi nodes are not expected to have associated Elements.

1a6670a

C#: Accept bugfix.

6010640

C#: Accept harmless CFG changes.

a5c99f9

C#: Accept CFG changes.

5d58909

C#: Compile-time constants no longer have CFG nodes.

49cc931

C#: Accept irrelevant changes.

e90243c

The additions are unintentional, but the fault lies with the shared SignAnalysis code. The removals are due to compile-time constant initializers no longer having CFG nodes.

C#: GuardedExpr no longer contains expressions guarded solely by disj…

88256ee

…unctions.

C#: Accept data flow inconsistency check for read+write calls.

773881f

C#: Accept fixed consistency check.

a997d9f

C#: Accept CFG changes for "first" relation.

a695819

C#: Accept changed location for phi nodes.

a7d4b00

C#: CFG and data flow nodes now exist for LHSs.

371bc30

C#: Fix perf.

1d9c0ae

C#: Fix some bad join orders.

bfbd0f7

C#: Rework DataFlowCallable-to-cfg relation in terms of basic blocks …

bbd403d

…for performance.

C#: Accept new CFG in tests.

2d5a184

C#: Accept fewer CallContextSpecificCall due to no splitting.

aaf9bb2

C#: Improve perf of UnsynchronizedStaticAccess.ql.

452913f

C#/Cfg: A bit more qldoc.

d5c9fd1

aschackmull force-pushed the csharp/cfg2 branch from 53d735b to d5c9fd1 Compare April 10, 2026 13:47

aschackmull marked this pull request as ready for review April 13, 2026 07:06

Copilot AI review requested due to automatic review settings April 13, 2026 07:06

aschackmull requested review from a team as code owners April 13, 2026 07:06

Copilot started reviewing on behalf of aschackmull April 13, 2026 07:07 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

C#: Add change note.

88160ef

github-actions bot added the documentation label Apr 13, 2026

hvitved reviewed Apr 20, 2026

View reviewed changes


		private string loopHeaderTag() { result = "[LoopHeader]" }

		private string patternMatchTrueTag() { result = "[match-true]" }

		@@ -23,290 +23,23 @@ import csharp
		private import semmle.code.csharp.commons.Assertions
		private import semmle.code.csharp.commons.Constants

Conversation

aschackmull commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

hvitved left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aschackmull commented Mar 24, 2026 •

edited

Loading