C#: Include parameters and their defaults in the CFG#21759
Open
hvitved wants to merge 8 commits intogithub:mainfrom
Open
C#: Include parameters and their defaults in the CFG#21759hvitved wants to merge 8 commits intogithub:mainfrom
hvitved wants to merge 8 commits intogithub:mainfrom
Conversation
764c65c to
f09d4e5
Compare
3b34de4 to
889f721
Compare
889f721 to
12d3acf
Compare
12d3acf to
3a62ffa
Compare
Contributor
Author
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the shared control-flow graph (CFG) library to include callable parameters and their default values as CFG nodes, and wires that functionality into the C# libraries (including SSA/dataflow integration and schema support).
Changes:
- Extend the shared CFG AST signature with
Parameter+callableGetParameter, and model default-argument control flow usingMatchingsuccessors. - Update the C# CFG implementation to emit parameter/default nodes, update SSA to create parameter + parameter-default definitions (so phis are inserted naturally), and adjust related dataflow helpers.
- Update the C# dbscheme and add upgrade/downgrade artifacts; update/extend C# CFG & SSA/dataflow tests and expected outputs (including new default-parameter test cases).
Show a summary per file
| File | Description |
|---|---|
| shared/controlflow/codeql/controlflow/ControlFlowGraph.qll | Adds parameter support to the shared CFG library and integrates default-value branching into successor modeling. |
| java/ql/lib/semmle/code/java/ControlFlowGraph.qll | Adapts Java CFG wiring to the new shared CFG signature (context type rename; parameter stubs). |
| csharp/ql/test/query-tests/Nullness/NullMaybe.expected | Updates expected nullness output impacted by new parameter/default SSA/CFG modeling. |
| csharp/ql/test/library-tests/obinit/ObInit.expected | Updates expected CFG output due to parameter nodes being present at callable entry. |
| csharp/ql/test/library-tests/dataflow/ssa/SsaUltimateDef.expected | Updates SSA ultimate-definition expectations to include parameter/default definitions and phis. |
| csharp/ql/test/library-tests/dataflow/ssa/SsaRead.expected | Updates SSA read expectations for parameters now represented in SSA/CFG. |
| csharp/ql/test/library-tests/dataflow/ssa/SsaImplicitParameterDef.ql | Updates test query to use the new parameter-definition class name. |
| csharp/ql/test/library-tests/dataflow/ssa/SsaImplicitParameterDef.expected | Updates expected results for the renamed/reworked parameter SSA definition representation. |
| csharp/ql/test/library-tests/dataflow/ssa/SsaExplicitDef.expected | Updates expected explicit-def results reflecting parameter SSA defs now emitted. |
| csharp/ql/test/library-tests/dataflow/ssa/SsaDefElement.expected | Updates expected def-element results for parameter/default SSA def elements. |
| csharp/ql/test/library-tests/dataflow/ssa/SsaDef.expected | Updates expected SSA defs to include parameter/default defs and phis. |
| csharp/ql/test/library-tests/dataflow/ssa/SSAPhi.expected | Updates expected phi nodes created due to parameter/default merge points. |
| csharp/ql/test/library-tests/dataflow/ssa/DefaultParam.cs | New SSA test case for default parameters. |
| csharp/ql/test/library-tests/dataflow/ssa/DefAdjacentRead.expected | Updates adjacency expectations due to parameter node introduction. |
| csharp/ql/test/library-tests/dataflow/ssa/BaseSsaConsistency.ql | Updates SSA consistency test to refer to the new parameter-definition type. |
| csharp/ql/test/library-tests/dataflow/local/TaintTrackingStep.expected | Updates expected local taint-tracking step output for default-parameter flow. |
| csharp/ql/test/library-tests/dataflow/local/TaintTracking.expected | Updates expected local taint-tracking alerts to include default-parameter cases. |
| csharp/ql/test/library-tests/dataflow/local/LocalDataFlow.cs | Adds a default-parameter dataflow test method. |
| csharp/ql/test/library-tests/dataflow/local/DataFlowStep.expected | Updates expected local dataflow-step output for default-parameter flow. |
| csharp/ql/test/library-tests/dataflow/local/DataFlow.expected | Updates expected local dataflow results to include default-parameter cases. |
| csharp/ql/test/library-tests/dataflow/defuse/parameterUseEquivalence.ql | Updates def-use equivalence test to use the new parameter-definition type. |
| csharp/ql/test/library-tests/dataflow/defuse/defUseEquivalence.ql | Updates equivalence logic to account for parameter CFG nodes when computing reachability. |
| csharp/ql/test/library-tests/csharp8/switchexprcontrolflow.expected | Updates expected CFG output due to parameter nodes at entry. |
| csharp/ql/test/library-tests/csharp7/DefUse.ql | Updates SSA def-use test to use the new parameter-definition type. |
| csharp/ql/test/library-tests/controlflow/graph/Nodes.expected | Updates expected CFG node listing to include parameters as CFG elements. |
| csharp/ql/test/library-tests/controlflow/graph/DefaultParam.cs | New CFG test case for default parameters. |
| csharp/ql/test/library-tests/controlflow/graph/Condition.expected | Adds expected conditional edges for default-parameter “match/no-match” successors. |
| csharp/ql/lib/upgrades/19b8cc3e2dc768d4cbc03d6e3773b709bbebd036/upgrade.properties | Adds a C# database upgrade for schema change related to parameters as CFG elements. |
| csharp/ql/lib/upgrades/19b8cc3e2dc768d4cbc03d6e3773b709bbebd036/old.dbscheme | Upgrade scaffolding for the dbscheme change (dummy line approach). |
| csharp/ql/lib/semmlecode.csharp.dbscheme | Extends @control_flow_element to include @parameter. |
| csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll | Updates SSA internals to represent parameter entry/default defs and handle multi-body callables. |
| csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll | Adjusts internal dataflow helpers to reflect parameter CFG nodes and multi-body behavior. |
| csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll | Exposes/renames parameter SSA definitions, adds parameter-default SSA definitions, updates doc/comments accordingly. |
| csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll | Updates nullness analysis to use the new parameter-definition type and removes old “null default” special-casing. |
| csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraph.qll | New internal C# AST/CFG wiring module implementing parameters + default values for shared CFG. |
| csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll | Updates guard logic to use the new parameter-definition type. |
| csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowGraph.qll | Switches to the new internal CFG module and updates callable-context plumbing. |
| csharp/ql/lib/semmle/code/csharp/Variable.qll | Makes Parameter a ControlFlowElement to align with new CFG modeling. |
| csharp/ql/lib/semmle/code/csharp/PrintAst.qll | Excludes parameters from generic ControlFlowElement printing (handled by a dedicated node). |
| csharp/ql/lib/semmle/code/csharp/ExprOrStmtParent.qll | Adjusts parent/child hierarchy to accommodate parameters as top-level expression parents and CFG elements. |
| csharp/ql/lib/semmle/code/csharp/Assignable.qll | Adds an assignable-definition for parameter default values and aligns assignable/SSA integration. |
| csharp/ql/lib/printCfg.ql | Updates CFG printing query imports/types for the new internal CFG module and callable type usage. |
| csharp/downgrades/a5765bf26f38b38424eeb97f83104b5f5b41db0d/upgrade.properties | Adds downgrade metadata to revert parameters-as-control-flow-elements schema change. |
| csharp/downgrades/a5765bf26f38b38424eeb97f83104b5f5b41db0d/old.dbscheme | Downgrade scaffolding for the dbscheme change. |
Copilot's findings
- Files reviewed: 49/51 changed files
- Comments generated: 1
| p = n and | ||
| c = p.getCallable(l) and | ||
| ( | ||
| // In case `c` has multiple bodies, we want each body to gets its own implicit |
There was a problem hiding this comment.
Grammar: change "to gets" to "to get" in this comment.
Suggested change
| // In case `c` has multiple bodies, we want each body to gets its own implicit | |
| // In case `c` has multiple bodies, we want each body to get its own implicit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds support in the shared CFG library for including parameters and their default values, and applies this to C# (even though, technically, default values are inlined into the callers).
We use
Matchingsuccessors to denote whether an argument has been supplied or not. For example, this C# programgives rise to the following CFG (skipping the parts inside the body):
flowchart TD 1["Entry"] 10["After i [no-match]"] 11["i"] 12["0"] 13["{...}"] 4["b"] 5["After s [match]"] 6["After s [no-match]"] 7["s"] 8['""'] 9["After i [match]"] 1 --> 4 4 --> 7 5 --> 11 6 --> 8 7 -- match --> 5 7 -- no-match --> 6 8 --> 11 9 --> 13 10 --> 12 11 -- match --> 9 11 -- no-match --> 10 12 --> 13With parameters and their defaults added to the CFG, we replace existing SSA definitions at function entry with their CFG nodes (except when functions have multiple bodies), and we also add SSA definitions corresponding to defaults, which means that phi nodes will automatically be inserted.
Commit-by-commit review is strongly encouraged.