C#: Include parameters and their defaults in the CFG#21759
C#: Include parameters and their defaults in the CFG#21759hvitved merged 9 commits intogithub:mainfrom
Conversation
764c65c to
f09d4e5
Compare
12d3acf to
3a62ffa
Compare
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
6385a7f to
99b5cec
Compare
| } | ||
|
|
||
| pragma[nomagic] | ||
| Ast::Parameter callableGetParameter(Ast::Callable c, CallableContext ctx, int index) { |
There was a problem hiding this comment.
Are parameters duplicated across different compilations in the multibody case while sharing a callable? I thought perhaps they were closer tied to the callable than the body, but I admit I don't know.
There was a problem hiding this comment.
I guess having looked at a bunch of code that deals with multiple locations for parameters led me to believe that the parameter entities were shared across compilations.
There was a problem hiding this comment.
It depends whether the parameters coincide; when they do, they are collapsed to one (occurring in both compilations), so if the same source code is compiled twice, they will be collapsed, but if two methods with the same fully-qualified-names are extracted, but have different signatures, then both set of parameters will exist in the DB, and the above will prevent them from getting mixed up.
Co-authored-by: Anders Schack-Mulligen <aschackmull@users.noreply.github.com>
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.