Skip to content

C#: Replace SSA classes with shared code.#21762

Merged
aschackmull merged 30 commits intogithub:mainfrom
aschackmull:csharp/ssa2
May 4, 2026
Merged

C#: Replace SSA classes with shared code.#21762
aschackmull merged 30 commits intogithub:mainfrom
aschackmull:csharp/ssa2

Conversation

@aschackmull
Copy link
Copy Markdown
Contributor

@aschackmull aschackmull commented Apr 28, 2026

This aligns the C# SSA classes with the shared SSA signature by using the classes provided by the shared library.

@github-actions github-actions Bot added the C# label Apr 28, 2026
@aschackmull aschackmull force-pushed the csharp/ssa2 branch 2 times, most recently from ef22df2 to 7b3cdc8 Compare April 29, 2026 07:52
@aschackmull aschackmull marked this pull request as ready for review May 1, 2026 11:15
@aschackmull aschackmull requested a review from a team as a code owner May 1, 2026 11:15
Copilot AI review requested due to automatic review settings May 1, 2026 11:15
@aschackmull aschackmull requested a review from a team as a code owner May 1, 2026 11:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates the C# SSA library to the shared SSA signature by switching C# SSA consumers to shared SSA classes (for example SsaDefinition, SsaExplicitWrite, SsaPhiDefinition) and deprecating the legacy C#-specific SSA wrapper classes.

Changes:

  • Update shared SSA toString() for SSA definitions (including phi definitions) to match the shared formatting.
  • Rework the C# SSA library surface to expose shared SSA types and add migration helpers (for example Ssa::ssaGetAFirstUse), while deprecating legacy types/APIs.
  • Bulk-update C# queries and test queries/expectations to the new SSA API and renamed classes.
Show a summary per file
File Description
shared/ssa/codeql/ssa/Ssa.qll Adjust SSA definition/phi toString() formatting in the shared SSA library.
csharp/ql/test/library-tests/dataflow/ssa/SsaUltimateDef.ql Update test query to use shared SSA definition types.
csharp/ql/test/library-tests/dataflow/ssa/SsaUltimateDef.expected Update expected output for renamed SSA types / locations.
csharp/ql/test/library-tests/dataflow/ssa/SsaRead.ql Update test query to use SsaDefinition.
csharp/ql/test/library-tests/dataflow/ssa/SsaRead.expected Update expected output for shared SSA API changes.
csharp/ql/test/library-tests/dataflow/ssa/SsaImplicitQualifier.expected Update expected output for qualifier SSA behavior/printing changes.
csharp/ql/test/library-tests/dataflow/ssa/SsaImplicitParameterDef.ql Update test query to use SsaParameterInit.
csharp/ql/test/library-tests/dataflow/ssa/SsaExplicitDef.ql Update test query to use SsaExplicitWrite and new accessor names.
csharp/ql/test/library-tests/dataflow/ssa/SsaExplicitDef.expected Update expected output for explicit write changes.
csharp/ql/test/library-tests/dataflow/ssa/SsaDefElement.ql Remove obsolete test query relying on legacy SSA API.
csharp/ql/test/library-tests/dataflow/ssa/SsaDefElement.expected Remove corresponding obsolete expected file.
csharp/ql/test/library-tests/dataflow/ssa/SsaDef.ql Update test query to use SsaDefinition.
csharp/ql/test/library-tests/dataflow/ssa/SsaDef.expected Update expected output for SsaDefinition output changes.
csharp/ql/test/library-tests/dataflow/ssa/SSAPhi.ql Update phi test query to use SsaPhiDefinition/SsaDefinition.
csharp/ql/test/library-tests/dataflow/ssa/SSAPhi.expected Update expected output for phi definition formatting/locations.
csharp/ql/test/library-tests/dataflow/ssa/IsLiveOutRefParameterDefinition.ql Switch from method call to Ssa::isLiveOutRefParameterDefinition(def, p) helper.
csharp/ql/test/library-tests/dataflow/ssa/DefAdjacentRead.expected Update expected output for SSA adjacency results after migration.
csharp/ql/test/library-tests/dataflow/ssa/BaseSsaConsistency.ql Update SSA consistency test to use shared write types.
csharp/ql/test/library-tests/dataflow/ssa-large/countssa.ql Update SSA large-count test to use SsaExplicitWrite.
csharp/ql/test/library-tests/dataflow/local/TaintTrackingStep.expected Update expected taint-tracking steps for shared SSA API changes.
csharp/ql/test/library-tests/dataflow/local/DataFlowStep.expected Update expected data-flow steps for shared SSA API changes.
csharp/ql/test/library-tests/dataflow/defuse/useUseEquivalence.ql Update def/use equivalence test to shared SSA types and helper for first use.
csharp/ql/test/library-tests/dataflow/defuse/parameterUseEquivalence.ql Update parameter-use equivalence test to SsaParameterInit.
csharp/ql/test/library-tests/dataflow/defuse/defUseEquivalence.ql Update def/use equivalence test to shared explicit write type/accessors.
csharp/ql/test/library-tests/dataflow/callablereturnsarg/Common.qll Update out/ref tracking helper to shared SSA write types and predicates.
csharp/ql/test/library-tests/csharp7/LocalTaintFlow.expected Update expected flow steps for SSA locations/formatting changes.
csharp/ql/test/library-tests/csharp7/DefUse.ql Update C#7 def/use test to shared SSA definition types.
csharp/ql/src/Likely Bugs/Dynamic/BadDynamicCall.ql Update query to shared SSA types/accessors to resolve ultimate definition.
csharp/ql/src/Dead Code/DeadStoreOfLocal.ql Update query to treat SSA explicit writes via SsaExplicitWrite.
csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql Switch constant-condition integration to shared SSA definition class reference.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/SsaUtils.qll Update range analysis SSA utilities to shared SSA types and accessors.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/SsaReadPositionSpecific.qll Update SSA type aliases to CS::SsaDefinition / CS::SsaPhiDefinition.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll Update sign analysis SSA types and explicit write handling to shared SSA API.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/RangeUtils.qll Update range utilities to shared SSA definition types.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/ModulusAnalysisSpecific.qll Update modulus analysis to use shared SsaPhiDefinition.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll Refactor C# SSA implementation to use shared SsaImplCommon input signature and provide deprecated compatibility shims.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll Update internal data-flow SSA integration points to shared SSA types and helper predicates.
csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll Restructure BaseSSA to expose SSA via shared implementation modules.
csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll Re-export shared SSA API from the public C# SSA library; add migration helper(s) and deprecations.
csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll Update nullness analysis to shared SSA types and new first-use logic.
csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll Simplify guard SSA integration by importing Ssa rather than duplicating SSA wrapper classes.
csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowReachability.qll Switch reachability integration to import Ssa shared SSA types.
csharp/ql/lib/semmle/code/csharp/Assignable.qll Update AssignableDefinition.getAFirstRead implementation to use Ssa::ssaGetAFirstUse.
csharp/ql/lib/change-notes/2026-05-01-ssa-replacement.md Add change note documenting SSA rename/deprecation and migration guidance location.
csharp/ql/consistency-queries/SsaConsistency.ql Update consistency query to use SsaExplicitWrite and new accessors.

Copilot's findings

  • Files reviewed: 45/45 changed files
  • Comments generated: 1

Comment thread csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for doing this. Some minor comments.

Comment thread csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll
Comment on lines +187 to +190
exists(ControlFlowNode cfn |
SsaImpl::firstReadSameVar(def, cfn) and
result.getControlFlowNode() = cfn
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest folding this into the cached predicate firstReadSameVar, and then rename that predicate to firstUse, as for Java.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fold it in, but I'm a bit hesitant to rename the predicate as it doesn't match the semantics of the Java version. In Java the firstUse predicate allows flow through phi nodes (it has samevar = false), whereas the C# version restricts to cases where it's the same SSA variable being read (samevar = true).

)
}

predicate isLiveOutRefParameterDefinition = SsaImpl::isLiveOutRefParameterDefinition/2;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather that we do not expose it, and the internal uses that we have go directly to SsaImpl instead. The deprecation comment below should then not mention using isLiveOutRefParameterDefinition(SsaDefinition, Parameter).

/** Gets the callable to which this SSA definition belongs. */
final Callable getEnclosingCallable() {
/**
* DEPRECATED: Use `getSourceVariable().getEnclosingCallable()` instead.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we instead add this to the shared implementation? Seems convenient enough to have.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I guess we can do that - it does require the introduction of the Callable type plus an enclosing-callable predicate for SourceVariables in the input signature.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or alternatively have callable be part of the CfgSig from BasicBlock.qll and then use CfgSig in Ssa.qll.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OTOH this wasn't actually used - so is it really worth it given that we have to expand the SsaInputSig with two more declarations?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or alternatively have callable be part of the CfgSig from BasicBlock.qll and then use CfgSig in Ssa.qll.

That will require extending the CfgSig with another parameter so it'll be CfgSig<Location, Callable> rather than just CfgSig<Location>. That's something that I've actually considered quite a bit before, but it's tricky since we'll force every other piece of code that depends on CfgSig to have access to Callable, which may introduce cascading additional parameterisation. (Of course CfgSig could wrap up Callable as an existential type instead of a parameter, but I fear that leads to the kind of trouble that has led other languages to add type constraints.)

@aschackmull aschackmull merged commit b67ebd1 into github:main May 4, 2026
140 checks passed
@aschackmull aschackmull deleted the csharp/ssa2 branch May 4, 2026 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants