From dc4abc952f072a18ac8f1ba8952d1daed3ac174d Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Fri, 17 Oct 2025 11:55:02 +0200 Subject: [PATCH] C#: Replace references to getANullCheck. --- .../ql/lib/semmle/code/csharp/controlflow/Guards.qll | 11 +++++++++++ .../ql/lib/semmle/code/csharp/dataflow/Nullness.qll | 2 +- .../ql/test/query-tests/Nullness/NullCheck.expected | 5 +---- csharp/ql/test/query-tests/Nullness/NullCheck.ql | 5 +++-- .../ql/test/query-tests/Nullness/NullMaybe.expected | 2 ++ 5 files changed, 18 insertions(+), 7 deletions(-) diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll index 5492b2d5810..5f4c5c75246 100644 --- a/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll +++ b/csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll @@ -776,6 +776,17 @@ class DereferenceableExpr extends Expr { or result = this.getANullnessNullCheck(v, isNull) } + + /** Holds if `guard` suggests that this expression may be `null`. */ + predicate guardSuggestsMaybeNull(Guards::Guard guard) { + guard = this.getANullnessNullCheck(_, true) + or + LogicInput::additionalNullCheck(guard, _, this, true) + or + guard.isEquality(this, any(Expr n | nullValueImplied(n)), _) + or + Guards::nullGuard(guard, any(GuardValue v | exists(v.asBooleanValue())), this, true) + } } /** diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll index 6e36008ab2e..55c0324e7c5 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll @@ -193,7 +193,7 @@ private predicate defMaybeNull( ( // A variable compared to `null` might be `null` exists(G::DereferenceableExpr de | de = def.getARead() | - reason = de.getANullCheck(_, true) and + de.guardSuggestsMaybeNull(reason) and msg = "as suggested by $@ null check" and node = def.getControlFlowNode() and not de = any(Ssa::PhiNode phi).getARead() and diff --git a/csharp/ql/test/query-tests/Nullness/NullCheck.expected b/csharp/ql/test/query-tests/Nullness/NullCheck.expected index 5161037eba0..ad941d2ae07 100644 --- a/csharp/ql/test/query-tests/Nullness/NullCheck.expected +++ b/csharp/ql/test/query-tests/Nullness/NullCheck.expected @@ -33,8 +33,6 @@ | C.cs:45:22:45:30 | ... != ... | C.cs:45:22:45:22 | access to local variable s | | C.cs:56:23:56:24 | access to local variable o2 | C.cs:56:23:56:24 | access to local variable o2 | | C.cs:71:26:71:27 | access to local variable o3 | C.cs:71:26:71:27 | access to local variable o3 | -| C.cs:78:13:78:24 | call to method IsNotNull | C.cs:78:23:78:23 | access to local variable o | -| C.cs:81:14:81:22 | call to method IsNull | C.cs:81:21:81:21 | access to local variable o | | C.cs:113:22:113:36 | ... == ... | C.cs:113:22:113:28 | access to local variable colours | | C.cs:113:22:113:36 | ... == ... | C.cs:113:33:113:36 | null | | C.cs:120:13:120:28 | ... == ... | C.cs:120:13:120:20 | access to local variable children | @@ -144,8 +142,7 @@ | E.cs:422:13:422:22 | access to property HasValue | E.cs:422:13:422:13 | access to parameter i | | E.cs:429:13:429:22 | access to property HasValue | E.cs:429:13:429:13 | access to parameter i | | E.cs:437:23:437:31 | ... is ... | E.cs:437:23:437:23 | access to parameter s | -| Forwarding.cs:19:14:19:23 | call to method IsNull | Forwarding.cs:19:14:19:14 | access to local variable s | -| Forwarding.cs:24:13:24:25 | call to method IsNotNull | Forwarding.cs:24:13:24:13 | access to local variable s | +| E.cs:447:23:447:35 | ... is ... | E.cs:447:23:447:23 | access to parameter s | | Forwarding.cs:45:66:45:75 | call to method IsNull | Forwarding.cs:45:66:45:66 | access to parameter o | | Forwarding.cs:59:13:59:21 | ... == ... | Forwarding.cs:59:13:59:13 | access to parameter o | | Forwarding.cs:78:16:78:39 | call to method ReferenceEquals | Forwarding.cs:78:32:78:32 | access to parameter o | diff --git a/csharp/ql/test/query-tests/Nullness/NullCheck.ql b/csharp/ql/test/query-tests/Nullness/NullCheck.ql index 0635ae823e7..5ab3414b971 100644 --- a/csharp/ql/test/query-tests/Nullness/NullCheck.ql +++ b/csharp/ql/test/query-tests/Nullness/NullCheck.ql @@ -1,5 +1,6 @@ import csharp import semmle.code.csharp.controlflow.Guards -from DereferenceableExpr de -select de.getANullCheck(_, true), de +from DereferenceableExpr de, Guards::Guard reason +where de.guardSuggestsMaybeNull(reason) +select reason, de diff --git a/csharp/ql/test/query-tests/Nullness/NullMaybe.expected b/csharp/ql/test/query-tests/Nullness/NullMaybe.expected index ce9b6d8b182..6a0d8372e3e 100644 --- a/csharp/ql/test/query-tests/Nullness/NullMaybe.expected +++ b/csharp/ql/test/query-tests/Nullness/NullMaybe.expected @@ -57,7 +57,9 @@ | E.cs:423:38:423:38 | access to parameter i | Variable $@ may be null at this access because it has a nullable type. | E.cs:420:27:420:27 | i | i | E.cs:420:27:420:27 | i | this | | E.cs:430:39:430:39 | access to parameter i | Variable $@ may be null at this access because it has a nullable type. | E.cs:427:27:427:27 | i | i | E.cs:427:27:427:27 | i | this | | E.cs:444:13:444:13 | access to parameter s | Variable $@ may be null at this access as suggested by $@ null check. | E.cs:435:29:435:29 | s | s | E.cs:437:23:437:31 | ... is ... | this | +| E.cs:444:13:444:13 | access to parameter s | Variable $@ may be null at this access as suggested by $@ null check. | E.cs:435:29:435:29 | s | s | E.cs:447:23:447:35 | ... is ... | this | | E.cs:459:13:459:13 | access to parameter s | Variable $@ may be null at this access as suggested by $@ null check. | E.cs:435:29:435:29 | s | s | E.cs:437:23:437:31 | ... is ... | this | +| E.cs:459:13:459:13 | access to parameter s | Variable $@ may be null at this access as suggested by $@ null check. | E.cs:435:29:435:29 | s | s | E.cs:447:23:447:35 | ... is ... | this | | GuardedString.cs:35:31:35:31 | access to local variable s | Variable $@ may be null at this access because of $@ assignment. | GuardedString.cs:7:16:7:16 | s | s | GuardedString.cs:7:16:7:32 | String s = ... | this | | NullMaybeBad.cs:7:27:7:27 | access to parameter o | Variable $@ may be null at this access because of $@ null argument. | NullMaybeBad.cs:5:25:5:25 | o | o | NullMaybeBad.cs:13:17:13:20 | null | this | | Params.cs:14:17:14:20 | access to parameter args | Variable $@ may be null at this access because of $@ null argument. | Params.cs:12:36:12:39 | args | args | Params.cs:20:12:20:15 | null | this |