Skip to content

Commit ed006d7

Browse files
hvitvedhvitved
authored
Merge pull request #7231 from hvitved/csharp/dataflow/consistency-queries
C#: Enable data-flow consistency queries
2 parents bebf4ca + 77fcb8a commit ed006d7

File tree

12 files changed

+197
-38
lines changed

12 files changed

+197
-38
lines changed

‎cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,23 @@ module Consistency {
1515
class ConsistencyConfiguration extends TConsistencyConfiguration {
1616
string toString() { none() }
1717

18+
/** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
19+
predicate uniqueEnclosingCallableExclude(Node n) { none() }
20+
21+
/** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
22+
predicate uniqueNodeLocationExclude(Node n) { none() }
23+
24+
/** Holds if `n` should be excluded from the consistency test `missingLocation`. */
25+
predicate missingLocationExclude(Node n) { none() }
26+
1827
/** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
1928
predicate postWithInFlowExclude(Node n) { none() }
2029

2130
/** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
2231
predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
32+
33+
/** Holds if `n` should be excluded from the consistency test `reverseRead`. */
34+
predicate reverseReadExclude(Node n) { none() }
2335
}
2436

2537
private class RelevantNode extends Node {
@@ -46,6 +58,7 @@ module Consistency {
4658
n instanceof RelevantNode and
4759
c = count(nodeGetEnclosingCallable(n)) and
4860
c != 1 and
61+
not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and
4962
msg = "Node should have one enclosing callable but has " + c + "."
5063
)
5164
}
@@ -66,6 +79,7 @@ module Consistency {
6679
n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
6780
) and
6881
c != 1 and
82+
not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and
6983
msg = "Node should have one location but has " + c + "."
7084
)
7185
}
@@ -76,7 +90,8 @@ module Consistency {
7690
strictcount(Node n |
7791
not exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
7892
n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
79-
)
93+
) and
94+
not any(ConsistencyConfiguration conf).missingLocationExclude(n)
8095
) and
8196
msg = "Nodes without location: " + c
8297
)
@@ -172,6 +187,7 @@ module Consistency {
172187

173188
query predicate reverseRead(Node n, string msg) {
174189
exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and
190+
not any(ConsistencyConfiguration conf).reverseReadExclude(n) and
175191
msg = "Origin of readStep is missing a PostUpdateNode."
176192
}
177193

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,23 @@ module Consistency {
1515
class ConsistencyConfiguration extends TConsistencyConfiguration {
1616
string toString() { none() }
1717

18+
/** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
19+
predicate uniqueEnclosingCallableExclude(Node n) { none() }
20+
21+
/** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
22+
predicate uniqueNodeLocationExclude(Node n) { none() }
23+
24+
/** Holds if `n` should be excluded from the consistency test `missingLocation`. */
25+
predicate missingLocationExclude(Node n) { none() }
26+
1827
/** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
1928
predicate postWithInFlowExclude(Node n) { none() }
2029

2130
/** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
2231
predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
32+
33+
/** Holds if `n` should be excluded from the consistency test `reverseRead`. */
34+
predicate reverseReadExclude(Node n) { none() }
2335
}
2436

2537
private class RelevantNode extends Node {
@@ -46,6 +58,7 @@ module Consistency {
4658
n instanceof RelevantNode and
4759
c = count(nodeGetEnclosingCallable(n)) and
4860
c != 1 and
61+
not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and
4962
msg = "Node should have one enclosing callable but has " + c + "."
5063
)
5164
}
@@ -66,6 +79,7 @@ module Consistency {
6679
n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
6780
) and
6881
c != 1 and
82+
not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and
6983
msg = "Node should have one location but has " + c + "."
7084
)
7185
}
@@ -76,7 +90,8 @@ module Consistency {
7690
strictcount(Node n |
7791
not exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
7892
n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
79-
)
93+
) and
94+
not any(ConsistencyConfiguration conf).missingLocationExclude(n)
8095
) and
8196
msg = "Nodes without location: " + c
8297
)
@@ -172,6 +187,7 @@ module Consistency {
172187

173188
query predicate reverseRead(Node n, string msg) {
174189
exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and
190+
not any(ConsistencyConfiguration conf).reverseReadExclude(n) and
175191
msg = "Origin of readStep is missing a PostUpdateNode."
176192
}
177193

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import csharp
2+
import cil
3+
import semmle.code.csharp.dataflow.internal.DataFlowPrivate
4+
import semmle.code.csharp.dataflow.internal.DataFlowPublic
5+
import semmle.code.csharp.dataflow.internal.DataFlowImplConsistency::Consistency
6+
7+
private class MyConsistencyConfiguration extends ConsistencyConfiguration {
8+
override predicate uniqueEnclosingCallableExclude(Node n) {
9+
// TODO: Remove once static initializers are folded into the
10+
// static constructors
11+
exists(ControlFlow::Node cfn |
12+
cfn.getElement() = any(FieldOrProperty f | f.isStatic()).getAChild+() and
13+
cfn = n.getControlFlowNode()
14+
)
15+
}
16+
17+
override predicate uniqueNodeLocationExclude(Node n) {
18+
// Methods with multiple implementations
19+
n instanceof ParameterNode
20+
or
21+
this.missingLocationExclude(n)
22+
}
23+
24+
override predicate missingLocationExclude(Node n) {
25+
// Some CIL methods are missing locations
26+
n.asParameter() instanceof CIL::Parameter
27+
}
28+
29+
override predicate postWithInFlowExclude(Node n) {
30+
n instanceof SummaryNode
31+
or
32+
n.asExpr().(ObjectCreation).hasInitializer()
33+
}
34+
35+
override predicate argHasPostUpdateExclude(ArgumentNode n) {
36+
n instanceof SummaryNode
37+
or
38+
n.asExpr().(Expr).stripCasts().getType() =
39+
any(Type t |
40+
not t instanceof RefType and
41+
not t = any(TypeParameter tp | not tp.isValueType())
42+
or
43+
t instanceof NullType
44+
)
45+
or
46+
n instanceof ImplicitCapturedArgumentNode
47+
or
48+
n instanceof ParamsArgumentNode
49+
or
50+
n.asExpr() instanceof CIL::Expr
51+
}
52+
53+
override predicate reverseReadExclude(Node n) { n.asExpr() = any(AwaitExpr ae).getExpr() }
54+
}

‎csharp/ql/lib/semmle/code/cil/Method.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class Method extends DotNet::Callable, Element, Member, TypeContainer, DataFlowN
8989

9090
override Location getLocation() { result = Element.super.getLocation() }
9191

92-
override Location getALocation() { cil_method_location(this.getUnboundDeclaration(), result) }
92+
override Location getALocation() { cil_method_location(this.getUnboundMethod+(), result) }
9393

9494
override MethodParameter getParameter(int n) {
9595
if this.isStatic()

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ module Gvn {
8080
LeafType() {
8181
not this instanceof GenericType and
8282
not this instanceof TypeParameter and
83-
not this instanceof DynamicType
83+
not this instanceof DynamicType and
84+
not this instanceof TupleType
8485
}
8586
}
8687

@@ -478,6 +479,8 @@ module Gvn {
478479
GvnType getGlobalValueNumber(Type t) {
479480
result = TLeafGvnType(t)
480481
or
482+
result = TLeafGvnType(t.(TupleType).getUnderlyingType())
483+
or
481484
t instanceof DynamicType and
482485
result = TLeafGvnType(any(ObjectType ot))
483486
or

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,7 @@ class IEnumerableFlow extends LibraryTypeDataFlow, RefType {
585585
sink = getDelegateFlowSinkArg(m, 1, 1) and
586586
sinkAp = AccessPath::empty()
587587
or
588-
source = TCallableFlowSourceDelegateArg(1) and
588+
source = getDelegateFlowSourceArg(m, 1) and
589589
sourceAp = AccessPath::empty() and
590590
sink = TCallableFlowSinkReturn() and
591591
sinkAp = AccessPath::empty()
@@ -603,7 +603,7 @@ class IEnumerableFlow extends LibraryTypeDataFlow, RefType {
603603
sink = getDelegateFlowSinkArg(m, 2, 0) and
604604
sinkAp = AccessPath::empty()
605605
or
606-
source = TCallableFlowSourceDelegateArg(2) and
606+
source = getDelegateFlowSourceArg(m, 2) and
607607
sourceAp = AccessPath::empty() and
608608
sink = TCallableFlowSinkReturn() and
609609
sinkAp = AccessPath::empty()
@@ -621,12 +621,12 @@ class IEnumerableFlow extends LibraryTypeDataFlow, RefType {
621621
sink = getDelegateFlowSinkArg(m, 2, 0) and
622622
sinkAp = AccessPath::empty()
623623
or
624-
source = TCallableFlowSourceDelegateArg(2) and
624+
source = getDelegateFlowSourceArg(m, 2) and
625625
sourceAp = AccessPath::empty() and
626626
sink = getDelegateFlowSinkArg(m, 3, 0) and
627627
sinkAp = AccessPath::empty()
628628
or
629-
source = TCallableFlowSourceDelegateArg(3) and
629+
source = getDelegateFlowSourceArg(m, 3) and
630630
sourceAp = AccessPath::empty() and
631631
sink = TCallableFlowSinkReturn() and
632632
sinkAp = AccessPath::empty()
@@ -841,7 +841,7 @@ class IEnumerableFlow extends LibraryTypeDataFlow, RefType {
841841
sink = getDelegateFlowSinkArg(m, 4, 1) and
842842
sinkAp = AccessPath::empty()
843843
or
844-
source = TCallableFlowSourceDelegateArg(4) and
844+
source = getDelegateFlowSourceArg(m, 4) and
845845
sourceAp = AccessPath::empty() and
846846
sink = TCallableFlowSinkReturn() and
847847
sinkAp = AccessPath::element()
@@ -924,7 +924,7 @@ class IEnumerableFlow extends LibraryTypeDataFlow, RefType {
924924
sink = getDelegateFlowSinkArg(m, 1, 0) and
925925
sinkAp = AccessPath::empty()
926926
or
927-
source = TCallableFlowSourceDelegateArg(1) and
927+
source = getDelegateFlowSourceArg(m, 1) and
928928
sourceAp = AccessPath::empty() and
929929
sink = TCallableFlowSinkReturn() and
930930
sinkAp = AccessPath::element()
@@ -943,12 +943,12 @@ class IEnumerableFlow extends LibraryTypeDataFlow, RefType {
943943
sink = getDelegateFlowSinkArg(m, 2, 0) and
944944
sinkAp = AccessPath::empty()
945945
or
946-
source = TCallableFlowSourceDelegateArg(1) and
946+
source = getDelegateFlowSourceArg(m, 1) and
947947
sourceAp = AccessPath::element() and
948948
sink = getDelegateFlowSinkArg(m, 2, 1) and
949949
sinkAp = AccessPath::empty()
950950
or
951-
source = TCallableFlowSourceDelegateArg(2) and
951+
source = getDelegateFlowSourceArg(m, 2) and
952952
sourceAp = AccessPath::empty() and
953953
sink = TCallableFlowSinkReturn() and
954954
sinkAp = AccessPath::element()

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,23 @@ module Consistency {
1515
class ConsistencyConfiguration extends TConsistencyConfiguration {
1616
string toString() { none() }
1717

18+
/** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
19+
predicate uniqueEnclosingCallableExclude(Node n) { none() }
20+
21+
/** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
22+
predicate uniqueNodeLocationExclude(Node n) { none() }
23+
24+
/** Holds if `n` should be excluded from the consistency test `missingLocation`. */
25+
predicate missingLocationExclude(Node n) { none() }
26+
1827
/** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
1928
predicate postWithInFlowExclude(Node n) { none() }
2029

2130
/** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
2231
predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
32+
33+
/** Holds if `n` should be excluded from the consistency test `reverseRead`. */
34+
predicate reverseReadExclude(Node n) { none() }
2335
}
2436

2537
private class RelevantNode extends Node {
@@ -46,6 +58,7 @@ module Consistency {
4658
n instanceof RelevantNode and
4759
c = count(nodeGetEnclosingCallable(n)) and
4860
c != 1 and
61+
not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and
4962
msg = "Node should have one enclosing callable but has " + c + "."
5063
)
5164
}
@@ -66,6 +79,7 @@ module Consistency {
6679
n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
6780
) and
6881
c != 1 and
82+
not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and
6983
msg = "Node should have one location but has " + c + "."
7084
)
7185
}
@@ -76,7 +90,8 @@ module Consistency {
7690
strictcount(Node n |
7791
not exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
7892
n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
79-
)
93+
) and
94+
not any(ConsistencyConfiguration conf).missingLocationExclude(n)
8095
) and
8196
msg = "Nodes without location: " + c
8297
)
@@ -172,6 +187,7 @@ module Consistency {
172187

173188
query predicate reverseRead(Node n, string msg) {
174189
exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and
190+
not any(ConsistencyConfiguration conf).reverseReadExclude(n) and
175191
msg = "Origin of readStep is missing a PostUpdateNode."
176192
}
177193

0 commit comments

Comments
 (0)