Merge remote-tracking branch 'upstream/master' into Expr-location-workaround

cpp/ql/src/Critical/FileClosed.qll

@@ -1,5 +1,6 @@
 import semmle.code.cpp.pointsto.PointsTo
 
+/** Holds if there exists a call to a function that might close the file specified by `e`. */
 predicate closed(Expr e) {
   fcloseCall(_, e) or
   exists(ExprCall c |
@@ -8,10 +9,19 @@ predicate closed(Expr e) {
   )
 }
 
+/** An expression for which there exists a function call that might close it. */
 class ClosedExpr extends PointsToExpr {
   ClosedExpr() { closed(this) }
 
   override predicate interesting() { closed(this) }
 }
 
+/**
+ * Holds if `fc` is a call to a function that opens a file that might be closed. For example:
+ * ```
+ * FILE* f = fopen("file.txt", "r");
+ * ...
+ * fclose(f);
+ * ```
+ */
 predicate fopenCallMayBeClosed(FunctionCall fc) { fopenCall(fc) and anythingPointsTo(fc) }

cpp/ql/src/Critical/LoopBounds.qll

@@ -2,12 +2,24 @@
 
 import cpp
 
+/**
+ * An assignment to a variable with the value `0`. For example:
+ * ```
+ * int x;
+ * x = 0;
+ * ```
+ * but not:
+ * ```
+ * int x = 0;
+ * ```
+ */
 class ZeroAssignment extends AssignExpr {
   ZeroAssignment() {
     this.getAnOperand() instanceof VariableAccess and
     this.getAnOperand() instanceof Zero
   }
 
+  /** Gets a variable that is assigned the value `0`. */
   Variable assignedVariable() { result.getAnAccess() = this.getAnOperand() }
 }
 

cpp/ql/src/Critical/MemoryFreed.qll

@@ -4,15 +4,24 @@ private predicate freed(Expr e) {
   e = any(DeallocationExpr de).getFreedExpr()
   or
   exists(ExprCall c |
-    // cautiously assume that any ExprCall could be a freeCall.
+    // cautiously assume that any `ExprCall` could be a deallocation expression.
     c.getAnArgument() = e
   )
 }
 
+/** An expression that might be deallocated. */
 class FreedExpr extends PointsToExpr {
   FreedExpr() { freed(this) }
 
   override predicate interesting() { freed(this) }
 }
 
+/**
+ * An allocation expression that might be deallocated. For example:
+ * ```
+ * int* p = new int;
+ * ...
+ * delete p;
+ * ```
+ */
 predicate allocMayBeFreed(AllocationExpr alloc) { anythingPointsTo(alloc) }

cpp/ql/src/Critical/Negativity.qll

@@ -1,10 +1,19 @@
 import cpp
 
+/**
+ * Holds if `val` is an access to the variable `v`, or if `val`
+ * is an assignment with an access to `v` on the left-hand side.
+ */
 predicate valueOfVar(Variable v, Expr val) {
   val = v.getAnAccess() or
   val.(AssignExpr).getLValue() = v.getAnAccess()
 }
 
+/**
+ * Holds if either:
+ * - `cond` is an (in)equality expression that compares the variable `v` to the value `-1`, or
+ * - `cond` is a relational expression that compares the variable `v` to a constant.
+ */
 predicate boundsCheckExpr(Variable v, Expr cond) {
   exists(EQExpr eq |
     cond = eq and
@@ -43,6 +52,18 @@ predicate boundsCheckExpr(Variable v, Expr cond) {
   )
 }
 
+/**
+ * Holds if `node` is an expression in a conditional statement and `succ` is an
+ * immediate successor of `node` that may be reached after evaluating `node`.
+ * For example, given
+ * ```
+ * if (a < 10 && b) func1();
+ * else func2();
+ * ```
+ * this predicate holds when either:
+ * - `node` is `a < 10` and `succ` is `func2()` or `b`, or
+ * - `node` is `b` and `succ` is `func1()` or `func2()`
+ */
 predicate conditionalSuccessor(ControlFlowNode node, ControlFlowNode succ) {
   if node.isCondition()
   then succ = node.getATrueSuccessor() or succ = node.getAFalseSuccessor()
@@ -52,6 +73,12 @@ predicate conditionalSuccessor(ControlFlowNode node, ControlFlowNode succ) {
     )
 }
 
+/**
+ * Holds if the current value of the variable `v` at control-flow
+ * node `n` has been used either in:
+ * - an (in)equality comparison with the value `-1`, or
+ * - a relational comparison that compares `v` to a constant.
+ */
 predicate boundsChecked(Variable v, ControlFlowNode node) {
   exists(Expr test |
     boundsCheckExpr(v, test) and
@@ -63,6 +90,14 @@ predicate boundsChecked(Variable v, ControlFlowNode node) {
   )
 }
 
+/**
+ * Holds if `cond` compares `v` to some common error values. Specifically, this
+ * predicate holds when:
+ * - `cond` checks that `v` is equal to `-1`, or
+ * - `cond` checks that `v` is less than `0`, or
+ * - `cond` checks that `v` is less than or equal to `-1`, or
+ * - `cond` checks that `v` is not some common success value (see `successCondition`).
+ */
 predicate errorCondition(Variable v, Expr cond) {
   exists(EQExpr eq |
     cond = eq and
@@ -88,6 +123,14 @@ predicate errorCondition(Variable v, Expr cond) {
   )
 }
 
+/**
+ * Holds if `cond` compares `v` to some common success values. Specifically, this
+ * predicate holds when:
+ * - `cond` checks that `v` is not equal to `-1`, or
+ * - `cond` checks that `v` is greater than or equal than `0`, or
+ * - `cond` checks that `v` is greater than `-1`, or
+ * - `cond` checks that `v` is not some common error value (see `errorCondition`).
+ */
 predicate successCondition(Variable v, Expr cond) {
   exists(NEExpr ne |
     cond = ne and
@@ -113,6 +156,11 @@ predicate successCondition(Variable v, Expr cond) {
   )
 }
 
+/**
+ * Holds if there exists a comparison operation that checks whether `v`
+ * represents some common *error* values, and `n` may be reached
+ * immediately following the comparison operation.
+ */
 predicate errorSuccessor(Variable v, ControlFlowNode n) {
   exists(Expr cond |
     errorCondition(v, cond) and n = cond.getATrueSuccessor()
@@ -121,6 +169,11 @@ predicate errorSuccessor(Variable v, ControlFlowNode n) {
   )
 }
 
+/**
+ * Holds if there exists a comparison operation that checks whether `v`
+ * represents some common *success* values, and `n` may be reached
+ * immediately following the comparison operation.
+ */
 predicate successSuccessor(Variable v, ControlFlowNode n) {
   exists(Expr cond |
     successCondition(v, cond) and n = cond.getATrueSuccessor()
@@ -129,6 +182,10 @@ predicate successSuccessor(Variable v, ControlFlowNode n) {
   )
 }
 
+/**
+ * Holds if the current value of the variable `v` at control-flow node
+ * `n` may have been checked against a common set of *error* values.
+ */
 predicate checkedError(Variable v, ControlFlowNode n) {
   errorSuccessor(v, n)
   or
@@ -139,6 +196,10 @@ predicate checkedError(Variable v, ControlFlowNode n) {
   )
 }
 
+/**
+ * Holds if the current value of the variable `v` at control-flow node
+ * `n` may have been checked against a common set of *success* values.
+ */
 predicate checkedSuccess(Variable v, ControlFlowNode n) {
   successSuccessor(v, n)
   or

cpp/ql/src/Critical/NewDelete.qll

@@ -5,17 +5,34 @@
 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
  * a string describing the type of the allocation.
  */
 predicate allocExpr(Expr alloc, string kind) {
-  isAllocationExpr(alloc) and
-  not alloc.isFromUninstantiatedTemplate(_) and
   (
-    alloc instanceof FunctionCall and
-    kind = "malloc"
+    exists(Function target |
+      alloc.(AllocationExpr).(FunctionCall).getTarget() = target and
+      (
+        target.getName() = "operator new" and
+        kind = "new" and
+        // exclude placement new and custom overloads as they
+        // may not conform to assumptions
+        not target.getNumberOfParameters() > 1
+        or
+        target.getName() = "operator new[]" and
+        kind = "new[]" and
+        // exclude placement new and custom overloads as they
+        // may not conform to assumptions
+        not target.getNumberOfParameters() > 1
+        or
+        not target instanceof OperatorNewAllocationFunction and
+        kind = "malloc"
+      )
+    )
     or
     alloc instanceof NewExpr and
     kind = "new" and
@@ -28,7 +45,8 @@ predicate allocExpr(Expr alloc, string kind) {
     // exclude placement new and custom overloads as they
     // may not conform to assumptions
     not alloc.(NewArrayExpr).getAllocatorCall().getTarget().getNumberOfParameters() > 1
-  )
+  ) and
+  not alloc.isFromUninstantiatedTemplate(_)
 }
 
 /**
@@ -110,8 +128,20 @@ predicate allocReaches(Expr e, Expr alloc, string kind) {
  * describing the type of that free or delete.
  */
 predicate freeExpr(Expr free, Expr freed, string kind) {
-  freeCall(free, freed) and
-  kind = "free"
+  exists(Function target |
+    freed = free.(DeallocationExpr).getFreedExpr() and
+    free.(FunctionCall).getTarget() = target and
+    (
+      target.getName() = "operator delete" and
+      kind = "delete"
+      or
+      target.getName() = "operator delete[]" and
+      kind = "delete[]"
+      or
+      not target instanceof OperatorDeleteDeallocationFunction and
+      kind = "free"
+    )
+  )
   or
   free.(DeleteExpr).getExpr() = freed and
   kind = "delete"

cpp/ql/src/Documentation/CaptionedComments.qll

@@ -4,6 +4,10 @@
 
 import cpp
 
+/**
+ * Gets a string representation of the comment `c` containing the caption 'TODO' or 'FIXME'.
+ * If `c` spans multiple lines, all lines after the first are abbreviated as [...].
+ */
 string getCommentTextCaptioned(Comment c, string caption) {
   (caption = "TODO" or caption = "FIXME") and
   exists(

cpp/ql/src/Documentation/CommentedOutCode.qll

@@ -1,3 +1,7 @@
+/**
+ * Provides classes and predicates for identifying C/C++ comments that look like code.
+ */
+
 import cpp
 
 /**
@@ -137,8 +141,14 @@ class CommentBlock extends Comment {
     )
   }
 
+  /**
+   * Gets the last comment associated with this comment block.
+   */
   Comment lastComment() { result = this.getComment(max(int i | exists(this.getComment(i)))) }
 
+  /**
+   * Gets the contents of the `i`'th comment associated with this comment block.
+   */
   string getLine(int i) {
     this instanceof CStyleComment and
     result = this.getContents().regexpCapture("(?s)/\\*+(.*)\\*+/", 1).splitAt("\n", i)
@@ -146,14 +156,24 @@ class CommentBlock extends Comment {
     this instanceof CppStyleComment and result = this.getComment(i).getContents().suffix(2)
   }
 
+  /**
+   * Gets the number of lines in the comments associated with this comment block.
+   */
   int numLines() {
     result = strictcount(int i, string line | line = this.getLine(i) and line.trim() != "")
   }
 
+  /**
+   * Gets the number of lines that look like code in the comments associated with this comment block.
+   */
   int numCodeLines() {
     result = strictcount(int i, string line | line = this.getLine(i) and looksLikeCode(line))
   }
 
+  /**
+   * Holds if the comment block is a C-style comment, and each
+   * comment line starts with a *.
+   */
   predicate isDocumentation() {
     // If a C-style comment starts each line with a *, then it's
     // probably documentation rather than code.
@@ -161,6 +181,12 @@ class CommentBlock extends Comment {
     forex(int i | i in [1 .. this.numLines() - 1] | this.getLine(i).trim().matches("*%"))
   }
 
+  /**
+   * Holds if this comment block looks like code that has been commented out. Specifically:
+   * 1. It does not look like documentation (see `isDocumentation`).
+   * 2. It is not in a header file without any declaration entries or top level declarations.
+   * 3. More than half of the lines in the comment block look like code.
+   */
   predicate isCommentedOutCode() {
     not this.isDocumentation() and
     not this.getFile().(HeaderFile).noTopLevelCode() and

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

@@ -30,7 +30,7 @@ predicate allowedTypedefs(TypedefType t) {
  * Gets a type which appears literally in the declaration of `d`.
  */
 Type getAnImmediateUsedType(Declaration d) {
-  d.isDefined() and
+  d.hasDefinition() and
   (
     result = d.(Function).getType() or
     result = d.(Variable).getType()

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

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

cpp/ql/src/Likely Bugs/ReturnConstTypeMember.cpp

@@ -8,6 +8,6 @@ struct S {
   
   // Whereas here it does make a semantic difference.
   auto getValCorrect() const -> int {
-    return val
+    return val;
   }
 };

cpp/ql/src/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.qll

@@ -6,6 +6,7 @@
 
 import cpp
 
+pragma[inline]
 private predicate arithTypesMatch(Type arg, Type parm) {
   arg = parm
   or

cpp/ql/src/Likely Bugs/Underspecified Functions/TooFewArguments.qll

@@ -6,29 +6,50 @@
 
 import cpp
 
-// True if function was ()-declared, but not (void)-declared or K&R-defined
+/**
+ * Holds if `fde` has a parameter declaration that's clear on the minimum
+ * number of parameters. This is essentially true for everything except
+ * `()`-declarations.
+ */
+private predicate hasDefiniteNumberOfParameters(FunctionDeclarationEntry fde) {
+  fde.hasVoidParamList()
+  or
+  fde.getNumberOfParameters() > 0
+  or
+  fde.isDefinition()
+}
+
+/* Holds if function was ()-declared, but not (void)-declared or K&R-defined. */
 private predicate hasZeroParamDecl(Function f) {
   exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
-    not fde.hasVoidParamList() and fde.getNumberOfParameters() = 0 and not fde.isDefinition()
+    not hasDefiniteNumberOfParameters(fde)
   )
 }
 
-// True if this file (or header) was compiled as a C file
+/* Holds if this file (or header) was compiled as a C file. */
 private predicate isCompiledAsC(File f) {
   f.compiledAsC()
   or
   exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
 }
 
+/** Holds if `fc` is a call to `f` with too few arguments. */
 predicate tooFewArguments(FunctionCall fc, Function f) {
   f = fc.getTarget() and
   not f.isVarargs() and
   not f instanceof BuiltInFunction and
+  // This query should only have results on C (not C++) functions that have a
+  // `()` parameter list somewhere. If it has results on other functions, then
+  // it's probably because the extractor only saw a partial compilation.
   hasZeroParamDecl(f) and
   isCompiledAsC(f.getFile()) and
-  // There is an explicit declaration of the function whose parameter count is larger
-  // than the number of call arguments
-  exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
+  // Produce an alert when all declarations that are authoritative on the
+  // parameter count specify a parameter count larger than the number of call
+  // arguments.
+  forex(FunctionDeclarationEntry fde |
+    fde = f.getADeclarationEntry() and
+    hasDefiniteNumberOfParameters(fde)
+  |
     fde.getNumberOfParameters() > fc.getNumberOfArguments()
   )
 }

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

@@ -1,27 +0,0 @@
-/**
- * @name Churned lines per file
- * @description Number of churned lines per file, across the revision
- *              history in the database.
- * @kind treemap
- * @id cpp/historical-churn
- * @treemap.warnOn highValues
- * @metricType file
- * @metricAggregate avg sum max
- * @tags external-data
- * @deprecated
- */
-
-import cpp
-import external.VCS
-
-from File f, int n
-where
-  n =
-    sum(Commit entry, int churn |
-      churn = entry.getRecentChurnForFile(f) and
-      not artificialChange(entry)
-    |
-      churn
-    ) and
-  exists(f.getMetrics().getNumberOfLinesOfCode())
-select f, n order by n desc

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

@@ -1,27 +0,0 @@
-/**
- * @name Added lines per file
- * @description Number of added lines per file, across the revision
- *              history in the database.
- * @kind treemap
- * @id cpp/historical-lines-added
- * @treemap.warnOn highValues
- * @metricType file
- * @metricAggregate avg sum max
- * @tags external-data
- * @deprecated
- */
-
-import cpp
-import external.VCS
-
-from File f, int n
-where
-  n =
-    sum(Commit entry, int churn |
-      churn = entry.getRecentAdditionsForFile(f) and
-      not artificialChange(entry)
-    |
-      churn
-    ) and
-  exists(f.getMetrics().getNumberOfLinesOfCode())
-select f, n order by n desc

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

@@ -1,27 +0,0 @@
-/**
- * @name Deleted lines per file
- * @description Number of deleted lines per file, across the revision
- *              history in the database.
- * @kind treemap
- * @id cpp/historical-lines-deleted
- * @treemap.warnOn highValues
- * @metricType file
- * @metricAggregate avg sum max
- * @tags external-data
- * @deprecated
- */
-
-import cpp
-import external.VCS
-
-from File f, int n
-where
-  n =
-    sum(Commit entry, int churn |
-      churn = entry.getRecentDeletionsForFile(f) and
-      not artificialChange(entry)
-    |
-      churn
-    ) and
-  exists(f.getMetrics().getNumberOfLinesOfCode())
-select f, n order by n desc

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

@@ -1,18 +0,0 @@
-/**
- * @name Number of authors
- * @description Number of distinct authors for each file.
- * @kind treemap
- * @id cpp/historical-number-of-authors
- * @treemap.warnOn highValues
- * @metricType file
- * @metricAggregate avg min max
- * @tags external-data
- * @deprecated
- */
-
-import cpp
-import external.VCS
-
-from File f
-where exists(f.getMetrics().getNumberOfLinesOfCode())
-select f, count(Author author | author.getAnEditedFile() = f)

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

@@ -1,19 +0,0 @@
-/**
- * @name Number of file-level changes
- * @description The number of file-level changes made (by version
- *              control history).
- * @kind treemap
- * @id cpp/historical-number-of-changes
- * @treemap.warnOn highValues
- * @metricType file
- * @metricAggregate avg min max sum
- * @tags external-data
- * @deprecated
- */
-
-import cpp
-import external.VCS
-
-from File f
-where exists(f.getMetrics().getNumberOfLinesOfCode())
-select f, count(Commit svn | f = svn.getAnAffectedFile() and not artificialChange(svn))

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

@@ -1,21 +0,0 @@
-/**
- * @name Number of co-committed files
- * @description The average number of other files that are touched
- *              whenever a file is affected by a commit.
- * @kind treemap
- * @id cpp/historical-number-of-co-commits
- * @treemap.warnOn highValues
- * @metricType file
- * @metricAggregate avg min max
- * @tags external-data
- * @deprecated
- */
-
-import cpp
-import external.VCS
-
-int committedFiles(Commit commit) { result = count(commit.getAnAffectedFile()) }
-
-from File f
-where exists(f.getMetrics().getNumberOfLinesOfCode())
-select f, avg(Commit commit | commit.getAnAffectedFile() = f | committedFiles(commit) - 1)

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

@@ -1,37 +0,0 @@
-/**
- * @name Number of re-commits for each file
- * @description A re-commit is taken to mean a commit to a file that
- *              was touched less than five days ago.
- * @kind treemap
- * @id cpp/historical-number-of-re-commits
- * @treemap.warnOn highValues
- * @metricType file
- * @metricAggregate avg min max
- * @tags external-data
- * @deprecated
- */
-
-import cpp
-import external.VCS
-
-predicate inRange(Commit first, Commit second) {
-  first.getAnAffectedFile() = second.getAnAffectedFile() and
-  first != second and
-  exists(int n |
-    n = first.getDate().daysTo(second.getDate()) and
-    n >= 0 and
-    n < 5
-  )
-}
-
-int recommitsForFile(File f) {
-  result =
-    count(Commit recommit |
-      f = recommit.getAnAffectedFile() and
-      exists(Commit prev | inRange(prev, recommit))
-    )
-}
-
-from File f
-where exists(f.getMetrics().getNumberOfLinesOfCode())
-select f, recommitsForFile(f)

cpp/ql/src/Metrics/History/HNumberOfRecentAuthors.ql

@@ -1,27 +0,0 @@
-/**
- * @name Number of recent authors
- * @description Number of distinct authors that have recently made
- *              changes.
- * @kind treemap
- * @id cpp/historical-number-of-recent-authors
- * @treemap.warnOn highValues
- * @metricType file
- * @metricAggregate avg min max
- * @tags external-data
- * @deprecated
- */
-
-import cpp
-import external.VCS
-
-from File f
-where exists(f.getMetrics().getNumberOfLinesOfCode())
-select f,
-  count(Author author |
-    exists(Commit e |
-      e = author.getACommit() and
-      f = e.getAnAffectedFile() and
-      e.daysToNow() <= 180 and
-      not artificialChange(e)
-    )
-  )

cpp/ql/src/Metrics/History/HNumberOfRecentChangedFiles.ql

@@ -1,24 +0,0 @@
-/**
- * @name Recently changed files
- * @description Number of files recently edited.
- * @kind treemap
- * @id cpp/historical-number-of-recent-changed-files
- * @treemap.warnOn highValues
- * @metricType file
- * @metricAggregate avg min max sum
- * @tags external-data
- * @deprecated
- */
-
-import cpp
-import external.VCS
-
-from File f
-where
-  exists(Commit e |
-    e.getAnAffectedFile() = f and
-    e.daysToNow() <= 180 and
-    not artificialChange(e)
-  ) and
-  exists(f.getMetrics().getNumberOfLinesOfCode())
-select f, 1

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

@@ -1,25 +0,0 @@
-/**
- * @name Recent changes
- * @description Number of recent commits to this file.
- * @kind treemap
- * @id cpp/historical-number-of-recent-changes
- * @treemap.warnOn highValues
- * @metricType file
- * @metricAggregate avg min max sum
- * @tags external-data
- * @deprecated
- */
-
-import cpp
-import external.VCS
-
-from File f, int n
-where
-  n =
-    count(Commit e |
-      e.getAnAffectedFile() = f and
-      e.daysToNow() <= 180 and
-      not artificialChange(e)
-    ) and
-  exists(f.getMetrics().getNumberOfLinesOfCode())
-select f, n order by n desc

cpp/ql/src/Options.qll

@@ -4,7 +4,7 @@
  *
  * By default they fall back to the reasonable defaults provided in
  * `DefaultOptions.qll`, but by modifying this file, you can customize
- * the standard Semmle analyses to give better results for your project.
+ * the standard analyses to give better results for your project.
  */
 
 import cpp

cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql

@@ -2,7 +2,7 @@
  * @name Uncontrolled data used in path expression
  * @description Accessing paths influenced by users can allow an
  *              attacker to access unexpected resources.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision medium
  * @id cpp/path-injection
@@ -17,6 +17,7 @@ import cpp
 import semmle.code.cpp.security.FunctionWithWrappers
 import semmle.code.cpp.security.Security
 import semmle.code.cpp.security.TaintTracking
+import TaintedWithPath
 
 /**
  * A function for opening a file.
@@ -51,12 +52,19 @@ class FileFunction extends FunctionWithWrappers {
   override predicate interestingArg(int arg) { arg = 0 }
 }
 
+class TaintedPathConfiguration extends TaintTrackingConfiguration {
+  override predicate isSink(Element tainted) {
+    exists(FileFunction fileFunction | fileFunction.outermostWrapperFunctionCall(tainted, _))
+  }
+}
+
 from
-  FileFunction fileFunction, Expr taintedArg, Expr taintSource, string taintCause, string callChain
+  FileFunction fileFunction, Expr taintedArg, Expr taintSource, PathNode sourceNode,
+  PathNode sinkNode, string taintCause, string callChain
 where
   fileFunction.outermostWrapperFunctionCall(taintedArg, callChain) and
-  tainted(taintSource, taintedArg) and
+  taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and
   isUserInput(taintSource, taintCause)
-select taintedArg,
+select taintedArg, sourceNode, sinkNode,
   "This argument to a file access function is derived from $@ and then passed to " + callChain,
   taintSource, "user input (" + taintCause + ")"

cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql

@@ -2,7 +2,7 @@
  * @name CGI script vulnerable to cross-site scripting
  * @description Writing user input directly to a web page
  *              allows for a cross-site scripting vulnerability.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id cpp/cgi-xss
@@ -13,6 +13,7 @@
 import cpp
 import semmle.code.cpp.commons.Environment
 import semmle.code.cpp.security.TaintTracking
+import TaintedWithPath
 
 /** A call that prints its arguments to `stdout`. */
 class PrintStdoutCall extends FunctionCall {
@@ -27,8 +28,13 @@ class QueryString extends EnvironmentRead {
   QueryString() { getEnvironmentVariable() = "QUERY_STRING" }
 }
 
-from QueryString query, PrintStdoutCall call, Element printedArg
-where
-  call.getAnArgument() = printedArg and
-  tainted(query, printedArg)
-select printedArg, "Cross-site scripting vulnerability due to $@.", query, "this query data"
+class Configuration extends TaintTrackingConfiguration {
+  override predicate isSink(Element tainted) {
+    exists(PrintStdoutCall call | call.getAnArgument() = tainted)
+  }
+}
+
+from QueryString query, Element printedArg, PathNode sourceNode, PathNode sinkNode
+where taintedWithPath(query, printedArg, sourceNode, sinkNode)
+select printedArg, sourceNode, sinkNode, "Cross-site scripting vulnerability due to $@.", query,
+  "this query data"

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

@@ -3,7 +3,7 @@
  * @description Including user-supplied data in a SQL query without
  *              neutralizing special elements can make code vulnerable
  *              to SQL Injection.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id cpp/sql-injection
@@ -15,6 +15,7 @@ import cpp
 import semmle.code.cpp.security.Security
 import semmle.code.cpp.security.FunctionWithWrappers
 import semmle.code.cpp.security.TaintTracking
+import TaintedWithPath
 
 class SQLLikeFunction extends FunctionWithWrappers {
   SQLLikeFunction() { sqlArgument(this.getName(), _) }
@@ -22,11 +23,19 @@ class SQLLikeFunction extends FunctionWithWrappers {
   override predicate interestingArg(int arg) { sqlArgument(this.getName(), arg) }
 }
 
-from SQLLikeFunction runSql, Expr taintedArg, Expr taintSource, string taintCause, string callChain
+class Configuration extends TaintTrackingConfiguration {
+  override predicate isSink(Element tainted) {
+    exists(SQLLikeFunction runSql | runSql.outermostWrapperFunctionCall(tainted, _))
+  }
+}
+
+from
+  SQLLikeFunction runSql, Expr taintedArg, Expr taintSource, PathNode sourceNode, PathNode sinkNode,
+  string taintCause, string callChain
 where
   runSql.outermostWrapperFunctionCall(taintedArg, callChain) and
-  tainted(taintSource, taintedArg) and
+  taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and
   isUserInput(taintSource, taintCause)
-select taintedArg,
+select taintedArg, sourceNode, sinkNode,
   "This argument to a SQL query function is derived from $@ and then passed to " + callChain,
   taintSource, "user input (" + taintCause + ")"

cpp/ql/src/Security/CWE/CWE-114/UncontrolledProcessOperation.ql

@@ -3,7 +3,7 @@
  * @description Using externally controlled strings in a process
  *              operation can allow an attacker to execute malicious
  *              commands.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision medium
  * @id cpp/uncontrolled-process-operation
@@ -14,13 +14,24 @@
 import cpp
 import semmle.code.cpp.security.Security
 import semmle.code.cpp.security.TaintTracking
+import TaintedWithPath
 
-from string processOperation, int processOperationArg, FunctionCall call, Expr arg, Element source
+predicate isProcessOperationExplanation(Expr arg, string processOperation) {
+  exists(int processOperationArg, FunctionCall call |
+    isProcessOperationArgument(processOperation, processOperationArg) and
+    call.getTarget().getName() = processOperation and
+    call.getArgument(processOperationArg) = arg
+  )
+}
+
+class Configuration extends TaintTrackingConfiguration {
+  override predicate isSink(Element arg) { isProcessOperationExplanation(arg, _) }
+}
+
+from string processOperation, Expr arg, Expr source, PathNode sourceNode, PathNode sinkNode
 where
-  isProcessOperationArgument(processOperation, processOperationArg) and
-  call.getTarget().getName() = processOperation and
-  call.getArgument(processOperationArg) = arg and
-  tainted(source, arg)
-select arg,
+  isProcessOperationExplanation(arg, processOperation) and
+  taintedWithPath(source, arg, sourceNode, sinkNode)
+select arg, sourceNode, sinkNode,
   "The value of this argument may come from $@ and is being passed to " + processOperation, source,
   source.toString()

cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.ql

@@ -2,7 +2,7 @@
  * @name Unbounded write
  * @description Buffer write operations that do not control the length
  *              of data written may overflow.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision medium
  * @id cpp/unbounded-write
@@ -16,6 +16,7 @@
 import semmle.code.cpp.security.BufferWrite
 import semmle.code.cpp.security.Security
 import semmle.code.cpp.security.TaintTracking
+import TaintedWithPath
 
 /*
  * --- Summary of CWE-120 alerts ---
@@ -54,32 +55,48 @@ predicate isUnboundedWrite(BufferWrite bw) {
  * }
  */
 
+/**
+ * Holds if `e` is a source buffer going into an unbounded write `bw` or a
+ * qualifier of (a qualifier of ...) such a source.
+ */
+predicate unboundedWriteSource(Expr e, BufferWrite bw) {
+  isUnboundedWrite(bw) and e = bw.getASource()
+  or
+  exists(FieldAccess fa | unboundedWriteSource(fa, bw) and e = fa.getQualifier())
+}
+
 /*
  * --- user input reach ---
  */
 
-/**
- * Identifies expressions that are potentially tainted with user
- * input.  Most of the work for this is actually done by the
- * TaintTracking library.
- */
-predicate tainted2(Expr expr, Expr inputSource, string inputCause) {
-  taintedIncludingGlobalVars(inputSource, expr, _) and
-  inputCause = inputSource.toString()
-  or
-  exists(Expr e | tainted2(e, inputSource, inputCause) |
-    // field accesses of a tainted struct are tainted
-    e = expr.(FieldAccess).getQualifier()
-  )
+class Configuration extends TaintTrackingConfiguration {
+  override predicate isSink(Element tainted) { unboundedWriteSource(tainted, _) }
+
+  override predicate taintThroughGlobals() { any() }
 }
 
 /*
  * --- put it together ---
  */
 
-from BufferWrite bw, Expr inputSource, string inputCause
+/*
+ * An unbounded write is, for example `strcpy(..., tainted)`. We're looking
+ * for a tainted source buffer of an unbounded write, where this source buffer
+ * is a sink in the taint-tracking analysis.
+ *
+ * In the case of `gets` and `scanf`, where the source buffer is implicit, the
+ * `BufferWrite` library reports the source buffer to be the same as the
+ * destination buffer. Since those destination-buffer arguments are also
+ * modeled in the taint-tracking library as being _sources_ of taint, they are
+ * in practice reported as being tainted because the `security.TaintTracking`
+ * library does not distinguish between taint going into an argument and out of
+ * an argument. Thus, we get the desired alerts.
+ */
+
+from BufferWrite bw, Expr inputSource, Expr tainted, PathNode sourceNode, PathNode sinkNode
 where
-  isUnboundedWrite(bw) and
-  tainted2(bw.getASource(), inputSource, inputCause)
-select bw, "This '" + bw.getBWDesc() + "' with input from $@ may overflow the destination.",
-  inputSource, inputCause
+  taintedWithPath(inputSource, tainted, sourceNode, sinkNode) and
+  unboundedWriteSource(tainted, bw)
+select bw, sourceNode, sinkNode,
+  "This '" + bw.getBWDesc() + "' with input from $@ may overflow the destination.", inputSource,
+  inputSource.toString()

cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatString.ql

@@ -3,7 +3,7 @@
  * @description Using externally-controlled format strings in
  *              printf-style functions can lead to buffer overflows
  *              or data representation problems.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision medium
  * @id cpp/tainted-format-string
@@ -16,12 +16,21 @@ import cpp
 import semmle.code.cpp.security.Security
 import semmle.code.cpp.security.FunctionWithWrappers
 import semmle.code.cpp.security.TaintTracking
+import TaintedWithPath
 
-from PrintfLikeFunction printf, Expr arg, string printfFunction, Expr userValue, string cause
+class Configuration extends TaintTrackingConfiguration {
+  override predicate isSink(Element tainted) {
+    exists(PrintfLikeFunction printf | printf.outermostWrapperFunctionCall(tainted, _))
+  }
+}
+
+from
+  PrintfLikeFunction printf, Expr arg, PathNode sourceNode, PathNode sinkNode,
+  string printfFunction, Expr userValue, string cause
 where
   printf.outermostWrapperFunctionCall(arg, printfFunction) and
-  tainted(userValue, arg) and
+  taintedWithPath(userValue, arg, sourceNode, sinkNode) and
   isUserInput(userValue, cause)
-select arg,
+select arg, sourceNode, sinkNode,
   "The value of this argument may come from $@ and is being used as a formatting argument to " +
     printfFunction, userValue, cause

cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatStringThroughGlobalVar.ql

@@ -3,7 +3,7 @@
  * @description Using externally-controlled format strings in
  *              printf-style functions can lead to buffer overflows
  *              or data representation problems.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision medium
  * @id cpp/tainted-format-string-through-global
@@ -16,15 +16,24 @@ import cpp
 import semmle.code.cpp.security.FunctionWithWrappers
 import semmle.code.cpp.security.Security
 import semmle.code.cpp.security.TaintTracking
+import TaintedWithPath
+
+class Configuration extends TaintTrackingConfiguration {
+  override predicate isSink(Element tainted) {
+    exists(PrintfLikeFunction printf | printf.outermostWrapperFunctionCall(tainted, _))
+  }
+
+  override predicate taintThroughGlobals() { any() }
+}
 
 from
-  PrintfLikeFunction printf, Expr arg, string printfFunction, Expr userValue, string cause,
-  string globalVar
+  PrintfLikeFunction printf, Expr arg, PathNode sourceNode, PathNode sinkNode,
+  string printfFunction, Expr userValue, string cause
 where
   printf.outermostWrapperFunctionCall(arg, printfFunction) and
-  not tainted(_, arg) and
-  taintedIncludingGlobalVars(userValue, arg, globalVar) and
+  not taintedWithoutGlobals(arg) and
+  taintedWithPath(userValue, arg, sourceNode, sinkNode) and
   isUserInput(userValue, cause)
-select arg,
-  "This value may flow through $@, originating from $@, and is a formatting argument to " +
-    printfFunction + ".", globalVarFromId(globalVar), globalVar, userValue, cause
+select arg, sourceNode, sinkNode,
+  "The value of this argument may come from $@ and is being used as a formatting argument to " +
+    printfFunction, userValue, cause

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

@@ -2,7 +2,7 @@
  * @name Uncontrolled data in arithmetic expression
  * @description Arithmetic operations on uncontrolled data that is not
  *              validated can cause overflows.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision medium
  * @id cpp/uncontrolled-arithmetic
@@ -15,6 +15,7 @@ import cpp
 import semmle.code.cpp.security.Overflow
 import semmle.code.cpp.security.Security
 import semmle.code.cpp.security.TaintTracking
+import TaintedWithPath
 
 predicate isRandCall(FunctionCall fc) { fc.getTarget().getName() = "rand" }
 
@@ -40,9 +41,22 @@ class SecurityOptionsArith extends SecurityOptions {
   }
 }
 
-predicate taintedVarAccess(Expr origin, VariableAccess va) {
-  isUserInput(origin, _) and
-  tainted(origin, va)
+predicate isDiv(VariableAccess va) { exists(AssignDivExpr div | div.getLValue() = va) }
+
+predicate missingGuard(VariableAccess va, string effect) {
+  exists(Operation op | op.getAnOperand() = va |
+    missingGuardAgainstUnderflow(op, va) and effect = "underflow"
+    or
+    missingGuardAgainstOverflow(op, va) and effect = "overflow"
+  )
+}
+
+class Configuration extends TaintTrackingConfiguration {
+  override predicate isSink(Element e) {
+    isDiv(e)
+    or
+    missingGuard(e, _)
+  }
 }
 
 /**
@@ -50,19 +64,17 @@ predicate taintedVarAccess(Expr origin, VariableAccess va) {
  * range.
  */
 predicate guardedByAssignDiv(Expr origin) {
-  isUserInput(origin, _) and
-  exists(AssignDivExpr div, VariableAccess va | tainted(origin, va) and div.getLValue() = va)
+  exists(VariableAccess va |
+    taintedWithPath(origin, va, _, _) and
+    isDiv(va)
+  )
 }
 
-from Expr origin, Operation op, VariableAccess va, string effect
+from Expr origin, VariableAccess va, string effect, PathNode sourceNode, PathNode sinkNode
 where
-  taintedVarAccess(origin, va) and
-  op.getAnOperand() = va and
-  (
-    missingGuardAgainstUnderflow(op, va) and effect = "underflow"
-    or
-    missingGuardAgainstOverflow(op, va) and effect = "overflow"
-  ) and
+  taintedWithPath(origin, va, sourceNode, sinkNode) and
+  missingGuard(va, effect) and
   not guardedByAssignDiv(origin)
-select va, "$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".",
-  origin, "Uncontrolled value"
+select va, sourceNode, sinkNode,
+  "$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".", origin,
+  "Uncontrolled value"

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

@@ -2,7 +2,7 @@
  * @name Overflow in uncontrolled allocation size
  * @description Allocating memory with a size controlled by an external
  *              user can result in integer overflow.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id cpp/uncontrolled-allocation-size
@@ -13,21 +13,33 @@
 
 import cpp
 import semmle.code.cpp.security.TaintTracking
+import TaintedWithPath
 
-predicate taintedAllocSize(Expr e, Expr source, string taintCause) {
-  (
-    isAllocationExpr(e) or
-    any(MulExpr me | me.getAChild() instanceof SizeofOperator) = e
-  ) and
+/**
+ * Holds if `alloc` is an allocation, and `tainted` is a child of it that is a
+ * taint sink.
+ */
+predicate allocSink(Expr alloc, Expr tainted) {
+  isAllocationExpr(alloc) and
+  tainted = alloc.getAChild() and
+  tainted.getUnspecifiedType() instanceof IntegralType
+}
+
+class TaintedAllocationSizeConfiguration extends TaintTrackingConfiguration {
+  override predicate isSink(Element tainted) { allocSink(_, tainted) }
+}
+
+predicate taintedAllocSize(
+  Expr source, Expr alloc, PathNode sourceNode, PathNode sinkNode, string taintCause
+) {
+  isUserInput(source, taintCause) and
   exists(Expr tainted |
-    tainted = e.getAChild() and
-    tainted.getUnspecifiedType() instanceof IntegralType and
-    isUserInput(source, taintCause) and
-    tainted(source, tainted)
+    allocSink(alloc, tainted) and
+    taintedWithPath(source, tainted, sourceNode, sinkNode)
   )
 }
 
-from Expr e, Expr source, string taintCause
-where taintedAllocSize(e, source, taintCause)
-select e, "This allocation size is derived from $@ and might overflow", source,
-  "user input (" + taintCause + ")"
+from Expr source, Expr alloc, PathNode sourceNode, PathNode sinkNode, string taintCause
+where taintedAllocSize(source, alloc, sourceNode, sinkNode, taintCause)
+select alloc, sourceNode, sinkNode, "This allocation size is derived from $@ and might overflow",
+  source, "user input (" + taintCause + ")"

cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql

@@ -3,7 +3,7 @@
  * @description Authentication by checking that the peer's address
  *              matches a known IP or web address is unsafe as it is
  *              vulnerable to spoofing attacks.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision medium
  * @id cpp/user-controlled-bypass
@@ -12,6 +12,7 @@
  */
 
 import semmle.code.cpp.security.TaintTracking
+import TaintedWithPath
 
 predicate hardCodedAddressOrIP(StringLiteral txt) {
   exists(string s | s = txt.getValueText() |
@@ -102,16 +103,21 @@ predicate useOfHardCodedAddressOrIP(Expr use) {
  * untrusted input then it might be vulnerable to a spoofing
  * attack.
  */
-predicate hardCodedAddressInCondition(Expr source, Expr condition) {
-  // One of the sub-expressions of the condition is tainted.
-  exists(Expr taintedExpr | taintedExpr.getParent+() = condition | tainted(source, taintedExpr)) and
+predicate hardCodedAddressInCondition(Expr subexpression, Expr condition) {
+  subexpression = condition.getAChild+() and
   // One of the sub-expressions of the condition is a hard-coded
   // IP or web-address.
-  exists(Expr use | use.getParent+() = condition | useOfHardCodedAddressOrIP(use)) and
+  exists(Expr use | use = condition.getAChild+() | useOfHardCodedAddressOrIP(use)) and
   condition = any(IfStmt ifStmt).getCondition()
 }
 
-from Expr source, Expr condition
-where hardCodedAddressInCondition(source, condition)
-select condition, "Untrusted input $@ might be vulnerable to a spoofing attack.", source,
-  source.toString()
+class Configuration extends TaintTrackingConfiguration {
+  override predicate isSink(Element sink) { hardCodedAddressInCondition(sink, _) }
+}
+
+from Expr subexpression, Expr source, Expr condition, PathNode sourceNode, PathNode sinkNode
+where
+  hardCodedAddressInCondition(subexpression, condition) and
+  taintedWithPath(source, subexpression, sourceNode, sinkNode)
+select condition, sourceNode, sinkNode,
+  "Untrusted input $@ might be vulnerable to a spoofing attack.", source, source.toString()

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

@@ -2,7 +2,7 @@
  * @name Cleartext storage of sensitive information in buffer
  * @description Storing sensitive information in cleartext can expose it
  *              to an attacker.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision medium
  * @id cpp/cleartext-storage-buffer
@@ -14,12 +14,20 @@ import cpp
 import semmle.code.cpp.security.BufferWrite
 import semmle.code.cpp.security.TaintTracking
 import semmle.code.cpp.security.SensitiveExprs
+import TaintedWithPath
 
-from BufferWrite w, Expr taintedArg, Expr taintSource, string taintCause, SensitiveExpr dest
+class Configuration extends TaintTrackingConfiguration {
+  override predicate isSink(Element tainted) { exists(BufferWrite w | w.getASource() = tainted) }
+}
+
+from
+  BufferWrite w, Expr taintedArg, Expr taintSource, PathNode sourceNode, PathNode sinkNode,
+  string taintCause, SensitiveExpr dest
 where
-  tainted(taintSource, taintedArg) and
+  taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and
   isUserInput(taintSource, taintCause) and
   w.getASource() = taintedArg and
   dest = w.getDest()
-select w, "This write into buffer '" + dest.toString() + "' may contain unencrypted data from $@",
+select w, sourceNode, sinkNode,
+  "This write into buffer '" + dest.toString() + "' may contain unencrypted data from $@",
   taintSource, "user input (" + taintCause + ")"

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

@@ -2,7 +2,7 @@
  * @name Cleartext storage of sensitive information in an SQLite database
  * @description Storing sensitive information in a non-encrypted
  *              database can expose it to an attacker.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision medium
  * @id cpp/cleartext-storage-database
@@ -13,6 +13,7 @@
 import cpp
 import semmle.code.cpp.security.SensitiveExprs
 import semmle.code.cpp.security.TaintTracking
+import TaintedWithPath
 
 class UserInputIsSensitiveExpr extends SecurityOptions {
   override predicate isUserInput(Expr expr, string cause) {
@@ -32,10 +33,21 @@ predicate sqlite_encryption_used() {
   any(FunctionCall fc).getTarget().getName().matches("sqlite%\\_key\\_%")
 }
 
-from SensitiveExpr taintSource, Expr taintedArg, SqliteFunctionCall sqliteCall
+class Configuration extends TaintTrackingConfiguration {
+  override predicate isSink(Element taintedArg) {
+    exists(SqliteFunctionCall sqliteCall |
+      taintedArg = sqliteCall.getASource() and
+      not sqlite_encryption_used()
+    )
+  }
+}
+
+from
+  SensitiveExpr taintSource, Expr taintedArg, SqliteFunctionCall sqliteCall, PathNode sourceNode,
+  PathNode sinkNode
 where
-  tainted(taintSource, taintedArg) and
-  taintedArg = sqliteCall.getASource() and
-  not sqlite_encryption_used()
-select sqliteCall, "This SQLite call may store $@ in a non-encrypted SQLite database", taintSource,
+  taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and
+  taintedArg = sqliteCall.getASource()
+select sqliteCall, sourceNode, sinkNode,
+  "This SQLite call may store $@ in a non-encrypted SQLite database", taintSource,
   "sensitive information"

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

@@ -198,12 +198,12 @@ class InitializationFunction extends Function {
     )
     or
     // If we have no definition, we look at SAL annotations
-    not this.isDefined() and
+    not this.hasDefinition() and
     this.getParameter(i).(SALParameter).isOut() and
     evidence = SuggestiveSALAnnotation()
     or
     // We have some external information that this function conditionally initializes
-    not this.isDefined() and
+    not this.hasDefinition() and
     any(ValidatedExternalCondInitFunction vc).isExternallyVerified(this, i) and
     evidence = ExternalEvidence()
   }
@@ -406,7 +406,7 @@ class ConditionalInitializationFunction extends InitializationFunction {
        * Explicitly ignore pure virtual functions.
        */
 
-      this.isDefined() and
+      this.hasDefinition() and
       this.paramNotReassignedAt(this, i, c) and
       not this instanceof PureVirtualFunction
     )
@@ -616,11 +616,11 @@ private predicate functionSignature(Function f, string qualifiedName, string typ
  * are never statically linked together.
  */
 private Function getAPossibleDefinition(Function undefinedFunction) {
-  not undefinedFunction.isDefined() and
+  not undefinedFunction.hasDefinition() and
   exists(string qn, string typeSig |
     functionSignature(undefinedFunction, qn, typeSig) and functionSignature(result, qn, typeSig)
   ) and
-  result.isDefined()
+  result.hasDefinition()
 }
 
 /**
@@ -631,7 +631,7 @@ private Function getAPossibleDefinition(Function undefinedFunction) {
  */
 private Function getTarget1(Call c) {
   result = VirtualDispatch::getAViableTarget(c) and
-  result.isDefined()
+  result.hasDefinition()
 }
 
 /**

cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScaling.ql

@@ -21,7 +21,7 @@ where
   destBase = baseType(destType) and
   destBase.getSize() != sourceBase.getSize() and
   not dest.isInMacroExpansion() and
-  // If the source type is a char* or void* then don't
+  // If the source type is a `char*` or `void*` then don't
   // produce a result, because it is likely to be a false
   // positive.
   not sourceBase instanceof CharType and

cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScalingChar.ql

@@ -21,7 +21,7 @@ where
   destBase = baseType(destType) and
   destBase.getSize() != sourceBase.getSize() and
   not dest.isInMacroExpansion() and
-  // If the source type is a char* or void* then don't
+  // If the source type is a `char*` or `void*` then don't
   // produce a result, because it is likely to be a false
   // positive.
   not sourceBase instanceof CharType and

cpp/ql/src/Security/CWE/CWE-468/SuspiciousAddWithSizeof.ql

@@ -24,9 +24,9 @@ private predicate isCharSzPtrExpr(Expr e) {
 from Expr sizeofExpr, Expr e
 where
   // If we see an addWithSizeof then we expect the type of
-  // the pointer expression to be char* or void*. Otherwise it
+  // the pointer expression to be `char*` or `void*`. Otherwise it
   // is probably a mistake.
   addWithSizeof(e, sizeofExpr, _) and not isCharSzPtrExpr(e)
 select sizeofExpr,
-  "Suspicious sizeof offset in a pointer arithmetic expression. " + "The type of the pointer is " +
-    e.getFullyConverted().getType().toString() + "."
+  "Suspicious sizeof offset in a pointer arithmetic expression. The type of the pointer is $@.",
+  e.getFullyConverted().getType() as t, t.toString()

cpp/ql/src/Security/CWE/CWE-807/TaintedCondition.ql

@@ -3,7 +3,7 @@
  * @description Using untrusted inputs in a statement that makes a
  *              security decision makes code vulnerable to
  *              attack.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision medium
  * @id cpp/tainted-permissions-check
@@ -12,14 +12,9 @@
  */
 
 import semmle.code.cpp.security.TaintTracking
+import TaintedWithPath
 
-/**
- * Holds if there is an 'if' statement whose condition `condition`
- * is influenced by tainted data `source`, and the body contains
- * `raise` which escalates privilege.
- */
-predicate cwe807violation(Expr source, Expr condition, Expr raise) {
-  tainted(source, condition) and
+predicate sensitiveCondition(Expr condition, Expr raise) {
   raisesPrivilege(raise) and
   exists(IfStmt ifstmt |
     ifstmt.getCondition() = condition and
@@ -27,7 +22,19 @@ predicate cwe807violation(Expr source, Expr condition, Expr raise) {
   )
 }
 
-from Expr source, Expr condition, Expr raise
-where cwe807violation(source, condition, raise)
-select condition, "Reliance on untrusted input $@ to raise privilege at $@", source,
-  source.toString(), raise, raise.toString()
+class Configuration extends TaintTrackingConfiguration {
+  override predicate isSink(Element tainted) { sensitiveCondition(tainted, _) }
+}
+
+/*
+ * Produce an alert if there is an 'if' statement whose condition `condition`
+ * is influenced by tainted data `source`, and the body contains
+ * `raise` which escalates privilege.
+ */
+
+from Expr source, Expr condition, Expr raise, PathNode sourceNode, PathNode sinkNode
+where
+  taintedWithPath(source, condition, sourceNode, sinkNode) and
+  sensitiveCondition(condition, raise)
+select condition, sourceNode, sinkNode, "Reliance on untrusted input $@ to raise privilege at $@",
+  source, source.toString(), raise, raise.toString()

cpp/ql/src/codeql-suites/cpp-lgtm-full.qls

@@ -12,3 +12,8 @@
       - Critical/FileNeverClosed.ql
       - Critical/MemoryMayNotBeFreed.ql
       - Critical/MemoryNeverFreed.ql
+# These are only for IDE use.
+- exclude:
+    tags contain:
+      - ide-contextual-queries/local-definitions
+      - ide-contextual-queries/local-references

cpp/ql/src/default.qll

@@ -1 +1,7 @@
+/**
+ * DEPRECATED: use `import cpp` instead of `import default`.
+ *
+ * Provides classes and predicates for working with C/C++ code.
+ */
+
 import cpp

cpp/ql/src/definitions.qll

@@ -132,6 +132,7 @@ private predicate constructorCallTypeMention(ConstructorCall cc, TypeMention tm)
  *  - `"X"` for macro accesses
  *  - `"I"` for import / include directives
  */
+cached
 Top definitionOf(Top e, string kind) {
   (
     // call -> function called
@@ -213,3 +214,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/semmle/code/cpp/rangeanalysis/ArrayLengthAnalysis.qll

@@ -0,0 +1,282 @@
+/**
+ * Provides precise tracking of how big the memory pointed to by pointers is.
+ * For each pointer, we start tracking (starting from the allocation or an array declaration)
+ * 1) how long is the chunk of memory allocated
+ * 2) where the current pointer is in this chunk of memory
+ * As computing this information is obviously not possible for all pointers,
+ * we do not guarantee the existence of length/offset information for all pointers.
+ * However, when it exists it is guaranteed to be accurate.
+ *
+ * The  length and offset are tracked in a similar way to the Rangeanalysis.
+ * Each length is a `ValueNumber + delta`, and each Offset is an `Operand + delta`.
+ * We choose to track a `ValueNumber` for length, because the Rangeanalysis offers
+ * integer bounds on instructions and operands in terms of `ValueNumber`s,
+ * and `Operand` for offset because integer bounds on `Operand`s are
+ * tighter than bounds on `Instruction`s.
+ */
+
+import cpp
+import semmle.code.cpp.ir.IR
+private import semmle.code.cpp.ir.ValueNumbering
+private import semmle.code.cpp.ir.internal.CppType
+private import semmle.code.cpp.models.interfaces.Allocation
+private import semmle.code.cpp.rangeanalysis.RangeUtils
+
+private newtype TLength =
+  TZeroLength() or
+  TVNLength(ValueNumber vn) {
+    not vn.getAnInstruction() instanceof ConstantInstruction and
+    exists(Instruction i |
+      vn.getAnInstruction() = i and
+      (
+        i.getResultIRType() instanceof IRSignedIntegerType or
+        i.getResultIRType() instanceof IRUnsignedIntegerType
+      )
+    |
+      i instanceof PhiInstruction
+      or
+      i instanceof InitializeParameterInstruction
+      or
+      i instanceof CallInstruction
+      or
+      i.(LoadInstruction).getSourceAddress() instanceof VariableAddressInstruction
+      or
+      i.(LoadInstruction).getSourceAddress() instanceof FieldAddressInstruction
+      or
+      i.getAUse() instanceof ArgumentOperand
+    )
+  }
+
+/**
+ * Array lengths are represented in a ValueNumber | Zero + delta format.
+ * This class keeps track of the ValueNumber or Zero.
+ * The delta is tracked in the predicate `knownArrayLength`.
+ */
+class Length extends TLength {
+  string toString() { none() } // overridden in subclasses
+}
+
+/**
+ * This length class corresponds to an array having a constant length
+ * that is tracked by the delta value.
+ */
+class ZeroLength extends Length, TZeroLength {
+  override string toString() { result = "ZeroLength" }
+}
+
+/**
+ * This length class corresponds to an array having variable length, i.e. the
+ * length is tracked by a value number. One example is an array having length
+ * `count` for an integer variable `count` in the program.
+ */
+class VNLength extends Length, TVNLength {
+  ValueNumber vn;
+
+  VNLength() { this = TVNLength(vn) }
+
+  /** Gets an instruction with this value number bound. */
+  Instruction getInstruction() { this = TVNLength(valueNumber(result)) }
+
+  ValueNumber getValueNumber() { result = vn }
+
+  override string toString() { result = "VNLength(" + vn.getExampleInstruction().toString() + ")" }
+}
+
+private newtype TOffset =
+  TZeroOffset() or
+  TOpOffset(Operand op) {
+    op.getAnyDef().getResultIRType() instanceof IRSignedIntegerType or
+    op.getAnyDef().getResultIRType() instanceof IRUnsignedIntegerType
+  }
+
+/**
+ * This class describes the offset of a pointer in a chunk of memory.
+ * It is either an `Operand` or zero, an additional integer delta is added later.
+ */
+class Offset extends TOffset {
+  string toString() { none() } // overridden in subclasses
+}
+
+/**
+ * This class represents a fixed offset, only specified by a delta.
+ */
+class ZeroOffset extends Offset, TZeroOffset {
+  override string toString() { result = "ZeroOffset" }
+}
+
+/**
+ * This class represents an offset of an operand.
+ */
+class OpOffset extends Offset, TOpOffset {
+  Operand op;
+
+  OpOffset() { this = TOpOffset(op) }
+
+  Operand getOperand() { result = op }
+
+  override string toString() { result = "OpOffset(" + op.getDef().toString() + ")" }
+}
+
+private int getBaseSizeForPointerType(PointerType type) { result = type.getBaseType().getSize() }
+
+/**
+ * Holds if pointer `prev` that points at offset `prevOffset + prevOffsetDelta`
+ * steps to `array` that points to `offset + offsetDelta` in one step.
+ * This predicate does not contain any recursive steps.
+ */
+bindingset[prevOffset, prevOffsetDelta]
+predicate simpleArrayLengthStep(
+  Instruction array, Offset offset, int offsetDelta, Instruction prev, Offset prevOffset,
+  int prevOffsetDelta
+) {
+  // array assign
+  array.(CopyInstruction).getSourceValue() = prev and
+  offset = prevOffset and
+  offsetDelta = prevOffsetDelta
+  or
+  // pointer add with constant
+  array.(PointerAddInstruction).getLeft() = prev and
+  offset = prevOffset and
+  offsetDelta = prevOffsetDelta + getConstantValue(array.(PointerAddInstruction).getRight())
+  or
+  // pointer add with variable
+  array.(PointerAddInstruction).getLeft() = prev and
+  prevOffset instanceof ZeroOffset and
+  offset.(OpOffset).getOperand() = array.(PointerAddInstruction).getRightOperand() and
+  offsetDelta = prevOffsetDelta and
+  not exists(getConstantValue(array.(PointerAddInstruction).getRight()))
+  or
+  // pointer sub with constant
+  array.(PointerSubInstruction).getLeft() = prev and
+  offset = prevOffset and
+  offsetDelta = prevOffsetDelta - getConstantValue(array.(PointerSubInstruction).getRight())
+  or
+  // array to pointer decay
+  array.(ConvertInstruction).getUnary() = prev and
+  array.getConvertedResultExpression() instanceof ArrayToPointerConversion and
+  offset = prevOffset and
+  offsetDelta = prevOffsetDelta
+  or
+  // cast of pointer to pointer with the same element size
+  exists(PointerType fromTyp, PointerType toTyp |
+    array.(PtrToPtrCastInstruction).getUnary() = prev and
+    prev.getResultLanguageType().hasType(fromTyp, false) and
+    array.getResultLanguageType().hasType(toTyp, false) and
+    offset = prevOffset and
+    offsetDelta = prevOffsetDelta and
+    if fromTyp instanceof VoidPointerType
+    then getBaseSizeForPointerType(toTyp) = 1
+    else (
+      if toTyp instanceof VoidPointerType
+      then getBaseSizeForPointerType(fromTyp) = 1
+      else getBaseSizeForPointerType(toTyp) = getBaseSizeForPointerType(fromTyp)
+    )
+  )
+}
+
+/**
+ * Parses a `sizeExpr` of malloc into a variable part (`lengthExpr`) and an integer offset (`delta`).
+ */
+private predicate deconstructMallocSizeExpr(Expr sizeExpr, Expr lengthExpr, int delta) {
+  sizeExpr instanceof AddExpr and
+  exists(Expr constantExpr |
+    lengthExpr = sizeExpr.(AddExpr).getAnOperand() and
+    constantExpr = sizeExpr.(AddExpr).getAnOperand() and
+    lengthExpr != constantExpr and
+    delta = constantExpr.getValue().toInt()
+  )
+  or
+  sizeExpr instanceof SubExpr and
+  exists(Expr constantExpr |
+    lengthExpr = sizeExpr.(SubExpr).getLeftOperand() and
+    constantExpr = sizeExpr.(SubExpr).getRightOperand() and
+    delta = -constantExpr.getValue().toInt()
+  )
+}
+
+/**
+ * Holds if the instruction `array` is a dynamic memory allocation of `length`+`delta` elements.
+ */
+private predicate allocation(Instruction array, Length length, int delta) {
+  exists(AllocationExpr alloc, Type ptrTyp |
+    array.getUnconvertedResultExpression() = alloc and
+    array.getResultLanguageType().hasType(ptrTyp, false) and
+    // ensure that we have the same type of the allocation and the pointer
+    ptrTyp.stripTopLevelSpecifiers().(PointerType).getBaseType().getUnspecifiedType() =
+      alloc.getAllocatedElementType().getUnspecifiedType() and
+    // ensure that the size multiplier of the allocation is the same as the
+    // size of the type we are allocating
+    alloc.getSizeMult() = getBaseSizeForPointerType(ptrTyp) and
+    (
+      length instanceof ZeroLength and
+      delta = alloc.getSizeExpr().getValue().toInt()
+      or
+      not exists(alloc.getSizeExpr().getValue().toInt()) and
+      (
+        exists(Expr lengthExpr |
+          deconstructMallocSizeExpr(alloc.getSizeExpr(), lengthExpr, delta) and
+          length.(VNLength).getInstruction().getConvertedResultExpression() = lengthExpr
+        )
+        or
+        not exists(int d | deconstructMallocSizeExpr(alloc.getSizeExpr(), _, d)) and
+        length.(VNLength).getInstruction().getConvertedResultExpression() = alloc.getSizeExpr() and
+        delta = 0
+      )
+    )
+  )
+}
+
+/**
+ * Holds if `array` is declared as an array with length `length + lengthDelta`
+ */
+private predicate arrayDeclaration(Instruction array, Length length, int lengthDelta) {
+  (
+    array instanceof VariableAddressInstruction or
+    array instanceof FieldAddressInstruction
+  ) and
+  exists(ArrayType type | array.getResultLanguageType().hasType(type, _) |
+    length instanceof ZeroLength and
+    lengthDelta = type.getArraySize()
+  )
+}
+
+/**
+ * Holds if `array` is declared as an array or allocated
+ * with length `length + lengthDelta`
+ */
+predicate arrayAllocationOrDeclaration(Instruction array, Length length, int lengthDelta) {
+  allocation(array, length, lengthDelta)
+  or
+  // declaration of variable of array type
+  arrayDeclaration(array, length, lengthDelta)
+}
+
+/**
+ * Holds if the instruction `array` represents a pointer to a chunk of memory that holds
+ * `length + lengthDelta` elements, using only local analysis.
+ * `array` points at `offset + offsetDelta` in the chunk of memory.
+ * The pointer is in-bounds if `offset + offsetDelta < length + lengthDelta` and
+ * `offset + offsetDelta >= 0` holds.
+ * The pointer is out-of-bounds if `offset + offsetDelta >= length + lengthDelta`
+ * or `offset + offsetDelta < 0` holds.
+ * All pointers in this predicate are guaranteed to be non-null,
+ * but are not guaranteed to be live.
+ */
+predicate knownArrayLength(
+  Instruction array, Length length, int lengthDelta, Offset offset, int offsetDelta
+) {
+  arrayAllocationOrDeclaration(array, length, lengthDelta) and
+  offset instanceof ZeroOffset and
+  offsetDelta = 0
+  or
+  // simple step (no phi nodes)
+  exists(Instruction prev, Offset prevOffset, int prevOffsetDelta |
+    knownArrayLength(prev, length, lengthDelta, prevOffset, prevOffsetDelta) and
+    simpleArrayLengthStep(array, offset, offsetDelta, prev, prevOffset, prevOffsetDelta)
+  )
+  or
+  // merge control flow after phi node - but only if all the bounds agree
+  forex(Instruction input | array.(PhiInstruction).getAnInput() = input |
+    knownArrayLength(input, length, lengthDelta, offset, offsetDelta)
+  )
+}

cpp/ql/src/experimental/semmle/code/cpp/rangeanalysis/InBoundsPointerDeref.qll

@@ -0,0 +1,105 @@
+/**
+ * This library proves that a subset of pointer dereferences in a program are
+ * safe, i.e. in-bounds.
+ * It does so by first defining what a pointer dereference is (on the IR
+ * `Instruction` level), and then using the array length analysis and the range
+ * analysis together to prove that some of these pointer dereferences are safe.
+ *
+ * The analysis is soundy, i.e. it is sound if no undefined behaviour is present
+ * in the program.
+ * Furthermore, it crucially depends on the soundiness of the range analysis and
+ * the array length analysis.
+ */
+
+import cpp
+private import experimental.semmle.code.cpp.rangeanalysis.ArrayLengthAnalysis
+private import semmle.code.cpp.rangeanalysis.RangeAnalysis
+
+/**
+ * Gets the instruction that computes the address of memory that `i` accesses.
+ * Only holds if `i` dereferences a pointer, not when the computation of the
+ * memory address is constant, or if the address of a local variable is loaded/stored to.
+ */
+private Instruction getMemoryAddressInstruction(Instruction i) {
+  (
+    result = i.(FieldAddressInstruction).getObjectAddress() or
+    result = i.(LoadInstruction).getSourceAddress() or
+    result = i.(StoreInstruction).getDestinationAddress()
+  ) and
+  not result instanceof FieldAddressInstruction and
+  not result instanceof VariableAddressInstruction and
+  not result instanceof ConstantValueInstruction
+}
+
+/**
+ * All instructions that dereference a pointer.
+ */
+class PointerDereferenceInstruction extends Instruction {
+  PointerDereferenceInstruction() { exists(getMemoryAddressInstruction(this)) }
+
+  Instruction getAddress() { result = getMemoryAddressInstruction(this) }
+}
+
+/**
+ * Holds if `ptrDeref` can be proven to always access allocated memory.
+ */
+predicate inBounds(PointerDereferenceInstruction ptrDeref) {
+  exists(Length length, int lengthDelta, Offset offset, int offsetDelta |
+    knownArrayLength(ptrDeref.getAddress(), length, lengthDelta, offset, offsetDelta) and
+    // lower bound - note that we treat a pointer that accesses an array of
+    // length 0 as on upper-bound violation, but not as a lower-bound violation
+    (
+      offset instanceof ZeroOffset and
+      offsetDelta >= 0
+      or
+      offset instanceof OpOffset and
+      exists(int lowerBoundDelta |
+        boundedOperand(offset.(OpOffset).getOperand(), any(ZeroBound b), lowerBoundDelta,
+          /*upper*/ false, _) and
+        lowerBoundDelta + offsetDelta >= 0
+      )
+    ) and
+    // upper bound
+    (
+      // both offset and length are only integers
+      length instanceof ZeroLength and
+      offset instanceof ZeroOffset and
+      offsetDelta < lengthDelta
+      or
+      exists(int lengthBound |
+        // array length is variable+integer, and there's a fixed (integer-only)
+        // lower bound on the variable, so we can guarantee this access is always in-bounds
+        length instanceof VNLength and
+        offset instanceof ZeroOffset and
+        boundedInstruction(length.(VNLength).getInstruction(), any(ZeroBound b), lengthBound,
+          /* upper*/ false, _) and
+        offsetDelta < lengthBound + lengthDelta
+      )
+      or
+      exists(int offsetBoundDelta |
+        length instanceof ZeroLength and
+        offset instanceof OpOffset and
+        boundedOperand(offset.(OpOffset).getOperand(), any(ZeroBound b), offsetBoundDelta,
+          /* upper */ true, _) and
+        // offset <= offsetBoundDelta, so offset + offsetDelta <= offsetDelta + offsetBoundDelta
+        // Thus, in-bounds if offsetDelta + offsetBoundDelta < lengthDelta
+        // as we have length instanceof ZeroLength
+        offsetDelta + offsetBoundDelta < lengthDelta
+      )
+      or
+      exists(ValueNumberBound b, int offsetBoundDelta |
+        length instanceof VNLength and
+        offset instanceof OpOffset and
+        b.getValueNumber() = length.(VNLength).getValueNumber() and
+        // It holds that offset <= length + offsetBoundDelta
+        boundedOperand(offset.(OpOffset).getOperand(), b, offsetBoundDelta, /*upper*/ true, _) and
+        // it also holds that
+        offsetDelta < lengthDelta - offsetBoundDelta
+        // taking both inequalities together we get
+        //    offset <= length + offsetBoundDelta
+        // => offset + offsetDelta <= length + offsetBoundDelta + offsetDelta < length + offsetBoundDelta + lengthDelta - offsetBoundDelta
+        // as required
+      )
+    )
+  )
+}

cpp/ql/src/external/CodeDuplication.qll

@@ -1,3 +1,5 @@
+/** Provides classes for detecting duplicate or similar code. */
+
 import cpp
 
 private string relativePath(File file) { result = file.getRelativePath().replaceAll("\\", "/") }
@@ -8,9 +10,12 @@ private predicate tokenLocation(string path, int sl, int sc, int ec, int el, Cop
   tokens(copy, index, sl, sc, ec, el)
 }
 
+/** A token block used for detection of duplicate and similar code. */
 class Copy extends @duplication_or_similarity {
+  /** Gets the index of the last token in this block. */
   private int lastToken() { result = max(int i | tokens(this, i, _, _, _, _) | i) }
 
+  /** Gets the index of the token in this block starting at the location `loc`, if any. */
   int tokenStartingAt(Location loc) {
     exists(string filepath, int startline, int startcol |
       loc.hasLocationInfo(filepath, startline, startcol, _, _) and
@@ -18,6 +23,7 @@ class Copy extends @duplication_or_similarity {
     )
   }
 
+  /** Gets the index of the token in this block ending at the location `loc`, if any. */
   int tokenEndingAt(Location loc) {
     exists(string filepath, int endline, int endcol |
       loc.hasLocationInfo(filepath, _, _, endline, endcol) and
@@ -25,24 +31,38 @@ class Copy extends @duplication_or_similarity {
     )
   }
 
+  /** Gets the line on which the first token in this block starts. */
   int sourceStartLine() { tokens(this, 0, result, _, _, _) }
 
+  /** Gets the column on which the first token in this block starts. */
   int sourceStartColumn() { tokens(this, 0, _, result, _, _) }
 
+  /** Gets the line on which the last token in this block ends. */
   int sourceEndLine() { tokens(this, lastToken(), _, _, result, _) }
 
+  /** Gets the column on which the last token in this block ends. */
   int sourceEndColumn() { tokens(this, lastToken(), _, _, _, result) }
 
+  /** Gets the number of lines containing at least (part of) one token in this block. */
   int sourceLines() { result = this.sourceEndLine() + 1 - this.sourceStartLine() }
 
+  /** Gets an opaque identifier for the equivalence class of this block. */
   int getEquivalenceClass() { duplicateCode(this, _, result) or similarCode(this, _, result) }
 
+  /** Gets the source file in which this block appears. */
   File sourceFile() {
     exists(string name | duplicateCode(this, name, _) or similarCode(this, name, _) |
       name.replaceAll("\\", "/") = relativePath(result)
     )
   }
 
+  /**
+   * Holds if this element is at the specified location.
+   * The location spans column `startcolumn` of line `startline` to
+   * column `endcolumn` of line `endline` in file `filepath`.
+   * For more information, see
+   * [Locations](https://help.semmle.com/QL/learn-ql/ql/locations.html).
+   */
   predicate hasLocationInfo(
     string filepath, int startline, int startcolumn, int endline, int endcolumn
   ) {
@@ -53,25 +73,30 @@ class Copy extends @duplication_or_similarity {
     endcolumn = sourceEndColumn()
   }
 
+  /** Gets a textual representation of this element. */
   string toString() { none() }
 }
 
+/** A block of duplicated code. */
 class DuplicateBlock extends Copy, @duplication {
   override string toString() { result = "Duplicate code: " + sourceLines() + " duplicated lines." }
 }
 
+/** A block of similar code. */
 class SimilarBlock extends Copy, @similarity {
   override string toString() {
     result = "Similar code: " + sourceLines() + " almost duplicated lines."
   }
 }
 
+/** Gets a function with a body and a location. */
 FunctionDeclarationEntry sourceMethod() {
   result.isDefinition() and
   exists(result.getLocation()) and
   numlines(unresolveElement(result.getFunction()), _, _, _)
 }
 
+/** Gets the number of member functions in `c` with a body and a location. */
 int numberOfSourceMethods(Class c) {
   result =
     count(FunctionDeclarationEntry m |
@@ -108,6 +133,10 @@ private predicate duplicateStatement(
   )
 }
 
+/**
+ * Holds if `m1` is a function with `total` lines, and `m2` is a function
+ * that has `duplicate` lines in common with `m1`.
+ */
 predicate duplicateStatements(
   FunctionDeclarationEntry m1, FunctionDeclarationEntry m2, int duplicate, int total
 ) {
@@ -115,13 +144,16 @@ predicate duplicateStatements(
   total = strictcount(statementInMethod(m1))
 }
 
-/**
- * Find pairs of methods are identical
- */
+/** Holds if `m` and other are identical functions. */
 predicate duplicateMethod(FunctionDeclarationEntry m, FunctionDeclarationEntry other) {
   exists(int total | duplicateStatements(m, other, total, total))
 }
 
+/**
+ * INTERNAL: do not use.
+ *
+ * Holds if `line` in `f` is similar to a line somewhere else.
+ */
 predicate similarLines(File f, int line) {
   exists(SimilarBlock b | b.sourceFile() = f and line in [b.sourceStartLine() .. b.sourceEndLine()])
 }
@@ -152,6 +184,7 @@ private predicate similarLinesCoveredFiles(File f, File otherFile) {
   )
 }
 
+/** Holds if `coveredLines` lines of `f` are similar to lines in `otherFile`. */
 predicate similarLinesCovered(File f, int coveredLines, File otherFile) {
   exists(int numLines | numLines = f.getMetrics().getNumberOfLines() |
     similarLinesCoveredFiles(f, otherFile) and
@@ -166,6 +199,11 @@ predicate similarLinesCovered(File f, int coveredLines, File otherFile) {
   )
 }
 
+/**
+ * INTERNAL: do not use.
+ *
+ * Holds if `line` in `f` is duplicated by a line somewhere else.
+ */
 predicate duplicateLines(File f, int line) {
   exists(DuplicateBlock b |
     b.sourceFile() = f and line in [b.sourceStartLine() .. b.sourceEndLine()]
@@ -182,6 +220,7 @@ private predicate duplicateLinesPerEquivalenceClass(int equivClass, int lines, F
     )
 }
 
+/** Holds if `coveredLines` lines of `f` are duplicates of lines in `otherFile`. */
 predicate duplicateLinesCovered(File f, int coveredLines, File otherFile) {
   exists(int numLines | numLines = f.getMetrics().getNumberOfLines() |
     exists(int coveredApprox |
@@ -206,6 +245,7 @@ predicate duplicateLinesCovered(File f, int coveredLines, File otherFile) {
   )
 }
 
+/** Holds if most of `f` (`percent`%) is similar to `other`. */
 predicate similarFiles(File f, File other, int percent) {
   exists(int covered, int total |
     similarLinesCovered(f, covered, other) and
@@ -216,6 +256,7 @@ predicate similarFiles(File f, File other, int percent) {
   not duplicateFiles(f, other, _)
 }
 
+/** Holds if most of `f` (`percent`%) is duplicated by `other`. */
 predicate duplicateFiles(File f, File other, int percent) {
   exists(int covered, int total |
     duplicateLinesCovered(f, covered, other) and
@@ -225,6 +266,10 @@ predicate duplicateFiles(File f, File other, int percent) {
   )
 }
 
+/**
+ * Holds if most member functions of `c` (`numDup` out of `total`) are
+ * duplicates of member functions in `other`.
+ */
 predicate mostlyDuplicateClassBase(Class c, Class other, int numDup, int total) {
   numDup =
     strictcount(FunctionDeclarationEntry m1 |
@@ -240,6 +285,11 @@ predicate mostlyDuplicateClassBase(Class c, Class other, int numDup, int total)
   (numDup * 100) / total > 80
 }
 
+/**
+ * Holds if most member functions of `c` are duplicates of member functions in
+ * `other`. Provides the human-readable `message` to describe the amount of
+ * duplication.
+ */
 predicate mostlyDuplicateClass(Class c, Class other, string message) {
   exists(int numDup, int total |
     mostlyDuplicateClassBase(c, other, numDup, total) and
@@ -264,12 +314,21 @@ predicate mostlyDuplicateClass(Class c, Class other, string message) {
   )
 }
 
+/** Holds if `f` and `other` are similar or duplicates. */
 predicate fileLevelDuplication(File f, File other) {
   similarFiles(f, other, _) or duplicateFiles(f, other, _)
 }
 
+/**
+ * Holds if most member functions of `c` are duplicates of member functions in
+ * `other`.
+ */
 predicate classLevelDuplication(Class c, Class other) { mostlyDuplicateClass(c, other, _) }
 
+/**
+ * Holds if `line` in `f` should be allowed to be duplicated. This is the case
+ * for `#include` directives.
+ */
 predicate whitelistedLineForDuplication(File f, int line) {
   exists(Include i | i.getFile() = f and i.getLocation().getStartLine() = line)
 }

cpp/ql/src/external/DefectFilter.qll

@@ -1,31 +1,52 @@
+/** Provides a class for working with defect query results stored in dashboard databases. */
+
 import cpp
 
+/**
+ * Holds if `id` in the opaque identifier of a result reported by query `queryPath`,
+ * such that `message` is the associated message and the location of the result spans
+ * column `startcolumn` of line `startline` to column `endcolumn` of line `endline`
+ * in file `filepath`.
+ *
+ * For more information, see [Locations](https://help.semmle.com/QL/learn-ql/ql/locations.html).
+ */
 external predicate defectResults(
   int id, string queryPath, string file, int startline, int startcol, int endline, int endcol,
   string message
 );
 
+/**
+ * A defect query result stored in a dashboard database.
+ */
 class DefectResult extends int {
   DefectResult() { defectResults(this, _, _, _, _, _, _, _) }
 
+  /** Gets the path of the query that reported the result. */
   string getQueryPath() { defectResults(this, result, _, _, _, _, _, _) }
 
+  /** Gets the file in which this query result was reported. */
   File getFile() {
     exists(string path |
       defectResults(this, _, path, _, _, _, _, _) and result.getAbsolutePath() = path
     )
   }
 
+  /** Gets the line on which the location of this query result starts. */
   int getStartLine() { defectResults(this, _, _, result, _, _, _, _) }
 
+  /** Gets the column on which the location of this query result starts. */
   int getStartColumn() { defectResults(this, _, _, _, result, _, _, _) }
 
+  /** Gets the line on which the location of this query result ends. */
   int getEndLine() { defectResults(this, _, _, _, _, result, _, _) }
 
+  /** Gets the column on which the location of this query result ends. */
   int getEndColumn() { defectResults(this, _, _, _, _, _, result, _) }
 
+  /** Gets the message associated with this query result. */
   string getMessage() { defectResults(this, _, _, _, _, _, _, result) }
 
+  /** Gets the URL corresponding to the location of this query result. */
   string getURL() {
     result =
       "file://" + getFile().getAbsolutePath() + ":" + getStartLine() + ":" + getStartColumn() + ":" +

cpp/ql/src/external/ExternalArtifact.qll

@@ -1,26 +1,45 @@
+/**
+ * Provides classes for working with external data.
+ */
+
 import cpp
 
+/**
+ * An external data item.
+ */
 class ExternalData extends @externalDataElement {
+  /** Gets the path of the file this data was loaded from. */
   string getDataPath() { externalData(this, result, _, _) }
 
+  /**
+   * Gets the path of the file this data was loaded from, with its
+   * extension replaced by `.ql`.
+   */
   string getQueryPath() { result = getDataPath().regexpReplaceAll("\\.[^.]*$", ".ql") }
 
+  /** Gets the number of fields in this data item. */
   int getNumFields() { result = 1 + max(int i | externalData(this, _, i, _) | i) }
 
-  string getField(int index) { externalData(this, _, index, result) }
+  /** Gets the value of the `i`th field of this data item. */
+  string getField(int i) { externalData(this, _, i, result) }
 
-  int getFieldAsInt(int index) { result = getField(index).toInt() }
+  /** Gets the integer value of the `i`th field of this data item. */
+  int getFieldAsInt(int i) { result = getField(i).toInt() }
 
-  float getFieldAsFloat(int index) { result = getField(index).toFloat() }
+  /** Gets the floating-point value of the `i`th field of this data item. */
+  float getFieldAsFloat(int i) { result = getField(i).toFloat() }
 
-  date getFieldAsDate(int index) { result = getField(index).toDate() }
+  /** Gets the value of the `i`th field of this data item, interpreted as a date. */
+  date getFieldAsDate(int i) { result = getField(i).toDate() }
 
+  /** Gets a textual representation of this data item. */
   string toString() { result = getQueryPath() + ": " + buildTupleString(0) }
 
-  private string buildTupleString(int start) {
-    start = getNumFields() - 1 and result = getField(start)
+  /** Gets a textual representation of this data item, starting with the `n`th field. */
+  private string buildTupleString(int n) {
+    n = getNumFields() - 1 and result = getField(n)
     or
-    start < getNumFields() - 1 and result = getField(start) + "," + buildTupleString(start + 1)
+    n < getNumFields() - 1 and result = getField(n) + "," + buildTupleString(n + 1)
   }
 }
 
@@ -33,7 +52,9 @@ class DefectExternalData extends ExternalData {
     this.getNumFields() = 2
   }
 
+  /** Gets the URL associated with this data item. */
   string getURL() { result = getField(0) }
 
+  /** Gets the message associated with this data item. */
   string getMessage() { result = getField(1) }
 }

cpp/ql/src/external/MetricFilter.qll

@@ -1,31 +1,58 @@
+/** Provides a class for working with metric query results stored in dashboard databases. */
+
 import cpp
 
+/**
+ * Holds if `id` in the opaque identifier of a result reported by query `queryPath`,
+ * such that `value` is the reported metric value and the location of the result spans
+ * column `startcolumn` of line `startline` to column `endcolumn` of line `endline`
+ * in file `filepath`.
+ *
+ * For more information, see [Locations](https://help.semmle.com/QL/learn-ql/ql/locations.html).
+ */
 external predicate metricResults(
   int id, string queryPath, string file, int startline, int startcol, int endline, int endcol,
   float value
 );
 
+/**
+ * A metric query result stored in a dashboard database.
+ */
 class MetricResult extends int {
   MetricResult() { metricResults(this, _, _, _, _, _, _, _) }
 
+  /** Gets the path of the query that reported the result. */
   string getQueryPath() { metricResults(this, result, _, _, _, _, _, _) }
 
+  /** Gets the file in which this query result was reported. */
   File getFile() {
     exists(string path |
       metricResults(this, _, path, _, _, _, _, _) and result.getAbsolutePath() = path
     )
   }
 
+  /** Gets the line on which the location of this query result starts. */
   int getStartLine() { metricResults(this, _, _, result, _, _, _, _) }
 
+  /** Gets the column on which the location of this query result starts. */
   int getStartColumn() { metricResults(this, _, _, _, result, _, _, _) }
 
+  /** Gets the line on which the location of this query result ends. */
   int getEndLine() { metricResults(this, _, _, _, _, result, _, _) }
 
+  /** Gets the column on which the location of this query result ends. */
   int getEndColumn() { metricResults(this, _, _, _, _, _, result, _) }
 
+  /**
+   * Holds if there is a `Location` entity whose location is the same as
+   * the location of this query result.
+   */
   predicate hasMatchingLocation() { exists(this.getMatchingLocation()) }
 
+  /**
+   * Gets the `Location` entity whose location is the same as the location
+   * of this query result.
+   */
   Location getMatchingLocation() {
     result.getFile() = this.getFile() and
     result.getStartLine() = this.getStartLine() and
@@ -34,8 +61,10 @@ class MetricResult extends int {
     result.getEndColumn() = this.getEndColumn()
   }
 
+  /** Gets the value associated with this query result. */
   float getValue() { metricResults(this, _, _, _, _, _, _, result) }
 
+  /** Gets the URL corresponding to the location of this query result. */
   string getURL() {
     result =
       "file://" + getFile().getAbsolutePath() + ":" + getStartLine() + ":" + getStartColumn() + ":" +

cpp/ql/src/external/VCS.qll

@@ -1,92 +0,0 @@
-import cpp
-
-class Commit extends @svnentry {
-  Commit() {
-    svnaffectedfiles(this, _, _) and
-    exists(date svnDate, date snapshotDate |
-      svnentries(this, _, _, svnDate, _) and
-      snapshotDate(snapshotDate) and
-      svnDate <= snapshotDate
-    )
-  }
-
-  string toString() { result = this.getRevisionName() }
-
-  string getRevisionName() { svnentries(this, result, _, _, _) }
-
-  string getAuthor() { svnentries(this, _, result, _, _) }
-
-  date getDate() { svnentries(this, _, _, result, _) }
-
-  int getChangeSize() { svnentries(this, _, _, _, result) }
-
-  string getMessage() { svnentrymsg(this, result) }
-
-  string getAnAffectedFilePath(string action) {
-    exists(File rawFile | svnaffectedfiles(this, unresolveElement(rawFile), action) |
-      result = rawFile.getAbsolutePath()
-    )
-  }
-
-  string getAnAffectedFilePath() { result = getAnAffectedFilePath(_) }
-
-  File getAnAffectedFile(string action) {
-    // Workaround for incorrect keys in SVN data
-    exists(File svnFile | svnFile.getAbsolutePath() = result.getAbsolutePath() |
-      svnaffectedfiles(this, unresolveElement(svnFile), action)
-    ) and
-    exists(result.getMetrics().getNumberOfLinesOfCode())
-  }
-
-  File getAnAffectedFile() { exists(string action | result = this.getAnAffectedFile(action)) }
-
-  private predicate churnForFile(File f, int added, int deleted) {
-    // Workaround for incorrect keys in SVN data
-    exists(File svnFile | svnFile.getAbsolutePath() = f.getAbsolutePath() |
-      svnchurn(this, unresolveElement(svnFile), added, deleted)
-    ) and
-    exists(f.getMetrics().getNumberOfLinesOfCode())
-  }
-
-  int getRecentChurnForFile(File f) {
-    exists(int added, int deleted | churnForFile(f, added, deleted) and result = added + deleted)
-  }
-
-  int getRecentAdditionsForFile(File f) { churnForFile(f, result, _) }
-
-  int getRecentDeletionsForFile(File f) { churnForFile(f, _, result) }
-
-  predicate isRecent() { recentCommit(this) }
-
-  int daysToNow() {
-    exists(date now | snapshotDate(now) | result = getDate().daysTo(now) and result >= 0)
-  }
-}
-
-class Author extends string {
-  Author() { exists(Commit e | this = e.getAuthor()) }
-
-  Commit getACommit() { result.getAuthor() = this }
-
-  File getAnEditedFile() { result = this.getACommit().getAnAffectedFile() }
-}
-
-predicate recentCommit(Commit e) {
-  exists(date snapshotDate, date commitDate, int days |
-    snapshotDate(snapshotDate) and
-    e.getDate() = commitDate and
-    days = commitDate.daysTo(snapshotDate) and
-    days >= 0 and
-    days <= 60
-  )
-}
-
-date firstChange(File f) {
-  result = min(Commit e, date toMin | f = e.getAnAffectedFile() and toMin = e.getDate() | toMin)
-}
-
-predicate firstCommit(Commit e) {
-  not exists(File f | f = e.getAnAffectedFile() | firstChange(f) < e.getDate())
-}
-
-predicate artificialChange(Commit e) { firstCommit(e) or e.getChangeSize() >= 50000 }

cpp/ql/src/external/tests/DefectFromSVN.ql

@@ -1,20 +0,0 @@
-/**
- * @name Defect from SVN
- * @description A test case for creating a defect from SVN data.
- * @kind problem
- * @problem.severity warning
- * @tags external-data
- * @deprecated
- */
-
-import cpp
-import external.ExternalArtifact
-import external.VCS
-
-predicate numCommits(File f, int i) { i = count(Commit e | e.getAnAffectedFile() = f) }
-
-predicate maxCommits(int i) { i = max(File f, int j | numCommits(f, j) | j) }
-
-from File f, int i
-where numCommits(f, i) and maxCommits(i)
-select f, "This file has " + i + " commits."

cpp/ql/src/external/tests/MetricFromSVN.ql

@@ -1,17 +0,0 @@
-/**
- * @name Metric from SVN
- * @description Find number of commits for a file
- * @treemap.warnOn lowValues
- * @metricType file
- * @tags external-data
- * @deprecated
- */
-
-import cpp
-import external.VCS
-
-predicate numCommits(File f, int i) { i = count(Commit e | e.getAnAffectedFile() = f) }
-
-from File f, int i
-where numCommits(f, i)
-select f, i

cpp/ql/src/filters/RecentDefects.ql

@@ -1,25 +0,0 @@
-/**
- * @name Filter: exclude results from files that have not recently been
- *               edited
- * @description Use this filter to return results only if they are
- *              located in files that have been modified in the 60 days
- *              before the date of the snapshot.
- * @kind problem
- * @id cpp/recent-defects-filter
- * @tags filter
- *       external-data
- * @deprecated
- */
-
-import cpp
-import external.DefectFilter
-import external.VCS
-
-pragma[noopt]
-private predicate recent(File file) {
-  exists(Commit e | file = e.getAnAffectedFile() | e.isRecent() and not artificialChange(e))
-}
-
-from DefectResult res
-where recent(res.getFile())
-select res, res.getMessage()

cpp/ql/src/filters/RecentDefectsForMetric.ql

@@ -1,25 +0,0 @@
-/**
- * @name Metric filter: exclude results from files that have not
- *       recently been edited
- * @description Use this filter to return results only if they are
- *              located in files that have been modified in the 60 days
- *              before the snapshot.
- * @kind treemap
- * @id cpp/recent-defects-for-metric-filter
- * @tags filter
- *       external-data
- * @deprecated
- */
-
-import cpp
-import external.MetricFilter
-import external.VCS
-
-pragma[noopt]
-private predicate recent(File file) {
-  exists(Commit e | file = e.getAnAffectedFile() | e.isRecent() and not artificialChange(e))
-}
-
-from MetricResult res
-where recent(res.getFile())
-select res, res.getValue()

cpp/ql/src/localDefinitions.ql

@@ -0,0 +1,16 @@
+/**
+ * @name Jump-to-definition links
+ * @description Generates use-definition pairs that provide the data
+ *              for jump-to-definition in the code viewer.
+ * @kind definitions
+ * @id cpp/ide-jump-to-definition
+ * @tags ide-contextual-queries/local-definitions
+ */
+
+import definitions
+
+external string selectedSourceFile();
+
+from Top e, Top def, string kind
+where def = definitionOf(e, kind) and e.getFile() = getEncodedFile(selectedSourceFile())
+select e, def, kind

cpp/ql/src/localReferences.ql

@@ -0,0 +1,16 @@
+/**
+ * @name Find-references links
+ * @description Generates use-definition pairs that provide the data
+ *              for find-references in the code viewer.
+ * @kind definitions
+ * @id cpp/ide-find-references
+ * @tags ide-contextual-queries/local-references
+ */
+
+import definitions
+
+external string selectedSourceFile();
+
+from Top e, Top def, string kind
+where def = definitionOf(e, kind) and def.getFile() = getEncodedFile(selectedSourceFile())
+select e, def, kind

cpp/ql/src/objc.qll

@@ -1 +1,7 @@
+/**
+ * DEPRECATED: Objective C is no longer supported.
+ *
+ * Import `cpp` instead of `objc`.
+ */
+
 import cpp

cpp/ql/src/qlpack.yml

@@ -2,3 +2,4 @@ name: codeql-cpp
 version: 0.0.0
 dbscheme: semmlecode.cpp.dbscheme
 suites: codeql-suites
+extractor: cpp

cpp/ql/src/semmle/code/cpp/ASTConsistency.ql

@@ -0,0 +1,9 @@
+/**
+ * @name AST Consistency Check
+ * @description Performs consistency checks on the Abstract Syntax Tree. This query should have no results.
+ * @kind table
+ * @id cpp/ast-consistency-check
+ */
+
+import cpp
+import CastConsistency

cpp/ql/src/semmle/code/cpp/ASTSanity.ql

@@ -1,9 +0,0 @@
-/**
- * @name AST Sanity Check
- * @description Performs sanity checks on the Abstract Syntax Tree. This query should have no results.
- * @kind table
- * @id cpp/ast-sanity-check
- */
-
-import cpp
-import CastSanity

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

@@ -458,6 +458,15 @@ class Class extends UserType {
     exists(ClassDerivation d | d.getDerivedClass() = this and d = result)
   }
 
+  /**
+   * Gets class derivation number `index` of this class/struct, for example the
+   * `public B` is derivation 1 in the following code:
+   * ```
+   * class D : public A, public B, public C {
+   *   ...
+   * };
+   * ```
+   */
   ClassDerivation getDerivation(int index) {
     exists(ClassDerivation d | d.getDerivedClass() = this and d.getIndex() = index and d = result)
   }
@@ -900,6 +909,22 @@ class AbstractClass extends Class {
 class TemplateClass extends Class {
   TemplateClass() { usertypes(underlyingElement(this), _, 6) }
 
+  /**
+   * Gets a class instantiated from this template.
+   *
+   * For example for `MyTemplateClass<T>` in the following code, the results are
+   * `MyTemplateClass<int>` and `MyTemplateClass<long>`:
+   * ```
+   * template<class T>
+   * class MyTemplateClass {
+   *   ...
+   * };
+   *
+   * MyTemplateClass<int> instance;
+   *
+   * MyTemplateClass<long> instance;
+   * ```
+   */
   Class getAnInstantiation() {
     result.isConstructedFrom(this) and
     exists(result.getATemplateArgument())

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

@@ -13,8 +13,20 @@ class Comment extends Locatable, @comment {
 
   override Location getLocation() { comments(underlyingElement(this), _, result) }
 
+  /**
+   * Gets the text of this comment, including the opening `//` or `/*`, and the closing `*``/` if
+   * present.
+   */
   string getContents() { comments(underlyingElement(this), result, _) }
 
+  /**
+   * Gets the AST element this comment is associated with. For example, the comment in the
+   * following code is associated with the declaration of `j`.
+   * ```
+   * int i;
+   * int j; // Comment on j
+   * ```
+   */
   Element getCommentedElement() {
     commentbinding(underlyingElement(this), unresolveElement(result))
   }

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

@@ -21,9 +21,9 @@ private predicate idOf(@compilation x, int y) = equivalenceRelation(id/2)(x, y)
  * Three things happen to each file during a compilation:
  *
  *   1. The file is compiled by a real compiler, such as gcc or VC.
- *   2. The file is parsed by Semmle's C++ front-end.
+ *   2. The file is parsed by the CodeQL C++ front-end.
  *   3. The parsed representation is converted to database tables by
- *      Semmle's extractor.
+ *      the CodeQL extractor.
  *
  * This class provides CPU and elapsed time information for steps 2 and 3,
  * but not for step 1.
@@ -40,6 +40,7 @@ class Compilation extends @compilation {
   /** Gets a file compiled during this invocation. */
   File getAFileCompiled() { result = getFileCompiled(_) }
 
+  /** Gets the `i`th file compiled during this invocation */
   File getFileCompiled(int i) { compilation_compiling_files(this, i, unresolveElement(result)) }
 
   /**

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

@@ -25,7 +25,7 @@ private import semmle.code.cpp.internal.QualifiedName as Q
  * `DeclarationEntry`, because they always have a unique source location.
  * `EnumConstant` and `FriendDecl` are both examples of this.
  */
-abstract class Declaration extends Locatable, @declaration {
+class Declaration extends Locatable, @declaration {
   /**
    * Gets the innermost namespace which contains this declaration.
    *
@@ -98,7 +98,12 @@ abstract class Declaration extends Locatable, @declaration {
     this.hasQualifiedName(namespaceQualifier, "", baseName)
   }
 
-  override string toString() { result = this.getName() }
+  /**
+   * Gets a description of this `Declaration` for display purposes.
+   */
+  string getDescription() { result = this.getName() }
+
+  final override string toString() { result = this.getDescription() }
 
   /**
    * Gets the name of this declaration.
@@ -161,6 +166,7 @@ abstract class Declaration extends Locatable, @declaration {
   /** Holds if the declaration has a definition. */
   predicate hasDefinition() { exists(this.getDefinition()) }
 
+  /** DEPRECATED: Use `hasDefinition` instead. */
   predicate isDefined() { hasDefinition() }
 
   /** Gets the preferred location of this declaration, if any. */
@@ -303,7 +309,7 @@ abstract class DeclarationEntry extends Locatable {
    * available), or the name declared by this entry otherwise.
    */
   string getCanonicalName() {
-    if getDeclaration().isDefined()
+    if getDeclaration().hasDefinition()
     then result = getDeclaration().getDefinition().getName()
     else result = getName()
   }

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

@@ -11,6 +11,7 @@ class Diagnostic extends Locatable, @diagnostic {
   /** Gets the error code for this compiler message. */
   string getTag() { diagnostics(underlyingElement(this), _, result, _, _, _) }
 
+  /** Holds if `s` is the error code for this compiler message. */
   predicate hasTag(string s) { this.getTag() = s }
 
   /**

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

@@ -3,7 +3,7 @@ import semmle.code.cpp.Declaration
 import semmle.code.cpp.metrics.MetricFile
 
 /** A file or folder. */
-abstract class Container extends Locatable, @container {
+class Container extends Locatable, @container {
   /**
    * Gets the absolute, canonical path of this container, using forward slashes
    * as path separator.
@@ -28,7 +28,7 @@ abstract class Container extends Locatable, @container {
    * a bare root prefix, that is, the path has no path segments. A container
    * whose absolute path has no segments is always a `Folder`, not a `File`.
    */
-  abstract string getAbsolutePath();
+  string getAbsolutePath() { none() } // overridden by subclasses
 
   /**
    * DEPRECATED: Use `getLocation` instead.
@@ -36,7 +36,7 @@ abstract class Container extends Locatable, @container {
    *
    * For more information see [Providing URLs](https://help.semmle.com/QL/learn-ql/ql/locations.html#providing-urls).
    */
-  abstract deprecated string getURL();
+  deprecated string getURL() { none() } // overridden by subclasses
 
   /**
    * Gets the relative path of this file or folder from the root folder of the
@@ -261,18 +261,6 @@ class File extends Container, @file {
   /** Holds if this file was compiled as C++ (at any point). */
   predicate compiledAsCpp() { fileannotations(underlyingElement(this), 1, "compiled as c++", "1") }
 
-  /**
-   * DEPRECATED: Objective-C is no longer supported.
-   * Holds if this file was compiled as Objective C (at any point).
-   */
-  deprecated predicate compiledAsObjC() { none() }
-
-  /**
-   * DEPRECATED: Objective-C is no longer supported.
-   * Holds if this file was compiled as Objective C++ (at any point).
-   */
-  deprecated predicate compiledAsObjCpp() { none() }
-
   /**
    * Holds if this file was compiled by a Microsoft compiler (at any point).
    *
@@ -316,14 +304,6 @@ class File extends Container, @file {
     exists(Include i | i.getFile() = this and i.getIncludedFile() = result)
   }
 
-  /**
-   * DEPRECATED: use `getParentContainer` instead.
-   * Gets the folder which contains this file.
-   */
-  deprecated Folder getParent() {
-    containerparent(unresolveElement(result), underlyingElement(this))
-  }
-
   /**
    * Holds if this file may be from source. This predicate holds for all files
    * except the dummy file, whose name is the empty string, which contains
@@ -341,28 +321,6 @@ class File extends Container, @file {
   /** Gets the metric file. */
   MetricFile getMetrics() { result = this }
 
-  /**
-   * DEPRECATED: Use `getAbsolutePath` instead.
-   * Gets the full name of this file, for example:
-   * "/usr/home/me/myprogram.c".
-   */
-  deprecated string getName() { files(underlyingElement(this), result, _, _, _) }
-
-  /**
-   * DEPRECATED: Use `getAbsolutePath` instead.
-   * Holds if this file has the specified full name.
-   *
-   * Example usage: `f.hasName("/usr/home/me/myprogram.c")`.
-   */
-  deprecated predicate hasName(string name) { name = this.getName() }
-
-  /**
-   * DEPRECATED: Use `getAbsolutePath` instead.
-   * Gets the full name of this file, for example
-   * "/usr/home/me/myprogram.c".
-   */
-  deprecated string getFullName() { result = this.getName() }
-
   /**
    * Gets the remainder of the base name after the first dot character. Note
    * that the name of this predicate is in plural form, unlike `getExtension`,
@@ -377,22 +335,6 @@ class File extends Container, @file {
    */
   string getExtensions() { files(underlyingElement(this), _, _, result, _) }
 
-  /**
-   * DEPRECATED: Use `getBaseName` instead.
-   * Gets the name and extension(s), but not path, of a file. For example,
-   * if the full name is "/path/to/filename.a.bcd" then the filename is
-   * "filename.a.bcd".
-   */
-  deprecated string getFileName() {
-    // [a/b.c/d/]fileName
-    //         ^ beginAfter
-    exists(string fullName, int beginAfter |
-      fullName = this.getName() and
-      beginAfter = max(int i | i = -1 or fullName.charAt(i) = "/" | i) and
-      result = fullName.suffix(beginAfter + 1)
-    )
-  }
-
   /**
    * Gets the short name of this file, that is, the prefix of its base name up
    * to (but not including) the first dot character if there is one, or the

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

@@ -103,6 +103,9 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
 
   /**
    * Holds if this function is declared to be `constexpr`.
+   *
+   * Note that this does not hold if the function has been declared
+   * `consteval`.
    */
   predicate isDeclaredConstexpr() { this.hasSpecifier("declared_constexpr") }
 
@@ -115,9 +118,16 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
    * template <typename T> constexpr int g(T x) { return f(x); }
    * ```
    * `g<int>` is declared constexpr, but is not constexpr.
+   *
+   * Will also hold if this function is `consteval`.
    */
   predicate isConstexpr() { this.hasSpecifier("is_constexpr") }
 
+  /**
+   * Holds if this function is declared to be `consteval`.
+   */
+  predicate isConsteval() { this.hasSpecifier("is_consteval") }
+
   /**
    * Holds if this function is declared with `__attribute__((naked))` or
    * `__declspec(naked)`.
@@ -174,17 +184,8 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
    * For example: for a function `int Foo(int p1, int p2)` this would
    * return `int p1, int p2`.
    */
-  string getParameterString() { result = getParameterStringFrom(0) }
-
-  private string getParameterStringFrom(int index) {
-    index = getNumberOfParameters() and
-    result = ""
-    or
-    index = getNumberOfParameters() - 1 and
-    result = getParameter(index).getTypedName()
-    or
-    index < getNumberOfParameters() - 1 and
-    result = getParameter(index).getTypedName() + ", " + getParameterStringFrom(index + 1)
+  string getParameterString() {
+    result = concat(int i | | min(getParameter(i).getTypedName()), ", " order by i)
   }
 
   /** Gets a call to this function. */
@@ -606,18 +607,8 @@ class FunctionDeclarationEntry extends DeclarationEntry, @fun_decl {
    * For example: for a function 'int Foo(int p1, int p2)' this would
    * return 'int p1, int p2'.
    */
-  string getParameterString() { result = getParameterStringFrom(0) }
-
-  private string getParameterStringFrom(int index) {
-    index = getNumberOfParameters() and
-    result = ""
-    or
-    index = getNumberOfParameters() - 1 and
-    result = getParameterDeclarationEntry(index).getTypedName()
-    or
-    index < getNumberOfParameters() - 1 and
-    result =
-      getParameterDeclarationEntry(index).getTypedName() + ", " + getParameterStringFrom(index + 1)
+  string getParameterString() {
+    result = concat(int i | | min(getParameterDeclarationEntry(i).getTypedName()), ", " order by i)
   }
 
   /**

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

@@ -79,7 +79,10 @@ class Namespace extends NameQualifyingElement, @namespace {
   /** Gets the metric namespace. */
   MetricNamespace getMetrics() { result = this }
 
-  override string toString() { result = this.getQualifiedName() }
+  /** Gets a version of the `QualifiedName` that is more suitable for display purposes. */
+  string getFriendlyName() { result = this.getQualifiedName() }
+
+  final override string toString() { result = getFriendlyName() }
 
   /** Gets a declaration of (part of) this namespace. */
   NamespaceDeclarationEntry getADeclarationEntry() { result.getNamespace() = this }
@@ -104,7 +107,7 @@ class NamespaceDeclarationEntry extends Locatable, @namespace_decl {
     namespace_decls(underlyingElement(this), unresolveElement(result), _, _)
   }
 
-  override string toString() { result = this.getNamespace().toString() }
+  override string toString() { result = this.getNamespace().getFriendlyName() }
 
   /**
    * Gets the location of the token preceding the namespace declaration
@@ -130,7 +133,7 @@ class NamespaceDeclarationEntry extends Locatable, @namespace_decl {
 /**
  * A C++ `using` directive or `using` declaration.
  */
-abstract class UsingEntry extends Locatable, @using {
+class UsingEntry extends Locatable, @using {
   override Location getLocation() { usings(underlyingElement(this), _, result) }
 }
 
@@ -150,7 +153,7 @@ class UsingDeclarationEntry extends UsingEntry {
    */
   Declaration getDeclaration() { usings(underlyingElement(this), unresolveElement(result), _) }
 
-  override string toString() { result = "using " + this.getDeclaration().toString() }
+  override string toString() { result = "using " + this.getDeclaration().getDescription() }
 }
 
 /**
@@ -169,7 +172,7 @@ class UsingDirectiveEntry extends UsingEntry {
    */
   Namespace getNamespace() { usings(underlyingElement(this), unresolveElement(result), _) }
 
-  override string toString() { result = "using namespace " + this.getNamespace().toString() }
+  override string toString() { result = "using namespace " + this.getNamespace().getFriendlyName() }
 }
 
 /**
@@ -204,7 +207,7 @@ class GlobalNamespace extends Namespace {
    */
   deprecated string getFullName() { result = this.getName() }
 
-  override string toString() { result = "(global namespace)" }
+  override string getFriendlyName() { result = "(global namespace)" }
 }
 
 /**

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

@@ -376,6 +376,8 @@ private predicate isIntegralType(@builtintype type, int kind) {
     kind = 43
     or
     kind = 44
+    or
+    kind = 51
   )
 }
 
@@ -463,6 +465,8 @@ private predicate integralTypeMapping(int original, int canonical, int unsigned,
   original = 43 and canonical = 43 and unsigned = -1 and signed = -1 // char16_t
   or
   original = 44 and canonical = 44 and unsigned = -1 and signed = -1 // char32_t
+  or
+  original = 51 and canonical = 51 and unsigned = -1 and signed = -1 // char8_t
 }
 
 /**
@@ -697,28 +701,188 @@ class Int128Type extends IntegralType {
   override string getCanonicalQLClass() { result = "Int128Type" }
 }
 
+private newtype TTypeDomain =
+  TRealDomain() or
+  TComplexDomain() or
+  TImaginaryDomain()
+
 /**
- * The C/C++ floating point types. See 4.5.  This includes `float`,
- * `double` and `long double` types.
- * ```
- * float f;
- * double d;
- * long double ld;
- * ```
+ * The type domain of a floating-point type. One of `RealDomain`, `ComplexDomain`, or
+ * `ImaginaryDomain`.
+ */
+class TypeDomain extends TTypeDomain {
+  /** Gets a textual representation of this type domain. */
+  string toString() { none() }
+}
+
+/**
+ * The type domain of a floating-point type that represents a real number.
+ */
+class RealDomain extends TypeDomain, TRealDomain {
+  final override string toString() { result = "real" }
+}
+
+/**
+ * The type domain of a floating-point type that represents a complex number.
+ */
+class ComplexDomain extends TypeDomain, TComplexDomain {
+  final override string toString() { result = "complex" }
+}
+
+/**
+ * The type domain of a floating-point type that represents an imaginary number.
+ */
+class ImaginaryDomain extends TypeDomain, TImaginaryDomain {
+  final override string toString() { result = "imaginary" }
+}
+
+/**
+ * Data for floating-point types.
+ *
+ * kind: The original type kind. Can be any floating-point type kind.
+ * base: The numeric base of the number's representation. Can be 2 (binary) or 10 (decimal).
+ * domain: The type domain of the type. Can be `RealDomain`, `ComplexDomain`, or `ImaginaryDomain`.
+ * realKind: The type kind of the corresponding real type. For example, the corresponding real type
+ *   of `_Complex double` is `double`.
+ * extended: `true` if the number is an extended-precision floating-point number, such as
+ *   `_Float32x`.
+ */
+private predicate floatingPointTypeMapping(
+  int kind, int base, TTypeDomain domain, int realKind, boolean extended
+) {
+  // float
+  kind = 24 and base = 2 and domain = TRealDomain() and realKind = 24 and extended = false
+  or
+  // double
+  kind = 25 and base = 2 and domain = TRealDomain() and realKind = 25 and extended = false
+  or
+  // long double
+  kind = 26 and base = 2 and domain = TRealDomain() and realKind = 26 and extended = false
+  or
+  // _Complex float
+  kind = 27 and base = 2 and domain = TComplexDomain() and realKind = 24 and extended = false
+  or
+  // _Complex double
+  kind = 28 and base = 2 and domain = TComplexDomain() and realKind = 25 and extended = false
+  or
+  // _Complex long double
+  kind = 29 and base = 2 and domain = TComplexDomain() and realKind = 26 and extended = false
+  or
+  // _Imaginary float
+  kind = 30 and base = 2 and domain = TImaginaryDomain() and realKind = 24 and extended = false
+  or
+  // _Imaginary double
+  kind = 31 and base = 2 and domain = TImaginaryDomain() and realKind = 25 and extended = false
+  or
+  // _Imaginary long double
+  kind = 32 and base = 2 and domain = TImaginaryDomain() and realKind = 26 and extended = false
+  or
+  // __float128
+  kind = 38 and base = 2 and domain = TRealDomain() and realKind = 38 and extended = false
+  or
+  // _Complex __float128
+  kind = 39 and base = 2 and domain = TComplexDomain() and realKind = 38 and extended = false
+  or
+  // _Decimal32
+  kind = 40 and base = 10 and domain = TRealDomain() and realKind = 40 and extended = false
+  or
+  // _Decimal64
+  kind = 41 and base = 10 and domain = TRealDomain() and realKind = 41 and extended = false
+  or
+  // _Decimal128
+  kind = 42 and base = 10 and domain = TRealDomain() and realKind = 42 and extended = false
+  or
+  // _Float32
+  kind = 45 and base = 2 and domain = TRealDomain() and realKind = 45 and extended = false
+  or
+  // _Float32x
+  kind = 46 and base = 2 and domain = TRealDomain() and realKind = 46 and extended = true
+  or
+  // _Float64
+  kind = 47 and base = 2 and domain = TRealDomain() and realKind = 47 and extended = false
+  or
+  // _Float64x
+  kind = 48 and base = 2 and domain = TRealDomain() and realKind = 48 and extended = true
+  or
+  // _Float128
+  kind = 49 and base = 2 and domain = TRealDomain() and realKind = 49 and extended = false
+  or
+  // _Float128x
+  kind = 50 and base = 2 and domain = TRealDomain() and realKind = 50 and extended = true
+}
+
+/**
+ * The C/C++ floating point types. See 4.5.  This includes `float`, `double` and `long double`, the
+ * fixed-size floating-point types like `_Float32`, the extended-precision floating-point types like
+ * `_Float64x`, and the decimal floating-point types like `_Decimal32`. It also includes the complex
+ * and imaginary versions of all of these types.
  */
 class FloatingPointType extends ArithmeticType {
+  final int base;
+  final TypeDomain domain;
+  final int realKind;
+  final boolean extended;
+
   FloatingPointType() {
     exists(int kind |
       builtintypes(underlyingElement(this), _, kind, _, _, _) and
-      (
-        kind >= 24 and kind <= 32
-        or
-        kind >= 38 and kind <= 42
-        or
-        kind >= 45 and kind <= 50
-      )
+      floatingPointTypeMapping(kind, base, domain, realKind, extended)
     )
   }
+
+  /** Gets the numeric base of this type's representation: 2 (binary) or 10 (decimal). */
+  final int getBase() { result = base }
+
+  /**
+   * Gets the type domain of this type. Can be `RealDomain`, `ComplexDomain`, or `ImaginaryDomain`.
+   */
+  final TypeDomain getDomain() { result = domain }
+
+  /**
+   * Gets the corresponding real type of this type. For example, the corresponding real type of
+   * `_Complex double` is `double`.
+   */
+  final RealNumberType getRealType() {
+    builtintypes(unresolveElement(result), _, realKind, _, _, _)
+  }
+
+  /** Holds if this type is an extended precision floating-point type, such as `_Float32x`. */
+  final predicate isExtendedPrecision() { extended = true }
+}
+
+/**
+ * A floating-point type representing a real number.
+ */
+class RealNumberType extends FloatingPointType {
+  RealNumberType() { domain instanceof RealDomain }
+}
+
+/**
+ * A floating-point type representing a complex number.
+ */
+class ComplexNumberType extends FloatingPointType {
+  ComplexNumberType() { domain instanceof ComplexDomain }
+}
+
+/**
+ * A floating-point type representing an imaginary number.
+ */
+class ImaginaryNumberType extends FloatingPointType {
+  ImaginaryNumberType() { domain instanceof ImaginaryDomain }
+}
+
+/**
+ * A floating-point type whose representation is base 2.
+ */
+class BinaryFloatingPointType extends FloatingPointType {
+  BinaryFloatingPointType() { base = 2 }
+}
+
+/**
+ * A floating-point type whose representation is base 10.
+ */
+class DecimalFloatingPointType extends FloatingPointType {
+  DecimalFloatingPointType() { base = 10 }
 }
 
 /**
@@ -727,7 +891,7 @@ class FloatingPointType extends ArithmeticType {
  * float f;
  * ```
  */
-class FloatType extends FloatingPointType {
+class FloatType extends RealNumberType, BinaryFloatingPointType {
   FloatType() { builtintypes(underlyingElement(this), _, 24, _, _, _) }
 
   override string getCanonicalQLClass() { result = "FloatType" }
@@ -739,7 +903,7 @@ class FloatType extends FloatingPointType {
  * double d;
  * ```
  */
-class DoubleType extends FloatingPointType {
+class DoubleType extends RealNumberType, BinaryFloatingPointType {
   DoubleType() { builtintypes(underlyingElement(this), _, 25, _, _, _) }
 
   override string getCanonicalQLClass() { result = "DoubleType" }
@@ -751,7 +915,7 @@ class DoubleType extends FloatingPointType {
  * long double ld;
  * ```
  */
-class LongDoubleType extends FloatingPointType {
+class LongDoubleType extends RealNumberType, BinaryFloatingPointType {
   LongDoubleType() { builtintypes(underlyingElement(this), _, 26, _, _, _) }
 
   override string getCanonicalQLClass() { result = "LongDoubleType" }
@@ -763,7 +927,7 @@ class LongDoubleType extends FloatingPointType {
  * __float128 f128;
  * ```
  */
-class Float128Type extends FloatingPointType {
+class Float128Type extends RealNumberType, BinaryFloatingPointType {
   Float128Type() { builtintypes(underlyingElement(this), _, 38, _, _, _) }
 
   override string getCanonicalQLClass() { result = "Float128Type" }
@@ -775,7 +939,7 @@ class Float128Type extends FloatingPointType {
  * _Decimal32 d32;
  * ```
  */
-class Decimal32Type extends FloatingPointType {
+class Decimal32Type extends RealNumberType, DecimalFloatingPointType {
   Decimal32Type() { builtintypes(underlyingElement(this), _, 40, _, _, _) }
 
   override string getCanonicalQLClass() { result = "Decimal32Type" }
@@ -787,7 +951,7 @@ class Decimal32Type extends FloatingPointType {
  * _Decimal64 d64;
  * ```
  */
-class Decimal64Type extends FloatingPointType {
+class Decimal64Type extends RealNumberType, DecimalFloatingPointType {
   Decimal64Type() { builtintypes(underlyingElement(this), _, 41, _, _, _) }
 
   override string getCanonicalQLClass() { result = "Decimal64Type" }
@@ -799,7 +963,7 @@ class Decimal64Type extends FloatingPointType {
  * _Decimal128 d128;
  * ```
  */
-class Decimal128Type extends FloatingPointType {
+class Decimal128Type extends RealNumberType, DecimalFloatingPointType {
   Decimal128Type() { builtintypes(underlyingElement(this), _, 42, _, _, _) }
 
   override string getCanonicalQLClass() { result = "Decimal128Type" }
@@ -833,6 +997,18 @@ class WideCharType extends IntegralType {
   override string getCanonicalQLClass() { result = "WideCharType" }
 }
 
+/**
+ * The C/C++ `char8_t` type.  This is available starting with C++20.
+ * ```
+ * char8_t c8;
+ * ```
+ */
+class Char8Type extends IntegralType {
+  Char8Type() { builtintypes(underlyingElement(this), _, 51, _, _, _) }
+
+  override string getCanonicalQLClass() { result = "Char8Type" }
+}
+
 /**
  * The C/C++ `char16_t` type.  This is available starting with C11 and C++11.
  * ```

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

@@ -38,7 +38,7 @@ class UserType extends Type, Declaration, NameQualifyingElement, AccessHolder, @
   override Specifier getASpecifier() { result = Type.super.getASpecifier() }
 
   override Location getLocation() {
-    if isDefined()
+    if hasDefinition()
     then result = this.getDefinitionLocation()
     else result = this.getADeclarationLocation()
   }

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

@@ -126,10 +126,7 @@ class Variable extends Declaration, @variable {
     or
     exists(AssignExpr ae | ae.getLValue().(Access).getTarget() = this and result = ae.getRValue())
     or
-    exists(AggregateLiteral l |
-      this.getDeclaringType() = l.getType() and
-      result = l.getChild(this.(Field).getInitializationOrder())
-    )
+    exists(ClassAggregateLiteral l | result = l.getFieldExpr(this))
   }
 
   /**
@@ -263,24 +260,33 @@ class ParameterDeclarationEntry extends VariableDeclarationEntry {
    */
   int getIndex() { param_decl_bind(underlyingElement(this), result, _) }
 
+  private string getAnonymousParameterDescription() {
+    not exists(getName()) and
+    exists(string idx |
+      idx =
+        ((getIndex() + 1).toString() + "th")
+            .replaceAll("1th", "1st")
+            .replaceAll("2th", "2nd")
+            .replaceAll("3th", "3rd")
+            .replaceAll("11st", "11th")
+            .replaceAll("12nd", "12th")
+            .replaceAll("13rd", "13th") and
+      if exists(getCanonicalName())
+      then result = "declaration of " + getCanonicalName() + " as anonymous " + idx + " parameter"
+      else result = "declaration of " + idx + " parameter"
+    )
+  }
+
   override string toString() {
-    if exists(getName())
-    then result = super.toString()
-    else
-      exists(string idx |
-        idx =
-          ((getIndex() + 1).toString() + "th")
-              .replaceAll("1th", "1st")
-              .replaceAll("2th", "2nd")
-              .replaceAll("3th", "3rd")
-              .replaceAll("11st", "11th")
-              .replaceAll("12nd", "12th")
-              .replaceAll("13rd", "13th")
-      |
-        if exists(getCanonicalName())
-        then result = "declaration of " + getCanonicalName() + " as anonymous " + idx + " parameter"
-        else result = "declaration of " + idx + " parameter"
-      )
+    isDefinition() and
+    result = "definition of " + getName()
+    or
+    not isDefinition() and
+    if getName() = getCanonicalName()
+    then result = "declaration of " + getName()
+    else result = "declaration of " + getCanonicalName() + " as " + getName()
+    or
+    result = getAnonymousParameterDescription()
   }
 
   /**

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

@@ -116,7 +116,7 @@ class XMLFile extends XMLParent, File {
   XMLFile() { xmlEncoding(this, _) }
 
   /** Gets a printable representation of this XML file. */
-  override string toString() { result = XMLParent.super.toString() }
+  override string toString() { result = getName() }
 
   /** Gets the name of this XML file. */
   override string getName() { result = File.super.getAbsolutePath() }
@@ -236,7 +236,7 @@ class XMLElement extends @xmlelement, XMLParent, XMLLocatable {
   string getAttributeValue(string name) { result = this.getAttribute(name).getValue() }
 
   /** Gets a printable representation of this XML element. */
-  override string toString() { result = XMLParent.super.toString() }
+  override string toString() { result = getName() }
 }
 
 /**

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

@@ -23,6 +23,8 @@ predicate freeFunction(Function f, int argNum) { argNum = f.(DeallocationFunctio
 
 /**
  * A call to a library routine that frees memory.
+ *
+ * DEPRECATED: Use `DeallocationExpr` instead (this also includes `delete` expressions).
  */
 predicate freeCall(FunctionCall fc, Expr arg) { arg = fc.(DeallocationExpr).getFreedExpr() }
 

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

@@ -12,12 +12,12 @@ abstract class Assertion extends Locatable {
 }
 
 /**
- * A libc assert, as defined in assert.h. A macro with the head
- * "assert(expr)" that expands to a conditional expression which
- * may terminate the program.
+ * A libc assert, as defined in assert.h. A macro with a head
+ * that matches the prefix "assert(", and expands to a conditional
+ * expression which may terminate the program.
  */
 class LibcAssert extends MacroInvocation, Assertion {
-  LibcAssert() { this.getMacro().getHead() = "assert(expr)" }
+  LibcAssert() { this.getMacro().getHead().matches("assert(%") }
 
   override Expr getAsserted() {
     exists(ConditionalExpr ce | this.getAGeneratedElement() = ce | result = ce.getCondition())

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

@@ -92,13 +92,7 @@ int getBufferSize(Expr bufferExpr, Element why) {
     // dataflow (all sources must be the same size)
     bufferExprNode = DataFlow::exprNode(bufferExpr) and
     result =
-      min(Expr def |
-        DataFlow::localFlowStep(DataFlow::exprNode(def), bufferExprNode)
-      |
-        getBufferSize(def, _)
-      ) and
-    result =
-      max(Expr def |
+      unique(Expr def |
         DataFlow::localFlowStep(DataFlow::exprNode(def), bufferExprNode)
       |
         getBufferSize(def, _)

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

@@ -532,13 +532,7 @@ library class ExprEvaluator extends int {
       interestingVariableAccess(e, va, v, true) and
       // All assignments must have the same int value
       result =
-        min(Expr value |
-          value = v.getAnAssignedValue() and not ignoreVariableAssignment(e, v, value)
-        |
-          getValueInternalNonSubExpr(value)
-        ) and
-      result =
-        max(Expr value |
+        unique(Expr value |
           value = v.getAnAssignedValue() and not ignoreVariableAssignment(e, v, value)
         |
           getValueInternalNonSubExpr(value)

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

@@ -4,6 +4,14 @@
  * passed to a function, or similar.
  */
 
+/*
+ * Maintainer note: this file is one of several files that are similar but not
+ * identical. Many changes to this file will also apply to the others:
+ * - AddressConstantExpression.qll
+ * - AddressFlow.qll
+ * - EscapesTree.qll
+ */
+
 private import cpp
 
 /**
@@ -11,15 +19,13 @@ private import cpp
  * template functions, these functions are essentially casts, so we treat them
  * as such.
  */
-private predicate stdIdentityFunction(Function f) {
-  f.getNamespace().getParentNamespace() instanceof GlobalNamespace and
-  f.getNamespace().getName() = "std" and
-  (
-    f.getName() = "move"
-    or
-    f.getName() = "forward"
-  )
-}
+private predicate stdIdentityFunction(Function f) { f.hasQualifiedName("std", ["move", "forward"]) }
+
+/**
+ * Holds if `f` is an instantiation of `std::addressof`, which effectively
+ * converts a reference to a pointer.
+ */
+private predicate stdAddressOf(Function f) { f.hasQualifiedName("std", "addressof") }
 
 private predicate lvalueToLvalueStepPure(Expr lvalueIn, Expr lvalueOut) {
   lvalueIn = lvalueOut.(DotFieldAccess).getQualifier().getFullyConverted()
@@ -91,12 +97,17 @@ private predicate lvalueToReferenceStep(Expr lvalueIn, Expr referenceOut) {
 }
 
 private predicate referenceToLvalueStep(Expr referenceIn, Expr lvalueOut) {
-  // This probably cannot happen. It would require an expression to be
-  // converted to a reference and back again without an intermediate variable
-  // assignment.
   referenceIn.getConversion() = lvalueOut.(ReferenceDereferenceExpr)
 }
 
+private predicate referenceToPointerStep(Expr referenceIn, Expr pointerOut) {
+  pointerOut =
+    any(FunctionCall call |
+      stdAddressOf(call.getTarget()) and
+      referenceIn = call.getArgument(0).getFullyConverted()
+    )
+}
+
 private predicate referenceToReferenceStep(Expr referenceIn, Expr referenceOut) {
   referenceOut =
     any(FunctionCall call |
@@ -145,6 +156,12 @@ private predicate pointerFromVariableAccess(VariableAccess va, Expr pointer) {
     pointerToPointerStep(prev, pointer)
   )
   or
+  // reference -> pointer
+  exists(Expr prev |
+    referenceFromVariableAccess(va, prev) and
+    referenceToPointerStep(prev, pointer)
+  )
+  or
   // lvalue -> pointer
   exists(Expr prev |
     lvalueFromVariableAccess(va, prev) and
@@ -166,10 +183,14 @@ private predicate referenceFromVariableAccess(VariableAccess va, Expr reference)
   )
 }
 
-private predicate valueMayEscapeAt(Expr e) {
+private predicate addressMayEscapeAt(Expr e) {
   exists(Call call |
     e = call.getAnArgument().getFullyConverted() and
-    not stdIdentityFunction(call.getTarget())
+    not stdIdentityFunction(call.getTarget()) and
+    not stdAddressOf(call.getTarget())
+    or
+    e = call.getQualifier().getFullyConverted() and
+    e.getUnderlyingType() instanceof PointerType
   )
   or
   exists(AssignExpr assign | e = assign.getRValue().getFullyConverted())
@@ -187,8 +208,8 @@ private predicate valueMayEscapeAt(Expr e) {
   exists(AsmStmt asm | e = asm.getAChild().(Expr).getFullyConverted())
 }
 
-private predicate valueMayEscapeMutablyAt(Expr e) {
-  valueMayEscapeAt(e) and
+private predicate addressMayEscapeMutablyAt(Expr e) {
+  addressMayEscapeAt(e) and
   exists(Type t | t = e.getType().getUnderlyingType() |
     exists(PointerType pt |
       pt = t
@@ -207,6 +228,22 @@ private predicate valueMayEscapeMutablyAt(Expr e) {
   )
 }
 
+private predicate lvalueMayEscapeAt(Expr e) {
+  // A call qualifier, like `q` in `q.f()`, is special in that the address of
+  // `q` escapes even though `q` is not a pointer or a reference.
+  exists(Call call |
+    e = call.getQualifier().getFullyConverted() and
+    e.getType().getUnspecifiedType() instanceof Class
+  )
+}
+
+private predicate lvalueMayEscapeMutablyAt(Expr e) {
+  lvalueMayEscapeAt(e) and
+  // A qualifier of a call to a const member function is converted to a const
+  // class type.
+  not e.getType().isConst()
+}
+
 private predicate addressFromVariableAccess(VariableAccess va, Expr e) {
   pointerFromVariableAccess(va, e)
   or
@@ -253,8 +290,11 @@ private module EscapesTree_Cached {
    */
   cached
   predicate variableAddressEscapesTree(VariableAccess va, Expr e) {
-    valueMayEscapeAt(e) and
+    addressMayEscapeAt(e) and
     addressFromVariableAccess(va, e)
+    or
+    lvalueMayEscapeAt(e) and
+    lvalueFromVariableAccess(va, e)
   }
 
   /**
@@ -283,8 +323,11 @@ private module EscapesTree_Cached {
    */
   cached
   predicate variableAddressEscapesTreeNonConst(VariableAccess va, Expr e) {
-    valueMayEscapeMutablyAt(e) and
+    addressMayEscapeMutablyAt(e) and
     addressFromVariableAccess(va, e)
+    or
+    lvalueMayEscapeMutablyAt(e) and
+    lvalueFromVariableAccess(va, e)
   }
 
   /**

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

@@ -1,6 +1,6 @@
 /**
  * DEPRECATED: Recursion through `DataFlow::Configuration` is impossible in
- * Semmle Core 1.17 and above. There is no need for this module because it's
+ * any supported tooling. There is no need for this module because it's
  * impossible to accidentally depend on recursion through
  * `DataFlow::Configuration` in current releases.
  *

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

@@ -0,0 +1,247 @@
+/**
+ * Provides a local analysis for identifying where a variable address
+ * is effectively taken. Array-like offsets are allowed to pass through but
+ * not field-like offsets.
+ *
+ * This library is specialized to meet the needs of `FlowVar.qll`.
+ */
+
+/*
+ * Maintainer note: this file is one of several files that are similar but not
+ * identical. Many changes to this file will also apply to the others:
+ * - AddressConstantExpression.qll
+ * - AddressFlow.qll
+ * - EscapesTree.qll
+ */
+
+private import cpp
+
+/**
+ * Holds if `f` is an instantiation of the `std::move` or `std::forward`
+ * template functions, these functions are essentially casts, so we treat them
+ * as such.
+ */
+private predicate stdIdentityFunction(Function f) { f.hasQualifiedName("std", ["move", "forward"]) }
+
+/**
+ * Holds if `f` is an instantiation of `std::addressof`, which effectively
+ * converts a reference to a pointer.
+ */
+private predicate stdAddressOf(Function f) { f.hasQualifiedName("std", "addressof") }
+
+private predicate lvalueToLvalueStep(Expr lvalueIn, Expr lvalueOut) {
+  lvalueIn.getConversion() = lvalueOut.(ParenthesisExpr)
+  or
+  // When an object is implicitly converted to a reference to one of its base
+  // classes, it gets two `Conversion`s: there is first an implicit
+  // `CStyleCast` to its base class followed by a `ReferenceToExpr` to a
+  // reference to its base class. Whereas an explicit cast to the base class
+  // would produce an rvalue, which would not be convertible to an lvalue
+  // reference, this implicit cast instead produces an lvalue. The following
+  // case ensures that we propagate the property of being an lvalue through
+  // such casts.
+  lvalueIn.getConversion() = lvalueOut and
+  lvalueOut.(CStyleCast).isImplicit()
+  or
+  // C++ only
+  lvalueIn = lvalueOut.(PrefixCrementOperation).getOperand().getFullyConverted()
+  or
+  // C++ only
+  lvalueIn = lvalueOut.(Assignment).getLValue().getFullyConverted()
+}
+
+private predicate pointerToLvalueStep(Expr pointerIn, Expr lvalueOut) {
+  pointerIn = lvalueOut.(ArrayExpr).getArrayBase().getFullyConverted()
+  or
+  pointerIn = lvalueOut.(PointerDereferenceExpr).getOperand().getFullyConverted()
+}
+
+private predicate lvalueToPointerStep(Expr lvalueIn, Expr pointerOut) {
+  lvalueIn.getConversion() = pointerOut.(ArrayToPointerConversion)
+  or
+  lvalueIn = pointerOut.(AddressOfExpr).getOperand().getFullyConverted()
+}
+
+private predicate pointerToPointerStep(Expr pointerIn, Expr pointerOut) {
+  (
+    pointerOut instanceof PointerAddExpr
+    or
+    pointerOut instanceof PointerSubExpr
+  ) and
+  pointerIn = pointerOut.getAChild().getFullyConverted() and
+  pointerIn.getUnspecifiedType() instanceof PointerType
+  or
+  pointerIn = pointerOut.(UnaryPlusExpr).getOperand().getFullyConverted()
+  or
+  pointerIn.getConversion() = pointerOut.(Cast)
+  or
+  pointerIn.getConversion() = pointerOut.(ParenthesisExpr)
+  or
+  pointerIn = pointerOut.(ConditionalExpr).getThen().getFullyConverted()
+  or
+  pointerIn = pointerOut.(ConditionalExpr).getElse().getFullyConverted()
+  or
+  pointerIn = pointerOut.(CommaExpr).getRightOperand().getFullyConverted()
+  or
+  pointerIn = pointerOut.(StmtExpr).getResultExpr().getFullyConverted()
+}
+
+private predicate lvalueToReferenceStep(Expr lvalueIn, Expr referenceOut) {
+  lvalueIn.getConversion() = referenceOut.(ReferenceToExpr)
+}
+
+private predicate referenceToLvalueStep(Expr referenceIn, Expr lvalueOut) {
+  referenceIn.getConversion() = lvalueOut.(ReferenceDereferenceExpr)
+}
+
+private predicate referenceToPointerStep(Expr referenceIn, Expr pointerOut) {
+  pointerOut =
+    any(FunctionCall call |
+      stdAddressOf(call.getTarget()) and
+      referenceIn = call.getArgument(0).getFullyConverted()
+    )
+}
+
+private predicate referenceToReferenceStep(Expr referenceIn, Expr referenceOut) {
+  referenceOut =
+    any(FunctionCall call |
+      stdIdentityFunction(call.getTarget()) and
+      referenceIn = call.getArgument(0).getFullyConverted()
+    )
+  or
+  referenceIn.getConversion() = referenceOut.(Cast)
+  or
+  referenceIn.getConversion() = referenceOut.(ParenthesisExpr)
+}
+
+private predicate assignmentTo(Expr updated, ControlFlowNode node) {
+  updated = node.(Assignment).getLValue().getFullyConverted()
+  or
+  updated = node.(CrementOperation).getOperand().getFullyConverted()
+}
+
+private predicate lvalueToUpdate(Expr lvalue, Expr outer, ControlFlowNode node) {
+  (
+    exists(Call call | node = call |
+      outer = call.getQualifier().getFullyConverted() and
+      outer.getUnspecifiedType() instanceof Class and
+      not call.getTarget().hasSpecifier("const")
+    )
+    or
+    assignmentTo(outer, node)
+    or
+    exists(DotFieldAccess fa |
+      // `fa.otherField = ...` or `f(&fa)` or similar
+      outer = fa.getQualifier().getFullyConverted() and
+      valueToUpdate(fa, _, node)
+    )
+  ) and
+  lvalue = outer
+  or
+  exists(Expr lvalueMid |
+    lvalueToLvalueStep(lvalue, lvalueMid) and
+    lvalueToUpdate(lvalueMid, outer, node)
+  )
+  or
+  exists(Expr pointerMid |
+    lvalueToPointerStep(lvalue, pointerMid) and
+    pointerToUpdate(pointerMid, outer, node)
+  )
+  or
+  exists(Expr referenceMid |
+    lvalueToReferenceStep(lvalue, referenceMid) and
+    referenceToUpdate(referenceMid, outer, node)
+  )
+}
+
+private predicate pointerToUpdate(Expr pointer, Expr outer, ControlFlowNode node) {
+  (
+    exists(Call call | node = call |
+      outer = call.getAnArgument().getFullyConverted() and
+      exists(PointerType pt | pt = outer.getType().stripTopLevelSpecifiers() |
+        not pt.getBaseType().isConst()
+      )
+      or
+      outer = call.getQualifier().getFullyConverted() and
+      outer.getUnspecifiedType() instanceof PointerType and
+      not call.getTarget().hasSpecifier("const")
+    )
+    or
+    exists(PointerFieldAccess fa |
+      // `fa.otherField = ...` or `f(&fa)` or similar
+      outer = fa.getQualifier().getFullyConverted() and
+      valueToUpdate(fa, _, node)
+    )
+  ) and
+  pointer = outer
+  or
+  exists(Expr lvalueMid |
+    pointerToLvalueStep(pointer, lvalueMid) and
+    lvalueToUpdate(lvalueMid, outer, node)
+  )
+  or
+  exists(Expr pointerMid |
+    pointerToPointerStep(pointer, pointerMid) and
+    pointerToUpdate(pointerMid, outer, node)
+  )
+}
+
+private predicate referenceToUpdate(Expr reference, Expr outer, ControlFlowNode node) {
+  exists(Call call |
+    node = call and
+    outer = call.getAnArgument().getFullyConverted() and
+    not stdIdentityFunction(call.getTarget()) and
+    not stdAddressOf(call.getTarget()) and
+    exists(ReferenceType rt | rt = outer.getType().stripTopLevelSpecifiers() |
+      not rt.getBaseType().isConst()
+    )
+  ) and
+  reference = outer
+  or
+  exists(Expr lvalueMid |
+    referenceToLvalueStep(reference, lvalueMid) and
+    lvalueToUpdate(lvalueMid, outer, node)
+  )
+  or
+  exists(Expr pointerMid |
+    referenceToPointerStep(reference, pointerMid) and
+    pointerToUpdate(pointerMid, outer, node)
+  )
+  or
+  exists(Expr referenceMid |
+    referenceToReferenceStep(reference, referenceMid) and
+    referenceToUpdate(referenceMid, outer, node)
+  )
+}
+
+/**
+ * Holds if `node` is a control-flow node that may modify `inner` (or what it
+ * points to) through `outer`. The two expressions may be `Conversion`s. Plain
+ * assignments to variables are not included in this predicate since they are
+ * assumed to be analyzed by SSA or similar means.
+ *
+ * For example, in `f(& (*a).x)`, there are two results:
+ * - `inner` = `... .x`, `outer` = `&...`, `node` = `f(...)`.
+ * - `inner` = `a`, `outer` = `(...)`, `node` = `f(...)`.
+ */
+cached
+predicate valueToUpdate(Expr inner, Expr outer, ControlFlowNode node) {
+  (
+    lvalueToUpdate(inner, outer, node)
+    or
+    pointerToUpdate(inner, outer, node)
+    or
+    referenceToUpdate(inner, outer, node)
+  ) and
+  (
+    inner instanceof VariableAccess and
+    // Don't track non-field assignments
+    (assignmentTo(outer, _) implies inner instanceof FieldAccess)
+    or
+    inner instanceof ThisExpr
+    or
+    inner instanceof Call
+    // `inner` could also be `*` or `ReferenceDereferenceExpr`, but we
+    // can't do anything useful with those at the moment.
+  )
+}

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

@@ -26,13 +26,30 @@ private module Cached {
     )
   }
 
-  /** Provides predicates for calculating flow-through summaries. */
+  pragma[nomagic]
+  private ReturnPosition viableReturnPos(DataFlowCall call, ReturnKindExt kind) {
+    viableCallable(call) = result.getCallable() and
+    kind = result.getKind()
+  }
+
+  /**
+   * Holds if a value at return position `pos` can be returned to `out` via `call`,
+   * taking virtual dispatch into account.
+   */
   cached
+  predicate viableReturnPosOut(DataFlowCall call, ReturnPosition pos, Node out) {
+    exists(ReturnKindExt kind |
+      pos = viableReturnPos(call, kind) and
+      out = kind.getAnOutNode(call)
+    )
+  }
+
+  /** Provides predicates for calculating flow-through summaries. */
   private module FlowThrough {
     /**
      * The first flow-through approximation:
      *
-     * - Input/output access paths are abstracted with a Boolean parameter
+     * - Input access paths are abstracted with a Boolean parameter
      *   that indicates (non-)emptiness.
      */
     private module Cand {
@@ -40,83 +57,47 @@ private module Cached {
        * Holds if `p` can flow to `node` in the same callable using only
        * value-preserving steps.
        *
-       * `read` indicates whether it is contents of `p` that can flow to `node`,
-       * and `stored` indicates whether it flows to contents of `node`.
+       * `read` indicates whether it is contents of `p` that can flow to `node`.
        */
       pragma[nomagic]
-      private predicate parameterValueFlowCand(
-        ParameterNode p, Node node, boolean read, boolean stored
-      ) {
+      private predicate parameterValueFlowCand(ParameterNode p, Node node, boolean read) {
         p = node and
-        read = false and
-        stored = false
+        read = false
         or
         // local flow
         exists(Node mid |
-          parameterValueFlowCand(p, mid, read, stored) and
+          parameterValueFlowCand(p, mid, read) and
           simpleLocalFlowStep(mid, node)
         )
         or
         // read
-        exists(Node mid, boolean readMid, boolean storedMid |
-          parameterValueFlowCand(p, mid, readMid, storedMid) and
-          readStep(mid, _, node) and
-          stored = false
-        |
-          // value neither read nor stored prior to read
-          readMid = false and
-          storedMid = false and
-          read = true
-          or
-          // value (possibly read and then) stored prior to read (same content)
-          read = readMid and
-          storedMid = true
-        )
-        or
-        // store
         exists(Node mid |
-          parameterValueFlowCand(p, mid, read, false) and
-          storeStep(mid, _, node) and
-          stored = true
+          parameterValueFlowCand(p, mid, false) and
+          readStep(mid, _, node) and
+          read = true
         )
         or
-        // flow through: no prior read or store
+        // flow through: no prior read
         exists(ArgumentNode arg |
-          parameterValueFlowArgCand(p, arg, false, false) and
-          argumentValueFlowsThroughCand(arg, node, read, stored)
+          parameterValueFlowArgCand(p, arg, false) and
+          argumentValueFlowsThroughCand(arg, node, read)
         )
         or
-        // flow through: no read or store inside method
+        // flow through: no read inside method
         exists(ArgumentNode arg |
-          parameterValueFlowArgCand(p, arg, read, stored) and
-          argumentValueFlowsThroughCand(arg, node, false, false)
-        )
-        or
-        // flow through: possible prior read and prior store with compatible
-        // flow-through method
-        exists(ArgumentNode arg, boolean mid |
-          parameterValueFlowArgCand(p, arg, read, mid) and
-          argumentValueFlowsThroughCand(arg, node, mid, stored)
+          parameterValueFlowArgCand(p, arg, read) and
+          argumentValueFlowsThroughCand(arg, node, false)
         )
       }
 
       pragma[nomagic]
-      private predicate parameterValueFlowArgCand(
-        ParameterNode p, ArgumentNode arg, boolean read, boolean stored
-      ) {
-        parameterValueFlowCand(p, arg, read, stored)
+      private predicate parameterValueFlowArgCand(ParameterNode p, ArgumentNode arg, boolean read) {
+        parameterValueFlowCand(p, arg, read)
       }
 
       pragma[nomagic]
       predicate parameterValueFlowsToPreUpdateCand(ParameterNode p, PostUpdateNode n) {
-        parameterValueFlowCand(p, n.getPreUpdateNode(), false, false)
-      }
-
-      pragma[nomagic]
-      private predicate parameterValueFlowsToPostUpdateCand(
-        ParameterNode p, PostUpdateNode n, boolean read
-      ) {
-        parameterValueFlowCand(p, n, read, true)
+        parameterValueFlowCand(p, n.getPreUpdateNode(), false)
       }
 
       /**
@@ -125,33 +106,21 @@ private module Cached {
        * into account.
        *
        * `read` indicates whether it is contents of `p` that can flow to the return
-       * node, and `stored` indicates whether it flows to contents of the return
        * node.
        */
-      predicate parameterValueFlowReturnCand(
-        ParameterNode p, ReturnKindExt kind, boolean read, boolean stored
-      ) {
+      predicate parameterValueFlowReturnCand(ParameterNode p, ReturnKind kind, boolean read) {
         exists(ReturnNode ret |
-          parameterValueFlowCand(p, ret, read, stored) and
-          kind = TValueReturn(ret.getKind())
-        )
-        or
-        exists(ParameterNode p2, int pos2, PostUpdateNode n |
-          parameterValueFlowsToPostUpdateCand(p, n, read) and
-          parameterValueFlowsToPreUpdateCand(p2, n) and
-          p2.isParameterOf(_, pos2) and
-          kind = TParamUpdate(pos2) and
-          p != p2 and
-          stored = true
+          parameterValueFlowCand(p, ret, read) and
+          kind = ret.getKind()
         )
       }
 
       pragma[nomagic]
       private predicate argumentValueFlowsThroughCand0(
-        DataFlowCall call, ArgumentNode arg, ReturnKindExt kind, boolean read, boolean stored
+        DataFlowCall call, ArgumentNode arg, ReturnKind kind, boolean read
       ) {
         exists(ParameterNode param | viableParamArg(call, param, arg) |
-          parameterValueFlowReturnCand(param, kind, read, stored)
+          parameterValueFlowReturnCand(param, kind, read)
         )
       }
 
@@ -159,22 +128,19 @@ private module Cached {
        * Holds if `arg` flows to `out` through a call using only value-preserving steps,
        * not taking call contexts into account.
        *
-       * `read` indicates whether it is contents of `arg` that can flow to `out`, and
-       * `stored` indicates whether it flows to contents of `out`.
+       * `read` indicates whether it is contents of `arg` that can flow to `out`.
        */
-      predicate argumentValueFlowsThroughCand(
-        ArgumentNode arg, Node out, boolean read, boolean stored
-      ) {
-        exists(DataFlowCall call, ReturnKindExt kind |
-          argumentValueFlowsThroughCand0(call, arg, kind, read, stored) and
-          out = kind.getAnOutNode(call)
+      predicate argumentValueFlowsThroughCand(ArgumentNode arg, Node out, boolean read) {
+        exists(DataFlowCall call, ReturnKind kind |
+          argumentValueFlowsThroughCand0(call, arg, kind, read) and
+          out = getAnOutNode(call, kind)
         )
       }
 
       predicate cand(ParameterNode p, Node n) {
-        parameterValueFlowCand(p, n, _, _) and
+        parameterValueFlowCand(p, n, _) and
         (
-          parameterValueFlowReturnCand(p, _, _, _)
+          parameterValueFlowReturnCand(p, _, _)
           or
           parameterValueFlowsToPreUpdateCand(p, _)
         )
@@ -187,7 +153,6 @@ private module Cached {
         (
           n instanceof ParameterNode or
           n instanceof OutNode or
-          n instanceof PostUpdateNode or
           readStep(_, _, n) or
           n instanceof CastNode
         )
@@ -200,10 +165,6 @@ private module Cached {
           or
           n instanceof ReturnNode
           or
-          Cand::parameterValueFlowsToPreUpdateCand(_, n)
-          or
-          storeStep(n, _, _)
-          or
           readStep(n, _, _)
           or
           n instanceof CastNode
@@ -237,230 +198,140 @@ private module Cached {
     /**
      * The final flow-through calculation:
      *
-     * - Input/output access paths are abstracted with a `ContentOption` parameter
+     * - Input access paths are abstracted with a `ContentOption` parameter
      *   that represents the head of the access path. `TContentNone()` means that
      *   the access path is unrestricted.
      * - Types are checked using the `compatibleTypes()` relation.
      */
-    cached
     private module Final {
       /**
        * Holds if `p` can flow to `node` in the same callable using only
        * value-preserving steps, not taking call contexts into account.
        *
        * `contentIn` describes the content of `p` that can flow to `node`
-       * (if any), and `contentOut` describes the content of `node` that
-       * it flows to (if any).
+       * (if any).
        */
-      private predicate parameterValueFlow(
-        ParameterNode p, Node node, ContentOption contentIn, ContentOption contentOut
-      ) {
-        parameterValueFlow0(p, node, contentIn, contentOut) and
+      predicate parameterValueFlow(ParameterNode p, Node node, ContentOption contentIn) {
+        parameterValueFlow0(p, node, contentIn) and
         if node instanceof CastingNode
         then
           // normal flow through
           contentIn = TContentNone() and
-          contentOut = TContentNone() and
           compatibleTypes(getErasedNodeTypeBound(p), getErasedNodeTypeBound(node))
           or
           // getter
           exists(Content fIn |
             contentIn.getContent() = fIn and
-            contentOut = TContentNone() and
             compatibleTypes(fIn.getType(), getErasedNodeTypeBound(node))
           )
-          or
-          // (getter+)setter
-          exists(Content fOut |
-            contentOut.getContent() = fOut and
-            compatibleTypes(fOut.getContainerType(), getErasedNodeTypeBound(node))
-          )
         else any()
       }
 
       pragma[nomagic]
-      private predicate parameterValueFlow0(
-        ParameterNode p, Node node, ContentOption contentIn, ContentOption contentOut
-      ) {
+      private predicate parameterValueFlow0(ParameterNode p, Node node, ContentOption contentIn) {
         p = node and
         Cand::cand(p, _) and
-        contentIn = TContentNone() and
-        contentOut = TContentNone()
+        contentIn = TContentNone()
         or
         // local flow
         exists(Node mid |
-          parameterValueFlow(p, mid, contentIn, contentOut) and
+          parameterValueFlow(p, mid, contentIn) and
           LocalFlowBigStep::localFlowBigStep(mid, node)
         )
         or
         // read
-        exists(Node mid, Content f, ContentOption contentInMid, ContentOption contentOutMid |
-          parameterValueFlow(p, mid, contentInMid, contentOutMid) and
-          readStep(mid, f, node)
-        |
-          // value neither read nor stored prior to read
-          contentInMid = TContentNone() and
-          contentOutMid = TContentNone() and
-          contentIn.getContent() = f and
-          contentOut = TContentNone() and
-          Cand::parameterValueFlowReturnCand(p, _, true, _) and
-          compatibleTypes(getErasedNodeTypeBound(p), f.getContainerType())
-          or
-          // value (possibly read and then) stored prior to read (same content)
-          contentIn = contentInMid and
-          contentOutMid.getContent() = f and
-          contentOut = TContentNone()
-        )
-        or
-        // store
         exists(Node mid, Content f |
-          parameterValueFlow(p, mid, contentIn, TContentNone()) and
-          storeStep(mid, f, node) and
-          contentOut.getContent() = f
-        |
-          contentIn = TContentNone() and
-          compatibleTypes(getErasedNodeTypeBound(p), f.getType())
-          or
-          compatibleTypes(contentIn.getContent().getType(), f.getType())
+          parameterValueFlow(p, mid, TContentNone()) and
+          readStep(mid, f, node) and
+          contentIn.getContent() = f and
+          Cand::parameterValueFlowReturnCand(p, _, true) and
+          compatibleTypes(getErasedNodeTypeBound(p), f.getContainerType())
         )
         or
-        // flow through: no prior read or store
+        // flow through: no prior read
         exists(ArgumentNode arg |
-          parameterValueFlowArg(p, arg, TContentNone(), TContentNone()) and
-          argumentValueFlowsThrough(_, arg, contentIn, contentOut, node)
+          parameterValueFlowArg(p, arg, TContentNone()) and
+          argumentValueFlowsThrough(arg, contentIn, node)
         )
         or
-        // flow through: no read or store inside method
+        // flow through: no read inside method
         exists(ArgumentNode arg |
-          parameterValueFlowArg(p, arg, contentIn, contentOut) and
-          argumentValueFlowsThrough(_, arg, TContentNone(), TContentNone(), node)
-        )
-        or
-        // flow through: possible prior read and prior store with compatible
-        // flow-through method
-        exists(ArgumentNode arg, ContentOption contentMid |
-          parameterValueFlowArg(p, arg, contentIn, contentMid) and
-          argumentValueFlowsThrough(_, arg, contentMid, contentOut, node)
+          parameterValueFlowArg(p, arg, contentIn) and
+          argumentValueFlowsThrough(arg, TContentNone(), node)
         )
       }
 
       pragma[nomagic]
       private predicate parameterValueFlowArg(
-        ParameterNode p, ArgumentNode arg, ContentOption contentIn, ContentOption contentOut
+        ParameterNode p, ArgumentNode arg, ContentOption contentIn
       ) {
-        parameterValueFlow(p, arg, contentIn, contentOut) and
-        Cand::argumentValueFlowsThroughCand(arg, _, _, _)
+        parameterValueFlow(p, arg, contentIn) and
+        Cand::argumentValueFlowsThroughCand(arg, _, _)
       }
 
       pragma[nomagic]
       private predicate argumentValueFlowsThrough0(
-        DataFlowCall call, ArgumentNode arg, ReturnKindExt kind, ContentOption contentIn,
-        ContentOption contentOut
+        DataFlowCall call, ArgumentNode arg, ReturnKind kind, ContentOption contentIn
       ) {
         exists(ParameterNode param | viableParamArg(call, param, arg) |
-          parameterValueFlowReturn(param, _, kind, contentIn, contentOut)
+          parameterValueFlowReturn(param, kind, contentIn)
         )
       }
 
       /**
-       * Holds if `arg` flows to `out` through `call` using only value-preserving steps,
+       * Holds if `arg` flows to `out` through a call using only value-preserving steps,
        * not taking call contexts into account.
        *
-       * `contentIn` describes the content of `arg` that can flow to `out` (if any), and
-       * `contentOut` describes the content of `out` that it flows to (if any).
+       * `contentIn` describes the content of `arg` that can flow to `out` (if any).
        */
-      cached
-      predicate argumentValueFlowsThrough(
-        DataFlowCall call, ArgumentNode arg, ContentOption contentIn, ContentOption contentOut,
-        Node out
-      ) {
-        exists(ReturnKindExt kind |
-          argumentValueFlowsThrough0(call, arg, kind, contentIn, contentOut) and
-          out = kind.getAnOutNode(call)
+      pragma[nomagic]
+      predicate argumentValueFlowsThrough(ArgumentNode arg, ContentOption contentIn, Node out) {
+        exists(DataFlowCall call, ReturnKind kind |
+          argumentValueFlowsThrough0(call, arg, kind, contentIn) and
+          out = getAnOutNode(call, kind)
         |
           // normal flow through
           contentIn = TContentNone() and
-          contentOut = TContentNone() and
           compatibleTypes(getErasedNodeTypeBound(arg), getErasedNodeTypeBound(out))
           or
           // getter
           exists(Content fIn |
             contentIn.getContent() = fIn and
-            contentOut = TContentNone() and
             compatibleTypes(getErasedNodeTypeBound(arg), fIn.getContainerType()) and
             compatibleTypes(fIn.getType(), getErasedNodeTypeBound(out))
           )
-          or
-          // setter
-          exists(Content fOut |
-            contentIn = TContentNone() and
-            contentOut.getContent() = fOut and
-            compatibleTypes(getErasedNodeTypeBound(arg), fOut.getType()) and
-            compatibleTypes(fOut.getContainerType(), getErasedNodeTypeBound(out))
-          )
-          or
-          // getter+setter
-          exists(Content fIn, Content fOut |
-            contentIn.getContent() = fIn and
-            contentOut.getContent() = fOut and
-            compatibleTypes(getErasedNodeTypeBound(arg), fIn.getContainerType()) and
-            compatibleTypes(fOut.getContainerType(), getErasedNodeTypeBound(out))
-          )
         )
       }
 
-      /**
-       * Holds if `p` can flow to the pre-update node associated with post-update
-       * node `n`, in the same callable, using only value-preserving steps.
-       */
-      cached
-      predicate parameterValueFlowsToPreUpdate(ParameterNode p, PostUpdateNode n) {
-        parameterValueFlow(p, n.getPreUpdateNode(), TContentNone(), TContentNone())
-      }
-
-      pragma[nomagic]
-      private predicate parameterValueFlowsToPostUpdate(
-        ParameterNode p, PostUpdateNode n, ContentOption contentIn, ContentOption contentOut
-      ) {
-        parameterValueFlow(p, n, contentIn, contentOut) and
-        contentOut.hasContent()
-      }
-
       /**
        * Holds if `p` can flow to a return node of kind `kind` in the same
        * callable using only value-preserving steps.
        *
        * `contentIn` describes the content of `p` that can flow to the return
-       * node (if any), and `contentOut` describes the content of the return
-       * node that it flows to (if any).
+       * node (if any).
        */
-      cached
-      predicate parameterValueFlowReturn(
-        ParameterNode p, Node ret, ReturnKindExt kind, ContentOption contentIn,
-        ContentOption contentOut
+      private predicate parameterValueFlowReturn(
+        ParameterNode p, ReturnKind kind, ContentOption contentIn
       ) {
-        ret =
-          any(ReturnNode n |
-            parameterValueFlow(p, n, contentIn, contentOut) and
-            kind = TValueReturn(n.getKind())
-          )
-        or
-        ret =
-          any(PostUpdateNode n |
-            exists(ParameterNode p2, int pos2 |
-              parameterValueFlowsToPostUpdate(p, n, contentIn, contentOut) and
-              parameterValueFlowsToPreUpdate(p2, n) and
-              p2.isParameterOf(_, pos2) and
-              kind = TParamUpdate(pos2) and
-              p != p2
-            )
-          )
+        exists(ReturnNode ret |
+          parameterValueFlow(p, ret, contentIn) and
+          kind = ret.getKind()
+        )
       }
     }
 
     import Final
   }
 
+  /**
+   * Holds if `p` can flow to the pre-update node associated with post-update
+   * node `n`, in the same callable, using only value-preserving steps.
+   */
+  cached
+  predicate parameterValueFlowsToPreUpdate(ParameterNode p, PostUpdateNode n) {
+    parameterValueFlow(p, n.getPreUpdateNode(), TContentNone())
+  }
+
   /**
    * Holds if data can flow from `node1` to `node2` via a direct assignment to
    * `f`.
@@ -469,14 +340,14 @@ private module Cached {
    * been stored into, in order to handle cases like `x.f1.f2 = y`.
    */
   cached
-  predicate storeDirect(Node node1, Content f, Node node2) {
+  predicate store(Node node1, Content f, Node node2) {
     storeStep(node1, f, node2) and readStep(_, f, _)
     or
     exists(Node n1, Node n2 |
       n1 = node1.(PostUpdateNode).getPreUpdateNode() and
       n2 = node2.(PostUpdateNode).getPreUpdateNode()
     |
-      argumentValueFlowsThrough(_, n2, TContentSome(f), TContentNone(), n1)
+      argumentValueFlowsThrough(n2, TContentSome(f), n1)
       or
       readStep(n2, f, n1)
     )
@@ -520,6 +391,21 @@ private module Cached {
   newtype TReturnKindExt =
     TValueReturn(ReturnKind kind) or
     TParamUpdate(int pos) { exists(ParameterNode p | p.isParameterOf(_, pos)) }
+
+  cached
+  newtype TBooleanOption =
+    TBooleanNone() or
+    TBooleanSome(boolean b) { b = true or b = false }
+
+  cached
+  newtype TAccessPathFront =
+    TFrontNil(DataFlowType t) or
+    TFrontHead(Content f)
+
+  cached
+  newtype TAccessPathFrontOption =
+    TAccessPathFrontNone() or
+    TAccessPathFrontSome(AccessPathFront apf)
 }
 
 /**
@@ -529,8 +415,7 @@ class CastingNode extends Node {
   CastingNode() {
     this instanceof ParameterNode or
     this instanceof CastNode or
-    this instanceof OutNode or
-    this.(PostUpdateNode).getPreUpdateNode() instanceof ArgumentNode
+    this instanceof OutNodeExt
   }
 }
 
@@ -538,7 +423,7 @@ newtype TContentOption =
   TContentNone() or
   TContentSome(Content f)
 
-class ContentOption extends TContentOption {
+private class ContentOption extends TContentOption {
   Content getContent() { this = TContentSome(result) }
 
   predicate hasContent() { exists(this.getContent()) }
@@ -678,6 +563,18 @@ class ReturnNodeExt extends Node {
   }
 }
 
+/**
+ * A node to which data can flow from a call. Either an ordinary out node
+ * or a post-update node associated with a call argument.
+ */
+class OutNodeExt extends Node {
+  OutNodeExt() {
+    this instanceof OutNode
+    or
+    this.(PostUpdateNode).getPreUpdateNode() instanceof ArgumentNode
+  }
+}
+
 /**
  * An extended return kind. A return kind describes how data can be returned
  * from a callable. This can either be through a returned value or an updated
@@ -688,7 +585,7 @@ abstract class ReturnKindExt extends TReturnKindExt {
   abstract string toString();
 
   /** Gets a node corresponding to data flow out of `call`. */
-  abstract Node getAnOutNode(DataFlowCall call);
+  abstract OutNodeExt getAnOutNode(DataFlowCall call);
 }
 
 class ValueReturnKind extends ReturnKindExt, TValueReturn {
@@ -700,7 +597,9 @@ class ValueReturnKind extends ReturnKindExt, TValueReturn {
 
   override string toString() { result = kind.toString() }
 
-  override Node getAnOutNode(DataFlowCall call) { result = getAnOutNode(call, this.getKind()) }
+  override OutNodeExt getAnOutNode(DataFlowCall call) {
+    result = getAnOutNode(call, this.getKind())
+  }
 }
 
 class ParamUpdateReturnKind extends ReturnKindExt, TParamUpdate {
@@ -712,9 +611,9 @@ class ParamUpdateReturnKind extends ReturnKindExt, TParamUpdate {
 
   override string toString() { result = "param update " + pos }
 
-  override PostUpdateNode getAnOutNode(DataFlowCall call) {
+  override OutNodeExt getAnOutNode(DataFlowCall call) {
     exists(ArgumentNode arg |
-      result.getPreUpdateNode() = arg and
+      result.(PostUpdateNode).getPreUpdateNode() = arg and
       arg.argumentOf(call, this.getPosition())
     )
   }
@@ -779,77 +678,58 @@ DataFlowCallable resolveCall(DataFlowCall call, CallContext cc) {
   result = viableCallable(call) and cc instanceof CallContextReturn
 }
 
-newtype TSummary =
-  TSummaryVal() or
-  TSummaryTaint() or
-  TSummaryReadVal(Content f) or
-  TSummaryReadTaint(Content f) or
-  TSummaryTaintStore(Content f)
-
-/**
- * A summary of flow through a callable. This can either be value-preserving
- * if no additional steps are used, taint-flow if at least one additional step
- * is used, or any one of those combined with a store or a read. Summaries
- * recorded at a return node are restricted to include at least one additional
- * step, as the value-based summaries are calculated independent of the
- * configuration.
- */
-class Summary extends TSummary {
-  string toString() {
-    result = "Val" and this = TSummaryVal()
-    or
-    result = "Taint" and this = TSummaryTaint()
-    or
-    exists(Content f |
-      result = "ReadVal " + f.toString() and this = TSummaryReadVal(f)
-      or
-      result = "ReadTaint " + f.toString() and this = TSummaryReadTaint(f)
-      or
-      result = "TaintStore " + f.toString() and this = TSummaryTaintStore(f)
-    )
-  }
-
-  /** Gets the summary that results from extending this with an additional step. */
-  Summary additionalStep() {
-    this = TSummaryVal() and result = TSummaryTaint()
-    or
-    this = TSummaryTaint() and result = TSummaryTaint()
-    or
-    exists(Content f | this = TSummaryReadVal(f) and result = TSummaryReadTaint(f))
-    or
-    exists(Content f | this = TSummaryReadTaint(f) and result = TSummaryReadTaint(f))
-  }
-
-  /** Gets the summary that results from extending this with a read. */
-  Summary readStep(Content f) { this = TSummaryVal() and result = TSummaryReadVal(f) }
-
-  /** Gets the summary that results from extending this with a store. */
-  Summary storeStep(Content f) { this = TSummaryTaint() and result = TSummaryTaintStore(f) }
-
-  /** Gets the summary that results from extending this with `step`. */
-  bindingset[this, step]
-  Summary compose(Summary step) {
-    this = TSummaryVal() and result = step
-    or
-    this = TSummaryTaint() and
-    (step = TSummaryTaint() or step = TSummaryTaintStore(_)) and
-    result = step
-    or
-    exists(Content f |
-      this = TSummaryReadVal(f) and step = TSummaryTaint() and result = TSummaryReadTaint(f)
-    )
-    or
-    this = TSummaryReadTaint(_) and step = TSummaryTaint() and result = this
-  }
-
-  /** Holds if this summary does not include any taint steps. */
-  predicate isPartial() {
-    this = TSummaryVal() or
-    this = TSummaryReadVal(_)
-  }
-}
-
 pragma[noinline]
 DataFlowType getErasedNodeTypeBound(Node n) { result = getErasedRepr(n.getTypeBound()) }
 
-predicate readDirect = readStep/3;
+predicate read = readStep/3;
+
+/** An optional Boolean value. */
+class BooleanOption extends TBooleanOption {
+  string toString() {
+    this = TBooleanNone() and result = "<none>"
+    or
+    this = TBooleanSome(any(boolean b | result = b.toString()))
+  }
+}
+
+/**
+ * The front of an access path. This is either a head or a nil.
+ */
+abstract class AccessPathFront extends TAccessPathFront {
+  abstract string toString();
+
+  abstract DataFlowType getType();
+
+  abstract boolean toBoolNonEmpty();
+
+  predicate headUsesContent(Content f) { this = TFrontHead(f) }
+}
+
+class AccessPathFrontNil extends AccessPathFront, TFrontNil {
+  override string toString() {
+    exists(DataFlowType t | this = TFrontNil(t) | result = ppReprType(t))
+  }
+
+  override DataFlowType getType() { this = TFrontNil(result) }
+
+  override boolean toBoolNonEmpty() { result = false }
+}
+
+class AccessPathFrontHead extends AccessPathFront, TFrontHead {
+  override string toString() { exists(Content f | this = TFrontHead(f) | result = f.toString()) }
+
+  override DataFlowType getType() {
+    exists(Content head | this = TFrontHead(head) | result = head.getContainerType())
+  }
+
+  override boolean toBoolNonEmpty() { result = true }
+}
+
+/** An optional access path front. */
+class AccessPathFrontOption extends TAccessPathFrontOption {
+  string toString() {
+    this = TAccessPathFrontNone() and result = "<none>"
+    or
+    this = TAccessPathFrontSome(any(AccessPathFront apf | result = apf.toString()))
+  }
+}

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

@@ -311,9 +311,6 @@ predicate isImmutableOrUnobservable(Node n) {
     or
     dt.getBaseType() instanceof RoutineType
   )
-  or
-  // Isn't something we can track
-  n.asExpr() instanceof Call
   // The above list of cases isn't exhaustive, but it narrows down the
   // consistency alerts enough that most of them are interesting.
 }

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

@@ -20,10 +20,12 @@ private newtype TNode =
   TInstanceParameterNode(MemberFunction f) { exists(f.getBlock()) and not f.isStatic() } or
   TPreConstructorInitThis(ConstructorFieldInit cfi) or
   TPostConstructorInitThis(ConstructorFieldInit cfi) or
-  TThisArgumentPostUpdate(ThisExpr ta) {
-    exists(Call c, int i |
-      ta = c.getArgument(i) and
-      not c.getTarget().getParameter(i).getUnderlyingType().(PointerType).getBaseType().isConst()
+  TInnerPartialDefinitionNode(Expr e) {
+    exists(PartialDefinition def, Expr outer |
+      def.definesExpressions(e, outer) and
+      // This condition ensures that we don't get two post-update nodes sharing
+      // the same pre-update node.
+      e != outer
     )
   } or
   TUninitializedNode(LocalVariable v) { not v.hasInitializer() } or
@@ -43,7 +45,7 @@ class Node extends TNode {
   /**
    * INTERNAL: Do not use. Alternative name for `getFunction`.
    */
-  Function getEnclosingCallable() { result = this.getFunction() }
+  final Function getEnclosingCallable() { result = unique(Function f | f = this.getFunction() | f) }
 
   /** Gets the type of this node. */
   Type getType() { none() } // overridden in subclasses
@@ -66,7 +68,7 @@ class Node extends TNode {
    * a partial definition of `&x`).
    */
   Expr asPartialDefinition() {
-    result = this.(PartialDefinitionNode).getPartialDefinition().getDefinedExpr()
+    this.(PartialDefinitionNode).getPartialDefinition().definesExpressions(_, result)
   }
 
   /**
@@ -177,25 +179,23 @@ class ImplicitParameterNode extends ParameterNode, TInstanceParameterNode {
  * returned. This node will have its `getArgument()` equal to `&x`.
  */
 class DefinitionByReferenceNode extends PartialDefinitionNode {
-  VariableAccess va;
+  Expr inner;
   Expr argument;
 
   DefinitionByReferenceNode() {
-    exists(DefinitionByReference def |
-      def = this.getPartialDefinition() and
-      argument = def.getDefinedExpr() and
-      va = def.getVariableAccess()
-    )
+    this.getPartialDefinition().(DefinitionByReference).definesExpressions(inner, argument)
   }
 
-  override Function getFunction() { result = va.getEnclosingFunction() }
+  override Function getFunction() { result = inner.getEnclosingFunction() }
 
-  override Type getType() { result = va.getType() }
+  override Type getType() { result = inner.getType() }
 
   override string toString() { result = "ref arg " + argument.toString() }
 
   override Location getLocation() { result = argument.getLocation() }
 
+  override ExprNode getPreUpdateNode() { result.getExpr() = argument }
+
   /** Gets the argument corresponding to this node. */
   Expr getArgument() { result = argument }
 
@@ -276,23 +276,32 @@ private class PartialDefinitionNode extends PostUpdateNode, TPartialDefinitionNo
 
   PartialDefinitionNode() { this = TPartialDefinitionNode(pd) }
 
-  override Node getPreUpdateNode() { result.asExpr() = pd.getDefinedExpr() }
+  override Node getPreUpdateNode() { pd.definesExpressions(_, result.asExpr()) }
 
-  override Location getLocation() { result = pd.getLocation() }
+  override Location getLocation() { result = pd.getActualLocation() }
 
   PartialDefinition getPartialDefinition() { result = pd }
 
   override string toString() { result = getPreUpdateNode().toString() + " [post update]" }
 }
 
-private class ThisArgumentPostUpdateNode extends PostUpdateNode, TThisArgumentPostUpdate {
-  ThisExpr thisExpr;
+/**
+ * A post-update node on the `e->f` in `f(&e->f)` (and other forms).
+ */
+private class InnerPartialDefinitionNode extends TInnerPartialDefinitionNode, PostUpdateNode {
+  Expr e;
 
-  ThisArgumentPostUpdateNode() { this = TThisArgumentPostUpdate(thisExpr) }
+  InnerPartialDefinitionNode() { this = TInnerPartialDefinitionNode(e) }
 
-  override Node getPreUpdateNode() { result.asExpr() = thisExpr }
+  override ExprNode getPreUpdateNode() { result.getExpr() = e }
 
-  override string toString() { result = "ref arg this" }
+  override Function getFunction() { result = e.getEnclosingFunction() }
+
+  override Type getType() { result = e.getType() }
+
+  override string toString() { result = e.toString() + " [inner post update]" }
+
+  override Location getLocation() { result = e.getLocation() }
 }
 
 /**
@@ -475,8 +484,6 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
   // Expr -> Expr
   exprToExprStep_nocfg(nodeFrom.asExpr(), nodeTo.asExpr())
   or
-  exprToExprStep_nocfg(nodeFrom.(PostUpdateNode).getPreUpdateNode().asExpr(), nodeTo.asExpr())
-  or
   // Node -> FlowVar -> VariableAccess
   exists(FlowVar var |
     (
@@ -499,6 +506,14 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
   or
   // post-update-`this` -> following-`this`-ref
   ThisFlow::adjacentThisRefs(nodeFrom.(PostUpdateNode).getPreUpdateNode(), nodeTo)
+  or
+  // In `f(&x->a)`, this step provides the flow from post-`&` to post-`x->a`,
+  // from which there is field flow to `x` via reverse read.
+  exists(PartialDefinition def, Expr inner, Expr outer |
+    def.definesExpressions(inner, outer) and
+    inner = nodeTo.(InnerPartialDefinitionNode).getPreUpdateNode().asExpr() and
+    outer = nodeFrom.(PartialDefinitionNode).getPreUpdateNode().asExpr()
+  )
 }
 
 /**
@@ -636,7 +651,7 @@ private module FieldFlow {
     exists(FieldConfiguration cfg | cfg.hasFlow(node1, node2)) and
     // This configuration should not be able to cross function boundaries, but
     // we double-check here just to be sure.
-    node1.getFunction() = node2.getFunction()
+    node1.getEnclosingCallable() = node2.getEnclosingCallable()
   }
 }
 

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

@@ -5,6 +5,7 @@
 import cpp
 private import semmle.code.cpp.controlflow.SSA
 private import semmle.code.cpp.dataflow.internal.SubBasicBlocks
+private import semmle.code.cpp.dataflow.internal.AddressFlow
 
 /**
  * A conceptual variable that is assigned only once, like an SSA variable. This
@@ -56,15 +57,9 @@ class FlowVar extends TFlowVar {
   abstract predicate definedByExpr(Expr e, ControlFlowNode node);
 
   /**
-   * Holds if this `FlowVar` corresponds to the data written by a call that
-   * passes a variable as argument `arg`.
-   */
-  cached
-  abstract predicate definedByReference(Expr arg);
-
-  /**
-   * Holds if this `FlowVar` is a `PartialDefinition` whose defined expression
-   * is `e`.
+   * Holds if this `FlowVar` is a `PartialDefinition` whose outer defined
+   * expression is `e`. For example, in `f(&x)`, the outer defined expression
+   * is `&x`.
    */
   cached
   abstract predicate definedPartiallyAt(Expr e);
@@ -113,44 +108,21 @@ class FlowVar extends TFlowVar {
  * ```
  */
 private module PartialDefinitions {
-  private newtype TPartialDefinition =
-    TExplicitFieldStoreQualifier(Expr qualifier, ControlFlowNode node) {
-      exists(FieldAccess fa | qualifier = fa.getQualifier() |
-        isInstanceFieldWrite(fa, node)
-        or
-        exists(PartialDefinition pd |
-          node = pd.getSubBasicBlockStart() and
-          fa = pd.getDefinedExpr()
-        )
-      )
-    } or
-    TExplicitCallQualifier(Expr qualifier) {
-      exists(Call call |
-        qualifier = call.getQualifier() and
-        not call.getTarget().hasSpecifier("const")
-      )
-    } or
-    TReferenceArgument(Expr arg, VariableAccess va) { referenceArgument(va, arg) }
-
-  private predicate isInstanceFieldWrite(FieldAccess fa, ControlFlowNode node) {
-    assignmentLikeOperation(node, _, fa, _)
-  }
-
-  class PartialDefinition extends TPartialDefinition {
-    Expr definedExpr;
+  class PartialDefinition extends Expr {
+    Expr innerDefinedExpr;
     ControlFlowNode node;
 
     PartialDefinition() {
-      this = TExplicitFieldStoreQualifier(definedExpr, node)
-      or
-      this = TExplicitCallQualifier(definedExpr) and node = definedExpr
-      or
-      this = TReferenceArgument(definedExpr, node)
+      exists(Expr convertedInner |
+        valueToUpdate(convertedInner, this.getFullyConverted(), node) and
+        innerDefinedExpr = convertedInner.getUnconverted() and
+        not this instanceof Conversion
+      )
     }
 
-    predicate partiallyDefines(Variable v) { definedExpr = v.getAnAccess() }
+    predicate partiallyDefines(Variable v) { innerDefinedExpr = v.getAnAccess() }
 
-    predicate partiallyDefinesThis(ThisExpr e) { definedExpr = e }
+    predicate partiallyDefinesThis(ThisExpr e) { innerDefinedExpr = e }
 
     /**
      * Gets the subBasicBlock where this `PartialDefinition` is defined.
@@ -158,69 +130,37 @@ private module PartialDefinitions {
     ControlFlowNode getSubBasicBlockStart() { result = node }
 
     /**
-     * Gets the expression that is being partially defined. For example in the
-     * following code:
-     * ```
-     * x.y = 1;
-     * ```
-     * The expression `x` is being partially defined.
+     * Holds if this partial definition may modify `inner` (or what it points
+     * to) through `outer`. These expressions will never be `Conversion`s.
+     *
+     * For example, in `f(& (*a).x)`, there are two results:
+     * - `inner` = `... .x`, `outer` = `&...`
+     * - `inner` = `a`, `outer` = `*`
      */
-    Expr getDefinedExpr() { result = definedExpr }
-
-    Location getLocation() {
-      not exists(definedExpr.getLocation()) and result = definedExpr.getParent().getLocation()
-      or
-      definedExpr.getLocation() instanceof UnknownLocation and
-      result = definedExpr.getParent().getLocation()
-      or
-      result = definedExpr.getLocation() and not result instanceof UnknownLocation
+    predicate definesExpressions(Expr inner, Expr outer) {
+      inner = innerDefinedExpr and
+      outer = this
     }
 
-    string toString() { result = "partial def of " + definedExpr }
+    /**
+     * Gets the location of this element, adjusted to avoid unknown locations
+     * on compiler-generated `ThisExpr`s.
+     */
+    Location getActualLocation() {
+      not exists(this.getLocation()) and result = this.getParent().getLocation()
+      or
+      this.getLocation() instanceof UnknownLocation and
+      result = this.getParent().getLocation()
+      or
+      result = this.getLocation() and not result instanceof UnknownLocation
+    }
   }
 
   /**
    * A partial definition that's a definition by reference.
    */
-  class DefinitionByReference extends PartialDefinition, TReferenceArgument {
-    VariableAccess va;
-
-    DefinitionByReference() {
-      // `this` is not restricted in this charpred. That's because the full
-      // extent of this class includes the charpred of the superclass, which
-      // relates `this` to `definedExpr`, and `va` is functionally determined
-      // by `definedExpr`.
-      referenceArgument(va, definedExpr)
-    }
-
-    VariableAccess getVariableAccess() { result = va }
-
-    override predicate partiallyDefines(Variable v) { va = v.getAnAccess() }
-  }
-
-  private predicate referenceArgument(VariableAccess va, Expr argument) {
-    argument = any(Call c).getAnArgument() and
-    exists(Type argumentType |
-      argumentType = argument.getFullyConverted().getType().stripTopLevelSpecifiers()
-    |
-      argumentType instanceof ReferenceType and
-      not argumentType.(ReferenceType).getBaseType().isConst() and
-      va = argument
-      or
-      argumentType instanceof PointerType and
-      not argumentType.(PointerType).getBaseType().isConst() and
-      (
-        // f(variable)
-        va = argument
-        or
-        // f(&variable)
-        va = argument.(AddressOfExpr).getOperand()
-        or
-        // f(&array[0])
-        va.getType().getUnspecifiedType() instanceof ArrayType and
-        va = argument.(AddressOfExpr).getOperand().(ArrayExpr).getArrayBase()
-      )
-    )
+  class DefinitionByReference extends PartialDefinition {
+    DefinitionByReference() { exists(Call c | this = c.getAnArgument() or this = c.getQualifier()) }
   }
 }
 
@@ -337,10 +277,6 @@ module FlowVar_internal {
       )
     }
 
-    override predicate definedByReference(Expr arg) {
-      none() // Not supported for SSA. See `fullySupportedSsaVariable`.
-    }
-
     override predicate definedPartiallyAt(Expr e) { none() }
 
     override predicate definedByInitialValue(StackVariable param) {
@@ -425,19 +361,11 @@ module FlowVar_internal {
       node = sbb.getANode()
     }
 
-    override predicate definedByReference(Expr arg) {
-      exists(DefinitionByReference def |
-        def.partiallyDefines(v) and
-        sbb = def.getSubBasicBlockStart() and
-        arg = def.getDefinedExpr()
-      )
-    }
-
     override predicate definedPartiallyAt(Expr e) {
       exists(PartialDefinition p |
         p.partiallyDefines(v) and
         sbb = p.getSubBasicBlockStart() and
-        e = p.getDefinedExpr()
+        p.definesExpressions(_, e)
       )
     }
 
@@ -450,11 +378,6 @@ module FlowVar_internal {
       this.definedByInitialValue(_) and
       result = "initial value of " + v
       or
-      exists(Expr arg |
-        this.definedByReference(arg) and
-        result = "definition by reference of " + v
-      )
-      or
       exists(Expr partialDef |
         this.definedPartiallyAt(partialDef) and
         result = "partial definition at " + partialDef
@@ -463,7 +386,6 @@ module FlowVar_internal {
       // impossible case
       not this.definedByExpr(_, _) and
       not this.definedByInitialValue(_) and
-      not this.definedByReference(_) and
       not this.definedPartiallyAt(_) and
       result = "undefined " + v
     }

cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking1/TaintTrackingImpl.qll

@@ -79,13 +79,6 @@ abstract class Configuration extends DataFlow::Configuration {
     defaultTaintBarrier(node)
   }
 
-  /** DEPRECATED: override `isSanitizerIn` and `isSanitizerOut` instead. */
-  deprecated predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { none() }
-
-  deprecated final override predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) {
-    isSanitizerEdge(node1, node2)
-  }
-
   /** Holds if data flow into `node` is prohibited. */
   predicate isSanitizerIn(DataFlow::Node node) { none() }
 

cpp/ql/src/semmle/code/cpp/dataflow/internal/tainttracking2/TaintTrackingImpl.qll

@@ -79,13 +79,6 @@ abstract class Configuration extends DataFlow::Configuration {
     defaultTaintBarrier(node)
   }
 
-  /** DEPRECATED: override `isSanitizerIn` and `isSanitizerOut` instead. */
-  deprecated predicate isSanitizerEdge(DataFlow::Node node1, DataFlow::Node node2) { none() }
-
-  deprecated final override predicate isBarrierEdge(DataFlow::Node node1, DataFlow::Node node2) {
-    isSanitizerEdge(node1, node2)
-  }
-
   /** Holds if data flow into `node` is prohibited. */
   predicate isSanitizerIn(DataFlow::Node node) { none() }
 

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

@@ -6,9 +6,13 @@ private import semmle.code.cpp.dataflow.EscapesTree
 /**
  * A C/C++ access expression. This refers to a function, variable, or enum constant.
  */
-abstract class Access extends Expr, NameQualifiableElement {
+class Access extends Expr, NameQualifiableElement, @access {
+  // As `@access` is a union type containing `@routineexpr` (which describes function accesses
+  // that are called), we need to exclude function calls.
+  Access() { this instanceof @routineexpr implies not iscall(underlyingElement(this), _) }
+
   /** Gets the accessed function, variable, or enum constant. */
-  abstract Declaration getTarget();
+  Declaration getTarget() { none() } // overridden in subclasses
 
   override predicate mayBeImpure() { none() }
 
@@ -338,7 +342,7 @@ class PointerToFieldLiteral extends ImplicitThisFieldAccess {
  * int myFunctionTarget(int);
  *
  * void myFunction() {
- *   int (*myFunctionPointer)(int) = &myTarget;
+ *   int (*myFunctionPointer)(int) = &myFunctionTarget;
  * }
  * ```
  */

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

@@ -4,7 +4,7 @@ import semmle.code.cpp.exprs.Expr
  * A C/C++ builtin operation. This is the root QL class encompassing
  * built-in functionality.
  */
-abstract class BuiltInOperation extends Expr {
+class BuiltInOperation extends Expr, @builtin_op {
   override string getCanonicalQLClass() { result = "BuiltInOperation" }
 }
 
@@ -12,17 +12,7 @@ abstract class BuiltInOperation extends Expr {
  * A C/C++ built-in operation that is used to support functions with variable numbers of arguments.
  * This includes `va_start`, `va_end`, `va_copy`, and `va_arg`.
  */
-class VarArgsExpr extends BuiltInOperation {
-  VarArgsExpr() {
-    this instanceof BuiltInVarArgsStart
-    or
-    this instanceof BuiltInVarArgsEnd
-    or
-    this instanceof BuiltInVarArg
-    or
-    this instanceof BuiltInVarArgCopy
-  }
-}
+class VarArgsExpr extends BuiltInOperation, @var_args_expr { }
 
 /**
  * A C/C++ `__builtin_va_start` built-in operation (used by some

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

@@ -9,9 +9,8 @@ private import semmle.code.cpp.dataflow.EscapesTree
  */
 abstract class Call extends Expr, NameQualifiableElement {
   /**
-   * Gets the number of actual parameters in this call; use
-   * `getArgument(i)` with `i` between `0` and `result - 1` to
-   * retrieve actuals.
+   * Gets the number of arguments (actual parameters) of this call. The count
+   * does _not_ include the qualifier of the call, if any.
    */
   int getNumberOfArguments() { result = count(this.getAnArgument()) }
 
@@ -32,21 +31,24 @@ abstract class Call extends Expr, NameQualifiableElement {
   Expr getQualifier() { result = this.getChild(-1) }
 
   /**
-   * Gets an argument for this call.
+   * Gets an argument for this call. To get the qualifier of this call, if
+   * any, use `getQualifier()`.
    */
   Expr getAnArgument() { exists(int i | result = this.getChild(i) and i >= 0) }
 
   /**
    * Gets the nth argument for this call.
    *
-   * The range of `n` is from `0` to `getNumberOfArguments() - 1`.
+   * The range of `n` is from `0` to `getNumberOfArguments() - 1`. To get the
+   * qualifier of this call, if any, use `getQualifier()`.
    */
   Expr getArgument(int n) { result = this.getChild(n) and n >= 0 }
 
   /**
-   * Gets a sub expression of the argument at position `index`. If the
+   * Gets a subexpression of the argument at position `index`. If the
    * argument itself contains calls, such calls will be considered
-   * leafs in the expression tree.
+   * leaves in the expression tree. The qualifier of the call, if any, is not
+   * considered to be an argument.
    *
    * Example: the call `f(2, 3 + 4, g(4 + 5))` has sub expression(s)
    * `2` at index 0; `3`, `4`, and `3 + 4` at index 1; and `g(4 + 5)`
@@ -80,7 +82,8 @@ abstract class Call extends Expr, NameQualifiableElement {
 
   /**
    * Holds if this call passes the variable accessed by `va` by
-   * reference as the `i`th argument.
+   * reference as the `i`th argument. The qualifier of a call to a member
+   * function is `i = -1`.
    *
    * A variable is passed by reference if the `i`th parameter of the function
    * receives an address that points within the object denoted by `va`. For a
@@ -99,11 +102,15 @@ abstract class Call extends Expr, NameQualifiableElement {
    */
   predicate passesByReference(int i, VariableAccess va) {
     variableAddressEscapesTree(va, this.getArgument(i).getFullyConverted())
+    or
+    variableAddressEscapesTree(va, this.getQualifier().getFullyConverted()) and
+    i = -1
   }
 
   /**
    * Holds if this call passes the variable accessed by `va` by
-   * reference to non-const data as the `i`th argument.
+   * reference to non-const data as the `i`th argument. The qualifier of a
+   * call to a member function is `i = -1`.
    *
    * A variable is passed by reference if the `i`th parameter of the function
    * receives an address that points within the object denoted by `va`. For a
@@ -122,6 +129,9 @@ abstract class Call extends Expr, NameQualifiableElement {
    */
   predicate passesByReferenceNonConst(int i, VariableAccess va) {
     variableAddressEscapesTreeNonConst(va, this.getArgument(i).getFullyConverted())
+    or
+    variableAddressEscapesTreeNonConst(va, this.getQualifier().getFullyConverted()) and
+    i = -1
   }
 }
 

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

@@ -7,7 +7,7 @@ private import semmle.code.cpp.internal.ResolveClass
  * Instances of this class are not present in the main AST which is navigated by parent/child links. Instead,
  * instances of this class are attached to nodes in the main AST via special conversion links.
  */
-abstract class Conversion extends Expr {
+class Conversion extends Expr, @conversion {
   /** Gets the expression being converted. */
   Expr getExpr() { result.getConversion() = this }
 
@@ -34,10 +34,10 @@ abstract class Conversion extends Expr {
  * a `PointerBaseClassConversion`, or some other semantic conversion. Similarly,
  * a `PointerDerivedClassConversion` may also be a `CStyleCast` or a `StaticCast`.
  *
- * This is an abstract root QL class representing the different casts.  For
+ * This is a root QL class representing the different casts.  For
  * specific examples, consult the documentation for any of QL classes mentioned above.
  */
-abstract class Cast extends Conversion, @cast {
+class Cast extends Conversion, @cast {
   /**
    * Gets a string describing the semantic conversion operation being performed by
    * this cast.
@@ -48,10 +48,13 @@ abstract class Cast extends Conversion, @cast {
 /**
  * INTERNAL: Do not use.
  * Query predicates used to check invariants that should hold for all `Cast`
- * nodes. To run all sanity queries for the ASTs, including the ones below,
- * run "semmle/code/cpp/ASTSanity.ql".
+ * nodes. To run all consistency queries for the ASTs, including the ones below,
+ * run "semmle/code/cpp/ASTConsistency.ql".
  */
-module CastSanity {
+module CastConsistency {
+  /**
+   * Holds if the cast has more than one result for `Cast.getSemanticConversionString()`.
+   */
   query predicate multipleSemanticConversionStrings(Cast cast, Type fromType, string kind) {
     // Every cast should have exactly one semantic conversion kind
     count(cast.getSemanticConversionString()) > 1 and
@@ -59,12 +62,19 @@ module CastSanity {
     fromType = cast.getExpr().getUnspecifiedType()
   }
 
+  /**
+   * Holds if the cast has no result for `Cast.getSemanticConversionString()`.
+   */
   query predicate missingSemanticConversionString(Cast cast, Type fromType) {
     // Every cast should have exactly one semantic conversion kind
     not exists(cast.getSemanticConversionString()) and
     fromType = cast.getExpr().getUnspecifiedType()
   }
 
+  /**
+   * Holds if the cast has a result for `Cast.getSemanticConversionString()` that indicates that the
+   * kind of its semantic conversion is not known.
+   */
   query predicate unknownSemanticConversionString(Cast cast, Type fromType) {
     // Every cast should have a known semantic conversion kind
     cast.getSemanticConversionString() = "unknown conversion" and
@@ -699,7 +709,7 @@ class SizeofPackOperator extends Expr, @sizeof_pack {
 /**
  * A C/C++ sizeof expression.
  */
-abstract class SizeofOperator extends Expr, @runtime_sizeof {
+class SizeofOperator extends Expr, @runtime_sizeof {
   override int getPrecedence() { result = 16 }
 }
 
@@ -762,7 +772,7 @@ class SizeofTypeOperator extends SizeofOperator {
 /**
  * A C++11 `alignof` expression.
  */
-abstract class AlignofOperator extends Expr, @runtime_alignof {
+class AlignofOperator extends Expr, @runtime_alignof {
   override int getPrecedence() { result = 16 }
 }
 

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

@@ -86,7 +86,7 @@ class Closure extends Class {
     result.getName() = "operator()"
   }
 
-  override string toString() { result = "decltype([...](...){...})" }
+  override string getDescription() { result = "decltype([...](...){...})" }
 }
 
 /**
@@ -99,7 +99,7 @@ class Closure extends Class {
  * ```
  */
 class LambdaCapture extends Locatable, @lambdacapture {
-  override string toString() { result = getField().toString() }
+  override string toString() { result = getField().getName() }
 
   override string getCanonicalQLClass() { result = "LambdaCapture" }
 

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

@@ -145,8 +145,6 @@ class HexLiteral extends Literal {
 
 /**
  * A C/C++ aggregate literal.
- *
- * For example:
  */
 class AggregateLiteral extends Expr, @aggregateliteral {
   override string getCanonicalQLClass() { result = "AggregateLiteral" }

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

@@ -1,3 +1,11 @@
+/*
+ * Maintainer note: this file is one of several files that are similar but not
+ * identical. Many changes to this file will also apply to the others:
+ * - AddressConstantExpression.qll
+ * - AddressFlow.qll
+ * - EscapesTree.qll
+ */
+
 private import cpp
 
 predicate addressConstantExpression(Expr e) {

cpp/ql/src/semmle/code/cpp/ir/IRConsistency.ql

@@ -0,0 +1,8 @@
+/**
+ * @name IR Consistency Check
+ * @description Performs consistency checks on the Intermediate Representation. This query should have no results.
+ * @kind table
+ * @id cpp/ir-consistency-check
+ */
+
+import implementation.aliased_ssa.IRConsistency

cpp/ql/src/semmle/code/cpp/ir/IRSanity.ql

@@ -1,8 +0,0 @@
-/**
- * @name IR Sanity Check
- * @description Performs sanity checks on the Intermediate Representation. This query should have no results.
- * @kind table
- * @id cpp/ir-sanity-check
- */
-
-import implementation.aliased_ssa.IRSanity

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

@@ -1,9 +1,12 @@
 import cpp
 import semmle.code.cpp.security.Security
 private import semmle.code.cpp.ir.dataflow.DataFlow
+private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
 private import semmle.code.cpp.ir.dataflow.DataFlow2
+private import semmle.code.cpp.ir.dataflow.DataFlow3
 private import semmle.code.cpp.ir.IR
 private import semmle.code.cpp.ir.dataflow.internal.DataFlowDispatch as Dispatch
+private import semmle.code.cpp.controlflow.IRGuards
 private import semmle.code.cpp.models.interfaces.Taint
 private import semmle.code.cpp.models.interfaces.DataFlow
 
@@ -143,7 +146,17 @@ private predicate writesVariable(StoreInstruction store, Variable var) {
 }
 
 /**
- * A variable that has any kind of upper-bound check anywhere in the program
+ * A variable that has any kind of upper-bound check anywhere in the program.  This is
+ * biased towards being inclusive because there are a lot of valid ways of doing an
+ * upper bounds checks if we don't consider where it occurs, for example:
+ * ```
+ *   if (x < 10) { sink(x); }
+ *
+ *   if (10 > y) { sink(y); }
+ *
+ *   if (z > 10) { z = 10; }
+ *   sink(z);
+ * ```
  */
 // TODO: This coarse overapproximation, ported from the old taint tracking
 // library, could be replaced with an actual semantic check that a particular
@@ -152,18 +165,41 @@ private predicate writesVariable(StoreInstruction store, Variable var) {
 // previously suppressed by this predicate by coincidence.
 private predicate hasUpperBoundsCheck(Variable var) {
   exists(RelationalOperation oper, VariableAccess access |
-    oper.getLeftOperand() = access and
+    oper.getAnOperand() = access and
     access.getTarget() = var and
     // Comparing to 0 is not an upper bound check
-    not oper.getRightOperand().getValue() = "0"
+    not oper.getAnOperand().getValue() = "0"
   )
 }
 
+private predicate nodeIsBarrierEqualityCandidate(
+  DataFlow::Node node, Operand access, Variable checkedVar
+) {
+  readsVariable(node.asInstruction(), checkedVar) and
+  any(IRGuardCondition guard).ensuresEq(access, _, _, node.asInstruction().getBlock(), true)
+}
+
 private predicate nodeIsBarrier(DataFlow::Node node) {
   exists(Variable checkedVar |
     readsVariable(node.asInstruction(), checkedVar) and
     hasUpperBoundsCheck(checkedVar)
   )
+  or
+  exists(Variable checkedVar, Operand access |
+    /*
+     * This node is guarded by a condition that forces the accessed variable
+     * to equal something else.  For example:
+     * ```
+     * x = taintsource()
+     * if (x == 10) {
+     *   taintsink(x); // not considered tainted
+     * }
+     * ```
+     */
+
+    nodeIsBarrierEqualityCandidate(node, access, checkedVar) and
+    readsVariable(access.getDef(), checkedVar)
+  )
 }
 
 private predicate nodeIsBarrierIn(DataFlow::Node node) {
@@ -171,6 +207,7 @@ private predicate nodeIsBarrierIn(DataFlow::Node node) {
   node = getNodeForSource(any(Expr e))
 }
 
+cached
 private predicate instructionTaintStep(Instruction i1, Instruction i2) {
   // Expressions computed from tainted data are also tainted
   exists(CallInstruction call, int argIndex | call = i2 |
@@ -187,11 +224,44 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
   // Flow through pointer dereference
   i2.(LoadInstruction).getSourceAddress() = i1
   or
-  i2.(UnaryInstruction).getUnary() = i1
+  // Flow through partial reads of arrays and unions
+  i2.(LoadInstruction).getSourceValueOperand().getAnyDef() = i1 and
+  not i1.isResultConflated() and
+  (
+    i1.getResultType() instanceof ArrayType or
+    i1.getResultType() instanceof Union
+  )
   or
-  i2.(ChiInstruction).getPartial() = i1 and
+  // Unary instructions tend to preserve enough information in practice that we
+  // want taint to flow through.
+  // The exception is `FieldAddressInstruction`. Together with the rule for
+  // `LoadInstruction` above and for `ChiInstruction` below, flow through
+  // `FieldAddressInstruction` could cause flow into one field to come out an
+  // unrelated field. This would happen across function boundaries, where the IR
+  // would not be able to match loads to stores.
+  i2.(UnaryInstruction).getUnary() = i1 and
+  (
+    not i2 instanceof FieldAddressInstruction
+    or
+    i2.(FieldAddressInstruction).getField().getDeclaringType() instanceof Union
+  )
+  or
+  // Flow out of definition-by-reference
+  i2.(ChiInstruction).getPartial() = i1.(WriteSideEffectInstruction) and
   not i2.isResultConflated()
   or
+  // Flow from an element to an array or union that contains it.
+  i2.(ChiInstruction).getPartial() = i1 and
+  not i2.isResultConflated() and
+  exists(Type t | i2.getResultLanguageType().hasType(t, false) |
+    t instanceof Union
+    or
+    t instanceof ArrayType
+    or
+    // Buffers of unknown size
+    t instanceof UnknownType
+  )
+  or
   exists(BinaryInstruction bin |
     bin = i2 and
     predictableInstruction(i2.getAnOperand().getDef()) and
@@ -348,6 +418,16 @@ private Element adjustedSink(DataFlow::Node sink) {
   result.(AssignOperation).getAnOperand() = sink.asExpr()
 }
 
+/**
+ * Holds if `tainted` may contain taint from `source`.
+ *
+ * A tainted expression is either directly user input, or is
+ * computed from user input in a way that users can probably
+ * control the exact output of the computation.
+ *
+ * This doesn't include data flow through global variables.
+ * If you need that you must call `taintedIncludingGlobalVars`.
+ */
 cached
 predicate tainted(Expr source, Element tainted) {
   exists(DefaultTaintTrackingCfg cfg, DataFlow::Node sink |
@@ -356,6 +436,21 @@ predicate tainted(Expr source, Element tainted) {
   )
 }
 
+/**
+ * Holds if `tainted` may contain taint from `source`, where the taint passed
+ * through a global variable named `globalVar`.
+ *
+ * A tainted expression is either directly user input, or is
+ * computed from user input in a way that users can probably
+ * control the exact output of the computation.
+ *
+ * This version gives the same results as tainted but also includes
+ * data flow through global variables.
+ *
+ * The parameter `globalVar` is the qualified name of the last global variable
+ * used to move the value from source to tainted. If the taint did not pass
+ * through a global variable, then `globalVar = ""`.
+ */
 cached
 predicate taintedIncludingGlobalVars(Expr source, Element tainted, string globalVar) {
   tainted(source, tainted) and
@@ -373,11 +468,245 @@ predicate taintedIncludingGlobalVars(Expr source, Element tainted, string global
   )
 }
 
+/**
+ * Gets the global variable whose qualified name is `id`. Use this predicate
+ * together with `taintedIncludingGlobalVars`. Example:
+ *
+ * ```
+ * exists(string varName |
+ *   taintedIncludingGlobalVars(source, tainted, varName) and
+ *   var = globalVarFromId(varName)
+ * )
+ * ```
+ */
 GlobalOrNamespaceVariable globalVarFromId(string id) { id = result.getQualifiedName() }
 
+/**
+ * Resolve potential target function(s) for `call`.
+ *
+ * If `call` is a call through a function pointer (`ExprCall`) or
+ * targets a virtual method, simple data flow analysis is performed
+ * in order to identify target(s).
+ */
 Function resolveCall(Call call) {
   exists(CallInstruction callInstruction |
     callInstruction.getAST() = call and
     result = Dispatch::viableCallable(callInstruction)
   )
 }
+
+/**
+ * Provides definitions for augmenting source/sink pairs with data-flow paths
+ * between them. From a `@kind path-problem` query, import this module in the
+ * global scope, extend `TaintTrackingConfiguration`, and use `taintedWithPath`
+ * in place of `tainted`.
+ *
+ * Importing this module will also import the query predicates that contain the
+ * taint paths.
+ */
+module TaintedWithPath {
+  private newtype TSingleton = MkSingleton()
+
+  /**
+   * A taint-tracking configuration that matches sources and sinks in the same
+   * way as the `tainted` predicate.
+   *
+   * Override `isSink` and `taintThroughGlobals` as needed, but do not provide
+   * a characteristic predicate.
+   */
+  class TaintTrackingConfiguration extends TSingleton {
+    /** Override this to specify which elements are sinks in this configuration. */
+    abstract predicate isSink(Element e);
+
+    /**
+     * Override this predicate to `any()` to allow taint to flow through global
+     * variables.
+     */
+    predicate taintThroughGlobals() { none() }
+
+    /** Gets a textual representation of this element. */
+    string toString() { result = "TaintTrackingConfiguration" }
+  }
+
+  private class AdjustedConfiguration extends DataFlow3::Configuration {
+    AdjustedConfiguration() { this = "AdjustedConfiguration" }
+
+    override predicate isSource(DataFlow::Node source) { source = getNodeForSource(_) }
+
+    override predicate isSink(DataFlow::Node sink) {
+      exists(TaintTrackingConfiguration cfg | cfg.isSink(adjustedSink(sink)))
+    }
+
+    override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
+      instructionTaintStep(n1.asInstruction(), n2.asInstruction())
+      or
+      exists(TaintTrackingConfiguration cfg | cfg.taintThroughGlobals() |
+        writesVariable(n1.asInstruction(), n2.asVariable().(GlobalOrNamespaceVariable))
+        or
+        readsVariable(n2.asInstruction(), n1.asVariable().(GlobalOrNamespaceVariable))
+      )
+    }
+
+    override predicate isBarrier(DataFlow::Node node) { nodeIsBarrier(node) }
+
+    override predicate isBarrierIn(DataFlow::Node node) { nodeIsBarrierIn(node) }
+  }
+
+  /*
+   * A sink `Element` may map to multiple `DataFlowX::PathNode`s via (the
+   * inverse of) `adjustedSink`. For example, an `Expr` maps to all its
+   * conversions, and a `Variable` maps to all loads and stores from it. Because
+   * the path node is part of the tuple that constitutes the alert, this leads
+   * to duplicate alerts.
+   *
+   * To avoid showing duplicates, we edit the graph to replace the final node
+   * coming from the data-flow library with a node that matches exactly the
+   * `Element` sink that's requested.
+   *
+   * The same is done for sources.
+   */
+
+  private newtype TPathNode =
+    TWrapPathNode(DataFlow3::PathNode n) or
+    // There's a single newtype constructor for both sources and sinks since
+    // that makes it easiest to deal with the case where source = sink.
+    TEndpointPathNode(Element e) {
+      exists(AdjustedConfiguration cfg, DataFlow3::Node sourceNode, DataFlow3::Node sinkNode |
+        cfg.hasFlow(sourceNode, sinkNode)
+      |
+        sourceNode = getNodeForSource(e)
+        or
+        e = adjustedSink(sinkNode) and
+        exists(TaintTrackingConfiguration ttCfg | ttCfg.isSink(e))
+      )
+    }
+
+  /** An opaque type used for the nodes of a data-flow path. */
+  class PathNode extends TPathNode {
+    /** Gets a textual representation of this element. */
+    string toString() { none() }
+
+    /**
+     * Holds if this element is at the specified location.
+     * The location spans column `startcolumn` of line `startline` to
+     * column `endcolumn` of line `endline` in file `filepath`.
+     * For more information, see
+     * [Locations](https://help.semmle.com/QL/learn-ql/ql/locations.html).
+     */
+    predicate hasLocationInfo(
+      string filepath, int startline, int startcolumn, int endline, int endcolumn
+    ) {
+      none()
+    }
+  }
+
+  private class WrapPathNode extends PathNode, TWrapPathNode {
+    DataFlow3::PathNode inner() { this = TWrapPathNode(result) }
+
+    override string toString() { result = this.inner().toString() }
+
+    override predicate hasLocationInfo(
+      string filepath, int startline, int startcolumn, int endline, int endcolumn
+    ) {
+      this.inner().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    }
+  }
+
+  private class EndpointPathNode extends PathNode, TEndpointPathNode {
+    Expr inner() { this = TEndpointPathNode(result) }
+
+    override string toString() { result = this.inner().toString() }
+
+    override predicate hasLocationInfo(
+      string filepath, int startline, int startcolumn, int endline, int endcolumn
+    ) {
+      this
+          .inner()
+          .getLocation()
+          .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    }
+  }
+
+  /** A PathNode whose `Element` is a source. It may also be a sink. */
+  private class InitialPathNode extends EndpointPathNode {
+    InitialPathNode() { exists(getNodeForSource(this.inner())) }
+  }
+
+  /** A PathNode whose `Element` is a sink. It may also be a source. */
+  private class FinalPathNode extends EndpointPathNode {
+    FinalPathNode() { exists(TaintTrackingConfiguration cfg | cfg.isSink(this.inner())) }
+  }
+
+  /** Holds if `(a,b)` is an edge in the graph of data flow path explanations. */
+  query predicate edges(PathNode a, PathNode b) {
+    DataFlow3::PathGraph::edges(a.(WrapPathNode).inner(), b.(WrapPathNode).inner())
+    or
+    // To avoid showing trivial-looking steps, we _replace_ the last node instead
+    // of adding an edge out of it.
+    exists(WrapPathNode sinkNode |
+      DataFlow3::PathGraph::edges(a.(WrapPathNode).inner(), sinkNode.inner()) and
+      b.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
+    )
+    or
+    // Same for the first node
+    exists(WrapPathNode sourceNode |
+      DataFlow3::PathGraph::edges(sourceNode.inner(), b.(WrapPathNode).inner()) and
+      sourceNode.inner().getNode() = getNodeForSource(a.(InitialPathNode).inner())
+    )
+    or
+    // Finally, handle the case where the path goes directly from a source to a
+    // sink, meaning that they both need to be translated.
+    exists(WrapPathNode sinkNode, WrapPathNode sourceNode |
+      DataFlow3::PathGraph::edges(sourceNode.inner(), sinkNode.inner()) and
+      sourceNode.inner().getNode() = getNodeForSource(a.(InitialPathNode).inner()) and
+      b.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
+    )
+  }
+
+  /** Holds if `n` is a node in the graph of data flow path explanations. */
+  query predicate nodes(PathNode n, string key, string val) {
+    key = "semmle.label" and val = n.toString()
+  }
+
+  /**
+   * Holds if `tainted` may contain taint from `source`, where `sourceNode` and
+   * `sinkNode` are the corresponding `PathNode`s that can be used in a query
+   * to provide path explanations. Extend `TaintTrackingConfiguration` to use
+   * this predicate.
+   *
+   * A tainted expression is either directly user input, or is computed from
+   * user input in a way that users can probably control the exact output of
+   * the computation.
+   */
+  predicate taintedWithPath(Expr source, Element tainted, PathNode sourceNode, PathNode sinkNode) {
+    exists(AdjustedConfiguration cfg, DataFlow3::Node flowSource, DataFlow3::Node flowSink |
+      source = sourceNode.(InitialPathNode).inner() and
+      flowSource = getNodeForSource(source) and
+      cfg.hasFlow(flowSource, flowSink) and
+      tainted = adjustedSink(flowSink) and
+      tainted = sinkNode.(FinalPathNode).inner()
+    )
+  }
+
+  private predicate isGlobalVariablePathNode(WrapPathNode n) {
+    n.inner().getNode().asVariable() instanceof GlobalOrNamespaceVariable
+  }
+
+  private predicate edgesWithoutGlobals(PathNode a, PathNode b) {
+    edges(a, b) and
+    not isGlobalVariablePathNode(a) and
+    not isGlobalVariablePathNode(b)
+  }
+
+  /**
+   * Holds if `tainted` can be reached from a taint source without passing
+   * through a global variable.
+   */
+  predicate taintedWithoutGlobals(Element tainted) {
+    exists(PathNode sourceNode, FinalPathNode sinkNode |
+      sourceNode.(WrapPathNode).inner().getNode() = getNodeForSource(_) and
+      edgesWithoutGlobals+(sourceNode, sinkNode) and
+      tainted = sinkNode.inner()
+    )
+  }
+}