Skip to content

Commit aa0c78c

Browse files
committed
C#: Teach guards library about more null guards
1 parent 40fafc5 commit aa0c78c

File tree

5 files changed

+81
-8
lines changed

5 files changed

+81
-8
lines changed

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,11 +277,12 @@ class DereferenceableExpr extends Expr {
277277
private Expr getABooleanNullCheck(BooleanValue v, boolean isNull) {
278278
exists(boolean branch | branch = v.getValue() |
279279
// Comparison with `null`, for example `x != null`
280-
exists(ComparisonTest ct, ComparisonKind ck, NullLiteral nl |
280+
exists(ComparisonTest ct, ComparisonKind ck, Expr e |
281281
ct.getExpr() = result and
282282
ct.getAnArgument() = this and
283-
ct.getAnArgument() = nl and
284-
this != nl and
283+
ct.getAnArgument() = e and
284+
e = any(NullValue nv | nv.isNull()).getAnExpr() and
285+
this != e and
285286
ck = ct.getComparisonKind()
286287
|
287288
ck.isEquality() and isNull = branch

csharp/ql/test/query-tests/Nullness/E.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ static void Ex33(string s, object o)
360360
{
361361
var x = s ?? o as string;
362362
if (x != (string)null)
363-
x.ToString(); // GOOD (FALSE POSITIVE)
363+
x.ToString(); // GOOD
364364
}
365365
}
366366

csharp/ql/test/query-tests/Nullness/Implications.expected

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,37 +203,51 @@
203203
| B.cs:12:13:12:24 | access to local variable eqCallAlways | non-null | B.cs:7:26:7:29 | null | non-null |
204204
| B.cs:12:13:12:24 | access to local variable eqCallAlways | null | B.cs:7:26:7:29 | null | null |
205205
| B.cs:12:13:12:32 | call to operator == | false | B.cs:12:13:12:24 | access to local variable eqCallAlways | non-null |
206+
| B.cs:12:13:12:32 | call to operator == | false | B.cs:12:29:12:32 | null | non-null |
206207
| B.cs:12:13:12:32 | call to operator == | true | B.cs:12:13:12:24 | access to local variable eqCallAlways | null |
208+
| B.cs:12:13:12:32 | call to operator == | true | B.cs:12:29:12:32 | null | null |
207209
| B.cs:13:13:13:24 | access to local variable eqCallAlways | non-null | B.cs:7:26:7:29 | null | non-null |
208210
| B.cs:13:13:13:24 | access to local variable eqCallAlways | null | B.cs:7:26:7:29 | null | null |
209211
| B.cs:15:13:15:14 | access to local variable b2 | non-null | B.cs:8:16:8:19 | null | non-null |
210212
| B.cs:15:13:15:14 | access to local variable b2 | null | B.cs:8:16:8:19 | null | null |
211213
| B.cs:15:13:15:22 | call to operator != | false | B.cs:15:13:15:14 | access to local variable b2 | null |
214+
| B.cs:15:13:15:22 | call to operator != | false | B.cs:15:19:15:22 | null | null |
212215
| B.cs:15:13:15:22 | call to operator != | true | B.cs:15:13:15:14 | access to local variable b2 | non-null |
216+
| B.cs:15:13:15:22 | call to operator != | true | B.cs:15:19:15:22 | null | non-null |
213217
| B.cs:16:13:16:14 | access to local variable b2 | non-null | B.cs:8:16:8:19 | null | non-null |
214218
| B.cs:16:13:16:14 | access to local variable b2 | null | B.cs:8:16:8:19 | null | null |
215219
| B.cs:18:13:18:14 | access to local variable b3 | non-null | B.cs:9:16:9:19 | null | non-null |
216220
| B.cs:18:13:18:14 | access to local variable b3 | null | B.cs:9:16:9:19 | null | null |
217221
| B.cs:18:13:18:22 | call to operator == | false | B.cs:18:13:18:14 | access to local variable b3 | non-null |
222+
| B.cs:18:13:18:22 | call to operator == | false | B.cs:18:19:18:22 | null | non-null |
218223
| B.cs:18:13:18:22 | call to operator == | true | B.cs:18:13:18:14 | access to local variable b3 | null |
224+
| B.cs:18:13:18:22 | call to operator == | true | B.cs:18:19:18:22 | null | null |
219225
| B.cs:20:13:20:14 | access to local variable b3 | non-null | B.cs:9:16:9:19 | null | non-null |
220226
| B.cs:20:13:20:14 | access to local variable b3 | null | B.cs:9:16:9:19 | null | null |
221227
| B.cs:22:13:22:25 | access to local variable neqCallAlways | non-null | B.cs:10:27:10:30 | null | non-null |
222228
| B.cs:22:13:22:25 | access to local variable neqCallAlways | null | B.cs:10:27:10:30 | null | null |
223229
| B.cs:22:13:22:33 | call to operator != | false | B.cs:22:13:22:25 | access to local variable neqCallAlways | null |
230+
| B.cs:22:13:22:33 | call to operator != | false | B.cs:22:30:22:33 | null | null |
224231
| B.cs:22:13:22:33 | call to operator != | true | B.cs:22:13:22:25 | access to local variable neqCallAlways | non-null |
232+
| B.cs:22:13:22:33 | call to operator != | true | B.cs:22:30:22:33 | null | non-null |
225233
| B.cs:24:13:24:25 | access to local variable neqCallAlways | non-null | B.cs:10:27:10:30 | null | non-null |
226234
| B.cs:24:13:24:25 | access to local variable neqCallAlways | null | B.cs:10:27:10:30 | null | null |
227235
| B.cs:34:16:34:26 | !... | false | B.cs:34:18:34:25 | call to operator == | true |
228236
| B.cs:34:16:34:26 | !... | true | B.cs:34:18:34:25 | call to operator == | false |
229237
| B.cs:53:17:53:25 | (...) ... | non-null | B.cs:53:25:53:25 | access to local variable o | non-null |
230238
| B.cs:53:17:53:25 | (...) ... | null | B.cs:53:25:53:25 | access to local variable o | null |
231239
| B.cs:53:17:53:33 | ... != ... | false | B.cs:53:17:53:25 | (...) ... | null |
240+
| B.cs:53:17:53:33 | ... != ... | false | B.cs:53:30:53:33 | null | null |
232241
| B.cs:53:17:53:33 | ... != ... | true | B.cs:53:17:53:25 | (...) ... | non-null |
242+
| B.cs:53:17:53:33 | ... != ... | true | B.cs:53:30:53:33 | null | non-null |
233243
| B.cs:53:25:53:25 | access to local variable o | non-null | B.cs:52:24:52:27 | null | non-null |
234244
| B.cs:53:25:53:25 | access to local variable o | null | B.cs:52:24:52:27 | null | null |
235245
| B.cs:55:26:55:26 | access to local variable o | non-null | B.cs:52:24:52:27 | null | non-null |
236246
| B.cs:55:26:55:26 | access to local variable o | null | B.cs:52:24:52:27 | null | null |
247+
| B.cs:55:26:55:36 | call to method Equals | false | B.cs:55:26:55:26 | access to local variable o | non-null |
248+
| B.cs:55:26:55:36 | call to method Equals | false | B.cs:55:35:55:35 | access to local variable o | non-null |
249+
| B.cs:55:26:55:36 | call to method Equals | true | B.cs:55:26:55:26 | access to local variable o | null |
250+
| B.cs:55:26:55:36 | call to method Equals | true | B.cs:55:35:55:35 | access to local variable o | null |
237251
| B.cs:55:35:55:35 | access to local variable o | non-null | B.cs:52:24:52:27 | null | non-null |
238252
| B.cs:55:35:55:35 | access to local variable o | null | B.cs:52:24:52:27 | null | null |
239253
| C.cs:11:13:11:30 | !... | false | C.cs:11:15:11:29 | !... | true |
@@ -245,15 +259,19 @@
245259
| C.cs:11:19:11:19 | access to local variable o | non-null | C.cs:10:20:10:23 | null | non-null |
246260
| C.cs:11:19:11:19 | access to local variable o | null | C.cs:10:20:10:23 | null | null |
247261
| C.cs:11:19:11:27 | ... == ... | false | C.cs:11:19:11:19 | access to local variable o | non-null |
262+
| C.cs:11:19:11:27 | ... == ... | false | C.cs:11:24:11:27 | null | non-null |
248263
| C.cs:11:19:11:27 | ... == ... | true | C.cs:11:19:11:19 | access to local variable o | null |
264+
| C.cs:11:19:11:27 | ... == ... | true | C.cs:11:24:11:27 | null | null |
249265
| C.cs:13:13:13:13 | access to local variable o | non-null | C.cs:10:20:10:23 | null | non-null |
250266
| C.cs:13:13:13:13 | access to local variable o | null | C.cs:10:20:10:23 | null | null |
251267
| C.cs:16:13:16:24 | !... | false | C.cs:16:15:16:23 | ... != ... | true |
252268
| C.cs:16:13:16:24 | !... | true | C.cs:16:15:16:23 | ... != ... | false |
253269
| C.cs:16:15:16:15 | access to local variable o | non-null | C.cs:10:20:10:23 | null | non-null |
254270
| C.cs:16:15:16:15 | access to local variable o | null | C.cs:10:20:10:23 | null | null |
255271
| C.cs:16:15:16:23 | ... != ... | false | C.cs:16:15:16:15 | access to local variable o | null |
272+
| C.cs:16:15:16:23 | ... != ... | false | C.cs:16:20:16:23 | null | null |
256273
| C.cs:16:15:16:23 | ... != ... | true | C.cs:16:15:16:15 | access to local variable o | non-null |
274+
| C.cs:16:15:16:23 | ... != ... | true | C.cs:16:20:16:23 | null | non-null |
257275
| C.cs:18:13:18:13 | access to local variable o | non-null | C.cs:10:20:10:23 | null | non-null |
258276
| C.cs:18:13:18:13 | access to local variable o | null | C.cs:10:20:10:23 | null | null |
259277
| C.cs:24:13:24:21 | ... != ... | false | C.cs:24:13:24:13 | access to parameter o | null |
@@ -362,7 +380,9 @@
362380
| C.cs:114:22:114:28 | access to local variable colours | non-null | C.cs:113:26:113:29 | null | non-null |
363381
| C.cs:114:22:114:28 | access to local variable colours | null | C.cs:113:26:113:29 | null | null |
364382
| C.cs:114:22:114:36 | ... == ... | false | C.cs:114:22:114:28 | access to local variable colours | non-null |
383+
| C.cs:114:22:114:36 | ... == ... | false | C.cs:114:33:114:36 | null | non-null |
365384
| C.cs:114:22:114:36 | ... == ... | true | C.cs:114:22:114:28 | access to local variable colours | null |
385+
| C.cs:114:22:114:36 | ... == ... | true | C.cs:114:33:114:36 | null | null |
366386
| C.cs:114:22:114:59 | ... \|\| ... | false | C.cs:114:22:114:36 | ... == ... | false |
367387
| C.cs:114:22:114:59 | ... \|\| ... | false | C.cs:114:41:114:59 | ... == ... | false |
368388
| C.cs:114:22:114:90 | ... ? ... : ... | null | C.cs:114:22:114:59 | ... \|\| ... | false |
@@ -376,10 +396,14 @@
376396
| C.cs:121:13:121:20 | access to local variable children | non-null | C.cs:119:29:119:32 | null | non-null |
377397
| C.cs:121:13:121:20 | access to local variable children | null | C.cs:119:29:119:32 | null | null |
378398
| C.cs:121:13:121:28 | ... == ... | false | C.cs:121:13:121:20 | access to local variable children | non-null |
399+
| C.cs:121:13:121:28 | ... == ... | false | C.cs:121:25:121:28 | null | non-null |
379400
| C.cs:121:13:121:28 | ... == ... | true | C.cs:121:13:121:20 | access to local variable children | null |
401+
| C.cs:121:13:121:28 | ... == ... | true | C.cs:121:25:121:28 | null | null |
380402
| C.cs:123:13:123:31 | ... > ... | true | C.cs:123:13:123:20 | access to local variable children | non-empty |
381403
| C.cs:130:13:130:38 | ... == ... | false | C.cs:130:14:130:29 | ... = ... | non-null |
404+
| C.cs:130:13:130:38 | ... == ... | false | C.cs:130:35:130:38 | null | non-null |
382405
| C.cs:130:13:130:38 | ... == ... | true | C.cs:130:14:130:29 | ... = ... | null |
406+
| C.cs:130:13:130:38 | ... == ... | true | C.cs:130:35:130:38 | null | null |
383407
| C.cs:130:13:130:55 | ... \|\| ... | false | C.cs:130:13:130:38 | ... == ... | false |
384408
| C.cs:130:13:130:55 | ... \|\| ... | false | C.cs:130:43:130:55 | ... > ... | false |
385409
| C.cs:130:14:130:29 | ... = ... | empty | C.cs:130:14:130:15 | access to local variable ok | empty |
@@ -417,7 +441,9 @@
417441
| C.cs:138:35:138:37 | access to local variable ok2 | non-null | C.cs:138:23:138:29 | "hello" | non-null |
418442
| C.cs:138:35:138:37 | access to local variable ok2 | null | C.cs:138:23:138:29 | "hello" | null |
419443
| C.cs:146:13:146:39 | ... != ... | false | C.cs:146:14:146:30 | ... = ... | null |
444+
| C.cs:146:13:146:39 | ... != ... | false | C.cs:146:36:146:39 | null | null |
420445
| C.cs:146:13:146:39 | ... != ... | true | C.cs:146:14:146:30 | ... = ... | non-null |
446+
| C.cs:146:13:146:39 | ... != ... | true | C.cs:146:36:146:39 | null | non-null |
421447
| C.cs:146:13:146:57 | ... && ... | true | C.cs:146:13:146:39 | ... != ... | true |
422448
| C.cs:146:13:146:57 | ... && ... | true | C.cs:146:44:146:57 | ... > ... | true |
423449
| C.cs:146:14:146:30 | ... = ... | empty | C.cs:146:14:146:15 | access to local variable xx | empty |
@@ -445,19 +471,25 @@
445471
| C.cs:158:16:158:16 | access to local variable s | non-null | C.cs:156:17:156:20 | null | non-null |
446472
| C.cs:158:16:158:16 | access to local variable s | null | C.cs:156:17:156:20 | null | null |
447473
| C.cs:158:16:158:24 | ... != ... | false | C.cs:158:16:158:16 | access to local variable s | null |
474+
| C.cs:158:16:158:24 | ... != ... | false | C.cs:158:21:158:24 | null | null |
448475
| C.cs:158:16:158:24 | ... != ... | true | C.cs:158:16:158:16 | access to local variable s | non-null |
476+
| C.cs:158:16:158:24 | ... != ... | true | C.cs:158:21:158:24 | null | non-null |
449477
| C.cs:166:16:166:16 | access to local variable s | empty | C.cs:164:17:164:20 | null | empty |
450478
| C.cs:166:16:166:16 | access to local variable s | non-empty | C.cs:164:17:164:20 | null | non-empty |
451479
| C.cs:166:16:166:16 | access to local variable s | non-null | C.cs:164:17:164:20 | null | non-null |
452480
| C.cs:166:16:166:16 | access to local variable s | null | C.cs:164:17:164:20 | null | null |
453481
| C.cs:166:16:166:24 | ... != ... | false | C.cs:166:16:166:16 | access to local variable s | null |
482+
| C.cs:166:16:166:24 | ... != ... | false | C.cs:166:21:166:24 | null | null |
454483
| C.cs:166:16:166:24 | ... != ... | true | C.cs:166:16:166:16 | access to local variable s | non-null |
484+
| C.cs:166:16:166:24 | ... != ... | true | C.cs:166:21:166:24 | null | non-null |
455485
| C.cs:171:13:171:13 | access to local variable s | non-null | C.cs:168:13:168:16 | null | non-null |
456486
| C.cs:171:13:171:13 | access to local variable s | null | C.cs:168:13:168:16 | null | null |
457487
| C.cs:173:16:173:16 | access to local variable s | non-null | C.cs:168:13:168:16 | null | non-null |
458488
| C.cs:173:16:173:16 | access to local variable s | null | C.cs:168:13:168:16 | null | null |
459489
| C.cs:173:16:173:24 | ... != ... | false | C.cs:173:16:173:16 | access to local variable s | null |
490+
| C.cs:173:16:173:24 | ... != ... | false | C.cs:173:21:173:24 | null | null |
460491
| C.cs:173:16:173:24 | ... != ... | true | C.cs:173:16:173:16 | access to local variable s | non-null |
492+
| C.cs:173:16:173:24 | ... != ... | true | C.cs:173:21:173:24 | null | non-null |
461493
| C.cs:187:16:187:24 | ... != ... | false | C.cs:187:16:187:16 | access to local variable s | null |
462494
| C.cs:187:16:187:24 | ... != ... | true | C.cs:187:16:187:16 | access to local variable s | non-null |
463495
| C.cs:211:17:211:35 | ... ? ... : ... | non-null | C.cs:211:17:211:23 | call to method Maybe | false |
@@ -515,7 +547,9 @@
515547
| C.cs:236:14:236:21 | ... = ... | null | C.cs:236:14:236:14 | access to local variable s | null |
516548
| C.cs:236:14:236:21 | ... = ... | null | C.cs:236:18:236:21 | null | null |
517549
| C.cs:236:24:236:32 | ... == ... | false | C.cs:236:24:236:24 | access to local variable s | non-null |
550+
| C.cs:236:24:236:32 | ... == ... | false | C.cs:236:29:236:32 | null | non-null |
518551
| C.cs:236:24:236:32 | ... == ... | true | C.cs:236:24:236:24 | access to local variable s | null |
552+
| C.cs:236:24:236:32 | ... == ... | true | C.cs:236:29:236:32 | null | null |
519553
| C.cs:236:35:236:42 | ... = ... | empty | C.cs:236:35:236:35 | access to local variable s | empty |
520554
| C.cs:236:35:236:42 | ... = ... | empty | C.cs:236:39:236:42 | null | empty |
521555
| C.cs:236:35:236:42 | ... = ... | non-empty | C.cs:236:35:236:35 | access to local variable s | non-empty |
@@ -794,7 +828,9 @@
794828
| D.cs:212:18:212:18 | access to local variable n | non-null | D.cs:211:20:211:23 | null | non-null |
795829
| D.cs:212:18:212:18 | access to local variable n | null | D.cs:211:20:211:23 | null | null |
796830
| D.cs:212:18:212:26 | ... == ... | false | D.cs:212:18:212:18 | access to local variable n | non-null |
831+
| D.cs:212:18:212:26 | ... == ... | false | D.cs:212:23:212:26 | null | non-null |
797832
| D.cs:212:18:212:26 | ... == ... | true | D.cs:212:18:212:18 | access to local variable n | null |
833+
| D.cs:212:18:212:26 | ... == ... | true | D.cs:212:23:212:26 | null | null |
798834
| D.cs:212:18:212:45 | ... ? ... : ... | non-null | D.cs:212:18:212:26 | ... == ... | true |
799835
| D.cs:212:18:212:45 | ... ? ... : ... | non-null | D.cs:212:30:212:41 | object creation of type Object | non-null |
800836
| D.cs:212:18:212:45 | ... ? ... : ... | null | D.cs:212:18:212:26 | ... == ... | false |
@@ -1189,6 +1225,8 @@
11891225
| E.cs:362:13:362:13 | access to local variable x | non-empty | E.cs:361:17:361:32 | ... ?? ... | non-empty |
11901226
| E.cs:362:13:362:13 | access to local variable x | non-null | E.cs:361:17:361:32 | ... ?? ... | non-null |
11911227
| E.cs:362:13:362:13 | access to local variable x | null | E.cs:361:17:361:32 | ... ?? ... | null |
1228+
| E.cs:362:13:362:29 | ... != ... | false | E.cs:362:13:362:13 | access to local variable x | null |
1229+
| E.cs:362:13:362:29 | ... != ... | true | E.cs:362:13:362:13 | access to local variable x | non-null |
11921230
| E.cs:362:18:362:29 | (...) ... | non-null | E.cs:362:26:362:29 | null | non-null |
11931231
| E.cs:362:18:362:29 | (...) ... | null | E.cs:362:26:362:29 | null | null |
11941232
| E.cs:363:13:363:13 | access to local variable x | non-null | E.cs:361:17:361:32 | ... ?? ... | non-null |

0 commit comments

Comments
 (0)