Merge branch 'main' into use-shared-ssa-in-ir-dataflow

.codeqlmanifest.json

@@ -1,5 +1,4 @@
-{ "provide": [ "ruby/.codeqlmanifest.json",
-                "*/ql/src/qlpack.yml",
+{ "provide": [ "*/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",

.devcontainer/devcontainer.json

@@ -1,14 +1,9 @@
 {
   "extensions": [
-    "rust-lang.rust",
-    "bungcip.better-toml",
     "github.vscode-codeql",
     "slevesque.vscode-zipexplorer"
   ],
   "settings": {
-    "files.watcherExclude": {
-      "**/target/**": true
-    },
     "codeQL.runningQueries.memory": 2048
   }
 }

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

@@ -1,14 +0,0 @@
-name: Fetch CodeQL
-description: Fetches the latest version of CodeQL
-runs:
-  using: composite
-  steps:
-    - name: Fetch CodeQL
-      shell: bash
-      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 -d "${RUNNER_TEMP}" codeql-linux64.zip
-        echo "${RUNNER_TEMP}/codeql" >> "${GITHUB_PATH}"
-      env:
-        GITHUB_TOKEN: ${{ github.token }}

.github/dependabot.yml

@@ -1,18 +0,0 @@
-version: 2
-updates:
-  - package-ecosystem: "cargo"
-    directory: "ruby/node-types"
-    schedule:
-      interval: "daily"
-  - package-ecosystem: "cargo"
-    directory: "ruby/generator"
-    schedule:
-      interval: "daily"
-  - package-ecosystem: "cargo"
-    directory: "ruby/extractor"
-    schedule:
-      interval: "daily"
-  - package-ecosystem: "cargo"
-    directory: "ruby/autobuilder"
-    schedule:
-      interval: "daily"

.github/labeler.yml

@@ -18,10 +18,6 @@ Python:
   - python/**/*
   - change-notes/**/*python*
 
-Ruby:
-  - ruby/**/*
-  - change-notes/**/*ruby*
-
 documentation:
   - "**/*.qhelp"
   - "**/*.md"

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

@@ -1,31 +0,0 @@
-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,63 +0,0 @@
-name: Query help preview
-
-permissions:
-  contents: read
-
-on:
-  pull_request:
-    branches:
-      - main
-      - "rc/*"
-    paths:
-      - "ruby/**/*.qhelp"
-
-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: |
-          (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
-        run: |
-          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}" 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 < "${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/ruby-build.yml

@@ -1,224 +0,0 @@
-name: "Ruby: Build"
-
-on:
-  push:
-    paths:
-      - "ruby/**"
-      - .github/workflows/ruby-build.yml
-    branches:
-      - main
-      - "rc/*"
-  pull_request:
-    paths:
-      - "ruby/**"
-      - .github/workflows/ruby-build.yml
-    branches:
-      - main
-      - "rc/*"
-  workflow_dispatch:
-    inputs:
-      tag:
-        description: "Version tag to create"
-        required: false
-
-env:
-  CARGO_TERM_COLOR: always
-
-defaults:
-  run:
-    working-directory: ruby
-
-jobs:
-  build:
-    strategy:
-      fail-fast: false
-      matrix:
-        os: [ubuntu-latest, macos-latest, windows-latest]
-
-    runs-on: ${{ matrix.os }}
-
-    steps:
-      - uses: actions/checkout@v2
-      - name: Install GNU tar
-        if: runner.os == 'macOS'
-        run: |
-          brew install gnu-tar
-          echo "/usr/local/opt/gnu-tar/libexec/gnubin" >> $GITHUB_PATH
-      - uses: actions/cache@v2
-        with:
-          path: |
-            ~/.cargo/registry
-            ~/.cargo/git
-            ruby/target
-          key: ${{ runner.os }}-rust-cargo-${{ hashFiles('**/Cargo.lock') }}
-      - name: Check formatting
-        run: cargo fmt --all -- --check
-      - name: Build
-        run: cargo build --verbose
-      - name: Run tests
-        run: cargo test --verbose
-      - name: Release build
-        run: cargo build --release
-      - name: Generate dbscheme
-        if: ${{ matrix.os == 'ubuntu-latest' }}
-        run: target/release/ruby-generator --dbscheme ql/lib/ruby.dbscheme --library ql/lib/codeql/ruby/ast/internal/TreeSitter.qll
-      - uses: actions/upload-artifact@v2
-        if: ${{ matrix.os == 'ubuntu-latest' }}
-        with:
-          name: ruby.dbscheme
-          path: ruby/ql/lib/ruby.dbscheme
-      - uses: actions/upload-artifact@v2
-        if: ${{ matrix.os == 'ubuntu-latest' }}
-        with:
-          name: TreeSitter.qll
-          path: ruby/ql/lib/codeql/ruby/ast/internal/TreeSitter.qll
-      - uses: actions/upload-artifact@v2
-        with:
-          name: extractor-${{ matrix.os }}
-          path: |
-            ruby/target/release/ruby-autobuilder
-            ruby/target/release/ruby-autobuilder.exe
-            ruby/target/release/ruby-extractor
-            ruby/target/release/ruby-extractor.exe
-          retention-days: 1
-  compile-queries:
-    runs-on: ubuntu-latest
-    env:
-      CODEQL_THREADS: 4 # TODO: remove this once it's set by the CLI
-    steps:
-      - uses: actions/checkout@v2
-      - name: Fetch CodeQL
-        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
-        env:
-          GITHUB_TOKEN: ${{ github.token }}
-      - name: Build Query Pack
-        run: |
-          codeql/codeql pack create ql/lib --output target/packs
-          codeql/codeql pack install ql/src
-          codeql/codeql pack create ql/src --output target/packs
-          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}/{}" \;)
-      - uses: actions/upload-artifact@v2
-        with:
-          name: codeql-ruby-queries
-          path: |
-            ruby/target/packs/*
-          retention-days: 1
-
-  package:
-    runs-on: ubuntu-latest
-    needs: [build, compile-queries]
-    steps:
-      - uses: actions/checkout@v2
-      - uses: actions/download-artifact@v2
-        with:
-          name: ruby.dbscheme
-          path: ruby/ruby
-      - uses: actions/download-artifact@v2
-        with:
-          name: extractor-ubuntu-latest
-          path: ruby/linux64
-      - uses: actions/download-artifact@v2
-        with:
-          name: extractor-windows-latest
-          path: ruby/win64
-      - uses: actions/download-artifact@v2
-        with:
-          name: extractor-macos-latest
-          path: ruby/osx64
-      - run: |
-          mkdir -p ruby
-          cp -r codeql-extractor.yml tools ql/lib/ruby.dbscheme.stats ruby/
-          mkdir -p ruby/tools/{linux64,osx64,win64}
-          cp linux64/ruby-autobuilder ruby/tools/linux64/autobuilder
-          cp osx64/ruby-autobuilder ruby/tools/osx64/autobuilder
-          cp win64/ruby-autobuilder.exe ruby/tools/win64/autobuilder.exe
-          cp linux64/ruby-extractor ruby/tools/linux64/extractor
-          cp osx64/ruby-extractor ruby/tools/osx64/extractor
-          cp win64/ruby-extractor.exe ruby/tools/win64/extractor.exe
-          chmod +x ruby/tools/{linux64,osx64}/{autobuilder,extractor}
-          zip -rq codeql-ruby.zip ruby
-      - uses: actions/upload-artifact@v2
-        with:
-          name: codeql-ruby-pack
-          path: ruby/codeql-ruby.zip
-          retention-days: 1
-      - uses: actions/download-artifact@v2
-        with:
-          name: codeql-ruby-queries
-          path: ruby/qlpacks
-      - run: |
-          echo '{
-            "provide": [
-            "ruby/codeql-extractor.yml",
-            "qlpacks/*/*/*/qlpack.yml"
-            ]
-          }' > .codeqlmanifest.json
-          zip -rq codeql-ruby-bundle.zip .codeqlmanifest.json ruby qlpacks
-      - uses: actions/upload-artifact@v2
-        with:
-          name: codeql-ruby-bundle
-          path: ruby/codeql-ruby-bundle.zip
-          retention-days: 1
-
-  test:
-    defaults:
-      run:
-        working-directory: ${{ github.workspace }}
-    strategy:
-      fail-fast: false
-      matrix:
-        os: [ubuntu-latest, macos-latest, windows-latest]
-
-    runs-on: ${{ matrix.os }}
-    needs: [package]
-    steps:
-      - uses: actions/checkout@v2
-        with:
-          repository: Shopify/example-ruby-app
-          ref: 67a0decc5eb550f3a9228eda53925c3afd40dfe9
-      - name: Fetch CodeQL
-        shell: bash
-        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.zip "$LATEST"
-          unzip -q codeql.zip
-        env:
-          GITHUB_TOKEN: ${{ github.token }}
-        working-directory: ${{ runner.temp }}
-      - name: Download Ruby bundle
-        uses: actions/download-artifact@v2
-        with:
-          name: codeql-ruby-bundle
-          path: ${{ runner.temp }}
-      - name: Unzip Ruby bundle
-        shell: bash
-        run: unzip -q -d "${{ runner.temp }}/ruby-bundle" "${{ runner.temp }}/codeql-ruby-bundle.zip"
-      - name: Prepare test files
-        shell: bash
-        run: |
-          echo "import ruby select count(File f)" > "test.ql"
-          echo "| 4 |" > "test.expected"
-          echo 'name: sample-tests
-          version: 0.0.0
-          dependencies:
-            codeql/ruby-all: 0.0.1
-          extractor: ruby
-          tests: .
-          ' > qlpack.yml
-      - name: Run QL test
-        shell: bash
-        run: |
-          "${{ runner.temp }}/codeql/codeql" test run --search-path "${{ runner.temp }}/ruby-bundle" --additional-packs "${{ runner.temp }}/ruby-bundle" .
-      - name: Create database
-        shell: bash
-        run: |
-          "${{ runner.temp }}/codeql/codeql" database create --search-path "${{ runner.temp }}/ruby-bundle" --language ruby --source-root . ../database
-      - name: Analyze database
-        shell: bash
-        run: |
-          "${{ runner.temp }}/codeql/codeql" database analyze --search-path "${{ runner.temp }}/ruby-bundle" --format=sarifv2.1.0 --output=out.sarif ../database ruby-code-scanning.qls

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

@@ -1,73 +0,0 @@
-name: "Ruby: Collect database stats"
-
-on:
-  push:
-    branches:
-      - main
-      - "rc/*"
-    paths:
-      - ruby/ql/lib/ruby.dbscheme
-      - .github/workflows/ruby-dataset-measure.yml
-  pull_request:
-    branches:
-      - main
-      - "rc/*"
-    paths:
-      - ruby/ql/lib/ruby.dbscheme
-      - .github/workflows/ruby-dataset-measure.yml
-  workflow_dispatch:
-
-jobs:
-  measure:
-    env:
-      CODEQL_THREADS: 4 # TODO: remove this once it's set by the CLI
-    strategy:
-      fail-fast: false
-      matrix:
-        repo: [rails/rails, discourse/discourse, spree/spree]
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v2
-
-      - uses: ./.github/actions/fetch-codeql
-
-      - uses: ./ruby/actions/create-extractor-pack
-
-      - 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 "${{ github.workspace }}/ruby" \
-            --threads 4 \
-            --language ruby --source-root "${{ github.workspace }}/repo" \
-            "${{ runner.temp }}/database"
-      - name: Measure database
-        run: |
-          mkdir -p "stats/${{ matrix.repo }}"
-          codeql dataset measure --threads 4 --output "stats/${{ matrix.repo }}/stats.xml" "${{ runner.temp }}/database/db-ruby"
-      - 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' | sort | xargs python ruby/scripts/merge_stats.py --output ruby/ql/lib/ruby.dbscheme.stats --normalise ruby_tokeninfo
-      - uses: actions/upload-artifact@v2
-        with:
-          name: ruby.dbscheme.stats
-          path: ruby/ql/lib/ruby.dbscheme.stats

.github/workflows/ruby-qltest.yml

@@ -1,50 +0,0 @@
-name: "Ruby: Run QL Tests"
-
-on:
-  push:
-    paths:
-      - "ruby/**"
-      - .github/workflows/ruby-qltest.yml
-    branches:
-      - main
-      - "rc/*"
-  pull_request:
-    paths:
-      - "ruby/**"
-      - .github/workflows/ruby-qltest.yml
-    branches:
-      - main
-      - "rc/*"
-
-env:
-  CARGO_TERM_COLOR: always
-
-defaults:
-  run:
-    working-directory: ruby
-
-jobs:
-  qltest:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v2
-      - uses: ./.github/actions/fetch-codeql
-      - 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
-        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"
-        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
-          diff -q testdb/ruby.dbscheme ql/lib/ruby.dbscheme

.github/workflows/sync-files.yml

@@ -1,20 +0,0 @@
-name: Check synchronized files
-
-on:
-  push:
-    branches:
-      - main
-      - 'rc/*'
-  pull_request:
-    branches:
-      - main
-      - 'rc/*'
-
-jobs:
-  sync:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v2
-      - name: Check synchronized files
-        run: python config/sync-files.py
-

.gitignore

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

CODEOWNERS

@@ -3,7 +3,6 @@
 /java/ @github/codeql-java
 /javascript/ @github/codeql-javascript
 /python/ @github/codeql-python
-/ruby/ @github/codeql-ruby
 
 # Make @xcorail (GitHub Security Lab) a code owner for experimental queries so he gets pinged when we promote a query out of experimental
 /cpp/**/experimental/**/* @github/codeql-c-analysis @xcorail
@@ -11,7 +10,6 @@
 /java/**/experimental/**/* @github/codeql-java @xcorail
 /javascript/**/experimental/**/* @github/codeql-javascript @xcorail
 /python/**/experimental/**/* @github/codeql-python @xcorail
-/ruby/**/experimental/**/* @github/codeql-ruby @xcorail
 
 # Notify members of codeql-go about PRs to the shared data-flow library files
 /java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll @github/codeql-java @github/codeql-go
@@ -24,4 +22,4 @@
 /docs/codeql-cli/ @github/codeql-cli-reviewers
 /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
+/docs/query-*-style-guide.md @github/codeql-analysis-reviewers

CONTRIBUTING.md

@@ -11,14 +11,13 @@ If you have an idea for a query that you would like to share with other CodeQL u
 
 1. **Directory structure**
 
-    There are six language-specific query directories in this repository:
+    There are five 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

@@ -24,17 +24,14 @@
     "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl.qll",
     "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl2.qll",
     "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl3.qll",
-    "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll",
-    "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl.qll",
-    "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl2.qll"
+    "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll"
   ],
   "DataFlow Java/C++/C#/Python Common": [
     "java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll",
     "cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll",
     "cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll",
     "csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll",
-    "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplCommon.qll",
-    "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplCommon.qll"
+    "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplCommon.qll"
   ],
   "TaintTracking::Configuration Java/C++/C#/Python": [
     "cpp/ql/lib/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll",
@@ -52,21 +49,18 @@
     "python/ql/lib/semmle/python/dataflow/new/internal/tainttracking1/TaintTrackingImpl.qll",
     "python/ql/lib/semmle/python/dataflow/new/internal/tainttracking2/TaintTrackingImpl.qll",
     "python/ql/lib/semmle/python/dataflow/new/internal/tainttracking3/TaintTrackingImpl.qll",
-    "python/ql/lib/semmle/python/dataflow/new/internal/tainttracking4/TaintTrackingImpl.qll",
-    "ruby/ql/lib/codeql/ruby/dataflow/internal/tainttracking1/TaintTrackingImpl.qll"
+    "python/ql/lib/semmle/python/dataflow/new/internal/tainttracking4/TaintTrackingImpl.qll"
   ],
   "DataFlow Java/C++/C#/Python Consistency checks": [
     "java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll",
     "cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll",
     "cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll",
     "csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplConsistency.qll",
-    "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll",
-    "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplConsistency.qll"
+    "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll"
   ],
   "DataFlow Java/C# Flow Summaries": [
     "java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll",
-    "csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll",
-    "ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll"
+    "csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll"
   ],
   "SsaReadPosition Java/C#": [
     "java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/SsaReadPositionCommon.qll",
@@ -375,8 +369,7 @@
     "cpp/ql/test/TestUtilities/InlineExpectationsTest.qll",
     "csharp/ql/test/TestUtilities/InlineExpectationsTest.qll",
     "java/ql/test/TestUtilities/InlineExpectationsTest.qll",
-    "python/ql/test/TestUtilities/InlineExpectationsTest.qll",
-    "ruby/ql/test/TestUtilities/InlineExpectationsTest.qll"
+    "python/ql/test/TestUtilities/InlineExpectationsTest.qll"
   ],
   "C++ ExternalAPIs": [
     "cpp/ql/src/Security/CWE/CWE-020/ExternalAPIs.qll",
@@ -448,9 +441,7 @@
     "csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImplCommon.qll",
     "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",
-    "cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll"
+    "csharp/ql/lib/semmle/code/cil/internal/SsaImplCommon.qll"
   ],
   "CryptoAlgorithms Python/JS": [
     "javascript/ql/lib/semmle/javascript/security/CryptoAlgorithms.qll",
@@ -470,28 +461,13 @@
   ],
   "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/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",
-    "ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImplShared.qll"
-  ],
-  "TypeTracker": [
-    "python/ql/lib/semmle/python/dataflow/new/internal/TypeTracker.qll",
-    "ruby/ql/lib/codeql/ruby/typetracking/TypeTracker.qll"
+    "python/ql/lib/semmle/python/security/performance/SuperlinearBackTracking.qll"
   ],
   "CodeQL Tutorial": [
     "cpp/ql/lib/tutorial.qll",
     "csharp/ql/lib/tutorial.qll",
     "java/ql/lib/tutorial.qll",
     "javascript/ql/lib/tutorial.qll",
-    "python/ql/lib/tutorial.qll",
-    "ruby/ql/lib/tutorial.qll"
+    "python/ql/lib/tutorial.qll"
   ]
-}
+}

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

@@ -1,4 +0,0 @@
-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

@@ -1,2 +0,0 @@
-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/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()))
+          fieldInBase.accessInDirectDerived(cd.getASpecifier().(AccessSpecifier)))
     )
   }
 
@@ -261,7 +261,8 @@ class Class extends UserType {
    * includes the case of `base` = `this`.
    */
   AccessSpecifier accessOfBaseMember(Declaration member) {
-    result = this.accessOfBaseMember(member.getDeclaringType(), member.getASpecifier())
+    result =
+      this.accessOfBaseMember(member.getDeclaringType(), member.getASpecifier().(AccessSpecifier))
   }
 
   /**
@@ -318,7 +319,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)
+      this.cannotAccessCopyConstructorOnAny(t.(Class))
     )
     or
     // - T has direct or virtual base class that cannot be copied (has deleted,
@@ -391,7 +392,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)
+      this.cannotAccessCopyAssignmentOperatorOnAny(t.(Class))
     )
     or
     exists(Class c | c = this.getADirectOrVirtualBase() |

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

@@ -490,7 +490,8 @@ class AccessHolder extends Declaration, TAccessHolder {
    */
   pragma[inline]
   predicate canAccessMember(Declaration member, Class derived) {
-    this.couldAccessMember(member.getDeclaringType(), member.getASpecifier(), derived)
+    this.couldAccessMember(member.getDeclaringType(), member.getASpecifier().(AccessSpecifier),
+      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) and
+    getDeclarationEntries(mid, dest.(DeclarationEntry)) 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) and
+    getDeclarationEntries(mid, dest.(DeclarationEntry)) and
     // must be definition
-    dest.isDefinition()
+    dest.(DeclarationEntry).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) and
+    getDeclarationEntries(mid, dest.(DeclarationEntry)) 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 invocation `mi`, as
- * opposed to being passed in as an argument.
+ * 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
@@ -93,8 +93,8 @@ predicate functionContainsPreprocCode(Function f) {
  *   M(y + 1);
  * ```
  */
-private predicate isFromMacroInvocation(Element e, MacroInvocation mi) {
-  exists(Location eLocation, Location miLocation |
+predicate isFromMacroDefinition(Element e) {
+  exists(MacroInvocation mi, Location eLocation, Location miLocation |
     mi.getAnExpandedElement() = e and
     eLocation = e.getLocation() and
     miLocation = mi.getLocation() and
@@ -109,36 +109,3 @@ private predicate isFromMacroInvocation(Element e, MacroInvocation mi) {
     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,33 +3,17 @@ private import semmle.code.cpp.models.interfaces.ArrayFunction
 private import semmle.code.cpp.models.implementations.Strcat
 import semmle.code.cpp.dataflow.DataFlow
 
-/**
- * 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
-    )
+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` may add a null terminator to the string
- * accessed by `va`.
+ * Holds if the expression `e` may add a null terminator to the string in
+ * variable `v`.
  */
 predicate mayAddNullTerminator(Expr e, VariableAccess va) {
   // Assignment: dereferencing or array access
@@ -46,10 +30,14 @@ predicate mayAddNullTerminator(Expr e, VariableAccess va) {
   )
   or
   // Assignment to another stack variable
-  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)
+  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
   )
   or
   // Assignment to non-stack variable
@@ -131,9 +119,14 @@ 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 |
-        mayAddNullTerminator(pragma[only_bind_into](e), p.getAnAccess()) and
-        controlFlowNodeSuccessorTransitive(e, use)
+      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
       )
     )
   )

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

@@ -6,8 +6,6 @@ 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
 
 class PrintfFormatAttribute extends FormatAttribute {
   PrintfFormatAttribute() { this.getArchetype() = ["printf", "__printf__"] }
@@ -177,7 +175,9 @@ class FormattingFunctionCall extends Expr {
   /**
    * Gets the index at which the format string occurs in the argument list.
    */
-  int getFormatParameterIndex() { result = this.getTarget().getFormatParameterIndex() }
+  int getFormatParameterIndex() {
+    result = this.getTarget().(FormattingFunction).getFormatParameterIndex()
+  }
 
   /**
    * Gets the format expression used in this call.
@@ -191,7 +191,7 @@ class FormattingFunctionCall extends Expr {
     exists(int i |
       result = this.getArgument(i) and
       n >= 0 and
-      n = i - this.getTarget().getFirstFormatArgumentIndex()
+      n = i - this.getTarget().(FormattingFunction).getFirstFormatArgumentIndex()
     )
   }
 
@@ -251,7 +251,7 @@ class FormattingFunctionCall extends Expr {
   int getNumFormatArgument() {
     result = count(this.getFormatArgument(_)) and
     // format arguments must be known
-    exists(this.getTarget().getFirstFormatArgumentIndex())
+    exists(this.getTarget().(FormattingFunction).getFirstFormatArgumentIndex())
   }
 
   /**
@@ -270,18 +270,6 @@ 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.
  */
@@ -301,27 +289,35 @@ class FormatLiteral extends Literal {
    * a `char *` (either way, `%S` will have the opposite meaning).
    * DEPRECATED: Use getDefaultCharType() instead.
    */
-  deprecated predicate isWideCharDefault() { this.getUse().getTarget().isWideCharDefault() }
+  deprecated predicate isWideCharDefault() {
+    this.getUse().getTarget().(FormattingFunction).isWideCharDefault()
+  }
 
   /**
    * Gets the default character type expected for `%s` by this format literal.  Typically
    * `char` or `wchar_t`.
    */
-  Type getDefaultCharType() { result = this.getUse().getTarget().getDefaultCharType() }
+  Type getDefaultCharType() {
+    result = this.getUse().getTarget().(FormattingFunction).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().getNonDefaultCharType() }
+  Type getNonDefaultCharType() {
+    result = this.getUse().getTarget().(FormattingFunction).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().getWideCharType() }
+  Type getWideCharType() {
+    result = this.getUse().getTarget().(FormattingFunction).getWideCharType()
+  }
 
   /**
    * Holds if this `FormatLiteral` is in a context that supports
@@ -900,7 +896,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().getFormatCharType().getSize() > 1 and // wide function
+      this.getUse().getTarget().(FormattingFunction).getFormatCharType().getSize() > 1 and // wide function
       (
         conv = "c" and
         result = this.getNonDefaultCharType()
@@ -1060,63 +1056,39 @@ class FormatLiteral extends Literal {
         or
         this.getConversionChar(n).toLowerCase() = ["d", "i"] and
         // e.g. -2^31 = "-2147483648"
-        len =
-          min(float cand |
-            // 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.
-            cand = 1 + lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8 - 1))
-            or
-            // 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 |
-              arg = this.getUse().getConversionArgument(n) and
-              lower = lowerBound(arg.getFullyConverted()) and
-              upper = upperBound(arg.getFullyConverted())
-            |
-              cand =
-                max(int cand0 |
-                  // Include the sign bit in the length if it can be negative
-                  (
-                    if lower < 0
-                    then cand0 = 1 + lengthInBase10(lower.abs())
-                    else cand0 = lengthInBase10(lower)
-                  )
-                  or
-                  (
-                    if upper < 0
-                    then cand0 = 1 + lengthInBase10(upper.abs())
-                    else cand0 = lengthInBase10(upper)
-                  )
-                )
-            )
-          )
+        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 '-'.
+        )
         or
         this.getConversionChar(n).toLowerCase() = "u" and
         // e.g. 2^32 - 1 = "4294967295"
-        len =
-          min(float cand |
-            // The first case handles length sub-specifiers
-            cand = 2.pow(this.getIntegralDisplayType(n).getSize() * 8)
-            or
-            // 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 |
-              arg = this.getUse().getConversionArgument(n) and
-              lower = lowerBound(arg.getFullyConverted())
-            |
-              cand =
-                max(float cand0 |
-                  // If lower can be negative we use `(unsigned)-1` as the candidate value.
-                  lower < 0 and
-                  cand0 = 2.pow(any(IntType t | t.isUnsigned()).getSize() * 8)
-                  or
-                  cand0 = upperBound(arg.getFullyConverted())
-                )
-            )
-          |
-            lengthInBase10(cand)
-          )
+        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)
+        )
         or
         this.getConversionChar(n).toLowerCase() = "x" and
         // e.g. "12345678"

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) }
+predicate definitionReaches(Expr def, Expr node) { def.(Def).reaches(true, _, node.(DefOrUse)) }
 
 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)) }
+  predicate isPhiNode(StackVariable v) { exists(StandardSSA x | x.phi_node(v, this.(BasicBlock))) }
 
   /** 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)
+    if phi_node(v, node.(BasicBlock))
     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().getADeclaration()
+      this.getDeclaration() = for.getBeginEndDeclaration().(DeclStmt).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.getParentNode()
+      parent = mid.(Node).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)
+        result = this.getCompoundValue(e, req.(CompileTimeVariableExpr))
       ) 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)
+      result = this.getVariableValue(e, val.(VariableAccess))
       or
       exists(FunctionCall call | call = val and not callWithMultipleTargets(call) |
         result = this.getFunctionValue(call.getTarget())
@@ -663,7 +663,7 @@ library class ExprEvaluator extends int {
     this.interestingInternal(_, req, false) and
     (
       result = req.(CompileTimeConstantInt).getIntValue() or
-      result = this.getCompoundValueNonSubExpr(req)
+      result = this.getCompoundValueNonSubExpr(req.(CompileTimeVariableExpr))
     ) 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)
+      result = this.getVariableValueNonSubExpr(val.(VariableAccess))
       or
       exists(FunctionCall call | call = val and not callWithMultipleTargets(call) |
         result = this.getFunctionValue(call.getTarget())

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

@@ -10,7 +10,6 @@
 private import DataFlowImplCommon
 private import DataFlowImplSpecific::Private
 import DataFlowImplSpecific::Public
-import DataFlowImplCommonPublic
 
 /**
  * A configuration of interprocedural data flow analysis. This defines
@@ -95,22 +94,6 @@ abstract class Configuration extends string {
    */
   int fieldFlowBranchLimit() { result = 2 }
 
-  /**
-   * Gets a data flow configuration feature to add restrictions to the set of
-   * valid flow paths.
-   *
-   * - `FeatureHasSourceCallContext`:
-   *    Assume that sources have some existing call context to disallow
-   *    conflicting return-flow directly following the source.
-   * - `FeatureHasSinkCallContext`:
-   *    Assume that sinks have some existing call context to disallow
-   *    conflicting argument-to-parameter flow directly preceding the sink.
-   * - `FeatureEqualSourceSinkCallContext`:
-   *    Implies both of the above and additionally ensures that the entire flow
-   *    path preserves the call context.
-   */
-  FlowFeature getAFeature() { none() }
-
   /**
    * Holds if data may flow from `source` to `sink` for this configuration.
    */
@@ -366,8 +349,7 @@ private predicate jumpStep(NodeEx node1, NodeEx node2, Configuration config) {
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -383,8 +365,7 @@ private predicate additionalJumpStep(NodeEx node1, NodeEx node2, Configuration c
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -420,20 +401,6 @@ private predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx a
  */
 private predicate useFieldFlow(Configuration config) { config.fieldFlowBranchLimit() >= 1 }
 
-private predicate hasSourceCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSourceCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
-private predicate hasSinkCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSinkCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
 private module Stage1 {
   class ApApprox = Unit;
 
@@ -454,7 +421,7 @@ private module Stage1 {
     not fullBarrier(node, config) and
     (
       sourceNode(node, config) and
-      if hasSourceCallCtx(config) then cc = true else cc = false
+      cc = false
       or
       exists(NodeEx mid |
         fwdFlow(mid, cc, config) and
@@ -584,7 +551,7 @@ private module Stage1 {
   private predicate revFlow0(NodeEx node, boolean toReturn, Configuration config) {
     fwdFlow(node, config) and
     sinkNode(node, config) and
-    if hasSinkCallCtx(config) then toReturn = true else toReturn = false
+    toReturn = false
     or
     exists(NodeEx mid |
       localFlowStep(node, mid, config) and
@@ -970,8 +937,6 @@ private module Stage2 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1039,7 +1004,7 @@ private module Stage2 {
   predicate fwdFlow(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1250,7 +1215,7 @@ private module Stage2 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -1651,8 +1616,6 @@ private module Stage3 {
 
   Cc ccNone() { result = false }
 
-  CcCall ccSomeCall() { result = true }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1734,7 +1697,7 @@ private module Stage3 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1945,7 +1908,7 @@ private module Stage3 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -2403,8 +2366,6 @@ private module Stage4 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = LocalCallContext;
 
   bindingset[call, c, outercc]
@@ -2500,7 +2461,7 @@ private module Stage4 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -2711,7 +2672,7 @@ private module Stage4 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -3103,11 +3064,7 @@ private newtype TPathNode =
     // A PathNode is introduced by a source ...
     Stage4::revFlow(node, config) and
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap = TAccessPathNil(node.getDataFlowType())
     or
@@ -3119,10 +3076,17 @@ private newtype TPathNode =
     )
   } or
   TPathNodeSink(NodeEx node, Configuration config) {
-    exists(PathNodeMid sink |
-      sink.isAtSink() and
-      node = sink.getNodeEx() and
-      config = sink.getConfiguration()
+    sinkNode(node, pragma[only_bind_into](config)) and
+    Stage4::revFlow(node, pragma[only_bind_into](config)) and
+    (
+      // A sink that is also a source ...
+      sourceNode(node, config)
+      or
+      // ... or a sink that can be reached from a source
+      exists(PathNodeMid mid |
+        pathStep(mid, node, _, _, TAccessPathNil(_)) and
+        pragma[only_bind_into](config) = mid.getConfiguration()
+      )
     )
   }
 
@@ -3439,46 +3403,22 @@ private class PathNodeMid extends PathNodeImpl, TPathNodeMid {
     // an intermediate step to another intermediate node
     result = this.getSuccMid()
     or
-    // a final step to a sink
-    result = this.getSuccMid().projectToSink()
+    // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
+    exists(PathNodeMid mid, PathNodeSink sink |
+      mid = this.getSuccMid() and
+      mid.getNodeEx() = sink.getNodeEx() and
+      mid.getAp() instanceof AccessPathNil and
+      sink.getConfiguration() = unbindConf(mid.getConfiguration()) and
+      result = sink
+    )
   }
 
   override predicate isSource() {
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap instanceof AccessPathNil
   }
-
-  predicate isAtSink() {
-    sinkNode(node, config) and
-    ap instanceof AccessPathNil and
-    if hasSinkCallCtx(config)
-    then
-      // For `FeatureHasSinkCallContext` the condition `cc instanceof CallContextNoCall`
-      // is exactly what we need to check. This also implies
-      // `sc instanceof SummaryCtxNone`.
-      // For `FeatureEqualSourceSinkCallContext` the initial call context was
-      // set to `CallContextSomeCall` and jumps are disallowed, so
-      // `cc instanceof CallContextNoCall` never holds. On the other hand,
-      // in this case there's never any need to enter a call except to identify
-      // a summary, so the condition in `pathIntoCallable` enforces this, which
-      // means that `sc instanceof SummaryCtxNone` holds if and only if we are
-      // in the call context of the source.
-      sc instanceof SummaryCtxNone or
-      cc instanceof CallContextNoCall
-    else any()
-  }
-
-  PathNodeSink projectToSink() {
-    this.isAtSink() and
-    result.getNodeEx() = node and
-    result.getConfiguration() = unbindConf(config)
-  }
 }
 
 /**
@@ -3632,7 +3572,7 @@ private predicate pathIntoArg(
   )
 }
 
-pragma[nomagic]
+pragma[noinline]
 private predicate parameterCand(
   DataFlowCallable callable, int i, AccessPathApprox apa, Configuration config
 ) {
@@ -3673,11 +3613,7 @@ private predicate pathIntoCallable(
       sc = TSummaryCtxSome(p, ap)
       or
       not exists(TSummaryCtxSome(p, ap)) and
-      sc = TSummaryCtxNone() and
-      // When the call contexts of source and sink needs to match then there's
-      // never any reason to enter a callable except to find a summary. See also
-      // the comment in `PathNodeMid::isAtSink`.
-      not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+      sc = TSummaryCtxNone()
     )
   |
     if recordDataFlowCallSite(call, callable)
@@ -3740,14 +3676,13 @@ private module Subpaths {
    */
   pragma[nomagic]
   private predicate subpaths01(
-    PathNodeImpl arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
+    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
     exists(Configuration config |
       pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
       pathIntoCallable(arg, par, _, innercc, sc, _, config) and
-      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config)) and
-      not arg.isHidden()
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
     )
   }
 
@@ -3781,17 +3716,8 @@ private module Subpaths {
       innercc = ret.getCallContext() and
       sc = ret.getSummaryCtx() and
       ret.getConfiguration() = unbindConf(getPathNodeConf(arg)) and
-      apout = ret.getAp()
-    )
-  }
-
-  private PathNodeImpl localStepToHidden(PathNodeImpl n) {
-    n.getASuccessorImpl() = result and
-    result.isHidden() and
-    exists(NodeEx n1, NodeEx n2 | n1 = n.getNodeEx() and n2 = result.getNodeEx() |
-      localFlowBigStep(n1, n2, _, _, _, _) or
-      store(n1, _, n2, _, _) or
-      read(n1, _, n2, _)
+      apout = ret.getAp() and
+      not ret.isHidden()
     )
   }
 
@@ -3800,12 +3726,11 @@ private module Subpaths {
    * a subpath between `par` and `ret` with the connecting edges `arg -> par` and
    * `ret -> out` is summarized as the edge `arg -> out`.
    */
-  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeImpl ret, PathNodeMid out) {
+  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
       pragma[only_bind_into](arg).getASuccessor() = par and
       pragma[only_bind_into](arg).getASuccessor() = out and
-      subpaths03(arg, p, localStepToHidden*(ret), o, apout) and
-      not ret.isHidden() and
+      subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and
       out.getAp() = apout

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

@@ -10,7 +10,6 @@
 private import DataFlowImplCommon
 private import DataFlowImplSpecific::Private
 import DataFlowImplSpecific::Public
-import DataFlowImplCommonPublic
 
 /**
  * A configuration of interprocedural data flow analysis. This defines
@@ -95,22 +94,6 @@ abstract class Configuration extends string {
    */
   int fieldFlowBranchLimit() { result = 2 }
 
-  /**
-   * Gets a data flow configuration feature to add restrictions to the set of
-   * valid flow paths.
-   *
-   * - `FeatureHasSourceCallContext`:
-   *    Assume that sources have some existing call context to disallow
-   *    conflicting return-flow directly following the source.
-   * - `FeatureHasSinkCallContext`:
-   *    Assume that sinks have some existing call context to disallow
-   *    conflicting argument-to-parameter flow directly preceding the sink.
-   * - `FeatureEqualSourceSinkCallContext`:
-   *    Implies both of the above and additionally ensures that the entire flow
-   *    path preserves the call context.
-   */
-  FlowFeature getAFeature() { none() }
-
   /**
    * Holds if data may flow from `source` to `sink` for this configuration.
    */
@@ -366,8 +349,7 @@ private predicate jumpStep(NodeEx node1, NodeEx node2, Configuration config) {
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -383,8 +365,7 @@ private predicate additionalJumpStep(NodeEx node1, NodeEx node2, Configuration c
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -420,20 +401,6 @@ private predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx a
  */
 private predicate useFieldFlow(Configuration config) { config.fieldFlowBranchLimit() >= 1 }
 
-private predicate hasSourceCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSourceCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
-private predicate hasSinkCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSinkCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
 private module Stage1 {
   class ApApprox = Unit;
 
@@ -454,7 +421,7 @@ private module Stage1 {
     not fullBarrier(node, config) and
     (
       sourceNode(node, config) and
-      if hasSourceCallCtx(config) then cc = true else cc = false
+      cc = false
       or
       exists(NodeEx mid |
         fwdFlow(mid, cc, config) and
@@ -584,7 +551,7 @@ private module Stage1 {
   private predicate revFlow0(NodeEx node, boolean toReturn, Configuration config) {
     fwdFlow(node, config) and
     sinkNode(node, config) and
-    if hasSinkCallCtx(config) then toReturn = true else toReturn = false
+    toReturn = false
     or
     exists(NodeEx mid |
       localFlowStep(node, mid, config) and
@@ -970,8 +937,6 @@ private module Stage2 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1039,7 +1004,7 @@ private module Stage2 {
   predicate fwdFlow(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1250,7 +1215,7 @@ private module Stage2 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -1651,8 +1616,6 @@ private module Stage3 {
 
   Cc ccNone() { result = false }
 
-  CcCall ccSomeCall() { result = true }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1734,7 +1697,7 @@ private module Stage3 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1945,7 +1908,7 @@ private module Stage3 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -2403,8 +2366,6 @@ private module Stage4 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = LocalCallContext;
 
   bindingset[call, c, outercc]
@@ -2500,7 +2461,7 @@ private module Stage4 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -2711,7 +2672,7 @@ private module Stage4 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -3103,11 +3064,7 @@ private newtype TPathNode =
     // A PathNode is introduced by a source ...
     Stage4::revFlow(node, config) and
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap = TAccessPathNil(node.getDataFlowType())
     or
@@ -3119,10 +3076,17 @@ private newtype TPathNode =
     )
   } or
   TPathNodeSink(NodeEx node, Configuration config) {
-    exists(PathNodeMid sink |
-      sink.isAtSink() and
-      node = sink.getNodeEx() and
-      config = sink.getConfiguration()
+    sinkNode(node, pragma[only_bind_into](config)) and
+    Stage4::revFlow(node, pragma[only_bind_into](config)) and
+    (
+      // A sink that is also a source ...
+      sourceNode(node, config)
+      or
+      // ... or a sink that can be reached from a source
+      exists(PathNodeMid mid |
+        pathStep(mid, node, _, _, TAccessPathNil(_)) and
+        pragma[only_bind_into](config) = mid.getConfiguration()
+      )
     )
   }
 
@@ -3439,46 +3403,22 @@ private class PathNodeMid extends PathNodeImpl, TPathNodeMid {
     // an intermediate step to another intermediate node
     result = this.getSuccMid()
     or
-    // a final step to a sink
-    result = this.getSuccMid().projectToSink()
+    // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
+    exists(PathNodeMid mid, PathNodeSink sink |
+      mid = this.getSuccMid() and
+      mid.getNodeEx() = sink.getNodeEx() and
+      mid.getAp() instanceof AccessPathNil and
+      sink.getConfiguration() = unbindConf(mid.getConfiguration()) and
+      result = sink
+    )
   }
 
   override predicate isSource() {
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap instanceof AccessPathNil
   }
-
-  predicate isAtSink() {
-    sinkNode(node, config) and
-    ap instanceof AccessPathNil and
-    if hasSinkCallCtx(config)
-    then
-      // For `FeatureHasSinkCallContext` the condition `cc instanceof CallContextNoCall`
-      // is exactly what we need to check. This also implies
-      // `sc instanceof SummaryCtxNone`.
-      // For `FeatureEqualSourceSinkCallContext` the initial call context was
-      // set to `CallContextSomeCall` and jumps are disallowed, so
-      // `cc instanceof CallContextNoCall` never holds. On the other hand,
-      // in this case there's never any need to enter a call except to identify
-      // a summary, so the condition in `pathIntoCallable` enforces this, which
-      // means that `sc instanceof SummaryCtxNone` holds if and only if we are
-      // in the call context of the source.
-      sc instanceof SummaryCtxNone or
-      cc instanceof CallContextNoCall
-    else any()
-  }
-
-  PathNodeSink projectToSink() {
-    this.isAtSink() and
-    result.getNodeEx() = node and
-    result.getConfiguration() = unbindConf(config)
-  }
 }
 
 /**
@@ -3632,7 +3572,7 @@ private predicate pathIntoArg(
   )
 }
 
-pragma[nomagic]
+pragma[noinline]
 private predicate parameterCand(
   DataFlowCallable callable, int i, AccessPathApprox apa, Configuration config
 ) {
@@ -3673,11 +3613,7 @@ private predicate pathIntoCallable(
       sc = TSummaryCtxSome(p, ap)
       or
       not exists(TSummaryCtxSome(p, ap)) and
-      sc = TSummaryCtxNone() and
-      // When the call contexts of source and sink needs to match then there's
-      // never any reason to enter a callable except to find a summary. See also
-      // the comment in `PathNodeMid::isAtSink`.
-      not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+      sc = TSummaryCtxNone()
     )
   |
     if recordDataFlowCallSite(call, callable)
@@ -3740,14 +3676,13 @@ private module Subpaths {
    */
   pragma[nomagic]
   private predicate subpaths01(
-    PathNodeImpl arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
+    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
     exists(Configuration config |
       pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
       pathIntoCallable(arg, par, _, innercc, sc, _, config) and
-      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config)) and
-      not arg.isHidden()
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
     )
   }
 
@@ -3781,17 +3716,8 @@ private module Subpaths {
       innercc = ret.getCallContext() and
       sc = ret.getSummaryCtx() and
       ret.getConfiguration() = unbindConf(getPathNodeConf(arg)) and
-      apout = ret.getAp()
-    )
-  }
-
-  private PathNodeImpl localStepToHidden(PathNodeImpl n) {
-    n.getASuccessorImpl() = result and
-    result.isHidden() and
-    exists(NodeEx n1, NodeEx n2 | n1 = n.getNodeEx() and n2 = result.getNodeEx() |
-      localFlowBigStep(n1, n2, _, _, _, _) or
-      store(n1, _, n2, _, _) or
-      read(n1, _, n2, _)
+      apout = ret.getAp() and
+      not ret.isHidden()
     )
   }
 
@@ -3800,12 +3726,11 @@ private module Subpaths {
    * a subpath between `par` and `ret` with the connecting edges `arg -> par` and
    * `ret -> out` is summarized as the edge `arg -> out`.
    */
-  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeImpl ret, PathNodeMid out) {
+  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
       pragma[only_bind_into](arg).getASuccessor() = par and
       pragma[only_bind_into](arg).getASuccessor() = out and
-      subpaths03(arg, p, localStepToHidden*(ret), o, apout) and
-      not ret.isHidden() and
+      subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and
       out.getAp() = apout

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

@@ -10,7 +10,6 @@
 private import DataFlowImplCommon
 private import DataFlowImplSpecific::Private
 import DataFlowImplSpecific::Public
-import DataFlowImplCommonPublic
 
 /**
  * A configuration of interprocedural data flow analysis. This defines
@@ -95,22 +94,6 @@ abstract class Configuration extends string {
    */
   int fieldFlowBranchLimit() { result = 2 }
 
-  /**
-   * Gets a data flow configuration feature to add restrictions to the set of
-   * valid flow paths.
-   *
-   * - `FeatureHasSourceCallContext`:
-   *    Assume that sources have some existing call context to disallow
-   *    conflicting return-flow directly following the source.
-   * - `FeatureHasSinkCallContext`:
-   *    Assume that sinks have some existing call context to disallow
-   *    conflicting argument-to-parameter flow directly preceding the sink.
-   * - `FeatureEqualSourceSinkCallContext`:
-   *    Implies both of the above and additionally ensures that the entire flow
-   *    path preserves the call context.
-   */
-  FlowFeature getAFeature() { none() }
-
   /**
    * Holds if data may flow from `source` to `sink` for this configuration.
    */
@@ -366,8 +349,7 @@ private predicate jumpStep(NodeEx node1, NodeEx node2, Configuration config) {
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -383,8 +365,7 @@ private predicate additionalJumpStep(NodeEx node1, NodeEx node2, Configuration c
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -420,20 +401,6 @@ private predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx a
  */
 private predicate useFieldFlow(Configuration config) { config.fieldFlowBranchLimit() >= 1 }
 
-private predicate hasSourceCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSourceCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
-private predicate hasSinkCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSinkCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
 private module Stage1 {
   class ApApprox = Unit;
 
@@ -454,7 +421,7 @@ private module Stage1 {
     not fullBarrier(node, config) and
     (
       sourceNode(node, config) and
-      if hasSourceCallCtx(config) then cc = true else cc = false
+      cc = false
       or
       exists(NodeEx mid |
         fwdFlow(mid, cc, config) and
@@ -584,7 +551,7 @@ private module Stage1 {
   private predicate revFlow0(NodeEx node, boolean toReturn, Configuration config) {
     fwdFlow(node, config) and
     sinkNode(node, config) and
-    if hasSinkCallCtx(config) then toReturn = true else toReturn = false
+    toReturn = false
     or
     exists(NodeEx mid |
       localFlowStep(node, mid, config) and
@@ -970,8 +937,6 @@ private module Stage2 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1039,7 +1004,7 @@ private module Stage2 {
   predicate fwdFlow(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1250,7 +1215,7 @@ private module Stage2 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -1651,8 +1616,6 @@ private module Stage3 {
 
   Cc ccNone() { result = false }
 
-  CcCall ccSomeCall() { result = true }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1734,7 +1697,7 @@ private module Stage3 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1945,7 +1908,7 @@ private module Stage3 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -2403,8 +2366,6 @@ private module Stage4 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = LocalCallContext;
 
   bindingset[call, c, outercc]
@@ -2500,7 +2461,7 @@ private module Stage4 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -2711,7 +2672,7 @@ private module Stage4 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -3103,11 +3064,7 @@ private newtype TPathNode =
     // A PathNode is introduced by a source ...
     Stage4::revFlow(node, config) and
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap = TAccessPathNil(node.getDataFlowType())
     or
@@ -3119,10 +3076,17 @@ private newtype TPathNode =
     )
   } or
   TPathNodeSink(NodeEx node, Configuration config) {
-    exists(PathNodeMid sink |
-      sink.isAtSink() and
-      node = sink.getNodeEx() and
-      config = sink.getConfiguration()
+    sinkNode(node, pragma[only_bind_into](config)) and
+    Stage4::revFlow(node, pragma[only_bind_into](config)) and
+    (
+      // A sink that is also a source ...
+      sourceNode(node, config)
+      or
+      // ... or a sink that can be reached from a source
+      exists(PathNodeMid mid |
+        pathStep(mid, node, _, _, TAccessPathNil(_)) and
+        pragma[only_bind_into](config) = mid.getConfiguration()
+      )
     )
   }
 
@@ -3439,46 +3403,22 @@ private class PathNodeMid extends PathNodeImpl, TPathNodeMid {
     // an intermediate step to another intermediate node
     result = this.getSuccMid()
     or
-    // a final step to a sink
-    result = this.getSuccMid().projectToSink()
+    // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
+    exists(PathNodeMid mid, PathNodeSink sink |
+      mid = this.getSuccMid() and
+      mid.getNodeEx() = sink.getNodeEx() and
+      mid.getAp() instanceof AccessPathNil and
+      sink.getConfiguration() = unbindConf(mid.getConfiguration()) and
+      result = sink
+    )
   }
 
   override predicate isSource() {
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap instanceof AccessPathNil
   }
-
-  predicate isAtSink() {
-    sinkNode(node, config) and
-    ap instanceof AccessPathNil and
-    if hasSinkCallCtx(config)
-    then
-      // For `FeatureHasSinkCallContext` the condition `cc instanceof CallContextNoCall`
-      // is exactly what we need to check. This also implies
-      // `sc instanceof SummaryCtxNone`.
-      // For `FeatureEqualSourceSinkCallContext` the initial call context was
-      // set to `CallContextSomeCall` and jumps are disallowed, so
-      // `cc instanceof CallContextNoCall` never holds. On the other hand,
-      // in this case there's never any need to enter a call except to identify
-      // a summary, so the condition in `pathIntoCallable` enforces this, which
-      // means that `sc instanceof SummaryCtxNone` holds if and only if we are
-      // in the call context of the source.
-      sc instanceof SummaryCtxNone or
-      cc instanceof CallContextNoCall
-    else any()
-  }
-
-  PathNodeSink projectToSink() {
-    this.isAtSink() and
-    result.getNodeEx() = node and
-    result.getConfiguration() = unbindConf(config)
-  }
 }
 
 /**
@@ -3632,7 +3572,7 @@ private predicate pathIntoArg(
   )
 }
 
-pragma[nomagic]
+pragma[noinline]
 private predicate parameterCand(
   DataFlowCallable callable, int i, AccessPathApprox apa, Configuration config
 ) {
@@ -3673,11 +3613,7 @@ private predicate pathIntoCallable(
       sc = TSummaryCtxSome(p, ap)
       or
       not exists(TSummaryCtxSome(p, ap)) and
-      sc = TSummaryCtxNone() and
-      // When the call contexts of source and sink needs to match then there's
-      // never any reason to enter a callable except to find a summary. See also
-      // the comment in `PathNodeMid::isAtSink`.
-      not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+      sc = TSummaryCtxNone()
     )
   |
     if recordDataFlowCallSite(call, callable)
@@ -3740,14 +3676,13 @@ private module Subpaths {
    */
   pragma[nomagic]
   private predicate subpaths01(
-    PathNodeImpl arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
+    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
     exists(Configuration config |
       pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
       pathIntoCallable(arg, par, _, innercc, sc, _, config) and
-      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config)) and
-      not arg.isHidden()
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
     )
   }
 
@@ -3781,17 +3716,8 @@ private module Subpaths {
       innercc = ret.getCallContext() and
       sc = ret.getSummaryCtx() and
       ret.getConfiguration() = unbindConf(getPathNodeConf(arg)) and
-      apout = ret.getAp()
-    )
-  }
-
-  private PathNodeImpl localStepToHidden(PathNodeImpl n) {
-    n.getASuccessorImpl() = result and
-    result.isHidden() and
-    exists(NodeEx n1, NodeEx n2 | n1 = n.getNodeEx() and n2 = result.getNodeEx() |
-      localFlowBigStep(n1, n2, _, _, _, _) or
-      store(n1, _, n2, _, _) or
-      read(n1, _, n2, _)
+      apout = ret.getAp() and
+      not ret.isHidden()
     )
   }
 
@@ -3800,12 +3726,11 @@ private module Subpaths {
    * a subpath between `par` and `ret` with the connecting edges `arg -> par` and
    * `ret -> out` is summarized as the edge `arg -> out`.
    */
-  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeImpl ret, PathNodeMid out) {
+  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
       pragma[only_bind_into](arg).getASuccessor() = par and
       pragma[only_bind_into](arg).getASuccessor() = out and
-      subpaths03(arg, p, localStepToHidden*(ret), o, apout) and
-      not ret.isHidden() and
+      subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and
       out.getAp() = apout

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

@@ -10,7 +10,6 @@
 private import DataFlowImplCommon
 private import DataFlowImplSpecific::Private
 import DataFlowImplSpecific::Public
-import DataFlowImplCommonPublic
 
 /**
  * A configuration of interprocedural data flow analysis. This defines
@@ -95,22 +94,6 @@ abstract class Configuration extends string {
    */
   int fieldFlowBranchLimit() { result = 2 }
 
-  /**
-   * Gets a data flow configuration feature to add restrictions to the set of
-   * valid flow paths.
-   *
-   * - `FeatureHasSourceCallContext`:
-   *    Assume that sources have some existing call context to disallow
-   *    conflicting return-flow directly following the source.
-   * - `FeatureHasSinkCallContext`:
-   *    Assume that sinks have some existing call context to disallow
-   *    conflicting argument-to-parameter flow directly preceding the sink.
-   * - `FeatureEqualSourceSinkCallContext`:
-   *    Implies both of the above and additionally ensures that the entire flow
-   *    path preserves the call context.
-   */
-  FlowFeature getAFeature() { none() }
-
   /**
    * Holds if data may flow from `source` to `sink` for this configuration.
    */
@@ -366,8 +349,7 @@ private predicate jumpStep(NodeEx node1, NodeEx node2, Configuration config) {
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -383,8 +365,7 @@ private predicate additionalJumpStep(NodeEx node1, NodeEx node2, Configuration c
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -420,20 +401,6 @@ private predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx a
  */
 private predicate useFieldFlow(Configuration config) { config.fieldFlowBranchLimit() >= 1 }
 
-private predicate hasSourceCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSourceCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
-private predicate hasSinkCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSinkCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
 private module Stage1 {
   class ApApprox = Unit;
 
@@ -454,7 +421,7 @@ private module Stage1 {
     not fullBarrier(node, config) and
     (
       sourceNode(node, config) and
-      if hasSourceCallCtx(config) then cc = true else cc = false
+      cc = false
       or
       exists(NodeEx mid |
         fwdFlow(mid, cc, config) and
@@ -584,7 +551,7 @@ private module Stage1 {
   private predicate revFlow0(NodeEx node, boolean toReturn, Configuration config) {
     fwdFlow(node, config) and
     sinkNode(node, config) and
-    if hasSinkCallCtx(config) then toReturn = true else toReturn = false
+    toReturn = false
     or
     exists(NodeEx mid |
       localFlowStep(node, mid, config) and
@@ -970,8 +937,6 @@ private module Stage2 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1039,7 +1004,7 @@ private module Stage2 {
   predicate fwdFlow(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1250,7 +1215,7 @@ private module Stage2 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -1651,8 +1616,6 @@ private module Stage3 {
 
   Cc ccNone() { result = false }
 
-  CcCall ccSomeCall() { result = true }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1734,7 +1697,7 @@ private module Stage3 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1945,7 +1908,7 @@ private module Stage3 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -2403,8 +2366,6 @@ private module Stage4 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = LocalCallContext;
 
   bindingset[call, c, outercc]
@@ -2500,7 +2461,7 @@ private module Stage4 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -2711,7 +2672,7 @@ private module Stage4 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -3103,11 +3064,7 @@ private newtype TPathNode =
     // A PathNode is introduced by a source ...
     Stage4::revFlow(node, config) and
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap = TAccessPathNil(node.getDataFlowType())
     or
@@ -3119,10 +3076,17 @@ private newtype TPathNode =
     )
   } or
   TPathNodeSink(NodeEx node, Configuration config) {
-    exists(PathNodeMid sink |
-      sink.isAtSink() and
-      node = sink.getNodeEx() and
-      config = sink.getConfiguration()
+    sinkNode(node, pragma[only_bind_into](config)) and
+    Stage4::revFlow(node, pragma[only_bind_into](config)) and
+    (
+      // A sink that is also a source ...
+      sourceNode(node, config)
+      or
+      // ... or a sink that can be reached from a source
+      exists(PathNodeMid mid |
+        pathStep(mid, node, _, _, TAccessPathNil(_)) and
+        pragma[only_bind_into](config) = mid.getConfiguration()
+      )
     )
   }
 
@@ -3439,46 +3403,22 @@ private class PathNodeMid extends PathNodeImpl, TPathNodeMid {
     // an intermediate step to another intermediate node
     result = this.getSuccMid()
     or
-    // a final step to a sink
-    result = this.getSuccMid().projectToSink()
+    // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
+    exists(PathNodeMid mid, PathNodeSink sink |
+      mid = this.getSuccMid() and
+      mid.getNodeEx() = sink.getNodeEx() and
+      mid.getAp() instanceof AccessPathNil and
+      sink.getConfiguration() = unbindConf(mid.getConfiguration()) and
+      result = sink
+    )
   }
 
   override predicate isSource() {
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap instanceof AccessPathNil
   }
-
-  predicate isAtSink() {
-    sinkNode(node, config) and
-    ap instanceof AccessPathNil and
-    if hasSinkCallCtx(config)
-    then
-      // For `FeatureHasSinkCallContext` the condition `cc instanceof CallContextNoCall`
-      // is exactly what we need to check. This also implies
-      // `sc instanceof SummaryCtxNone`.
-      // For `FeatureEqualSourceSinkCallContext` the initial call context was
-      // set to `CallContextSomeCall` and jumps are disallowed, so
-      // `cc instanceof CallContextNoCall` never holds. On the other hand,
-      // in this case there's never any need to enter a call except to identify
-      // a summary, so the condition in `pathIntoCallable` enforces this, which
-      // means that `sc instanceof SummaryCtxNone` holds if and only if we are
-      // in the call context of the source.
-      sc instanceof SummaryCtxNone or
-      cc instanceof CallContextNoCall
-    else any()
-  }
-
-  PathNodeSink projectToSink() {
-    this.isAtSink() and
-    result.getNodeEx() = node and
-    result.getConfiguration() = unbindConf(config)
-  }
 }
 
 /**
@@ -3632,7 +3572,7 @@ private predicate pathIntoArg(
   )
 }
 
-pragma[nomagic]
+pragma[noinline]
 private predicate parameterCand(
   DataFlowCallable callable, int i, AccessPathApprox apa, Configuration config
 ) {
@@ -3673,11 +3613,7 @@ private predicate pathIntoCallable(
       sc = TSummaryCtxSome(p, ap)
       or
       not exists(TSummaryCtxSome(p, ap)) and
-      sc = TSummaryCtxNone() and
-      // When the call contexts of source and sink needs to match then there's
-      // never any reason to enter a callable except to find a summary. See also
-      // the comment in `PathNodeMid::isAtSink`.
-      not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+      sc = TSummaryCtxNone()
     )
   |
     if recordDataFlowCallSite(call, callable)
@@ -3740,14 +3676,13 @@ private module Subpaths {
    */
   pragma[nomagic]
   private predicate subpaths01(
-    PathNodeImpl arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
+    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
     exists(Configuration config |
       pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
       pathIntoCallable(arg, par, _, innercc, sc, _, config) and
-      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config)) and
-      not arg.isHidden()
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
     )
   }
 
@@ -3781,17 +3716,8 @@ private module Subpaths {
       innercc = ret.getCallContext() and
       sc = ret.getSummaryCtx() and
       ret.getConfiguration() = unbindConf(getPathNodeConf(arg)) and
-      apout = ret.getAp()
-    )
-  }
-
-  private PathNodeImpl localStepToHidden(PathNodeImpl n) {
-    n.getASuccessorImpl() = result and
-    result.isHidden() and
-    exists(NodeEx n1, NodeEx n2 | n1 = n.getNodeEx() and n2 = result.getNodeEx() |
-      localFlowBigStep(n1, n2, _, _, _, _) or
-      store(n1, _, n2, _, _) or
-      read(n1, _, n2, _)
+      apout = ret.getAp() and
+      not ret.isHidden()
     )
   }
 
@@ -3800,12 +3726,11 @@ private module Subpaths {
    * a subpath between `par` and `ret` with the connecting edges `arg -> par` and
    * `ret -> out` is summarized as the edge `arg -> out`.
    */
-  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeImpl ret, PathNodeMid out) {
+  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
       pragma[only_bind_into](arg).getASuccessor() = par and
       pragma[only_bind_into](arg).getASuccessor() = out and
-      subpaths03(arg, p, localStepToHidden*(ret), o, apout) and
-      not ret.isHidden() and
+      subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and
       out.getAp() = apout

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

@@ -2,42 +2,6 @@ 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.
  *
@@ -287,7 +251,7 @@ private module Cached {
   predicate forceCachingInSameStage() { any() }
 
   cached
-  predicate nodeEnclosingCallable(Node n, DataFlowCallable c) { c = nodeGetEnclosingCallable(n) }
+  predicate nodeEnclosingCallable(Node n, DataFlowCallable c) { c = n.getEnclosingCallable() }
 
   cached
   predicate callEnclosingCallable(DataFlowCall call, DataFlowCallable c) {
@@ -352,7 +316,9 @@ private module Cached {
   }
 
   cached
-  predicate parameterNode(Node p, DataFlowCallable c, int pos) { isParameterNode(p, c, pos) }
+  predicate parameterNode(Node n, DataFlowCallable c, int i) {
+    n.(ParameterNode).isParameterOf(c, i)
+  }
 
   cached
   predicate argumentNode(Node n, DataFlowCall call, int pos) {

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

@@ -31,7 +31,7 @@ module Consistency {
   query predicate uniqueEnclosingCallable(Node n, string msg) {
     exists(int c |
       n instanceof RelevantNode and
-      c = count(nodeGetEnclosingCallable(n)) and
+      c = count(n.getEnclosingCallable()) and
       c != 1 and
       msg = "Node should have one enclosing callable but has " + c + "."
     )
@@ -85,13 +85,13 @@ module Consistency {
   }
 
   query predicate parameterCallable(ParameterNode p, string msg) {
-    exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and
+    exists(DataFlowCallable c | p.isParameterOf(c, _) and c != p.getEnclosingCallable()) and
     msg = "Callable mismatch for parameter."
   }
 
   query predicate localFlowIsLocal(Node n1, Node n2, string msg) {
     simpleLocalFlowStep(n1, n2) and
-    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    n1.getEnclosingCallable() != n2.getEnclosingCallable() and
     msg = "Local flow step does not preserve enclosing callable."
   }
 
@@ -106,7 +106,7 @@ module Consistency {
   query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) {
     isUnreachableInCall(n, call) and
     exists(DataFlowCallable c |
-      c = nodeGetEnclosingCallable(n) and
+      c = n.getEnclosingCallable() and
       not viableCallable(call) = c
     ) and
     msg = "Call context for isUnreachableInCall is inconsistent with call graph."
@@ -120,7 +120,7 @@ module Consistency {
       n.(ArgumentNode).argumentOf(call, _) and
       msg = "ArgumentNode and call does not share enclosing callable."
     ) and
-    nodeGetEnclosingCallable(n) != call.getEnclosingCallable()
+    n.getEnclosingCallable() != call.getEnclosingCallable()
   }
 
   // This predicate helps the compiler forget that in some languages
@@ -151,7 +151,7 @@ module Consistency {
   }
 
   query predicate postIsInSameCallable(PostUpdateNode n, string msg) {
-    nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and
+    n.getEnclosingCallable() != n.getPreUpdateNode().getEnclosingCallable() and
     msg = "PostUpdateNode does not share callable with its pre-update node."
   }
 

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

@@ -10,7 +10,6 @@
 private import DataFlowImplCommon
 private import DataFlowImplSpecific::Private
 import DataFlowImplSpecific::Public
-import DataFlowImplCommonPublic
 
 /**
  * A configuration of interprocedural data flow analysis. This defines
@@ -95,22 +94,6 @@ abstract class Configuration extends string {
    */
   int fieldFlowBranchLimit() { result = 2 }
 
-  /**
-   * Gets a data flow configuration feature to add restrictions to the set of
-   * valid flow paths.
-   *
-   * - `FeatureHasSourceCallContext`:
-   *    Assume that sources have some existing call context to disallow
-   *    conflicting return-flow directly following the source.
-   * - `FeatureHasSinkCallContext`:
-   *    Assume that sinks have some existing call context to disallow
-   *    conflicting argument-to-parameter flow directly preceding the sink.
-   * - `FeatureEqualSourceSinkCallContext`:
-   *    Implies both of the above and additionally ensures that the entire flow
-   *    path preserves the call context.
-   */
-  FlowFeature getAFeature() { none() }
-
   /**
    * Holds if data may flow from `source` to `sink` for this configuration.
    */
@@ -366,8 +349,7 @@ private predicate jumpStep(NodeEx node1, NodeEx node2, Configuration config) {
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -383,8 +365,7 @@ private predicate additionalJumpStep(NodeEx node1, NodeEx node2, Configuration c
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -420,20 +401,6 @@ private predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx a
  */
 private predicate useFieldFlow(Configuration config) { config.fieldFlowBranchLimit() >= 1 }
 
-private predicate hasSourceCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSourceCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
-private predicate hasSinkCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSinkCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
 private module Stage1 {
   class ApApprox = Unit;
 
@@ -454,7 +421,7 @@ private module Stage1 {
     not fullBarrier(node, config) and
     (
       sourceNode(node, config) and
-      if hasSourceCallCtx(config) then cc = true else cc = false
+      cc = false
       or
       exists(NodeEx mid |
         fwdFlow(mid, cc, config) and
@@ -584,7 +551,7 @@ private module Stage1 {
   private predicate revFlow0(NodeEx node, boolean toReturn, Configuration config) {
     fwdFlow(node, config) and
     sinkNode(node, config) and
-    if hasSinkCallCtx(config) then toReturn = true else toReturn = false
+    toReturn = false
     or
     exists(NodeEx mid |
       localFlowStep(node, mid, config) and
@@ -970,8 +937,6 @@ private module Stage2 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1039,7 +1004,7 @@ private module Stage2 {
   predicate fwdFlow(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1250,7 +1215,7 @@ private module Stage2 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -1651,8 +1616,6 @@ private module Stage3 {
 
   Cc ccNone() { result = false }
 
-  CcCall ccSomeCall() { result = true }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1734,7 +1697,7 @@ private module Stage3 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1945,7 +1908,7 @@ private module Stage3 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -2403,8 +2366,6 @@ private module Stage4 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = LocalCallContext;
 
   bindingset[call, c, outercc]
@@ -2500,7 +2461,7 @@ private module Stage4 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -2711,7 +2672,7 @@ private module Stage4 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -3103,11 +3064,7 @@ private newtype TPathNode =
     // A PathNode is introduced by a source ...
     Stage4::revFlow(node, config) and
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap = TAccessPathNil(node.getDataFlowType())
     or
@@ -3119,10 +3076,17 @@ private newtype TPathNode =
     )
   } or
   TPathNodeSink(NodeEx node, Configuration config) {
-    exists(PathNodeMid sink |
-      sink.isAtSink() and
-      node = sink.getNodeEx() and
-      config = sink.getConfiguration()
+    sinkNode(node, pragma[only_bind_into](config)) and
+    Stage4::revFlow(node, pragma[only_bind_into](config)) and
+    (
+      // A sink that is also a source ...
+      sourceNode(node, config)
+      or
+      // ... or a sink that can be reached from a source
+      exists(PathNodeMid mid |
+        pathStep(mid, node, _, _, TAccessPathNil(_)) and
+        pragma[only_bind_into](config) = mid.getConfiguration()
+      )
     )
   }
 
@@ -3439,46 +3403,22 @@ private class PathNodeMid extends PathNodeImpl, TPathNodeMid {
     // an intermediate step to another intermediate node
     result = this.getSuccMid()
     or
-    // a final step to a sink
-    result = this.getSuccMid().projectToSink()
+    // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
+    exists(PathNodeMid mid, PathNodeSink sink |
+      mid = this.getSuccMid() and
+      mid.getNodeEx() = sink.getNodeEx() and
+      mid.getAp() instanceof AccessPathNil and
+      sink.getConfiguration() = unbindConf(mid.getConfiguration()) and
+      result = sink
+    )
   }
 
   override predicate isSource() {
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap instanceof AccessPathNil
   }
-
-  predicate isAtSink() {
-    sinkNode(node, config) and
-    ap instanceof AccessPathNil and
-    if hasSinkCallCtx(config)
-    then
-      // For `FeatureHasSinkCallContext` the condition `cc instanceof CallContextNoCall`
-      // is exactly what we need to check. This also implies
-      // `sc instanceof SummaryCtxNone`.
-      // For `FeatureEqualSourceSinkCallContext` the initial call context was
-      // set to `CallContextSomeCall` and jumps are disallowed, so
-      // `cc instanceof CallContextNoCall` never holds. On the other hand,
-      // in this case there's never any need to enter a call except to identify
-      // a summary, so the condition in `pathIntoCallable` enforces this, which
-      // means that `sc instanceof SummaryCtxNone` holds if and only if we are
-      // in the call context of the source.
-      sc instanceof SummaryCtxNone or
-      cc instanceof CallContextNoCall
-    else any()
-  }
-
-  PathNodeSink projectToSink() {
-    this.isAtSink() and
-    result.getNodeEx() = node and
-    result.getConfiguration() = unbindConf(config)
-  }
 }
 
 /**
@@ -3632,7 +3572,7 @@ private predicate pathIntoArg(
   )
 }
 
-pragma[nomagic]
+pragma[noinline]
 private predicate parameterCand(
   DataFlowCallable callable, int i, AccessPathApprox apa, Configuration config
 ) {
@@ -3673,11 +3613,7 @@ private predicate pathIntoCallable(
       sc = TSummaryCtxSome(p, ap)
       or
       not exists(TSummaryCtxSome(p, ap)) and
-      sc = TSummaryCtxNone() and
-      // When the call contexts of source and sink needs to match then there's
-      // never any reason to enter a callable except to find a summary. See also
-      // the comment in `PathNodeMid::isAtSink`.
-      not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+      sc = TSummaryCtxNone()
     )
   |
     if recordDataFlowCallSite(call, callable)
@@ -3740,14 +3676,13 @@ private module Subpaths {
    */
   pragma[nomagic]
   private predicate subpaths01(
-    PathNodeImpl arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
+    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
     exists(Configuration config |
       pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
       pathIntoCallable(arg, par, _, innercc, sc, _, config) and
-      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config)) and
-      not arg.isHidden()
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
     )
   }
 
@@ -3781,17 +3716,8 @@ private module Subpaths {
       innercc = ret.getCallContext() and
       sc = ret.getSummaryCtx() and
       ret.getConfiguration() = unbindConf(getPathNodeConf(arg)) and
-      apout = ret.getAp()
-    )
-  }
-
-  private PathNodeImpl localStepToHidden(PathNodeImpl n) {
-    n.getASuccessorImpl() = result and
-    result.isHidden() and
-    exists(NodeEx n1, NodeEx n2 | n1 = n.getNodeEx() and n2 = result.getNodeEx() |
-      localFlowBigStep(n1, n2, _, _, _, _) or
-      store(n1, _, n2, _, _) or
-      read(n1, _, n2, _)
+      apout = ret.getAp() and
+      not ret.isHidden()
     )
   }
 
@@ -3800,12 +3726,11 @@ private module Subpaths {
    * a subpath between `par` and `ret` with the connecting edges `arg -> par` and
    * `ret -> out` is summarized as the edge `arg -> out`.
    */
-  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeImpl ret, PathNodeMid out) {
+  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
       pragma[only_bind_into](arg).getASuccessor() = par and
       pragma[only_bind_into](arg).getASuccessor() = out and
-      subpaths03(arg, p, localStepToHidden*(ret), o, apout) and
-      not ret.isHidden() and
+      subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and
       out.getAp() = apout

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

@@ -3,12 +3,6 @@ private import DataFlowUtil
 private import DataFlowDispatch
 private import FlowVar
 
-/** 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, int pos) { p.isParameterOf(c, pos) }
-
 /** Gets the instance argument of a non-static call. */
 private Node getInstanceArgument(Call call) {
   result.asExpr() = call.getQualifier()

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()
+    result = this.getParent().(Stmt)
     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.isStatic()
+      v.(Variable).isStatic()
       or
       v instanceof GlobalOrNamespaceVariable
     )

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

@@ -474,24 +474,6 @@ 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.asExpr() or
-        result = node.asParameter()
-      )
-      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/DataFlowImpl.qll

@@ -10,7 +10,6 @@
 private import DataFlowImplCommon
 private import DataFlowImplSpecific::Private
 import DataFlowImplSpecific::Public
-import DataFlowImplCommonPublic
 
 /**
  * A configuration of interprocedural data flow analysis. This defines
@@ -95,22 +94,6 @@ abstract class Configuration extends string {
    */
   int fieldFlowBranchLimit() { result = 2 }
 
-  /**
-   * Gets a data flow configuration feature to add restrictions to the set of
-   * valid flow paths.
-   *
-   * - `FeatureHasSourceCallContext`:
-   *    Assume that sources have some existing call context to disallow
-   *    conflicting return-flow directly following the source.
-   * - `FeatureHasSinkCallContext`:
-   *    Assume that sinks have some existing call context to disallow
-   *    conflicting argument-to-parameter flow directly preceding the sink.
-   * - `FeatureEqualSourceSinkCallContext`:
-   *    Implies both of the above and additionally ensures that the entire flow
-   *    path preserves the call context.
-   */
-  FlowFeature getAFeature() { none() }
-
   /**
    * Holds if data may flow from `source` to `sink` for this configuration.
    */
@@ -366,8 +349,7 @@ private predicate jumpStep(NodeEx node1, NodeEx node2, Configuration config) {
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -383,8 +365,7 @@ private predicate additionalJumpStep(NodeEx node1, NodeEx node2, Configuration c
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -420,20 +401,6 @@ private predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx a
  */
 private predicate useFieldFlow(Configuration config) { config.fieldFlowBranchLimit() >= 1 }
 
-private predicate hasSourceCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSourceCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
-private predicate hasSinkCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSinkCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
 private module Stage1 {
   class ApApprox = Unit;
 
@@ -454,7 +421,7 @@ private module Stage1 {
     not fullBarrier(node, config) and
     (
       sourceNode(node, config) and
-      if hasSourceCallCtx(config) then cc = true else cc = false
+      cc = false
       or
       exists(NodeEx mid |
         fwdFlow(mid, cc, config) and
@@ -584,7 +551,7 @@ private module Stage1 {
   private predicate revFlow0(NodeEx node, boolean toReturn, Configuration config) {
     fwdFlow(node, config) and
     sinkNode(node, config) and
-    if hasSinkCallCtx(config) then toReturn = true else toReturn = false
+    toReturn = false
     or
     exists(NodeEx mid |
       localFlowStep(node, mid, config) and
@@ -970,8 +937,6 @@ private module Stage2 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1039,7 +1004,7 @@ private module Stage2 {
   predicate fwdFlow(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1250,7 +1215,7 @@ private module Stage2 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -1651,8 +1616,6 @@ private module Stage3 {
 
   Cc ccNone() { result = false }
 
-  CcCall ccSomeCall() { result = true }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1734,7 +1697,7 @@ private module Stage3 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1945,7 +1908,7 @@ private module Stage3 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -2403,8 +2366,6 @@ private module Stage4 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = LocalCallContext;
 
   bindingset[call, c, outercc]
@@ -2500,7 +2461,7 @@ private module Stage4 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -2711,7 +2672,7 @@ private module Stage4 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -3103,11 +3064,7 @@ private newtype TPathNode =
     // A PathNode is introduced by a source ...
     Stage4::revFlow(node, config) and
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap = TAccessPathNil(node.getDataFlowType())
     or
@@ -3119,10 +3076,17 @@ private newtype TPathNode =
     )
   } or
   TPathNodeSink(NodeEx node, Configuration config) {
-    exists(PathNodeMid sink |
-      sink.isAtSink() and
-      node = sink.getNodeEx() and
-      config = sink.getConfiguration()
+    sinkNode(node, pragma[only_bind_into](config)) and
+    Stage4::revFlow(node, pragma[only_bind_into](config)) and
+    (
+      // A sink that is also a source ...
+      sourceNode(node, config)
+      or
+      // ... or a sink that can be reached from a source
+      exists(PathNodeMid mid |
+        pathStep(mid, node, _, _, TAccessPathNil(_)) and
+        pragma[only_bind_into](config) = mid.getConfiguration()
+      )
     )
   }
 
@@ -3439,46 +3403,22 @@ private class PathNodeMid extends PathNodeImpl, TPathNodeMid {
     // an intermediate step to another intermediate node
     result = this.getSuccMid()
     or
-    // a final step to a sink
-    result = this.getSuccMid().projectToSink()
+    // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
+    exists(PathNodeMid mid, PathNodeSink sink |
+      mid = this.getSuccMid() and
+      mid.getNodeEx() = sink.getNodeEx() and
+      mid.getAp() instanceof AccessPathNil and
+      sink.getConfiguration() = unbindConf(mid.getConfiguration()) and
+      result = sink
+    )
   }
 
   override predicate isSource() {
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap instanceof AccessPathNil
   }
-
-  predicate isAtSink() {
-    sinkNode(node, config) and
-    ap instanceof AccessPathNil and
-    if hasSinkCallCtx(config)
-    then
-      // For `FeatureHasSinkCallContext` the condition `cc instanceof CallContextNoCall`
-      // is exactly what we need to check. This also implies
-      // `sc instanceof SummaryCtxNone`.
-      // For `FeatureEqualSourceSinkCallContext` the initial call context was
-      // set to `CallContextSomeCall` and jumps are disallowed, so
-      // `cc instanceof CallContextNoCall` never holds. On the other hand,
-      // in this case there's never any need to enter a call except to identify
-      // a summary, so the condition in `pathIntoCallable` enforces this, which
-      // means that `sc instanceof SummaryCtxNone` holds if and only if we are
-      // in the call context of the source.
-      sc instanceof SummaryCtxNone or
-      cc instanceof CallContextNoCall
-    else any()
-  }
-
-  PathNodeSink projectToSink() {
-    this.isAtSink() and
-    result.getNodeEx() = node and
-    result.getConfiguration() = unbindConf(config)
-  }
 }
 
 /**
@@ -3632,7 +3572,7 @@ private predicate pathIntoArg(
   )
 }
 
-pragma[nomagic]
+pragma[noinline]
 private predicate parameterCand(
   DataFlowCallable callable, int i, AccessPathApprox apa, Configuration config
 ) {
@@ -3673,11 +3613,7 @@ private predicate pathIntoCallable(
       sc = TSummaryCtxSome(p, ap)
       or
       not exists(TSummaryCtxSome(p, ap)) and
-      sc = TSummaryCtxNone() and
-      // When the call contexts of source and sink needs to match then there's
-      // never any reason to enter a callable except to find a summary. See also
-      // the comment in `PathNodeMid::isAtSink`.
-      not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+      sc = TSummaryCtxNone()
     )
   |
     if recordDataFlowCallSite(call, callable)
@@ -3740,14 +3676,13 @@ private module Subpaths {
    */
   pragma[nomagic]
   private predicate subpaths01(
-    PathNodeImpl arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
+    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
     exists(Configuration config |
       pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
       pathIntoCallable(arg, par, _, innercc, sc, _, config) and
-      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config)) and
-      not arg.isHidden()
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
     )
   }
 
@@ -3781,17 +3716,8 @@ private module Subpaths {
       innercc = ret.getCallContext() and
       sc = ret.getSummaryCtx() and
       ret.getConfiguration() = unbindConf(getPathNodeConf(arg)) and
-      apout = ret.getAp()
-    )
-  }
-
-  private PathNodeImpl localStepToHidden(PathNodeImpl n) {
-    n.getASuccessorImpl() = result and
-    result.isHidden() and
-    exists(NodeEx n1, NodeEx n2 | n1 = n.getNodeEx() and n2 = result.getNodeEx() |
-      localFlowBigStep(n1, n2, _, _, _, _) or
-      store(n1, _, n2, _, _) or
-      read(n1, _, n2, _)
+      apout = ret.getAp() and
+      not ret.isHidden()
     )
   }
 
@@ -3800,12 +3726,11 @@ private module Subpaths {
    * a subpath between `par` and `ret` with the connecting edges `arg -> par` and
    * `ret -> out` is summarized as the edge `arg -> out`.
    */
-  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeImpl ret, PathNodeMid out) {
+  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
       pragma[only_bind_into](arg).getASuccessor() = par and
       pragma[only_bind_into](arg).getASuccessor() = out and
-      subpaths03(arg, p, localStepToHidden*(ret), o, apout) and
-      not ret.isHidden() and
+      subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and
       out.getAp() = apout

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

@@ -10,7 +10,6 @@
 private import DataFlowImplCommon
 private import DataFlowImplSpecific::Private
 import DataFlowImplSpecific::Public
-import DataFlowImplCommonPublic
 
 /**
  * A configuration of interprocedural data flow analysis. This defines
@@ -95,22 +94,6 @@ abstract class Configuration extends string {
    */
   int fieldFlowBranchLimit() { result = 2 }
 
-  /**
-   * Gets a data flow configuration feature to add restrictions to the set of
-   * valid flow paths.
-   *
-   * - `FeatureHasSourceCallContext`:
-   *    Assume that sources have some existing call context to disallow
-   *    conflicting return-flow directly following the source.
-   * - `FeatureHasSinkCallContext`:
-   *    Assume that sinks have some existing call context to disallow
-   *    conflicting argument-to-parameter flow directly preceding the sink.
-   * - `FeatureEqualSourceSinkCallContext`:
-   *    Implies both of the above and additionally ensures that the entire flow
-   *    path preserves the call context.
-   */
-  FlowFeature getAFeature() { none() }
-
   /**
    * Holds if data may flow from `source` to `sink` for this configuration.
    */
@@ -366,8 +349,7 @@ private predicate jumpStep(NodeEx node1, NodeEx node2, Configuration config) {
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -383,8 +365,7 @@ private predicate additionalJumpStep(NodeEx node1, NodeEx node2, Configuration c
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -420,20 +401,6 @@ private predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx a
  */
 private predicate useFieldFlow(Configuration config) { config.fieldFlowBranchLimit() >= 1 }
 
-private predicate hasSourceCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSourceCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
-private predicate hasSinkCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSinkCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
 private module Stage1 {
   class ApApprox = Unit;
 
@@ -454,7 +421,7 @@ private module Stage1 {
     not fullBarrier(node, config) and
     (
       sourceNode(node, config) and
-      if hasSourceCallCtx(config) then cc = true else cc = false
+      cc = false
       or
       exists(NodeEx mid |
         fwdFlow(mid, cc, config) and
@@ -584,7 +551,7 @@ private module Stage1 {
   private predicate revFlow0(NodeEx node, boolean toReturn, Configuration config) {
     fwdFlow(node, config) and
     sinkNode(node, config) and
-    if hasSinkCallCtx(config) then toReturn = true else toReturn = false
+    toReturn = false
     or
     exists(NodeEx mid |
       localFlowStep(node, mid, config) and
@@ -970,8 +937,6 @@ private module Stage2 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1039,7 +1004,7 @@ private module Stage2 {
   predicate fwdFlow(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1250,7 +1215,7 @@ private module Stage2 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -1651,8 +1616,6 @@ private module Stage3 {
 
   Cc ccNone() { result = false }
 
-  CcCall ccSomeCall() { result = true }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1734,7 +1697,7 @@ private module Stage3 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1945,7 +1908,7 @@ private module Stage3 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -2403,8 +2366,6 @@ private module Stage4 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = LocalCallContext;
 
   bindingset[call, c, outercc]
@@ -2500,7 +2461,7 @@ private module Stage4 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -2711,7 +2672,7 @@ private module Stage4 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -3103,11 +3064,7 @@ private newtype TPathNode =
     // A PathNode is introduced by a source ...
     Stage4::revFlow(node, config) and
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap = TAccessPathNil(node.getDataFlowType())
     or
@@ -3119,10 +3076,17 @@ private newtype TPathNode =
     )
   } or
   TPathNodeSink(NodeEx node, Configuration config) {
-    exists(PathNodeMid sink |
-      sink.isAtSink() and
-      node = sink.getNodeEx() and
-      config = sink.getConfiguration()
+    sinkNode(node, pragma[only_bind_into](config)) and
+    Stage4::revFlow(node, pragma[only_bind_into](config)) and
+    (
+      // A sink that is also a source ...
+      sourceNode(node, config)
+      or
+      // ... or a sink that can be reached from a source
+      exists(PathNodeMid mid |
+        pathStep(mid, node, _, _, TAccessPathNil(_)) and
+        pragma[only_bind_into](config) = mid.getConfiguration()
+      )
     )
   }
 
@@ -3439,46 +3403,22 @@ private class PathNodeMid extends PathNodeImpl, TPathNodeMid {
     // an intermediate step to another intermediate node
     result = this.getSuccMid()
     or
-    // a final step to a sink
-    result = this.getSuccMid().projectToSink()
+    // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
+    exists(PathNodeMid mid, PathNodeSink sink |
+      mid = this.getSuccMid() and
+      mid.getNodeEx() = sink.getNodeEx() and
+      mid.getAp() instanceof AccessPathNil and
+      sink.getConfiguration() = unbindConf(mid.getConfiguration()) and
+      result = sink
+    )
   }
 
   override predicate isSource() {
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap instanceof AccessPathNil
   }
-
-  predicate isAtSink() {
-    sinkNode(node, config) and
-    ap instanceof AccessPathNil and
-    if hasSinkCallCtx(config)
-    then
-      // For `FeatureHasSinkCallContext` the condition `cc instanceof CallContextNoCall`
-      // is exactly what we need to check. This also implies
-      // `sc instanceof SummaryCtxNone`.
-      // For `FeatureEqualSourceSinkCallContext` the initial call context was
-      // set to `CallContextSomeCall` and jumps are disallowed, so
-      // `cc instanceof CallContextNoCall` never holds. On the other hand,
-      // in this case there's never any need to enter a call except to identify
-      // a summary, so the condition in `pathIntoCallable` enforces this, which
-      // means that `sc instanceof SummaryCtxNone` holds if and only if we are
-      // in the call context of the source.
-      sc instanceof SummaryCtxNone or
-      cc instanceof CallContextNoCall
-    else any()
-  }
-
-  PathNodeSink projectToSink() {
-    this.isAtSink() and
-    result.getNodeEx() = node and
-    result.getConfiguration() = unbindConf(config)
-  }
 }
 
 /**
@@ -3632,7 +3572,7 @@ private predicate pathIntoArg(
   )
 }
 
-pragma[nomagic]
+pragma[noinline]
 private predicate parameterCand(
   DataFlowCallable callable, int i, AccessPathApprox apa, Configuration config
 ) {
@@ -3673,11 +3613,7 @@ private predicate pathIntoCallable(
       sc = TSummaryCtxSome(p, ap)
       or
       not exists(TSummaryCtxSome(p, ap)) and
-      sc = TSummaryCtxNone() and
-      // When the call contexts of source and sink needs to match then there's
-      // never any reason to enter a callable except to find a summary. See also
-      // the comment in `PathNodeMid::isAtSink`.
-      not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+      sc = TSummaryCtxNone()
     )
   |
     if recordDataFlowCallSite(call, callable)
@@ -3740,14 +3676,13 @@ private module Subpaths {
    */
   pragma[nomagic]
   private predicate subpaths01(
-    PathNodeImpl arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
+    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
     exists(Configuration config |
       pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
       pathIntoCallable(arg, par, _, innercc, sc, _, config) and
-      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config)) and
-      not arg.isHidden()
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
     )
   }
 
@@ -3781,17 +3716,8 @@ private module Subpaths {
       innercc = ret.getCallContext() and
       sc = ret.getSummaryCtx() and
       ret.getConfiguration() = unbindConf(getPathNodeConf(arg)) and
-      apout = ret.getAp()
-    )
-  }
-
-  private PathNodeImpl localStepToHidden(PathNodeImpl n) {
-    n.getASuccessorImpl() = result and
-    result.isHidden() and
-    exists(NodeEx n1, NodeEx n2 | n1 = n.getNodeEx() and n2 = result.getNodeEx() |
-      localFlowBigStep(n1, n2, _, _, _, _) or
-      store(n1, _, n2, _, _) or
-      read(n1, _, n2, _)
+      apout = ret.getAp() and
+      not ret.isHidden()
     )
   }
 
@@ -3800,12 +3726,11 @@ private module Subpaths {
    * a subpath between `par` and `ret` with the connecting edges `arg -> par` and
    * `ret -> out` is summarized as the edge `arg -> out`.
    */
-  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeImpl ret, PathNodeMid out) {
+  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
       pragma[only_bind_into](arg).getASuccessor() = par and
       pragma[only_bind_into](arg).getASuccessor() = out and
-      subpaths03(arg, p, localStepToHidden*(ret), o, apout) and
-      not ret.isHidden() and
+      subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and
       out.getAp() = apout

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

@@ -10,7 +10,6 @@
 private import DataFlowImplCommon
 private import DataFlowImplSpecific::Private
 import DataFlowImplSpecific::Public
-import DataFlowImplCommonPublic
 
 /**
  * A configuration of interprocedural data flow analysis. This defines
@@ -95,22 +94,6 @@ abstract class Configuration extends string {
    */
   int fieldFlowBranchLimit() { result = 2 }
 
-  /**
-   * Gets a data flow configuration feature to add restrictions to the set of
-   * valid flow paths.
-   *
-   * - `FeatureHasSourceCallContext`:
-   *    Assume that sources have some existing call context to disallow
-   *    conflicting return-flow directly following the source.
-   * - `FeatureHasSinkCallContext`:
-   *    Assume that sinks have some existing call context to disallow
-   *    conflicting argument-to-parameter flow directly preceding the sink.
-   * - `FeatureEqualSourceSinkCallContext`:
-   *    Implies both of the above and additionally ensures that the entire flow
-   *    path preserves the call context.
-   */
-  FlowFeature getAFeature() { none() }
-
   /**
    * Holds if data may flow from `source` to `sink` for this configuration.
    */
@@ -366,8 +349,7 @@ private predicate jumpStep(NodeEx node1, NodeEx node2, Configuration config) {
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -383,8 +365,7 @@ private predicate additionalJumpStep(NodeEx node1, NodeEx node2, Configuration c
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -420,20 +401,6 @@ private predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx a
  */
 private predicate useFieldFlow(Configuration config) { config.fieldFlowBranchLimit() >= 1 }
 
-private predicate hasSourceCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSourceCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
-private predicate hasSinkCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSinkCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
 private module Stage1 {
   class ApApprox = Unit;
 
@@ -454,7 +421,7 @@ private module Stage1 {
     not fullBarrier(node, config) and
     (
       sourceNode(node, config) and
-      if hasSourceCallCtx(config) then cc = true else cc = false
+      cc = false
       or
       exists(NodeEx mid |
         fwdFlow(mid, cc, config) and
@@ -584,7 +551,7 @@ private module Stage1 {
   private predicate revFlow0(NodeEx node, boolean toReturn, Configuration config) {
     fwdFlow(node, config) and
     sinkNode(node, config) and
-    if hasSinkCallCtx(config) then toReturn = true else toReturn = false
+    toReturn = false
     or
     exists(NodeEx mid |
       localFlowStep(node, mid, config) and
@@ -970,8 +937,6 @@ private module Stage2 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1039,7 +1004,7 @@ private module Stage2 {
   predicate fwdFlow(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1250,7 +1215,7 @@ private module Stage2 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -1651,8 +1616,6 @@ private module Stage3 {
 
   Cc ccNone() { result = false }
 
-  CcCall ccSomeCall() { result = true }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1734,7 +1697,7 @@ private module Stage3 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1945,7 +1908,7 @@ private module Stage3 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -2403,8 +2366,6 @@ private module Stage4 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = LocalCallContext;
 
   bindingset[call, c, outercc]
@@ -2500,7 +2461,7 @@ private module Stage4 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -2711,7 +2672,7 @@ private module Stage4 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -3103,11 +3064,7 @@ private newtype TPathNode =
     // A PathNode is introduced by a source ...
     Stage4::revFlow(node, config) and
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap = TAccessPathNil(node.getDataFlowType())
     or
@@ -3119,10 +3076,17 @@ private newtype TPathNode =
     )
   } or
   TPathNodeSink(NodeEx node, Configuration config) {
-    exists(PathNodeMid sink |
-      sink.isAtSink() and
-      node = sink.getNodeEx() and
-      config = sink.getConfiguration()
+    sinkNode(node, pragma[only_bind_into](config)) and
+    Stage4::revFlow(node, pragma[only_bind_into](config)) and
+    (
+      // A sink that is also a source ...
+      sourceNode(node, config)
+      or
+      // ... or a sink that can be reached from a source
+      exists(PathNodeMid mid |
+        pathStep(mid, node, _, _, TAccessPathNil(_)) and
+        pragma[only_bind_into](config) = mid.getConfiguration()
+      )
     )
   }
 
@@ -3439,46 +3403,22 @@ private class PathNodeMid extends PathNodeImpl, TPathNodeMid {
     // an intermediate step to another intermediate node
     result = this.getSuccMid()
     or
-    // a final step to a sink
-    result = this.getSuccMid().projectToSink()
+    // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
+    exists(PathNodeMid mid, PathNodeSink sink |
+      mid = this.getSuccMid() and
+      mid.getNodeEx() = sink.getNodeEx() and
+      mid.getAp() instanceof AccessPathNil and
+      sink.getConfiguration() = unbindConf(mid.getConfiguration()) and
+      result = sink
+    )
   }
 
   override predicate isSource() {
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap instanceof AccessPathNil
   }
-
-  predicate isAtSink() {
-    sinkNode(node, config) and
-    ap instanceof AccessPathNil and
-    if hasSinkCallCtx(config)
-    then
-      // For `FeatureHasSinkCallContext` the condition `cc instanceof CallContextNoCall`
-      // is exactly what we need to check. This also implies
-      // `sc instanceof SummaryCtxNone`.
-      // For `FeatureEqualSourceSinkCallContext` the initial call context was
-      // set to `CallContextSomeCall` and jumps are disallowed, so
-      // `cc instanceof CallContextNoCall` never holds. On the other hand,
-      // in this case there's never any need to enter a call except to identify
-      // a summary, so the condition in `pathIntoCallable` enforces this, which
-      // means that `sc instanceof SummaryCtxNone` holds if and only if we are
-      // in the call context of the source.
-      sc instanceof SummaryCtxNone or
-      cc instanceof CallContextNoCall
-    else any()
-  }
-
-  PathNodeSink projectToSink() {
-    this.isAtSink() and
-    result.getNodeEx() = node and
-    result.getConfiguration() = unbindConf(config)
-  }
 }
 
 /**
@@ -3632,7 +3572,7 @@ private predicate pathIntoArg(
   )
 }
 
-pragma[nomagic]
+pragma[noinline]
 private predicate parameterCand(
   DataFlowCallable callable, int i, AccessPathApprox apa, Configuration config
 ) {
@@ -3673,11 +3613,7 @@ private predicate pathIntoCallable(
       sc = TSummaryCtxSome(p, ap)
       or
       not exists(TSummaryCtxSome(p, ap)) and
-      sc = TSummaryCtxNone() and
-      // When the call contexts of source and sink needs to match then there's
-      // never any reason to enter a callable except to find a summary. See also
-      // the comment in `PathNodeMid::isAtSink`.
-      not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+      sc = TSummaryCtxNone()
     )
   |
     if recordDataFlowCallSite(call, callable)
@@ -3740,14 +3676,13 @@ private module Subpaths {
    */
   pragma[nomagic]
   private predicate subpaths01(
-    PathNodeImpl arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
+    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
     exists(Configuration config |
       pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
       pathIntoCallable(arg, par, _, innercc, sc, _, config) and
-      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config)) and
-      not arg.isHidden()
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
     )
   }
 
@@ -3781,17 +3716,8 @@ private module Subpaths {
       innercc = ret.getCallContext() and
       sc = ret.getSummaryCtx() and
       ret.getConfiguration() = unbindConf(getPathNodeConf(arg)) and
-      apout = ret.getAp()
-    )
-  }
-
-  private PathNodeImpl localStepToHidden(PathNodeImpl n) {
-    n.getASuccessorImpl() = result and
-    result.isHidden() and
-    exists(NodeEx n1, NodeEx n2 | n1 = n.getNodeEx() and n2 = result.getNodeEx() |
-      localFlowBigStep(n1, n2, _, _, _, _) or
-      store(n1, _, n2, _, _) or
-      read(n1, _, n2, _)
+      apout = ret.getAp() and
+      not ret.isHidden()
     )
   }
 
@@ -3800,12 +3726,11 @@ private module Subpaths {
    * a subpath between `par` and `ret` with the connecting edges `arg -> par` and
    * `ret -> out` is summarized as the edge `arg -> out`.
    */
-  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeImpl ret, PathNodeMid out) {
+  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
       pragma[only_bind_into](arg).getASuccessor() = par and
       pragma[only_bind_into](arg).getASuccessor() = out and
-      subpaths03(arg, p, localStepToHidden*(ret), o, apout) and
-      not ret.isHidden() and
+      subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and
       out.getAp() = apout

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

@@ -10,7 +10,6 @@
 private import DataFlowImplCommon
 private import DataFlowImplSpecific::Private
 import DataFlowImplSpecific::Public
-import DataFlowImplCommonPublic
 
 /**
  * A configuration of interprocedural data flow analysis. This defines
@@ -95,22 +94,6 @@ abstract class Configuration extends string {
    */
   int fieldFlowBranchLimit() { result = 2 }
 
-  /**
-   * Gets a data flow configuration feature to add restrictions to the set of
-   * valid flow paths.
-   *
-   * - `FeatureHasSourceCallContext`:
-   *    Assume that sources have some existing call context to disallow
-   *    conflicting return-flow directly following the source.
-   * - `FeatureHasSinkCallContext`:
-   *    Assume that sinks have some existing call context to disallow
-   *    conflicting argument-to-parameter flow directly preceding the sink.
-   * - `FeatureEqualSourceSinkCallContext`:
-   *    Implies both of the above and additionally ensures that the entire flow
-   *    path preserves the call context.
-   */
-  FlowFeature getAFeature() { none() }
-
   /**
    * Holds if data may flow from `source` to `sink` for this configuration.
    */
@@ -366,8 +349,7 @@ private predicate jumpStep(NodeEx node1, NodeEx node2, Configuration config) {
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -383,8 +365,7 @@ private predicate additionalJumpStep(NodeEx node1, NodeEx node2, Configuration c
     not outBarrier(node1, config) and
     not inBarrier(node2, config) and
     not fullBarrier(node1, config) and
-    not fullBarrier(node2, config) and
-    not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+    not fullBarrier(node2, config)
   )
 }
 
@@ -420,20 +401,6 @@ private predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx a
  */
 private predicate useFieldFlow(Configuration config) { config.fieldFlowBranchLimit() >= 1 }
 
-private predicate hasSourceCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSourceCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
-private predicate hasSinkCallCtx(Configuration config) {
-  exists(FlowFeature feature | feature = config.getAFeature() |
-    feature instanceof FeatureHasSinkCallContext or
-    feature instanceof FeatureEqualSourceSinkCallContext
-  )
-}
-
 private module Stage1 {
   class ApApprox = Unit;
 
@@ -454,7 +421,7 @@ private module Stage1 {
     not fullBarrier(node, config) and
     (
       sourceNode(node, config) and
-      if hasSourceCallCtx(config) then cc = true else cc = false
+      cc = false
       or
       exists(NodeEx mid |
         fwdFlow(mid, cc, config) and
@@ -584,7 +551,7 @@ private module Stage1 {
   private predicate revFlow0(NodeEx node, boolean toReturn, Configuration config) {
     fwdFlow(node, config) and
     sinkNode(node, config) and
-    if hasSinkCallCtx(config) then toReturn = true else toReturn = false
+    toReturn = false
     or
     exists(NodeEx mid |
       localFlowStep(node, mid, config) and
@@ -970,8 +937,6 @@ private module Stage2 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1039,7 +1004,7 @@ private module Stage2 {
   predicate fwdFlow(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1250,7 +1215,7 @@ private module Stage2 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -1651,8 +1616,6 @@ private module Stage3 {
 
   Cc ccNone() { result = false }
 
-  CcCall ccSomeCall() { result = true }
-
   private class LocalCc = Unit;
 
   bindingset[call, c, outercc]
@@ -1734,7 +1697,7 @@ private module Stage3 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -1945,7 +1908,7 @@ private module Stage3 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -2403,8 +2366,6 @@ private module Stage4 {
 
   Cc ccNone() { result instanceof CallContextAny }
 
-  CcCall ccSomeCall() { result instanceof CallContextSomeCall }
-
   private class LocalCc = LocalCallContext;
 
   bindingset[call, c, outercc]
@@ -2500,7 +2461,7 @@ private module Stage4 {
   private predicate fwdFlow0(NodeEx node, Cc cc, ApOption argAp, Ap ap, Configuration config) {
     flowCand(node, _, config) and
     sourceNode(node, config) and
-    (if hasSourceCallCtx(config) then cc = ccSomeCall() else cc = ccNone()) and
+    cc = ccNone() and
     argAp = apNone() and
     ap = getApNil(node)
     or
@@ -2711,7 +2672,7 @@ private module Stage4 {
   ) {
     fwdFlow(node, _, _, ap, config) and
     sinkNode(node, config) and
-    (if hasSinkCallCtx(config) then toReturn = true else toReturn = false) and
+    toReturn = false and
     returnAp = apNone() and
     ap instanceof ApNil
     or
@@ -3103,11 +3064,7 @@ private newtype TPathNode =
     // A PathNode is introduced by a source ...
     Stage4::revFlow(node, config) and
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap = TAccessPathNil(node.getDataFlowType())
     or
@@ -3119,10 +3076,17 @@ private newtype TPathNode =
     )
   } or
   TPathNodeSink(NodeEx node, Configuration config) {
-    exists(PathNodeMid sink |
-      sink.isAtSink() and
-      node = sink.getNodeEx() and
-      config = sink.getConfiguration()
+    sinkNode(node, pragma[only_bind_into](config)) and
+    Stage4::revFlow(node, pragma[only_bind_into](config)) and
+    (
+      // A sink that is also a source ...
+      sourceNode(node, config)
+      or
+      // ... or a sink that can be reached from a source
+      exists(PathNodeMid mid |
+        pathStep(mid, node, _, _, TAccessPathNil(_)) and
+        pragma[only_bind_into](config) = mid.getConfiguration()
+      )
     )
   }
 
@@ -3439,46 +3403,22 @@ private class PathNodeMid extends PathNodeImpl, TPathNodeMid {
     // an intermediate step to another intermediate node
     result = this.getSuccMid()
     or
-    // a final step to a sink
-    result = this.getSuccMid().projectToSink()
+    // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
+    exists(PathNodeMid mid, PathNodeSink sink |
+      mid = this.getSuccMid() and
+      mid.getNodeEx() = sink.getNodeEx() and
+      mid.getAp() instanceof AccessPathNil and
+      sink.getConfiguration() = unbindConf(mid.getConfiguration()) and
+      result = sink
+    )
   }
 
   override predicate isSource() {
     sourceNode(node, config) and
-    (
-      if hasSourceCallCtx(config)
-      then cc instanceof CallContextSomeCall
-      else cc instanceof CallContextAny
-    ) and
+    cc instanceof CallContextAny and
     sc instanceof SummaryCtxNone and
     ap instanceof AccessPathNil
   }
-
-  predicate isAtSink() {
-    sinkNode(node, config) and
-    ap instanceof AccessPathNil and
-    if hasSinkCallCtx(config)
-    then
-      // For `FeatureHasSinkCallContext` the condition `cc instanceof CallContextNoCall`
-      // is exactly what we need to check. This also implies
-      // `sc instanceof SummaryCtxNone`.
-      // For `FeatureEqualSourceSinkCallContext` the initial call context was
-      // set to `CallContextSomeCall` and jumps are disallowed, so
-      // `cc instanceof CallContextNoCall` never holds. On the other hand,
-      // in this case there's never any need to enter a call except to identify
-      // a summary, so the condition in `pathIntoCallable` enforces this, which
-      // means that `sc instanceof SummaryCtxNone` holds if and only if we are
-      // in the call context of the source.
-      sc instanceof SummaryCtxNone or
-      cc instanceof CallContextNoCall
-    else any()
-  }
-
-  PathNodeSink projectToSink() {
-    this.isAtSink() and
-    result.getNodeEx() = node and
-    result.getConfiguration() = unbindConf(config)
-  }
 }
 
 /**
@@ -3632,7 +3572,7 @@ private predicate pathIntoArg(
   )
 }
 
-pragma[nomagic]
+pragma[noinline]
 private predicate parameterCand(
   DataFlowCallable callable, int i, AccessPathApprox apa, Configuration config
 ) {
@@ -3673,11 +3613,7 @@ private predicate pathIntoCallable(
       sc = TSummaryCtxSome(p, ap)
       or
       not exists(TSummaryCtxSome(p, ap)) and
-      sc = TSummaryCtxNone() and
-      // When the call contexts of source and sink needs to match then there's
-      // never any reason to enter a callable except to find a summary. See also
-      // the comment in `PathNodeMid::isAtSink`.
-      not config.getAFeature() instanceof FeatureEqualSourceSinkCallContext
+      sc = TSummaryCtxNone()
     )
   |
     if recordDataFlowCallSite(call, callable)
@@ -3740,14 +3676,13 @@ private module Subpaths {
    */
   pragma[nomagic]
   private predicate subpaths01(
-    PathNodeImpl arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
+    PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
     exists(Configuration config |
       pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
       pathIntoCallable(arg, par, _, innercc, sc, _, config) and
-      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config)) and
-      not arg.isHidden()
+      paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _, unbindConf(config))
     )
   }
 
@@ -3781,17 +3716,8 @@ private module Subpaths {
       innercc = ret.getCallContext() and
       sc = ret.getSummaryCtx() and
       ret.getConfiguration() = unbindConf(getPathNodeConf(arg)) and
-      apout = ret.getAp()
-    )
-  }
-
-  private PathNodeImpl localStepToHidden(PathNodeImpl n) {
-    n.getASuccessorImpl() = result and
-    result.isHidden() and
-    exists(NodeEx n1, NodeEx n2 | n1 = n.getNodeEx() and n2 = result.getNodeEx() |
-      localFlowBigStep(n1, n2, _, _, _, _) or
-      store(n1, _, n2, _, _) or
-      read(n1, _, n2, _)
+      apout = ret.getAp() and
+      not ret.isHidden()
     )
   }
 
@@ -3800,12 +3726,11 @@ private module Subpaths {
    * a subpath between `par` and `ret` with the connecting edges `arg -> par` and
    * `ret -> out` is summarized as the edge `arg -> out`.
    */
-  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeImpl ret, PathNodeMid out) {
+  predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
       pragma[only_bind_into](arg).getASuccessor() = par and
       pragma[only_bind_into](arg).getASuccessor() = out and
-      subpaths03(arg, p, localStepToHidden*(ret), o, apout) and
-      not ret.isHidden() and
+      subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and
       out.getAp() = apout

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

@@ -2,42 +2,6 @@ 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.
  *
@@ -287,7 +251,7 @@ private module Cached {
   predicate forceCachingInSameStage() { any() }
 
   cached
-  predicate nodeEnclosingCallable(Node n, DataFlowCallable c) { c = nodeGetEnclosingCallable(n) }
+  predicate nodeEnclosingCallable(Node n, DataFlowCallable c) { c = n.getEnclosingCallable() }
 
   cached
   predicate callEnclosingCallable(DataFlowCall call, DataFlowCallable c) {
@@ -352,7 +316,9 @@ private module Cached {
   }
 
   cached
-  predicate parameterNode(Node p, DataFlowCallable c, int pos) { isParameterNode(p, c, pos) }
+  predicate parameterNode(Node n, DataFlowCallable c, int i) {
+    n.(ParameterNode).isParameterOf(c, i)
+  }
 
   cached
   predicate argumentNode(Node n, DataFlowCall call, int pos) {

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

@@ -31,7 +31,7 @@ module Consistency {
   query predicate uniqueEnclosingCallable(Node n, string msg) {
     exists(int c |
       n instanceof RelevantNode and
-      c = count(nodeGetEnclosingCallable(n)) and
+      c = count(n.getEnclosingCallable()) and
       c != 1 and
       msg = "Node should have one enclosing callable but has " + c + "."
     )
@@ -85,13 +85,13 @@ module Consistency {
   }
 
   query predicate parameterCallable(ParameterNode p, string msg) {
-    exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and
+    exists(DataFlowCallable c | p.isParameterOf(c, _) and c != p.getEnclosingCallable()) and
     msg = "Callable mismatch for parameter."
   }
 
   query predicate localFlowIsLocal(Node n1, Node n2, string msg) {
     simpleLocalFlowStep(n1, n2) and
-    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    n1.getEnclosingCallable() != n2.getEnclosingCallable() and
     msg = "Local flow step does not preserve enclosing callable."
   }
 
@@ -106,7 +106,7 @@ module Consistency {
   query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) {
     isUnreachableInCall(n, call) and
     exists(DataFlowCallable c |
-      c = nodeGetEnclosingCallable(n) and
+      c = n.getEnclosingCallable() and
       not viableCallable(call) = c
     ) and
     msg = "Call context for isUnreachableInCall is inconsistent with call graph."
@@ -120,7 +120,7 @@ module Consistency {
       n.(ArgumentNode).argumentOf(call, _) and
       msg = "ArgumentNode and call does not share enclosing callable."
     ) and
-    nodeGetEnclosingCallable(n) != call.getEnclosingCallable()
+    n.getEnclosingCallable() != call.getEnclosingCallable()
   }
 
   // This predicate helps the compiler forget that in some languages
@@ -151,7 +151,7 @@ module Consistency {
   }
 
   query predicate postIsInSameCallable(PostUpdateNode n, string msg) {
-    nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and
+    n.getEnclosingCallable() != n.getPreUpdateNode().getEnclosingCallable() and
     msg = "PostUpdateNode does not share callable with its pre-update node."
   }
 

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

@@ -3,12 +3,6 @@ private import DataFlowUtil
 private import semmle.code.cpp.ir.IR
 private import DataFlowDispatch
 
-/** 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, int pos) { p.isParameterOf(c, pos) }
-
 /**
  * A data flow node that occurs as the argument of a call and is passed as-is
  * to the callable. Instance arguments (`this` pointer) and read side effects
@@ -193,7 +187,7 @@ predicate jumpStep(Node n1, Node n2) { none() }
  * Thus, `node2` references an object with a field `f` that contains the
  * value of `node1`.
  */
-predicate storeStep(StoreNodeInstr node1, FieldContent f, StoreNodeInstr node2) {
+predicate storeStep(StoreNode node1, FieldContent f, StoreNode node2) {
   exists(FieldAddressInstruction fai |
     node1.getInstruction() = fai and
     node2.getInstruction() = fai.getObjectAddress() and

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

@@ -11,49 +11,10 @@ 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
+private import Ssa 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
@@ -245,7 +206,13 @@ class OperandNode extends Node, TOperandNode {
  * 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 {
+abstract class StoreNode extends Node {
+  /** Gets the underlying instruction, if any. */
+  Instruction getInstruction() { none() }
+
+  /** Gets the underlying operand, if any. */
+  Operand getOperand() { none() }
+
   /** Holds if this node should receive flow from `addr`. */
   abstract predicate flowInto(Instruction addr);
 
@@ -253,7 +220,7 @@ abstract private class StoreNode extends Node {
 
   /** Holds if this `StoreNode` is the root of the address computation used by a store operation. */
   predicate isTerminal() {
-    not exists(this.getInner()) and
+    not exists(this.getAPredecessor()) and
     not storeStep(this, _, _)
   }
 
@@ -265,22 +232,23 @@ abstract private class StoreNode extends Node {
 
   /**
    * Gets the `StoreNode` that computes the address used by this `StoreNode`.
+   * The boolean `readEffect` is `true` if the predecessor is accessed through the
+   * address of a `ReadSideEffectInstruction`.
    */
-  abstract StoreNode getInner();
+  abstract StoreNode getAPredecessor();
 
-  /** The inverse of `StoreNode.getInner`. */
-  final StoreNode getOuter() { result.getInner() = this }
+  /** The inverse of `StoreNode.getAPredecessor`. */
+  final StoreNode getASuccessor() { result.getAPredecessor() = this }
 }
 
-class StoreNodeInstr extends StoreNode, TStoreNodeInstr {
+private 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 Instruction getInstruction() { result = instr }
 
   override Function getFunction() { result = this.getInstruction().getEnclosingFunction() }
 
@@ -296,45 +264,19 @@ class StoreNodeInstr extends StoreNode, TStoreNodeInstr {
     Ssa::explicitWrite(_, result, this.getInstruction())
   }
 
-  override StoreNodeInstr getInner() {
+  override StoreNode getAPredecessor() {
     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 {
+private 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 Operand getOperand() { result = operand }
 
   override Function getFunction() { result = operand.getDef().getEnclosingFunction() }
 
@@ -348,32 +290,7 @@ class StoreNodeOperand extends StoreNode, TStoreNodeOperand {
     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() }
+  override StoreNode getAPredecessor() { operand.getDef() = result.getInstruction() }
 }
 
 /**
@@ -411,20 +328,22 @@ class ReadNode extends Node, TReadNode {
    * 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()) }
+  final ReadNode getAPredecessor() {
+    Ssa::addressFlow(result.getInstruction(), this.getInstruction())
+  }
 
-  /** The inverse of `ReadNode.getInner`. */
-  final ReadNode getOuter() { result.getInner() = this }
+  /** The inverse of `ReadNode.getAPredecessor`. */
+  final ReadNode getASuccessor() { result.getAPredecessor() = 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 exists(this.getASuccessor()) 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 exists(this.getAPredecessor()) and
     not readStep(_, _, this)
   }
 }
@@ -451,17 +370,11 @@ class SsaPhiNode extends Node, TSsaPhiNode {
   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) {
-    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)
+  final predicate hasInputAtRankInBlock(IRBlock block, int rnk) {
+    exists(Ssa::Definition input |
+      Ssa::phiHasInputFromBlock(phi, input, _) and input.definesAt(_, block, rnk)
+    )
   }
 
   override string toString() { result = "Phi" }
@@ -821,6 +734,21 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
   or
   // Adjacent-def-use and adjacent-use-use flow
   adjacentDefUseFlow(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 |
+    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 nodeTo.asInstruction() instanceof ReturnIndirectionInstruction and
+    Ssa::ssaFlow(init, nodeTo)
+  )
 }
 
 private predicate adjacentDefUseFlow(Node nodeFrom, Node nodeTo) {
@@ -870,7 +798,7 @@ private module ReadNodeFlow {
   /** Holds if the read node `nodeTo` should receive flow from the read node `nodeFrom`. */
   predicate flowThrough(ReadNode nodeFrom, ReadNode nodeTo) {
     not readStep(nodeFrom, _, _) and
-    nodeFrom.getOuter() = nodeTo
+    nodeFrom.getASuccessor() = nodeTo
   }
 
   /**
@@ -883,7 +811,7 @@ private module ReadNodeFlow {
     // Use-use flow to another use of the same variable instruction
     Ssa::ssaFlow(nFrom, nodeTo)
     or
-    not exists(nFrom.getInner()) and
+    not exists(nFrom.getAPredecessor()) and
     exists(Node store |
       Ssa::explicitWrite(_, store.asInstruction(), nFrom.getInstruction()) and
       Ssa::ssaFlow(store, nodeTo)
@@ -916,7 +844,7 @@ private module StoreNodeFlow {
   predicate flowThrough(StoreNode nFrom, StoreNode nodeTo) {
     // Flow through a post update node that doesn't need a store step.
     not storeStep(nFrom, _, _) and
-    nodeTo.getOuter() = nFrom
+    nodeTo.getASuccessor() = nFrom
   }
 
   /**
@@ -924,7 +852,7 @@ private module StoreNodeFlow {
    * This happens because we have traversed an entire chain of field dereferences
    * after a store operation.
    */
-  predicate flowOutOf(StoreNodeInstr nFrom, Node nodeTo) {
+  predicate flowOutOf(StoreNode nFrom, Node nodeTo) {
     nFrom.isTerminal() and
     Ssa::ssaFlow(nFrom, nodeTo)
   }
@@ -1053,11 +981,7 @@ predicate localInstructionFlow(Instruction e1, Instruction e2) {
 predicate localExprFlow(Expr e1, Expr e2) { localFlow(exprNode(e1), exprNode(e2)) }
 
 private newtype TContent =
-  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
+  TFieldContent(Field f) or
   TCollectionContent() or // Not used in C/C++
   TArrayContent() // Not used in C/C++.
 

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

@@ -1,45 +1,12 @@
 import SsaImplCommon
+import SsaImplSpecific
 private import cpp as Cpp
 private import semmle.code.cpp.ir.IR
 private import DataFlowUtil
-private import DataFlowImplCommon as DataFlowImplCommon
+private import DataFlowPrivate
 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
@@ -76,10 +43,21 @@ private class DefOrUse extends TDefOrUse {
 
   /** Holds if this definition or use has rank `rank` in block `block`. */
   cached
-  final predicate hasRankInBlock(IRBlock block, int rnk) { rnk = getRank(this, block) }
+  final predicate hasRankInBlock(IRBlock block, int rnk) {
+    block = getBlock() and
+    rnk = getRank(this, block)
+  }
 
-  /** Gets the location of this element. */
-  abstract Cpp::Location getLocation();
+  /**
+   * Holds if this element is at the specified location.
+   * The location spans column `startcolumn` of line `startline` to
+   * column `endcolumn` of line `endline` in file `filepath`.
+   * For more information, see
+   * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
+   */
+  abstract predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  );
 }
 
 private Instruction toInstruction(DefOrUse defOrUse) {
@@ -95,27 +73,32 @@ abstract class Def extends DefOrUse {
   Instruction getInstruction() { result = store }
 
   /** Gets the variable that is defined by this definition. */
-  abstract SourceVariable getSourceVariable();
+  abstract SourceVariable getVariable();
 
   /** Holds if this definition is guaranteed to happen. */
   abstract predicate isCertain();
 
   override Instruction asDef() { result = this.getInstruction() }
 
-  override string toString() { result = "Def" }
+  override string toString() { result = "Def(" + store.getDumpString() + ")" }
 
   override IRBlock getBlock() { result = this.getInstruction().getBlock() }
 
-  override Cpp::Location getLocation() { result = store.getLocation() }
+  override predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    store.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+  }
 }
 
 private class ExplicitDef extends Def, TExplicitDef {
   ExplicitDef() { this = TExplicitDef(store) }
 
-  override SourceVariable getSourceVariable() {
+  override SourceVariable getVariable() {
     exists(VariableInstruction var |
       explicitWrite(_, this.getInstruction(), var) and
-      result.(SourceIRVariable).getIRVariable() = var.getIRVariable()
+      result.getVariable() = var.getIRVariable() and
+      not result.isIndirection()
     )
   }
 
@@ -125,12 +108,12 @@ private class ExplicitDef extends Def, TExplicitDef {
 private class ParameterDef extends Def, TInitializeParam {
   ParameterDef() { this = TInitializeParam(store) }
 
-  override SourceVariable getSourceVariable() {
-    result.(SourceIRVariable).getIRVariable() =
-      store.(InitializeParameterInstruction).getIRVariable()
+  override SourceVariable getVariable() {
+    result.getVariable() = store.(InitializeParameterInstruction).getIRVariable() and
+    not result.isIndirection()
     or
-    result.(SourceIRVariableIndirection).getUnderlyingIRVariable() =
-      store.(InitializeIndirectionInstruction).getIRVariable()
+    result.getVariable() = store.(InitializeIndirectionInstruction).getIRVariable() and
+    result.isIndirection()
   }
 
   override predicate isCertain() { any() }
@@ -144,25 +127,32 @@ abstract class Use extends DefOrUse {
   /** Gets the underlying operand of this use. */
   Operand getOperand() { result = use }
 
-  override string toString() { result = "Use" }
+  override string toString() { result = "Use(" + use.getDumpString() + ")" }
 
   /** Gets the variable that is used by this use. */
-  abstract SourceVariable getSourceVariable();
+  abstract SourceVariable getVariable();
 
   override IRBlock getBlock() { result = use.getUse().getBlock() }
 
-  override Cpp::Location getLocation() { result = use.getLocation() }
+  override predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    use.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+  }
 }
 
 private class ExplicitUse extends Use, TExplicitUse {
   ExplicitUse() { this = TExplicitUse(use) }
 
-  override SourceVariable getSourceVariable() {
+  override SourceVariable getVariable() {
     exists(VariableInstruction var |
       use.getDef() = var and
-      if use.getUse() instanceof ReadSideEffectInstruction
-      then result.(SourceIRVariableIndirection).getUnderlyingIRVariable() = var.getIRVariable()
-      else result.(SourceIRVariable).getIRVariable() = var.getIRVariable()
+      result.getVariable() = var.getIRVariable() and
+      (
+        if use.getUse() instanceof ReadSideEffectInstruction
+        then result.isIndirection()
+        else not result.isIndirection()
+      )
     )
   }
 }
@@ -170,10 +160,11 @@ private class ExplicitUse extends Use, TExplicitUse {
 private class ReturnParameterIndirection extends Use, TReturnParamIndirection {
   ReturnParameterIndirection() { this = TReturnParamIndirection(use) }
 
-  override SourceVariable getSourceVariable() {
+  override SourceVariable getVariable() {
     exists(ReturnIndirectionInstruction ret |
       returnParameterIndirection(use, ret) and
-      result.(SourceIRVariableIndirection).getUnderlyingIRVariable() = ret.getIRVariable()
+      result.getVariable() = ret.getIRVariable() and
+      result.isIndirection()
     )
   }
 }
@@ -206,16 +197,8 @@ predicate addressFlow(Instruction iFrom, Instruction iTo) {
   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
@@ -234,7 +217,8 @@ predicate addressFlowTC(Instruction iFrom, Instruction iTo) {
 
 /**
  * Gets the destination address of `instr` if it is a `StoreInstruction` or
- * a `WriteSideEffectInstruction`.
+ * a `WriteSideEffectInstruction`. The destination address of a `WriteSideEffectInstruction` is adjusted
+ * in the case of calls to operator `new` to give the destination address of a subsequent store (if any).
  */
 Instruction getDestinationAddress(Instruction instr) {
   result =
@@ -267,9 +251,6 @@ Operand getSourceAddressOperand(Instruction instr) {
     [
       instr.(LoadInstruction).getSourceAddressOperand(),
       instr.(ReadSideEffectInstruction).getArgumentOperand(),
-      // `ReferenceToInstruction` is really more of an address-of operation,
-      // but by including it in this list we break out of `flowOutOfAddressStep` at an
-      // instruction that, at the source level, looks like a use of a variable.
       instr.(ReferenceToInstruction).getSourceAddressOperand()
     ]
 }
@@ -296,8 +277,6 @@ Operand getSourceValueOperand(Instruction instr) {
   or
   result = instr.(ReadSideEffectInstruction).getSideEffectOperand()
   or
-  // See the comment on the `ReferenceToInstruction` disjunct in `getSourceAddressOperand` for why
-  // this case is included.
   result = instr.(ReferenceToInstruction).getSourceValueOperand()
 }
 
@@ -311,14 +290,9 @@ 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
+          i instanceof FieldAddressInstruction or i instanceof PointerArithmeticInstruction
         ), store.getDestinationAddress())
     then certain = false
     else certain = true
@@ -330,23 +304,59 @@ predicate explicitWrite(boolean certain, Instruction instr, Instruction address)
 
 cached
 private module Cached {
-  private predicate defUseFlow(Node nodeFrom, Node nodeTo) {
-    exists(IRBlock bb1, int i1, IRBlock bb2, int i2, DefOrUse defOrUse, Use use |
+  /**
+   * 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` to an `OperandNode`.
+    exists(IRBlock bb1, int i1, IRBlock bb2, int i2, DefOrUse defOrUse, Use use, SourceVariable v |
       defOrUse.hasRankInBlock(bb1, i1) and
       use.hasRankInBlock(bb2, i2) and
+      use.getVariable() = v 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
+    or
+    // Use-use flow from a `ReadNode` to an `OperandNode`.
+    exists(ReadNode read, IRBlock bb1, int i1, IRBlock bb2, int i2, Use use1, Use use2 |
+      read = nodeFrom and
+      use1.hasRankInBlock(bb1, i1) and
+      use2.hasRankInBlock(bb2, i2) and
+      use1.getOperand().getDef() = read.getInstruction() and
       adjacentDefRead(_, bb1, i1, bb2, i2) and
+      flowOutOfAddressStep(use2.getOperand(), nodeTo)
+    )
+    or
+    // Flow from phi nodes
+    exists(PhiNode phi, Use use, IRBlock block, int rnk |
+      phi = nodeFrom.(SsaPhiNode).getPhiNode() and
+      use.hasRankInBlock(block, rnk) and
+      phi.getSourceVariable() = use.getVariable() and
+      flowOutOfAddressStep(use.getOperand(), nodeTo) and
+      adjacentDefRead(_, phi.getBasicBlock(), -1, block, rnk)
+    )
+    or
+    // Flow to phi nodes
+    exists(Def def, StoreNode store, IRBlock block, int rnk |
+      store = nodeFrom and
+      store.isTerminal() and
+      def.getInstruction() = store.getStoreInstruction() and
+      def.hasRankInBlock(block, rnk) and
+      nodeTo.(SsaPhiNode).hasInputAtRankInBlock(block, rnk)
+    )
+    or
+    // Def-use flow from a `StoreNode` to an `OperandNode`.
+    exists(
+      StoreNode store, IRBlock bb1, int i1, IRBlock bb2, int i2, Def def, Use use, Definition ssaDef
+    |
+      store = nodeFrom and
+      store.isTerminal() and
+      def.getInstruction() = store.getStoreInstruction() and
+      def.hasRankInBlock(bb1, i1) and
+      adjacentDefRead(ssaDef, bb1, i1, bb2, i2) and
       use.hasRankInBlock(bb2, i2) and
       flowOutOfAddressStep(use.getOperand(), nodeTo)
     )
@@ -356,110 +366,30 @@ private module Cached {
     // 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
+    exists(
+      StoreNode store, WriteSideEffectInstruction write, IRBlock bb, int i1, int i2, Operand op
+    |
+      store = nodeFrom and
+      store.getInstruction().(CallInstruction).getStaticCallTarget() instanceof
         Alloc::OperatorNewAllocationFunction and
-      write = nodeFrom.getStoreInstruction() and
+      write = store.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
+      valueFlow*(store.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
+        valueFlow(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) {
+  private predicate valueFlow(Instruction iFrom, Instruction iTo) {
     iTo.(CopyValueInstruction).getSourceValue() = iFrom
     or
     iTo.(ConvertInstruction).getUnary() = iFrom
@@ -479,31 +409,15 @@ private module Cached {
     )
   }
 
-  /**
-   * 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) {
+  private predicate flowOutOfAddressStep(Operand operand, Node nTo) {
     // Flow into a read node
-    exists(ReadNode readNode | readNode = nodeTo |
+    exists(ReadNode readNode | readNode = nTo |
       readNode.isInitial() and
       operand.getDef() = readNode.getInstruction()
     )
     or
-    exists(StoreNodeInstr storeNode, Instruction def |
-      storeNode = nodeTo and
+    exists(StoreNode storeNode, Instruction def |
+      storeNode = nTo and
       def = operand.getDef()
     |
       storeNode.isTerminal() and
@@ -513,51 +427,51 @@ private module Cached {
       explicitWrite(false, storeNode.getStoreInstruction(), def)
     )
     or
-    operand = getSourceAddressOperand(nodeTo.asInstruction())
+    operand = getSourceAddressOperand(nTo.asInstruction())
     or
     exists(ReturnIndirectionInstruction ret |
       ret.getSourceAddressOperand() = operand and
-      ret = nodeTo.asInstruction()
+      ret = nTo.asInstruction()
     )
     or
     exists(ReturnValueInstruction ret |
       ret.getReturnAddressOperand() = operand and
-      nodeTo.asInstruction() = ret
+      nTo.asInstruction() = ret
     )
     or
     exists(CallInstruction call, int index, ReadSideEffectInstruction read |
       call.getArgumentOperand(index) = operand and
       read = getSideEffectFor(call, index) and
-      nodeTo.asOperand() = read.getSideEffectOperand()
+      nTo.asOperand() = read.getSideEffectOperand()
     )
     or
     exists(CopyInstruction copy |
       not exists(getSourceAddressOperand(copy)) and
       copy.getSourceValueOperand() = operand and
-      flowOutOfAddressStep(copy.getAUse(), nodeTo)
+      flowOutOfAddressStep(copy.getAUse(), nTo)
     )
     or
     exists(ConvertInstruction convert |
       convert.getUnaryOperand() = operand and
-      flowOutOfAddressStep(convert.getAUse(), nodeTo)
+      flowOutOfAddressStep(convert.getAUse(), nTo)
     )
     or
     exists(CheckedConvertOrNullInstruction convert |
       convert.getUnaryOperand() = operand and
-      flowOutOfAddressStep(convert.getAUse(), nodeTo)
+      flowOutOfAddressStep(convert.getAUse(), nTo)
     )
     or
     exists(InheritanceConversionInstruction convert |
       convert.getUnaryOperand() = operand and
-      flowOutOfAddressStep(convert.getAUse(), nodeTo)
+      flowOutOfAddressStep(convert.getAUse(), nTo)
     )
     or
     exists(PointerArithmeticInstruction arith |
       arith.getLeftOperand() = operand and
-      flowOutOfAddressStep(arith.getAUse(), nodeTo)
+      flowOutOfAddressStep(arith.getAUse(), nTo)
     )
     or
-    // Flow through a modeled function that has parameter -> return value flow.
+    // Flow through a modelled function that has parameter -> return value flow.
     exists(
       CallInstruction call, int index, DataFlow::FunctionInput input,
       DataFlow::FunctionOutput output
@@ -567,34 +481,9 @@ private module Cached {
       not getSideEffectFor(call, index) instanceof ReadSideEffectInstruction and
       input.isParameter(index) and
       output.isReturnValue() and
-      flowOutOfAddressStep(call.getAUse(), nodeTo)
+      flowOutOfAddressStep(call.getAUse(), nTo)
     )
   }
 }
 
 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/SsaImplCommon.qll

@@ -141,25 +141,26 @@ private module Liveness {
 
 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 `bb1` strictly dominates `bb2`. */
+private predicate strictlyDominates(BasicBlock bb1, BasicBlock bb2) {
+  bb1 = getImmediateBasicBlockDominator+(bb2)
+}
+
+/** Holds if `bb1` dominates a predecessor of `bb2`. */
+private predicate dominatesPredecessor(BasicBlock bb1, BasicBlock bb2) {
+  exists(BasicBlock pred | pred = getABasicBlockPredecessor(bb2) |
+    bb1 = pred
+    or
+    strictlyDominates(bb1, pred)
   )
 }
 
+/** Holds if `df` is in the dominance frontier of `bb`. */
+private predicate inDominanceFrontier(BasicBlock bb, BasicBlock df) {
+  dominatesPredecessor(bb, df) and
+  not strictlyDominates(bb, df)
+}
+
 /**
  * Holds if `bb` is in the dominance frontier of a block containing a
  * definition of `v`.
@@ -634,28 +635,3 @@ class UncertainWriteDefinition extends WriteDefinition {
     )
   }
 }
-
-/** 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(_, def0, bb, i)))
-  }
-
-  query predicate readWithoutDef(SourceVariable v, BasicBlock bb, int i) {
-    variableRead(bb, i, v, _) and
-    not ssaDefReachesRead(_, _, bb, i)
-  }
-
-  query predicate deadDef(RelevantDefinition def, SourceVariable v) {
-    v = def.getSourceVariable() and
-    not ssaDefReachesRead(_, def, _, _) and
-    not phiHasInputFromBlock(_, def, _)
-  }
-}

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

@@ -1,10 +1,11 @@
 private import semmle.code.cpp.ir.IR
-private import SsaInternals as Ssa
+private import DataFlowUtil
+private import DataFlowPrivate
+private import DataFlowImplCommon as DataFlowImplCommon
+private import Ssa as Ssa
 
 class BasicBlock = IRBlock;
 
-class SourceVariable = Ssa::SourceVariable;
-
 BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.immediatelyDominates(bb) }
 
 BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() }
@@ -13,6 +14,51 @@ class ExitBasicBlock extends IRBlock {
   ExitBasicBlock() { this.getLastInstruction() instanceof ExitFunctionInstruction }
 }
 
-predicate variableWrite = Ssa::variableWrite/4;
+private newtype TSourceVariable =
+  TSourceIRVariable(IRVariable var) or
+  TSourceIRVariableIndirection(InitializeIndirectionInstruction init)
 
-predicate variableRead = Ssa::variableRead/4;
+abstract class SourceVariable extends TSourceVariable {
+  IRVariable var;
+
+  IRVariable getVariable() { result = var }
+
+  abstract string toString();
+
+  predicate isIndirection() { none() }
+}
+
+class SourceIRVariable extends SourceVariable, TSourceIRVariable {
+  SourceIRVariable() { this = TSourceIRVariable(var) }
+
+  override string toString() { result = this.getVariable().toString() }
+}
+
+class SourceIRVariableIndirection extends SourceVariable, TSourceIRVariableIndirection {
+  InitializeIndirectionInstruction init;
+
+  SourceIRVariableIndirection() {
+    this = TSourceIRVariableIndirection(init) and var = init.getIRVariable()
+  }
+
+  override string toString() { result = "*" + this.getVariable().toString() }
+
+  override predicate isIndirection() { any() }
+}
+
+predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) {
+  DataFlowImplCommon::forceCachingInSameStage() and
+  exists(Ssa::Def def |
+    def.hasRankInBlock(bb, i) and
+    v = def.getVariable() and
+    (if def.isCertain() then certain = true else certain = false)
+  )
+}
+
+predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain) {
+  exists(Ssa::Use use |
+    use.hasRankInBlock(bb, i) and
+    v = use.getVariable() and
+    certain = true
+  )
+}

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.getExpr()) }
+  final TranslatedExpr getOperand() { result = getTranslatedExpr(expr.(Conversion).getExpr()) }
 }
 
 /**
@@ -1305,9 +1305,9 @@ class TranslatedBinaryOperation extends TranslatedSingleInstructionExpr {
   }
 
   override Opcode getOpcode() {
-    result = binaryArithmeticOpcode(expr) or
-    result = binaryBitwiseOpcode(expr) or
-    result = comparisonOpcode(expr)
+    result = binaryArithmeticOpcode(expr.(BinaryArithmeticOperation)) or
+    result = binaryBitwiseOpcode(expr.(BinaryBitwiseOperation)) or
+    result = comparisonOpcode(expr.(ComparisonOperation))
   }
 
   override int getInstructionElementSize(InstructionTag tag) {

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

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

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

@@ -34,16 +34,6 @@ 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)
+    result = this.enumBitSize(t.(Enum))
     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)
+    result = this.enumAlignment(t.(Enum))
     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())
+    result = getAnInitialField(t.getAField().getUnspecifiedType().(PaddedType))
   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())
+      result = getAnInitialField(firstField.getUnspecifiedType().(PaddedType))
     )
 }
 

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)) }
+  predicate isPhiNode(StackVariable v) { exists(RangeSSA x | x.phi_node(v, this.(BasicBlock))) }
 
   /**
    * DEPRECATED: Use isGuardPhi/4 instead

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.getFormat().(FormatLiteral).getConversionChar(n)
+    conv = ffc.(FormattingFunctionCall).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.getExtent().getFullyConverted())
+  hc = hashCons(new.(NewArrayExpr).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.getExtent().getFullyConverted()) <= 1
+  count(new.(NewArrayExpr).getExtent().getFullyConverted()) <= 1
 }
 
 private predicate mk_NewArrayExpr(

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

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

cpp/ql/src/Architecture/index.qhelp

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

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

@@ -62,7 +62,7 @@ class Thing extends Locatable {
   }
 
   Thing callsOrAccesses() {
-    this.(Function).calls(result)
+    this.(Function).calls(result.(Function))
     or
     this.(Function).accesses(result.(Function))
     or

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

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

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 |
+    exists(FunctionCall fc | fc = e.(FunctionCall) |
       not (
         whitelistFunction(fc.getTarget(), _) or
         fc.getTarget().hasDefinition()
       )
     )
     or
-    exists(Parameter p | p = e.(VariableAccess).getTarget() |
+    exists(Parameter p | p = e.(VariableAccess).getTarget().(Parameter) |
       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()
+  result = e.getEnclosingElement().(BinaryOperation)
   or
   result = getATopLevelBinaryOperationExpression(e.getEnclosingElement())
 }

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().getABaseClass+()
+    call.getEnclosingFunction().getDeclaringType().(Class).getABaseClass+()
 }
 
 from PureExprInVoidContext peivc, Locatable parent, Locatable info, string info_text, string tail

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()) and
+  loc = sum(FunctionDeclarationEntry fde | fde.getFile() = f | fde.getNumberOfLines()).(float) and
   if loc > 0
   then
     // Weighted average of complexity by function length

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

@@ -0,0 +1,42 @@
+<!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

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

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

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

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

@@ -0,0 +1,48 @@
+<!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

@@ -0,0 +1,30 @@
+<!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

@@ -0,0 +1,51 @@
+<!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

@@ -0,0 +1,53 @@
+<!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>

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

@@ -0,0 +1,63 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+This metric measures the number of recent changes to files that have occurred
+below this location in the tree. A recent change is taken to mean a change
+that has occurred in the last <code>180</code> days.
+</p>
+
+<p>
+All code that has changed a great deal may be more than usually prone to
+defects, but this is particularly true of code that has been changing
+dramatically in the recent past, because it has not yet had a chance to be
+properly field-tested in order to iron out the bugs.
+</p>
+
+</overview>
+<recommendation>
+
+<p>
+There is more than one reason why a file may have been changing a lot
+recently:
+</p>
+
+<ul>
+<li>
+The file may be part of a new subsystem that is being written. New code is
+always going to change a lot in a short period of time, but it is important
+to ensure that it is properly code reviewed and unit tested before integrating
+it into a working product.
+</li>
+
+<li>
+The file may be being heavily refactored. Large refactorings are sometimes
+essential, but they are also quite risky. You should write proper regression
+tests before starting on a major refactoring, and check that they still pass
+once you're done.
+</li>
+
+<li>
+The same bit of code may be being changed repeatedly because it is difficult
+to get right. Aside from vigorous code reviewing and testing, it may be a good
+idea to rethink the system design - if something is that hard
+to get right (and it's not an inherently difficult concept), you might be making life unnecessarily hard for yourself and
+risking introducing insidious defects.
+</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>

cpp/ql/src/Security/CWE/CWE-313/CleartextSqliteDatabase.ql

@@ -29,7 +29,7 @@ class SqliteFunctionCall extends FunctionCall {
 }
 
 predicate sqlite_encryption_used() {
-  any(StringLiteral l).getValue().toLowerCase().matches("pragma key%") or
+  any(StringLiteral l).getValue().toLowerCase().regexpMatch("pragma key.*") or
   any(StringLiteral l).getValue().toLowerCase().matches("%attach%database%key%") or
   any(FunctionCall fc).getTarget().getName().matches("sqlite%\\_key\\_%")
 }

cpp/ql/src/Security/CWE/CWE-319/UseOfHttp.cpp

@@ -1,9 +0,0 @@
-
-void openUrl(char *url)
-{
-	// ...
-}
-
-openUrl("http://example.com"); // BAD
-
-openUrl("https://example.com"); // GOOD: Opening a connection to a URL using HTTPS enforces SSL.

cpp/ql/src/Security/CWE/CWE-319/UseOfHttp.qhelp

@@ -1,35 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-
-<p>Constructing URLs with the HTTP protocol can lead to unsecured connections.</p>
-
-</overview>
-<recommendation>
-
-<p>When you construct a URL, ensure that you use an HTTPS URL rather than an HTTP URL. Then, any connections that are made using that URL are secure SSL connections.</p>
-
-</recommendation>
-<example>
-
-<p>The following example shows two ways of opening a connection using a URL. When the connection is 
-opened using an HTTP URL rather than an HTTPS URL, the connection is unsecured. When the connection is opened using an HTTPS URL, the connection is a secure SSL connection.</p>
-
-<sample src="UseOfHttp.cpp" />
-
-</example>
-<references>
-
-<li>
-OWASP:
-<a href="https://cheatsheetseries.owasp.org/cheatsheets/Transport_Layer_Protection_Cheat_Sheet.html">Transport Layer Protection Cheat Sheet</a>.
-</li>
-<li>
-OWASP Top 10:
-<a href="https://owasp.org/Top10/A08_2021-Software_and_Data_Integrity_Failures/">A08:2021 - Software and Data Integrity Failures</a>.
-</li>
-
-</references>
-</qhelp>

cpp/ql/src/Security/CWE/CWE-319/UseOfHttp.ql

@@ -1,90 +0,0 @@
-/**
- * @name Failure to use HTTPS URLs
- * @description Non-HTTPS connections can be intercepted by third parties.
- * @kind path-problem
- * @problem.severity warning
- * @precision medium
- * @id cpp/non-https-url
- * @tags security
- *       external/cwe/cwe-319
- *       external/cwe/cwe-345
- */
-
-import cpp
-import semmle.code.cpp.dataflow.TaintTracking
-import DataFlow::PathGraph
-
-/**
- * A string matching private host names of IPv4 and IPv6, which only matches
- * the host portion therefore checking for port is not necessary.
- * Several examples are localhost, reserved IPv4 IP addresses including
- * 127.0.0.1, 10.x.x.x, 172.16.x,x, 192.168.x,x, and reserved IPv6 addresses
- * including [0:0:0:0:0:0:0:1] and [::1]
- */
-class PrivateHostName extends string {
-  bindingset[this]
-  PrivateHostName() {
-    this.regexpMatch("(?i)localhost(?:[:/?#].*)?|127\\.0\\.0\\.1(?:[:/?#].*)?|10(?:\\.[0-9]+){3}(?:[:/?#].*)?|172\\.16(?:\\.[0-9]+){2}(?:[:/?#].*)?|192.168(?:\\.[0-9]+){2}(?:[:/?#].*)?|\\[?0:0:0:0:0:0:0:1\\]?(?:[:/?#].*)?|\\[?::1\\]?(?:[:/?#].*)?")
-  }
-}
-
-/**
- * A string containing an HTTP URL not in a private domain.
- */
-class HttpStringLiteral extends StringLiteral {
-  HttpStringLiteral() {
-    exists(string s | this.getValue() = s |
-      s = "http"
-      or
-      exists(string tail |
-        tail = s.regexpCapture("http://(.*)", 1) and not tail instanceof PrivateHostName
-      ) and
-      not TaintTracking::localExprTaint(any(StringLiteral p |
-          p.getValue() instanceof PrivateHostName
-        ), this.getParent*())
-    )
-  }
-}
-
-/**
- * Taint tracking configuration for HTTP connections.
- */
-class HttpStringToUrlOpenConfig extends TaintTracking::Configuration {
-  HttpStringToUrlOpenConfig() { this = "HttpStringToUrlOpenConfig" }
-
-  override predicate isSource(DataFlow::Node src) {
-    // Sources are strings containing an HTTP URL not in a private domain.
-    src.asExpr() instanceof HttpStringLiteral
-  }
-
-  override predicate isSink(DataFlow::Node sink) {
-    // Sinks can be anything that demonstrates the string is likely to be
-    // accessed as a URL, for example using it in a network access. Some
-    // URLs are only ever displayed or used for data processing.
-    exists(FunctionCall fc |
-      fc.getTarget()
-          .hasGlobalOrStdName([
-              "system", "gethostbyname", "gethostbyname2", "gethostbyname_r", "getaddrinfo",
-              "X509_load_http", "X509_CRL_load_http"
-            ]) and
-      sink.asExpr() = fc.getArgument(0)
-      or
-      fc.getTarget().hasGlobalOrStdName(["send", "URLDownloadToFile", "URLDownloadToCacheFile"]) and
-      sink.asExpr() = fc.getArgument(1)
-      or
-      fc.getTarget().hasGlobalOrStdName(["curl_easy_setopt", "getnameinfo"]) and
-      sink.asExpr() = fc.getArgument(2)
-      or
-      fc.getTarget().hasGlobalOrStdName(["ShellExecute", "ShellExecuteA", "ShellExecuteW"]) and
-      sink.asExpr() = fc.getArgument(3)
-    )
-  }
-}
-
-from
-  HttpStringToUrlOpenConfig config, DataFlow::PathNode source, DataFlow::PathNode sink,
-  HttpStringLiteral str
-where
-  config.hasFlowPath(source, sink) and
-  str = source.getNode().asExpr()
-select str, source, sink, "A URL may be constructed with the HTTP protocol."

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

@@ -324,8 +324,10 @@ abstract class DataOutput extends Element {
 /**
  * Data that is output via standard output or standard error.
  */
-class StandardOutput extends DataOutput instanceof OutputWrite {
-  override Expr getASource() { result = OutputWrite.super.getASource() }
+class StandardOutput extends DataOutput {
+  StandardOutput() { this instanceof OutputWrite }
+
+  override Expr getASource() { result = this.(OutputWrite).getASource() }
 }
 
 private predicate socketCallOrIndirect(FunctionCall call) {
@@ -376,5 +378,5 @@ class SocketOutput extends DataOutput {
 from SystemData sd, DataOutput ow
 where
   sd.getAnExprIndirect() = ow.getASource() or
-  sd.getAnExprIndirect() = ow.getASource().getAChild*()
+  sd.getAnExprIndirect() = ow.getASource().(Expr).getAChild*()
 select ow, "This operation exposes system data from $@.", sd, sd.toString()

cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.cpp

@@ -1,24 +0,0 @@
-...
-  chroot("/myFold/myTmp"); // BAD
-...
-  chdir("/myFold/myTmp"); // BAD
-...
-  int fd = open("/myFold/myTmp", O_RDONLY | O_DIRECTORY);
-  fchdir(fd); // BAD
-...
-  if (chdir("/myFold/myTmp") == -1) {
-    exit(-1);
-  }
-  if (chroot("/myFold/myTmp") == -1) {  // GOOD
-    exit(-1);
-  }
-...
-  if (chdir("/myFold/myTmp") == -1) { // GOOD
-    exit(-1);
-  }
-...
-  int fd = open("/myFold/myTmp", O_RDONLY | O_DIRECTORY);
-  if(fchdir(fd) == -1) { // GOOD
-    exit(-1);
-  }
-...

cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.qhelp

@@ -1,23 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>Working with changing directories, without checking the return value or pinning the directory, may not be safe. Requires the attention of developers.</p>
-
-</overview>
-
-<example>
-<p>The following example demonstrates erroneous and corrected work with changing working directories.</p>
-<sample src="IncorrectChangingWorkingDirectory.cpp" />
-
-</example>
-<references>
-
-<li>
-  CERT C Coding Standard:
-  <a href="https://wiki.sei.cmu.edu/confluence/display/c/POS05-C.+Limit+access+to+files+by+creating+a+jail">POS05-C. Limit access to files by creating a jail.</a>
-</li>
-
-</references>
-</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.ql

@@ -1,69 +0,0 @@
-/**
- * @name Find work with changing working directories, with security errors.
- * @description Not validating the return value or pinning the directory can be unsafe.
- * @kind problem
- * @id cpp/work-with-changing-working-directories
- * @problem.severity warning
- * @precision medium
- * @tags correctness
- *       security
- *       external/cwe/cwe-243
- *       external/cwe/cwe-252
- */
-
-import cpp
-import semmle.code.cpp.commons.Exclusions
-
-/** Holds if a `fc` function call is available before or after a `chdir` function call. */
-predicate inExistsChdir(FunctionCall fcp) {
-  exists(FunctionCall fctmp |
-    (
-      fctmp.getTarget().hasGlobalOrStdName("chdir") or
-      fctmp.getTarget().hasGlobalOrStdName("fchdir")
-    ) and
-    (
-      fcp.getBasicBlock().getASuccessor*() = fctmp.getBasicBlock() or
-      fctmp.getBasicBlock().getASuccessor*() = fcp.getBasicBlock()
-    )
-  )
-}
-
-/** Holds if a `fc` function call is available before or after a function call containing a `chdir` call. */
-predicate outExistsChdir(FunctionCall fcp) {
-  exists(FunctionCall fctmp |
-    exists(FunctionCall fctmp2 |
-      (
-        fctmp2.getTarget().hasGlobalOrStdName("chdir") or
-        fctmp2.getTarget().hasGlobalOrStdName("fchdir")
-      ) and
-      // we are looking for a call containing calls chdir and fchdir
-      fctmp2.getEnclosingStmt().getParentStmt*() = fctmp.getTarget().getEntryPoint().getChildStmt*()
-    ) and
-    (
-      fcp.getBasicBlock().getASuccessor*() = fctmp.getBasicBlock() or
-      fctmp.getBasicBlock().getASuccessor*() = fcp.getBasicBlock()
-    )
-  )
-}
-
-from FunctionCall fc, string msg
-where
-  fc.getTarget().hasGlobalOrStdName("chroot") and
-  not inExistsChdir(fc) and
-  not outExistsChdir(fc) and
-  // in this section I want to exclude calls to functions containing chroot that have a direct path to chdir, or to a function containing chdir
-  exists(FunctionCall fctmp |
-    fc.getEnclosingStmt().getParentStmt*() = fctmp.getTarget().getEntryPoint().getChildStmt*() and
-    not inExistsChdir(fctmp) and
-    not outExistsChdir(fctmp)
-  ) and
-  msg = "Creation of 'chroot' jail without changing the working directory"
-  or
-  (
-    fc.getTarget().hasGlobalOrStdName("chdir") or
-    fc.getTarget().hasGlobalOrStdName("fchdir")
-  ) and
-  fc instanceof ExprInVoidContext and
-  not isFromMacroDefinition(fc) and
-  msg = "Unchecked return value for call to '" + fc.getTarget().getName() + "'."
-select fc, msg

cpp/ql/src/experimental/Security/CWE/CWE-377/InsecureTemporaryFile.cpp

@@ -1,14 +0,0 @@
-...
-  fp = fopen("/tmp/name.tmp","w"); // BAD
-...
-  char filename = tmpnam(NULL);
-  fp = fopen(filename,"w"); // BAD
-...
-
-  strcat (filename, "/tmp/name.XXXXXX");
-  fd = mkstemp(filename);
-  if ( fd < 0 ) {
-    return error;
-  }
-  fp = fdopen(fd,"w") // GOOD
-...

cpp/ql/src/experimental/Security/CWE/CWE-377/InsecureTemporaryFile.qhelp

@@ -1,23 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>Working with a file, without checking its existence and its rights, as well as working with names that can be predicted, may not be safe. Requires the attention of developers.</p>
-
-</overview>
-
-<example>
-<p>The following example demonstrates erroneous and corrected work with file.</p>
-<sample src="InsecureTemporaryFile.cpp" />
-
-</example>
-<references>
-
-<li>
-  CERT C Coding Standard:
-  <a href="https://wiki.sei.cmu.edu/confluence/display/c/CON33-C.+Avoid+race+conditions+when+using+library+functions">CON33-C. Avoid race conditions when using library functions</a>.
-</li>
-
-</references>
-</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-377/InsecureTemporaryFile.ql

@@ -1,112 +0,0 @@
-/**
- * @name Insecure generation of filenames.
- * @description Using a predictable filename when creating a temporary file can lead to an attacker-controlled input.
- * @kind problem
- * @id cpp/insecure-generation-of-filename
- * @problem.severity warning
- * @precision medium
- * @tags correctness
- *       security
- *       external/cwe/cwe-377
- */
-
-import cpp
-import semmle.code.cpp.valuenumbering.GlobalValueNumbering
-
-/** Holds for a function `f` that has an argument at index `apos` used to read the file. */
-predicate numberArgumentRead(Function f, int apos) {
-  f.hasGlobalOrStdName("fgets") and apos = 2
-  or
-  f.hasGlobalOrStdName("fread") and apos = 3
-  or
-  f.hasGlobalOrStdName("read") and apos = 0
-  or
-  f.hasGlobalOrStdName("fscanf") and apos = 0
-}
-
-/** Holds for a function `f` that has an argument at index `apos` used to write to file */
-predicate numberArgumentWrite(Function f, int apos) {
-  f.hasGlobalOrStdName("fprintf") and apos = 0
-  or
-  f.hasGlobalOrStdName("fputs") and apos = 1
-  or
-  f.hasGlobalOrStdName("write") and apos = 0
-  or
-  f.hasGlobalOrStdName("fwrite") and apos = 3
-  or
-  f.hasGlobalOrStdName("fflush") and apos = 0
-}
-
-from FunctionCall fc, string msg
-where
-  // search for functions for generating a name, without a guarantee of the absence of a file during the period of work with it.
-  (
-    fc.getTarget().hasGlobalOrStdName("tmpnam") or
-    fc.getTarget().hasGlobalOrStdName("tmpnam_s") or
-    fc.getTarget().hasGlobalOrStdName("tmpnam_r")
-  ) and
-  not exists(FunctionCall fctmp |
-    (
-      fctmp.getTarget().hasGlobalOrStdName("mktemp") or
-      fctmp.getTarget().hasGlobalOrStdName("mkstemp") or
-      fctmp.getTarget().hasGlobalOrStdName("mkstemps") or
-      fctmp.getTarget().hasGlobalOrStdName("mkdtemp")
-    ) and
-    (
-      fc.getBasicBlock().getASuccessor*() = fctmp.getBasicBlock() or
-      fctmp.getBasicBlock().getASuccessor*() = fc.getBasicBlock()
-    )
-  ) and
-  msg =
-    "Finding the name of a file that does not exist does not mean that it will not be exist at the next operation."
-  or
-  // finding places to work with a file without setting permissions, but with predictable names.
-  (
-    fc.getTarget().hasGlobalOrStdName("fopen") or
-    fc.getTarget().hasGlobalOrStdName("open")
-  ) and
-  fc.getNumberOfArguments() = 2 and
-  exists(FunctionCall fctmp, int i |
-    numberArgumentWrite(fctmp.getTarget(), i) and
-    globalValueNumber(fc) = globalValueNumber(fctmp.getArgument(i))
-  ) and
-  not exists(FunctionCall fctmp, int i |
-    numberArgumentRead(fctmp.getTarget(), i) and
-    globalValueNumber(fc) = globalValueNumber(fctmp.getArgument(i))
-  ) and
-  exists(FunctionCall fctmp |
-    (
-      fctmp.getTarget().hasGlobalOrStdName("strcat") or
-      fctmp.getTarget().hasGlobalOrStdName("strcpy")
-    ) and
-    globalValueNumber(fc.getArgument(0)) = globalValueNumber(fctmp.getAnArgument())
-    or
-    fctmp.getTarget().hasGlobalOrStdName("getenv") and
-    globalValueNumber(fc.getArgument(0)) = globalValueNumber(fctmp)
-    or
-    (
-      fctmp.getTarget().hasGlobalOrStdName("asprintf") or
-      fctmp.getTarget().hasGlobalOrStdName("vasprintf") or
-      fctmp.getTarget().hasGlobalOrStdName("xasprintf") or
-      fctmp.getTarget().hasGlobalOrStdName("xvasprintf ")
-    ) and
-    exists(Variable vrtmp |
-      vrtmp = fc.getArgument(0).(VariableAccess).getTarget() and
-      vrtmp = fctmp.getArgument(0).(AddressOfExpr).getAddressable().(Variable) and
-      not vrtmp instanceof Field
-    )
-  ) and
-  not exists(FunctionCall fctmp |
-    (
-      fctmp.getTarget().hasGlobalOrStdName("umask") or
-      fctmp.getTarget().hasGlobalOrStdName("fchmod") or
-      fctmp.getTarget().hasGlobalOrStdName("chmod")
-    ) and
-    (
-      fc.getBasicBlock().getASuccessor*() = fctmp.getBasicBlock() or
-      fctmp.getBasicBlock().getASuccessor*() = fc.getBasicBlock()
-    )
-  ) and
-  msg =
-    "Creating a file for writing without evaluating its existence and setting permissions can be unsafe."
-select fc, msg

cpp/ql/src/experimental/Security/CWE/CWE-783/OperatorPrecedenceLogicErrorWhenUseBitwiseOrLogicalOperations.ql

@@ -188,7 +188,8 @@ where
   isBitwiseandBitwise(exp) and
   isDifferentResults(exp.(BinaryBitwiseOperation).getLeftOperand(),
     exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation).getLeftOperand(),
-    exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation).getRightOperand(), exp,
-    exp.(BinaryBitwiseOperation).getRightOperand()) and
+    exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation).getRightOperand(),
+    exp.(BinaryBitwiseOperation),
+    exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation)) and
   msg = "specify the priority with parentheses."
 select exp, msg

cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql

@@ -142,7 +142,7 @@ class Resource extends MemberVariable {
 
   predicate acquisitionWithRequiredKind(Assignment acquireAssign, string kind) {
     // acquireAssign is an assignment to this resource
-    acquireAssign.getLValue() = this.getAnAccess() and
+    acquireAssign.(Assignment).getLValue() = this.getAnAccess() and
     // Should be in this class, but *any* member method will do
     this.inSameClass(acquireAssign) and
     // Check that it is an acquisition function and return the corresponding kind

cpp/ql/src/jsf/4.15 Declarations and Definitions/AV Rule 135.ql

@@ -31,7 +31,7 @@ from Variable v, Variable shadowed
 where
   not v.getParentScope().(BlockStmt).isInMacroExpansion() and
   (
-    v.(LocalVariableOrParameter).shadowsGlobal(shadowed) or
+    v.(LocalVariableOrParameter).shadowsGlobal(shadowed.(GlobalVariable)) or
     localShadowsParameter(v, shadowed) or
     shadowing(v, shadowed)
   )

cpp/ql/src/jsf/4.22 Pointers and References/AV Rule 173.ql

@@ -26,7 +26,7 @@ import cpp
 from Assignment a, Variable global, Variable local
 where
   a.fromSource() and
-  global.getAnAccess() = a.getLValue() and
+  global.getAnAccess() = a.getLValue().(VariableAccess) and
   local.getAnAccess() = a.getRValue().(AddressOfExpr).getOperand() and
   local.hasSpecifier("auto") and
   (

cpp/ql/src/jsf/4.28 Portable Code/AV Rule 210.ql

@@ -49,11 +49,11 @@ class ExposingIntegralUnion extends Union {
     exists(MemberVariable mv1, MemberVariable mv2, IntegralType mv1tp, IntegralType mv2tp |
       mv1 = this.getAMemberVariable() and
       mv2 = this.getAMemberVariable() and
-      mv1tp = mv1.getUnderlyingType() and
+      mv1tp = mv1.getUnderlyingType().(IntegralType) and
       (
-        mv2tp = mv2.getUnderlyingType()
+        mv2tp = mv2.getUnderlyingType().(IntegralType)
         or
-        mv2tp = mv2.getUnderlyingType().(ArrayType).getBaseType().getUnderlyingType()
+        mv2tp = mv2.getUnderlyingType().(ArrayType).getBaseType().getUnderlyingType().(IntegralType)
       ) and
       mv1tp.getSize() > mv2tp.getSize()
     )

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-243/semmle/tests/IncorrectChangingWorkingDirectory.expected

@@ -1,2 +0,0 @@
-| test.cpp:12:7:12:12 | call to chroot | Creation of 'chroot' jail without changing the working directory |
-| test.cpp:29:3:29:7 | call to chdir | Unchecked return value for call to 'chdir'. |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-243/semmle/tests/IncorrectChangingWorkingDirectory.qlref

@@ -1 +0,0 @@
-experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-243/semmle/tests/test.cpp

@@ -1,46 +0,0 @@
-typedef int FILE;
-#define size_t int
-size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
-FILE *fopen(const char *filename, const char *mode);
-int fread(char *buf, int size, int count, FILE *fp);
-int fclose(FILE *fp);
-int chroot(char *path);
-int chdir(char *path);
-void exit(int status);
-
-int funTest1(){
-  if (chroot("/myFold/myTmp") == -1) {  // BAD
-    exit(-1);
-  }
-  return 0;
-}
-
-int funTest2(){  
-  if (chdir("/myFold/myTmp") == -1) { // GOOD
-    exit(-1);
-  }
-  if (chroot("/myFold/myTmp") == -1) {  // GOOD
-    exit(-1);
-  }
-  return 0;
-}
-
-int funTest3(){  
-  chdir("/myFold/myTmp"); // BAD
-  return 0;
-}
-int main(int argc, char *argv[])
-{
-  if(argc = 0) {
-    funTest3();
-    return 2;
-  }
-  if(argc = 1)
-    funTest1();
-  else
-    funTest2();
-  FILE *fp = fopen(argv[1], "w");
-  fwrite("12345", 5, 1, fp);
-  fclose(fp);
-  return 0;
-}

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/InsecureTemporaryFile.expected

@@ -1,2 +0,0 @@
-| test.cpp:16:20:16:25 | call to tmpnam | Finding the name of a file that does not exist does not mean that it will not be exist at the next operation. |
-| test.cpp:42:8:42:12 | call to fopen | Creating a file for writing without evaluating its existence and setting permissions can be unsafe. |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/InsecureTemporaryFile.qlref

@@ -1 +0,0 @@
-experimental/Security/CWE/CWE-377/InsecureTemporaryFile.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/test.cpp

@@ -1,68 +0,0 @@
-typedef int FILE;
-#define NULL (0)
-FILE *fopen(char *filename, const char *mode);
-FILE *fdopen(int handle, char *mode);
-char * tmpnam(char * name);
-int mkstemp(char * name);
-char * strcat(char *str1, const char *str2);
-int umask(int pmode);
-int chmod(char * filename,int pmode);
-int fprintf(FILE *fp,const char *fmt, ...);
-int fclose(FILE *stream);
-
-int funcTest1()
-{
-  FILE *fp;
-  char *filename = tmpnam(NULL); // BAD
-  fp = fopen(filename,"w");
-  fprintf(fp,"%s\n","data to file");
-  fclose(fp);
-  return 0;
-}
-
-int funcTest2()
-{
-  FILE *fp;
-  int fd;
-  char filename[80];
-  strcat (filename, "/tmp/name.XXXXXX");
-  fd = mkstemp(filename);
-  if ( fd < 0 ) {
-    return 1;
-  }
-  fp = fdopen(fd,"w"); // GOOD
-  return 0;
-}
-
-int funcTest3()
-{
-  FILE *fp;
-  char filename[80];
-  strcat(filename, "/tmp/tmp.name");
-  fp = fopen(filename,"w"); // BAD
-  fprintf(fp,"%s\n","data to file");
-  fclose(fp);
-  return 0;
-}
-
-int funcTest4()
-{
-  FILE *fp;
-  char filename[80];
-  umask(0022);
-  strcat(filename, "/tmp/tmp.name");
-  fp = fopen(filename,"w"); // GOOD
-  chmod(filename,0666);
-  fprintf(fp,"%s\n","data to file");
-  fclose(fp);
-  return 0;
-}
-
-int main(int argc, char *argv[])
-{
-  funcTest1();
-  funcTest2();
-  funcTest3();
-  funcTest4();
-  return 0;
-}

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

@@ -14,7 +14,9 @@ import semmle.code.cpp.controlflow.SSA
 
 select count(SsaDefinition d, StackVariable v, Expr u |
     d.getAUse(v) = u and
-    not exists(BasicBlock bd, BasicBlock bu | bd.contains(mkElement(d)) and bu.contains(u) |
+    not exists(BasicBlock bd, BasicBlock bu |
+      bd.contains(mkElement(d).(ControlFlowNode)) and bu.contains(u)
+    |
       bbStrictlyDominates(bd, bu)
       or
       exists(int i, int j |

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_path_to_sink/dispatch.cpp

@@ -4,8 +4,8 @@ using SinkFunction = void (*)(int);
 
 void notSink(int notSinkParam);
 
-void callsSink(int sinkParam) { // $ ir-path=31:28 ir-path=32:31 ir-path=34:22
-  sink(sinkParam); // $ ir-sink=31:28 ir-sink=32:31 ir-sink=34:22 ast=31:28 ast=32:31 ast=34:22 MISSING: ast,ir=28
+void callsSink(int sinkParam) {
+  sink(sinkParam); // $ ast,ir=31:28 ast,ir=32:31 ast,ir=34:22 MISSING: ast,ir=28
 }
 
 struct {
@@ -25,13 +25,13 @@ void assignGlobals() {
 };
 
 void testStruct() {
-  globalStruct.sinkPtr(atoi(getenv("TAINTED"))); // $ ir MISSING: ast
+  globalStruct.sinkPtr(atoi(getenv("TAINTED"))); // $ MISSING: ast,ir
   globalStruct.notSinkPtr(atoi(getenv("TAINTED"))); // clean
 
-  globalUnion.sinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path
-  globalUnion.notSinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path
+  globalUnion.sinkPtr(atoi(getenv("TAINTED"))); // $ ast,ir
+  globalUnion.notSinkPtr(atoi(getenv("TAINTED"))); // $ ast,ir
 
-  globalSinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path
+  globalSinkPtr(atoi(getenv("TAINTED"))); // $ ast,ir
 }
 
 class B {
@@ -48,19 +48,19 @@ class D2 : public D1 {
 
 class D3 : public D2 {
     public:
-    void f(const char* p) override { // $ ir-path=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
-        sink(p); // $ ir-sink=58:10 ir-sink=60:17 ir-sink=61:28 ir-sink=62:29 ir-sink=63:33 ast=58:10 ast=60:17 ast=61:28 ast=62:29 ast=63:33 SPURIOUS: ast=73:30 ir-sink=73:30
+    void f(const char* p) override {
+        sink(p); // $ ast,ir=58:10 ast,ir=60:17 ast,ir=61:28 ast,ir=62:29 ast,ir=63:33 SPURIOUS: ast,ir=73:30
     }
 };
 
 void test_dynamic_cast() {
     B* b = new D3();
-    b->f(getenv("VAR")); // $ ast ir-path
+    b->f(getenv("VAR")); // $ ast,ir
 
-    ((D2*)b)->f(getenv("VAR")); // $ ast ir-path
-    static_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path
-    dynamic_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path
-    reinterpret_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path
+    ((D2*)b)->f(getenv("VAR")); // $ ast,ir
+    static_cast<D2*>(b)->f(getenv("VAR")); // $ ast,ir
+    dynamic_cast<D2*>(b)->f(getenv("VAR")); // $ ast,ir
+    reinterpret_cast<D2*>(b)->f(getenv("VAR")); // $ ast,ir
 
     B* b2 = new D2();
     b2->f(getenv("VAR"));
@@ -70,5 +70,5 @@ void test_dynamic_cast() {
     dynamic_cast<D2*>(b2)->f(getenv("VAR"));
     reinterpret_cast<D2*>(b2)->f(getenv("VAR"));
 
-    dynamic_cast<D3*>(b2)->f(getenv("VAR")); // $ SPURIOUS: ast ir-path
+    dynamic_cast<D3*>(b2)->f(getenv("VAR")); // $ SPURIOUS: ast,ir
 }

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_path_to_sink/tainted.ql

@@ -7,10 +7,9 @@ import cpp
 import semmle.code.cpp.security.TaintTrackingImpl as ASTTaintTracking
 import semmle.code.cpp.ir.dataflow.DefaultTaintTracking as IRDefaultTaintTracking
 import IRDefaultTaintTracking::TaintedWithPath as TaintedWithPath
-import TaintedWithPath::Private
 import TestUtilities.InlineExpectationsTest
 
-predicate isSinkArgument(Element sink) {
+predicate isSink(Element sink) {
   exists(FunctionCall call |
     call.getTarget().getName() = "sink" and
     sink = call.getAnArgument()
@@ -20,34 +19,31 @@ predicate isSinkArgument(Element sink) {
 predicate astTaint(Expr source, Element sink) { ASTTaintTracking::tainted(source, sink) }
 
 class SourceConfiguration extends TaintedWithPath::TaintTrackingConfiguration {
-  override predicate isSink(Element e) { isSinkArgument(e) }
+  override predicate isSink(Element e) { any() }
 }
 
-predicate irTaint(Element source, Element sink, string tag) {
-  exists(TaintedWithPath::PathNode sinkNode, TaintedWithPath::PathNode predNode |
-    TaintedWithPath::taintedWithPath(source, _, _, sinkNode) and
-    predNode = getAPredecessor*(sinkNode) and
-    sink = getElementFromPathNode(predNode) and
-    // Make sure the path is actually reachable from this predecessor.
-    // Otherwise, we could pick `predNode` to be b when `source` is
-    // `source1` in this dataflow graph:
-    // source1 ---> a ---> c ---> sinkNode
-    //                   ^
-    // source2 ---> b --/
-    source = getElementFromPathNode(getAPredecessor*(predNode)) and
-    if sinkNode = predNode then tag = "ir-sink" else tag = "ir-path"
-  )
+predicate irTaint(Expr source, Element sink) {
+  TaintedWithPath::taintedWithPath(source, sink, _, _)
 }
 
 class IRDefaultTaintTrackingTest extends InlineExpectationsTest {
   IRDefaultTaintTrackingTest() { this = "IRDefaultTaintTrackingTest" }
 
-  override string getARelevantTag() { result = ["ir-path", "ir-sink"] }
+  override string getARelevantTag() { result = "ir" }
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
-    exists(Element source, Element tainted, int n |
-      irTaint(source, tainted, tag) and
-      n = strictcount(Element otherSource | irTaint(otherSource, tainted, _)) and
+    exists(Expr source, Element tainted, int n |
+      tag = "ir" and
+      irTaint(source, tainted) and
+      (
+        isSink(tainted)
+        or
+        exists(Element sink |
+          isSink(sink) and
+          irTaint(tainted, sink)
+        )
+      ) and
+      n = strictcount(Expr otherSource | irTaint(otherSource, tainted)) and
       (
         n = 1 and value = ""
         or
@@ -74,10 +70,10 @@ class ASTTaintTrackingTest extends InlineExpectationsTest {
       tag = "ast" and
       astTaint(source, tainted) and
       (
-        isSinkArgument(tainted)
+        isSink(tainted)
         or
         exists(Element sink |
-          isSinkArgument(sink) and
+          isSink(sink) and
           astTaint(tainted, sink)
         )
       ) and

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_path_to_sink/test_diff.cpp

@@ -13,8 +13,8 @@ struct S {
     }
 };
 
-void calls_sink_with_argv(const char* a) { // $ ir-path=96:26 ir-path=98:18
-    sink(a); // $ ast=96:26 ast=98:18 ir-sink=96:26 ir-sink=98:18
+void calls_sink_with_argv(const char* a) {
+    sink(a); // $ ast,ir=96:26 ast,ir=98:18
 }
 
 extern int i;
@@ -26,8 +26,8 @@ public:
 
 class DerivedCallsSink : public BaseWithPureVirtual {
 public:
-    void f(const char* p) override { // $ ir-path
-        sink(p); // $ ir-sink ast=108:10 SPURIOUS: ast=111:10
+    void f(const char* p) override {
+        sink(p); // $ ir ast=108:10 SPURIOUS: ast=111:10
     }
 };
 
@@ -38,8 +38,8 @@ public:
 
 class DerivedCallsSinkDiamond1 : virtual public BaseWithPureVirtual {
 public:
-    void f(const char* p) override { // $ ir-path
-        sink(p); // $ ast ir-sink
+    void f(const char* p) override {
+        sink(p); // $ ast,ir
     }
 };
 
@@ -49,7 +49,7 @@ public:
 };
 
 class DerivesMultiple : public DerivedCallsSinkDiamond1, public DerivedDoesNotCallSinkDiamond2 {
-    void f(const char* p) override { // $ ir-path
+    void f(const char* p) override {
         DerivedCallsSinkDiamond1::f(p);
     }
 };
@@ -57,15 +57,15 @@ class DerivesMultiple : public DerivedCallsSinkDiamond1, public DerivedDoesNotCa
 template<typename T>
 class CRTP {
 public:
-    void f(const char* p) { // $ ir-path
+    void f(const char* p) {
         static_cast<T*>(this)->g(p);
     }
 };
 
 class CRTPCallsSink : public CRTP<CRTPCallsSink> {
     public:
-    void g(const char* p) { // $ ir-path
-        sink(p); // $ ast ir-sink
+    void g(const char* p) {
+        sink(p); // $ ast,ir
     }
 };
 
@@ -78,8 +78,8 @@ class Derived2 : public Derived1 {
 
 class Derived3 : public Derived2 {
     public:
-    void f(const char* p) override { // $ ir-path=124:19 ir-path=126:43 ir-path=128:44
-        sink(p); // $ ast,ir-sink=124:19 ast,ir-sink=126:43 ast,ir-sink=128:44
+    void f(const char* p) override {
+        sink(p); // $ ast,ir=124:19 ast,ir=126:43 ast,ir=128:44
     }
 };
 
@@ -89,41 +89,41 @@ class CRTPDoesNotCallSink : public CRTP<CRTPDoesNotCallSink> {
 };
 
 int main(int argc, char *argv[]) {
-    sink(argv[0]); // $ ast,ir-path,ir-sink
+    sink(argv[0]); // $ ast,ir
 
-    sink(reinterpret_cast<int>(argv)); // $ ast,ir-sink
+    sink(reinterpret_cast<int>(argv)); // $ ast,ir
 
-    calls_sink_with_argv(argv[1]); // $ ast,ir-path
+    calls_sink_with_argv(argv[1]); // $ ast,ir
 
-    char*** p = &argv; // $ ast,ir-path
+    char*** p = &argv; // $ ast,ir
 
-    sink(*p[0]); // $ ast,ir-sink
+    sink(*p[0]); // $ ast,ir
 
-    calls_sink_with_argv(*p[i]); // $ MISSING: ast,ir-path
+    calls_sink_with_argv(*p[i]); // $ MISSING: ast,ir
 
-    sink(*(argv + 1)); // $ ast,ir-path ir-sink
+    sink(*(argv + 1)); // $ ast,ir
 
     BaseWithPureVirtual* b = new DerivedCallsSink;
 
-    b->f(argv[1]); // $ ast,ir-path
+    b->f(argv[1]); // $ ast,ir
 
     b = new DerivedDoesNotCallSink;
     b->f(argv[0]); // $ SPURIOUS: ast
 
     BaseWithPureVirtual* b2 = new DerivesMultiple;
 
-    b2->f(argv[i]); // $ ast,ir-path
+    b2->f(argv[i]); // $ ast,ir
 
     CRTP<CRTPDoesNotCallSink> crtp_not_call_sink;
     crtp_not_call_sink.f(argv[0]); // clean
 
     CRTP<CRTPCallsSink> crtp_calls_sink;
-    crtp_calls_sink.f(argv[0]); // $ ast,ir-path
+    crtp_calls_sink.f(argv[0]); // $ ast,ir
 
     Derived1* calls_sink = new Derived3;
-    calls_sink->f(argv[1]); // $ ast,ir-path
+    calls_sink->f(argv[1]); // $ ast,ir
 
-    static_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast,ir-path
+    static_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast,ir
 
-    dynamic_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast,ir-path
+    dynamic_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast,ir
 }