Skip to content

Commit b67ebd1

Browse files
authored
Merge pull request #21762 from aschackmull/csharp/ssa2
C#: Replace SSA classes with shared code.
2 parents 04a8ef0 + 02f5fe9 commit b67ebd1

45 files changed

Lines changed: 596 additions & 862 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

csharp/ql/consistency-queries/SsaConsistency.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ query predicate localDeclWithSsaDef(LocalVariableDeclExpr d) {
77
// Local variables in C# must be initialized before every use, so uninitialized
88
// local variables should not have an SSA definition, as that would imply that
99
// the declaration is live (can reach a use without passing through a definition)
10-
exists(ExplicitDefinition def |
11-
d = def.getADefinition().(AssignableDefinitions::LocalVariableDefinition).getDeclaration()
10+
exists(SsaExplicitWrite def |
11+
d = def.getDefinition().(AssignableDefinitions::LocalVariableDefinition).getDeclaration()
1212
|
1313
not d = any(ForeachStmt fs).getVariableDeclExpr() and
1414
not d = any(SpecificCatchClause scc).getVariableDeclExpr() and
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: deprecated
3+
---
4+
* The QL classes in the C# SSA library have been renamed to improve consistency between languages. Any custom QL code that makes use of SSA needs to be updated. The old classes have been deprecated and include more detailed migration instructions in their qldoc.

csharp/ql/lib/semmle/code/csharp/Assignable.qll

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -500,11 +500,7 @@ class AssignableDefinition extends TAssignableDefinition {
500500
*/
501501
pragma[nomagic]
502502
AssignableRead getAFirstRead() {
503-
exists(ControlFlowNode cfn | cfn = result.getControlFlowNode() |
504-
exists(Ssa::ExplicitDefinition def | result = def.getAFirstReadAtNode(cfn) |
505-
this = def.getADefinition()
506-
)
507-
)
503+
exists(SsaExplicitWrite def | result = Ssa::ssaGetAFirstUse(def) | this = def.getDefinition())
508504
}
509505

510506
/** Gets a textual representation of this assignable definition. */

csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowReachability.qll

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,7 @@ private module ControlFlowInput implements InputSig<Location, ControlFlowNode, B
2626

2727
class Expr = CS::Expr;
2828

29-
class SourceVariable = Ssa::SourceVariable;
30-
31-
class SsaDefinition = Ssa::Definition;
32-
33-
class SsaExplicitWrite extends SsaDefinition instanceof Ssa::ExplicitDefinition {
34-
Expr getValue() { result = super.getADefinition().getSource() }
35-
}
36-
37-
class SsaPhiDefinition = Ssa::PhiNode;
38-
39-
class SsaUncertainWrite = Ssa::UncertainDefinition;
29+
import Ssa
4030

4131
class GuardValue = Guards::GuardValue;
4232

csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll

Lines changed: 17 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -191,23 +191,7 @@ private module GuardsImpl = SharedGuards::Make<Location, Cfg, GuardsInput>;
191191
class GuardValue = GuardsImpl::GuardValue;
192192

193193
private module LogicInput implements GuardsImpl::LogicInputSig {
194-
class SsaDefinition extends Ssa::Definition {
195-
Expr getARead() { super.getARead() = result }
196-
}
197-
198-
class SsaExplicitWrite extends SsaDefinition instanceof Ssa::ExplicitDefinition {
199-
Expr getValue() { result = super.getADefinition().getSource() }
200-
}
201-
202-
class SsaPhiDefinition extends SsaDefinition instanceof Ssa::PhiNode {
203-
predicate hasInputFromBlock(SsaDefinition inp, BasicBlock bb) {
204-
super.hasInputFromBlock(inp, bb)
205-
}
206-
}
207-
208-
class SsaParameterInit extends SsaDefinition instanceof Ssa::ParameterDefinition {
209-
Parameter getParameter() { result = super.getParameter() }
210-
}
194+
import Ssa
211195

212196
predicate additionalNullCheck(GuardsImpl::PreGuard guard, GuardValue val, Expr e, boolean isNull) {
213197
// Comparison with a non-`null` value, for example `x?.Length > 0`
@@ -586,30 +570,30 @@ class AccessOrCallExpr extends Expr {
586570
* An expression can have more than one SSA qualifier in the presence
587571
* of control flow splitting.
588572
*/
589-
Ssa::Definition getAnSsaQualifier(ControlFlowNode cfn) { result = getAnSsaQualifier(this, cfn) }
573+
SsaDefinition getAnSsaQualifier(ControlFlowNode cfn) { result = getAnSsaQualifier(this, cfn) }
590574
}
591575

592576
private Declaration getDeclarationTarget(Expr e) {
593577
e = any(AssignableAccess aa | result = aa.getTarget()) or
594578
result = e.(Call).getTarget()
595579
}
596580

597-
private Ssa::Definition getAnSsaQualifier(Expr e, ControlFlowNode cfn) {
581+
private SsaDefinition getAnSsaQualifier(Expr e, ControlFlowNode cfn) {
598582
e = getATrackedAccess(result, cfn)
599583
or
600584
not e = getATrackedAccess(_, _) and
601585
result = getAnSsaQualifier(e.(QualifiableExpr).getQualifier(), cfn)
602586
}
603587

604-
private AssignableAccess getATrackedAccess(Ssa::Definition def, ControlFlowNode cfn) {
605-
result = def.getAReadAtNode(cfn)
588+
private AssignableAccess getATrackedAccess(SsaDefinition def, ControlFlowNode cfn) {
589+
result = def.getARead() and cfn = result.getControlFlowNode()
606590
or
607-
result = def.(Ssa::ExplicitDefinition).getADefinition().getTargetAccess() and
591+
result = def.(SsaExplicitWrite).getDefinition().getTargetAccess() and
608592
cfn = def.getControlFlowNode()
609593
}
610594

611595
private predicate ssaMustHaveValue(Expr e, GuardValue v) {
612-
exists(Ssa::Definition def, BasicBlock bb |
596+
exists(SsaDefinition def, BasicBlock bb |
613597
e = def.getARead() and
614598
e.getBasicBlock() = bb and
615599
Guards::ssaControls(def, bb, v)
@@ -841,29 +825,25 @@ module Internal {
841825
)
842826
or
843827
e =
844-
any(Ssa::Definition def |
845-
forex(Ssa::Definition u | u = def.getAnUltimateDefinition() | nullDef(u))
828+
any(SsaDefinition def |
829+
forex(SsaDefinition u | u = def.getAnUltimateDefinition() | nullDef(u))
846830
).getARead()
847831
}
848832

849-
private predicate nullDef(Ssa::ExplicitDefinition def) {
850-
nullValueImplied(def.getADefinition().getSource())
851-
}
833+
private predicate nullDef(SsaExplicitWrite def) { nullValueImplied(def.getValue()) }
852834

853835
predicate nonNullValueImplied(Expr e) {
854836
nonNullValue(e)
855837
or
856838
exists(Expr e1 | nonNullValueImplied(e1) and nonNullValueImpliedUnary(e1, e))
857839
or
858840
e =
859-
any(Ssa::Definition def |
860-
forex(Ssa::Definition u | u = def.getAnUltimateDefinition() | nonNullDef(u))
841+
any(SsaDefinition def |
842+
forex(SsaDefinition u | u = def.getAnUltimateDefinition() | nonNullDef(u))
861843
).getARead()
862844
}
863845

864-
private predicate nonNullDef(Ssa::ExplicitDefinition def) {
865-
nonNullValueImplied(def.getADefinition().getSource())
866-
}
846+
private predicate nonNullDef(SsaExplicitWrite def) { nonNullValueImplied(def.getValue()) }
867847

868848
/** A callable that always returns a non-`null` value. */
869849
private class NonNullCallable extends Callable {
@@ -1120,7 +1100,7 @@ module Internal {
11201100
private predicate nodeIsGuardedBySameSubExprSsaDef0(
11211101
ControlFlowNode cfn, BasicBlock guardedBB, AccessOrCallExpr guarded, Guard g,
11221102
ControlFlowNode subCfn, BasicBlock subCfnBB, AccessOrCallExpr sub, GuardValue v,
1123-
Ssa::Definition def
1103+
SsaDefinition def
11241104
) {
11251105
nodeIsGuardedBySameSubExpr(cfn, guardedBB, guarded, g, sub, v) and
11261106
def = sub.getAnSsaQualifier(subCfn) and
@@ -1130,7 +1110,7 @@ module Internal {
11301110
pragma[nomagic]
11311111
private predicate nodeIsGuardedBySameSubExprSsaDef(
11321112
ControlFlowNode guardedCfn, AccessOrCallExpr guarded, Guard g, ControlFlowNode subCfn,
1133-
AccessOrCallExpr sub, GuardValue v, Ssa::Definition def
1113+
AccessOrCallExpr sub, GuardValue v, SsaDefinition def
11341114
) {
11351115
exists(BasicBlock guardedBB, BasicBlock subCfnBB |
11361116
nodeIsGuardedBySameSubExprSsaDef0(guardedCfn, guardedBB, guarded, g, subCfn, subCfnBB, sub,
@@ -1149,7 +1129,7 @@ module Internal {
11491129
cached
11501130
predicate isGuardedByExpr(AccessOrCallExpr guarded, Guard g, AccessOrCallExpr sub, GuardValue v) {
11511131
isGuardedByExpr0(guarded, g, sub, v) and
1152-
forall(ControlFlowNode subCfn, Ssa::Definition def |
1132+
forall(ControlFlowNode subCfn, SsaDefinition def |
11531133
nodeIsGuardedBySameSubExprSsaDef(_, guarded, g, subCfn, sub, v, def)
11541134
|
11551135
def = guarded.getAnSsaQualifier(_)
@@ -1161,7 +1141,7 @@ module Internal {
11611141
ControlFlowNodes::ElementNode guarded, Guard g, AccessOrCallExpr sub, GuardValue v
11621142
) {
11631143
nodeIsGuardedBySameSubExpr(guarded, _, _, g, sub, v) and
1164-
forall(ControlFlowNode subCfn, Ssa::Definition def |
1144+
forall(ControlFlowNode subCfn, SsaDefinition def |
11651145
nodeIsGuardedBySameSubExprSsaDef(guarded, _, g, subCfn, sub, v, def)
11661146
|
11671147
def =

csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll

Lines changed: 35 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ class AlwaysNullExpr extends Expr {
6767
exists(AlwaysNullExpr e1, AlwaysNullExpr e2 | G::Internal::nullValueImpliedBinary(e1, e2, this))
6868
or
6969
this =
70-
any(Ssa::Definition def |
71-
forex(Ssa::Definition u | u = def.getAnUltimateDefinition() | nullDef(u))
70+
any(SsaDefinition def |
71+
forex(SsaDefinition u | u = def.getAnUltimateDefinition() | nullDef(u))
7272
).getARead()
7373
or
7474
exists(Callable target |
@@ -80,9 +80,7 @@ class AlwaysNullExpr extends Expr {
8080
}
8181

8282
/** Holds if SSA definition `def` is always `null`. */
83-
private predicate nullDef(Ssa::ExplicitDefinition def) {
84-
def.getADefinition().getSource() instanceof AlwaysNullExpr
85-
}
83+
private predicate nullDef(SsaExplicitWrite def) { def.getValue() instanceof AlwaysNullExpr }
8684

8785
/** An expression that is never `null`. */
8886
class NonNullExpr extends Expr {
@@ -94,8 +92,8 @@ class NonNullExpr extends Expr {
9492
this instanceof G::NullGuardedExpr
9593
or
9694
this =
97-
any(Ssa::Definition def |
98-
forex(Ssa::Definition u | u = def.getAnUltimateDefinition() | nonNullDef(u))
95+
any(SsaDefinition def |
96+
forex(SsaDefinition u | u = def.getAnUltimateDefinition() | nonNullDef(u))
9997
).getARead()
10098
or
10199
exists(Callable target |
@@ -108,10 +106,10 @@ class NonNullExpr extends Expr {
108106
}
109107

110108
/** Holds if SSA definition `def` is never `null`. */
111-
private predicate nonNullDef(Ssa::ExplicitDefinition def) {
112-
def.getADefinition().getSource() instanceof NonNullExpr
109+
private predicate nonNullDef(SsaExplicitWrite def) {
110+
def.getValue() instanceof NonNullExpr
113111
or
114-
exists(AssignableDefinition ad | ad = def.getADefinition() |
112+
exists(AssignableDefinition ad | ad = def.getDefinition() |
115113
ad instanceof AssignableDefinitions::PatternDefinition
116114
or
117115
ad =
@@ -124,13 +122,11 @@ private predicate nonNullDef(Ssa::ExplicitDefinition def) {
124122
}
125123

126124
/**
127-
* Holds if `node` is a dereference `d` of SSA definition `def`.
125+
* Holds if `d` is a dereference of SSA definition `def`.
128126
*/
129-
private predicate dereferenceAt(ControlFlowNode node, Ssa::Definition def, Dereference d) {
130-
d = def.getAReadAtNode(node)
131-
}
127+
private predicate dereferenceAt(SsaDefinition def, Dereference d) { d = def.getARead() }
132128

133-
private predicate isMaybeNullArgument(Ssa::ParameterDefinition def, MaybeNullExpr arg) {
129+
private predicate isMaybeNullArgument(SsaParameterInit def, MaybeNullExpr arg) {
134130
exists(AssignableDefinitions::ImplicitParameterDefinition pdef, Parameter p |
135131
p = def.getParameter()
136132
|
@@ -182,18 +178,18 @@ private predicate hasMultipleParamsArguments(Call c) {
182178
}
183179

184180
/** Holds if `def` is an SSA definition that may be `null`. */
185-
private predicate defMaybeNull(Ssa::Definition def, ControlFlowNode node, string msg, Element reason) {
181+
private predicate defMaybeNull(SsaDefinition def, ControlFlowNode node, string msg, Element reason) {
186182
not nonNullDef(def) and
187183
(
188184
// A variable compared to `null` might be `null`
189185
exists(G::DereferenceableExpr de | de = def.getARead() |
190186
de.guardSuggestsMaybeNull(reason) and
191187
msg = "as suggested by $@ null check" and
192188
node = def.getControlFlowNode() and
193-
not de = any(Ssa::PhiNode phi).getARead() and
189+
not de = any(SsaPhiDefinition phi).getARead() and
194190
// Don't use a check as reason if there is a `null` assignment
195191
// or argument
196-
not def.(Ssa::ExplicitDefinition).getADefinition().getSource() instanceof MaybeNullExpr and
192+
not def.(SsaExplicitWrite).getValue() instanceof MaybeNullExpr and
197193
not isMaybeNullArgument(def, _)
198194
)
199195
or
@@ -207,41 +203,41 @@ private predicate defMaybeNull(Ssa::Definition def, ControlFlowNode node, string
207203
)
208204
or
209205
// If the source of a variable is `null` then the variable may be `null`
210-
exists(AssignableDefinition adef | adef = def.(Ssa::ExplicitDefinition).getADefinition() |
206+
exists(AssignableDefinition adef | adef = def.(SsaExplicitWrite).getDefinition() |
211207
adef.getSource() = maybeNullExpr(node.asExpr()) and
212208
reason = adef.getExpr() and
213209
msg = "because of $@ assignment"
214210
)
215211
or
216212
// A variable of nullable type may be null
217-
exists(Dereference d | dereferenceAt(_, def, d) |
213+
exists(Dereference d | dereferenceAt(def, d) |
218214
node = def.getControlFlowNode() and
219215
d.hasNullableType() and
220-
not def instanceof Ssa::PhiNode and
216+
not def instanceof SsaPhiDefinition and
221217
reason = def.getSourceVariable().getAssignable() and
222218
msg = "because it has a nullable type"
223219
)
224220
)
225221
}
226222

227-
private Ssa::Definition getAPseudoInput(Ssa::Definition def) {
228-
result = def.(Ssa::PhiNode).getAnInput()
223+
private SsaDefinition getAPseudoInput(SsaDefinition def) {
224+
result = def.(SsaPhiDefinition).getAnInput()
229225
}
230226

231227
// `def.getAnUltimateDefinition()` includes inputs into uncertain
232228
// definitions, but we only want inputs into pseudo nodes
233-
private Ssa::Definition getAnUltimateDefinition(Ssa::Definition def) {
229+
private SsaDefinition getAnUltimateDefinition(SsaDefinition def) {
234230
result = getAPseudoInput*(def) and
235-
not result instanceof Ssa::PhiNode
231+
not result instanceof SsaPhiDefinition
236232
}
237233

238234
/**
239235
* Holds if SSA definition `def` can reach a read at `cfn`, without passing
240236
* through an intermediate dereference that always throws a null reference
241237
* exception.
242238
*/
243-
private predicate defReaches(Ssa::Definition def, ControlFlowNode cfn) {
244-
exists(def.getAFirstReadAtNode(cfn))
239+
private predicate defReaches(SsaDefinition def, ControlFlowNode cfn) {
240+
Ssa::ssaGetAFirstUse(def).getControlFlowNode() = cfn
245241
or
246242
exists(ControlFlowNode mid | defReaches(def, mid) |
247243
SsaImpl::adjacentReadPairSameVar(_, mid, cfn) and
@@ -250,11 +246,12 @@ private predicate defReaches(Ssa::Definition def, ControlFlowNode cfn) {
250246
}
251247

252248
private module NullnessConfig implements ControlFlowReachability::ConfigSig {
253-
predicate source(ControlFlowNode node, Ssa::Definition def) { defMaybeNull(def, node, _, _) }
249+
predicate source(ControlFlowNode node, SsaDefinition def) { defMaybeNull(def, node, _, _) }
254250

255-
predicate sink(ControlFlowNode node, Ssa::Definition def) {
251+
predicate sink(ControlFlowNode node, SsaDefinition def) {
256252
exists(Dereference d |
257-
dereferenceAt(node, def, d) and
253+
dereferenceAt(def, d) and
254+
node = d.getControlFlowNode() and
258255
not d instanceof NonNullExpr
259256
)
260257
}
@@ -267,11 +264,12 @@ private module NullnessConfig implements ControlFlowReachability::ConfigSig {
267264
private module NullnessFlow = ControlFlowReachability::Flow<NullnessConfig>;
268265

269266
predicate maybeNullDeref(Dereference d, Ssa::SourceVariable v, string msg, Element reason) {
270-
exists(Ssa::Definition origin, Ssa::Definition ssa, ControlFlowNode src, ControlFlowNode sink |
267+
exists(SsaDefinition origin, SsaDefinition ssa, ControlFlowNode src, ControlFlowNode sink |
271268
defMaybeNull(origin, src, msg, reason) and
272269
NullnessFlow::flow(src, origin, sink, ssa) and
273270
ssa.getSourceVariable() = v and
274-
dereferenceAt(sink, ssa, d) and
271+
dereferenceAt(ssa, d) and
272+
sink = d.getControlFlowNode() and
275273
not d.isAlwaysNull(v)
276274
)
277275
}
@@ -322,9 +320,7 @@ class Dereference extends G::DereferenceableExpr {
322320
not p.getAnnotatedType().isNullableRefType()
323321
or
324322
p.fromSource() and
325-
exists(
326-
Ssa::ParameterDefinition def, AssignableDefinitions::ImplicitParameterDefinition pdef
327-
|
323+
exists(SsaParameterInit def, AssignableDefinitions::ImplicitParameterDefinition pdef |
328324
p = def.getParameter()
329325
|
330326
p.getUnboundDeclaration() = pdef.getParameter() and
@@ -334,9 +330,9 @@ class Dereference extends G::DereferenceableExpr {
334330
)
335331
}
336332

337-
private predicate isAlwaysNull0(Ssa::Definition def) {
338-
forall(Ssa::Definition input | input = getAnUltimateDefinition(def) |
339-
input.(Ssa::ExplicitDefinition).getADefinition().getSource() instanceof AlwaysNullExpr
333+
private predicate isAlwaysNull0(SsaDefinition def) {
334+
forall(SsaDefinition input | input = getAnUltimateDefinition(def) |
335+
input.(SsaExplicitWrite).getValue() instanceof AlwaysNullExpr
340336
) and
341337
not nonNullDef(def) and
342338
this = def.getARead() and
@@ -352,7 +348,7 @@ class Dereference extends G::DereferenceableExpr {
352348
// Exclude fields and properties, as they may not have an accurate SSA representation
353349
v.getAssignable() instanceof LocalScopeVariable and
354350
(
355-
forex(Ssa::Definition def0 | this = def0.getARead() | this.isAlwaysNull0(def0))
351+
forex(SsaDefinition def0 | this = def0.getARead() | this.isAlwaysNull0(def0))
356352
or
357353
exists(G::GuardValue nv |
358354
this.(G::GuardedExpr).mustHaveValue(nv) and

0 commit comments

Comments
 (0)