Skip to content

Commit f7a515c

Browse files
committed
C#: Prune CFG for obviously impossible nullness/matching edges
1 parent 9a1e148 commit f7a515c

File tree

8 files changed

+111
-205
lines changed

8 files changed

+111
-205
lines changed

csharp/ql/src/semmle/code/csharp/controlflow/Completion.qll

Lines changed: 95 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,11 @@ class Completion extends TCompletion {
8181
or
8282
if mustHaveBooleanCompletion(cfe) then
8383
exists(boolean value |
84-
isConstant(cfe, value) |
84+
isBooleanConstant(cfe, value) |
8585
this = TBooleanCompletion(value, value)
8686
)
8787
or
88-
not isConstant(cfe, _) and
88+
not isBooleanConstant(cfe, _) and
8989
exists(boolean b | this = TBooleanCompletion(b, b))
9090
or
9191
// Corner case: In `if (x ?? y) { ... }`, `x` must have both a `true`
@@ -94,8 +94,20 @@ class Completion extends TCompletion {
9494
mustHaveNullnessCompletion(cfe) and
9595
this = TNullnessCompletion(true)
9696
else if mustHaveNullnessCompletion(cfe) then
97+
exists(boolean value |
98+
isNullnessConstant(cfe, value) |
99+
this = TNullnessCompletion(value)
100+
)
101+
or
102+
not isNullnessConstant(cfe, _) and
97103
this = TNullnessCompletion(_)
98-
else if mustHaveMatchingCompletion(cfe) then
104+
else if mustHaveMatchingCompletion(_, cfe) then
105+
exists(boolean value |
106+
isMatchingConstant(cfe, value) |
107+
this = TMatchingCompletion(value)
108+
)
109+
or
110+
not isMatchingConstant(cfe, _) and
99111
this = TMatchingCompletion(_)
100112
else if mustHaveEmptinessCompletion(cfe) then
101113
this = TEmptinessCompletion(_)
@@ -118,14 +130,82 @@ class Completion extends TCompletion {
118130
}
119131
}
120132

121-
private predicate isConstant(Expr e, boolean value) {
122-
e.getValue() = "true" and
123-
value = true
124-
or
125-
e.getValue() = "false" and
126-
value = false
127-
or
128-
isConstantComparison(e, value)
133+
/** Holds if expression `e` has the Boolean constant value `value`. */
134+
private predicate isBooleanConstant(Expr e, boolean value) {
135+
mustHaveBooleanCompletion(e) and
136+
(
137+
e.getValue() = "true" and
138+
value = true
139+
or
140+
e.getValue() = "false" and
141+
value = false
142+
or
143+
isConstantComparison(e, value)
144+
)
145+
}
146+
147+
/**
148+
* Holds if expression `e` is constantly `null` (`value = true`) or constantly
149+
* non-`null` (`value = false`).
150+
*/
151+
private predicate isNullnessConstant(Expr e, boolean value) {
152+
mustHaveNullnessCompletion(e) and
153+
exists(Expr stripped |
154+
stripped = e.stripCasts() |
155+
stripped.getType() = any(ValueType t |
156+
not t instanceof NullableType and
157+
// Extractor bug: the type of `x?.Length` is reported as `int`, but it should
158+
// be `int?`
159+
not getQualifier*(stripped).(QualifiableExpr).isConditional()
160+
) and
161+
value = false
162+
or
163+
stripped instanceof NullLiteral and
164+
value = true
165+
or
166+
stripped.hasValue() and
167+
not stripped instanceof NullLiteral and
168+
value = false
169+
)
170+
}
171+
172+
private Expr getQualifier(QualifiableExpr e) {
173+
// `e.getQualifier()` does not work for calls to extension methods
174+
result = e.getChildExpr(-1)
175+
}
176+
177+
/**
178+
* Holds if expression `e` constantly matches (`value = true`) or constantly
179+
* non-matches (`value = false`).
180+
*/
181+
private predicate isMatchingConstant(Expr e, boolean value) {
182+
exists(SwitchStmt ss |
183+
mustHaveMatchingCompletion(ss, e) |
184+
exists(Expr stripped |
185+
stripped = ss.getCondition().stripCasts() |
186+
exists(ConstCase cc, string strippedValue |
187+
cc = ss.getAConstCase() and
188+
e = cc.getExpr() and
189+
strippedValue = stripped.getValue() |
190+
if strippedValue = e.getValue() then
191+
value = true
192+
else
193+
value = false
194+
)
195+
or
196+
exists(TypeCase tc, Type t, Type strippedType |
197+
tc = ss.getATypeCase() |
198+
e = tc.getTypeAccess() and
199+
t = e.getType() and
200+
strippedType = stripped.getType() and
201+
not t.isImplicitlyConvertibleTo(strippedType) and
202+
not t instanceof Interface and
203+
not t.containsTypeParameters() and
204+
not strippedType.containsTypeParameters() and
205+
value = false
206+
)
207+
)
208+
)
129209
}
130210

131211
/** A control flow element that is inside a `try` block. */
@@ -325,12 +405,10 @@ private predicate inNullnessContext(Expr e, boolean isNullnessCompletionForParen
325405
* Holds if a normal completion of `e` must be a matching completion. Thats is,
326406
* whether `e` determines a match in a `switch` statement.
327407
*/
328-
private predicate mustHaveMatchingCompletion(Expr e) {
329-
exists(SwitchStmt ss |
330-
e = ss.getAConstCase().getExpr()
331-
or
332-
e = ss.getATypeCase().getTypeAccess() // use type access to represent the type test
333-
)
408+
private predicate mustHaveMatchingCompletion(SwitchStmt ss, Expr e) {
409+
e = ss.getAConstCase().getExpr()
410+
or
411+
e = ss.getATypeCase().getTypeAccess() // use type access to represent the type test
334412
}
335413

336414
/**

csharp/ql/src/semmle/code/csharp/controlflow/ControlFlowGraph.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2022,6 +2022,7 @@ module Internal {
20222022
c instanceof SimpleCompletion
20232023
or
20242024
cfe = tc.getTypeAccess() and
2025+
c.isValidFor(cfe) and
20252026
c = any(MatchingCompletion mc |
20262027
if mc.isMatch() then
20272028
if exists(tc.getVariableDeclExpr()) then

csharp/ql/test/library-tests/controlflow/graph/BasicBlock.expected

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,7 @@
7070
| ConditionalAccess.cs:19:12:19:13 | enter M6 | ConditionalAccess.cs:19:40:19:41 | access to parameter s1 | 2 |
7171
| ConditionalAccess.cs:19:12:19:13 | exit M6 | ConditionalAccess.cs:19:12:19:13 | exit M6 | 1 |
7272
| ConditionalAccess.cs:19:58:19:59 | access to parameter s2 | ConditionalAccess.cs:19:43:19:60 | call to method CommaJoinWith | 2 |
73-
| ConditionalAccess.cs:21:10:21:11 | enter M7 | ConditionalAccess.cs:23:18:23:29 | (...) ... | 6 |
74-
| ConditionalAccess.cs:23:13:23:38 | Nullable<Int32> j = ... | ConditionalAccess.cs:24:18:24:24 | (...) ... | 5 |
75-
| ConditionalAccess.cs:23:32:23:38 | access to property Length | ConditionalAccess.cs:23:32:23:38 | access to property Length | 1 |
76-
| ConditionalAccess.cs:24:13:24:37 | String s = ... | ConditionalAccess.cs:25:13:25:14 | "" | 4 |
77-
| ConditionalAccess.cs:24:27:24:37 | call to method ToString | ConditionalAccess.cs:24:27:24:37 | call to method ToString | 1 |
78-
| ConditionalAccess.cs:25:9:25:32 | ... = ... | ConditionalAccess.cs:21:10:21:11 | exit M7 | 2 |
79-
| ConditionalAccess.cs:25:31:25:31 | access to local variable s | ConditionalAccess.cs:25:16:25:32 | call to method CommaJoinWith | 2 |
73+
| ConditionalAccess.cs:21:10:21:11 | enter M7 | ConditionalAccess.cs:21:10:21:11 | exit M7 | 20 |
8074
| ConditionalAccess.cs:31:26:31:38 | enter CommaJoinWith | ConditionalAccess.cs:31:26:31:38 | exit CommaJoinWith | 7 |
8175
| ExitMethods.cs:6:10:6:11 | enter M1 | ExitMethods.cs:6:10:6:11 | exit M1 | 7 |
8276
| ExitMethods.cs:12:10:12:11 | enter M2 | ExitMethods.cs:12:10:12:11 | exit M2 | 7 |
@@ -142,20 +136,13 @@
142136
| NullCoalescing.cs:9:41:9:41 | access to parameter s | NullCoalescing.cs:9:41:9:41 | access to parameter s | 1 |
143137
| NullCoalescing.cs:9:45:9:45 | access to parameter s | NullCoalescing.cs:9:45:9:45 | access to parameter s | 1 |
144138
| NullCoalescing.cs:9:51:9:58 | ... ?? ... | NullCoalescing.cs:9:51:9:52 | "" | 2 |
145-
| NullCoalescing.cs:9:57:9:58 | "" | NullCoalescing.cs:9:57:9:58 | "" | 1 |
146139
| NullCoalescing.cs:11:9:11:10 | enter M5 | NullCoalescing.cs:11:44:11:45 | access to parameter b1 | 4 |
147140
| NullCoalescing.cs:11:9:11:10 | exit M5 | NullCoalescing.cs:11:9:11:10 | exit M5 | 1 |
148141
| NullCoalescing.cs:11:51:11:58 | ... && ... | NullCoalescing.cs:11:51:11:52 | access to parameter b2 | 2 |
149142
| NullCoalescing.cs:11:57:11:58 | access to parameter b3 | NullCoalescing.cs:11:57:11:58 | access to parameter b3 | 1 |
150143
| NullCoalescing.cs:11:64:11:64 | 0 | NullCoalescing.cs:11:64:11:64 | 0 | 1 |
151144
| NullCoalescing.cs:11:68:11:68 | 1 | NullCoalescing.cs:11:68:11:68 | 1 | 1 |
152-
| NullCoalescing.cs:13:10:13:11 | enter M6 | NullCoalescing.cs:15:17:15:26 | (...) ... | 7 |
153-
| NullCoalescing.cs:15:13:15:31 | Int32 j = ... | NullCoalescing.cs:16:17:16:18 | "" | 5 |
154-
| NullCoalescing.cs:15:31:15:31 | 0 | NullCoalescing.cs:15:31:15:31 | 0 | 1 |
155-
| NullCoalescing.cs:16:13:16:25 | String s = ... | NullCoalescing.cs:17:13:17:19 | (...) ... | 6 |
156-
| NullCoalescing.cs:16:23:16:25 | "a" | NullCoalescing.cs:16:23:16:25 | "a" | 1 |
157-
| NullCoalescing.cs:17:9:17:24 | ... = ... | NullCoalescing.cs:13:10:13:11 | exit M6 | 2 |
158-
| NullCoalescing.cs:17:24:17:24 | 1 | NullCoalescing.cs:17:24:17:24 | 1 | 1 |
145+
| NullCoalescing.cs:13:10:13:11 | enter M6 | NullCoalescing.cs:13:10:13:11 | exit M6 | 21 |
159146
| Patterns.cs:5:10:5:13 | enter Test | Patterns.cs:8:13:8:23 | ... is ... | 10 |
160147
| Patterns.cs:9:9:11:9 | {...} | Patterns.cs:10:13:10:42 | call to method WriteLine | 6 |
161148
| Patterns.cs:12:14:18:9 | if (...) ... | Patterns.cs:12:18:12:31 | ... is ... | 4 |
@@ -204,15 +191,9 @@
204191
| Switch.cs:50:13:50:39 | case Boolean: | Switch.cs:50:18:50:21 | access to type Boolean | 2 |
205192
| Switch.cs:50:30:50:30 | access to parameter o | Switch.cs:50:30:50:38 | ... != ... | 3 |
206193
| Switch.cs:51:17:51:22 | break; | Switch.cs:51:17:51:22 | break; | 1 |
207-
| Switch.cs:55:10:55:11 | enter M5 | Switch.cs:59:18:59:18 | 2 | 8 |
208-
| Switch.cs:55:10:55:11 | exit M5 | Switch.cs:55:10:55:11 | exit M5 | 1 |
209-
| Switch.cs:60:15:60:20 | break; | Switch.cs:60:15:60:20 | break; | 1 |
210-
| Switch.cs:61:13:61:20 | case ...: | Switch.cs:61:18:61:18 | 3 | 2 |
211-
| Switch.cs:62:15:62:20 | break; | Switch.cs:62:15:62:20 | break; | 1 |
212-
| Switch.cs:66:10:66:11 | enter M6 | Switch.cs:70:18:70:20 | access to type Int32 | 7 |
194+
| Switch.cs:55:10:55:11 | enter M5 | Switch.cs:55:10:55:11 | exit M5 | 12 |
195+
| Switch.cs:66:10:66:11 | enter M6 | Switch.cs:72:18:72:19 | "" | 9 |
213196
| Switch.cs:66:10:66:11 | exit M6 | Switch.cs:66:10:66:11 | exit M6 | 1 |
214-
| Switch.cs:71:15:71:20 | break; | Switch.cs:71:15:71:20 | break; | 1 |
215-
| Switch.cs:72:13:72:21 | case ...: | Switch.cs:72:18:72:19 | "" | 2 |
216197
| Switch.cs:73:15:73:20 | break; | Switch.cs:73:15:73:20 | break; | 1 |
217198
| Switch.cs:77:10:77:11 | enter M7 | Switch.cs:81:18:81:18 | 1 | 6 |
218199
| Switch.cs:77:10:77:11 | exit M7 | Switch.cs:77:10:77:11 | exit M7 | 1 |

0 commit comments

Comments
 (0)