Merge branch 'main' into experimental-strong-params

Author: thiggy1342

.github/workflows/check-change-note.yml

@@ -10,6 +10,7 @@ on:
       - "*/ql/lib/**/*.qll"
       - "!**/experimental/**"
       - "!ql/**"
+      - "!swift/**"
       - ".github/workflows/check-change-note.yml"
 
 jobs:

.github/workflows/ql-for-ql-build.yml

@@ -50,9 +50,6 @@ jobs:
           path: ${{ runner.temp }}/query-pack.zip
 
   extractors:
-    strategy:
-      fail-fast: false
-
     runs-on: ubuntu-latest
 
     steps:
@@ -195,9 +192,36 @@ jobs:
           category: "ql-for-ql-${{ matrix.folder }}"
       - name: Copy sarif file to CWD
         run: cp ../results/ql.sarif ./${{ matrix.folder }}.sarif
+      - name: Fixup the $scema in sarif  # Until https://github.com/microsoft/sarif-vscode-extension/pull/436/ is part in a stable release
+        run: |
+          sed -i 's/\$schema.*/\$schema": "https:\/\/raw.githubusercontent.com\/oasis-tcs\/sarif-spec\/master\/Schemata\/sarif-schema-2.1.0",/' ${{ matrix.folder }}.sarif 
       - name: Sarif as artifact
         uses: actions/upload-artifact@v3
         with:
           name: ${{ matrix.folder }}.sarif
           path: ${{ matrix.folder }}.sarif
 
+  combine:
+    runs-on: ubuntu-latest
+    needs:
+      - analyze
+
+    steps:
+      - uses: actions/checkout@v3
+      - name: Make a folder for artifacts. 
+        run: mkdir -p results
+      - name: Download all sarif files
+        uses: actions/download-artifact@v3
+        with: 
+            path: results
+      - uses: actions/setup-node@v3
+        with:
+            node-version: 16
+      - name: Combine all sarif files
+        run: | 
+          node ./ql/scripts/merge-sarif.js results/**/*.sarif combined.sarif
+      - name: Upload combined sarif file
+        uses: actions/upload-artifact@v3
+        with:
+            name: combined.sarif
+            path: combined.sarif

.github/workflows/ql-for-ql-dataset_measure.yml

@@ -36,7 +36,7 @@ jobs:
             ql/target
           key: ${{ runner.os }}-qltest-cargo-${{ hashFiles('ql/**/Cargo.lock') }}
       - name: Build Extractor
-        run: cd ql; env "PATH=$PATH:`dirname ${CODEQL}`" ./create-extractor-pack.sh
+        run: cd ql; env "PATH=$PATH:`dirname ${CODEQL}`" ./scripts/create-extractor-pack.sh
         env:
           CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
       - name: Checkout ${{ matrix.repo }}

.github/workflows/ql-for-ql-tests.yml

@@ -36,7 +36,7 @@ jobs:
         run: |
           cd ql;
           codeqlpath=$(dirname ${{ steps.find-codeql.outputs.codeql-path }});
-          env "PATH=$PATH:$codeqlpath" ./create-extractor-pack.sh
+          env "PATH=$PATH:$codeqlpath" ./scripts/create-extractor-pack.sh
       - name: Run QL tests
         run: |
           "${CODEQL}" test run --check-databases --check-unused-labels --check-repeated-labels --check-redefined-labels --check-use-before-definition --search-path "${{ github.workspace }}/ql/extractor-pack" --consistency-queries ql/ql/consistency-queries ql/ql/test

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/lib/semmle/code/cpp/exprs/Call.qll

@@ -255,8 +255,10 @@ class FunctionCall extends Call, @funbindexpr {
   /**
    * Gets the function called by this call.
    *
-   * In the case of virtual function calls, the result is the most-specific function in the override tree (as
-   * determined by the compiler) such that the target at runtime will be one of `result.getAnOverridingFunction*()`.
+   * In the case of virtual function calls, the result is the most-specific function in the override tree
+   * such that the target at runtime will be one of `result.getAnOverridingFunction*()`. The most-specific
+   * function is determined by the compiler based on the compile time type of the object the function is a
+   * member of.
    */
   override Function getTarget() { funbind(underlyingElement(this), unresolveElement(result)) }
 

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) {}
 }