diff --git a/java/ql/lib/semmle/code/java/controlflow/Guards.qll b/java/ql/lib/semmle/code/java/controlflow/Guards.qll index 778ebe6e8789..a3cffbae459c 100644 --- a/java/ql/lib/semmle/code/java/controlflow/Guards.qll +++ b/java/ql/lib/semmle/code/java/controlflow/Guards.qll @@ -141,6 +141,7 @@ private predicate isNonFallThroughPredecessor(SwitchCase sc, ControlFlowNode pre private module GuardsInput implements SharedGuards::InputSig { private import java as J + private import semmle.code.java.dataflow.internal.BaseSSA private import semmle.code.java.dataflow.NullGuards as NullGuards import SuccessorType @@ -216,6 +217,12 @@ private module GuardsInput implements SharedGuards::InputSig { f.isFinal() and f.getInitializer() = NullGuards::baseNotNullExpr() ) + or + exists(CatchClause cc, LocalVariableDeclExpr decl, BaseSsaUpdate v | + decl = cc.getVariable() and + decl = v.getDefiningExpr() and + this = v.getAUse() + ) } } diff --git a/java/ql/lib/semmle/code/java/dataflow/Nullness.qll b/java/ql/lib/semmle/code/java/dataflow/Nullness.qll index 02f228d17dbe..756c5d1ae9f7 100644 --- a/java/ql/lib/semmle/code/java/dataflow/Nullness.qll +++ b/java/ql/lib/semmle/code/java/dataflow/Nullness.qll @@ -653,7 +653,7 @@ private Expr trackingVarGuard( result = integerGuard(trackvar.getAnAccess(), branch, k, isA) or exists(int k2 | - result = integerGuard(trackvar.getAnAccess(), branch.booleanNot(), k2, true) and + result = integerGuard(trackvar.getAnAccess(), branch, k2, true) and isA = false and k2 != k ) diff --git a/java/ql/src/change-notes/2025-08-22-nullness-fn.md b/java/ql/src/change-notes/2025-08-22-nullness-fn.md new file mode 100644 index 000000000000..d8d77a470f68 --- /dev/null +++ b/java/ql/src/change-notes/2025-08-22-nullness-fn.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Fixed a bug that was causing false negatives in rare cases in the query `java/dereferenced-value-may-be-null`. diff --git a/java/ql/test/query-tests/Nullness/B.java b/java/ql/test/query-tests/Nullness/B.java index 99bd6f4a1bae..b21d581535df 100644 --- a/java/ql/test/query-tests/Nullness/B.java +++ b/java/ql/test/query-tests/Nullness/B.java @@ -436,4 +436,83 @@ public void corrCondLoop2(boolean a[]) { } } } + + public void loopCorrTest1(int[] a) { + boolean ready = a.length > 7; + Object x = new Object(); + for (int i = 0; i < a.length; i++) { + // condition correlates with itself through iterations when ready isn't updated + if (!ready) { + x = null; + } else { + x.hashCode(); // Spurious NPE - false positive + } + if ((a[i] & 1) != 0) { + ready = (a[i] & 2) != 0; + x = new Object(); + } + } + } + + public void loopCorrTest2(boolean[] a) { + Object x = new Object(); + boolean cur = a[0]; + for (int i = 1; i < a.length; i++) { + boolean prev = cur; + cur = a[i]; + if (!prev) { + // correctly guarded by !cur from the _previous_ iteration + x.hashCode(); // Spurious NPE - false positive + } else { + x = new Object(); + } + if (cur) { + x = null; + } + } + } + + public void loopCorrTest3(String[] ss) { + Object x = null; + Object t = null; + for (String s : ss) { + if (t == null) { + t = s; + } else { + if (t instanceof String) { + x = new Object(); + t = new Object(); + } + // correctly guarded by t: null -> String -> Object + x.hashCode(); // Spurious NPE - false positive + } + } + } + + public void initCorr(boolean b) { + Object o2 = b ? null : ""; + if (b) + o2 = ""; + else + o2.hashCode(); // OK + } + + public void complexLoopTest(int[] xs, int[] ys) { + int len = ys != null ? ys.length : 0; + for (int i = 0, j = 0; i < xs.length; i++) { + if (j < len && ys[j] == 42) { // OK + j++; + } else if (j > 0) { + ys[0]++; // OK + } + } + } + + public void trackTest(Object o, int n) { + boolean isnull = o == null; + int c = -1; + if (maybe) { } + if (c == 100) { return; } + o.hashCode(); // NPE + } } diff --git a/java/ql/test/query-tests/Nullness/NullMaybe.expected b/java/ql/test/query-tests/Nullness/NullMaybe.expected index 9f2920293b07..f0d671d58bd0 100644 --- a/java/ql/test/query-tests/Nullness/NullMaybe.expected +++ b/java/ql/test/query-tests/Nullness/NullMaybe.expected @@ -18,6 +18,10 @@ | B.java:279:7:279:7 | a | Variable $@ may be null at this access because of $@ assignment. | B.java:276:5:276:19 | int[] a | a | B.java:276:11:276:18 | a | this | | B.java:292:7:292:7 | b | Variable $@ may be null at this access because of $@ assignment. | B.java:287:5:287:44 | int[] b | b | B.java:287:11:287:43 | b | this | | B.java:408:7:408:7 | x | Variable $@ may be null at this access as suggested by $@ null guard. | B.java:374:23:374:30 | x | x | B.java:375:23:375:31 | ... != ... | this | +| B.java:448:9:448:9 | x | Variable $@ may be null at this access because of $@ assignment. | B.java:442:5:442:28 | Object x | x | B.java:446:9:446:16 | ...=... | this | +| B.java:465:9:465:9 | x | Variable $@ may be null at this access because of $@ assignment. | B.java:458:5:458:28 | Object x | x | B.java:470:9:470:16 | ...=... | this | +| B.java:487:9:487:9 | x | Variable $@ may be null at this access because of $@ assignment. | B.java:476:5:476:20 | Object x | x | B.java:476:12:476:19 | x | this | +| B.java:516:5:516:5 | o | Variable $@ may be null at this access as suggested by $@ null guard. | B.java:511:25:511:32 | o | o | B.java:512:22:512:30 | ... == ... | this | | C.java:9:44:9:45 | a2 | Variable $@ may be null at this access as suggested by $@ null guard. | C.java:6:5:6:23 | long[][] a2 | a2 | C.java:7:34:7:54 | ... != ... | this | | C.java:9:44:9:45 | a2 | Variable $@ may be null at this access because of $@ assignment. | C.java:6:5:6:23 | long[][] a2 | a2 | C.java:6:14:6:22 | a2 | this | | C.java:10:17:10:18 | a3 | Variable $@ may be null at this access as suggested by $@ null guard. | C.java:8:5:8:21 | long[] a3 | a3 | C.java:9:38:9:58 | ... != ... | this |