Skip to content

Commit ea7d9c9

Browse files
hvitvedhvitved
committed
C#: Use separate newtype branch for AssignableDefinitionNode 
1 parent 7ce7b58 commit ea7d9c9

File tree

4 files changed

+81
-42
lines changed

4 files changed

+81
-42
lines changed

‎csharp/ql/consistency-queries/DataFlowConsistency.ql

+10-4
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,25 @@
11
import csharp
22
import cil
3+
private import semmle.code.csharp.controlflow.internal.ControlFlowGraphImpl as ControlFlowGraphImpl
34
private import semmle.code.csharp.dataflow.internal.DataFlowImplSpecific
45
private import semmle.code.csharp.dataflow.internal.TaintTrackingImplSpecific
56
private import codeql.dataflow.internal.DataFlowImplConsistency
67

78
private module Input implements InputSig<CsharpDataFlow> {
89
private import CsharpDataFlow
910

11+
private predicate isStaticAssignable(Assignable a) { a.(Modifiable).isStatic() }
12+
13+
predicate uniqueEnclosingCallableExclude(Node node) {
14+
// TODO: Remove once static initializers are folded into the
15+
// static constructors
16+
isStaticAssignable(ControlFlowGraphImpl::getNodeCfgScope(node.getControlFlowNode()))
17+
}
18+
1019
predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) {
1120
// TODO: Remove once static initializers are folded into the
1221
// static constructors
13-
exists(ControlFlow::Node cfn |
14-
cfn.getAstNode() = any(FieldOrProperty f | f.isStatic()).getAChild+() and
15-
cfn = call.getControlFlowNode()
16-
)
22+
isStaticAssignable(ControlFlowGraphImpl::getNodeCfgScope(call.getControlFlowNode()))
1723
}
1824

1925
predicate uniqueNodeLocationExclude(Node n) {

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

+4
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,8 @@ module CsharpDataFlow implements InputSig {
2424
predicate mayBenefitFromCallContext = Private::mayBenefitFromCallContext/1;
2525

2626
predicate viableImplInCallContext = Private::viableImplInCallContext/2;
27+
28+
predicate neverSkipInPathGraph(Node n) {
29+
exists(n.(AssignableDefinitionNode).getDefinition().getTargetAccess())
30+
}
2731
}

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

+64-30
Original file line numberDiff line numberDiff line change
@@ -208,16 +208,10 @@ predicate hasNodePath(ControlFlowReachabilityConfiguration conf, ExprNode n1, No
208208
cfn2 = n2.(ExprNode).getControlFlowNode()
209209
)
210210
or
211-
exists(
212-
ControlFlow::Node cfn, AssignableDefinition def, ControlFlow::Node cfnDef,
213-
Ssa::ExplicitDefinition ssaDef
214-
|
215-
conf.hasDefPath(_, cfn, def, cfnDef)
216-
|
211+
exists(ControlFlow::Node cfn, AssignableDefinition def, ControlFlow::Node cfnDef |
212+
conf.hasDefPath(_, cfn, def, cfnDef) and
217213
cfn = n1.getControlFlowNode() and
218-
ssaDef.getADefinition() = def and
219-
ssaDef.getControlFlowNode() = cfnDef and
220-
n2.(SsaDefinitionExtNode).getDefinitionExt() = ssaDef
214+
n2 = TAssignableDefinitionNode(def, cfnDef)
221215
)
222216
}
223217

@@ -544,6 +538,13 @@ module LocalFlow {
544538
ThisFlow::adjacentThisRefs(nodeFrom.(PostUpdateNode).getPreUpdateNode(), nodeTo)
545539
or
546540
CilFlow::localFlowStepCil(nodeFrom, nodeTo)
541+
or
542+
exists(AssignableDefinition def, ControlFlow::Node cfn, Ssa::ExplicitDefinition ssaDef |
543+
ssaDef.getADefinition() = def and
544+
ssaDef.getControlFlowNode() = cfn and
545+
nodeFrom = TAssignableDefinitionNode(def, cfn) and
546+
nodeTo.(SsaDefinitionExtNode).getDefinitionExt() = ssaDef
547+
)
547548
}
548549

549550
/**
@@ -599,7 +600,7 @@ module LocalFlow {
599600
or
600601
hasNodePath(any(LocalExprStepConfiguration x), node1, node2) and
601602
(
602-
node2 instanceof SsaDefinitionExtNode or
603+
node2 instanceof AssignableDefinitionNode or
603604
node2.asExpr() instanceof Cast or
604605
node2.asExpr() instanceof AssignExpr
605606
)
@@ -906,6 +907,11 @@ private module Cached {
906907
not def.(Ssa::ExplicitDefinition).getADefinition() instanceof
907908
AssignableDefinitions::ImplicitParameterDefinition
908909
} or
910+
TAssignableDefinitionNode(AssignableDefinition def, ControlFlow::Node cfn) {
911+
cfn = def.getAControlFlowNode() and
912+
// Handled by `TExplicitParameterNode` below
913+
not def instanceof AssignableDefinitions::ImplicitParameterDefinition
914+
} or
909915
TExplicitParameterNode(DotNet::Parameter p) {
910916
p = any(DataFlowCallable dfc).asCallable().getAParameter()
911917
} or
@@ -1044,15 +1050,7 @@ import Cached
10441050

10451051
/** Holds if `n` should be hidden from path explanations. */
10461052
predicate nodeIsHidden(Node n) {
1047-
exists(SsaImpl::DefinitionExt def | def = n.(SsaDefinitionExtNode).getDefinitionExt() |
1048-
def instanceof Ssa::PhiNode
1049-
or
1050-
def instanceof SsaImpl::PhiReadNode
1051-
or
1052-
def instanceof Ssa::ImplicitEntryDefinition
1053-
or
1054-
def instanceof Ssa::ImplicitCallDefinition
1055-
)
1053+
n instanceof SsaDefinitionExtNode
10561054
or
10571055
exists(Parameter p | p = n.(ParameterNode).getParameter() | not p.fromSource())
10581056
or
@@ -1080,6 +1078,8 @@ predicate nodeIsHidden(Node n) {
10801078
n instanceof InstanceParameterAccessNode
10811079
or
10821080
n instanceof PrimaryConstructorThisAccessNode
1081+
or
1082+
n = any(AssignableDefinitionNode def | not exists(def.getDefinition().getTargetAccess()))
10831083
}
10841084

10851085
/** A CIL SSA definition, viewed as a node in a data flow graph. */
@@ -1128,6 +1128,43 @@ class SsaDefinitionExtNode extends NodeImpl, TSsaDefinitionExtNode {
11281128
override string toStringImpl() { result = def.toString() }
11291129
}
11301130

1131+
/** A definition, viewed as a node in a data flow graph. */
1132+
class AssignableDefinitionNodeImpl extends NodeImpl, TAssignableDefinitionNode {
1133+
private AssignableDefinition def;
1134+
private ControlFlow::Node cfn_;
1135+
1136+
AssignableDefinitionNodeImpl() { this = TAssignableDefinitionNode(def, cfn_) }
1137+
1138+
/** Gets the underlying definition. */
1139+
AssignableDefinition getDefinition() { result = def }
1140+
1141+
/** Gets the underlying definition, at control flow node `cfn`, if any. */
1142+
AssignableDefinition getDefinitionAtNode(ControlFlow::Node cfn) {
1143+
result = def and
1144+
cfn = cfn_
1145+
}
1146+
1147+
override DataFlowCallable getEnclosingCallableImpl() {
1148+
result.asCallable() = cfn_.getEnclosingCallable()
1149+
}
1150+
1151+
override Type getTypeImpl() { result = def.getTarget().getType() }
1152+
1153+
override ControlFlow::Node getControlFlowNodeImpl() { result = cfn_ }
1154+
1155+
override Location getLocationImpl() {
1156+
result = def.getTargetAccess().getLocation()
1157+
or
1158+
not exists(def.getTargetAccess()) and result = def.getLocation()
1159+
}
1160+
1161+
override string toStringImpl() {
1162+
result = def.getTargetAccess().toString()
1163+
or
1164+
not exists(def.getTargetAccess()) and result = def.toString()
1165+
}
1166+
}
1167+
11311168
abstract class ParameterNodeImpl extends NodeImpl {
11321169
abstract predicate isParameterOf(DataFlowCallable c, ParameterPosition pos);
11331170
}
@@ -2021,12 +2058,11 @@ private PropertyContent getResultContent() {
20212058
}
20222059

20232060
private predicate primaryConstructorParameterStore(
2024-
SsaDefinitionExtNode node1, PrimaryConstructorParameterContent c, Node node2
2061+
AssignableDefinitionNode node1, PrimaryConstructorParameterContent c, Node node2
20252062
) {
2026-
exists(Ssa::ExplicitDefinition def, ControlFlow::Node cfn, Parameter p |
2027-
def = node1.getDefinitionExt() and
2028-
p = def.getSourceVariable().getAssignable() and
2029-
cfn = def.getControlFlowNode() and
2063+
exists(AssignableDefinition def, ControlFlow::Node cfn, Parameter p |
2064+
node1 = TAssignableDefinitionNode(def, cfn) and
2065+
p = def.getTarget() and
20302066
node2 = TInstanceParameterAccessNode(cfn, true) and
20312067
c.getParameter() = p
20322068
)
@@ -2193,18 +2229,16 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
21932229
hasNodePath(x, node1, node2)
21942230
or
21952231
// item = variable in node1 = (..., variable, ...)
2196-
exists(AssignableDefinitions::TupleAssignmentDefinition tad, Ssa::ExplicitDefinition def |
2197-
node2.(SsaDefinitionExtNode).getDefinitionExt() = def and
2198-
def.getADefinition() = tad and
2232+
exists(AssignableDefinitions::TupleAssignmentDefinition tad |
2233+
node2.(AssignableDefinitionNode).getDefinition() = tad and
21992234
tad.getLeaf() = item and
22002235
hasNodePath(x, node1, node2)
22012236
)
22022237
or
22032238
// item = variable in node1 = (..., variable, ...) in a case/is var (..., ...)
22042239
te = any(PatternExpr pe).getAChildExpr*() and
2205-
exists(AssignableDefinitions::LocalVariableDefinition lvd, Ssa::ExplicitDefinition def |
2206-
node2.(SsaDefinitionExtNode).getDefinitionExt() = def and
2207-
def.getADefinition() = lvd and
2240+
exists(AssignableDefinitions::LocalVariableDefinition lvd |
2241+
node2.(AssignableDefinitionNode).getDefinition() = lvd and
22082242
lvd.getDeclaration() = item and
22092243
hasNodePath(x, node1, node2)
22102244
)

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

+3-8
Original file line numberDiff line numberDiff line change
@@ -109,18 +109,13 @@ class ParameterNode extends Node instanceof ParameterNodeImpl {
109109
}
110110

111111
/** A definition, viewed as a node in a data flow graph. */
112-
class AssignableDefinitionNode extends Node, TSsaDefinitionExtNode {
113-
private Ssa::ExplicitDefinition edef;
114-
115-
AssignableDefinitionNode() { this = TSsaDefinitionExtNode(edef) }
116-
112+
class AssignableDefinitionNode extends Node instanceof AssignableDefinitionNodeImpl {
117113
/** Gets the underlying definition. */
118-
AssignableDefinition getDefinition() { result = this.getDefinitionAtNode(_) }
114+
AssignableDefinition getDefinition() { result = super.getDefinition() }
119115

120116
/** Gets the underlying definition, at control flow node `cfn`, if any. */
121117
AssignableDefinition getDefinitionAtNode(ControlFlow::Node cfn) {
122-
result = edef.getADefinition() and
123-
cfn = edef.getControlFlowNode()
118+
result = super.getDefinitionAtNode(cfn)
124119
}
125120
}
126121

0 commit comments

Comments
 (0)