Merge branch 'master' into strcpy-fixups

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

@@ -8,29 +8,38 @@
  * @tags maintainability
  *       readability
  */
+
 import cpp
 
-
-/* Names of parameters in the implementation of a function.
-   Notice that we need to exclude parameter names used in prototype
-   declarations and only include the ones from the actual definition.
-   We also exclude names from functions that have multiple definitions.
-   This should not happen in a single application but since we
-   have a system wide view it is likely to happen for instance for
-   the main function. */
+/**
+ * Gets the parameter of `f` with name `name`, which has to come from the
+ * _definition_ of `f` and not a prototype declaration.
+ * We also exclude names from functions that have multiple definitions.
+ * This should not happen in a single application but since we
+ * have a system wide view it is likely to happen for instance for
+ * the main function.
+ */
 ParameterDeclarationEntry functionParameterNames(Function f, string name) {
   exists(FunctionDeclarationEntry fe |
-    result.getFunctionDeclarationEntry() = fe
-    and fe.getFunction() = f
-    and fe.getLocation() = f.getDefinitionLocation()
-    and strictcount(f.getDefinitionLocation()) = 1
-    and result.getName() = name
+    result.getFunctionDeclarationEntry() = fe and
+    fe.getFunction() = f and
+    fe.getLocation() = f.getDefinitionLocation() and
+    result.getFile() = fe.getFile() and // Work around CPP-331
+    strictcount(f.getDefinitionLocation()) = 1 and
+    result.getName() = name
   )
 }
 
-from Function f, LocalVariable lv, ParameterDeclarationEntry pde
-where f = lv.getFunction() and
-      pde = functionParameterNames(f, lv.getName()) and
-      not lv.isInMacroExpansion()
-select lv, "Local variable '"+ lv.getName() +"' hides a $@.",
-       pde, "parameter of the same name"
+/** Gets a local variable in `f` with name `name`. */
+pragma[nomagic]
+LocalVariable localVariableNames(Function f, string name) {
+  name = result.getName() and
+  f = result.getFunction()
+}
+
+from Function f, LocalVariable lv, ParameterDeclarationEntry pde, string name
+where
+  lv = localVariableNames(f, name) and
+  pde = functionParameterNames(f, name) and
+  not lv.isInMacroExpansion()
+select lv, "Local variable '" + lv.getName() + "' hides a $@.", pde, "parameter of the same name"

cpp/ql/src/Critical/DeadCodeCondition.ql

@@ -7,51 +7,64 @@
  * @tags reliability
  *       external/cwe/cwe-561
  */
+
 import cpp
 
-predicate testAndBranch(Expr e, Stmt branch)
-{
-  exists(IfStmt ifstmt | ifstmt.getCondition() = e and
-    (ifstmt.getThen() = branch or ifstmt.getElse() = branch))
+predicate testAndBranch(Expr e, Stmt branch) {
+  exists(IfStmt ifstmt |
+    ifstmt.getCondition() = e and
+    (ifstmt.getThen() = branch or ifstmt.getElse() = branch)
+  )
   or
-  exists(WhileStmt while | while.getCondition() = e and
-    while.getStmt() = branch)
+  exists(WhileStmt while |
+    while.getCondition() = e and
+    while.getStmt() = branch
+  )
 }
 
-predicate choice(LocalScopeVariable v, Stmt branch, string value)
-{
+predicate choice(LocalScopeVariable v, Stmt branch, string value) {
   exists(AnalysedExpr e |
     testAndBranch(e, branch) and
     (
       (e.getNullSuccessor(v) = branch and value = "null")
       or
       (e.getNonNullSuccessor(v) = branch and value = "non-null")
-    ))
+    )
+  )
 }
 
-
-predicate guarded(LocalScopeVariable v, Stmt loopstart, AnalysedExpr child)
-{
+predicate guarded(LocalScopeVariable v, Stmt loopstart, AnalysedExpr child) {
   choice(v, loopstart, _) and
   loopstart.getChildStmt*() = child.getEnclosingStmt() and
   (definition(v, child) or exists(child.getNullSuccessor(v)))
 }
 
-predicate addressLeak(Variable v, Stmt leak)
-{
+predicate addressLeak(Variable v, Stmt leak) {
   exists(VariableAccess access |
     v.getAnAccess() = access and
     access.getEnclosingStmt() = leak and
-    access.isAddressOfAccess())
+    access.isAddressOfAccess()
+  )
 }
 
-from LocalScopeVariable v, Stmt branch, AnalysedExpr cond, string context, string test, string testresult
-where choice(v, branch, context)
-  and forall(ControlFlowNode def | definition(v, def) and definitionReaches(def, cond) | not guarded(v, branch, def))
-  and not cond.isDef(v)
-  and guarded(v, branch, cond)
-  and exists(cond.getNullSuccessor(v))
-  and not addressLeak(v, branch.getChildStmt*())
-  and ((cond.isNullCheck(v) and test = "null") or (cond.isValidCheck(v) and test = "non-null"))
-  and (if context = test then testresult = "succeed" else testresult = "fail")
-select cond, "Variable '" + v.getName() + "' is always " + context + " here, this check will always " + testresult + "."
+from
+  LocalScopeVariable v, Stmt branch, AnalysedExpr cond, string context, string test,
+  string testresult
+where
+  choice(v, branch, context) and
+  forall(ControlFlowNode def | definition(v, def) and definitionReaches(def, cond) |
+    not guarded(v, branch, def)
+  ) and
+  not cond.isDef(v) and
+  guarded(v, branch, cond) and
+  exists(cond.getNullSuccessor(v)) and
+  not addressLeak(v, branch.getChildStmt*()) and
+  (
+    (cond.isNullCheck(v) and test = "null")
+    or
+    (cond.isValidCheck(v) and test = "non-null")
+  ) and
+  (if context = test then testresult = "succeed" else testresult = "fail")
+select cond,
+  "Variable '" + v.getName() + "' is always " + context + " here, this check will always " +
+    testresult + "."

cpp/ql/src/Critical/LargeParameter.ql

@@ -18,6 +18,7 @@ where f.getAParameter() = p
   and t.getSize() = size
   and size > 64
   and not t.getUnderlyingType() instanceof ArrayType
+  and not f instanceof CopyAssignmentOperator
 select
   p, "This parameter of type $@ is " + size.toString() + " bytes - consider passing a pointer/reference instead.",
   t, t.toString()

cpp/ql/src/Critical/NotInitialised.ql

@@ -7,34 +7,46 @@
  * @tags reliability
  *       external/cwe/cwe-457
  */
+
 import cpp
 
 // See also InitialisationNotRun.ql and GlobalUseBeforeInit.ql
 
-// Holds if s defines variable v (conservative)
+/**
+ * Holds if `s` defines variable `v` (conservative).
+ */
 predicate defines(ControlFlowNode s, Variable lv) {
   exists(VariableAccess va | va = s and va.getTarget() = lv and va.isUsedAsLValue())
 }
 
-// Holds if s uses variable v (conservative)
+/**
+ * Holds if `s` uses variable `v` (conservative).
+ */
 predicate uses(ControlFlowNode s, Variable lv) {
-  exists(VariableAccess va | va = s and va.getTarget() = lv and va.isRValue()
-    and not va.getParent+() instanceof SizeofOperator)
+  exists(VariableAccess va |
+    va = s and
+    va.getTarget() = lv and
+    va.isRValue() and
+    not va.getParent+() instanceof SizeofOperator
+  )
 }
 
-// Holds if there is a path from the declaration of lv to n such that lv is
-// definitely not defined before n
+/**
+ * Holds if there is a path from the declaration of `lv` to `n` such that `lv` is
+ * definitely not defined before `n`.
+ */
 predicate noDefPath(LocalVariable lv, ControlFlowNode n) {
-     n.(DeclStmt).getADeclaration() = lv and not exists(lv.getInitializer())
-  or exists(ControlFlowNode p | noDefPath(lv, p) and n = p.getASuccessor() and not defines(p, lv))
+  n.(DeclStmt).getADeclaration() = lv and not exists(lv.getInitializer())
+  or
+  exists(ControlFlowNode p | noDefPath(lv, p) and n = p.getASuccessor() and not defines(p, lv))
 }
 
-predicate isAggregateType(Type t) {
-  t instanceof Class or t instanceof ArrayType
-}
+predicate isAggregateType(Type t) { t instanceof Class or t instanceof ArrayType }
 
-// Holds if va is a use of a local variable that has not been previously
-// defined
+/**
+ * Holds if `va` is a use of a local variable that has not been previously
+ * defined.
+ */
 predicate undefinedLocalUse(VariableAccess va) {
   exists(LocalVariable lv |
     // it is hard to tell when a struct or array has been initialized, so we
@@ -43,17 +55,21 @@ predicate undefinedLocalUse(VariableAccess va) {
     not lv.getType().hasName("va_list") and
     va = lv.getAnAccess() and
     noDefPath(lv, va) and
-    uses(va, lv))
+    uses(va, lv)
+  )
 }
 
-// Holds if gv is a potentially uninitialized global variable
+/**
+ * Holds if `gv` is a potentially uninitialized global variable.
+ */
 predicate uninitialisedGlobal(GlobalVariable gv) {
   exists(VariableAccess va |
     not isAggregateType(gv.getUnderlyingType()) and
     va = gv.getAnAccess() and
     va.isRValue() and
     not gv.hasInitializer() and
-    not gv.hasSpecifier("extern"))
+    not gv.hasSpecifier("extern")
+  )
 }
 
 from Element elt

cpp/ql/src/Critical/SizeCheck.ql

@@ -11,56 +11,61 @@
  *       external/cwe/cwe-131
  *       external/cwe/cwe-122
  */
+
 import cpp
 
-class Allocation extends FunctionCall
-{
+class Allocation extends FunctionCall {
   Allocation() {
     exists(string name |
       this.getTarget().hasQualifiedName(name) and
-      (name = "malloc" or name = "calloc" or name = "realloc"))
+      (name = "malloc" or name = "calloc" or name = "realloc")
+    )
   }
 
   string getName() { result = this.getTarget().getQualifiedName() }
 
   int getSize() {
-      (this.getName() = "malloc" and
-      this.getArgument(0).getValue().toInt() = result)
+    (
+      this.getName() = "malloc" and
+      this.getArgument(0).getValue().toInt() = result
+    )
     or
-      (this.getName() = "realloc" and
-      this.getArgument(1).getValue().toInt() = result)
+    (
+      this.getName() = "realloc" and
+      this.getArgument(1).getValue().toInt() = result
+    )
     or
-      (this.getName() = "calloc" and
-      result =
-        this.getArgument(0).getValue().toInt() *
-        this.getArgument(1).getValue().toInt())
+    (
+      this.getName() = "calloc" and
+      result = this.getArgument(0).getValue().toInt() * this.getArgument(1).getValue().toInt()
+    )
   }
 }
 
-predicate baseType(Allocation alloc, Type base)
-{
+predicate baseType(Allocation alloc, Type base) {
   exists(PointerType pointer |
     pointer.getBaseType() = base and
     (
       exists(AssignExpr assign |
-        assign.getRValue() = alloc and assign.getLValue().getType() = pointer)
+        assign.getRValue() = alloc and assign.getLValue().getType() = pointer
+      )
       or
-      exists(Variable v |
-        v.getInitializer().getExpr() = alloc and v.getType() = pointer)
+      exists(Variable v | v.getInitializer().getExpr() = alloc and v.getType() = pointer)
     )
   )
 }
 
-predicate decideOnSize(Type t, int size)
-{
+predicate decideOnSize(Type t, int size) {
   // If the codebase has more than one type with the same name, it can have more than one size.
   size = min(t.getSize())
 }
 
 from Allocation alloc, Type base, int basesize, int allocated
-where baseType(alloc, base)
-  and allocated = alloc.getSize()
-  and decideOnSize(base, basesize)
-  and basesize > allocated
-select alloc, "Type '" + base.getName() + "' is " + basesize.toString() +
-  " bytes, but only " + allocated.toString() + " bytes are allocated."
+where
+  baseType(alloc, base) and
+  allocated = alloc.getSize() and
+  decideOnSize(base, basesize) and
+  basesize > allocated
+select alloc,
+  "Type '" + base.getName() + "' is " + basesize.toString() + " bytes, but only " +
+    allocated.toString() + " bytes are allocated."

cpp/ql/src/Critical/SizeCheck2.ql

@@ -11,54 +11,60 @@
  *       external/cwe/cwe-131
  *       external/cwe/cwe-122
  */
+
 import cpp
 
-class Allocation extends FunctionCall
-{
+class Allocation extends FunctionCall {
   Allocation() {
     exists(string name |
       this.getTarget().hasQualifiedName(name) and
-      (name = "malloc" or name = "calloc" or name = "realloc"))
+      (name = "malloc" or name = "calloc" or name = "realloc")
+    )
   }
 
   string getName() { result = this.getTarget().getQualifiedName() }
 
   int getSize() {
-      (this.getName() = "malloc" and
-      this.getArgument(0).getValue().toInt() = result)
+    (
+      this.getName() = "malloc" and
+      this.getArgument(0).getValue().toInt() = result
+    )
     or
-      (this.getName() = "realloc" and
-      this.getArgument(1).getValue().toInt() = result)
+    (
+      this.getName() = "realloc" and
+      this.getArgument(1).getValue().toInt() = result
+    )
     or
-      (this.getName() = "calloc" and
-      result =
-        this.getArgument(0).getValue().toInt() *
-        this.getArgument(1).getValue().toInt())
+    (
+      this.getName() = "calloc" and
+      result = this.getArgument(0).getValue().toInt() * this.getArgument(1).getValue().toInt()
+    )
   }
 }
 
-predicate baseType(Allocation alloc, Type base)
-{
+predicate baseType(Allocation alloc, Type base) {
   exists(PointerType pointer |
     pointer.getBaseType() = base and
     (
       exists(AssignExpr assign |
-        assign.getRValue() = alloc and assign.getLValue().getType() = pointer)
+        assign.getRValue() = alloc and assign.getLValue().getType() = pointer
+      )
       or
-      exists(Variable v |
-        v.getInitializer().getExpr() = alloc and v.getType() = pointer)
+      exists(Variable v | v.getInitializer().getExpr() = alloc and v.getType() = pointer)
     )
   )
 }
 
 from Allocation alloc, Type base, int basesize, int allocated
-where baseType(alloc, base)
-  and allocated = alloc.getSize()
+where
+  baseType(alloc, base) and
+  allocated = alloc.getSize() and
   // If the codebase has more than one type with the same name, check if any matches
-  and not exists(int size | base.getSize() = size |
-    size = 0
-    or (allocated / size) * size = allocated)
-  and basesize = min(base.getSize())
-select alloc, "Allocated memory (" + allocated.toString() +
-  " bytes) is not a multiple of the size of '" +
-  base.getName() + "' (" + basesize.toString() + " bytes)."
+  not exists(int size | base.getSize() = size |
+    size = 0 or
+    (allocated / size) * size = allocated
+  ) and
+  basesize = min(base.getSize())
+select alloc,
+  "Allocated memory (" + allocated.toString() + " bytes) is not a multiple of the size of '" +
+    base.getName() + "' (" + basesize.toString() + " bytes)."

cpp/ql/src/Documentation/CommentedOutCode.qll

@@ -20,6 +20,18 @@ private predicate looksLikeCode(string line) {
     exists(string trimmed |
            trimmed = line.regexpReplaceAll("(?i)(^\\s+|&#?[a-z0-9]{1,31};|\\s+$)", "") |
            trimmed.regexpMatch(".*[{};]")
+           and (
+              // If this line looks like code because it ends with a closing
+              // brace that's preceded by something other than whitespace ...
+              trimmed.regexpMatch(".*.\\}")
+              implies
+              // ... then there has to be ") {" (or some variation)
+              // on the line, suggesting it's a statement like `if`
+              // or a function declaration. Otherwise it's likely to be a
+              // benign use of braces such as a JSON example or explanatory
+              // pseudocode.
+              trimmed.regexpMatch(".*(\\)|const|volatile|override|final|noexcept|&)\\s*\\{.*")
+           )
            and not trimmed.regexpMatch("(>.*|.*[\\\\@][{}].*|(optional|repeated) .*;|.*(\\{\\{\\{|\\}\\}\\}).*|\\{[-0-9a-zA-Z]+\\})"))
 }
 

cpp/ql/src/Header Cleanup/Cleanup-DuplicateIncludeGuard.ql

@@ -9,6 +9,7 @@
  *       maintainability
  *       modularity
  */
+
 import cpp
 import semmle.code.cpp.headers.MultipleInclusion
 
@@ -20,9 +21,15 @@ import semmle.code.cpp.headers.MultipleInclusion
  * However one case must be a correctIncludeGuard to prove that this macro really is intended
  * to be an include guard.
  */
+
 from HeaderFile hf, PreprocessorDirective ifndef, string macroName, int num
-where hasIncludeGuard(hf, ifndef, _, macroName)
-and exists(HeaderFile other | hasIncludeGuard(other, _, _, macroName) and hf.getShortName() != other.getShortName())
-and num = strictcount(HeaderFile other | hasIncludeGuard(other, _, _, macroName))
-and correctIncludeGuard(_, _, _, _, macroName)
-select ifndef, "The macro name '" + macroName + "' of this include guard is used in " + num + " different header files."
+where
+  hasIncludeGuard(hf, ifndef, _, macroName) and
+  exists(HeaderFile other |
+    hasIncludeGuard(other, _, _, macroName) and hf.getShortName() != other.getShortName()
+  ) and
+  num = strictcount(HeaderFile other | hasIncludeGuard(other, _, _, macroName)) and
+  correctIncludeGuard(_, _, _, _, macroName)
+select ifndef,
+  "The macro name '" + macroName + "' of this include guard is used in " + num +
+    " different header files."

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

@@ -0,0 +1,7 @@
+double getWidth();
+
+void f() {
+	int width = getWidth();
+	
+	// ...
+}

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

@@ -0,0 +1,28 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>This rule finds function calls whose result type is a floating point type, which are implicitly cast to an integral type. Such code may not behave as intended when the floating point return value has a fractional part, or takes an extreme value outside the range that can be represented by the integer type.</p>
+
+</overview>
+<recommendation>
+<p>Consider changing the surrounding expression to match the floating point type. If rounding is intended, explicitly round using a standard function such as `trunc`, `floor` or `round`.</p>
+
+</recommendation>
+<example><sample src="LossyFunctionResultCast.cpp" />
+<p>In this example, the result of the call to <code>getWidth()</code> is implicitly cast to <code>int</code>, resulting in an unintended loss of accuracy. To fix this, the type of variable <code>width</code> could be changed from <code>int</code> to <code>double</code>.</p>
+
+</example>
+<references>
+
+<li>
+  Microsoft Visual C++ Documentation: <a href="https://docs.microsoft.com/en-us/cpp/cpp/type-conversions-and-type-safety-modern-cpp?view=vs-2017">Type Conversions and Type Safety (Modern C++)</a>.
+</li>
+<li>
+  Cplusplus.com: <a href="http://www.cplusplus.com/doc/tutorial/typecasting/">Type conversions</a>.
+</li>
+
+</references>
+</qhelp>

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

@@ -1,18 +1,68 @@
 /**
  * @name Lossy function result cast
  * @description Finds function calls whose result type is a floating point type, and which are casted to an integral type.
- *              Includes only expressions with implicit cast and excludes function calls to ceil, floor and round. {This is a gcc check; doesn't seem wildly useful.}
+ *              Includes only expressions with implicit cast and excludes function calls to ceil, floor and round.
  * @kind problem
  * @id cpp/lossy-function-result-cast
  * @problem.severity warning
+ * @precision medium
+ * @tags correctness
  */
+
 import cpp
+import semmle.code.cpp.dataflow.DataFlow
+
+predicate whitelist(Function f) {
+  exists(string fName |
+    fName = f.getName() and
+    (
+      fName = "ceil" or
+      fName = "ceilf" or
+      fName = "ceill" or
+      fName = "floor" or
+      fName = "floorf" or
+      fName = "floorl" or
+      fName = "nearbyint" or
+      fName = "nearbyintf" or
+      fName = "nearbyintl" or
+      fName = "rint" or
+      fName = "rintf" or
+      fName = "rintl" or
+      fName = "round" or
+      fName = "roundf" or
+      fName = "roundl" or
+      fName = "trunc" or
+      fName = "truncf" or
+      fName = "truncl" or
+      fName.matches("__builtin_%")
+    )
+  )
+}
+
+predicate whitelistPow(FunctionCall fc) {
+  (
+    fc.getTarget().getName() = "pow" or
+    fc.getTarget().getName() = "powf" or
+    fc.getTarget().getName() = "powl"
+  ) and exists(float value |
+    value = fc.getArgument(0).getValue().toFloat() and
+    (value.floor() - value).abs() < 0.001
+  )
+}
+
+predicate whiteListWrapped(FunctionCall fc) {
+  whitelist(fc.getTarget()) or
+  whitelistPow(fc) or
+  exists(Expr e, ReturnStmt rs |
+    whiteListWrapped(e) and
+    DataFlow::localFlow(DataFlow::exprNode(e), DataFlow::exprNode(rs.getExpr())) and
+    fc.getTarget() = rs.getEnclosingFunction()
+  )
+}
 
 from FunctionCall c, FloatingPointType t1, IntegralType t2
 where t1 = c.getTarget().getType().getUnderlyingType() and
       t2 = c.getActualType() and
       c.hasImplicitConversion() and
-      not c.getTarget().getName() = "ceil" and
-      not c.getTarget().getName() = "floor" and
-      not c.getTarget().getName() = "round"
-select c
+      not whiteListWrapped(c)
+select c, "Return value of type " + t1.toString() + " is implicitly converted to " + t2.toString() + " here."

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

@@ -1,7 +1,6 @@
 /**
  * @name Potentially overflowing call to snprintf
  * @description Using the return value from snprintf without proper checks can cause overflow.
- *
  * @kind problem
  * @problem.severity warning
  * @precision high
@@ -20,44 +19,44 @@ import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
  * true if there is an arithmetic operation on the path that
  * might overflow.
  */
-predicate flowsToExpr(
-  Expr source, Expr sink, boolean pathMightOverflow) {
+predicate flowsToExpr(Expr source, Expr sink, boolean pathMightOverflow) {
   // Might the current expression overflow?
-  exists (boolean otherMightOverflow
-  | flowsToExprImpl(source, sink, otherMightOverflow)
-  | if convertedExprMightOverflow(sink)
-      then pathMightOverflow = true
-      else pathMightOverflow = otherMightOverflow)
+  exists(boolean otherMightOverflow | flowsToExprImpl(source, sink, otherMightOverflow) |
+    if convertedExprMightOverflow(sink)
+    then pathMightOverflow = true
+    else pathMightOverflow = otherMightOverflow
+  )
 }
 
 /**
  * Implementation of `flowsToExpr`. Does everything except
  * checking whether the current expression might overflow.
  */
-predicate flowsToExprImpl(
-  Expr source, Expr sink, boolean pathMightOverflow) {
-  (source = sink and pathMightOverflow = false and
-   source.(FunctionCall).getTarget().(Snprintf).returnsFullFormatLength())
+predicate flowsToExprImpl(Expr source, Expr sink, boolean pathMightOverflow) {
+  (
+    source = sink and
+    pathMightOverflow = false and
+    source.(FunctionCall).getTarget().(Snprintf).returnsFullFormatLength()
+  )
   or
-  exists (RangeSsaDefinition def, LocalScopeVariable v
-  | flowsToDef(source, def, v, pathMightOverflow) and
-    sink = def.getAUse(v))
+  exists(RangeSsaDefinition def, LocalScopeVariable v |
+    flowsToDef(source, def, v, pathMightOverflow) and
+    sink = def.getAUse(v)
+  )
   or
-  flowsToExpr(
-    source, sink.(UnaryArithmeticOperation).getOperand(), pathMightOverflow)
+  flowsToExpr(source, sink.(UnaryArithmeticOperation).getOperand(), pathMightOverflow)
   or
-  flowsToExpr(
-    source, sink.(BinaryArithmeticOperation).getAnOperand(), pathMightOverflow)
+  flowsToExpr(source, sink.(BinaryArithmeticOperation).getAnOperand(), pathMightOverflow)
   or
-  flowsToExpr(
-    source, sink.(Assignment).getRValue(), pathMightOverflow)
+  flowsToExpr(source, sink.(Assignment).getRValue(), pathMightOverflow)
   or
-  flowsToExpr(
-    source, sink.(AssignOperation).getLValue(), pathMightOverflow)
+  flowsToExpr(source, sink.(AssignOperation).getLValue(), pathMightOverflow)
   or
-  exists (FormattingFunctionCall call
-  | sink = call and
-    flowsToExpr(source, call.getArgument(call.getTarget().getSizeParameterIndex()), pathMightOverflow))
+  exists(FormattingFunctionCall call |
+    sink = call and
+    flowsToExpr(source, call.getArgument(call.getTarget().getSizeParameterIndex()),
+      pathMightOverflow)
+  )
 }
 
 /**
@@ -67,14 +66,14 @@ predicate flowsToExprImpl(
  * on the path that might overflow.
  */
 predicate flowsToDef(
-  Expr source, RangeSsaDefinition def, LocalScopeVariable v,
-  boolean pathMightOverflow) {
+  Expr source, RangeSsaDefinition def, LocalScopeVariable v, boolean pathMightOverflow
+) {
   // Might the current definition overflow?
-  exists (boolean otherMightOverflow
-  | flowsToDefImpl(source, def, v, otherMightOverflow)
-  | if defMightOverflow(def, v)
-      then pathMightOverflow = true
-      else pathMightOverflow = otherMightOverflow)
+  exists(boolean otherMightOverflow | flowsToDefImpl(source, def, v, otherMightOverflow) |
+    if defMightOverflow(def, v)
+    then pathMightOverflow = true
+    else pathMightOverflow = otherMightOverflow
+  )
 }
 
 /**
@@ -89,26 +88,29 @@ predicate flowsToDef(
  * the path. But it is a good way to reduce the number of false positives.
  */
 predicate flowsToDefImpl(
-  Expr source, RangeSsaDefinition def, LocalScopeVariable v,
-  boolean pathMightOverflow) {
+  Expr source, RangeSsaDefinition def, LocalScopeVariable v, boolean pathMightOverflow
+) {
   // Assignment or initialization: `e = v;`
-  exists (Expr e
-  | e = def.getDefiningValue(v) and
-    flowsToExpr(source, e, pathMightOverflow))
+  exists(Expr e |
+    e = def.getDefiningValue(v) and
+    flowsToExpr(source, e, pathMightOverflow)
+  )
   or
   // `x++`
-  exists (CrementOperation crem
-  | def = crem and
+  exists(CrementOperation crem |
+    def = crem and
     crem.getOperand() = v.getAnAccess() and
-    flowsToExpr(source, crem.getOperand(), pathMightOverflow))
+    flowsToExpr(source, crem.getOperand(), pathMightOverflow)
+  )
   or
   // Phi definition.
   flowsToDef(source, def.getAPhiInput(v), v, pathMightOverflow)
 }
 
 from FormattingFunctionCall call, Expr sink
-where flowsToExpr(call, sink, true)
-and sink = call.getArgument(call.getTarget().getSizeParameterIndex())
-select
-  call, "The $@ of this snprintf call is derived from its return value, which may exceed the size of the buffer and overflow.",
+where
+  flowsToExpr(call, sink, true) and
+  sink = call.getArgument(call.getTarget().getSizeParameterIndex())
+select call,
+  "The $@ of this snprintf call is derived from its return value, which may exceed the size of the buffer and overflow.",
   sink, "size argument"

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

@@ -91,12 +91,27 @@ predicate definedInIfDef(Function f) {
   )
 }
 
+/**
+ * Holds if `call` has the form `B::f()` or `q.B::f()`, where `B` is a base
+ * class of the class containing `call`.
+ *
+ * This is most often used for calling base-class functions from within
+ * overrides. Those functions may have no side effect in the current
+ * implementation, but we should not advise callers to rely on this. That would
+ * break encapsulation.
+ */
+predicate baseCall(FunctionCall call) {
+  call.getNameQualifier().getQualifyingElement() =
+    call.getEnclosingFunction().getDeclaringType().(Class).getABaseClass+()
+}
+
 from PureExprInVoidContext peivc, Locatable parent,
   Locatable info, string info_text, string tail
 where // EQExprs are covered by CompareWhereAssignMeant.ql
       not peivc instanceof EQExpr and
       // as is operator==
       not peivc.(FunctionCall).getTarget().hasName("operator==") and
+      not baseCall(peivc) and
       not accessInInitOfForStmt(peivc) and
       not peivc.isCompilerGenerated() and
       not exists(Macro m | peivc = m.getAnInvocation().getAnExpandedElement()) and

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

@@ -8,6 +8,7 @@
  * @problem.severity warning
  * @tags maintainability
  */
+
 import cpp
 
 /**
@@ -15,15 +16,15 @@ import cpp
  * or template argument).
  */
 predicate simple(Literal l) {
-    l instanceof OctalLiteral or
-    l instanceof HexLiteral or
-    l instanceof CharLiteral or
-    l.getValueText() = "true" or
-    l.getValueText() = "false" or
-    // Parsing doubles is too slow...
-    //exists(l.getValueText().toFloat())
-    // Instead, check whether the literal starts with a letter.
-    not l.getValueText().regexpMatch("[a-zA-Z_].*")
+  l instanceof OctalLiteral or
+  l instanceof HexLiteral or
+  l instanceof CharLiteral or
+  l.getValueText() = "true" or
+  l.getValueText() = "false" or
+  // Parsing doubles is too slow...
+  //exists(l.getValueText().toFloat())
+  // Instead, check whether the literal starts with a letter.
+  not l.getValueText().regexpMatch("[a-zA-Z_].*")
 }
 
 predicate booleanLiteral(Literal l) {
@@ -32,18 +33,23 @@ predicate booleanLiteral(Literal l) {
 }
 
 string boolLiteralInLogicalOp(Literal literal) {
-   booleanLiteral(literal) and literal.getParent() instanceof BinaryLogicalOperation and
-  result = "Literal value " + literal.getValueText() + " is used in a logical expression; simplify or use a constant."
+  booleanLiteral(literal) and
+  literal.getParent() instanceof BinaryLogicalOperation and
+  result = "Literal value " + literal.getValueText() +
+      " is used in a logical expression; simplify or use a constant."
 }
 
 string comparisonOnLiterals(ComparisonOperation op) {
-  simple(op.getLeftOperand()) and simple(op.getRightOperand()) and
+  simple(op.getLeftOperand()) and
+  simple(op.getRightOperand()) and
   not op.getAnOperand().isInMacroExpansion() and
-  if op.isConstant() then result = "This comparison involves two literals and is always " + op.getValue() + "."
+  if op.isConstant()
+  then result = "This comparison involves two literals and is always " + op.getValue() + "."
   else result = "This comparison involves two literals and should be simplified."
 }
 
 from Expr e, string msg
-where (msg = boolLiteralInLogicalOp(e) or msg = comparisonOnLiterals(e)) and
-    not e.isInMacroExpansion()
+where
+  (msg = boolLiteralInLogicalOp(e) or msg = comparisonOnLiterals(e)) and
+  not e.isInMacroExpansion()
 select e, msg

cpp/ql/src/Power of 10/Rule 1/UseOfJmp.ql

@@ -11,8 +11,11 @@ import cpp
 class ForbiddenFunction extends Function {
   ForbiddenFunction() {
     exists(string name | name = this.getName() |
-        name = "setjmp" or name = "longjmp" or
-        name = "sigsetjmp" or name = "siglongjmp")
+      name = "setjmp" or
+      name = "longjmp" or
+      name = "sigsetjmp" or
+      name = "siglongjmp"
+    )
   }
 }
 

cpp/ql/src/Power of 10/Rule 2/ExitPermanentLoop.ql

@@ -10,13 +10,15 @@ import cpp
 
 Stmt exitFrom(Loop l) {
   l.getAChild+() = result and
-  (result instanceof ReturnStmt or
-    exists(BreakStmt break | break = result |
-      not l.getAChild*() = break.getTarget())
+  (
+    result instanceof ReturnStmt
+    or
+    exists(BreakStmt break | break = result | not l.getAChild*() = break.getTarget())
   )
 }
 
 from Loop l, Stmt exit
-where l.getControllingExpr().getValue().toInt() != 0 and
-      exit = exitFrom(l)
+where
+  l.getControllingExpr().getValue().toInt() != 0 and
+  exit = exitFrom(l)
 select exit, "$@ should not be exited.", l, "This permanent loop"

cpp/ql/src/Power of 10/Rule 7/CheckArguments.ql

@@ -9,16 +9,17 @@
 import cpp
 
 predicate flow(Parameter p, ControlFlowNode n) {
-  (exists(p.getAnAccess()) and n = p.getFunction().getBlock()) or
-  exists(ControlFlowNode mid | flow(p, mid) and not mid = p.getAnAccess() and n = mid.getASuccessor())
+  (exists(p.getAnAccess()) and n = p.getFunction().getBlock())
+  or
+  exists(ControlFlowNode mid |
+    flow(p, mid) and not mid = p.getAnAccess() and n = mid.getASuccessor()
+  )
 }
 
-VariableAccess firstAccess(Parameter p) {
-  flow(p, result) and result = p.getAnAccess()
-}
+VariableAccess firstAccess(Parameter p) { flow(p, result) and result = p.getAnAccess() }
 
 from Parameter p, VariableAccess va
-where va = firstAccess(p) and
-    not exists(Expr e | e.isCondition() | e.getAChild*() = va)
+where
+  va = firstAccess(p) and
+  not exists(Expr e | e.isCondition() | e.getAChild*() = va)
 select va, "This use of parameter " + p.getName() + " has not been checked."
-

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

@@ -15,9 +15,9 @@ import IncorrectPointerScalingCommon
 
 private predicate isCharSzPtrExpr(Expr e) {
   exists (PointerType pt
-  | pt = e.getFullyConverted().getUnderlyingType()
-  | pt.getBaseType().getUnspecifiedType() instanceof CharType
-  or pt.getBaseType().getUnspecifiedType() instanceof VoidType)
+  | pt = e.getFullyConverted().getType().getUnspecifiedType()
+  | pt.getBaseType() instanceof CharType
+  or pt.getBaseType() instanceof VoidType)
 }
 
 from Expr sizeofExpr, Expr e

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

@@ -140,12 +140,12 @@ class Resource extends MemberVariable {
     )
   }
 
-  predicate acquisitionWithRequiredRelease(Assignment acquireAssign, string kind) {
+  predicate acquisitionWithRequiredKind(Assignment acquireAssign, string kind) {
     // acquireAssign is an assignment to this resource
     acquireAssign.(Assignment).getLValue() = this.getAnAccess() and
     // Should be in this class, but *any* member method will do
     this.inSameClass(acquireAssign) and
-    // Check that it is an acquisition function and return the corresponding free
+    // Check that it is an acquisition function and return the corresponding kind
     acquireExpr(acquireAssign.getRValue(), kind)
   }
 
@@ -158,15 +158,22 @@ predicate unreleasedResource(Resource r, Expr acquire, File f, int acquireLine)
     // Note: there could be several release functions, because there could be
     // several functions called 'fclose' for example. We want to check that
     // *none* of these functions are called to release the resource
-    r.acquisitionWithRequiredRelease(acquire, _) and
-    not exists(Expr releaseExpr, string releaseName |
-         r.acquisitionWithRequiredRelease(acquire, releaseName) and
-         releaseExpr = r.getAReleaseExpr(releaseName) and
+    r.acquisitionWithRequiredKind(acquire, _) and
+    not exists(Expr releaseExpr, string kind |
+         r.acquisitionWithRequiredKind(acquire, kind) and
+         releaseExpr = r.getAReleaseExpr(kind) and
          r.inDestructor(releaseExpr)
     )
     and f = acquire.getFile()
     and acquireLine = acquire.getLocation().getStartLine()
 
+    and not exists(ExprCall exprCall |
+      // expression call (function pointer or lambda) with `r` as an
+      // argument, which could release it.
+      exprCall.getAnArgument() = r.getAnAccess() and
+      r.inDestructor(exprCall)
+    )
+
     // check that any destructor for this class has a block; if it doesn't,
     // we must be missing information.
     and forall(Class c, Destructor d |
@@ -181,10 +188,10 @@ predicate unreleasedResource(Resource r, Expr acquire, File f, int acquireLine)
 
 predicate freedInSameMethod(Resource r, Expr acquire) {
   unreleasedResource(r, acquire, _, _) and
-  exists(Expr releaseExpr, string releaseName |
-    r.acquisitionWithRequiredRelease(acquire, releaseName) and
-    releaseExpr = r.getAReleaseExpr(releaseName) and
-    releaseExpr.getEnclosingFunction() = acquire.getEnclosingFunction()
+  exists(Expr releaseExpr, string kind |
+    r.acquisitionWithRequiredKind(acquire, kind) and
+    releaseExpr = r.getAReleaseExpr(kind) and
+    releaseExpr.getEnclosingElement+() = acquire.getEnclosingFunction()
   )
 }
 
@@ -221,16 +228,21 @@ predicate leakedInSameMethod(Resource r, Expr acquire) {
           fc = acquire.getAChild*() // e.g. `r = new MyClass(this)`
         )
       )
+    ) or exists(FunctionAccess fa, string kind |
+      // the address of a function that releases `r` is taken (and likely
+      // used to release `r` at some point).
+      r.acquisitionWithRequiredKind(acquire, kind) and
+      fa.getTarget() = r.getAReleaseExpr(kind).getEnclosingFunction()
     )
   )
 }
 
 pragma[noopt] predicate badRelease(Resource r, Expr acquire, Function functionCallingRelease, int line) {
   unreleasedResource(r, acquire, _, _) and
-  exists(Expr releaseExpr, string releaseName,
+  exists(Expr releaseExpr, string kind,
          Location releaseExprLocation, Function acquireFunction |
-    r.acquisitionWithRequiredRelease(acquire, releaseName) and
-    releaseExpr = r.getAReleaseExpr(releaseName) and
+    r.acquisitionWithRequiredKind(acquire, kind) and
+    releaseExpr = r.getAReleaseExpr(kind) and
     releaseExpr.getEnclosingFunction() = functionCallingRelease and
     functionCallingRelease.getDeclaringType() = r.getDeclaringType() and
     releaseExprLocation = releaseExpr.getLocation() and

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

@@ -3,11 +3,45 @@ import semmle.code.cpp.File
 import semmle.code.cpp.Preprocessor
 
 /**
- * Holds if `c` is a comment which is usually seen in autogenerated files.
- * For example, comments containing 'autogenerated' or 'generated by'.
+ * Holds if comment `c` indicates that it might be in an auto-generated file, for
+ * example because it contains the text "auto-generated by".
  */
-predicate isAutogeneratedComment(Comment c) {
-  c.getContents().regexpMatch("(?si).*(?:auto[ -]?generated|generated (?:by|file)|changes made in this file will be lost).*")
+private bindingset[comment] predicate autogeneratedComment(string comment) {
+  // ?s = include newlines in anything (`.`)
+  // ?i = ignore case
+  exists(string cond |
+    cond =
+      // generated by (not mid-sentence)
+      "(^ generated by[^a-z])|" +
+      "(! generated by[^a-z])|" +
+
+      // generated file
+      "(generated file)|" +
+
+      // file [is/was/has been] generated
+      "(file( is| was| has been)? generated)|" +
+
+      // changes made in this file will be lost
+      "(changes made in this file will be lost)|" +
+  
+      // do not edit/modify
+      "(^ do(n't|nt| not) (hand-?)?(edit|modify))|" +
+      "(! do(n't|nt| not) (hand-?)?(edit|modify))" and
+  
+    comment.regexpMatch("(?si).*(" +
+      // replace `generated` with a regexp that also catches things like
+      // `auto-generated`.
+      cond.replaceAll("generated", "(auto[\\w-]*[\\s/\\*\\r\\n]*)?generated")
+
+      // replace `!` with a regexp for end-of-sentence / separator characters.
+      .replaceAll("!", "[\\.\\?\\!\\-\\;\\,]")
+
+      // replace ` ` with a regexp for one or more whitespace characters
+      // (including newlines and `/*`).
+      .replaceAll(" ", "[\\s/\\*\\r\\n]+") +
+      ").*"
+    )
+  )
 }
 
 /**
@@ -25,6 +59,48 @@ predicate hasPragmaDifferentFile(File f) {
   )
 }
 
+/**
+ * The line where the first comment in file `f` begins (maximum of 5).  This allows
+ * us to skip past any preprocessor logic or similar code before the first comment.
+ */
+private int fileFirstComment(File f) {
+  result = min(int line |
+    exists(Comment c |
+      c.getFile() = f and
+      c.getLocation().getStartLine() = line and
+      line < 5
+    )
+  ).minimum(5)
+}
+
+/**
+ * The line where the initial comments of file `f` end.  This is just before the
+ * first bit of code, excluding anything skipped over by `fileFirstComment`.
+ */
+private int fileHeaderLimit(File f) {
+  exists(int fc |
+    fc = fileFirstComment(f) and
+    result = min(int line |
+      exists(DeclarationEntry de, Location l |
+        l = de.getLocation() and
+        l.getFile() = f and
+        line = l.getStartLine() - 1 and
+        line > fc
+      ) or exists(PreprocessorDirective pd, Location l |
+        l = pd.getLocation() and
+        l.getFile() = f and
+        line = l.getStartLine() - 1 and
+        line > fc
+      ) or exists(NamespaceDeclarationEntry nde, Location l |
+        l = nde.getLocation() and
+        l.getFile() = f and
+        line = l.getStartLine() - 1 and
+        line > fc
+      ) or line = f.getMetrics().getNumberOfLines()
+    )
+  )
+}
+
 /**
  * Holds if the file is probably an autogenerated file.
  *
@@ -36,12 +112,13 @@ predicate hasPragmaDifferentFile(File f) {
  */
 class AutogeneratedFile extends File {
   cached AutogeneratedFile() {
-    exists(int limit, int head |
-      head <= 5 and
-      limit = max(int line | locations_default(_, underlyingElement(this), head, _, line, _)) + 5
-    |
-      exists (Comment c | c.getFile() = this and c.getLocation().getStartLine() <= limit and isAutogeneratedComment(c))
-    )
-    or hasPragmaDifferentFile(this)
+    autogeneratedComment(
+      strictconcat(Comment c |
+        c.getFile() = this and
+        c.getLocation().getStartLine() <= fileHeaderLimit(this) |
+        c.getContents() order by c.getLocation().getStartLine()
+      )
+    ) or
+    hasPragmaDifferentFile(this)
   }
 }

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

@@ -67,7 +67,10 @@ class NameQualifier extends NameQualifiableElement, @namequalifier {
  * the latter is qualified by `N1`.
  */
 class NameQualifiableElement extends Element, @namequalifiableelement {
-  /** Gets the name qualifier associated with this element. */
+  /**
+   * Gets the name qualifier associated with this element. For example, the
+   * name qualifier of `N::f()` is `N`.
+   */
   NameQualifier getNameQualifier() {
     namequalifiers(unresolveElement(result),underlyingElement(this),_,_)
   }

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

@@ -66,6 +66,11 @@ class Namespace extends NameQualifyingElement, @namespace {
   /** Gets a child namespace of this namespace. */
   Namespace getAChildNamespace() { namespacembrs(underlyingElement(this),unresolveElement(result)) }
 
+  /** Holds if the namespace is inline. */
+  predicate isInline() {
+    namespace_inline(underlyingElement(this))
+  }
+
   /** Holds if this namespace may be from source. */
   override predicate fromSource() { this.getADeclaration().fromSource() }
 

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

@@ -280,6 +280,32 @@ class IRGuardCondition extends Instruction {
           ne.controls(controlled, testIsTrue.booleanNot())) 
     }
 
+    /**
+     * Holds if `branch` jumps directly to `succ` when this condition is `testIsTrue`.
+     * 
+     * This predicate is intended to help with situations in which an inference can only be made
+     * based on an edge between a block with multiple successors and a block with multiple
+     * predecessors. For example, in the following situation, an inference can be made about the
+     * value of `x` at the end of the `if` statement, but there is no block which is controlled by
+     * the `if` statement when `x >= y`.
+     * ```
+     * if (x < y) {
+     *   x = y;
+     * }
+     * return x;
+     * ```
+     */
+    predicate hasBranchEdge(ConditionalBranchInstruction branch, IRBlock succ, boolean testIsTrue) {
+      branch.getCondition() = this and
+      (
+        testIsTrue = true and
+        succ.getFirstInstruction() = branch.getTrueSuccessor()
+        or
+        testIsTrue = false and
+        succ.getFirstInstruction() = branch.getFalseSuccessor()
+      )
+    }
+
     /** Holds if (determined by this guard) `left < right + k` evaluates to `isLessThan` if this expression evaluates to `testIsTrue`. */
     cached predicate comparesLt(Operand left, Operand right, int k, boolean isLessThan, boolean testIsTrue) {
         compares_lt(this, left, right, k, isLessThan, testIsTrue)

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

@@ -827,7 +827,8 @@ private predicate straightLineDense(Node scope, int rnk, Node nrnk, Spec spec) {
  * but most cases should be handled through one of the convenience predicates
  * as outlined in the comment at the top of this file.
  */
-private predicate subEdge(Node n1, Pos p1, Node n2, Pos p2) {
+// The parameters are ordered this way for performance.
+private predicate subEdge(Pos p1, Node n1, Node n2, Pos p2) {
   exists(Node scope, int rnk, Spec spec1, Spec spec2 |
     straightLineDense(scope, rnk, n1, spec1) and
     straightLineDense(scope, rnk + 1, n2, spec2) and
@@ -997,13 +998,13 @@ private predicate subEdge(Node n1, Pos p1, Node n2, Pos p2) {
  * predicate includes all sub-edges except those with true/false labels (see
  * `conditionJumps`).
  */
-private predicate subEdgeIncludingDestructors(Node n1, Pos p1, Node n2, Pos p2) {
-  subEdge(n1, p1, n2, p2)
+private predicate subEdgeIncludingDestructors(Pos p1, Node n1, Node n2, Pos p2) {
+  subEdge(p1, n1, n2, p2)
   or
   // If `n1` has sub-nodes to accomodate destructors, but there are none to be
   // called, connect the "before destructors" node directly to the "after
   // destructors" node. For performance, only do this when the nodes exist.
-  exists(Pos afterDtors | afterDtors.isAfterDestructors() | subEdge(n1, afterDtors, _, _)) and
+  exists(Pos afterDtors | afterDtors.isAfterDestructors() | subEdge(afterDtors, n1, _, _)) and
   not exists(getDestructorCallAfterNode(n1, 0)) and
   p1.nodeBeforeDestructors(n1, n1) and
   p2.nodeAfterDestructors(n2, n1)
@@ -1286,22 +1287,27 @@ private predicate conditionJumps(Expr test, boolean truth, Node n2, Pos p2) {
   )
 }
 
+// Factored out for performance. See QL-796.
+private predicate normalGroupMemberBaseCase(Node memberNode, Pos memberPos, Node atNode) {
+  memberNode = atNode and
+  memberPos.isAt() and
+  // We check for excludeNode here as it's slower to check in all the leaf
+  // cases during construction of the sub-graph.
+  not excludeNode(atNode)
+}
+
 /**
  * Holds if the sub-node `(memberNode, memberPos)` can reach `at(atNode)` by
  * following sub-edges forward without crossing another "at" node. Here,
  * `memberPos.isAt()` holds only when `memberNode = atNode`.
  */
 private predicate normalGroupMember(Node memberNode, Pos memberPos, Node atNode) {
-  memberNode = atNode and
-  memberPos.isAt() and
-  // We check for excludeNode here as it's slower to check in all the leaf
-  // cases during construction of the sub-graph.
-  not excludeNode(atNode)
+  normalGroupMemberBaseCase(memberNode, memberPos, atNode)
   or
   exists(Node succNode, Pos succPos |
     normalGroupMember(succNode, succPos, atNode) and
     not memberPos.isAt() and
-    subEdgeIncludingDestructors(memberNode, memberPos, succNode, succPos)
+    subEdgeIncludingDestructors(memberPos, memberNode, succNode, succPos)
   )
 }
 
@@ -1317,7 +1323,7 @@ private predicate precedesCondition(Node memberNode, Pos memberPos, Node test) {
   or
   exists(Node succNode, Pos succPos |
     precedesCondition(succNode, succPos, test) and
-    subEdgeIncludingDestructors(memberNode, memberPos, succNode, succPos) and
+    subEdgeIncludingDestructors(memberPos, memberNode, succNode, succPos) and
     // Unlike the similar TC in normalGroupMember we're here including the
     // At-node in the group. This should generalize better to the case where
     // the base case isn't always an After-node.
@@ -1355,7 +1361,7 @@ private module Cached {
   cached
   predicate qlCFGSuccessor(Node n1, Node n2) {
     exists(Node memberNode, Pos memberPos |
-      subEdgeIncludingDestructors(n1, any(Pos at | at.isAt()), memberNode, memberPos) and
+      subEdgeIncludingDestructors(any(Pos at | at.isAt()), n1, memberNode, memberPos) and
       normalGroupMember(memberNode, memberPos, n2)
     )
     or

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

@@ -335,10 +335,10 @@ class Expr extends StmtParent, @expr {
   /** Gets the fully converted form of this expression, including all type casts and other conversions. */
   cached
   Expr getFullyConverted() {
-    if this.hasConversion() then
-      result = this.getConversion().getFullyConverted()
-    else
-      result = this
+    hasNoConversions(this) and
+    result = this
+    or
+    result = this.getConversion().getFullyConverted()
   }
 
   /**
@@ -977,3 +977,6 @@ private predicate isStandardPlacementNewAllocator(Function operatorNew) {
   operatorNew.getNumberOfParameters() = 2 and
   operatorNew.getParameter(1).getType() instanceof VoidPointerType
 }
+
+// Pulled out for performance. See QL-796.
+private predicate hasNoConversions(Expr e) { not e.hasConversion() }

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

@@ -106,6 +106,32 @@ module InstructionSanity {
     not instr instanceof UnreachedInstruction
   }
 
+  /**
+   * Holds if there are multiple (`n`) edges of kind `kind` from `source`,
+   * where `target` is among the targets of those edges.
+   */
+  query predicate ambiguousSuccessors(
+    Instruction source, EdgeKind kind, int n, Instruction target
+  ) {
+    n = strictcount(Instruction t | source.getSuccessor(kind) = t) and
+    n > 1 and
+    source.getSuccessor(kind) = target
+  }
+
+  /**
+   * Holds if `instr` in `f` is part of a loop even though the AST of `f`
+   * contains no element that can cause loops.
+   */
+  query predicate unexplainedLoop(Function f, Instruction instr) {
+    exists(IRBlock block |
+      instr.getBlock() = block and
+      block.getFunction() = f and
+      block.getASuccessor+() = block
+    ) and
+    not exists(Loop l | l.getEnclosingFunction() = f) and
+    not exists(GotoStmt s | s.getEnclosingFunction() = f)
+  }
+
   /**
    * Holds if a `Phi` instruction is present in a block with fewer than two
    * predecessors.
@@ -130,7 +156,7 @@ module InstructionSanity {
   query predicate instructionWithoutUniqueBlock(Instruction instr, int blockCount) {
     blockCount = count(instr.getBlock()) and
     blockCount != 1
-  } 
+  }
 }
 
 /**
@@ -750,6 +776,15 @@ class BinaryInstruction extends Instruction {
   final Instruction getRightOperand() {
     result = getAnOperand().(RightOperand).getDefinitionInstruction()
   }
+  
+  /**
+   * Holds if this instruction's operands are `op1` and `op2`, in either order.
+   */
+  final predicate hasOperands(Operand op1, Operand op2) {
+    op1 = getAnOperand().(LeftOperand) and op2 = getAnOperand().(RightOperand)
+    or
+    op1 = getAnOperand().(RightOperand) and op2 = getAnOperand().(LeftOperand)
+  }
 }
 
 class AddInstruction extends BinaryInstruction {

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

@@ -21,6 +21,10 @@ class Operand extends TOperand {
     result = "Operand"
   }
 
+  Location getLocation() {
+    result = getInstruction().getLocation()
+  }
+
   /**
    * Gets the `Instruction` that consumes this operand.
    */

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/ValueNumbering.qll

@@ -83,6 +83,10 @@ class ValueNumber extends TValueNumber {
       instr order by instr.getBlock().getDisplayIndex(), instr.getDisplayIndexInBlock()
     )
   }
+  
+  final Operand getAUse() {
+    this = valueNumber(result.getDefinitionInstruction())
+  }
 }
 
 /**
@@ -107,6 +111,7 @@ private class CongruentCopyInstruction extends CopyInstruction {
       def = this.getSourceValue() and
       (
         def.getResultMemoryAccess() instanceof IndirectMemoryAccess or
+        def.getResultMemoryAccess() instanceof PhiMemoryAccess or
         not def.hasMemoryResult()
       )
     )
@@ -211,7 +216,7 @@ private predicate uniqueValueNumber(Instruction instr, FunctionIR funcIR) {
 /**
  * Gets the value number assigned to `instr`, if any. Returns at most one result.
  */
-ValueNumber valueNumber(Instruction instr) {
+cached ValueNumber valueNumber(Instruction instr) {
   result = nonUniqueValueNumber(instr) or
   exists(FunctionIR funcIR |
     uniqueValueNumber(instr, funcIR) and
@@ -219,6 +224,13 @@ ValueNumber valueNumber(Instruction instr) {
   )
 }
 
+/**
+ * Gets the value number assigned to `instr`, if any. Returns at most one result.
+ */
+ValueNumber valueNumberOfOperand(Operand op) {
+  result = valueNumber(op.getDefinitionInstruction())
+}
+
 /**
  * Gets the value number assigned to `instr`, if any, unless that instruction is assigned a unique
  * value number.

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

@@ -106,6 +106,32 @@ module InstructionSanity {
     not instr instanceof UnreachedInstruction
   }
 
+  /**
+   * Holds if there are multiple (`n`) edges of kind `kind` from `source`,
+   * where `target` is among the targets of those edges.
+   */
+  query predicate ambiguousSuccessors(
+    Instruction source, EdgeKind kind, int n, Instruction target
+  ) {
+    n = strictcount(Instruction t | source.getSuccessor(kind) = t) and
+    n > 1 and
+    source.getSuccessor(kind) = target
+  }
+
+  /**
+   * Holds if `instr` in `f` is part of a loop even though the AST of `f`
+   * contains no element that can cause loops.
+   */
+  query predicate unexplainedLoop(Function f, Instruction instr) {
+    exists(IRBlock block |
+      instr.getBlock() = block and
+      block.getFunction() = f and
+      block.getASuccessor+() = block
+    ) and
+    not exists(Loop l | l.getEnclosingFunction() = f) and
+    not exists(GotoStmt s | s.getEnclosingFunction() = f)
+  }
+
   /**
    * Holds if a `Phi` instruction is present in a block with fewer than two
    * predecessors.
@@ -130,7 +156,7 @@ module InstructionSanity {
   query predicate instructionWithoutUniqueBlock(Instruction instr, int blockCount) {
     blockCount = count(instr.getBlock()) and
     blockCount != 1
-  } 
+  }
 }
 
 /**
@@ -750,6 +776,15 @@ class BinaryInstruction extends Instruction {
   final Instruction getRightOperand() {
     result = getAnOperand().(RightOperand).getDefinitionInstruction()
   }
+  
+  /**
+   * Holds if this instruction's operands are `op1` and `op2`, in either order.
+   */
+  final predicate hasOperands(Operand op1, Operand op2) {
+    op1 = getAnOperand().(LeftOperand) and op2 = getAnOperand().(RightOperand)
+    or
+    op1 = getAnOperand().(RightOperand) and op2 = getAnOperand().(LeftOperand)
+  }
 }
 
 class AddInstruction extends BinaryInstruction {

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

@@ -21,6 +21,10 @@ class Operand extends TOperand {
     result = "Operand"
   }
 
+  Location getLocation() {
+    result = getInstruction().getLocation()
+  }
+
   /**
    * Gets the `Instruction` that consumes this operand.
    */

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/ValueNumbering.qll

@@ -83,6 +83,10 @@ class ValueNumber extends TValueNumber {
       instr order by instr.getBlock().getDisplayIndex(), instr.getDisplayIndexInBlock()
     )
   }
+  
+  final Operand getAUse() {
+    this = valueNumber(result.getDefinitionInstruction())
+  }
 }
 
 /**
@@ -107,6 +111,7 @@ private class CongruentCopyInstruction extends CopyInstruction {
       def = this.getSourceValue() and
       (
         def.getResultMemoryAccess() instanceof IndirectMemoryAccess or
+        def.getResultMemoryAccess() instanceof PhiMemoryAccess or
         not def.hasMemoryResult()
       )
     )
@@ -211,7 +216,7 @@ private predicate uniqueValueNumber(Instruction instr, FunctionIR funcIR) {
 /**
  * Gets the value number assigned to `instr`, if any. Returns at most one result.
  */
-ValueNumber valueNumber(Instruction instr) {
+cached ValueNumber valueNumber(Instruction instr) {
   result = nonUniqueValueNumber(instr) or
   exists(FunctionIR funcIR |
     uniqueValueNumber(instr, funcIR) and
@@ -219,6 +224,13 @@ ValueNumber valueNumber(Instruction instr) {
   )
 }
 
+/**
+ * Gets the value number assigned to `instr`, if any. Returns at most one result.
+ */
+ValueNumber valueNumberOfOperand(Operand op) {
+  result = valueNumber(op.getDefinitionInstruction())
+}
+
 /**
  * Gets the value number assigned to `instr`, if any, unless that instruction is assigned a unique
  * value number.

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

@@ -959,7 +959,7 @@ class TranslatedFunctionAccess extends TranslatedNonConstantExpr {
 /**
  * IR translation of an expression whose value is not known at compile time.
  */
-abstract class TranslatedNonConstantExpr extends TranslatedCoreExpr {
+abstract class TranslatedNonConstantExpr extends TranslatedCoreExpr, TTranslatedValueExpr {
   TranslatedNonConstantExpr() {
     this = TTranslatedValueExpr(expr) and
     not expr.isConstant()
@@ -971,7 +971,7 @@ abstract class TranslatedNonConstantExpr extends TranslatedCoreExpr {
  * includes not only literals, but also "integral constant expressions" (e.g.
  * `1 + 2`).
  */
-abstract class TranslatedConstantExpr extends TranslatedCoreExpr {
+abstract class TranslatedConstantExpr extends TranslatedCoreExpr, TTranslatedValueExpr {
   TranslatedConstantExpr() {
     this = TTranslatedValueExpr(expr) and
     expr.isConstant()

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/reachability/ReachableBlock.qll

@@ -13,6 +13,7 @@ predicate isInfeasibleInstructionSuccessor(Instruction instr, EdgeKind kind) {
   )
 }
 
+pragma[noinline]
 predicate isInfeasibleEdge(IRBlockBase block, EdgeKind kind) {
   isInfeasibleInstructionSuccessor(block.getLastInstruction(), kind)
 }

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

@@ -106,6 +106,32 @@ module InstructionSanity {
     not instr instanceof UnreachedInstruction
   }
 
+  /**
+   * Holds if there are multiple (`n`) edges of kind `kind` from `source`,
+   * where `target` is among the targets of those edges.
+   */
+  query predicate ambiguousSuccessors(
+    Instruction source, EdgeKind kind, int n, Instruction target
+  ) {
+    n = strictcount(Instruction t | source.getSuccessor(kind) = t) and
+    n > 1 and
+    source.getSuccessor(kind) = target
+  }
+
+  /**
+   * Holds if `instr` in `f` is part of a loop even though the AST of `f`
+   * contains no element that can cause loops.
+   */
+  query predicate unexplainedLoop(Function f, Instruction instr) {
+    exists(IRBlock block |
+      instr.getBlock() = block and
+      block.getFunction() = f and
+      block.getASuccessor+() = block
+    ) and
+    not exists(Loop l | l.getEnclosingFunction() = f) and
+    not exists(GotoStmt s | s.getEnclosingFunction() = f)
+  }
+
   /**
    * Holds if a `Phi` instruction is present in a block with fewer than two
    * predecessors.
@@ -130,7 +156,7 @@ module InstructionSanity {
   query predicate instructionWithoutUniqueBlock(Instruction instr, int blockCount) {
     blockCount = count(instr.getBlock()) and
     blockCount != 1
-  } 
+  }
 }
 
 /**
@@ -750,6 +776,15 @@ class BinaryInstruction extends Instruction {
   final Instruction getRightOperand() {
     result = getAnOperand().(RightOperand).getDefinitionInstruction()
   }
+  
+  /**
+   * Holds if this instruction's operands are `op1` and `op2`, in either order.
+   */
+  final predicate hasOperands(Operand op1, Operand op2) {
+    op1 = getAnOperand().(LeftOperand) and op2 = getAnOperand().(RightOperand)
+    or
+    op1 = getAnOperand().(RightOperand) and op2 = getAnOperand().(LeftOperand)
+  }
 }
 
 class AddInstruction extends BinaryInstruction {

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

@@ -21,6 +21,10 @@ class Operand extends TOperand {
     result = "Operand"
   }
 
+  Location getLocation() {
+    result = getInstruction().getLocation()
+  }
+
   /**
    * Gets the `Instruction` that consumes this operand.
    */

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/ValueNumbering.qll

@@ -83,6 +83,10 @@ class ValueNumber extends TValueNumber {
       instr order by instr.getBlock().getDisplayIndex(), instr.getDisplayIndexInBlock()
     )
   }
+  
+  final Operand getAUse() {
+    this = valueNumber(result.getDefinitionInstruction())
+  }
 }
 
 /**
@@ -107,6 +111,7 @@ private class CongruentCopyInstruction extends CopyInstruction {
       def = this.getSourceValue() and
       (
         def.getResultMemoryAccess() instanceof IndirectMemoryAccess or
+        def.getResultMemoryAccess() instanceof PhiMemoryAccess or
         not def.hasMemoryResult()
       )
     )
@@ -211,7 +216,7 @@ private predicate uniqueValueNumber(Instruction instr, FunctionIR funcIR) {
 /**
  * Gets the value number assigned to `instr`, if any. Returns at most one result.
  */
-ValueNumber valueNumber(Instruction instr) {
+cached ValueNumber valueNumber(Instruction instr) {
   result = nonUniqueValueNumber(instr) or
   exists(FunctionIR funcIR |
     uniqueValueNumber(instr, funcIR) and
@@ -219,6 +224,13 @@ ValueNumber valueNumber(Instruction instr) {
   )
 }
 
+/**
+ * Gets the value number assigned to `instr`, if any. Returns at most one result.
+ */
+ValueNumber valueNumberOfOperand(Operand op) {
+  result = valueNumber(op.getDefinitionInstruction())
+}
+
 /**
  * Gets the value number assigned to `instr`, if any, unless that instruction is assigned a unique
  * value number.

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/reachability/ReachableBlock.qll

@@ -13,6 +13,7 @@ predicate isInfeasibleInstructionSuccessor(Instruction instr, EdgeKind kind) {
   )
 }
 
+pragma[noinline]
 predicate isInfeasibleEdge(IRBlockBase block, EdgeKind kind) {
   isInfeasibleInstructionSuccessor(block.getLastInstruction(), kind)
 }

cpp/ql/src/semmle/code/cpp/rangeanalysis/Bound.qll

@@ -0,0 +1,80 @@
+import cpp
+private import semmle.code.cpp.ir.IR
+private import semmle.code.cpp.ir.ValueNumbering
+
+private newtype TBound =
+  TBoundZero() or
+  TBoundValueNumber(ValueNumber vn) {
+    exists(Instruction i |
+      vn.getAnInstruction() = i and
+      (
+        i.getResultType() instanceof IntegralType or
+        i.getResultType() instanceof PointerType
+      ) and
+      not vn.getAnInstruction() instanceof ConstantInstruction
+    |
+      i instanceof PhiInstruction
+      or
+      i instanceof InitializeParameterInstruction
+      or
+      i instanceof CallInstruction
+      or
+      i instanceof VariableAddressInstruction
+      or
+      i instanceof FieldAddressInstruction
+      or
+      i.(LoadInstruction).getSourceAddress() instanceof VariableAddressInstruction
+      or
+      i.(LoadInstruction).getSourceAddress() instanceof FieldAddressInstruction
+      or
+      i.getAUse() instanceof ArgumentOperand
+    )
+  }
+
+/**
+ * A bound that may be inferred for an expression plus/minus an integer delta.
+ */
+abstract class Bound extends TBound {
+  abstract string toString();
+
+  /** Gets an expression that equals this bound plus `delta`. */
+  abstract Instruction getInstruction(int delta);
+
+  /** Gets an expression that equals this bound. */
+  Instruction getInstruction() { result = getInstruction(0) }
+
+  abstract Location getLocation();
+}
+
+/**
+ * The bound that corresponds to the integer 0. This is used to represent all
+ * integer bounds as bounds are always accompanied by an added integer delta.
+ */
+class ZeroBound extends Bound, TBoundZero {
+  override string toString() { result = "0" }
+
+  override Instruction getInstruction(int delta) { result.(ConstantValueInstruction).getValue().toInt() = delta }
+  
+  override Location getLocation() {
+    result instanceof UnknownDefaultLocation
+  }
+}
+/**
+ * A bound corresponding to the value of an `Instruction`.
+ */
+class ValueNumberBound extends Bound, TBoundValueNumber {
+  ValueNumber vn;
+  
+  ValueNumberBound() {
+    this = TBoundValueNumber(vn)
+  }
+  
+  /** Gets the SSA variable that equals this bound. */
+  override Instruction getInstruction(int delta) { this = TBoundValueNumber(valueNumber(result)) and delta = 0}
+
+  override string toString() { result = vn.getExampleInstruction().toString() }
+
+  override Location getLocation() {
+    result = vn.getLocation()
+  }
+}

cpp/ql/src/semmle/code/cpp/rangeanalysis/RangeAnalysis.qll

@@ -0,0 +1,605 @@
+/**
+ * Provides classes and predicates for range analysis.
+ *
+ * An inferred bound can either be a specific integer or a `ValueNumber`
+ * representing the abstract value of a set of `Instruction`s.
+ *
+ * If an inferred bound relies directly on a condition, then this condition is
+ * reported as the reason for the bound.
+ */
+
+/*
+ * This library tackles range analysis as a flow problem. Consider e.g.:
+ * ```
+ *   len = arr.length;
+ *   if (x < len) { ... y = x-1; ... y ... }
+ * ```
+ * In this case we would like to infer `y <= arr.length - 2`, and this is
+ * accomplished by tracking the bound through a sequence of steps:
+ * ```
+ *   arr.length --> len = .. --> x < len --> x-1 --> y = .. --> y
+ * ```
+ *
+ * In its simplest form the step relation `I1 --> I2` relates two `Instruction`s
+ * such that `I1 <= B` implies `I2 <= B` for any `B` (with a second separate
+ * step relation handling lower bounds). Examples of such steps include
+ * assignments `I2 = I1` and conditions `x <= I1` where `I2` is a use of `x`
+ * guarded by the condition.
+ *
+ * In order to handle subtractions and additions with constants, and strict
+ * comparisons, the step relation is augmented with an integer delta. With this
+ * generalization `I1 --(delta)--> I2` relates two `Instruction`s and an integer
+ * such that `I1 <= B` implies `I2 <= B + delta` for any `B`. This corresponds
+ * to the predicate `boundFlowStep`.
+ *
+ * The complete range analysis is then implemented as the transitive closure of
+ * the step relation summing the deltas along the way. If `I1` transitively
+ * steps to `I2`, `delta` is the sum of deltas along the path, and `B` is an
+ * interesting bound equal to the value of `I1` then `I2 <= B + delta`. This
+ * corresponds to the predicate `boundedInstruction`.
+ *
+ * Bounds come in two forms: either they are relative to zero (and thus provide
+ * a constant bound), or they are relative to some program value. This value is
+ * represented by the `ValueNumber` class, each instance of which represents a
+ * set of `Instructions` that must have the same value.
+ * 
+ * Phi nodes need a little bit of extra handling. Consider `x0 = phi(x1, x2)`.
+ * There are essentially two cases:
+ * - If `x1 <= B + d1` and `x2 <= B + d2` then `x0 <= B + max(d1,d2)`.
+ * - If `x1 <= B + d1` and `x2 <= x0 + d2` with `d2 <= 0` then `x0 <= B + d1`.
+ * The first case is for whenever a bound can be proven without taking looping
+ * into account. The second case is relevant when `x2` comes from a back-edge
+ * where we can prove that the variable has been non-increasing through the
+ * loop-iteration as this means that any upper bound that holds prior to the
+ * loop also holds for the variable during the loop.
+ * This generalizes to a phi node with `n` inputs, so if
+ * `x0 = phi(x1, ..., xn)` and `xi <= B + delta` for one of the inputs, then we
+ * also have `x0 <= B + delta` if we can prove either:
+ * - `xj <= B + d` with `d <= delta` or
+ * - `xj <= x0 + d` with `d <= 0`
+ * for each input `xj`.
+ *
+ * As all inferred bounds can be related directly to a path in the source code
+ * the only source of non-termination is if successive redundant (and thereby
+ * increasingly worse) bounds are calculated along a loop in the source code.
+ * We prevent this by weakening the bound to a small finite set of bounds when
+ * a path follows a second back-edge (we postpone weakening till the second
+ * back-edge as a precise bound might require traversing a loop once).
+*/
+
+import cpp
+
+private import semmle.code.cpp.ir.IR
+private import semmle.code.cpp.controlflow.IRGuards
+private import semmle.code.cpp.ir.ValueNumbering
+private import RangeUtils
+private import SignAnalysis
+import Bound
+
+cached private module RangeAnalysisCache {
+
+  cached module RangeAnalysisPublic {
+    /**
+     * Holds if `b + delta` is a valid bound for `i`.
+     * - `upper = true`  : `i <= b + delta`
+     * - `upper = false` : `i >= b + delta`
+     *
+     * The reason for the bound is given by `reason` and may be either a condition
+     * or `NoReason` if the bound was proven directly without the use of a bounding
+     * condition.
+     */
+    cached predicate boundedInstruction(Instruction i, Bound b, int delta, boolean upper, Reason reason) {
+      boundedInstruction(i, b, delta, upper, _, _, reason)
+    }
+    
+    /**
+     * Holds if `b + delta` is a valid bound for `op`.
+     * - `upper = true`  : `op <= b + delta`
+     * - `upper = false` : `op >= b + delta`
+     *
+     * The reason for the bound is given by `reason` and may be either a condition
+     * or `NoReason` if the bound was proven directly without the use of a bounding
+     * condition.
+     */
+    cached predicate boundedOperand(Operand op, Bound b, int delta, boolean upper, Reason reason) {
+      boundedNonPhiOperand(op, b, delta, upper, _, _, reason)
+      or
+      boundedPhiOperand(op, b, delta, upper, _, _, reason)
+    }
+  }
+
+  /**
+   * Holds if `guard = boundFlowCond(_, _, _, _, _) or guard = eqFlowCond(_, _, _, _, _)`.
+   */
+  cached predicate possibleReason(IRGuardCondition guard) {
+    guard = boundFlowCond(_, _, _, _, _)
+    or
+    guard = eqFlowCond(_, _, _, _, _)
+  }
+
+}
+private import RangeAnalysisCache
+import RangeAnalysisPublic
+
+/**
+ * Gets a condition that tests whether `vn` equals `bound + delta`.
+ *
+ * If the condition evaluates to `testIsTrue`:
+ * - `isEq = true`  : `vn == bound + delta`
+ * - `isEq = false` : `vn != bound + delta`
+ */
+private IRGuardCondition eqFlowCond(ValueNumber vn, Operand bound, int delta,
+  boolean isEq, boolean testIsTrue)
+{
+  result.comparesEq(vn.getAUse(), bound, delta, isEq, testIsTrue)
+}
+
+/**
+ * Holds if `op1 + delta` is a valid bound for `op2`.
+ * - `upper = true`  : `op2 <= op1 + delta`
+ * - `upper = false` : `op2 >= op1 + delta`
+ */
+private predicate boundFlowStepSsa(
+  NonPhiOperand op2, Operand op1, int delta, boolean upper, Reason reason
+) {
+  exists(IRGuardCondition guard, boolean testIsTrue |
+    guard = boundFlowCond(valueNumberOfOperand(op2), op1, delta, upper, testIsTrue) and
+    guard.controls(op2.getInstruction().getBlock(), testIsTrue) and
+    reason = TCondReason(guard)
+  )
+}
+
+/**
+ * Gets a condition that tests whether `vn` is bounded by `bound + delta`.
+ *
+ * If the condition evaluates to `testIsTrue`:
+ * - `upper = true`  : `vn <= bound + delta`
+ * - `upper = false` : `vn >= bound + delta`
+ */
+private IRGuardCondition boundFlowCond(ValueNumber vn, NonPhiOperand bound, int delta, boolean upper,
+  boolean testIsTrue)
+{
+  exists(int d |
+    result.comparesLt(vn.getAUse(), bound, d, upper, testIsTrue) and
+    // `comparesLt` provides bounds of the form `x < y + k` or `x >= y + k`, but we need
+    // `x <= y + k` so we strengthen here. `testIsTrue` has the same semantics in `comparesLt` as
+    // it does here, so we don't need to account for it.
+    if upper = true
+      then delta = d-1
+      else delta = d
+  )
+  or
+  result = eqFlowCond(vn, bound, delta, true, testIsTrue) and
+  (upper = true or upper = false)
+}
+
+private newtype TReason =
+  TNoReason() or
+  TCondReason(IRGuardCondition guard) { possibleReason(guard) }
+
+/**
+ * A reason for an inferred bound. This can either be `CondReason` if the bound
+ * is due to a specific condition, or `NoReason` if the bound is inferred
+ * without going through a bounding condition.
+ */
+abstract class Reason extends TReason {
+  abstract string toString();
+}
+class NoReason extends Reason, TNoReason {
+  override string toString() { result = "NoReason" }
+}
+class CondReason extends Reason, TCondReason {
+  IRGuardCondition getCond() { this = TCondReason(result) }
+  override string toString() { result = getCond().toString() }
+}
+
+/**
+ * Holds if a cast from `fromtyp` to `totyp` can be ignored for the purpose of
+ * range analysis.
+ */
+pragma[inline]
+private predicate safeCast(IntegralType fromtyp, IntegralType totyp) {
+  fromtyp.getSize() < totyp.getSize() and
+  (
+    fromtyp.isUnsigned()
+    or
+    totyp.isSigned()
+  ) or
+  fromtyp.getSize() <= totyp.getSize() and
+  (
+    fromtyp.isSigned() and
+    totyp.isSigned()
+    or
+    fromtyp.isUnsigned() and
+    totyp.isUnsigned()
+  )
+}
+
+private class SafeCastInstruction extends ConvertInstruction {
+  SafeCastInstruction() {
+    safeCast(getResultType(), getOperand().getResultType())
+    or
+    getResultType() instanceof PointerType and
+    getOperand().getResultType() instanceof PointerType
+  }
+}
+
+/**
+ * Holds if `typ` is a small integral type with the given lower and upper bounds.
+ */
+private predicate typeBound(IntegralType typ, int lowerbound, int upperbound) {
+  typ.isSigned() and typ.getSize() = 1 and lowerbound = -128 and upperbound = 127
+  or
+  typ.isUnsigned() and typ.getSize() = 1 and lowerbound = 0 and upperbound = 255
+  or
+  typ.isSigned() and typ.getSize() = 2 and lowerbound = -32768 and upperbound = 32767
+  or
+  typ.isUnsigned() and typ.getSize() = 2 and lowerbound = 0 and upperbound = 65535
+}
+
+/**
+ * A cast to a small integral type that may overflow or underflow.
+ */
+private class NarrowingCastInstruction extends ConvertInstruction {
+  NarrowingCastInstruction() {
+    not this instanceof SafeCastInstruction and
+    typeBound(getResultType(), _, _)
+  }
+  /** Gets the lower bound of the resulting type. */
+  int getLowerBound() { typeBound(getResultType(), result, _) }
+  /** Gets the upper bound of the resulting type. */
+  int getUpperBound() { typeBound(getResultType(), _, result) }
+}
+
+/**
+ * Holds if `op + delta` is a valid bound for `i`.
+ * - `upper = true`  : `i <= op + delta`
+ * - `upper = false` : `i >= op + delta`
+ */
+private predicate boundFlowStep(Instruction i, NonPhiOperand op, int delta, boolean upper) {
+  valueFlowStep(i, op, delta) and
+  (upper = true or upper = false)
+  or
+  i.(SafeCastInstruction).getAnOperand() = op and
+  delta = 0 and
+  (upper = true or upper = false)
+  or
+  exists(Operand x |
+    i.(AddInstruction).getAnOperand() = op and
+    i.(AddInstruction).getAnOperand() = x and
+    op != x
+    |
+    not exists(getValue(getConstantValue(op.getInstruction()))) and
+    not exists(getValue(getConstantValue(x.getInstruction()))) and
+    if(strictlyPositive(x))
+    then (
+      upper = false and delta = 1
+    ) else
+      if positive(x)
+      then (
+        upper = false and delta = 0
+      ) else
+        if strictlyNegative(x)
+        then (
+          upper = true and delta = -1
+        ) else if negative(x) then (upper = true and delta = 0) else none()
+  )
+  or
+  exists(Operand x |
+    exists(SubInstruction sub |
+      i = sub and
+      sub.getAnOperand().(LeftOperand) = op and
+      sub.getAnOperand().(RightOperand) = x
+    )
+  |
+    // `x` with constant value is covered by valueFlowStep
+    not exists(getValue(getConstantValue(x.getInstruction()))) and
+    if strictlyPositive(x)
+    then (
+      upper = true and delta = -1
+    ) else
+      if positive(x)
+      then (
+        upper = true and delta = 0
+      ) else
+        if strictlyNegative(x)
+        then (
+          upper = false and delta = 1
+        ) else if negative(x) then (upper = false and delta = 0) else none()
+  )
+  or
+  i.(RemInstruction).getAnOperand().(RightOperand) = op and positive(op) and delta = -1 and upper = true
+  or
+  i.(RemInstruction).getAnOperand().(LeftOperand) = op and positive(op) and delta = 0 and upper = true
+  or
+  i.(BitAndInstruction).getAnOperand() = op and positive(op) and delta = 0 and upper = true
+  or
+  i.(BitOrInstruction).getAnOperand() = op and positiveInstruction(i) and delta = 0 and upper = false
+  // TODO: min, max, rand
+}
+
+private predicate boundFlowStepMul(Instruction i1, Operand op, int factor) {
+  exists(Instruction c, int k | k = getValue(getConstantValue(c)) and k > 0 |
+    i1.(MulInstruction).hasOperands(op, c.getAUse()) and factor = k
+    or
+    exists(ShiftLeftInstruction i |
+      i = i1 and i.getAnOperand().(LeftOperand) = op and i.getAnOperand().(RightOperand) = c.getAUse() and factor = 2.pow(k)
+    )
+  )
+}
+
+private predicate boundFlowStepDiv(Instruction i1, Operand op, int factor) {
+  exists(Instruction c, int k | k = getValue(getConstantValue(c)) and k > 0 |
+    exists(DivInstruction i |
+      i = i1 and i.getAnOperand().(LeftOperand) = op and i.getRightOperand() = c and factor = k
+    )
+    or
+    exists(ShiftRightInstruction i |
+      i = i1 and i.getAnOperand().(LeftOperand) = op and i.getRightOperand() = c and factor = 2.pow(k)
+    )
+  )
+}
+
+/**
+ * Holds if `b` is a valid bound for `op`
+ */
+pragma[noinline]
+private predicate boundedNonPhiOperand(NonPhiOperand op, Bound b, int delta, boolean upper,
+  boolean fromBackEdge, int origdelta, Reason reason
+) {
+  exists(NonPhiOperand op2, int d1, int d2 |
+    boundFlowStepSsa(op, op2, d1, upper, reason) and
+    boundedNonPhiOperand(op2, b, d2, upper, fromBackEdge, origdelta, _) and
+    delta = d1 + d2
+  )
+  or
+  boundedInstruction(op.getDefinitionInstruction(), b, delta, upper, fromBackEdge, origdelta, reason)
+  or
+  exists(int d, Reason r1, Reason r2 |
+    boundedNonPhiOperand(op, b, d, upper, fromBackEdge, origdelta, r2)
+  |
+    unequalOperand(op, b, d, r1) and
+    (
+      upper = true and delta = d - 1
+      or upper = false and delta = d + 1
+    ) and
+    (
+      reason = r1
+      or
+      reason = r2 and not r2 instanceof NoReason
+    )
+  )
+}
+
+/**
+ * Holds if `op1 + delta` is a valid bound for `op2`.
+ * - `upper = true`  : `op2 <= op1 + delta`
+ * - `upper = false` : `op2 >= op1 + delta`
+ */
+private predicate boundFlowStepPhi(
+  PhiOperand op2, Operand op1, int delta, boolean upper, Reason reason
+) {
+  op2.getDefinitionInstruction().getAnOperand().(CopySourceOperand) = op1 and
+  (upper = true or upper = false) and
+  reason = TNoReason() and
+  delta = 0
+  or
+  exists(IRGuardCondition guard, boolean testIsTrue |
+    guard = boundFlowCond(valueNumberOfOperand(op2), op1, delta, upper, testIsTrue) and
+    (
+      guard.hasBranchEdge(op2.getPredecessorBlock().getLastInstruction(), op2.getInstruction().getBlock(), testIsTrue)
+      or
+      guard.controls(op2.getPredecessorBlock(), testIsTrue)
+    ) and
+    reason = TCondReason(guard)
+  )
+}
+
+
+private predicate boundedPhiOperand(
+  PhiOperand op, Bound b, int delta, boolean upper, boolean fromBackEdge, int origdelta,
+  Reason reason
+) {
+  exists(NonPhiOperand op2, int d1, int d2, Reason r1, Reason r2 |
+    boundFlowStepPhi(op, op2, d1, upper, r1) and
+    boundedNonPhiOperand(op2, b, d2, upper, fromBackEdge, origdelta, r2) and
+    delta = d1 + d2 and
+    (if r1 instanceof NoReason then reason = r2 else reason = r1)
+  )
+  or
+  boundedInstruction(op.getDefinitionInstruction(), b, delta, upper, fromBackEdge, origdelta, reason)
+  or
+  exists(int d, Reason r1, Reason r2 |
+    boundedInstruction(op.getDefinitionInstruction(), b, d, upper, fromBackEdge, origdelta, r2)
+  |
+    unequalOperand(op, b, d, r1) and
+    (
+      upper = true and delta = d - 1
+      or upper = false and delta = d + 1
+    ) and
+    (
+      reason = r1
+      or
+      reason = r2 and not r2 instanceof NoReason
+    )
+  )
+}
+
+/** Holds if `op2 != op1 + delta` at `pos`. */
+private predicate unequalFlowStep(
+ Operand op2, Operand op1, int delta, Reason reason
+) {
+  exists(IRGuardCondition guard, boolean testIsTrue |
+    guard = eqFlowCond(valueNumberOfOperand(op2), op1, delta, false, testIsTrue) and
+    guard.controls(op2.getInstruction().getBlock(), testIsTrue) and
+    reason = TCondReason(guard)
+  )
+}
+
+/**
+ * Holds if `op != b + delta` at `pos`.
+ */
+private predicate unequalOperand(Operand op, Bound b, int delta, Reason reason) {
+  exists(Operand op2, int d1, int d2 |
+    unequalFlowStep(op, op2, d1, reason) and
+    boundedNonPhiOperand(op2, b, d2, true, _, _, _) and
+    boundedNonPhiOperand(op2, b, d2, false, _, _, _) and
+    delta = d1 + d2
+  )
+}
+
+private predicate boundedPhiCandValidForEdge(
+  PhiInstruction phi, Bound b, int delta, boolean upper, boolean fromBackEdge, int origdelta,
+  Reason reason, PhiOperand op
+) {
+  boundedPhiCand(phi, upper, b, delta, fromBackEdge, origdelta, reason) and
+  (
+    exists(int d | boundedPhiInp1(phi, op, b, d, upper) | upper = true and d <= delta)
+    or
+    exists(int d | boundedPhiInp1(phi, op, b, d, upper) | upper = false and d >= delta)
+    or
+    selfBoundedPhiInp(phi, op, upper)
+  )
+}
+
+/** Weakens a delta to lie in the range `[-1..1]`. */
+bindingset[delta, upper]
+private int weakenDelta(boolean upper, int delta) {
+  delta in [-1 .. 1] and result = delta
+  or
+  upper = true and result = -1 and delta < -1
+  or
+  upper = false and result = 1 and delta > 1
+}
+
+private predicate boundedPhiInp(
+  PhiInstruction phi, PhiOperand op, Bound b, int delta, boolean upper, boolean fromBackEdge,
+  int origdelta, Reason reason
+) {
+  phi.getAnOperand() = op and
+  exists(int d, boolean fromBackEdge0 |
+    boundedPhiOperand(op, b, d, upper, fromBackEdge0, origdelta, reason)
+    or
+    b.(ValueNumberBound).getInstruction() = op.getDefinitionInstruction() and
+    d = 0 and
+    (upper = true or upper = false) and
+    fromBackEdge0 = false and
+    origdelta = 0 and
+    reason = TNoReason()
+  |
+    if backEdge(phi, op)
+    then
+      fromBackEdge = true and
+      (
+        fromBackEdge0 = true and delta = weakenDelta(upper, d - origdelta) + origdelta
+        or
+        fromBackEdge0 = false and delta = d
+      )
+    else (
+      delta = d and fromBackEdge = fromBackEdge0
+    )
+  )
+}
+
+pragma[noinline]
+private predicate boundedPhiInp1(
+  PhiInstruction phi, PhiOperand op, Bound b, int delta, boolean upper
+) {
+  boundedPhiInp(phi, op, b, delta, upper, _, _, _)
+}
+
+private predicate selfBoundedPhiInp(PhiInstruction phi, PhiOperand op, boolean upper) {
+  exists(int d, ValueNumberBound phibound |
+    phibound.getInstruction() = phi and
+    boundedPhiInp(phi, op, phibound, d, upper, _, _, _) and
+    (
+      upper = true and d <= 0
+      or
+      upper = false and d >= 0
+    )
+  )
+}
+
+pragma[noinline]
+private predicate boundedPhiCand(
+  PhiInstruction phi, boolean upper, Bound b, int delta, boolean fromBackEdge, int origdelta,
+  Reason reason
+) {
+  exists(PhiOperand op |
+    boundedPhiInp(phi, op, b, delta, upper, fromBackEdge, origdelta, reason)
+  )
+}
+
+/**
+ * Holds if the value being cast has an upper (for `upper = true`) or lower
+ * (for `upper = false`) bound within the bounds of the resulting type.
+ * For `upper = true` this means that the cast will not overflow and for
+ * `upper = false` this means that the cast will not underflow.
+ */
+private predicate safeNarrowingCast(NarrowingCastInstruction cast, boolean upper) {
+  exists(int bound | boundedNonPhiOperand(cast.getAnOperand(), any(ZeroBound zb), bound, upper, _, _, _) |
+    upper = true and bound <= cast.getUpperBound()
+    or
+    upper = false and bound >= cast.getLowerBound()
+  )
+}
+
+pragma[noinline]
+private predicate boundedCastExpr(
+  NarrowingCastInstruction cast, Bound b, int delta, boolean upper, boolean fromBackEdge, int origdelta,
+  Reason reason
+) {
+  boundedNonPhiOperand(cast.getAnOperand(), b, delta, upper, fromBackEdge, origdelta, reason)
+}
+/**
+ * Holds if `b + delta` is a valid bound for `i`.
+ * - `upper = true`  : `i <= b + delta`
+ * - `upper = false` : `i >= b + delta`
+ */
+private predicate boundedInstruction(
+  Instruction i, Bound b, int delta, boolean upper, boolean fromBackEdge, int origdelta, Reason reason
+) {
+  isReducibleCFG(i.getFunction()) and
+  (
+    i instanceof PhiInstruction and
+    forex(PhiOperand op | op = i.getAnOperand() |
+      boundedPhiCandValidForEdge(i, b, delta, upper, fromBackEdge, origdelta, reason, op)
+    )
+    or
+    i = b.getInstruction(delta) and
+    (upper = true or upper = false) and
+    fromBackEdge = false and
+    origdelta = delta and
+    reason = TNoReason()
+    or
+    exists(Operand mid, int d1, int d2 |
+      boundFlowStep(i, mid, d1, upper) and
+      boundedNonPhiOperand(mid, b, d2, upper, fromBackEdge, origdelta, reason) and
+      delta = d1 + d2 and
+      not exists(getValue(getConstantValue(i)))
+    )
+    or
+    exists(Operand mid, int factor, int d |
+      boundFlowStepMul(i, mid, factor) and
+      boundedNonPhiOperand(mid, b, d, upper, fromBackEdge, origdelta, reason) and
+      b instanceof ZeroBound and
+      delta = d*factor and
+      not exists(getValue(getConstantValue(i)))
+    )
+    or
+    exists(Operand mid, int factor, int d |
+      boundFlowStepDiv(i, mid, factor) and
+      boundedNonPhiOperand(mid, b, d, upper, fromBackEdge, origdelta, reason) and
+      d >= 0 and
+      b instanceof ZeroBound and
+      delta = d / factor and
+      not exists(getValue(getConstantValue(i)))
+    )
+    or
+    exists(NarrowingCastInstruction cast |
+      cast = i and
+      safeNarrowingCast(cast, upper.booleanNot()) and
+      boundedCastExpr(cast, b, delta, upper, fromBackEdge, origdelta, reason)
+    )
+  )
+}

cpp/ql/src/semmle/code/cpp/rangeanalysis/RangeUtils.qll

@@ -0,0 +1,86 @@
+import cpp
+
+private import semmle.code.cpp.ir.IR
+// TODO: move this dependency
+import semmle.code.cpp.ir.internal.IntegerConstant
+
+// TODO: move this out of test code
+language[monotonicAggregates]
+IntValue getConstantValue(Instruction instr) {
+  result = instr.(IntegerConstantInstruction).getValue().toInt() or
+  exists(BinaryInstruction binInstr, IntValue left, IntValue right |
+    binInstr = instr and
+    left = getConstantValue(binInstr.getLeftOperand()) and
+    right = getConstantValue(binInstr.getRightOperand()) and
+    (
+      binInstr instanceof AddInstruction and result = add(left, right) or
+      binInstr instanceof SubInstruction and result = sub(left, right) or
+      binInstr instanceof MulInstruction and result = mul(left, right) or
+      binInstr instanceof DivInstruction and result = div(left, right)
+    )
+  ) or
+  result = getConstantValue(instr.(CopyInstruction).getSourceValue()) or
+  exists(PhiInstruction phi |
+    phi = instr and
+    result = max(PhiOperand operand | operand = phi.getAnOperand() | getConstantValue(operand.getDefinitionInstruction())) and
+    result = min(PhiOperand operand | operand = phi.getAnOperand() | getConstantValue(operand.getDefinitionInstruction()))
+  )
+}
+
+predicate valueFlowStep(Instruction i, Operand op, int delta) {
+  i.getAnOperand().(CopySourceOperand) = op and delta = 0
+  or
+  exists(Operand x |
+    i.(AddInstruction).getAnOperand() = op and
+    i.(AddInstruction).getAnOperand() = x and
+    op  != x
+    |
+    delta = getValue(getConstantValue(x.getDefinitionInstruction()))
+  )
+  or
+  exists(Operand x |
+    i.(SubInstruction).getAnOperand().(LeftOperand) = op and
+    i.(SubInstruction).getAnOperand().(RightOperand) = x
+    |
+    delta = -getValue(getConstantValue(x.getDefinitionInstruction()))
+  )
+  or
+  exists(Operand x |
+    i.(PointerAddInstruction).getAnOperand() = op and
+    i.(PointerAddInstruction).getAnOperand() = x and
+    op  != x
+    |
+    delta = i.(PointerAddInstruction).getElementSize() *
+      getValue(getConstantValue(x.getDefinitionInstruction()))
+  )
+  or
+  exists(Operand x |
+    i.(PointerSubInstruction).getAnOperand().(LeftOperand) = op and
+    i.(PointerSubInstruction).getAnOperand().(RightOperand) = x
+    |
+    delta = i.(PointerSubInstruction).getElementSize() * 
+      -getValue(getConstantValue(x.getDefinitionInstruction()))
+  )
+}
+
+predicate isReducibleCFG(Function f) {
+  not exists(LabelStmt l, GotoStmt goto |
+    goto.getTarget() = l and
+    l.getLocation().isBefore(goto.getLocation()) and
+    l.getEnclosingFunction() = f
+  ) and
+  not exists(LabelStmt ls, Loop l |
+    ls.getParent*() = l and
+    l.getEnclosingFunction() = f
+  ) and
+  not exists(SwitchCase cs |
+    cs.getSwitchStmt().getStmt() != cs.getParentStmt() and
+    cs.getEnclosingFunction() = f
+  )
+}
+
+predicate backEdge(PhiInstruction phi, PhiOperand op) {
+  phi.getAnOperand() = op and
+  phi.getBlock().dominates(op.getPredecessorBlock())
+  // TODO: identify backedges during IR construction
+}

cpp/ql/src/semmle/code/cpp/valuenumbering/GlobalValueNumbering.qll

@@ -3,8 +3,8 @@
  * See https://en.wikipedia.org/wiki/Global_value_numbering
  *
  * The predicate `globalValueNumber` converts an expression into a `GVN`,
- * which is an abstract type presenting the value of the expression. If
- * two expressions have the `GVN` then they compute the same value.
+ * which is an abstract type representing the value of the expression. If
+ * two expressions have the same `GVN` then they compute the same value.
  * For example:
  *
  * ```

cpp/ql/src/semmlecode.cpp.dbscheme

@@ -1039,6 +1039,10 @@ namespaces(
     string name: string ref
 );
 
+namespace_inline(
+    unique int id: @namespace ref
+);
+
 namespacembrs(
     int parentid: @namespace ref,
     unique int memberid: @namespacembr ref

cpp/ql/test/library-tests/ir/ir/aliased_ssa_sanity.expected

@@ -3,6 +3,8 @@ unexpectedOperand
 duplicateOperand
 missingPhiOperand
 instructionWithoutSuccessor
+ambiguousSuccessors
+unexplainedLoop
 unnecessaryPhiInstruction
 operandAcrossFunctions
 instructionWithoutUniqueBlock

cpp/ql/test/library-tests/ir/ir/raw_sanity.expected

@@ -3,6 +3,8 @@ unexpectedOperand
 duplicateOperand
 missingPhiOperand
 instructionWithoutSuccessor
+ambiguousSuccessors
+unexplainedLoop
 unnecessaryPhiInstruction
 operandAcrossFunctions
 instructionWithoutUniqueBlock

cpp/ql/test/library-tests/ir/ir/unaliased_ssa_sanity.expected

@@ -3,6 +3,8 @@ unexpectedOperand
 duplicateOperand
 missingPhiOperand
 instructionWithoutSuccessor
+ambiguousSuccessors
+unexplainedLoop
 unnecessaryPhiInstruction
 operandAcrossFunctions
 instructionWithoutUniqueBlock

cpp/ql/test/library-tests/qlcfg/cfg.ql

@@ -1,35 +0,0 @@
-// query-type: graph
-import cpp
-import semmle.code.cpp.controlflow.internal.CFG
-
-class DestructorCallEnhanced extends DestructorCall {
-    override string toString() {
-        if exists(this.getQualifier().(VariableAccess).getTarget().getName())
-        then result = "call to " + this.getQualifier().(VariableAccess).getTarget().getName() + "." + this.getTarget().getName()
-        else result = super.toString()
-    }
-}
-
-string scope(ControlFlowNode x) {
-  if exists(x.getControlFlowScope().getQualifiedName())
-  then result = x.getControlFlowScope().getQualifiedName()
-  else result = "<no scope>"
-}
-
-predicate isNode(boolean isEdge, ControlFlowNode x, ControlFlowNode y, string label) {
-    isEdge = false and x = y and label = x.toString()
-}
-
-predicate isSuccessor(boolean isEdge, ControlFlowNode x, ControlFlowNode y, string label) {
-    exists(string truelabel, string falselabel |
-           isEdge = true
-       and qlCFGSuccessor(x, y)
-       and if qlCFGTrueSuccessor(x, y) then truelabel  = "T" else truelabel  = ""
-       and if qlCFGFalseSuccessor(x, y) then falselabel = "F" else falselabel = ""
-       and label = truelabel + falselabel)
-}
-
-from boolean isEdge, ControlFlowNode x, ControlFlowNode y, string label
-where isNode(isEdge, x, y, label) or isSuccessor(isEdge, x, y, label)
-select scope(x), isEdge, x, y, label
-

cpp/ql/test/library-tests/qlcfg/destructors.cpp

@@ -36,3 +36,23 @@ void destructor_catch() {
     HasDtor d2 = { 0 };
   }
 }
+
+namespace cond_destruct {
+  struct C {
+      C();
+      C(const C&) = delete;
+      ~C();
+      int getInt() const;
+      void *data;
+  };
+
+  int f(int x) {
+      C local;
+      const C &ref = x ? (const C&)C() : (const C&)local;
+      return ref.getInt();
+      // If `x` was true, `ref` refers to a temporary object whose lifetime was
+      // extended to coincide with `ref`. Before the function returns, it
+      // should destruct `ref` if and only if the first branch was taken in the
+      // ?: expression.
+  }
+}

cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/test.cpp

@@ -0,0 +1,70 @@
+template <typename T>
+class vector {
+public:
+  T& operator[](int);
+  const T& operator[](int) const;
+};
+
+int test1(vector<int> vec, int b) {
+  int x = -1;
+  if (b) {
+    x = vec[3];
+  }
+  return x;
+}
+
+// Regression test for ODASA-6013.
+int test2(int x) {
+  int x0 = static_cast<char>(x);
+  return x0;
+}
+
+// Tests for conversion to bool
+bool test3(bool b, int x, int y) {
+  // The purpose the assignments to `x` below is to generate a lot of
+  // potential upper and lower bounds for `x`, so that the logic in
+  // boolConversionLowerBound and boolConversionUpperBound gets exercized.
+  if (y == 0) {
+    x = 0;
+  }
+  if (y == -1) {
+    x = -1;
+  }
+  if (y == 1) {
+    x = 1;
+  }
+  if (y == -128) {
+    x = -128;
+  }
+  if (y == 128) {
+    x = 128;
+  }
+  if (y == -1024) {
+    x = -1024;
+  }
+  if (y == 1024) {
+    x = 1024;
+  }
+
+  int t = 0;
+
+  if (x == 0) {
+    bool xb = (bool)x; // (bool)x == false
+    t += (int)xb;
+  }
+
+  if (x > 0) {
+    bool xb = (bool)x; // (bool)x == true
+    t += (int)xb;
+  }
+
+  if (x < 0) {
+    bool xb = (bool)x; // (bool)x == true
+    t += (int)xb;
+  }
+
+  bool xb = (bool)x; // Value of (bool)x is unknown.
+  t += (int)xb;
+
+  return b || (bool)t;
+}

cpp/ql/test/library-tests/rangeanalysis/rangeanalysis/RangeAnalysis.expected

@@ -0,0 +1,56 @@
+| test.cpp:10:10:10:10 | Store: x | test.cpp:6:15:6:15 | InitializeParameter: x | 0 | false | NoReason | file://:0:0:0:0 | file://:0:0:0:0 |
+| test.cpp:10:10:10:10 | Store: x | test.cpp:6:22:6:22 | InitializeParameter: y | 0 | false | CompareLT: ... < ... | test.cpp:7:7:7:11 | test.cpp:7:7:7:11 |
+| test.cpp:10:10:10:10 | Store: x | test.cpp:6:22:6:22 | InitializeParameter: y | 0 | false | NoReason | file://:0:0:0:0 | file://:0:0:0:0 |
+| test.cpp:20:10:20:10 | Store: x | test.cpp:14:15:14:15 | InitializeParameter: x | -2 | false | NoReason | file://:0:0:0:0 | file://:0:0:0:0 |
+| test.cpp:20:10:20:10 | Store: x | test.cpp:14:22:14:22 | InitializeParameter: y | -2 | false | CompareLT: ... < ... | test.cpp:15:7:15:11 | test.cpp:15:7:15:11 |
+| test.cpp:27:10:27:10 | Load: i | file://:0:0:0:0 | 0 | 0 | false | NoReason | file://:0:0:0:0 | file://:0:0:0:0 |
+| test.cpp:27:10:27:10 | Load: i | test.cpp:24:15:24:15 | InitializeParameter: x | -1 | true | CompareLT: ... < ... | test.cpp:26:14:26:18 | test.cpp:26:14:26:18 |
+| test.cpp:30:10:30:10 | Load: i | file://:0:0:0:0 | 0 | 1 | false | CompareGT: ... > ... | test.cpp:29:14:29:18 | test.cpp:29:14:29:18 |
+| test.cpp:30:10:30:10 | Load: i | test.cpp:24:15:24:15 | InitializeParameter: x | 0 | true | NoReason | file://:0:0:0:0 | file://:0:0:0:0 |
+| test.cpp:30:10:30:10 | Load: i | test.cpp:26:14:26:14 | Phi: i | 0 | true | CompareLT: ... < ... | test.cpp:26:14:26:18 | test.cpp:26:14:26:18 |
+| test.cpp:33:10:33:10 | Load: i | file://:0:0:0:0 | 0 | 0 | false | NoReason | file://:0:0:0:0 | file://:0:0:0:0 |
+| test.cpp:33:10:33:10 | Load: i | test.cpp:24:15:24:15 | InitializeParameter: x | 1 | true | CompareLT: ... < ... | test.cpp:32:14:32:22 | test.cpp:32:14:32:22 |
+| test.cpp:33:10:33:10 | Load: i | test.cpp:26:14:26:14 | Phi: i | 1 | true | CompareLT: ... < ... | test.cpp:32:14:32:22 | test.cpp:32:14:32:22 |
+| test.cpp:33:10:33:10 | Load: i | test.cpp:29:14:29:14 | Phi: i | 0 | false | CompareGT: ... > ... | test.cpp:29:14:29:18 | test.cpp:29:14:29:18 |
+| test.cpp:40:10:40:14 | Load: begin | test.cpp:38:16:38:20 | InitializeParameter: begin | 0 | false | NoReason | file://:0:0:0:0 | file://:0:0:0:0 |
+| test.cpp:40:10:40:14 | Load: begin | test.cpp:38:28:38:30 | InitializeParameter: end | -1 | true | CompareLT: ... < ... | test.cpp:39:10:39:20 | test.cpp:39:10:39:20 |
+| test.cpp:49:12:49:12 | Load: x | test.cpp:46:22:46:22 | InitializeParameter: y | -1 | true | CompareLT: ... < ... | test.cpp:48:9:48:13 | test.cpp:48:9:48:13 |
+| test.cpp:49:12:49:12 | Load: x | test.cpp:46:29:46:29 | InitializeParameter: z | -2 | true | CompareLT: ... < ... | test.cpp:48:9:48:13 | test.cpp:48:9:48:13 |
+| test.cpp:54:12:54:12 | Load: x | test.cpp:46:22:46:22 | InitializeParameter: y | -1 | true | CompareLT: ... < ... | test.cpp:52:7:52:11 | test.cpp:52:7:52:11 |
+| test.cpp:62:10:62:13 | Load: iter | test.cpp:60:17:60:17 | InitializeParameter: p | 0 | false | NoReason | file://:0:0:0:0 | file://:0:0:0:0 |
+| test.cpp:62:10:62:13 | Load: iter | test.cpp:60:17:60:17 | InitializeParameter: p | 3 | true | CompareLT: ... < ... | test.cpp:61:32:61:51 | test.cpp:61:32:61:51 |
+| test.cpp:62:10:62:13 | Load: iter | test.cpp:61:39:61:51 | Convert: (char *)... | -1 | true | CompareLT: ... < ... | test.cpp:61:32:61:51 | test.cpp:61:32:61:51 |
+| test.cpp:67:10:67:13 | Load: iter | test.cpp:60:17:60:17 | InitializeParameter: p | 0 | false | NoReason | file://:0:0:0:0 | file://:0:0:0:0 |
+| test.cpp:67:10:67:13 | Load: iter | test.cpp:60:17:60:17 | InitializeParameter: p | 3 | true | CompareLT: ... < ... | test.cpp:66:32:66:41 | test.cpp:66:32:66:41 |
+| test.cpp:67:10:67:13 | Load: iter | test.cpp:61:32:61:35 | Phi: iter | -1 | true | CompareLT: ... < ... | test.cpp:66:32:66:41 | test.cpp:66:32:66:41 |
+| test.cpp:67:10:67:13 | Load: iter | test.cpp:61:39:61:51 | Convert: (char *)... | -1 | true | CompareLT: ... < ... | test.cpp:66:32:66:41 | test.cpp:66:32:66:41 |
+| test.cpp:77:12:77:12 | Load: i | file://:0:0:0:0 | 0 | 0 | false | NoReason | file://:0:0:0:0 | file://:0:0:0:0 |
+| test.cpp:77:12:77:12 | Load: i | test.cpp:72:15:72:15 | InitializeParameter: x | -1 | true | CompareLT: ... < ... | test.cpp:76:20:76:24 | test.cpp:76:20:76:24 |
+| test.cpp:77:12:77:12 | Load: i | test.cpp:72:22:72:22 | InitializeParameter: y | -1 | true | CompareLT: ... < ... | test.cpp:76:20:76:24 | test.cpp:76:20:76:24 |
+| test.cpp:85:10:85:10 | Load: x | file://:0:0:0:0 | 0 | 2 | false | CompareGT: ... > ... | test.cpp:84:7:84:11 | test.cpp:84:7:84:11 |
+| test.cpp:87:10:87:10 | Load: x | file://:0:0:0:0 | 0 | 1 | true | CompareGT: ... > ... | test.cpp:84:7:84:11 | test.cpp:84:7:84:11 |
+| test.cpp:90:10:90:10 | Load: x | file://:0:0:0:0 | 0 | 1 | false | CompareGE: ... >= ... | test.cpp:89:7:89:12 | test.cpp:89:7:89:12 |
+| test.cpp:92:10:92:10 | Load: x | file://:0:0:0:0 | 0 | 0 | true | CompareGE: ... >= ... | test.cpp:89:7:89:12 | test.cpp:89:7:89:12 |
+| test.cpp:95:10:95:10 | Load: x | file://:0:0:0:0 | 0 | 0 | true | CompareLT: ... < ... | test.cpp:94:7:94:11 | test.cpp:94:7:94:11 |
+| test.cpp:97:10:97:10 | Load: x | file://:0:0:0:0 | 0 | 1 | false | CompareLT: ... < ... | test.cpp:94:7:94:11 | test.cpp:94:7:94:11 |
+| test.cpp:100:10:100:10 | Load: x | file://:0:0:0:0 | 0 | 1 | true | CompareLE: ... <= ... | test.cpp:99:7:99:12 | test.cpp:99:7:99:12 |
+| test.cpp:102:10:102:10 | Load: x | file://:0:0:0:0 | 0 | 2 | false | CompareLE: ... <= ... | test.cpp:99:7:99:12 | test.cpp:99:7:99:12 |
+| test.cpp:107:5:107:10 | Phi: test10 | test.cpp:114:3:114:6 | Phi: call to sink | -1 | true | CompareLT: ... < ... | test.cpp:115:18:115:22 | test.cpp:115:18:115:22 |
+| test.cpp:140:10:140:10 | Store: i | file://:0:0:0:0 | 0 | 1 | false | NoReason | file://:0:0:0:0 | file://:0:0:0:0 |
+| test.cpp:140:10:140:10 | Store: i | test.cpp:135:16:135:16 | InitializeParameter: x | 0 | false | CompareLT: ... < ... | test.cpp:139:11:139:15 | test.cpp:139:11:139:15 |
+| test.cpp:140:10:140:10 | Store: i | test.cpp:138:5:138:5 | Phi: i | 1 | false | NoReason | file://:0:0:0:0 | file://:0:0:0:0 |
+| test.cpp:140:10:140:10 | Store: i | test.cpp:138:5:138:5 | Phi: i | 1 | true | NoReason | file://:0:0:0:0 | file://:0:0:0:0 |
+| test.cpp:156:12:156:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | -1 | false | CompareEQ: ... == ... | test.cpp:155:9:155:16 | test.cpp:155:9:155:16 |
+| test.cpp:156:12:156:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | -1 | true | CompareEQ: ... == ... | test.cpp:155:9:155:16 | test.cpp:155:9:155:16 |
+| test.cpp:156:12:156:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | -1 | true | CompareLT: ... < ... | test.cpp:154:6:154:10 | test.cpp:154:6:154:10 |
+| test.cpp:158:12:158:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | -2 | true | CompareEQ: ... == ... | test.cpp:155:9:155:16 | test.cpp:155:9:155:16 |
+| test.cpp:158:12:158:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | -2 | true | CompareLT: ... < ... | test.cpp:154:6:154:10 | test.cpp:154:6:154:10 |
+| test.cpp:161:12:161:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | -2 | true | CompareLT: ... < ... | test.cpp:154:6:154:10 | test.cpp:154:6:154:10 |
+| test.cpp:161:12:161:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | -2 | true | CompareNE: ... != ... | test.cpp:160:9:160:16 | test.cpp:160:9:160:16 |
+| test.cpp:163:12:163:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | -1 | false | CompareNE: ... != ... | test.cpp:160:9:160:16 | test.cpp:160:9:160:16 |
+| test.cpp:163:12:163:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | -1 | true | CompareLT: ... < ... | test.cpp:154:6:154:10 | test.cpp:154:6:154:10 |
+| test.cpp:163:12:163:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | -1 | true | CompareNE: ... != ... | test.cpp:160:9:160:16 | test.cpp:160:9:160:16 |
+| test.cpp:167:12:167:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | 0 | false | CompareLT: ... < ... | test.cpp:154:6:154:10 | test.cpp:154:6:154:10 |
+| test.cpp:167:12:167:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | 1 | false | CompareEQ: ... == ... | test.cpp:166:9:166:16 | test.cpp:166:9:166:16 |
+| test.cpp:167:12:167:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | 1 | true | CompareEQ: ... == ... | test.cpp:166:9:166:16 | test.cpp:166:9:166:16 |
+| test.cpp:169:12:169:12 | Load: x | test.cpp:153:23:153:23 | InitializeParameter: y | 0 | false | CompareLT: ... < ... | test.cpp:154:6:154:10 | test.cpp:154:6:154:10 |

cpp/ql/test/library-tests/rangeanalysis/rangeanalysis/RangeAnalysis.ql

@@ -0,0 +1,25 @@
+import semmle.code.cpp.rangeanalysis.RangeAnalysis
+import semmle.code.cpp.ir.IR
+import semmle.code.cpp.controlflow.IRGuards
+import semmle.code.cpp.ir.ValueNumbering
+
+query predicate instructionBounds(Instruction i, Bound b, int delta, boolean upper, Reason reason, 
+  Location reasonLoc)
+{
+  (
+    i.getAUse() instanceof ArgumentOperand
+    or
+    i.getAUse() instanceof ReturnValueOperand
+  ) and
+  (
+    upper = true and
+    delta = min(int d | boundedInstruction(i, b, d, upper, reason))
+    or
+    upper = false and
+    delta = max(int d | boundedInstruction(i, b, d, upper, reason))
+  ) and
+  not valueNumber(b.getInstruction()) = valueNumber(i)
+  and if reason instanceof CondReason
+    then reasonLoc = reason.(CondReason).getCond().getLocation()
+    else reasonLoc instanceof UnknownDefaultLocation
+}

cpp/ql/test/library-tests/rangeanalysis/rangeanalysis/test.cpp

@@ -1,70 +1,172 @@
-template <typename T>
-class vector {
-public:
-  T& operator[](int);
-  const T& operator[](int) const;
-};
 
-int test1(vector<int> vec, int b) {
-  int x = -1;
-  if (b) {
-    x = vec[3];
+void sink(...);
+int source();
+
+// Guards, inference, critical edges
+int test1(int x, int y) {
+  if (x < y) {
+    x = y;
   }
   return x;
 }
 
-// Regression test for ODASA-6013.
-int test2(int x) {
-  int x0 = static_cast<char>(x);
-  return x0;
+// Bounds mergers at phi nodes
+int test2(int x, int y) {
+  if (x < y) {
+    x = y;
+  } else {
+    x = x-2;
+  }
+  return x;
 }
 
-// Tests for conversion to bool
-bool test3(bool b, int x, int y) {
-  // The purpose the assignments to `x` below is to generate a lot of
-  // potential upper and lower bounds for `x`, so that the logic in
-  // boolConversionLowerBound and boolConversionUpperBound gets exercized.
-  if (y == 0) {
-    x = 0;
+// for loops
+int test3(int x) {
+  int i;
+  for(i = 0; i < x; i++) {
+    sink(i);
   }
-  if (y == -1) {
-    x = -1;
+  for(i = x; i > 0; i--) {
+    sink(i);
   }
-  if (y == 1) {
-    x = 1;
+  for(i = 0; i < x + 2; i++) {
+    sink(i);
+  }
+}
+
+// pointer bounds
+int test4(int *begin, int *end) {
+  while (begin < end) {
+    sink(begin);
+    begin++;
+  }
+}
+
+// bound propagation through conditionals
+int test5(int x, int y, int z) {
+  if (y < z) {
+    if (x < y) {
+      sink(x);
+    }
+  }
+  if (x < y) {
+    if (y < z) {
+      sink(x); // x < z is not inferred here
+    }
+  }
+}
+
+// pointer arithmetic and sizes
+void test6(int *p) {
+  for (char *iter = (char *)p; iter < (char *)(p+1); iter++) {
+    sink(iter);
+  }
+
+  char *end = (char *)(p+1);
+  for (char *iter = (char *)p; iter < end; iter++) {
+    sink(iter);
+  }
+}
+
+// inference from equality
+int test8(int x, int y) {
+  int *p = new int[x];
+
+  if (x == y) {
+    for(int i = 0; i < y; ++i) {
+      sink(i);
+    }
+  }
+}
+
+// >, >=, <=
+void test9(int x) {
+  if (x > 1) {
+    sink(x);
+  } else {
+    sink(x);
+  }
+  if (x >= 1) {
+    sink(x);
+  } else {
+    sink(x);
+  }
+  if (x < 1) {
+    sink(x);
+  } else {
+    sink(x);
+  }
+  if (x <= 1) {
+    sink(x);
+  } else {
+    sink(x);
+  }
+}
+
+// Phi nodes as bounds
+int test10(int y, int z, bool use_y) {
+  int x;
+  if(use_y) {
+    x = y;
+  } else {
+    x = z;
+  }
+  sink();
+  for(int i = 0; i < x; i++) {
+    return i;
+  }
+}
+
+// Irreducible CFGs
+int test11(int y, int x) {
+  int i = 0;
+  if (x < y) {
+    x = y;
+  } else {
+    goto inLoop;
+  }
+  for(i = 0; i < x; i++) {
+    inLoop:
+    sink(i);
+  }
+}
+
+// do-while
+int test12(int x) {
+  int i = 0;
+  do {
+    i++;
+  } while(i < x);
+  return i;
+}
+
+// do while false
+int test13(int x) {
+  int i = 0;
+  do {
+    i++;
+  } while(false);
+  return i;
+}
+
+// unequal bound narrowing
+int test14(int x, int y) {
+  if(x < y) {
+    if (x == y-1) {
+      sink(x);
+    } else {
+      sink(x);
+    }
+    if (x != y-1) {
+      sink(x);
+    } else {
+      sink(x);
+    }
+  } else {
+    if (y == x-1) {
+      sink(x);
+    } else {
+      sink(x);
+    }
   }
-  if (y == -128) {
-    x = -128;
-  }
-  if (y == 128) {
-    x = 128;
-  }
-  if (y == -1024) {
-    x = -1024;
-  }
-  if (y == 1024) {
-    x = 1024;
-  }
-
-  int t = 0;
-
-  if (x == 0) {
-    bool xb = (bool)x; // (bool)x == false
-    t += (int)xb;
-  }
-
-  if (x > 0) {
-    bool xb = (bool)x; // (bool)x == true
-    t += (int)xb;
-  }
-
-  if (x < 0) {
-    bool xb = (bool)x; // (bool)x == true
-    t += (int)xb;
-  }
-
-  bool xb = (bool)x; // Value of (bool)x is unknown.
-  t += (int)xb;
-
-  return b || (bool)t;
 }

cpp/ql/test/library-tests/rangeanalysis/signanalysis/SignAnalysis.expected

@@ -65,9 +65,9 @@
 | test.c:34:5:34:14 | Store: ... += ... | positive |
 | test.c:34:14:34:14 | Load: i | positive |
 | test.c:36:10:36:14 | Load: total | positive |
-| test.c:36:10:36:18 | Add: ... + ... | positive |
-| test.c:36:10:36:18 | Store: ... + ... | positive |
-| test.c:36:18:36:18 | Load: i | positive |
+| test.c:36:10:36:18 | Add: ... + ... | positive strictlyPositive |
+| test.c:36:10:36:18 | Store: ... + ... | positive strictlyPositive |
+| test.c:36:18:36:18 | Load: i | positive strictlyPositive |
 | test.c:42:15:42:15 | Load: i | positive |
 | test.c:42:15:42:15 | Phi: i | positive |
 | test.c:42:15:42:15 | Phi: i | positive |
@@ -81,9 +81,9 @@
 | test.c:43:5:43:14 | Store: ... += ... | positive |
 | test.c:43:14:43:14 | Load: i | positive |
 | test.c:45:10:45:14 | Load: total | positive |
-| test.c:45:10:45:18 | Add: ... + ... | positive |
-| test.c:45:10:45:18 | Store: ... + ... | positive |
-| test.c:45:18:45:18 | Load: i | positive |
+| test.c:45:10:45:18 | Add: ... + ... | positive strictlyPositive |
+| test.c:45:10:45:18 | Store: ... + ... | positive strictlyPositive |
+| test.c:45:18:45:18 | Load: i | positive strictlyPositive |
 | test.c:51:15:51:15 | Load: i | positive |
 | test.c:51:15:51:15 | Phi: i | positive |
 | test.c:51:15:51:15 | Phi: i | positive |
@@ -448,10 +448,10 @@
 | test.c:343:5:343:7 | Constant: ... ++ | positive strictlyPositive |
 | test.c:343:5:343:7 | Load: ... ++ | positive |
 | test.c:343:5:343:7 | Store: ... ++ | positive strictlyPositive |
-| test.c:345:3:345:7 | Store: ... = ... | positive |
-| test.c:345:7:345:7 | Load: i | positive |
+| test.c:345:3:345:7 | Store: ... = ... | positive strictlyPositive |
+| test.c:345:7:345:7 | Load: i | positive strictlyPositive |
 | test.c:346:7:346:7 | Load: x | positive |
-| test.c:347:9:347:9 | Load: d | positive |
+| test.c:347:9:347:9 | Load: d | positive strictlyPositive |
 | test.c:348:14:348:14 | Constant: 1 | positive strictlyPositive |
 | test.c:348:14:348:14 | Store: 1 | positive strictlyPositive |
 | test.c:355:42:355:42 | InitializeParameter: x | positive |
@@ -674,6 +674,8 @@
 | test.cpp:45:12:45:15 | Constant: 1024 | positive strictlyPositive |
 | test.cpp:46:5:46:12 | Store: ... = ... | positive strictlyPositive |
 | test.cpp:46:9:46:12 | Constant: 1024 | positive strictlyPositive |
+| test.cpp:57:21:57:21 | Load: x | positive strictlyPositive |
+| test.cpp:62:21:62:21 | Load: x | negative strictlyNegative |
 | test.cpp:69:10:69:21 | Constant: ... \|\| ... | positive strictlyPositive |
 | test.cpp:69:10:69:21 | Load: ... \|\| ... | positive |
 | test.cpp:69:10:69:21 | Phi: ... \|\| ... | positive |

cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/ir_gvn.expected

@@ -365,7 +365,7 @@ test.cpp:
 
 #   53|   Block 1
 #   53|     m1_0(unsigned int)  = Phi                  : from 0:m0_11, from 8:m8_4
-#   53|         valnum = unique
+#   53|         valnum = m1_0
 #   53|     r1_1(glval<char *>) = VariableAddress[str] : 
 #   53|         valnum = r0_3
 #   53|     r1_2(char *)        = Load                 : r1_1, m0_4
@@ -395,11 +395,11 @@ test.cpp:
 
 #   56|   Block 3
 #   56|     m3_0(char *)        = Phi                  : from 2:m2_3, from 5:m5_4
-#   56|         valnum = unique
+#   56|         valnum = m3_0
 #   56|     r3_1(glval<char *>) = VariableAddress[ptr] : 
 #   56|         valnum = r0_7
 #   56|     r3_2(char *)        = Load                 : r3_1, m3_0
-#   56|         valnum = unique
+#   56|         valnum = m3_0
 #   56|     r3_3(char)          = Load                 : r3_2, m0_1
 #   56|         valnum = unique
 #   56|     r3_4(int)           = Convert              : r3_3
@@ -422,7 +422,7 @@ test.cpp:
 #   56|     r4_0(glval<char *>) = VariableAddress[ptr] : 
 #   56|         valnum = r0_7
 #   56|     r4_1(char *)        = Load                 : r4_0, m3_0
-#   56|         valnum = unique
+#   56|         valnum = m3_0
 #   56|     r4_2(char)          = Load                 : r4_1, m0_1
 #   56|         valnum = unique
 #   56|     r4_3(int)           = Convert              : r4_2
@@ -439,7 +439,7 @@ test.cpp:
 #   56|     r5_0(glval<char *>) = VariableAddress[ptr] : 
 #   56|         valnum = r0_7
 #   56|     r5_1(char *)        = Load                 : r5_0, m3_0
-#   56|         valnum = unique
+#   56|         valnum = m3_0
 #   56|     r5_2(int)           = Constant[1]          : 
 #   56|         valnum = unique
 #   56|     r5_3(char *)        = PointerAdd[1]        : r5_1, r5_2
@@ -452,7 +452,7 @@ test.cpp:
 #   59|     r6_0(glval<char *>) = VariableAddress[ptr] : 
 #   59|         valnum = r0_7
 #   59|     r6_1(char *)        = Load                 : r6_0, m3_0
-#   59|         valnum = unique
+#   59|         valnum = m3_0
 #   59|     r6_2(char)          = Load                 : r6_1, m0_1
 #   59|         valnum = unique
 #   59|     r6_3(int)           = Convert              : r6_2
@@ -473,7 +473,7 @@ test.cpp:
 #   62|     r8_0(glval<unsigned int>) = VariableAddress[result] : 
 #   62|         valnum = r0_9
 #   62|     r8_1(unsigned int)        = Load                    : r8_0, m1_0
-#   62|         valnum = unique
+#   62|         valnum = m1_0
 #   62|     r8_2(unsigned int)        = Constant[1]             : 
 #   62|         valnum = unique
 #   62|     r8_3(unsigned int)        = Add                     : r8_1, r8_2
@@ -489,9 +489,9 @@ test.cpp:
 #   65|     r9_2(glval<unsigned int>) = VariableAddress[result]  : 
 #   65|         valnum = r0_9
 #   65|     r9_3(unsigned int)        = Load                     : r9_2, m1_0
-#   65|         valnum = r9_3
+#   65|         valnum = m1_0
 #   65|     m9_4(unsigned int)        = Store                    : r9_1, r9_3
-#   65|         valnum = r9_3
+#   65|         valnum = m1_0
 #   49|     r9_5(glval<unsigned int>) = VariableAddress[#return] : 
 #   49|         valnum = r9_1
 #   49|     v9_6(void)                = ReturnValue              : r9_5, m9_4
@@ -639,15 +639,15 @@ test.cpp:
 
 #   88|   Block 3
 #   88|     m3_0(int)        = Phi                        : from 1:m1_3, from 2:m2_3
-#   88|         valnum = unique
+#   88|         valnum = m3_0
 #   88|     r3_1(glval<int>) = VariableAddress[#temp88:7] : 
 #   88|         valnum = r1_2
 #   88|     r3_2(int)        = Load                       : r3_1, m3_0
-#   88|         valnum = r3_2
+#   88|         valnum = m3_0
 #   88|     r3_3(glval<int>) = VariableAddress[v]         : 
 #   88|         valnum = r0_9
 #   88|     m3_4(int)        = Store                      : r3_3, r3_2
-#   88|         valnum = r3_2
+#   88|         valnum = m3_0
 #   89|     v3_5(void)       = NoOp                       : 
 #   84|     v3_6(void)       = ReturnVoid                 : 
 #   84|     v3_7(void)       = UnmodeledUse               : mu*

cpp/ql/test/query-tests/Critical/LargeParameter/test.cpp

@@ -44,3 +44,11 @@ void myFunction2(mySmallStruct *a, myLargeStruct *b) // GOOD
 void myFunction3(mySmallStruct &a, myLargeStruct &b) // GOOD
 {
 }
+
+struct CustomAssignmentOp {
+  // GOOD: it's an accepted pattern to implement copy assignment via copy and
+  // swap. This delegates the resource management involved in copying to the
+  // copy constructor so that logic only has to be written once.
+  CustomAssignmentOp &operator=(CustomAssignmentOp rhs);
+  char data[4096];
+};

cpp/ql/test/query-tests/Documentation/CommentedOutCode/CommentedOutCode.expected

@@ -1,3 +1,6 @@
+| test2.cpp:37:1:37:39 | // int myFunction() { return myValue; } | This comment appears to contain commented-out code |
+| test2.cpp:39:1:39:45 | // int myFunction() const { return myValue; } | This comment appears to contain commented-out code |
+| test2.cpp:41:1:41:54 | // int myFunction() const noexcept { return myValue; } | This comment appears to contain commented-out code |
 | test.c:2:1:2:22 | // commented out code; | This comment appears to contain commented-out code |
 | test.c:4:1:7:8 | // some; | This comment appears to contain commented-out code |
 | test.c:9:1:13:8 | // also; | This comment appears to contain commented-out code |

cpp/ql/test/query-tests/Documentation/CommentedOutCode/test2.cpp

@@ -0,0 +1,41 @@
+/*
+ * This sentence contains a semicolon;
+ * however, this doesn't make it code.
+ */
+
+// This sentence contains a semicolon;
+// however, this doesn't make it code.
+
+/* Mention a ';' */
+
+/* Mention a '{' */
+
+/* JSON example: {"foo":"bar"} */
+
+/* JSON example in backticks: `{"foo":"bar"}` */
+
+/* JSON example in quotes: '{"foo":"bar"}' */
+
+/*
+ * Code example: `return 0;`.
+ */
+
+// Code example:
+//
+//     return 0;
+
+// Code example:
+//
+// ```
+// return 0;
+// ```
+
+// { 1, 2, 3, 4 }
+
+// Example: { 1, 2, 3, 4 }
+
+// int myFunction() { return myValue; }
+
+// int myFunction() const { return myValue; }
+
+// int myFunction() const noexcept { return myValue; }

cpp/ql/test/query-tests/Likely Bugs/Conversion/LossyFunctionResultCast/ImplicitDowncastFromBitfield.expected

@@ -0,0 +1,9 @@
+| test.cpp:33:6:33:13 | call to getFloat | Return value of type float is implicitly converted to bool here. |
+| test.cpp:35:13:35:20 | call to getFloat | Return value of type float is implicitly converted to int here. |
+| test.cpp:38:6:38:14 | call to getDouble | Return value of type double is implicitly converted to bool here. |
+| test.cpp:40:13:40:21 | call to getDouble | Return value of type double is implicitly converted to int here. |
+| test.cpp:43:6:43:12 | call to getMyLD | Return value of type long double is implicitly converted to bool here. |
+| test.cpp:45:13:45:19 | call to getMyLD | Return value of type long double is implicitly converted to int here. |
+| test.cpp:101:10:101:12 | call to pow | Return value of type double is implicitly converted to int here. |
+| test.cpp:103:10:103:12 | call to pow | Return value of type double is implicitly converted to int here. |
+| test.cpp:105:10:105:12 | call to pow | Return value of type double is implicitly converted to int here. |

cpp/ql/test/query-tests/Likely Bugs/Conversion/LossyFunctionResultCast/ImplicitDowncastFromBitfield.qlref

@@ -0,0 +1 @@
+Likely Bugs/Conversion/LossyFunctionResultCast.ql

cpp/ql/test/query-tests/Likely Bugs/Conversion/LossyFunctionResultCast/test.cpp

@@ -0,0 +1,131 @@
+
+typedef long double MYLD;
+
+bool getBool();
+int getInt();
+float getFloat();
+double getDouble();
+MYLD getMyLD();
+float *getFloatPtr();
+float &getFloatRef();
+const float &getConstFloatRef();
+
+void setPosInt(int x);
+void setPosFloat(float x);
+
+double round(double x);
+float roundf(float x);
+
+void test1()
+{
+	// simple
+
+	if (getBool())
+	{
+		setPosInt(getBool());
+		setPosFloat(getBool());
+	}
+	if (getInt())
+	{
+		setPosInt(getInt());
+		setPosFloat(getInt());
+	}
+	if (getFloat()) // BAD
+	{
+		setPosInt(getFloat()); // BAD
+		setPosFloat(getFloat());
+	}
+	if (getDouble()) // BAD
+	{
+		setPosInt(getDouble()); // BAD
+		setPosFloat(getDouble());
+	}
+	if (getMyLD()) // BAD
+	{
+		setPosInt(getMyLD()); // BAD
+		setPosFloat(getMyLD());
+	}
+	if (getFloatPtr())
+	{
+		// ...
+	}
+	if (getFloatRef()) // BAD [NOT DETECTED]
+	{
+		setPosInt(getFloatRef()); // BAD [NOT DETECTED]
+		setPosFloat(getFloatRef());
+	}
+	if (getConstFloatRef()) // BAD [NOT DETECTED]
+	{
+		setPosInt(getConstFloatRef()); // BAD [NOT DETECTED]
+		setPosFloat(getConstFloatRef());
+	}
+
+	// explicit cast
+
+	if ((bool)getInt())
+	{
+		setPosInt(getInt());
+		setPosFloat((float)getInt());
+	}
+	if ((bool)getFloat())
+	{
+		setPosInt((int)getFloat());
+		setPosFloat(getFloat());
+	}
+
+	// explicit rounding
+
+	if (roundf(getFloat()))
+	{
+		setPosInt(roundf(getFloat()));
+		setPosFloat(roundf(getFloat()));
+	}
+	if (round(getDouble()))
+	{
+		setPosInt(round(getDouble()));
+		setPosFloat(round(getDouble()));
+	}
+}
+
+double pow(double x, double y);
+
+int test2(double v, double w, int n)
+{
+	switch (n)
+	{
+	case 1:
+		return pow(2, v); // GOOD
+	case 2:
+		return pow(10, v); // GOOD
+	case 3:
+		return pow(2.5, v); // BAD
+	case 4:
+		return pow(v, 2); // BAD
+	case 5:
+		return pow(v, w); // BAD
+	};
+}
+
+double myRound1(double v)
+{
+	return round(v);
+}
+
+double myRound2(double v)
+{
+	double result = round(v);
+
+	return result;
+}
+
+double myRound3(double v)
+{
+	return (v > 0) ? round(v) : 0;
+}
+
+void test3()
+{
+	int i = myRound1(1.5); // GOOD
+	int j = myRound2(2.5); // GOOD
+	int k = myRound3(3.5); // GOOD
+}

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/test.cpp

@@ -64,3 +64,45 @@ void testFunc2()
 	MyAssignable v1, v2;
 	v2 = v1;
 }
+
+namespace std {
+  typedef unsigned long size_t;
+}
+
+void* operator new  (std::size_t size, void* ptr) noexcept;
+
+struct Base {
+  Base() { }
+
+  Base &operator=(const Base &rhs) {
+    return *this;
+  }
+
+  virtual ~Base() { }
+};
+
+struct Derived : Base {
+  Derived &operator=(const Derived &rhs) {
+    if (&rhs == this) {
+      return *this;
+    }
+
+    // In case base class has data, now or in the future, copy that first.
+    Base::operator=(rhs); // GOOD
+
+    this->m_x = rhs.m_x;
+    return *this;
+  }
+
+  int m_x;
+};
+
+void use_Base() {
+  Base base_buffer[1];
+  Base *base = new(&base_buffer[0]) Base();
+
+  // In case the destructor does something, now or in the future, call it. It
+  // won't get called automatically because the object was allocated with
+  // placement new.
+  base->~Base(); // GOOD (because the call has void type)
+}

cpp/ql/test/query-tests/Security/CWE/CWE-468/semmle/SuspiciousAddWithSizeof/SuspiciousAddWithSizeof.expected

@@ -4,3 +4,4 @@
 | test.cpp:30:25:30:35 | sizeof(int) | Suspicious sizeof offset in a pointer arithmetic expression. The type of the pointer is int *. |
 | test.cpp:38:30:38:40 | sizeof(int) | Suspicious sizeof offset in a pointer arithmetic expression. The type of the pointer is int *. |
 | test.cpp:61:27:61:37 | sizeof(int) | Suspicious sizeof offset in a pointer arithmetic expression. The type of the pointer is int *. |
+| test.cpp:89:43:89:55 | sizeof(MyABC) | Suspicious sizeof offset in a pointer arithmetic expression. The type of the pointer is myInt *const. |

cpp/ql/test/query-tests/Security/CWE/CWE-468/semmle/SuspiciousAddWithSizeof/test.cpp

@@ -64,3 +64,32 @@ void test7(int i) {
   v = *(int *)(voidPointer + i); // GOOD (actually rather dubious, but this could be correct code)
   v = *(int *)(voidPointer + (i * sizeof(int))); // GOOD
 }
+
+typedef unsigned long size_t;
+
+void *malloc(size_t size);
+
+class MyABC
+{
+public:
+  int a, b, c;
+};
+
+typedef unsigned char myChar;
+typedef unsigned int myInt;
+
+class MyTest8Class
+{
+public:
+  MyTest8Class() :
+    myCharsPointer((myChar *)malloc(sizeof(MyABC) * 2)),
+    myIntsPointer((myInt *)malloc(sizeof(MyABC) * 2))
+  {
+    myChar *secondPtr = myCharsPointer + sizeof(MyABC); // GOOD
+    myInt *secondPtrInt = myIntsPointer + sizeof(MyABC); // BAD
+  }
+
+private:
+  myChar * const myCharsPointer;
+  myInt * const myIntsPointer;
+};

cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 79/AV Rule 79.expected

@@ -10,6 +10,7 @@
 | DeleteThis.cpp:60:3:60:24 | ... = ... | Resource ptr14 is acquired by class MyClass3 but not released anywhere in this class. |
 | DeleteThis.cpp:127:3:127:20 | ... = ... | Resource d is acquired by class MyClass9 but not released anywhere in this class. |
 | ExternalOwners.cpp:49:3:49:20 | ... = ... | Resource a is acquired by class MyScreen but not released anywhere in this class. |
+| Lambda.cpp:24:3:24:21 | ... = ... | Resource r4 is acquired by class testLambda but not released anywhere in this class. |
 | ListDelete.cpp:21:3:21:21 | ... = ... | Resource first is acquired by class MyThingColection but not released anywhere in this class. |
 | NoDestructor.cpp:23:3:23:20 | ... = ... | Resource n is acquired by class MyClass5 but not released anywhere in this class. |
 | PlacementNew.cpp:36:3:36:36 | ... = ... | Resource p1 is acquired by class MyTestForPlacementNew but not released anywhere in this class. |

cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 79/Lambda.cpp

@@ -0,0 +1,56 @@
+
+class testLambda
+{
+public:
+	testLambda()
+	{
+		r1 = new char[4096]; // GOOD
+		deleter1 = [](char *r) {
+			delete [] r;
+		};
+
+		r2 = new char[4096]; // GOOD
+		auto deleter2 = [this]() {
+			delete [] r2;
+		};
+		deleter2();
+
+		r3 = new char[4096]; // GOOD
+		auto deleter3 = [&r = r3]() {
+			delete [] r;
+		};
+		deleter3(); 
+
+		r4 = new char[4096]; // BAD
+
+		r5 = new char[4096]; // GOOD
+		deleter5 = &deleter_for_r5;
+
+		r6 = new char[4096]; // GOOD
+		deleter6 = &testLambda::deleter_for_r6;
+	}
+
+	static void deleter_for_r5(char *r)
+	{
+		delete [] r;
+	}
+
+	void deleter_for_r6()
+	{
+		delete [] r6;
+	}
+
+	~testLambda()
+	{
+		deleter1(r1);
+		deleter5(r5);
+		((*this).*deleter6)();
+	}
+
+private:
+	char *r1, *r2, *r3, *r4, *r5, *r6;
+
+	void (*deleter1)(char *r);
+	void (*deleter5)(char *r);
+	void (testLambda::*deleter6)();
+};