Merge pull request #9785 from bdrodes/main

C++: Nullness.qll bug fixes

Author: jketema

cpp/ql/lib/change-notes/2022-07-12-cover-more-nullness-cases.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* `AnalysedExpr::isNullCheck` and `AnalysedExpr::isValidCheck` have been updated to handle variable accesses on the left-hand side of the the C++ logical and variable declarations in conditions.

cpp/ql/lib/semmle/code/cpp/controlflow/Nullness.qll

@@ -46,7 +46,7 @@ predicate nullCheckExpr(Expr checkExpr, Variable var) {
     or
     exists(LogicalAndExpr op, AnalysedExpr child |
       expr = op and
-      op.getRightOperand() = child and
+      op.getAnOperand() = child and
       nullCheckExpr(child, v)
     )
     or
@@ -99,7 +99,7 @@ predicate validCheckExpr(Expr checkExpr, Variable var) {
     or
     exists(LogicalAndExpr op, AnalysedExpr child |
       expr = op and
-      op.getRightOperand() = child and
+      op.getAnOperand() = child and
       validCheckExpr(child, v)
     )
     or
@@ -169,7 +169,10 @@ class AnalysedExpr extends Expr {
    */
   predicate isDef(LocalScopeVariable v) {
     this.inCondition() and
-    this.(Assignment).getLValue() = v.getAnAccess()
+    (
+      this.(Assignment).getLValue() = v.getAnAccess() or
+      this.(ConditionDeclExpr).getVariableAccess() = v.getAnAccess()
+    )
   }
 
   /**

cpp/ql/test/library-tests/controlflow/nullness/nullness.expected

@@ -7,9 +7,14 @@
 | test.cpp:15:8:15:23 | call to __builtin_expect | test.cpp:5:13:5:13 | v | is not null | is valid |
 | test.cpp:16:8:16:23 | call to __builtin_expect | test.cpp:5:13:5:13 | v | is null | is not valid |
 | test.cpp:17:9:17:17 | ... && ... | test.cpp:5:13:5:13 | v | is not null | is valid |
-| test.cpp:18:9:18:17 | ... && ... | test.cpp:5:13:5:13 | v | is not null | is not valid |
+| test.cpp:18:9:18:17 | ... && ... | test.cpp:5:13:5:13 | v | is not null | is valid |
 | test.cpp:19:9:19:18 | ... && ... | test.cpp:5:13:5:13 | v | is null | is not valid |
-| test.cpp:20:9:20:18 | ... && ... | test.cpp:5:13:5:13 | v | is not null | is not valid |
+| test.cpp:20:9:20:18 | ... && ... | test.cpp:5:13:5:13 | v | is null | is not valid |
 | test.cpp:21:9:21:14 | ... = ... | test.cpp:5:13:5:13 | v | is null | is not valid |
 | test.cpp:21:9:21:14 | ... = ... | test.cpp:7:10:7:10 | b | is not null | is valid |
-| test.cpp:22:17:22:17 | b | test.cpp:7:10:7:10 | b | is not null | is valid |
+| test.cpp:22:9:22:14 | ... = ... | test.cpp:5:13:5:13 | v | is not null | is not valid |
+| test.cpp:22:9:22:14 | ... = ... | test.cpp:7:13:7:13 | c | is not null | is not valid |
+| test.cpp:22:17:22:17 | c | test.cpp:7:13:7:13 | c | is not null | is valid |
+| test.cpp:23:21:23:21 | x | test.cpp:23:14:23:14 | x | is not null | is valid |
+| test.cpp:24:9:24:18 | (condition decl) | test.cpp:5:13:5:13 | v | is not null | is not valid |
+| test.cpp:24:9:24:18 | (condition decl) | test.cpp:24:14:24:14 | y | is not null | is valid |

cpp/ql/test/library-tests/controlflow/nullness/nullness.ql

@@ -2,7 +2,6 @@ import cpp
 
 from AnalysedExpr a, LocalScopeVariable v, string isNullCheck, string isValidCheck
 where
-  a.getParent() instanceof IfStmt and
   v.getAnAccess().getEnclosingStmt() = a.getParent() and
   (if a.isNullCheck(v) then isNullCheck = "is null" else isNullCheck = "is not null") and
   (if a.isValidCheck(v) then isValidCheck = "is valid" else isValidCheck = "is not valid")

cpp/ql/test/library-tests/controlflow/nullness/test.cpp

@@ -4,7 +4,7 @@ long __builtin_expect(long);
 
 void f(int *v) {
     int *w;
-    bool b;
+    bool b, c;
 
     if (v) {}
     if (!v) {}
@@ -19,5 +19,7 @@ void f(int *v) {
     if (true && !v) {}
     if (!v && true) {}
     if (b = !v) {}
-    if (b = !v; b) {}
+    if (c = !v; c) {}
+    if (int *x = v; x) {}
+    if (int *y = v) {}
 }