Skip to content

Commit f70168b

Browse files
hvitvedhvitved
committed
C#: Use parameter CFG nodes in SSA
1 parent af98779 commit f70168b

21 files changed

Lines changed: 154 additions & 61 deletions

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

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ private class RefArg extends AssignableAccess {
204204
AssignableDefinition getAnAnalyzableRefDef(Parameter p) {
205205
this.isAnalyzable(p) and
206206
result.getTarget() = p and
207-
not result = TImplicitParameterDefinition(_)
207+
not result = TImplicitParameterDefinition(_, _)
208208
}
209209

210210
/**
@@ -271,6 +271,11 @@ module AssignableInternal {
271271
def = TPatternDefinition(result)
272272
or
273273
def = TAssignOperationDefinition(result)
274+
or
275+
exists(Parameter p |
276+
def = TImplicitParameterDefinition(p, true) and
277+
result = p.getDefaultValue()
278+
)
274279
}
275280

276281
/** A local variable declaration at the top-level of a pattern. */
@@ -304,12 +309,18 @@ module AssignableInternal {
304309
not lvde instanceof TopLevelPatternDecl and
305310
not lvde.isOutArgument()
306311
} or
307-
TImplicitParameterDefinition(Parameter p) {
312+
TImplicitParameterDefinition(Parameter p, boolean isDefault) {
308313
exists(Callable c | p = c.getAParameter() |
309314
c.hasBody()
310315
or
311316
// Same as `c.(Constructor).hasInitializer()`, but avoids negative recursion warning
312317
c.getAChildExpr() instanceof @constructor_init_expr
318+
) and
319+
(
320+
isDefault = false
321+
or
322+
p.hasDefaultValue() and
323+
isDefault = true
313324
)
314325
} or
315326
TAddressOfDefinition(AddressOfExpr aoe) or
@@ -349,6 +360,8 @@ module AssignableInternal {
349360
any(AssignableDefinitions::PatternDefinition pd | result = pd.getDeclaration().getVariable())
350361
or
351362
def = any(AssignableDefinitions::InitializerDefinition init | result = init.getAssignable())
363+
or
364+
def = TImplicitParameterDefinition(result, _)
352365
}
353366

354367
// Not defined by dispatch in order to avoid too conservative negative recursion error
@@ -492,11 +505,6 @@ class AssignableDefinition extends TAssignableDefinition {
492505
exists(Ssa::ExplicitDefinition def | result = def.getAFirstReadAtNode(cfn) |
493506
this = def.getADefinition()
494507
)
495-
or
496-
exists(Ssa::ImplicitParameterDefinition def | result = def.getAFirstReadAtNode(cfn) |
497-
this.(AssignableDefinitions::ImplicitParameterDefinition).getParameter() =
498-
def.getParameter()
499-
)
500508
)
501509
}
502510

@@ -673,7 +681,7 @@ module AssignableDefinitions {
673681
class ImplicitParameterDefinition extends AssignableDefinition, TImplicitParameterDefinition {
674682
Parameter p;
675683

676-
ImplicitParameterDefinition() { this = TImplicitParameterDefinition(p) }
684+
ImplicitParameterDefinition() { this = TImplicitParameterDefinition(p, false) }
677685

678686
/** Gets the underlying parameter. */
679687
Parameter getParameter() { result = p }
@@ -688,7 +696,32 @@ module AssignableDefinitions {
688696

689697
override string toString() { result = p.toString() }
690698

691-
override Location getLocation() { result = this.getTarget().getLocation() }
699+
override Location getLocation() { result = p.getLocation() }
700+
}
701+
702+
/**
703+
* A default value assigned to a parameter.
704+
*/
705+
class ParameterDefaultDefinition extends AssignableDefinition, TImplicitParameterDefinition {
706+
Parameter p;
707+
708+
ParameterDefaultDefinition() { this = TImplicitParameterDefinition(p, true) }
709+
710+
/** Gets the underlying parameter. */
711+
Parameter getParameter() { result = p }
712+
713+
/** Gets the default value expression for the parameter. */
714+
Expr getDefaultValue() { result = p.getDefaultValue() }
715+
716+
override Expr getSource() { result = p.getDefaultValue() }
717+
718+
override Expr getElement() { result = p.getDefaultValue() }
719+
720+
override Callable getEnclosingCallable() { result = p.getCallable() }
721+
722+
override string toString() { result = p.toString() + " = ..." }
723+
724+
override Location getLocation() { result = p.getDefaultValue().getLocation() }
692725
}
693726

694727
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ private module LogicInput implements GuardsImpl::LogicInputSig {
205205
}
206206
}
207207

208-
class SsaParameterInit extends SsaDefinition instanceof Ssa::ImplicitParameterDefinition {
208+
class SsaParameterInit extends SsaDefinition instanceof Ssa::ParameterDefinition {
209209
Parameter getParameter() { result = super.getParameter() }
210210
}
211211

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ private predicate dereferenceAt(ControlFlowNode node, Ssa::Definition def, Deref
130130
d = def.getAReadAtNode(node)
131131
}
132132

133-
private predicate isMaybeNullArgument(Ssa::ImplicitParameterDefinition def, MaybeNullExpr arg) {
133+
private predicate isMaybeNullArgument(Ssa::ParameterDefinition def, MaybeNullExpr arg) {
134134
exists(AssignableDefinitions::ImplicitParameterDefinition pdef, Parameter p |
135135
p = def.getParameter()
136136
|
@@ -181,7 +181,7 @@ private predicate hasMultipleParamsArguments(Call c) {
181181
)
182182
}
183183

184-
private predicate isNullDefaultArgument(Ssa::ImplicitParameterDefinition def, AlwaysNullExpr arg) {
184+
private predicate isNullDefaultArgument(Ssa::ParameterDefinition def, AlwaysNullExpr arg) {
185185
exists(AssignableDefinitions::ImplicitParameterDefinition pdef, Parameter p |
186186
p = def.getParameter()
187187
|
@@ -337,8 +337,7 @@ class Dereference extends G::DereferenceableExpr {
337337
or
338338
p.fromSource() and
339339
exists(
340-
Ssa::ImplicitParameterDefinition def,
341-
AssignableDefinitions::ImplicitParameterDefinition pdef
340+
Ssa::ParameterDefinition def, AssignableDefinitions::ImplicitParameterDefinition pdef
342341
|
343342
p = def.getParameter()
344343
|

‎csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll‎

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -482,8 +482,8 @@ module Ssa {
482482

483483
/**
484484
* An SSA definition representing the implicit initialization of a variable
485-
* at the beginning of a callable. Either a parameter, a local scope variable
486-
* captured by the callable, or a field or property accessed inside the callable.
485+
* at the beginning of a callable. Either a local scope variable captured by
486+
* the callable or a field or property accessed inside the callable.
487487
*/
488488
class ImplicitEntryDefinition extends ImplicitDefinition {
489489
ImplicitEntryDefinition() {
@@ -507,51 +507,42 @@ module Ssa {
507507
override Location getLocation() { result = this.getCallable().getLocation() }
508508
}
509509

510-
private module NearestLocationInput implements NearestLocationInputSig {
511-
class C = ImplicitParameterDefinition;
512-
513-
predicate relevantLocations(ImplicitParameterDefinition def, Location l1, Location l2) {
514-
not def.getBasicBlock() instanceof EntryBasicBlock and
515-
l1 = def.getParameter().getALocation() and
516-
l2 = def.getBasicBlock().getLocation()
517-
}
518-
}
519-
520-
pragma[nomagic]
521-
private predicate implicitEntryDef(ImplicitEntryDefinition def, SourceVariable v, Callable c) {
522-
v = def.getSourceVariable() and
523-
c = def.getCallable()
524-
}
510+
deprecated class ImplicitParameterDefinition = ParameterDefinition;
525511

526512
/**
527513
* An SSA definition representing the implicit initialization of a parameter
528514
* at the beginning of a callable.
529515
*/
530-
class ImplicitParameterDefinition extends ImplicitEntryDefinition {
516+
class ParameterDefinition extends ExplicitDefinition {
531517
private Parameter p;
532518

533-
ImplicitParameterDefinition() {
534-
exists(SourceVariable sv, Callable c |
535-
implicitEntryDef(this, sv, c) and
536-
localScopeSourceVariable(sv, p, _, c)
537-
)
519+
ParameterDefinition() {
520+
p = ad.(AssignableDefinitions::ImplicitParameterDefinition).getParameter()
538521
}
539522

540523
/** Gets the parameter that this entry definition represents. */
541524
Parameter getParameter() { result = p }
542525

543-
override Element getElement() { result = this.getParameter() }
544-
545526
override string toString() {
546527
result = "SSA param(" + pragma[only_bind_out](this.getParameter()) + ")"
547528
}
529+
}
548530

549-
override Location getLocation() {
550-
not NearestLocation<NearestLocationInput>::nearestLocation(this, _, _) and
551-
result = p.getLocation()
552-
or
553-
// multi-bodied method: use matching parameter location
554-
NearestLocation<NearestLocationInput>::nearestLocation(this, result, _)
531+
/**
532+
* An SSA definition representing the default value of a parameter.
533+
*/
534+
class ParameterDefaultDefinition extends ExplicitDefinition {
535+
private Parameter p;
536+
537+
ParameterDefaultDefinition() {
538+
p = ad.(AssignableDefinitions::ParameterDefaultDefinition).getParameter()
539+
}
540+
541+
/** Gets the parameter that this entry definition represents. */
542+
Parameter getParameter() { result = p }
543+
544+
override string toString() {
545+
result = "SSA param_default(" + pragma[only_bind_out](this.getParameter()) + ")"
555546
}
556547
}
557548

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1288,7 +1288,7 @@ private module NearestLocationInputParamAfterCallable implements NearestLocation
12881288

12891289
private module ParameterNodes {
12901290
pragma[nomagic]
1291-
private predicate ssaParamDef(Ssa::ImplicitParameterDefinition ssaDef, Parameter p, Location l) {
1291+
private predicate ssaParamDef(Ssa::ParameterDefinition ssaDef, Parameter p, Location l) {
12921292
p = ssaDef.getParameter() and
12931293
l = ssaDef.getLocation()
12941294
}
@@ -1343,7 +1343,7 @@ private module ParameterNodes {
13431343
}
13441344

13451345
/** Gets the SSA definition corresponding to this parameter, if any. */
1346-
Ssa::ImplicitParameterDefinition getSsaDefinition() {
1346+
Ssa::ParameterDefinition getSsaDefinition() {
13471347
exists(Parameter p, Location l |
13481348
l = this.getParameterLocation(p) and
13491349
ssaParamDef(result, p, l)

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ private module SourceVariableImpl {
126126
*/
127127
predicate variableDefinition(BasicBlock bb, int i, Ssa::SourceVariable v, AssignableDefinition ad) {
128128
ad = v.getADefinition() and
129-
ad.getExpr().getControlFlowNode() = bb.getNode(i) and
130129
// In cases like `(x, x) = (0, 1)`, we discard the first (dead) definition of `x`
131130
not exists(TupleAssignmentDefinition first, TupleAssignmentDefinition second | first = ad |
132131
second.getAssignment() = first.getAssignment() and
@@ -136,7 +135,13 @@ private module SourceVariableImpl {
136135
// In cases like `M(out x, out x)`, there is no inherent evaluation order, so we
137136
// collapse the two definitions of `x`, using the first access as the representative,
138137
// and expose both definitions in `ExplicitDefinition.getADefinition()`
139-
not ad = getASameOutRefDefAfter(v, _)
138+
not ad = getASameOutRefDefAfter(v, _) and
139+
(
140+
ad.getExpr().getControlFlowNode() = bb.getNode(i)
141+
or
142+
ad.(AssignableDefinitions::ImplicitParameterDefinition).getParameter().getControlFlowNode() =
143+
bb.getNode(i)
144+
)
140145
}
141146

142147
/**
@@ -771,8 +776,6 @@ private module Cached {
771776
or
772777
// Each tracked field and property has an implicit entry definition
773778
v instanceof PlainFieldOrPropSourceVariable
774-
or
775-
v.getAssignable() instanceof Parameter
776779
)
777780
}
778781

@@ -963,7 +966,7 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
963966
predicate ssaDefHasSource(WriteDefinition def) {
964967
// exclude flow directly from RHS to SSA definition, as we instead want to
965968
// go from RHS to matching assignable definition, and from there to SSA definition
966-
def instanceof Ssa::ImplicitParameterDefinition
969+
def instanceof Ssa::ParameterDefinition
967970
}
968971

969972
/**

‎csharp/ql/test/library-tests/csharp7/DefUse.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ where
66
(
77
ult.(Ssa::ExplicitDefinition).getADefinition() = def
88
or
9-
ult.(Ssa::ImplicitParameterDefinition).getParameter() =
9+
ult.(Ssa::ParameterDefinition).getParameter() =
1010
def.(AssignableDefinitions::ImplicitParameterDefinition).getParameter()
1111
) and
1212
read = ssaDef.getARead()

‎csharp/ql/test/library-tests/dataflow/defuse/parameterUseEquivalence.ql‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@ private LocalScopeVariableRead getAReachableUncertainRead(
2424
AssignableDefinitions::ImplicitParameterDefinition p
2525
) {
2626
exists(Ssa::Definition ssaDef |
27-
p.getParameter() =
28-
ssaDef.getAnUltimateDefinition().(Ssa::ImplicitParameterDefinition).getParameter()
27+
p.getParameter() = ssaDef.getAnUltimateDefinition().(Ssa::ParameterDefinition).getParameter()
2928
|
3029
result = ssaDef.getARead()
3130
)

‎csharp/ql/test/library-tests/dataflow/local/DataFlow.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
| LocalDataFlow.cs:315:15:315:20 | access to local variable sink73 |
66
| LocalDataFlow.cs:316:15:316:20 | access to local variable sink74 |
77
| LocalDataFlow.cs:342:15:342:21 | access to parameter tainted |
8+
| LocalDataFlow.cs:387:15:387:15 | access to parameter s |
89
| SSA.cs:9:15:9:22 | access to local variable ssaSink0 |
910
| SSA.cs:25:15:25:22 | access to local variable ssaSink1 |
1011
| SSA.cs:43:15:43:22 | access to local variable ssaSink2 |

‎csharp/ql/test/library-tests/dataflow/local/DataFlowStep.expected‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,9 @@
529529
| LocalDataFlow.cs:381:17:381:29 | "not tainted" | LocalDataFlow.cs:381:13:381:13 | access to local variable x |
530530
| LocalDataFlow.cs:385:34:385:34 | SSA param(s) | LocalDataFlow.cs:387:15:387:15 | access to parameter s |
531531
| LocalDataFlow.cs:385:34:385:34 | s | LocalDataFlow.cs:385:34:385:34 | SSA param(s) |
532+
| LocalDataFlow.cs:385:38:385:51 | "taint source" | LocalDataFlow.cs:385:38:385:51 | s = ... |
533+
| LocalDataFlow.cs:385:38:385:51 | SSA param_default(s) | LocalDataFlow.cs:387:15:387:15 | access to parameter s |
534+
| LocalDataFlow.cs:385:38:385:51 | s = ... | LocalDataFlow.cs:385:38:385:51 | SSA param_default(s) |
532535
| SSA.cs:3:14:3:16 | this | SSA.cs:3:14:3:16 | this access |
533536
| SSA.cs:5:17:5:17 | SSA entry def(this.S) | SSA.cs:67:9:67:14 | access to field S |
534537
| SSA.cs:5:17:5:17 | this | SSA.cs:67:9:67:12 | this access |

0 commit comments

Comments
 (0)