Python: Fix problem if import is used

I fixed it in both predicates... I think we might still be able to remove
`newDirectAlias` -- but with it being better, it will allow us to better test if `newImportAlias` actually cover everything we need!

.github/labeler.yml

@@ -45,7 +45,11 @@ documentation:
 
 # Since these are all shared files that need to be synced, just pick _one_ copy of each.
 "DataFlow Library":
-  - "shared/dataflow/**/*"
+  - "java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl.qll"
+  - "java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll"
+  - "java/ql/lib/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll"
+  - "java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll"
+  - "java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll"
 
 "ATM":
   - javascript/ql/experimental/adaptivethreatmodeling/**/*

.github/workflows/check-implicit-this.yml

@@ -13,7 +13,7 @@ jobs:
   check:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - name: Check that implicit this warnings is enabled for all packs
         shell: bash
         run: |

.github/workflows/check-qldoc.yml

@@ -15,7 +15,7 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
         with:
           fetch-depth: 2
 

.github/workflows/check-query-ids.yml

@@ -16,6 +16,6 @@ jobs:
     name: Check query IDs
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - name: Check for duplicate query IDs
         run: python3 misc/scripts/check-query-ids.py

.github/workflows/codeql-analysis.yml

@@ -33,7 +33,7 @@ jobs:
         dotnet-version: 7.0.102
 
     - name: Checkout repository
-      uses: actions/checkout@v4
+      uses: actions/checkout@v3
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL

.github/workflows/compile-queries.yml

@@ -13,7 +13,7 @@ jobs:
     runs-on: ubuntu-latest-xl
 
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - name: Setup CodeQL
         uses: ./.github/actions/fetch-codeql
         with:

.github/workflows/csharp-qltest.yml

@@ -29,7 +29,7 @@ jobs:
   qlupgrade:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: ./.github/actions/fetch-codeql
       - name: Check DB upgrade scripts
         run: |
@@ -52,7 +52,8 @@ jobs:
       matrix:
         slice: ["1/2", "2/2"]
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
+      - uses: ./.github/actions/fetch-codeql
       - uses: ./csharp/actions/create-extractor-pack
       - name: Cache compilation cache
         id: query-cache
@@ -61,41 +62,25 @@ jobs:
           key: csharp-qltest-${{ matrix.slice }}
       - name: Run QL tests
         run: |
-          codeql test run --threads=0 --ram 50000 --slice ${{ matrix.slice }} --search-path extractor-pack --check-databases --check-undefined-labels --check-repeated-labels --check-redefined-labels --consistency-queries ql/consistency-queries ql/test --compilation-cache "${{ steps.query-cache.outputs.cache-dir }}"
+          CODEQL_PATH=$(gh codeql version --format=json | jq -r .unpackedLocation)
+          # The legacy ASP extractor is not in this repo, so take the one from the nightly build
+          mv "$CODEQL_PATH/csharp/tools/extractor-asp.jar" "${{ github.workspace }}/csharp/extractor-pack/tools"
+          # Safe guard against using the bundled extractor
+          rm -rf "$CODEQL_PATH/csharp"
+          codeql test run --threads=0 --ram 50000 --slice ${{ matrix.slice }} --search-path "${{ github.workspace }}/csharp/extractor-pack" --check-databases --check-undefined-labels --check-repeated-labels --check-redefined-labels --consistency-queries ql/consistency-queries ql/test --compilation-cache "${{ steps.query-cache.outputs.cache-dir }}"
         env:
           GITHUB_TOKEN: ${{ github.token }}
   unit-tests:
-    strategy:
+    runs-on: ubuntu-latest
-      matrix:
-        os: [ubuntu-latest, windows-2019]
-    runs-on: ${{ matrix.os }}
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - name: Setup dotnet
         uses: actions/setup-dotnet@v3
         with:
           dotnet-version: 7.0.102
       - name: Extractor unit tests
         run: |
-          dotnet test -p:RuntimeFrameworkVersion=7.0.2 extractor/Semmle.Util.Tests
+          dotnet test -p:RuntimeFrameworkVersion=7.0.2 "${{ github.workspace }}/csharp/extractor/Semmle.Util.Tests"
-          dotnet test -p:RuntimeFrameworkVersion=7.0.2 extractor/Semmle.Extraction.Tests
+          dotnet test -p:RuntimeFrameworkVersion=7.0.2 "${{ github.workspace }}/csharp/extractor/Semmle.Extraction.Tests"
-          dotnet test -p:RuntimeFrameworkVersion=7.0.2 autobuilder/Semmle.Autobuild.CSharp.Tests
+          dotnet test -p:RuntimeFrameworkVersion=7.0.2 "${{ github.workspace }}/csharp/autobuilder/Semmle.Autobuild.CSharp.Tests"
           dotnet test -p:RuntimeFrameworkVersion=7.0.2 "${{ github.workspace }}/cpp/autobuilder/Semmle.Autobuild.Cpp.Tests"
-        shell: bash
-  stubgentest:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v4
-      - uses: ./csharp/actions/create-extractor-pack
-      - name: Run stub generator tests
-        run: |
-          # Generate (Asp)NetCore stubs
-          STUBS_PATH=stubs_output
-          python3 ql/src/Stubs/make_stubs_nuget.py webapp Swashbuckle.AspNetCore.Swagger latest "$STUBS_PATH"
-          rm -rf ql/test/resources/stubs/_frameworks
-          # Update existing stubs in the repo with the freshly generated ones
-          mv "$STUBS_PATH/output/stubs/_frameworks" ql/test/resources/stubs/
-          git status
-          codeql test run --threads=0 --search-path extractor-pack --check-databases --check-undefined-labels --check-repeated-labels --check-redefined-labels --consistency-queries ql/consistency-queries -- ql/test/library-tests/dataflow/flowsources/aspremote
-        env:
-          GITHUB_TOKEN: ${{ github.token }}

.github/workflows/csv-coverage-metrics.yml

@@ -19,7 +19,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout repository
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
       - name: Setup CodeQL
         uses: ./.github/actions/fetch-codeql
       - name: Create empty database
@@ -47,7 +47,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout repository
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
       - name: Setup CodeQL
         uses: ./.github/actions/fetch-codeql
       - name: Create empty database

.github/workflows/csv-coverage-pr-artifacts.yml

@@ -31,11 +31,11 @@ jobs:
           GITHUB_CONTEXT: ${{ toJSON(github.event) }}
         run: echo "$GITHUB_CONTEXT"
       - name: Clone self (github/codeql) - MERGE
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
         with:
           path: merge
       - name: Clone self (github/codeql) - BASE
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
         with:
           fetch-depth: 2
           path: base

.github/workflows/csv-coverage-pr-comment.yml

@@ -20,7 +20,7 @@ jobs:
         GITHUB_CONTEXT: ${{ toJSON(github.event) }}
       run: echo "$GITHUB_CONTEXT"
     - name: Clone self (github/codeql)
-      uses: actions/checkout@v4
+      uses: actions/checkout@v3
     - name: Set up Python 3.8
       uses: actions/setup-python@v4
       with:

.github/workflows/csv-coverage-timeseries.yml

@@ -9,11 +9,11 @@ jobs:
 
     steps:
       - name: Clone self (github/codeql)
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
         with:
           path: script
       - name: Clone self (github/codeql) for analysis
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
         with:
           path: codeqlModels
           fetch-depth: 0

.github/workflows/csv-coverage-update.yml

@@ -17,7 +17,7 @@ jobs:
           GITHUB_CONTEXT: ${{ toJSON(github.event) }}
         run: echo "$GITHUB_CONTEXT"
       - name: Clone self (github/codeql)
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
         with:
           path: ql
           fetch-depth: 0

.github/workflows/csv-coverage.yml

@@ -13,11 +13,11 @@ jobs:
 
     steps:
       - name: Clone self (github/codeql)
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
         with:
           path: script
       - name: Clone self (github/codeql) for analysis
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
         with:
           path: codeqlModels
           ref: ${{ github.event.inputs.qlModelShaOverride || github.ref }}

.github/workflows/fast-forward.yml

@@ -25,7 +25,7 @@ jobs:
           exit 1
 
       - name: Checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
 
       - name: Git config
         shell: bash

.github/workflows/go-tests-other-os.yml

@@ -21,7 +21,7 @@ jobs:
         id: go
 
       - name: Check out code
-        uses: actions/checkout@v4
+        uses: actions/checkout@v2
 
       - name: Set up CodeQL CLI
         uses: ./.github/actions/fetch-codeql
@@ -56,7 +56,7 @@ jobs:
         id: go
 
       - name: Check out code
-        uses: actions/checkout@v4
+        uses: actions/checkout@v2
 
       - name: Set up CodeQL CLI
         uses: ./.github/actions/fetch-codeql

.github/workflows/go-tests.yml

@@ -29,7 +29,7 @@ jobs:
         id: go
 
       - name: Check out code
-        uses: actions/checkout@v4
+        uses: actions/checkout@v2
 
       - name: Set up CodeQL CLI
         uses: ./.github/actions/fetch-codeql

.github/workflows/js-ml-tests.yml

@@ -0,0 +1,65 @@
+name: JS ML-powered queries tests
+
+on:
+  push:
+    paths:
+      - "javascript/ql/experimental/adaptivethreatmodeling/**"
+      - .github/workflows/js-ml-tests.yml
+      - .github/actions/fetch-codeql/action.yml
+      - codeql-workspace.yml
+    branches:
+      - main
+      - "rc/*"
+  pull_request:
+    paths:
+      - "javascript/ql/experimental/adaptivethreatmodeling/**"
+      - .github/workflows/js-ml-tests.yml
+      - .github/actions/fetch-codeql/action.yml
+      - codeql-workspace.yml
+  workflow_dispatch:
+
+defaults:
+  run:
+    working-directory: javascript/ql/experimental/adaptivethreatmodeling
+
+jobs:
+  qltest:
+    name: Test QL
+    runs-on: ubuntu-latest-xl
+    steps:
+      - uses: actions/checkout@v3
+
+      - uses: ./.github/actions/fetch-codeql
+
+      - name: Install pack dependencies
+        run: |
+          for pack in modelbuilding src test; do
+            codeql pack install --mode verify -- "${pack}"
+          done
+      
+      - name: Cache compilation cache
+        id: query-cache
+        uses: ./.github/actions/cache-query-compilation
+        with: 
+          key: js-ml-test
+
+      - name: Check QL compilation
+        run: |
+          codeql query compile \
+            --check-only \
+            --ram 50000 \
+            --additional-packs "${{ github.workspace }}" \
+            --threads=0 \
+            --compilation-cache "${{ steps.query-cache.outputs.cache-dir }}" \
+            -- \
+            lib modelbuilding src
+
+      - name: Run QL tests
+        run: |
+          codeql test run \
+            --threads=0 \
+            --ram 50000 \
+            --additional-packs "${{ github.workspace }}" \
+            --compilation-cache "${{ steps.query-cache.outputs.cache-dir }}" \
+            -- \
+            test

.github/workflows/mad_modelDiff.yml

@@ -27,12 +27,12 @@ jobs:
         slug: ${{fromJson(github.event.inputs.projects || '["apache/commons-codec", "apache/commons-io", "apache/commons-beanutils", "apache/commons-logging", "apache/commons-fileupload", "apache/commons-lang", "apache/commons-validator", "apache/commons-csv", "apache/dubbo"]' )}}
     steps:
       - name: Clone github/codeql from PR
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
         if: github.event.pull_request
         with:
           path: codeql-pr
       - name: Clone github/codeql from main
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
         with:
           path: codeql-main
           ref: main

.github/workflows/mad_regenerate-models.yml

@@ -27,11 +27,11 @@ jobs:
             ref: "placeholder"
     steps:
       - name: Clone self (github/codeql)
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
       - name: Setup CodeQL binaries
         uses: ./.github/actions/fetch-codeql
       - name: Clone repositories
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
         with:
           path: repos/${{ matrix.ref }}
           ref: ${{ matrix.ref }}

.github/workflows/qhelp-pr-preview.yml

@@ -43,7 +43,7 @@ jobs:
           if-no-files-found: error
           retention-days: 1
 
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
         with:
           fetch-depth: 2
           persist-credentials: false

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

@@ -14,7 +14,7 @@ jobs:
     runs-on: ubuntu-latest-xl
     steps:
       ### Build the queries ###
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
         with:
           fetch-depth: 0
       - name: Find codeql

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

@@ -21,7 +21,7 @@ jobs:
           - github/codeql
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
 
       - name: Find codeql
         id: find-codeql
@@ -42,7 +42,7 @@ jobs:
         env:
           CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
       - name: Checkout ${{ matrix.repo }}
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
         with:
           repository: ${{ matrix.repo }}
           path: ${{ github.workspace }}/repo
@@ -71,7 +71,7 @@ jobs:
     runs-on: ubuntu-latest
     needs: measure
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: actions/download-artifact@v3
         with:
           name: measurements

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

@@ -21,7 +21,7 @@ jobs:
   qltest:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - name: Find codeql
         id: find-codeql
         uses: github/codeql-action/init@v2
@@ -61,7 +61,7 @@ jobs:
     needs: [qltest]
     runs-on: ${{ matrix.os }}
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - name: Install GNU tar 
         if: runner.os == 'macOS'
         run: |

.github/workflows/query-list.yml

@@ -20,7 +20,7 @@ jobs:
 
     steps:
     - name: Clone self (github/codeql)
-      uses: actions/checkout@v4
+      uses: actions/checkout@v3
       with:
         path: codeql 
     - name: Set up Python 3.8

.github/workflows/ruby-build.yml

@@ -42,7 +42,7 @@ jobs:
     runs-on: ${{ matrix.os }}
 
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - name: Install GNU tar
         if: runner.os == 'macOS'
         run: |
@@ -113,7 +113,7 @@ jobs:
   compile-queries:
     runs-on: ubuntu-latest-xl
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - name: Fetch CodeQL
         uses: ./.github/actions/fetch-codeql
       - name: Cache compilation cache
@@ -145,7 +145,7 @@ jobs:
     runs-on: ubuntu-latest
     needs: [build, compile-queries]
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: actions/download-artifact@v3
         with:
           name: ruby.dbscheme
@@ -206,7 +206,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     needs: [package]
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - name: Fetch CodeQL
         uses: ./.github/actions/fetch-codeql
 

.github/workflows/ruby-dataset-measure.yml

@@ -27,14 +27,14 @@ jobs:
         repo: [rails/rails, discourse/discourse, spree/spree, ruby/ruby]
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
 
       - uses: ./.github/actions/fetch-codeql
 
       - uses: ./ruby/actions/create-extractor-pack
 
       - name: Checkout ${{ matrix.repo }}
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
         with:
           repository: ${{ matrix.repo }}
           path: ${{ github.workspace }}/repo
@@ -59,7 +59,7 @@ jobs:
     runs-on: ubuntu-latest
     needs: measure
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: actions/download-artifact@v3
         with:
           name: measurements

.github/workflows/ruby-qltest.yml

@@ -33,7 +33,7 @@ jobs:
   qlupgrade:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: ./.github/actions/fetch-codeql
       - name: Check DB upgrade scripts
         run: |
@@ -54,7 +54,7 @@ jobs:
     strategy:
       fail-fast: false
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: ./.github/actions/fetch-codeql
       - uses: ./ruby/actions/create-extractor-pack
       - name: Cache compilation cache

.github/workflows/swift.yml

@@ -39,31 +39,31 @@ jobs:
   build-and-test-macos:
     runs-on: macos-12-xl
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: ./swift/actions/build-and-test
   build-and-test-linux:
     runs-on: ubuntu-latest-xl
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: ./swift/actions/build-and-test
   qltests-linux:
     needs: build-and-test-linux
     runs-on: ubuntu-latest-xl
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: ./swift/actions/run-ql-tests
   qltests-macos:
     if : ${{ github.event_name == 'pull_request' }}
     needs: build-and-test-macos
     runs-on: macos-12-xl
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: ./swift/actions/run-ql-tests
   integration-tests-linux:
     needs: build-and-test-linux
     runs-on: ubuntu-latest-xl
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: ./swift/actions/run-integration-tests
   integration-tests-macos:
     if : ${{ github.event_name == 'pull_request' }}
@@ -71,13 +71,13 @@ jobs:
     runs-on: macos-12-xl
     timeout-minutes: 60
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: ./swift/actions/run-integration-tests
   codegen:
     if : ${{ github.event_name == 'pull_request' }}
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: bazelbuild/setup-bazelisk@v2
       - uses: actions/setup-python@v4
         with:
@@ -102,6 +102,6 @@ jobs:
     if : ${{ github.event_name == 'pull_request' }}
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - uses: ./.github/actions/fetch-codeql
       - uses: ./swift/actions/database-upgrade-scripts

.github/workflows/sync-files.yml

@@ -14,7 +14,7 @@ jobs:
   sync:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - name: Check synchronized files
         run: python config/sync-files.py
       - name: Check dbscheme fragments

.github/workflows/tree-sitter-extractor-test.yml

@@ -27,7 +27,7 @@ jobs:
   test:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - name: Check formatting
         run: cargo fmt --all -- --check
       - name: Run tests
@@ -35,12 +35,12 @@ jobs:
   fmt:
     runs-on: ubuntu-latest  
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - name: Check formatting
         run: cargo fmt --check
   clippy:
     runs-on: ubuntu-latest  
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v3
       - name: Run clippy
         run: cargo clippy -- --no-deps -D warnings -A clippy::new_without_default -A clippy::too_many_arguments

.github/workflows/validate-change-notes.yml

@@ -20,7 +20,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout repository
-        uses: actions/checkout@v4
+        uses: actions/checkout@v3
 
       - name: Setup CodeQL
         uses: ./.github/actions/fetch-codeql

config/identical-files.json

@@ -28,6 +28,8 @@
     "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl1.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl2.qll",
+    "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplForHttpClientLibraries.qll",
+    "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplForPathname.qll",
     "swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImpl1.qll"
   ],
   "TaintTracking Legacy Configuration Java/C++/C#/Go/Python/Ruby/Swift": [
@@ -550,4 +552,4 @@
     "python/ql/test/experimental/dataflow/model-summaries/InlineTaintTest.ext.yml",
     "python/ql/test/experimental/dataflow/model-summaries/NormalDataflowTest.ext.yml"
   ]
-}
+}

cpp/downgrades/dbe9c8eb5fc6f54b7ae08c7317d0795b24961564/upgrade.properties

@@ -1,2 +0,0 @@
-description: Make __is_trivial a builtin operation
-compatibility: full

cpp/ql/lib/CHANGELOG.md

@@ -1,3 +1,18 @@
+## 0.9.2
+
+### Deprecated APIs
+
+* `getAllocatorCall` on `DeleteExpr` and `DeleteArrayExpr` has been deprecated. `getDeallocatorCall` should be used instead.
+
+### New Features
+
+* Added `DeleteOrDeleteArrayExpr` as a super type of `DeleteExpr` and `DeleteArrayExpr`
+
+### Minor Analysis Improvements
+
+* `delete` and `delete[]` are now modeled as calls to the relevant `operator delete` in the IR. In the case of a dynamic delete call a new instruction `VirtualDeleteFunctionAddress` is used to represent a function that dispatches to the correct delete implementation.
+* Only the 2 level indirection of `argv` (corresponding to `**argv`) is consided for `FlowSource`.
+
 ## 0.9.1
 
 No user-facing changes.

cpp/ql/lib/change-notes/2023-08-24-no-taint-argv-indirections.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* Only the 2 level indirection of `argv` (corresponding to `**argv`) is consided for `FlowSource`.

cpp/ql/lib/change-notes/2023-08-25-delete-or-delete-array.md

@@ -1,4 +0,0 @@
----
-category: feature
----
-* Added `DeleteOrDeleteArrayExpr` as a super type of `DeleteExpr` and `DeleteArrayExpr`

cpp/ql/lib/change-notes/2023-08-25-getAllocatorCall-deprecated.md

@@ -1,4 +0,0 @@
----
-category: deprecated
----
-* `getAllocatorCall` on `DeleteExpr` and `DeleteArrayExpr` has been deprecated. `getDeallocatorCall` should be used instead.

cpp/ql/lib/change-notes/2023-08-29-delete-ir.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* `delete` and `delete[]` are now modeled as calls to the relevant `operator delete` in the IR. In the case of a dynamic delete call a new instruction `VirtualDeleteFunctionAddress` is used to represent a function that dispatches to the correct delete implementation.

cpp/ql/lib/change-notes/2023-09-06-as-defining-argument-off-by-one-fix.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* The `DataFlow::asDefiningArgument` predicate now takes its argument from the range starting at `1` instead of `2`. Queries that depend on the single-parameter version of `DataFlow::asDefiningArgument` should have their arguments updated accordingly.

cpp/ql/lib/change-notes/2023-09-07-return-from-end.md

@@ -1,5 +0,0 @@
----
-category: minorAnalysis
----
-* Treat functions that reach the end of the function as returning in the IR.
-  They used to be treated as unreachable but it is allowed in C. 

cpp/ql/lib/change-notes/2023-09-08-more-unreachble.md

@@ -1,5 +0,0 @@
----
-category: minorAnalysis
----
-* Functions that do not return due to calling functions that don't return (e.g. `exit`) are now detected as
- non-returning in the IR and dataflow.

cpp/ql/lib/change-notes/released/0.9.2.md

@@ -0,0 +1,14 @@
+## 0.9.2
+
+### Deprecated APIs
+
+* `getAllocatorCall` on `DeleteExpr` and `DeleteArrayExpr` has been deprecated. `getDeallocatorCall` should be used instead.
+
+### New Features
+
+* Added `DeleteOrDeleteArrayExpr` as a super type of `DeleteExpr` and `DeleteArrayExpr`
+
+### Minor Analysis Improvements
+
+* `delete` and `delete[]` are now modeled as calls to the relevant `operator delete` in the IR. In the case of a dynamic delete call a new instruction `VirtualDeleteFunctionAddress` is used to represent a function that dispatches to the correct delete implementation.
+* Only the 2 level indirection of `argv` (corresponding to `**argv`) is consided for `FlowSource`.

cpp/ql/lib/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.9.1
+lastReleaseVersion: 0.9.2

cpp/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-all
-version: 0.9.2-dev
+version: 0.9.2
 groups: cpp
 dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp

cpp/ql/lib/semmle/code/cpp/File.qll

@@ -5,35 +5,155 @@
 import semmle.code.cpp.Element
 import semmle.code.cpp.Declaration
 import semmle.code.cpp.metrics.MetricFile
-private import codeql.util.FileSystem
-
-private module Input implements InputSig {
-  abstract class ContainerBase extends @container {
-    abstract string getAbsolutePath();
-
-    ContainerBase getParentContainer() {
-      containerparent(unresolveElement(result), underlyingElement(this))
-    }
-
-    string toString() { result = this.getAbsolutePath() }
-  }
-
-  class FolderBase extends ContainerBase, @folder {
-    override string getAbsolutePath() { folders(underlyingElement(this), result) }
-  }
-
-  class FileBase extends ContainerBase, @file {
-    override string getAbsolutePath() { files(underlyingElement(this), result) }
-  }
-
-  predicate hasSourceLocationPrefix = sourceLocationPrefix/1;
-}
-
-private module Impl = Make<Input>;
 
 /** A file or folder. */
-class Container extends Locatable, Impl::Container {
+class Container extends Locatable, @container {
-  override string toString() { result = Impl::Container.super.toString() }
+  /**
+   * Gets the absolute, canonical path of this container, using forward slashes
+   * as path separator.
+   *
+   * The path starts with a _root prefix_ followed by zero or more _path
+   * segments_ separated by forward slashes.
+   *
+   * The root prefix is of one of the following forms:
+   *
+   *   1. A single forward slash `/` (Unix-style)
+   *   2. An upper-case drive letter followed by a colon and a forward slash,
+   *      such as `C:/` (Windows-style)
+   *   3. Two forward slashes, a computer name, and then another forward slash,
+   *      such as `//FileServer/` (UNC-style)
+   *
+   * Path segments are never empty (that is, absolute paths never contain two
+   * contiguous slashes, except as part of a UNC-style root prefix). Also, path
+   * segments never contain forward slashes, and no path segment is of the
+   * form `.` (one dot) or `..` (two dots).
+   *
+   * Note that an absolute path never ends with a forward slash, except if it is
+   * a bare root prefix, that is, the path has no path segments. A container
+   * whose absolute path has no segments is always a `Folder`, not a `File`.
+   */
+  string getAbsolutePath() { none() } // overridden by subclasses
+
+  /**
+   * Gets the relative path of this file or folder from the root folder of the
+   * analyzed source location. The relative path of the root folder itself is
+   * the empty string.
+   *
+   * This has no result if the container is outside the source root, that is,
+   * if the root folder is not a reflexive, transitive parent of this container.
+   */
+  string getRelativePath() {
+    exists(string absPath, string pref |
+      absPath = this.getAbsolutePath() and sourceLocationPrefix(pref)
+    |
+      absPath = pref and result = ""
+      or
+      absPath = pref.regexpReplaceAll("/$", "") + "/" + result and
+      not result.matches("/%")
+    )
+  }
+
+  /**
+   * Gets the base name of this container including extension, that is, the last
+   * segment of its absolute path, or the empty string if it has no segments.
+   *
+   * Here are some examples of absolute paths and the corresponding base names
+   * (surrounded with quotes to avoid ambiguity):
+   *
+   * <table border="1">
+   * <tr><th>Absolute path</th><th>Base name</th></tr>
+   * <tr><td>"/tmp/tst.js"</td><td>"tst.js"</td></tr>
+   * <tr><td>"C:/Program Files (x86)"</td><td>"Program Files (x86)"</td></tr>
+   * <tr><td>"/"</td><td>""</td></tr>
+   * <tr><td>"C:/"</td><td>""</td></tr>
+   * <tr><td>"D:/"</td><td>""</td></tr>
+   * <tr><td>"//FileServer/"</td><td>""</td></tr>
+   * </table>
+   */
+  string getBaseName() {
+    result = this.getAbsolutePath().regexpCapture(".*/(([^/]*?)(?:\\.([^.]*))?)", 1)
+  }
+
+  /**
+   * Gets the extension of this container, that is, the suffix of its base name
+   * after the last dot character, if any.
+   *
+   * In particular,
+   *
+   *  - if the name does not include a dot, there is no extension, so this
+   *    predicate has no result;
+   *  - if the name ends in a dot, the extension is the empty string;
+   *  - if the name contains multiple dots, the extension follows the last dot.
+   *
+   * Here are some examples of absolute paths and the corresponding extensions
+   * (surrounded with quotes to avoid ambiguity):
+   *
+   * <table border="1">
+   * <tr><th>Absolute path</th><th>Extension</th></tr>
+   * <tr><td>"/tmp/tst.js"</td><td>"js"</td></tr>
+   * <tr><td>"/tmp/.classpath"</td><td>"classpath"</td></tr>
+   * <tr><td>"/bin/bash"</td><td>not defined</td></tr>
+   * <tr><td>"/tmp/tst2."</td><td>""</td></tr>
+   * <tr><td>"/tmp/x.tar.gz"</td><td>"gz"</td></tr>
+   * </table>
+   */
+  string getExtension() {
+    result = this.getAbsolutePath().regexpCapture(".*/([^/]*?)(\\.([^.]*))?", 3)
+  }
+
+  /**
+   * Gets the stem of this container, that is, the prefix of its base name up to
+   * (but not including) the last dot character if there is one, or the entire
+   * base name if there is not.
+   *
+   * Here are some examples of absolute paths and the corresponding stems
+   * (surrounded with quotes to avoid ambiguity):
+   *
+   * <table border="1">
+   * <tr><th>Absolute path</th><th>Stem</th></tr>
+   * <tr><td>"/tmp/tst.js"</td><td>"tst"</td></tr>
+   * <tr><td>"/tmp/.classpath"</td><td>""</td></tr>
+   * <tr><td>"/bin/bash"</td><td>"bash"</td></tr>
+   * <tr><td>"/tmp/tst2."</td><td>"tst2"</td></tr>
+   * <tr><td>"/tmp/x.tar.gz"</td><td>"x.tar"</td></tr>
+   * </table>
+   */
+  string getStem() {
+    result = this.getAbsolutePath().regexpCapture(".*/([^/]*?)(?:\\.([^.]*))?", 1)
+  }
+
+  /** Gets the parent container of this file or folder, if any. */
+  Container getParentContainer() {
+    containerparent(unresolveElement(result), underlyingElement(this))
+  }
+
+  /** Gets a file or sub-folder in this container. */
+  Container getAChildContainer() { this = result.getParentContainer() }
+
+  /** Gets a file in this container. */
+  File getAFile() { result = this.getAChildContainer() }
+
+  /** Gets the file in this container that has the given `baseName`, if any. */
+  File getFile(string baseName) {
+    result = this.getAFile() and
+    result.getBaseName() = baseName
+  }
+
+  /** Gets a sub-folder in this container. */
+  Folder getAFolder() { result = this.getAChildContainer() }
+
+  /** Gets the sub-folder in this container that has the given `baseName`, if any. */
+  Folder getFolder(string baseName) {
+    result = this.getAFolder() and
+    result.getBaseName() = baseName
+  }
+
+  /**
+   * Gets a textual representation of the path of this container.
+   *
+   * This is the absolute path of the container.
+   */
+  override string toString() { result = this.getAbsolutePath() }
 }
 
 /**
@@ -46,7 +166,9 @@ class Container extends Locatable, Impl::Container {
  *
  * To get the full path, use `getAbsolutePath`.
  */
-class Folder extends Container, Impl::Folder {
+class Folder extends Container, @folder {
+  override string getAbsolutePath() { folders(underlyingElement(this), result) }
+
   override Location getLocation() {
     result.getContainer() = this and
     result.hasLocationInfo(_, 0, 0, 0, 0)
@@ -67,7 +189,9 @@ class Folder extends Container, Impl::Folder {
  * The base name further decomposes into the _stem_ and _extension_ -- see
  * `getStem` and `getExtension`. To get the full path, use `getAbsolutePath`.
  */
-class File extends Container, Impl::File {
+class File extends Container, @file {
+  override string getAbsolutePath() { files(underlyingElement(this), result) }
+
   override string getAPrimaryQlClass() { result = "File" }
 
   override Location getLocation() {

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

@@ -26,18 +26,17 @@ predicate callDereferences(FunctionCall fc, int i) {
 }
 
 /**
- * Holds if evaluation of `op` dereferences `e` directly.
+ * Holds if evaluation of `op` dereferences `e`.
- *
- * This predicate does not recurse through function calls or arithmetic operations. To find
- * such cases, use `dereferencedByOperation`.
  */
-predicate directDereferencedByOperation(Expr op, Expr e) {
+predicate dereferencedByOperation(Expr op, Expr e) {
   exists(PointerDereferenceExpr deref |
     deref.getAChild() = e and
     deref = op and
     not deref.getParent*() instanceof SizeofOperator
   )
   or
+  exists(CrementOperation crement | dereferencedByOperation(e, op) and crement.getOperand() = e)
+  or
   exists(ArrayExpr ae |
     (
       not ae.getParent() instanceof AddressOfExpr and
@@ -51,24 +50,6 @@ predicate directDereferencedByOperation(Expr op, Expr e) {
     )
   )
   or
-  // ptr->Field
-  e = op.(FieldAccess).getQualifier() and isClassPointerType(e.getType())
-  or
-  // ptr->method()
-  e = op.(Call).getQualifier() and isClassPointerType(e.getType())
-}
-
-/**
- * Holds if evaluation of `op` dereferences `e`.
- *
- * This includes the set of operations identified via `directDereferencedByOperation`, as well
- * as calls to function that are known to dereference an argument.
- */
-predicate dereferencedByOperation(Expr op, Expr e) {
-  directDereferencedByOperation(op, e)
-  or
-  exists(CrementOperation crement | dereferencedByOperation(e, op) and crement.getOperand() = e)
-  or
   exists(AddressOfExpr addof, ArrayExpr ae |
     dereferencedByOperation(addof, op) and
     addof.getOperand() = ae and
@@ -93,6 +74,12 @@ predicate dereferencedByOperation(Expr op, Expr e) {
     e = fc.getArgument(i) and
     op = fc
   )
+  or
+  // ptr->Field
+  e = op.(FieldAccess).getQualifier() and isClassPointerType(e.getType())
+  or
+  // ptr->method()
+  e = op.(Call).getQualifier() and isClassPointerType(e.getType())
 }
 
 private predicate isClassPointerType(Type t) {

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

@@ -240,7 +240,7 @@ private class GuardConditionFromIR extends GuardCondition {
    */
   private predicate controlsBlock(BasicBlock controlled, boolean testIsTrue) {
     exists(IRBlock irb |
-      ir.controls(irb, testIsTrue) and
+      forex(IRGuardCondition inst | inst = ir | inst.controls(irb, testIsTrue)) and
       irb.getAnInstruction().getAst().(ControlFlowNode).getBasicBlock() = controlled and
       not isUnreachedBlock(irb)
     )

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowDispatch.qll

@@ -79,3 +79,13 @@ class ArgumentPosition extends int {
 /** Holds if arguments at position `apos` match parameters at position `ppos`. */
 pragma[inline]
 predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos = apos }
+
+/**
+ * Holds if flow from `call`'s argument `arg` to parameter `p` is permissible.
+ *
+ * This is a temporary hook to support technical debt in the Go language; do not use.
+ */
+pragma[inline]
+predicate golangSpecificParamArgFilter(DataFlowCall call, ParameterNode p, ArgumentNode arg) {
+  any()
+}

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl1.qll

@@ -297,10 +297,6 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
-  predicate isBarrierIn(Node node, FlowState state) { none() }
-
-  predicate isBarrierOut(Node node, FlowState state) { none() }
-
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll

@@ -297,10 +297,6 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
-  predicate isBarrierIn(Node node, FlowState state) { none() }
-
-  predicate isBarrierOut(Node node, FlowState state) { none() }
-
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll

@@ -297,10 +297,6 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
-  predicate isBarrierIn(Node node, FlowState state) { none() }
-
-  predicate isBarrierOut(Node node, FlowState state) { none() }
-
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll

@@ -297,10 +297,6 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
-  predicate isBarrierIn(Node node, FlowState state) { none() }
-
-  predicate isBarrierOut(Node node, FlowState state) { none() }
-
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll

@@ -297,10 +297,6 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
-  predicate isBarrierIn(Node node, FlowState state) { none() }
-
-  predicate isBarrierOut(Node node, FlowState state) { none() }
-
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll

@@ -208,8 +208,6 @@ predicate expectsContent(Node n, ContentSet c) { none() }
 
 predicate typeStrongerThan(DataFlowType t1, DataFlowType t2) { none() }
 
-predicate localMustFlowStep(Node node1, Node node2) { none() }
-
 /** Gets the type of `n` used for type pruning. */
 Type getNodeType(Node n) {
   suppressUnusedNode(n) and
@@ -297,3 +295,12 @@ class ContentApprox = Unit;
 /** Gets an approximated value for content `c`. */
 pragma[inline]
 ContentApprox getContentApprox(Content c) { any() }
+
+/**
+ * Gets an additional term that is added to the `join` and `branch` computations to reflect
+ * an additional forward or backwards branching factor that is not taken into account
+ * when calculating the (virtual) dispatch cost.
+ *
+ * Argument `arg` is part of a path from a source to a sink, and `p` is the target parameter.
+ */
+int getAdditionalFlowIntoCallNodeTerm(ArgumentNode arg, ParameterNode p) { none() }

cpp/ql/lib/semmle/code/cpp/exprs/BuiltInOperations.qll

@@ -1547,21 +1547,3 @@ class BuiltInBitCast extends BuiltInOperation, @builtinbitcast {
 
   override string getAPrimaryQlClass() { result = "BuiltInBitCast" }
 }
-
-/**
- * A C++ `__is_trivial` built-in operation (used by some implementations of the
- * `<type_traits>` header).
- *
- * Returns `true` if a type is a trivial type.
- * ```
- *  template<typename _Tp>
- *    struct is_trivial
- *    : public integral_constant<bool, __is_trivial(_Tp)>
- *    {};
- * ```
- */
-class BuiltInIsTrivial extends BuiltInOperation, @istrivialexpr {
-  override string toString() { result = "__is_trivial" }
-
-  override string getAPrimaryQlClass() { result = "BuiltInIsTrivial" }
-}

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowDispatch.qll

@@ -271,3 +271,13 @@ DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
 /** Holds if arguments at position `apos` match parameters at position `ppos`. */
 pragma[inline]
 predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos = apos }
+
+/**
+ * Holds if flow from `call`'s argument `arg` to parameter `p` is permissible.
+ *
+ * This is a temporary hook to support technical debt in the Go language; do not use.
+ */
+pragma[inline]
+predicate golangSpecificParamArgFilter(DataFlowCall call, ParameterNode p, ArgumentNode arg) {
+  any()
+}

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl1.qll

@@ -297,10 +297,6 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
-  predicate isBarrierIn(Node node, FlowState state) { none() }
-
-  predicate isBarrierOut(Node node, FlowState state) { none() }
-
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll

@@ -297,10 +297,6 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
-  predicate isBarrierIn(Node node, FlowState state) { none() }
-
-  predicate isBarrierOut(Node node, FlowState state) { none() }
-
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll

@@ -297,10 +297,6 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
-  predicate isBarrierIn(Node node, FlowState state) { none() }
-
-  predicate isBarrierOut(Node node, FlowState state) { none() }
-
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll

@@ -297,10 +297,6 @@ private module Config implements FullStateConfigSig {
 
   predicate isBarrierOut(Node node) { any(Configuration config).isBarrierOut(node) }
 
-  predicate isBarrierIn(Node node, FlowState state) { none() }
-
-  predicate isBarrierOut(Node node, FlowState state) { none() }
-
   predicate isAdditionalFlowStep(Node node1, Node node2) {
     singleConfiguration() and
     any(Configuration config).isAdditionalFlowStep(node1, node2)

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplSpecific.qll

@@ -18,6 +18,4 @@ module CppDataFlow implements InputSig {
   import Public
 
   Node exprNode(DataFlowExpr e) { result = Public::exprNode(e) }
-
-  predicate getAdditionalFlowIntoCallNodeTerm = Private::getAdditionalFlowIntoCallNodeTerm/2;
 }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

@@ -804,8 +804,6 @@ predicate expectsContent(Node n, ContentSet c) { none() }
 
 predicate typeStrongerThan(DataFlowType t1, DataFlowType t2) { none() }
 
-predicate localMustFlowStep(Node node1, Node node2) { none() }
-
 /** Gets the type of `n` used for type pruning. */
 DataFlowType getNodeType(Node n) {
   suppressUnusedNode(n) and

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

@@ -193,23 +193,13 @@ class Node extends TIRDataFlowNode {
    * a `Conversion`, then the result is the underlying non-`Conversion` base
    * expression.
    */
-  Expr asExpr() { result = this.asExpr(_) }
+  Expr asExpr() { result = this.(ExprNode).getExpr() }
-
-  /**
-   * INTERNAL: Do not use.
-   */
-  Expr asExpr(int n) { result = this.(ExprNode).getExpr(n) }
-
-  /**
-   * INTERNAL: Do not use.
-   */
-  Expr asIndirectExpr(int n, int index) { result = this.(IndirectExprNode).getExpr(n, index) }
 
   /**
    * Gets the non-conversion expression that's indirectly tracked by this node
    * under `index` number of indirections.
    */
-  Expr asIndirectExpr(int index) { result = this.asIndirectExpr(_, index) }
+  Expr asIndirectExpr(int index) { result = this.(IndirectExprNode).getExpr(index) }
 
   /**
    * Gets the non-conversion expression that's indirectly tracked by this node
@@ -221,26 +211,15 @@ class Node extends TIRDataFlowNode {
    * Gets the expression corresponding to this node, if any. The returned
    * expression may be a `Conversion`.
    */
-  Expr asConvertedExpr() { result = this.asConvertedExpr(_) }
+  Expr asConvertedExpr() { result = this.(ExprNode).getConvertedExpr() }
-
-  /**
-   * Gets the expression corresponding to this node, if any. The returned
-   * expression may be a `Conversion`.
-   */
-  Expr asConvertedExpr(int n) { result = this.(ExprNode).getConvertedExpr(n) }
-
-  /**
-   * INTERNAL: Do not use.
-   */
-  Expr asIndirectConvertedExpr(int n, int index) {
-    result = this.(IndirectExprNode).getConvertedExpr(n, index)
-  }
 
   /**
    * Gets the expression that's indirectly tracked by this node
    * behind `index` number of indirections.
    */
-  Expr asIndirectConvertedExpr(int index) { result = this.asIndirectConvertedExpr(_, index) }
+  Expr asIndirectConvertedExpr(int index) {
+    result = this.(IndirectExprNode).getConvertedExpr(index)
+  }
 
   /**
    * Gets the expression that's indirectly tracked by this node behind a
@@ -275,7 +254,9 @@ class Node extends TIRDataFlowNode {
    * after the `f` has returned.
    */
   Expr asDefiningArgument(int index) {
-    this.(DefinitionByReferenceNode).getIndirectionIndex() = index and
+    // Subtract one because `DefinitionByReferenceNode` is defined to be in
+    // the range `[0 ... n - 1]` for some `n` instead of `[1 ... n]`.
+    this.(DefinitionByReferenceNode).getIndirectionIndex() = index - 1 and
     result = this.(DefinitionByReferenceNode).getArgument()
   }
 
@@ -412,10 +393,9 @@ class Node extends TIRDataFlowNode {
 }
 
 private string toExprString(Node n) {
-  result = n.asExpr(0).toString()
+  result = n.asExpr().toString()
   or
-  not exists(n.asExpr()) and
+  result = n.asIndirectExpr().toString() + " indirection"
-  result = n.asIndirectExpr(0, 1).toString() + " indirection"
 }
 
 /**
@@ -955,7 +935,7 @@ class RawIndirectOperand extends Node, TRawIndirectOperand {
   }
 
   override string toStringImpl() {
-    result = operandNode(this.getOperand()).toStringImpl() + " indirection"
+    result = instructionNode(this.getOperand().getDef()).toStringImpl() + " indirection"
   }
 }
 
@@ -1062,130 +1042,77 @@ class RawIndirectInstruction extends Node, TRawIndirectInstruction {
   }
 }
 
-private module GetConvertedResultExpression {
-  private import semmle.code.cpp.ir.implementation.raw.internal.TranslatedExpr
-  private import semmle.code.cpp.ir.implementation.raw.internal.InstructionTag
-
-  private Operand getAnInitializeDynamicAllocationInstructionAddress() {
-    result = any(InitializeDynamicAllocationInstruction init).getAllocationAddressOperand()
-  }
-
-  /**
-   * Gets the expression that should be returned as the result expression from `instr`.
-   *
-   * Note that this predicate may return multiple results in cases where a conversion belongs to a
-   * different AST element than its operand.
-   */
-  Expr getConvertedResultExpression(Instruction instr, int n) {
-    // Only fully converted instructions have a result for `asConvertedExpr`
-    not conversionFlow(unique(Operand op |
-        // The address operand of a `InitializeDynamicAllocationInstruction` is
-        // special: we need to handle it during dataflow (since it's
-        // effectively a store to an indirection), but it doesn't appear in
-        // source syntax, so dataflow node <-> expression conversion shouldn't
-        // care about it.
-        op = getAUse(instr) and not op = getAnInitializeDynamicAllocationInstructionAddress()
-      |
-        op
-      ), _, false, false) and
-    result = getConvertedResultExpressionImpl(instr) and
-    n = 0
-    or
-    // If the conversion also has a result then we return multiple results
-    exists(Operand operand | conversionFlow(operand, instr, false, false) |
-      n = 1 and
-      result = getConvertedResultExpressionImpl(operand.getDef())
-      or
-      result = getConvertedResultExpression(operand.getDef(), n - 1)
-    )
-  }
-
-  private Expr getConvertedResultExpressionImpl0(Instruction instr) {
-    // For an expression such as `i += 2` we pretend that the generated
-    // `StoreInstruction` contains the result of the expression even though
-    // this isn't totally aligned with the C/C++ standard.
-    exists(TranslatedAssignOperation tao |
-      result = tao.getExpr() and
-      instr = tao.getInstruction(any(AssignmentStoreTag tag))
-    )
-    or
-    // Similarly for `i++` and `++i` we pretend that the generated
-    // `StoreInstruction` is contains the result of the expression even though
-    // this isn't totally aligned with the C/C++ standard.
-    exists(TranslatedCrementOperation tco |
-      result = tco.getExpr() and
-      instr = tco.getInstruction(any(CrementStoreTag tag))
-    )
-    or
-    // IR construction inserts an additional cast to a `size_t` on the extent
-    // of a `new[]` expression. The resulting `ConvertInstruction` doesn't have
-    // a result for `getConvertedResultExpression`. We remap this here so that
-    // this `ConvertInstruction` maps to the result of the expression that
-    // represents the extent.
-    exists(TranslatedNonConstantAllocationSize tas |
-      result = tas.getExtent().getExpr() and
-      instr = tas.getInstruction(any(AllocationExtentConvertTag tag))
-    )
-    or
-    // There's no instruction that returns `ParenthesisExpr`, but some queries
-    // expect this
-    exists(TranslatedTransparentConversion ttc |
-      result = ttc.getExpr().(ParenthesisExpr) and
-      instr = ttc.getResult()
-    )
-  }
-
-  private Expr getConvertedResultExpressionImpl(Instruction instr) {
-    result = getConvertedResultExpressionImpl0(instr)
-    or
-    not exists(getConvertedResultExpressionImpl0(instr)) and
-    result = instr.getConvertedResultExpression()
-  }
-}
-
-private import GetConvertedResultExpression
-
 /** Holds if `node` is an `OperandNode` that should map `node.asExpr()` to `e`. */
-predicate exprNodeShouldBeOperand(OperandNode node, Expr e, int n) {
+predicate exprNodeShouldBeOperand(OperandNode node, Expr e) {
   exists(Instruction def |
     unique( | | getAUse(def)) = node.getOperand() and
-    e = getConvertedResultExpression(def, n)
+    e = def.getConvertedResultExpression()
   )
 }
 
+private predicate indirectExprNodeShouldBeIndirectOperand0(
+  VariableAddressInstruction instr, RawIndirectOperand node, Expr e
+) {
+  instr = node.getOperand().getDef() and
+  e = instr.getAst().(Expr).getUnconverted()
+}
+
 /** Holds if `node` should be an `IndirectOperand` that maps `node.asIndirectExpr()` to `e`. */
-private predicate indirectExprNodeShouldBeIndirectOperand(
+private predicate indirectExprNodeShouldBeIndirectOperand(RawIndirectOperand node, Expr e) {
-  IndirectOperand node, Expr e, int n, int indirectionIndex
+  exists(Instruction instr | instr = node.getOperand().getDef() |
-) {
+    exists(Expr e0 |
-  exists(Instruction def |
+      indirectExprNodeShouldBeIndirectOperand0(instr, node, e0) and
-    node.hasOperandAndIndirectionIndex(unique( | | getAUse(def)), indirectionIndex) and
+      e = e0.getFullyConverted()
-    e = getConvertedResultExpression(def, n)
+    )
+    or
+    not indirectExprNodeShouldBeIndirectOperand0(_, node, _) and
+    e = instr.getConvertedResultExpression()
   )
 }
 
-private predicate exprNodeShouldBeIndirectOutNode(IndirectArgumentOutNode node, Expr e, int n) {
+private predicate exprNodeShouldBeIndirectOutNode(IndirectArgumentOutNode node, Expr e) {
   exists(CallInstruction call |
     call.getStaticCallTarget() instanceof Constructor and
-    e = getConvertedResultExpression(call, n) and
+    e = call.getConvertedResultExpression() and
     call.getThisArgumentOperand() = node.getAddressOperand()
   )
 }
 
 /** Holds if `node` should be an instruction node that maps `node.asExpr()` to `e`. */
-predicate exprNodeShouldBeInstruction(Node node, Expr e, int n) {
+predicate exprNodeShouldBeInstruction(Node node, Expr e) {
-  not exprNodeShouldBeOperand(_, e, n) and
+  not exprNodeShouldBeOperand(_, e) and
-  not exprNodeShouldBeIndirectOutNode(_, e, n) and
+  not exprNodeShouldBeIndirectOutNode(_, e) and
-  e = getConvertedResultExpression(node.asInstruction(), n)
+  (
+    e = node.asInstruction().getConvertedResultExpression()
+    or
+    // The instruction that contains the result of an `AssignOperation` is
+    // the unloaded left operand (see the comments in `TranslatedAssignOperation`).
+    // That means that for cases like
+    // ```cpp
+    // int x = ...;
+    // x += 1;
+    // ```
+    // the result of `x += 1` is the `VariableAddressInstruction` that represents `x`. But
+    // that instruction doesn't receive the flow from this `AssignOperation`. So instead we
+    // map the operation to the `AddInstruction`.
+    node.asInstruction().getAst() = e.(AssignOperation)
+    or
+    // Same story for `CrementOperation`s (cf. the comments in the subclasses
+    // of `TranslatedCrementOperation`).
+    node.asInstruction().getAst() = e.(CrementOperation)
+  )
 }
 
 /** Holds if `node` should be an `IndirectInstruction` that maps `node.asIndirectExpr()` to `e`. */
-predicate indirectExprNodeShouldBeIndirectInstruction(
+predicate indirectExprNodeShouldBeIndirectInstruction(IndirectInstruction node, Expr e) {
-  IndirectInstruction node, Expr e, int n, int indirectionIndex
-) {
-  not indirectExprNodeShouldBeIndirectOperand(_, e, n, indirectionIndex) and
   exists(Instruction instr |
-    node.hasInstructionAndIndirectionIndex(instr, indirectionIndex) and
+    node.hasInstructionAndIndirectionIndex(instr, _) and
-    e = getConvertedResultExpression(instr, n)
+    not indirectExprNodeShouldBeIndirectOperand(_, e)
+  |
+    e = instr.(VariableAddressInstruction).getAst().(Expr).getFullyConverted()
+    or
+    not instr instanceof VariableAddressInstruction and
+    e = instr.getConvertedResultExpression()
   )
 }
 
@@ -1194,32 +1121,30 @@ abstract private class ExprNodeBase extends Node {
    * Gets the expression corresponding to this node, if any. The returned
    * expression may be a `Conversion`.
    */
-  abstract Expr getConvertedExpr(int n);
+  abstract Expr getConvertedExpr();
 
   /** Gets the non-conversion expression corresponding to this node, if any. */
-  final Expr getExpr(int n) { result = this.getConvertedExpr(n).getUnconverted() }
+  abstract Expr getExpr();
 }
 
 private class InstructionExprNode extends ExprNodeBase, InstructionNode {
-  InstructionExprNode() {
+  InstructionExprNode() { exprNodeShouldBeInstruction(this, _) }
-    exists(Expr e, int n |
-      exprNodeShouldBeInstruction(this, e, n) and
-      not exprNodeShouldBeInstruction(_, e, n + 1)
-    )
-  }
 
-  final override Expr getConvertedExpr(int n) { exprNodeShouldBeInstruction(this, result, n) }
+  final override Expr getConvertedExpr() { exprNodeShouldBeInstruction(this, result) }
+
+  final override Expr getExpr() { result = this.getConvertedExpr().getUnconverted() }
+
+  final override string toStringImpl() { result = this.getConvertedExpr().toString() }
 }
 
 private class OperandExprNode extends ExprNodeBase, OperandNode {
-  OperandExprNode() {
+  OperandExprNode() { exprNodeShouldBeOperand(this, _) }
-    exists(Expr e, int n |
-      exprNodeShouldBeOperand(this, e, n) and
-      not exprNodeShouldBeOperand(_, e, n + 1)
-    )
-  }
 
-  final override Expr getConvertedExpr(int n) { exprNodeShouldBeOperand(this, result, n) }
+  final override Expr getConvertedExpr() { exprNodeShouldBeOperand(this, result) }
+
+  final override Expr getExpr() { result = this.getConvertedExpr().getUnconverted() }
+
+  final override string toStringImpl() { result = this.getConvertedExpr().toString() }
 }
 
 abstract private class IndirectExprNodeBase extends Node {
@@ -1227,75 +1152,67 @@ abstract private class IndirectExprNodeBase extends Node {
    * Gets the expression corresponding to this node, if any. The returned
    * expression may be a `Conversion`.
    */
-  abstract Expr getConvertedExpr(int n, int indirectionIndex);
+  abstract Expr getConvertedExpr(int indirectionIndex);
 
   /** Gets the non-conversion expression corresponding to this node, if any. */
-  final Expr getExpr(int n, int indirectionIndex) {
+  abstract Expr getExpr(int indirectionIndex);
-    result = this.getConvertedExpr(n, indirectionIndex).getUnconverted()
+}
+
+private class IndirectOperandIndirectExprNode extends IndirectExprNodeBase, RawIndirectOperand {
+  IndirectOperandIndirectExprNode() { indirectExprNodeShouldBeIndirectOperand(this, _) }
+
+  final override Expr getConvertedExpr(int index) {
+    this.getIndirectionIndex() = index and
+    indirectExprNodeShouldBeIndirectOperand(this, result)
+  }
+
+  final override Expr getExpr(int index) {
+    this.getIndirectionIndex() = index and
+    result = this.getConvertedExpr(index).getUnconverted()
   }
 }
 
-private class IndirectOperandIndirectExprNode extends IndirectExprNodeBase instanceof IndirectOperand
+private class IndirectInstructionIndirectExprNode extends IndirectExprNodeBase,
+  RawIndirectInstruction
 {
-  IndirectOperandIndirectExprNode() {
+  IndirectInstructionIndirectExprNode() { indirectExprNodeShouldBeIndirectInstruction(this, _) }
-    exists(Expr e, int n, int indirectionIndex |
+
-      indirectExprNodeShouldBeIndirectOperand(this, e, n, indirectionIndex) and
+  final override Expr getConvertedExpr(int index) {
-      not indirectExprNodeShouldBeIndirectOperand(_, e, n + 1, indirectionIndex)
+    this.getIndirectionIndex() = index and
-    )
+    indirectExprNodeShouldBeIndirectInstruction(this, result)
   }
 
-  final override Expr getConvertedExpr(int n, int index) {
+  final override Expr getExpr(int index) {
-    indirectExprNodeShouldBeIndirectOperand(this, result, n, index)
+    this.getIndirectionIndex() = index and
-  }
+    result = this.getConvertedExpr(index).getUnconverted()
-}
-
-private class IndirectInstructionIndirectExprNode extends IndirectExprNodeBase instanceof IndirectInstruction
-{
-  IndirectInstructionIndirectExprNode() {
-    exists(Expr e, int n, int indirectionIndex |
-      indirectExprNodeShouldBeIndirectInstruction(this, e, n, indirectionIndex) and
-      not indirectExprNodeShouldBeIndirectInstruction(_, e, n + 1, indirectionIndex)
-    )
-  }
-
-  final override Expr getConvertedExpr(int n, int index) {
-    indirectExprNodeShouldBeIndirectInstruction(this, result, n, index)
   }
 }
 
 private class IndirectArgumentOutExprNode extends ExprNodeBase, IndirectArgumentOutNode {
-  IndirectArgumentOutExprNode() { exprNodeShouldBeIndirectOutNode(this, _, _) }
+  IndirectArgumentOutExprNode() { exprNodeShouldBeIndirectOutNode(this, _) }
 
-  final override Expr getConvertedExpr(int n) { exprNodeShouldBeIndirectOutNode(this, result, n) }
+  final override Expr getConvertedExpr() { exprNodeShouldBeIndirectOutNode(this, result) }
+
+  final override Expr getExpr() { result = this.getConvertedExpr() }
 }
 
 /**
  * An expression, viewed as a node in a data flow graph.
  */
 class ExprNode extends Node instanceof ExprNodeBase {
-  /**
-   * INTERNAL: Do not use.
-   */
-  Expr getExpr(int n) { result = super.getExpr(n) }
-
   /**
    * Gets the non-conversion expression corresponding to this node, if any. If
    * this node strictly (in the sense of `getConvertedExpr`) corresponds to a
    * `Conversion`, then the result is that `Conversion`'s non-`Conversion` base
    * expression.
    */
-  final Expr getExpr() { result = this.getExpr(_) }
+  Expr getExpr() { result = super.getExpr() }
-
-  /**
-   * INTERNAL: Do not use.
-   */
-  Expr getConvertedExpr(int n) { result = super.getConvertedExpr(n) }
 
   /**
    * Gets the expression corresponding to this node, if any. The returned
    * expression may be a `Conversion`.
    */
-  final Expr getConvertedExpr() { result = this.getConvertedExpr(_) }
+  Expr getConvertedExpr() { result = super.getConvertedExpr() }
 }
 
 /**
@@ -1308,27 +1225,13 @@ class IndirectExprNode extends Node instanceof IndirectExprNodeBase {
    * `Conversion`, then the result is that `Conversion`'s non-`Conversion` base
    * expression.
    */
-  final Expr getExpr(int indirectionIndex) { result = this.getExpr(_, indirectionIndex) }
+  Expr getExpr(int indirectionIndex) { result = super.getExpr(indirectionIndex) }
-
-  /**
-   * INTERNAL: Do not use.
-   */
-  Expr getExpr(int n, int indirectionIndex) { result = super.getExpr(n, indirectionIndex) }
-
-  /**
-   * INTERNAL: Do not use.
-   */
-  Expr getConvertedExpr(int n, int indirectionIndex) {
-    result = super.getConvertedExpr(n, indirectionIndex)
-  }
 
   /**
    * Gets the expression corresponding to this node, if any. The returned
    * expression may be a `Conversion`.
    */
-  Expr getConvertedExpr(int indirectionIndex) {
+  Expr getConvertedExpr(int indirectionIndex) { result = super.getConvertedExpr(indirectionIndex) }
-    result = this.getConvertedExpr(_, indirectionIndex)
-  }
 }
 
 /**
@@ -1354,9 +1257,6 @@ class ParameterNode extends Node {
    * pointer-indirection parameters are at further negative positions.
    */
   predicate isParameterOf(Function f, ParameterPosition pos) { none() } // overridden by subclasses
-
-  /** Gets the `Parameter` associated with this node, if it exists. */
-  Parameter getParameter() { none() } // overridden by subclasses
 }
 
 /** An explicit positional parameter, including `this`, but not `...`. */
@@ -1379,9 +1279,10 @@ private class ExplicitParameterNode extends ParameterNode, DirectParameterNode {
     f.getParameter(pos.(DirectPosition).getIndex()) = instr.getParameter()
   }
 
-  override string toStringImpl() { result = instr.getParameter().toString() }
+  /** Gets the `Parameter` associated with this node. */
+  Parameter getParameter() { result = instr.getParameter() }
 
-  override Parameter getParameter() { result = instr.getParameter() }
+  override string toStringImpl() { result = instr.getParameter().toString() }
 }
 
 /** An implicit `this` parameter. */
@@ -1543,7 +1444,7 @@ OperandNode operandNode(Operand operand) { result.getOperand() = operand }
  * _out of_ an expression, like when an argument is passed by reference, use
  * `definitionByReferenceNodeFromArgument` instead.
  */
-ExprNode exprNode(Expr e) { result.getExpr(_) = e }
+ExprNode exprNode(Expr e) { result.getExpr() = e }
 
 /**
  * Gets the `Node` corresponding to the value of evaluating `e`. Here, `e` may
@@ -1551,7 +1452,7 @@ ExprNode exprNode(Expr e) { result.getExpr(_) = e }
  * argument is passed by reference, use
  * `definitionByReferenceNodeFromArgument` instead.
  */
-ExprNode convertedExprNode(Expr e) { result.getConvertedExpr(_) = e }
+ExprNode convertedExprNode(Expr e) { result.getConvertedExpr() = e }
 
 /**
  * Gets the `Node` corresponding to the value of `p` at function entry.

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

@@ -447,16 +447,9 @@ class GlobalUse extends UseImpl, TGlobalUse {
   IRFunction getIRFunction() { result = f }
 
   final override predicate hasIndexInBlock(IRBlock block, int index) {
-    // Similar to the `FinalParameterUse` case, we want to generate flow out of
+    exists(ExitFunctionInstruction exit |
-    // globals at any exit so that we can flow out of non-returning functions.
+      exit = f.getExitFunctionInstruction() and
-    // Obviously this isn't correct as we can't actually flow but the global flow
+      block.getInstruction(index) = exit
-    // requires this if we want to flow into children.
-    exists(Instruction return |
-      return instanceof ReturnInstruction or
-      return instanceof UnreachedInstruction
-    |
-      block.getInstruction(index) = return and
-      return.getEnclosingIRFunction() = f
     )
   }
 
@@ -645,12 +638,24 @@ private predicate adjustForPointerArith(PostUpdateNode pun, UseOrPhi use) {
   )
 }
 
+/**
+ * Holds if `nodeFrom` flows to `nodeTo` because there is `def-use` or
+ * `use-use` flow from `defOrUse` to `use`.
+ *
+ * `uncertain` is `true` if the `defOrUse` is an uncertain definition.
+ */
+private predicate localSsaFlow(
+  SsaDefOrUse defOrUse, Node nodeFrom, UseOrPhi use, Node nodeTo, boolean uncertain
+) {
+  nodeToDefOrUse(nodeFrom, defOrUse, uncertain) and
+  adjacentDefRead(defOrUse, use) and
+  useToNode(use, nodeTo) and
+  nodeFrom != nodeTo
+}
+
 private predicate ssaFlowImpl(SsaDefOrUse defOrUse, Node nodeFrom, Node nodeTo, boolean uncertain) {
   exists(UseOrPhi use |
-    nodeToDefOrUse(nodeFrom, defOrUse, uncertain) and
+    localSsaFlow(defOrUse, nodeFrom, use, nodeTo, uncertain)
-    adjacentDefRead(defOrUse, use) and
-    useToNode(use, nodeTo) and
-    nodeFrom != nodeTo
     or
     // Initial global variable value to a first use
     nodeFrom.(InitialGlobalValue).getGlobalDef() = defOrUse and
@@ -728,15 +733,62 @@ private predicate isArgumentOfCallable(DataFlowCall call, Node n) {
   )
 }
 
-/** Holds if there is def-use or use-use flow from `pun` to `nodeTo`. */
+/**
-predicate postUpdateFlow(PostUpdateNode pun, Node nodeTo) {
+ * Holds if there is use-use flow from `pun`'s pre-update node to `n`.
-  exists(UseOrPhi use, Node preUpdate |
+ */
+private predicate postUpdateNodeToFirstUse(PostUpdateNode pun, Node n) {
+  exists(UseOrPhi use |
     adjustForPointerArith(pun, use) and
-    useToNode(use, nodeTo) and
+    useToNode(use, n)
+  )
+}
+
+private predicate stepUntilNotInCall(DataFlowCall call, Node n1, Node n2) {
+  isArgumentOfCallable(call, n1) and
+  exists(Node mid | localSsaFlow(_, n1, _, mid, _) |
+    isArgumentOfCallable(call, mid) and
+    stepUntilNotInCall(call, mid, n2)
+    or
+    not isArgumentOfCallable(call, mid) and
+    mid = n2
+  )
+}
+
+bindingset[n1, n2]
+pragma[inline_late]
+private predicate isArgumentOfSameCall(DataFlowCall call, Node n1, Node n2) {
+  isArgumentOfCallable(call, n1) and isArgumentOfCallable(call, n2)
+}
+
+/**
+ * Holds if there is def-use or use-use flow from `pun` to `nodeTo`.
+ *
+ * Note: This is more complex than it sounds. Consider a call such as:
+ * ```cpp
+ * write_first_argument(x, x);
+ * sink(x);
+ * ```
+ * Assume flow comes out of the first argument to `write_first_argument`. We
+ * don't want flow to go to the `x` that's also an argument to
+ * `write_first_argument` (because we just flowed out of that function, and we
+ * don't want to flow back into it again).
+ *
+ * We do, however, want flow from the output argument to `x` on the next line, and
+ * similarly we want flow from the second argument of `write_first_argument` to `x`
+ * on the next line.
+ */
+predicate postUpdateFlow(PostUpdateNode pun, Node nodeTo) {
+  exists(Node preUpdate, Node mid |
     preUpdate = pun.getPreUpdateNode() and
-    not exists(DataFlowCall call |
+    postUpdateNodeToFirstUse(pun, mid)
-      isArgumentOfCallable(call, preUpdate) and isArgumentOfCallable(call, nodeTo)
+  |
+    exists(DataFlowCall call |
+      isArgumentOfSameCall(call, preUpdate, mid) and
+      stepUntilNotInCall(call, mid, nodeTo)
     )
+    or
+    not isArgumentOfSameCall(_, preUpdate, mid) and
+    nodeTo = mid
   )
 }
 
@@ -766,7 +818,7 @@ predicate fromPhiNode(SsaPhiNode nodeFrom, Node nodeTo) {
     or
     exists(PhiNode phiTo |
       phi != phiTo and
-      lastRefRedefExt(phi, bb1, i1, phiTo) and
+      lastRefRedefExt(phi, _, _, phiTo) and
       nodeTo.(SsaPhiNode).getPhiNode() = phiTo
     )
   )

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll

@@ -405,6 +405,9 @@ predicate hasUnreachedInstruction(IRFunction func) {
   exists(Call c |
     c.getEnclosingFunction() = func.getFunction() and
     any(Options opt).exits(c.getTarget())
+  ) and
+  not exists(TranslatedUnreachableReturnStmt return |
+    return.getEnclosingFunction().getFunction() = func.getFunction()
   )
 }
 

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll

@@ -824,9 +824,6 @@ abstract class TranslatedElement extends TTranslatedElement {
   /** DEPRECATED: Alias for getAst */
   deprecated Locatable getAST() { result = this.getAst() }
 
-  /** Gets the location of this element. */
-  Location getLocation() { result = this.getAst().getLocation() }
-
   /**
    * Get the first instruction to be executed in the evaluation of this element.
    */

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll

@@ -1906,10 +1906,8 @@ class TranslatedNonConstantAllocationSize extends TranslatedAllocationSize {
   final override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
     resultType = getTypeForPRValue(expr.getAllocator().getParameter(0).getType()) and
     (
-      this.extentNeedsConversion() and
       // Convert the extent to `size_t`, because the AST doesn't do this already.
-      tag = AllocationExtentConvertTag() and
+      tag = AllocationExtentConvertTag() and opcode instanceof Opcode::Convert
-      opcode instanceof Opcode::Convert
       or
       tag = AllocationElementSizeTag() and opcode instanceof Opcode::Constant
       or
@@ -1920,7 +1918,6 @@ class TranslatedNonConstantAllocationSize extends TranslatedAllocationSize {
   final override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
     kind instanceof GotoEdge and
     (
-      this.extentNeedsConversion() and
       tag = AllocationExtentConvertTag() and
       result = this.getInstruction(AllocationElementSizeTag())
       or
@@ -1936,9 +1933,7 @@ class TranslatedNonConstantAllocationSize extends TranslatedAllocationSize {
 
   final override Instruction getChildSuccessor(TranslatedElement child) {
     child = this.getExtent() and
-    if this.extentNeedsConversion()
+    result = this.getInstruction(AllocationExtentConvertTag())
-    then result = this.getInstruction(AllocationExtentConvertTag())
-    else result = this.getInstruction(AllocationElementSizeTag())
   }
 
   final override string getInstructionConstantValue(InstructionTag tag) {
@@ -1950,31 +1945,19 @@ class TranslatedNonConstantAllocationSize extends TranslatedAllocationSize {
     tag = AllocationSizeTag() and
     (
       operandTag instanceof LeftOperandTag and
-      (
+      result = this.getInstruction(AllocationExtentConvertTag())
-        if this.extentNeedsConversion()
-        then result = this.getInstruction(AllocationExtentConvertTag())
-        else result = this.getExtent().getResult()
-      )
       or
       operandTag instanceof RightOperandTag and
       result = this.getInstruction(AllocationElementSizeTag())
     )
     or
-    this.extentNeedsConversion() and
     tag = AllocationExtentConvertTag() and
     operandTag instanceof UnaryOperandTag and
     result = this.getExtent().getResult()
   }
 
-  TranslatedExpr getExtent() { result = getTranslatedExpr(expr.getExtent().getFullyConverted()) }
+  private TranslatedExpr getExtent() {
-
+    result = getTranslatedExpr(expr.getExtent().getFullyConverted())
-  /**
-   * Holds if the result of `expr.getExtent()` does not have the same type as
-   * the allocator's size parameter.
-   */
-  private predicate extentNeedsConversion() {
-    expr.getExtent().getFullyConverted().getUnspecifiedType() !=
-      expr.getAllocator().getParameter(0).getUnspecifiedType()
   }
 }
 

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedGlobalVar.qll

@@ -22,6 +22,8 @@ class TranslatedStaticStorageDurationVarInit extends TranslatedRootElement,
 
   final override Declaration getFunction() { result = var }
 
+  final Location getLocation() { result = var.getLocation() }
+
   override Instruction getFirstInstruction() { result = this.getInstruction(EnterFunctionTag()) }
 
   override TranslatedElement getChild(int n) {

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll

@@ -442,26 +442,29 @@ class TranslatedReturnVoidStmt extends TranslatedReturnStmt {
 
 /**
  * The IR translation of an implicit `return` statement generated by the extractor to handle control
- * flow that reaches the end of a non-`void`-returning function body. Such control flow
+ * flow that reaches the end of a non-`void`-returning function body. Since such control flow
- * produces undefined behavior in C++ but not in C. However even in C using the return value is
+ * produces undefined behavior, we simply generate an `Unreached` instruction to prevent that flow
- * undefined behaviour. We make it return uninitialized memory to get as much flow as possible.
+ * from continuing on to pollute other analysis. The assumption is that the developer is certain
+ * that the implicit `return` is unreachable, even if the compiler cannot prove it.
  */
-class TranslatedNoValueReturnStmt extends TranslatedReturnStmt, TranslatedVariableInitialization {
+class TranslatedUnreachableReturnStmt extends TranslatedReturnStmt {
-  TranslatedNoValueReturnStmt() {
+  TranslatedUnreachableReturnStmt() {
     not stmt.hasExpr() and hasReturnValue(stmt.getEnclosingFunction())
   }
 
-  final override Instruction getInitializationSuccessor() {
+  override TranslatedElement getChild(int id) { none() }
-    result = this.getEnclosingFunction().getReturnSuccessorInstruction()
+
+  override Instruction getFirstInstruction() { result = this.getInstruction(OnlyInstructionTag()) }
+
+  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
+    tag = OnlyInstructionTag() and
+    opcode instanceof Opcode::Unreached and
+    resultType = getVoidType()
   }
 
-  final override Type getTargetType() { result = this.getEnclosingFunction().getReturnType() }
+  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) { none() }
 
-  final override TranslatedInitialization getInitialization() { none() }
+  override Instruction getChildSuccessor(TranslatedElement child) { none() }
-
-  final override IRVariable getIRVariable() {
-    result = this.getEnclosingFunction().getReturnVariable()
-  }
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/reachability/ReachableBlock.qll

@@ -10,65 +10,6 @@ predicate isInfeasibleInstructionSuccessor(Instruction instr, EdgeKind kind) {
   or
   instr.getSuccessor(kind) instanceof UnreachedInstruction and
   kind instanceof GotoEdge
-  or
-  isCallToNonReturningFunction(instr) and exists(instr.getSuccessor(kind))
-}
-
-/**
- * Holds if all calls to `f` never return (e.g. they call `exit` or loop forever)
- */
-private predicate isNonReturningFunction(IRFunction f) {
-  // If the function has an instruction with a missing successor then
-  // the analysis is probably going to be incorrect, so assume they exit.
-  not hasInstructionWithMissingSuccessor(f) and
-  (
-    // If all flows to the exit block are pass through an unreachable then f never returns.
-    any(UnreachedInstruction instr).getBlock().postDominates(f.getEntryBlock())
-    or
-    // If there is no flow to the exit block then f never returns.
-    not exists(IRBlock entry, IRBlock exit |
-      exit = f.getExitFunctionInstruction().getBlock() and
-      entry = f.getEntryBlock() and
-      exit = entry.getASuccessor*()
-    )
-    or
-    // If all flows to the exit block are pass through a call that never returns then f never returns.
-    exists(CallInstruction ci |
-      ci.getBlock().postDominates(f.getEntryBlock()) and
-      isCallToNonReturningFunction(ci)
-    )
-  )
-}
-
-/**
- * Holds if `f` has an instruction with a missing successor.
- * This matches `instructionWithoutSuccessor` from `IRConsistency`, but
- * avoids generating the error strings.
- */
-predicate hasInstructionWithMissingSuccessor(IRFunction f) {
-  exists(Instruction missingSucc |
-    missingSucc.getEnclosingIRFunction() = f and
-    not exists(missingSucc.getASuccessor()) and
-    not missingSucc instanceof ExitFunctionInstruction and
-    // Phi instructions aren't linked into the instruction-level flow graph.
-    not missingSucc instanceof PhiInstruction and
-    not missingSucc instanceof UnreachedInstruction
-  )
-}
-
-/**
- * Holds if the call `ci` never returns.
- */
-private predicate isCallToNonReturningFunction(CallInstruction ci) {
-  exists(IRFunction callee, Language::Function staticTarget |
-    staticTarget = ci.getStaticCallTarget() and
-    staticTarget = callee.getFunction() and
-    // We can't easily tell if the call is virtual or not
-    // if the callee is virtual. So assume that the call is virtual
-    // if the target is.
-    not staticTarget.isVirtual() and
-    isNonReturningFunction(callee)
-  )
 }
 
 pragma[noinline]

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/reachability/ReachableBlockInternal.qll

@@ -1,3 +1,2 @@
 import semmle.code.cpp.ir.implementation.raw.IR as IR
 import semmle.code.cpp.ir.implementation.raw.constant.ConstantAnalysis as ConstantAnalysis
-import semmle.code.cpp.ir.internal.IRCppLanguage as Language

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/reachability/ReachableBlock.qll

@@ -10,65 +10,6 @@ predicate isInfeasibleInstructionSuccessor(Instruction instr, EdgeKind kind) {
   or
   instr.getSuccessor(kind) instanceof UnreachedInstruction and
   kind instanceof GotoEdge
-  or
-  isCallToNonReturningFunction(instr) and exists(instr.getSuccessor(kind))
-}
-
-/**
- * Holds if all calls to `f` never return (e.g. they call `exit` or loop forever)
- */
-private predicate isNonReturningFunction(IRFunction f) {
-  // If the function has an instruction with a missing successor then
-  // the analysis is probably going to be incorrect, so assume they exit.
-  not hasInstructionWithMissingSuccessor(f) and
-  (
-    // If all flows to the exit block are pass through an unreachable then f never returns.
-    any(UnreachedInstruction instr).getBlock().postDominates(f.getEntryBlock())
-    or
-    // If there is no flow to the exit block then f never returns.
-    not exists(IRBlock entry, IRBlock exit |
-      exit = f.getExitFunctionInstruction().getBlock() and
-      entry = f.getEntryBlock() and
-      exit = entry.getASuccessor*()
-    )
-    or
-    // If all flows to the exit block are pass through a call that never returns then f never returns.
-    exists(CallInstruction ci |
-      ci.getBlock().postDominates(f.getEntryBlock()) and
-      isCallToNonReturningFunction(ci)
-    )
-  )
-}
-
-/**
- * Holds if `f` has an instruction with a missing successor.
- * This matches `instructionWithoutSuccessor` from `IRConsistency`, but
- * avoids generating the error strings.
- */
-predicate hasInstructionWithMissingSuccessor(IRFunction f) {
-  exists(Instruction missingSucc |
-    missingSucc.getEnclosingIRFunction() = f and
-    not exists(missingSucc.getASuccessor()) and
-    not missingSucc instanceof ExitFunctionInstruction and
-    // Phi instructions aren't linked into the instruction-level flow graph.
-    not missingSucc instanceof PhiInstruction and
-    not missingSucc instanceof UnreachedInstruction
-  )
-}
-
-/**
- * Holds if the call `ci` never returns.
- */
-private predicate isCallToNonReturningFunction(CallInstruction ci) {
-  exists(IRFunction callee, Language::Function staticTarget |
-    staticTarget = ci.getStaticCallTarget() and
-    staticTarget = callee.getFunction() and
-    // We can't easily tell if the call is virtual or not
-    // if the callee is virtual. So assume that the call is virtual
-    // if the target is.
-    not staticTarget.isVirtual() and
-    isNonReturningFunction(callee)
-  )
 }
 
 pragma[noinline]

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/reachability/ReachableBlockInternal.qll

@@ -1,3 +1,2 @@
 import semmle.code.cpp.ir.implementation.unaliased_ssa.IR as IR
 import semmle.code.cpp.ir.implementation.unaliased_ssa.constant.ConstantAnalysis as ConstantAnalysis
-import semmle.code.cpp.ir.internal.IRCppLanguage as Language

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeAnalysisStage.qll

@@ -917,46 +917,25 @@ module RangeStage<
     bounded(cast.getOperand(), b, delta, upper, fromBackEdge, origdelta, reason)
   }
 
-  pragma[nomagic]
-  private predicate initialBoundedUpper(SemExpr e) {
-    exists(D::Delta d |
-      initialBounded(e, _, d, false, _, _, _) and
-      D::toFloat(d) >= 0
-    )
-  }
-
-  private predicate noOverflow0(SemExpr e, boolean upper) {
-    exists(boolean lower | lower = upper.booleanNot() |
-      semExprDoesNotOverflow(lower, e)
-      or
-      upper = [true, false] and
-      not potentiallyOverflowingExpr(lower, e)
-    )
-  }
-
-  pragma[nomagic]
-  private predicate initialBoundedLower(SemExpr e) {
-    exists(D::Delta d |
-      initialBounded(e, _, d, true, _, _, _) and
-      D::toFloat(d) <= 0
-    )
-  }
-
-  pragma[nomagic]
-  private predicate noOverflow(SemExpr e, boolean upper) {
-    noOverflow0(e, upper)
-    or
-    upper = true and initialBoundedUpper(e)
-    or
-    upper = false and initialBoundedLower(e)
-  }
-
   predicate bounded(
     SemExpr e, SemBound b, D::Delta delta, boolean upper, boolean fromBackEdge, D::Delta origdelta,
     SemReason reason
   ) {
     initialBounded(e, b, delta, upper, fromBackEdge, origdelta, reason) and
-    noOverflow(e, upper)
+    (
+      semExprDoesNotOverflow(upper.booleanNot(), e)
+      or
+      not potentiallyOverflowingExpr(upper.booleanNot(), e)
+      or
+      exists(D::Delta otherDelta |
+        initialBounded(e, _, otherDelta, upper.booleanNot(), _, _, _) and
+        (
+          upper = true and D::toFloat(otherDelta) >= 0
+          or
+          upper = false and D::toFloat(otherDelta) <= 0
+        )
+      )
+    )
   }
 
   predicate potentiallyOverflowingExpr(boolean positively, SemExpr expr) {

cpp/ql/lib/semmle/code/cpp/security/InvalidPointerDereference/AllocationToInvalidPointer.qll

@@ -72,7 +72,7 @@ predicate hasSize(HeuristicAllocationExpr alloc, DataFlow::Node n, int state) {
     // Compute `delta` as the constant difference between `x` and `x + 1`.
     bounded1(any(Instruction instr | instr.getUnconvertedResultExpression() = size),
       any(LoadInstruction load | load.getUnconvertedResultExpression() = va), delta) and
-    n.asExpr() = va and
+    n.asConvertedExpr() = va.getFullyConverted() and
     state = delta
   )
 }
@@ -210,7 +210,7 @@ private module InterestingPointerAddInstruction {
     predicate isSource(DataFlow::Node source) {
       // The sources is the same as in the sources for the second
       // projection in the `AllocToInvalidPointerConfig` module.
-      hasSize(source.asExpr(), _, _)
+      hasSize(source.asConvertedExpr(), _, _)
     }
 
     int fieldFlowBranchLimit() { result = allocationToInvalidPointerFieldFlowBranchLimit() }
@@ -243,7 +243,7 @@ private module InterestingPointerAddInstruction {
    */
   predicate isInterestingSize(DataFlow::Node n) {
     exists(DataFlow::Node alloc |
-      hasSize(alloc.asExpr(), n, _) and
+      hasSize(alloc.asConvertedExpr(), n, _) and
       flow(alloc, _)
     )
   }
@@ -268,7 +268,7 @@ private module Config implements ProductFlow::StateConfigSig {
     // we use `state2` to remember that there was an offset (in this case an offset of `1`) added
     // to the size of the allocation. This state is then checked in `isSinkPair`.
     exists(unit) and
-    hasSize(allocSource.asExpr(), sizeSource, sizeAddend)
+    hasSize(allocSource.asConvertedExpr(), sizeSource, sizeAddend)
   }
 
   int fieldFlowBranchLimit1() { result = allocationToInvalidPointerFieldFlowBranchLimit() }

cpp/ql/lib/semmlecode.cpp.dbscheme

@@ -1755,7 +1755,6 @@ case @expr.kind of
             | @istriviallydestructibleexpr
             | @istriviallyassignableexpr
             | @isnothrowassignableexpr
-            | @istrivialexpr
             | @isstandardlayoutexpr
             | @istriviallycopyableexpr
             | @isliteraltypeexpr

cpp/ql/lib/upgrades/d77c09d8bdc172c9201dec293de1e14c931d3f05/upgrade.properties

@@ -1,2 +0,0 @@
-description: Make __is_trivial a builtin operation
-compatibility: full

cpp/ql/src/CHANGELOG.md

@@ -1,3 +1,15 @@
+## 0.7.4
+
+### New Queries
+
+* Added a new query, `cpp/invalid-pointer-deref`, to detect out-of-bounds pointer reads and writes.
+
+### Minor Analysis Improvements
+
+* The "Comparison where assignment was intended" query (`cpp/compare-where-assign-meant`) no longer reports comparisons that appear in macro expansions.
+* Some queries that had repeated results corresponding to different levels of indirection for `argv` now only have a single result.
+* The `cpp/non-constant-format` query no longer considers an assignment on the right-hand side of another assignment to be a source of non-constant format strings. As a result, the query may now produce fewer results.
+
 ## 0.7.3
 
 No user-facing changes.

cpp/ql/src/Critical/FlowAfterFree.qll

@@ -98,11 +98,8 @@ module FlowFromFree<isSinkSig/2 isASink, isExcludedSig/2 isExcluded> {
  * is being freed by a deallocation expression `dealloc`.
  */
 predicate isFree(DataFlow::Node n, Expr e, DeallocationExpr dealloc) {
-  exists(Expr conv |
+  e = dealloc.getFreedExpr() and
-    e = conv.getUnconverted() and
+  e = n.asExpr() and
-    conv = dealloc.getFreedExpr().getFullyConverted() and
-    conv = n.asConvertedExpr()
-  ) and
   // Ignore realloc functions
   not exists(dealloc.(FunctionCall).getTarget().(AllocationFunction).getReallocPtrArg())
 }

cpp/ql/src/Critical/UseAfterFree.ql

@@ -29,7 +29,8 @@ private predicate externalCallNeverDereferences(FormattingFunctionCall call, int
   )
 }
 
-predicate isUse0(Expr e) {
+predicate isUse0(DataFlow::Node n, Expr e) {
+  e = n.asExpr() and
   not isFree(_, e, _) and
   (
     e = any(PointerDereferenceExpr pde).getOperand()
@@ -42,7 +43,7 @@ predicate isUse0(Expr e) {
     or
     // Assume any function without a body will dereference the pointer
     exists(int i, Call call, Function f |
-      e = call.getArgument(i) and
+      n.asExpr() = call.getArgument(i) and
       f = call.getTarget() and
       not f.hasEntryPoint() and
       // Exclude known functions we know won't dereference the pointer.
@@ -56,7 +57,7 @@ module ParameterSinks {
   import semmle.code.cpp.ir.ValueNumbering
 
   predicate flowsToUse(DataFlow::Node n) {
-    isUse0(n.asExpr())
+    isUse0(n, _)
     or
     exists(DataFlow::Node succ |
       flowsToUse(succ) and
@@ -89,7 +90,7 @@ module ParameterSinks {
   ) {
     pragma[only_bind_out](source.asParameter()) = pragma[only_bind_out](init.getParameter()) and
     paramToUse(source, sink) and
-    isUse0(sink.asExpr())
+    isUse0(sink, _)
   }
 
   private InitializeParameterInstruction getAnAlwaysDereferencedParameter0() {
@@ -138,7 +139,7 @@ module IsUse {
   private import semmle.code.cpp.ir.dataflow.internal.DataFlowImplCommon
 
   predicate isUse(DataFlow::Node n, Expr e) {
-    isUse0(e) and n.asExpr() = e
+    isUse0(n, e)
     or
     exists(CallInstruction call, InitializeParameterInstruction init |
       n.asOperand().getDef().getUnconvertedResultExpression() = e and

cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll

@@ -296,7 +296,7 @@ deprecated class PossibleYearArithmeticOperationCheckConfiguration extends Taint
   }
 
   override predicate isSource(DataFlow::Node source) {
-    exists(Operation op | op = source.asExpr() |
+    exists(Operation op | op = source.asConvertedExpr() |
       op.getAChild*().getValue().toInt() = 365 and
       (
         not op.getParent() instanceof Expr or
@@ -321,7 +321,7 @@ deprecated class PossibleYearArithmeticOperationCheckConfiguration extends Taint
 
   override predicate isSink(DataFlow::Node sink) {
     exists(StructLikeClass dds, FieldAccess fa, AssignExpr aexpr |
-      aexpr.getRValue() = sink.asExpr()
+      aexpr.getRValue() = sink.asConvertedExpr()
     |
       (dds instanceof PackedTimeType or dds instanceof UnpackedTimeType) and
       fa.getQualifier().getUnderlyingType() = dds and
@@ -336,7 +336,7 @@ deprecated class PossibleYearArithmeticOperationCheckConfiguration extends Taint
  */
 private module PossibleYearArithmeticOperationCheckConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) {
-    exists(Operation op | op = source.asExpr() |
+    exists(Operation op | op = source.asConvertedExpr() |
       op.getAChild*().getValue().toInt() = 365 and
       (
         not op.getParent() instanceof Expr or
@@ -361,7 +361,7 @@ private module PossibleYearArithmeticOperationCheckConfig implements DataFlow::C
 
   predicate isSink(DataFlow::Node sink) {
     exists(StructLikeClass dds, FieldAccess fa, AssignExpr aexpr |
-      aexpr.getRValue() = sink.asExpr()
+      aexpr.getRValue() = sink.asConvertedExpr()
     |
       (dds instanceof PackedTimeType or dds instanceof UnpackedTimeType) and
       fa.getQualifier().getUnderlyingType() = dds and

cpp/ql/src/Metrics/Internal/ASTConsistency.ql

@@ -1,18 +0,0 @@
-/**
- * @name Count AST inconsistencies
- * @description Counts the various AST inconsistencies that may occur.
- *              This query is for internal use only and may change without notice.
- * @kind table
- * @id cpp/count-ast-inconsistencies
- */
-
-import cpp
-
-predicate hasDuplicateFunctionEntryPointLocation(Function func) {
-  count(func.getEntryPoint().getLocation()) > 1
-}
-
-predicate hasDuplicateFunctionEntryPoint(Function func) { count(func.getEntryPoint()) > 1 }
-
-select count(Function f | hasDuplicateFunctionEntryPoint(f) | f) as duplicateFunctionEntryPoint,
-  count(Function f | hasDuplicateFunctionEntryPointLocation(f) | f) as duplicateFunctionEntryPointLocation

cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql

@@ -30,7 +30,7 @@ Expr asSinkExpr(DataFlow::Node node) {
   result = node.asIndirectArgument()
   or
   // We want the conversion so we only get one node for the expression
-  result = node.asExpr()
+  result = node.asConvertedExpr()
 }
 
 module SqlTaintedConfig implements DataFlow::ConfigSig {

cpp/ql/src/Security/CWE/CWE-119/OverrunWriteProductFlow.ql

@@ -38,7 +38,7 @@ predicate hasSize(HeuristicAllocationExpr alloc, DataFlow::Node n, int state) {
     // Compute `delta` as the constant difference between `x` and `x + 1`.
     bounded(any(Instruction instr | instr.getUnconvertedResultExpression() = size),
       any(LoadInstruction load | load.getUnconvertedResultExpression() = va), delta) and
-    n.asExpr() = va and
+    n.asConvertedExpr() = va.getFullyConverted() and
     state = delta
   )
 }
@@ -213,7 +213,7 @@ module StringSizeConfig implements ProductFlow::StateConfigSig {
     // we use `state2` to remember that there was an offset (in this case an offset of `1`) added
     // to the size of the allocation. This state is then checked in `isSinkPair`.
     exists(state1) and
-    hasSize(bufSource.asExpr(), sizeSource, state2) and
+    hasSize(bufSource.asConvertedExpr(), sizeSource, state2) and
     validState(sizeSource, state2)
   }
 

cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql

@@ -26,7 +26,7 @@ import TaintedAllocationSize::PathGraph
  * taint sink.
  */
 predicate allocSink(HeuristicAllocationExpr alloc, DataFlow::Node sink) {
-  exists(Expr e | e = sink.asExpr() |
+  exists(Expr e | e = sink.asConvertedExpr() |
     e = alloc.getAChild() and
     e.getUnspecifiedType() instanceof IntegralType
   )

cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql

@@ -206,22 +206,25 @@ class Encrypted extends Expr {
  * operation `nsr`.
  */
 predicate isSinkSendRecv(DataFlow::Node sink, NetworkSendRecv nsr) {
-  [sink.asIndirectExpr(), sink.asExpr()] = nsr.getDataExpr()
+  [sink.asIndirectConvertedExpr(), sink.asConvertedExpr()] = nsr.getDataExpr().getFullyConverted()
 }
 
 /**
  * Holds if `sink` is a node that is encrypted by `enc`.
  */
-predicate isSinkEncrypt(DataFlow::Node sink, Encrypted enc) { sink.asExpr() = enc }
+predicate isSinkEncrypt(DataFlow::Node sink, Encrypted enc) {
+  sink.asConvertedExpr() = enc.getFullyConverted()
+}
 
 /**
  * Holds if `source` represents a use of a sensitive variable, or data returned by a
  * function returning sensitive data.
  */
 predicate isSourceImpl(DataFlow::Node source) {
-  exists(VariableAccess e |
+  exists(Expr e |
-    e = source.asExpr() and
+    e = source.asConvertedExpr() and
-    e.getTarget() instanceof SourceVariable
+    e.getUnconverted().(VariableAccess).getTarget() instanceof SourceVariable and
+    not e.hasConversion()
   )
   or
   source.asExpr().(FunctionCall).getTarget() instanceof SourceFunction

cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql

@@ -33,6 +33,14 @@ module ExposedSystemDataConfig implements DataFlow::ConfigSig {
 module ExposedSystemData = TaintTracking::Global<ExposedSystemDataConfig>;
 
 from ExposedSystemData::PathNode source, ExposedSystemData::PathNode sink
-where ExposedSystemData::flowPath(source, sink)
+where
+  ExposedSystemData::flowPath(source, sink) and
+  not exists(
+    DataFlow::Node alt // remove duplicate results on conversions
+  |
+    ExposedSystemData::flow(source.getNode(), alt) and
+    alt.asConvertedExpr() = sink.getNode().asIndirectExpr() and
+    alt != sink.getNode()
+  )
 select sink, source, sink, "This operation exposes system data from $@.", source,
   source.getNode().toString()

cpp/ql/src/Security/CWE/CWE-497/SystemData.qll

@@ -34,7 +34,7 @@ class EnvData extends SystemData {
         .regexpMatch(".*(user|host|admin|root|home|path|http|ssl|snmp|sock|port|proxy|pass|token|crypt|key).*")
   }
 
-  override DataFlow::Node getAnExpr() { result.asIndirectExpr() = this }
+  override DataFlow::Node getAnExpr() { result.asIndirectConvertedExpr() = this }
 
   override predicate isSensitive() {
     this.(EnvironmentRead)
@@ -50,7 +50,7 @@ class EnvData extends SystemData {
 class SqlClientInfo extends SystemData {
   SqlClientInfo() { this.(FunctionCall).getTarget().hasName("mysql_get_client_info") }
 
-  override DataFlow::Node getAnExpr() { result.asIndirectExpr() = this }
+  override DataFlow::Node getAnExpr() { result.asIndirectConvertedExpr() = this }
 
   override predicate isSensitive() { any() }
 }

cpp/ql/src/Security/CWE/CWE-611/Xerces.qll

@@ -70,7 +70,7 @@ class XercesDomParserLibrary extends XmlLibrary {
     // sink is the read of the qualifier of a call to `AbstractDOMParser.parse`.
     exists(Call call |
       call.getTarget().getClassAndName("parse") instanceof AbstractDomParserClass and
-      call.getQualifier() = node.asIndirectExpr()
+      call.getQualifier() = node.asIndirectConvertedExpr()
     ) and
     flowstate instanceof XercesFlowState and
     not encodeXercesFlowState(flowstate, 1, 1) // safe configuration
@@ -114,7 +114,7 @@ class CreateLSParserLibrary extends XmlLibrary {
     // sink is the read of the qualifier of a call to `DOMLSParserClass.parse`.
     exists(Call call |
       call.getTarget().getClassAndName("parse") instanceof DomLSParserClass and
-      call.getQualifier() = node.asIndirectExpr()
+      call.getQualifier() = node.asIndirectConvertedExpr()
     ) and
     flowstate instanceof XercesFlowState and
     not encodeXercesFlowState(flowstate, 1, 1) // safe configuration
@@ -155,7 +155,7 @@ class SaxParserLibrary extends XmlLibrary {
     // sink is the read of the qualifier of a call to `SAXParser.parse`.
     exists(Call call |
       call.getTarget().getClassAndName("parse") instanceof SaxParserClass and
-      call.getQualifier() = node.asIndirectExpr()
+      call.getQualifier() = node.asIndirectConvertedExpr()
     ) and
     flowstate instanceof XercesFlowState and
     not encodeXercesFlowState(flowstate, 1, 1) // safe configuration
@@ -192,7 +192,7 @@ class Sax2XmlReaderLibrary extends XmlLibrary {
     // sink is the read of the qualifier of a call to `SAX2XMLReader.parse`.
     exists(Call call |
       call.getTarget().getClassAndName("parse") instanceof Sax2XmlReader and
-      call.getQualifier() = node.asIndirectExpr()
+      call.getQualifier() = node.asIndirectConvertedExpr()
     ) and
     flowstate instanceof XercesFlowState and
     not encodeXercesFlowState(flowstate, 1, 1) // safe configuration

cpp/ql/src/change-notes/2023-08-21-invalid-pointer-deref.md

@@ -1,4 +0,0 @@
----
-category: newQuery
----
-* Added a new query, `cpp/invalid-pointer-deref`, to detect out-of-bounds pointer reads and writes.

cpp/ql/src/change-notes/2023-08-24-no-taint-argv-indirections.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* Some queries that had repeated results corresponding to different levels of indirection for `argv` now only have a single result.

cpp/ql/src/change-notes/2023-08-24-remove-non-constant-assign-sources.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* The `cpp/non-constant-format` query no longer considers an assignment on the right-hand side of another assignment to be a source of non-constant format strings. As a result, the query may now produce fewer results.

cpp/ql/src/change-notes/2023-08-25-compare-where-assign-meant.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* The "Comparison where assignment was intended" query (`cpp/compare-where-assign-meant`) no longer reports comparisons that appear in macro expansions.

cpp/ql/src/change-notes/2023-09-06-deduplicated-results.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* The number of duplicated dataflow paths reported by queries has been significantly reduced.

cpp/ql/src/change-notes/2023-09-08-unreachble-edges.md

@@ -1,5 +0,0 @@
----
-category: minorAnalysis
----
-* The queries `cpp/double-free` and `cpp/use-after-free` find fewer false positives
-  in cases where a non-returning function is called.

cpp/ql/src/change-notes/released/0.7.4.md

@@ -0,0 +1,11 @@
+## 0.7.4
+
+### New Queries
+
+* Added a new query, `cpp/invalid-pointer-deref`, to detect out-of-bounds pointer reads and writes.
+
+### Minor Analysis Improvements
+
+* The "Comparison where assignment was intended" query (`cpp/compare-where-assign-meant`) no longer reports comparisons that appear in macro expansions.
+* Some queries that had repeated results corresponding to different levels of indirection for `argv` now only have a single result.
+* The `cpp/non-constant-format` query no longer considers an assignment on the right-hand side of another assignment to be a source of non-constant format strings. As a result, the query may now produce fewer results.