Merge pull request #4839 from github/docs/css-fixes-126

[CodeQL docs] Fix two CSS bugs (rc/1.26)

.github/workflows/generate-query-help-docs.yml

@@ -1,19 +1,16 @@
 name: Generate CodeQL query help documentation using Sphinx
 
 on:
-  workflow_dispatch:
-      inputs:
-        description:
-          description: A description of the purpose of this job. For human consumption.
-          required: false
   push:
     branches:
+     - main
+     - 'rc/**'
      - 'lgtm.com'
   pull_request:
     paths:
       - '.github/workflows/generate-query-help-docs.yml'
       - 'docs/codeql/query-help/**'
-      
+
 jobs:
   build:
     runs-on: ubuntu-latest
@@ -21,7 +18,7 @@ jobs:
     - name: Clone github/codeql
       uses: actions/checkout@v2
       with:
-        path: codeql
+        path: codeql 
     - name: Clone github/codeql-go
       uses: actions/checkout@v2
       with:

config/identical-files.json

@@ -358,14 +358,6 @@
     "cpp/ql/test/TestUtilities/InlineExpectationsTest.qll",
     "python/ql/test/TestUtilities/InlineExpectationsTest.qll"
   ],
-  "C++ ExternalAPIs": [
-    "cpp/ql/src/Security/CWE/CWE-020/ExternalAPIs.qll",
-    "cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIs.qll"
-  ],
-  "C++ SafeExternalAPIFunction": [
-    "cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll",
-    "cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll"
-  ],
   "XML": [
     "cpp/ql/src/semmle/code/cpp/XML.qll",
     "csharp/ql/src/semmle/code/csharp/XML.qll",
@@ -417,12 +409,5 @@
     "java/ql/src/Metrics/Files/CommentedOutCodeReferences.qhelp",
     "javascript/ql/src/Comments/CommentedOutCodeReferences.qhelp",
     "python/ql/src/Lexical/CommentedOutCodeReferences.qhelp"
-  ],
-  "IDE Contextual Queries": [
-    "cpp/ql/src/IDEContextual.qll",
-    "csharp/ql/src/IDEContextual.qll",
-    "java/ql/src/IDEContextual.qll",
-    "javascript/ql/src/IDEContextual.qll",
-    "python/ql/src/analysis/IDEContextual.qll"
   ]
 }

cpp/change-notes/2020-11-02-unused-local-variable.md

@@ -1,2 +0,0 @@
-lgtm,codescanning
-* Two issues causing the 'Unused local variable' query (`cpp/unused-local-variable`) to produce false positive results have been fixed.

cpp/change-notes/2020-11-05-formatting-function.md

@@ -1,4 +0,0 @@
-lgtm,codescanning
-* `FormattingFunction.getOutputParameterIndex` now has a parameter identifying whether the output at that index is a buffer or a stream.
-* `FormattingFunction` now has a predicate `isOutputGlobal` indicating when the output is to a global stream.
-* The `primitiveVariadicFormatter` and `variadicFormatter` predicates have more parameters exposing information about the function.

cpp/change-notes/2020-11-05-private-models.md

@@ -1,3 +0,0 @@
-lgtm,codescanning
-* Various classes in `semmle.code.cpp.models.implementations` have been made private. Users should not depend on library implementation details.
-* The `OperatorNewAllocationFunction`, `OperatorDeleteDeallocationFunction`, `Iterator` and `Snprintf` classes now have interfaces in `semmle.code.cpp.models.interfaces`.

cpp/change-notes/2020-11-12-unsafe-use-of-this.md

@@ -1,2 +0,0 @@
-lgtm,codescanning
-* A new query (`cpp/unsafe-use-of-this`) has been added. The query finds pure virtual function calls whose qualifier is an object under construction.

cpp/change-notes/2020-11-27-downgrade-to-recommendation.md

@@ -1,2 +0,0 @@
-lgtm,codescanning
-* The queries `cpp/local-variable-hides-global-variable` and `cpp/missing-header-guard` now have severity `recommendation` instead of `warning`.

cpp/config/suites/cpp/correctness

@@ -9,7 +9,6 @@
 + semmlecode-cpp-queries/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /Correctness/Dangerous Conversions
-+ semmlecode-cpp-queries/Likely Bugs/OO/UnsafeUseOfThis.ql: /Correctness/Dangerous Conversions
   # Consistent Use
 + semmlecode-cpp-queries/Critical/ReturnValueIgnored.ql: /Correctness/Consistent Use
 + semmlecode-cpp-queries/Likely Bugs/InconsistentCheckReturnNull.ql: /Correctness/Consistent Use

cpp/ql/src/Best Practices/Hiding/LocalVariableHidesGlobalVariable.ql

@@ -2,7 +2,7 @@
  * @name Local variable hides global variable
  * @description A local variable or parameter that hides a global variable of the same name. This may be confusing. Consider renaming one of the variables.
  * @kind problem
- * @problem.severity recommendation
+ * @problem.severity warning
  * @precision very-high
  * @id cpp/local-variable-hides-global-variable
  * @tags maintainability

cpp/ql/src/Best Practices/Magic Constants/MagicConstants.qll

@@ -8,41 +8,168 @@ import semmle.code.cpp.AutogeneratedFile
 predicate trivialPositiveIntValue(string s) {
   // Small numbers
   s = [0 .. 20].toString() or
-  s =
-    [
-      // Popular powers of two (decimal)
-      "16", "24", "32", "64", "128", "256", "512", "1024", "2048", "4096", "16384", "32768",
-      "65536", "1048576", "2147483648", "4294967296",
-      // Popular powers of two, minus one (decimal)
-      "15", "31", "63", "127", "255", "511", "1023", "2047", "4095", "16383", "32767", "65535",
-      "1048577", "2147483647", "4294967295",
-      // Popular powers of two (32-bit hex)
-      "0x00000001", "0x00000002", "0x00000004", "0x00000008", "0x00000010", "0x00000020",
-      "0x00000040", "0x00000080", "0x00000100", "0x00000200", "0x00000400", "0x00000800",
-      "0x00001000", "0x00002000", "0x00004000", "0x00008000", "0x00010000", "0x00020000",
-      "0x00040000", "0x00080000", "0x00100000", "0x00200000", "0x00400000", "0x00800000",
-      "0x01000000", "0x02000000", "0x04000000", "0x08000000", "0x10000000", "0x20000000",
-      "0x40000000", "0x80000000",
-      // Popular powers of two, minus one (32-bit hex)
-      "0x00000001", "0x00000003", "0x00000007", "0x0000000f", "0x0000001f", "0x0000003f",
-      "0x0000007f", "0x000000ff", "0x000001ff", "0x000003ff", "0x000007ff", "0x00000fff",
-      "0x00001fff", "0x00003fff", "0x00007fff", "0x0000ffff", "0x0001ffff", "0x0003ffff",
-      "0x0007ffff", "0x000fffff", "0x001fffff", "0x003fffff", "0x007fffff", "0x00ffffff",
-      "0x01ffffff", "0x03ffffff", "0x07ffffff", "0x0fffffff", "0x1fffffff", "0x3fffffff",
-      "0x7fffffff", "0xffffffff",
-      // Popular powers of two (16-bit hex)
-      "0x0001", "0x0002", "0x0004", "0x0008", "0x0010", "0x0020", "0x0040", "0x0080", "0x0100",
-      "0x0200", "0x0400", "0x0800", "0x1000", "0x2000", "0x4000", "0x8000",
-      // Popular powers of two, minus one (16-bit hex)
-      "0x0001", "0x0003", "0x0007", "0x000f", "0x001f", "0x003f", "0x007f", "0x00ff", "0x01ff",
-      "0x03ff", "0x07ff", "0x0fff", "0x1fff", "0x3fff", "0x7fff", "0xffff",
-      // Popular powers of two (8-bit hex)
-      "0x01", "0x02", "0x04", "0x08", "0x10", "0x20", "0x40", "0x80",
-      // Popular powers of two, minus one (8-bit hex)
-      "0x01", "0x03", "0x07", "0x0f", "0x1f", "0x3f", "0x7f", "0xff", "0x00",
-      // Powers of ten
-      "10", "100", "1000", "10000", "100000", "1000000", "10000000", "100000000", "1000000000"
-    ]
+  // Popular powers of two (decimal)
+  s = "16" or
+  s = "24" or
+  s = "32" or
+  s = "64" or
+  s = "128" or
+  s = "256" or
+  s = "512" or
+  s = "1024" or
+  s = "2048" or
+  s = "4096" or
+  s = "16384" or
+  s = "32768" or
+  s = "65536" or
+  s = "1048576" or
+  s = "2147483648" or
+  s = "4294967296" or
+  // Popular powers of two, minus one (decimal)
+  s = "15" or
+  s = "31" or
+  s = "63" or
+  s = "127" or
+  s = "255" or
+  s = "511" or
+  s = "1023" or
+  s = "2047" or
+  s = "4095" or
+  s = "16383" or
+  s = "32767" or
+  s = "65535" or
+  s = "1048577" or
+  s = "2147483647" or
+  s = "4294967295" or
+  // Popular powers of two (32-bit hex)
+  s = "0x00000001" or
+  s = "0x00000002" or
+  s = "0x00000004" or
+  s = "0x00000008" or
+  s = "0x00000010" or
+  s = "0x00000020" or
+  s = "0x00000040" or
+  s = "0x00000080" or
+  s = "0x00000100" or
+  s = "0x00000200" or
+  s = "0x00000400" or
+  s = "0x00000800" or
+  s = "0x00001000" or
+  s = "0x00002000" or
+  s = "0x00004000" or
+  s = "0x00008000" or
+  s = "0x00010000" or
+  s = "0x00020000" or
+  s = "0x00040000" or
+  s = "0x00080000" or
+  s = "0x00100000" or
+  s = "0x00200000" or
+  s = "0x00400000" or
+  s = "0x00800000" or
+  s = "0x01000000" or
+  s = "0x02000000" or
+  s = "0x04000000" or
+  s = "0x08000000" or
+  s = "0x10000000" or
+  s = "0x20000000" or
+  s = "0x40000000" or
+  s = "0x80000000" or
+  // Popular powers of two, minus one (32-bit hex)
+  s = "0x00000001" or
+  s = "0x00000003" or
+  s = "0x00000007" or
+  s = "0x0000000f" or
+  s = "0x0000001f" or
+  s = "0x0000003f" or
+  s = "0x0000007f" or
+  s = "0x000000ff" or
+  s = "0x000001ff" or
+  s = "0x000003ff" or
+  s = "0x000007ff" or
+  s = "0x00000fff" or
+  s = "0x00001fff" or
+  s = "0x00003fff" or
+  s = "0x00007fff" or
+  s = "0x0000ffff" or
+  s = "0x0001ffff" or
+  s = "0x0003ffff" or
+  s = "0x0007ffff" or
+  s = "0x000fffff" or
+  s = "0x001fffff" or
+  s = "0x003fffff" or
+  s = "0x007fffff" or
+  s = "0x00ffffff" or
+  s = "0x01ffffff" or
+  s = "0x03ffffff" or
+  s = "0x07ffffff" or
+  s = "0x0fffffff" or
+  s = "0x1fffffff" or
+  s = "0x3fffffff" or
+  s = "0x7fffffff" or
+  s = "0xffffffff" or
+  // Popular powers of two (16-bit hex)
+  s = "0x0001" or
+  s = "0x0002" or
+  s = "0x0004" or
+  s = "0x0008" or
+  s = "0x0010" or
+  s = "0x0020" or
+  s = "0x0040" or
+  s = "0x0080" or
+  s = "0x0100" or
+  s = "0x0200" or
+  s = "0x0400" or
+  s = "0x0800" or
+  s = "0x1000" or
+  s = "0x2000" or
+  s = "0x4000" or
+  s = "0x8000" or
+  // Popular powers of two, minus one (16-bit hex)
+  s = "0x0001" or
+  s = "0x0003" or
+  s = "0x0007" or
+  s = "0x000f" or
+  s = "0x001f" or
+  s = "0x003f" or
+  s = "0x007f" or
+  s = "0x00ff" or
+  s = "0x01ff" or
+  s = "0x03ff" or
+  s = "0x07ff" or
+  s = "0x0fff" or
+  s = "0x1fff" or
+  s = "0x3fff" or
+  s = "0x7fff" or
+  s = "0xffff" or
+  // Popular powers of two (8-bit hex)
+  s = "0x01" or
+  s = "0x02" or
+  s = "0x04" or
+  s = "0x08" or
+  s = "0x10" or
+  s = "0x20" or
+  s = "0x40" or
+  s = "0x80" or
+  // Popular powers of two, minus one (8-bit hex)
+  s = "0x01" or
+  s = "0x03" or
+  s = "0x07" or
+  s = "0x0f" or
+  s = "0x1f" or
+  s = "0x3f" or
+  s = "0x7f" or
+  s = "0xff" or
+  s = "0x00" or
+  // Powers of ten
+  s = "10" or
+  s = "100" or
+  s = "1000" or
+  s = "10000" or
+  s = "100000" or
+  s = "1000000" or
+  s = "10000000" or
+  s = "100000000" or
+  s = "1000000000"
 }
 
 predicate trivialIntValue(string s) {
@@ -108,7 +235,10 @@ predicate joiningStringTrivial(Literal lit) {
   // understand (which is against the spirit of these queries).
   stringLiteral(lit) and
   exists(FunctionCall fc |
-    fc.getTarget().getName() = ["operator+", "operator<<"] and
+    (
+      fc.getTarget().getName() = "operator+" or
+      fc.getTarget().getName() = "operator<<"
+    ) and
     fc.getAnArgument().getAChild*() = lit
   ) and
   lit.getValue().length() < 16
@@ -161,7 +291,8 @@ predicate arrayInitializerChild(AggregateLiteral parent, Expr e) {
 
 // i.e. not a constant folded expression
 predicate literallyLiteral(Literal lit) {
-  lit.getValueText()
+  lit
+      .getValueText()
       .regexpMatch(".*\".*|\\s*+[-+]?+\\s*+(0[xob][0-9a-fA-F]|[0-9])[0-9a-fA-F,._]*+([eE][-+]?+[0-9,._]*+)?+\\s*+[a-zA-Z]*+\\s*+")
 }
 

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

@@ -57,12 +57,5 @@ where
   not declarationHasSideEffects(v) and
   not exists(AsmStmt s | f = s.getEnclosingFunction()) and
   not v.getAnAttribute().getName() = "unused" and
-  not any(ErrorExpr e).getEnclosingFunction() = f and // unextracted expr may use `v`
-  not exists(
-    Literal l // this case can be removed when the `myFunction2( [obj](){} );` test case doesn't depend on this exclusion
-  |
-    l.getEnclosingFunction() = f and
-    not exists(l.getValue())
-  ) and
-  not any(ConditionDeclExpr cde).getEnclosingFunction() = f // this case can be removed when the `if (a = b; a)` test case doesn't depend on this exclusion
+  not any(ErrorExpr e).getEnclosingFunction() = f // unextracted expr likely used `v`
 select v, "Variable " + v.getName() + " is not used"

cpp/ql/src/Critical/NewDelete.qll

@@ -5,6 +5,8 @@
 import cpp
 import semmle.code.cpp.controlflow.SSA
 import semmle.code.cpp.dataflow.DataFlow
+import semmle.code.cpp.models.implementations.Allocation
+import semmle.code.cpp.models.implementations.Deallocation
 
 /**
  * Holds if `alloc` is a use of `malloc` or `new`.  `kind` is

cpp/ql/src/DefaultOptions.qll

@@ -59,9 +59,14 @@ class Options extends string {
   predicate exits(Function f) {
     f.getAnAttribute().hasName("noreturn")
     or
-    f.hasGlobalOrStdName([
-        "exit", "_exit", "abort", "__assert_fail", "longjmp", "__builtin_unreachable"
-      ])
+    exists(string name | f.hasGlobalOrStdName(name) |
+      name = "exit" or
+      name = "_exit" or
+      name = "abort" or
+      name = "__assert_fail" or
+      name = "longjmp" or
+      name = "__builtin_unreachable"
+    )
     or
     CustomOptions::exits(f) // old Options.qll
   }

cpp/ql/src/IDEContextual.qll

@@ -1,22 +0,0 @@
-/**
- * Provides shared predicates related to contextual queries in the code viewer.
- */
-
-import semmle.files.FileSystem
-
-/**
- * Returns the `File` matching the given source file name as encoded by the VS
- * Code extension.
- */
-cached
-File getFileBySourceArchiveName(string name) {
-  // The name provided for a file in the source archive by the VS Code extension
-  // has some differences from the absolute path in the database:
-  // 1. colons are replaced by underscores
-  // 2. there's a leading slash, even for Windows paths: "C:/foo/bar" ->
-  //    "/C_/foo/bar"
-  // 3. double slashes in UNC prefixes are replaced with a single slash
-  // We can handle 2 and 3 together by unconditionally adding a leading slash
-  // before replacing double slashes.
-  name = ("/" + result.getAbsolutePath().replaceAll(":", "_")).replaceAll("//", "/")
-}

cpp/ql/src/JPL_C/LOC-2/Rule 05/HeapMemory.ql

@@ -21,7 +21,15 @@ class Initialization extends Function {
 }
 
 class Allocation extends FunctionCall {
-  Allocation() { this.getTarget().getName() = ["malloc", "calloc", "alloca", "sbrk", "valloc"] }
+  Allocation() {
+    exists(string name | name = this.getTarget().getName() |
+      name = "malloc" or
+      name = "calloc" or
+      name = "alloca" or
+      name = "sbrk" or
+      name = "valloc"
+    )
+  }
 }
 
 from Function f, Allocation a

cpp/ql/src/JPL_C/LOC-2/Rule 07/ThreadSafety.ql

@@ -13,8 +13,13 @@ import cpp
 
 class ForbiddenCall extends FunctionCall {
   ForbiddenCall() {
-    this.getTarget().getName() =
-      ["task_delay", "taskDelay", "sleep", "nanosleep", "clock_nanosleep"]
+    exists(string name | name = this.getTarget().getName() |
+      name = "task_delay" or
+      name = "taskDelay" or
+      name = "sleep" or
+      name = "nanosleep" or
+      name = "clock_nanosleep"
+    )
   }
 }
 

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

@@ -6,7 +6,12 @@ import cpp
 
 class SemaphoreCreation extends FunctionCall {
   SemaphoreCreation() {
-    this.getTarget().getName() = ["semBCreate", "semMCreate", "semCCreate", "semRWCreate"]
+    exists(string name | name = this.getTarget().getName() |
+      name = "semBCreate" or
+      name = "semMCreate" or
+      name = "semCCreate" or
+      name = "semRWCreate"
+    )
   }
 
   Variable getSemaphore() { result.getAnAccess() = this.getParent().(Assignment).getLValue() }
@@ -67,7 +72,11 @@ class SemaphoreGive extends UnlockOperation {
 }
 
 class LockingPrimitive extends FunctionCall, LockOperation {
-  LockingPrimitive() { this.getTarget().getName() = ["taskLock", "intLock", "taskRtpLock"] }
+  LockingPrimitive() {
+    exists(string name | name = this.getTarget().getName() |
+      name = "taskLock" or name = "intLock" or name = "taskRtpLock"
+    )
+  }
 
   override Function getLocked() { result = this.getTarget() }
 
@@ -80,7 +89,11 @@ class LockingPrimitive extends FunctionCall, LockOperation {
 }
 
 class UnlockingPrimitive extends FunctionCall, UnlockOperation {
-  UnlockingPrimitive() { this.getTarget().getName() = ["taskUnlock", "intUnlock", "taskRtpUnlock"] }
+  UnlockingPrimitive() {
+    exists(string name | name = this.getTarget().getName() |
+      name = "taskUnlock" or name = "intUnlock" or name = "taskRtpUnlock"
+    )
+  }
 
   Function getLocked() { result = getMatchingLock().getLocked() }
 

cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql

@@ -12,7 +12,18 @@
 import cpp
 
 predicate allowedTypedefs(TypedefType t) {
-  t.getName() = ["I64", "U64", "I32", "U32", "I16", "U16", "I8", "U8", "F64", "F32"]
+  exists(string name | name = t.getName() |
+    name = "I64" or
+    name = "U64" or
+    name = "I32" or
+    name = "U32" or
+    name = "I16" or
+    name = "U16" or
+    name = "I8" or
+    name = "U8" or
+    name = "F64" or
+    name = "F32"
+  )
 }
 
 /**

cpp/ql/src/JPL_C/Tasks.qll

@@ -5,8 +5,8 @@ import cpp
  */
 class Task extends Function {
   Task() {
-    exists(FunctionCall taskCreate |
-      taskCreate.getTarget().getName() = ["taskCreate", "taskSpawn"] and
+    exists(FunctionCall taskCreate, string name | name = "taskCreate" or name = "taskSpawn" |
+      name = taskCreate.getTarget().getName() and
       this = taskCreate.getArgument(4).(AddressOfExpr).getAddressable()
     )
   }

cpp/ql/src/Likely Bugs/Conversion/LossyFunctionResultCast.ql

@@ -13,17 +13,38 @@ import cpp
 import semmle.code.cpp.dataflow.DataFlow
 
 predicate whitelist(Function f) {
-  f.getName() =
-    [
-      "ceil", "ceilf", "ceill", "floor", "floorf", "floorl", "nearbyint", "nearbyintf",
-      "nearbyintl", "rint", "rintf", "rintl", "round", "roundf", "roundl", "trunc", "truncf",
-      "truncl"
-    ] or
-  f.getName().matches("__builtin_%")
+  exists(string fName |
+    fName = f.getName() and
+    (
+      fName = "ceil" or
+      fName = "ceilf" or
+      fName = "ceill" or
+      fName = "floor" or
+      fName = "floorf" or
+      fName = "floorl" or
+      fName = "nearbyint" or
+      fName = "nearbyintf" or
+      fName = "nearbyintl" or
+      fName = "rint" or
+      fName = "rintf" or
+      fName = "rintl" or
+      fName = "round" or
+      fName = "roundf" or
+      fName = "roundl" or
+      fName = "trunc" or
+      fName = "truncf" or
+      fName = "truncl" or
+      fName.matches("__builtin_%")
+    )
+  )
 }
 
 predicate whitelistPow(FunctionCall fc) {
-  fc.getTarget().getName() = ["pow", "powf", "powl"] and
+  (
+    fc.getTarget().getName() = "pow" or
+    fc.getTarget().getName() = "powf" or
+    fc.getTarget().getName() = "powl"
+  ) and
   exists(float value |
     value = fc.getArgument(0).getValue().toFloat() and
     (value.floor() - value).abs() < 0.001

cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql

@@ -50,12 +50,10 @@ class SafeTimeGatheringFunction extends Function {
 class TimeConversionFunction extends Function {
   TimeConversionFunction() {
     this.getQualifiedName() =
-      [
-        "FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime",
-        "SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime",
-        "TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime",
-        "RtlTimeToSecondsSince1970", "_mkgmtime"
-      ]
+      ["FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime",
+          "SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime",
+          "TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime",
+          "RtlTimeToSecondsSince1970", "_mkgmtime"]
   }
 }
 

cpp/ql/src/Likely Bugs/OO/UnsafeUseOfThis.cpp

@@ -1,20 +0,0 @@
-class Base {
-private:
-    // pure virtual member function used for initialization of derived classes.
-    virtual void construct() = 0;
-public:
-    Base() {
-        // wrong: the virtual table of `Derived` has not been initialized yet. So this
-        // call will resolve to `Base::construct`, which cannot be called as it is a pure
-        // virtual function.
-        construct();
-    }
-};
-
-class Derived : public Base {
-    int field;
-
-    void construct() override {
-        field = 1;
-    }
-};

cpp/ql/src/Likely Bugs/OO/UnsafeUseOfThis.qhelp

@@ -1,30 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-
-
-<overview>
-<p>This rule finds calls to pure virtual member functions in constructors and destructors. When executing the body of a constructor of class <code>T</code>, the virtual table of <code>T</code> refers to the virtual table of one of <code>T</code>'s base classes. This can produce unexpected behavior, including program abort that can lead to denial of service attacks. The same problem exists during destruction of an object.</p>
-
-</overview>
-<recommendation>
-<p>Do not rely on virtual dispatch in constructors and destructors. Instead, each class should be responsible for acquiring and releasing its resources. If a base class needs to refer to a derived class during initialization, use the Dynamic Binding During Initialization idiom.</p>
-
-</recommendation>
-<example><sample src="UnsafeUseOfThis.cpp" />
-
-
-
-</example>
-<references>
-
-<li>ISO C++ FAQ: <a href="https://isocpp.org/wiki/faq/strange-inheritance#calling-virtuals-from-ctors">When my base class's constructor calls a virtual function on its this object, why doesn't my derived class's override of that virtual function get invoked?</a>
-</li>
-<li>SEI CERT C++ Coding Standard <a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP50-CPP.+Do+not+invoke+virtual+functions+from+constructors+or+destructors">OOP50-CPP. Do not invoke virtual functions from constructors or destructors</a>
-</li>
-<li>ISO C++ FAQ: <a href="https://isocpp.org/wiki/faq/strange-inheritance#calling-virtuals-from-ctor-idiom">Okay, but is there a way to simulate that behavior as if dynamic binding worked on the this object within my base class's constructor?</a>
-</li>
-
-
-</references></qhelp>

cpp/ql/src/Likely Bugs/OO/UnsafeUseOfThis.ql

@@ -1,212 +0,0 @@
-/**
- * @name Unsafe use of this in constructor
- * @description A call to a pure virtual function using a 'this'
- *              pointer of an object that is under construction
- *              may lead to undefined behavior.
- * @kind path-problem
- * @id cpp/unsafe-use-of-this
- * @problem.severity error
- * @precision very-high
- * @tags correctness
- *       language-features
- *       security
- */
-
-import cpp
-// We don't actually use the global value numbering library in this query, but without it we end up
-// recomputing the IR.
-private import semmle.code.cpp.valuenumbering.GlobalValueNumbering
-private import semmle.code.cpp.ir.IR
-
-bindingset[n, result]
-int unbind(int n) { result >= n and result <= n }
-
-/** Holds if `p` is the `n`'th parameter of the non-virtual function `f`. */
-predicate parameterOf(Parameter p, Function f, int n) {
-  not f.isVirtual() and f.getParameter(n) = p
-}
-
-/**
- * Holds if `instr` is the `n`'th argument to a call to the non-virtual function `f`, and
- * `init` is the corresponding initiazation instruction that receives the value of `instr` in `f`.
- */
-predicate flowIntoParameter(
-  CallInstruction call, Instruction instr, Function f, int n, InitializeParameterInstruction init
-) {
-  not f.isVirtual() and
-  call.getPositionalArgument(n) = instr and
-  f = call.getStaticCallTarget() and
-  getEnclosingNonVirtualFunctionInitializeParameter(init, f) and
-  init.getParameter().getIndex() = unbind(n)
-}
-
-/**
- * Holds if `instr` is an argument to a call to the function `f`, and `init` is the
- * corresponding initialization instruction that receives the value of `instr` in `f`.
- */
-pragma[noinline]
-predicate getPositionalArgumentInitParam(
-  CallInstruction call, Instruction instr, InitializeParameterInstruction init, Function f
-) {
-  exists(int n |
-    parameterOf(_, f, n) and
-    flowIntoParameter(call, instr, f, unbind(n), init)
-  )
-}
-
-/**
- * Holds if `instr` is the qualifier to a call to the non-virtual function `f`, and
- * `init` is the corresponding initiazation instruction that receives the value of
- * `instr` in `f`.
- */
-pragma[noinline]
-predicate getThisArgumentInitParam(
-  CallInstruction call, Instruction instr, InitializeParameterInstruction init, Function f
-) {
-  not f.isVirtual() and
-  call.getStaticCallTarget() = f and
-  getEnclosingNonVirtualFunctionInitializeParameter(init, f) and
-  call.getThisArgument() = instr and
-  init.getIRVariable() instanceof IRThisVariable
-}
-
-/** Holds if `instr` is a `this` pointer used by the call instruction `call`. */
-predicate isSink(Instruction instr, CallInstruction call) {
-  exists(PureVirtualFunction func |
-    call.getStaticCallTarget() = func and
-    call.getThisArgument() = instr and
-    // Weed out implicit calls to destructors of a base class
-    not func instanceof Destructor
-  )
-}
-
-/** Holds if `init` initializes the `this` pointer in class `c`. */
-predicate isSource(InitializeParameterInstruction init, string msg, Class c) {
-  (
-    exists(Constructor func |
-      not func instanceof CopyConstructor and
-      not func instanceof MoveConstructor and
-      func = init.getEnclosingFunction() and
-      msg = "construction"
-    )
-    or
-    init.getEnclosingFunction() instanceof Destructor and msg = "destruction"
-  ) and
-  init.getIRVariable() instanceof IRThisVariable and
-  init.getEnclosingFunction().getDeclaringType() = c
-}
-
-/**
- * Holds if `instr` flows to a sink (which is a use of the value of `instr` as a `this` pointer).
- */
-predicate flowsToSink(Instruction instr, Instruction sink) {
-  flowsFromSource(instr) and
-  (
-    isSink(instr, _) and instr = sink
-    or
-    exists(Instruction mid |
-      successor(instr, mid) and
-      flowsToSink(mid, sink)
-    )
-  )
-}
-
-/** Holds if `instr` flows from a source. */
-predicate flowsFromSource(Instruction instr) {
-  isSource(instr, _, _)
-  or
-  exists(Instruction mid |
-    successor(mid, instr) and
-    flowsFromSource(mid)
-  )
-}
-
-/** Holds if `f` is the enclosing non-virtual function of `init`. */
-predicate getEnclosingNonVirtualFunctionInitializeParameter(
-  InitializeParameterInstruction init, Function f
-) {
-  not f.isVirtual() and
-  init.getEnclosingFunction() = f
-}
-
-/** Holds if `f` is the enclosing non-virtual function of `init`. */
-predicate getEnclosingNonVirtualFunctionInitializeIndirection(
-  InitializeIndirectionInstruction init, Function f
-) {
-  not f.isVirtual() and
-  init.getEnclosingFunction() = f
-}
-
-/**
- * Holds if `instr` is an argument (or argument indirection) to a call, and
- * `succ` is the corresponding initialization instruction in the call target.
- */
-predicate flowThroughCallable(Instruction instr, Instruction succ) {
-  // Flow from an argument to a parameter
-  exists(CallInstruction call, InitializeParameterInstruction init | init = succ |
-    getPositionalArgumentInitParam(call, instr, init, call.getStaticCallTarget())
-    or
-    getThisArgumentInitParam(call, instr, init, call.getStaticCallTarget())
-  )
-  or
-  // Flow from argument indirection to parameter indirection
-  exists(
-    CallInstruction call, ReadSideEffectInstruction read, InitializeIndirectionInstruction init
-  |
-    init = succ and
-    read.getPrimaryInstruction() = call and
-    getEnclosingNonVirtualFunctionInitializeIndirection(init, call.getStaticCallTarget())
-  |
-    exists(int n |
-      read.getSideEffectOperand().getAnyDef() = instr and
-      read.getIndex() = n and
-      init.getParameter().getIndex() = unbind(n)
-    )
-    or
-    call.getThisArgument() = instr and
-    init.getIRVariable() instanceof IRThisVariable
-  )
-}
-
-/** Holds if `instr` flows to `succ`. */
-predicate successor(Instruction instr, Instruction succ) {
-  succ.(CopyInstruction).getSourceValue() = instr or
-  succ.(CheckedConvertOrNullInstruction).getUnary() = instr or
-  succ.(ChiInstruction).getTotal() = instr or
-  succ.(ConvertInstruction).getUnary() = instr or
-  succ.(InheritanceConversionInstruction).getUnary() = instr or
-  flowThroughCallable(instr, succ)
-}
-
-/**
- * Holds if:
- * - `source` is an initialization of a `this` pointer of type `sourceClass`, and
- * - `sink` is a use of the `this` pointer, and
- * - `call` invokes a pure virtual function using `sink` as the `this` pointer, and
- * - `msg` is a string describing whether `source` is from a constructor or destructor.
- */
-predicate flows(
-  Instruction source, string msg, Class sourceClass, Instruction sink, CallInstruction call
-) {
-  isSource(source, msg, sourceClass) and
-  flowsToSink(source, sink) and
-  isSink(sink, call)
-}
-
-query predicate edges(Instruction a, Instruction b) { successor(a, b) and flowsToSink(b, _) }
-
-query predicate nodes(Instruction n, string key, string val) {
-  flowsToSink(n, _) and
-  key = "semmle.label" and
-  val = n.toString()
-}
-
-from Instruction source, Instruction sink, CallInstruction call, string msg, Class sourceClass
-where
-  flows(source, msg, sourceClass, sink, call) and
-  // Only raise an alert if there is no override of the pure virtual function in any base class.
-  not exists(Class c | c = sourceClass.getABaseClass*() |
-    c.getAMemberFunction().getAnOverriddenFunction() = call.getStaticCallTarget()
-  )
-select call.getUnconvertedResultExpression(), source, sink,
-  "Call to pure virtual function during " + msg

cpp/ql/src/Security/CWE/CWE-020/CountUntrustedDataToExternalAPI.qhelp

@@ -1,48 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
-all external APIs that are used with untrusted data, along with how frequently the API is used, and how many 
-unique sources of untrusted data flow to this API. This query is designed primarily to help identify which APIs 
-may be relevant for security analysis of this application.</p>
-
-<p>An external API is defined as a call to a function that is not defined in the source code, and is not
-modeled as a taint step in the default taint library. External APIs may be from the C++ standard library,
-third party dependencies or from internal dependencies. The query will report the function name, along with
-either <code>[param x]</code>, where <code>x</code> indicates the position of the parameter receiving the
-untrusted data or <code>[qualifier]</code> indicating the untrusted data is used as the qualifier to the
-function call.</p>
-
-</overview>
-<recommendation>
-
-<p>For each result:</p>
-
-<ul>
-  <li>If the result highlights a known sink, no action is required.</li>
-  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query.</li>
-  <li>If the result represents a call to an external API which transfers taint, add the appropriate modeling, and 
-  re-run the query to determine what new results have appeared due to this additional modeling.</li>
-</ul>
-
-<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIFunction</code> 
-class to exclude known safe external APIs from future analysis.</p>
-
-</recommendation>
-<example>
-
-<p>If the query were to return the API <code>fputs [param 1]</code> 
-then we should first consider whether this a security relevant sink. In this case, this is writing to a <code>FILE*</code>, so we should 
-consider whether this is an XSS sink. If it is, we should confirm that it is handled by the XSS query.</p>
-
-<p>If the query were to return the API <code>strcat [param 1]</code>, then this should be
-reviewed as a possible taint step, because tainted data would flow from the 1st argument to the 0th argument of the call.</p>
-
-<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
-</example>
-<references>
-
-</references>
-</qhelp>

cpp/ql/src/Security/CWE/CWE-020/CountUntrustedDataToExternalAPI.ql

@@ -1,17 +0,0 @@
-/**
- * @name Frequency counts for external APIs that are used with untrusted data
- * @description This reports the external APIs that are used with untrusted data, along with how
- *              frequently the API is called, and how many unique sources of untrusted data flow
- *              to it.
- * @id cpp/count-untrusted-data-external-api
- * @kind table
- * @tags security external/cwe/cwe-20
- */
-
-import cpp
-import ExternalAPIs
-
-from ExternalAPIUsedWithUntrustedData externalAPI
-select externalAPI, count(externalAPI.getUntrustedDataNode()) as numberOfUses,
-  externalAPI.getNumberOfUntrustedSources() as numberOfUntrustedSources order by
-    numberOfUntrustedSources desc

cpp/ql/src/Security/CWE/CWE-020/ExternalAPISinkExample.cpp

@@ -1,13 +0,0 @@
-#include <cstdio>
-
-void do_get(FILE* request, FILE* response) {
-  char page[1024];
-  fgets(page, 1024, request);
-
-  char buffer[1024];
-  strcat(buffer, "The page \"");
-  strcat(buffer, page);
-  strcat(buffer, "\" was not found.");
-
-  fputs(buffer, response);
-}

cpp/ql/src/Security/CWE/CWE-020/ExternalAPITaintStepExample.cpp

@@ -1,13 +0,0 @@
-#include <cstdio>
-
-void do_get(FILE* request, FILE* response) {
-  char user_id[1024];
-  fgets(user_id, 1024, request);
-
-  char buffer[1024];
-  strcat(buffer, "SELECT * FROM user WHERE user_id='");
-  strcat(buffer, user_id);
-  strcat(buffer, "'");
-
-  // ...
-}

cpp/ql/src/Security/CWE/CWE-020/ExternalAPIs.qll

@@ -1,50 +0,0 @@
-/**
- * Definitions for reasoning about untrusted data used in APIs defined outside the
- * database.
- */
-
-private import cpp
-private import semmle.code.cpp.models.interfaces.DataFlow
-private import semmle.code.cpp.models.interfaces.Taint
-import ExternalAPIsSpecific
-
-/** A node representing untrusted data being passed to an external API. */
-class UntrustedExternalAPIDataNode extends ExternalAPIDataNode {
-  UntrustedExternalAPIDataNode() { any(UntrustedDataToExternalAPIConfig c).hasFlow(_, this) }
-
-  /** Gets a source of untrusted data which is passed to this external API data node. */
-  DataFlow::Node getAnUntrustedSource() {
-    any(UntrustedDataToExternalAPIConfig c).hasFlow(result, this)
-  }
-}
-
-private newtype TExternalAPI =
-  TExternalAPIParameter(Function f, int index) {
-    exists(UntrustedExternalAPIDataNode n |
-      f = n.getExternalFunction() and
-      index = n.getIndex()
-    )
-  }
-
-/** An external API which is used with untrusted data. */
-class ExternalAPIUsedWithUntrustedData extends TExternalAPI {
-  /** Gets a possibly untrusted use of this external API. */
-  UntrustedExternalAPIDataNode getUntrustedDataNode() {
-    this = TExternalAPIParameter(result.getExternalFunction(), result.getIndex())
-  }
-
-  /** Gets the number of untrusted sources used with this external API. */
-  int getNumberOfUntrustedSources() {
-    result = strictcount(getUntrustedDataNode().getAnUntrustedSource())
-  }
-
-  /** Gets a textual representation of this element. */
-  string toString() {
-    exists(Function f, int index, string indexString |
-      if index = -1 then indexString = "qualifier" else indexString = "param " + index
-    |
-      this = TExternalAPIParameter(f, index) and
-      result = f.toString() + " [" + indexString + "]"
-    )
-  }
-}

cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsSpecific.qll

@@ -1,56 +0,0 @@
-/**
- * Provides AST-specific definitions for use in the `ExternalAPI` library.
- */
-
-import semmle.code.cpp.dataflow.TaintTracking
-import semmle.code.cpp.models.interfaces.FlowSource
-import semmle.code.cpp.models.interfaces.DataFlow
-import SafeExternalAPIFunction
-
-/** A node representing untrusted data being passed to an external API. */
-class ExternalAPIDataNode extends DataFlow::Node {
-  Call call;
-  int i;
-
-  ExternalAPIDataNode() {
-    // Argument to call to a function
-    (
-      this.asExpr() = call.getArgument(i)
-      or
-      i = -1 and this.asExpr() = call.getQualifier()
-    ) and
-    exists(Function f |
-      f = call.getTarget() and
-      // Defined outside the source archive
-      not f.hasDefinition() and
-      // Not already modeled as a dataflow or taint step
-      not f instanceof DataFlowFunction and
-      not f instanceof TaintFunction and
-      // Not a call to a known safe external API
-      not f instanceof SafeExternalAPIFunction
-    )
-  }
-
-  /** Gets the called API `Function`. */
-  Function getExternalFunction() { result = call.getTarget() }
-
-  /** Gets the index which is passed untrusted data (where -1 indicates the qualifier). */
-  int getIndex() { result = i }
-
-  /** Gets the description of the function being called. */
-  string getFunctionDescription() { result = getExternalFunction().toString() }
-}
-
-/** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalAPIDataNode`s. */
-class UntrustedDataToExternalAPIConfig extends TaintTracking::Configuration {
-  UntrustedDataToExternalAPIConfig() { this = "UntrustedDataToExternalAPIConfig" }
-
-  override predicate isSource(DataFlow::Node source) {
-    exists(RemoteFlowFunction remoteFlow |
-      remoteFlow = source.asExpr().(Call).getTarget() and
-      remoteFlow.hasRemoteFlowSource(_, _)
-    )
-  }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof ExternalAPIDataNode }
-}

cpp/ql/src/Security/CWE/CWE-020/IRCountUntrustedDataToExternalAPI.qhelp

@@ -1,48 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
-all external APIs that are used with untrusted data, along with how frequently the API is used, and how many 
-unique sources of untrusted data flow to this API. This query is designed primarily to help identify which APIs 
-may be relevant for security analysis of this application.</p>
-
-<p>An external API is defined as a call to a function that is not defined in the source code, and is not
-modeled as a taint step in the default taint library. External APIs may be from the C++ standard library,
-third party dependencies or from internal dependencies. The query will report the function name, along with
-either <code>[param x]</code>, where <code>x</code> indicates the position of the parameter receiving the
-untrusted data or <code>[qualifier]</code> indicating the untrusted data is used as the qualifier to the
-function call.</p>
-
-</overview>
-<recommendation>
-
-<p>For each result:</p>
-
-<ul>
-  <li>If the result highlights a known sink, no action is required.</li>
-  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query.</li>
-  <li>If the result represents a call to an external API which transfers taint, add the appropriate modeling, and 
-  re-run the query to determine what new results have appeared due to this additional modeling.</li>
-</ul>
-
-<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIFunction</code> 
-class to exclude known safe external APIs from future analysis.</p>
-
-</recommendation>
-<example>
-
-<p>If the query were to return the API <code>fputs [param 1]</code> 
-then we should first consider whether this a security relevant sink. In this case, this is writing to a <code>FILE*</code>, so we should 
-consider whether this is an XSS sink. If it is, we should confirm that it is handled by the XSS query.</p>
-
-<p>If the query were to return the API <code>strcat [param 1]</code>, then this should be
-reviewed as a possible taint step, because tainted data would flow from the 1st argument to the 0th argument of the call.</p>
-
-<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
-</example>
-<references>
-
-</references>
-</qhelp>

cpp/ql/src/Security/CWE/CWE-020/IRCountUntrustedDataToExternalAPI.ql

@@ -1,17 +0,0 @@
-/**
- * @name Frequency counts for external APIs that are used with untrusted data
- * @description This reports the external APIs that are used with untrusted data, along with how
- *              frequently the API is called, and how many unique sources of untrusted data flow
- *              to it.
- * @id cpp/count-untrusted-data-external-api-ir
- * @kind table
- * @tags security external/cwe/cwe-20
- */
-
-import cpp
-import ir.ExternalAPIs
-
-from ExternalAPIUsedWithUntrustedData externalAPI
-select externalAPI, count(externalAPI.getUntrustedDataNode()) as numberOfUses,
-  externalAPI.getNumberOfUntrustedSources() as numberOfUntrustedSources order by
-    numberOfUntrustedSources desc

cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp

@@ -1,59 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
-external APIs that use untrusted data. The results are not filtered, so you can audit all examples.
-The query provides data for security reviews of the application and you can also use it to identify external APIs
-that should be modeled as either taint steps, or sinks for specific problems.</p>
-
-<p>An external API is defined as a call to a function that is not defined in the source code, and is not modeled
-as a taint step in the default taint library. External APIs may be from the
-C++ standard library, third-party dependencies or from internal dependencies. The query reports uses of 
-untrusted data in either the qualifier or as one of the arguments of external APIs.</p>
-
-</overview>
-<recommendation>
-
-<p>For each result:</p>
-
-<ul>
-  <li>If the result highlights a known sink, confirm that the result is reported by the relevant query, or 
-  that the result is a false positive because this data is sanitized.</li>
-  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query, 
-  and confirm that the result is either found, or is safe due to appropriate sanitization.</li>
-  <li>If the result represents a call to an external API that transfers taint, add the appropriate modeling, and 
-  re-run the query to determine what new results have appeared due to this additional modeling.</li>
-</ul>
-
-<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIFunction</code> 
-class to exclude known safe external APIs from future analysis.</p>
-
-</recommendation>
-<example>
-
-<p>In this first example, input is read from <code>fgets</code> and then ultimately used in a call to the 
-<code>fputs</code> external API:</p>
-
-<sample src="ExternalAPISinkExample.cpp" />
-
-<p>This is an XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled, 
-and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to 
-some existing sanitization.</p>
-
-<p>In this second example, again a request parameter is read from <code>fgets</code>.</p>
-
-<sample src="ExternalAPITaintStepExample.cpp" />
-
-<p>If the query reported the call to <code>strcat</code> on line 9, this would suggest that this external API is 
-not currently modeled as a taint step in the taint tracking library. The next step would be to model this as a taint step, then
-re-run the query to determine what additional results might be found. In this example, it seems likely that <code>buffer</code>
-will be executed as an SQL query, potentially leading to an SQL injection vulnerability.</p>
-
-<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
-</example>
-<references>
-
-</references>
-</qhelp>

cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql

@@ -1,21 +0,0 @@
-/**
- * @name Untrusted data passed to external API
- * @description Data provided remotely is used in this external API without sanitization, which could be a security risk.
- * @id cpp/untrusted-data-to-external-api-ir
- * @kind path-problem
- * @precision low
- * @problem.severity error
- * @tags security external/cwe/cwe-20
- */
-
-import cpp
-import semmle.code.cpp.ir.dataflow.TaintTracking
-import ir.ExternalAPIs
-import semmle.code.cpp.security.FlowSources
-import DataFlow::PathGraph
-
-from UntrustedDataToExternalAPIConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
-where config.hasFlowPath(source, sink)
-select sink, source, sink,
-  "Call to " + sink.getNode().(ExternalAPIDataNode).getExternalFunction().toString() +
-    " with untrusted data from $@.", source, source.getNode().(RemoteFlowSource).getSourceType()

cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll

@@ -1,24 +0,0 @@
-/**
- * Provides a class for modeling external functions that are "safe" from a security perspective.
- */
-
-private import cpp
-private import semmle.code.cpp.models.interfaces.SideEffect
-
-/**
- * A `Function` that is considered a "safe" external API from a security perspective.
- */
-abstract class SafeExternalAPIFunction extends Function { }
-
-/** The default set of "safe" external APIs. */
-private class DefaultSafeExternalAPIFunction extends SafeExternalAPIFunction {
-  DefaultSafeExternalAPIFunction() {
-    // If a function does not write to any of its arguments, we consider it safe to
-    // pass untrusted data to it. This means that string functions such as `strcmp`
-    // and `strlen`, as well as memory functions such as `memcmp`, are considered safe.
-    exists(SideEffectFunction model | model = this |
-      model.hasOnlySpecificWriteSideEffects() and
-      not model.hasSpecificWriteSideEffect(_, _, _)
-    )
-  }
-}

cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp

@@ -1,59 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
-external APIs that use untrusted data. The results are not filtered, so you can audit all examples.
-The query provides data for security reviews of the application and you can also use it to identify external APIs
-that should be modeled as either taint steps, or sinks for specific problems.</p>
-
-<p>An external API is defined as a call to a function that is not defined in the source code, and is not modeled
-as a taint step in the default taint library. External APIs may be from the
-C++ standard library, third-party dependencies or from internal dependencies. The query reports uses of 
-untrusted data in either the qualifier or as one of the arguments of external APIs.</p>
-
-</overview>
-<recommendation>
-
-<p>For each result:</p>
-
-<ul>
-  <li>If the result highlights a known sink, confirm that the result is reported by the relevant query, or 
-  that the result is a false positive because this data is sanitized.</li>
-  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query, 
-  and confirm that the result is either found, or is safe due to appropriate sanitization.</li>
-  <li>If the result represents a call to an external API that transfers taint, add the appropriate modeling, and 
-  re-run the query to determine what new results have appeared due to this additional modeling.</li>
-</ul>
-
-<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIFunction</code> 
-class to exclude known safe external APIs from future analysis.</p>
-
-</recommendation>
-<example>
-
-<p>In this first example, input is read from <code>fgets</code> and then ultimately used in a call to the 
-<code>fputs</code> external API:</p>
-
-<sample src="ExternalAPISinkExample.cpp" />
-
-<p>This is an XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled, 
-and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to 
-some existing sanitization.</p>
-
-<p>In this second example, again a request parameter is read from <code>fgets</code>.</p>
-
-<sample src="ExternalAPITaintStepExample.cpp" />
-
-<p>If the query reported the call to <code>strcat</code> on line 9, this would suggest that this external API is 
-not currently modeled as a taint step in the taint tracking library. The next step would be to model this as a taint step, then
-re-run the query to determine what additional results might be found. In this example, it seems likely that <code>buffer</code>
-will be executed as an SQL query, potentially leading to an SQL injection vulnerability.</p>
-
-<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
-</example>
-<references>
-
-</references>
-</qhelp>

cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql

@@ -1,20 +0,0 @@
-/**
- * @name Untrusted data passed to external API
- * @description Data provided remotely is used in this external API without sanitization, which could be a security risk.
- * @id cpp/untrusted-data-to-external-api
- * @kind path-problem
- * @precision low
- * @problem.severity error
- * @tags security external/cwe/cwe-20
- */
-
-import cpp
-import semmle.code.cpp.dataflow.TaintTracking
-import ExternalAPIs
-import DataFlow::PathGraph
-
-from UntrustedDataToExternalAPIConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
-where config.hasFlowPath(source, sink)
-select sink, source, sink,
-  "Call to " + sink.getNode().(ExternalAPIDataNode).getExternalFunction().toString() +
-    " with untrusted data from $@.", source, source.toString()

cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIs.qll

@@ -1,50 +0,0 @@
-/**
- * Definitions for reasoning about untrusted data used in APIs defined outside the
- * database.
- */
-
-private import cpp
-private import semmle.code.cpp.models.interfaces.DataFlow
-private import semmle.code.cpp.models.interfaces.Taint
-import ExternalAPIsSpecific
-
-/** A node representing untrusted data being passed to an external API. */
-class UntrustedExternalAPIDataNode extends ExternalAPIDataNode {
-  UntrustedExternalAPIDataNode() { any(UntrustedDataToExternalAPIConfig c).hasFlow(_, this) }
-
-  /** Gets a source of untrusted data which is passed to this external API data node. */
-  DataFlow::Node getAnUntrustedSource() {
-    any(UntrustedDataToExternalAPIConfig c).hasFlow(result, this)
-  }
-}
-
-private newtype TExternalAPI =
-  TExternalAPIParameter(Function f, int index) {
-    exists(UntrustedExternalAPIDataNode n |
-      f = n.getExternalFunction() and
-      index = n.getIndex()
-    )
-  }
-
-/** An external API which is used with untrusted data. */
-class ExternalAPIUsedWithUntrustedData extends TExternalAPI {
-  /** Gets a possibly untrusted use of this external API. */
-  UntrustedExternalAPIDataNode getUntrustedDataNode() {
-    this = TExternalAPIParameter(result.getExternalFunction(), result.getIndex())
-  }
-
-  /** Gets the number of untrusted sources used with this external API. */
-  int getNumberOfUntrustedSources() {
-    result = strictcount(getUntrustedDataNode().getAnUntrustedSource())
-  }
-
-  /** Gets a textual representation of this element. */
-  string toString() {
-    exists(Function f, int index, string indexString |
-      if index = -1 then indexString = "qualifier" else indexString = "param " + index
-    |
-      this = TExternalAPIParameter(f, index) and
-      result = f.toString() + " [" + indexString + "]"
-    )
-  }
-}

cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIsSpecific.qll

@@ -1,51 +0,0 @@
-/**
- * Provides IR-specific definitions for use in the `ExternalAPI` library.
- */
-
-import semmle.code.cpp.ir.dataflow.TaintTracking
-private import semmle.code.cpp.security.FlowSources
-private import semmle.code.cpp.models.interfaces.DataFlow
-import SafeExternalAPIFunction
-
-/** A node representing untrusted data being passed to an external API. */
-class ExternalAPIDataNode extends DataFlow::Node {
-  Call call;
-  int i;
-
-  ExternalAPIDataNode() {
-    // Argument to call to a function
-    (
-      this.asExpr() = call.getArgument(i)
-      or
-      i = -1 and this.asExpr() = call.getQualifier()
-    ) and
-    exists(Function f |
-      f = call.getTarget() and
-      // Defined outside the source archive
-      not f.hasDefinition() and
-      // Not already modeled as a dataflow or taint step
-      not f instanceof DataFlowFunction and
-      not f instanceof TaintFunction and
-      // Not a call to a known safe external API
-      not f instanceof SafeExternalAPIFunction
-    )
-  }
-
-  /** Gets the called API `Function`. */
-  Function getExternalFunction() { result = call.getTarget() }
-
-  /** Gets the index which is passed untrusted data (where -1 indicates the qualifier). */
-  int getIndex() { result = i }
-
-  /** Gets the description of the function being called. */
-  string getFunctionDescription() { result = getExternalFunction().toString() }
-}
-
-/** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalAPIDataNode`s. */
-class UntrustedDataToExternalAPIConfig extends TaintTracking::Configuration {
-  UntrustedDataToExternalAPIConfig() { this = "UntrustedDataToExternalAPIConfigIR" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof ExternalAPIDataNode }
-}

cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll

@@ -1,24 +0,0 @@
-/**
- * Provides a class for modeling external functions that are "safe" from a security perspective.
- */
-
-private import cpp
-private import semmle.code.cpp.models.interfaces.SideEffect
-
-/**
- * A `Function` that is considered a "safe" external API from a security perspective.
- */
-abstract class SafeExternalAPIFunction extends Function { }
-
-/** The default set of "safe" external APIs. */
-private class DefaultSafeExternalAPIFunction extends SafeExternalAPIFunction {
-  DefaultSafeExternalAPIFunction() {
-    // If a function does not write to any of its arguments, we consider it safe to
-    // pass untrusted data to it. This means that string functions such as `strcmp`
-    // and `strlen`, as well as memory functions such as `memcmp`, are considered safe.
-    exists(SideEffectFunction model | model = this |
-      model.hasOnlySpecificWriteSideEffects() and
-      not model.hasSpecificWriteSideEffect(_, _, _)
-    )
-  }
-}

cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll

@@ -189,7 +189,8 @@ class InitializationFunction extends Function {
       // Field wise assignment to the parameter
       any(Assignment e).getLValue() = getAFieldAccess(this.getParameter(i)) or
       i =
-        this.(MemberFunction)
+        this
+            .(MemberFunction)
             .getAnOverridingFunction+()
             .(InitializationFunction)
             .initializedParameter() or
@@ -326,35 +327,52 @@ class InitializationFunction extends Function {
       // Return value is not a success code but the output functions never fail.
       name.matches("_Interlocked%")
       or
-      name =
-        [
-          // Functions that never fail, according to MSDN.
-          "QueryPerformanceCounter", "QueryPerformanceFrequency",
-          // Functions that never fail post-Vista, according to MSDN.
-          "InitializeCriticalSectionAndSpinCount",
-          // `rand_s` writes 0 to a non-null argument if it fails, according to MSDN.
-          "rand_s",
-          // IntersectRect initializes the argument regardless of whether the input intersects
-          "IntersectRect", "SetRect", "UnionRect",
-          // These functions appears to have an incorrect CFG, which leads to false positives
-          "PhysicalToLogicalDPIPoint", "LogicalToPhysicalDPIPoint",
-          // Sets NtProductType to default on error
-          "RtlGetNtProductType",
-          // Our CFG is not sophisticated enough to detect that the argument is always initialized
-          "StringCchLengthA",
-          // All paths init the argument, and always returns SUCCESS.
-          "RtlUnicodeToMultiByteSize",
-          // All paths init the argument, and always returns SUCCESS.
-          "RtlMultiByteToUnicodeSize",
-          // All paths init the argument, and always returns SUCCESS.
-          "RtlUnicodeToMultiByteN",
-          // Always initializes argument
-          "RtlGetFirstRange",
-          // Destination range is zeroed out on failure, assuming first two parameters are valid
-          "memcpy_s",
-          // This zeroes the memory unconditionally
-          "SeCreateAccessState"
-        ]
+      // Functions that never fail, according to MSDN.
+      name = "QueryPerformanceCounter"
+      or
+      name = "QueryPerformanceFrequency"
+      or
+      // Functions that never fail post-Vista, according to MSDN.
+      name = "InitializeCriticalSectionAndSpinCount"
+      or
+      // `rand_s` writes 0 to a non-null argument if it fails, according to MSDN.
+      name = "rand_s"
+      or
+      // IntersectRect initializes the argument regardless of whether the input intersects
+      name = "IntersectRect"
+      or
+      name = "SetRect"
+      or
+      name = "UnionRect"
+      or
+      // These functions appears to have an incorrect CFG, which leads to false positives
+      name = "PhysicalToLogicalDPIPoint"
+      or
+      name = "LogicalToPhysicalDPIPoint"
+      or
+      // Sets NtProductType to default on error
+      name = "RtlGetNtProductType"
+      or
+      // Our CFG is not sophisticated enough to detect that the argument is always initialized
+      name = "StringCchLengthA"
+      or
+      // All paths init the argument, and always returns SUCCESS.
+      name = "RtlUnicodeToMultiByteSize"
+      or
+      // All paths init the argument, and always returns SUCCESS.
+      name = "RtlMultiByteToUnicodeSize"
+      or
+      // All paths init the argument, and always returns SUCCESS.
+      name = "RtlUnicodeToMultiByteN"
+      or
+      // Always initializes argument
+      name = "RtlGetFirstRange"
+      or
+      // Destination range is zeroed out on failure, assuming first two parameters are valid
+      name = "memcpy_s"
+      or
+      // This zeroes the memory unconditionally
+      name = "SeCreateAccessState"
     )
   }
 }
@@ -457,9 +475,12 @@ class ConditionalInitializationCall extends FunctionCall {
       fa.getASuccessor+() = result
     ) and
     result =
-      this.getArgument(getTarget(this)
-            .(ConditionalInitializationFunction)
-            .conditionallyInitializedParameter(_)).(AddressOfExpr).getOperand()
+      this
+          .getArgument(getTarget(this)
+                .(ConditionalInitializationFunction)
+                .conditionallyInitializedParameter(_))
+          .(AddressOfExpr)
+          .getOperand()
   }
 
   Variable getStatusVariable() {

cpp/ql/src/Security/CWE/CWE-732/FilePermissions.qll

@@ -140,9 +140,12 @@ class FopenCreationExpr extends FileCreationExpr {
 
 class FopensCreationExpr extends FileCreationExpr {
   FopensCreationExpr() {
-    this.getTarget().getName() = ["fopen_s", "_wfopen_s"] and
+    exists(string name | name = this.getTarget().getName() |
+      name = "fopen_s" or
+      name = "_wfopen_s"
+    ) and
     exists(string mode |
-      mode = ["w", "a"] and
+      (mode = "w" or mode = "a") and
       this.getArgument(2).getValue().matches(mode + "%")
     )
   }

cpp/ql/src/definitions.qll

@@ -4,7 +4,6 @@
  */
 
 import cpp
-import IDEContextual
 
 /**
  * Any element that might be the source or target of a jump-to-definition
@@ -208,3 +207,11 @@ Top definitionOf(Top e, string kind) {
   // later on.
   strictcount(result.getLocation()) < 10
 }
+
+/**
+ * Returns an appropriately encoded version of a filename `name`
+ * passed by the VS Code extension in order to coincide with the
+ * output of `.getFile()` on locatable entities.
+ */
+cached
+File getEncodedFile(string name) { result.getAbsolutePath().replaceAll(":", "_") = name }

cpp/ql/src/experimental/Security/CWE/CWE-120/MemoryUnsafeFunctionScan.cpp

@@ -13,10 +13,9 @@ int main(int argc, char **argv)
     char buf1[10];
     scanf("%s", buf1);
 
-    // GOOD, length is specified. The length should be one less than the size of the destination buffer, since the last character is the NULL terminator.
-    char buf2[20];
-    char buf3[10];
-    sscanf(buf2, "%9s", buf3);
+    // GOOD, length is specified. The length should be one less than the size of the buffer, since the last character is the NULL terminator.
+    char buf2[10];
+    sscanf(buf2, "%9s");
 
     // BAD, do not use scanf without specifying a length first
     char file[10];

cpp/ql/src/experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.c

@@ -1,11 +0,0 @@
-unsigned long sizeArray;
-
-// BAD: let's consider several values, taking ULONG_MAX =18446744073709551615
-// sizeArray = 60; (sizeArray - 10) = 50; true
-// sizeArray = 10; (sizeArray - 10) = 0; false
-// sizeArray = 1; (sizeArray - 10) = 18446744073709551607; true
-// sizeArray = 0; (sizeArray - 10) = 18446744073709551606; true
-if (sizeArray - 10 > 0)
-
-// GOOD: Prevent overflow by checking the input
-if (sizeArray > 10)

cpp/ql/src/experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.qhelp

@@ -1,33 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>The code compares the unsigned difference with zero. 
-It is highly probable that the condition is wrong if the difference expression has the unsigned type.  
-The condition holds in all the cases when difference is not equal to zero. 
-It means that we may use condition not equal. But the programmer probably wanted to compare the difference of elements.</p>
-
-<p>False positives include code in which the first difference element is always greater than or equal to the second. 
-For comparison, ">" such conditions are equivalent to "! =", And are recommended for replacement. 
-For comparison "> =", the conditions are always true and are recommended to be excluded.</p>
-
-</overview>
-<recommendation>
-
-<p>Use a simple comparison of two elements, instead of comparing their difference to zero.</p>
-
-</recommendation>
-<example>
-<p>The following example demonstrates an erroneous and corrected use of comparison.</p>
-<sample src="UnsignedDifferenceExpressionComparedZero.c" />
-
-</example>
-<references>
-
-<li>CERT C Coding Standard:
-<a href="https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules">INT02-C. Understand integer conversion rules</a>.
-</li>
-
-</references>
-</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql

@@ -1,23 +0,0 @@
-/**
- * @name Unsigned difference expression compared to zero
- * @description It is highly probable that the condition is wrong if the difference expression has the unsigned type.
- *              The condition holds in all the cases when difference is not equal to zero. It means that we may use condition not equal.
- *              But the programmer probably wanted to compare the difference of elements.
- * @kind problem
- * @id cpp/unsigned-difference-expression-compared-zero
- * @problem.severity warning
- * @precision medium
- * @tags security
- *       external/cwe/cwe-191
- */
-
-import cpp
-import semmle.code.cpp.commons.Exclusions
-
-from RelationalOperation ro, SubExpr sub
-where
-  not isFromMacroDefinition(ro) and
-  ro.getLesserOperand().getValue().toInt() = 0 and
-  ro.getGreaterOperand() = sub and
-  sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned()
-select ro, "Difference in condition is always greater than or equal to zero"

cpp/ql/src/experimental/semmle/code/cpp/security/PrivateCleartextWrite.qll

@@ -7,6 +7,7 @@ import semmle.code.cpp.dataflow.TaintTracking
 import experimental.semmle.code.cpp.security.PrivateData
 import semmle.code.cpp.security.FileWrite
 import semmle.code.cpp.security.BufferWrite
+import semmle.code.cpp.dataflow.TaintTracking
 
 module PrivateCleartextWrite {
   /**

cpp/ql/src/jsf/4.07 Header Files/AV Rule 35.ql

@@ -4,7 +4,7 @@
  *              the file from being included twice). This prevents errors and
  *              inefficiencies caused by repeated inclusion.
  * @kind problem
- * @problem.severity recommendation
+ * @problem.severity warning
  * @precision high
  * @id cpp/missing-header-guard
  * @tags efficiency

cpp/ql/src/jsf/4.09 Style/AV Rule 48.ql

@@ -31,7 +31,8 @@ predicate canonicalName1(Declaration d, string canonical) {
 
 predicate canonicalName2(Declaration d, string canonical) {
   canonical =
-    d.getName()
+    d
+        .getName()
         .replaceAll("_", "")
         .replaceAll("0", "O")
         .replaceAll("D", "O")

cpp/ql/src/localDefinitions.ql

@@ -12,5 +12,5 @@ import definitions
 external string selectedSourceFile();
 
 from Top e, Top def, string kind
-where def = definitionOf(e, kind) and e.getFile() = getFileBySourceArchiveName(selectedSourceFile())
+where def = definitionOf(e, kind) and e.getFile() = getEncodedFile(selectedSourceFile())
 select e, def, kind

cpp/ql/src/localReferences.ql

@@ -12,6 +12,5 @@ import definitions
 external string selectedSourceFile();
 
 from Top e, Top def, string kind
-where
-  def = definitionOf(e, kind) and def.getFile() = getFileBySourceArchiveName(selectedSourceFile())
+where def = definitionOf(e, kind) and def.getFile() = getEncodedFile(selectedSourceFile())
 select e, def, kind

cpp/ql/src/printAst.ql

@@ -22,6 +22,6 @@ class Cfg extends PrintASTConfiguration {
    * Print All functions from the selected file.
    */
   override predicate shouldPrintFunction(Function func) {
-    func.getFile() = getFileBySourceArchiveName(selectedSourceFile())
+    func.getFile() = getEncodedFile(selectedSourceFile())
   }
 }

cpp/ql/src/semmle/code/cpp/AutogeneratedFile.qll

@@ -35,7 +35,8 @@ private predicate autogeneratedComment(string comment) {
         .regexpMatch("(?si).*(" +
             // replace `generated` with a regexp that also catches things like
             // `auto-generated`.
-            cond.replaceAll("generated", "(auto[\\w-]*[\\s/\\*\\r\\n]*)?generated")
+            cond
+                .replaceAll("generated", "(auto[\\w-]*[\\s/\\*\\r\\n]*)?generated")
                 // replace `!` with a regexp for end-of-sentence / separator characters.
                 .replaceAll("!", "[\\.\\?\\!\\-\\;\\,]")
                 // replace ` ` with a regexp for one or more whitespace characters

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

@@ -236,8 +236,9 @@ class Class extends UserType {
     or
     exists(ClassDerivation cd | cd.getBaseClass() = base |
       result =
-        this.accessOfBaseMemberMulti(cd.getDerivedClass(),
-          fieldInBase.accessInDirectDerived(cd.getASpecifier().(AccessSpecifier)))
+        this
+            .accessOfBaseMemberMulti(cd.getDerivedClass(),
+              fieldInBase.accessInDirectDerived(cd.getASpecifier().(AccessSpecifier)))
     )
   }
 
@@ -976,12 +977,7 @@ class ClassTemplateInstantiation extends Class {
  * specialization - see `FullClassTemplateSpecialization` and
  * `PartialClassTemplateSpecialization`).
  */
-class ClassTemplateSpecialization extends Class {
-  ClassTemplateSpecialization() {
-    isFullClassTemplateSpecialization(this) or
-    isPartialClassTemplateSpecialization(this)
-  }
-
+abstract class ClassTemplateSpecialization extends Class {
   /**
    * Gets the primary template for the specialization, for example on
    * `S<T,int>`, the result is `S<T,U>`.
@@ -1001,16 +997,6 @@ class ClassTemplateSpecialization extends Class {
   override string getAPrimaryQlClass() { result = "ClassTemplateSpecialization" }
 }
 
-private predicate isFullClassTemplateSpecialization(Class c) {
-  // This class has template arguments, but none of them involves a template parameter.
-  exists(c.getATemplateArgument()) and
-  not exists(Type ta | ta = c.getATemplateArgument() and ta.involvesTemplateParameter()) and
-  // This class does not have any instantiations.
-  not exists(c.(TemplateClass).getAnInstantiation()) and
-  // This class is not an instantiation of a class template.
-  not c instanceof ClassTemplateInstantiation
-}
-
 /**
  * A full specialization of a class template.  For example `MyTemplateClass<int>`
  * in the following code is a `FullClassTemplateSpecialization`:
@@ -1027,31 +1013,19 @@ private predicate isFullClassTemplateSpecialization(Class c) {
  * ```
  */
 class FullClassTemplateSpecialization extends ClassTemplateSpecialization {
-  FullClassTemplateSpecialization() { isFullClassTemplateSpecialization(this) }
+  FullClassTemplateSpecialization() {
+    // This class has template arguments, but none of them involves a template parameter.
+    exists(getATemplateArgument()) and
+    not exists(Type ta | ta = getATemplateArgument() and ta.involvesTemplateParameter()) and
+    // This class does not have any instantiations.
+    not exists(this.(TemplateClass).getAnInstantiation()) and
+    // This class is not an instantiation of a class template.
+    not this instanceof ClassTemplateInstantiation
+  }
 
   override string getAPrimaryQlClass() { result = "FullClassTemplateSpecialization" }
 }
 
-private predicate isPartialClassTemplateSpecialization(Class c) {
-  /*
-   * (a) At least one of this class's template arguments involves a
-   *     template parameter in some respect, for example T, T*, etc.
-   *
-   * (b) It is not the case that the n template arguments of this class
-   *     are a set of n distinct template parameters.
-   *
-   * template <typename T,U> class X {};      // class template
-   * template <typename T> class X<T,T> {};   // partial class template specialization
-   * template <typename T> class X<T,int> {}; // partial class template specialization
-   * template <typename T> class Y {};        // class template
-   * template <typename T> class Y<T*> {};    // partial class template specialization
-   */
-
-  exists(Type ta | ta = c.getATemplateArgument() and ta.involvesTemplateParameter()) and
-  count(TemplateParameter tp | tp = c.getATemplateArgument()) !=
-    count(int i | exists(c.getTemplateArgument(i)))
-}
-
 /**
  * A partial specialization of a class template.  For example `MyTemplateClass<int, T>`
  * in the following code is a `PartialClassTemplateSpecialization`:
@@ -1068,7 +1042,25 @@ private predicate isPartialClassTemplateSpecialization(Class c) {
  * ```
  */
 class PartialClassTemplateSpecialization extends ClassTemplateSpecialization {
-  PartialClassTemplateSpecialization() { isPartialClassTemplateSpecialization(this) }
+  PartialClassTemplateSpecialization() {
+    /*
+     * (a) At least one of this class's template arguments involves a
+     *     template parameter in some respect, for example T, T*, etc.
+     *
+     * (b) It is not the case that the n template arguments of this class
+     *     are a set of n distinct template parameters.
+     *
+     * template <typename T,U> class X {};      // class template
+     * template <typename T> class X<T,T> {};   // partial class template specialization
+     * template <typename T> class X<T,int> {}; // partial class template specialization
+     * template <typename T> class Y {};        // class template
+     * template <typename T> class Y<T*> {};    // partial class template specialization
+     */
+
+    exists(Type ta | ta = getATemplateArgument() and ta.involvesTemplateParameter()) and
+    count(TemplateParameter tp | tp = getATemplateArgument()) !=
+      count(int i | exists(getTemplateArgument(i)))
+  }
 
   override string getAPrimaryQlClass() { result = "PartialClassTemplateSpecialization" }
 }

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

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

cpp/ql/src/semmle/code/cpp/Element.qll

@@ -65,10 +65,11 @@ class ElementBase extends @element {
    * which they belong; for example, `AddExpr` is a primary class, but
    * `BinaryOperation` is not.
    *
-   * This predicate can have multiple results if multiple primary classes match.
-   * For some elements, this predicate may not have a result.
+   * This predicate always has a result. If no primary class can be
+   * determined, the result is `"???"`. If multiple primary classes match,
+   * this predicate can have multiple results.
    */
-  string getAPrimaryQlClass() { none() }
+  string getAPrimaryQlClass() { result = "???" }
 }
 
 /**

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

@@ -363,8 +363,20 @@ class File extends Container, @file {
  */
 class HeaderFile extends File {
   HeaderFile() {
-    this.getExtension().toLowerCase() =
-      ["h", "r", "hpp", "hxx", "h++", "hh", "hp", "tcc", "tpp", "txx", "t++"]
+    exists(string ext | ext = this.getExtension().toLowerCase() |
+      ext = "h" or
+      ext = "r" or
+      /*    ---   */ ext = "hpp" or
+      ext = "hxx" or
+      ext = "h++" or
+      ext = "hh" or
+      ext = "hp" or
+      ext = "tcc" or
+      ext = "tpp" or
+      ext = "txx" or
+      ext = "t++"
+      /*    ---         ---    */
+    )
     or
     not exists(this.getExtension()) and
     exists(Include i | i.getIncludedFile() = this)
@@ -394,7 +406,7 @@ class HeaderFile extends File {
  * `File.compiledAsC`.
  */
 class CFile extends File {
-  CFile() { this.getExtension().toLowerCase() = ["c", "i"] }
+  CFile() { exists(string ext | ext = this.getExtension().toLowerCase() | ext = "c" or ext = "i") }
 
   override string getAPrimaryQlClass() { result = "CFile" }
 }
@@ -407,10 +419,21 @@ class CFile extends File {
  */
 class CppFile extends File {
   CppFile() {
-    this.getExtension().toLowerCase() =
-      ["cpp", "cxx", "c++", "cc", "cp", "icc", "ipp", "ixx", "i++", "ii"]
-    // Note: .C files are indistinguishable from .c files on some
-    // file systems, so we just treat them as CFile's.
+    exists(string ext | ext = this.getExtension().toLowerCase() |
+      /*     ---     */ ext = "cpp" or
+      ext = "cxx" or
+      ext = "c++" or
+      ext = "cc" or
+      ext = "cp" or
+      ext = "icc" or
+      ext = "ipp" or
+      ext = "ixx" or
+      ext = "i++" or
+      ext = "ii"
+      /*  ---    */
+      // Note: .C files are indistinguishable from .c files on some
+      // file systems, so we just treat them as CFile's.
+    )
   }
 
   override string getAPrimaryQlClass() { result = "CppFile" }

cpp/ql/src/semmle/code/cpp/Function.qll

@@ -445,15 +445,50 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
       // ... and likewise for destructors.
       this.(Destructor).getADestruction().mayBeGloballyImpure()
     else
-      // Unless it's a function that we know is side-effect-free, it may
-      // have side-effects.
-      not this.hasGlobalOrStdName([
-          "strcmp", "wcscmp", "_mbscmp", "strlen", "wcslen", "_mbslen", "_mbslen_l", "_mbstrlen",
-          "_mbstrlen_l", "strnlen", "strnlen_s", "wcsnlen", "wcsnlen_s", "_mbsnlen", "_mbsnlen_l",
-          "_mbstrnlen", "_mbstrnlen_l", "strncmp", "wcsncmp", "_mbsncmp", "_mbsncmp_l", "strchr",
-          "memchr", "wmemchr", "memcmp", "wmemcmp", "_memicmp", "_memicmp_l", "feof", "isdigit",
-          "isxdigit", "abs", "fabs", "labs", "floor", "ceil", "atoi", "atol", "atoll", "atof"
-        ])
+      not exists(string name | this.hasGlobalOrStdName(name) |
+        // Unless it's a function that we know is side-effect-free, it may
+        // have side-effects.
+        name = "strcmp" or
+        name = "wcscmp" or
+        name = "_mbscmp" or
+        name = "strlen" or
+        name = "wcslen" or
+        name = "_mbslen" or
+        name = "_mbslen_l" or
+        name = "_mbstrlen" or
+        name = "_mbstrlen_l" or
+        name = "strnlen" or
+        name = "strnlen_s" or
+        name = "wcsnlen" or
+        name = "wcsnlen_s" or
+        name = "_mbsnlen" or
+        name = "_mbsnlen_l" or
+        name = "_mbstrnlen" or
+        name = "_mbstrnlen_l" or
+        name = "strncmp" or
+        name = "wcsncmp" or
+        name = "_mbsncmp" or
+        name = "_mbsncmp_l" or
+        name = "strchr" or
+        name = "memchr" or
+        name = "wmemchr" or
+        name = "memcmp" or
+        name = "wmemcmp" or
+        name = "_memicmp" or
+        name = "_memicmp_l" or
+        name = "feof" or
+        name = "isdigit" or
+        name = "isxdigit" or
+        name = "abs" or
+        name = "fabs" or
+        name = "labs" or
+        name = "floor" or
+        name = "ceil" or
+        name = "atoi" or
+        name = "atol" or
+        name = "atoll" or
+        name = "atof"
+      )
   }
 
   /**

cpp/ql/src/semmle/code/cpp/MemberFunction.qll

@@ -205,21 +205,12 @@ class Constructor extends MemberFunction {
 /**
  * A function that defines an implicit conversion.
  */
-class ImplicitConversionFunction extends MemberFunction {
-  ImplicitConversionFunction() {
-    // ConversionOperator
-    functions(underlyingElement(this), _, 4)
-    or
-    // ConversionConstructor (deprecated)
-    strictcount(Parameter p | p = getAParameter() and not p.hasInitializer()) = 1 and
-    not hasSpecifier("explicit")
-  }
-
+abstract class ImplicitConversionFunction extends MemberFunction {
   /** Gets the type this `ImplicitConversionFunction` takes as input. */
-  Type getSourceType() { none() } // overridden in subclasses
+  abstract Type getSourceType();
 
   /** Gets the type this `ImplicitConversionFunction` converts to. */
-  Type getDestType() { none() } // overridden in subclasses
+  abstract Type getDestType();
 }
 
 /**

cpp/ql/src/semmle/code/cpp/Print.qll

@@ -60,7 +60,7 @@ private string getTemplateArgumentString(Declaration d, int i) {
 /**
  * A `Declaration` extended to add methods for generating strings useful only for dumps and debugging.
  */
-private class DumpDeclaration extends Declaration {
+abstract private class DumpDeclaration extends Declaration {
   DumpDeclaration() { shouldPrintDeclaration(this) }
 
   /**
@@ -304,7 +304,7 @@ private class SpecifiedDumpType extends DerivedDumpType, SpecifiedType {
       basePrefix = getBaseType().(DumpType).getDeclaratorPrefix() and
       if getBaseType().getUnspecifiedType() instanceof RoutineType
       then result = basePrefix
-      else result = basePrefix + " " + getSpecifierString()
+      else result = basePrefix + " " + getSpecifierString().trim()
     )
   }
 
@@ -312,7 +312,7 @@ private class SpecifiedDumpType extends DerivedDumpType, SpecifiedType {
     exists(string baseSuffix |
       baseSuffix = getBaseType().(DumpType).getDeclaratorSuffixBeforeQualifiers() and
       if getBaseType().getUnspecifiedType() instanceof RoutineType
-      then result = baseSuffix + " " + getSpecifierString()
+      then result = baseSuffix + " " + getSpecifierString().trim()
       else result = baseSuffix
     )
   }
@@ -385,7 +385,7 @@ private class DumpFunction extends DumpDeclaration, Function {
 
   private string getACVQualifier() {
     result = getASpecifier().getName() and
-    result = ["const", "volatile"]
+    (result = "const" or result = "volatile")
   }
 
   private string getDeclaratorSuffix() {

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

@@ -34,7 +34,8 @@ private predicate shouldPrintFunction(Function func) {
 bindingset[s]
 private string escapeString(string s) {
   result =
-    s.replaceAll("\\", "\\\\")
+    s
+        .replaceAll("\\", "\\\\")
         .replaceAll("\n", "\\n")
         .replaceAll("\r", "\\r")
         .replaceAll("\t", "\\t")
@@ -90,8 +91,7 @@ private newtype TPrintASTNode =
   TDeclarationEntryNode(DeclStmt stmt, DeclarationEntry entry) {
     // We create a unique node for each pair of (stmt, entry), to avoid having one node with
     // multiple parents due to extractor bug CPP-413.
-    stmt.getADeclarationEntry() = entry and
-    shouldPrintFunction(stmt.getEnclosingFunction())
+    stmt.getADeclarationEntry() = entry
   } or
   TParametersNode(Function func) { shouldPrintFunction(func) } or
   TConstructorInitializersNode(Constructor ctor) {
@@ -234,27 +234,11 @@ class PrintASTNode extends TPrintASTNode {
   private Function getEnclosingFunction() { result = getParent*().(FunctionNode).getFunction() }
 }
 
-/**
- * Class that restricts the elements that we compute `qlClass` for.
- */
-private class PrintableElement extends Element {
-  PrintableElement() {
-    exists(TASTNode(this))
-    or
-    exists(TDeclarationEntryNode(_, this))
-    or
-    this instanceof Type
-  }
-
-  pragma[noinline]
-  string getAPrimaryQlClass0() { result = getAPrimaryQlClass() }
-}
-
 /**
  * Retrieves the canonical QL class(es) for entity `el`
  */
-private string qlClass(PrintableElement el) {
-  result = "[" + concat(el.getAPrimaryQlClass0(), ",") + "] "
+private string qlClass(ElementBase el) {
+  result = "[" + concat(el.getAPrimaryQlClass(), ",") + "] "
   // Alternative implementation -- do not delete. It is useful for QL class discovery.
   //result = "["+ concat(el.getAQlClass(), ",") + "] "
 }

cpp/ql/src/semmle/code/cpp/Type.qll

@@ -577,9 +577,7 @@ class BoolType extends IntegralType {
  * unsigned char e, f;
  * ```
  */
-class CharType extends IntegralType {
-  CharType() { builtintypes(underlyingElement(this), _, [5, 6, 7], _, _, _) }
-}
+abstract class CharType extends IntegralType { }
 
 /**
  * The C/C++ `char` type (which is distinct from `signed char` and
@@ -1306,16 +1304,14 @@ class SpecifiedType extends DerivedType {
   }
 
   /**
-   * INTERNAL: Do not use.
-   *
    * Gets all the specifiers of this type as a string in a fixed order (the order
    * only depends on the specifiers, not on the source program). This is intended
    * for debugging queries only and is an expensive operation.
    */
-  string getSpecifierString() { result = concat(this.getASpecifier().getName(), " ") }
+  string getSpecifierString() { internalSpecString(this, result, 1) }
 
   override string explain() {
-    result = this.getSpecifierString() + " {" + this.getBaseType().explain() + "}"
+    result = this.getSpecifierString() + "{" + this.getBaseType().explain() + "}"
   }
 
   override predicate isDeeplyConst() {
@@ -1714,6 +1710,28 @@ class AutoType extends TemplateParameter {
   }
 }
 
+//
+// Internal implementation predicates
+//
+private predicate allSpecifiers(int i, string s) { s = rank[i](string t | specifiers(_, t) | t) }
+
+private predicate internalSpecString(Type t, string res, int i) {
+  (
+    if allSpecifiers(i, t.getASpecifier().getName())
+    then
+      exists(string spec, string rest |
+        allSpecifiers(i, spec) and
+        res = spec + " " + rest and
+        internalSpecString(t, rest, i + 1)
+      )
+    else (
+      allSpecifiers(i, _) and internalSpecString(t, res, i + 1)
+    )
+  )
+  or
+  i = count(Specifier s) + 1 and res = ""
+}
+
 private predicate suppressUnusedThis(Type t) { any() }
 
 /** A source code location referring to a type */

cpp/ql/src/semmle/code/cpp/XML.qll

@@ -4,11 +4,8 @@
 
 import semmle.files.FileSystem
 
-private class TXMLLocatable =
-  @xmldtd or @xmlelement or @xmlattribute or @xmlnamespace or @xmlcomment or @xmlcharacters;
-
 /** An XML element that has a location. */
-class XMLLocatable extends @xmllocatable, TXMLLocatable {
+abstract class XMLLocatable extends @xmllocatable {
   /** Gets the source location for this element. */
   Location getLocation() { xmllocations(this, result) }
 
@@ -36,7 +33,7 @@ class XMLLocatable extends @xmllocatable, TXMLLocatable {
   }
 
   /** Gets a textual representation of this element. */
-  string toString() { none() } // overridden in subclasses
+  abstract string toString();
 }
 
 /**
@@ -54,7 +51,7 @@ class XMLParent extends @xmlparent {
    * Gets a printable representation of this XML parent.
    * (Intended to be overridden in subclasses.)
    */
-  string getName() { none() } // overridden in subclasses
+  abstract string getName();
 
   /** Gets the file to which this XML parent belongs. */
   XMLFile getFile() { result = this or xmlElements(this, _, _, _, result) }

cpp/ql/src/semmle/code/cpp/commons/DateTime.qll

@@ -14,7 +14,11 @@ class PackedTimeType extends Type {
   }
 }
 
-private predicate timeType(string typeName) { typeName = ["_SYSTEMTIME", "SYSTEMTIME", "tm"] }
+private predicate timeType(string typeName) {
+  typeName = "_SYSTEMTIME" or
+  typeName = "SYSTEMTIME" or
+  typeName = "tm"
+}
 
 /**
  * A type that is used to represent times and dates in an 'unpacked' form, that is,

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

@@ -6,6 +6,7 @@ import semmle.code.cpp.Type
 import semmle.code.cpp.commons.CommonType
 import semmle.code.cpp.commons.StringAnalysis
 import semmle.code.cpp.models.interfaces.FormattingFunction
+import semmle.code.cpp.models.implementations.Printf
 
 class PrintfFormatAttribute extends FormatAttribute {
   PrintfFormatAttribute() { getArchetype() = ["printf", "__printf__"] }
@@ -34,87 +35,66 @@ class AttributeFormattingFunction extends FormattingFunction {
 
 /**
  * A standard function such as `vprintf` that has a format parameter
- * and a variable argument list of type `va_arg`. `formatParamIndex` indicates
- * the format parameter and `type` indicates the type of `vprintf`:
- *  - `""` is a `vprintf` variant, `outputParamIndex` is `-1`.
- *  - `"f"` is a `vfprintf` variant, `outputParamIndex` indicates the output stream parameter.
- *  - `"s"` is a `vsprintf` variant, `outputParamIndex` indicates the output buffer parameter.
- *  - `"?"` if the type cannot be deteremined.  `outputParamIndex` is `-1`.
+ * and a variable argument list of type `va_arg`.
  */
-predicate primitiveVariadicFormatter(
-  TopLevelFunction f, string type, int formatParamIndex, int outputParamIndex
-) {
-  type = f.getName().regexpCapture("_?_?va?([fs]?)n?w?printf(_s)?(_p)?(_l)?", 1) and
+predicate primitiveVariadicFormatter(TopLevelFunction f, int formatParamIndex) {
+  f.getName().regexpMatch("_?_?va?[fs]?n?w?printf(_s)?(_p)?(_l)?") and
   (
     if f.getName().matches("%\\_l")
     then formatParamIndex = f.getNumberOfParameters() - 3
     else formatParamIndex = f.getNumberOfParameters() - 2
-  ) and
-  if type = "" then outputParamIndex = -1 else outputParamIndex = 0 // Conveniently, these buffer parameters are all at index 0.
-}
-
-private predicate callsVariadicFormatter(
-  Function f, string type, int formatParamIndex, int outputParamIndex
-) {
-  // calls a variadic formatter with `formatParamIndex`, `outputParamIndex` linked
-  exists(FunctionCall fc, int format, int output |
-    variadicFormatter(fc.getTarget(), type, format, output) and
-    fc.getEnclosingFunction() = f and
-    fc.getArgument(format) = f.getParameter(formatParamIndex).getAnAccess() and
-    fc.getArgument(output) = f.getParameter(outputParamIndex).getAnAccess()
-  )
-  or
-  // calls a variadic formatter with only `formatParamIndex` linked
-  exists(FunctionCall fc, string calledType, int format, int output |
-    variadicFormatter(fc.getTarget(), calledType, format, output) and
-    fc.getEnclosingFunction() = f and
-    fc.getArgument(format) = f.getParameter(formatParamIndex).getAnAccess() and
-    not fc.getArgument(output) = f.getParameter(_).getAnAccess() and
-    (
-      calledType = "" and
-      type = ""
-      or
-      calledType != "" and
-      type = "?" // we probably should have an `outputParamIndex` link but have lost it.
-    ) and
-    outputParamIndex = -1
   )
 }
 
 /**
- * Holds if `f` is a function such as `vprintf` that has a format parameter
- * and a variable argument list of type `va_arg`. `formatParamIndex` indicates
- * the format parameter and `type` indicates the type of `vprintf`:
- *  - `""` is a `vprintf` variant, `outputParamIndex` is `-1`.
- *  - `"f"` is a `vfprintf` variant, `outputParamIndex` indicates the output stream parameter.
- *  - `"s"` is a `vsprintf` variant, `outputParamIndex` indicates the output buffer parameter.
- *  - `"?"` if the type cannot be deteremined.  `outputParamIndex` is `-1`.
+ * A standard function such as `vsprintf` that has an output parameter
+ * and a variable argument list of type `va_arg`.
  */
-predicate variadicFormatter(Function f, string type, int formatParamIndex, int outputParamIndex) {
-  primitiveVariadicFormatter(f, type, formatParamIndex, outputParamIndex)
+private predicate primitiveVariadicFormatterOutput(TopLevelFunction f, int outputParamIndex) {
+  // note: this might look like the regular expression in `primitiveVariadicFormatter`, but
+  // there is one important difference: the [fs] part is not optional, as these classify
+  // the `printf` variants that write to a buffer.
+  // Conveniently, these buffer parameters are all at index 0.
+  f.getName().regexpMatch("_?_?va?[fs]n?w?printf(_s)?(_p)?(_l)?") and outputParamIndex = 0
+}
+
+private predicate callsVariadicFormatter(Function f, int formatParamIndex) {
+  exists(FunctionCall fc, int i |
+    variadicFormatter(fc.getTarget(), i) and
+    fc.getEnclosingFunction() = f and
+    fc.getArgument(i) = f.getParameter(formatParamIndex).getAnAccess()
+  )
+}
+
+private predicate callsVariadicFormatterOutput(Function f, int outputParamIndex) {
+  exists(FunctionCall fc, int i |
+    fc.getEnclosingFunction() = f and
+    variadicFormatterOutput(fc.getTarget(), i) and
+    fc.getArgument(i) = f.getParameter(outputParamIndex).getAnAccess()
+  )
+}
+
+/**
+ * Holds if `f` is a function such as `vprintf` that takes variable argument list
+ * of type `va_arg` and writes formatted output to a buffer given as a parameter at
+ * index `outputParamIndex`, if any.
+ */
+private predicate variadicFormatterOutput(Function f, int outputParamIndex) {
+  primitiveVariadicFormatterOutput(f, outputParamIndex)
   or
   not f.isVarargs() and
-  callsVariadicFormatter(f, type, formatParamIndex, outputParamIndex)
-}
-
-/**
- * A standard function such as `vprintf` that has a format parameter
- * and a variable argument list of type `va_arg`.
- *
- * DEPRECATED: Use the four argument version instead.
- */
-deprecated predicate primitiveVariadicFormatter(TopLevelFunction f, int formatParamIndex) {
-  primitiveVariadicFormatter(f, _, formatParamIndex, _)
+  callsVariadicFormatterOutput(f, outputParamIndex)
 }
 
 /**
  * Holds if `f` is a function such as `vprintf` that has a format parameter
  * (at `formatParamIndex`) and a variable argument list of type `va_arg`.
- *
- * DEPRECATED: Use the four argument version instead.
  */
-deprecated predicate variadicFormatter(Function f, int formatParamIndex) {
-  variadicFormatter(f, _, formatParamIndex, _)
+predicate variadicFormatter(Function f, int formatParamIndex) {
+  primitiveVariadicFormatter(f, formatParamIndex)
+  or
+  not f.isVarargs() and
+  callsVariadicFormatter(f, formatParamIndex)
 }
 
 /**
@@ -124,17 +104,11 @@ deprecated predicate variadicFormatter(Function f, int formatParamIndex) {
 class UserDefinedFormattingFunction extends FormattingFunction {
   override string getAPrimaryQlClass() { result = "UserDefinedFormattingFunction" }
 
-  UserDefinedFormattingFunction() { isVarargs() and callsVariadicFormatter(this, _, _, _) }
+  UserDefinedFormattingFunction() { isVarargs() and callsVariadicFormatter(this, _) }
 
-  override int getFormatParameterIndex() { callsVariadicFormatter(this, _, result, _) }
+  override int getFormatParameterIndex() { callsVariadicFormatter(this, result) }
 
-  override int getOutputParameterIndex(boolean isStream) {
-    callsVariadicFormatter(this, "f", _, result) and isStream = true
-    or
-    callsVariadicFormatter(this, "s", _, result) and isStream = false
-  }
-
-  override predicate isOutputGlobal() { callsVariadicFormatter(this, "", _, _) }
+  override int getOutputParameterIndex() { callsVariadicFormatterOutput(this, result) }
 }
 
 /**
@@ -1116,7 +1090,8 @@ class FormatLiteral extends Literal {
     then result = this.getFormat().substring(0, this.getConvSpecOffset(0))
     else
       result =
-        this.getFormat()
+        this
+            .getFormat()
             .substring(this.getConvSpecOffset(n - 1) + this.getConvSpec(n - 1).length(),
               this.getConvSpecOffset(n))
   }
@@ -1132,7 +1107,8 @@ class FormatLiteral extends Literal {
         if n > 0
         then
           result =
-            this.getFormat()
+            this
+                .getFormat()
                 .substring(this.getConvSpecOffset(n - 1) + this.getConvSpec(n - 1).length(),
                   this.getFormat().length())
         else result = this.getFormat()

cpp/ql/src/semmle/code/cpp/commons/Strcat.qll

@@ -9,14 +9,12 @@ import cpp
 class StrcatFunction extends Function {
   StrcatFunction() {
     getName() =
-      [
-        "strcat", // strcat(dst, src)
-        "strncat", // strncat(dst, src, max_amount)
-        "wcscat", // wcscat(dst, src)
-        "_mbscat", // _mbscat(dst, src)
-        "wcsncat", // wcsncat(dst, src, max_amount)
-        "_mbsncat", // _mbsncat(dst, src, max_amount)
-        "_mbsncat_l" // _mbsncat_l(dst, src, max_amount, locale)
-      ]
+      ["strcat", // strcat(dst, src)
+          "strncat", // strncat(dst, src, max_amount)
+          "wcscat", // wcscat(dst, src)
+          "_mbscat", // _mbscat(dst, src)
+          "wcsncat", // wcsncat(dst, src, max_amount)
+          "_mbsncat", // _mbsncat(dst, src, max_amount)
+          "_mbsncat_l"] // _mbsncat_l(dst, src, max_amount, locale)
   }
 }

cpp/ql/src/semmle/code/cpp/controlflow/Dataflow.qll

@@ -13,7 +13,7 @@ import Dereferenced
  * predicates that implement this analysis.
  */
 abstract class DataflowAnnotation extends string {
-  DataflowAnnotation() { this = ["pointer-null", "pointer-valid"] }
+  DataflowAnnotation() { this = "pointer-null" or this = "pointer-valid" }
 
   /** Holds if this annotation is the default annotation. */
   abstract predicate isDefault();
@@ -98,7 +98,7 @@ abstract class DataflowAnnotation extends string {
  * respectively.
  */
 class NullnessAnnotation extends DataflowAnnotation {
-  NullnessAnnotation() { this = ["pointer-null", "pointer-valid"] }
+  NullnessAnnotation() { this = "pointer-null" or this = "pointer-valid" }
 
   override predicate isDefault() { this = "pointer-valid" }
 

cpp/ql/src/semmle/code/cpp/dataflow/EscapesTree.qll

@@ -83,8 +83,6 @@ private predicate pointerToPointerStep(Expr pointerIn, Expr pointerOut) {
   or
   pointerIn.getConversion() = pointerOut.(ParenthesisExpr)
   or
-  pointerIn.getConversion() = pointerOut.(TemporaryObjectExpr)
-  or
   pointerIn = pointerOut.(ConditionalExpr).getThen().getFullyConverted()
   or
   pointerIn = pointerOut.(ConditionalExpr).getElse().getFullyConverted()
@@ -212,9 +210,14 @@ private predicate addressMayEscapeAt(Expr e) {
 
 private predicate addressMayEscapeMutablyAt(Expr e) {
   addressMayEscapeAt(e) and
-  exists(Type t | t = e.getType().stripTopLevelSpecifiers() |
-    t instanceof PointerType and
-    not t.(PointerType).getBaseType().isConst()
+  exists(Type t | t = e.getType().getUnderlyingType() |
+    exists(PointerType pt |
+      pt = t
+      or
+      pt = t.(SpecifiedType).getBaseType()
+    |
+      not pt.getBaseType().isConst()
+    )
     or
     t instanceof ReferenceType and
     not t.(ReferenceType).getBaseType().isConst()
@@ -222,15 +225,6 @@ private predicate addressMayEscapeMutablyAt(Expr e) {
     // If the address has been cast to an integral type, conservatively assume that it may eventually be cast back to a
     // pointer to non-const type.
     t instanceof IntegralType
-    or
-    // If we go through a temporary object step, we can take a reference to a temporary const pointer
-    // object, where the pointer doesn't point to a const value
-    exists(TemporaryObjectExpr temp, PointerType pt |
-      temp.getConversion() = e.(ReferenceToExpr) and
-      pt = temp.getType().stripTopLevelSpecifiers()
-    |
-      not pt.getBaseType().isConst()
-    )
   )
 }
 
@@ -258,7 +252,7 @@ private predicate addressFromVariableAccess(VariableAccess va, Expr e) {
   // `e` could be a pointer that is converted to a reference as the final step,
   // meaning that we pass a value that is two dereferences away from referring
   // to `va`. This happens, for example, with `void std::vector::push_back(T&&
-  // value);` when called as `v.push_back(&x)`, for a variable `x`. It
+  // value);` when called as `v.push_back(&x)`, for a static variable `x`. It
   // can also happen when taking a reference to a const pointer to a
   // (potentially non-const) value.
   exists(Expr pointerValue |

cpp/ql/src/semmle/code/cpp/dataflow/internal/AddressFlow.qll

@@ -81,8 +81,6 @@ private predicate pointerToPointerStep(Expr pointerIn, Expr pointerOut) {
   or
   pointerIn.getConversion() = pointerOut.(ParenthesisExpr)
   or
-  pointerIn.getConversion() = pointerOut.(TemporaryObjectExpr)
-  or
   pointerIn = pointerOut.(ConditionalExpr).getThen().getFullyConverted()
   or
   pointerIn = pointerOut.(ConditionalExpr).getElse().getFullyConverted()

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

@@ -802,9 +802,14 @@ abstract class AccessPathFront extends TAccessPathFront {
 
   abstract boolean toBoolNonEmpty();
 
-  TypedContent getHead() { this = TFrontHead(result) }
+  predicate headUsesContent(TypedContent tc) { this = TFrontHead(tc) }
 
-  predicate isClearedAt(Node n) { clearsContent(n, getHead().getContent()) }
+  predicate isClearedAt(Node n) {
+    exists(TypedContent tc |
+      this.headUsesContent(tc) and
+      clearsContent(n, tc.getContent())
+    )
+  }
 }
 
 class AccessPathFrontNil extends AccessPathFront, TFrontNil {

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

@@ -280,15 +280,6 @@ predicate isUnreachableInCall(Node n, DataFlowCall call) { none() } // stub impl
 
 int accessPathLimit() { result = 5 }
 
-/** The unit type. */
-private newtype TUnit = TMkUnit()
-
-/** The trivial type with a single element. */
-class Unit extends TUnit {
-  /** Gets a textual representation of this element. */
-  string toString() { result = "unit" }
-}
-
 /**
  * Holds if `n` does not require a `PostUpdateNode` as it either cannot be
  * modified or its modification cannot be observed, for example if it is a

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll

@@ -677,11 +677,6 @@ private predicate exprToExprStep_nocfg(Expr fromExpr, Expr toExpr) {
       exists(DataFlowFunction f, FunctionInput inModel, FunctionOutput outModel |
         f.hasDataFlow(inModel, outModel) and
         (
-          exists(int iIn |
-            inModel.isParameterDeref(iIn) and
-            call.passesByReference(iIn, fromExpr)
-          )
-          or
           exists(int iIn |
             inModel.isParameter(iIn) and
             fromExpr = call.getArgument(iIn)

cpp/ql/src/semmle/code/cpp/exprs/Call.qll

@@ -441,6 +441,40 @@ class ConstructorCall extends FunctionCall {
   override Constructor getTarget() { result = super.getTarget() }
 }
 
+/**
+ * A C++ `throw` expression.
+ * ```
+ * throw Exc(2);
+ * ```
+ */
+class ThrowExpr extends Expr, @throw_expr {
+  /**
+   * Gets the expression that will be thrown, if any. There is no result if
+   * `this` is a `ReThrowExpr`.
+   */
+  Expr getExpr() { result = this.getChild(0) }
+
+  override string getAPrimaryQlClass() { result = "ThrowExpr" }
+
+  override string toString() { result = "throw ..." }
+
+  override int getPrecedence() { result = 1 }
+}
+
+/**
+ * A C++ `throw` expression with no argument (which causes the current exception to be re-thrown).
+ * ```
+ * throw;
+ * ```
+ */
+class ReThrowExpr extends ThrowExpr {
+  ReThrowExpr() { this.getType() instanceof VoidType }
+
+  override string getAPrimaryQlClass() { result = "ReThrowExpr" }
+
+  override string toString() { result = "re-throw exception " }
+}
+
 /**
  * A call to a destructor.
  * ```

cpp/ql/src/semmle/code/cpp/exprs/Cast.qll

@@ -840,28 +840,6 @@ class ArrayToPointerConversion extends Conversion, @array_to_pointer {
   override predicate mayBeGloballyImpure() { none() }
 }
 
-/**
- * A node representing a temporary object created as part of an expression.
- *
- * This is most commonly seen in the following cases:
- * ```c++
- * // when binding a reference to a prvalue
- * const std::string& r = std::string("text");
- *
- * // when performing member access on a class prvalue
- * strlen(std::string("text").c_str());
- *
- * // when a prvalue of a type with a destructor is discarded
- * s.substr(0, 5);  // Return value is discarded but requires destruction
- * ```
- */
-class TemporaryObjectExpr extends Conversion, @temp_init {
-  /** Gets a textual representation of this conversion. */
-  override string toString() { result = "temporary object" }
-
-  override string getAPrimaryQlClass() { result = "TemporaryObjectExpr" }
-}
-
 /**
  * A node representing the Cast sub-class of entity `cast`.
  */

cpp/ql/src/semmle/code/cpp/exprs/ComparisonOperation.qll

@@ -64,7 +64,7 @@ class RelationalOperation extends ComparisonOperation, @rel_op_expr {
    * if the overall expression evaluates to `true`; for example on
    * `x <= 20` this is the `20`, and on `y > 0` it is `y`.
    */
-  Expr getGreaterOperand() { none() } // overridden in subclasses
+  abstract Expr getGreaterOperand();
 
   /**
    * Gets the operand on the "lesser" (or "lesser-or-equal") side
@@ -72,7 +72,7 @@ class RelationalOperation extends ComparisonOperation, @rel_op_expr {
    * if the overall expression evaluates to `true`; for example on
    * `x <= 20` this is `x`, and on `y > 0` it is the `0`.
    */
-  Expr getLesserOperand() { none() } // overridden in subclasses
+  abstract Expr getLesserOperand();
 }
 
 /**

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

@@ -6,6 +6,7 @@ import semmle.code.cpp.Element
 private import semmle.code.cpp.Enclosing
 private import semmle.code.cpp.internal.ResolveClass
 private import semmle.code.cpp.internal.AddressConstantExpression
+private import semmle.code.cpp.models.implementations.Allocation
 
 /**
  * A C/C++ expression.
@@ -838,7 +839,7 @@ class NewOrNewArrayExpr extends Expr, @any_new_expr {
    * For example, for `new int` the result is `int`.
    * For `new int[5]` the result is `int[5]`.
    */
-  Type getAllocatedType() { none() } // overridden in subclasses
+  abstract Type getAllocatedType();
 
   /**
    * Gets the pointer `p` if this expression is of the form `new(p) T...`.
@@ -847,7 +848,8 @@ class NewOrNewArrayExpr extends Expr, @any_new_expr {
    */
   Expr getPlacementPointer() {
     result =
-      this.getAllocatorCall()
+      this
+          .getAllocatorCall()
           .getArgument(this.getAllocator().(OperatorNewAllocationFunction).getPlacementArgument())
   }
 }
@@ -1144,40 +1146,6 @@ class BlockExpr extends Literal {
   Function getFunction() { code_block(underlyingElement(this), unresolveElement(result)) }
 }
 
-/**
- * A C++ `throw` expression.
- * ```
- * throw Exc(2);
- * ```
- */
-class ThrowExpr extends Expr, @throw_expr {
-  /**
-   * Gets the expression that will be thrown, if any. There is no result if
-   * `this` is a `ReThrowExpr`.
-   */
-  Expr getExpr() { result = this.getChild(0) }
-
-  override string getAPrimaryQlClass() { result = "ThrowExpr" }
-
-  override string toString() { result = "throw ..." }
-
-  override int getPrecedence() { result = 1 }
-}
-
-/**
- * A C++ `throw` expression with no argument (which causes the current exception to be re-thrown).
- * ```
- * throw;
- * ```
- */
-class ReThrowExpr extends ThrowExpr {
-  ReThrowExpr() { this.getType() instanceof VoidType }
-
-  override string getAPrimaryQlClass() { result = "ReThrowExpr" }
-
-  override string toString() { result = "re-throw exception " }
-}
-
 /**
  * A C++11 `noexcept` expression, returning `true` if its subexpression is guaranteed
  * not to `throw` exceptions.  For example:

cpp/ql/src/semmle/code/cpp/exprs/Literal.qll

@@ -47,17 +47,7 @@ class LabelLiteral extends Literal {
 }
 
 /** A character literal or a string literal. */
-class TextLiteral extends Literal {
-  TextLiteral() {
-    // String Literal
-    // Note that `AggregateLiteral`s can also have an array type, but they derive from
-    // @aggregateliteral rather than @literal.
-    this.getType() instanceof ArrayType
-    or
-    // Char literal
-    this.getValueText().regexpMatch("(?s)\\s*L?'.*")
-  }
-
+abstract class TextLiteral extends Literal {
   /** Gets a hex escape sequence that appears in the character or string literal (see [lex.ccon] in the C++ Standard). */
   string getAHexEscapeSequence(int occurrence, int offset) {
     result = getValueText().regexpFind("(?<!\\\\)\\\\x[0-9a-fA-F]+", occurrence, offset)

cpp/ql/src/semmle/code/cpp/exprs/LogicalOperation.qll

@@ -39,7 +39,7 @@ class BinaryLogicalOperation extends BinaryOperation, @bin_log_op_expr {
    * is true, `x` and `y` must also be true, so `impliesValue(x, true, true)` and
    * `impliesValue(y, true, true)` hold.
    */
-  predicate impliesValue(Expr part, boolean partIsTrue, boolean wholeIsTrue) { none() } // overridden in subclasses
+  abstract predicate impliesValue(Expr part, boolean partIsTrue, boolean wholeIsTrue);
 }
 
 /**

cpp/ql/src/semmle/code/cpp/headers/MultipleInclusion.qll

@@ -160,7 +160,8 @@ predicate ifndefDirective(PreprocessorDirective ppd, string macro) {
   ppd instanceof PreprocessorIf and
   exists(string head | head = ppd.getHead() |
     macro =
-      head.replaceAll("(", " ")
+      head
+          .replaceAll("(", " ")
           .replaceAll(")", "")
           .replaceAll("\t", " ")
           .regexpCapture("[ ]*![ ]*defined[ ]+([^ ]*)[ ]*", 1)

cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll

@@ -59,7 +59,7 @@ class Namespace extends @namespace {
   }
 }
 
-class Declaration extends @declaration {
+abstract class Declaration extends @declaration {
   string toString() { result = "QualifiedName Declaration" }
 
   /** Gets the name of this declaration. */

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

@@ -36,12 +36,24 @@ private predicate predictableInstruction(Instruction instr) {
  * library's `returnArgument` predicate.
  */
 predicate predictableOnlyFlow(string name) {
-  name =
-    [
-      "strcasestr", "strchnul", "strchr", "strchrnul", "strcmp", "strcspn", "strncmp", "strndup",
-      "strnlen", "strrchr", "strspn", "strstr", "strtod", "strtof", "strtol", "strtoll", "strtoq",
-      "strtoul"
-    ]
+  name = "strcasestr" or
+  name = "strchnul" or
+  name = "strchr" or
+  name = "strchrnul" or
+  name = "strcmp" or
+  name = "strcspn" or
+  name = "strncmp" or
+  name = "strndup" or
+  name = "strnlen" or
+  name = "strrchr" or
+  name = "strspn" or
+  name = "strstr" or
+  name = "strtod" or
+  name = "strtof" or
+  name = "strtol" or
+  name = "strtoll" or
+  name = "strtoq" or
+  name = "strtoul"
 }
 
 private DataFlow::Node getNodeForSource(Expr source) {
@@ -71,7 +83,7 @@ private class DefaultTaintTrackingCfg extends DataFlow::Configuration {
   override predicate isSink(DataFlow::Node sink) { exists(adjustedSink(sink)) }
 
   override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
-    commonTaintStep(n1, n2)
+    instructionTaintStep(n1.asInstruction(), n2.asInstruction())
   }
 
   override predicate isBarrier(DataFlow::Node node) { nodeIsBarrier(node) }
@@ -89,7 +101,7 @@ private class ToGlobalVarTaintTrackingCfg extends DataFlow::Configuration {
   }
 
   override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
-    commonTaintStep(n1, n2)
+    instructionTaintStep(n1.asInstruction(), n2.asInstruction())
     or
     writesVariable(n1.asInstruction(), n2.asVariable().(GlobalOrNamespaceVariable))
     or
@@ -113,7 +125,7 @@ private class FromGlobalVarTaintTrackingCfg extends DataFlow2::Configuration {
   override predicate isSink(DataFlow::Node sink) { exists(adjustedSink(sink)) }
 
   override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
-    commonTaintStep(n1, n2)
+    instructionTaintStep(n1.asInstruction(), n2.asInstruction())
     or
     // Additional step for flow out of variables. There is no flow _into_
     // variables in this configuration, so this step only serves to take flow
@@ -203,62 +215,19 @@ private predicate nodeIsBarrierIn(DataFlow::Node node) {
 }
 
 cached
-private predicate commonTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
-  instructionToInstructionTaintStep(fromNode.asInstruction(), toNode.asInstruction())
-  or
-  operandToInstructionTaintStep(fromNode.asOperand(), toNode.asInstruction())
-  or
-  operandToOperandTaintStep(fromNode.asOperand(), toNode.asOperand())
-}
-
-private predicate operandToOperandTaintStep(Operand fromOperand, Operand toOperand) {
-  exists(ReadSideEffectInstruction readInstr |
-    fromOperand = readInstr.getArgumentOperand() and
-    toOperand = readInstr.getSideEffectOperand()
-  )
-}
-
-private predicate operandToInstructionTaintStep(Operand fromOperand, Instruction toInstr) {
+private predicate instructionTaintStep(Instruction i1, Instruction i2) {
   // Expressions computed from tainted data are also tainted
-  exists(CallInstruction call, int argIndex | call = toInstr |
+  exists(CallInstruction call, int argIndex | call = i2 |
     isPureFunction(call.getStaticCallTarget().getName()) and
-    fromOperand = getACallArgumentOrIndirection(call, argIndex) and
-    forall(Operand argOperand | argOperand = call.getAnArgumentOperand() |
-      argOperand = getACallArgumentOrIndirection(call, argIndex) or
-      predictableInstruction(argOperand.getAnyDef())
+    i1 = getACallArgumentOrIndirection(call, argIndex) and
+    forall(Instruction arg | arg = call.getAnArgument() |
+      arg = getACallArgumentOrIndirection(call, argIndex) or predictableInstruction(arg)
     ) and
     // flow through `strlen` tends to cause dubious results, if the length is
     // bounded.
     not call.getStaticCallTarget().getName() = "strlen"
   )
   or
-  // Flow from argument to return value
-  toInstr =
-    any(CallInstruction call |
-      exists(int indexIn |
-        modelTaintToReturnValue(call.getStaticCallTarget(), indexIn) and
-        fromOperand = getACallArgumentOrIndirection(call, indexIn) and
-        not predictableOnlyFlow(call.getStaticCallTarget().getName())
-      )
-    )
-  or
-  // Flow from input argument to output argument
-  // TODO: This won't work in practice as long as all aliased memory is tracked
-  // together in a single virtual variable.
-  // TODO: Will this work on the test for `TaintedPath.ql`, where the output arg
-  // is a pointer addition expression?
-  toInstr =
-    any(WriteSideEffectInstruction outInstr |
-      exists(CallInstruction call, int indexIn, int indexOut |
-        modelTaintToParameter(call.getStaticCallTarget(), indexIn, indexOut) and
-        fromOperand = getACallArgumentOrIndirection(call, indexIn) and
-        outInstr.getIndex() = indexOut and
-        outInstr.getPrimaryInstruction() = call
-      )
-    )
-}
-
-private predicate instructionToInstructionTaintStep(Instruction i1, Instruction i2) {
   // Flow through pointer dereference
   i2.(LoadInstruction).getSourceAddress() = i1
   or
@@ -322,6 +291,29 @@ private predicate instructionToInstructionTaintStep(Instruction i1, Instruction
     read.getAnOperand().(SideEffectOperand).getAnyDef() = i1 and
     read.getArgumentDef() = i2
   )
+  or
+  // Flow from argument to return value
+  i2 =
+    any(CallInstruction call |
+      exists(int indexIn |
+        modelTaintToReturnValue(call.getStaticCallTarget(), indexIn) and
+        i1 = getACallArgumentOrIndirection(call, indexIn) and
+        not predictableOnlyFlow(call.getStaticCallTarget().getName())
+      )
+    )
+  or
+  // Flow from input argument to output argument
+  // TODO: Will this work on the test for `TaintedPath.ql`, where the output arg
+  // is a pointer addition expression?
+  i2 =
+    any(WriteSideEffectInstruction outNode |
+      exists(CallInstruction call, int indexIn, int indexOut |
+        modelTaintToParameter(call.getStaticCallTarget(), indexIn, indexOut) and
+        i1 = getACallArgumentOrIndirection(call, indexIn) and
+        outNode.getIndex() = indexOut and
+        outNode.getPrimaryInstruction() = call
+      )
+    )
 }
 
 pragma[noinline]
@@ -337,25 +329,15 @@ private InitializeParameterInstruction getInitializeParameter(IRFunction f, Para
 }
 
 /**
- * Returns the index of the side effect instruction corresponding to the specified function output,
- * if one exists.
- */
-private int getWriteSideEffectIndex(FunctionOutput output) {
-  output.isParameterDeref(result)
-  or
-  output.isQualifierObject() and result = -1
-}
-
-/**
- * Get an operand that goes into argument `argumentIndex` of `call`. This
+ * Get an instruction that goes into argument `argumentIndex` of `call`. This
  * can be either directly or through one pointer indirection.
  */
-private Operand getACallArgumentOrIndirection(CallInstruction call, int argumentIndex) {
-  result = call.getPositionalArgumentOperand(argumentIndex)
+private Instruction getACallArgumentOrIndirection(CallInstruction call, int argumentIndex) {
+  result = call.getPositionalArgument(argumentIndex)
   or
   exists(ReadSideEffectInstruction readSE |
     // TODO: why are read side effect operands imprecise?
-    result = readSE.getSideEffectOperand() and
+    result = readSE.getSideEffectOperand().getAnyDef() and
     readSE.getPrimaryInstruction() = call and
     readSE.getIndex() = argumentIndex
   )
@@ -369,7 +351,7 @@ private predicate modelTaintToParameter(Function f, int parameterIn, int paramet
       f.(TaintFunction).hasTaintFlow(modelIn, modelOut)
     ) and
     (modelIn.isParameter(parameterIn) or modelIn.isParameterDeref(parameterIn)) and
-    parameterOut = getWriteSideEffectIndex(modelOut)
+    modelOut.isParameterDeref(parameterOut)
   )
 }
 
@@ -558,7 +540,7 @@ module TaintedWithPath {
     }
 
     override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
-      commonTaintStep(n1, n2)
+      instructionTaintStep(n1.asInstruction(), n2.asInstruction())
       or
       exists(TaintTrackingConfiguration cfg | cfg.taintThroughGlobals() |
         writesVariable(n1.asInstruction(), n2.asVariable().(GlobalOrNamespaceVariable))
@@ -640,7 +622,8 @@ module TaintedWithPath {
     override predicate hasLocationInfo(
       string filepath, int startline, int startcolumn, int endline, int endcolumn
     ) {
-      this.inner()
+      this
+          .inner()
           .getLocation()
           .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
     }

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

@@ -802,9 +802,14 @@ abstract class AccessPathFront extends TAccessPathFront {
 
   abstract boolean toBoolNonEmpty();
 
-  TypedContent getHead() { this = TFrontHead(result) }
+  predicate headUsesContent(TypedContent tc) { this = TFrontHead(tc) }
 
-  predicate isClearedAt(Node n) { clearsContent(n, getHead().getContent()) }
+  predicate isClearedAt(Node n) {
+    exists(TypedContent tc |
+      this.headUsesContent(tc) and
+      clearsContent(n, tc.getContent())
+    )
+  }
 }
 
 class AccessPathFrontNil extends AccessPathFront, TFrontNil {

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

@@ -28,7 +28,11 @@ private class PrimaryArgumentNode extends ArgumentNode {
 
   PrimaryArgumentNode() { exists(CallInstruction call | op = call.getAnArgumentOperand()) }
 
-  override predicate argumentOf(DataFlowCall call, int pos) { op = call.getArgumentOperand(pos) }
+  override predicate argumentOf(DataFlowCall call, int pos) {
+    op = call.getPositionalArgumentOperand(pos)
+    or
+    op = call.getThisArgumentOperand() and pos = -1
+  }
 
   override string toString() {
     result = "Argument " + op.(PositionalArgumentOperand).getIndex()
@@ -106,10 +110,10 @@ class ReturnIndirectionNode extends ReturnNode {
   override ReturnIndirectionInstruction primary;
 
   override ReturnKind getKind() {
-    exists(int index |
-      primary.hasIndex(index) and
-      result = TIndirectReturnKind(index)
-    )
+    result = TIndirectReturnKind(-1) and
+    primary.isThisIndirection()
+    or
+    result = TIndirectReturnKind(primary.getParameter().getIndex())
   }
 }
 
@@ -224,7 +228,7 @@ private class ArrayContent extends Content, TArrayContent {
 private predicate fieldStoreStepNoChi(Node node1, FieldContent f, PostUpdateNode node2) {
   exists(StoreInstruction store, Class c |
     store = node2.asInstruction() and
-    store.getSourceValueOperand() = node1.asOperand() and
+    store.getSourceValue() = node1.asInstruction() and
     getWrittenField(store, f.(FieldContent).getAField(), c) and
     f.hasOffset(c, _, _)
   )
@@ -239,20 +243,18 @@ pragma[noinline]
 private predicate getWrittenField(Instruction instr, Field f, Class c) {
   exists(FieldAddressInstruction fa |
     fa =
-      getFieldInstruction([
-          instr.(StoreInstruction).getDestinationAddress(),
-          instr.(WriteSideEffectInstruction).getDestinationAddress()
-        ]) and
+      getFieldInstruction([instr.(StoreInstruction).getDestinationAddress(),
+            instr.(WriteSideEffectInstruction).getDestinationAddress()]) and
     f = fa.getField() and
     c = f.getDeclaringType()
   )
 }
 
 private predicate fieldStoreStepChi(Node node1, FieldContent f, PostUpdateNode node2) {
-  exists(ChiPartialOperand operand, ChiInstruction chi |
-    chi.getPartialOperand() = operand and
-    node1.asOperand() = operand and
+  exists(StoreInstruction store, ChiInstruction chi |
+    node1.asInstruction() = store and
     node2.asInstruction() = chi and
+    chi.getPartial() = store and
     exists(Class c |
       c = chi.getResultType() and
       exists(int startBit, int endBit |
@@ -260,7 +262,7 @@ private predicate fieldStoreStepChi(Node node1, FieldContent f, PostUpdateNode n
         f.hasOffset(c, startBit, endBit)
       )
       or
-      getWrittenField(operand.getDef(), f.getAField(), c) and
+      getWrittenField(store, f.getAField(), c) and
       f.hasOffset(c, _, _)
     )
   )
@@ -268,13 +270,8 @@ private predicate fieldStoreStepChi(Node node1, FieldContent f, PostUpdateNode n
 
 private predicate arrayStoreStepChi(Node node1, ArrayContent a, PostUpdateNode node2) {
   a = TArrayContent() and
-  exists(ChiPartialOperand operand, ChiInstruction chi, StoreInstruction store |
-    chi.getPartialOperand() = operand and
-    store = operand.getDef() and
-    node1.asOperand() = operand and
-    // This `ChiInstruction` will always have a non-conflated result because both `ArrayStoreNode`
-    // and `PointerStoreNode` require it in their characteristic predicates.
-    node2.asInstruction() = chi and
+  exists(StoreInstruction store |
+    node1.asInstruction() = store and
     (
       // `x[i] = taint()`
       // This matches the characteristic predicate in `ArrayStoreNode`.
@@ -283,7 +280,10 @@ private predicate arrayStoreStepChi(Node node1, ArrayContent a, PostUpdateNode n
       // `*p = taint()`
       // This matches the characteristic predicate in `PointerStoreNode`.
       store.getDestinationAddress().(CopyValueInstruction).getUnary() instanceof LoadInstruction
-    )
+    ) and
+    // This `ChiInstruction` will always have a non-conflated result because both `ArrayStoreNode`
+    // and `PointerStoreNode` require it in their characteristic predicates.
+    node2.asInstruction().(ChiInstruction).getPartial() = store
   )
 }
 
@@ -304,7 +304,7 @@ predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
 private predicate fieldStoreStepAfterArraySuppression(
   Node node1, FieldContent f, PostUpdateNode node2
 ) {
-  exists(WriteSideEffectInstruction write, ChiInstruction chi, Class c |
+  exists(BufferMayWriteSideEffectInstruction write, ChiInstruction chi, Class c |
     not chi.isResultConflated() and
     node1.asInstruction() = chi and
     node2.asInstruction() = chi and
@@ -332,17 +332,17 @@ private predicate getLoadedField(LoadInstruction load, Field f, Class c) {
  * `node2`.
  */
 private predicate fieldReadStep(Node node1, FieldContent f, Node node2) {
-  exists(LoadOperand operand |
-    node2.asOperand() = operand and
-    node1.asInstruction() = operand.getAnyDef() and
+  exists(LoadInstruction load |
+    node2.asInstruction() = load and
+    node1.asInstruction() = load.getSourceValueOperand().getAnyDef() and
     exists(Class c |
-      c = operand.getAnyDef().getResultType() and
+      c = load.getSourceValueOperand().getAnyDef().getResultType() and
       exists(int startBit, int endBit |
-        operand.getUsedInterval(unbindInt(startBit), unbindInt(endBit)) and
+        load.getSourceValueOperand().getUsedInterval(unbindInt(startBit), unbindInt(endBit)) and
         f.hasOffset(c, startBit, endBit)
       )
       or
-      getLoadedField(operand.getUse(), f.getAField(), c) and
+      getLoadedField(load, f.getAField(), c) and
       f.hasOffset(c, _, _)
     )
   )
@@ -363,7 +363,7 @@ private predicate fieldReadStep(Node node1, FieldContent f, Node node2) {
  */
 predicate suppressArrayRead(Node node1, ArrayContent a, Node node2) {
   a = TArrayContent() and
-  exists(WriteSideEffectInstruction write, ChiInstruction chi |
+  exists(BufferMayWriteSideEffectInstruction write, ChiInstruction chi |
     node1.asInstruction() = write and
     node2.asInstruction() = chi and
     chi.getPartial() = write and
@@ -384,20 +384,20 @@ private Instruction skipOneCopyValueInstructionRec(CopyValueInstruction copy) {
   result = skipOneCopyValueInstructionRec(copy.getUnary())
 }
 
-private Instruction skipCopyValueInstructions(Operand op) {
-  not result instanceof CopyValueInstruction and result = op.getDef()
+private Instruction skipCopyValueInstructions(Instruction instr) {
+  not result instanceof CopyValueInstruction and result = instr
   or
-  result = skipOneCopyValueInstructionRec(op.getDef())
+  result = skipOneCopyValueInstructionRec(instr)
 }
 
 private predicate arrayReadStep(Node node1, ArrayContent a, Node node2) {
   a = TArrayContent() and
   // Explicit dereferences such as `*p` or `p[i]` where `p` is a pointer or array.
-  exists(LoadOperand operand, Instruction address |
-    operand.isDefinitionInexact() and
-    node1.asInstruction() = operand.getAnyDef() and
-    operand = node2.asOperand() and
-    address = skipCopyValueInstructions(operand.getAddressOperand()) and
+  exists(LoadInstruction load, Instruction address |
+    load.getSourceValueOperand().isDefinitionInexact() and
+    node1.asInstruction() = load.getSourceValueOperand().getAnyDef() and
+    load = node2.asInstruction() and
+    address = skipCopyValueInstructions(load.getSourceAddress()) and
     (
       address instanceof LoadInstruction or
       address instanceof ArrayToPointerConvertInstruction or
@@ -418,18 +418,18 @@ private predicate arrayReadStep(Node node1, ArrayContent a, Node node2) {
  * use(x);
  * ```
  * the load on `x` in `use(x)` will exactly overlap with its definition (in this case the definition
- * is a `WriteSideEffect`). This predicate pops the `ArrayContent` (pushed by the store in `f`)
+ * is a `BufferMayWriteSideEffect`). This predicate pops the `ArrayContent` (pushed by the store in `f`)
  * from the access path.
  */
 private predicate exactReadStep(Node node1, ArrayContent a, Node node2) {
   a = TArrayContent() and
-  exists(WriteSideEffectInstruction write, ChiInstruction chi |
+  exists(BufferMayWriteSideEffectInstruction write, ChiInstruction chi |
     not chi.isResultConflated() and
     chi.getPartial() = write and
     node1.asInstruction() = write and
     node2.asInstruction() = chi and
     // To distinquish this case from the `arrayReadStep` case we require that the entire variable was
-    // overwritten by the `WriteSideEffectInstruction` (i.e., there is a load that reads the
+    // overwritten by the `BufferMayWriteSideEffectInstruction` (i.e., there is a load that reads the
     // entire variable).
     exists(LoadInstruction load | load.getSourceValue() = chi)
   )
@@ -496,6 +496,13 @@ class DataFlowType = IRType;
 
 /** A function call relevant for data flow. */
 class DataFlowCall extends CallInstruction {
+  /**
+   * Gets the nth argument for this call.
+   *
+   * The range of `n` is from `0` to `getNumberOfArguments() - 1`.
+   */
+  Node getArgument(int n) { result.asInstruction() = this.getPositionalArgument(n) }
+
   Function getEnclosingCallable() { result = this.getEnclosingFunction() }
 }
 
@@ -503,15 +510,6 @@ predicate isUnreachableInCall(Node n, DataFlowCall call) { none() } // stub impl
 
 int accessPathLimit() { result = 5 }
 
-/** The unit type. */
-private newtype TUnit = TMkUnit()
-
-/** The trivial type with a single element. */
-class Unit extends TUnit {
-  /** Gets a textual representation of this element. */
-  string toString() { result = "unit" }
-}
-
 /**
  * Holds if `n` does not require a `PostUpdateNode` as it either cannot be
  * modified or its modification cannot be observed, for example if it is a

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

@@ -95,7 +95,7 @@ class Node extends TIRDataFlowNode {
    * Gets the uninitialized local variable corresponding to this node, if
    * any.
    */
-  deprecated LocalVariable asUninitialized() { none() }
+  LocalVariable asUninitialized() { none() }
 
   /**
    * Gets an upper bound on the type of this node.
@@ -266,8 +266,10 @@ class ParameterIndirectionNode extends ParameterNode {
 
   override predicate isParameterOf(Function f, int pos) {
     exists(int index |
-      instr.getEnclosingFunction() = f and
-      instr.hasIndex(index)
+      f.getParameter(index) = instr.getParameter()
+      or
+      index = -1 and
+      instr.getIRVariable().(IRThisVariable).getEnclosingFunction() = f
     |
       pos = getArgumentPosOfSideEffect(index)
     )
@@ -394,16 +396,16 @@ private FieldAddressInstruction getFieldInstruction(Instruction instr) {
 
 /**
  * The target of a `fieldStoreStepAfterArraySuppression` store step, which is used to convert
- * an `ArrayContent` to a `FieldContent` when the `WriteSideEffect` instruction stores
+ * an `ArrayContent` to a `FieldContent` when the `BufferMayWriteSideEffect` instruction stores
  * into a field. See the QLDoc for `suppressArrayRead` for an example of where such a conversion
  * is inserted.
  */
-private class WriteSideEffectFieldStoreQualifierNode extends PartialDefinitionNode {
+private class BufferMayWriteSideEffectFieldStoreQualifierNode extends PartialDefinitionNode {
   override ChiInstruction instr;
-  WriteSideEffectInstruction write;
+  BufferMayWriteSideEffectInstruction write;
   FieldAddressInstruction field;
 
-  WriteSideEffectFieldStoreQualifierNode() {
+  BufferMayWriteSideEffectFieldStoreQualifierNode() {
     not instr.isResultConflated() and
     instr.getPartial() = write and
     field = getFieldInstruction(write.getDestinationAddress())
@@ -474,8 +476,16 @@ class DefinitionByReferenceNode extends InstructionNode {
       instr
           .getPrimaryInstruction()
           .(CallInstruction)
-          .getArgument(instr.getIndex())
+          .getPositionalArgument(instr.getIndex())
           .getUnconvertedResultExpression()
+    or
+    result =
+      instr
+          .getPrimaryInstruction()
+          .(CallInstruction)
+          .getThisArgument()
+          .getUnconvertedResultExpression() and
+    instr.getIndex() = -1
   }
 
   /** Gets the parameter through which this value is assigned. */

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRBlock.qll

@@ -26,6 +26,15 @@ class IRBlockBase extends TIRBlock {
   /** Gets the source location of the first non-`Phi` instruction in this block. */
   final Language::Location getLocation() { result = getFirstInstruction().getLocation() }
 
+  /**
+   * INTERNAL: Do not use.
+   *
+   * Gets a string that uniquely identifies this block within its enclosing function.
+   *
+   * This predicate is used by debugging and printing code only.
+   */
+  final string getUniqueId() { result = getFirstInstruction(this).getUniqueId() }
+
   /**
    * INTERNAL: Do not use.
    *
@@ -38,15 +47,14 @@ class IRBlockBase extends TIRBlock {
       config.shouldEvaluateDebugStringsForFunction(this.getEnclosingFunction())
     ) and
     this =
-      rank[result + 1](IRBlock funcBlock, int sortOverride, int sortKey1, int sortKey2 |
+      rank[result + 1](IRBlock funcBlock, int sortOverride |
         funcBlock.getEnclosingFunction() = getEnclosingFunction() and
-        funcBlock.getFirstInstruction().hasSortKeys(sortKey1, sortKey2) and
         // Ensure that the block containing `EnterFunction` always comes first.
         if funcBlock.getFirstInstruction() instanceof EnterFunctionInstruction
         then sortOverride = 0
         else sortOverride = 1
       |
-        funcBlock order by sortOverride, sortKey1, sortKey2
+        funcBlock order by sortOverride, funcBlock.getUniqueId()
       )
   }
 
@@ -163,46 +171,6 @@ class IRBlock extends IRBlockBase {
     not strictlyDominates(result)
   }
 
-  /**
-   * Holds if this block immediately post-dominates `block`.
-   *
-   * Block `A` immediate post-dominates block `B` if block `A` strictly post-dominates block `B` and
-   * block `B` is a direct successor of block `A`.
-   */
-  final predicate immediatelyPostDominates(IRBlock block) {
-    blockImmediatelyPostDominates(this, block)
-  }
-
-  /**
-   * Holds if this block strictly post-dominates `block`.
-   *
-   * Block `A` strictly post-dominates block `B` if block `A` post-dominates block `B` and blocks `A`
-   * and `B` are not the same block.
-   */
-  final predicate strictlyPostDominates(IRBlock block) {
-    blockImmediatelyPostDominates+(this, block)
-  }
-
-  /**
-   * Holds if this block is a post-dominator of `block`.
-   *
-   * Block `A` post-dominates block `B` if any control flow path from `B` to the exit block of the
-   * function must pass through block `A`. A block always post-dominates itself.
-   */
-  final predicate postDominates(IRBlock block) { strictlyPostDominates(block) or this = block }
-
-  /**
-   * Gets a block on the post-dominance frontier of this block.
-   *
-   * The post-dominance frontier of block `A` is the set of blocks `B` such that block `A` does not
-   * post-dominate block `B`, but block `A` does post-dominate an immediate successor of block `B`.
-   */
-  pragma[noinline]
-  final IRBlock postPominanceFrontier() {
-    postDominates(result.getASuccessor()) and
-    not strictlyPostDominates(result)
-  }
-
   /**
    * Holds if this block is reachable from the entry block of its function.
    */
@@ -320,12 +288,3 @@ private module Cached {
 }
 
 private Instruction getFirstInstruction(TIRBlock block) { block = MkIRBlock(result) }
-
-private predicate blockFunctionExit(IRBlock exit) {
-  exit.getLastInstruction() instanceof ExitFunctionInstruction
-}
-
-private predicate blockPredecessor(IRBlock src, IRBlock pred) { src.getAPredecessor() = pred }
-
-private predicate blockImmediatelyPostDominates(IRBlock postDominator, IRBlock block) =
-  idominance(blockFunctionExit/1, blockPredecessor/2)(_, postDominator, block)

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRConsistency.qll

@@ -494,34 +494,4 @@ module InstructionConsistency {
       irFunc = getInstructionIRFunction(instr, irFuncText)
     )
   }
-
-  /**
-   * Holds if the object address operand for the given `FieldAddress` instruction does not have an
-   * address type.
-   */
-  query predicate fieldAddressOnNonPointer(
-    FieldAddressInstruction instr, string message, OptionalIRFunction irFunc, string irFuncText
-  ) {
-    not instr.getObjectAddressOperand().getIRType() instanceof IRAddressType and
-    message =
-      "FieldAddress instruction '" + instr.toString() +
-        "' has an object address operand that is not an address, in function '$@'." and
-    irFunc = getInstructionIRFunction(instr, irFuncText)
-  }
-
-  /**
-   * Holds if the `this` argument operand for the given `Call` instruction does not have an address
-   * type.
-   */
-  query predicate thisArgumentIsNonPointer(
-    CallInstruction instr, string message, OptionalIRFunction irFunc, string irFuncText
-  ) {
-    exists(ThisArgumentOperand thisOperand | thisOperand = instr.getThisArgumentOperand() |
-      not thisOperand.getIRType() instanceof IRAddressType
-    ) and
-    message =
-      "Call instruction '" + instr.toString() +
-        "' has a `this` argument operand that is not an address, in function '$@'." and
-    irFunc = getInstructionIRFunction(instr, irFuncText)
-  }
 }

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

@@ -171,16 +171,6 @@ class Instruction extends Construction::TStageInstruction {
    */
   final string getUniqueId() { result = Construction::getInstructionUniqueId(this) }
 
-  /**
-   * INTERNAL: Do not use.
-   *
-   * Gets two sort keys for this instruction - used to order instructions for printing
-   * in test outputs.
-   */
-  final predicate hasSortKeys(int key1, int key2) {
-    Construction::instructionHasSortKeys(this, key1, key2)
-  }
-
   /**
    * Gets the basic block that contains this instruction.
    */
@@ -582,17 +572,6 @@ class InitializeParameterInstruction extends VariableInstruction {
    * Gets the parameter initialized by this instruction.
    */
   final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
-
-  /**
-   * Holds if this instruction initializes the parameter with index `index`, or
-   * if `index` is `-1` and this instruction initializes `this`.
-   */
-  pragma[noinline]
-  final predicate hasIndex(int index) {
-    index >= 0 and index = this.getParameter().getIndex()
-    or
-    index = -1 and this.getIRVariable() instanceof IRThisVariable
-  }
 }
 
 /**
@@ -616,18 +595,6 @@ class InitializeIndirectionInstruction extends VariableInstruction {
    * Gets the parameter initialized by this instruction.
    */
   final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
-
-  /**
-   * Holds if this instruction initializes the memory pointed to by the parameter with
-   * index `index`, or if `index` is `-1` and this instruction initializes the memory
-   * pointed to by `this`.
-   */
-  pragma[noinline]
-  final predicate hasIndex(int index) {
-    index >= 0 and index = this.getParameter().getIndex()
-    or
-    index = -1 and this.getIRVariable() instanceof IRThisVariable
-  }
 }
 
 /**
@@ -802,17 +769,6 @@ class ReturnIndirectionInstruction extends VariableInstruction {
    * Holds if this instruction is the return indirection for `this`.
    */
   final predicate isThisIndirection() { var instanceof IRThisVariable }
-
-  /**
-   * Holds if this instruction is the return indirection for the parameter with index `index`, or
-   * if this instruction is the return indirection for `this` and `index` is `-1`.
-   */
-  pragma[noinline]
-  final predicate hasIndex(int index) {
-    index >= 0 and index = this.getParameter().getIndex()
-    or
-    index = -1 and this.isThisIndirection()
-  }
 }
 
 /**
@@ -1611,7 +1567,6 @@ class CallInstruction extends Instruction {
   /**
    * Gets the argument operand at the specified index.
    */
-  pragma[noinline]
   final PositionalArgumentOperand getPositionalArgumentOperand(int index) {
     result = getAnOperand() and
     result.getIndex() = index
@@ -1620,27 +1575,10 @@ class CallInstruction extends Instruction {
   /**
    * Gets the argument at the specified index.
    */
-  pragma[noinline]
   final Instruction getPositionalArgument(int index) {
     result = getPositionalArgumentOperand(index).getDef()
   }
 
-  /**
-   * Gets the argument operand at the specified index, or `this` if `index` is `-1`.
-   */
-  pragma[noinline]
-  final ArgumentOperand getArgumentOperand(int index) {
-    index >= 0 and result = getPositionalArgumentOperand(index)
-    or
-    index = -1 and result = getThisArgumentOperand()
-  }
-
-  /**
-   * Gets the argument at the specified index, or `this` if `index` is `-1`.
-   */
-  pragma[noinline]
-  final Instruction getArgument(int index) { result = getArgumentOperand(index).getDef() }
-
   /**
    * Gets the number of arguments of the call, including the `this` pointer, if any.
    */

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll

@@ -562,9 +562,6 @@ private Overlap getVariableMemoryLocationOverlap(
       use.getEndBitOffset())
 }
 
-bindingset[result, b]
-private boolean unbindBool(boolean b) { result != b.booleanNot() }
-
 MemoryLocation getResultMemoryLocation(Instruction instr) {
   exists(MemoryAccessKind kind, boolean isMayAccess |
     kind = instr.getResultMemoryAccess() and
@@ -577,8 +574,7 @@ MemoryLocation getResultMemoryLocation(Instruction instr) {
           exists(Allocation var, IRType type, IntValue startBitOffset, IntValue endBitOffset |
             hasResultMemoryAccess(instr, var, type, _, startBitOffset, endBitOffset, isMayAccess) and
             result =
-              TVariableMemoryLocation(var, type, _, startBitOffset, endBitOffset,
-                unbindBool(isMayAccess))
+              TVariableMemoryLocation(var, type, _, startBitOffset, endBitOffset, isMayAccess)
           )
         else result = TUnknownMemoryLocation(instr.getEnclosingIRFunction(), isMayAccess)
       )
@@ -586,7 +582,7 @@ MemoryLocation getResultMemoryLocation(Instruction instr) {
       kind instanceof EntireAllocationMemoryAccess and
       result =
         TEntireAllocationMemoryLocation(getAddressOperandAllocation(instr.getResultAddressOperand()),
-          unbindBool(isMayAccess))
+          isMayAccess)
       or
       kind instanceof EscapedMemoryAccess and
       result = TAllAliasedMemory(instr.getEnclosingIRFunction(), isMayAccess)

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

@@ -403,27 +403,16 @@ private import PhiInsertion
  * instruction for the virtual variable as a whole.
  */
 private module PhiInsertion {
-  /**
-   * Holds if `phiBlock` is a block in the dominance frontier of a block that has a definition of the
-   * memory location `defLocation`.
-   */
-  pragma[noinline]
-  private predicate dominanceFrontierOfDefinition(
-    Alias::MemoryLocation defLocation, OldBlock phiBlock
-  ) {
-    exists(OldBlock defBlock |
-      phiBlock = Dominance::getDominanceFrontier(defBlock) and
-      definitionHasDefinitionInBlock(defLocation, defBlock)
-    )
-  }
-
   /**
    * Holds if a `Phi` instruction needs to be inserted for location `defLocation` at the beginning of block `phiBlock`.
    */
   predicate definitionHasPhiNode(Alias::MemoryLocation defLocation, OldBlock phiBlock) {
-    dominanceFrontierOfDefinition(defLocation, phiBlock) and
-    /* We can also eliminate those nodes where the definition is not live on any incoming edge */
-    definitionLiveOnEntryToBlock(defLocation, phiBlock)
+    exists(OldBlock defBlock |
+      phiBlock = Dominance::getDominanceFrontier(defBlock) and
+      definitionHasDefinitionInBlock(defLocation, defBlock) and
+      /* We can also eliminate those nodes where the definition is not live on any incoming edge */
+      definitionLiveOnEntryToBlock(defLocation, phiBlock)
+    )
   }
 
   /**
@@ -867,8 +856,7 @@ private module CachedForDebugging {
     exists(Alias::MemoryLocation location, OldBlock phiBlock, string specificity |
       instr = getPhi(phiBlock, location) and
       result =
-        "Phi Block(" + phiBlock.getFirstInstruction().getUniqueId() + ")[" + specificity + "]: " +
-          location.getUniqueId() and
+        "Phi Block(" + phiBlock.getUniqueId() + ")[" + specificity + "]: " + location.getUniqueId() and
       if location instanceof Alias::VirtualVariable
       then
         // Sort Phi nodes for virtual variables before Phi nodes for member locations.
@@ -885,24 +873,6 @@ private module CachedForDebugging {
     result.getAST() = var.getAST() and
     result.getTag() = var.getTag()
   }
-
-  cached
-  predicate instructionHasSortKeys(Instruction instr, int key1, int key2) {
-    exists(OldInstruction oldInstr |
-      oldInstr = getOldInstruction(instr) and
-      oldInstr.hasSortKeys(key1, key2)
-    )
-    or
-    instr instanceof TUnreachedInstruction and
-    key1 = maxValue() and
-    key2 = maxValue()
-  }
-
-  /**
-   * Returns the value of the maximum representable integer.
-   */
-  cached
-  int maxValue() { result = 2147483647 }
 }
 
 module SSAConsistency {

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRBlock.qll

@@ -26,6 +26,15 @@ class IRBlockBase extends TIRBlock {
   /** Gets the source location of the first non-`Phi` instruction in this block. */
   final Language::Location getLocation() { result = getFirstInstruction().getLocation() }
 
+  /**
+   * INTERNAL: Do not use.
+   *
+   * Gets a string that uniquely identifies this block within its enclosing function.
+   *
+   * This predicate is used by debugging and printing code only.
+   */
+  final string getUniqueId() { result = getFirstInstruction(this).getUniqueId() }
+
   /**
    * INTERNAL: Do not use.
    *
@@ -38,15 +47,14 @@ class IRBlockBase extends TIRBlock {
       config.shouldEvaluateDebugStringsForFunction(this.getEnclosingFunction())
     ) and
     this =
-      rank[result + 1](IRBlock funcBlock, int sortOverride, int sortKey1, int sortKey2 |
+      rank[result + 1](IRBlock funcBlock, int sortOverride |
         funcBlock.getEnclosingFunction() = getEnclosingFunction() and
-        funcBlock.getFirstInstruction().hasSortKeys(sortKey1, sortKey2) and
         // Ensure that the block containing `EnterFunction` always comes first.
         if funcBlock.getFirstInstruction() instanceof EnterFunctionInstruction
         then sortOverride = 0
         else sortOverride = 1
       |
-        funcBlock order by sortOverride, sortKey1, sortKey2
+        funcBlock order by sortOverride, funcBlock.getUniqueId()
       )
   }
 
@@ -163,46 +171,6 @@ class IRBlock extends IRBlockBase {
     not strictlyDominates(result)
   }
 
-  /**
-   * Holds if this block immediately post-dominates `block`.
-   *
-   * Block `A` immediate post-dominates block `B` if block `A` strictly post-dominates block `B` and
-   * block `B` is a direct successor of block `A`.
-   */
-  final predicate immediatelyPostDominates(IRBlock block) {
-    blockImmediatelyPostDominates(this, block)
-  }
-
-  /**
-   * Holds if this block strictly post-dominates `block`.
-   *
-   * Block `A` strictly post-dominates block `B` if block `A` post-dominates block `B` and blocks `A`
-   * and `B` are not the same block.
-   */
-  final predicate strictlyPostDominates(IRBlock block) {
-    blockImmediatelyPostDominates+(this, block)
-  }
-
-  /**
-   * Holds if this block is a post-dominator of `block`.
-   *
-   * Block `A` post-dominates block `B` if any control flow path from `B` to the exit block of the
-   * function must pass through block `A`. A block always post-dominates itself.
-   */
-  final predicate postDominates(IRBlock block) { strictlyPostDominates(block) or this = block }
-
-  /**
-   * Gets a block on the post-dominance frontier of this block.
-   *
-   * The post-dominance frontier of block `A` is the set of blocks `B` such that block `A` does not
-   * post-dominate block `B`, but block `A` does post-dominate an immediate successor of block `B`.
-   */
-  pragma[noinline]
-  final IRBlock postPominanceFrontier() {
-    postDominates(result.getASuccessor()) and
-    not strictlyPostDominates(result)
-  }
-
   /**
    * Holds if this block is reachable from the entry block of its function.
    */
@@ -320,12 +288,3 @@ private module Cached {
 }
 
 private Instruction getFirstInstruction(TIRBlock block) { block = MkIRBlock(result) }
-
-private predicate blockFunctionExit(IRBlock exit) {
-  exit.getLastInstruction() instanceof ExitFunctionInstruction
-}
-
-private predicate blockPredecessor(IRBlock src, IRBlock pred) { src.getAPredecessor() = pred }
-
-private predicate blockImmediatelyPostDominates(IRBlock postDominator, IRBlock block) =
-  idominance(blockFunctionExit/1, blockPredecessor/2)(_, postDominator, block)