Skip to content

Commit d675304

Browse files
hvitvedhvitved
authored
Merge pull request #16875 from hvitved/csharp/ssa-param-def
C#: Move implicit entry definitions inside method bodies in SSA construction
2 parents 456c649 + c5c97ac commit d675304

31 files changed

+326
-137
lines changed

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -393,14 +393,18 @@ private import AssignableInternal
393393
*/
394394
class AssignableDefinition extends TAssignableDefinition {
395395
/**
396+
* DEPRECATED: Use `this.getExpr().getAControlFlowNode()` instead.
397+
*
396398
* Gets a control flow node that updates the targeted assignable when
397399
* reached.
398400
*
399401
* Multiple definitions may relate to the same control flow node. For example,
400402
* the definitions of `x` and `y` in `M(out x, out y)` and `(x, y) = (0, 1)`
401403
* relate to the same call to `M` and assignment node, respectively.
402404
*/
403-
ControlFlow::Node getAControlFlowNode() { result = this.getExpr().getAControlFlowNode() }
405+
deprecated ControlFlow::Node getAControlFlowNode() {
406+
result = this.getExpr().getAControlFlowNode()
407+
}
404408

405409
/**
406410
* Gets the underlying expression that updates the targeted assignable when
@@ -477,6 +481,11 @@ class AssignableDefinition extends TAssignableDefinition {
477481
exists(Ssa::ExplicitDefinition def | result = def.getAFirstReadAtNode(cfn) |
478482
this = def.getADefinition()
479483
)
484+
or
485+
exists(Ssa::ImplicitParameterDefinition def | result = def.getAFirstReadAtNode(cfn) |
486+
this.(AssignableDefinitions::ImplicitParameterDefinition).getParameter() =
487+
def.getParameter()
488+
)
480489
)
481490
}
482491

@@ -550,7 +559,7 @@ module AssignableDefinitions {
550559
ControlFlow::BasicBlock bb, Parameter p, AssignableDefinition def
551560
) {
552561
def = any(RefArg arg).getAnAnalyzableRefDef(p) and
553-
bb.getANode() = def.getAControlFlowNode()
562+
bb.getANode() = def.getExpr().getAControlFlowNode()
554563
}
555564

556565
/**
@@ -662,7 +671,9 @@ module AssignableDefinitions {
662671
/** Gets the underlying parameter. */
663672
Parameter getParameter() { result = p }
664673

665-
override ControlFlow::Node getAControlFlowNode() { result = p.getCallable().getEntryPoint() }
674+
deprecated override ControlFlow::Node getAControlFlowNode() {
675+
result = p.getCallable().getEntryPoint()
676+
}
666677

667678
override Parameter getElement() { result = p }
668679

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66

77
import csharp
8+
private import internal.Location
89

910
/**
1011
* INTERNAL: Do not use.
@@ -65,14 +66,6 @@ private ControlFlowElement getBody(Callable c) {
6566
result = getStatementBody(c)
6667
}
6768

68-
pragma[nomagic]
69-
private SourceLocation getASourceLocation(Element e) {
70-
result = e.getALocation().(SourceLocation) and
71-
not exists(e.getALocation().(SourceLocation).getMappedLocation())
72-
or
73-
result = e.getALocation().(SourceLocation).getMappedLocation()
74-
}
75-
7669
pragma[nomagic]
7770
private predicate hasNoSourceLocation(Element e) { not exists(getASourceLocation(e)) }
7871

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,27 @@ class Location extends @location {
5050

5151
/** Gets the 1-based column number (inclusive) where this location ends. */
5252
final int getEndColumn() { this.hasLocationInfo(_, _, _, _, result) }
53+
54+
/** Holds if this location starts strictly before the specified location. */
55+
bindingset[this, other]
56+
pragma[inline_late]
57+
predicate strictlyBefore(Location other) {
58+
this.getFile() = other.getFile() and
59+
(
60+
this.getStartLine() < other.getStartLine()
61+
or
62+
this.getStartLine() = other.getStartLine() and this.getStartColumn() < other.getStartColumn()
63+
)
64+
}
65+
66+
/** Holds if this location starts before the specified location. */
67+
bindingset[this, other]
68+
pragma[inline_late]
69+
predicate before(Location other) {
70+
this.getStartLine() < other.getStartLine()
71+
or
72+
this.getStartLine() = other.getStartLine() and this.getStartColumn() <= other.getStartColumn()
73+
}
5374
}
5475

5576
/** An empty location. */

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,9 @@ private ControlFlowElement getANullCheck(
157157
exists(Expr e, G::AbstractValue v | v.branch(result, s, e) | exprImpliesSsaDef(e, v, def, nv))
158158
}
159159

160-
private predicate isMaybeNullArgument(Ssa::ExplicitDefinition def, MaybeNullExpr arg) {
160+
private predicate isMaybeNullArgument(Ssa::ImplicitParameterDefinition def, MaybeNullExpr arg) {
161161
exists(AssignableDefinitions::ImplicitParameterDefinition pdef, Parameter p |
162-
pdef = def.getADefinition()
162+
p = def.getParameter()
163163
|
164164
p = pdef.getParameter().getUnboundDeclaration() and
165165
arg = p.getAnAssignedArgument() and
@@ -208,9 +208,9 @@ private predicate hasMultipleParamsArguments(Call c) {
208208
)
209209
}
210210

211-
private predicate isNullDefaultArgument(Ssa::ExplicitDefinition def, AlwaysNullExpr arg) {
211+
private predicate isNullDefaultArgument(Ssa::ImplicitParameterDefinition def, AlwaysNullExpr arg) {
212212
exists(AssignableDefinitions::ImplicitParameterDefinition pdef, Parameter p |
213-
pdef = def.getADefinition()
213+
p = def.getParameter()
214214
|
215215
p = pdef.getParameter().getUnboundDeclaration() and
216216
arg = p.getDefaultValue() and
@@ -543,9 +543,10 @@ class Dereference extends G::DereferenceableExpr {
543543
p.fromSource() // assume all non-source extension methods perform a dereference
544544
implies
545545
exists(
546-
Ssa::ExplicitDefinition def, AssignableDefinitions::ImplicitParameterDefinition pdef
546+
Ssa::ImplicitParameterDefinition def,
547+
AssignableDefinitions::ImplicitParameterDefinition pdef
547548
|
548-
pdef = def.getADefinition()
549+
p = def.getParameter()
549550
|
550551
p.getUnboundDeclaration() = pdef.getParameter() and
551552
def.getARead() instanceof Dereference

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

Lines changed: 73 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,24 @@ import csharp
99
*/
1010
module Ssa {
1111
private import internal.SsaImpl as SsaImpl
12+
private import semmle.code.csharp.internal.Location
13+
14+
pragma[nomagic]
15+
private predicate assignableDefinitionLocalScopeVariable(
16+
AssignableDefinition ad, LocalScopeVariable v, Callable c
17+
) {
18+
ad.getTarget() = v and
19+
ad.getEnclosingCallable() = c
20+
}
21+
22+
pragma[nomagic]
23+
private predicate localScopeSourceVariable(
24+
SourceVariables::LocalScopeSourceVariable sv, LocalScopeVariable v, Callable c1, Callable c2
25+
) {
26+
sv.getAssignable() = v and
27+
sv.getEnclosingCallable() = c1 and
28+
v.getCallable() = c2
29+
}
1230

1331
/**
1432
* A variable that can be SSA converted.
@@ -34,11 +52,10 @@ module Ssa {
3452
or
3553
// Local variable declaration without initializer
3654
not exists(result.getTargetAccess()) and
37-
this =
38-
any(SourceVariables::LocalScopeSourceVariable v |
39-
result.getTarget() = v.getAssignable() and
40-
result.getEnclosingCallable() = v.getEnclosingCallable()
41-
)
55+
exists(LocalScopeVariable v, Callable c |
56+
assignableDefinitionLocalScopeVariable(result, v, c) and
57+
localScopeSourceVariable(this, v, c, _)
58+
)
4259
}
4360

4461
/**
@@ -507,9 +524,7 @@ module Ssa {
507524
override Element getElement() { result = ad.getElement() }
508525

509526
override string toString() {
510-
if this.getADefinition() instanceof AssignableDefinitions::ImplicitParameterDefinition
511-
then result = SsaImpl::getToStringPrefix(this) + "SSA param(" + this.getSourceVariable() + ")"
512-
else result = SsaImpl::getToStringPrefix(this) + "SSA def(" + this.getSourceVariable() + ")"
527+
result = SsaImpl::getToStringPrefix(this) + "SSA def(" + this.getSourceVariable() + ")"
513528
}
514529

515530
override Location getLocation() { result = ad.getLocation() }
@@ -537,7 +552,7 @@ module Ssa {
537552

538553
/**
539554
* An SSA definition representing the implicit initialization of a variable
540-
* at the beginning of a callable. Either the variable is a local scope variable
555+
* at the beginning of a callable. Either a parameter, a local scope variable
541556
* captured by the callable, or a field or property accessed inside the callable.
542557
*/
543558
class ImplicitEntryDefinition extends ImplicitDefinition {
@@ -551,7 +566,7 @@ module Ssa {
551566
/** Gets the callable that this entry definition belongs to. */
552567
final Callable getCallable() { result = this.getBasicBlock().getCallable() }
553568

554-
override Callable getElement() { result = this.getCallable() }
569+
override Element getElement() { result = this.getCallable() }
555570

556571
override string toString() {
557572
if this.getSourceVariable().getAssignable() instanceof LocalScopeVariable
@@ -566,6 +581,54 @@ module Ssa {
566581
override Location getLocation() { result = this.getCallable().getLocation() }
567582
}
568583

584+
private module NearestLocationInput implements NearestLocationInputSig {
585+
class C = ImplicitParameterDefinition;
586+
587+
predicate relevantLocations(ImplicitParameterDefinition def, Location l1, Location l2) {
588+
not def.getBasicBlock() instanceof ControlFlow::BasicBlocks::EntryBlock and
589+
l1 = def.getParameter().getALocation() and
590+
l2 = def.getBasicBlock().getLocation()
591+
}
592+
}
593+
594+
pragma[nomagic]
595+
private predicate implicitEntryDef(ImplicitEntryDefinition def, SourceVariable v, Callable c) {
596+
v = def.getSourceVariable() and
597+
c = def.getCallable()
598+
}
599+
600+
/**
601+
* An SSA definition representing the implicit initialization of a parameter
602+
* at the beginning of a callable.
603+
*/
604+
class ImplicitParameterDefinition extends ImplicitEntryDefinition {
605+
private Parameter p;
606+
607+
ImplicitParameterDefinition() {
608+
exists(SourceVariable sv, Callable c |
609+
implicitEntryDef(this, sv, c) and
610+
localScopeSourceVariable(sv, p, _, c)
611+
)
612+
}
613+
614+
/** Gets the parameter that this entry definition represents. */
615+
Parameter getParameter() { result = p }
616+
617+
override Element getElement() { result = this.getParameter() }
618+
619+
override string toString() {
620+
result = SsaImpl::getToStringPrefix(this) + "SSA param(" + this.getParameter() + ")"
621+
}
622+
623+
override Location getLocation() {
624+
not NearestLocation<NearestLocationInput>::nearestLocation(this, _, _) and
625+
result = p.getLocation()
626+
or
627+
// multi-bodied method: use matching parameter location
628+
NearestLocation<NearestLocationInput>::nearestLocation(this, result, _)
629+
}
630+
}
631+
569632
/**
570633
* An SSA definition representing the potential definition of a variable
571634
* via a call.

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

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ module BaseSsa {
1414
private predicate definitionAt(
1515
AssignableDefinition def, ControlFlow::BasicBlock bb, int i, SsaInput::SourceVariable v
1616
) {
17-
bb.getNode(i) = def.getAControlFlowNode() and
17+
bb.getNode(i) = def.getExpr().getAControlFlowNode() and
1818
v = def.getTarget() and
1919
// In cases like `(x, x) = (0, 1)`, we discard the first (dead) definition of `x`
2020
not exists(TupleAssignmentDefinition first, TupleAssignmentDefinition second | first = def |
@@ -27,8 +27,19 @@ module BaseSsa {
2727
private predicate implicitEntryDef(
2828
Callable c, ControlFlow::BasicBlocks::EntryBlock bb, SsaInput::SourceVariable v
2929
) {
30-
v.isReadonlyCapturedBy(c) and
31-
c = bb.getCallable()
30+
exists(ControlFlow::ControlFlow::BasicBlocks::EntryBlock entry |
31+
c = entry.getCallable() and
32+
// In case `c` has multiple bodies, we want each body to gets its own implicit
33+
// entry definition. In case `c` doesn't have multiple bodies, the line below
34+
// is simply the same as `bb = entry`, because `entry.getFirstNode().getASuccessor()`
35+
// will be in the entry block.
36+
bb = entry.getFirstNode().getASuccessor().getBasicBlock() and
37+
c = v.getCallable()
38+
|
39+
v.isReadonlyCapturedBy(c)
40+
or
41+
v instanceof Parameter
42+
)
3243
}
3344

3445
private module SsaInput implements SsaImplCommon::InputSig<Location> {
@@ -85,6 +96,13 @@ module BaseSsa {
8596
)
8697
}
8798

99+
final predicate isImplicitEntryDefinition(SsaInput::SourceVariable v) {
100+
exists(ControlFlow::BasicBlock bb |
101+
this.definesAt(v, bb, -1) and
102+
implicitEntryDef(_, bb, v)
103+
)
104+
}
105+
88106
private Definition getAPhiInputOrPriorDefinition() {
89107
result = this.(PhiNode).getAnInput() or
90108
SsaImpl::uncertainWriteDefinitionInput(this, result)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ abstract class ControlFlowReachabilityConfiguration extends string {
199199
exists(ControlFlow::BasicBlock bb, boolean isSuccessor, int i, int j |
200200
this.reachesBasicBlockDefinitionBase(e, def, isSuccessor, cfn, i, bb) and
201201
cfnDef = bb.getNode(j) and
202-
def.getAControlFlowNode() = cfnDef
202+
def.getExpr().getAControlFlowNode() = cfnDef
203203
|
204204
isSuccessor = true and j >= i
205205
or
@@ -208,7 +208,7 @@ abstract class ControlFlowReachabilityConfiguration extends string {
208208
or
209209
exists(ControlFlow::BasicBlock bb |
210210
this.reachesBasicBlockDefinitionRec(e, def, _, cfn, bb) and
211-
def.getAControlFlowNode() = cfnDef and
211+
def.getExpr().getAControlFlowNode() = cfnDef and
212212
cfnDef = bb.getANode()
213213
)
214214
}

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

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ private ControlFlow::Node getAPrimaryConstructorParameterCfn(ParameterAccess pa)
4444
(
4545
result = pa.(ParameterRead).getAControlFlowNode()
4646
or
47-
pa = any(AssignableDefinition def | result = def.getAControlFlowNode()).getTargetAccess()
47+
pa =
48+
any(AssignableDefinition def | result = def.getExpr().getAControlFlowNode()).getTargetAccess()
4849
)
4950
}
5051

@@ -354,9 +355,7 @@ module VariableCapture {
354355

355356
VariableWrite() {
356357
def.getTarget() = v.asLocalScopeVariable() and
357-
this = def.getAControlFlowNode() and
358-
// the shared variable capture library inserts implicit parameter definitions
359-
not def instanceof AssignableDefinitions::ImplicitParameterDefinition
358+
this = def.getExpr().getAControlFlowNode()
360359
}
361360

362361
ControlFlow::Node getRhs() { LocalFlow::defAssigns(def, this, result) }
@@ -1100,13 +1099,10 @@ private module Cached {
11001099
TExprNode(ControlFlow::Nodes::ElementNode cfn) { cfn.getAstNode() instanceof Expr } or
11011100
TSsaDefinitionExtNode(SsaImpl::DefinitionExt def) {
11021101
// Handled by `TExplicitParameterNode` below
1103-
not def.(Ssa::ExplicitDefinition).getADefinition() instanceof
1104-
AssignableDefinitions::ImplicitParameterDefinition
1102+
not def instanceof Ssa::ImplicitParameterDefinition
11051103
} or
11061104
TAssignableDefinitionNode(AssignableDefinition def, ControlFlow::Node cfn) {
1107-
cfn = def.getAControlFlowNode() and
1108-
// Handled by `TExplicitParameterNode` below
1109-
not def instanceof AssignableDefinitions::ImplicitParameterDefinition
1105+
cfn = def.getExpr().getAControlFlowNode()
11101106
} or
11111107
TExplicitParameterNode(Parameter p) {
11121108
p = any(DataFlowCallable dfc).asCallable().getAParameter()
@@ -1353,10 +1349,7 @@ private module ParameterNodes {
13531349
ExplicitParameterNode() { this = TExplicitParameterNode(parameter) }
13541350

13551351
/** Gets the SSA definition corresponding to this parameter, if any. */
1356-
Ssa::ExplicitDefinition getSsaDefinition() {
1357-
result.getADefinition().(AssignableDefinitions::ImplicitParameterDefinition).getParameter() =
1358-
parameter
1359-
}
1352+
Ssa::ImplicitParameterDefinition getSsaDefinition() { result.getParameter() = parameter }
13601353

13611354
override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
13621355
c.asCallable().getParameter(pos.getPosition()) = parameter

0 commit comments

Comments
 (0)