repatch

This reverts commit a0f479d503.

.codeqlmanifest.json

@@ -1,11 +1,25 @@
-{ "provide": [ "ruby/.codeqlmanifest.json",
-                "*/ql/src/qlpack.yml",
-               "*/ql/lib/qlpack.yml",
-               "*/ql/test/qlpack.yml",
-               "cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/qlpack.yml",
-               "*/ql/examples/qlpack.yml",
-               "*/upgrades/qlpack.yml",
-               "javascript/ql/experimental/adaptivethreatmodeling/lib/qlpack.yml",
-               "javascript/ql/experimental/adaptivethreatmodeling/src/qlpack.yml",
-               "misc/legacy-support/*/qlpack.yml",
-               "misc/suite-helpers/qlpack.yml" ] }
+{
+    "provide": [
+        "*/ql/src/qlpack.yml",
+        "*/ql/lib/qlpack.yml",
+        "*/ql/test/qlpack.yml",
+        "*/ql/examples/qlpack.yml",
+        "*/upgrades/qlpack.yml",
+        "cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/qlpack.yml",
+        "javascript/ql/experimental/adaptivethreatmodeling/lib/qlpack.yml",
+        "javascript/ql/experimental/adaptivethreatmodeling/src/qlpack.yml",
+        "misc/legacy-support/*/qlpack.yml",
+        "misc/suite-helpers/qlpack.yml",
+        "ruby/extractor-pack/codeql-extractor.yml",
+        "ruby/ql/consistency-queries/qlpack.yml",
+        "ql/ql/consistency-queries/qlpack.yml",
+        "ql/extractor-pack/codeql-extractor.yml"
+    ],
+    "versionPolicies": {
+      "default": {
+        "requireChangeNotes": true,
+        "committedPrereleaseSuffix": "dev",
+        "committedVersion": "nextPatchRelease"
+      }
+    }
+}

.github/actions/fetch-codeql/action.yml

@@ -8,7 +8,7 @@ runs:
       run: |
         LATEST=$(gh release list --repo https://github.com/github/codeql-cli-binaries | cut -f 1 | grep -v beta | sort --version-sort | tail -1)
         gh release download --repo https://github.com/github/codeql-cli-binaries --pattern codeql-linux64.zip "$LATEST"
-        unzip -q codeql-linux64.zip
-        echo "${{ github.workspace }}/codeql" >> $GITHUB_PATH
+        unzip -q -d "${RUNNER_TEMP}" codeql-linux64.zip
+        echo "${RUNNER_TEMP}/codeql" >> "${GITHUB_PATH}"
       env:
         GITHUB_TOKEN: ${{ github.token }}

.github/workflows/post-pr-comment.yml

@@ -0,0 +1,31 @@
+name: Post pull-request comment
+on:
+  workflow_run:
+    workflows: ["Query help preview"]
+    types:
+      - completed
+
+permissions:
+  pull-requests: write
+
+jobs:
+  post_comment:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Download artifact
+        run: gh run download "${WORKFLOW_RUN_ID}" --repo "${GITHUB_REPOSITORY}" --name "comment"
+        env:
+          GITHUB_TOKEN: ${{ github.token }}
+          WORKFLOW_RUN_ID: ${{ github.event.workflow_run.id }}
+      - run: |
+          PR="$(grep -o '^[0-9]\+$' pr.txt)"
+          PR_HEAD_SHA="$(gh api "/repos/${GITHUB_REPOSITORY}/pulls/${PR}" --jq .head.sha)"
+          # Check that the pull-request head SHA matches the head SHA of the workflow run
+          if [ "${WORKFLOW_RUN_HEAD_SHA}" != "${PR_HEAD_SHA}" ]; then
+            echo "PR head SHA ${PR_HEAD_SHA} does not match workflow_run event SHA ${WORKFLOW_RUN_HEAD_SHA}. Stopping." 1>&2
+            exit 1
+          fi
+          gh pr comment "${PR}" --repo "${GITHUB_REPOSITORY}" -F comment.txt
+        env:
+          GITHUB_TOKEN: ${{ github.token }}
+          WORKFLOW_RUN_HEAD_SHA: ${{ github.event.workflow_run.head_commit.id }}

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

@@ -1,10 +1,13 @@
 name: Query help preview
 
+permissions:
+  contents: read
+
 on:
   pull_request:
     branches:
       - main
-      - 'rc/*'
+      - "rc/*"
     paths:
       - "ruby/**/*.qhelp"
 
@@ -12,28 +15,49 @@ jobs:
   qhelp:
     runs-on: ubuntu-latest
     steps:
+      - run: echo "${{  github.event.number }}" > pr.txt
+      - uses: actions/upload-artifact@v2
+        with:
+          name: comment
+          path: pr.txt
+          retention-days: 1
       - uses: actions/checkout@v2
         with:
           fetch-depth: 2
+          persist-credentials: false
+      - uses: ./.github/actions/fetch-codeql
       - name: Determine changed files
         id: changes
         run: |
-          echo -n "::set-output name=qhelp_files::"
-          (git diff --name-only --diff-filter=ACMRT HEAD~1 HEAD | grep .qhelp$ | grep -v .inc.qhelp;
-           git diff --name-only --diff-filter=ACMRT HEAD~1 HEAD | grep .inc.qhelp$ | xargs -d '\n' -rn1 basename | xargs -d '\n' -rn1 git grep -l) |
-           sort -u | xargs -d '\n' -n1 printf "'%s' "
-
-      - uses: ./.github/actions/fetch-codeql
+          (git diff -z --name-only --diff-filter=ACMRT HEAD~1 HEAD | grep -z '.qhelp$' | grep -z -v '.inc.qhelp';
+           git diff -z --name-only --diff-filter=ACMRT HEAD~1 HEAD | grep -z '.inc.qhelp$' | xargs --null -rn1 basename | xargs --null -rn1 git grep -z -l) |
+           grep -z '.qhelp$' | grep -z -v '^-' | sort -z -u > "${RUNNER_TEMP}/paths.txt"
 
       - name: QHelp preview
-        if: ${{ steps.changes.outputs.qhelp_files }}
         run: |
-          ( echo "QHelp previews:";
-          for path in ${{ steps.changes.outputs.qhelp_files }} ; do
+          EXIT_CODE=0
+          echo "QHelp previews:" > comment.txt
+          while read -r -d $'\0' path; do
+            if [ ! -f "${path}" ]; then
+               exit 1
+            fi
             echo "<details> <summary>${path}</summary>"
             echo
-            codeql generate query-help --format=markdown ${path}
+            codeql generate query-help --format=markdown -- "./${path}" 2> errors.txt || EXIT_CODE="$?"
+            if [ -s errors.txt ]; then
+               echo "# errors/warnings:"
+               echo '```'
+               cat errors.txt
+               cat errors.txt 1>&2
+               echo '```'
+            fi
             echo "</details>"
-          done) | gh pr comment "${{ github.event.pull_request.number }}" -F -
-        env:
-          GITHUB_TOKEN: ${{ github.token }}
+          done < "${RUNNER_TEMP}/paths.txt" >> comment.txt
+          exit "${EXIT_CODE}"
+
+      - if: always()
+        uses: actions/upload-artifact@v2
+        with:
+          name: comment
+          path: comment.txt
+          retention-days: 1

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

@@ -0,0 +1,152 @@
+name: Run QL for QL
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+
+env:
+  CARGO_TERM_COLOR: always
+
+jobs:
+  queries:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Find codeql
+        id: find-codeql
+        uses: github/codeql-action/init@esbena/ql
+        with:
+          languages: javascript # does not matter
+      - name: Get CodeQL version
+        id: get-codeql-version
+        run: |
+          echo "::set-output name=version::$("${CODEQL}" --version | head -n 1 | rev | cut -d " " -f 1 | rev)"
+        shell: bash
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+      - name: Cache queries
+        id: cache-queries
+        uses: actions/cache@v2
+        with:
+          path: ${{ runner.temp }}/query-pack.zip
+          key: queries-${{ hashFiles('ql/**/*.ql*') }}-${{ hashFiles('ql/ql/src/ql.dbscheme*') }}-${{ steps.get-codeql-version.outputs.version }}
+      - name: Build query pack
+        if: steps.cache-queries.outputs.cache-hit != 'true'
+        run: |
+          cd ql/ql/src
+          "${CODEQL}" pack create
+          cd .codeql/pack/codeql/ql-all/0.0.0
+          zip "${PACKZIP}" -r .
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+          PACKZIP: ${{ runner.temp }}/query-pack.zip
+      - name: Upload query pack
+        uses: actions/upload-artifact@v2
+        with:
+          name: query-pack-zip
+          path: ${{ runner.temp }}/query-pack.zip
+
+  extractors:
+    strategy:
+      fail-fast: false
+
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/cache@v2
+        with:
+          path: |
+            ~/.cargo/registry
+            ~/.cargo/git
+            ql/target
+          key: ${{ runner.os }}-rust-cargo-${{ hashFiles('**/Cargo.lock') }}
+      - name: Check formatting
+        run: cd ql; cargo fmt --all -- --check
+      - name: Build
+        run: cd ql; cargo build --verbose
+      - name: Run tests
+        run: cd ql; cargo test --verbose
+      - name: Release build
+        run: cd ql; cargo build --release
+      - name: Generate dbscheme
+        run: ql/target/release/ql-generator --dbscheme ql/ql/src/ql.dbscheme --library ql/ql/src/codeql_ql/ast/internal/TreeSitter.qll
+      - uses: actions/upload-artifact@v2
+        with:
+          name: extractor-ubuntu-latest
+          path: |
+            ql/target/release/ql-extractor
+            ql/target/release/ql-extractor.exe
+          retention-days: 1
+  package:
+    runs-on: ubuntu-latest
+
+    needs:
+      - extractors
+      - queries
+
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/download-artifact@v2
+        with:
+          name: query-pack-zip
+          path: query-pack-zip
+      - uses: actions/download-artifact@v2
+        with:
+          name: extractor-ubuntu-latest
+          path: linux64
+      - run: |
+          unzip query-pack-zip/*.zip -d pack
+          cp -r ql/codeql-extractor.yml ql/tools ql/ql/src/ql.dbscheme.stats pack/
+          mkdir -p pack/tools/linux64
+          if [[ -f linux64/ql-extractor ]]; then
+            cp linux64/ql-extractor pack/tools/linux64/extractor
+            chmod +x pack/tools/linux64/extractor
+          fi
+          cd pack
+          zip -rq ../codeql-ql.zip .
+      - uses: actions/upload-artifact@v2
+        with:
+          name: codeql-ql-pack
+          path: codeql-ql.zip
+          retention-days: 1
+  analyze: 
+    runs-on: ubuntu-latest
+    
+    needs: 
+      - package
+    
+    steps: 
+      - name: Download pack
+        uses: actions/download-artifact@v2
+        with:
+          name: codeql-ql-pack
+          path: ${{ runner.temp }}/codeql-ql-pack-artifact
+
+      - name: Prepare pack
+        run: |
+          unzip "${PACK_ARTIFACT}/*.zip" -d "${PACK}"
+        env:
+          PACK_ARTIFACT: ${{ runner.temp }}/codeql-ql-pack-artifact
+          PACK: ${{ runner.temp }}/pack
+      - name: Hack codeql-action options
+        run: |
+          JSON=$(jq -nc --arg pack "${PACK}" '.resolve.queries=["--search-path", $pack] | .resolve.extractor=["--search-path", $pack] | .database.init=["--search-path", $pack]')
+          echo "CODEQL_ACTION_EXTRA_OPTIONS=${JSON}" >> ${GITHUB_ENV}
+        env:
+          PACK: ${{ runner.temp }}/pack
+
+      - name: Checkout repository
+        uses: actions/checkout@v2
+
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@esbena/ql
+        with:
+          languages: ql
+          db-location: ${{ runner.temp }}/db
+
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@esbena/ql
+

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

@@ -0,0 +1,84 @@
+name: Collect database stats for QL for QL
+
+on:
+  push:
+    branches: [main]
+    paths:
+      - ql/ql/src/ql.dbscheme
+  pull_request:
+    branches: [main]
+    paths:
+      - ql/ql/src/ql.dbscheme
+  workflow_dispatch:
+
+jobs:
+  measure:
+    env:
+      CODEQL_THREADS: 4 # TODO: remove this once it's set by the CLI
+    strategy:
+      matrix:
+        repo: 
+          - github/codeql
+          - github/codeql-go
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+
+      - name: Find codeql
+        id: find-codeql
+        uses: github/codeql-action/init@esbena/ql
+        with:
+          languages: javascript # does not matter
+      - uses: actions/cache@v2
+        with:
+          path: |
+            ~/.cargo/registry
+            ~/.cargo/git
+            ql/target
+          key: ${{ runner.os }}-qltest-cargo-${{ hashFiles('**/Cargo.lock') }}
+      - name: Build Extractor
+        run: cd ql; env "PATH=$PATH:`dirname ${CODEQL}`" ./create-extractor-pack.sh
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+      - name: Checkout ${{ matrix.repo }}
+        uses: actions/checkout@v2
+        with:
+          repository: ${{ matrix.repo }}
+          path: ${{ github.workspace }}/repo
+      - name: Create database
+        run: |
+          "${CODEQL}" database create \
+            --search-path "ql/extractor-pack" \
+            --threads 4 \
+            --language ql --source-root "${{ github.workspace }}/repo" \
+            "${{ runner.temp }}/database"
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+      - name: Measure database
+        run: |
+          mkdir -p "stats/${{ matrix.repo }}"
+          "${CODEQL}" dataset measure --threads 4 --output "stats/${{ matrix.repo }}/stats.xml" "${{ runner.temp }}/database/db-ql"
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+      - uses: actions/upload-artifact@v2
+        with:
+          name: measurements
+          path: stats
+          retention-days: 1
+
+  merge:
+    runs-on: ubuntu-latest
+    needs: measure
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/download-artifact@v2
+        with:
+          name: measurements
+          path: stats
+      - run: |
+          python -m pip install --user lxml
+          find stats -name 'stats.xml' -print0 | sort -z | xargs -0 python ql/scripts/merge_stats.py --output ql/ql/src/ql.dbscheme.stats --normalise ql_tokeninfo
+      - uses: actions/upload-artifact@v2
+        with:
+          name: ql.dbscheme.stats
+          path: ql/ql/src/ql.dbscheme.stats

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

@@ -0,0 +1,52 @@
+name: Run QL for QL Tests
+
+on:
+  push:
+    branches: [main]
+    paths:
+      - ql/*
+  pull_request:
+    branches: [main]
+    paths:
+      - ql/*
+
+env:
+  CARGO_TERM_COLOR: always
+
+jobs:
+  qltest:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Find codeql
+        id: find-codeql
+        uses: github/codeql-action/init@esbena/ql
+        with:
+          languages: javascript # does not matter
+      - uses: actions/cache@v2
+        with:
+          path: |
+            ~/.cargo/registry
+            ~/.cargo/git
+            ql/target
+          key: ${{ runner.os }}-qltest-cargo-${{ hashFiles('**/Cargo.lock') }}
+      - name: Build extractor
+        run: |
+          cd ql;
+          codeqlpath=$(dirname ${{ steps.find-codeql.outputs.codeql-path }});
+          env "PATH=$PATH:$codeqlpath" ./create-extractor-pack.sh
+      - name: Run QL tests
+        run: | 
+          "${CODEQL}" test run --check-databases --check-unused-labels --check-repeated-labels --check-redefined-labels --check-use-before-definition --search-path "${{ github.workspace }}/ql/extractor-pack" --consistency-queries ql/ql/consistency-queries ql/ql/test
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+      - name: Check QL formatting
+        run: | 
+          find ql/ql "(" -name "*.ql" -or -name "*.qll" ")" -print0 | xargs -0 "${CODEQL}" query format --check-only
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
+      - name: Check QL compilation
+        run: | 
+          "${CODEQL}" query compile --check-only --threads=4 --warnings=error --search-path "${{ github.workspace }}/ql/extractor-pack" "ql/ql/src" "ql/ql/examples"
+        env:
+          CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}

.github/workflows/ruby-build.yml

@@ -3,16 +3,18 @@ name: "Ruby: Build"
 on:
   push:
     paths:
-      - 'ruby/**'
+      - "ruby/**"
+      - .github/workflows/ruby-build.yml
     branches:
       - main
-      - 'rc/*'
+      - "rc/*"
   pull_request:
     paths:
-      - 'ruby/**'
+      - "ruby/**"
+      - .github/workflows/ruby-build.yml
     branches:
       - main
-      - 'rc/*'
+      - "rc/*"
   workflow_dispatch:
     inputs:
       tag:
@@ -100,16 +102,6 @@ jobs:
           PACK_FOLDER=$(readlink -f target/packs/codeql/ruby-queries/*)
           codeql/codeql generate query-help --format=sarifv2.1.0 --output="${PACK_FOLDER}/rules.sarif" ql/src
           (cd ql/src; find queries \( -name '*.qhelp' -o -name '*.rb' -o -name '*.erb' \) -exec bash -c 'mkdir -p "'"${PACK_FOLDER}"'/$(dirname "{}")"' \; -exec cp "{}" "${PACK_FOLDER}/{}" \;)
-      - name: Compile with previous CodeQL versions
-        run: |
-          for version in  $(gh release list --repo https://github.com/github/codeql-cli-binaries | cut -f 1 | sort --version-sort | tail -3 | head -2); do
-            rm -f codeql-linux64.zip
-            gh release download --repo https://github.com/github/codeql-cli-binaries --pattern codeql-linux64.zip "$version"
-            rm -rf codeql; unzip -q codeql-linux64.zip
-            codeql/codeql query compile target/packs/*
-          done
-        env:
-          GITHUB_TOKEN: ${{ github.token }}
       - uses: actions/upload-artifact@v2
         with:
           name: codeql-ruby-queries

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

@@ -4,15 +4,17 @@ on:
   push:
     branches:
       - main
-      - 'rc/*'
+      - "rc/*"
     paths:
       - ruby/ql/lib/ruby.dbscheme
+      - .github/workflows/ruby-dataset-measure.yml
   pull_request:
     branches:
       - main
-      - 'rc/*'
+      - "rc/*"
     paths:
       - ruby/ql/lib/ruby.dbscheme
+      - .github/workflows/ruby-dataset-measure.yml
   workflow_dispatch:
 
 jobs:
@@ -22,7 +24,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        repo: [rails/rails, discourse/discourse, spree/spree]
+        repo: [rails/rails, discourse/discourse, spree/spree, ruby/ruby]
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
@@ -39,7 +41,7 @@ jobs:
       - name: Create database
         run: |
           codeql database create \
-            --search-path "${{ github.workspace }}/ruby" \
+            --search-path "${{ github.workspace }}/ruby/extractor-pack" \
             --threads 4 \
             --language ruby --source-root "${{ github.workspace }}/repo" \
             "${{ runner.temp }}/database"

.github/workflows/ruby-qltest.yml

@@ -3,16 +3,18 @@ name: "Ruby: Run QL Tests"
 on:
   push:
     paths:
-      - 'ruby/**'
+      - "ruby/**"
+      - .github/workflows/ruby-qltest.yml
     branches:
       - main
-      - 'rc/*'
+      - "rc/*"
   pull_request:
     paths:
-      - 'ruby/**'
+      - "ruby/**"
+      - .github/workflows/ruby-qltest.yml
     branches:
       - main
-      - 'rc/*'
+      - "rc/*"
 
 env:
   CARGO_TERM_COLOR: always
@@ -30,19 +32,19 @@ jobs:
       - uses: ./ruby/actions/create-extractor-pack
       - 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 }}/ruby" --additional-packs "${{ github.workspace }}"  --consistency-queries ql/consistency-queries ql/test
+          codeql test run --search-path "${{ github.workspace }}/ruby/extractor-pack" --check-databases --check-unused-labels --check-repeated-labels --check-redefined-labels --check-use-before-definition --consistency-queries ql/consistency-queries ql/test
         env:
           GITHUB_TOKEN: ${{ github.token }}
       - name: Check QL formatting
         run: find ql "(" -name "*.ql" -or -name "*.qll" ")" -print0 | xargs -0 codeql query format --check-only
       - name: Check QL compilation
         run: |
-          codeql query compile --check-only --threads=4 --warnings=error --search-path "${{ github.workspace }}/ruby" --additional-packs "${{ github.workspace }}" "ql/src" "ql/examples"
+          codeql query compile --check-only --threads=4 --warnings=error "ql/src" "ql/examples"
         env:
           GITHUB_TOKEN: ${{ github.token }}
       - name: Check DB upgrade scripts
         run: |
           echo >empty.trap
           codeql dataset import -S ql/lib/upgrades/initial/ruby.dbscheme testdb empty.trap
-          codeql dataset upgrade testdb --additional-packs ql/lib/upgrades
+          codeql dataset upgrade testdb --additional-packs ql/lib
           diff -q testdb/ruby.dbscheme ql/lib/ruby.dbscheme

.gitignore

@@ -27,3 +27,6 @@ csharp/extractor/Semmle.Extraction.CSharp.Driver/Properties/launchSettings.json
 
 # Avoid committing cached package components
 .codeql
+
+# Compiled class file
+*.class

CODEOWNERS

@@ -25,3 +25,6 @@
 /docs/codeql-for-visual-studio-code/ @github/codeql-vscode-reviewers
 /docs/ql-language-reference/ @github/codeql-frontend-reviewers
 /docs/query-*-style-guide.md @github/codeql-analysis-reviewers
+
+# QL for QL reviewers
+/ql/ @erik-krogh @tausbn

CONTRIBUTING.md

@@ -11,13 +11,14 @@ If you have an idea for a query that you would like to share with other CodeQL u
 
 1. **Directory structure**
 
-    There are five language-specific query directories in this repository:
+    There are six language-specific query directories in this repository:
 
       * C/C++: `cpp/ql/src`
       * C#: `csharp/ql/src`
       * Java: `java/ql/src`
       * JavaScript: `javascript/ql/src`
       * Python: `python/ql/src`
+      * Ruby: `ruby/ql/src`
 
     Each language-specific directory contains further subdirectories that group queries based on their `@tags` or purpose.
     - Experimental queries and libraries are stored in the `experimental` subdirectory within each language-specific directory in the [CodeQL repository](https://github.com/github/codeql). For example, experimental Java queries and libraries are stored in `java/ql/src/experimental` and any corresponding tests in `java/ql/test/experimental`.

config/identical-files.json

@@ -449,7 +449,8 @@
     "csharp/ql/lib/semmle/code/csharp/controlflow/internal/pressa/SsaImplCommon.qll",
     "csharp/ql/lib/semmle/code/csharp/dataflow/internal/basessa/SsaImplCommon.qll",
     "csharp/ql/lib/semmle/code/cil/internal/SsaImplCommon.qll",
-    "ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImplCommon.qll"
+    "ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImplCommon.qll",
+    "cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll"
   ],
   "CryptoAlgorithms Python/JS": [
     "javascript/ql/lib/semmle/javascript/security/CryptoAlgorithms.qll",
@@ -459,9 +460,10 @@
     "javascript/ql/lib/semmle/javascript/security/internal/SensitiveDataHeuristics.qll",
     "python/ql/lib/semmle/python/security/internal/SensitiveDataHeuristics.qll"
   ],
-  "ReDoS Util Python/JS": [
+  "ReDoS Util Python/JS/Ruby": [
     "javascript/ql/lib/semmle/javascript/security/performance/ReDoSUtil.qll",
-    "python/ql/lib/semmle/python/security/performance/ReDoSUtil.qll"
+    "python/ql/lib/semmle/python/security/performance/ReDoSUtil.qll",
+    "ruby/ql/lib/codeql/ruby/security/performance/ReDoSUtil.qll"
   ],
   "ReDoS Exponential Python/JS": [
     "javascript/ql/lib/semmle/javascript/security/performance/ExponentialBackTracking.qll",
@@ -470,7 +472,12 @@
   "ReDoS Polynomial Python/JS": [
     "javascript/ql/lib/semmle/javascript/security/performance/SuperlinearBackTracking.qll",
     "python/ql/lib/semmle/python/security/performance/SuperlinearBackTracking.qll",
-    "ruby/ql/lib/codeql/ruby/regexp/SuperlinearBackTracking.qll"
+    "ruby/ql/lib/codeql/ruby/security/performance/SuperlinearBackTracking.qll"
+  ],
+  "BadTagFilterQuery Python/JS/Ruby": [
+    "javascript/ql/lib/semmle/javascript/security/BadTagFilterQuery.qll",
+    "python/ql/lib/semmle/python/security/BadTagFilterQuery.qll",
+    "ruby/ql/lib/codeql/ruby/security/BadTagFilterQuery.qll"
   ],
   "CFG": [
     "csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImplShared.qll",

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

@@ -17,7 +17,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.Build" Version="16.9.0" />
+    <PackageReference Include="Microsoft.Build" Version="16.11.0" />
   </ItemGroup>
 
   <ItemGroup>

cpp/change-notes/2021-11-01-isFromSystemMacroDefinition.md

@@ -0,0 +1,4 @@
+lgtm,codescanning
+* The QL library `semmle.code.cpp.commons.Exclusions` now contains a predicate
+  `isFromSystemMacroDefinition` for identifying code that originates from a
+  macro outside the project being analyzed.

cpp/change-notes/2021-11-09-use-of-http.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* A new query `cpp/non-https-url` has been added for C/C++. The query flags uses of `http` URLs that might be better replaced with `https`.

cpp/change-notes/2021-11-25-certificate-not-checked.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* A new query `cpp/certificate-not-checked` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.

cpp/change-notes/2021-11-25-certificate-result-conflation.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* A new query `cpp/certificate-result-conflation` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.

cpp/ql/lib/CHANGELOG.md

@@ -0,0 +1,7 @@
+## 0.0.4
+
+### New Features
+
+* The QL library `semmle.code.cpp.commons.Exclusions` now contains a predicate
+  `isFromSystemMacroDefinition` for identifying code that originates from a
+  macro outside the project being analyzed.

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

@@ -0,0 +1,7 @@
+## 0.0.4
+
+### New Features
+
+* The QL library `semmle.code.cpp.commons.Exclusions` now contains a predicate
+  `isFromSystemMacroDefinition` for identifying code that originates from a
+  macro outside the project being analyzed.

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

@@ -0,0 +1,2 @@
+---
+lastReleaseVersion: 0.0.4

cpp/ql/lib/qlpack.yml

@@ -1,7 +1,8 @@
 name: codeql/cpp-all
-version: 0.0.2
+version: 0.0.5-dev
+groups: cpp
 dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp
 library: true
 dependencies:
-  codeql/cpp-upgrades: 0.0.2
+  codeql/cpp-upgrades: ^0.0.3

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

@@ -237,7 +237,7 @@ class Class extends UserType {
     exists(ClassDerivation cd | cd.getBaseClass() = base |
       result =
         this.accessOfBaseMemberMulti(cd.getDerivedClass(),
-          fieldInBase.accessInDirectDerived(cd.getASpecifier().(AccessSpecifier)))
+          fieldInBase.accessInDirectDerived(cd.getASpecifier()))
     )
   }
 
@@ -261,8 +261,7 @@ class Class extends UserType {
    * includes the case of `base` = `this`.
    */
   AccessSpecifier accessOfBaseMember(Declaration member) {
-    result =
-      this.accessOfBaseMember(member.getDeclaringType(), member.getASpecifier().(AccessSpecifier))
+    result = this.accessOfBaseMember(member.getDeclaringType(), member.getASpecifier())
   }
 
   /**
@@ -319,7 +318,7 @@ class Class extends UserType {
     exists(Type t | t = this.getAFieldSubobjectType().getUnspecifiedType() |
       // Note: Overload resolution is not implemented -- all copy
       // constructors are considered equal.
-      this.cannotAccessCopyConstructorOnAny(t.(Class))
+      this.cannotAccessCopyConstructorOnAny(t)
     )
     or
     // - T has direct or virtual base class that cannot be copied (has deleted,
@@ -392,7 +391,7 @@ class Class extends UserType {
     exists(Type t | t = this.getAFieldSubobjectType().getUnspecifiedType() |
       // Note: Overload resolution is not implemented -- all copy assignment
       // operators are considered equal.
-      this.cannotAccessCopyAssignmentOperatorOnAny(t.(Class))
+      this.cannotAccessCopyAssignmentOperatorOnAny(t)
     )
     or
     exists(Class c | c = this.getADirectOrVirtualBase() |

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

@@ -490,8 +490,7 @@ class AccessHolder extends Declaration, TAccessHolder {
    */
   pragma[inline]
   predicate canAccessMember(Declaration member, Class derived) {
-    this.couldAccessMember(member.getDeclaringType(), member.getASpecifier().(AccessSpecifier),
-      derived)
+    this.couldAccessMember(member.getDeclaringType(), member.getASpecifier(), derived)
   }
 
   /**

cpp/ql/lib/semmle/code/cpp/commons/Dependency.qll

@@ -275,7 +275,7 @@ private predicate dependsOnDeclarationEntry(Element src, DeclarationEntry dest)
     dependsOnTransitive(src, mid) and
     not mid instanceof Type and
     not mid instanceof EnumConstant and
-    getDeclarationEntries(mid, dest.(DeclarationEntry)) and
+    getDeclarationEntries(mid, dest) and
     not dest instanceof TypeDeclarationEntry
   )
   or
@@ -283,9 +283,9 @@ private predicate dependsOnDeclarationEntry(Element src, DeclarationEntry dest)
     // dependency from a Type / Variable / Function use -> any (visible) definition
     dependsOnTransitive(src, mid) and
     not mid instanceof EnumConstant and
-    getDeclarationEntries(mid, dest.(DeclarationEntry)) and
+    getDeclarationEntries(mid, dest) and
     // must be definition
-    dest.(DeclarationEntry).isDefinition()
+    dest.isDefinition()
   )
 }
 
@@ -307,7 +307,7 @@ private predicate dependsOnFull(DependsSource src, Symbol dest, int category) {
     // dependency from a Variable / Function use -> non-visible definition (link time)
     dependsOnTransitive(src, mid) and
     not mid instanceof EnumConstant and
-    getDeclarationEntries(mid, dest.(DeclarationEntry)) and
+    getDeclarationEntries(mid, dest) and
     not dest instanceof TypeDeclarationEntry and
     // must be definition
     dest.(DeclarationEntry).isDefinition() and

cpp/ql/lib/semmle/code/cpp/commons/Exclusions.qll

@@ -81,8 +81,8 @@ predicate functionContainsPreprocCode(Function f) {
 }
 
 /**
- * Holds if `e` is completely or partially from a macro definition, as opposed
- * to being passed in as an argument.
+ * Holds if `e` is completely or partially from a macro invocation `mi`, as
+ * opposed to being passed in as an argument.
  *
  * In the following example, the call to `f` is from a macro definition,
  * while `y`, `+`, `1`, and `;` are not. This assumes that no identifier apart
@@ -93,8 +93,8 @@ predicate functionContainsPreprocCode(Function f) {
  *   M(y + 1);
  * ```
  */
-predicate isFromMacroDefinition(Element e) {
-  exists(MacroInvocation mi, Location eLocation, Location miLocation |
+private predicate isFromMacroInvocation(Element e, MacroInvocation mi) {
+  exists(Location eLocation, Location miLocation |
     mi.getAnExpandedElement() = e and
     eLocation = e.getLocation() and
     miLocation = mi.getLocation() and
@@ -109,3 +109,36 @@ predicate isFromMacroDefinition(Element e) {
     eLocation.getEndColumn() >= miLocation.getEndColumn()
   )
 }
+
+/**
+ * Holds if `e` is completely or partially from a macro definition, as opposed
+ * to being passed in as an argument.
+ *
+ * In the following example, the call to `f` is from a macro definition,
+ * while `y`, `+`, `1`, and `;` are not. This assumes that no identifier apart
+ * from `M` refers to a macro.
+ * ```
+ * #define M(x) f(x)
+ * ...
+ *   M(y + 1);
+ * ```
+ */
+predicate isFromMacroDefinition(Element e) { isFromMacroInvocation(e, _) }
+
+/**
+ * Holds if `e` is completely or partially from a _system macro_ definition, as
+ * opposed to being passed in as an argument. A system macro is a macro whose
+ * definition is outside the source directory of the database.
+ *
+ * If the system macro is invoked through a non-system macro, then this
+ * predicate does not hold.
+ *
+ * See also `isFromMacroDefinition`.
+ */
+predicate isFromSystemMacroDefinition(Element e) {
+  exists(MacroInvocation mi |
+    isFromMacroInvocation(e, mi) and
+    // Has no relative path in the database, meaning it's a system file.
+    not exists(mi.getMacro().getFile().getRelativePath())
+  )
+}

cpp/ql/lib/semmle/code/cpp/commons/NullTermination.qll

@@ -3,17 +3,33 @@ private import semmle.code.cpp.models.interfaces.ArrayFunction
 private import semmle.code.cpp.models.implementations.Strcat
 import semmle.code.cpp.dataflow.DataFlow
 
-private predicate mayAddNullTerminatorHelper(Expr e, VariableAccess va, Expr e0) {
-  exists(StackVariable v0, Expr val |
-    exprDefinition(v0, e, val) and
-    val.getAChild*() = va and
-    mayAddNullTerminator(e0, v0.getAnAccess())
+/**
+ * Holds if the expression `e` assigns something including `va` to a
+ * stack variable `v0`.
+ */
+private predicate mayAddNullTerminatorHelper(Expr e, VariableAccess va, StackVariable v0) {
+  exists(Expr val |
+    exprDefinition(v0, e, val) and // `e` is `v0 := val`
+    val.getAChild*() = va
+  )
+}
+
+bindingset[n1, n2]
+private predicate controlFlowNodeSuccessorTransitive(ControlFlowNode n1, ControlFlowNode n2) {
+  exists(BasicBlock bb1, int pos1, BasicBlock bb2, int pos2 |
+    pragma[only_bind_into](bb1).getNode(pos1) = n1 and
+    pragma[only_bind_into](bb2).getNode(pos2) = n2 and
+    (
+      bb1 = bb2 and pos1 < pos2
+      or
+      bb1.getASuccessor+() = bb2
+    )
   )
 }
 
 /**
- * Holds if the expression `e` may add a null terminator to the string in
- * variable `v`.
+ * Holds if the expression `e` may add a null terminator to the string
+ * accessed by `va`.
  */
 predicate mayAddNullTerminator(Expr e, VariableAccess va) {
   // Assignment: dereferencing or array access
@@ -30,14 +46,10 @@ predicate mayAddNullTerminator(Expr e, VariableAccess va) {
   )
   or
   // Assignment to another stack variable
-  exists(Expr e0, BasicBlock bb, int pos, BasicBlock bb0, int pos0 |
-    mayAddNullTerminatorHelper(e, va, e0) and
-    bb.getNode(pos) = e and
-    bb0.getNode(pos0) = e0
-  |
-    bb = bb0 and pos < pos0
-    or
-    bb.getASuccessor+() = bb0
+  exists(StackVariable v0, Expr e0 |
+    mayAddNullTerminatorHelper(e, va, v0) and
+    mayAddNullTerminator(pragma[only_bind_into](e0), pragma[only_bind_into](v0.getAnAccess())) and
+    controlFlowNodeSuccessorTransitive(e, e0)
   )
   or
   // Assignment to non-stack variable
@@ -119,14 +131,9 @@ predicate variableMustBeNullTerminated(VariableAccess va) {
       variableMustBeNullTerminated(use) and
       // Simplified: check that `p` may not be null terminated on *any*
       // path to `use` (including the one found via `parameterUsePair`)
-      not exists(Expr e, BasicBlock bb1, int pos1, BasicBlock bb2, int pos2 |
-        mayAddNullTerminator(e, p.getAnAccess()) and
-        bb1.getNode(pos1) = e and
-        bb2.getNode(pos2) = use
-      |
-        bb1 = bb2 and pos1 < pos2
-        or
-        bb1.getASuccessor+() = bb2
+      not exists(Expr e |
+        mayAddNullTerminator(pragma[only_bind_into](e), p.getAnAccess()) and
+        controlFlowNodeSuccessorTransitive(e, use)
       )
     )
   )

cpp/ql/lib/semmle/code/cpp/commons/Printf.qll

@@ -6,6 +6,85 @@ import semmle.code.cpp.Type
 import semmle.code.cpp.commons.CommonType
 import semmle.code.cpp.commons.StringAnalysis
 import semmle.code.cpp.models.interfaces.FormattingFunction
+private import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+private import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
+
+private newtype TBufferWriteEstimationReason =
+  TNoSpecifiedEstimateReason() or
+  TTypeBoundsAnalysis() or
+  TValueFlowAnalysis()
+
+/**
+ * A reason for a specific buffer write size estimate.
+ */
+abstract class BufferWriteEstimationReason extends TBufferWriteEstimationReason {
+  /**
+   * Returns the name of the concrete class.
+   */
+  abstract string toString();
+
+  /**
+   * Returns a human readable representation of this reason.
+   */
+  abstract string getDescription();
+
+  /**
+   * Combine estimate reasons. Used to give a reason for the size of a format string
+   * conversion given reasons coming from its individual specifiers.
+   */
+  abstract BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other);
+}
+
+/**
+ * No particular reason given. This is currently used for backward compatibility so that
+ * classes derived from BufferWrite and overriding `getMaxData/0` still work with the
+ * queries as intended.
+ */
+class NoSpecifiedEstimateReason extends BufferWriteEstimationReason, TNoSpecifiedEstimateReason {
+  override string toString() { result = "NoSpecifiedEstimateReason" }
+
+  override string getDescription() { result = "no reason specified" }
+
+  override BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
+    // this reason should not be used in format specifiers, so it should not be combined
+    // with other reasons
+    none()
+  }
+}
+
+/**
+ * The estimation comes from rough bounds just based on the type (e.g.
+ * `0 <= x < 2^32` for an unsigned 32 bit integer).
+ */
+class TypeBoundsAnalysis extends BufferWriteEstimationReason, TTypeBoundsAnalysis {
+  override string toString() { result = "TypeBoundsAnalysis" }
+
+  override string getDescription() { result = "based on type bounds" }
+
+  override BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
+    other != TNoSpecifiedEstimateReason() and result = TTypeBoundsAnalysis()
+  }
+}
+
+/**
+ * The estimation comes from non trivial bounds found via actual flow analysis.
+ * For example
+ * ```
+ * unsigned u = x;
+ * if (u < 1000) {
+ *    //...  <- estimation done here based on u
+ * }
+ * ```
+ */
+class ValueFlowAnalysis extends BufferWriteEstimationReason, TValueFlowAnalysis {
+  override string toString() { result = "ValueFlowAnalysis" }
+
+  override string getDescription() { result = "based on flow analysis of value bounds" }
+
+  override BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
+    other != TNoSpecifiedEstimateReason() and result = other
+  }
+}
 
 class PrintfFormatAttribute extends FormatAttribute {
   PrintfFormatAttribute() { this.getArchetype() = ["printf", "__printf__"] }
@@ -175,9 +254,7 @@ class FormattingFunctionCall extends Expr {
   /**
    * Gets the index at which the format string occurs in the argument list.
    */
-  int getFormatParameterIndex() {
-    result = this.getTarget().(FormattingFunction).getFormatParameterIndex()
-  }
+  int getFormatParameterIndex() { result = this.getTarget().getFormatParameterIndex() }
 
   /**
    * Gets the format expression used in this call.
@@ -191,7 +268,7 @@ class FormattingFunctionCall extends Expr {
     exists(int i |
       result = this.getArgument(i) and
       n >= 0 and
-      n = i - this.getTarget().(FormattingFunction).getFirstFormatArgumentIndex()
+      n = i - this.getTarget().getFirstFormatArgumentIndex()
     )
   }
 
@@ -251,7 +328,7 @@ class FormattingFunctionCall extends Expr {
   int getNumFormatArgument() {
     result = count(this.getFormatArgument(_)) and
     // format arguments must be known
-    exists(this.getTarget().(FormattingFunction).getFirstFormatArgumentIndex())
+    exists(this.getTarget().getFirstFormatArgumentIndex())
   }
 
   /**
@@ -270,6 +347,18 @@ class FormattingFunctionCall extends Expr {
   }
 }
 
+/**
+ * Gets the number of digits required to represent the integer represented by `f`.
+ *
+ * `f` is assumed to be nonnegative.
+ */
+bindingset[f]
+private int lengthInBase10(float f) {
+  f = 0 and result = 1
+  or
+  result = f.log10().floor() + 1
+}
+
 /**
  * A class to represent format strings that occur as arguments to invocations of formatting functions.
  */
@@ -289,35 +378,27 @@ class FormatLiteral extends Literal {
    * a `char *` (either way, `%S` will have the opposite meaning).
    * DEPRECATED: Use getDefaultCharType() instead.
    */
-  deprecated predicate isWideCharDefault() {
-    this.getUse().getTarget().(FormattingFunction).isWideCharDefault()
-  }
+  deprecated predicate isWideCharDefault() { this.getUse().getTarget().isWideCharDefault() }
 
   /**
    * Gets the default character type expected for `%s` by this format literal.  Typically
    * `char` or `wchar_t`.
    */
-  Type getDefaultCharType() {
-    result = this.getUse().getTarget().(FormattingFunction).getDefaultCharType()
-  }
+  Type getDefaultCharType() { result = this.getUse().getTarget().getDefaultCharType() }
 
   /**
    * Gets the non-default character type expected for `%S` by this format literal.  Typically
    * `wchar_t` or `char`.  On some snapshots there may be multiple results where we can't tell
    * which is correct for a particular function.
    */
-  Type getNonDefaultCharType() {
-    result = this.getUse().getTarget().(FormattingFunction).getNonDefaultCharType()
-  }
+  Type getNonDefaultCharType() { result = this.getUse().getTarget().getNonDefaultCharType() }
 
   /**
    * Gets the wide character type for this format literal.  This is usually `wchar_t`.  On some
    * snapshots there may be multiple results where we can't tell which is correct for a
    * particular function.
    */
-  Type getWideCharType() {
-    result = this.getUse().getTarget().(FormattingFunction).getWideCharType()
-  }
+  Type getWideCharType() { result = this.getUse().getTarget().getWideCharType() }
 
   /**
    * Holds if this `FormatLiteral` is in a context that supports
@@ -896,7 +977,7 @@ class FormatLiteral extends Literal {
     exists(string len, string conv |
       this.parseConvSpec(n, _, _, _, _, _, len, conv) and
       (len != "l" and len != "w" and len != "h") and
-      this.getUse().getTarget().(FormattingFunction).getFormatCharType().getSize() > 1 and // wide function
+      this.getUse().getTarget().getFormatCharType().getSize() > 1 and // wide function
       (
         conv = "c" and
         result = this.getNonDefaultCharType()
@@ -986,7 +1067,14 @@ class FormatLiteral extends Literal {
    * conversion specifier of this format string; has no result if this cannot
    * be determined.
    */
-  int getMaxConvertedLength(int n) {
+  int getMaxConvertedLength(int n) { result = max(getMaxConvertedLength(n, _)) }
+
+  /**
+   * Gets the maximum length of the string that can be produced by the nth
+   * conversion specifier of this format string, specifying the estimation reason;
+   * has no result if this cannot be determined.
+   */
+  int getMaxConvertedLength(int n, BufferWriteEstimationReason reason) {
     exists(int len |
       (
         (
@@ -998,10 +1086,12 @@ class FormatLiteral extends Literal {
       ) and
       (
         this.getConversionChar(n) = "%" and
-        len = 1
+        len = 1 and
+        reason = TValueFlowAnalysis()
         or
         this.getConversionChar(n).toLowerCase() = "c" and
-        len = 1 // e.g. 'a'
+        len = 1 and
+        reason = TValueFlowAnalysis() // e.g. 'a'
         or
         this.getConversionChar(n).toLowerCase() = "f" and
         exists(int dot, int afterdot |
@@ -1015,7 +1105,8 @@ class FormatLiteral extends Literal {
             afterdot = 6
           ) and
           len = 1 + 309 + dot + afterdot
-        ) // e.g. -1e308="-100000"...
+        ) and
+        reason = TTypeBoundsAnalysis() // e.g. -1e308="-100000"...
         or
         this.getConversionChar(n).toLowerCase() = "e" and
         exists(int dot, int afterdot |
@@ -1029,7 +1120,8 @@ class FormatLiteral extends Literal {
             afterdot = 6
           ) and
           len = 1 + 1 + dot + afterdot + 1 + 1 + 3
-        ) // -1e308="-1.000000e+308"
+        ) and
+        reason = TTypeBoundsAnalysis() // -1e308="-1.000000e+308"
         or
         this.getConversionChar(n).toLowerCase() = "g" and
         exists(int dot, int afterdot |
@@ -1052,42 +1144,79 @@ class FormatLiteral extends Literal {
           //       (e.g. 123456, 0.000123456 are just OK)
           //       so case %f can be at most P characters + 4 zeroes, sign, dot = P + 6
           len = (afterdot.maximum(1) + 6).maximum(1 + 1 + dot + afterdot + 1 + 1 + 3)
-        ) // (e.g. "-1.59203e-319")
+        ) and
+        reason = TTypeBoundsAnalysis() // (e.g. "-1.59203e-319")
         or
         this.getConversionChar(n).toLowerCase() = ["d", "i"] and
         // e.g. -2^31 = "-2147483648"
-        exists(int sizeBits |
-          sizeBits =
-            min(int bits |
-              bits = this.getIntegralDisplayType(n).getSize() * 8
-              or
-              exists(IntegralType t |
-                t = this.getUse().getConversionArgument(n).getType().getUnderlyingType()
-              |
-                t.isSigned() and bits = t.getSize() * 8
-              )
-            ) and
-          len = 1 + ((sizeBits - 1) / 10.0.log2()).ceil()
-          // this calculation is as %u (below) only we take out the sign bit (- 1) and allow a whole
-          // character for it to be expressed as '-'.
+        exists(float typeBasedBound, float valueBasedBound |
+          // The first case handles length sub-specifiers
+          // Subtract one in the exponent because one bit is for the sign.
+          // Add 1 to account for the possible sign in the output.
+          typeBasedBound =
+            1 + lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8 - 1)) and
+          // The second case uses range analysis to deduce a length that's shorter than the length
+          // of the number -2^31.
+          exists(Expr arg, float lower, float upper, float typeLower, float typeUpper |
+            arg = this.getUse().getConversionArgument(n) and
+            lower = lowerBound(arg.getFullyConverted()) and
+            upper = upperBound(arg.getFullyConverted()) and
+            typeLower = exprMinVal(arg.getFullyConverted()) and
+            typeUpper = exprMaxVal(arg.getFullyConverted())
+          |
+            valueBasedBound =
+              max(int cand |
+                // Include the sign bit in the length if it can be negative
+                (
+                  if lower < 0
+                  then cand = 1 + lengthInBase10(lower.abs())
+                  else cand = lengthInBase10(lower)
+                )
+                or
+                (
+                  if upper < 0
+                  then cand = 1 + lengthInBase10(upper.abs())
+                  else cand = lengthInBase10(upper)
+                )
+              ) and
+            (
+              if lower > typeLower or upper < typeUpper
+              then reason = TValueFlowAnalysis()
+              else reason = TTypeBoundsAnalysis()
+            )
+          ) and
+          len = valueBasedBound.minimum(typeBasedBound)
         )
         or
         this.getConversionChar(n).toLowerCase() = "u" and
         // e.g. 2^32 - 1 = "4294967295"
-        exists(int sizeBits |
-          sizeBits =
-            min(int bits |
-              bits = this.getIntegralDisplayType(n).getSize() * 8
-              or
-              exists(IntegralType t |
-                t = this.getUse().getConversionArgument(n).getType().getUnderlyingType()
-              |
-                t.isUnsigned() and bits = t.getSize() * 8
-              )
-            ) and
-          len = (sizeBits / 10.0.log2()).ceil()
-          // convert the size from bits to decimal characters, and round up as you can't have
-          // fractional characters (10.0.log2() is the number of bits expressed per decimal character)
+        exists(float typeBasedBound, float valueBasedBound |
+          // The first case handles length sub-specifiers
+          typeBasedBound = lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8) - 1) and
+          // The second case uses range analysis to deduce a length that's shorter than
+          // the length of the number 2^31 - 1.
+          exists(Expr arg, float lower, float upper, float typeLower, float typeUpper |
+            arg = this.getUse().getConversionArgument(n) and
+            lower = lowerBound(arg.getFullyConverted()) and
+            upper = upperBound(arg.getFullyConverted()) and
+            typeLower = exprMinVal(arg.getFullyConverted()) and
+            typeUpper = exprMaxVal(arg.getFullyConverted())
+          |
+            valueBasedBound =
+              lengthInBase10(max(float cand |
+                  // If lower can be negative we use `(unsigned)-1` as the candidate value.
+                  lower < 0 and
+                  cand = 2.pow(any(IntType t | t.isUnsigned()).getSize() * 8)
+                  or
+                  cand = upper
+                )) and
+            (
+              if lower > typeLower or upper < typeUpper
+              then reason = TValueFlowAnalysis()
+              else reason = TTypeBoundsAnalysis()
+            )
+          ) and
+          len = valueBasedBound.minimum(typeBasedBound)
         )
         or
         this.getConversionChar(n).toLowerCase() = "x" and
@@ -1107,7 +1236,8 @@ class FormatLiteral extends Literal {
           (
             if this.hasAlternateFlag(n) then len = 2 + baseLen else len = baseLen // "0x"
           )
-        )
+        ) and
+        reason = TTypeBoundsAnalysis()
         or
         this.getConversionChar(n).toLowerCase() = "p" and
         exists(PointerType ptrType, int baseLen |
@@ -1116,7 +1246,8 @@ class FormatLiteral extends Literal {
           (
             if this.hasAlternateFlag(n) then len = 2 + baseLen else len = baseLen // "0x"
           )
-        )
+        ) and
+        reason = TValueFlowAnalysis()
         or
         this.getConversionChar(n).toLowerCase() = "o" and
         // e.g. 2^32 - 1 = "37777777777"
@@ -1135,14 +1266,16 @@ class FormatLiteral extends Literal {
           (
             if this.hasAlternateFlag(n) then len = 1 + baseLen else len = baseLen // "0"
           )
-        )
+        ) and
+        reason = TTypeBoundsAnalysis()
         or
         this.getConversionChar(n).toLowerCase() = "s" and
         len =
           min(int v |
             v = this.getPrecision(n) or
             v = this.getUse().getFormatArgument(n).(AnalysedString).getMaxLength() - 1 // (don't count null terminator)
-          )
+          ) and
+        reason = TValueFlowAnalysis()
       )
     )
   }
@@ -1154,10 +1287,19 @@ class FormatLiteral extends Literal {
    * determining whether a buffer overflow is caused by long float to string
    * conversions.
    */
-  int getMaxConvertedLengthLimited(int n) {
+  int getMaxConvertedLengthLimited(int n) { result = max(getMaxConvertedLengthLimited(n, _)) }
+
+  /**
+   * Gets the maximum length of the string that can be produced by the nth
+   * conversion specifier of this format string, specifying the reason for the
+   * estimation, except that float to string conversions are assumed to be 8
+   * characters.  This is helpful for determining whether a buffer overflow is
+   * caused by long float to string conversions.
+   */
+  int getMaxConvertedLengthLimited(int n, BufferWriteEstimationReason reason) {
     if this.getConversionChar(n).toLowerCase() = "f"
-    then result = this.getMaxConvertedLength(n).minimum(8)
-    else result = this.getMaxConvertedLength(n)
+    then result = this.getMaxConvertedLength(n, reason).minimum(8)
+    else result = this.getMaxConvertedLength(n, reason)
   }
 
   /**
@@ -1197,29 +1339,35 @@ class FormatLiteral extends Literal {
     )
   }
 
-  private int getMaxConvertedLengthAfter(int n) {
+  private int getMaxConvertedLengthAfter(int n, BufferWriteEstimationReason reason) {
     if n = this.getNumConvSpec()
-    then result = this.getConstantSuffix().length() + 1
+    then result = this.getConstantSuffix().length() + 1 and reason = TValueFlowAnalysis()
     else
-      result =
-        this.getConstantPart(n).length() + this.getMaxConvertedLength(n) +
-          this.getMaxConvertedLengthAfter(n + 1)
+      exists(BufferWriteEstimationReason headReason, BufferWriteEstimationReason tailReason |
+        result =
+          this.getConstantPart(n).length() + this.getMaxConvertedLength(n, headReason) +
+            this.getMaxConvertedLengthAfter(n + 1, tailReason) and
+        reason = headReason.combineWith(tailReason)
+      )
   }
 
-  private int getMaxConvertedLengthAfterLimited(int n) {
+  private int getMaxConvertedLengthAfterLimited(int n, BufferWriteEstimationReason reason) {
     if n = this.getNumConvSpec()
-    then result = this.getConstantSuffix().length() + 1
+    then result = this.getConstantSuffix().length() + 1 and reason = TValueFlowAnalysis()
     else
-      result =
-        this.getConstantPart(n).length() + this.getMaxConvertedLengthLimited(n) +
-          this.getMaxConvertedLengthAfterLimited(n + 1)
+      exists(BufferWriteEstimationReason headReason, BufferWriteEstimationReason tailReason |
+        result =
+          this.getConstantPart(n).length() + this.getMaxConvertedLengthLimited(n, headReason) +
+            this.getMaxConvertedLengthAfterLimited(n + 1, tailReason) and
+        reason = headReason.combineWith(tailReason)
+      )
   }
 
   /**
    * Gets the maximum length of the string that can be produced by this format
    * string.  Has no result if this cannot be determined.
    */
-  int getMaxConvertedLength() { result = this.getMaxConvertedLengthAfter(0) }
+  int getMaxConvertedLength() { result = this.getMaxConvertedLengthAfter(0, _) }
 
   /**
    * Gets the maximum length of the string that can be produced by this format
@@ -1227,5 +1375,24 @@ class FormatLiteral extends Literal {
    * characters.  This is helpful for determining whether a buffer overflow
    * is caused by long float to string conversions.
    */
-  int getMaxConvertedLengthLimited() { result = this.getMaxConvertedLengthAfterLimited(0) }
+  int getMaxConvertedLengthLimited() { result = this.getMaxConvertedLengthAfterLimited(0, _) }
+
+  /**
+   * Gets the maximum length of the string that can be produced by this format
+   * string, specifying the reason for the estimate. Has no result if no estimate
+   * can be found.
+   */
+  int getMaxConvertedLengthWithReason(BufferWriteEstimationReason reason) {
+    result = this.getMaxConvertedLengthAfter(0, reason)
+  }
+
+  /**
+   * Gets the maximum length of the string that can be produced by this format
+   * string, specifying the reason for the estimate, except that float to string
+   * conversions are assumed to be 8 characters.  This is helpful for determining
+   * whether a buffer overflow is caused by long float to string conversions.
+   */
+  int getMaxConvertedLengthLimitedWithReason(BufferWriteEstimationReason reason) {
+    result = this.getMaxConvertedLengthAfterLimited(0, reason)
+  }
 }

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

@@ -25,7 +25,7 @@ predicate definitionUsePair(SemanticStackVariable var, Expr def, Expr use) {
  * Holds if the definition `def` of some stack variable can reach `node`, which
  * is a definition or use, without crossing definitions of the same variable.
  */
-predicate definitionReaches(Expr def, Expr node) { def.(Def).reaches(true, _, node.(DefOrUse)) }
+predicate definitionReaches(Expr def, Expr node) { def.(Def).reaches(true, _, node) }
 
 private predicate hasAddressOfAccess(SemanticStackVariable var) {
   var.getAnAccess().isAddressOfAccessNonConst()

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

@@ -62,7 +62,7 @@ class SsaDefinition extends ControlFlowNodeBase {
   BasicBlock getBasicBlock() { result.contains(this.getDefinition()) }
 
   /** Holds if this definition is a phi node for variable `v`. */
-  predicate isPhiNode(StackVariable v) { exists(StandardSSA x | x.phi_node(v, this.(BasicBlock))) }
+  predicate isPhiNode(StackVariable v) { exists(StandardSSA x | x.phi_node(v, this)) }
 
   /** Gets the location of this definition. */
   Location getLocation() { result = this.(ControlFlowNode).getLocation() }

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

@@ -292,7 +292,7 @@ library class SSAHelper extends int {
    */
   cached
   string toString(ControlFlowNode node, StackVariable v) {
-    if phi_node(v, node.(BasicBlock))
+    if phi_node(v, node)
     then result = "SSA phi(" + v.getName() + ")"
     else (
       ssa_defn(v, node, _, _) and result = "SSA def(" + v.getName() + ")"

cpp/ql/lib/semmle/code/cpp/controlflow/internal/CFG.qll

@@ -231,7 +231,7 @@ private class PostOrderInitializer extends Initializer {
       or
       this.getDeclaration() = for.getRangeVariable()
       or
-      this.getDeclaration() = for.getBeginEndDeclaration().(DeclStmt).getADeclaration()
+      this.getDeclaration() = for.getBeginEndDeclaration().getADeclaration()
     )
   }
 }
@@ -1143,7 +1143,7 @@ private class ExceptionSource extends Node {
       this.reachesParent(mid) and
       not mid = any(TryStmt try).getStmt() and
       not mid = any(MicrosoftTryStmt try).getStmt() and
-      parent = mid.(Node).getParentNode()
+      parent = mid.getParentNode()
     )
   }
 

cpp/ql/lib/semmle/code/cpp/controlflow/internal/ConstantExprs.qll

@@ -484,7 +484,7 @@ library class ExprEvaluator extends int {
       this.interestingInternal(e, req, true) and
       (
         result = req.(CompileTimeConstantInt).getIntValue() or
-        result = this.getCompoundValue(e, req.(CompileTimeVariableExpr))
+        result = this.getCompoundValue(e, req)
       ) and
       (
         req.getUnderlyingType().(IntegralType).isSigned() or
@@ -611,7 +611,7 @@ library class ExprEvaluator extends int {
       or
       exists(AssignExpr req | req = val | result = this.getValueInternal(e, req.getRValue()))
       or
-      result = this.getVariableValue(e, val.(VariableAccess))
+      result = this.getVariableValue(e, val)
       or
       exists(FunctionCall call | call = val and not callWithMultipleTargets(call) |
         result = this.getFunctionValue(call.getTarget())
@@ -626,9 +626,9 @@ library class ExprEvaluator extends int {
       // All assignments must have the same int value
       result =
         unique(Expr value |
-          value = v.getAnAssignedValue() and not ignoreVariableAssignment(e, v, value)
+          value = v.getAnAssignedValue() and not this.ignoreVariableAssignment(e, v, value)
         |
-          getValueInternalNonSubExpr(value)
+          this.getValueInternalNonSubExpr(value)
         )
     )
   }
@@ -663,7 +663,7 @@ library class ExprEvaluator extends int {
     this.interestingInternal(_, req, false) and
     (
       result = req.(CompileTimeConstantInt).getIntValue() or
-      result = this.getCompoundValueNonSubExpr(req.(CompileTimeVariableExpr))
+      result = this.getCompoundValueNonSubExpr(req)
     ) and
     (
       req.getUnderlyingType().(IntegralType).isSigned() or
@@ -787,7 +787,7 @@ library class ExprEvaluator extends int {
       or
       exists(AssignExpr req | req = val | result = this.getValueInternalNonSubExpr(req.getRValue()))
       or
-      result = this.getVariableValueNonSubExpr(val.(VariableAccess))
+      result = this.getVariableValueNonSubExpr(val)
       or
       exists(FunctionCall call | call = val and not callWithMultipleTargets(call) |
         result = this.getFunctionValue(call.getTarget())

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

@@ -1,4 +1,6 @@
 private import cpp
+private import semmle.code.cpp.dataflow.internal.DataFlowPrivate
+private import semmle.code.cpp.dataflow.internal.DataFlowUtil
 
 /**
  * Gets a function that might be called by `call`.
@@ -63,3 +65,17 @@ predicate mayBenefitFromCallContext(Call call, Function f) { none() }
  * restricted to those `call`s for which a context might make a difference.
  */
 Function viableImplInCallContext(Call call, Call ctx) { none() }
+
+/** A parameter position represented by an integer. */
+class ParameterPosition extends int {
+  ParameterPosition() { any(ParameterNode p).isParameterOf(_, this) }
+}
+
+/** An argument position represented by an integer. */
+class ArgumentPosition extends int {
+  ArgumentPosition() { any(ArgumentNode a).argumentOf(_, this) }
+}
+
+/** Holds if arguments at position `apos` match parameters at position `ppos`. */
+pragma[inline]
+predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos = apos }

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

@@ -2,6 +2,42 @@ private import DataFlowImplSpecific::Private
 private import DataFlowImplSpecific::Public
 import Cached
 
+module DataFlowImplCommonPublic {
+  private newtype TFlowFeature =
+    TFeatureHasSourceCallContext() or
+    TFeatureHasSinkCallContext() or
+    TFeatureEqualSourceSinkCallContext()
+
+  /** A flow configuration feature for use in `Configuration::getAFeature()`. */
+  class FlowFeature extends TFlowFeature {
+    string toString() { none() }
+  }
+
+  /**
+   * A flow configuration feature that implies that sources have some existing
+   * call context.
+   */
+  class FeatureHasSourceCallContext extends FlowFeature, TFeatureHasSourceCallContext {
+    override string toString() { result = "FeatureHasSourceCallContext" }
+  }
+
+  /**
+   * A flow configuration feature that implies that sinks have some existing
+   * call context.
+   */
+  class FeatureHasSinkCallContext extends FlowFeature, TFeatureHasSinkCallContext {
+    override string toString() { result = "FeatureHasSinkCallContext" }
+  }
+
+  /**
+   * A flow configuration feature that implies that source-sink pairs have some
+   * shared existing call context.
+   */
+  class FeatureEqualSourceSinkCallContext extends FlowFeature, TFeatureEqualSourceSinkCallContext {
+    override string toString() { result = "FeatureEqualSourceSinkCallContext" }
+  }
+}
+
 /**
  * The cost limits for the `AccessPathFront` to `AccessPathApprox` expansion.
  *
@@ -26,6 +62,18 @@ predicate accessPathCostLimits(int apLimit, int tupleLimit) {
   tupleLimit = 1000
 }
 
+/**
+ * Holds if `arg` is an argument of `call` with an argument position that matches
+ * parameter position `ppos`.
+ */
+pragma[noinline]
+predicate argumentPositionMatch(DataFlowCall call, ArgNode arg, ParameterPosition ppos) {
+  exists(ArgumentPosition apos |
+    arg.argumentOf(call, apos) and
+    parameterMatch(ppos, apos)
+  )
+}
+
 /**
  * Provides a simple data-flow analysis for resolving lambda calls. The analysis
  * currently excludes read-steps, store-steps, and flow-through.
@@ -35,25 +83,27 @@ predicate accessPathCostLimits(int apLimit, int tupleLimit) {
  * calls. For this reason, we cannot reuse the code from `DataFlowImpl.qll` directly.
  */
 private module LambdaFlow {
-  private predicate viableParamNonLambda(DataFlowCall call, int i, ParamNode p) {
-    p.isParameterOf(viableCallable(call), i)
+  pragma[noinline]
+  private predicate viableParamNonLambda(DataFlowCall call, ParameterPosition ppos, ParamNode p) {
+    p.isParameterOf(viableCallable(call), ppos)
   }
 
-  private predicate viableParamLambda(DataFlowCall call, int i, ParamNode p) {
-    p.isParameterOf(viableCallableLambda(call, _), i)
+  pragma[noinline]
+  private predicate viableParamLambda(DataFlowCall call, ParameterPosition ppos, ParamNode p) {
+    p.isParameterOf(viableCallableLambda(call, _), ppos)
   }
 
   private predicate viableParamArgNonLambda(DataFlowCall call, ParamNode p, ArgNode arg) {
-    exists(int i |
-      viableParamNonLambda(call, i, p) and
-      arg.argumentOf(call, i)
+    exists(ParameterPosition ppos |
+      viableParamNonLambda(call, ppos, p) and
+      argumentPositionMatch(call, arg, ppos)
     )
   }
 
   private predicate viableParamArgLambda(DataFlowCall call, ParamNode p, ArgNode arg) {
-    exists(int i |
-      viableParamLambda(call, i, p) and
-      arg.argumentOf(call, i)
+    exists(ParameterPosition ppos |
+      viableParamLambda(call, ppos, p) and
+      argumentPositionMatch(call, arg, ppos)
     )
   }
 
@@ -251,7 +301,7 @@ private module Cached {
   predicate forceCachingInSameStage() { any() }
 
   cached
-  predicate nodeEnclosingCallable(Node n, DataFlowCallable c) { c = n.getEnclosingCallable() }
+  predicate nodeEnclosingCallable(Node n, DataFlowCallable c) { c = nodeGetEnclosingCallable(n) }
 
   cached
   predicate callEnclosingCallable(DataFlowCall call, DataFlowCallable c) {
@@ -286,7 +336,7 @@ private module Cached {
     or
     exists(ArgNode arg |
       result.(PostUpdateNode).getPreUpdateNode() = arg and
-      arg.argumentOf(call, k.(ParamUpdateReturnKind).getPosition())
+      arg.argumentOf(call, k.(ParamUpdateReturnKind).getAMatchingArgumentPosition())
     )
   }
 
@@ -294,7 +344,7 @@ private module Cached {
   predicate returnNodeExt(Node n, ReturnKindExt k) {
     k = TValueReturn(n.(ReturnNode).getKind())
     or
-    exists(ParamNode p, int pos |
+    exists(ParamNode p, ParameterPosition pos |
       parameterValueFlowsToPreUpdate(p, n) and
       p.isParameterOf(_, pos) and
       k = TParamUpdate(pos)
@@ -316,13 +366,13 @@ private module Cached {
   }
 
   cached
-  predicate parameterNode(Node n, DataFlowCallable c, int i) {
-    n.(ParameterNode).isParameterOf(c, i)
+  predicate parameterNode(Node p, DataFlowCallable c, ParameterPosition pos) {
+    isParameterNode(p, c, pos)
   }
 
   cached
-  predicate argumentNode(Node n, DataFlowCall call, int pos) {
-    n.(ArgumentNode).argumentOf(call, pos)
+  predicate argumentNode(Node n, DataFlowCall call, ArgumentPosition pos) {
+    isArgumentNode(n, call, pos)
   }
 
   /**
@@ -340,12 +390,12 @@ private module Cached {
   }
 
   /**
-   * Holds if `p` is the `i`th parameter of a viable dispatch target of `call`.
-   * The instance parameter is considered to have index `-1`.
+   * Holds if `p` is the parameter of a viable dispatch target of `call`,
+   * and `p` has position `ppos`.
    */
   pragma[nomagic]
-  private predicate viableParam(DataFlowCall call, int i, ParamNode p) {
-    p.isParameterOf(viableCallableExt(call), i)
+  private predicate viableParam(DataFlowCall call, ParameterPosition ppos, ParamNode p) {
+    p.isParameterOf(viableCallableExt(call), ppos)
   }
 
   /**
@@ -354,9 +404,9 @@ private module Cached {
    */
   cached
   predicate viableParamArg(DataFlowCall call, ParamNode p, ArgNode arg) {
-    exists(int i |
-      viableParam(call, i, p) and
-      arg.argumentOf(call, i) and
+    exists(ParameterPosition ppos |
+      viableParam(call, ppos, p) and
+      argumentPositionMatch(call, arg, ppos) and
       compatibleTypes(getNodeDataFlowType(arg), getNodeDataFlowType(p))
     )
   }
@@ -828,7 +878,7 @@ private module Cached {
   cached
   newtype TReturnKindExt =
     TValueReturn(ReturnKind kind) or
-    TParamUpdate(int pos) { exists(ParamNode p | p.isParameterOf(_, pos)) }
+    TParamUpdate(ParameterPosition pos) { exists(ParamNode p | p.isParameterOf(_, pos)) }
 
   cached
   newtype TBooleanOption =
@@ -1020,9 +1070,9 @@ class ParamNode extends Node {
 
   /**
    * Holds if this node is the parameter of callable `c` at the specified
-   * (zero-based) position.
+   * position.
    */
-  predicate isParameterOf(DataFlowCallable c, int i) { parameterNode(this, c, i) }
+  predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { parameterNode(this, c, pos) }
 }
 
 /** A data-flow node that represents a call argument. */
@@ -1030,7 +1080,9 @@ class ArgNode extends Node {
   ArgNode() { argumentNode(this, _, _) }
 
   /** Holds if this argument occurs at the given position in the given call. */
-  final predicate argumentOf(DataFlowCall call, int pos) { argumentNode(this, call, pos) }
+  final predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
+    argumentNode(this, call, pos)
+  }
 }
 
 /**
@@ -1076,11 +1128,14 @@ class ValueReturnKind extends ReturnKindExt, TValueReturn {
 }
 
 class ParamUpdateReturnKind extends ReturnKindExt, TParamUpdate {
-  private int pos;
+  private ParameterPosition pos;
 
   ParamUpdateReturnKind() { this = TParamUpdate(pos) }
 
-  int getPosition() { result = pos }
+  ParameterPosition getPosition() { result = pos }
+
+  pragma[nomagic]
+  ArgumentPosition getAMatchingArgumentPosition() { parameterMatch(pos, result) }
 
   override string toString() { result = "param update " + pos }
 }

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

@@ -9,6 +9,19 @@ private import tainttracking1.TaintTrackingParameter::Private
 private import tainttracking1.TaintTrackingParameter::Public
 
 module Consistency {
+  private newtype TConsistencyConfiguration = MkConsistencyConfiguration()
+
+  /** A class for configuring the consistency queries. */
+  class ConsistencyConfiguration extends TConsistencyConfiguration {
+    string toString() { none() }
+
+    /** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
+    predicate postWithInFlowExclude(Node n) { none() }
+
+    /** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
+    predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
+  }
+
   private class RelevantNode extends Node {
     RelevantNode() {
       this instanceof ArgumentNode or
@@ -31,7 +44,7 @@ module Consistency {
   query predicate uniqueEnclosingCallable(Node n, string msg) {
     exists(int c |
       n instanceof RelevantNode and
-      c = count(n.getEnclosingCallable()) and
+      c = count(nodeGetEnclosingCallable(n)) and
       c != 1 and
       msg = "Node should have one enclosing callable but has " + c + "."
     )
@@ -85,13 +98,13 @@ module Consistency {
   }
 
   query predicate parameterCallable(ParameterNode p, string msg) {
-    exists(DataFlowCallable c | p.isParameterOf(c, _) and c != p.getEnclosingCallable()) and
+    exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and
     msg = "Callable mismatch for parameter."
   }
 
   query predicate localFlowIsLocal(Node n1, Node n2, string msg) {
     simpleLocalFlowStep(n1, n2) and
-    n1.getEnclosingCallable() != n2.getEnclosingCallable() and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
     msg = "Local flow step does not preserve enclosing callable."
   }
 
@@ -106,7 +119,7 @@ module Consistency {
   query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) {
     isUnreachableInCall(n, call) and
     exists(DataFlowCallable c |
-      c = n.getEnclosingCallable() and
+      c = nodeGetEnclosingCallable(n) and
       not viableCallable(call) = c
     ) and
     msg = "Call context for isUnreachableInCall is inconsistent with call graph."
@@ -120,7 +133,7 @@ module Consistency {
       n.(ArgumentNode).argumentOf(call, _) and
       msg = "ArgumentNode and call does not share enclosing callable."
     ) and
-    n.getEnclosingCallable() != call.getEnclosingCallable()
+    nodeGetEnclosingCallable(n) != call.getEnclosingCallable()
   }
 
   // This predicate helps the compiler forget that in some languages
@@ -151,7 +164,7 @@ module Consistency {
   }
 
   query predicate postIsInSameCallable(PostUpdateNode n, string msg) {
-    n.getEnclosingCallable() != n.getPreUpdateNode().getEnclosingCallable() and
+    nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and
     msg = "PostUpdateNode does not share callable with its pre-update node."
   }
 
@@ -164,7 +177,7 @@ module Consistency {
 
   query predicate argHasPostUpdate(ArgumentNode n, string msg) {
     not hasPost(n) and
-    not isImmutableOrUnobservable(n) and
+    not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and
     msg = "ArgumentNode is missing PostUpdateNode."
   }
 
@@ -177,6 +190,7 @@ module Consistency {
     isPostUpdateNode(n) and
     not clearsContent(n, _) and
     simpleLocalFlowStep(_, n) and
+    not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
     msg = "PostUpdateNode should not be the target of local flow."
   }
 }

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

@@ -2,6 +2,20 @@ private import cpp
 private import DataFlowUtil
 private import DataFlowDispatch
 private import FlowVar
+private import DataFlowImplConsistency
+
+/** Gets the callable in which this node occurs. */
+DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.getEnclosingCallable() }
+
+/** Holds if `p` is a `ParameterNode` of `c` with position `pos`. */
+predicate isParameterNode(ParameterNode p, DataFlowCallable c, ParameterPosition pos) {
+  p.isParameterOf(c, pos)
+}
+
+/** Holds if `arg` is an `ArgumentNode` of `c` with position `pos`. */
+predicate isArgumentNode(ArgumentNode arg, DataFlowCall c, ArgumentPosition pos) {
+  arg.argumentOf(c, pos)
+}
 
 /** Gets the instance argument of a non-static call. */
 private Node getInstanceArgument(Call call) {
@@ -253,27 +267,6 @@ class Unit extends TUnit {
   string toString() { result = "unit" }
 }
 
-/**
- * Holds if `n` does not require a `PostUpdateNode` as it either cannot be
- * modified or its modification cannot be observed, for example if it is a
- * freshly created object that is not saved in a variable.
- *
- * This predicate is only used for consistency checks.
- */
-predicate isImmutableOrUnobservable(Node n) {
-  // Is the null pointer (or something that's not really a pointer)
-  exists(n.asExpr().getValue())
-  or
-  // Isn't a pointer or is a pointer to const
-  forall(DerivedType dt | dt = n.asExpr().getActualType() |
-    dt.getBaseType().isConst()
-    or
-    dt.getBaseType() instanceof RoutineType
-  )
-  // The above list of cases isn't exhaustive, but it narrows down the
-  // consistency alerts enough that most of them are interesting.
-}
-
 /** Holds if `n` should be hidden from path explanations. */
 predicate nodeIsHidden(Node n) { none() }
 
@@ -296,3 +289,19 @@ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preserves
  * by default as a heuristic.
  */
 predicate allowParameterReturnInSelf(ParameterNode p) { none() }
+
+private class MyConsistencyConfiguration extends Consistency::ConsistencyConfiguration {
+  override predicate argHasPostUpdateExclude(ArgumentNode n) {
+    // Is the null pointer (or something that's not really a pointer)
+    exists(n.asExpr().getValue())
+    or
+    // Isn't a pointer or is a pointer to const
+    forall(DerivedType dt | dt = n.asExpr().getActualType() |
+      dt.getBaseType().isConst()
+      or
+      dt.getBaseType() instanceof RoutineType
+    )
+    // The above list of cases isn't exhaustive, but it narrows down the
+    // consistency alerts enough that most of them are interesting.
+  }
+}

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

@@ -31,7 +31,7 @@ class Expr extends StmtParent, @expr {
   override Stmt getEnclosingStmt() {
     result = this.getParent().(Expr).getEnclosingStmt()
     or
-    result = this.getParent().(Stmt)
+    result = this.getParent()
     or
     exists(Expr other | result = other.getEnclosingStmt() and other.getConversion() = this)
     or

cpp/ql/lib/semmle/code/cpp/internal/AddressConstantExpression.qll

@@ -31,7 +31,7 @@ private predicate addressConstantVariable(Variable v) {
 private predicate constantAddressLValue(Expr lvalue) {
   lvalue.(VariableAccess).getTarget() =
     any(Variable v |
-      v.(Variable).isStatic()
+      v.isStatic()
       or
       v instanceof GlobalOrNamespaceVariable
     )

cpp/ql/lib/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

@@ -474,6 +474,25 @@ module TaintedWithPath {
     }
   }
 
+  /**
+   * INTERNAL: Do not use.
+   */
+  module Private {
+    /** Gets a predecessor `PathNode` of `pathNode`, if any. */
+    PathNode getAPredecessor(PathNode pathNode) { edges(result, pathNode) }
+
+    /** Gets the element that `pathNode` wraps, if any. */
+    Element getElementFromPathNode(PathNode pathNode) {
+      exists(DataFlow::Node node | node = pathNode.(WrapPathNode).inner().getNode() |
+        result = node.asInstruction().getAST()
+        or
+        result = node.asOperand().getDef().getAST()
+      )
+      or
+      result = pathNode.(EndpointPathNode).inner()
+    }
+  }
+
   private class WrapPathNode extends PathNode, TWrapPathNode {
     DataFlow3::PathNode inner() { this = TWrapPathNode(result) }
 

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

@@ -2,6 +2,7 @@ private import cpp
 private import semmle.code.cpp.ir.IR
 private import semmle.code.cpp.ir.dataflow.DataFlow
 private import semmle.code.cpp.ir.dataflow.internal.DataFlowPrivate
+private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
 private import DataFlowImplCommon as DataFlowImplCommon
 
 /**
@@ -63,8 +64,10 @@ private module VirtualDispatch {
       |
         // Call argument
         exists(DataFlowCall call, int i |
-          other.(DataFlow::ParameterNode).isParameterOf(call.getStaticCallTarget(), i) and
-          src.(ArgumentNode).argumentOf(call, i)
+          other
+              .(DataFlow::ParameterNode)
+              .isParameterOf(pragma[only_bind_into](call).getStaticCallTarget(), i) and
+          src.(ArgumentNode).argumentOf(call, pragma[only_bind_into](pragma[only_bind_out](i)))
         ) and
         allowOtherFromArg = true and
         allowFromArg = true
@@ -128,6 +131,7 @@ private module VirtualDispatch {
    *
    * Used to fix a join ordering issue in flowsFrom.
    */
+  pragma[noinline]
   private predicate returnNodeWithKindAndEnclosingCallable(
     ReturnNode node, ReturnKind kind, DataFlowCallable callable
   ) {
@@ -263,3 +267,17 @@ Function viableImplInCallContext(CallInstruction call, CallInstruction ctx) {
     result = ctx.getArgument(i).getUnconvertedResultExpression().(FunctionAccess).getTarget()
   )
 }
+
+/** A parameter position represented by an integer. */
+class ParameterPosition extends int {
+  ParameterPosition() { any(ParameterNode p).isParameterOf(_, this) }
+}
+
+/** An argument position represented by an integer. */
+class ArgumentPosition extends int {
+  ArgumentPosition() { any(ArgumentNode a).argumentOf(_, this) }
+}
+
+/** Holds if arguments at position `apos` match parameters at position `ppos`. */
+pragma[inline]
+predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos = apos }

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

@@ -2,6 +2,42 @@ private import DataFlowImplSpecific::Private
 private import DataFlowImplSpecific::Public
 import Cached
 
+module DataFlowImplCommonPublic {
+  private newtype TFlowFeature =
+    TFeatureHasSourceCallContext() or
+    TFeatureHasSinkCallContext() or
+    TFeatureEqualSourceSinkCallContext()
+
+  /** A flow configuration feature for use in `Configuration::getAFeature()`. */
+  class FlowFeature extends TFlowFeature {
+    string toString() { none() }
+  }
+
+  /**
+   * A flow configuration feature that implies that sources have some existing
+   * call context.
+   */
+  class FeatureHasSourceCallContext extends FlowFeature, TFeatureHasSourceCallContext {
+    override string toString() { result = "FeatureHasSourceCallContext" }
+  }
+
+  /**
+   * A flow configuration feature that implies that sinks have some existing
+   * call context.
+   */
+  class FeatureHasSinkCallContext extends FlowFeature, TFeatureHasSinkCallContext {
+    override string toString() { result = "FeatureHasSinkCallContext" }
+  }
+
+  /**
+   * A flow configuration feature that implies that source-sink pairs have some
+   * shared existing call context.
+   */
+  class FeatureEqualSourceSinkCallContext extends FlowFeature, TFeatureEqualSourceSinkCallContext {
+    override string toString() { result = "FeatureEqualSourceSinkCallContext" }
+  }
+}
+
 /**
  * The cost limits for the `AccessPathFront` to `AccessPathApprox` expansion.
  *
@@ -26,6 +62,18 @@ predicate accessPathCostLimits(int apLimit, int tupleLimit) {
   tupleLimit = 1000
 }
 
+/**
+ * Holds if `arg` is an argument of `call` with an argument position that matches
+ * parameter position `ppos`.
+ */
+pragma[noinline]
+predicate argumentPositionMatch(DataFlowCall call, ArgNode arg, ParameterPosition ppos) {
+  exists(ArgumentPosition apos |
+    arg.argumentOf(call, apos) and
+    parameterMatch(ppos, apos)
+  )
+}
+
 /**
  * Provides a simple data-flow analysis for resolving lambda calls. The analysis
  * currently excludes read-steps, store-steps, and flow-through.
@@ -35,25 +83,27 @@ predicate accessPathCostLimits(int apLimit, int tupleLimit) {
  * calls. For this reason, we cannot reuse the code from `DataFlowImpl.qll` directly.
  */
 private module LambdaFlow {
-  private predicate viableParamNonLambda(DataFlowCall call, int i, ParamNode p) {
-    p.isParameterOf(viableCallable(call), i)
+  pragma[noinline]
+  private predicate viableParamNonLambda(DataFlowCall call, ParameterPosition ppos, ParamNode p) {
+    p.isParameterOf(viableCallable(call), ppos)
   }
 
-  private predicate viableParamLambda(DataFlowCall call, int i, ParamNode p) {
-    p.isParameterOf(viableCallableLambda(call, _), i)
+  pragma[noinline]
+  private predicate viableParamLambda(DataFlowCall call, ParameterPosition ppos, ParamNode p) {
+    p.isParameterOf(viableCallableLambda(call, _), ppos)
   }
 
   private predicate viableParamArgNonLambda(DataFlowCall call, ParamNode p, ArgNode arg) {
-    exists(int i |
-      viableParamNonLambda(call, i, p) and
-      arg.argumentOf(call, i)
+    exists(ParameterPosition ppos |
+      viableParamNonLambda(call, ppos, p) and
+      argumentPositionMatch(call, arg, ppos)
     )
   }
 
   private predicate viableParamArgLambda(DataFlowCall call, ParamNode p, ArgNode arg) {
-    exists(int i |
-      viableParamLambda(call, i, p) and
-      arg.argumentOf(call, i)
+    exists(ParameterPosition ppos |
+      viableParamLambda(call, ppos, p) and
+      argumentPositionMatch(call, arg, ppos)
     )
   }
 
@@ -251,7 +301,7 @@ private module Cached {
   predicate forceCachingInSameStage() { any() }
 
   cached
-  predicate nodeEnclosingCallable(Node n, DataFlowCallable c) { c = n.getEnclosingCallable() }
+  predicate nodeEnclosingCallable(Node n, DataFlowCallable c) { c = nodeGetEnclosingCallable(n) }
 
   cached
   predicate callEnclosingCallable(DataFlowCall call, DataFlowCallable c) {
@@ -286,7 +336,7 @@ private module Cached {
     or
     exists(ArgNode arg |
       result.(PostUpdateNode).getPreUpdateNode() = arg and
-      arg.argumentOf(call, k.(ParamUpdateReturnKind).getPosition())
+      arg.argumentOf(call, k.(ParamUpdateReturnKind).getAMatchingArgumentPosition())
     )
   }
 
@@ -294,7 +344,7 @@ private module Cached {
   predicate returnNodeExt(Node n, ReturnKindExt k) {
     k = TValueReturn(n.(ReturnNode).getKind())
     or
-    exists(ParamNode p, int pos |
+    exists(ParamNode p, ParameterPosition pos |
       parameterValueFlowsToPreUpdate(p, n) and
       p.isParameterOf(_, pos) and
       k = TParamUpdate(pos)
@@ -316,13 +366,13 @@ private module Cached {
   }
 
   cached
-  predicate parameterNode(Node n, DataFlowCallable c, int i) {
-    n.(ParameterNode).isParameterOf(c, i)
+  predicate parameterNode(Node p, DataFlowCallable c, ParameterPosition pos) {
+    isParameterNode(p, c, pos)
   }
 
   cached
-  predicate argumentNode(Node n, DataFlowCall call, int pos) {
-    n.(ArgumentNode).argumentOf(call, pos)
+  predicate argumentNode(Node n, DataFlowCall call, ArgumentPosition pos) {
+    isArgumentNode(n, call, pos)
   }
 
   /**
@@ -340,12 +390,12 @@ private module Cached {
   }
 
   /**
-   * Holds if `p` is the `i`th parameter of a viable dispatch target of `call`.
-   * The instance parameter is considered to have index `-1`.
+   * Holds if `p` is the parameter of a viable dispatch target of `call`,
+   * and `p` has position `ppos`.
    */
   pragma[nomagic]
-  private predicate viableParam(DataFlowCall call, int i, ParamNode p) {
-    p.isParameterOf(viableCallableExt(call), i)
+  private predicate viableParam(DataFlowCall call, ParameterPosition ppos, ParamNode p) {
+    p.isParameterOf(viableCallableExt(call), ppos)
   }
 
   /**
@@ -354,9 +404,9 @@ private module Cached {
    */
   cached
   predicate viableParamArg(DataFlowCall call, ParamNode p, ArgNode arg) {
-    exists(int i |
-      viableParam(call, i, p) and
-      arg.argumentOf(call, i) and
+    exists(ParameterPosition ppos |
+      viableParam(call, ppos, p) and
+      argumentPositionMatch(call, arg, ppos) and
       compatibleTypes(getNodeDataFlowType(arg), getNodeDataFlowType(p))
     )
   }
@@ -828,7 +878,7 @@ private module Cached {
   cached
   newtype TReturnKindExt =
     TValueReturn(ReturnKind kind) or
-    TParamUpdate(int pos) { exists(ParamNode p | p.isParameterOf(_, pos)) }
+    TParamUpdate(ParameterPosition pos) { exists(ParamNode p | p.isParameterOf(_, pos)) }
 
   cached
   newtype TBooleanOption =
@@ -1020,9 +1070,9 @@ class ParamNode extends Node {
 
   /**
    * Holds if this node is the parameter of callable `c` at the specified
-   * (zero-based) position.
+   * position.
    */
-  predicate isParameterOf(DataFlowCallable c, int i) { parameterNode(this, c, i) }
+  predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { parameterNode(this, c, pos) }
 }
 
 /** A data-flow node that represents a call argument. */
@@ -1030,7 +1080,9 @@ class ArgNode extends Node {
   ArgNode() { argumentNode(this, _, _) }
 
   /** Holds if this argument occurs at the given position in the given call. */
-  final predicate argumentOf(DataFlowCall call, int pos) { argumentNode(this, call, pos) }
+  final predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
+    argumentNode(this, call, pos)
+  }
 }
 
 /**
@@ -1076,11 +1128,14 @@ class ValueReturnKind extends ReturnKindExt, TValueReturn {
 }
 
 class ParamUpdateReturnKind extends ReturnKindExt, TParamUpdate {
-  private int pos;
+  private ParameterPosition pos;
 
   ParamUpdateReturnKind() { this = TParamUpdate(pos) }
 
-  int getPosition() { result = pos }
+  ParameterPosition getPosition() { result = pos }
+
+  pragma[nomagic]
+  ArgumentPosition getAMatchingArgumentPosition() { parameterMatch(pos, result) }
 
   override string toString() { result = "param update " + pos }
 }

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

@@ -9,6 +9,19 @@ private import tainttracking1.TaintTrackingParameter::Private
 private import tainttracking1.TaintTrackingParameter::Public
 
 module Consistency {
+  private newtype TConsistencyConfiguration = MkConsistencyConfiguration()
+
+  /** A class for configuring the consistency queries. */
+  class ConsistencyConfiguration extends TConsistencyConfiguration {
+    string toString() { none() }
+
+    /** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
+    predicate postWithInFlowExclude(Node n) { none() }
+
+    /** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
+    predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
+  }
+
   private class RelevantNode extends Node {
     RelevantNode() {
       this instanceof ArgumentNode or
@@ -31,7 +44,7 @@ module Consistency {
   query predicate uniqueEnclosingCallable(Node n, string msg) {
     exists(int c |
       n instanceof RelevantNode and
-      c = count(n.getEnclosingCallable()) and
+      c = count(nodeGetEnclosingCallable(n)) and
       c != 1 and
       msg = "Node should have one enclosing callable but has " + c + "."
     )
@@ -85,13 +98,13 @@ module Consistency {
   }
 
   query predicate parameterCallable(ParameterNode p, string msg) {
-    exists(DataFlowCallable c | p.isParameterOf(c, _) and c != p.getEnclosingCallable()) and
+    exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and
     msg = "Callable mismatch for parameter."
   }
 
   query predicate localFlowIsLocal(Node n1, Node n2, string msg) {
     simpleLocalFlowStep(n1, n2) and
-    n1.getEnclosingCallable() != n2.getEnclosingCallable() and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
     msg = "Local flow step does not preserve enclosing callable."
   }
 
@@ -106,7 +119,7 @@ module Consistency {
   query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) {
     isUnreachableInCall(n, call) and
     exists(DataFlowCallable c |
-      c = n.getEnclosingCallable() and
+      c = nodeGetEnclosingCallable(n) and
       not viableCallable(call) = c
     ) and
     msg = "Call context for isUnreachableInCall is inconsistent with call graph."
@@ -120,7 +133,7 @@ module Consistency {
       n.(ArgumentNode).argumentOf(call, _) and
       msg = "ArgumentNode and call does not share enclosing callable."
     ) and
-    n.getEnclosingCallable() != call.getEnclosingCallable()
+    nodeGetEnclosingCallable(n) != call.getEnclosingCallable()
   }
 
   // This predicate helps the compiler forget that in some languages
@@ -151,7 +164,7 @@ module Consistency {
   }
 
   query predicate postIsInSameCallable(PostUpdateNode n, string msg) {
-    n.getEnclosingCallable() != n.getPreUpdateNode().getEnclosingCallable() and
+    nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and
     msg = "PostUpdateNode does not share callable with its pre-update node."
   }
 
@@ -164,7 +177,7 @@ module Consistency {
 
   query predicate argHasPostUpdate(ArgumentNode n, string msg) {
     not hasPost(n) and
-    not isImmutableOrUnobservable(n) and
+    not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and
     msg = "ArgumentNode is missing PostUpdateNode."
   }
 
@@ -177,6 +190,7 @@ module Consistency {
     isPostUpdateNode(n) and
     not clearsContent(n, _) and
     simpleLocalFlowStep(_, n) and
+    not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
     msg = "PostUpdateNode should not be the target of local flow."
   }
 }

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

@@ -2,6 +2,20 @@ private import cpp
 private import DataFlowUtil
 private import semmle.code.cpp.ir.IR
 private import DataFlowDispatch
+private import DataFlowImplConsistency
+
+/** Gets the callable in which this node occurs. */
+DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.getEnclosingCallable() }
+
+/** Holds if `p` is a `ParameterNode` of `c` with position `pos`. */
+predicate isParameterNode(ParameterNode p, DataFlowCallable c, ParameterPosition pos) {
+  p.isParameterOf(c, pos)
+}
+
+/** Holds if `arg` is an `ArgumentNode` of `c` with position `pos`. */
+predicate isArgumentNode(ArgumentNode arg, DataFlowCall c, ArgumentPosition pos) {
+  arg.argumentOf(c, pos)
+}
 
 /**
  * A data flow node that occurs as the argument of a call and is passed as-is
@@ -106,11 +120,9 @@ class ReturnNode extends InstructionNode {
   Instruction primary;
 
   ReturnNode() {
-    exists(ReturnValueInstruction ret | instr = ret.getReturnValue() and primary = ret)
+    exists(ReturnValueInstruction ret | instr = ret and primary = ret)
     or
-    exists(ReturnIndirectionInstruction rii |
-      instr = rii.getSideEffectOperand().getAnyDef() and primary = rii
-    )
+    exists(ReturnIndirectionInstruction rii | instr = rii and primary = rii)
   }
 
   /** Gets the kind of this returned value. */
@@ -184,108 +196,16 @@ OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) {
  */
 predicate jumpStep(Node n1, Node n2) { none() }
 
-private predicate fieldStoreStepNoChi(Node node1, FieldContent f, PostUpdateNode node2) {
-  exists(StoreInstruction store, Class c |
-    store = node2.asInstruction() and
-    store.getSourceValueOperand() = node1.asOperand() and
-    getWrittenField(store, f.(FieldContent).getAField(), c) and
-    f.hasOffset(c, _, _)
-  )
-}
-
-private FieldAddressInstruction getFieldInstruction(Instruction instr) {
-  result = instr or
-  result = instr.(CopyValueInstruction).getUnary()
-}
-
-pragma[noinline]
-private predicate getWrittenField(Instruction instr, Field f, Class c) {
-  exists(FieldAddressInstruction fa |
-    fa =
-      getFieldInstruction([
-          instr.(StoreInstruction).getDestinationAddress(),
-          instr.(WriteSideEffectInstruction).getDestinationAddress()
-        ]) and
-    f = fa.getField() and
-    c = f.getDeclaringType()
-  )
-}
-
-private predicate fieldStoreStepChi(Node node1, FieldContent f, PostUpdateNode node2) {
-  exists(ChiPartialOperand operand, ChiInstruction chi |
-    chi.getPartialOperand() = operand and
-    node1.asOperand() = operand and
-    node2.asInstruction() = chi and
-    exists(Class c |
-      c = chi.getResultType() and
-      exists(int startBit, int endBit |
-        chi.getUpdatedInterval(startBit, endBit) and
-        f.hasOffset(c, startBit, endBit)
-      )
-      or
-      getWrittenField(operand.getDef(), f.getAField(), c) and
-      f.hasOffset(c, _, _)
-    )
-  )
-}
-
-private predicate arrayStoreStepChi(Node node1, ArrayContent a, PostUpdateNode node2) {
-  exists(a) and
-  exists(ChiPartialOperand operand, ChiInstruction chi, StoreInstruction store |
-    chi.getPartialOperand() = operand and
-    store = operand.getDef() and
-    node1.asOperand() = operand and
-    // This `ChiInstruction` will always have a non-conflated result because both `ArrayStoreNode`
-    // and `PointerStoreNode` require it in their characteristic predicates.
-    node2.asInstruction() = chi and
-    (
-      // `x[i] = taint()`
-      // This matches the characteristic predicate in `ArrayStoreNode`.
-      store.getDestinationAddress() instanceof PointerAddInstruction
-      or
-      // `*p = taint()`
-      // This matches the characteristic predicate in `PointerStoreNode`.
-      store.getDestinationAddress().(CopyValueInstruction).getUnary() instanceof LoadInstruction
-    )
-  )
-}
-
 /**
  * Holds if data can flow from `node1` to `node2` via an assignment to `f`.
  * Thus, `node2` references an object with a field `f` that contains the
  * value of `node1`.
  */
-predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
-  fieldStoreStepNoChi(node1, f, node2) or
-  fieldStoreStepChi(node1, f, node2) or
-  arrayStoreStepChi(node1, f, node2) or
-  fieldStoreStepAfterArraySuppression(node1, f, node2)
-}
-
-// This predicate pushes the correct `FieldContent` onto the access path when the
-// `suppressArrayRead` predicate has popped off an `ArrayContent`.
-private predicate fieldStoreStepAfterArraySuppression(
-  Node node1, FieldContent f, PostUpdateNode node2
-) {
-  exists(WriteSideEffectInstruction write, ChiInstruction chi, Class c |
-    not chi.isResultConflated() and
-    node1.asInstruction() = chi and
-    node2.asInstruction() = chi and
-    chi.getPartial() = write and
-    getWrittenField(write, f.getAField(), c) and
-    f.hasOffset(c, _, _)
-  )
-}
-
-bindingset[result, i]
-private int unbindInt(int i) { i <= result and i >= result }
-
-pragma[noinline]
-private predicate getLoadedField(LoadInstruction load, Field f, Class c) {
-  exists(FieldAddressInstruction fa |
-    fa = load.getSourceAddress() and
-    f = fa.getField() and
-    c = f.getDeclaringType()
+predicate storeStep(StoreNodeInstr node1, FieldContent f, StoreNodeInstr node2) {
+  exists(FieldAddressInstruction fai |
+    node1.getInstruction() = fai and
+    node2.getInstruction() = fai.getObjectAddress() and
+    f.getField() = fai.getField()
   )
 }
 
@@ -294,122 +214,14 @@ private predicate getLoadedField(LoadInstruction load, Field f, Class c) {
  * Thus, `node1` references an object with a field `f` whose value ends up in
  * `node2`.
  */
-private predicate fieldReadStep(Node node1, FieldContent f, Node node2) {
-  exists(LoadOperand operand |
-    node2.asOperand() = operand and
-    node1.asInstruction() = operand.getAnyDef() and
-    exists(Class c |
-      c = operand.getAnyDef().getResultType() and
-      exists(int startBit, int endBit |
-        operand.getUsedInterval(unbindInt(startBit), unbindInt(endBit)) and
-        f.hasOffset(c, startBit, endBit)
-      )
-      or
-      getLoadedField(operand.getUse(), f.getAField(), c) and
-      f.hasOffset(c, _, _)
-    )
+predicate readStep(ReadNode node1, FieldContent f, ReadNode node2) {
+  exists(FieldAddressInstruction fai |
+    node1.getInstruction() = fai.getObjectAddress() and
+    node2.getInstruction() = fai and
+    f.getField() = fai.getField()
   )
 }
 
-/**
- * When a store step happens in a function that looks like an array write such as:
- * ```cpp
- * void f(int* pa) {
- *   pa = source();
- * }
- * ```
- * it can be a write to an array, but it can also happen that `f` is called as `f(&a.x)`. If that is
- * the case, the `ArrayContent` that was written by the call to `f` should be popped off the access
- * path, and a `FieldContent` containing `x` should be pushed instead.
- * So this case pops `ArrayContent` off the access path, and the `fieldStoreStepAfterArraySuppression`
- * predicate in `storeStep` ensures that we push the right `FieldContent` onto the access path.
- */
-predicate suppressArrayRead(Node node1, ArrayContent a, Node node2) {
-  exists(a) and
-  exists(WriteSideEffectInstruction write, ChiInstruction chi |
-    node1.asInstruction() = write and
-    node2.asInstruction() = chi and
-    chi.getPartial() = write and
-    getWrittenField(write, _, _)
-  )
-}
-
-private class ArrayToPointerConvertInstruction extends ConvertInstruction {
-  ArrayToPointerConvertInstruction() {
-    this.getUnary().getResultType() instanceof ArrayType and
-    this.getResultType() instanceof PointerType
-  }
-}
-
-private Instruction skipOneCopyValueInstructionRec(CopyValueInstruction copy) {
-  copy.getUnary() = result and not result instanceof CopyValueInstruction
-  or
-  result = skipOneCopyValueInstructionRec(copy.getUnary())
-}
-
-private Instruction skipCopyValueInstructions(Operand op) {
-  not result instanceof CopyValueInstruction and result = op.getDef()
-  or
-  result = skipOneCopyValueInstructionRec(op.getDef())
-}
-
-private predicate arrayReadStep(Node node1, ArrayContent a, Node node2) {
-  exists(a) and
-  // Explicit dereferences such as `*p` or `p[i]` where `p` is a pointer or array.
-  exists(LoadOperand operand, Instruction address |
-    operand.isDefinitionInexact() and
-    node1.asInstruction() = operand.getAnyDef() and
-    operand = node2.asOperand() and
-    address = skipCopyValueInstructions(operand.getAddressOperand()) and
-    (
-      address instanceof LoadInstruction or
-      address instanceof ArrayToPointerConvertInstruction or
-      address instanceof PointerOffsetInstruction
-    )
-  )
-}
-
-/**
- * In cases such as:
- * ```cpp
- * void f(int* pa) {
- *   *pa = source();
- * }
- * ...
- * int x;
- * f(&x);
- * use(x);
- * ```
- * the load on `x` in `use(x)` will exactly overlap with its definition (in this case the definition
- * is a `WriteSideEffect`). This predicate pops the `ArrayContent` (pushed by the store in `f`)
- * from the access path.
- */
-private predicate exactReadStep(Node node1, ArrayContent a, Node node2) {
-  exists(a) and
-  exists(WriteSideEffectInstruction write, ChiInstruction chi |
-    not chi.isResultConflated() and
-    chi.getPartial() = write and
-    node1.asInstruction() = write and
-    node2.asInstruction() = chi and
-    // To distinquish this case from the `arrayReadStep` case we require that the entire variable was
-    // overwritten by the `WriteSideEffectInstruction` (i.e., there is a load that reads the
-    // entire variable).
-    exists(LoadInstruction load | load.getSourceValue() = chi)
-  )
-}
-
-/**
- * Holds if data can flow from `node1` to `node2` via a read of `f`.
- * Thus, `node1` references an object with a field `f` whose value ends up in
- * `node2`.
- */
-predicate readStep(Node node1, Content f, Node node2) {
-  fieldReadStep(node1, f, node2) or
-  arrayReadStep(node1, f, node2) or
-  exactReadStep(node1, f, node2) or
-  suppressArrayRead(node1, f, node2)
-}
-
 /**
  * Holds if values stored inside content `c` are cleared at node `n`.
  */
@@ -441,7 +253,7 @@ private predicate suppressUnusedNode(Node n) { any() }
 // Java QL library compatibility wrappers
 //////////////////////////////////////////////////////////////////////////////
 /** A node that performs a type cast. */
-class CastNode extends InstructionNode {
+class CastNode extends Node {
   CastNode() { none() } // stub implementation
 }
 
@@ -481,21 +293,18 @@ class Unit extends TUnit {
   string toString() { result = "unit" }
 }
 
-/**
- * Holds if `n` does not require a `PostUpdateNode` as it either cannot be
- * modified or its modification cannot be observed, for example if it is a
- * freshly created object that is not saved in a variable.
- *
- * This predicate is only used for consistency checks.
- */
-predicate isImmutableOrUnobservable(Node n) {
-  // The rules for whether an IR argument gets a post-update node are too
-  // complex to model here.
-  any()
-}
-
 /** Holds if `n` should be hidden from path explanations. */
-predicate nodeIsHidden(Node n) { n instanceof OperandNode and not n instanceof ArgumentNode }
+predicate nodeIsHidden(Node n) {
+  n instanceof OperandNode and not n instanceof ArgumentNode
+  or
+  StoreNodeFlow::flowThrough(n, _) and
+  not StoreNodeFlow::flowOutOf(n, _) and
+  not StoreNodeFlow::flowInto(_, n)
+  or
+  ReadNodeFlow::flowThrough(n, _) and
+  not ReadNodeFlow::flowOutOf(n, _) and
+  not ReadNodeFlow::flowInto(_, n)
+}
 
 class LambdaCallKind = Unit;
 
@@ -516,3 +325,11 @@ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preserves
  * by default as a heuristic.
  */
 predicate allowParameterReturnInSelf(ParameterNode p) { none() }
+
+private class MyConsistencyConfiguration extends Consistency::ConsistencyConfiguration {
+  override predicate argHasPostUpdateExclude(ArgumentNode n) {
+    // The rules for whether an IR argument gets a post-update node are too
+    // complex to model here.
+    any()
+  }
+}

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

@@ -10,19 +10,78 @@ private import semmle.code.cpp.ir.ValueNumbering
 private import semmle.code.cpp.ir.IR
 private import semmle.code.cpp.controlflow.IRGuards
 private import semmle.code.cpp.models.interfaces.DataFlow
+private import DataFlowPrivate
+private import SsaInternals as Ssa
 
 cached
 private module Cached {
+  /**
+   * The IR dataflow graph consists of the following nodes:
+   * - `InstructionNode`, which represents an `Instruction` in the graph.
+   * - `OperandNode`, which represents an `Operand` in the graph.
+   * - `VariableNode`, which is used to model global variables.
+   * - Two kinds of `StoreNode`s:
+   *   1. `StoreNodeInstr`, which represents the value of an address computed by an `Instruction` that
+   *      has been updated by a write operation.
+   *   2. `StoreNodeOperand`, which represents the value of an address in an `ArgumentOperand` after a
+   *      function call that may have changed the value.
+   * - `ReadNode`, which represents the result of reading a field of an object.
+   * - `SsaPhiNode`, which represents phi nodes as computed by the shared SSA library.
+   *
+   * The following section describes how flow is generally transferred between these nodes:
+   * - Flow between `InstructionNode`s and `OperandNode`s follow the def-use information as computed by
+   *   the IR. Because the IR compute must-alias information for memory operands, we only follow def-use
+   *   flow for register operands.
+   * - Flow can enter a `StoreNode` in two ways (both done in `StoreNode.flowInto`):
+   *   1. Flow is transferred from a `StoreValueOperand` to a `StoreNodeInstr`. Flow will then proceed
+   *      along the chain of addresses computed by `StoreNodeInstr.getInner` to identify field writes
+   *      and call `storeStep` accordingly (i.e., for an expression like `a.b.c = x`, we visit `c`, then
+   *      `b`, then `a`).
+   *   2. Flow is transfered from a `WriteSideEffectInstruction` to a `StoreNodeOperand` after flow
+   *      returns to a caller. Flow will then proceed to the defining instruction of the operand (because
+   *      the `StoreNodeInstr` computed by `StoreNodeOperand.getInner()` is the `StoreNode` containing
+   *      the defining instruction), and then along the chain computed by `StoreNodeInstr.getInner` like
+   *      above.
+   *   In both cases, flow leaves a `StoreNode` once the entire chain has been traversed, and the shared
+   *   SSA library is used to find the next use of the variable at the end of the chain.
+   * - Flow can enter a `ReadNode` through an `OperandNode` that represents an address of some variable.
+   *   Flow will then proceed along the chain of addresses computed by `ReadNode.getOuter` (i.e., for an
+   *   expression like `use(a.b.c)` we visit `a`, then `b`, then `c`) and call `readStep` accordingly.
+   *   Once the entire chain has been traversed, flow is transferred to the load instruction that reads
+   *   the final address of the chain.
+   * - Flow can enter a `SsaPhiNode` from an `InstructionNode`, a `StoreNode` or another `SsaPhiNode`
+   *   (in `toPhiNode`), depending on which node provided the previous definition of the underlying
+   *   variable. Flow leaves a `SsaPhiNode` (in `fromPhiNode`) by using the shared SSA library to
+   *   determine the next use of the variable.
+   */
   cached
   newtype TIRDataFlowNode =
     TInstructionNode(Instruction i) or
     TOperandNode(Operand op) or
-    TVariableNode(Variable var)
+    TVariableNode(Variable var) or
+    TStoreNodeInstr(Instruction i) { Ssa::explicitWrite(_, _, i) } or
+    TStoreNodeOperand(ArgumentOperand op) { Ssa::explicitWrite(_, _, op.getDef()) } or
+    TReadNode(Instruction i) { needsPostReadNode(i) } or
+    TSsaPhiNode(Ssa::PhiNode phi)
 
   cached
   predicate localFlowStepCached(Node nodeFrom, Node nodeTo) {
     simpleLocalFlowStep(nodeFrom, nodeTo)
   }
+
+  private predicate needsPostReadNode(Instruction iFrom) {
+    // If the instruction generates an address that flows to a load.
+    Ssa::addressFlowTC(iFrom, Ssa::getSourceAddress(_)) and
+    (
+      // And it is either a field address
+      iFrom instanceof FieldAddressInstruction
+      or
+      // Or it is instruction that either uses or is used for an address that needs a post read node.
+      exists(Instruction mid | needsPostReadNode(mid) |
+        Ssa::addressFlow(mid, iFrom) or Ssa::addressFlow(iFrom, mid)
+      )
+    )
+  }
 }
 
 private import Cached
@@ -180,6 +239,234 @@ class OperandNode extends Node, TOperandNode {
   override string toString() { result = this.getOperand().toString() }
 }
 
+/**
+ * INTERNAL: do not use.
+ *
+ * A `StoreNode` is a node that has been (or is about to be) the
+ * source or target of a `storeStep`.
+ */
+abstract private class StoreNode extends Node {
+  /** Holds if this node should receive flow from `addr`. */
+  abstract predicate flowInto(Instruction addr);
+
+  override Declaration getEnclosingCallable() { result = this.getFunction() }
+
+  /** Holds if this `StoreNode` is the root of the address computation used by a store operation. */
+  predicate isTerminal() {
+    not exists(this.getInner()) and
+    not storeStep(this, _, _)
+  }
+
+  /** Gets the store operation that uses the address computed by this `StoreNode`. */
+  abstract Instruction getStoreInstruction();
+
+  /** Holds if the store operation associated with this `StoreNode` overwrites the entire variable. */
+  final predicate isCertain() { Ssa::explicitWrite(true, this.getStoreInstruction(), _) }
+
+  /**
+   * Gets the `StoreNode` that computes the address used by this `StoreNode`.
+   */
+  abstract StoreNode getInner();
+
+  /** The inverse of `StoreNode.getInner`. */
+  final StoreNode getOuter() { result.getInner() = this }
+}
+
+class StoreNodeInstr extends StoreNode, TStoreNodeInstr {
+  Instruction instr;
+
+  StoreNodeInstr() { this = TStoreNodeInstr(instr) }
+
+  override predicate flowInto(Instruction addr) { this.getInstruction() = addr }
+
+  /** Gets the underlying instruction. */
+  Instruction getInstruction() { result = instr }
+
+  override Function getFunction() { result = this.getInstruction().getEnclosingFunction() }
+
+  override IRType getType() { result = this.getInstruction().getResultIRType() }
+
+  override Location getLocation() { result = this.getInstruction().getLocation() }
+
+  override string toString() {
+    result = instructionNode(this.getInstruction()).toString() + " [store]"
+  }
+
+  override Instruction getStoreInstruction() {
+    Ssa::explicitWrite(_, result, this.getInstruction())
+  }
+
+  override StoreNodeInstr getInner() {
+    Ssa::addressFlow(result.getInstruction(), this.getInstruction())
+  }
+}
+
+/**
+ * To avoid having `PostUpdateNode`s with multiple pre-update nodes (which can cause performance
+ * problems) we attach the `PostUpdateNode` that represent output arguments to an operand instead of
+ * an instruction.
+ *
+ * To see why we need this, consider the expression `b->set(new C())`. The IR of this expression looks
+ * like (simplified):
+ * ```
+ * r1(glval<unknown>) = FunctionAddress[set]            :
+ * r2(glval<unknown>) = FunctionAddress[operator new]   :
+ * r3(unsigned long)  = Constant[8]                     :
+ * r4(void *)         = Call[operator new]              : func:r2, 0:r3
+ * r5(C *)            = Convert                         : r4
+ * r6(glval<unknown>) = FunctionAddress[C]              :
+ * v1(void)           = Call[C]                         : func:r6, this:r5
+ * v2(void)           = Call[set]                       : func:r1, this:r0, 0:r5
+ * ```
+ *
+ * Notice that both the call to `C` and the call to `set` will have an argument that is the
+ * result of calling `operator new` (i.e., `r4`). If we only have `PostUpdateNode`s that are
+ * instructions, both `PostUpdateNode`s would have `r4` as their pre-update node.
+ *
+ * We avoid this issue by having a `PostUpdateNode` for each argument, and let the pre-update node of
+ * each `PostUpdateNode` be the argument _operand_, instead of the defining instruction.
+ */
+class StoreNodeOperand extends StoreNode, TStoreNodeOperand {
+  ArgumentOperand operand;
+
+  StoreNodeOperand() { this = TStoreNodeOperand(operand) }
+
+  override predicate flowInto(Instruction addr) { this.getOperand().getDef() = addr }
+
+  /** Gets the underlying operand. */
+  Operand getOperand() { result = operand }
+
+  override Function getFunction() { result = operand.getDef().getEnclosingFunction() }
+
+  override IRType getType() { result = operand.getIRType() }
+
+  override Location getLocation() { result = operand.getLocation() }
+
+  override string toString() { result = operandNode(this.getOperand()).toString() + " [store]" }
+
+  override WriteSideEffectInstruction getStoreInstruction() {
+    Ssa::explicitWrite(_, result, operand.getDef())
+  }
+
+  /**
+   * The result of `StoreNodeOperand.getInner` is the `StoreNodeInstr` representation the instruction
+   * that defines this operand. This means the graph of `getInner` looks like this:
+   * ```
+   * I---I---I
+   *  \   \   \
+   *   O   O   O
+   * ```
+   * where each `StoreNodeOperand` "hooks" into the chain computed by `StoreNodeInstr.getInner`.
+   * This means that the chain of `getInner` calls on the argument `&o.f` on an expression
+   * like `func(&o.f)` is:
+   * ```
+   * r4---r3---r2
+   *  \
+   *   0:r4
+   * ```
+   * where the IR for `func(&o.f)` looks like (simplified):
+   * ```
+   * r1(glval<unknown>) = FunctionAddress[func]        :
+   * r2(glval<O>)       = VariableAddress[o]           :
+   * r3(glval<int>)     = FieldAddress[f]              : r2
+   * r4(int *)          = CopyValue                    : r3
+   * v1(void)           = Call[func]                   : func:r1, 0:r4
+   * ```
+   */
+  override StoreNodeInstr getInner() { operand.getDef() = result.getInstruction() }
+}
+
+/**
+ * INTERNAL: do not use.
+ *
+ * A `ReadNode` is a node that has been (or is about to be) the
+ * source or target of a `readStep`.
+ */
+class ReadNode extends Node, TReadNode {
+  Instruction i;
+
+  ReadNode() { this = TReadNode(i) }
+
+  /** Gets the underlying instruction. */
+  Instruction getInstruction() { result = i }
+
+  override Declaration getEnclosingCallable() { result = this.getFunction() }
+
+  override Function getFunction() { result = this.getInstruction().getEnclosingFunction() }
+
+  override IRType getType() { result = this.getInstruction().getResultIRType() }
+
+  override Location getLocation() { result = this.getInstruction().getLocation() }
+
+  override string toString() {
+    result = instructionNode(this.getInstruction()).toString() + " [read]"
+  }
+
+  /** Gets a load instruction that uses the address computed by this read node. */
+  final Instruction getALoadInstruction() {
+    Ssa::addressFlowTC(this.getInstruction(), Ssa::getSourceAddress(result))
+  }
+
+  /**
+   * Gets a read node with an underlying instruction that is used by this
+   * underlying instruction to compute an address of a load instruction.
+   */
+  final ReadNode getInner() { Ssa::addressFlow(result.getInstruction(), this.getInstruction()) }
+
+  /** The inverse of `ReadNode.getInner`. */
+  final ReadNode getOuter() { result.getInner() = this }
+
+  /** Holds if this read node computes a value that will not be used for any future read nodes. */
+  final predicate isTerminal() {
+    not exists(this.getOuter()) and
+    not readStep(this, _, _)
+  }
+
+  /** Holds if this read node computes a value that has not yet been used for any read operations. */
+  final predicate isInitial() {
+    not exists(this.getInner()) and
+    not readStep(_, _, this)
+  }
+}
+
+/**
+ * INTERNAL: do not use.
+ *
+ * A phi node produced by the shared SSA library, viewed as a node in a data flow graph.
+ */
+class SsaPhiNode extends Node, TSsaPhiNode {
+  Ssa::PhiNode phi;
+
+  SsaPhiNode() { this = TSsaPhiNode(phi) }
+
+  /* Get the phi node associated with this node. */
+  Ssa::PhiNode getPhiNode() { result = phi }
+
+  override Declaration getEnclosingCallable() { result = this.getFunction() }
+
+  override Function getFunction() { result = phi.getBasicBlock().getEnclosingFunction() }
+
+  override IRType getType() { result instanceof IRVoidType }
+
+  override Location getLocation() { result = phi.getBasicBlock().getLocation() }
+
+  /** Holds if this phi node has input from the `rnk`'th write operation in block `block`. */
+  final predicate hasInputAtRankInBlock(IRBlock block, int rnk) {
+    this.hasInputAtRankInBlock(block, rnk, _)
+  }
+
+  /**
+   * Holds if this phi node has input from the definition `input` (which is the `rnk`'th write
+   * operation in block `block`).
+   */
+  cached
+  final predicate hasInputAtRankInBlock(IRBlock block, int rnk, Ssa::Definition input) {
+    Ssa::phiHasInputFromBlock(phi, input, _) and input.definesAt(_, block, rnk)
+  }
+
+  override string toString() { result = "Phi" }
+}
+
 /**
  * An expression, viewed as a node in a data flow graph.
  */
@@ -313,15 +600,14 @@ deprecated class UninitializedNode extends Node {
  * Nodes corresponding to AST elements, for example `ExprNode`, usually refer
  * to the value before the update with the exception of `ClassInstanceExpr`,
  * which represents the value after the constructor has run.
- *
- * This class exists to match the interface used by Java. There are currently no non-abstract
- * classes that extend it. When we implement field flow, we can revisit this.
  */
-abstract class PostUpdateNode extends InstructionNode {
+abstract class PostUpdateNode extends Node {
   /**
    * Gets the node before the state update.
    */
   abstract Node getPreUpdateNode();
+
+  override string toString() { result = this.getPreUpdateNode() + " [post update]" }
 }
 
 /**
@@ -332,7 +618,7 @@ abstract class PostUpdateNode extends InstructionNode {
  * value, but does not necessarily replace it entirely. For example:
  * ```
  * x.y = 1; // a partial definition of the object `x`.
- * x.y.z = 1; // a partial definition of the object `x.y`.
+ * x.y.z = 1; // a partial definition of the object `x.y` and `x`.
  * x.setY(1); // a partial definition of the object `x`.
  * setY(&x); // a partial definition of the object `x`.
  * ```
@@ -341,135 +627,34 @@ abstract private class PartialDefinitionNode extends PostUpdateNode {
   abstract Expr getDefinedExpr();
 }
 
-private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
-  override ChiInstruction instr;
-  StoreInstruction store;
-
-  ExplicitFieldStoreQualifierNode() {
-    not instr.isResultConflated() and
-    instr.getPartial() = store and
-    (
-      instr.getUpdatedInterval(_, _) or
-      store.getDestinationAddress() instanceof FieldAddressInstruction
-    )
+private class FieldPartialDefinitionNode extends PartialDefinitionNode, StoreNodeInstr {
+  FieldPartialDefinitionNode() {
+    this.getInstruction() = any(FieldAddressInstruction fai).getObjectAddress()
   }
 
-  // By using an operand as the result of this predicate we avoid the dataflow inconsistency errors
-  // caused by having multiple nodes sharing the same pre update node. This inconsistency error can cause
-  // a tuple explosion in the big step dataflow relation since it can make many nodes be the entry node
-  // into a big step.
-  override Node getPreUpdateNode() { result.asOperand() = instr.getTotalOperand() }
+  override Node getPreUpdateNode() { result.asInstruction() = this.getInstruction() }
+
+  override Expr getDefinedExpr() { result = this.getInstruction().getUnconvertedResultExpression() }
+
+  override string toString() { result = PartialDefinitionNode.super.toString() }
+}
+
+private class NonPartialDefinitionPostUpdate extends PostUpdateNode, StoreNodeInstr {
+  NonPartialDefinitionPostUpdate() { not this instanceof PartialDefinitionNode }
+
+  override Node getPreUpdateNode() { result.asInstruction() = this.getInstruction() }
+
+  override string toString() { result = PostUpdateNode.super.toString() }
+}
+
+private class ArgumentPostUpdateNode extends PartialDefinitionNode, StoreNodeOperand {
+  override ArgumentNode getPreUpdateNode() { result.asOperand() = operand }
 
   override Expr getDefinedExpr() {
-    result =
-      store
-          .getDestinationAddress()
-          .(FieldAddressInstruction)
-          .getObjectAddress()
-          .getUnconvertedResultExpression()
-  }
-}
-
-/**
- * Not every store instruction generates a chi instruction that we can attach a PostUpdateNode to.
- * For instance, an update to a field of a struct containing only one field. Even if the store does
- * have a chi instruction, a subsequent use of the result of the store may be linked directly to the
- * result of the store as an inexact definition if the store totally overlaps the use. For these
- * cases we attach the PostUpdateNode to the store instruction. There's no obvious pre update node
- * for this case (as the entire memory is updated), so `getPreUpdateNode` is implemented as
- * `none()`.
- */
-private class ExplicitSingleFieldStoreQualifierNode extends PartialDefinitionNode {
-  override StoreInstruction instr;
-
-  ExplicitSingleFieldStoreQualifierNode() {
-    (
-      instr.getAUse().isDefinitionInexact()
-      or
-      not exists(ChiInstruction chi | chi.getPartial() = instr)
-    ) and
-    // Without this condition any store would create a `PostUpdateNode`.
-    instr.getDestinationAddress() instanceof FieldAddressInstruction
+    result = this.getOperand().getDef().getUnconvertedResultExpression()
   }
 
-  override Node getPreUpdateNode() { none() }
-
-  override Expr getDefinedExpr() {
-    result =
-      instr
-          .getDestinationAddress()
-          .(FieldAddressInstruction)
-          .getObjectAddress()
-          .getUnconvertedResultExpression()
-  }
-}
-
-private FieldAddressInstruction getFieldInstruction(Instruction instr) {
-  result = instr or
-  result = instr.(CopyValueInstruction).getUnary()
-}
-
-/**
- * The target of a `fieldStoreStepAfterArraySuppression` store step, which is used to convert
- * an `ArrayContent` to a `FieldContent` when the `WriteSideEffect` instruction stores
- * into a field. See the QLDoc for `suppressArrayRead` for an example of where such a conversion
- * is inserted.
- */
-private class WriteSideEffectFieldStoreQualifierNode extends PartialDefinitionNode {
-  override ChiInstruction instr;
-  WriteSideEffectInstruction write;
-  FieldAddressInstruction field;
-
-  WriteSideEffectFieldStoreQualifierNode() {
-    not instr.isResultConflated() and
-    instr.getPartial() = write and
-    field = getFieldInstruction(write.getDestinationAddress())
-  }
-
-  override Node getPreUpdateNode() { result.asOperand() = instr.getTotalOperand() }
-
-  override Expr getDefinedExpr() {
-    result = field.getObjectAddress().getUnconvertedResultExpression()
-  }
-}
-
-/**
- * The `PostUpdateNode` that is the target of a `arrayStoreStepChi` store step. The overriden
- * `ChiInstruction` corresponds to the instruction represented by `node2` in `arrayStoreStepChi`.
- */
-private class ArrayStoreNode extends PartialDefinitionNode {
-  override ChiInstruction instr;
-  PointerAddInstruction add;
-
-  ArrayStoreNode() {
-    not instr.isResultConflated() and
-    exists(StoreInstruction store |
-      instr.getPartial() = store and
-      add = store.getDestinationAddress()
-    )
-  }
-
-  override Node getPreUpdateNode() { result.asOperand() = instr.getTotalOperand() }
-
-  override Expr getDefinedExpr() { result = add.getLeft().getUnconvertedResultExpression() }
-}
-
-/**
- * The `PostUpdateNode` that is the target of a `arrayStoreStepChi` store step. The overriden
- * `ChiInstruction` corresponds to the instruction represented by `node2` in `arrayStoreStepChi`.
- */
-private class PointerStoreNode extends PostUpdateNode {
-  override ChiInstruction instr;
-
-  PointerStoreNode() {
-    not instr.isResultConflated() and
-    exists(StoreInstruction store |
-      instr.getPartial() = store and
-      store.getDestinationAddress().(CopyValueInstruction).getUnary() instanceof LoadInstruction
-    )
-  }
-
-  override Node getPreUpdateNode() { result.asOperand() = instr.getTotalOperand() }
+  override string toString() { result = PartialDefinitionNode.super.toString() }
 }
 
 /**
@@ -548,6 +733,11 @@ class VariableNode extends Node, TVariableNode {
  */
 InstructionNode instructionNode(Instruction instr) { result.getInstruction() = instr }
 
+/**
+ * Gets the node corresponding to `operand`.
+ */
+OperandNode operandNode(Operand operand) { result.getOperand() = operand }
+
 /**
  * DEPRECATED: use `definitionByReferenceNodeFromArgument` instead.
  *
@@ -614,61 +804,176 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
   or
   // Instruction -> Operand flow
   simpleOperandLocalFlowStep(nodeFrom.asInstruction(), nodeTo.asOperand())
+  or
+  // Flow into, through, and out of store nodes
+  StoreNodeFlow::flowInto(nodeFrom.asInstruction(), nodeTo)
+  or
+  StoreNodeFlow::flowThrough(nodeFrom, nodeTo)
+  or
+  StoreNodeFlow::flowOutOf(nodeFrom, nodeTo)
+  or
+  // Flow into, through, and out of read nodes
+  ReadNodeFlow::flowInto(nodeFrom, nodeTo)
+  or
+  ReadNodeFlow::flowThrough(nodeFrom, nodeTo)
+  or
+  ReadNodeFlow::flowOutOf(nodeFrom, nodeTo)
+  or
+  // Adjacent-def-use and adjacent-use-use flow
+  adjacentDefUseFlow(nodeFrom, nodeTo)
 }
 
-pragma[noinline]
-private predicate getFieldSizeOfClass(Class c, Type type, int size) {
-  exists(Field f |
-    f.getDeclaringType() = c and
-    f.getUnderlyingType() = type and
-    type.getSize() = size
+private predicate adjacentDefUseFlow(Node nodeFrom, Node nodeTo) {
+  // Flow that isn't already covered by field flow out of store/read nodes.
+  not nodeFrom.asInstruction() = any(StoreNode pun).getStoreInstruction() and
+  not nodeFrom.asInstruction() = any(ReadNode pun).getALoadInstruction() and
+  (
+    //Def-use flow
+    Ssa::ssaFlow(nodeFrom, nodeTo)
+    or
+    // Use-use flow through stores.
+    exists(Instruction loadAddress, Node store |
+      loadAddress = Ssa::getSourceAddressFromNode(nodeFrom) and
+      Ssa::explicitWrite(_, store.asInstruction(), loadAddress) and
+      Ssa::ssaFlow(store, nodeTo)
+    )
   )
 }
 
-private predicate isSingleFieldClass(Type type, Operand op) {
-  exists(int size, Class c |
-    c = op.getType().getUnderlyingType() and
-    c.getSize() = size and
-    getFieldSizeOfClass(c, type, size)
-  )
+/**
+ * INTERNAL: Do not use.
+ */
+module ReadNodeFlow {
+  /** Holds if the read node `nodeTo` should receive flow from `nodeFrom`. */
+  predicate flowInto(Node nodeFrom, ReadNode nodeTo) {
+    nodeTo.isInitial() and
+    (
+      // If we entered through an address operand.
+      nodeFrom.asOperand().getDef() = nodeTo.getInstruction()
+      or
+      // If we entered flow through a memory-producing instruction.
+      // This can happen if we have flow to an `InitializeParameterIndirection` through
+      // a `ReadSideEffectInstruction`.
+      exists(Instruction load, Instruction def |
+        def = nodeFrom.asInstruction() and
+        def = Ssa::getSourceValueOperand(load).getAnyDef() and
+        not def = any(StoreNode store).getStoreInstruction() and
+        pragma[only_bind_into](nodeTo).getALoadInstruction() = load
+      )
+    )
+  }
+
+  /**
+   * Holds if the read node `nodeTo` should receive flow from the read node `nodeFrom`.
+   *
+   * This happens when `readFrom` is _not_ the source of a `readStep`, and `nodeTo` is
+   * the `ReadNode` that represents an address that directly depends on `nodeFrom`.
+   */
+  predicate flowThrough(ReadNode nodeFrom, ReadNode nodeTo) {
+    not readStep(nodeFrom, _, _) and
+    nodeFrom.getOuter() = nodeTo
+  }
+
+  /**
+   * Holds if flow should leave the read node `nFrom` and enter the node `nodeTo`.
+   * This happens either because there is use-use flow from one of the variables used in
+   * the read operation, or because we have traversed all the field dereferences in the
+   * read operation.
+   */
+  predicate flowOutOf(ReadNode nFrom, Node nodeTo) {
+    // Use-use flow to another use of the same variable instruction
+    Ssa::ssaFlow(nFrom, nodeTo)
+    or
+    not exists(nFrom.getInner()) and
+    exists(Node store |
+      Ssa::explicitWrite(_, store.asInstruction(), nFrom.getInstruction()) and
+      Ssa::ssaFlow(store, nodeTo)
+    )
+    or
+    // Flow out of read nodes and into memory instructions if we cannot move any further through
+    // read nodes.
+    nFrom.isTerminal() and
+    (
+      exists(Instruction load |
+        load = nodeTo.asInstruction() and
+        Ssa::getSourceAddress(load) = nFrom.getInstruction()
+      )
+      or
+      exists(CallInstruction call, int i |
+        call.getArgument(i) = nodeTo.asInstruction() and
+        call.getArgument(i) = nFrom.getInstruction()
+      )
+    )
+  }
+}
+
+/**
+ * INTERNAL: Do not use.
+ */
+module StoreNodeFlow {
+  /** Holds if the store node `nodeTo` should receive flow from `nodeFrom`. */
+  predicate flowInto(Instruction instrFrom, StoreNode nodeTo) {
+    nodeTo.flowInto(Ssa::getDestinationAddress(instrFrom))
+  }
+
+  /**
+   * Holds if the store node `nodeTo` should receive flow from `nodeFom`.
+   *
+   * This happens when `nodeFrom` is _not_ the source of a `storeStep`, and `nodeFrom` is
+   * the `Storenode` that represents an address that directly depends on `nodeTo`.
+   */
+  predicate flowThrough(StoreNode nodeFrom, StoreNode nodeTo) {
+    // Flow through a post update node that doesn't need a store step.
+    not storeStep(nodeFrom, _, _) and
+    nodeTo.getOuter() = nodeFrom
+  }
+
+  /**
+   * Holds if flow should leave the store node `nodeFrom` and enter the node `nodeTo`.
+   * This happens because we have traversed an entire chain of field dereferences
+   * after a store operation.
+   */
+  predicate flowOutOf(StoreNodeInstr nFrom, Node nodeTo) {
+    nFrom.isTerminal() and
+    Ssa::ssaFlow(nFrom, nodeTo)
+  }
 }
 
 private predicate simpleOperandLocalFlowStep(Instruction iFrom, Operand opTo) {
   // Propagate flow from an instruction to its exact uses.
+  // We do this for all instruction/operand pairs, except when the operand is the
+  // side effect operand of a ReturnIndirectionInstruction, or the load operand of a LoadInstruction.
+  // This is because we get these flows through the shared SSA library already, and including this
+  // flow here will create multiple dataflow paths which creates a blowup in stage 3 of dataflow.
+  (
+    not any(ReturnIndirectionInstruction ret).getSideEffectOperand() = opTo and
+    not any(LoadInstruction load).getSourceValueOperand() = opTo and
+    not any(ReturnValueInstruction ret).getReturnValueOperand() = opTo
+  ) and
   opTo.getDef() = iFrom
-  or
-  opTo = any(ReadSideEffectInstruction read).getSideEffectOperand() and
-  not iFrom.isResultConflated() and
-  iFrom = opTo.getAnyDef()
-  or
-  // Loading a single `int` from an `int *` parameter is not an exact load since
-  // the parameter may point to an entire array rather than a single `int`. The
-  // following rule ensures that any flow going into the
-  // `InitializeIndirectionInstruction`, even if it's for a different array
-  // element, will propagate to a load of the first element.
-  //
-  // Since we're linking `InitializeIndirectionInstruction` and
-  // `LoadInstruction` together directly, this rule will break if there's any
-  // reassignment of the parameter indirection, including a conditional one that
-  // leads to a phi node.
-  exists(InitializeIndirectionInstruction init |
-    iFrom = init and
-    opTo.(LoadOperand).getAnyDef() = init and
-    // Check that the types match. Otherwise we can get flow from an object to
-    // its fields, which leads to field conflation when there's flow from other
-    // fields to the object elsewhere.
-    init.getParameter().getType().getUnspecifiedType().(DerivedType).getBaseType() =
-      opTo.getType().getUnspecifiedType()
-  )
-  or
-  // Flow from stores to structs with a single field to a load of that field.
-  exists(LoadInstruction load |
-    load.getSourceValueOperand() = opTo and
-    opTo.getAnyDef() = iFrom and
-    isSingleFieldClass(pragma[only_bind_out](pragma[only_bind_out](iFrom).getResultType()), opTo)
+}
+
+pragma[noinline]
+private predicate getAddressType(LoadInstruction load, Type t) {
+  exists(Instruction address |
+    address = load.getSourceAddress() and
+    t = address.getResultType()
   )
 }
 
+/**
+ * Like the AST dataflow library, we want to conflate the address and value of a reference. This class
+ * represents the `LoadInstruction` that is generated from a reference dereference.
+ */
+private class ReferenceDereferenceInstruction extends LoadInstruction {
+  ReferenceDereferenceInstruction() {
+    exists(ReferenceType ref |
+      getAddressType(this, ref) and
+      this.getResultType() = ref.getBaseType()
+    )
+  }
+}
+
 private predicate simpleInstructionLocalFlowStep(Operand opFrom, Instruction iTo) {
   iTo.(CopyInstruction).getSourceValueOperand() = opFrom
   or
@@ -681,40 +986,8 @@ private predicate simpleInstructionLocalFlowStep(Operand opFrom, Instruction iTo
   or
   iTo.(InheritanceConversionInstruction).getUnaryOperand() = opFrom
   or
-  // A chi instruction represents a point where a new value (the _partial_
-  // operand) may overwrite an old value (the _total_ operand), but the alias
-  // analysis couldn't determine that it surely will overwrite every bit of it or
-  // that it surely will overwrite no bit of it.
-  //
-  // By allowing flow through the total operand, we ensure that flow is not lost
-  // due to shortcomings of the alias analysis. We may get false flow in cases
-  // where the data is indeed overwritten.
-  //
-  // Flow through the partial operand belongs in the taint-tracking libraries
-  // for now.
-  iTo.getAnOperand().(ChiTotalOperand) = opFrom
-  or
-  // Add flow from write side-effects to non-conflated chi instructions through their
-  // partial operands. From there, a `readStep` will find subsequent reads of that field.
-  // Consider the following example:
-  // ```
-  // void setX(Point* p, int new_x) {
-  //   p->x = new_x;
-  // }
-  // ...
-  // setX(&p, taint());
-  // ```
-  // Here, a `WriteSideEffectInstruction` will provide a new definition for `p->x` after the call to
-  // `setX`, which will be melded into `p` through a chi instruction.
-  exists(ChiInstruction chi | chi = iTo |
-    opFrom.getAnyDef() instanceof WriteSideEffectInstruction and
-    chi.getPartialOperand() = opFrom and
-    not chi.isResultConflated() and
-    // In a call such as `set_value(&x->val);` we don't want the memory representing `x` to receive
-    // dataflow by a simple step. Instead, this is handled by field flow. If we add a simple step here
-    // we can get field-to-object flow.
-    not chi.isPartialUpdate()
-  )
+  // Conflate references and values like in AST dataflow.
+  iTo.(ReferenceDereferenceInstruction).getSourceAddressOperand() = opFrom
   or
   // Flow through modeled functions
   modelFlow(opFrom, iTo)
@@ -788,25 +1061,14 @@ predicate localInstructionFlow(Instruction e1, Instruction e2) {
  */
 predicate localExprFlow(Expr e1, Expr e2) { localFlow(exprNode(e1), exprNode(e2)) }
 
-/**
- * Gets a field corresponding to the bit range `[startBit..endBit)` of class `c`, if any.
- */
-private Field getAField(Class c, int startBit, int endBit) {
-  result.getDeclaringType() = c and
-  startBit = 8 * result.getByteOffset() and
-  endBit = 8 * result.getType().getSize() + startBit
-  or
-  exists(Field f, Class cInner |
-    f = c.getAField() and
-    cInner = f.getUnderlyingType() and
-    result = getAField(cInner, startBit - 8 * f.getByteOffset(), endBit - 8 * f.getByteOffset())
-  )
-}
-
 private newtype TContent =
-  TFieldContent(Class c, int startBit, int endBit) { exists(getAField(c, startBit, endBit)) } or
-  TCollectionContent() or
-  TArrayContent()
+  TFieldContent(Field f) {
+    // As reads and writes to union fields can create flow even though the reads and writes
+    // target different fields, we don't want a read (write) to create a read (write) step.
+    not f.getDeclaringType() instanceof Union
+  } or
+  TCollectionContent() or // Not used in C/C++
+  TArrayContent() // Not used in C/C++.
 
 /**
  * A description of the way data may be stored inside an object. Examples
@@ -824,18 +1086,13 @@ class Content extends TContent {
 
 /** A reference through an instance field. */
 class FieldContent extends Content, TFieldContent {
-  Class c;
-  int startBit;
-  int endBit;
+  Field f;
 
-  FieldContent() { this = TFieldContent(c, startBit, endBit) }
+  FieldContent() { this = TFieldContent(f) }
 
-  // Ensure that there's just 1 result for `toString`.
-  override string toString() { result = min(Field f | f = this.getAField() | f.toString()) }
+  override string toString() { result = f.toString() }
 
-  predicate hasOffset(Class cl, int start, int end) { cl = c and start = startBit and end = endBit }
-
-  Field getAField() { result = getAField(c, startBit, endBit) }
+  Field getField() { result = f }
 }
 
 /** A reference through an array. */

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

@@ -0,0 +1,662 @@
+/**
+ * Provides a language-independent implementation of static single assignment
+ * (SSA) form.
+ */
+
+private import SsaImplSpecific
+
+private BasicBlock getABasicBlockPredecessor(BasicBlock bb) { getABasicBlockSuccessor(result) = bb }
+
+/**
+ * Liveness analysis (based on source variables) to restrict the size of the
+ * SSA representation.
+ */
+private module Liveness {
+  /**
+   * A classification of variable references into reads (of a given kind) and
+   * (certain or uncertain) writes.
+   */
+  private newtype TRefKind =
+    Read(boolean certain) { certain in [false, true] } or
+    Write(boolean certain) { certain in [false, true] }
+
+  private class RefKind extends TRefKind {
+    string toString() {
+      exists(boolean certain | this = Read(certain) and result = "read (" + certain + ")")
+      or
+      exists(boolean certain | this = Write(certain) and result = "write (" + certain + ")")
+    }
+
+    int getOrder() {
+      this = Read(_) and
+      result = 0
+      or
+      this = Write(_) and
+      result = 1
+    }
+  }
+
+  /**
+   * Holds if the `i`th node of basic block `bb` is a reference to `v` of kind `k`.
+   */
+  private predicate ref(BasicBlock bb, int i, SourceVariable v, RefKind k) {
+    exists(boolean certain | variableRead(bb, i, v, certain) | k = Read(certain))
+    or
+    exists(boolean certain | variableWrite(bb, i, v, certain) | k = Write(certain))
+  }
+
+  private newtype OrderedRefIndex =
+    MkOrderedRefIndex(int i, int tag) {
+      exists(RefKind rk | ref(_, i, _, rk) | tag = rk.getOrder())
+    }
+
+  private OrderedRefIndex refOrd(BasicBlock bb, int i, SourceVariable v, RefKind k, int ord) {
+    ref(bb, i, v, k) and
+    result = MkOrderedRefIndex(i, ord) and
+    ord = k.getOrder()
+  }
+
+  /**
+   * Gets the (1-based) rank of the reference to `v` at the `i`th node of
+   * basic block `bb`, which has the given reference kind `k`.
+   *
+   * Reads are considered before writes when they happen at the same index.
+   */
+  private int refRank(BasicBlock bb, int i, SourceVariable v, RefKind k) {
+    refOrd(bb, i, v, k, _) =
+      rank[result](int j, int ord, OrderedRefIndex res |
+        res = refOrd(bb, j, v, _, ord)
+      |
+        res order by j, ord
+      )
+  }
+
+  private int maxRefRank(BasicBlock bb, SourceVariable v) {
+    result = refRank(bb, _, v, _) and
+    not result + 1 = refRank(bb, _, v, _)
+  }
+
+  /**
+   * Gets the (1-based) rank of the first reference to `v` inside basic block `bb`
+   * that is either a read or a certain write.
+   */
+  private int firstReadOrCertainWrite(BasicBlock bb, SourceVariable v) {
+    result =
+      min(int r, RefKind k |
+        r = refRank(bb, _, v, k) and
+        k != Write(false)
+      |
+        r
+      )
+  }
+
+  /**
+   * Holds if source variable `v` is live at the beginning of basic block `bb`.
+   */
+  predicate liveAtEntry(BasicBlock bb, SourceVariable v) {
+    // The first read or certain write to `v` inside `bb` is a read
+    refRank(bb, _, v, Read(_)) = firstReadOrCertainWrite(bb, v)
+    or
+    // There is no certain write to `v` inside `bb`, but `v` is live at entry
+    // to a successor basic block of `bb`
+    not exists(firstReadOrCertainWrite(bb, v)) and
+    liveAtExit(bb, v)
+  }
+
+  /**
+   * Holds if source variable `v` is live at the end of basic block `bb`.
+   */
+  predicate liveAtExit(BasicBlock bb, SourceVariable v) {
+    liveAtEntry(getABasicBlockSuccessor(bb), v)
+  }
+
+  /**
+   * Holds if variable `v` is live in basic block `bb` at index `i`.
+   * The rank of `i` is `rnk` as defined by `refRank()`.
+   */
+  private predicate liveAtRank(BasicBlock bb, int i, SourceVariable v, int rnk) {
+    exists(RefKind kind | rnk = refRank(bb, i, v, kind) |
+      rnk = maxRefRank(bb, v) and
+      liveAtExit(bb, v)
+      or
+      ref(bb, i, v, kind) and
+      kind = Read(_)
+      or
+      exists(RefKind nextKind |
+        liveAtRank(bb, _, v, rnk + 1) and
+        rnk + 1 = refRank(bb, _, v, nextKind) and
+        nextKind != Write(true)
+      )
+    )
+  }
+
+  /**
+   * Holds if variable `v` is live after the (certain or uncertain) write at
+   * index `i` inside basic block `bb`.
+   */
+  predicate liveAfterWrite(BasicBlock bb, int i, SourceVariable v) {
+    exists(int rnk | rnk = refRank(bb, i, v, Write(_)) | liveAtRank(bb, i, v, rnk))
+  }
+}
+
+private import Liveness
+
+/**
+ * Holds if `df` is in the dominance frontier of `bb`.
+ *
+ * This is equivalent to:
+ *
+ * ```ql
+ * bb = getImmediateBasicBlockDominator*(getABasicBlockPredecessor(df)) and
+ * not bb = getImmediateBasicBlockDominator+(df)
+ * ```
+ */
+private predicate inDominanceFrontier(BasicBlock bb, BasicBlock df) {
+  bb = getABasicBlockPredecessor(df) and not bb = getImmediateBasicBlockDominator(df)
+  or
+  exists(BasicBlock prev | inDominanceFrontier(prev, df) |
+    bb = getImmediateBasicBlockDominator(prev) and
+    not bb = getImmediateBasicBlockDominator(df)
+  )
+}
+
+/**
+ * Holds if `bb` is in the dominance frontier of a block containing a
+ * definition of `v`.
+ */
+pragma[noinline]
+private predicate inDefDominanceFrontier(BasicBlock bb, SourceVariable v) {
+  exists(BasicBlock defbb, Definition def |
+    def.definesAt(v, defbb, _) and
+    inDominanceFrontier(defbb, bb)
+  )
+}
+
+cached
+newtype TDefinition =
+  TWriteDef(SourceVariable v, BasicBlock bb, int i) {
+    variableWrite(bb, i, v, _) and
+    liveAfterWrite(bb, i, v)
+  } or
+  TPhiNode(SourceVariable v, BasicBlock bb) {
+    inDefDominanceFrontier(bb, v) and
+    liveAtEntry(bb, v)
+  }
+
+private module SsaDefReaches {
+  newtype TSsaRefKind =
+    SsaRead() or
+    SsaDef()
+
+  /**
+   * A classification of SSA variable references into reads and definitions.
+   */
+  class SsaRefKind extends TSsaRefKind {
+    string toString() {
+      this = SsaRead() and
+      result = "SsaRead"
+      or
+      this = SsaDef() and
+      result = "SsaDef"
+    }
+
+    int getOrder() {
+      this = SsaRead() and
+      result = 0
+      or
+      this = SsaDef() and
+      result = 1
+    }
+  }
+
+  /**
+   * Holds if the `i`th node of basic block `bb` is a reference to `v`,
+   * either a read (when `k` is `SsaRead()`) or an SSA definition (when `k`
+   * is `SsaDef()`).
+   *
+   * Unlike `Liveness::ref`, this includes `phi` nodes.
+   */
+  predicate ssaRef(BasicBlock bb, int i, SourceVariable v, SsaRefKind k) {
+    variableRead(bb, i, v, _) and
+    k = SsaRead()
+    or
+    exists(Definition def | def.definesAt(v, bb, i)) and
+    k = SsaDef()
+  }
+
+  private newtype OrderedSsaRefIndex =
+    MkOrderedSsaRefIndex(int i, SsaRefKind k) { ssaRef(_, i, _, k) }
+
+  private OrderedSsaRefIndex ssaRefOrd(BasicBlock bb, int i, SourceVariable v, SsaRefKind k, int ord) {
+    ssaRef(bb, i, v, k) and
+    result = MkOrderedSsaRefIndex(i, k) and
+    ord = k.getOrder()
+  }
+
+  /**
+   * Gets the (1-based) rank of the reference to `v` at the `i`th node of basic
+   * block `bb`, which has the given reference kind `k`.
+   *
+   * For example, if `bb` is a basic block with a phi node for `v` (considered
+   * to be at index -1), reads `v` at node 2, and defines it at node 5, we have:
+   *
+   * ```ql
+   * ssaRefRank(bb, -1, v, SsaDef()) = 1    // phi node
+   * ssaRefRank(bb,  2, v, Read())   = 2    // read at node 2
+   * ssaRefRank(bb,  5, v, SsaDef()) = 3    // definition at node 5
+   * ```
+   *
+   * Reads are considered before writes when they happen at the same index.
+   */
+  int ssaRefRank(BasicBlock bb, int i, SourceVariable v, SsaRefKind k) {
+    ssaRefOrd(bb, i, v, k, _) =
+      rank[result](int j, int ord, OrderedSsaRefIndex res |
+        res = ssaRefOrd(bb, j, v, _, ord)
+      |
+        res order by j, ord
+      )
+  }
+
+  int maxSsaRefRank(BasicBlock bb, SourceVariable v) {
+    result = ssaRefRank(bb, _, v, _) and
+    not result + 1 = ssaRefRank(bb, _, v, _)
+  }
+
+  /**
+   * Holds if the SSA definition `def` reaches rank index `rnk` in its own
+   * basic block `bb`.
+   */
+  predicate ssaDefReachesRank(BasicBlock bb, Definition def, int rnk, SourceVariable v) {
+    exists(int i |
+      rnk = ssaRefRank(bb, i, v, SsaDef()) and
+      def.definesAt(v, bb, i)
+    )
+    or
+    ssaDefReachesRank(bb, def, rnk - 1, v) and
+    rnk = ssaRefRank(bb, _, v, SsaRead())
+  }
+
+  /**
+   * Holds if the SSA definition of `v` at `def` reaches index `i` in the same
+   * basic block `bb`, without crossing another SSA definition of `v`.
+   */
+  predicate ssaDefReachesReadWithinBlock(SourceVariable v, Definition def, BasicBlock bb, int i) {
+    exists(int rnk |
+      ssaDefReachesRank(bb, def, rnk, v) and
+      rnk = ssaRefRank(bb, i, v, SsaRead())
+    )
+  }
+
+  /**
+   * Holds if the SSA definition of `v` at `def` reaches uncertain SSA definition
+   * `redef` in the same basic block, without crossing another SSA definition of `v`.
+   */
+  predicate ssaDefReachesUncertainDefWithinBlock(
+    SourceVariable v, Definition def, UncertainWriteDefinition redef
+  ) {
+    exists(BasicBlock bb, int rnk, int i |
+      ssaDefReachesRank(bb, def, rnk, v) and
+      rnk = ssaRefRank(bb, i, v, SsaDef()) - 1 and
+      redef.definesAt(v, bb, i)
+    )
+  }
+
+  /**
+   * Same as `ssaRefRank()`, but restricted to a particular SSA definition `def`.
+   */
+  int ssaDefRank(Definition def, SourceVariable v, BasicBlock bb, int i, SsaRefKind k) {
+    v = def.getSourceVariable() and
+    result = ssaRefRank(bb, i, v, k) and
+    (
+      ssaDefReachesRead(_, def, bb, i)
+      or
+      def.definesAt(_, bb, i)
+    )
+  }
+
+  /**
+   * Holds if the reference to `def` at index `i` in basic block `bb` is the
+   * last reference to `v` inside `bb`.
+   */
+  pragma[noinline]
+  predicate lastSsaRef(Definition def, SourceVariable v, BasicBlock bb, int i) {
+    ssaDefRank(def, v, bb, i, _) = maxSsaRefRank(bb, v)
+  }
+
+  predicate defOccursInBlock(Definition def, BasicBlock bb, SourceVariable v) {
+    exists(ssaDefRank(def, v, bb, _, _))
+  }
+
+  pragma[noinline]
+  private predicate ssaDefReachesThroughBlock(Definition def, BasicBlock bb) {
+    ssaDefReachesEndOfBlock(bb, def, _) and
+    not defOccursInBlock(_, bb, def.getSourceVariable())
+  }
+
+  /**
+   * Holds if `def` is accessed in basic block `bb1` (either a read or a write),
+   * `bb2` is a transitive successor of `bb1`, `def` is live at the end of `bb1`,
+   * and the underlying variable for `def` is neither read nor written in any block
+   * on the path between `bb1` and `bb2`.
+   */
+  predicate varBlockReaches(Definition def, BasicBlock bb1, BasicBlock bb2) {
+    defOccursInBlock(def, bb1, _) and
+    bb2 = getABasicBlockSuccessor(bb1)
+    or
+    exists(BasicBlock mid |
+      varBlockReaches(def, bb1, mid) and
+      ssaDefReachesThroughBlock(def, mid) and
+      bb2 = getABasicBlockSuccessor(mid)
+    )
+  }
+
+  /**
+   * Holds if `def` is accessed in basic block `bb1` (either a read or a write),
+   * `def` is read at index `i2` in basic block `bb2`, `bb2` is in a transitive
+   * successor block of `bb1`, and `def` is neither read nor written in any block
+   * on a path between `bb1` and `bb2`.
+   */
+  predicate defAdjacentRead(Definition def, BasicBlock bb1, BasicBlock bb2, int i2) {
+    varBlockReaches(def, bb1, bb2) and
+    ssaRefRank(bb2, i2, def.getSourceVariable(), SsaRead()) = 1
+  }
+}
+
+private import SsaDefReaches
+
+pragma[nomagic]
+predicate liveThrough(BasicBlock bb, SourceVariable v) {
+  liveAtExit(bb, v) and
+  not ssaRef(bb, _, v, SsaDef())
+}
+
+/**
+ * NB: If this predicate is exposed, it should be cached.
+ *
+ * Holds if the SSA definition of `v` at `def` reaches the end of basic
+ * block `bb`, at which point it is still live, without crossing another
+ * SSA definition of `v`.
+ */
+pragma[nomagic]
+predicate ssaDefReachesEndOfBlock(BasicBlock bb, Definition def, SourceVariable v) {
+  exists(int last | last = maxSsaRefRank(bb, v) |
+    ssaDefReachesRank(bb, def, last, v) and
+    liveAtExit(bb, v)
+  )
+  or
+  // The construction of SSA form ensures that each read of a variable is
+  // dominated by its definition. An SSA definition therefore reaches a
+  // control flow node if it is the _closest_ SSA definition that dominates
+  // the node. If two definitions dominate a node then one must dominate the
+  // other, so therefore the definition of _closest_ is given by the dominator
+  // tree. Thus, reaching definitions can be calculated in terms of dominance.
+  ssaDefReachesEndOfBlock(getImmediateBasicBlockDominator(bb), def, pragma[only_bind_into](v)) and
+  liveThrough(bb, pragma[only_bind_into](v))
+}
+
+/**
+ * NB: If this predicate is exposed, it should be cached.
+ *
+ * Holds if `inp` is an input to the phi node `phi` along the edge originating in `bb`.
+ */
+pragma[nomagic]
+predicate phiHasInputFromBlock(PhiNode phi, Definition inp, BasicBlock bb) {
+  exists(SourceVariable v, BasicBlock bbDef |
+    phi.definesAt(v, bbDef, _) and
+    getABasicBlockPredecessor(bbDef) = bb and
+    ssaDefReachesEndOfBlock(bb, inp, v)
+  )
+}
+
+/**
+ * NB: If this predicate is exposed, it should be cached.
+ *
+ * Holds if the SSA definition of `v` at `def` reaches a read at index `i` in
+ * basic block `bb`, without crossing another SSA definition of `v`. The read
+ * is of kind `rk`.
+ */
+pragma[nomagic]
+predicate ssaDefReachesRead(SourceVariable v, Definition def, BasicBlock bb, int i) {
+  ssaDefReachesReadWithinBlock(v, def, bb, i)
+  or
+  variableRead(bb, i, v, _) and
+  ssaDefReachesEndOfBlock(getABasicBlockPredecessor(bb), def, v) and
+  not ssaDefReachesReadWithinBlock(v, _, bb, i)
+}
+
+/**
+ * NB: If this predicate is exposed, it should be cached.
+ *
+ * Holds if `def` is accessed at index `i1` in basic block `bb1` (either a read
+ * or a write), `def` is read at index `i2` in basic block `bb2`, and there is a
+ * path between them without any read of `def`.
+ */
+pragma[nomagic]
+predicate adjacentDefRead(Definition def, BasicBlock bb1, int i1, BasicBlock bb2, int i2) {
+  exists(int rnk |
+    rnk = ssaDefRank(def, _, bb1, i1, _) and
+    rnk + 1 = ssaDefRank(def, _, bb1, i2, SsaRead()) and
+    variableRead(bb1, i2, _, _) and
+    bb2 = bb1
+  )
+  or
+  lastSsaRef(def, _, bb1, i1) and
+  defAdjacentRead(def, bb1, bb2, i2)
+}
+
+pragma[noinline]
+private predicate adjacentDefRead(
+  Definition def, BasicBlock bb1, int i1, BasicBlock bb2, int i2, SourceVariable v
+) {
+  adjacentDefRead(def, bb1, i1, bb2, i2) and
+  v = def.getSourceVariable()
+}
+
+private predicate adjacentDefReachesRead(
+  Definition def, BasicBlock bb1, int i1, BasicBlock bb2, int i2
+) {
+  exists(SourceVariable v | adjacentDefRead(def, bb1, i1, bb2, i2, v) |
+    ssaRef(bb1, i1, v, SsaDef())
+    or
+    variableRead(bb1, i1, v, true)
+  )
+  or
+  exists(BasicBlock bb3, int i3 |
+    adjacentDefReachesRead(def, bb1, i1, bb3, i3) and
+    variableRead(bb3, i3, _, false) and
+    adjacentDefRead(def, bb3, i3, bb2, i2)
+  )
+}
+
+/**
+ * NB: If this predicate is exposed, it should be cached.
+ *
+ * Same as `adjacentDefRead`, but ignores uncertain reads.
+ */
+pragma[nomagic]
+predicate adjacentDefNoUncertainReads(Definition def, BasicBlock bb1, int i1, BasicBlock bb2, int i2) {
+  adjacentDefReachesRead(def, bb1, i1, bb2, i2) and
+  variableRead(bb2, i2, _, true)
+}
+
+/**
+ * NB: If this predicate is exposed, it should be cached.
+ *
+ * Holds if the node at index `i` in `bb` is a last reference to SSA definition
+ * `def`. The reference is last because it can reach another write `next`,
+ * without passing through another read or write.
+ */
+pragma[nomagic]
+predicate lastRefRedef(Definition def, BasicBlock bb, int i, Definition next) {
+  exists(SourceVariable v |
+    // Next reference to `v` inside `bb` is a write
+    exists(int rnk, int j |
+      rnk = ssaDefRank(def, v, bb, i, _) and
+      next.definesAt(v, bb, j) and
+      rnk + 1 = ssaRefRank(bb, j, v, SsaDef())
+    )
+    or
+    // Can reach a write using one or more steps
+    lastSsaRef(def, v, bb, i) and
+    exists(BasicBlock bb2 |
+      varBlockReaches(def, bb, bb2) and
+      1 = ssaDefRank(next, v, bb2, _, SsaDef())
+    )
+  )
+}
+
+/**
+ * NB: If this predicate is exposed, it should be cached.
+ *
+ * Holds if `inp` is an immediately preceding definition of uncertain definition
+ * `def`. Since `def` is uncertain, the value from the preceding definition might
+ * still be valid.
+ */
+pragma[nomagic]
+predicate uncertainWriteDefinitionInput(UncertainWriteDefinition def, Definition inp) {
+  lastRefRedef(inp, _, _, def)
+}
+
+private predicate adjacentDefReachesUncertainRead(
+  Definition def, BasicBlock bb1, int i1, BasicBlock bb2, int i2
+) {
+  adjacentDefReachesRead(def, bb1, i1, bb2, i2) and
+  variableRead(bb2, i2, _, false)
+}
+
+/**
+ * NB: If this predicate is exposed, it should be cached.
+ *
+ * Same as `lastRefRedef`, but ignores uncertain reads.
+ */
+pragma[nomagic]
+predicate lastRefRedefNoUncertainReads(Definition def, BasicBlock bb, int i, Definition next) {
+  lastRefRedef(def, bb, i, next) and
+  not variableRead(bb, i, def.getSourceVariable(), false)
+  or
+  exists(BasicBlock bb0, int i0 |
+    lastRefRedef(def, bb0, i0, next) and
+    adjacentDefReachesUncertainRead(def, bb, i, bb0, i0)
+  )
+}
+
+/**
+ * NB: If this predicate is exposed, it should be cached.
+ *
+ * Holds if the node at index `i` in `bb` is a last reference to SSA
+ * definition `def`.
+ *
+ * That is, the node can reach the end of the enclosing callable, or another
+ * SSA definition for the underlying source variable, without passing through
+ * another read.
+ */
+pragma[nomagic]
+predicate lastRef(Definition def, BasicBlock bb, int i) {
+  lastRefRedef(def, bb, i, _)
+  or
+  lastSsaRef(def, _, bb, i) and
+  (
+    // Can reach exit directly
+    bb instanceof ExitBasicBlock
+    or
+    // Can reach a block using one or more steps, where `def` is no longer live
+    exists(BasicBlock bb2 | varBlockReaches(def, bb, bb2) |
+      not defOccursInBlock(def, bb2, _) and
+      not ssaDefReachesEndOfBlock(bb2, def, _)
+    )
+  )
+}
+
+/**
+ * NB: If this predicate is exposed, it should be cached.
+ *
+ * Same as `lastRefRedef`, but ignores uncertain reads.
+ */
+pragma[nomagic]
+predicate lastRefNoUncertainReads(Definition def, BasicBlock bb, int i) {
+  lastRef(def, bb, i) and
+  not variableRead(bb, i, def.getSourceVariable(), false)
+  or
+  exists(BasicBlock bb0, int i0 |
+    lastRef(def, bb0, i0) and
+    adjacentDefReachesUncertainRead(def, bb, i, bb0, i0)
+  )
+}
+
+/** A static single assignment (SSA) definition. */
+class Definition extends TDefinition {
+  /** Gets the source variable underlying this SSA definition. */
+  SourceVariable getSourceVariable() { this.definesAt(result, _, _) }
+
+  /**
+   * Holds if this SSA definition defines `v` at index `i` in basic block `bb`.
+   * Phi nodes are considered to be at index `-1`, while normal variable writes
+   * are at the index of the control flow node they wrap.
+   */
+  final predicate definesAt(SourceVariable v, BasicBlock bb, int i) {
+    this = TWriteDef(v, bb, i)
+    or
+    this = TPhiNode(v, bb) and i = -1
+  }
+
+  /** Gets the basic block to which this SSA definition belongs. */
+  final BasicBlock getBasicBlock() { this.definesAt(_, result, _) }
+
+  /** Gets a textual representation of this SSA definition. */
+  string toString() { none() }
+}
+
+/** An SSA definition that corresponds to a write. */
+class WriteDefinition extends Definition, TWriteDef {
+  private SourceVariable v;
+  private BasicBlock bb;
+  private int i;
+
+  WriteDefinition() { this = TWriteDef(v, bb, i) }
+
+  override string toString() { result = "WriteDef" }
+}
+
+/** A phi node. */
+class PhiNode extends Definition, TPhiNode {
+  override string toString() { result = "Phi" }
+}
+
+/**
+ * An SSA definition that represents an uncertain update of the underlying
+ * source variable.
+ */
+class UncertainWriteDefinition extends WriteDefinition {
+  UncertainWriteDefinition() {
+    exists(SourceVariable v, BasicBlock bb, int i |
+      this.definesAt(v, bb, i) and
+      variableWrite(bb, i, v, false)
+    )
+  }
+}
+
+/** Provides a set of consistency queries. */
+module Consistency {
+  abstract class RelevantDefinition extends Definition {
+    abstract predicate hasLocationInfo(
+      string filepath, int startline, int startcolumn, int endline, int endcolumn
+    );
+  }
+
+  query predicate nonUniqueDef(RelevantDefinition def, SourceVariable v, BasicBlock bb, int i) {
+    ssaDefReachesRead(v, def, bb, i) and
+    not exists(unique(Definition def0 | ssaDefReachesRead(v, def0, bb, i)))
+  }
+
+  query predicate readWithoutDef(SourceVariable v, BasicBlock bb, int i) {
+    variableRead(bb, i, v, _) and
+    not ssaDefReachesRead(v, _, bb, i)
+  }
+
+  query predicate deadDef(RelevantDefinition def, SourceVariable v) {
+    v = def.getSourceVariable() and
+    not ssaDefReachesRead(_, def, _, _) and
+    not phiHasInputFromBlock(_, def, _) and
+    not uncertainWriteDefinitionInput(_, def)
+  }
+}

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

@@ -0,0 +1,18 @@
+private import semmle.code.cpp.ir.IR
+private import SsaInternals as Ssa
+
+class BasicBlock = IRBlock;
+
+class SourceVariable = Ssa::SourceVariable;
+
+BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.immediatelyDominates(bb) }
+
+BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() }
+
+class ExitBasicBlock extends IRBlock {
+  ExitBasicBlock() { this.getLastInstruction() instanceof ExitFunctionInstruction }
+}
+
+predicate variableWrite = Ssa::variableWrite/4;
+
+predicate variableRead = Ssa::variableRead/4;

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

@@ -0,0 +1,639 @@
+import SsaImplCommon
+private import cpp as Cpp
+private import semmle.code.cpp.ir.IR
+private import DataFlowUtil
+private import DataFlowImplCommon as DataFlowImplCommon
+private import semmle.code.cpp.models.interfaces.Allocation as Alloc
+private import semmle.code.cpp.models.interfaces.DataFlow as DataFlow
+
+private module SourceVariables {
+  private newtype TSourceVariable =
+    TSourceIRVariable(IRVariable var) or
+    TSourceIRVariableIndirection(InitializeIndirectionInstruction init)
+
+  abstract class SourceVariable extends TSourceVariable {
+    IRVariable var;
+
+    abstract string toString();
+  }
+
+  class SourceIRVariable extends SourceVariable, TSourceIRVariable {
+    SourceIRVariable() { this = TSourceIRVariable(var) }
+
+    IRVariable getIRVariable() { result = var }
+
+    override string toString() { result = this.getIRVariable().toString() }
+  }
+
+  class SourceIRVariableIndirection extends SourceVariable, TSourceIRVariableIndirection {
+    InitializeIndirectionInstruction init;
+
+    SourceIRVariableIndirection() {
+      this = TSourceIRVariableIndirection(init) and var = init.getIRVariable()
+    }
+
+    IRVariable getUnderlyingIRVariable() { result = var }
+
+    override string toString() { result = "*" + this.getUnderlyingIRVariable().toString() }
+  }
+}
+
+import SourceVariables
+
+cached
+private newtype TDefOrUse =
+  TExplicitDef(Instruction store) { explicitWrite(_, store, _) } or
+  TInitializeParam(Instruction instr) {
+    instr instanceof InitializeParameterInstruction
+    or
+    instr instanceof InitializeIndirectionInstruction
+  } or
+  TExplicitUse(Operand op) { isExplicitUse(op) } or
+  TReturnParamIndirection(Operand op) { returnParameterIndirection(op, _) }
+
+pragma[nomagic]
+private int getRank(DefOrUse defOrUse, IRBlock block) {
+  defOrUse =
+    rank[result](int i, DefOrUse cand |
+      block.getInstruction(i) = toInstruction(cand)
+    |
+      cand order by i
+    )
+}
+
+private class DefOrUse extends TDefOrUse {
+  /** Gets the instruction associated with this definition, if any. */
+  Instruction asDef() { none() }
+
+  /** Gets the operand associated with this use, if any. */
+  Operand asUse() { none() }
+
+  /** Gets a textual representation of this element. */
+  abstract string toString();
+
+  /** Gets the block of this definition or use. */
+  abstract IRBlock getBlock();
+
+  /** Holds if this definition or use has rank `rank` in block `block`. */
+  cached
+  final predicate hasRankInBlock(IRBlock block, int rnk) { rnk = getRank(this, block) }
+
+  /** Gets the location of this element. */
+  abstract Cpp::Location getLocation();
+}
+
+private Instruction toInstruction(DefOrUse defOrUse) {
+  result = defOrUse.asDef()
+  or
+  result = defOrUse.asUse().getUse()
+}
+
+abstract class Def extends DefOrUse {
+  Instruction store;
+
+  /** Gets the instruction of this definition. */
+  Instruction getInstruction() { result = store }
+
+  /** Gets the variable that is defined by this definition. */
+  abstract SourceVariable getSourceVariable();
+
+  /** Holds if this definition is guaranteed to happen. */
+  abstract predicate isCertain();
+
+  override Instruction asDef() { result = this.getInstruction() }
+
+  override string toString() { result = "Def" }
+
+  override IRBlock getBlock() { result = this.getInstruction().getBlock() }
+
+  override Cpp::Location getLocation() { result = store.getLocation() }
+}
+
+private class ExplicitDef extends Def, TExplicitDef {
+  ExplicitDef() { this = TExplicitDef(store) }
+
+  override SourceVariable getSourceVariable() {
+    exists(VariableInstruction var |
+      explicitWrite(_, this.getInstruction(), var) and
+      result.(SourceIRVariable).getIRVariable() = var.getIRVariable()
+    )
+  }
+
+  override predicate isCertain() { explicitWrite(true, this.getInstruction(), _) }
+}
+
+private class ParameterDef extends Def, TInitializeParam {
+  ParameterDef() { this = TInitializeParam(store) }
+
+  override SourceVariable getSourceVariable() {
+    result.(SourceIRVariable).getIRVariable() =
+      store.(InitializeParameterInstruction).getIRVariable()
+    or
+    result.(SourceIRVariableIndirection).getUnderlyingIRVariable() =
+      store.(InitializeIndirectionInstruction).getIRVariable()
+  }
+
+  override predicate isCertain() { any() }
+}
+
+abstract class Use extends DefOrUse {
+  Operand use;
+
+  override Operand asUse() { result = use }
+
+  /** Gets the underlying operand of this use. */
+  Operand getOperand() { result = use }
+
+  override string toString() { result = "Use" }
+
+  /** Gets the variable that is used by this use. */
+  abstract SourceVariable getSourceVariable();
+
+  override IRBlock getBlock() { result = use.getUse().getBlock() }
+
+  override Cpp::Location getLocation() { result = use.getLocation() }
+}
+
+private class ExplicitUse extends Use, TExplicitUse {
+  ExplicitUse() { this = TExplicitUse(use) }
+
+  override SourceVariable getSourceVariable() {
+    exists(VariableInstruction var |
+      use.getDef() = var and
+      if use.getUse() instanceof ReadSideEffectInstruction
+      then result.(SourceIRVariableIndirection).getUnderlyingIRVariable() = var.getIRVariable()
+      else result.(SourceIRVariable).getIRVariable() = var.getIRVariable()
+    )
+  }
+}
+
+private class ReturnParameterIndirection extends Use, TReturnParamIndirection {
+  ReturnParameterIndirection() { this = TReturnParamIndirection(use) }
+
+  override SourceVariable getSourceVariable() {
+    exists(ReturnIndirectionInstruction ret |
+      returnParameterIndirection(use, ret) and
+      result.(SourceIRVariableIndirection).getUnderlyingIRVariable() = ret.getIRVariable()
+    )
+  }
+}
+
+private predicate isExplicitUse(Operand op) {
+  op.getDef() instanceof VariableAddressInstruction and
+  not exists(LoadInstruction load |
+    load.getSourceAddressOperand() = op and
+    load.getAUse().getUse() instanceof InitializeIndirectionInstruction
+  )
+}
+
+private predicate returnParameterIndirection(Operand op, ReturnIndirectionInstruction ret) {
+  ret.getSourceAddressOperand() = op
+}
+
+/**
+ * Holds if `iFrom` computes an address that is used by `iTo`.
+ */
+predicate addressFlow(Instruction iFrom, Instruction iTo) {
+  iTo.(CopyValueInstruction).getSourceValue() = iFrom
+  or
+  iTo.(ConvertInstruction).getUnary() = iFrom
+  or
+  iTo.(CheckedConvertOrNullInstruction).getUnary() = iFrom
+  or
+  iTo.(InheritanceConversionInstruction).getUnary() = iFrom
+  or
+  iTo.(PointerArithmeticInstruction).getLeft() = iFrom
+  or
+  iTo.(FieldAddressInstruction).getObjectAddress() = iFrom
+  or
+  // We traverse `LoadInstruction`s since we want to conclude that the
+  // destination of the store operation `*x = source()` is derived from `x`.
+  iTo.(LoadInstruction).getSourceAddress() = iFrom
+  or
+  // We want to include `ReadSideEffectInstruction`s for the same reason that we include
+  // `LoadInstruction`s, but only when a `WriteSideEffectInstruction` for the same index exists as well
+  // (as otherwise we know that the callee won't override the data). However, given an index `i`, the
+  // destination of the `WriteSideEffectInstruction` for `i` is identical to the source address of the
+  // `ReadSideEffectInstruction` for `i`. So we don't have to talk about the `ReadSideEffectInstruction`
+  // at all.
+  exists(WriteSideEffectInstruction write |
+    write.getPrimaryInstruction() = iTo and
+    write.getDestinationAddress() = iFrom
+  )
+}
+
+/**
+ * The reflexive, transitive closure of `addressFlow` that ends as the address of a
+ * store or read operation.
+ */
+cached
+predicate addressFlowTC(Instruction iFrom, Instruction iTo) {
+  iTo = [getDestinationAddress(_), getSourceAddress(_)] and
+  addressFlow*(iFrom, iTo)
+}
+
+/**
+ * Gets the destination address of `instr` if it is a `StoreInstruction` or
+ * a `WriteSideEffectInstruction`.
+ */
+Instruction getDestinationAddress(Instruction instr) {
+  result =
+    [
+      instr.(StoreInstruction).getDestinationAddress(),
+      instr.(WriteSideEffectInstruction).getDestinationAddress()
+    ]
+}
+
+/** Gets the source address of `instr` if it is an instruction that behaves like a `LoadInstruction`. */
+Instruction getSourceAddress(Instruction instr) { result = getSourceAddressOperand(instr).getDef() }
+
+/**
+ * Gets the operand that represents the source address of `instr` if it is an
+ * instruction that behaves like a `LoadInstruction`.
+ */
+Operand getSourceAddressOperand(Instruction instr) {
+  result =
+    [
+      instr.(LoadInstruction).getSourceAddressOperand(),
+      instr.(ReadSideEffectInstruction).getArgumentOperand()
+    ]
+}
+
+/**
+ * Gets the source address of `node` if it's an instruction or operand that
+ * behaves like a `LoadInstruction`.
+ */
+Instruction getSourceAddressFromNode(Node node) {
+  result = getSourceAddress(node.asInstruction())
+  or
+  result = getSourceAddress(node.asOperand().(SideEffectOperand).getUse())
+}
+
+/** Gets the source value of `instr` if it's an instruction that behaves like a `LoadInstruction`. */
+Instruction getSourceValue(Instruction instr) { result = getSourceValueOperand(instr).getDef() }
+
+/**
+ * Gets the operand that represents the source value of `instr` if it's an instruction
+ * that behaves like a `LoadInstruction`.
+ */
+Operand getSourceValueOperand(Instruction instr) {
+  result = instr.(LoadInstruction).getSourceValueOperand()
+  or
+  result = instr.(ReadSideEffectInstruction).getSideEffectOperand()
+}
+
+/**
+ * Holds if `instr` is a `StoreInstruction` or a `WriteSideEffectInstruction` that writes to an address.
+ * The addresses is computed using `address`, and `certain` is `true` if the write is guaranteed to overwrite
+ * the entire variable.
+ */
+cached
+predicate explicitWrite(boolean certain, Instruction instr, Instruction address) {
+  exists(StoreInstruction store |
+    store = instr and addressFlowTC(address, store.getDestinationAddress())
+  |
+    // Set `certain = false` if the address is derived from any instructions that prevents us from
+    // concluding that the entire variable is overridden.
+    if
+      addressFlowTC(any(Instruction i |
+          i instanceof FieldAddressInstruction or
+          i instanceof PointerArithmeticInstruction or
+          i instanceof LoadInstruction or
+          i instanceof InheritanceConversionInstruction
+        ), store.getDestinationAddress())
+    then certain = false
+    else certain = true
+  )
+  or
+  addressFlowTC(address, instr.(WriteSideEffectInstruction).getDestinationAddress()) and
+  certain = false
+}
+
+cached
+private module Cached {
+  private predicate defUseFlow(Node nodeFrom, Node nodeTo) {
+    exists(IRBlock bb1, int i1, IRBlock bb2, int i2, DefOrUse defOrUse, Use use |
+      defOrUse.hasRankInBlock(bb1, i1) and
+      use.hasRankInBlock(bb2, i2) and
+      adjacentDefRead(_, bb1, i1, bb2, i2) and
+      nodeFrom.asInstruction() = toInstruction(defOrUse) and
+      flowOutOfAddressStep(use.getOperand(), nodeTo)
+    )
+  }
+
+  private predicate fromStoreNode(StoreNodeInstr nodeFrom, Node nodeTo) {
+    // Def-use flow from a `StoreNode`.
+    exists(IRBlock bb1, int i1, IRBlock bb2, int i2, Def def, Use use |
+      nodeFrom.isTerminal() and
+      def.getInstruction() = nodeFrom.getStoreInstruction() and
+      def.hasRankInBlock(bb1, i1) and
+      adjacentDefRead(_, bb1, i1, bb2, i2) and
+      use.hasRankInBlock(bb2, i2) and
+      flowOutOfAddressStep(use.getOperand(), nodeTo)
+    )
+    or
+    // This final case is a bit annoying. The write side effect on an expression like `a = new A;` writes
+    // to a fresh address returned by `operator new`, and there's no easy way to use the shared SSA
+    // library to hook that up to the assignment to `a`. So instead we flow to the _first_ use of the
+    // value computed by `operator new` that occurs after `nodeFrom` (to avoid a loop in the
+    // dataflow graph).
+    exists(WriteSideEffectInstruction write, IRBlock bb, int i1, int i2, Operand op |
+      nodeFrom.getInstruction().(CallInstruction).getStaticCallTarget() instanceof
+        Alloc::OperatorNewAllocationFunction and
+      write = nodeFrom.getStoreInstruction() and
+      bb.getInstruction(i1) = write and
+      bb.getInstruction(i2) = op.getUse() and
+      // Flow to an instruction that occurs later in the block.
+      conversionFlow*(nodeFrom.getInstruction(), op.getDef()) and
+      nodeTo.asOperand() = op and
+      i2 > i1 and
+      // There is no previous instruction that also occurs after `nodeFrom`.
+      not exists(Instruction instr, int i |
+        bb.getInstruction(i) = instr and
+        conversionFlow(instr, op.getDef()) and
+        i1 < i and
+        i < i2
+      )
+    )
+  }
+
+  private predicate fromReadNode(ReadNode nodeFrom, Node nodeTo) {
+    exists(IRBlock bb1, int i1, IRBlock bb2, int i2, Use use1, Use use2 |
+      use1.hasRankInBlock(bb1, i1) and
+      use2.hasRankInBlock(bb2, i2) and
+      use1.getOperand().getDef() = nodeFrom.getInstruction() and
+      adjacentDefRead(_, bb1, i1, bb2, i2) and
+      flowOutOfAddressStep(use2.getOperand(), nodeTo)
+    )
+  }
+
+  private predicate fromPhiNode(SsaPhiNode nodeFrom, Node nodeTo) {
+    exists(PhiNode phi, Use use, IRBlock block, int rnk |
+      phi = nodeFrom.getPhiNode() and
+      adjacentDefRead(phi, _, _, block, rnk) and
+      use.hasRankInBlock(block, rnk) and
+      flowOutOfAddressStep(use.getOperand(), nodeTo)
+    )
+  }
+
+  private predicate toPhiNode(Node nodeFrom, SsaPhiNode nodeTo) {
+    // Flow to phi nodes
+    exists(Def def, IRBlock block, int rnk |
+      def.hasRankInBlock(block, rnk) and
+      nodeTo.hasInputAtRankInBlock(block, rnk)
+    |
+      exists(StoreNodeInstr storeNode |
+        storeNode = nodeFrom and
+        storeNode.isTerminal() and
+        def.getInstruction() = storeNode.getStoreInstruction()
+      )
+      or
+      def.getInstruction() = nodeFrom.asInstruction()
+    )
+    or
+    // Phi -> phi flow
+    nodeTo.hasInputAtRankInBlock(_, _, nodeFrom.(SsaPhiNode).getPhiNode())
+  }
+
+  /**
+   * Holds if `nodeFrom` is a read or write, and `nTo` is the next subsequent read of the variable
+   * written (or read) by `storeOrRead`.
+   */
+  cached
+  predicate ssaFlow(Node nodeFrom, Node nodeTo) {
+    // Def-use/use-use flow from an `InstructionNode`.
+    defUseFlow(nodeFrom, nodeTo)
+    or
+    // Def-use flow from a `StoreNode`.
+    fromStoreNode(nodeFrom, nodeTo)
+    or
+    // Use-use flow from a `ReadNode`.
+    fromReadNode(nodeFrom, nodeTo)
+    or
+    fromPhiNode(nodeFrom, nodeTo)
+    or
+    toPhiNode(nodeFrom, nodeTo)
+    or
+    // When we want to transfer flow out of a `StoreNode` we perform two steps:
+    // 1. Find the next use of the address being stored to
+    // 2. Find the `LoadInstruction` that loads the address
+    // When the address being stored into doesn't have a `LoadInstruction` associated with it because it's
+    // passed into a `CallInstruction` we transfer flow to the `ReadSideEffect`, which will then flow into
+    // the callee. We then pickup the flow from the `InitializeIndirectionInstruction` and use the shared
+    // SSA library to determine where the next use of the address that received the flow is.
+    exists(Node init, Node mid |
+      nodeFrom.asInstruction().(InitializeIndirectionInstruction).getIRVariable() =
+        init.asInstruction().(InitializeParameterInstruction).getIRVariable() and
+      // No need for the flow if the next use is the instruction that returns the flow out of the callee.
+      not mid.asInstruction() instanceof ReturnIndirectionInstruction and
+      // Find the next use of the address
+      ssaFlow(init, mid) and
+      // And flow to the next load of that address
+      flowOutOfAddressStep([mid.asInstruction().getAUse(), mid.asOperand()], nodeTo)
+    )
+  }
+
+  /**
+   * Holds if `iTo` is a conversion-like instruction that copies
+   * the value computed by `iFrom`.
+   *
+   * This predicate is used by `fromStoreNode` to find the next use of a pointer that
+   * points to freshly allocated memory.
+   */
+  private predicate conversionFlow(Instruction iFrom, Instruction iTo) {
+    iTo.(CopyValueInstruction).getSourceValue() = iFrom
+    or
+    iTo.(ConvertInstruction).getUnary() = iFrom
+    or
+    iTo.(CheckedConvertOrNullInstruction).getUnary() = iFrom
+    or
+    iTo.(InheritanceConversionInstruction).getUnary() = iFrom
+  }
+
+  pragma[noinline]
+  private predicate callTargetHasInputOutput(
+    CallInstruction call, DataFlow::FunctionInput input, DataFlow::FunctionOutput output
+  ) {
+    exists(DataFlow::DataFlowFunction func |
+      call.getStaticCallTarget() = func and
+      func.hasDataFlow(input, output)
+    )
+  }
+
+  /**
+   * The role of `flowOutOfAddressStep` is to select the node for which we want dataflow to end up in
+   * after the shared SSA library's `adjacentDefRead` predicate has determined that `operand` is the
+   * next use of some variable.
+   *
+   * More precisely, this predicate holds if `operand` is an operand that represents an address, and:
+   * - `nodeTo` is the next load of that address, or
+   * - `nodeTo` is a `ReadNode` that uses the definition of `operand` to start a sequence of reads, or
+   * - `nodeTo` is the outer-most `StoreNode` that uses the address represented by `operand`. We obtain
+   *    use-use flow in this case since `StoreNodeFlow::flowOutOf` will then provide flow to the next of
+   *    of `operand`.
+   *
+   * There is one final (slightly annoying) case: When `operand` is a an argument to a modeled function
+   * without any `ReadSideEffect` (such as `std::move`). Here, the address flows from the argument to
+   * the return value, which might then be read later.
+   */
+  private predicate flowOutOfAddressStep(Operand operand, Node nodeTo) {
+    // Flow into a read node
+    exists(ReadNode readNode | readNode = nodeTo |
+      readNode.isInitial() and
+      operand.getDef() = readNode.getInstruction()
+    )
+    or
+    exists(StoreNodeInstr storeNode, Instruction def |
+      storeNode = nodeTo and
+      def = operand.getDef()
+    |
+      storeNode.isTerminal() and
+      not addressFlow(def, _) and
+      // Only transfer flow to a store node if it doesn't immediately overwrite the address
+      // we've just written to.
+      explicitWrite(false, storeNode.getStoreInstruction(), def)
+    )
+    or
+    // The destination of a store operation has undergone lvalue-to-rvalue conversion and is now a
+    // right-hand-side of a store operation.
+    // Find the next use of the variable in that store operation, and recursively find the load of that
+    // pointer. For example, consider this case:
+    //
+    // ```cpp
+    // int x = source();
+    // int* p = &x;
+    // sink(*p);
+    // ```
+    //
+    // if we want to find the load of the address of `x`, we see that the pointer is stored into `p`,
+    // and we then need to recursively look for the load of `p`.
+    exists(
+      Def def, StoreInstruction store, IRBlock block1, int rnk1, Use use, IRBlock block2, int rnk2
+    |
+      store = def.getInstruction() and
+      store.getSourceValueOperand() = operand and
+      def.hasRankInBlock(block1, rnk1) and
+      use.hasRankInBlock(block2, rnk2) and
+      adjacentDefRead(_, block1, rnk1, block2, rnk2)
+    |
+      // The shared SSA library has determined that `use` is the next use of the operand
+      // so we find the next load of that use (but only if there is no `PostUpdateNode`) we
+      // need to flow into first.
+      not StoreNodeFlow::flowInto(store, _) and
+      flowOutOfAddressStep(use.getOperand(), nodeTo)
+      or
+      // It may also be the case that `store` gives rise to another store step. So let's make sure that
+      // we also take those into account.
+      StoreNodeFlow::flowInto(store, nodeTo)
+    )
+    or
+    // As we find the next load of an address, we might come across another use of the same variable.
+    // In that case, we recursively find the next use of _that_ operand, and continue searching for
+    // the next load of that operand. For example, consider this case:
+    //
+    // ```cpp
+    // int x = source();
+    // use(&x);
+    // int* p = &x;
+    // sink(*p);
+    // ```
+    //
+    // The next use of `x` after its definition is `use(&x)`, but there is a later load of the address
+    // of `x` that we want to flow to. So we use the shared SSA library to find the next load.
+    not operand = getSourceAddressOperand(_) and
+    exists(Use use1, Use use2, IRBlock block1, int rnk1, IRBlock block2, int rnk2 |
+      use1.getOperand() = operand and
+      use1.hasRankInBlock(block1, rnk1) and
+      // Don't flow to the next use if this use is part of a store operation that totally
+      // overrides a variable.
+      not explicitWrite(true, _, use1.getOperand().getDef()) and
+      adjacentDefRead(_, block1, rnk1, block2, rnk2) and
+      use2.hasRankInBlock(block2, rnk2) and
+      flowOutOfAddressStep(use2.getOperand(), nodeTo)
+    )
+    or
+    operand = getSourceAddressOperand(nodeTo.asInstruction())
+    or
+    exists(ReturnIndirectionInstruction ret |
+      ret.getSourceAddressOperand() = operand and
+      ret = nodeTo.asInstruction()
+    )
+    or
+    exists(ReturnValueInstruction ret |
+      ret.getReturnAddressOperand() = operand and
+      nodeTo.asInstruction() = ret
+    )
+    or
+    exists(CallInstruction call, int index, ReadSideEffectInstruction read |
+      call.getArgumentOperand(index) = operand and
+      read = getSideEffectFor(call, index) and
+      nodeTo.asOperand() = read.getSideEffectOperand()
+    )
+    or
+    exists(CopyInstruction copy |
+      not exists(getSourceAddressOperand(copy)) and
+      copy.getSourceValueOperand() = operand and
+      flowOutOfAddressStep(copy.getAUse(), nodeTo)
+    )
+    or
+    exists(ConvertInstruction convert |
+      convert.getUnaryOperand() = operand and
+      flowOutOfAddressStep(convert.getAUse(), nodeTo)
+    )
+    or
+    exists(CheckedConvertOrNullInstruction convert |
+      convert.getUnaryOperand() = operand and
+      flowOutOfAddressStep(convert.getAUse(), nodeTo)
+    )
+    or
+    exists(InheritanceConversionInstruction convert |
+      convert.getUnaryOperand() = operand and
+      flowOutOfAddressStep(convert.getAUse(), nodeTo)
+    )
+    or
+    exists(PointerArithmeticInstruction arith |
+      arith.getLeftOperand() = operand and
+      flowOutOfAddressStep(arith.getAUse(), nodeTo)
+    )
+    or
+    // Flow through a modeled function that has parameter -> return value flow.
+    exists(
+      CallInstruction call, int index, DataFlow::FunctionInput input,
+      DataFlow::FunctionOutput output
+    |
+      callTargetHasInputOutput(call, input, output) and
+      call.getArgumentOperand(index) = operand and
+      not getSideEffectFor(call, index) instanceof ReadSideEffectInstruction and
+      input.isParameter(index) and
+      output.isReturnValue() and
+      flowOutOfAddressStep(call.getAUse(), nodeTo)
+    )
+  }
+}
+
+import Cached
+
+/**
+ * Holds if the `i`'th write in block `bb` writes to the variable `v`.
+ * `certain` is `true` if the write is guaranteed to overwrite the entire variable.
+ */
+predicate variableWrite(IRBlock bb, int i, SourceVariable v, boolean certain) {
+  DataFlowImplCommon::forceCachingInSameStage() and
+  exists(Def def |
+    def.hasRankInBlock(bb, i) and
+    v = def.getSourceVariable() and
+    (if def.isCertain() then certain = true else certain = false)
+  )
+}
+
+/**
+ * Holds if the `i`'th read in block `bb` reads to the variable `v`.
+ * `certain` is `true` if the read is guaranteed. For C++, this is always the case.
+ */
+predicate variableRead(IRBlock bb, int i, SourceVariable v, boolean certain) {
+  exists(Use use |
+    use.hasRankInBlock(bb, i) and
+    v = use.getSourceVariable() and
+    certain = true
+  )
+}

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

@@ -44,8 +44,6 @@ private predicate instructionToOperandTaintStep(Instruction fromInstr, Operand t
     fromInstr = readInstr.getArgumentDef() and
     toOperand = readInstr.getSideEffectOperand()
   )
-  or
-  toOperand.(LoadOperand).getAnyDef() = fromInstr
 }
 
 /**
@@ -84,8 +82,6 @@ private predicate operandToInstructionTaintStep(Operand opFrom, Instruction inst
     instrTo.(FieldAddressInstruction).getField().getDeclaringType() instanceof Union
   )
   or
-  instrTo.(LoadInstruction).getSourceAddressOperand() = opFrom
-  or
   // Flow from an element to an array or union that contains it.
   instrTo.(ChiInstruction).getPartialOperand() = opFrom and
   not instrTo.isResultConflated() and

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

@@ -762,11 +762,21 @@ class ReturnValueInstruction extends ReturnInstruction {
    */
   final LoadOperand getReturnValueOperand() { result = this.getAnOperand() }
 
+  /**
+   * Gets the operand that provides the address of the value being returned by the function.
+   */
+  final AddressOperand getReturnAddressOperand() { result = this.getAnOperand() }
+
   /**
    * Gets the instruction whose result provides the value being returned by the function, if an
    * exact definition is available.
    */
   final Instruction getReturnValue() { result = this.getReturnValueOperand().getDef() }
+
+  /**
+   * Gets the instruction whose result provides the address of the value being returned by the function.
+   */
+  final Instruction getReturnAddress() { result = this.getReturnAddressOperand().getDef() }
 }
 
 /**

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

@@ -20,6 +20,14 @@ private import internal.OperandInternal
 private class TStageOperand =
   TRegisterOperand or TNonSSAMemoryOperand or TPhiOperand or TChiOperand;
 
+/**
+ * A known location. Testing `loc instanceof KnownLocation` will account for non existing locations, as
+ * opposed to testing `not loc isntanceof UnknownLocation`
+ */
+private class KnownLocation extends Language::Location {
+  KnownLocation() { not this instanceof Language::UnknownLocation }
+}
+
 /**
  * An operand of an `Instruction`. The operand represents a use of the result of one instruction
  * (the defining instruction) in another instruction (the use instruction)
@@ -45,8 +53,10 @@ class Operand extends TStageOperand {
 
   /**
    * Gets the location of the source code for this operand.
+   * By default this is where the operand is used, but some subclasses may override this
+   * using `getAnyDef()` if it makes more sense.
    */
-  final Language::Location getLocation() { result = this.getUse().getLocation() }
+  Language::Location getLocation() { result = this.getUse().getLocation() }
 
   /**
    * Gets the function that contains this operand.
@@ -269,6 +279,10 @@ class RegisterOperand extends NonPhiOperand, TRegisterOperand {
 
   final override string toString() { result = tag.toString() }
 
+  // most `RegisterOperands` have a more meaningful location at the definition
+  // the only exception are specific cases of `ThisArgumentOperand`
+  override Language::Location getLocation() { result = this.getAnyDef().getLocation() }
+
   final override Instruction getAnyDef() { result = defInstr }
 
   final override Overlap getDefinitionOverlap() {
@@ -293,7 +307,7 @@ class NonPhiMemoryOperand extends NonPhiOperand, MemoryOperand, TNonPhiMemoryOpe
   final override string toString() { result = tag.toString() }
 
   final override Instruction getAnyDef() {
-    result = unique(Instruction defInstr | hasDefinition(defInstr, _))
+    result = unique(Instruction defInstr | this.hasDefinition(defInstr, _))
   }
 
   final override Overlap getDefinitionOverlap() { this.hasDefinition(_, result) }
@@ -401,11 +415,19 @@ class ArgumentOperand extends RegisterOperand {
 }
 
 /**
- * An operand representing the implicit 'this' argument to a member function
+ * An operand representing the implicit `this` argument to a member function
  * call.
  */
 class ThisArgumentOperand extends ArgumentOperand {
   override ThisArgumentOperandTag tag;
+
+  // in most cases the def location makes more sense, but in some corner cases it
+  // has an unknown location: in those cases we fall back to the use location
+  override Language::Location getLocation() {
+    if this.getAnyDef().getLocation() instanceof KnownLocation
+    then result = this.getAnyDef().getLocation()
+    else result = this.getUse().getLocation()
+  }
 }
 
 /**

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

@@ -762,11 +762,21 @@ class ReturnValueInstruction extends ReturnInstruction {
    */
   final LoadOperand getReturnValueOperand() { result = this.getAnOperand() }
 
+  /**
+   * Gets the operand that provides the address of the value being returned by the function.
+   */
+  final AddressOperand getReturnAddressOperand() { result = this.getAnOperand() }
+
   /**
    * Gets the instruction whose result provides the value being returned by the function, if an
    * exact definition is available.
    */
   final Instruction getReturnValue() { result = this.getReturnValueOperand().getDef() }
+
+  /**
+   * Gets the instruction whose result provides the address of the value being returned by the function.
+   */
+  final Instruction getReturnAddress() { result = this.getReturnAddressOperand().getDef() }
 }
 
 /**

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

@@ -20,6 +20,14 @@ private import internal.OperandInternal
 private class TStageOperand =
   TRegisterOperand or TNonSSAMemoryOperand or TPhiOperand or TChiOperand;
 
+/**
+ * A known location. Testing `loc instanceof KnownLocation` will account for non existing locations, as
+ * opposed to testing `not loc isntanceof UnknownLocation`
+ */
+private class KnownLocation extends Language::Location {
+  KnownLocation() { not this instanceof Language::UnknownLocation }
+}
+
 /**
  * An operand of an `Instruction`. The operand represents a use of the result of one instruction
  * (the defining instruction) in another instruction (the use instruction)
@@ -45,8 +53,10 @@ class Operand extends TStageOperand {
 
   /**
    * Gets the location of the source code for this operand.
+   * By default this is where the operand is used, but some subclasses may override this
+   * using `getAnyDef()` if it makes more sense.
    */
-  final Language::Location getLocation() { result = this.getUse().getLocation() }
+  Language::Location getLocation() { result = this.getUse().getLocation() }
 
   /**
    * Gets the function that contains this operand.
@@ -269,6 +279,10 @@ class RegisterOperand extends NonPhiOperand, TRegisterOperand {
 
   final override string toString() { result = tag.toString() }
 
+  // most `RegisterOperands` have a more meaningful location at the definition
+  // the only exception are specific cases of `ThisArgumentOperand`
+  override Language::Location getLocation() { result = this.getAnyDef().getLocation() }
+
   final override Instruction getAnyDef() { result = defInstr }
 
   final override Overlap getDefinitionOverlap() {
@@ -293,7 +307,7 @@ class NonPhiMemoryOperand extends NonPhiOperand, MemoryOperand, TNonPhiMemoryOpe
   final override string toString() { result = tag.toString() }
 
   final override Instruction getAnyDef() {
-    result = unique(Instruction defInstr | hasDefinition(defInstr, _))
+    result = unique(Instruction defInstr | this.hasDefinition(defInstr, _))
   }
 
   final override Overlap getDefinitionOverlap() { this.hasDefinition(_, result) }
@@ -401,11 +415,19 @@ class ArgumentOperand extends RegisterOperand {
 }
 
 /**
- * An operand representing the implicit 'this' argument to a member function
+ * An operand representing the implicit `this` argument to a member function
  * call.
  */
 class ThisArgumentOperand extends ArgumentOperand {
   override ThisArgumentOperandTag tag;
+
+  // in most cases the def location makes more sense, but in some corner cases it
+  // has an unknown location: in those cases we fall back to the use location
+  override Language::Location getLocation() {
+    if this.getAnyDef().getLocation() instanceof KnownLocation
+    then result = this.getAnyDef().getLocation()
+    else result = this.getUse().getLocation()
+  }
 }
 
 /**

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

@@ -1032,7 +1032,7 @@ abstract class TranslatedConversion extends TranslatedNonConstantExpr {
 
   final override TranslatedElement getChild(int id) { id = 0 and result = this.getOperand() }
 
-  final TranslatedExpr getOperand() { result = getTranslatedExpr(expr.(Conversion).getExpr()) }
+  final TranslatedExpr getOperand() { result = getTranslatedExpr(expr.getExpr()) }
 }
 
 /**
@@ -1305,9 +1305,9 @@ class TranslatedBinaryOperation extends TranslatedSingleInstructionExpr {
   }
 
   override Opcode getOpcode() {
-    result = binaryArithmeticOpcode(expr.(BinaryArithmeticOperation)) or
-    result = binaryBitwiseOpcode(expr.(BinaryBitwiseOperation)) or
-    result = comparisonOpcode(expr.(ComparisonOperation))
+    result = binaryArithmeticOpcode(expr) or
+    result = binaryBitwiseOpcode(expr) or
+    result = comparisonOpcode(expr)
   }
 
   override int getInstructionElementSize(InstructionTag tag) {

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

@@ -103,9 +103,7 @@ class TranslatedDeclStmt extends TranslatedStmt {
 class TranslatedExprStmt extends TranslatedStmt {
   override ExprStmt stmt;
 
-  TranslatedExpr getExpr() {
-    result = getTranslatedExpr(stmt.(ExprStmt).getExpr().getFullyConverted())
-  }
+  TranslatedExpr getExpr() { result = getTranslatedExpr(stmt.getExpr().getFullyConverted()) }
 
   override TranslatedElement getChild(int id) { id = 0 and result = getExpr() }
 

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

@@ -762,11 +762,21 @@ class ReturnValueInstruction extends ReturnInstruction {
    */
   final LoadOperand getReturnValueOperand() { result = this.getAnOperand() }
 
+  /**
+   * Gets the operand that provides the address of the value being returned by the function.
+   */
+  final AddressOperand getReturnAddressOperand() { result = this.getAnOperand() }
+
   /**
    * Gets the instruction whose result provides the value being returned by the function, if an
    * exact definition is available.
    */
   final Instruction getReturnValue() { result = this.getReturnValueOperand().getDef() }
+
+  /**
+   * Gets the instruction whose result provides the address of the value being returned by the function.
+   */
+  final Instruction getReturnAddress() { result = this.getReturnAddressOperand().getDef() }
 }
 
 /**

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

@@ -20,6 +20,14 @@ private import internal.OperandInternal
 private class TStageOperand =
   TRegisterOperand or TNonSSAMemoryOperand or TPhiOperand or TChiOperand;
 
+/**
+ * A known location. Testing `loc instanceof KnownLocation` will account for non existing locations, as
+ * opposed to testing `not loc isntanceof UnknownLocation`
+ */
+private class KnownLocation extends Language::Location {
+  KnownLocation() { not this instanceof Language::UnknownLocation }
+}
+
 /**
  * An operand of an `Instruction`. The operand represents a use of the result of one instruction
  * (the defining instruction) in another instruction (the use instruction)
@@ -45,8 +53,10 @@ class Operand extends TStageOperand {
 
   /**
    * Gets the location of the source code for this operand.
+   * By default this is where the operand is used, but some subclasses may override this
+   * using `getAnyDef()` if it makes more sense.
    */
-  final Language::Location getLocation() { result = this.getUse().getLocation() }
+  Language::Location getLocation() { result = this.getUse().getLocation() }
 
   /**
    * Gets the function that contains this operand.
@@ -269,6 +279,10 @@ class RegisterOperand extends NonPhiOperand, TRegisterOperand {
 
   final override string toString() { result = tag.toString() }
 
+  // most `RegisterOperands` have a more meaningful location at the definition
+  // the only exception are specific cases of `ThisArgumentOperand`
+  override Language::Location getLocation() { result = this.getAnyDef().getLocation() }
+
   final override Instruction getAnyDef() { result = defInstr }
 
   final override Overlap getDefinitionOverlap() {
@@ -293,7 +307,7 @@ class NonPhiMemoryOperand extends NonPhiOperand, MemoryOperand, TNonPhiMemoryOpe
   final override string toString() { result = tag.toString() }
 
   final override Instruction getAnyDef() {
-    result = unique(Instruction defInstr | hasDefinition(defInstr, _))
+    result = unique(Instruction defInstr | this.hasDefinition(defInstr, _))
   }
 
   final override Overlap getDefinitionOverlap() { this.hasDefinition(_, result) }
@@ -401,11 +415,19 @@ class ArgumentOperand extends RegisterOperand {
 }
 
 /**
- * An operand representing the implicit 'this' argument to a member function
+ * An operand representing the implicit `this` argument to a member function
  * call.
  */
 class ThisArgumentOperand extends ArgumentOperand {
   override ThisArgumentOperandTag tag;
+
+  // in most cases the def location makes more sense, but in some corner cases it
+  // has an unknown location: in those cases we fall back to the use location
+  override Language::Location getLocation() {
+    if this.getAnyDef().getLocation() instanceof KnownLocation
+    then result = this.getAnyDef().getLocation()
+    else result = this.getUse().getLocation()
+  }
 }
 
 /**

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

@@ -34,6 +34,16 @@ private class IteratorByTraits extends Iterator {
   IteratorByTraits() { exists(IteratorTraits it | it.getIteratorType() = this) }
 }
 
+/**
+ * The C++ standard includes an `std::iterator_traits` specialization for pointer types. When
+ * this specialization is included in the database, a pointer type `T*` will be an instance
+ * of the `IteratorByTraits` class. However, if the `T*` specialization is not in the database,
+ * we need to explicitly include them with this class.
+ */
+private class IteratorByPointer extends Iterator instanceof PointerType {
+  IteratorByPointer() { not this instanceof IteratorByTraits }
+}
+
 /**
  * A type which has the typedefs expected for an iterator.
  */

cpp/ql/lib/semmle/code/cpp/padding/Padding.qll

@@ -88,7 +88,7 @@ abstract class Architecture extends string {
     or
     t instanceof LongLongType and result = this.longLongSize()
     or
-    result = this.enumBitSize(t.(Enum))
+    result = this.enumBitSize(t)
     or
     result = this.integralBitSize(t.(SpecifiedType).getBaseType())
     or
@@ -183,7 +183,7 @@ abstract class Architecture extends string {
     or
     t instanceof ReferenceType and result = this.pointerSize()
     or
-    result = this.enumAlignment(t.(Enum))
+    result = this.enumAlignment(t)
     or
     result = this.alignment(t.(SpecifiedType).getBaseType())
     or
@@ -232,14 +232,14 @@ private Field getAnInitialField(PaddedType t) {
     result = t.getAField()
     or
     // Initial field of the type of a field of the union
-    result = getAnInitialField(t.getAField().getUnspecifiedType().(PaddedType))
+    result = getAnInitialField(t.getAField().getUnspecifiedType())
   else
     exists(Field firstField | t.fieldIndex(firstField) = 1 |
       // The first field of `t`
       result = firstField
       or
       // Initial field of the first field of `t`
-      result = getAnInitialField(firstField.getUnspecifiedType().(PaddedType))
+      result = getAnInitialField(firstField.getUnspecifiedType())
     )
 }
 

cpp/ql/lib/semmle/code/cpp/rangeanalysis/RangeSSA.qll

@@ -91,7 +91,7 @@ class RangeSsaDefinition extends ControlFlowNodeBase {
   BasicBlock getBasicBlock() { result.contains(this.getDefinition()) }
 
   /** Whether this definition is a phi node for variable `v`. */
-  predicate isPhiNode(StackVariable v) { exists(RangeSSA x | x.phi_node(v, this.(BasicBlock))) }
+  predicate isPhiNode(StackVariable v) { exists(RangeSSA x | x.phi_node(v, this)) }
 
   /**
    * DEPRECATED: Use isGuardPhi/4 instead

cpp/ql/lib/semmle/code/cpp/security/BufferWrite.qll

@@ -71,13 +71,30 @@ abstract class BufferWrite extends Expr {
    */
   int getMaxData() { none() }
 
+  /**
+   * Gets an upper bound to the amount of data that's being written (if one
+   * can be found), specifying the reason for the estimation.
+   */
+  int getMaxData(BufferWriteEstimationReason reason) {
+    reason instanceof NoSpecifiedEstimateReason and result = getMaxData()
+  }
+
   /**
    * Gets an upper bound to the amount of data that's being written (if one
    * can be found), except that float to string conversions are assumed to be
    * much smaller (8 bytes) than their true maximum length.  This can be
    * helpful in determining the cause of a buffer overflow issue.
    */
-  int getMaxDataLimited() { result = this.getMaxData() }
+  int getMaxDataLimited() { result = getMaxData() }
+
+  /**
+   * Gets an upper bound to the amount of data that's being written (if one
+   * can be found), specifying the reason for the estimation, except that
+   * float to string conversions are assumed to be much smaller (8 bytes)
+   * than their true maximum length.  This can be helpful in determining the
+   * cause of a buffer overflow issue.
+   */
+  int getMaxDataLimited(BufferWriteEstimationReason reason) { result = getMaxData(reason) }
 
   /**
    * Gets the size of a single character of the type this
@@ -135,10 +152,16 @@ class StrCopyBW extends BufferWriteCall {
     result = this.getArgument(this.getParamSize()).getValue().toInt() * this.getCharSize()
   }
 
-  override int getMaxData() {
+  private int getMaxDataImpl(BufferWriteEstimationReason reason) {
+    // when result exists, it is an exact flow analysis
+    reason instanceof ValueFlowAnalysis and
     result =
       this.getArgument(this.getParamSrc()).(AnalysedString).getMaxLength() * this.getCharSize()
   }
+
+  override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
+
+  override int getMaxData() { result = max(getMaxDataImpl(_)) }
 }
 
 /**
@@ -173,10 +196,16 @@ class StrCatBW extends BufferWriteCall {
     result = this.getArgument(this.getParamSize()).getValue().toInt() * this.getCharSize()
   }
 
-  override int getMaxData() {
+  private int getMaxDataImpl(BufferWriteEstimationReason reason) {
+    // when result exists, it is an exact flow analysis
+    reason instanceof ValueFlowAnalysis and
     result =
       this.getArgument(this.getParamSrc()).(AnalysedString).getMaxLength() * this.getCharSize()
   }
+
+  override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
+
+  override int getMaxData() { result = max(getMaxDataImpl(_)) }
 }
 
 /**
@@ -233,19 +262,29 @@ class SprintfBW extends BufferWriteCall {
 
   override Expr getDest() { result = this.getArgument(f.getOutputParameterIndex(false)) }
 
-  override int getMaxData() {
+  private int getMaxDataImpl(BufferWriteEstimationReason reason) {
     exists(FormatLiteral fl |
       fl = this.(FormattingFunctionCall).getFormat() and
-      result = fl.getMaxConvertedLength() * this.getCharSize()
+      result = fl.getMaxConvertedLengthWithReason(reason) * this.getCharSize()
     )
   }
 
-  override int getMaxDataLimited() {
+  override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
+
+  override int getMaxData() { result = max(getMaxDataImpl(_)) }
+
+  private int getMaxDataLimitedImpl(BufferWriteEstimationReason reason) {
     exists(FormatLiteral fl |
       fl = this.(FormattingFunctionCall).getFormat() and
-      result = fl.getMaxConvertedLengthLimited() * this.getCharSize()
+      result = fl.getMaxConvertedLengthLimitedWithReason(reason) * this.getCharSize()
     )
   }
+
+  override int getMaxDataLimited(BufferWriteEstimationReason reason) {
+    result = getMaxDataLimitedImpl(reason)
+  }
+
+  override int getMaxDataLimited() { result = max(getMaxDataLimitedImpl(_)) }
 }
 
 /**
@@ -336,19 +375,29 @@ class SnprintfBW extends BufferWriteCall {
     result = this.getArgument(this.getParamSize()).getValue().toInt() * this.getCharSize()
   }
 
-  override int getMaxData() {
+  private int getMaxDataImpl(BufferWriteEstimationReason reason) {
     exists(FormatLiteral fl |
       fl = this.(FormattingFunctionCall).getFormat() and
-      result = fl.getMaxConvertedLength() * this.getCharSize()
+      result = fl.getMaxConvertedLengthWithReason(reason) * this.getCharSize()
     )
   }
 
-  override int getMaxDataLimited() {
+  override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
+
+  override int getMaxData() { result = max(getMaxDataImpl(_)) }
+
+  private int getMaxDataLimitedImpl(BufferWriteEstimationReason reason) {
     exists(FormatLiteral fl |
       fl = this.(FormattingFunctionCall).getFormat() and
-      result = fl.getMaxConvertedLengthLimited() * this.getCharSize()
+      result = fl.getMaxConvertedLengthLimitedWithReason(reason) * this.getCharSize()
     )
   }
+
+  override int getMaxDataLimited(BufferWriteEstimationReason reason) {
+    result = getMaxDataLimitedImpl(reason)
+  }
+
+  override int getMaxDataLimited() { result = max(getMaxDataLimitedImpl(_)) }
 }
 
 /**
@@ -436,7 +485,9 @@ class ScanfBW extends BufferWrite {
 
   override Expr getDest() { result = this }
 
-  override int getMaxData() {
+  private int getMaxDataImpl(BufferWriteEstimationReason reason) {
+    // when this returns, it is based on exact flow analysis
+    reason instanceof ValueFlowAnalysis and
     exists(ScanfFunctionCall fc, ScanfFormatLiteral fl, int arg |
       this = fc.getArgument(arg) and
       fl = fc.getFormat() and
@@ -444,6 +495,10 @@ class ScanfBW extends BufferWrite {
     )
   }
 
+  override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
+
+  override int getMaxData() { result = max(getMaxDataImpl(_)) }
+
   override string getBWDesc() {
     exists(FunctionCall fc |
       this = fc.getArgument(_) and
@@ -474,8 +529,14 @@ class RealpathBW extends BufferWriteCall {
 
   override Expr getASource() { result = this.getArgument(0) }
 
-  override int getMaxData() {
+  private int getMaxDataImpl(BufferWriteEstimationReason reason) {
+    // although there may be some unknown invariants guaranteeing that a real path is shorter than PATH_MAX, we can consider providing less than PATH_MAX a problem with high precision
+    reason instanceof ValueFlowAnalysis and
     result = path_max() and
     this = this // Suppress a compiler warning
   }
+
+  override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
+
+  override int getMaxData() { result = max(getMaxDataImpl(_)) }
 }

cpp/ql/lib/semmle/code/cpp/security/FileWrite.qll

@@ -173,6 +173,6 @@ private predicate fileWriteWithConvChar(FormattingFunctionCall ffc, Expr source,
     source = ffc.getFormatArgument(n)
   |
     exists(f.getOutputParameterIndex(true)) and
-    conv = ffc.(FormattingFunctionCall).getFormat().(FormatLiteral).getConversionChar(n)
+    conv = ffc.getFormat().(FormatLiteral).getConversionChar(n)
   )
 }

cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll

@@ -589,7 +589,7 @@ private predicate mk_HasAlloc(HashCons hc, NewOrNewArrayExpr new) {
 }
 
 private predicate mk_HasExtent(HashCons hc, NewArrayExpr new) {
-  hc = hashCons(new.(NewArrayExpr).getExtent().getFullyConverted())
+  hc = hashCons(new.getExtent().getFullyConverted())
 }
 
 private predicate analyzableNewExpr(NewExpr new) {
@@ -619,7 +619,7 @@ private predicate analyzableNewArrayExpr(NewArrayExpr new) {
   strictcount(new.getAllocatedType().getUnspecifiedType()) = 1 and
   count(new.getAllocatorCall().getFullyConverted()) <= 1 and
   count(new.getInitializer().getFullyConverted()) <= 1 and
-  count(new.(NewArrayExpr).getExtent().getFullyConverted()) <= 1
+  count(new.getExtent().getFullyConverted()) <= 1
 }
 
 private predicate mk_NewArrayExpr(

cpp/ql/src/AlertSuppression.ql

@@ -18,12 +18,12 @@ class SuppressionComment extends Comment {
     (
       this instanceof CppStyleComment and
       // strip the beginning slashes
-      text = getContents().suffix(2)
+      text = this.getContents().suffix(2)
       or
       this instanceof CStyleComment and
       // strip both the beginning /* and the end */ the comment
       exists(string text0 |
-        text0 = getContents().suffix(2) and
+        text0 = this.getContents().suffix(2) and
         text = text0.prefix(text0.length() - 2)
       ) and
       // The /* */ comment must be a single-line comment

cpp/ql/src/Architecture/General Class-Level Information/index.qhelp

@@ -1,13 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-
-
-<overview>
-<p>about General Class-Level Information</p>
-
-<!--TOC-->
-
-</overview>
-</qhelp>

cpp/ql/src/Architecture/Refactoring Opportunities/ClassesWithManyFields.ql

@@ -153,12 +153,12 @@ class ExtClass extends Class {
   }
 
   predicate hasLocationInfo(string path, int startline, int startcol, int endline, int endcol) {
-    if hasOneVariableGroup()
+    if this.hasOneVariableGroup()
     then
       exists(VariableDeclarationGroup vdg | vdg.getClass() = this |
         vdg.hasLocationInfo(path, startline, startcol, endline, endcol)
       )
-    else getLocation().hasLocationInfo(path, startline, startcol, endline, endcol)
+    else this.getLocation().hasLocationInfo(path, startline, startcol, endline, endcol)
   }
 }
 

cpp/ql/src/Architecture/index.qhelp

@@ -1,13 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-
-
-<overview>
-<p>about Architecture</p>
-
-<!--TOC-->
-
-</overview>
-</qhelp>

cpp/ql/src/Best Practices/Likely Errors/EmptyBlock.ql

@@ -81,9 +81,8 @@ class BlockOrNonChild extends Element {
 predicate emptyBlockContainsNonchild(BlockStmt b) {
   emptyBlock(_, b) and
   exists(BlockOrNonChild c, AffectedFile file |
-    c.(BlockOrNonChild).getStartRankIn(file) = 1 + b.(BlockOrNonChild).getStartRankIn(file) and
-    c.(BlockOrNonChild).getNonContiguousEndRankIn(file) <
-      b.(BlockOrNonChild).getNonContiguousEndRankIn(file)
+    c.getStartRankIn(file) = 1 + b.(BlockOrNonChild).getStartRankIn(file) and
+    c.getNonContiguousEndRankIn(file) < b.(BlockOrNonChild).getNonContiguousEndRankIn(file)
   )
 }
 

cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql

@@ -50,6 +50,16 @@ predicate reachableThing(Thing t) {
   exists(Thing mid | reachableThing(mid) and mid.callsOrAccesses() = t)
 }
 
+pragma[nomagic]
+predicate callsOrAccessesPlus(Thing thing1, FunctionToRemove thing2) {
+  thing1.callsOrAccesses() = thing2
+  or
+  exists(Thing mid |
+    thing1.callsOrAccesses() = mid and
+    callsOrAccessesPlus(mid, thing2)
+  )
+}
+
 class Thing extends Locatable {
   Thing() {
     this instanceof Function or
@@ -62,7 +72,7 @@ class Thing extends Locatable {
   }
 
   Thing callsOrAccesses() {
-    this.(Function).calls(result.(Function))
+    this.(Function).calls(result)
     or
     this.(Function).accesses(result.(Function))
     or
@@ -81,7 +91,7 @@ class FunctionToRemove extends Function {
   }
 
   Thing getOther() {
-    result.callsOrAccesses+() = this and
+    callsOrAccessesPlus(result, this) and
     this != result and
     // We will already be reporting the enclosing function of a
     // local variable, so don't also report the variable

cpp/ql/src/Best Practices/index.qhelp

@@ -1,16 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-
-
-<overview>
-<p>about best practices</p>
-
-<!--TOC-->
-
-
-
-
-</overview>
-</qhelp>

cpp/ql/src/CHANGELOG.md

@@ -0,0 +1,5 @@
+## 0.0.4
+
+### New Queries
+
+* A new query `cpp/non-https-url` has been added for C/C++. The query flags uses of `http` URLs that might be better replaced with `https`.

cpp/ql/src/Critical/OverflowStatic.ql

@@ -103,9 +103,9 @@ class CallWithBufferSize extends FunctionCall {
     // `upperBound(e)` defaults to `exprMaxVal(e)` when `e` isn't analyzable. So to get a meaningful
     // result in this case we pick the minimum value obtainable from dataflow and range analysis.
     result =
-      upperBound(statedSizeExpr())
+      upperBound(this.statedSizeExpr())
           .minimum(min(Expr statedSizeSrc |
-              DataFlow::localExprFlow(statedSizeSrc, statedSizeExpr())
+              DataFlow::localExprFlow(statedSizeSrc, this.statedSizeExpr())
             |
               statedSizeSrc.getValue().toInt()
             ))

cpp/ql/src/JPL_C/LOC-2/Rule 09/Semaphores.qll

@@ -22,7 +22,7 @@ abstract class LockOperation extends FunctionCall {
   ControlFlowNode getAReachedNode() {
     result = this
     or
-    exists(ControlFlowNode mid | mid = getAReachedNode() |
+    exists(ControlFlowNode mid | mid = this.getAReachedNode() |
       not mid != this.getMatchingUnlock() and
       result = mid.getASuccessor()
     )

cpp/ql/src/Likely Bugs/Arithmetic/ComparisonWithCancelingSubExpr.ql

@@ -85,7 +85,8 @@ private predicate cancelingSubExprs(ComparisonOperation cmp, VariableAccess a1,
   exists(Variable v |
     exists(float m | m < 0 and cmpLinearSubVariable(cmp, v, a1, m)) and
     exists(float m | m > 0 and cmpLinearSubVariable(cmp, v, a2, m))
-  )
+  ) and
+  not any(ClassTemplateInstantiation inst).getATemplateArgument() = cmp.getParent*()
 }
 
 from ComparisonOperation cmp, VariableAccess a1, VariableAccess a2

cpp/ql/src/Likely Bugs/Arithmetic/PointlessSelfComparison.qll

@@ -29,7 +29,9 @@ predicate pointlessSelfComparison(ComparisonOperation cmp) {
     not exists(lhs.getQualifier()) and // Avoid structure fields
     not exists(rhs.getQualifier()) and // Avoid structure fields
     not convertedExprMightOverflow(lhs) and
-    not convertedExprMightOverflow(rhs)
+    not convertedExprMightOverflow(rhs) and
+    // Don't warn if the comparison is part of a template argument.
+    not any(ClassTemplateInstantiation inst).getATemplateArgument() = cmp.getParent*()
   )
 }
 

cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql

@@ -63,14 +63,14 @@ predicate cannotContainString(Type t) {
 
 predicate isNonConst(DataFlow::Node node) {
   exists(Expr e | e = node.asExpr() |
-    exists(FunctionCall fc | fc = e.(FunctionCall) |
+    exists(FunctionCall fc | fc = e |
       not (
         whitelistFunction(fc.getTarget(), _) or
         fc.getTarget().hasDefinition()
       )
     )
     or
-    exists(Parameter p | p = e.(VariableAccess).getTarget().(Parameter) |
+    exists(Parameter p | p = e.(VariableAccess).getTarget() |
       p.getFunction().getName() = "main" and p.getType() instanceof PointerType
     )
     or

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

@@ -10,7 +10,7 @@ import semmle.code.cpp.commons.DateTime
  * Get the top-level `BinaryOperation` enclosing the expression e.
  */
 private BinaryOperation getATopLevelBinaryOperationExpression(Expr e) {
-  result = e.getEnclosingElement().(BinaryOperation)
+  result = e.getEnclosingElement()
   or
   result = getATopLevelBinaryOperationExpression(e.getEnclosingElement())
 }
@@ -156,8 +156,8 @@ abstract class LeapYearFieldAccess extends YearFieldAccess {
     //
     // https://aa.usno.navy.mil/faq/docs/calendars.php
     this.isUsedInMod4Operation() and
-    additionalModulusCheckForLeapYear(400) and
-    additionalModulusCheckForLeapYear(100)
+    this.additionalModulusCheckForLeapYear(400) and
+    this.additionalModulusCheckForLeapYear(100)
   }
 }
 
@@ -176,17 +176,17 @@ class StructTmLeapYearFieldAccess extends LeapYearFieldAccess {
 
   override predicate isUsedInCorrectLeapYearCheck() {
     this.isUsedInMod4Operation() and
-    additionalModulusCheckForLeapYear(400) and
-    additionalModulusCheckForLeapYear(100) and
+    this.additionalModulusCheckForLeapYear(400) and
+    this.additionalModulusCheckForLeapYear(100) and
     // tm_year represents years since 1900
     (
-      additionalAdditionOrSubstractionCheckForLeapYear(1900)
+      this.additionalAdditionOrSubstractionCheckForLeapYear(1900)
       or
       // some systems may use 2000 for 2-digit year conversions
-      additionalAdditionOrSubstractionCheckForLeapYear(2000)
+      this.additionalAdditionOrSubstractionCheckForLeapYear(2000)
       or
       // converting from/to Unix epoch
-      additionalAdditionOrSubstractionCheckForLeapYear(1970)
+      this.additionalAdditionOrSubstractionCheckForLeapYear(1970)
     )
   }
 }

cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql

@@ -66,7 +66,7 @@ predicate functionDefinedInIfDefRecursive(Function f) {
  */
 predicate baseCall(FunctionCall call) {
   call.getNameQualifier().getQualifyingElement() =
-    call.getEnclosingFunction().getDeclaringType().(Class).getABaseClass+()
+    call.getEnclosingFunction().getDeclaringType().getABaseClass+()
 }
 
 from PureExprInVoidContext peivc, Locatable parent, Locatable info, string info_text, string tail

cpp/ql/src/Likely Bugs/Memory Management/AllocaInLoop.ql

@@ -57,7 +57,7 @@ class LoopWithAlloca extends Stmt {
     or
     // `e == 0`
     exists(EQExpr eq |
-      conditionRequires(eq, truth.booleanNot()) and
+      this.conditionRequires(eq, truth.booleanNot()) and
       eq.getAnOperand().getValue().toInt() = 0 and
       e = eq.getAnOperand() and
       not exists(e.getValue())
@@ -65,7 +65,7 @@ class LoopWithAlloca extends Stmt {
     or
     // `e != 0`
     exists(NEExpr eq |
-      conditionRequires(eq, truth) and
+      this.conditionRequires(eq, truth) and
       eq.getAnOperand().getValue().toInt() = 0 and
       e = eq.getAnOperand() and
       not exists(e.getValue())
@@ -73,7 +73,7 @@ class LoopWithAlloca extends Stmt {
     or
     // `(bool)e == true`
     exists(EQExpr eq |
-      conditionRequires(eq, truth) and
+      this.conditionRequires(eq, truth) and
       eq.getAnOperand().getValue().toInt() = 1 and
       e = eq.getAnOperand() and
       e.getUnspecifiedType() instanceof BoolType and
@@ -82,7 +82,7 @@ class LoopWithAlloca extends Stmt {
     or
     // `(bool)e != true`
     exists(NEExpr eq |
-      conditionRequires(eq, truth.booleanNot()) and
+      this.conditionRequires(eq, truth.booleanNot()) and
       eq.getAnOperand().getValue().toInt() = 1 and
       e = eq.getAnOperand() and
       e.getUnspecifiedType() instanceof BoolType and
@@ -90,7 +90,7 @@ class LoopWithAlloca extends Stmt {
     )
     or
     exists(NotExpr notExpr |
-      conditionRequires(notExpr, truth.booleanNot()) and
+      this.conditionRequires(notExpr, truth.booleanNot()) and
       e = notExpr.getOperand()
     )
     or
@@ -98,7 +98,7 @@ class LoopWithAlloca extends Stmt {
     // requires both of its operand to be true as well.
     exists(LogicalAndExpr andExpr |
       truth = true and
-      conditionRequires(andExpr, truth) and
+      this.conditionRequires(andExpr, truth) and
       e = andExpr.getAnOperand()
     )
     or
@@ -106,7 +106,7 @@ class LoopWithAlloca extends Stmt {
     // it requires both of its operand to be false as well.
     exists(LogicalOrExpr orExpr |
       truth = false and
-      conditionRequires(orExpr, truth) and
+      this.conditionRequires(orExpr, truth) and
       e = orExpr.getAnOperand()
     )
   }
@@ -141,9 +141,9 @@ class LoopWithAlloca extends Stmt {
    * `conditionRequiresInequality`.
    */
   private Variable getAControllingVariable() {
-    conditionRequires(result.getAnAccess(), _)
+    this.conditionRequires(result.getAnAccess(), _)
     or
-    conditionRequiresInequality(result.getAnAccess(), _, _)
+    this.conditionRequiresInequality(result.getAnAccess(), _, _)
   }
 
   /**

cpp/ql/src/Likely Bugs/Memory Management/NtohlArrayNoBound.qll

@@ -61,72 +61,72 @@ class PointerArithmeticAccess extends BufferAccess, Expr {
  * A pair of buffer accesses through a call to memcpy.
  */
 class MemCpy extends BufferAccess, FunctionCall {
-  MemCpy() { getTarget().hasName("memcpy") }
+  MemCpy() { this.getTarget().hasName("memcpy") }
 
   override Expr getPointer() {
-    result = getArgument(0) or
-    result = getArgument(1)
+    result = this.getArgument(0) or
+    result = this.getArgument(1)
   }
 
-  override Expr getAccessedLength() { result = getArgument(2) }
+  override Expr getAccessedLength() { result = this.getArgument(2) }
 }
 
 class StrncpySizeExpr extends BufferAccess, FunctionCall {
-  StrncpySizeExpr() { getTarget().hasName("strncpy") }
+  StrncpySizeExpr() { this.getTarget().hasName("strncpy") }
 
   override Expr getPointer() {
-    result = getArgument(0) or
-    result = getArgument(1)
+    result = this.getArgument(0) or
+    result = this.getArgument(1)
   }
 
-  override Expr getAccessedLength() { result = getArgument(2) }
+  override Expr getAccessedLength() { result = this.getArgument(2) }
 }
 
 class RecvSizeExpr extends BufferAccess, FunctionCall {
-  RecvSizeExpr() { getTarget().hasName("recv") }
+  RecvSizeExpr() { this.getTarget().hasName("recv") }
 
-  override Expr getPointer() { result = getArgument(1) }
+  override Expr getPointer() { result = this.getArgument(1) }
 
-  override Expr getAccessedLength() { result = getArgument(2) }
+  override Expr getAccessedLength() { result = this.getArgument(2) }
 }
 
 class SendSizeExpr extends BufferAccess, FunctionCall {
-  SendSizeExpr() { getTarget().hasName("send") }
+  SendSizeExpr() { this.getTarget().hasName("send") }
 
-  override Expr getPointer() { result = getArgument(1) }
+  override Expr getPointer() { result = this.getArgument(1) }
 
-  override Expr getAccessedLength() { result = getArgument(2) }
+  override Expr getAccessedLength() { result = this.getArgument(2) }
 }
 
 class SnprintfSizeExpr extends BufferAccess, FunctionCall {
-  SnprintfSizeExpr() { getTarget().hasName("snprintf") }
+  SnprintfSizeExpr() { this.getTarget().hasName("snprintf") }
 
-  override Expr getPointer() { result = getArgument(0) }
+  override Expr getPointer() { result = this.getArgument(0) }
 
-  override Expr getAccessedLength() { result = getArgument(1) }
+  override Expr getAccessedLength() { result = this.getArgument(1) }
 }
 
 class MemcmpSizeExpr extends BufferAccess, FunctionCall {
-  MemcmpSizeExpr() { getTarget().hasName("Memcmp") }
+  MemcmpSizeExpr() { this.getTarget().hasName("Memcmp") }
 
   override Expr getPointer() {
-    result = getArgument(0) or
-    result = getArgument(1)
+    result = this.getArgument(0) or
+    result = this.getArgument(1)
   }
 
-  override Expr getAccessedLength() { result = getArgument(2) }
+  override Expr getAccessedLength() { result = this.getArgument(2) }
 }
 
 class MallocSizeExpr extends BufferAccess, FunctionCall {
-  MallocSizeExpr() { getTarget().hasName("malloc") }
+  MallocSizeExpr() { this.getTarget().hasName("malloc") }
 
   override Expr getPointer() { none() }
 
-  override Expr getAccessedLength() { result = getArgument(0) }
+  override Expr getAccessedLength() { result = this.getArgument(0) }
 }
 
 class NetworkFunctionCall extends FunctionCall {
-  NetworkFunctionCall() { getTarget().hasName(["ntohd", "ntohf", "ntohl", "ntohll", "ntohs"]) }
+  NetworkFunctionCall() { this.getTarget().hasName(["ntohd", "ntohf", "ntohl", "ntohll", "ntohs"]) }
 }
 
 class NetworkToBufferSizeConfiguration extends DataFlow::Configuration {

cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql

@@ -3,8 +3,10 @@
  * @description Using the TLS or SSLv23 protocol from the boost::asio library, but not disabling deprecated protocols, or disabling minimum-recommended protocols.
  * @kind problem
  * @problem.severity error
+ * @security-severity 7.5
  * @id cpp/boost/tls-settings-misconfiguration
  * @tags security
+ *       external/cwe/cwe-326
  */
 
 import cpp

cpp/ql/src/Likely Bugs/Protocols/UseOfDeprecatedHardcodedProtocol.ql

@@ -3,8 +3,10 @@
  * @description Using a deprecated hard-coded protocol using the boost::asio library.
  * @kind problem
  * @problem.severity error
+ * @security-severity 7.5
  * @id cpp/boost/use-of-deprecated-hardcoded-security-protocol
  * @tags security
+ *       external/cwe/cwe-327
  */
 
 import cpp

cpp/ql/src/Metrics/Files/FCyclomaticComplexity.ql

@@ -15,7 +15,7 @@ import cpp
 from File f, float complexity, float loc
 where
   f.fromSource() and
-  loc = sum(FunctionDeclarationEntry fde | fde.getFile() = f | fde.getNumberOfLines()).(float) and
+  loc = sum(FunctionDeclarationEntry fde | fde.getFile() = f | fde.getNumberOfLines()) and
   if loc > 0
   then
     // Weighted average of complexity by function length

cpp/ql/src/Metrics/History/HChurn.qhelp

@@ -1,42 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>
-This metric measures the number of lines of text that have been added, deleted
-or modified in files below this location in the tree.
-</p>
-
-<p>
-Code churn is known to be a good (if not the best) predictor of defects in a
-code component (see e.g. [Nagappan] or [Khoshgoftaar]). The intuition is that
-files, packages or projects that have experienced a disproportionately high
-amount of churn for the amount of code involved may have been harder to write,
-and are thus likely to contain more bugs.
-</p>
-
-</overview>
-<recommendation>
-
-<p>
-It is a fact of life that some code is going to be changed more than the rest,
-and little can be done to change this. However, bearing in mind code churn's
-effectiveness as a defect predictor, code that has been repeatedly changed
-should be subjected to vigorous testing and code review.
-</p>
-
-</recommendation>
-<references>
-
-
-<li>
-N. Nagappan et al. <em>Change Bursts as Defect Predictors</em>. In Proceedings of the 21st IEEE International Symposium on Software Reliability Engineering, 2010.
-</li>
-<li>
-T. M. Khoshgoftaar and R. M. Szabo. <em>Improving code churn predictions during the system test and maintenance phases</em>. In ICSM '94, 1994, pp. 58-67.
-</li>
-
-
-</references>
-</qhelp>

cpp/ql/src/Metrics/History/HLinesAdded.qhelp

@@ -1,6 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<include src="HChurn.qhelp" />
-</qhelp>

cpp/ql/src/Metrics/History/HLinesDeleted.qhelp

@@ -1,6 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<include src="HChurn.qhelp" />
-</qhelp>

cpp/ql/src/Metrics/History/HNumberOfAuthors.qhelp

@@ -1,48 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>
-This metric measures the number of different authors (by examining the
-version control history)
-for files below this location in the tree. (This is a better version
-of the metric that counts the number of different authors using Javadoc
-tags.)
-</p>
-
-<p>
-Files that have been changed by a large number of different authors are
-by definition the product of many minds. New authors working on a file
-may be less familiar with the design and implementation of the code than
-the original authors, which can be a potential source of bugs. Furthermore,
-code that has been worked on by many people, if not carefully maintained,
-often ends up lacking conceptual integrity. For both of these reasons, any
-code that has been worked on by an unusually high number of different people
-merits careful inspection in code reviews.
-</p>
-
-</overview>
-<recommendation>
-
-<p>
-There is clearly no way to reduce the number of authors that have worked
-on a file - it is impossible to rewrite history. However, files highlighted
-by this metric should be given special attention in a code review, and may
-ultimately be good candidates for refactoring/rewriting by an individual,
-experienced developer.
-</p>
-
-
-
-</recommendation>
-<references>
-
-
-<li>
-F. P. Brooks Jr. <em>The Mythical Man-Month</em>, Chapter 4. Addison-Wesley, 1974.
-</li>
-
-
-</references>
-</qhelp>

cpp/ql/src/Metrics/History/HNumberOfChanges.qhelp

@@ -1,30 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>
-This metric measures the total number of file-level changes made to files
-below this location in the tree. For an individual file, it measures the
-number of commits that have affected that file. For a directory of files, it
-measures the sum of the file-level changes for each of the files in the
-directory.
-</p>
-
-<p>
-For example, suppose we have a directory containing two files, A and B. If the
-number of file-level changes to A is <code>100</code>, and the number of
-file-level changes to B is <code>80</code>, then the total number of
-file-level changes to the directory is <code>180</code>. Note that this is
-likely to be different (in some cases very different) from the number of
-commits that affected any file in the directory, since more than one file can
-be changed by a single commit. (Note what would happen if we performed
-<code>80</code> commits on A and B, followed by another <code>20</code>
-commits on A alone - the total number of file-level changes would be
-<code>180</code>, but the number of commits involved would be
-<code>100</code>.)
-</p>
-
-
-</overview>
-</qhelp>

cpp/ql/src/Metrics/History/HNumberOfCoCommits.qhelp

@@ -1,51 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>
-This metric measures the average number of co-committed files for the files
-below this location in the tree.
-</p>
-
-<p>
-A co-committed file is one that is committed at the same time as a given file.
-For instance, if you commit files A, B and C together, then B and C would be
-the co-committed files of A for that commit. The value of the metric for an
-individual file is the average number of such co-committed files over all
-commits. The value of the metric for a directory is the aggregation of these
-averages - for instance, if we are using <code>max</code> as our aggregation
-function, the value would be the maximum of the average number of co-commits
-over all files in the directory.
-</p>
-
-<p>
-An unusually high value for this metric may indicate that the file in question
-is too tightly-coupled to other files, and it is difficult to change it in
-isolation. Alternatively, it may just be an indication that you commit lots of
-unrelated changes at the same time.
-</p>
-
-</overview>
-<recommendation>
-
-<p>
-Examine the file in question to see what the problem is.
-</p>
-
-<ul>
-<li>
-If the file is too tightly coupled, it will have high values for its afferent
-and/or efferent coupling metrics, and you should apply the advice given there.
-</li>
-
-<li>
-If the file is not tightly coupled, but you find that you are committing lots
-of unrelated changes at the same time, then you may want to revisit your commit
-practices.
-</li>
-</ul>
-
-
-</recommendation>
-</qhelp>

cpp/ql/src/Metrics/History/HNumberOfReCommits.qhelp

@@ -1,53 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>
-This metric measures the number of file re-commits that have occurred below
-this location in the tree. A re-commit is taken to mean a commit to a file
-that was touched less than five days ago.
-</p>
-
-<p>
-In a system that is being developed using a controlled change process (where
-changes are not committed until they are in some sense 'complete'), re-commits
-can be (but are not always) an indication that an initial change was not
-successful and had to be revisited within a short time period. The intuition
-is that the original change may have been difficult to get right, and hence
-the code in the file may be more than usually defect-prone. The concept is
-somewhat similar to that of 'change bursts', as described in [Nagappan]. 
-</p>
-
-</overview>
-<recommendation>
-
-<p>
-High numbers of re-commits can be addressed on two levels: preventative and
-corrective.
-</p>
-
-<ul>
-<li>
-On the preventative side, a high number of re-commits may be an indication
-that your code review process needs an overhaul.
-</li>
-
-<li>
-On the corrective side, code that has experienced a high number of re-commits
-should be vigorously code reviewed and tested.
-</li>
-</ul>
-
-
-</recommendation>
-<references>
-
-
-<li>
-N. Nagappan et al. <em>Change Bursts as Defect Predictors</em>. In Proceedings of the 21st IEEE International Symposium on Software Reliability Engineering, 2010.
-</li>
-
-
-</references>
-</qhelp>