TEST DO NOT MERGE

.bazelrc

@@ -10,19 +10,8 @@ common --override_module=semmle_code=%workspace%/misc/bazel/semmle_code_stub
 
 build --repo_env=CC=clang --repo_env=CXX=clang++
 
-# we use transitions that break builds of `...`, so for `test` to work with that we need the following
-test --build_tests_only
-
-# this requires developer mode, but is required to have pack installer functioning
-startup --windows_enable_symlinks
-common --enable_runfiles
-
-# with the above, we can avoid building python zips which is the default on windows as that's expensive
-build --nobuild_python_zip
-
-common --registry=file:///%workspace%/misc/bazel/registry
-common --registry=https://bcr.bazel.build
-
-common --@rules_dotnet//dotnet/settings:strict_deps=false
+build:linux --cxxopt=-std=c++20
+build:macos --cxxopt=-std=c++20 --cpu=darwin_x86_64
+build:windows --cxxopt=/std:c++20 --cxxopt=/Zc:preprocessor
 
 try-import %workspace%/local.bazelrc

.bazelrc.internal

@@ -1,10 +0,0 @@
-# this file should contain bazel settings required to build things from `semmle-code`
-
-common --registry=file:///%workspace%/ql/misc/bazel/registry
-common --registry=https://bcr.bazel.build
-
-# See bazelbuild/rules_dotnet#413: strict_deps in C# also appliy to 3rd-party deps, and when we pull
-# in (for example) the xunit package, there's no code in this at all, it just depends transitively on
-# its implementation packages without providing any code itself.
-# We either can depend on internal implementation details, or turn of strict deps.
-common --@rules_dotnet//dotnet/settings:strict_deps=false

.bazelversion

@@ -1 +1 @@
-7.2.1
+7.1.0

.gitattributes

@@ -50,40 +50,26 @@
 *.dll -text
 *.pdb -text
 
-/java/ql/test/stubs/**/*.java linguist-generated=true
-/java/ql/test/experimental/stubs/**/*.java linguist-generated=true
-/java/kotlin-extractor/deps/*.jar filter=lfs diff=lfs merge=lfs -text
+java/ql/test/stubs/**/*.java linguist-generated=true
+java/ql/test/experimental/stubs/**/*.java linguist-generated=true
 
 # Force git not to modify line endings for go or html files under the go/ql directory
-/go/ql/**/*.go -text
-/go/ql/**/*.html -text
+go/ql/**/*.go -text
+go/ql/**/*.html -text
 # Force git not to modify line endings for go dbschemes
-/go/*.dbscheme -text
+go/*.dbscheme -text
 # Preserve unusual line ending from codeql-go merge
-/go/extractor/opencsv/CSVReader.java -text
+go/extractor/opencsv/CSVReader.java -text
 
 # For some languages, upgrade script testing references really old dbscheme
 # files from legacy upgrades that have CRLF line endings. Since upgrade
 # resolution relies on object hashes, we must suppress line ending conversion
 # for those testing dbscheme files.
-/*/ql/lib/upgrades/initial/*.dbscheme -text
+*/ql/lib/upgrades/initial/*.dbscheme -text
 
 # Auto-generated modeling for Python
-/python/ql/lib/semmle/python/frameworks/data/internal/subclass-capture/*.yml linguist-generated=true
+python/ql/lib/semmle/python/frameworks/data/internal/subclass-capture/*.yml linguist-generated=true
 
 # auto-generated bazel lock file
-/ruby/extractor/cargo-bazel-lock.json linguist-generated=true
-/ruby/extractor/cargo-bazel-lock.json -merge
-
-# auto-generated files for the C# build
-/csharp/paket.lock linguist-generated=true
-# needs eol=crlf, as `paket` touches this file and saves it as crlf
-/csharp/.paket/Paket.Restore.targets linguist-generated=true eol=crlf
-/csharp/paket.main.bzl linguist-generated=true
-/csharp/paket.main_extension.bzl linguist-generated=true
-
-# ripunzip tool
-/misc/ripunzip/ripunzip-* filter=lfs diff=lfs merge=lfs -text
-
-# swift prebuilt resources
-/swift/third_party/resource-dir/*.zip filter=lfs diff=lfs merge=lfs -text
+ruby/extractor/cargo-bazel-lock.json linguist-generated=true
+ruby/extractor/cargo-bazel-lock.json -merge

.github/workflows/build-ripunzip.yml

@@ -1,74 +0,0 @@
-name: Build runzip
-
-on:
-  workflow_dispatch:
-    inputs:
-      ripunzip-version:
-        description: "what reference to checktout from google/runzip"
-        required: false
-        default: v1.2.1
-      openssl-version:
-        description: "what reference to checkout from openssl/openssl for Linux"
-        required: false
-        default: openssl-3.3.0
-
-jobs:
-  build:
-    strategy:
-      fail-fast: false
-      matrix:
-        os: [ubuntu-20.04, macos-12, windows-2019]
-    runs-on: ${{ matrix.os }}
-    steps:
-      - uses: actions/checkout@v4
-        with:
-          repository: google/ripunzip
-          ref: ${{ inputs.ripunzip-version }}
-      # we need to avoid ripunzip dynamically linking into libssl
-      # see https://github.com/sfackler/rust-openssl/issues/183
-      - if: runner.os == 'Linux'
-        name: checkout openssl
-        uses: actions/checkout@v4
-        with:
-          repository: openssl/openssl
-          path: openssl
-          ref: ${{ inputs.openssl-version }}
-      - if: runner.os == 'Linux'
-        name: build and install openssl with fPIC
-        shell: bash
-        working-directory: openssl
-        run: |
-          ./config -fPIC --prefix=$HOME/.local --openssldir=$HOME/.local/ssl
-          make -j $(nproc)
-          make install_sw -j $(nproc)
-      - if: runner.os == 'Linux'
-        name: build (linux)
-        shell: bash
-        run: |
-          env OPENSSL_LIB_DIR=$HOME/.local/lib64 OPENSSL_INCLUDE_DIR=$HOME/.local/include OPENSSL_STATIC=yes cargo build --release
-          mv target/release/ripunzip ripunzip-linux
-      - if: runner.os == 'Windows'
-        name: build (windows)
-        shell: bash
-        run: |
-          cargo build --release
-          mv target/release/ripunzip ripunzip-windows
-      - name: build (macOS)
-        if: runner.os == 'macOS'
-        shell: bash
-        run: |
-          rustup target install x86_64-apple-darwin
-          rustup target install aarch64-apple-darwin
-          cargo build --target x86_64-apple-darwin --release
-          cargo build --target aarch64-apple-darwin --release
-          lipo -create -output ripunzip-macos \
-            -arch x86_64 target/x86_64-apple-darwin/release/ripunzip \
-            -arch arm64 target/aarch64-apple-darwin/release/ripunzip
-      - uses: actions/upload-artifact@v4
-        with:
-          name: ripunzip-${{ runner.os }}
-          path: ripunzip-*
-      - name: Check built binary
-        shell: bash
-        run: |
-          ./ripunzip-* --version

.github/workflows/buildifier.yml

@@ -24,5 +24,5 @@ jobs:
           extra_args: >
             buildifier --all-files 2>&1 ||
             (
-              echo -e "In order to format all bazel files, please run:\n  bazel run //misc/bazel:buildifier"; exit 1
+              echo -e "In order to format all bazel files, please run:\n  bazel run //:buildifier"; exit 1
             )

.github/workflows/codeql-analysis.yml

@@ -56,9 +56,7 @@ jobs:
     #    uses a compiled language
 
     - run: |
-       cd csharp
-       dotnet tool restore
-       dotnet build .
+       dotnet build csharp
 
     - name: Perform CodeQL Analysis
       uses: github/codeql-action/analyze@main

.github/workflows/csharp-qltest.yml

@@ -65,7 +65,7 @@ jobs:
           key: csharp-qltest-${{ matrix.slice }}
       - name: Run QL tests
         run: |
-          codeql test run --threads=0 --ram 50000 --slice ${{ matrix.slice }} --search-path "${{ github.workspace }}" --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 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 }}"
         env:
           GITHUB_TOKEN: ${{ github.token }}
   unit-tests:
@@ -81,11 +81,10 @@ jobs:
           dotnet-version: 8.0.101
       - name: Extractor unit tests
         run: |
-          dotnet tool restore
           dotnet test -p:RuntimeFrameworkVersion=8.0.1 extractor/Semmle.Util.Tests
           dotnet test -p:RuntimeFrameworkVersion=8.0.1 extractor/Semmle.Extraction.Tests
           dotnet test -p:RuntimeFrameworkVersion=8.0.1 autobuilder/Semmle.Autobuild.CSharp.Tests
-          dotnet test -p:RuntimeFrameworkVersion=8.0.1 autobuilder/Semmle.Autobuild.Cpp.Tests
+          dotnet test -p:RuntimeFrameworkVersion=8.0.1 "${{ github.workspace }}/cpp/autobuilder/Semmle.Autobuild.Cpp.Tests"
         shell: bash
   stubgentest:
     runs-on: ubuntu-latest
@@ -101,6 +100,6 @@ jobs:
           # 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 "${{ github.workspace }}" --check-databases --check-undefined-labels --check-repeated-labels --check-redefined-labels --consistency-queries ql/consistency-queries -- ql/test/library-tests/dataflow/flowsources/aspremote
+          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/go-tests-other-os.yml

@@ -7,9 +7,8 @@ on:
       - .github/workflows/go-tests-other-os.yml
       - .github/actions/**
       - codeql-workspace.yml
-      - MODULE.bazel
-      - .bazelrc
-      - misc/bazel/**
+env:
+  GO_VERSION: '~1.22.0'
 
 permissions:
   contents: read
@@ -19,17 +18,72 @@ jobs:
     name: Test MacOS
     runs-on: macos-latest
     steps:
+      - name: Set up Go ${{ env.GO_VERSION }}
+        uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.GO_VERSION }}
+          cache: false
+        id: go
+
       - name: Check out code
         uses: actions/checkout@v4
-      - name: Run tests
-        uses: ./go/actions/test
+
+      - name: Set up CodeQL CLI
+        uses: ./.github/actions/fetch-codeql
+
+      - name: Enable problem matchers in repository
+        shell: bash
+        run: 'find .github/problem-matchers -name \*.json -exec echo "::add-matcher::{}" \;'
+
+      - name: Build
+        run: |
+          cd go
+          make
+
+      - name: Cache compilation cache
+        id: query-cache
+        uses: ./.github/actions/cache-query-compilation
+        with:
+          key: go-qltest
+      - name: Test
+        run: |
+          cd go
+          make test cache="${{ steps.query-cache.outputs.cache-dir }}"
 
   test-win:
     if: github.repository_owner == 'github'
     name: Test Windows
     runs-on: windows-latest-xl
     steps:
+      - name: Set up Go ${{ env.GO_VERSION }}
+        uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.GO_VERSION }}
+          cache: false
+        id: go
+
       - name: Check out code
         uses: actions/checkout@v4
-      - name: Run tests
-        uses: ./go/actions/test
+
+      - name: Set up CodeQL CLI
+        uses: ./.github/actions/fetch-codeql
+
+      - name: Enable problem matchers in repository
+        shell: bash
+        run: 'find .github/problem-matchers -name \*.json -exec echo "::add-matcher::{}" \;'
+
+      - name: Build
+        run: |
+          cd go
+          make
+
+      - name: Cache compilation cache
+        id: query-cache
+        uses: ./.github/actions/cache-query-compilation
+        with:
+          key: go-qltest
+
+      - name: Test
+        run: |
+          cd go
+          make test cache="${{ steps.query-cache.outputs.cache-dir }}"

.github/workflows/go-tests.yml

@@ -15,9 +15,9 @@ on:
       - .github/workflows/go-tests.yml
       - .github/actions/**
       - codeql-workspace.yml
-      - MODULE.bazel
-      - .bazelrc
-      - misc/bazel/**
+
+env:
+  GO_VERSION: '~1.22.0'
 
 permissions:
   contents: read
@@ -28,9 +28,51 @@ jobs:
     name: Test Linux (Ubuntu)
     runs-on: ubuntu-latest-xl
     steps:
+      - name: Set up Go ${{ env.GO_VERSION }}
+        uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.GO_VERSION }}
+          cache: false
+        id: go
+
       - name: Check out code
         uses: actions/checkout@v4
-      - name: Run tests
-        uses: ./go/actions/test
+
+      - name: Set up CodeQL CLI
+        uses: ./.github/actions/fetch-codeql
+
+      - name: Enable problem matchers in repository
+        shell: bash
+        run: 'find .github/problem-matchers -name \*.json -exec echo "::add-matcher::{}" \;'
+
+      - name: Build
+        run: |
+          cd go
+          make
+
+      - name: Check that all Go code is autoformatted
+        run: |
+          cd go
+          make check-formatting
+
+      - name: Compile qhelp files to markdown
+        run: |
+          cd go
+          env QHELP_OUT_DIR=qhelp-out make qhelp-to-markdown
+
+      - name: Upload qhelp markdown
+        uses: actions/upload-artifact@v3
         with:
-          run-code-checks: true
+          name: qhelp-markdown
+          path: go/qhelp-out/**/*.md
+
+      - name: Cache compilation cache
+        id: query-cache
+        uses: ./.github/actions/cache-query-compilation
+        with:
+          key: go-qltest
+
+      - name: Test
+        run: |
+          cd go
+          make test cache="${{ steps.query-cache.outputs.cache-dir }}"

.github/workflows/kotlin-build.yml

@@ -1,28 +0,0 @@
-name: "Kotlin Build"
-
-on:
-  pull_request:
-    paths:
-      - "java/kotlin-extractor/**"
-      - "misc/bazel/**"
-      - "misc/codegen/**"
-      - "*.bazel*"
-      - .github/workflows/kotlin-build.yml
-    branches:
-      - main
-      - rc/*
-      - codeql-cli-*
-
-permissions:
-  contents: read
-
-jobs:
-  build:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v4
-      - run: |
-          bazel query //java/kotlin-extractor/...
-          # only build the default version as a quick check that we can build from `codeql`
-          # the full official build will be checked by QLucie
-          bazel build //java/kotlin-extractor

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

@@ -49,20 +49,20 @@ jobs:
           key: ${{ runner.os }}-${{ steps.os_version.outputs.version }}-rust-cargo-${{ hashFiles('ql/**/Cargo.lock') }}
       - name: Release build
         if: steps.cache-extractor.outputs.cache-hit != 'true'
-        run: cd ql; ./scripts/create-extractor-pack.sh
+        run: cd ql; ./scripts/create-extractor-pack.sh       
         env:
-          GH_TOKEN: ${{ github.token }}
+          GH_TOKEN: ${{ github.token }}   
       - name: Cache compilation cache
         id: query-cache
         uses: ./.github/actions/cache-query-compilation
-        with:
+        with: 
           key: run-ql-for-ql
       - name: Make database and analyze
         run: |
           ./ql/target/release/buramu | tee deprecated.blame # Add a blame file for the extractor to parse.
-          ${CODEQL} database create -l=ql ${DB} --search-path "${{ github.workspace }}"
+          ${CODEQL} database create -l=ql --search-path ql/extractor-pack ${DB}
           ${CODEQL} database analyze -j0 --format=sarif-latest --output=ql-for-ql.sarif ${DB} ql/ql/src/codeql-suites/ql-code-scanning.qls  --compilation-cache "${{ steps.query-cache.outputs.cache-dir }}"
-        env:
+        env: 
           CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
           DB: ${{ runner.temp }}/DB
           LGTM_INDEX_FILTERS: |

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

@@ -53,8 +53,8 @@ jobs:
       - name: Create database
         run: |
           "${CODEQL}" database create \
-          --search-path "${{ github.workspace }}"
-          --threads 4 \
+            --search-path "ql/extractor-pack" \
+            --threads 4 \
             --language ql --source-root "${{ github.workspace }}/repo" \
             "${{ runner.temp }}/database"
         env:

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

@@ -49,15 +49,15 @@ jobs:
       - name: Cache compilation cache
         id: query-cache
         uses: ./.github/actions/cache-query-compilation
-        with:
+        with: 
           key: ql-for-ql-tests
       - name: Run QL tests
         run: |
-          "${CODEQL}" test run --check-databases --check-unused-labels --check-repeated-labels --check-redefined-labels --check-use-before-definition --search-path "${{ github.workspace }}" --consistency-queries ql/ql/consistency-queries --compilation-cache "${{ steps.query-cache.outputs.cache-dir }}" ql/ql/test
+          "${CODEQL}" test run --check-databases --check-unused-labels --check-repeated-labels --check-redefined-labels --check-use-before-definition --search-path "${{ github.workspace }}/ql/extractor-pack" --consistency-queries ql/ql/consistency-queries --compilation-cache "${{ steps.query-cache.outputs.cache-dir }}" ql/ql/test
         env:
           CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
 
-  other-os:
+  other-os: 
     strategy:
       matrix:
         os: [macos-latest, windows-latest]
@@ -65,7 +65,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v4
-      - name: Install GNU tar
+      - name: Install GNU tar 
         if: runner.os == 'macOS'
         run: |
           brew install gnu-tar
@@ -100,7 +100,7 @@ jobs:
       - name: Run a single QL tests - Unix
         if: runner.os != 'Windows'
         run: |
-          "${CODEQL}" test run --check-databases --search-path "${{ github.workspace }}" ql/ql/test/queries/style/DeadCode/DeadCode.qlref
+          "${CODEQL}" test run --check-databases --search-path "${{ github.workspace }}/ql/extractor-pack" ql/ql/test/queries/style/DeadCode/DeadCode.qlref
         env:
           CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
       - name: Run a single QL tests - Windows
@@ -108,4 +108,5 @@ jobs:
         shell: pwsh
         run: |
           $Env:PATH += ";$(dirname ${{ steps.find-codeql.outputs.codeql-path }})"
-          codeql test run --check-databases --search-path "${{ github.workspace }}" ql/ql/test/queries/style/DeadCode/DeadCode.qlref
+          codeql test run --check-databases --search-path "${{ github.workspace }}/ql/extractor-pack" ql/ql/test/queries/style/DeadCode/DeadCode.qlref
+      

.github/workflows/ruby-build.yml

@@ -7,7 +7,6 @@ on:
       - .github/workflows/ruby-build.yml
       - .github/actions/fetch-codeql/action.yml
       - codeql-workspace.yml
-      - "shared/tree-sitter-extractor/**"
     branches:
       - main
       - "rc/*"
@@ -17,7 +16,6 @@ on:
       - .github/workflows/ruby-build.yml
       - .github/actions/fetch-codeql/action.yml
       - codeql-workspace.yml
-      - "shared/tree-sitter-extractor/**"
     branches:
       - main
       - "rc/*"

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

@@ -44,7 +44,7 @@ jobs:
       - name: Create database
         run: |
           codeql database create \
-            --search-path "${{ github.workspace }}" \
+            --search-path "${{ github.workspace }}/ruby/extractor-pack" \
             --threads 4 \
             --language ruby --source-root "${{ github.workspace }}/repo" \
             "${{ runner.temp }}/database"

.github/workflows/ruby-qltest.yml

@@ -64,10 +64,10 @@ jobs:
       - name: Cache compilation cache
         id: query-cache
         uses: ./.github/actions/cache-query-compilation
-        with:
+        with: 
           key: ruby-qltest
       - name: Run QL tests
         run: |
-          codeql test run --threads=0 --ram 50000 --search-path "${{ github.workspace }}" --check-databases --check-undefined-labels --check-unused-labels --check-repeated-labels --check-redefined-labels --check-use-before-definition --consistency-queries ql/consistency-queries ql/test --compilation-cache "${{ steps.query-cache.outputs.cache-dir }}"
+          codeql test run --threads=0 --ram 50000 --search-path "${{ github.workspace }}/ruby/extractor-pack" --check-databases --check-undefined-labels --check-unused-labels --check-repeated-labels --check-redefined-labels --check-use-before-definition --consistency-queries ql/consistency-queries ql/test --compilation-cache "${{ steps.query-cache.outputs.cache-dir }}"
         env:
           GITHUB_TOKEN: ${{ github.token }}

.github/workflows/swift.yml

@@ -68,6 +68,21 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - uses: ./swift/actions/run-ql-tests
+  integration-tests-linux:
+    if: github.repository_owner == 'github'
+    needs: build-and-test-linux
+    runs-on: ubuntu-latest-xl
+    steps:
+      - uses: actions/checkout@v4
+      - uses: ./swift/actions/run-integration-tests
+  integration-tests-macos:
+    if: ${{ github.repository_owner == 'github' && github.event_name == 'pull_request' }}
+    needs: build-and-test-macos
+    runs-on: macos-12-xl
+    timeout-minutes: 60
+    steps:
+      - uses: actions/checkout@v4
+      - uses: ./swift/actions/run-integration-tests
   clang-format:
     if : ${{ github.event_name == 'pull_request' }}
     runs-on: ubuntu-latest

.github/workflows/zipmerge-test.yml

@@ -1,23 +0,0 @@
-name: "Test zipmerge code"
-
-on:
-  pull_request:
-    paths:
-      - "misc/bazel/internal/zipmerge/**"
-      - "MODULE.bazel"
-      - ".bazelrc*"
-    branches:
-      - main
-      - "rc/*"
-
-permissions:
-  contents: read
-
-jobs:
-  test:
-    runs-on: ubuntu-latest
-
-    steps:
-      - uses: actions/checkout@v4
-      - run: |
-          bazel test //misc/bazel/internal/zipmerge:test --test_output=all

.gitignore

@@ -62,6 +62,3 @@ node_modules/
 
 # Temporary folders for working with generated models
 .model-temp
-
-# bazel-built in-tree extractor packs
-/*/extractor-pack

.lfsconfig

@@ -1,7 +0,0 @@
-[lfs]
-# codeql is publicly forked by many users, and we don't want any LFS file polluting their working
-# copies. We therefore exclude everything by default.
-# For files required by bazel builds, use rules in `misc/bazel/lfs.bzl` to download them on demand.
-# we go for `fetchinclude` to something not exsiting rather than `fetchexclude = *` because the
-# former is easier to override (with `git -c` or a local git config) to fetch something specific
-fetchinclude = /nothing

.pre-commit-config.yaml

@@ -26,17 +26,9 @@ repos:
         name: Format bazel files
         files: \.(bazel|bzl)
         language: system
-        entry: bazel run //misc/bazel:buildifier
+        entry: bazel run //:buildifier
         pass_filenames: false
 
-#      DISABLED: can be enabled by copying this config and installing `pre-commit` with `--config` on the copy
-#      - id: go-gen
-#        name: Check checked in generated files in go
-#        files: ^go/.*
-#        language: system
-#        entry: bazel run //go:gen
-#        pass_filenames: false
-
       - id: codeql-format
         name: Fix QL file formatting
         files: \.qll?$

BUILD.bazel

@@ -1 +1,9 @@
-exports_files(["LICENSE"])
+load("@buildifier_prebuilt//:rules.bzl", "buildifier")
+
+buildifier(
+    name = "buildifier",
+    lint_mode = "fix",
+    exclude_patterns = [
+        "./.git/*",
+    ],
+)

CODEOWNERS

@@ -1,7 +1,6 @@
 /cpp/ @github/codeql-c-analysis
+/cpp/autobuilder/ @github/codeql-c-extractor
 /csharp/ @github/codeql-csharp
-/csharp/autobuilder/Semmle.Autobuild.Cpp @github/codeql-c-extractor
-/csharp/autobuilder/Semmle.Autobuild.Cpp.Tests @github/codeql-c-extractor
 /go/ @github/codeql-go
 /java/ @github/codeql-java
 /javascript/ @github/codeql-javascript

CONTRIBUTING.md

@@ -4,8 +4,6 @@ We welcome contributions to our CodeQL libraries and queries. Got an idea for a
 
 There is lots of useful documentation to help you write queries, ranging from information about query file structure to tutorials for specific target languages. For more information on the documentation available, see [CodeQL queries](https://codeql.github.com/docs/writing-codeql-queries/codeql-queries) on [codeql.github.com](https://codeql.github.com).
 
-Note that the CodeQL for Visual Studio Code documentation has been migrated to https://docs.github.com/en/code-security/codeql-for-vs-code/, but you can still contribute to it via a different repository. For more information, see [Contributing to GitHub Docs documentation](https://docs.github.com/en/contributing)."
-
 ## Change notes
 
 Any nontrivial user-visible change to a query pack or library pack should have a change note. For details on how to add a change note for your change, see [this guide](docs/change-notes.md).
@@ -45,7 +43,7 @@ If you have an idea for a query that you would like to share with other CodeQL u
 
 3. **Formatting**
 
-    - The queries and libraries must be autoformatted, for example using the "Format Document" command in [CodeQL for Visual Studio Code](https://docs.github.com/en/code-security/codeql-for-vs-code/).
+    - The queries and libraries must be autoformatted, for example using the "Format Document" command in [CodeQL for Visual Studio Code](https://codeql.github.com/docs/codeql-for-visual-studio-code/about-codeql-for-visual-studio-code).
 
     If you prefer, you can either:
     1. install the [pre-commit framework](https://pre-commit.com/) and install the configured hooks on this repo via `pre-commit install`, or

MODULE.bazel

@@ -13,54 +13,17 @@ local_path_override(
 
 # see https://registry.bazel.build/ for a list of available packages
 
-bazel_dep(name = "platforms", version = "0.0.10")
-bazel_dep(name = "rules_go", version = "0.48.0")
+bazel_dep(name = "platforms", version = "0.0.8")
 bazel_dep(name = "rules_pkg", version = "0.10.1")
-bazel_dep(name = "rules_nodejs", version = "6.2.0-codeql.1")
-bazel_dep(name = "rules_python", version = "0.32.2")
-bazel_dep(name = "bazel_skylib", version = "1.6.1")
+bazel_dep(name = "rules_nodejs", version = "6.0.3")
+bazel_dep(name = "rules_python", version = "0.31.0")
+bazel_dep(name = "bazel_skylib", version = "1.5.0")
 bazel_dep(name = "abseil-cpp", version = "20240116.0", repo_name = "absl")
 bazel_dep(name = "nlohmann_json", version = "3.11.3", repo_name = "json")
 bazel_dep(name = "fmt", version = "10.0.0")
-bazel_dep(name = "rules_kotlin", version = "1.9.4-codeql.1")
-bazel_dep(name = "gazelle", version = "0.37.0")
-bazel_dep(name = "rules_dotnet", version = "0.15.1")
-bazel_dep(name = "googletest", version = "1.14.0.bcr.1")
-bazel_dep(name = "rules_rust", version = "0.46.0")
 
 bazel_dep(name = "buildifier_prebuilt", version = "6.4.0", dev_dependency = True)
 
-crate = use_extension(
-    "@rules_rust//crate_universe:extension.bzl",
-    "crate",
-)
-crate.from_cargo(
-    name = "py_deps",
-    cargo_lockfile = "//python/extractor/tsg-python:Cargo.lock",
-    manifests = [
-        "//python/extractor/tsg-python:Cargo.toml",
-        "//python/extractor/tsg-python/tsp:Cargo.toml",
-    ],
-)
-crate.from_cargo(
-    name = "ruby_deps",
-    cargo_lockfile = "//ruby/extractor:Cargo.lock",
-    manifests = [
-        "//ruby/extractor:Cargo.toml",
-        "//ruby/extractor/codeql-extractor-fake-crate:Cargo.toml",
-    ],
-)
-use_repo(crate, "py_deps", "ruby_deps")
-
-dotnet = use_extension("@rules_dotnet//dotnet:extensions.bzl", "dotnet")
-dotnet.toolchain(dotnet_version = "8.0.101")
-use_repo(dotnet, "dotnet_toolchains")
-
-register_toolchains("@dotnet_toolchains//:all")
-
-csharp_main_extension = use_extension("//csharp:paket.main_extension.bzl", "main_extension")
-use_repo(csharp_main_extension, "paket.main")
-
 pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip")
 pip.parse(
     hub_name = "codegen_deps",
@@ -85,92 +48,10 @@ use_repo(
 node = use_extension("@rules_nodejs//nodejs:extensions.bzl", "node")
 node.toolchain(
     name = "nodejs",
-    node_urls = [
-        "https://nodejs.org/dist/v{version}/{filename}",
-        "https://mirrors.dotsrc.org/nodejs/release/v{version}/{filename}",
-    ],
     node_version = "18.15.0",
 )
 use_repo(node, "nodejs", "nodejs_toolchains")
 
-kotlin_extractor_deps = use_extension("//java/kotlin-extractor:deps.bzl", "kotlin_extractor_deps")
-
-# following list can be kept in sync by running `bazel mod tidy` in `codeql`
-use_repo(
-    kotlin_extractor_deps,
-    "codeql_kotlin_defaults",
-    "codeql_kotlin_embeddable",
-    "kotlin-compiler-1.5.0",
-    "kotlin-compiler-1.5.10",
-    "kotlin-compiler-1.5.20",
-    "kotlin-compiler-1.5.30",
-    "kotlin-compiler-1.6.0",
-    "kotlin-compiler-1.6.20",
-    "kotlin-compiler-1.7.0",
-    "kotlin-compiler-1.7.20",
-    "kotlin-compiler-1.8.0",
-    "kotlin-compiler-1.9.0-Beta",
-    "kotlin-compiler-1.9.20-Beta",
-    "kotlin-compiler-2.0.0-RC1",
-    "kotlin-compiler-embeddable-1.5.0",
-    "kotlin-compiler-embeddable-1.5.10",
-    "kotlin-compiler-embeddable-1.5.20",
-    "kotlin-compiler-embeddable-1.5.30",
-    "kotlin-compiler-embeddable-1.6.0",
-    "kotlin-compiler-embeddable-1.6.20",
-    "kotlin-compiler-embeddable-1.7.0",
-    "kotlin-compiler-embeddable-1.7.20",
-    "kotlin-compiler-embeddable-1.8.0",
-    "kotlin-compiler-embeddable-1.9.0-Beta",
-    "kotlin-compiler-embeddable-1.9.20-Beta",
-    "kotlin-compiler-embeddable-2.0.0-RC1",
-    "kotlin-stdlib-1.5.0",
-    "kotlin-stdlib-1.5.10",
-    "kotlin-stdlib-1.5.20",
-    "kotlin-stdlib-1.5.30",
-    "kotlin-stdlib-1.6.0",
-    "kotlin-stdlib-1.6.20",
-    "kotlin-stdlib-1.7.0",
-    "kotlin-stdlib-1.7.20",
-    "kotlin-stdlib-1.8.0",
-    "kotlin-stdlib-1.9.0-Beta",
-    "kotlin-stdlib-1.9.20-Beta",
-    "kotlin-stdlib-2.0.0-RC1",
-)
-
-go_sdk = use_extension("@rules_go//go:extensions.bzl", "go_sdk")
-go_sdk.download(version = "1.22.2")
-
-lfs_files = use_repo_rule("//misc/bazel:lfs.bzl", "lfs_files")
-
-lfs_files(
-    name = "ripunzip-linux",
-    srcs = ["//misc/ripunzip:ripunzip-linux"],
-    executable = True,
-)
-
-lfs_files(
-    name = "ripunzip-windows",
-    srcs = ["//misc/ripunzip:ripunzip-windows.exe"],
-    executable = True,
-)
-
-lfs_files(
-    name = "ripunzip-macos",
-    srcs = ["//misc/ripunzip:ripunzip-macos"],
-    executable = True,
-)
-
-lfs_files(
-    name = "swift-resource-dir-linux",
-    srcs = ["//swift/third_party/resource-dir:resource-dir-linux.zip"],
-)
-
-lfs_files(
-    name = "swift-resource-dir-macos",
-    srcs = ["//swift/third_party/resource-dir:resource-dir-macos.zip"],
-)
-
 register_toolchains(
     "@nodejs_toolchains//:all",
 )

README.md

@@ -4,7 +4,7 @@ This open source repository contains the standard CodeQL libraries and queries t
 
 ## How do I learn CodeQL and run queries?
 
-There is extensive documentation about the [CodeQL language](https://codeql.github.com/docs/), writing CodeQL using the [CodeQL extension for Visual Studio Code](https://docs.github.com/en/code-security/codeql-for-vs-code/) and using the [CodeQL CLI](https://docs.github.com/en/code-security/codeql-cli).
+There is [extensive documentation](https://codeql.github.com/docs/) on getting started with writing CodeQL using the [CodeQL extension for Visual Studio Code](https://codeql.github.com/docs/codeql-for-visual-studio-code/) and the [CodeQL CLI](https://codeql.github.com/docs/codeql-cli/).
 
 ## Contributing
 

codeql-workspace.yml

@@ -6,16 +6,19 @@ provide:
   - "*/ql/consistency-queries/qlpack.yml"
   - "*/ql/automodel/src/qlpack.yml"
   - "*/ql/automodel/test/qlpack.yml"
-  - "*/extractor-pack/codeql-extractor.yml"
   - "python/extractor/qlpack.yml"
   - "shared/**/qlpack.yml"
   - "cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/qlpack.yml"
   - "go/ql/config/legacy-support/qlpack.yml"
+  - "go/build/codeql-extractor-go/codeql-extractor.yml"
   - "csharp/ql/campaigns/Solorigate/lib/qlpack.yml"
   - "csharp/ql/campaigns/Solorigate/src/qlpack.yml"
   - "csharp/ql/campaigns/Solorigate/test/qlpack.yml"
   - "misc/legacy-support/*/qlpack.yml"
   - "misc/suite-helpers/qlpack.yml"
+  - "ruby/extractor-pack/codeql-extractor.yml"
+  - "swift/extractor-pack/codeql-extractor.yml"
+  - "ql/extractor-pack/codeql-extractor.yml"
   - ".github/codeql/extensions/**/codeql-pack.yml"
 
 versionPolicies:

config/dbscheme-fragments.json

@@ -28,7 +28,6 @@
     "/*- Yaml dbscheme -*/",
     "/*- Blame dbscheme -*/",
     "/*- JSON dbscheme -*/",
-    "/*- Python dbscheme -*/",
-    "/*- Empty location -*/"
+    "/*- Python dbscheme -*/"
   ]
 }

config/identical-files.json

@@ -61,6 +61,10 @@
     "java/ql/src/utils/modelgenerator/internal/CaptureModels.qll",
     "csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll"
   ],
+  "Model as Data Generation Java/C# - CaptureModelsPrinting": [
+    "java/ql/src/utils/modelgenerator/internal/CaptureModelsPrinting.qll",
+    "csharp/ql/src/utils/modelgenerator/internal/CaptureModelsPrinting.qll"
+  ],
   "Sign Java/C#": [
     "java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/Sign.qll",
     "csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/Sign.qll"
@@ -181,6 +185,11 @@
     "cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysisImports.qll",
     "cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysisImports.qll"
   ],
+  "C++ IR ValueNumberingImports": [
+    "cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/gvn/internal/ValueNumberingImports.qll",
+    "cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/internal/ValueNumberingImports.qll",
+    "cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/internal/ValueNumberingImports.qll"
+  ],
   "IR SSA SSAConstruction": [
     "cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll",
     "cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll"
@@ -353,11 +362,7 @@
     "java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll"
   ],
   "Python model summaries test extension": [
-    "python/ql/test/library-tests/dataflow/model-summaries/InlineTaintTest.ext.yml",
-    "python/ql/test/library-tests/dataflow/model-summaries/NormalDataflowTest.ext.yml"
-  ],
-  "shared tree-sitter extractor cargo.toml": [
-    "shared/tree-sitter-extractor/Cargo.toml",
-    "ruby/extractor/codeql-extractor-fake-crate/Cargo.toml"
+    "python/ql/test/experimental/dataflow/model-summaries/InlineTaintTest.ext.yml",
+    "python/ql/test/experimental/dataflow/model-summaries/NormalDataflowTest.ext.yml"
   ]
-}
+}

cpp/autobuilder/.gitignore

@@ -0,0 +1,13 @@
+obj/
+TestResults/
+*.manifest
+*.pdb
+*.suo
+*.mdb
+*.vsmdi
+csharp.log
+**/bin/Debug
+**/bin/Release
+*.tlog
+.vs
+*.user

cpp/autobuilder/README.md

@@ -1 +0,0 @@
-The Windows autobuilder that used to live in this directory moved to `csharp/autobuilder/Semmle.Autobuild.Cpp`.

cpp/autobuilder/Semmle.Autobuild.Cpp.Tests/BuildScripts.cs

@@ -200,9 +200,9 @@ namespace Semmle.Autobuild.Cpp.Tests
 
     internal class TestDiagnosticWriter : IDiagnosticsWriter
     {
-        public IList<Semmle.Util.DiagnosticMessage> Diagnostics { get; } = new List<Semmle.Util.DiagnosticMessage>();
+        public IList<DiagnosticMessage> Diagnostics { get; } = new List<DiagnosticMessage>();
 
-        public void AddEntry(Semmle.Util.DiagnosticMessage message) => this.Diagnostics.Add(message);
+        public void AddEntry(DiagnosticMessage message) => this.Diagnostics.Add(message);
 
         public void Dispose() { }
     }

cpp/autobuilder/Semmle.Autobuild.Cpp.Tests/Semmle.Autobuild.Cpp.Tests.csproj

@@ -0,0 +1,26 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFramework>net8.0</TargetFramework>
+    <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
+    <RuntimeIdentifiers>win-x64;linux-x64;osx-x64</RuntimeIdentifiers>
+    <Nullable>enable</Nullable>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="System.IO.FileSystem" Version="4.3.0" />
+    <PackageReference Include="System.IO.FileSystem.Primitives" Version="4.3.0" />
+    <PackageReference Include="xunit" Version="2.6.2" />
+    <PackageReference Include="xunit.runner.visualstudio" Version="2.5.4">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
+    </PackageReference>
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\Semmle.Autobuild.Cpp\Semmle.Autobuild.Cpp.csproj" />
+    <ProjectReference Include="..\..\..\csharp\autobuilder\Semmle.Autobuild.Shared\Semmle.Autobuild.Shared.csproj" />
+  </ItemGroup>
+</Project>

cpp/autobuilder/Semmle.Autobuild.Cpp/Program.cs

@@ -22,12 +22,12 @@ namespace Semmle.Autobuild.Cpp
                 }
                 catch (InvalidEnvironmentException ex)
                 {
-                    Console.WriteLine($"The environment is invalid: {ex.Message}");
+                    Console.WriteLine("The environment is invalid: {0}", ex.Message);
                 }
             }
             catch (ArgumentOutOfRangeException ex)
             {
-                Console.WriteLine($"The value \"{ex.ActualValue}\" for parameter \"{ex.ParamName}\" is invalid");
+                Console.WriteLine("The value \"{0}\" for parameter \"{1}\" is invalid", ex.ActualValue, ex.ParamName);
             }
             return 1;
         }

cpp/autobuilder/Semmle.Autobuild.Cpp/Properties/AssemblyInfo.cs

@@ -0,0 +1,32 @@
+using System.Reflection;
+using System.Runtime.InteropServices;
+
+// General Information about an assembly is controlled through the following
+// set of attributes. Change these attribute values to modify the information
+// associated with an assembly.
+[assembly: AssemblyTitle("Semmle.Autobuild.Cpp")]
+[assembly: AssemblyDescription("")]
+[assembly: AssemblyConfiguration("")]
+[assembly: AssemblyCompany("GitHub")]
+[assembly: AssemblyProduct("CodeQL autobuilder for C++")]
+[assembly: AssemblyCopyright("Copyright © GitHub 2020")]
+[assembly: AssemblyTrademark("")]
+[assembly: AssemblyCulture("")]
+
+// Setting ComVisible to false makes the types in this assembly not visible
+// to COM components.  If you need to access a type in this assembly from
+// COM, set the ComVisible attribute to true on that type.
+[assembly: ComVisible(false)]
+
+// Version information for an assembly consists of the following four values:
+//
+//      Major Version
+//      Minor Version
+//      Build Number
+//      Revision
+//
+// You can specify all the values or you can default the Build and Revision Numbers
+// by using the '*' as shown below:
+// [assembly: AssemblyVersion("1.0.*")]
+[assembly: AssemblyVersion("1.0.0.0")]
+[assembly: AssemblyFileVersion("1.0.0.0")]

cpp/autobuilder/Semmle.Autobuild.Cpp/Semmle.Autobuild.Cpp.csproj

@@ -0,0 +1,28 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>net8.0</TargetFramework>
+    <AssemblyName>Semmle.Autobuild.Cpp</AssemblyName>
+    <RootNamespace>Semmle.Autobuild.Cpp</RootNamespace>
+    <ApplicationIcon />
+    <OutputType>Exe</OutputType>
+    <StartupObject />
+    <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
+    <RuntimeIdentifiers>win-x64;linux-x64;osx-x64</RuntimeIdentifiers>
+    <Nullable>enable</Nullable>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <Folder Include="Properties\" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.Build" Version="17.8.3" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\..\..\csharp\extractor\Semmle.Util\Semmle.Util.csproj" />
+    <ProjectReference Include="..\..\..\csharp\autobuilder\Semmle.Autobuild.Shared\Semmle.Autobuild.Shared.csproj" />
+  </ItemGroup>
+
+</Project>

cpp/downgrades/19887dbd33327fb07d54251786e0cb2578539775/upgrade.properties

@@ -1,4 +1,4 @@
 description: Revert support for repeated initializers, which are allowed in C with designated initializers.
 compatibility: full
-aggregate_field_init.rel: reorder aggregate_field_init.rel (@aggregateliteral aggregate, @expr initializer, @membervariable field, int position) aggregate initializer field
-aggregate_array_init.rel: reorder aggregate_array_init.rel (@aggregateliteral aggregate, @expr initializer, int element_index, int position) aggregate initializer element_index
+aggregate_field_init.rel: reorder aggregate_field_init.rel (int aggregate, int initializer, int field, int position) aggregate initializer field
+aggregate_array_init.rel: reorder aggregate_array_init.rel (int aggregate, int initializer, int element_index, int position) aggregate initializer element_index

cpp/downgrades/BUILD.bazel

@@ -6,7 +6,7 @@ pkg_files(
         ["**"],
         exclude = ["BUILD.bazel"],
     ),
-    prefix = "downgrades",
+    prefix = "cpp/downgrades",
     strip_prefix = strip_prefix.from_pkg(),
     visibility = ["//cpp:__pkg__"],
 )

cpp/ql/lib/BUILD.bazel

@@ -5,9 +5,11 @@ package(default_visibility = ["//cpp:__pkg__"])
 pkg_files(
     name = "dbscheme",
     srcs = ["semmlecode.cpp.dbscheme"],
+    prefix = "cpp",
 )
 
 pkg_files(
     name = "dbscheme-stats",
     srcs = ["semmlecode.cpp.dbscheme.stats"],
+    prefix = "cpp",
 )

cpp/ql/lib/CHANGELOG.md

@@ -1,40 +1,3 @@
-## 1.1.0
-
-### New Features
-
-* Data models can now be added with data extensions. In this way source, sink and summary models can be added in extension `.model.yml` files, rather than by writing classes in QL code. New models should be added in the `lib/ext` folder.
-
-### Minor Analysis Improvements
-
-* A partial model for the `Boost.Asio` network library has been added. This includes sources, sinks and summaries for certain functions in `Boost.Asio`, such as `read_until` and `write`.
-
-## 1.0.0
-
-### Breaking Changes
-
-* CodeQL package management is now generally available, and all GitHub-produced CodeQL packages have had their version numbers increased to 1.0.0.
-
-## 0.13.1
-
-No user-facing changes.
-
-## 0.13.0
-
-### Breaking Changes
-
-* Deleted the deprecated `GlobalValueNumberingImpl.qll` implementation.
-
-### New Features
-
-* Models-as-Data support has been added for C/C++. This feature allows flow sources, sinks and summaries to be expressed in compact strings as an alternative to modelling each source / sink / summary with explicit QL. See `dataflow/ExternalFlow.qll` for documentation and specification of the model format, and `models/implementations/ZMQ.qll` for a simple example of models. Importing models from `.yml` is not yet supported.
-
-### Minor Analysis Improvements
-
-* Source models have been added for the standard library function `getc` (and variations).
-* Source, sink and flow models for the ZeroMQ (ZMQ) networking library have been added.
-* Parameters of functions without definitions now have `ParameterNode`s.
-* The alias analysis used internally by various libraries has been improved to answer alias questions more conservatively. As a result, some queries may report fewer false positives.
-
 ## 0.12.11
 
 No user-facing changes.

cpp/ql/lib/change-notes/2024-04-05-sound-ir.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The alias analysis used internally by various libraries has been improved to answer alias questions more conservatively. As a result, some queries may report fewer false positives.

cpp/ql/lib/change-notes/2024-04-18-param-nodes.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Parameters of functions without definitions now have `ParameterNode`s.

cpp/ql/lib/change-notes/2024-06-10-builtin-expect.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* The "Guards" library (`semmle.code.cpp.controlflow.Guards`) now also infers guards from calls to the builtin operation `__builtin_expect`. As a result, some queries may produce fewer false positives.

cpp/ql/lib/change-notes/2024-06-13-double-free.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* The queries "Potential double free" (`cpp/double-free`) and "Potential use after free" (`cpp/use-after-free`) now produce fewer false positives.

cpp/ql/lib/change-notes/2024-06-20-extensible-allocation-deallocation.md

@@ -1,4 +0,0 @@
----
-category: feature
----
-* It is now possible to extend the classes `AllocationFunction` and `DeallocationFunction` via data extensions. Extensions of these classes should be added to the `lib/ext/allocation` and `lib/ext/deallocation` directories respectively.

cpp/ql/lib/change-notes/2024-10-04-getc.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Source models have been added for the standard library function `getc` (and variations).

cpp/ql/lib/change-notes/2024-10-04-models-as-data.md

@@ -0,0 +1,4 @@
+---
+category: feature
+---
+* Models-as-Data support has been added for C/C++. This feature allows flow sources, sinks and summaries to be expressed in compact strings as an alternative to modelling each source / sink / summary with explicit QL. See `dataflow/ExternalFlow.qll` for documentation and specification of the model format, and `models/implementations/ZMQ.qll` for a simple example of models. Importing models from `.yml` is not yet supported.

cpp/ql/lib/change-notes/2024-10-04-zmq.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Source, sink and flow models for the ZeroMQ (ZMQ) networking library have been added.

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

@@ -1,16 +0,0 @@
-## 0.13.0
-
-### Breaking Changes
-
-* Deleted the deprecated `GlobalValueNumberingImpl.qll` implementation.
-
-### New Features
-
-* Models-as-Data support has been added for C/C++. This feature allows flow sources, sinks and summaries to be expressed in compact strings as an alternative to modelling each source / sink / summary with explicit QL. See `dataflow/ExternalFlow.qll` for documentation and specification of the model format, and `models/implementations/ZMQ.qll` for a simple example of models. Importing models from `.yml` is not yet supported.
-
-### Minor Analysis Improvements
-
-* Source models have been added for the standard library function `getc` (and variations).
-* Source, sink and flow models for the ZeroMQ (ZMQ) networking library have been added.
-* Parameters of functions without definitions now have `ParameterNode`s.
-* The alias analysis used internally by various libraries has been improved to answer alias questions more conservatively. As a result, some queries may report fewer false positives.

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

@@ -1,3 +0,0 @@
-## 0.13.1
-
-No user-facing changes.

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

@@ -1,5 +0,0 @@
-## 1.0.0
-
-### Breaking Changes
-
-* CodeQL package management is now generally available, and all GitHub-produced CodeQL packages have had their version numbers increased to 1.0.0.

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

@@ -1,9 +0,0 @@
-## 1.1.0
-
-### New Features
-
-* Data models can now be added with data extensions. In this way source, sink and summary models can be added in extension `.model.yml` files, rather than by writing classes in QL code. New models should be added in the `lib/ext` folder.
-
-### Minor Analysis Improvements
-
-* A partial model for the `Boost.Asio` network library has been added. This includes sources, sinks and summaries for certain functions in `Boost.Asio`, such as `read_until` and `write`.

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 1.1.0
+lastReleaseVersion: 0.12.11

cpp/ql/lib/ext/Boost.Asio.model.yml

@@ -1,26 +0,0 @@
-extensions:
-  # partial model of the Boost::Asio network library
-extensions:
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: sourceModel
-    data: # namespace, type, subtypes, name, signature, ext, output, kind, provenance
-      - ["boost::asio", "", False, "read", "", "", "Argument[*1]", "remote", "manual"]
-      - ["boost::asio", "", False, "read_at", "", "", "Argument[*2]", "remote", "manual"]
-      - ["boost::asio", "", False, "read_until", "", "", "Argument[*1]", "remote", "manual"]
-      - ["boost::asio", "", False, "async_read", "", "", "Argument[*1]", "remote", "manual"]
-      - ["boost::asio", "", False, "async_read_at", "", "", "Argument[*2]", "remote", "manual"]
-      - ["boost::asio", "", False, "async_read_until", "", "", "Argument[*1]", "remote", "manual"]
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: sinkModel
-    data: # namespace, type, subtypes, name, signature, ext, input, kind, provenance
-      - ["boost::asio", "", False, "write", "", "", "Argument[*1]", "remote-sink", "manual"]
-      - ["boost::asio", "", False, "write_at", "", "", "Argument[*2]", "remote-sink", "manual"]
-      - ["boost::asio", "", False, "async_write", "", "", "Argument[*1]", "remote-sink", "manual"]
-      - ["boost::asio", "", False, "async_write_at", "", "", "Argument[*2]", "remote-sink", "manual"]
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: summaryModel
-    data: # namespace, type, subtypes, name, signature, ext, input, output, kind, provenance
-      - ["boost::asio", "", False, "buffer", "", "", "Argument[*0]", "ReturnValue", "taint", "manual"]

cpp/ql/lib/ext/allocation/Bsd.allocation.model.yml

@@ -1,7 +0,0 @@
-extensions:
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: allocationFunctionModel
-    data:
-      - ["", "", False, "kmem_alloc", "0", "", "", True]
-      - ["", "", False, "kmem_zalloc", "0", "", "", True]

cpp/ql/lib/ext/allocation/Glibc.allocation.model.yml

@@ -1,7 +0,0 @@
-extensions:
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: allocationFunctionModel
-    data:
-      - ["", "", False, "g_malloc", "0", "", "", True]
-      - ["", "", False, "g_try_malloc", "0", "", "", True]

cpp/ql/lib/ext/allocation/OpenSSL.allocation.model.yml

@@ -1,10 +0,0 @@
-extensions:
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: allocationFunctionModel
-    data:
-      - ["", "", False, "CRYPTO_malloc", "0", "", "", True]
-      - ["", "", False, "CRYPTO_zalloc", "0", "", "", True]
-      - ["", "", False, "CRYPTO_secure_malloc", "0", "", "", True]
-      - ["", "", False, "CRYPTO_secure_zalloc", "0", "", "", True]
-

cpp/ql/lib/ext/allocation/Std.allocation.model.yml

@@ -1,15 +0,0 @@
-extensions:
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: allocationFunctionModel
-    data:
-      - ["", "", False, "malloc", "0", "", "", True]
-      - ["std", "", False, "malloc", "0", "", "", True]
-      - ["bsl", "", False, "malloc", "0", "", "", True]
-      - ["", "", False, "alloca", "0", "", "", False]
-      - ["", "", False, "__builtin_alloca", "0", "", "", False]
-      - ["", "", False, "_alloca", "0", "", "", False]
-      - ["", "", False, "_malloca", "0", "", "", False]
-      - ["", "", False, "calloc", "1", "0", "", True]
-      - ["std", "", False, "calloc", "1", "0", "", True]
-      - ["bsl", "", False, "calloc", "1", "0", "", True]

cpp/ql/lib/ext/allocation/Windows.allocation.model.yml

@@ -1,29 +0,0 @@
-extensions:
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: allocationFunctionModel
-    data:
-      - ["", "", False, "MmAllocateContiguousMemory", "0", "", "", True]
-      - ["", "", False, "MmAllocateContiguousNodeMemory", "0", "", "", True]
-      - ["", "", False, "MmAllocateContiguousMemorySpecifyCache", "0", "", "", True]
-      - ["", "", False, "MmAllocateContiguousMemorySpecifyCacheNode", "0", "", "", True]
-      - ["", "", False, "MmAllocateNonCachedMemory", "0", "", "", True]
-      - ["", "", False, "MmAllocateMappingAddress", "0", "", "", True]
-      - ["", "", False, "CoTaskMemAlloc", "0", "", "", True]
-      - ["", "", False, "ExAllocatePool", "1", "", "", True]
-      - ["", "", False, "ExAllocatePool2", "1", "", "", True]
-      - ["", "", False, "ExAllocatePool3", "1", "", "", True]
-      - ["", "", False, "ExAllocatePoolWithTag", "1", "", "", True]
-      - ["", "", False, "ExAllocatePoolWithTagPriority", "1", "", "", True]
-      - ["", "", False, "ExAllocatePoolWithQuota", "1", "", "", True]
-      - ["", "", False, "ExAllocatePoolWithQuotaTag", "1", "", "", True]
-      - ["", "", False, "ExAllocatePoolZero", "1", "", "", True]
-      - ["", "", False, "IoAllocateMdl", "1", "", "", True]
-      - ["", "", False, "IoAllocateErrorLogEntry", "1", "", "", True]
-      - ["", "", False, "LocalAlloc", "1", "", "", True]
-      - ["", "", False, "GlobalAlloc", "1", "", "", True]
-      - ["", "", False, "VirtualAlloc", "1", "", "", True]
-      - ["", "", False, "HeapAlloc", "2", "", "", True]
-      - ["", "", False, "MmAllocatePagesForMdl", "3", "", "", True]
-      - ["", "", False, "MmAllocatePagesForMdlEx", "3", "", "", True]
-      - ["", "", False, "MmAllocateNodePagesForMdlEx", "3", "", "", True]

cpp/ql/lib/ext/allocation/empty.allocation.model.yml

@@ -1,5 +0,0 @@
-extensions:
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: allocationFunctionModel
-    data: []

cpp/ql/lib/ext/deallocation/Bsd.deallocation.model.yml

@@ -1,8 +0,0 @@
-extensions:
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: deallocationFunctionModel
-    data:
-      - ["", "", False, "pool_put", "1"]
-      - ["", "", False, "pool_cache_put", "1"]
-      - ["", "", False, "kmem_free", "0"]

cpp/ql/lib/ext/deallocation/Std.deallocation.model.yml

@@ -1,42 +0,0 @@
-extensions:
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: deallocationFunctionModel
-    data:
-      - ["", "", False, "free", "0"]
-      - ["std", "", False, "free", "0"]
-      - ["bsl", "", False, "free", "0"]
-      - ["", "", False, "realloc", "0"]
-      - ["std", "", False, "realloc", "0"]
-      - ["bsl", "", False, "realloc", "0"]
-      - ["", "", False, "CRYPTO_free", "0"]
-      - ["", "", False, "CRYPTO_secure_free", "0"]
-      - ["", "", False, "g_free", "0"]
-      - ["", "", False, "ExFreePool", "0"]
-      - ["", "", False, "ExFreePoolWithTag", "0"]
-      - ["", "", False, "ExDeleteTimer", "0"]
-      - ["", "", False, "IoFreeIrp", "0"]
-      - ["", "", False, "IoFreeMdl", "0"]
-      - ["", "", False, "IoFreeErrorLogEntry", "0"]
-      - ["", "", False, "IoFreeWorkItem", "0"]
-      - ["", "", False, "MmFreeContiguousMemory", "0"]
-      - ["", "", False, "MmFreeContiguousMemorySpecifyCache", "0"]
-      - ["", "", False, "MmFreeNonCachedMemory", "0"]
-      - ["", "", False, "MmFreeMappingAddress", "0"]
-      - ["", "", False, "MmFreePagesFromMdl", "0"]
-      - ["", "", False, "MmUnmapReservedMapping", "0"]
-      - ["", "", False, "MmUnmapLockedPages", "0"]
-      - ["", "", False, "NdisFreeGenericObject", "0"]
-      - ["", "", False, "NdisFreeMemory", "0"]
-      - ["", "", False, "NdisFreeMemoryWithTag", "0"]
-      - ["", "", False, "NdisFreeMdl", "0"]
-      - ["", "", False, "NdisFreeNetBufferListPool", "0"]
-      - ["", "", False, "NdisFreeNetBufferPool", "0"]
-      - ["", "", False, "LocalFree", "0"]
-      - ["", "", False, "GlobalFree", "0"]
-      - ["", "", False, "LocalReAlloc", "0"]
-      - ["", "", False, "GlobalReAlloc", "0"]
-      - ["", "", False, "VirtualFree", "0"]
-      - ["", "", False, "CoTaskMemFree", "0"]
-      - ["", "", False, "CoTaskMemRealloc", "0"]
-      - ["", "", False, "SysFreeString", "0"]

cpp/ql/lib/ext/deallocation/Windows.deallocation.model.yml

@@ -1,41 +0,0 @@
-extensions:
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: deallocationFunctionModel
-    data:
-      - ["", "", False, "ExFreePool", "0"]
-      - ["", "", False, "ExFreePoolWithTag", "0"]
-      - ["", "", False, "ExDeleteTimer", "0"]
-      - ["", "", False, "IoFreeIrp", "0"]
-      - ["", "", False, "IoFreeMdl", "0"]
-      - ["", "", False, "IoFreeErrorLogEntry", "0"]
-      - ["", "", False, "IoFreeWorkItem", "0"]
-      - ["", "", False, "MmFreeContiguousMemory", "0"]
-      - ["", "", False, "MmFreeContiguousMemorySpecifyCache", "0"]
-      - ["", "", False, "MmFreeNonCachedMemory", "0"]
-      - ["", "", False, "MmFreeMappingAddress", "0"]
-      - ["", "", False, "MmFreePagesFromMdl", "0"]
-      - ["", "", False, "MmUnmapReservedMapping", "0"]
-      - ["", "", False, "MmUnmapLockedPages", "0"]
-      - ["", "", False, "NdisFreeGenericObject", "0"]
-      - ["", "", False, "NdisFreeMemory", "0"]
-      - ["", "", False, "NdisFreeMemoryWithTag", "0"]
-      - ["", "", False, "NdisFreeMdl", "0"]
-      - ["", "", False, "NdisFreeNetBufferListPool", "0"]
-      - ["", "", False, "NdisFreeNetBufferPool", "0"]
-      - ["", "", False, "LocalFree", "0"]
-      - ["", "", False, "GlobalFree", "0"]
-      - ["", "", False, "LocalReAlloc", "0"]
-      - ["", "", False, "GlobalReAlloc", "0"]
-      - ["", "", False, "VirtualFree", "0"]
-      - ["", "", False, "CoTaskMemFree", "0"]
-      - ["", "", False, "CoTaskMemRealloc", "0"]
-      - ["", "", False, "SysFreeString", "0"]
-      - ["", "", False, "ExFreeToLookasideListEx", "1"]
-      - ["", "", False, "ExFreeToPagedLookasideList", "1"]
-      - ["", "", False, "ExFreeToNPagedLookasideList", "1"]
-      - ["", "", False, "NdisFreeMemoryWithTagPriority", "1"]
-      - ["", "", False, "StorPortFreeMdl", "1"]
-      - ["", "", False, "StorPortFreePool", "1"]
-      - ["", "", False, "HeapFree", "2"]
-      - ["", "", False, "HeapReAlloc", "2"]

cpp/ql/lib/ext/deallocation/empty.deallocation.model.yml

@@ -1,5 +0,0 @@
-extensions:
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: deallocationFunctionModel
-    data: []

cpp/ql/lib/ext/empty.model.yml

@@ -1,15 +0,0 @@
-extensions:
-  # Make sure that the extensible model predicates have at least one definition
-  # to avoid errors about undefined extensionals.
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: sourceModel
-    data: []
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: sinkModel
-    data: []
-  - addsTo:
-      pack: codeql/cpp-all
-      extensible: summaryModel
-    data: []

cpp/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-all
-version: 1.1.1-dev
+version: 0.12.12-dev
 groups: cpp
 dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp
@@ -14,8 +14,4 @@ dependencies:
   codeql/tutorial: ${workspace}
   codeql/util: ${workspace}
   codeql/xml: ${workspace}
-dataExtensions:
-  - ext/*.model.yml
-  - ext/deallocation/*.model.yml
-  - ext/allocation/*.model.yml
 warnOnImplicitThis: true

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

@@ -463,25 +463,6 @@ class StmtNode extends AstNode {
   }
 }
 
-/**
- * A node representing a child of a `Stmt` that is itself a `Stmt`.
- */
-class ChildStmtNode extends StmtNode {
-  Stmt childStmt;
-
-  ChildStmtNode() { exists(Stmt parent | parent.getAChild() = childStmt and childStmt = ast) }
-
-  override BaseAstNode getChildInternal(int childIndex) {
-    result = super.getChildInternal(childIndex)
-    or
-    exists(int destructorIndex |
-      result.getAst() = childStmt.getImplicitDestructorCall(destructorIndex) and
-      childIndex =
-        destructorIndex + max(int index | exists(childStmt.getChild(index)) or index = 0) + 1
-    )
-  }
-}
-
 /**
  * A node representing a `DeclStmt`.
  */
@@ -693,13 +674,6 @@ class FunctionNode extends FunctionOrGlobalOrNamespaceVariableNode {
 private string getChildAccessorWithoutConversions(Locatable parent, Element child) {
   shouldPrintDeclaration(getAnEnclosingDeclaration(parent)) and
   (
-    exists(Stmt s, int i | s.getChild(i) = parent |
-      exists(int n |
-        s.getChild(i).(Stmt).getImplicitDestructorCall(n) = child and
-        result = "getImplicitDestructorCall(" + n + ")"
-      )
-    )
-    or
     exists(Stmt s | s = parent |
       namedStmtChildPredicates(s, child, result)
       or

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

@@ -410,10 +410,6 @@ class LocalVariable extends LocalScopeVariable, @localvariable {
     or
     orphaned_variables(underlyingElement(this), unresolveElement(result))
   }
-
-  override predicate isStatic() {
-    super.isStatic() or orphaned_variables(underlyingElement(this), _)
-  }
 }
 
 /**

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

@@ -375,33 +375,6 @@ cached
 class IRGuardCondition extends Instruction {
   Instruction branch;
 
-  /*
-   * An `IRGuardCondition` supports reasoning about four different kinds of
-   * relations:
-   * 1. A unary equality relation of the form `e == k`
-   * 2. A binary equality relation of the form `e1 == e2 + k`
-   * 3. A unary inequality relation of the form `e < k`
-   * 4. A binary inequality relation of the form `e1 < e2 + k`
-   *
-   * where `k` is a constant.
-   *
-   * Furthermore, the unary relations (i.e., case 1 and case 3) are also
-   * inferred from `switch` statement guards: equality relations are inferred
-   * from the unique `case` statement, if any, and inequality relations are
-   * inferred from the [case range](https://gcc.gnu.org/onlinedocs/gcc/Case-Ranges.html)
-   * gcc extension.
-   *
-   * The implementation of all four follows the same structure: Each relation
-   * has a cached user-facing predicate that. For example,
-   * `GuardCondition::comparesEq` calls `compares_eq`. This predicate has
-   * several cases that recursively decompose the relation to bring it to a
-   * canonical form (i.e., a relation of the form `e1 == e2 + k`). The base
-   * case for this relation (i.e., `simple_comparison_eq`) handles
-   * `CompareEQInstruction`s and `CompareNEInstruction`, and recursive
-   * predicates (e.g., `complex_eq`) rewrites larger expressions such as
-   * `e1 + k1 == e2 + k2` into canonical the form `e1 == e2 + (k2 - k1)`.
-   */
-
   cached
   IRGuardCondition() { branch = getBranchForCondition(this) }
 
@@ -592,7 +565,7 @@ class IRGuardCondition extends Instruction {
   /** Holds if (determined by this guard) `op == k` evaluates to `areEqual` if this expression evaluates to `value`. */
   cached
   predicate comparesEq(Operand op, int k, boolean areEqual, AbstractValue value) {
-    unary_compares_eq(this, op, k, areEqual, false, value)
+    compares_eq(this, op, k, areEqual, value)
   }
 
   /**
@@ -613,7 +586,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresEq(Operand op, int k, IRBlock block, boolean areEqual) {
     exists(AbstractValue value |
-      unary_compares_eq(this, op, k, areEqual, false, value) and this.valueControls(block, value)
+      compares_eq(this, op, k, areEqual, value) and this.valueControls(block, value)
     )
   }
 
@@ -638,7 +611,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresEqEdge(Operand op, int k, IRBlock pred, IRBlock succ, boolean areEqual) {
     exists(AbstractValue value |
-      unary_compares_eq(this, op, k, areEqual, false, value) and
+      compares_eq(this, op, k, areEqual, value) and
       this.valueControlsEdge(pred, succ, value)
     )
   }
@@ -762,80 +735,33 @@ private predicate compares_eq(
   exists(AbstractValue dual | value = dual.getDualValue() |
     compares_eq(test.(LogicalNotInstruction).getUnary(), left, right, k, areEqual, dual)
   )
-  or
-  compares_eq(test.(BuiltinExpectCallInstruction).getCondition(), left, right, k, areEqual, value)
 }
 
-/**
- * Holds if `op == k` is `areEqual` given that `test` is equal to `value`.
- *
- * Many internal predicates in this file have a `inNonZeroCase` column.
- * Ideally, the `k` column would be a type such as `Option<int>::Option`, to
- * represent whether we have a concrete value `k` such that `op == k`, or whether
- * we only know that `op != 0`.
- * However, cannot instantiate `Option` with an infinite type. Thus the boolean
- * `inNonZeroCase` is used to distinquish the `Some` (where we have a concrete
- * value `k`) and `None` cases (where we only know that `op != 0`).
- *
- * Thus, if `inNonZeroCase = true` then `op != 0` and the value of `k` is
- * meaningless.
- *
- * To see why `inNonZeroCase` is needed consider the following C program:
- * ```c
- * char* p = ...;
- * if(p) {
- *   use(p);
- * }
- * ```
- * in C++ there would be an int-to-bool conversion on `p`. However, since C
- * does not have booleans there is no conversion. We want to be able to
- * conclude that `p` is non-zero in the true branch, so we need to give `k`
- * some value. However, simply setting `k = 1` would make the rest of the
- * analysis think that `k == 1` holds inside the branch. So we distinquish
- * between the above case and
- * ```c
- * if(p == 1) {
- *   use(p)
- * }
- * ```
- * by setting `inNonZeroCase` to `true` in the former case, but not in the
- * latter.
- */
-private predicate unary_compares_eq(
-  Instruction test, Operand op, int k, boolean areEqual, boolean inNonZeroCase, AbstractValue value
+/** Holds if `op == k` is `areEqual` given that `test` is equal to `value`. */
+private predicate compares_eq(
+  Instruction test, Operand op, int k, boolean areEqual, AbstractValue value
 ) {
   /* The simple case where the test *is* the comparison so areEqual = testIsTrue xor eq. */
-  exists(AbstractValue v |
-    unary_simple_comparison_eq(test, k, inNonZeroCase, v) and op.getDef() = test
-  |
+  exists(AbstractValue v | simple_comparison_eq(test, op, k, v) |
     areEqual = true and value = v
     or
     areEqual = false and value = v.getDualValue()
   )
   or
-  unary_complex_eq(test, op, k, areEqual, inNonZeroCase, value)
+  complex_eq(test, op, k, areEqual, value)
   or
   /* (x is true => (op == k)) => (!x is false => (op == k)) */
-  exists(AbstractValue dual, boolean inNonZeroCase0 |
-    value = dual.getDualValue() and
-    unary_compares_eq(test.(LogicalNotInstruction).getUnary(), op, k, inNonZeroCase0, areEqual, dual)
-  |
-    k = 0 and inNonZeroCase = inNonZeroCase0
-    or
-    k != 0 and inNonZeroCase = true
+  exists(AbstractValue dual | value = dual.getDualValue() |
+    compares_eq(test.(LogicalNotInstruction).getUnary(), op, k, areEqual, dual)
   )
   or
   // ((test is `areEqual` => op == const + k2) and const == `k1`) =>
   // test is `areEqual` => op == k1 + k2
-  inNonZeroCase = false and
   exists(int k1, int k2, ConstantInstruction const |
     compares_eq(test, op, const.getAUse(), k2, areEqual, value) and
     int_value(const) = k1 and
     k = k1 + k2
   )
-  or
-  unary_compares_eq(test.(BuiltinExpectCallInstruction).getCondition(), op, k, areEqual,
-    inNonZeroCase, value)
 }
 
 /** Rearrange various simple comparisons into `left == right + k` form. */
@@ -855,96 +781,14 @@ private predicate simple_comparison_eq(
   value.(BooleanValue).getValue() = false
 }
 
-/**
- * Rearrange various simple comparisons into `op == k` form.
- */
-private predicate unary_simple_comparison_eq(
-  Instruction test, int k, boolean inNonZeroCase, AbstractValue value
-) {
+/** Rearrange various simple comparisons into `op == k` form. */
+private predicate simple_comparison_eq(Instruction test, Operand op, int k, AbstractValue value) {
   exists(SwitchInstruction switch, CaseEdge case |
     test = switch.getExpression() and
+    op.getDef() = test and
     case = value.(MatchValue).getCase() and
     exists(switch.getSuccessor(case)) and
-    case.getValue().toInt() = k and
-    inNonZeroCase = false
-  )
-  or
-  // Any instruction with an integral type could potentially be part of a
-  // check for nullness when used in a guard. So we include all integral
-  // typed instructions here. However, since some of these instructions are
-  // already included as guards in other cases, we exclude those here.
-  // These are instructions that compute a binary equality or inequality
-  // relation. For example, the following:
-  // ```cpp
-  // if(a == b + 42) { ... }
-  // ```
-  // generates the following IR:
-  // ```
-  // r1(glval<int>) = VariableAddress[a]     :
-  // r2(int)        = Load[a]                : &:r1, m1
-  // r3(glval<int>) = VariableAddress[b]     :
-  // r4(int)        = Load[b]                : &:r3, m2
-  // r5(int)        = Constant[42]           :
-  // r6(int)        = Add                    : r4, r5
-  // r7(bool)       = CompareEQ              : r2, r6
-  // v1(void)       = ConditionalBranch      : r7
-  // ```
-  // and since `r7` is an integral typed instruction this predicate could
-  // include a case for when `r7` evaluates to true (in which case we would
-  // infer that `r6` was non-zero, and a case for when `r7` evaluates to false
-  // (in which case we would infer that `r6` was zero).
-  // However, since `a == b + 42` is already supported when reasoning about
-  // binary equalities we exclude those cases here.
-  not test.isGLValue() and
-  not simple_comparison_eq(test, _, _, _, _) and
-  not simple_comparison_lt(test, _, _, _) and
-  not test = any(SwitchInstruction switch).getExpression() and
-  (
-    test.getResultIRType() instanceof IRAddressType or
-    test.getResultIRType() instanceof IRIntegerType or
-    test.getResultIRType() instanceof IRBooleanType
-  ) and
-  (
-    k = 1 and
-    value.(BooleanValue).getValue() = true and
-    inNonZeroCase = true
-    or
-    k = 0 and
-    value.(BooleanValue).getValue() = false and
-    inNonZeroCase = false
-  )
-}
-
-/** A call to the builtin operation `__builtin_expect`. */
-private class BuiltinExpectCallInstruction extends CallInstruction {
-  BuiltinExpectCallInstruction() { this.getStaticCallTarget().hasName("__builtin_expect") }
-
-  /** Gets the condition of this call. */
-  Instruction getCondition() {
-    // The first parameter of `__builtin_expect` has type `long`. So we skip
-    // the conversion when inferring guards.
-    result = this.getArgument(0).(ConvertInstruction).getUnary()
-  }
-}
-
-/**
- * Holds if `left == right + k` is `areEqual` if `cmp` evaluates to `value`,
- * and `cmp` is an instruction that compares the value of
- * `__builtin_expect(left == right + k, _)` to `0`.
- */
-private predicate builtin_expect_eq(
-  CompareInstruction cmp, Operand left, Operand right, int k, boolean areEqual, AbstractValue value
-) {
-  exists(BuiltinExpectCallInstruction call, Instruction const, AbstractValue innerValue |
-    int_value(const) = 0 and
-    cmp.hasOperands(call.getAUse(), const.getAUse()) and
-    compares_eq(call.getCondition(), left, right, k, areEqual, innerValue)
-  |
-    cmp instanceof CompareNEInstruction and
-    value = innerValue
-    or
-    cmp instanceof CompareEQInstruction and
-    value.getDualValue() = innerValue
+    case.getValue().toInt() = k
   )
 }
 
@@ -954,39 +798,14 @@ private predicate complex_eq(
   sub_eq(cmp, left, right, k, areEqual, value)
   or
   add_eq(cmp, left, right, k, areEqual, value)
-  or
-  builtin_expect_eq(cmp, left, right, k, areEqual, value)
 }
 
-/**
- * Holds if `op == k` is `areEqual` if `cmp` evaluates to `value`, and `cmp` is
- * an instruction that compares the value of `__builtin_expect(op == k, _)` to `0`.
- */
-private predicate unary_builtin_expect_eq(
-  CompareInstruction cmp, Operand op, int k, boolean areEqual, boolean inNonZeroCase,
-  AbstractValue value
+private predicate complex_eq(
+  Instruction test, Operand op, int k, boolean areEqual, AbstractValue value
 ) {
-  exists(BuiltinExpectCallInstruction call, Instruction const, AbstractValue innerValue |
-    int_value(const) = 0 and
-    cmp.hasOperands(call.getAUse(), const.getAUse()) and
-    unary_compares_eq(call.getCondition(), op, k, areEqual, inNonZeroCase, innerValue)
-  |
-    cmp instanceof CompareNEInstruction and
-    value = innerValue
-    or
-    cmp instanceof CompareEQInstruction and
-    value.getDualValue() = innerValue
-  )
-}
-
-private predicate unary_complex_eq(
-  Instruction test, Operand op, int k, boolean areEqual, boolean inNonZeroCase, AbstractValue value
-) {
-  unary_sub_eq(test, op, k, areEqual, inNonZeroCase, value)
+  sub_eq(test, op, k, areEqual, value)
   or
-  unary_add_eq(test, op, k, areEqual, inNonZeroCase, value)
-  or
-  unary_builtin_expect_eq(test, op, k, areEqual, inNonZeroCase, value)
+  add_eq(test, op, k, areEqual, value)
 }
 
 /*
@@ -1015,8 +834,7 @@ private predicate compares_lt(
 
 /** Holds if `op < k` evaluates to `isLt` given that `test` evaluates to `value`. */
 private predicate compares_lt(Instruction test, Operand op, int k, boolean isLt, AbstractValue value) {
-  unary_simple_comparison_lt(test, k, isLt, value) and
-  op.getDef() = test
+  simple_comparison_lt(test, op, k, isLt, value)
   or
   complex_lt(test, op, k, isLt, value)
   or
@@ -1063,11 +881,12 @@ private predicate simple_comparison_lt(CompareInstruction cmp, Operand left, Ope
 }
 
 /** Rearrange various simple comparisons into `op < k` form. */
-private predicate unary_simple_comparison_lt(
-  Instruction test, int k, boolean isLt, AbstractValue value
+private predicate simple_comparison_lt(
+  Instruction test, Operand op, int k, boolean isLt, AbstractValue value
 ) {
   exists(SwitchInstruction switch, CaseEdge case |
     test = switch.getExpression() and
+    op.getDef() = test and
     case = value.(MatchValue).getCase() and
     exists(switch.getSuccessor(case)) and
     case.getMaxValue() > case.getMinValue()
@@ -1250,20 +1069,16 @@ private predicate sub_eq(
 }
 
 // op - x == c => op == (c+x)
-private predicate unary_sub_eq(
-  Instruction test, Operand op, int k, boolean areEqual, boolean inNonZeroCase, AbstractValue value
-) {
-  inNonZeroCase = false and
+private predicate sub_eq(Instruction test, Operand op, int k, boolean areEqual, AbstractValue value) {
   exists(SubInstruction sub, int c, int x |
-    unary_compares_eq(test, sub.getAUse(), c, areEqual, inNonZeroCase, value) and
+    compares_eq(test, sub.getAUse(), c, areEqual, value) and
     op = sub.getLeftOperand() and
     x = int_value(sub.getRight()) and
     k = c + x
   )
   or
-  inNonZeroCase = false and
   exists(PointerSubInstruction sub, int c, int x |
-    unary_compares_eq(test, sub.getAUse(), c, areEqual, inNonZeroCase, value) and
+    compares_eq(test, sub.getAUse(), c, areEqual, value) and
     op = sub.getLeftOperand() and
     x = int_value(sub.getRight()) and
     k = c + x
@@ -1317,13 +1132,11 @@ private predicate add_eq(
 }
 
 // left + x == right + c => left == right + (c-x)
-private predicate unary_add_eq(
-  Instruction test, Operand left, int k, boolean areEqual, boolean inNonZeroCase,
-  AbstractValue value
+private predicate add_eq(
+  Instruction test, Operand left, int k, boolean areEqual, AbstractValue value
 ) {
-  inNonZeroCase = false and
   exists(AddInstruction lhs, int c, int x |
-    unary_compares_eq(test, lhs.getAUse(), c, areEqual, inNonZeroCase, value) and
+    compares_eq(test, lhs.getAUse(), c, areEqual, value) and
     (
       left = lhs.getLeftOperand() and x = int_value(lhs.getRight())
       or
@@ -1332,9 +1145,8 @@ private predicate unary_add_eq(
     k = c - x
   )
   or
-  inNonZeroCase = false and
   exists(PointerAddInstruction lhs, int c, int x |
-    unary_compares_eq(test, lhs.getAUse(), c, areEqual, inNonZeroCase, value) and
+    compares_eq(test, lhs.getAUse(), c, areEqual, value) and
     (
       left = lhs.getLeftOperand() and x = int_value(lhs.getRight())
       or
@@ -1344,14 +1156,5 @@ private predicate unary_add_eq(
   )
 }
 
-private class IntegerOrPointerConstantInstruction extends ConstantInstruction {
-  IntegerOrPointerConstantInstruction() {
-    this instanceof IntegerConstantInstruction or
-    this instanceof PointerConstantInstruction
-  }
-}
-
 /** The int value of integer constant expression. */
-private int int_value(Instruction i) {
-  result = i.(IntegerOrPointerConstantInstruction).getValue().toInt()
-}
+private int int_value(Instruction i) { result = i.(IntegerConstantInstruction).getValue().toInt() }

cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll

@@ -78,7 +78,6 @@ private import internal.FlowSummaryImpl
 private import internal.FlowSummaryImpl::Public
 private import internal.FlowSummaryImpl::Private
 private import internal.FlowSummaryImpl::Private::External
-private import internal.ExternalFlowExtensions as Extensions
 private import codeql.mad.ModelValidation as SharedModelVal
 private import codeql.util.Unit
 
@@ -139,9 +138,6 @@ predicate sourceModel(
     row.splitAt(";", 7) = kind
   ) and
   provenance = "manual"
-  or
-  Extensions::sourceModel(namespace, type, subtypes, name, signature, ext, output, kind, provenance,
-    _)
 }
 
 /** Holds if a sink model exists for the given parameters. */
@@ -162,8 +158,6 @@ predicate sinkModel(
     row.splitAt(";", 7) = kind
   ) and
   provenance = "manual"
-  or
-  Extensions::sinkModel(namespace, type, subtypes, name, signature, ext, input, kind, provenance, _)
 }
 
 /** Holds if a summary model exists for the given parameters. */
@@ -185,9 +179,6 @@ predicate summaryModel(
     row.splitAt(";", 8) = kind
   ) and
   provenance = "manual"
-  or
-  Extensions::summaryModel(namespace, type, subtypes, name, signature, ext, input, output, kind,
-    provenance, _)
 }
 
 private predicate relevantNamespace(string namespace) {
@@ -212,10 +203,8 @@ private predicate canonicalNamespaceLink(string namespace, string subns) {
 }
 
 /**
- * Holds if MaD framework coverage of `namespace` is `n` api endpoints of the
- * kind `(kind, part)`, and `namespaces` is the number of subnamespaces of
- * `namespace` which have MaD framework coverage (including `namespace`
- * itself).
+ * Holds if CSV framework coverage of `namespace` is `n` api endpoints of the
+ * kind `(kind, part)`.
  */
 predicate modelCoverage(string namespace, int namespaces, string kind, string part, int n) {
   namespaces = strictcount(string subns | canonicalNamespaceLink(namespace, subns)) and
@@ -332,10 +321,10 @@ module CsvValidation {
       or
       summaryModel(namespace, type, _, name, signature, ext, _, _, _, _) and pred = "summary"
     |
-      not namespace.regexpMatch("[a-zA-Z0-9_\\.:]*") and
+      not namespace.regexpMatch("[a-zA-Z0-9_\\.]+") and
       result = "Dubious namespace \"" + namespace + "\" in " + pred + " model."
       or
-      not type.regexpMatch("[a-zA-Z0-9_<>,\\+]*") and
+      not type.regexpMatch("[a-zA-Z0-9_<>,\\+]+") and
       result = "Dubious type \"" + type + "\" in " + pred + " model."
       or
       not name.regexpMatch("[a-zA-Z0-9_<>,]*") and

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

@@ -9,7 +9,7 @@ private import DataFlowUtil
 /**
  * Gets a function that might be called by `call`.
  */
-DataFlowCallable viableCallable(DataFlowCall call) {
+Function viableCallable(DataFlowCall call) {
   result = call.(Call).getTarget()
   or
   // If the target of the call does not have a body in the snapshot, it might

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

@@ -216,7 +216,7 @@ predicate localMustFlowStep(Node node1, Node node2) { none() }
 
 /** Gets the type of `n` used for type pruning. */
 Type getNodeType(Node n) {
-  exists(n) and
+  suppressUnusedNode(n) and
   result instanceof VoidType // stub implementation
 }
 
@@ -227,10 +227,13 @@ string ppReprType(Type t) { none() } // stub implementation
  * Holds if `t1` and `t2` are compatible, that is, whether data can flow from
  * a node of type `t1` to a node of type `t2`.
  */
+pragma[inline]
 predicate compatibleTypes(Type t1, Type t2) {
-  t1 instanceof VoidType and t2 instanceof VoidType // stub implementation
+  any() // stub implementation
 }
 
+private predicate suppressUnusedNode(Node n) { any() }
+
 //////////////////////////////////////////////////////////////////////////////
 // Java QL library compatibility wrappers
 //////////////////////////////////////////////////////////////////////////////
@@ -239,17 +242,7 @@ class CastNode extends Node {
   CastNode() { none() } // stub implementation
 }
 
-class DataFlowCallable extends Function {
-  /** Gets a best-effort total ordering. */
-  int totalorder() {
-    this =
-      rank[result](DataFlowCallable c, string file, int startline, int startcolumn |
-        c.getLocation().hasLocationInfo(file, startline, startcolumn, _, _)
-      |
-        c order by file, startline, startcolumn
-      )
-  }
-}
+class DataFlowCallable = Function;
 
 class DataFlowExpr = Expr;
 
@@ -268,28 +261,10 @@ class DataFlowCall extends Expr instanceof Call {
   ExprNode getNode() { result.getExpr() = this }
 
   /** Gets the enclosing callable of this call. */
-  DataFlowCallable getEnclosingCallable() { result = this.getEnclosingFunction() }
-
-  /** Gets a best-effort total ordering. */
-  int totalorder() {
-    this =
-      rank[result](DataFlowCall c, int startline, int startcolumn |
-        c.getLocation().hasLocationInfo(_, startline, startcolumn, _, _)
-      |
-        c order by startline, startcolumn
-      )
-  }
+  Function getEnclosingCallable() { result = this.getEnclosingFunction() }
 }
 
-class NodeRegion instanceof Unit {
-  string toString() { result = "NodeRegion" }
-
-  predicate contains(Node n) { none() }
-
-  int totalOrder() { result = 1 }
-}
-
-predicate isUnreachableInCall(NodeRegion nr, DataFlowCall call) { none() } // stub implementation
+predicate isUnreachableInCall(Node n, DataFlowCall call) { none() } // stub implementation
 
 /**
  * Holds if access paths with `c` at their head always should be tracked at high

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

@@ -1,27 +0,0 @@
-/**
- * This module provides extensible predicates for defining MaD models.
- */
-
-/**
- * Holds if an external source model exists for the given parameters.
- */
-extensible predicate sourceModel(
-  string namespace, string type, boolean subtypes, string name, string signature, string ext,
-  string output, string kind, string provenance, QlBuiltins::ExtensionId madId
-);
-
-/**
- * Holds if an external sink model exists for the given parameters.
- */
-extensible predicate sinkModel(
-  string namespace, string type, boolean subtypes, string name, string signature, string ext,
-  string input, string kind, string provenance, QlBuiltins::ExtensionId madId
-);
-
-/**
- * Holds if an external summary model exists for the given parameters.
- */
-extensible predicate summaryModel(
-  string namespace, string type, boolean subtypes, string name, string signature, string ext,
-  string input, string output, string kind, string provenance, QlBuiltins::ExtensionId madId
-);

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

@@ -988,7 +988,7 @@ predicate localMustFlowStep(Node node1, Node node2) { none() }
 
 /** Gets the type of `n` used for type pruning. */
 DataFlowType getNodeType(Node n) {
-  exists(n) and
+  suppressUnusedNode(n) and
   result instanceof VoidType // stub implementation
 }
 
@@ -999,10 +999,13 @@ string ppReprType(DataFlowType t) { none() } // stub implementation
  * Holds if `t1` and `t2` are compatible, that is, whether data can flow from
  * a node of type `t1` to a node of type `t2`.
  */
+pragma[inline]
 predicate compatibleTypes(DataFlowType t1, DataFlowType t2) {
-  t1 instanceof VoidType and t2 instanceof VoidType // stub implementation
+  any() // stub implementation
 }
 
+private predicate suppressUnusedNode(Node n) { any() }
+
 //////////////////////////////////////////////////////////////////////////////
 // Java QL library compatibility wrappers
 //////////////////////////////////////////////////////////////////////////////
@@ -1059,16 +1062,6 @@ class DataFlowCallable extends TDataFlowCallable {
     result = this.asSummarizedCallable() or // SummarizedCallable = Function (in CPP)
     result = this.asSourceCallable()
   }
-
-  /** Gets a best-effort total ordering. */
-  int totalorder() {
-    this =
-      rank[result](DataFlowCallable c, string file, int startline, int startcolumn |
-        c.getLocation().hasLocationInfo(file, startline, startcolumn, _, _)
-      |
-        c order by file, startline, startcolumn
-      )
-  }
 }
 
 /**
@@ -1166,16 +1159,6 @@ class DataFlowCall extends TDataFlowCall {
    * Gets the location of this call.
    */
   Location getLocation() { none() }
-
-  /** Gets a best-effort total ordering. */
-  int totalorder() {
-    this =
-      rank[result](DataFlowCall c, int startline, int startcolumn |
-        c.getLocation().hasLocationInfo(_, startline, startcolumn, _, _)
-      |
-        c order by startline, startcolumn
-      )
-  }
 }
 
 /**
@@ -1264,53 +1247,43 @@ module IsUnreachableInCall {
     any(G::IRGuardCondition guard).ensuresLt(left, right, k, block, areEqual)
   }
 
-  class NodeRegion instanceof IRBlock {
-    string toString() { result = "NodeRegion" }
-
-    predicate contains(Node n) { this = n.getBasicBlock() }
-
-    int totalOrder() {
-      this =
-        rank[result](IRBlock b, int startline, int startcolumn |
-          b.getLocation().hasLocationInfo(_, startline, startcolumn, _, _)
-        |
-          b order by startline, startcolumn
-        )
-    }
-  }
-
-  predicate isUnreachableInCall(NodeRegion block, DataFlowCall call) {
+  predicate isUnreachableInCall(Node n, DataFlowCall call) {
     exists(
       InstructionDirectParameterNode paramNode, ConstantIntegralTypeArgumentNode arg,
-      IntegerConstantInstruction constant, int k, Operand left, Operand right, int argval
+      IntegerConstantInstruction constant, int k, Operand left, Operand right, IRBlock block
     |
       // arg flows into `paramNode`
-      DataFlowImplCommon::viableParamArg(call, pragma[only_bind_into](paramNode),
-        pragma[only_bind_into](arg)) and
+      DataFlowImplCommon::viableParamArg(call, paramNode, arg) and
       left = constant.getAUse() and
       right = valueNumber(paramNode.getInstruction()).getAUse() and
-      argval = arg.getValue()
+      block = n.getBasicBlock()
     |
       // and there's a guard condition which ensures that the result of `left == right + k` is `areEqual`
-      exists(boolean areEqual | ensuresEq(left, right, k, block, areEqual) |
+      exists(boolean areEqual |
+        ensuresEq(pragma[only_bind_into](left), pragma[only_bind_into](right),
+          pragma[only_bind_into](k), pragma[only_bind_into](block), areEqual)
+      |
         // this block ensures that left = right + k, but it holds that `left != right + k`
         areEqual = true and
-        constant.getValue().toInt() != argval + k
+        constant.getValue().toInt() != arg.getValue() + k
         or
         // this block ensures that or `left != right + k`, but it holds that `left = right + k`
         areEqual = false and
-        constant.getValue().toInt() = argval + k
+        constant.getValue().toInt() = arg.getValue() + k
       )
       or
       // or there's a guard condition which ensures that the result of `left < right + k` is `isLessThan`
-      exists(boolean isLessThan | ensuresLt(left, right, k, block, isLessThan) |
+      exists(boolean isLessThan |
+        ensuresLt(pragma[only_bind_into](left), pragma[only_bind_into](right),
+          pragma[only_bind_into](k), pragma[only_bind_into](block), isLessThan)
+      |
         isLessThan = true and
         // this block ensures that `left < right + k`, but it holds that `left >= right + k`
-        constant.getValue().toInt() >= argval + k
+        constant.getValue().toInt() >= arg.getValue() + k
         or
         // this block ensures that `left >= right + k`, but it holds that `left < right + k`
         isLessThan = false and
-        constant.getValue().toInt() < argval + k
+        constant.getValue().toInt() < arg.getValue() + k
       )
     )
   }
@@ -1333,8 +1306,6 @@ predicate nodeIsHidden(Node n) {
   n instanceof FinalGlobalValue
   or
   n instanceof InitialGlobalValue
-  or
-  n instanceof SsaPhiInputNode
 }
 
 predicate neverSkipInPathGraph(Node n) {
@@ -1633,8 +1604,6 @@ private Instruction getAnInstruction(Node n) {
   or
   result = n.(SsaPhiNode).getPhiNode().getBasicBlock().getFirstInstruction()
   or
-  result = n.(SsaPhiInputNode).getBasicBlock().getFirstInstruction()
-  or
   n.(IndirectInstruction).hasInstructionAndIndirectionIndex(result, _)
   or
   not n instanceof IndirectInstruction and
@@ -1696,311 +1665,3 @@ class DataFlowSecondLevelScope extends TDataFlowSecondLevelScope {
 
 /** Gets the second-level scope containing the node `n`, if any. */
 DataFlowSecondLevelScope getSecondLevelScope(Node n) { result.getANode() = n }
-
-/**
- * Module that defines flow through iterators.
- * For example,
- * ```cpp
- * auto it = v.begin();
- * *it = source();
- * ...
- * sink(v[0]);
- * ```
- */
-module IteratorFlow {
-  private import codeql.ssa.Ssa as SsaImpl
-  private import semmle.code.cpp.models.interfaces.Iterator as Interface
-  private import semmle.code.cpp.models.implementations.Iterator as Impl
-
-  /**
-   * A variable of some type that can produce an iterator.
-   */
-  class SourceVariable extends Ssa::SourceVariable {
-    SourceVariable() {
-      exists(Interface::GetIteratorFunction gets, Cpp::FunctionInput input, int i |
-        input.isParameterDerefOrQualifierObject(i) and
-        gets.getsIterator(input, _)
-      |
-        this.getType().stripType() = gets.getParameter(i).getType().stripType()
-        or
-        i = -1 and
-        this.getType().stripType() = gets.getDeclaringType()
-      )
-    }
-  }
-
-  private module SsaInput implements SsaImpl::InputSig<Location> {
-    import Ssa::InputSigCommon
-
-    class SourceVariable = IteratorFlow::SourceVariable;
-
-    /** A call to function that dereferences an iterator. */
-    private class IteratorPointerDereferenceCall extends CallInstruction {
-      IteratorPointerDereferenceCall() {
-        this.getStaticCallTarget() instanceof Impl::IteratorPointerDereferenceOperator
-      }
-    }
-
-    /** A call to a function that obtains an iterator. */
-    private class GetsIteratorCall extends CallInstruction {
-      GetsIteratorCall() { this.getStaticCallTarget() instanceof Impl::GetIteratorFunction }
-    }
-
-    /** A call to `operator++` or `operator--` on an iterator. */
-    private class IteratorCrementCall extends CallInstruction {
-      IteratorCrementCall() { this.getStaticCallTarget() instanceof Impl::IteratorCrementOperator }
-    }
-
-    /**
-     * Gets an ultimate definition of `def`.
-     *
-     * Note: Unlike `def.getAnUltimateDefinition()` this predicate also
-     * traverses back through iterator increment and decrement operations.
-     */
-    private Ssa::Def getAnUltimateDefinition(Ssa::Def def) {
-      result = def.getAnUltimateDefinition()
-      or
-      exists(IRBlock bb, int i, IteratorCrementCall crementCall, Ssa::SourceVariable sv |
-        crementCall = def.getValue().asInstruction().(StoreInstruction).getSourceValue() and
-        sv = def.getSourceVariable() and
-        bb.getInstruction(i) = crementCall and
-        Ssa::ssaDefReachesReadExt(sv, result.asDef(), bb, i)
-      )
-    }
-
-    /**
-     * Holds if `write` is an instruction that writes to address `address`
-     */
-    private predicate isIteratorWrite(Instruction write, Operand address) {
-      exists(Ssa::DefImpl writeDef, IRBlock bb, int i |
-        writeDef.hasIndexInBlock(bb, i, _) and
-        bb.getInstruction(i) = write and
-        address = writeDef.getAddressOperand()
-      )
-    }
-
-    /**
-     * Holds if `writeToDeref` is a write to an iterator that was obtained
-     * by `beginCall`. That is, the following instruction sequence holds:
-     * ```cpp
-     * it = container.begin(); // or a similar iterator-obtaining function call
-     * ...
-     * *it = value;
-     * ```
-     */
-    private predicate isIteratorStoreInstruction(
-      GetsIteratorCall beginCall, Instruction writeToDeref
-    ) {
-      exists(
-        StoreInstruction beginStore, IRBlock bbStar, int iStar, Ssa::Def def,
-        IteratorPointerDereferenceCall starCall, Ssa::Def ultimate, Operand address
-      |
-        isIteratorWrite(writeToDeref, address) and
-        operandForFullyConvertedCall(address, starCall) and
-        bbStar.getInstruction(iStar) = starCall and
-        Ssa::ssaDefReachesReadExt(_, def.asDef(), bbStar, iStar) and
-        ultimate = getAnUltimateDefinition*(def) and
-        beginStore = ultimate.getValue().asInstruction() and
-        operandForFullyConvertedCall(beginStore.getSourceValueOperand(), beginCall)
-      )
-    }
-
-    /**
-     * Holds if `(bb, i)` contains a write to an iterator that may have been obtained
-     * by calling `begin` (or related functions) on the variable `v`.
-     */
-    predicate variableWrite(IRBlock bb, int i, SourceVariable v, boolean certain) {
-      certain = false and
-      exists(GetsIteratorCall beginCall, Instruction writeToDeref, IRBlock bbQual, int iQual |
-        isIteratorStoreInstruction(beginCall, writeToDeref) and
-        bb.getInstruction(i) = writeToDeref and
-        bbQual.getInstruction(iQual) = beginCall and
-        Ssa::variableRead(bbQual, iQual, v, _)
-      )
-    }
-
-    /** Holds if `(bb, i)` reads the container variable `v`. */
-    predicate variableRead(IRBlock bb, int i, SourceVariable v, boolean certain) {
-      Ssa::variableRead(bb, i, v, certain)
-    }
-  }
-
-  private module IteratorSsa = SsaImpl::Make<Location, SsaInput>;
-
-  cached
-  private newtype TSsaDef =
-    TDef(IteratorSsa::DefinitionExt def) or
-    TPhi(PhiNode phi)
-
-  abstract private class SsaDef extends TSsaDef {
-    /** Gets a textual representation of this element. */
-    string toString() { none() }
-
-    /** Gets the underlying non-phi definition or use. */
-    IteratorSsa::DefinitionExt asDef() { none() }
-
-    /** Gets the underlying phi node. */
-    PhiNode asPhi() { none() }
-
-    /** Gets the location of this element. */
-    abstract Location getLocation();
-  }
-
-  private class Def extends TDef, SsaDef {
-    IteratorSsa::DefinitionExt def;
-
-    Def() { this = TDef(def) }
-
-    final override IteratorSsa::DefinitionExt asDef() { result = def }
-
-    final override Location getLocation() { result = this.getImpl().getLocation() }
-
-    /** Gets the variable written to by this definition. */
-    final SourceVariable getSourceVariable() { result = def.getSourceVariable() }
-
-    override string toString() { result = def.toString() }
-
-    /**
-     * Holds if this definition (or use) has index `index` in block `block`,
-     * and is a definition (or use) of the variable `sv`.
-     */
-    predicate hasIndexInBlock(IRBlock block, int index, SourceVariable sv) {
-      def.definesAt(sv, block, index, _)
-    }
-
-    private Ssa::DefImpl getImpl() {
-      exists(IRBlock bb, int i |
-        this.hasIndexInBlock(bb, i, _) and
-        result.hasIndexInBlock(bb, i)
-      )
-    }
-
-    /** Gets the value written by this definition (i.e., the "right-hand side"). */
-    Node0Impl getValue() { result = this.getImpl().getValue() }
-
-    /** Gets the indirection index of this definition. */
-    int getIndirectionIndex() { result = this.getImpl().getIndirectionIndex() }
-  }
-
-  private class Phi extends TPhi, SsaDef {
-    PhiNode phi;
-
-    Phi() { this = TPhi(phi) }
-
-    final override PhiNode asPhi() { result = phi }
-
-    final override Location getLocation() { result = phi.getBasicBlock().getLocation() }
-
-    override string toString() { result = phi.toString() }
-
-    SsaIteratorNode getNode() { result.getIteratorFlowNode() = phi }
-  }
-
-  private class PhiNode extends IteratorSsa::DefinitionExt {
-    PhiNode() {
-      this instanceof IteratorSsa::PhiNode or
-      this instanceof IteratorSsa::PhiReadNode
-    }
-
-    SsaIteratorNode getNode() { result.getIteratorFlowNode() = this }
-  }
-
-  cached
-  private module IteratorSsaCached {
-    cached
-    predicate adjacentDefRead(IRBlock bb1, int i1, SourceVariable sv, IRBlock bb2, int i2) {
-      IteratorSsa::adjacentDefReadExt(_, sv, bb1, i1, bb2, i2)
-      or
-      exists(PhiNode phi |
-        IteratorSsa::lastRefRedefExt(_, sv, bb1, i1, phi) and
-        phi.definesAt(sv, bb2, i2, _)
-      )
-    }
-
-    cached
-    Node getAPriorDefinition(IteratorSsa::DefinitionExt next) {
-      exists(IRBlock bb, int i, SourceVariable sv, IteratorSsa::DefinitionExt def |
-        IteratorSsa::lastRefRedefExt(pragma[only_bind_into](def), pragma[only_bind_into](sv),
-          pragma[only_bind_into](bb), pragma[only_bind_into](i), next) and
-        nodeToDefOrUse(result, sv, bb, i, _)
-      )
-    }
-  }
-
-  /** The set of nodes necessary for iterator flow. */
-  class IteratorFlowNode instanceof PhiNode {
-    /** Gets a textual representation of this node. */
-    string toString() { result = super.toString() }
-
-    /** Gets the type of this node. */
-    DataFlowType getType() {
-      exists(Ssa::SourceVariable sv |
-        super.definesAt(sv, _, _, _) and
-        result = sv.getType()
-      )
-    }
-
-    /** Gets the `Declaration` that contains this block. */
-    Declaration getFunction() { result = super.getBasicBlock().getEnclosingFunction() }
-
-    /** Gets the locatino of this node. */
-    Location getLocation() { result = super.getBasicBlock().getLocation() }
-  }
-
-  private import IteratorSsaCached
-
-  private predicate defToNode(Node node, Def def, boolean uncertain) {
-    (
-      nodeHasOperand(node, def.getValue().asOperand(), def.getIndirectionIndex())
-      or
-      nodeHasInstruction(node, def.getValue().asInstruction(), def.getIndirectionIndex())
-    ) and
-    uncertain = false
-  }
-
-  private predicate nodeToDefOrUse(
-    Node node, SourceVariable sv, IRBlock bb, int i, boolean uncertain
-  ) {
-    exists(Def def |
-      def.hasIndexInBlock(bb, i, sv) and
-      defToNode(node, def, uncertain)
-    )
-    or
-    useToNode(bb, i, sv, node) and
-    uncertain = false
-  }
-
-  private predicate useToNode(IRBlock bb, int i, SourceVariable sv, Node nodeTo) {
-    exists(PhiNode phi |
-      phi.definesAt(sv, bb, i, _) and
-      nodeTo = phi.getNode()
-    )
-    or
-    exists(Ssa::UseImpl use |
-      use.hasIndexInBlock(bb, i, sv) and
-      nodeTo = use.getNode()
-    )
-  }
-
-  /**
-   * Holds if `nodeFrom` flows to `nodeTo` in a single step.
-   */
-  predicate localFlowStep(Node nodeFrom, Node nodeTo) {
-    exists(
-      Node nFrom, SourceVariable sv, IRBlock bb1, int i1, IRBlock bb2, int i2, boolean uncertain
-    |
-      adjacentDefRead(bb1, i1, sv, bb2, i2) and
-      nodeToDefOrUse(nFrom, sv, bb1, i1, uncertain) and
-      useToNode(bb2, i2, sv, nodeTo)
-    |
-      if uncertain = true
-      then
-        nodeFrom =
-          [
-            nFrom,
-            getAPriorDefinition(any(IteratorSsa::DefinitionExt next | next.definesAt(sv, bb1, i1, _)))
-          ]
-      else nFrom = nodeFrom
-    )
-  }
-}

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

@@ -17,7 +17,6 @@ private import SsaInternals as Ssa
 private import DataFlowImplCommon as DataFlowImplCommon
 private import codeql.util.Unit
 private import Node0ToString
-import ExprNodes
 
 /**
  * The IR dataflow graph consists of the following nodes:
@@ -46,9 +45,7 @@ private newtype TIRDataFlowNode =
     or
     Ssa::isModifiableByCall(operand, indirectionIndex)
   } or
-  TSsaPhiInputNode(Ssa::PhiNode phi, IRBlock input) { phi.hasInputFromBlock(_, _, _, _, input) } or
   TSsaPhiNode(Ssa::PhiNode phi) or
-  TSsaIteratorNode(IteratorFlow::IteratorFlowNode n) or
   TRawIndirectOperand0(Node0Impl node, int indirectionIndex) {
     Ssa::hasRawIndirectOperand(node.asOperand(), indirectionIndex)
   } or
@@ -116,13 +113,6 @@ predicate conversionFlow(
       instrTo.(CheckedConvertOrNullInstruction).getUnaryOperand() = opFrom
       or
       instrTo.(InheritanceConversionInstruction).getUnaryOperand() = opFrom
-      or
-      exists(BuiltInInstruction builtIn |
-        builtIn = instrTo and
-        // __builtin_bit_cast
-        builtIn.getBuiltInOperation() instanceof BuiltInBitCast and
-        opFrom = builtIn.getAnOperand()
-      )
     )
     or
     additional = true and
@@ -167,12 +157,6 @@ class Node extends TIRDataFlowNode {
   /** Gets the operands corresponding to this node, if any. */
   Operand asOperand() { result = this.(OperandNode).getOperand() }
 
-  /**
-   * Gets the operand that is indirectly tracked by this node behind `index`
-   * number of indirections.
-   */
-  Operand asIndirectOperand(int index) { hasOperandAndIndex(this, result, index) }
-
   /**
    * Holds if this node is at index `i` in basic block `block`.
    *
@@ -185,9 +169,6 @@ class Node extends TIRDataFlowNode {
     or
     this.(SsaPhiNode).getPhiNode().getBasicBlock() = block and i = -1
     or
-    this.(SsaPhiInputNode).getBlock() = block and
-    i = block.getInstructionCount()
-    or
     this.(RawIndirectOperand).getOperand().getUse() = block.getInstruction(i)
     or
     this.(RawIndirectInstruction).getInstruction() = block.getInstruction(i)
@@ -640,7 +621,7 @@ class SsaPhiNode extends Node, TSsaPhiNode {
 
   final override Location getLocationImpl() { result = phi.getBasicBlock().getLocation() }
 
-  override string toStringImpl() { result = phi.toString() }
+  override string toStringImpl() { result = "Phi" }
 
   /**
    * Gets a node that is used as input to this phi node.
@@ -649,7 +630,7 @@ class SsaPhiNode extends Node, TSsaPhiNode {
    */
   cached
   final Node getAnInput(boolean fromBackEdge) {
-    result.(SsaPhiInputNode).getPhiNode() = phi and
+    localFlowStep(result, this) and
     exists(IRBlock bPhi, IRBlock bResult |
       bPhi = phi.getBasicBlock() and bResult = result.getBasicBlock()
     |
@@ -672,82 +653,6 @@ class SsaPhiNode extends Node, TSsaPhiNode {
   predicate isPhiRead() { phi.isPhiRead() }
 }
 
-/**
- * INTERNAL: Do not use.
- *
- * A node that is used as an input to a phi node.
- *
- * This class exists to allow more powerful barrier guards. Consider this
- * example:
- *
- * ```cpp
- * int x = source();
- * if(!safe(x)) {
- *   x = clear();
- * }
- * // phi node for x here
- * sink(x);
- * ```
- *
- * At the phi node for `x` it is neither the case that `x` is dominated by
- * `safe(x)`, or is the case that the phi is dominated by a clearing of `x`.
- *
- * By inserting a "phi input" node as the last entry in the basic block that
- * defines the inputs to the phi we can conclude that each of those inputs are
- * safe to pass to `sink`.
- */
-class SsaPhiInputNode extends Node, TSsaPhiInputNode {
-  Ssa::PhiNode phi;
-  IRBlock block;
-
-  SsaPhiInputNode() { this = TSsaPhiInputNode(phi, block) }
-
-  /** Gets the phi node associated with this node. */
-  Ssa::PhiNode getPhiNode() { result = phi }
-
-  /** Gets the basic block in which this input originates. */
-  IRBlock getBlock() { result = block }
-
-  override Declaration getEnclosingCallable() { result = this.getFunction() }
-
-  override Declaration getFunction() { result = phi.getBasicBlock().getEnclosingFunction() }
-
-  override DataFlowType getType() { result = this.getSourceVariable().getType() }
-
-  override predicate isGLValue() { phi.getSourceVariable().isGLValue() }
-
-  final override Location getLocationImpl() { result = block.getLastInstruction().getLocation() }
-
-  override string toStringImpl() { result = "Phi input" }
-
-  /** Gets the source variable underlying this phi node. */
-  Ssa::SourceVariable getSourceVariable() { result = phi.getSourceVariable() }
-}
-
-/**
- * INTERNAL: do not use.
- *
- * Dataflow nodes necessary for iterator flow
- */
-class SsaIteratorNode extends Node, TSsaIteratorNode {
-  IteratorFlow::IteratorFlowNode node;
-
-  SsaIteratorNode() { this = TSsaIteratorNode(node) }
-
-  /** Gets the phi node associated with this node. */
-  IteratorFlow::IteratorFlowNode getIteratorFlowNode() { result = node }
-
-  override Declaration getEnclosingCallable() { result = this.getFunction() }
-
-  override Declaration getFunction() { result = node.getFunction() }
-
-  override DataFlowType getType() { result = node.getType() }
-
-  final override Location getLocationImpl() { result = node.getLocation() }
-
-  override string toStringImpl() { result = node.toString() }
-}
-
 /**
  * INTERNAL: do not use.
  *
@@ -1285,11 +1190,11 @@ class UninitializedNode extends Node {
   LocalVariable v;
 
   UninitializedNode() {
-    exists(Ssa::Def def, Ssa::SourceVariable sv |
+    exists(Ssa::Def def |
       def.getIndirectionIndex() = 0 and
       def.getValue().asInstruction() instanceof UninitializedInstruction and
-      Ssa::defToNode(this, def, sv, _, _, _) and
-      v = sv.getBaseVariable().(Ssa::BaseIRVariable).getIRVariable().getAst()
+      Ssa::nodeToDefOrUse(this, def, _) and
+      v = def.getSourceVariable().getBaseVariable().(Ssa::BaseIRVariable).getIRVariable().getAst()
     )
   }
 
@@ -1297,6 +1202,466 @@ class UninitializedNode extends Node {
   LocalVariable getLocalVariable() { result = v }
 }
 
+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) {
+    // 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(AllocationExtentConvertTag())
+    )
+    or
+    // There's no instruction that returns `ParenthesisExpr`, but some queries
+    // expect this
+    exists(TranslatedTransparentConversion ttc |
+      result = ttc.getExpr().(ParenthesisExpr) and
+      instr = ttc.getResult()
+    )
+    or
+    // Certain expressions generate `CopyValueInstruction`s only when they
+    // are needed. Examples of this include crement operations and compound
+    // assignment operations. For example:
+    // ```cpp
+    // int x = ...
+    // int y = x++;
+    // ```
+    // this generate IR like:
+    // ```
+    // r1(glval<int>) = VariableAddress[x] :
+    // r2(int)        = Constant[0]        :
+    // m3(int)        = Store[x]           : &:r1, r2
+    // r4(glval<int>) = VariableAddress[y] :
+    // r5(glval<int>) = VariableAddress[x] :
+    // r6(int)        = Load[x]            : &:r5, m3
+    // r7(int)        = Constant[1]        :
+    // r8(int)        = Add                : r6, r7
+    // m9(int)        = Store[x]           : &:r5, r8
+    // r11(int)       = CopyValue         : r6
+    // m12(int)       = Store[y]          : &:r4, r11
+    // ```
+    // When the `CopyValueInstruction` is not generated there is no instruction
+    // whose `getConvertedResultExpression` maps back to the expression. When
+    // such an instruction doesn't exist it means that the old value is not
+    // needed, and in that case the only value that will propagate forward in
+    // the program is the value that's been updated. So in those cases we just
+    // use the result of `node.asDefinition()` as the result of `node.asExpr()`.
+    exists(TranslatedCoreExpr tco |
+      tco.getInstruction(_) = instr and
+      tco.producesExprResult() and
+      result = asDefinitionImpl0(instr)
+    )
+  }
+
+  private Expr getConvertedResultExpressionImpl(Instruction instr) {
+    result = getConvertedResultExpressionImpl0(instr)
+    or
+    not exists(getConvertedResultExpressionImpl0(instr)) and
+    result = instr.getConvertedResultExpression()
+  }
+
+  /**
+   * Gets the result for `node.asDefinition()` (when `node` is the instruction
+   * node that wraps `store`) in the cases where `store.getAst()` should not be
+   * used to define the result of `node.asDefinition()`.
+   */
+  private Expr asDefinitionImpl0(StoreInstruction store) {
+    // 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 |
+      store = tao.getInstruction(AssignmentStoreTag()) and
+      result = tao.getExpr()
+    )
+    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 |
+      store = tco.getInstruction(CrementStoreTag()) and
+      result = tco.getExpr()
+    )
+  }
+
+  /**
+   * Holds if the expression returned by `store.getAst()` should not be
+   * returned as the result of `node.asDefinition()` when `node` is the
+   * instruction node that wraps `store`.
+   */
+  private predicate excludeAsDefinitionResult(StoreInstruction store) {
+    // Exclude the store to the temporary generated by a ternary expression.
+    exists(TranslatedConditionalExpr tce |
+      store = tce.getInstruction(ConditionValueFalseStoreTag())
+      or
+      store = tce.getInstruction(ConditionValueTrueStoreTag())
+    )
+  }
+
+  /**
+   * Gets the expression that represents the result of `StoreInstruction` for
+   * dataflow purposes.
+   *
+   * For example, consider the following example
+   * ```cpp
+   * int x = 42;     // 1
+   * x = 34;         // 2
+   * ++x;            // 3
+   * x++;            // 4
+   * x += 1;         // 5
+   * int y = x += 2; // 6
+   * ```
+   * For (1) the result is `42`.
+   * For (2) the result is `x = 34`.
+   * For (3) the result is `++x`.
+   * For (4) the result is `x++`.
+   * For (5) the result is `x += 1`.
+   * For (6) there are two results:
+   *   - For the `StoreInstruction` generated by `x += 2` the result
+   *     is `x += 2`
+   *   - For the `StoreInstruction` generated by `int y = ...` the result
+   *     is also `x += 2`
+   */
+  Expr asDefinitionImpl(StoreInstruction store) {
+    not exists(asDefinitionImpl0(store)) and
+    not excludeAsDefinitionResult(store) and
+    result = store.getAst().(Expr).getUnconverted()
+    or
+    result = asDefinitionImpl0(store)
+  }
+}
+
+private import GetConvertedResultExpression
+
+/** Holds if `node` is an `OperandNode` that should map `node.asExpr()` to `e`. */
+predicate exprNodeShouldBeOperand(OperandNode node, Expr e, int n) {
+  not exprNodeShouldBeIndirectOperand(_, e, n) and
+  exists(Instruction def |
+    unique( | | getAUse(def)) = node.getOperand() and
+    e = getConvertedResultExpression(def, n)
+  )
+}
+
+/** Holds if `node` should be an `IndirectOperand` that maps `node.asIndirectExpr()` to `e`. */
+private predicate indirectExprNodeShouldBeIndirectOperand(
+  IndirectOperand node, Expr e, int n, int indirectionIndex
+) {
+  exists(Instruction def |
+    node.hasOperandAndIndirectionIndex(unique( | | getAUse(def)), indirectionIndex) and
+    e = getConvertedResultExpression(def, n)
+  )
+}
+
+/** Holds if `node` should be an `IndirectOperand` that maps `node.asExpr()` to `e`. */
+private predicate exprNodeShouldBeIndirectOperand(IndirectOperand node, Expr e, int n) {
+  exists(ArgumentOperand operand |
+    // When an argument (qualifier or positional) is a prvalue and the
+    // parameter (qualifier or positional) is a (const) reference, IR
+    // construction introduces a temporary `IRVariable`. The `VariableAddress`
+    // instruction has the argument as its `getConvertedResultExpression`
+    // result. However, the instruction actually represents the _address_ of
+    // the argument. So to fix this mismatch, we have the indirection of the
+    // `VariableAddressInstruction` map to the expression.
+    node.hasOperandAndIndirectionIndex(operand, 1) and
+    e = getConvertedResultExpression(operand.getDef(), n) and
+    operand.getDef().(VariableAddressInstruction).getIRVariable() instanceof IRTempVariable
+  )
+}
+
+private predicate exprNodeShouldBeIndirectOutNode(IndirectArgumentOutNode node, Expr e, int n) {
+  exists(CallInstruction call |
+    call.getStaticCallTarget() instanceof Constructor and
+    e = getConvertedResultExpression(call, n) 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) {
+  not exprNodeShouldBeOperand(_, e, n) and
+  not exprNodeShouldBeIndirectOutNode(_, e, n) and
+  not exprNodeShouldBeIndirectOperand(_, e, n) and
+  e = getConvertedResultExpression(node.asInstruction(), n)
+}
+
+/** Holds if `node` should be an `IndirectInstruction` that maps `node.asIndirectExpr()` to `e`. */
+predicate indirectExprNodeShouldBeIndirectInstruction(
+  IndirectInstruction node, Expr e, int n, int indirectionIndex
+) {
+  not indirectExprNodeShouldBeIndirectOperand(_, e, n, indirectionIndex) and
+  exists(Instruction instr |
+    node.hasInstructionAndIndirectionIndex(instr, indirectionIndex) and
+    e = getConvertedResultExpression(instr, n)
+  )
+}
+
+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);
+
+  /** Gets the non-conversion expression corresponding to this node, if any. */
+  final Expr getExpr(int n) { result = this.getConvertedExpr(n).getUnconverted() }
+}
+
+/**
+ * Holds if there exists a dataflow node whose `asExpr(n)` should evaluate
+ * to `e`.
+ */
+private predicate exprNodeShouldBe(Expr e, int n) {
+  exprNodeShouldBeInstruction(_, e, n) or
+  exprNodeShouldBeOperand(_, e, n) or
+  exprNodeShouldBeIndirectOutNode(_, e, n) or
+  exprNodeShouldBeIndirectOperand(_, e, n)
+}
+
+private class InstructionExprNode extends ExprNodeBase, InstructionNode {
+  InstructionExprNode() {
+    exists(Expr e, int n |
+      exprNodeShouldBeInstruction(this, e, n) and
+      not exists(Expr conv |
+        exprNodeShouldBe(conv, n + 1) and
+        conv.getUnconverted() = e.getUnconverted()
+      )
+    )
+  }
+
+  final override Expr getConvertedExpr(int n) { exprNodeShouldBeInstruction(this, result, n) }
+}
+
+private class OperandExprNode extends ExprNodeBase, OperandNode {
+  OperandExprNode() {
+    exists(Expr e, int n |
+      exprNodeShouldBeOperand(this, e, n) and
+      not exists(Expr conv |
+        exprNodeShouldBe(conv, n + 1) and
+        conv.getUnconverted() = e.getUnconverted()
+      )
+    )
+  }
+
+  final override Expr getConvertedExpr(int n) { exprNodeShouldBeOperand(this, result, n) }
+}
+
+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);
+
+  /** Gets the non-conversion expression corresponding to this node, if any. */
+  final Expr getExpr(int n, int indirectionIndex) {
+    result = this.getConvertedExpr(n, indirectionIndex).getUnconverted()
+  }
+}
+
+/** A signature for converting an indirect node to an expression. */
+private signature module IndirectNodeToIndirectExprSig {
+  /** The indirect node class to be converted to an expression */
+  class IndirectNode;
+
+  /**
+   * Holds if the indirect expression at indirection index `indirectionIndex`
+   * of `node` is `e`. The integer `n` specifies how many conversions has been
+   * applied to `node`.
+   */
+  predicate indirectNodeHasIndirectExpr(IndirectNode node, Expr e, int n, int indirectionIndex);
+}
+
+/**
+ * A module that implements the logic for deciding whether an indirect node
+ * should be an `IndirectExprNode`.
+ */
+private module IndirectNodeToIndirectExpr<IndirectNodeToIndirectExprSig Sig> {
+  import Sig
+
+  /**
+   * This predicate shifts the indirection index by one when `conv` is a
+   * `ReferenceDereferenceExpr`.
+   *
+   * This is necessary because `ReferenceDereferenceExpr` is a conversion
+   * in the AST, but appears as a `LoadInstruction` in the IR.
+   */
+  bindingset[e, indirectionIndex]
+  private predicate adjustForReference(
+    Expr e, int indirectionIndex, Expr conv, int adjustedIndirectionIndex
+  ) {
+    conv.(ReferenceDereferenceExpr).getExpr() = e and
+    adjustedIndirectionIndex = indirectionIndex - 1
+    or
+    not conv instanceof ReferenceDereferenceExpr and
+    conv = e and
+    adjustedIndirectionIndex = indirectionIndex
+  }
+
+  /** Holds if `node` should be an `IndirectExprNode`. */
+  predicate charpred(IndirectNode node) {
+    exists(Expr e, int n, int indirectionIndex |
+      indirectNodeHasIndirectExpr(node, e, n, indirectionIndex) and
+      not exists(Expr conv, int adjustedIndirectionIndex |
+        adjustForReference(e, indirectionIndex, conv, adjustedIndirectionIndex) and
+        indirectExprNodeShouldBe(conv, n + 1, adjustedIndirectionIndex)
+      )
+    )
+  }
+}
+
+private predicate indirectExprNodeShouldBe(Expr e, int n, int indirectionIndex) {
+  indirectExprNodeShouldBeIndirectOperand(_, e, n, indirectionIndex) or
+  indirectExprNodeShouldBeIndirectInstruction(_, e, n, indirectionIndex)
+}
+
+private module IndirectOperandIndirectExprNodeImpl implements IndirectNodeToIndirectExprSig {
+  class IndirectNode = IndirectOperand;
+
+  predicate indirectNodeHasIndirectExpr = indirectExprNodeShouldBeIndirectOperand/4;
+}
+
+module IndirectOperandToIndirectExpr =
+  IndirectNodeToIndirectExpr<IndirectOperandIndirectExprNodeImpl>;
+
+private class IndirectOperandIndirectExprNode extends IndirectExprNodeBase instanceof IndirectOperand
+{
+  IndirectOperandIndirectExprNode() { IndirectOperandToIndirectExpr::charpred(this) }
+
+  final override Expr getConvertedExpr(int n, int index) {
+    IndirectOperandToIndirectExpr::indirectNodeHasIndirectExpr(this, result, n, index)
+  }
+}
+
+private module IndirectInstructionIndirectExprNodeImpl implements IndirectNodeToIndirectExprSig {
+  class IndirectNode = IndirectInstruction;
+
+  predicate indirectNodeHasIndirectExpr = indirectExprNodeShouldBeIndirectInstruction/4;
+}
+
+module IndirectInstructionToIndirectExpr =
+  IndirectNodeToIndirectExpr<IndirectInstructionIndirectExprNodeImpl>;
+
+private class IndirectInstructionIndirectExprNode extends IndirectExprNodeBase instanceof IndirectInstruction
+{
+  IndirectInstructionIndirectExprNode() { IndirectInstructionToIndirectExpr::charpred(this) }
+
+  final override Expr getConvertedExpr(int n, int index) {
+    IndirectInstructionToIndirectExpr::indirectNodeHasIndirectExpr(this, result, n, index)
+  }
+}
+
+private class IndirectArgumentOutExprNode extends ExprNodeBase, IndirectArgumentOutNode {
+  IndirectArgumentOutExprNode() { exprNodeShouldBeIndirectOutNode(this, _, _) }
+
+  final override Expr getConvertedExpr(int n) { exprNodeShouldBeIndirectOutNode(this, result, n) }
+}
+
+private class IndirectOperandExprNode extends ExprNodeBase instanceof IndirectOperand {
+  IndirectOperandExprNode() { exprNodeShouldBeIndirectOperand(this, _, _) }
+
+  final override Expr getConvertedExpr(int n) { exprNodeShouldBeIndirectOperand(this, result, n) }
+}
+
+/**
+ * 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(_) }
+
+  /**
+   * 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(_) }
+}
+
+/**
+ * An indirect expression, viewed as a node in a data flow graph.
+ */
+class IndirectExprNode extends Node instanceof IndirectExprNodeBase {
+  /**
+   * 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(int indirectionIndex) { result = this.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) {
+    result = this.getConvertedExpr(_, indirectionIndex)
+  }
+}
+
 abstract private class AbstractParameterNode extends Node {
   /**
    * Holds if this node is the parameter of `f` at the specified position. The
@@ -1786,11 +2151,6 @@ private module Cached {
       // Def-use/Use-use flow
       Ssa::ssaFlow(nodeFrom, nodeTo)
       or
-      // Phi input -> Phi
-      nodeFrom.(SsaPhiInputNode).getPhiNode() = nodeTo.(SsaPhiNode).getPhiNode()
-      or
-      IteratorFlow::localFlowStep(nodeFrom, nodeTo)
-      or
       // Operand -> Instruction flow
       simpleInstructionLocalFlowStep(nodeFrom.asOperand(), nodeTo.asInstruction())
       or
@@ -2227,22 +2587,6 @@ class ContentSet instanceof Content {
   }
 }
 
-pragma[nomagic]
-private predicate guardControlsPhiInput(
-  IRGuardCondition g, boolean branch, Ssa::Definition def, IRBlock input, Ssa::PhiNode phi
-) {
-  phi.hasInputFromBlock(def, _, _, _, input) and
-  (
-    g.controls(input, branch)
-    or
-    exists(EdgeKind kind |
-      g.getBlock() = input and
-      kind = getConditionalEdge(branch) and
-      input.getSuccessor(kind) = phi.getBasicBlock()
-    )
-  )
-}
-
 /**
  * Holds if the guard `g` validates the expression `e` upon evaluating to `branch`.
  *
@@ -2291,21 +2635,13 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
    *
    * NOTE: If an indirect expression is tracked, use `getAnIndirectBarrierNode` instead.
    */
-  Node getABarrierNode() {
+  ExprNode getABarrierNode() {
     exists(IRGuardCondition g, Expr e, ValueNumber value, boolean edge |
       e = value.getAnInstruction().getConvertedResultExpression() and
-      result.asConvertedExpr() = e and
+      result.getConvertedExpr() = e and
       guardChecks(g, value.getAnInstruction().getConvertedResultExpression(), edge) and
       g.controls(result.getBasicBlock(), edge)
     )
-    or
-    exists(
-      IRGuardCondition g, boolean branch, Ssa::DefinitionExt def, IRBlock input, Ssa::PhiNode phi
-    |
-      guardChecks(g, def.getARead().asOperand().getDef().getConvertedResultExpression(), branch) and
-      guardControlsPhiInput(g, branch, def, input, phi) and
-      result = TSsaPhiInputNode(phi, input)
-    )
   }
 
   /**
@@ -2341,7 +2677,7 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
    *
    * NOTE: If a non-indirect expression is tracked, use `getABarrierNode` instead.
    */
-  Node getAnIndirectBarrierNode() { result = getAnIndirectBarrierNode(_) }
+  IndirectExprNode getAnIndirectBarrierNode() { result = getAnIndirectBarrierNode(_) }
 
   /**
    * Gets an indirect expression node with indirection index `indirectionIndex` that is
@@ -2377,23 +2713,13 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
    *
    * NOTE: If a non-indirect expression is tracked, use `getABarrierNode` instead.
    */
-  Node getAnIndirectBarrierNode(int indirectionIndex) {
+  IndirectExprNode getAnIndirectBarrierNode(int indirectionIndex) {
     exists(IRGuardCondition g, Expr e, ValueNumber value, boolean edge |
       e = value.getAnInstruction().getConvertedResultExpression() and
-      result.asIndirectConvertedExpr(indirectionIndex) = e and
+      result.getConvertedExpr(indirectionIndex) = e and
       guardChecks(g, value.getAnInstruction().getConvertedResultExpression(), edge) and
       g.controls(result.getBasicBlock(), edge)
     )
-    or
-    exists(
-      IRGuardCondition g, boolean branch, Ssa::DefinitionExt def, IRBlock input, Ssa::PhiNode phi
-    |
-      guardChecks(g,
-        def.getARead().asIndirectOperand(indirectionIndex).getDef().getConvertedResultExpression(),
-        branch) and
-      guardControlsPhiInput(g, branch, def, input, phi) and
-      result = TSsaPhiInputNode(phi, input)
-    )
   }
 }
 
@@ -2402,14 +2728,6 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
  */
 signature predicate instructionGuardChecksSig(IRGuardCondition g, Instruction instr, boolean branch);
 
-private EdgeKind getConditionalEdge(boolean branch) {
-  branch = true and
-  result instanceof TrueEdge
-  or
-  branch = false and
-  result instanceof FalseEdge
-}
-
 /**
  * Provides a set of barrier nodes for a guard that validates an instruction.
  *
@@ -2418,20 +2736,12 @@ private EdgeKind getConditionalEdge(boolean branch) {
  */
 module InstructionBarrierGuard<instructionGuardChecksSig/3 instructionGuardChecks> {
   /** Gets a node that is safely guarded by the given guard check. */
-  Node getABarrierNode() {
+  ExprNode getABarrierNode() {
     exists(IRGuardCondition g, ValueNumber value, boolean edge, Operand use |
       instructionGuardChecks(g, value.getAnInstruction(), edge) and
       use = value.getAnInstruction().getAUse() and
       result.asOperand() = use and
-      g.controls(result.getBasicBlock(), edge)
-    )
-    or
-    exists(
-      IRGuardCondition g, boolean branch, Ssa::DefinitionExt def, IRBlock input, Ssa::PhiNode phi
-    |
-      instructionGuardChecks(g, def.getARead().asOperand().getDef(), branch) and
-      guardControlsPhiInput(g, branch, def, input, phi) and
-      result = TSsaPhiInputNode(phi, input)
+      g.controls(use.getDef().getBlock(), edge)
     )
   }
 }

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

@@ -1,518 +0,0 @@
-/**
- * Provides the classes `ExprNode` and `IndirectExprNode` for converting between `Expr` and `Node`.
- */
-
-private import cpp
-private import semmle.code.cpp.ir.IR
-private import DataFlowUtil
-private import DataFlowPrivate
-private import semmle.code.cpp.ir.implementation.raw.internal.TranslatedExpr
-private import semmle.code.cpp.ir.implementation.raw.internal.InstructionTag
-
-cached
-private module Cached {
-  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.
-   */
-  private 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) {
-    // 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(AllocationExtentConvertTag())
-    )
-    or
-    // There's no instruction that returns `ParenthesisExpr`, but some queries
-    // expect this
-    exists(TranslatedTransparentConversion ttc |
-      result = ttc.getExpr().(ParenthesisExpr) and
-      instr = ttc.getResult()
-    )
-    or
-    // Certain expressions generate `CopyValueInstruction`s only when they
-    // are needed. Examples of this include crement operations and compound
-    // assignment operations. For example:
-    // ```cpp
-    // int x = ...
-    // int y = x++;
-    // ```
-    // this generate IR like:
-    // ```
-    // r1(glval<int>) = VariableAddress[x] :
-    // r2(int)        = Constant[0]        :
-    // m3(int)        = Store[x]           : &:r1, r2
-    // r4(glval<int>) = VariableAddress[y] :
-    // r5(glval<int>) = VariableAddress[x] :
-    // r6(int)        = Load[x]            : &:r5, m3
-    // r7(int)        = Constant[1]        :
-    // r8(int)        = Add                : r6, r7
-    // m9(int)        = Store[x]           : &:r5, r8
-    // r11(int)       = CopyValue         : r6
-    // m12(int)       = Store[y]          : &:r4, r11
-    // ```
-    // When the `CopyValueInstruction` is not generated there is no instruction
-    // whose `getConvertedResultExpression` maps back to the expression. When
-    // such an instruction doesn't exist it means that the old value is not
-    // needed, and in that case the only value that will propagate forward in
-    // the program is the value that's been updated. So in those cases we just
-    // use the result of `node.asDefinition()` as the result of `node.asExpr()`.
-    exists(TranslatedCoreExpr tco |
-      tco.getInstruction(_) = instr and
-      tco.producesExprResult() and
-      result = asDefinitionImpl0(instr)
-    )
-  }
-
-  private Expr getConvertedResultExpressionImpl(Instruction instr) {
-    result = getConvertedResultExpressionImpl0(instr)
-    or
-    not exists(getConvertedResultExpressionImpl0(instr)) and
-    result = instr.getConvertedResultExpression()
-  }
-
-  /**
-   * Gets the result for `node.asDefinition()` (when `node` is the instruction
-   * node that wraps `store`) in the cases where `store.getAst()` should not be
-   * used to define the result of `node.asDefinition()`.
-   */
-  private Expr asDefinitionImpl0(StoreInstruction store) {
-    // 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 |
-      store = tao.getInstruction(AssignmentStoreTag()) and
-      result = tao.getExpr()
-    )
-    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 |
-      store = tco.getInstruction(CrementStoreTag()) and
-      result = tco.getExpr()
-    )
-  }
-
-  /**
-   * Holds if the expression returned by `store.getAst()` should not be
-   * returned as the result of `node.asDefinition()` when `node` is the
-   * instruction node that wraps `store`.
-   */
-  private predicate excludeAsDefinitionResult(StoreInstruction store) {
-    // Exclude the store to the temporary generated by a ternary expression.
-    exists(TranslatedConditionalExpr tce |
-      store = tce.getInstruction(ConditionValueFalseStoreTag())
-      or
-      store = tce.getInstruction(ConditionValueTrueStoreTag())
-    )
-  }
-
-  /**
-   * Gets the expression that represents the result of `StoreInstruction` for
-   * dataflow purposes.
-   *
-   * For example, consider the following example
-   * ```cpp
-   * int x = 42;     // 1
-   * x = 34;         // 2
-   * ++x;            // 3
-   * x++;            // 4
-   * x += 1;         // 5
-   * int y = x += 2; // 6
-   * ```
-   * For (1) the result is `42`.
-   * For (2) the result is `x = 34`.
-   * For (3) the result is `++x`.
-   * For (4) the result is `x++`.
-   * For (5) the result is `x += 1`.
-   * For (6) there are two results:
-   *   - For the `StoreInstruction` generated by `x += 2` the result
-   *     is `x += 2`
-   *   - For the `StoreInstruction` generated by `int y = ...` the result
-   *     is also `x += 2`
-   */
-  cached
-  Expr asDefinitionImpl(StoreInstruction store) {
-    not exists(asDefinitionImpl0(store)) and
-    not excludeAsDefinitionResult(store) and
-    result = store.getAst().(Expr).getUnconverted()
-    or
-    result = asDefinitionImpl0(store)
-  }
-
-  /** Holds if `node` is an `OperandNode` that should map `node.asExpr()` to `e`. */
-  private predicate exprNodeShouldBeOperand(OperandNode node, Expr e, int n) {
-    not exprNodeShouldBeIndirectOperand(_, e, n) and
-    exists(Instruction def |
-      unique( | | getAUse(def)) = node.getOperand() and
-      e = getConvertedResultExpression(def, n)
-    )
-  }
-
-  /** Holds if `node` should be an `IndirectOperand` that maps `node.asIndirectExpr()` to `e`. */
-  private predicate indirectExprNodeShouldBeIndirectOperand(
-    IndirectOperand node, Expr e, int n, int indirectionIndex
-  ) {
-    exists(Instruction def |
-      node.hasOperandAndIndirectionIndex(unique( | | getAUse(def)), indirectionIndex) and
-      e = getConvertedResultExpression(def, n)
-    )
-  }
-
-  /** Holds if `operand`'s definition is a `VariableAddressInstruction` whose variable is a temporary */
-  private predicate isIRTempVariable(Operand operand) {
-    operand.getDef().(VariableAddressInstruction).getIRVariable() instanceof IRTempVariable
-  }
-
-  /**
-   * Holds if `node` is an indirect operand whose operand is an argument, and
-   * the `n`'th expression associated with the operand is `e`.
-   */
-  private predicate isIndirectOperandOfArgument(
-    IndirectOperand node, ArgumentOperand operand, Expr e, int n
-  ) {
-    node.hasOperandAndIndirectionIndex(operand, 1) and
-    e = getConvertedResultExpression(operand.getDef(), n)
-  }
-
-  /**
-   * Holds if `opFrom` is an operand to a conversion, and `opTo` is the unique
-   * use of the conversion.
-   */
-  private predicate isConversionStep(Operand opFrom, Operand opTo) {
-    exists(Instruction mid |
-      conversionFlow(opFrom, mid, false, false) and
-      opTo = unique( | | getAUse(mid))
-    )
-  }
-
-  /**
-   * Holds if an operand that satisfies `isIRTempVariable` flows to `op`
-   * through a (possibly empty) sequence of conversions.
-   */
-  private predicate irTempOperandConversionFlows(Operand op) {
-    isIRTempVariable(op)
-    or
-    exists(Operand mid |
-      irTempOperandConversionFlows(mid) and
-      isConversionStep(mid, op)
-    )
-  }
-
-  /** Holds if `node` should be an `IndirectOperand` that maps `node.asExpr()` to `e`. */
-  private predicate exprNodeShouldBeIndirectOperand(IndirectOperand node, Expr e, int n) {
-    exists(ArgumentOperand operand |
-      // When an argument (qualifier or positional) is a prvalue and the
-      // parameter (qualifier or positional) is a (const) reference, IR
-      // construction introduces a temporary `IRVariable`. The `VariableAddress`
-      // instruction has the argument as its `getConvertedResultExpression`
-      // result. However, the instruction actually represents the _address_ of
-      // the argument. So to fix this mismatch, we have the indirection of the
-      // `VariableAddressInstruction` map to the expression.
-      isIndirectOperandOfArgument(node, operand, e, n) and
-      irTempOperandConversionFlows(operand)
-    )
-  }
-
-  private predicate exprNodeShouldBeIndirectOutNode(IndirectArgumentOutNode node, Expr e, int n) {
-    exists(CallInstruction call |
-      call.getStaticCallTarget() instanceof Constructor and
-      e = getConvertedResultExpression(call, n) and
-      call.getThisArgumentOperand() = node.getAddressOperand()
-    )
-  }
-
-  /** Holds if `node` should be an instruction node that maps `node.asExpr()` to `e`. */
-  private predicate exprNodeShouldBeInstruction(Node node, Expr e, int n) {
-    not exprNodeShouldBeOperand(_, e, n) and
-    not exprNodeShouldBeIndirectOutNode(_, e, n) and
-    not exprNodeShouldBeIndirectOperand(_, e, n) and
-    e = getConvertedResultExpression(node.asInstruction(), n)
-  }
-
-  /** Holds if `node` should be an `IndirectInstruction` that maps `node.asIndirectExpr()` to `e`. */
-  private predicate indirectExprNodeShouldBeIndirectInstruction(
-    IndirectInstruction node, Expr e, int n, int indirectionIndex
-  ) {
-    not indirectExprNodeShouldBeIndirectOperand(_, e, n, indirectionIndex) and
-    exists(Instruction instr |
-      node.hasInstructionAndIndirectionIndex(instr, indirectionIndex) and
-      e = getConvertedResultExpression(instr, n)
-    )
-  }
-
-  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);
-
-    /** Gets the non-conversion expression corresponding to this node, if any. */
-    final Expr getExpr(int n) { result = this.getConvertedExpr(n).getUnconverted() }
-  }
-
-  /**
-   * Holds if there exists a dataflow node whose `asExpr(n)` should evaluate
-   * to `e`.
-   */
-  private predicate exprNodeShouldBe(Expr e, int n) {
-    exprNodeShouldBeInstruction(_, e, n) or
-    exprNodeShouldBeOperand(_, e, n) or
-    exprNodeShouldBeIndirectOutNode(_, e, n) or
-    exprNodeShouldBeIndirectOperand(_, e, n)
-  }
-
-  private class InstructionExprNode extends ExprNodeBase, InstructionNode {
-    InstructionExprNode() {
-      exists(Expr e, int n |
-        exprNodeShouldBeInstruction(this, e, n) and
-        not exists(Expr conv |
-          exprNodeShouldBe(conv, n + 1) and
-          conv.getUnconverted() = e.getUnconverted()
-        )
-      )
-    }
-
-    final override Expr getConvertedExpr(int n) { exprNodeShouldBeInstruction(this, result, n) }
-  }
-
-  private class OperandExprNode extends ExprNodeBase, OperandNode {
-    OperandExprNode() {
-      exists(Expr e, int n |
-        exprNodeShouldBeOperand(this, e, n) and
-        not exists(Expr conv |
-          exprNodeShouldBe(conv, n + 1) and
-          conv.getUnconverted() = e.getUnconverted()
-        )
-      )
-    }
-
-    final override Expr getConvertedExpr(int n) { exprNodeShouldBeOperand(this, result, n) }
-  }
-
-  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);
-
-    /** Gets the non-conversion expression corresponding to this node, if any. */
-    final Expr getExpr(int n, int indirectionIndex) {
-      result = this.getConvertedExpr(n, indirectionIndex).getUnconverted()
-    }
-  }
-
-  /** A signature for converting an indirect node to an expression. */
-  private signature module IndirectNodeToIndirectExprSig {
-    /** The indirect node class to be converted to an expression */
-    class IndirectNode;
-
-    /**
-     * Holds if the indirect expression at indirection index `indirectionIndex`
-     * of `node` is `e`. The integer `n` specifies how many conversions has been
-     * applied to `node`.
-     */
-    predicate indirectNodeHasIndirectExpr(IndirectNode node, Expr e, int n, int indirectionIndex);
-  }
-
-  /**
-   * A module that implements the logic for deciding whether an indirect node
-   * should be an `IndirectExprNode`.
-   */
-  private module IndirectNodeToIndirectExpr<IndirectNodeToIndirectExprSig Sig> {
-    import Sig
-
-    /**
-     * This predicate shifts the indirection index by one when `conv` is a
-     * `ReferenceDereferenceExpr`.
-     *
-     * This is necessary because `ReferenceDereferenceExpr` is a conversion
-     * in the AST, but appears as a `LoadInstruction` in the IR.
-     */
-    bindingset[e, indirectionIndex]
-    private predicate adjustForReference(
-      Expr e, int indirectionIndex, Expr conv, int adjustedIndirectionIndex
-    ) {
-      conv.(ReferenceDereferenceExpr).getExpr() = e and
-      adjustedIndirectionIndex = indirectionIndex - 1
-      or
-      not conv instanceof ReferenceDereferenceExpr and
-      conv = e and
-      adjustedIndirectionIndex = indirectionIndex
-    }
-
-    /** Holds if `node` should be an `IndirectExprNode`. */
-    predicate charpred(IndirectNode node) {
-      exists(Expr e, int n, int indirectionIndex |
-        indirectNodeHasIndirectExpr(node, e, n, indirectionIndex) and
-        not exists(Expr conv, int adjustedIndirectionIndex |
-          adjustForReference(e, indirectionIndex, conv, adjustedIndirectionIndex) and
-          indirectExprNodeShouldBe(conv, n + 1, adjustedIndirectionIndex)
-        )
-      )
-    }
-  }
-
-  private predicate indirectExprNodeShouldBe(Expr e, int n, int indirectionIndex) {
-    indirectExprNodeShouldBeIndirectOperand(_, e, n, indirectionIndex) or
-    indirectExprNodeShouldBeIndirectInstruction(_, e, n, indirectionIndex)
-  }
-
-  private module IndirectOperandIndirectExprNodeImpl implements IndirectNodeToIndirectExprSig {
-    class IndirectNode = IndirectOperand;
-
-    predicate indirectNodeHasIndirectExpr = indirectExprNodeShouldBeIndirectOperand/4;
-  }
-
-  module IndirectOperandToIndirectExpr =
-    IndirectNodeToIndirectExpr<IndirectOperandIndirectExprNodeImpl>;
-
-  private class IndirectOperandIndirectExprNode extends IndirectExprNodeBase instanceof IndirectOperand
-  {
-    IndirectOperandIndirectExprNode() { IndirectOperandToIndirectExpr::charpred(this) }
-
-    final override Expr getConvertedExpr(int n, int index) {
-      IndirectOperandToIndirectExpr::indirectNodeHasIndirectExpr(this, result, n, index)
-    }
-  }
-
-  private module IndirectInstructionIndirectExprNodeImpl implements IndirectNodeToIndirectExprSig {
-    class IndirectNode = IndirectInstruction;
-
-    predicate indirectNodeHasIndirectExpr = indirectExprNodeShouldBeIndirectInstruction/4;
-  }
-
-  module IndirectInstructionToIndirectExpr =
-    IndirectNodeToIndirectExpr<IndirectInstructionIndirectExprNodeImpl>;
-
-  private class IndirectInstructionIndirectExprNode extends IndirectExprNodeBase instanceof IndirectInstruction
-  {
-    IndirectInstructionIndirectExprNode() { IndirectInstructionToIndirectExpr::charpred(this) }
-
-    final override Expr getConvertedExpr(int n, int index) {
-      IndirectInstructionToIndirectExpr::indirectNodeHasIndirectExpr(this, result, n, index)
-    }
-  }
-
-  private class IndirectArgumentOutExprNode extends ExprNodeBase, IndirectArgumentOutNode {
-    IndirectArgumentOutExprNode() { exprNodeShouldBeIndirectOutNode(this, _, _) }
-
-    final override Expr getConvertedExpr(int n) { exprNodeShouldBeIndirectOutNode(this, result, n) }
-  }
-
-  private class IndirectOperandExprNode extends ExprNodeBase instanceof IndirectOperand {
-    IndirectOperandExprNode() { exprNodeShouldBeIndirectOperand(this, _, _) }
-
-    final override Expr getConvertedExpr(int n) { exprNodeShouldBeIndirectOperand(this, result, n) }
-  }
-
-  /**
-   * An expression, viewed as a node in a data flow graph.
-   */
-  cached
-  class ExprNode extends Node instanceof ExprNodeBase {
-    /**
-     * INTERNAL: Do not use.
-     */
-    cached
-    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.
-     */
-    cached
-    final Expr getExpr() { result = this.getExpr(_) }
-
-    /**
-     * INTERNAL: Do not use.
-     */
-    cached
-    Expr getConvertedExpr(int n) { result = super.getConvertedExpr(n) }
-
-    /**
-     * Gets the expression corresponding to this node, if any. The returned
-     * expression may be a `Conversion`.
-     */
-    cached
-    final Expr getConvertedExpr() { result = this.getConvertedExpr(_) }
-  }
-
-  /**
-   * An indirect expression, viewed as a node in a data flow graph.
-   */
-  cached
-  class IndirectExprNode extends Node instanceof IndirectExprNodeBase {
-    /**
-     * 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.
-     */
-    cached
-    final Expr getExpr(int indirectionIndex) { result = this.getExpr(_, indirectionIndex) }
-
-    /**
-     * INTERNAL: Do not use.
-     */
-    cached
-    Expr getExpr(int n, int indirectionIndex) { result = super.getExpr(n, indirectionIndex) }
-
-    /**
-     * INTERNAL: Do not use.
-     */
-    cached
-    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`.
-     */
-    cached
-    Expr getConvertedExpr(int indirectionIndex) {
-      result = this.getConvertedExpr(_, indirectionIndex)
-    }
-  }
-}
-
-import Cached

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

@@ -546,7 +546,7 @@ module ProductFlow {
       Flow1::PathGraph::edges(pred1, succ1, _, _) and
       exists(ReturnKindExt returnKind |
         succ1.getNode() = returnKind.getAnOutNode(call) and
-        paramReturnNode(_, pred1.asParameterReturnNode(), _, returnKind)
+        pred1.getNode().(ReturnNodeExt).getKind() = returnKind
       )
     }
 
@@ -574,7 +574,7 @@ module ProductFlow {
       Flow2::PathGraph::edges(pred2, succ2, _, _) and
       exists(ReturnKindExt returnKind |
         succ2.getNode() = returnKind.getAnOutNode(call) and
-        paramReturnNode(_, pred2.asParameterReturnNode(), _, returnKind)
+        pred2.getNode().(ReturnNodeExt).getKind() = returnKind
       )
     }
 

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

@@ -246,6 +246,14 @@ private module IteratorIndirections {
       baseType = super.getValueType()
     }
 
+    override predicate isAdditionalDereference(Instruction deref, Operand address) {
+      exists(CallInstruction call |
+        operandForFullyConvertedCall(getAUse(deref), call) and
+        this = call.getStaticCallTarget().getClassAndName("operator*") and
+        address = call.getThisArgumentOperand()
+      )
+    }
+
     override predicate isAdditionalWrite(Node0Impl value, Operand address, boolean certain) {
       exists(CallInstruction call | call.getArgumentOperand(0) = value.asOperand() |
         this = call.getStaticCallTarget().getClassAndName("operator=") and
@@ -254,6 +262,16 @@ private module IteratorIndirections {
       )
     }
 
+    override predicate isAdditionalTaintStep(Node node1, Node node2) {
+      exists(CallInstruction call |
+        // Taint through `operator+=` and `operator-=` on iterators.
+        call.getStaticCallTarget() instanceof Iterator::IteratorAssignArithmeticOperator and
+        node2.(IndirectArgumentOutNode).getPreUpdateNode() = node1 and
+        node1.(IndirectOperand).hasOperandAndIndirectionIndex(call.getArgumentOperand(0), _) and
+        node1.getType().getUnspecifiedType() = this
+      )
+    }
+
     override predicate isAdditionalConversionFlow(Operand opFrom, Instruction instrTo) {
       // This is a bit annoying: Consider the following snippet:
       // ```
@@ -571,6 +589,230 @@ private class BaseCallInstruction extends BaseSourceVariableInstruction, CallIns
 
 cached
 private module Cached {
+  private import semmle.code.cpp.models.interfaces.Iterator as Interfaces
+  private import semmle.code.cpp.models.implementations.Iterator as Iterator
+  private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs as IO
+
+  /**
+   * Holds if `next` is a instruction with a memory result that potentially
+   * updates the memory produced by `prev`.
+   */
+  private predicate memorySucc(Instruction prev, Instruction next) {
+    prev = next.(ChiInstruction).getTotal()
+    or
+    // Phi inputs can be inexact.
+    prev = next.(PhiInstruction).getAnInputOperand().getAnyDef()
+    or
+    prev = next.(CopyInstruction).getSourceValue()
+    or
+    exists(ReadSideEffectInstruction read |
+      next = read.getPrimaryInstruction() and
+      isAdditionalConversionFlow(_, next) and
+      prev = read.getSideEffectOperand().getAnyDef()
+    )
+  }
+
+  /**
+   * Holds if `iteratorDerefAddress` is an address of an iterator dereference (i.e., `*it`)
+   * that is used for a write operation that writes the value `value`. The `memory` instruction
+   * represents the memory that the IR's SSA analysis determined was read by the call to `operator*`.
+   *
+   * The `numberOfLoads` integer represents the number of dereferences this write corresponds to
+   * on the underlying container that produced the iterator.
+   */
+  private predicate isChiAfterIteratorDef(
+    Instruction memory, Operand iteratorDerefAddress, Node0Impl value, int numberOfLoads
+  ) {
+    exists(
+      BaseSourceVariableInstruction iteratorBase, ReadSideEffectInstruction read,
+      Operand iteratorAddress
+    |
+      numberOfLoads >= 0 and
+      isDef(_, value, iteratorDerefAddress, iteratorBase, numberOfLoads + 2, 0) and
+      isUse(_, iteratorAddress, iteratorBase, numberOfLoads + 1, 0) and
+      iteratorBase.getResultType() instanceof Interfaces::Iterator and
+      isDereference(iteratorAddress.getDef(), read.getArgumentDef().getAUse(), _) and
+      memory = read.getSideEffectOperand().getAnyDef()
+    )
+  }
+
+  private predicate isSource(Instruction instr, Operand iteratorAddress, int numberOfLoads) {
+    getAUse(instr) = iteratorAddress and
+    exists(BaseSourceVariableInstruction iteratorBase |
+      iteratorBase.getResultType() instanceof Interfaces::Iterator and
+      not iteratorBase.getResultType() instanceof Cpp::PointerType and
+      isUse(_, iteratorAddress, iteratorBase, numberOfLoads - 1, 0)
+    )
+  }
+
+  private predicate isSink(Instruction instr, CallInstruction call) {
+    getAUse(instr).(ArgumentOperand).getCall() = call and
+    // Only include operations that may modify the object that the iterator points to.
+    // The following is a non-exhaustive list of things that may modify the value of the
+    // iterator, but never the value of what the iterator points to.
+    // The more things we can exclude here, the faster the small dataflow-like analysis
+    // done by `convertsIntoArgument` will converge.
+    not exists(Function f | f = call.getStaticCallTarget() |
+      f instanceof Iterator::IteratorCrementOperator or
+      f instanceof Iterator::IteratorBinaryArithmeticOperator or
+      f instanceof Iterator::IteratorAssignArithmeticOperator or
+      f instanceof Iterator::IteratorCrementMemberOperator or
+      f instanceof Iterator::IteratorBinaryArithmeticMemberOperator or
+      f instanceof Iterator::IteratorAssignArithmeticMemberOperator or
+      f instanceof Iterator::IteratorAssignmentMemberOperator
+    )
+  }
+
+  private predicate convertsIntoArgumentFwd(Instruction instr) {
+    isSource(instr, _, _)
+    or
+    exists(Instruction prev | convertsIntoArgumentFwd(prev) |
+      conversionFlow(unique( | | getAUse(prev)), instr, false, _)
+    )
+  }
+
+  private predicate convertsIntoArgumentRev(Instruction instr) {
+    convertsIntoArgumentFwd(instr) and
+    (
+      isSink(instr, _)
+      or
+      exists(Instruction next | convertsIntoArgumentRev(next) |
+        conversionFlow(unique( | | getAUse(instr)), next, false, _)
+      )
+    )
+  }
+
+  private predicate convertsIntoArgument(
+    Operand iteratorAddress, CallInstruction call, int numberOfLoads
+  ) {
+    exists(Instruction iteratorAddressDef |
+      isSource(iteratorAddressDef, iteratorAddress, numberOfLoads) and
+      isSink(iteratorAddressDef, call) and
+      convertsIntoArgumentRev(pragma[only_bind_into](iteratorAddressDef))
+    )
+  }
+
+  private predicate isChiAfterIteratorArgument(
+    Instruction memory, Operand iteratorAddress, int numberOfLoads
+  ) {
+    // Ideally, `iteratorAddress` would be an `ArgumentOperand`, but there might be
+    // various conversions applied to it before it becomes an argument.
+    // So we do a small amount of flow to find the call that the iterator is passed to.
+    exists(CallInstruction call | convertsIntoArgument(iteratorAddress, call, numberOfLoads) |
+      exists(ReadSideEffectInstruction read |
+        read.getPrimaryInstruction() = call and
+        read.getSideEffectOperand().getAnyDef() = memory
+      )
+      or
+      exists(LoadInstruction load |
+        iteratorAddress.getDef() = load and
+        memory = load.getSourceValueOperand().getAnyDef()
+      )
+    )
+  }
+
+  /**
+   * Holds if `iterator` is a `StoreInstruction` that stores the result of some function
+   * returning an iterator into an address computed started at `containerBase`.
+   *
+   * For example, given a declaration like `std::vector<int>::iterator it = v.begin()`,
+   * the `iterator` will be the `StoreInstruction` generated by the write to `it`, and
+   * `containerBase` will be the address of `v`.
+   */
+  private predicate isChiAfterBegin(
+    BaseSourceVariableInstruction containerBase, StoreInstruction iterator
+  ) {
+    exists(
+      CallInstruction getIterator, Iterator::GetIteratorFunction getIteratorFunction,
+      IO::FunctionInput input, int i
+    |
+      getIterator = iterator.getSourceValue() and
+      getIteratorFunction = getIterator.getStaticCallTarget() and
+      getIteratorFunction.getsIterator(input, _) and
+      isDef(_, any(Node0Impl n | n.asInstruction() = iterator), _, _, 1, 0) and
+      input.isParameterDerefOrQualifierObject(i) and
+      isUse(_, getIterator.getArgumentOperand(i), containerBase, 0, 0)
+    )
+  }
+
+  /**
+   * Holds if `iteratorAddress` is an address of an iterator that is used for
+   * a read operation. The `memory` instruction represents the memory that
+   * the IR's SSA analysis determined was read by the call to `operator*`.
+   *
+   * Finally, the `numberOfLoads` integer represents the number of dereferences
+   * this read corresponds to on the underlying container that produced the iterator.
+   */
+  private predicate isChiBeforeIteratorUse(
+    Operand iteratorAddress, Instruction memory, int numberOfLoads
+  ) {
+    exists(
+      BaseSourceVariableInstruction iteratorBase, LoadInstruction load,
+      ReadSideEffectInstruction read, Operand iteratorDerefAddress
+    |
+      numberOfLoads >= 0 and
+      isUse(_, iteratorAddress, iteratorBase, numberOfLoads + 1, 0) and
+      isUse(_, iteratorDerefAddress, iteratorBase, numberOfLoads + 2, 0) and
+      iteratorBase.getResultType() instanceof Interfaces::Iterator and
+      load.getSourceAddressOperand() = iteratorDerefAddress and
+      read.getPrimaryInstruction() = load.getSourceAddress() and
+      memory = read.getSideEffectOperand().getAnyDef()
+    )
+  }
+
+  /**
+   * Holds if `iteratorDerefAddress` is an address of an iterator dereference (i.e., `*it`)
+   * that is used for a write operation that writes the value `value` to a container that
+   * created the iterator. `container` represents the base of the address of the container
+   * that was used to create the iterator.
+   */
+  cached
+  predicate isIteratorDef(
+    BaseSourceVariableInstruction container, Operand iteratorDerefAddress, Node0Impl value,
+    int numberOfLoads, int indirectionIndex
+  ) {
+    exists(Instruction memory, Instruction begin, int upper, int ind |
+      isChiAfterIteratorDef(memory, iteratorDerefAddress, value, numberOfLoads) and
+      memorySucc*(begin, memory) and
+      isChiAfterBegin(container, begin) and
+      upper = countIndirectionsForCppType(getResultLanguageType(container)) and
+      ind = numberOfLoads + [1 .. upper] and
+      indirectionIndex = ind - (numberOfLoads + 1)
+    )
+  }
+
+  /**
+   * Holds if `iteratorAddress` is an address of an iterator that is used for a
+   * read operation to read a value from a container that created the iterator.
+   * `container` represents the base of the address of the container that was used
+   * to create the iterator.
+   */
+  cached
+  predicate isIteratorUse(
+    BaseSourceVariableInstruction container, Operand iteratorAddress, int numberOfLoads,
+    int indirectionIndex
+  ) {
+    // Direct use
+    exists(Instruction begin, Instruction memory, int upper, int ind |
+      isChiBeforeIteratorUse(iteratorAddress, memory, numberOfLoads) and
+      memorySucc*(begin, memory) and
+      isChiAfterBegin(container, begin) and
+      upper = countIndirectionsForCppType(getResultLanguageType(container)) and
+      ind = numberOfLoads + [1 .. upper] and
+      indirectionIndex = ind - (numberOfLoads + 1)
+    )
+    or
+    // Use through function output
+    exists(Instruction memory, Instruction begin, int upper, int ind |
+      isChiAfterIteratorArgument(memory, iteratorAddress, numberOfLoads) and
+      memorySucc*(begin, memory) and
+      isChiAfterBegin(container, begin) and
+      upper = countIndirectionsForCppType(getResultLanguageType(container)) and
+      ind = numberOfLoads + [1 .. upper] and
+      indirectionIndex = ind - (numberOfLoads - 1)
+    )
+  }
+
   /** Holds if `op` is the only use of its defining instruction, and that op is used in a conversation */
   private predicate isConversion(Operand op) {
     exists(Instruction def, Operand use |

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/IRVariable.qll

@@ -17,11 +17,18 @@ private import Imports::IRType
  * The variable may be a user-declared variable (`IRUserVariable`) or a temporary variable generated
  * by the AST-to-IR translation (`IRTempVariable`).
  */
-abstract private class AbstractIRVariable extends TIRVariable {
+class IRVariable extends TIRVariable {
   Language::Declaration func;
 
+  IRVariable() {
+    this = TIRUserVariable(_, _, func) or
+    this = TIRTempVariable(func, _, _, _) or
+    this = TIRStringLiteral(func, _, _, _) or
+    this = TIRDynamicInitializationFlag(func, _, _)
+  }
+
   /** Gets a textual representation of this element. */
-  abstract string toString();
+  string toString() { none() }
 
   /**
    * Holds if this variable's value cannot be changed within a function. Currently used for string
@@ -42,13 +49,13 @@ abstract private class AbstractIRVariable extends TIRVariable {
   /**
    * Gets the type of the variable.
    */
-  abstract Language::LanguageType getLanguageType();
+  Language::LanguageType getLanguageType() { none() }
 
   /**
    * Gets the AST node that declared this variable, or that introduced this
    * variable as part of the AST-to-IR translation.
    */
-  abstract Language::AST getAst();
+  Language::AST getAst() { none() }
 
   /** DEPRECATED: Alias for getAst */
   deprecated Language::AST getAST() { result = this.getAst() }
@@ -57,7 +64,7 @@ abstract private class AbstractIRVariable extends TIRVariable {
    * Gets an identifier string for the variable. This identifier is unique
    * within the function.
    */
-  abstract string getUniqueId();
+  string getUniqueId() { none() }
 
   /**
    * Gets the source location of this variable.
@@ -67,7 +74,7 @@ abstract private class AbstractIRVariable extends TIRVariable {
   /**
    * Gets the IR for the function that references this variable.
    */
-  final IRFunction getEnclosingIRFunction() { result.getFunction() = this.getEnclosingFunction() }
+  final IRFunction getEnclosingIRFunction() { result.getFunction() = func }
 
   /**
    * Gets the function that references this variable.
@@ -75,18 +82,10 @@ abstract private class AbstractIRVariable extends TIRVariable {
   final Language::Declaration getEnclosingFunction() { result = func }
 }
 
-/**
- * A variable referenced by the IR for a function.
- *
- * The variable may be a user-declared variable (`IRUserVariable`) or a temporary variable generated
- * by the AST-to-IR translation (`IRTempVariable`).
- */
-final class IRVariable = AbstractIRVariable;
-
 /**
  * A user-declared variable referenced by the IR for a function.
  */
-class IRUserVariable extends AbstractIRVariable, TIRUserVariable {
+class IRUserVariable extends IRVariable, TIRUserVariable {
   Language::Variable var;
   Language::LanguageType type;
 
@@ -115,29 +114,26 @@ class IRUserVariable extends AbstractIRVariable, TIRUserVariable {
  * A variable (user-declared or temporary) that is allocated on the stack. This includes all
  * parameters, non-static local variables, and temporary variables.
  */
-abstract private class AbstractIRAutomaticVariable extends AbstractIRVariable { }
-
-/**
- * A variable (user-declared or temporary) that is allocated on the stack. This includes all
- * parameters, non-static local variables, and temporary variables.
- */
-final class IRAutomaticVariable = AbstractIRAutomaticVariable;
-
-/**
- * A user-declared variable that is allocated on the stack. This includes all parameters and
- * non-static local variables.
- */
-private class AbstractIRAutomaticUserVariable extends IRUserVariable, AbstractIRAutomaticVariable {
-  override Language::AutomaticVariable var;
-
-  final override Language::AutomaticVariable getVariable() { result = var }
+class IRAutomaticVariable extends IRVariable {
+  IRAutomaticVariable() {
+    exists(Language::Variable var |
+      this = TIRUserVariable(var, _, func) and
+      Language::isVariableAutomatic(var)
+    )
+    or
+    this = TIRTempVariable(func, _, _, _)
+  }
 }
 
 /**
  * A user-declared variable that is allocated on the stack. This includes all parameters and
  * non-static local variables.
  */
-final class IRAutomaticUserVariable = AbstractIRAutomaticUserVariable;
+class IRAutomaticUserVariable extends IRUserVariable, IRAutomaticVariable {
+  override Language::AutomaticVariable var;
+
+  final override Language::AutomaticVariable getVariable() { result = var }
+}
 
 /**
  * A user-declared variable that is not allocated on the stack. This includes all global variables,
@@ -155,10 +151,16 @@ class IRStaticUserVariable extends IRUserVariable {
  * A variable that is not user-declared. This includes temporary variables generated as part of IR
  * construction, as well as string literals.
  */
-abstract private class AbstractIRGeneratedVariable extends AbstractIRVariable {
+class IRGeneratedVariable extends IRVariable {
   Language::AST ast;
   Language::LanguageType type;
 
+  IRGeneratedVariable() {
+    this = TIRTempVariable(func, ast, _, type) or
+    this = TIRStringLiteral(func, ast, type, _) or
+    this = TIRDynamicInitializationFlag(func, ast, type)
+  }
+
   final override Language::LanguageType getLanguageType() { result = type }
 
   final override Language::AST getAst() { result = ast }
@@ -194,20 +196,12 @@ abstract private class AbstractIRGeneratedVariable extends AbstractIRVariable {
   string getBaseString() { none() }
 }
 
-/**
- * A variable that is not user-declared. This includes temporary variables generated as part of IR
- * construction, as well as string literals.
- */
-final class IRGeneratedVariable = AbstractIRGeneratedVariable;
-
 /**
  * A temporary variable introduced by IR construction. The most common examples are the variable
  * generated to hold the return value of a function, or the variable generated to hold the result of
  * a condition operator (`a ? b : c`).
  */
-class IRTempVariable extends AbstractIRGeneratedVariable, AbstractIRAutomaticVariable,
-  TIRTempVariable
-{
+class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable {
   TempVariableTag tag;
 
   IRTempVariable() { this = TIRTempVariable(func, ast, tag, type) }
@@ -247,7 +241,7 @@ class IRThrowVariable extends IRTempVariable {
  * A temporary variable generated to hold the contents of all arguments passed to the `...` of a
  * function that accepts a variable number of arguments.
  */
-class IREllipsisVariable extends IRTempVariable, AbstractIRParameter {
+class IREllipsisVariable extends IRTempVariable, IRParameter {
   IREllipsisVariable() { tag = EllipsisTempVar() }
 
   final override string toString() { result = "#ellipsis" }
@@ -258,7 +252,7 @@ class IREllipsisVariable extends IRTempVariable, AbstractIRParameter {
 /**
  * A temporary variable generated to hold the `this` pointer.
  */
-class IRThisVariable extends IRTempVariable, AbstractIRParameter {
+class IRThisVariable extends IRTempVariable, IRParameter {
   IRThisVariable() { tag = ThisTempVar() }
 
   final override string toString() { result = "#this" }
@@ -270,7 +264,7 @@ class IRThisVariable extends IRTempVariable, AbstractIRParameter {
  * A variable generated to represent the contents of a string literal. This variable acts much like
  * a read-only global variable.
  */
-class IRStringLiteral extends AbstractIRGeneratedVariable, TIRStringLiteral {
+class IRStringLiteral extends IRGeneratedVariable, TIRStringLiteral {
   Language::StringLiteral literal;
 
   IRStringLiteral() { this = TIRStringLiteral(func, ast, type, literal) }
@@ -294,7 +288,7 @@ class IRStringLiteral extends AbstractIRGeneratedVariable, TIRStringLiteral {
  * used to model the runtime initialization of static local variables in C++, as well as static
  * fields in C#.
  */
-class IRDynamicInitializationFlag extends AbstractIRGeneratedVariable, TIRDynamicInitializationFlag {
+class IRDynamicInitializationFlag extends IRGeneratedVariable, TIRDynamicInitializationFlag {
   Language::Variable var;
 
   IRDynamicInitializationFlag() {
@@ -320,24 +314,24 @@ class IRDynamicInitializationFlag extends AbstractIRGeneratedVariable, TIRDynami
  * An IR variable which acts like a function parameter, including positional parameters and the
  * temporary variables generated for `this` and ellipsis parameters.
  */
-abstract private class AbstractIRParameter extends AbstractIRAutomaticVariable {
+class IRParameter extends IRAutomaticVariable {
+  IRParameter() {
+    this.(IRAutomaticUserVariable).getVariable() instanceof Language::Parameter
+    or
+    this = TIRTempVariable(_, _, ThisTempVar(), _)
+    or
+    this = TIRTempVariable(_, _, EllipsisTempVar(), _)
+  }
+
   /**
    * Gets the zero-based index of this parameter. The `this` parameter has index -1.
    */
   int getIndex() { none() }
 }
 
-/**
- * An IR variable which acts like a function parameter, including positional parameters and the
- * temporary variables generated for `this` and ellipsis parameters.
- */
-final class IRParameter = AbstractIRParameter;
-
 /**
  * An IR variable representing a positional parameter.
  */
-class IRPositionalParameter extends AbstractIRParameter, AbstractIRAutomaticUserVariable {
-  IRPositionalParameter() { this.getVariable() instanceof Language::Parameter }
-
+class IRPositionalParameter extends IRParameter, IRAutomaticUserVariable {
   final override int getIndex() { result = this.getVariable().(Language::Parameter).getIndex() }
 }

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll

@@ -247,7 +247,8 @@ class Instruction extends Construction::TStageInstruction {
    * Gets the type of the result produced by this instruction. If the instruction does not produce
    * a result, its result type will be `IRVoidType`.
    */
-  final IRType getResultIRType() { result = Construction::getInstructionResultIRType(this) }
+  cached
+  final IRType getResultIRType() { result = this.getResultLanguageType().getIRType() }
 
   /**
    * Gets the type of the result produced by this instruction. If the
@@ -994,8 +995,9 @@ class ConstantInstruction extends ConstantValueInstruction {
  */
 class IntegerConstantInstruction extends ConstantInstruction {
   IntegerConstantInstruction() {
-    exists(IRType resultType | resultType = this.getResultIRType() |
-      resultType instanceof IRIntegerType or resultType instanceof IRBooleanType
+    exists(IRType resultType |
+      resultType = this.getResultIRType() and
+      (resultType instanceof IRIntegerType or resultType instanceof IRBooleanType)
     )
   }
 }
@@ -1007,17 +1009,6 @@ class FloatConstantInstruction extends ConstantInstruction {
   FloatConstantInstruction() { this.getResultIRType() instanceof IRFloatingPointType }
 }
 
-/**
- * An instruction whose result is a constant value of a pointer type.
- */
-class PointerConstantInstruction extends ConstantInstruction {
-  PointerConstantInstruction() {
-    exists(IRType resultType | resultType = this.getResultIRType() |
-      resultType instanceof IRAddressType or resultType instanceof IRFunctionAddressType
-    )
-  }
-}
-
 /**
  * An instruction whose result is the address of a string literal.
  */

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll

@@ -429,11 +429,6 @@ private module Cached {
     instr = unreachedInstruction(_) and result = Language::getVoidType()
   }
 
-  cached
-  IRType getInstructionResultIRType(Instruction instr) {
-    result = instr.getResultLanguageType().getIRType()
-  }
-
   /**
    * Holds if `opcode` is the opcode that specifies the operation performed by `instr`.
    *

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/IRVariable.qll

@@ -17,11 +17,18 @@ private import Imports::IRType
  * The variable may be a user-declared variable (`IRUserVariable`) or a temporary variable generated
  * by the AST-to-IR translation (`IRTempVariable`).
  */
-abstract private class AbstractIRVariable extends TIRVariable {
+class IRVariable extends TIRVariable {
   Language::Declaration func;
 
+  IRVariable() {
+    this = TIRUserVariable(_, _, func) or
+    this = TIRTempVariable(func, _, _, _) or
+    this = TIRStringLiteral(func, _, _, _) or
+    this = TIRDynamicInitializationFlag(func, _, _)
+  }
+
   /** Gets a textual representation of this element. */
-  abstract string toString();
+  string toString() { none() }
 
   /**
    * Holds if this variable's value cannot be changed within a function. Currently used for string
@@ -42,13 +49,13 @@ abstract private class AbstractIRVariable extends TIRVariable {
   /**
    * Gets the type of the variable.
    */
-  abstract Language::LanguageType getLanguageType();
+  Language::LanguageType getLanguageType() { none() }
 
   /**
    * Gets the AST node that declared this variable, or that introduced this
    * variable as part of the AST-to-IR translation.
    */
-  abstract Language::AST getAst();
+  Language::AST getAst() { none() }
 
   /** DEPRECATED: Alias for getAst */
   deprecated Language::AST getAST() { result = this.getAst() }
@@ -57,7 +64,7 @@ abstract private class AbstractIRVariable extends TIRVariable {
    * Gets an identifier string for the variable. This identifier is unique
    * within the function.
    */
-  abstract string getUniqueId();
+  string getUniqueId() { none() }
 
   /**
    * Gets the source location of this variable.
@@ -67,7 +74,7 @@ abstract private class AbstractIRVariable extends TIRVariable {
   /**
    * Gets the IR for the function that references this variable.
    */
-  final IRFunction getEnclosingIRFunction() { result.getFunction() = this.getEnclosingFunction() }
+  final IRFunction getEnclosingIRFunction() { result.getFunction() = func }
 
   /**
    * Gets the function that references this variable.
@@ -75,18 +82,10 @@ abstract private class AbstractIRVariable extends TIRVariable {
   final Language::Declaration getEnclosingFunction() { result = func }
 }
 
-/**
- * A variable referenced by the IR for a function.
- *
- * The variable may be a user-declared variable (`IRUserVariable`) or a temporary variable generated
- * by the AST-to-IR translation (`IRTempVariable`).
- */
-final class IRVariable = AbstractIRVariable;
-
 /**
  * A user-declared variable referenced by the IR for a function.
  */
-class IRUserVariable extends AbstractIRVariable, TIRUserVariable {
+class IRUserVariable extends IRVariable, TIRUserVariable {
   Language::Variable var;
   Language::LanguageType type;
 
@@ -115,29 +114,26 @@ class IRUserVariable extends AbstractIRVariable, TIRUserVariable {
  * A variable (user-declared or temporary) that is allocated on the stack. This includes all
  * parameters, non-static local variables, and temporary variables.
  */
-abstract private class AbstractIRAutomaticVariable extends AbstractIRVariable { }
-
-/**
- * A variable (user-declared or temporary) that is allocated on the stack. This includes all
- * parameters, non-static local variables, and temporary variables.
- */
-final class IRAutomaticVariable = AbstractIRAutomaticVariable;
-
-/**
- * A user-declared variable that is allocated on the stack. This includes all parameters and
- * non-static local variables.
- */
-private class AbstractIRAutomaticUserVariable extends IRUserVariable, AbstractIRAutomaticVariable {
-  override Language::AutomaticVariable var;
-
-  final override Language::AutomaticVariable getVariable() { result = var }
+class IRAutomaticVariable extends IRVariable {
+  IRAutomaticVariable() {
+    exists(Language::Variable var |
+      this = TIRUserVariable(var, _, func) and
+      Language::isVariableAutomatic(var)
+    )
+    or
+    this = TIRTempVariable(func, _, _, _)
+  }
 }
 
 /**
  * A user-declared variable that is allocated on the stack. This includes all parameters and
  * non-static local variables.
  */
-final class IRAutomaticUserVariable = AbstractIRAutomaticUserVariable;
+class IRAutomaticUserVariable extends IRUserVariable, IRAutomaticVariable {
+  override Language::AutomaticVariable var;
+
+  final override Language::AutomaticVariable getVariable() { result = var }
+}
 
 /**
  * A user-declared variable that is not allocated on the stack. This includes all global variables,
@@ -155,10 +151,16 @@ class IRStaticUserVariable extends IRUserVariable {
  * A variable that is not user-declared. This includes temporary variables generated as part of IR
  * construction, as well as string literals.
  */
-abstract private class AbstractIRGeneratedVariable extends AbstractIRVariable {
+class IRGeneratedVariable extends IRVariable {
   Language::AST ast;
   Language::LanguageType type;
 
+  IRGeneratedVariable() {
+    this = TIRTempVariable(func, ast, _, type) or
+    this = TIRStringLiteral(func, ast, type, _) or
+    this = TIRDynamicInitializationFlag(func, ast, type)
+  }
+
   final override Language::LanguageType getLanguageType() { result = type }
 
   final override Language::AST getAst() { result = ast }
@@ -194,20 +196,12 @@ abstract private class AbstractIRGeneratedVariable extends AbstractIRVariable {
   string getBaseString() { none() }
 }
 
-/**
- * A variable that is not user-declared. This includes temporary variables generated as part of IR
- * construction, as well as string literals.
- */
-final class IRGeneratedVariable = AbstractIRGeneratedVariable;
-
 /**
  * A temporary variable introduced by IR construction. The most common examples are the variable
  * generated to hold the return value of a function, or the variable generated to hold the result of
  * a condition operator (`a ? b : c`).
  */
-class IRTempVariable extends AbstractIRGeneratedVariable, AbstractIRAutomaticVariable,
-  TIRTempVariable
-{
+class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable {
   TempVariableTag tag;
 
   IRTempVariable() { this = TIRTempVariable(func, ast, tag, type) }
@@ -247,7 +241,7 @@ class IRThrowVariable extends IRTempVariable {
  * A temporary variable generated to hold the contents of all arguments passed to the `...` of a
  * function that accepts a variable number of arguments.
  */
-class IREllipsisVariable extends IRTempVariable, AbstractIRParameter {
+class IREllipsisVariable extends IRTempVariable, IRParameter {
   IREllipsisVariable() { tag = EllipsisTempVar() }
 
   final override string toString() { result = "#ellipsis" }
@@ -258,7 +252,7 @@ class IREllipsisVariable extends IRTempVariable, AbstractIRParameter {
 /**
  * A temporary variable generated to hold the `this` pointer.
  */
-class IRThisVariable extends IRTempVariable, AbstractIRParameter {
+class IRThisVariable extends IRTempVariable, IRParameter {
   IRThisVariable() { tag = ThisTempVar() }
 
   final override string toString() { result = "#this" }
@@ -270,7 +264,7 @@ class IRThisVariable extends IRTempVariable, AbstractIRParameter {
  * A variable generated to represent the contents of a string literal. This variable acts much like
  * a read-only global variable.
  */
-class IRStringLiteral extends AbstractIRGeneratedVariable, TIRStringLiteral {
+class IRStringLiteral extends IRGeneratedVariable, TIRStringLiteral {
   Language::StringLiteral literal;
 
   IRStringLiteral() { this = TIRStringLiteral(func, ast, type, literal) }
@@ -294,7 +288,7 @@ class IRStringLiteral extends AbstractIRGeneratedVariable, TIRStringLiteral {
  * used to model the runtime initialization of static local variables in C++, as well as static
  * fields in C#.
  */
-class IRDynamicInitializationFlag extends AbstractIRGeneratedVariable, TIRDynamicInitializationFlag {
+class IRDynamicInitializationFlag extends IRGeneratedVariable, TIRDynamicInitializationFlag {
   Language::Variable var;
 
   IRDynamicInitializationFlag() {
@@ -320,24 +314,24 @@ class IRDynamicInitializationFlag extends AbstractIRGeneratedVariable, TIRDynami
  * An IR variable which acts like a function parameter, including positional parameters and the
  * temporary variables generated for `this` and ellipsis parameters.
  */
-abstract private class AbstractIRParameter extends AbstractIRAutomaticVariable {
+class IRParameter extends IRAutomaticVariable {
+  IRParameter() {
+    this.(IRAutomaticUserVariable).getVariable() instanceof Language::Parameter
+    or
+    this = TIRTempVariable(_, _, ThisTempVar(), _)
+    or
+    this = TIRTempVariable(_, _, EllipsisTempVar(), _)
+  }
+
   /**
    * Gets the zero-based index of this parameter. The `this` parameter has index -1.
    */
   int getIndex() { none() }
 }
 
-/**
- * An IR variable which acts like a function parameter, including positional parameters and the
- * temporary variables generated for `this` and ellipsis parameters.
- */
-final class IRParameter = AbstractIRParameter;
-
 /**
  * An IR variable representing a positional parameter.
  */
-class IRPositionalParameter extends AbstractIRParameter, AbstractIRAutomaticUserVariable {
-  IRPositionalParameter() { this.getVariable() instanceof Language::Parameter }
-
+class IRPositionalParameter extends IRParameter, IRAutomaticUserVariable {
   final override int getIndex() { result = this.getVariable().(Language::Parameter).getIndex() }
 }

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/Instruction.qll

@@ -247,7 +247,8 @@ class Instruction extends Construction::TStageInstruction {
    * Gets the type of the result produced by this instruction. If the instruction does not produce
    * a result, its result type will be `IRVoidType`.
    */
-  final IRType getResultIRType() { result = Construction::getInstructionResultIRType(this) }
+  cached
+  final IRType getResultIRType() { result = this.getResultLanguageType().getIRType() }
 
   /**
    * Gets the type of the result produced by this instruction. If the
@@ -994,8 +995,9 @@ class ConstantInstruction extends ConstantValueInstruction {
  */
 class IntegerConstantInstruction extends ConstantInstruction {
   IntegerConstantInstruction() {
-    exists(IRType resultType | resultType = this.getResultIRType() |
-      resultType instanceof IRIntegerType or resultType instanceof IRBooleanType
+    exists(IRType resultType |
+      resultType = this.getResultIRType() and
+      (resultType instanceof IRIntegerType or resultType instanceof IRBooleanType)
     )
   }
 }
@@ -1007,17 +1009,6 @@ class FloatConstantInstruction extends ConstantInstruction {
   FloatConstantInstruction() { this.getResultIRType() instanceof IRFloatingPointType }
 }
 
-/**
- * An instruction whose result is a constant value of a pointer type.
- */
-class PointerConstantInstruction extends ConstantInstruction {
-  PointerConstantInstruction() {
-    exists(IRType resultType | resultType = this.getResultIRType() |
-      resultType instanceof IRAddressType or resultType instanceof IRFunctionAddressType
-    )
-  }
-}
-
 /**
  * An instruction whose result is the address of a string literal.
  */

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

@@ -1,3 +1,3 @@
-import semmle.code.cpp.ir.implementation.raw.IR
+import semmle.code.cpp.ir.implementation.aliased_ssa.IR
 import semmle.code.cpp.ir.internal.Overlap
 import semmle.code.cpp.ir.internal.IRCppLanguage as Language

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

@@ -377,10 +377,6 @@ CppType getInstructionResultType(TStageInstruction instr) {
   result = getVoidType()
 }
 
-IRType getInstructionResultIRType(Instruction instr) {
-  result = instr.getResultLanguageType().getIRType()
-}
-
 predicate getInstructionOpcode(Opcode opcode, TStageInstruction instr) {
   getInstructionTranslatedElement(instr).hasInstruction(opcode, getInstructionTag(instr), _)
   or

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

@@ -830,12 +830,6 @@ newtype TTranslatedElement =
       not ignoreExpr(dc)
     )
   } or
-  // The set of destructors to invoke after a handler for a `try` statement. These
-  // need to be special cased because the destructors need to run following an
-  // `ExceptionEdge`, but not following a `GotoEdge` edge.
-  TTranslatedDestructorsAfterHandler(Handler handler) {
-    exists(handler.getAnImplicitDestructorCall())
-  } or
   // A precise side effect of an argument to a `Call`
   TTranslatedArgumentExprSideEffect(Call call, Expr expr, int n, SideEffectOpcode opcode) {
     not ignoreExpr(expr) and

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

@@ -538,11 +538,6 @@ class TranslatedResultCopy extends TranslatedExpr, TTranslatedResultCopy {
   final override predicate producesExprResult() { any() }
 
   private TranslatedCoreExpr getOperand() { result.getExpr() = expr }
-
-  override predicate handlesDestructorsExplicitly() {
-    // The destructor calls will already have been generated by the translation of `expr`.
-    any()
-  }
 }
 
 class TranslatedCommaExpr extends TranslatedNonConstantExpr {
@@ -1844,6 +1839,9 @@ class TranslatedAssignExpr extends TranslatedNonConstantExpr {
     child = this.getRightOperand() and
     result = this.getLeftOperand().getFirstInstruction(kind)
     or
+    child = this.getRightOperand() and
+    result = this.getLeftOperand().getFirstInstruction(kind)
+    or
     kind instanceof GotoEdge and
     child = this.getLeftOperand() and
     result = this.getInstruction(AssignmentStoreTag())
@@ -3208,20 +3206,9 @@ class TranslatedBuiltInOperation extends TranslatedNonConstantExpr {
 
   final override Instruction getResult() { result = this.getInstruction(OnlyInstructionTag()) }
 
-  /**
-   * Gets the rnk'th (0-indexed) child for which a `TranslatedElement` exists.
-   *
-   * We use this predicate to filter out `TypeName` expressions that sometimes
-   * occur in builtin operations since the IR doesn't have an instruction to
-   * represent a reference to a type.
-   */
-  private TranslatedElement getRankedChild(int rnk) {
-    result = rank[rnk + 1](int id, TranslatedElement te | te = this.getChild(id) | te order by id)
-  }
-
   final override Instruction getFirstInstruction(EdgeKind kind) {
-    if exists(this.getRankedChild(0))
-    then result = this.getRankedChild(0).getFirstInstruction(kind)
+    if exists(this.getChild(0))
+    then result = this.getChild(0).getFirstInstruction(kind)
     else (
       kind instanceof GotoEdge and result = this.getInstruction(OnlyInstructionTag())
     )
@@ -3241,11 +3228,11 @@ class TranslatedBuiltInOperation extends TranslatedNonConstantExpr {
   }
 
   final override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
-    exists(int id | child = this.getRankedChild(id) |
-      result = this.getRankedChild(id + 1).getFirstInstruction(kind)
+    exists(int id | child = this.getChild(id) |
+      result = this.getChild(id + 1).getFirstInstruction(kind)
       or
       kind instanceof GotoEdge and
-      not exists(this.getRankedChild(id + 1)) and
+      not exists(this.getChild(id + 1)) and
       result = this.getInstruction(OnlyInstructionTag())
     )
   }
@@ -3260,7 +3247,7 @@ class TranslatedBuiltInOperation extends TranslatedNonConstantExpr {
     tag = OnlyInstructionTag() and
     exists(int index |
       operandTag = positionalArgumentOperand(index) and
-      result = this.getRankedChild(index).(TranslatedExpr).getResult()
+      result = this.getChild(index).(TranslatedExpr).getResult()
     )
   }
 

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

@@ -777,72 +777,6 @@ abstract class TranslatedHandler extends TranslatedStmt {
   TranslatedStmt getBlock() { result = getTranslatedStmt(stmt.getBlock()) }
 }
 
-/**
- * The IR translation of the destructor calls of the parent `TranslatedCatchByTypeHandler`.
- *
- * This object does not itself generate the destructor calls. Instead, its
- * children provide the actual calls.
- */
-class TranslatedDestructorsAfterHandler extends TranslatedElement,
-  TTranslatedDestructorsAfterHandler
-{
-  Handler handler;
-
-  TranslatedDestructorsAfterHandler() { this = TTranslatedDestructorsAfterHandler(handler) }
-
-  override string toString() { result = "Destructor calls after handler: " + handler }
-
-  private TranslatedCall getTranslatedImplicitDestructorCall(int id) {
-    result.getExpr() = handler.getImplicitDestructorCall(id)
-  }
-
-  override Instruction getFirstInstruction(EdgeKind kind) {
-    result = this.getChild(0).getFirstInstruction(kind)
-  }
-
-  override Handler getAst() { result = handler }
-
-  override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) { none() }
-
-  override TranslatedElement getChild(int id) {
-    result = this.getTranslatedImplicitDestructorCall(id)
-  }
-
-  override predicate handlesDestructorsExplicitly() { any() }
-
-  override Declaration getFunction() { result = handler.getEnclosingFunction() }
-
-  override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
-    exists(int id | child = this.getChild(id) |
-      // Transition to the next child, if any.
-      result = this.getChild(id + 1).getFirstInstruction(kind)
-      or
-      // And otherwise go to the next handler, if any.
-      not exists(this.getChild(id + 1)) and
-      result =
-        getTranslatedStmt(handler)
-            .getParent()
-            .(TranslatedTryStmt)
-            .getNextHandler(getTranslatedStmt(handler), kind)
-    )
-  }
-
-  override TranslatedElement getLastChild() {
-    result =
-      this.getTranslatedImplicitDestructorCall(max(int id |
-          exists(handler.getImplicitDestructorCall(id))
-        ))
-  }
-
-  override Instruction getALastInstructionInternal() {
-    result = this.getLastChild().getALastInstruction()
-  }
-
-  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
-    none()
-  }
-}
-
 /**
  * The IR translation of a C++ `catch` block that catches an exception with a
  * specific type (e.g. `catch (const std::exception&)`).
@@ -856,14 +790,10 @@ class TranslatedCatchByTypeHandler extends TranslatedHandler {
     resultType = getVoidType()
   }
 
-  override predicate handlesDestructorsExplicitly() { any() }
-
   override TranslatedElement getChildInternal(int id) {
     result = super.getChildInternal(id)
     or
     id = 0 and result = this.getParameter()
-    or
-    id = 1 and result = this.getDestructors()
   }
 
   override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
@@ -880,9 +810,7 @@ class TranslatedCatchByTypeHandler extends TranslatedHandler {
       result = this.getParameter().getFirstInstruction(kind)
       or
       kind instanceof ExceptionEdge and
-      if exists(this.getDestructors())
-      then result = this.getDestructors().getFirstInstruction(any(GotoEdge edge))
-      else result = this.getParent().(TranslatedTryStmt).getNextHandler(this, any(GotoEdge edge))
+      result = this.getParent().(TranslatedTryStmt).getNextHandler(this, any(GotoEdge edge))
     )
   }
 
@@ -894,8 +822,6 @@ class TranslatedCatchByTypeHandler extends TranslatedHandler {
   private TranslatedParameter getParameter() {
     result = getTranslatedParameter(stmt.getParameter())
   }
-
-  private TranslatedDestructorsAfterHandler getDestructors() { result.getAst() = stmt }
 }
 
 /**
@@ -916,7 +842,9 @@ class TranslatedCatchAnyHandler extends TranslatedHandler {
   }
 }
 
-abstract class TranslatedIfLikeStmt extends TranslatedStmt, ConditionContext {
+class TranslatedIfStmt extends TranslatedStmt, ConditionContext {
+  override IfStmt stmt;
+
   override Instruction getFirstInstruction(EdgeKind kind) {
     if this.hasInitialization()
     then result = this.getInitialization().getFirstInstruction(kind)
@@ -929,8 +857,6 @@ abstract class TranslatedIfLikeStmt extends TranslatedStmt, ConditionContext {
 
   override TranslatedElement getLastChild() { result = this.getElse() or result = this.getThen() }
 
-  override predicate handlesDestructorsExplicitly() { any() }
-
   override TranslatedElement getChildInternal(int id) {
     id = 0 and result = this.getInitialization()
     or
@@ -941,21 +867,25 @@ abstract class TranslatedIfLikeStmt extends TranslatedStmt, ConditionContext {
     id = 3 and result = this.getElse()
   }
 
-  abstract predicate hasInitialization();
+  private predicate hasInitialization() { exists(stmt.getInitialization()) }
 
-  abstract TranslatedStmt getInitialization();
+  private TranslatedStmt getInitialization() {
+    result = getTranslatedStmt(stmt.getInitialization())
+  }
 
-  abstract TranslatedCondition getCondition();
+  private TranslatedCondition getCondition() {
+    result = getTranslatedCondition(stmt.getCondition().getFullyConverted())
+  }
 
   private Instruction getFirstConditionInstruction(EdgeKind kind) {
     result = this.getCondition().getFirstInstruction(kind)
   }
 
-  abstract TranslatedStmt getThen();
+  private TranslatedStmt getThen() { result = getTranslatedStmt(stmt.getThen()) }
 
-  abstract TranslatedStmt getElse();
+  private TranslatedStmt getElse() { result = getTranslatedStmt(stmt.getElse()) }
 
-  abstract predicate hasElse();
+  private predicate hasElse() { exists(stmt.getElse()) }
 
   override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) { none() }
 
@@ -968,11 +898,7 @@ abstract class TranslatedIfLikeStmt extends TranslatedStmt, ConditionContext {
     child = this.getCondition() and
     if this.hasElse()
     then result = this.getElse().getFirstInstruction(kind)
-    else (
-      if this.hasAnImplicitDestructorCall()
-      then result = this.getChild(this.getFirstDestructorCallIndex()).getFirstInstruction(kind)
-      else result = this.getParent().getChildSuccessor(this, kind)
-    )
+    else result = this.getParent().getChildSuccessor(this, kind)
   }
 
   override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
@@ -980,24 +906,7 @@ abstract class TranslatedIfLikeStmt extends TranslatedStmt, ConditionContext {
     result = this.getFirstConditionInstruction(kind)
     or
     (child = this.getThen() or child = this.getElse()) and
-    (
-      if this.hasAnImplicitDestructorCall()
-      then result = this.getChild(this.getFirstDestructorCallIndex()).getFirstInstruction(kind)
-      else result = this.getParent().getChildSuccessor(this, kind)
-    )
-    or
-    exists(int destructorId |
-      destructorId >= this.getFirstDestructorCallIndex() and
-      child = this.getChild(destructorId) and
-      result = this.getChild(destructorId + 1).getFirstInstruction(kind)
-    )
-    or
-    exists(int lastDestructorIndex |
-      lastDestructorIndex =
-        max(int n | exists(this.getChild(n)) and n >= this.getFirstDestructorCallIndex()) and
-      child = this.getChild(lastDestructorIndex) and
-      result = this.getParent().getChildSuccessor(this, kind)
-    )
+    result = this.getParent().getChildSuccessor(this, kind)
   }
 
   override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
@@ -1005,44 +914,76 @@ abstract class TranslatedIfLikeStmt extends TranslatedStmt, ConditionContext {
   }
 }
 
-class TranslatedIfStmt extends TranslatedIfLikeStmt {
-  override IfStmt stmt;
-
-  override predicate hasInitialization() { exists(stmt.getInitialization()) }
-
-  override TranslatedStmt getInitialization() {
-    result = getTranslatedStmt(stmt.getInitialization())
-  }
-
-  override TranslatedCondition getCondition() {
-    result = getTranslatedCondition(stmt.getCondition().getFullyConverted())
-  }
-
-  override TranslatedStmt getThen() { result = getTranslatedStmt(stmt.getThen()) }
-
-  override TranslatedStmt getElse() { result = getTranslatedStmt(stmt.getElse()) }
-
-  override predicate hasElse() { exists(stmt.getElse()) }
-}
-
-class TranslatedConstExprIfStmt extends TranslatedIfLikeStmt {
+class TranslatedConstExprIfStmt extends TranslatedStmt, ConditionContext {
   override ConstexprIfStmt stmt;
 
-  override predicate hasInitialization() { exists(stmt.getInitialization()) }
+  override Instruction getFirstInstruction(EdgeKind kind) {
+    if this.hasInitialization()
+    then result = this.getInitialization().getFirstInstruction(kind)
+    else result = this.getFirstConditionInstruction(kind)
+  }
 
-  override TranslatedStmt getInitialization() {
+  override TranslatedElement getChildInternal(int id) {
+    id = 0 and result = this.getInitialization()
+    or
+    id = 1 and result = this.getCondition()
+    or
+    id = 2 and result = this.getThen()
+    or
+    id = 3 and result = this.getElse()
+  }
+
+  private predicate hasInitialization() { exists(stmt.getInitialization()) }
+
+  private TranslatedStmt getInitialization() {
     result = getTranslatedStmt(stmt.getInitialization())
   }
 
-  override TranslatedCondition getCondition() {
+  private TranslatedCondition getCondition() {
     result = getTranslatedCondition(stmt.getCondition().getFullyConverted())
   }
 
-  override TranslatedStmt getThen() { result = getTranslatedStmt(stmt.getThen()) }
+  private Instruction getFirstConditionInstruction(EdgeKind kind) {
+    result = this.getCondition().getFirstInstruction(kind)
+  }
 
-  override TranslatedStmt getElse() { result = getTranslatedStmt(stmt.getElse()) }
+  private TranslatedStmt getThen() { result = getTranslatedStmt(stmt.getThen()) }
 
-  override predicate hasElse() { exists(stmt.getElse()) }
+  private TranslatedStmt getElse() { result = getTranslatedStmt(stmt.getElse()) }
+
+  private predicate hasElse() { exists(stmt.getElse()) }
+
+  override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) { none() }
+
+  override Instruction getChildTrueSuccessor(TranslatedCondition child, EdgeKind kind) {
+    child = this.getCondition() and
+    result = this.getThen().getFirstInstruction(kind)
+  }
+
+  override Instruction getChildFalseSuccessor(TranslatedCondition child, EdgeKind kind) {
+    child = this.getCondition() and
+    if this.hasElse()
+    then result = this.getElse().getFirstInstruction(kind)
+    else result = this.getParent().getChildSuccessor(this, kind)
+  }
+
+  override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
+    child = this.getInitialization() and
+    result = this.getFirstConditionInstruction(kind)
+    or
+    (child = this.getThen() or child = this.getElse()) and
+    result = this.getParent().getChildSuccessor(this, kind)
+  }
+
+  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
+    none()
+  }
+
+  override Instruction getALastInstructionInternal() {
+    result = this.getThen().getALastInstruction()
+    or
+    result = this.getElse().getALastInstruction()
+  }
 }
 
 abstract class TranslatedLoop extends TranslatedStmt, ConditionContext {
@@ -1222,8 +1163,6 @@ class TranslatedForStmt extends TranslatedLoop {
 class TranslatedRangeBasedForStmt extends TranslatedStmt, ConditionContext {
   override RangeBasedForStmt stmt;
 
-  override predicate handlesDestructorsExplicitly() { any() }
-
   override TranslatedElement getChildInternal(int id) {
     id = 0 and result = this.getInitialization()
     or
@@ -1277,19 +1216,6 @@ class TranslatedRangeBasedForStmt extends TranslatedStmt, ConditionContext {
     or
     child = this.getUpdate() and
     result = this.getCondition().getFirstInstruction(kind)
-    or
-    exists(int destructorId |
-      destructorId >= this.getFirstDestructorCallIndex() and
-      child = this.getChild(destructorId) and
-      result = this.getChild(destructorId + 1).getFirstInstruction(kind)
-    )
-    or
-    exists(int lastDestructorIndex |
-      lastDestructorIndex =
-        max(int n | exists(this.getChild(n)) and n >= this.getFirstDestructorCallIndex()) and
-      child = this.getChild(lastDestructorIndex) and
-      result = this.getParent().getChildSuccessor(this, kind)
-    )
   }
 
   override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
@@ -1305,9 +1231,7 @@ class TranslatedRangeBasedForStmt extends TranslatedStmt, ConditionContext {
 
   override Instruction getChildFalseSuccessor(TranslatedCondition child, EdgeKind kind) {
     child = this.getCondition() and
-    if this.hasAnImplicitDestructorCall()
-    then result = this.getChild(this.getFirstDestructorCallIndex()).getFirstInstruction(kind)
-    else result = this.getParent().getChildSuccessor(this, kind)
+    result = this.getParent().getChildSuccessor(this, kind)
   }
 
   private TranslatedDeclStmt getRangeVariableDeclStmt() {
@@ -1352,11 +1276,6 @@ class TranslatedJumpStmt extends TranslatedStmt {
   override JumpStmt stmt;
 
   override Instruction getFirstInstruction(EdgeKind kind) {
-    // The first instruction is a destructor call, if any.
-    result = this.getChildInternal(0).getFirstInstruction(kind)
-    or
-    // Otherwise, the first (and only) instruction is a `NoOp`
-    not exists(this.getChildInternal(0)) and
     result = this.getInstruction(OnlyInstructionTag()) and
     kind instanceof GotoEdge
   }
@@ -1365,20 +1284,7 @@ class TranslatedJumpStmt extends TranslatedStmt {
     result = this.getInstruction(OnlyInstructionTag())
   }
 
-  private TranslatedCall getTranslatedImplicitDestructorCall(int id) {
-    result.getExpr() = stmt.getImplicitDestructorCall(id)
-  }
-
-  override TranslatedElement getLastChild() {
-    result =
-      this.getTranslatedImplicitDestructorCall(max(int id |
-          exists(stmt.getImplicitDestructorCall(id))
-        ))
-  }
-
-  override TranslatedElement getChildInternal(int id) {
-    result = this.getTranslatedImplicitDestructorCall(id)
-  }
+  override TranslatedElement getChildInternal(int id) { none() }
 
   override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
     tag = OnlyInstructionTag() and
@@ -1391,19 +1297,7 @@ class TranslatedJumpStmt extends TranslatedStmt {
     result = getTranslatedStmt(stmt.getTarget()).getFirstInstruction(kind)
   }
 
-  final override predicate handlesDestructorsExplicitly() { any() }
-
-  override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
-    exists(int id | child = this.getChildInternal(id) |
-      // Transition to the next destructor call, if any.
-      result = this.getChildInternal(id + 1).getFirstInstruction(kind)
-      or
-      // And otherwise, exit this element by flowing to the target of the jump.
-      not exists(this.getChildInternal(id + 1)) and
-      kind instanceof GotoEdge and
-      result = this.getInstruction(OnlyInstructionTag())
-    )
-  }
+  override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) { none() }
 }
 
 private EdgeKind getCaseEdge(SwitchCase switchCase) {

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/IRVariable.qll

@@ -17,11 +17,18 @@ private import Imports::IRType
  * The variable may be a user-declared variable (`IRUserVariable`) or a temporary variable generated
  * by the AST-to-IR translation (`IRTempVariable`).
  */
-abstract private class AbstractIRVariable extends TIRVariable {
+class IRVariable extends TIRVariable {
   Language::Declaration func;
 
+  IRVariable() {
+    this = TIRUserVariable(_, _, func) or
+    this = TIRTempVariable(func, _, _, _) or
+    this = TIRStringLiteral(func, _, _, _) or
+    this = TIRDynamicInitializationFlag(func, _, _)
+  }
+
   /** Gets a textual representation of this element. */
-  abstract string toString();
+  string toString() { none() }
 
   /**
    * Holds if this variable's value cannot be changed within a function. Currently used for string
@@ -42,13 +49,13 @@ abstract private class AbstractIRVariable extends TIRVariable {
   /**
    * Gets the type of the variable.
    */
-  abstract Language::LanguageType getLanguageType();
+  Language::LanguageType getLanguageType() { none() }
 
   /**
    * Gets the AST node that declared this variable, or that introduced this
    * variable as part of the AST-to-IR translation.
    */
-  abstract Language::AST getAst();
+  Language::AST getAst() { none() }
 
   /** DEPRECATED: Alias for getAst */
   deprecated Language::AST getAST() { result = this.getAst() }
@@ -57,7 +64,7 @@ abstract private class AbstractIRVariable extends TIRVariable {
    * Gets an identifier string for the variable. This identifier is unique
    * within the function.
    */
-  abstract string getUniqueId();
+  string getUniqueId() { none() }
 
   /**
    * Gets the source location of this variable.
@@ -67,7 +74,7 @@ abstract private class AbstractIRVariable extends TIRVariable {
   /**
    * Gets the IR for the function that references this variable.
    */
-  final IRFunction getEnclosingIRFunction() { result.getFunction() = this.getEnclosingFunction() }
+  final IRFunction getEnclosingIRFunction() { result.getFunction() = func }
 
   /**
    * Gets the function that references this variable.
@@ -75,18 +82,10 @@ abstract private class AbstractIRVariable extends TIRVariable {
   final Language::Declaration getEnclosingFunction() { result = func }
 }
 
-/**
- * A variable referenced by the IR for a function.
- *
- * The variable may be a user-declared variable (`IRUserVariable`) or a temporary variable generated
- * by the AST-to-IR translation (`IRTempVariable`).
- */
-final class IRVariable = AbstractIRVariable;
-
 /**
  * A user-declared variable referenced by the IR for a function.
  */
-class IRUserVariable extends AbstractIRVariable, TIRUserVariable {
+class IRUserVariable extends IRVariable, TIRUserVariable {
   Language::Variable var;
   Language::LanguageType type;
 
@@ -115,29 +114,26 @@ class IRUserVariable extends AbstractIRVariable, TIRUserVariable {
  * A variable (user-declared or temporary) that is allocated on the stack. This includes all
  * parameters, non-static local variables, and temporary variables.
  */
-abstract private class AbstractIRAutomaticVariable extends AbstractIRVariable { }
-
-/**
- * A variable (user-declared or temporary) that is allocated on the stack. This includes all
- * parameters, non-static local variables, and temporary variables.
- */
-final class IRAutomaticVariable = AbstractIRAutomaticVariable;
-
-/**
- * A user-declared variable that is allocated on the stack. This includes all parameters and
- * non-static local variables.
- */
-private class AbstractIRAutomaticUserVariable extends IRUserVariable, AbstractIRAutomaticVariable {
-  override Language::AutomaticVariable var;
-
-  final override Language::AutomaticVariable getVariable() { result = var }
+class IRAutomaticVariable extends IRVariable {
+  IRAutomaticVariable() {
+    exists(Language::Variable var |
+      this = TIRUserVariable(var, _, func) and
+      Language::isVariableAutomatic(var)
+    )
+    or
+    this = TIRTempVariable(func, _, _, _)
+  }
 }
 
 /**
  * A user-declared variable that is allocated on the stack. This includes all parameters and
  * non-static local variables.
  */
-final class IRAutomaticUserVariable = AbstractIRAutomaticUserVariable;
+class IRAutomaticUserVariable extends IRUserVariable, IRAutomaticVariable {
+  override Language::AutomaticVariable var;
+
+  final override Language::AutomaticVariable getVariable() { result = var }
+}
 
 /**
  * A user-declared variable that is not allocated on the stack. This includes all global variables,
@@ -155,10 +151,16 @@ class IRStaticUserVariable extends IRUserVariable {
  * A variable that is not user-declared. This includes temporary variables generated as part of IR
  * construction, as well as string literals.
  */
-abstract private class AbstractIRGeneratedVariable extends AbstractIRVariable {
+class IRGeneratedVariable extends IRVariable {
   Language::AST ast;
   Language::LanguageType type;
 
+  IRGeneratedVariable() {
+    this = TIRTempVariable(func, ast, _, type) or
+    this = TIRStringLiteral(func, ast, type, _) or
+    this = TIRDynamicInitializationFlag(func, ast, type)
+  }
+
   final override Language::LanguageType getLanguageType() { result = type }
 
   final override Language::AST getAst() { result = ast }
@@ -194,20 +196,12 @@ abstract private class AbstractIRGeneratedVariable extends AbstractIRVariable {
   string getBaseString() { none() }
 }
 
-/**
- * A variable that is not user-declared. This includes temporary variables generated as part of IR
- * construction, as well as string literals.
- */
-final class IRGeneratedVariable = AbstractIRGeneratedVariable;
-
 /**
  * A temporary variable introduced by IR construction. The most common examples are the variable
  * generated to hold the return value of a function, or the variable generated to hold the result of
  * a condition operator (`a ? b : c`).
  */
-class IRTempVariable extends AbstractIRGeneratedVariable, AbstractIRAutomaticVariable,
-  TIRTempVariable
-{
+class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable {
   TempVariableTag tag;
 
   IRTempVariable() { this = TIRTempVariable(func, ast, tag, type) }
@@ -247,7 +241,7 @@ class IRThrowVariable extends IRTempVariable {
  * A temporary variable generated to hold the contents of all arguments passed to the `...` of a
  * function that accepts a variable number of arguments.
  */
-class IREllipsisVariable extends IRTempVariable, AbstractIRParameter {
+class IREllipsisVariable extends IRTempVariable, IRParameter {
   IREllipsisVariable() { tag = EllipsisTempVar() }
 
   final override string toString() { result = "#ellipsis" }
@@ -258,7 +252,7 @@ class IREllipsisVariable extends IRTempVariable, AbstractIRParameter {
 /**
  * A temporary variable generated to hold the `this` pointer.
  */
-class IRThisVariable extends IRTempVariable, AbstractIRParameter {
+class IRThisVariable extends IRTempVariable, IRParameter {
   IRThisVariable() { tag = ThisTempVar() }
 
   final override string toString() { result = "#this" }
@@ -270,7 +264,7 @@ class IRThisVariable extends IRTempVariable, AbstractIRParameter {
  * A variable generated to represent the contents of a string literal. This variable acts much like
  * a read-only global variable.
  */
-class IRStringLiteral extends AbstractIRGeneratedVariable, TIRStringLiteral {
+class IRStringLiteral extends IRGeneratedVariable, TIRStringLiteral {
   Language::StringLiteral literal;
 
   IRStringLiteral() { this = TIRStringLiteral(func, ast, type, literal) }
@@ -294,7 +288,7 @@ class IRStringLiteral extends AbstractIRGeneratedVariable, TIRStringLiteral {
  * used to model the runtime initialization of static local variables in C++, as well as static
  * fields in C#.
  */
-class IRDynamicInitializationFlag extends AbstractIRGeneratedVariable, TIRDynamicInitializationFlag {
+class IRDynamicInitializationFlag extends IRGeneratedVariable, TIRDynamicInitializationFlag {
   Language::Variable var;
 
   IRDynamicInitializationFlag() {
@@ -320,24 +314,24 @@ class IRDynamicInitializationFlag extends AbstractIRGeneratedVariable, TIRDynami
  * An IR variable which acts like a function parameter, including positional parameters and the
  * temporary variables generated for `this` and ellipsis parameters.
  */
-abstract private class AbstractIRParameter extends AbstractIRAutomaticVariable {
+class IRParameter extends IRAutomaticVariable {
+  IRParameter() {
+    this.(IRAutomaticUserVariable).getVariable() instanceof Language::Parameter
+    or
+    this = TIRTempVariable(_, _, ThisTempVar(), _)
+    or
+    this = TIRTempVariable(_, _, EllipsisTempVar(), _)
+  }
+
   /**
    * Gets the zero-based index of this parameter. The `this` parameter has index -1.
    */
   int getIndex() { none() }
 }
 
-/**
- * An IR variable which acts like a function parameter, including positional parameters and the
- * temporary variables generated for `this` and ellipsis parameters.
- */
-final class IRParameter = AbstractIRParameter;
-
 /**
  * An IR variable representing a positional parameter.
  */
-class IRPositionalParameter extends AbstractIRParameter, AbstractIRAutomaticUserVariable {
-  IRPositionalParameter() { this.getVariable() instanceof Language::Parameter }
-
+class IRPositionalParameter extends IRParameter, IRAutomaticUserVariable {
   final override int getIndex() { result = this.getVariable().(Language::Parameter).getIndex() }
 }

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll

@@ -247,7 +247,8 @@ class Instruction extends Construction::TStageInstruction {
    * Gets the type of the result produced by this instruction. If the instruction does not produce
    * a result, its result type will be `IRVoidType`.
    */
-  final IRType getResultIRType() { result = Construction::getInstructionResultIRType(this) }
+  cached
+  final IRType getResultIRType() { result = this.getResultLanguageType().getIRType() }
 
   /**
    * Gets the type of the result produced by this instruction. If the
@@ -994,8 +995,9 @@ class ConstantInstruction extends ConstantValueInstruction {
  */
 class IntegerConstantInstruction extends ConstantInstruction {
   IntegerConstantInstruction() {
-    exists(IRType resultType | resultType = this.getResultIRType() |
-      resultType instanceof IRIntegerType or resultType instanceof IRBooleanType
+    exists(IRType resultType |
+      resultType = this.getResultIRType() and
+      (resultType instanceof IRIntegerType or resultType instanceof IRBooleanType)
     )
   }
 }
@@ -1007,17 +1009,6 @@ class FloatConstantInstruction extends ConstantInstruction {
   FloatConstantInstruction() { this.getResultIRType() instanceof IRFloatingPointType }
 }
 
-/**
- * An instruction whose result is a constant value of a pointer type.
- */
-class PointerConstantInstruction extends ConstantInstruction {
-  PointerConstantInstruction() {
-    exists(IRType resultType | resultType = this.getResultIRType() |
-      resultType instanceof IRAddressType or resultType instanceof IRFunctionAddressType
-    )
-  }
-}
-
 /**
  * An instruction whose result is the address of a string literal.
  */

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/internal/ValueNumberingImports.qll

@@ -1,3 +1,3 @@
-import semmle.code.cpp.ir.implementation.unaliased_ssa.IR
+import semmle.code.cpp.ir.implementation.aliased_ssa.IR
 import semmle.code.cpp.ir.internal.Overlap
 import semmle.code.cpp.ir.internal.IRCppLanguage as Language

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

@@ -429,11 +429,6 @@ private module Cached {
     instr = unreachedInstruction(_) and result = Language::getVoidType()
   }
 
-  cached
-  IRType getInstructionResultIRType(Instruction instr) {
-    result = instr.getResultLanguageType().getIRType()
-  }
-
   /**
    * Holds if `opcode` is the opcode that specifies the operation performed by `instr`.
    *

cpp/ql/lib/semmle/code/cpp/models/implementations/Allocation.qll

@@ -7,6 +7,119 @@
 import semmle.code.cpp.models.interfaces.Allocation
 import semmle.code.cpp.models.interfaces.Taint
 
+/**
+ * An allocation function (such as `malloc`) that has an argument for the size
+ * in bytes.
+ */
+private class MallocAllocationFunction extends AllocationFunction {
+  int sizeArg;
+
+  MallocAllocationFunction() {
+    // --- C library allocation
+    this.hasGlobalOrStdOrBslName("malloc") and // malloc(size)
+    sizeArg = 0
+    or
+    this.hasGlobalName([
+        // --- Windows Memory Management for Windows Drivers
+        "MmAllocateContiguousMemory", // MmAllocateContiguousMemory(size, maxaddress)
+        "MmAllocateContiguousNodeMemory", // MmAllocateContiguousNodeMemory(size, minaddress, maxaddress, bound, flag, prefer)
+        "MmAllocateContiguousMemorySpecifyCache", // MmAllocateContiguousMemorySpecifyCache(size, minaddress, maxaddress, bound, type)
+        "MmAllocateContiguousMemorySpecifyCacheNode", // MmAllocateContiguousMemorySpecifyCacheNode(size, minaddress, maxaddress, bound, type, prefer)
+        "MmAllocateNonCachedMemory", // MmAllocateNonCachedMemory(size)
+        "MmAllocateMappingAddress", // MmAllocateMappingAddress(size, tag)
+        // --- Windows COM allocation
+        "CoTaskMemAlloc", // CoTaskMemAlloc(size)
+        // --- Solaris/BSD kernel memory allocator
+        "kmem_alloc", // kmem_alloc(size, flags)
+        "kmem_zalloc", // kmem_zalloc(size, flags)
+        // --- OpenSSL memory allocation
+        "CRYPTO_malloc", // CRYPTO_malloc(size_t num, const char *file, int line)
+        "CRYPTO_zalloc", // CRYPTO_zalloc(size_t num, const char *file, int line)
+        "CRYPTO_secure_malloc", // CRYPTO_secure_malloc(size_t num, const char *file, int line)
+        "CRYPTO_secure_zalloc", // CRYPTO_secure_zalloc(size_t num, const char *file, int line)
+        "g_malloc", // g_malloc (n_bytes);
+        "g_try_malloc" // g_try_malloc(n_bytes);
+      ]) and
+    sizeArg = 0
+    or
+    this.hasGlobalName([
+        // --- Windows Memory Management for Windows Drivers
+        "ExAllocatePool", // ExAllocatePool(type, size)
+        "ExAllocatePool2", // ExAllocatePool2(flags, size, tag)
+        "ExAllocatePool3", // ExAllocatePool3(flags, size, tag, extparams, extparamscount)
+        "ExAllocatePoolWithTag", // ExAllocatePool(type, size, tag)
+        "ExAllocatePoolWithTagPriority", // ExAllocatePoolWithTagPriority(type, size, tag, priority)
+        "ExAllocatePoolWithQuota", // ExAllocatePoolWithQuota(type, size)
+        "ExAllocatePoolWithQuotaTag", // ExAllocatePoolWithQuotaTag(type, size, tag)
+        "ExAllocatePoolZero", // ExAllocatePoolZero(type, size, tag)
+        "IoAllocateMdl", // IoAllocateMdl(address, size, flag, flag, irp)
+        "IoAllocateErrorLogEntry", // IoAllocateErrorLogEntry(object, size)
+        // --- Windows Global / Local legacy allocation
+        "LocalAlloc", // LocalAlloc(flags, size)
+        "GlobalAlloc", // GlobalAlloc(flags, size)
+        // --- Windows System Services allocation
+        "VirtualAlloc" // VirtualAlloc(address, size, type, flag)
+      ]) and
+    sizeArg = 1
+    or
+    this.hasGlobalName("HeapAlloc") and // HeapAlloc(heap, flags, size)
+    sizeArg = 2
+    or
+    this.hasGlobalName([
+        // --- Windows Memory Management for Windows Drivers
+        "MmAllocatePagesForMdl", // MmAllocatePagesForMdl(minaddress, maxaddress, skip, size)
+        "MmAllocatePagesForMdlEx", // MmAllocatePagesForMdlEx(minaddress, maxaddress, skip, size, type, flags)
+        "MmAllocateNodePagesForMdlEx" // MmAllocateNodePagesForMdlEx(minaddress, maxaddress, skip, size, type, prefer, flags)
+      ]) and
+    sizeArg = 3
+  }
+
+  override int getSizeArg() { result = sizeArg }
+}
+
+/**
+ * An allocation function (such as `alloca`) that does not require a
+ * corresponding free (and has an argument for the size in bytes).
+ */
+private class AllocaAllocationFunction extends AllocationFunction {
+  int sizeArg;
+
+  AllocaAllocationFunction() {
+    this.hasGlobalName([
+        // --- stack allocation
+        "alloca", // // alloca(size)
+        "__builtin_alloca", // __builtin_alloca(size)
+        "_alloca", // _alloca(size)
+        "_malloca" // _malloca(size)
+      ]) and
+    sizeArg = 0
+  }
+
+  override int getSizeArg() { result = sizeArg }
+
+  override predicate requiresDealloc() { none() }
+}
+
+/**
+ * An allocation function (such as `calloc`) that has an argument for the size
+ * and another argument for the size of those units (in bytes).
+ */
+private class CallocAllocationFunction extends AllocationFunction {
+  int sizeArg;
+  int multArg;
+
+  CallocAllocationFunction() {
+    // --- C library allocation
+    this.hasGlobalOrStdOrBslName("calloc") and // calloc(num, size)
+    sizeArg = 1 and
+    multArg = 0
+  }
+
+  override int getSizeArg() { result = sizeArg }
+
+  override int getSizeMult() { result = multArg }
+}
+
 /**
  * An allocation function (such as `realloc`) that has an argument for the size
  * in bytes, and an argument for an existing pointer that is to be reallocated.
@@ -260,63 +373,6 @@ private class NewArrayAllocationExpr extends AllocationExpr, NewArrayExpr {
   override predicate requiresDealloc() { not exists(this.getPlacementPointer()) }
 }
 
-/**
- * Holds if `f` is an allocation function according to the
- * extensible `allocationFunctionModel` predicate.
- */
-private predicate isAllocationFunctionFromModel(
-  Function f, string namespace, string type, string name
-) {
-  exists(boolean subtypes | allocationFunctionModel(namespace, type, subtypes, name, _, _, _, _) |
-    if type = ""
-    then f.hasQualifiedName(namespace, "", name)
-    else
-      exists(Class c |
-        c.hasQualifiedName(namespace, type) and f.hasQualifiedName(namespace, _, name)
-      |
-        if subtypes = true
-        then f = c.getADerivedClass*().getAMemberFunction()
-        else f = c.getAMemberFunction()
-      )
-  )
-}
-
-/**
- * An allocation function modeled via the extensible `allocationFunctionModel` predicate.
- */
-private class AllocationFunctionFromModel extends AllocationFunction {
-  string namespace;
-  string type;
-  string name;
-
-  AllocationFunctionFromModel() { isAllocationFunctionFromModel(this, namespace, type, name) }
-
-  final override int getSizeArg() {
-    exists(string sizeArg |
-      allocationFunctionModel(namespace, type, _, name, sizeArg, _, _, _) and
-      result = sizeArg.toInt()
-    )
-  }
-
-  final override int getSizeMult() {
-    exists(string sizeMult |
-      allocationFunctionModel(namespace, type, _, name, _, sizeMult, _, _) and
-      result = sizeMult.toInt()
-    )
-  }
-
-  final override int getReallocPtrArg() {
-    exists(string reallocPtrArg |
-      allocationFunctionModel(namespace, type, _, name, _, _, reallocPtrArg, _) and
-      result = reallocPtrArg.toInt()
-    )
-  }
-
-  final override predicate requiresDealloc() {
-    allocationFunctionModel(namespace, type, _, name, _, _, _, true)
-  }
-}
-
 private module HeuristicAllocation {
   /** A class that maps an `AllocationExpr` to an `HeuristicAllocationExpr`. */
   private class HeuristicAllocationModeled extends HeuristicAllocationExpr instanceof AllocationExpr

cpp/ql/lib/semmle/code/cpp/models/implementations/Deallocation.qll

@@ -7,42 +7,61 @@
 import semmle.code.cpp.models.interfaces.Deallocation
 
 /**
- * Holds if `f` is an deallocation function according to the
- * extensible `deallocationFunctionModel` predicate.
+ * A deallocation function such as `free`.
  */
-private predicate isDeallocationFunctionFromModel(
-  Function f, string namespace, string type, string name
-) {
-  exists(boolean subtypes | deallocationFunctionModel(namespace, type, subtypes, name, _) |
-    if type = ""
-    then f.hasQualifiedName(namespace, "", name)
-    else
-      exists(Class c |
-        c.hasQualifiedName(namespace, type) and f.hasQualifiedName(namespace, _, name)
-      |
-        if subtypes = true
-        then f = c.getADerivedClass*().getAMemberFunction()
-        else f = c.getAMemberFunction()
-      )
-  )
-}
+private class StandardDeallocationFunction extends DeallocationFunction {
+  int freedArg;
 
-/**
- * A deallocation function modeled via the extensible `deallocationFunctionModel` predicate.
- */
-private class DeallocationFunctionFromModel extends DeallocationFunction {
-  string namespace;
-  string type;
-  string name;
-
-  DeallocationFunctionFromModel() { isDeallocationFunctionFromModel(this, namespace, type, name) }
-
-  final override int getFreedArg() {
-    exists(string freedArg |
-      deallocationFunctionModel(namespace, type, _, name, freedArg) and
-      result = freedArg.toInt()
-    )
+  StandardDeallocationFunction() {
+    this.hasGlobalOrStdOrBslName([
+        // --- C library allocation
+        "free", "realloc"
+      ]) and
+    freedArg = 0
+    or
+    this.hasGlobalName([
+        // --- OpenSSL memory deallocation
+        "CRYPTO_free", "CRYPTO_secure_free",
+        // --- glib memory deallocation
+        "g_free"
+      ]) and
+    freedArg = 0
+    or
+    this.hasGlobalOrStdName([
+        // --- Windows Memory Management for Windows Drivers
+        "ExFreePool", "ExFreePoolWithTag", "ExDeleteTimer", "IoFreeIrp", "IoFreeMdl",
+        "IoFreeErrorLogEntry", "IoFreeWorkItem", "MmFreeContiguousMemory",
+        "MmFreeContiguousMemorySpecifyCache", "MmFreeNonCachedMemory", "MmFreeMappingAddress",
+        "MmFreePagesFromMdl", "MmUnmapReservedMapping", "MmUnmapLockedPages",
+        "NdisFreeGenericObject", "NdisFreeMemory", "NdisFreeMemoryWithTag", "NdisFreeMdl",
+        "NdisFreeNetBufferListPool", "NdisFreeNetBufferPool",
+        // --- Windows Global / Local legacy allocation
+        "LocalFree", "GlobalFree", "LocalReAlloc", "GlobalReAlloc",
+        // --- Windows System Services allocation
+        "VirtualFree",
+        // --- Windows COM allocation
+        "CoTaskMemFree", "CoTaskMemRealloc",
+        // --- Windows Automation
+        "SysFreeString",
+        // --- Solaris/BSD kernel memory allocator
+        "kmem_free"
+      ]) and
+    freedArg = 0
+    or
+    this.hasGlobalOrStdName([
+        // --- Windows Memory Management for Windows Drivers
+        "ExFreeToLookasideListEx", "ExFreeToPagedLookasideList", "ExFreeToNPagedLookasideList",
+        "NdisFreeMemoryWithTagPriority", "StorPortFreeMdl", "StorPortFreePool",
+        // --- NetBSD pool manager
+        "pool_put", "pool_cache_put"
+      ]) and
+    freedArg = 1
+    or
+    this.hasGlobalOrStdName(["HeapFree", "HeapReAlloc"]) and
+    freedArg = 2
   }
+
+  override int getFreedArg() { result = freedArg }
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/models/implementations/Iterator.qll

@@ -560,7 +560,7 @@ private class IteratorAssignmentMemberOperatorModel extends IteratorAssignmentMe
   TaintFunction, SideEffectFunction, AliasFunction
 {
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    (input.isParameterDeref(0) or input.isParameter(0)) and
+    input.isParameterDeref(0) and
     output.isQualifierObject()
   }
 
@@ -579,34 +579,17 @@ private class IteratorAssignmentMemberOperatorModel extends IteratorAssignmentMe
   override predicate parameterEscapesOnlyViaReturn(int index) { index = -1 }
 }
 
-/**
- * A `begin` member function, or a related function, that returns an iterator.
- */
-class BeginFunction extends MemberFunction {
-  BeginFunction() {
-    this.hasName(["begin", "cbegin", "rbegin", "crbegin", "before_begin", "cbefore_begin"]) and
-    this.getType().getUnspecifiedType() instanceof Iterator
-  }
-}
-
-/**
- * An `end` member function, or a related function, that returns an iterator.
- */
-class EndFunction extends MemberFunction {
-  EndFunction() {
-    this.hasName(["end", "cend", "rend", "crend"]) and
-    this.getType().getUnspecifiedType() instanceof Iterator
-  }
-}
-
 /**
  * A `begin` or `end` member function, or a related member function, that
  * returns an iterator.
  */
 class BeginOrEndFunction extends MemberFunction {
   BeginOrEndFunction() {
-    this instanceof BeginFunction or
-    this instanceof EndFunction
+    this.hasName([
+        "begin", "cbegin", "rbegin", "crbegin", "end", "cend", "rend", "crend", "before_begin",
+        "cbefore_begin"
+      ]) and
+    this.getType().getUnspecifiedType() instanceof Iterator
   }
 }
 

cpp/ql/lib/semmle/code/cpp/models/interfaces/Allocation.qll

@@ -89,14 +89,6 @@ abstract class AllocationFunction extends Function {
   predicate requiresDealloc() { any() }
 }
 
-/**
- * Holds if an external allocation model exists for the given parameters.
- */
-extensible predicate allocationFunctionModel(
-  string namespace, string type, boolean subtypes, string name, string sizeArg, string multArg,
-  string reallocPtrArg, boolean requiresDealloc
-);
-
 /**
  * An `operator new` or `operator new[]` function that may be associated with
  * `new` or `new[]` expressions.  Note that `new` and `new[]` are not function

cpp/ql/lib/semmle/code/cpp/models/interfaces/Deallocation.qll

@@ -34,13 +34,6 @@ abstract class DeallocationFunction extends Function {
   int getFreedArg() { none() }
 }
 
-/**
- * Holds if an external deallocation model exists for the given parameters.
- */
-extensible predicate deallocationFunctionModel(
-  string namespace, string type, boolean subtypes, string name, string freedArg
-);
-
 /**
  * An `operator delete` or `operator delete[]` function that may be associated
  * with `delete` or `delete[]` expressions.  Note that `delete` and `delete[]`