C#: Adopt shared SSA data-flow integration #16936
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -1118,3 +1055,64 @@ | |||
| result = this.getSourceVariable().getEnclosingCallable() | |||
| } | |||
| } | |||
|
|
|||
| private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInputSig { | |||
| private import csharp as Cs | |||
Check warning
Code scanning / CodeQL
Names only differing by case Warning
hvitved
force-pushed
the
csharp/ssa-integration
branch
from
b5c4d66 to
6b1b308
Compare
hvitved
force-pushed
the
csharp/ssa-integration
branch
2 times, most recently
from
2b175c6 to
df69c4e
Compare
hvitved
force-pushed
the
csharp/ssa-integration
branch
2 times, most recently
from
5b70129 to
86f7735
Compare
hvitved
force-pushed
the
csharp/ssa-integration
branch
from
86f7735 to
591b745
Compare
shared/ssa/codeql/ssa/Ssa.qll
Outdated
|
|
||
| private newtype TNode = | ||
| TParamNode(DfInput::Parameter p) { | ||
| exists(WriteDefinition def | DfInput::ssaDefInitializesParam(def, p)) |
Check warning
Code scanning / CodeQL
Omittable 'exists' variable Warning
hvitved
force-pushed
the
csharp/ssa-integration
branch
from
591b745 to
ac08fc5
Compare
| exists(Guard g, Expr e, AbstractValue v | | ||
| guardChecks(g, e, v) and | ||
| g.controlsNode(result.getControlFlowNode(), e, v) |
There was a problem hiding this comment.
Why is this second disjunct not redundant now? Does it cover more than ssa variable reads?
There was a problem hiding this comment.
Good question: Yes, it covers more than SSA variable reads (it uses GVN, which--although it is unsound in general--is likely to produce valid guards).
hvitved
force-pushed
the
csharp/ssa-integration
branch
from
ac08fc5 to
6506034
Compare
hvitved
force-pushed
the
csharp/ssa-integration
branch
from
6506034 to
89a2381
Compare
| isUseStep = false | ||
| or | ||
| // Flow into phi (read)/uncertain SSA definition node from read | ||
| exists(Node read | LocalFlow::localFlowSsaInputFromRead(read, def, nodeTo) | | ||
| nodeFrom = read and | ||
| not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _) | ||
| or | ||
| nodeFrom.(PostUpdateNode).getPreUpdateNode() = read | ||
| ) | ||
| not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _) |
There was a problem hiding this comment.
There's a pipeline duplication here, better add isUseStep = true to the second disjunct.
This PR adopts the newly introduced shared SSA data-flow integration layer. A side-effect is that we get phi-input barrier guards for free, which had not previously been ported to C#.