Merge remote-tracking branch 'upstream/master' into infinite-loops-visible

Moved the change note to 1.23.

cpp/ql/examples/queries.xml

@@ -0,0 +1 @@
+<queries language="cpp"/>

cpp/ql/examples/snippets/addressof.ql

@@ -0,0 +1,16 @@
+/**
+ * @id cpp/examples/addressof
+ * @name Address of reference variable
+ * @description Finds address-of expressions (`&`) that take the address
+ *              of a reference variable
+ * @tags addressof
+ *       reference
+ */
+
+import cpp
+
+from AddressOfExpr addr, VariableAccess access
+where
+  access = addr.getOperand() and
+  access.getTarget().getType() instanceof ReferenceType
+select addr

cpp/ql/examples/snippets/arrayaccess.ql

@@ -0,0 +1,17 @@
+/**
+ * @id cpp/examples/arrayaccess
+ * @name Array access
+ * @description Finds array access expressions with an index expression
+ *              consisting of a postfix increment (`++`) expression.
+ * @tags array
+ *       access
+ *       index
+ *       postfix
+ *       increment
+ */
+
+import cpp
+
+from ArrayExpr a
+where a.getArrayOffset() instanceof PostfixIncrExpr
+select a

cpp/ql/examples/snippets/castexpr.ql

@@ -0,0 +1,17 @@
+/**
+ * @id cpp/examples/castexpr
+ * @name Cast expressions
+ * @description Finds casts from a floating point type to an integer type
+ * @tags cast
+ *       integer
+ *       float
+ *       type
+ */
+
+import cpp
+
+from Cast c
+where
+  c.getExpr().getType() instanceof FloatingPointType and
+  c.getType() instanceof IntegralType
+select c

cpp/ql/examples/snippets/catch_exception.ql

@@ -0,0 +1,15 @@
+/**
+ * @id cpp/examples/catch-exception
+ * @name Catch exception
+ * @description Finds places where we catch exceptions of type `parse_error`
+ * @tags catch
+ *       try
+ *       exception
+ */
+
+import cpp
+
+from CatchBlock catch
+// `stripType` converts `const parse_error &` to `parse_error`.
+where catch.getParameter().getType().stripType().hasName("parse_error")
+select catch

cpp/ql/examples/snippets/constructor_call.ql

@@ -0,0 +1,16 @@
+/**
+ * @id cpp/examples/constructor-call
+ * @name Call to constructor
+ * @description Finds places where we call `new MyClass(...)`
+ * @tags call
+ *       constructor
+ *       new
+ */
+
+import cpp
+
+from NewExpr new, Constructor c
+where
+  c = new.getInitializer().(ConstructorCall).getTarget() and
+  c.getName() = "MyClass"
+select new

cpp/ql/examples/snippets/derives_from_class.ql

@@ -0,0 +1,20 @@
+/**
+ * @id cpp/examples/derives-from-class
+ * @name Class derives from
+ * @description Finds classes that derive from `std::exception`
+ * @tags base
+ *       class
+ *       derive
+ *       inherit
+ *       override
+ *       subtype
+ *       supertype
+ */
+
+import cpp
+
+from Class type
+where
+  type.getABaseClass+().hasName("exception") and
+  type.getNamespace().getName() = "std"
+select type

cpp/ql/examples/snippets/emptyblock.ql

@@ -0,0 +1,14 @@
+/**
+ * @id cpp/examples/emptyblock
+ * @name Empty blocks
+ * @description Finds empty block statements
+ * @tags empty
+ *       block
+ *       statement
+ */
+
+import cpp
+
+from Block blk
+where blk.getNumStmt() = 0
+select blk

cpp/ql/examples/snippets/emptythen.ql

@@ -0,0 +1,17 @@
+/**
+ * @id cpp/examples/emptythen
+ * @name If statements with empty then branch
+ * @description Finds `if` statements where the `then` branch is
+ *              an empty block statement
+ * @tags if
+ *       then
+ *       empty
+ *       conditional
+ *       branch
+ */
+
+import cpp
+
+from IfStmt i
+where i.getThen().(Block).getNumStmt() = 0
+select i

cpp/ql/examples/snippets/eq_true.ql

@@ -0,0 +1,18 @@
+/**
+ * @id cpp/examples/eq-true
+ * @name Equality test on boolean
+ * @description Finds tests like `==true`, `!=true`
+ * @tags equal
+ *       comparison
+ *       test
+ *       boolean
+ */
+
+import cpp
+
+from EqualityOperation eq, Expr trueExpr
+where
+  trueExpr = eq.getAnOperand() and
+  trueExpr.getType() instanceof BoolType and
+  trueExpr.getValue().toInt() = 1
+select eq

cpp/ql/examples/snippets/field_access.ql

@@ -0,0 +1,17 @@
+/**
+ * @id cpp/examples/field-access
+ * @name Access of field
+ * @description Finds reads of `aDate` (defined on class `Order`)
+ * @tags access
+ *       field
+ *       read
+ */
+
+import cpp
+
+from Field f, FieldAccess access
+where
+  f.hasName("aDate") and
+  f.getDeclaringType().hasName("Order") and
+  f = access.getTarget()
+select access

cpp/ql/examples/snippets/function_call.ql

@@ -0,0 +1,18 @@
+/**
+ * @id cpp/examples/function-call
+ * @name Call to function
+ * @description Finds calls to `std::map<...>::find()`
+ * @tags call
+ *       function
+ *       method
+ */
+
+import cpp
+
+from FunctionCall call, Function fcn
+where
+  call.getTarget() = fcn and
+  fcn.getDeclaringType().getSimpleName() = "map" and
+  fcn.getDeclaringType().getNamespace().getName() = "std" and
+  fcn.hasName("find")
+select call

cpp/ql/examples/snippets/integer_literal.ql

@@ -0,0 +1,15 @@
+/**
+ * @id cpp/examples/integer-literal
+ * @name Integer literal
+ * @description Finds places where we use the integer literal `2`
+ * @tags integer
+ *       literal
+ */
+
+import cpp
+
+from Literal literal
+where
+  literal.getType() instanceof IntType and
+  literal.getValue().toInt() = 2
+select literal

cpp/ql/examples/snippets/mutualrecursion.ql

@@ -0,0 +1,17 @@
+/**
+ * @id cpp/examples/mutualrecursion
+ * @name Mutual recursion
+ * @description Finds pairs of functions that call each other
+ * @tags function
+ *       method
+ *       recursion
+ */
+
+import cpp
+
+from Function m, Function n
+where
+  exists(FunctionCall c | c.getEnclosingFunction() = m and c.getTarget() = n) and
+  exists(FunctionCall c | c.getEnclosingFunction() = n and c.getTarget() = m) and
+  m != n
+select m, n

cpp/ql/examples/snippets/override_method.ql

@@ -0,0 +1,18 @@
+/**
+ * @id cpp/examples/override-method
+ * @name Override of method
+ * @description Finds methods that override `std::exception::what()`
+ * @tags function
+ *       method
+ *       override
+ */
+
+import cpp
+
+from MemberFunction override, MemberFunction base
+where
+  base.getName() = "what" and
+  base.getDeclaringType().getName() = "exception" and
+  base.getDeclaringType().getNamespace().getName() = "std" and
+  override.overrides+(base)
+select override

cpp/ql/examples/snippets/returnstatement.ql

@@ -0,0 +1,14 @@
+/**
+ * @id cpp/examples/returnstatement
+ * @name Return statements
+ * @description Finds return statements that return `0`
+ * @tags return
+ *       statement
+ *       literal
+ */
+
+import cpp
+
+from ReturnStmt r
+where r.getExpr().(Literal).getValue().toInt() = 0
+select r

cpp/ql/examples/snippets/singletonblock.ql

@@ -0,0 +1,13 @@
+/**
+ * @id cpp/examples/singletonblock
+ * @name Singleton blocks
+ * @description Finds block statements containing a single statement
+ * @tags block
+ *       statement
+ */
+
+import cpp
+
+from Block b
+where b.getNumStmt() = 1
+select b

cpp/ql/examples/snippets/switchcase.ql

@@ -0,0 +1,17 @@
+/**
+ * @id cpp/examples/switchcase
+ * @name Switch statement case missing
+ * @description Finds switch statements with a missing enum constant case
+ *              and no default case
+ * @tags switch
+ *       case
+ *       enum
+ */
+
+import cpp
+
+from EnumSwitch es, EnumConstant ec
+where
+  ec = es.getAMissingCase() and
+  not es.hasDefaultCase()
+select es, ec

cpp/ql/examples/snippets/ternaryconditional.ql

@@ -0,0 +1,15 @@
+/**
+ * @id cpp/examples/ternaryconditional
+ * @name Conditional expressions
+ * @description Finds conditional expressions of the form `... ? ... : ...`
+ *              where the types of the resulting expressions differ
+ * @tags conditional
+ *       ternary
+ *       type
+ */
+
+import cpp
+
+from ConditionalExpr e
+where e.getThen().getType() != e.getElse().getType()
+select e

cpp/ql/examples/snippets/throw_exception.ql

@@ -0,0 +1,15 @@
+/**
+ * @id cpp/examples/throw-exception
+ * @name Throw exception of type
+ * @description Finds places where we throw `parse_error` or one of its sub-types
+ * @tags base
+ *       class
+ *       throw
+ *       exception
+ */
+
+import cpp
+
+from ThrowExpr throw
+where throw.getType().(Class).getABaseClass*().getName() = "parse_error"
+select throw

cpp/ql/examples/snippets/todocomment.ql

@@ -0,0 +1,14 @@
+/**
+ * @id cpp/examples/todocomment
+ * @name TODO comments
+ * @description Finds comments containing the word "TODO"
+ * @tags comment
+ *       matches
+ *       TODO
+ */
+
+import cpp
+
+from Comment c
+where c.getContents().matches("%TODO%")
+select c

cpp/ql/examples/snippets/toomanyparams.ql

@@ -0,0 +1,15 @@
+/**
+ * @id cpp/examples/toomanyparams
+ * @name Functions with many parameters
+ * @description Finds functions or methods with more than 10 parameters
+ * @tags function
+ *       method
+ *       parameter
+ *       argument
+ */
+
+import cpp
+
+from Function fcn
+where fcn.getNumberOfParameters() > 10
+select fcn

cpp/ql/examples/snippets/unusedlocalvar.ql

@@ -0,0 +1,16 @@
+/**
+ * @id cpp/examples/unusedlocalvar
+ * @name Unused local variable
+ * @description Finds local variables that are not accessed
+ * @tags variable
+ *       local
+ *       access
+ */
+
+import cpp
+
+from LocalScopeVariable v
+where
+  not v instanceof Parameter and
+  not exists(v.getAnAccess())
+select v

cpp/ql/examples/snippets/unusedmethod.ql

@@ -0,0 +1,18 @@
+/**
+ * @id cpp/examples/unusedmethod
+ * @name Unused private method
+ * @description Finds private non-virtual methods that are not accessed
+ * @tags method
+ *       access
+ *       private
+ *       virtual
+ */
+
+import cpp
+
+from MemberFunction fcn
+where
+  fcn.isPrivate() and
+  not fcn.isVirtual() and
+  not exists(FunctionCall call | fcn = call.getTarget())
+select fcn.getDefinition()

cpp/ql/examples/snippets/unusedparam.ql

@@ -0,0 +1,13 @@
+/**
+ * @id cpp/examples/unusedparam
+ * @name Unused parameter
+ * @description Finds parameters that are not accessed
+ * @tags parameter
+ *       access
+ */
+
+import cpp
+
+from Parameter p
+where p.isNamed() and not exists(p.getAnAccess())
+select p

cpp/ql/examples/snippets/voidreturntype.ql

@@ -0,0 +1,21 @@
+/**
+ * @id cpp/examples/voidreturntype
+ * @name Const method without return type
+ * @description Finds const methods whose return type is `void`
+ * @tags const
+ *       function
+ *       method
+ *       modifier
+ *       specifier
+ *       return
+ *       type
+ *       void
+ */
+
+import cpp
+
+from MemberFunction m
+where
+  m.hasSpecifier("const") and
+  m.getType() instanceof VoidType
+select m

cpp/ql/examples/snippets/volatilevariable.ql

@@ -0,0 +1,13 @@
+/**
+ * @id cpp/examples/volatilevariable
+ * @name Variable declared volatile
+ * @description Finds variables with a `volatile` modifier
+ * @tags variable
+ *       volatile
+ */
+
+import cpp
+
+from Variable f
+where f.isVolatile()
+select f

cpp/ql/src/AlertSuppression.ql

@@ -16,64 +16,56 @@ class SuppressionComment extends CppStyleComment {
 
   SuppressionComment() {
     text = getContents().suffix(2) and
-    ( // match `lgtm[...]` anywhere in the comment
+    (
+      // match `lgtm[...]` anywhere in the comment
       annotation = text.regexpFind("(?i)\\blgtm\\s*\\[[^\\]]*\\]", _, _)
       or
       // match `lgtm` at the start of the comment and after semicolon
-      annotation = text.regexpFind("(?i)(?<=^|;)\\s*lgtm(?!\\B|\\s*\\[)", _, _)
-                       .trim()
+      annotation = text.regexpFind("(?i)(?<=^|;)\\s*lgtm(?!\\B|\\s*\\[)", _, _).trim()
     )
   }
 
-
   /** Gets the text in this comment, excluding the leading //. */
-  string getText() {
-    result = text
-  }
+  string getText() { result = text }
 
   /** Gets the suppression annotation in this comment. */
-  string getAnnotation() {
-    result = annotation
-  }
+  string getAnnotation() { result = annotation }
 
   /**
-  * Holds if this comment applies to the range from column `startcolumn` of line `startline`
-  * to column `endcolumn` of line `endline` in file `filepath`.
-  */
+   * Holds if this comment applies to the range from column `startcolumn` of line `startline`
+   * to column `endcolumn` of line `endline` in file `filepath`.
+   */
   predicate covers(string filepath, int startline, int startcolumn, int endline, int endcolumn) {
     this.getLocation().hasLocationInfo(filepath, startline, _, endline, endcolumn) and
     startcolumn = 1
   }
 
   /** Gets the scope of this suppression. */
-  SuppressionScope getScope() {
-    result = this
-  }
+  SuppressionScope getScope() { result = this }
 }
 
 /**
  * The scope of an alert suppression comment.
  */
 class SuppressionScope extends ElementBase {
-  SuppressionScope() {
-    this instanceof SuppressionComment
-  }
+  SuppressionScope() { this instanceof SuppressionComment }
 
   /**
-  * Holds if this element is at the specified location.
-  * The location spans column `startcolumn` of line `startline` to
-  * column `endcolumn` of line `endline` in file `filepath`.
-  * For more information, see
-  * [Locations](https://help.semmle.com/QL/learn-ql/ql/locations.html).
-  */
-  predicate hasLocationInfo(string filepath, int startline, int startcolumn, int endline, int endcolumn) {
+   * Holds if this element is at the specified location.
+   * The location spans column `startcolumn` of line `startline` to
+   * column `endcolumn` of line `endline` in file `filepath`.
+   * For more information, see
+   * [Locations](https://help.semmle.com/QL/learn-ql/ql/locations.html).
+   */
+  predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
     this.(SuppressionComment).covers(filepath, startline, startcolumn, endline, endcolumn)
   }
 }
 
 from SuppressionComment c
-select c,                 // suppression comment
-       c.getText(),       // text of suppression comment (excluding delimiters)
-       c.getAnnotation(), // text of suppression annotation
-       c.getScope()       // scope of suppression
-
+select c, // suppression comment
+  c.getText(), // text of suppression comment (excluding delimiters)
+  c.getAnnotation(), // text of suppression annotation
+  c.getScope() // scope of suppression

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

@@ -42,9 +42,7 @@ newtype TVariableDeclarationInfo =
  */
 class VariableDeclarationLine extends TVariableDeclarationInfo {
   Class c;
-
   File f;
-
   int line;
 
   VariableDeclarationLine() {

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

@@ -12,8 +12,6 @@
 
 import cpp
 
-//see http://www.cs.ualberta.ca/~hoover/Courses/201/201-New-Notes/lectures/section/slice.htm
-//Does not find anything in rivers (unfortunately)
 from AssignExpr e, Class lhsType, Class rhsType
 where
   e.getLValue().getType() = lhsType and
@@ -22,6 +20,6 @@ where
   exists(Declaration m |
     rhsType.getAMember() = m and
     not m.(VirtualFunction).isPure()
-  ) //add additional checks for concrete members in in-between supertypes
+  ) // add additional checks for concrete members in in-between supertypes
 select e, "This assignment expression slices from type $@ to $@", rhsType, rhsType.getName(),
   lhsType, lhsType.getName()

cpp/ql/src/Best Practices/Magic Constants/JapaneseEraDate.qhelp

@@ -0,0 +1,17 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+  <p>
+    When eras change, date and time conversions that rely on a hard-coded era start date need to be reviewed. Conversions relying on Japanese dates in the current era can produce an ambiguous date. 
+    The values for the current Japanese era dates should be read from a source that will be updated, such as the Windows registry.
+  </p>
+</overview>
+
+<references>
+  <li>
+    <a href="https://blogs.msdn.microsoft.com/shawnste/2018/04/12/the-japanese-calendars-y2k-moment/">The Japanese Calendar's Y2K Moment</a>.
+  </li>
+</references>
+</qhelp>

cpp/ql/src/Best Practices/Magic Constants/JapaneseEraDate.ql

@@ -0,0 +1,72 @@
+/**
+ * @name Hard-coded Japanese era start date
+ * @description Japanese era changes can lead to code behaving differently. Avoid hard-coding Japanese era start dates.
+ * @kind problem
+ * @problem.severity warning
+ * @id cpp/japanese-era/exact-era-date
+ * @precision medium
+ * @tags reliability
+ *       japanese-era
+ */
+
+import cpp
+import semmle.code.cpp.commons.DateTime
+
+predicate assignedYear(Struct s, YearFieldAccess year, int value) {
+  exists(Operation yearAssignment |
+    s.getAField().getAnAccess() = year and
+    yearAssignment.getAnOperand() = year and
+    yearAssignment.getAnOperand().getValue().toInt() = value
+  )
+}
+
+predicate assignedMonth(Struct s, MonthFieldAccess month, int value) {
+  exists(Operation monthAssignment |
+    s.getAField().getAnAccess() = month and
+    monthAssignment.getAnOperand() = month and
+    monthAssignment.getAnOperand().getValue().toInt() = value
+  )
+}
+
+predicate assignedDay(Struct s, DayFieldAccess day, int value) {
+  exists(Operation dayAssignment |
+    s.getAField().getAnAccess() = day and
+    dayAssignment.getAnOperand() = day and
+    dayAssignment.getAnOperand().getValue().toInt() = value
+  )
+}
+
+predicate eraDate(int year, int month, int day) {
+  year = 1989 and month = 1 and day = 8
+  or
+  year = 2019 and month = 5 and day = 1
+}
+
+predicate badStructInitialization(Element target, string message) {
+  exists(
+    StructLikeClass s, YearFieldAccess year, MonthFieldAccess month, DayFieldAccess day,
+    int yearValue, int monthValue, int dayValue
+  |
+    eraDate(yearValue, monthValue, dayValue) and
+    assignedYear(s, year, yearValue) and
+    assignedMonth(s, month, monthValue) and
+    assignedDay(s, day, dayValue) and
+    target = year and
+    message = "A time struct that is initialized with exact Japanese calendar era start date."
+  )
+}
+
+predicate badCall(Element target, string message) {
+  exists(Call cc, int i |
+    eraDate(cc.getArgument(i).getValue().toInt(), cc.getArgument(i + 1).getValue().toInt(),
+      cc.getArgument(i + 2).getValue().toInt()) and
+    target = cc and
+    message = "Call that appears to have hard-coded Japanese era start date as parameter."
+  )
+}
+
+from Element target, string message
+where
+  badStructInitialization(target, message) or
+  badCall(target, message)
+select target, message

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

@@ -14,9 +14,7 @@
 import cpp
 
 predicate declarationHasSideEffects(Variable v) {
-  exists(Class c | c = v.getUnspecifiedType() |
-    c.hasConstructor() or c.hasDestructor()
-  )
+  exists(Class c | c = v.getUnspecifiedType() | c.hasConstructor() or c.hasDestructor())
 }
 
 from Variable v

cpp/ql/src/Critical/FileMayNotBeClosed.ql

@@ -26,20 +26,18 @@ class MinusOne extends NullValue {
  */
 predicate mayCallFunction(Expr call, Function f) {
   call.(FunctionCall).getTarget() = f or
-  call.(VariableCall).getVariable().getAnAssignedValue().
-    getAChild*().(FunctionAccess).getTarget() = f
+  call.(VariableCall).getVariable().getAnAssignedValue().getAChild*().(FunctionAccess).getTarget() =
+    f
 }
 
 predicate fopenCallOrIndirect(Expr e) {
   // direct fopen call
   fopenCall(e) and
-
   // We are only interested in fopen calls that are
   // actually closed somehow, as FileNeverClosed
   // will catch those that aren't.
   fopenCallMayBeClosed(e)
   or
-
   exists(ReturnStmt rtn |
     // indirect fopen call
     mayCallFunction(e, rtn.getEnclosingFunction()) and
@@ -86,7 +84,6 @@ class FOpenVariableReachability extends LocalScopeVariableReachabilityWithReassi
     exists(node.(AnalysedExpr).getNullSuccessor(v)) or
     fcloseCallOrIndirect(node, v) or
     assignedToFieldOrGlobal(v, node) or
-
     // node may be used directly in query
     v.getFunction() = node.(ReturnStmt).getEnclosingFunction()
   }
@@ -122,12 +119,10 @@ class FOpenReachability extends LocalScopeVariableReachabilityExt {
   }
 
   override predicate isBarrier(
-    ControlFlowNode source, ControlFlowNode node, ControlFlowNode next,
-    LocalScopeVariable v)
-  {
+    ControlFlowNode source, ControlFlowNode node, ControlFlowNode next, LocalScopeVariable v
+  ) {
     isSource(source, v) and
     next = node.getASuccessor() and
-
     // the file (stored in any variable `v0`) opened at `source` is closed or
     // assigned to a global at node, or NULL checked on the edge node -> next.
     exists(LocalScopeVariable v0 | fopenVariableReaches(v0, source, node) |
@@ -172,6 +167,4 @@ where
     fopenVariableReaches(v, def, ret) and
     ret.getAChild*() = v.getAnAccess()
   )
-select
-  def, "The file opened here may not be closed at $@.",
-  ret, "this exit point"
+select def, "The file opened here may not be closed at $@.", ret, "this exit point"

cpp/ql/src/Critical/MemoryMayNotBeFreed.ql

@@ -18,20 +18,18 @@ import semmle.code.cpp.controlflow.LocalScopeVariableReachability
  */
 predicate mayCallFunction(Expr call, Function f) {
   call.(FunctionCall).getTarget() = f or
-  call.(VariableCall).getVariable().getAnAssignedValue().
-    getAChild*().(FunctionAccess).getTarget() = f
+  call.(VariableCall).getVariable().getAnAssignedValue().getAChild*().(FunctionAccess).getTarget() =
+    f
 }
 
 predicate allocCallOrIndirect(Expr e) {
   // direct alloc call
   isAllocationExpr(e) and
-
   // We are only interested in alloc calls that are
   // actually freed somehow, as MemoryNeverFreed
   // will catch those that aren't.
   allocMayBeFreed(e)
   or
-
   exists(ReturnStmt rtn |
     // indirect alloc call
     mayCallFunction(e, rtn.getEnclosingFunction()) and
@@ -64,7 +62,6 @@ predicate verifiedRealloc(FunctionCall reallocCall, Variable v, ControlFlowNode
       newV.getAnAssignedValue() = reallocCall and
       node.(AnalysedExpr).getNonNullSuccessor(newV) = verified and
       // note: this case uses naive flow logic (getAnAssignedValue).
-
       // special case: if the result of the 'realloc' is assigned to the
       // same variable, we don't descriminate properly between the old
       // and the new allocation; better to not consider this a free at
@@ -116,7 +113,6 @@ class AllocVariableReachability extends LocalScopeVariableReachabilityWithReassi
     exists(node.(AnalysedExpr).getNullSuccessor(v)) or
     freeCallOrIndirect(node, v) or
     assignedToFieldOrGlobal(v, node) or
-
     // node may be used directly in query
     v.getFunction() = node.(ReturnStmt).getEnclosingFunction()
   }
@@ -152,12 +148,10 @@ class AllocReachability extends LocalScopeVariableReachabilityExt {
   }
 
   override predicate isBarrier(
-    ControlFlowNode source, ControlFlowNode node, ControlFlowNode next,
-    LocalScopeVariable v)
-  {
+    ControlFlowNode source, ControlFlowNode node, ControlFlowNode next, LocalScopeVariable v
+  ) {
     isSource(source, v) and
     next = node.getASuccessor() and
-
     // the memory (stored in any variable `v0`) allocated at `source` is freed or
     // assigned to a global at node, or NULL checked on the edge node -> next.
     exists(LocalScopeVariable v0 | allocatedVariableReaches(v0, source, node) |
@@ -202,6 +196,4 @@ where
     allocatedVariableReaches(v, def, ret) and
     ret.getAChild*() = v.getAnAccess()
   )
-select
-  def, "The memory allocated here may not be released at $@.",
-  ret, "this exit point"
+select def, "The memory allocated here may not be released at $@.", ret, "this exit point"

cpp/ql/src/Critical/NewDelete.qll

@@ -47,7 +47,7 @@ predicate allocExprOrIndirect(Expr alloc, string kind) {
       or
       exists(Expr e |
         allocExprOrIndirect(e, kind) and
-        DataFlow::localFlow(DataFlow::exprNode(e), DataFlow::exprNode(rtn.getExpr()))
+        DataFlow::localExprFlow(e, rtn.getExpr())
       )
     )
   )

cpp/ql/src/Critical/NotInitialised.ql

@@ -8,9 +8,11 @@
  *       external/cwe/cwe-457
  */
 
-import cpp
+/*
+ * See also InitialisationNotRun.ql and GlobalUseBeforeInit.ql
+ */
 
-// See also InitialisationNotRun.ql and GlobalUseBeforeInit.ql
+import cpp
 
 /**
  * Holds if `s` defines variable `v` (conservative).

cpp/ql/src/Critical/OverflowDestination.ql

@@ -33,12 +33,10 @@ predicate sourceSized(FunctionCall fc, Expr src) {
     fc.getArgument(2) = size and
     src = v.getAnAccess() and
     size.getAChild+() = v.getAnAccess() and
-
     // exception: `dest` is also referenced in the size argument
     not exists(Variable other |
       dest = other.getAnAccess() and size.getAChild+() = other.getAnAccess()
     ) and
-
     // exception: `src` and `dest` are both arrays of the same type and size
     not exists(ArrayType srctype, ArrayType desttype |
       dest.getType().getUnderlyingType() = desttype and

cpp/ql/src/Critical/OverflowStatic.ql

@@ -33,7 +33,6 @@ class BufferAccess extends ArrayExpr {
       staticBuffer(this.getArrayBase(), _, size) and
       size != 0
     ) and
-
     // exclude accesses in macro implementation of `strcmp`,
     // which are carefully controlled but can look dangerous.
     not exists(Macro m |
@@ -95,7 +94,7 @@ class CallWithBufferSize extends FunctionCall {
 
   int statedSizeValue() {
     exists(Expr statedSizeSrc |
-      DataFlow::localFlow(DataFlow::exprNode(statedSizeSrc), DataFlow::exprNode(statedSizeExpr())) and
+      DataFlow::localExprFlow(statedSizeSrc, statedSizeExpr()) and
       result = statedSizeSrc.getValue().toInt()
     )
   }

cpp/ql/src/DefaultOptions.qll

@@ -13,14 +13,11 @@ private import Options as CustomOptions
 
 /**
  * Default predicates that specify information about the behavior of
- * the program being analyzed. 
+ * the program being analyzed.
  */
-class Options extends string
-{
-  Options() {
-    this = "Options"
-  }
-  
+class Options extends string {
+  Options() { this = "Options" }
+
   /**
    * Holds if we wish to override the "may return NULL" inference for this
    * call. If this holds, then rather than trying to infer whether this
@@ -60,7 +57,8 @@ class Options extends string
    * `noreturn` attribute.
    */
   predicate exits(Function f) {
-    f.getAnAttribute().hasName("noreturn") or
+    f.getAnAttribute().hasName("noreturn")
+    or
     exists(string name | f.hasGlobalName(name) |
       name = "exit" or
       name = "_exit" or
@@ -68,7 +66,8 @@ class Options extends string
       name = "__assert_fail" or
       name = "longjmp" or
       name = "__builtin_unreachable"
-    ) or
+    )
+    or
     CustomOptions::exits(f) // old Options.qll
   }
 
@@ -108,14 +107,11 @@ class Options extends string
     fc.isInMacroExpansion()
     or
     // common way of sleeping using select:
-    (fc.getTarget().hasGlobalName("select") and
-     fc.getArgument(0).getValue() = "0")
+    fc.getTarget().hasGlobalName("select") and
+    fc.getArgument(0).getValue() = "0"
     or
     CustomOptions::okToIgnoreReturnValue(fc) // old Options.qll
   }
 }
 
-Options getOptions()
-{
-  any()
-}
+Options getOptions() { any() }

cpp/ql/src/Documentation/CaptionedComments.qll

@@ -1,26 +1,36 @@
 /**
  * Provides heuristics to find "todo" and "fixme" comments (in all caps).
  */
+
 import cpp
 
 string getCommentTextCaptioned(Comment c, string caption) {
-    (caption = "TODO" or caption = "FIXME") and
-    exists (string commentContents, string commentBody, int offset, string interestingSuffix, int endOfLine, string dontCare, string captionedLine, string followingLine
-           | commentContents = c.getContents()
-         and commentContents.matches("%" + caption + "%")
-         and // Add some '\n's so that any interesting line, and its
-             // following line, will definitely begin and end with '\n'.
-             commentBody = commentContents.regexpReplaceAll("(?s)^/\\*(.*)\\*/$|^//(.*)$", "\n$1$2\n\n")
-         and dontCare = commentBody.regexpFind("\\n[/* \\t\\x0B\\f\\r]*" + caption, _, offset)
-         and interestingSuffix = commentBody.suffix(offset)
-         and endOfLine = interestingSuffix.indexOf("\n", 1, 0)
-         and captionedLine = interestingSuffix.prefix(endOfLine).regexpReplaceAll("^[/*\\s]*" + caption + "\\s*:?", "").trim()
-         and followingLine = interestingSuffix.prefix(interestingSuffix.indexOf("\n", 2, 0)).suffix(endOfLine).trim()
-         and if captionedLine = ""
-             then result = caption + " comment"
-             else if followingLine = ""
-             then result = caption + " comment: " + captionedLine
-             else result = caption + " comment: " + captionedLine + " [...]"
-    )
+  (caption = "TODO" or caption = "FIXME") and
+  exists(
+    string commentContents, string commentBody, int offset, string interestingSuffix, int endOfLine,
+    string dontCare, string captionedLine, string followingLine
+  |
+    commentContents = c.getContents() and
+    commentContents.matches("%" + caption + "%") and
+    // Add some '\n's so that any interesting line, and its
+    // following line, will definitely begin and end with '\n'.
+    commentBody = commentContents.regexpReplaceAll("(?s)^/\\*(.*)\\*/$|^//(.*)$", "\n$1$2\n\n") and
+    dontCare = commentBody.regexpFind("\\n[/* \\t\\x0B\\f\\r]*" + caption, _, offset) and
+    interestingSuffix = commentBody.suffix(offset) and
+    endOfLine = interestingSuffix.indexOf("\n", 1, 0) and
+    captionedLine = interestingSuffix
+          .prefix(endOfLine)
+          .regexpReplaceAll("^[/*\\s]*" + caption + "\\s*:?", "")
+          .trim() and
+    followingLine = interestingSuffix
+          .prefix(interestingSuffix.indexOf("\n", 2, 0))
+          .suffix(endOfLine)
+          .trim() and
+    if captionedLine = ""
+    then result = caption + " comment"
+    else
+      if followingLine = ""
+      then result = caption + " comment: " + captionedLine
+      else result = caption + " comment: " + captionedLine + " [...]"
+  )
 }
-

cpp/ql/src/Documentation/CommentedOutCode.qll

@@ -6,44 +6,42 @@ import cpp
 bindingset[line]
 private predicate looksLikeCode(string line) {
   exists(string trimmed |
-    // trim leading and trailing whitespace, and HTML codes: 
+    // trim leading and trailing whitespace, and HTML codes:
     //  * HTML entities in common notation (e.g. > and é)
     //  * HTML entities in decimal notation (e.g. à)
     //  * HTML entities in hexadecimal notation (e.g. 灟)
     trimmed = line.regexpReplaceAll("(?i)(^\\s+|&#?[a-z0-9]{1,31};|\\s+$)", "")
   |
     (
+      // Match comment lines ending with '{', '}' or ';'
+      trimmed.regexpMatch(".*[{};]") and
       (
-        // Match comment lines ending with '{', '}' or ';'
-        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 definition. 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*\\{.*")
-        )
-      ) or (
-        // Match comment lines that look like preprocessor code
-        trimmed.regexpMatch("#\\s*(include|define|undef|if|ifdef|ifndef|elif|else|endif|error|pragma)\\b.*")
+        // 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 definition. 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 (
-      // Exclude lines that start with '>' or contain '@{' or '@}'.
-      // To account for the code generated by protobuf, we also insist that the comment
-      // does not begin with `optional` or `repeated` and end with a `;`, which would
-      // normally be a quoted bit of literal `.proto` specification above the associated
-      // declaration.
-      // To account for emacs folding markers, we ignore any line containing
-      // `{{{` or `}}}`.
-      // Finally, some code tends to embed GUIDs in comments, so we also exclude those.
-      not trimmed
+      or
+      // Match comment lines that look like preprocessor code
+      trimmed
+          .regexpMatch("#\\s*(include|define|undef|if|ifdef|ifndef|elif|else|endif|error|pragma)\\b.*")
+    ) and
+    // Exclude lines that start with '>' or contain '@{' or '@}'.
+    // To account for the code generated by protobuf, we also insist that the comment
+    // does not begin with `optional` or `repeated` and end with a `;`, which would
+    // normally be a quoted bit of literal `.proto` specification above the associated
+    // declaration.
+    // To account for emacs folding markers, we ignore any line containing
+    // `{{{` or `}}}`.
+    // Finally, some code tends to embed GUIDs in comments, so we also exclude those.
+    not trimmed
         .regexpMatch("(>.*|.*[\\\\@][{}].*|(optional|repeated) .*;|.*(\\{\\{\\{|\\}\\}\\}).*|\\{[-0-9a-zA-Z]+\\})")
-    )
   )
 }
 
@@ -76,7 +74,6 @@ private predicate preprocLine(File f, int line) {
 private int lineInFile(CppStyleComment c, File f) {
   f = c.getFile() and
   result = c.getLocation().getStartLine() and
-
   // Ignore comments on the same line as a preprocessor directive.
   not preprocLine(f, result)
 }
@@ -119,12 +116,11 @@ class CommentBlock extends Comment {
       this instanceof CppStyleComment
       implies
       not exists(CppStyleComment pred, File f | lineInFile(pred, f) + 1 = lineInFile(this, f))
-    ) and (
-      // Ignore comments on the same line as a preprocessor directive.
-      not exists(Location l |
-        l = this.getLocation() and
-        preprocLine(l.getFile(), l.getStartLine())
-      )
+    ) and
+    // Ignore comments on the same line as a preprocessor directive.
+    not exists(Location l |
+      l = this.getLocation() and
+      preprocLine(l.getFile(), l.getStartLine())
     )
   }
 

cpp/ql/src/Documentation/DocumentApi.ql

@@ -11,26 +11,26 @@
  * @tags maintainability
  *       documentation
  */
-import cpp
 
+import cpp
 
 predicate isCommented(FunctionDeclarationEntry f) {
   exists(Comment c | c.getCommentedElement() = f)
 }
 
 // Uses of 'f' in 'other'
-Call uses(File other, Function f) {
-  result.getTarget() = f and result.getFile() = other
-}
+Call uses(File other, Function f) { result.getTarget() = f and result.getFile() = other }
 
 from File callerFile, Function f, Call use, int numCalls
-where numCalls = strictcount(File other | exists(uses(other, f)) and other != f.getFile())
-  and not isCommented(f.getADeclarationEntry())
-  and not f instanceof Constructor
-  and not f instanceof Destructor
-  and not f.hasName("operator=")
-  and f.getMetrics().getNumberOfLinesOfCode() >= 5
-  and numCalls > 1
-  and use = uses(callerFile, f)
-  and callerFile != f.getFile()
-select f, "Functions called from other files should be documented (called from $@).", use, use.getFile().getRelativePath()
+where
+  numCalls = strictcount(File other | exists(uses(other, f)) and other != f.getFile()) and
+  not isCommented(f.getADeclarationEntry()) and
+  not f instanceof Constructor and
+  not f instanceof Destructor and
+  not f.hasName("operator=") and
+  f.getMetrics().getNumberOfLinesOfCode() >= 5 and
+  numCalls > 1 and
+  use = uses(callerFile, f) and
+  callerFile != f.getFile()
+select f, "Functions called from other files should be documented (called from $@).", use,
+  use.getFile().getRelativePath()

cpp/ql/src/Documentation/FixmeComments.ql

@@ -9,6 +9,7 @@
  *       documentation
  *       external/cwe/cwe-546
  */
+
 import cpp
 import Documentation.CaptionedComments
 

cpp/ql/src/Documentation/TodoComments.ql

@@ -9,10 +9,10 @@
  *       documentation
  *       external/cwe/cwe-546
  */
+
 import cpp
 import Documentation.CaptionedComments
 
 from Comment c, string message
 where message = getCommentTextCaptioned(c, "TODO")
 select c, message
-

cpp/ql/src/Documentation/UncommentedFunction.ql

@@ -10,10 +10,14 @@
  *       statistical
  *       non-attributable
  */
+
 import cpp
 
 from MetricFunction f, int n
-where n = f.getNumberOfLines() and n > 100 and
-      f.getCommentRatio() <= 0.02 and
-      not f.isMultiplyDefined()
-select f, "Poorly documented function: fewer than 2% comments for a function of " + n.toString() + " lines."
+where
+  n = f.getNumberOfLines() and
+  n > 100 and
+  f.getCommentRatio() <= 0.02 and
+  not f.isMultiplyDefined()
+select f,
+  "Poorly documented function: fewer than 2% comments for a function of " + n.toString() + " lines."

cpp/ql/src/JPL_C/LOC-2/Rule 03/ExitNonterminatingLoop.ql

@@ -11,20 +11,20 @@
 import cpp
 
 predicate markedAsNonterminating(Loop l) {
-  exists(Comment c | c.getContents().matches("%@non-terminating@%") |
-    c.getCommentedElement() = l
-  )
+  exists(Comment c | c.getContents().matches("%@non-terminating@%") | c.getCommentedElement() = l)
 }
 
 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 markedAsNonterminating(l) and
-      exit = exitFrom(l)
+where
+  markedAsNonterminating(l) and
+  exit = exitFrom(l)
 select exit, "$@ should not be exited.", l, "This permanent loop"

cpp/ql/src/JPL_C/LOC-2/Rule 03/LoopBounds.ql

@@ -1,7 +1,7 @@
 /**
  * @name Unbounded loop
  * @description All loops should have a fixed upper bound; the counter should also be incremented along all paths within the loop.
-                This check excludes loops that are meant to be nonterminating (like schedulers).
+ *              This check excludes loops that are meant to be nonterminating (like schedulers).
  * @kind problem
  * @id cpp/jpl-c/loop-bounds
  * @problem.severity warning
@@ -30,7 +30,8 @@ predicate upperBoundCheck(Loop loop, VariableAccess checked) {
       rop.getGreaterOperand().(VariableAccess).getTarget().isConst() or
       validVarForBound(loop, rop.getGreaterOperand().(VariableAccess).getTarget())
     ) and
-    not rop.getGreaterOperand() instanceof CharLiteral)
+    not rop.getGreaterOperand() instanceof CharLiteral
+  )
 }
 
 predicate lowerBoundCheck(Loop loop, VariableAccess checked) {
@@ -43,20 +44,23 @@ predicate lowerBoundCheck(Loop loop, VariableAccess checked) {
       rop.getLesserOperand().(VariableAccess).getTarget().isConst() or
       validVarForBound(loop, rop.getLesserOperand().(VariableAccess).getTarget())
     ) and
-    not rop.getLesserOperand() instanceof CharLiteral)
+    not rop.getLesserOperand() instanceof CharLiteral
+  )
 }
 
 VariableAccess getAnIncrement(Variable var) {
   result.getTarget() = var and
   (
     result.getParent() instanceof IncrementOperation
-  or
+    or
     exists(AssignAddExpr a | a.getLValue() = result and a.getRValue().getValue().toInt() > 0)
-  or
+    or
     exists(AssignExpr a | a.getLValue() = result |
-      a.getRValue() =
-        any(AddExpr ae | ae.getAnOperand() = var.getAnAccess() and
-            ae.getAnOperand().getValue().toInt() > 0))
+      a.getRValue() = any(AddExpr ae |
+          ae.getAnOperand() = var.getAnAccess() and
+          ae.getAnOperand().getValue().toInt() > 0
+        )
+    )
   )
 }
 
@@ -64,62 +68,75 @@ VariableAccess getADecrement(Variable var) {
   result.getTarget() = var and
   (
     result.getParent() instanceof DecrementOperation
-  or
+    or
     exists(AssignSubExpr a | a.getLValue() = result and a.getRValue().getValue().toInt() > 0)
-  or
+    or
     exists(AssignExpr a | a.getLValue() = result |
-      a.getRValue() =
-        any(SubExpr ae | ae.getLeftOperand() = var.getAnAccess() and
-            ae.getRightOperand().getValue().toInt() > 0))
+      a.getRValue() = any(SubExpr ae |
+          ae.getLeftOperand() = var.getAnAccess() and
+          ae.getRightOperand().getValue().toInt() > 0
+        )
+    )
   )
 }
 
-predicate inScope(Loop l, Stmt s) {
-  l.getAChild*() = s
-}
+predicate inScope(Loop l, Stmt s) { l.getAChild*() = s }
 
 predicate reachesNoInc(VariableAccess source, ControlFlowNode target) {
-  (upperBoundCheck(_, source) and source.getASuccessor() = target) or
-  exists(ControlFlowNode mid | reachesNoInc(source, mid) and not mid = getAnIncrement(source.getTarget()) |
-                          target = mid.getASuccessor() and
-                          inScope(source.getEnclosingStmt(), target.getEnclosingStmt()))
+  upperBoundCheck(_, source) and source.getASuccessor() = target
+  or
+  exists(ControlFlowNode mid |
+    reachesNoInc(source, mid) and not mid = getAnIncrement(source.getTarget())
+  |
+    target = mid.getASuccessor() and
+    inScope(source.getEnclosingStmt(), target.getEnclosingStmt())
+  )
 }
 
 predicate reachesNoDec(VariableAccess source, ControlFlowNode target) {
-  (lowerBoundCheck(_, source) and source.getASuccessor() = target) or
-  exists(ControlFlowNode mid | reachesNoDec(source, mid) and not mid = getADecrement(source.getTarget()) |
-                          target = mid.getASuccessor() and
-                          inScope(source.getEnclosingStmt(), target.getEnclosingStmt()))
-}
-
-predicate hasSafeBound(Loop l) {
-  exists(VariableAccess bound | upperBoundCheck(l, bound) |
-                          not reachesNoInc(bound, bound)
-  ) or exists(VariableAccess bound | lowerBoundCheck(l, bound) |
-                          not reachesNoDec(bound, bound)
-  ) or exists(l.getControllingExpr().getValue())
-}
-
-predicate markedAsNonterminating(Loop l) {
-  exists(Comment c | c.getContents().matches("%@non-terminating@%") |
-    c.getCommentedElement() = l
+  lowerBoundCheck(_, source) and source.getASuccessor() = target
+  or
+  exists(ControlFlowNode mid |
+    reachesNoDec(source, mid) and not mid = getADecrement(source.getTarget())
+  |
+    target = mid.getASuccessor() and
+    inScope(source.getEnclosingStmt(), target.getEnclosingStmt())
   )
 }
 
+predicate hasSafeBound(Loop l) {
+  exists(VariableAccess bound | upperBoundCheck(l, bound) | not reachesNoInc(bound, bound))
+  or
+  exists(VariableAccess bound | lowerBoundCheck(l, bound) | not reachesNoDec(bound, bound))
+  or
+  exists(l.getControllingExpr().getValue())
+}
+
+predicate markedAsNonterminating(Loop l) {
+  exists(Comment c | c.getContents().matches("%@non-terminating@%") | c.getCommentedElement() = l)
+}
+
 from Loop loop, string msg
-where not hasSafeBound(loop) and
-    not markedAsNonterminating(loop) and
-    (
-      (
-        not upperBoundCheck(loop, _) and
-        not lowerBoundCheck(loop, _) and
-        msg = "This loop does not have a fixed bound."
-      ) or exists(VariableAccess bound | upperBoundCheck(loop, bound) and
-                    reachesNoInc(bound, bound) and
-                    msg = "The loop counter " + bound.getTarget().getName() + " is not always incremented in the loop body."
-      ) or exists(VariableAccess bound | lowerBoundCheck(loop, bound) and
-                    reachesNoDec(bound, bound) and
-                    msg = "The loop counter " + bound.getTarget().getName() + " is not always decremented in the loop body."
-      )
+where
+  not hasSafeBound(loop) and
+  not markedAsNonterminating(loop) and
+  (
+    not upperBoundCheck(loop, _) and
+    not lowerBoundCheck(loop, _) and
+    msg = "This loop does not have a fixed bound."
+    or
+    exists(VariableAccess bound |
+      upperBoundCheck(loop, bound) and
+      reachesNoInc(bound, bound) and
+      msg = "The loop counter " + bound.getTarget().getName() +
+          " is not always incremented in the loop body."
     )
+    or
+    exists(VariableAccess bound |
+      lowerBoundCheck(loop, bound) and
+      reachesNoDec(bound, bound) and
+      msg = "The loop counter " + bound.getTarget().getName() +
+          " is not always decremented in the loop body."
+    )
+  )
 select loop, msg

cpp/ql/src/JPL_C/LOC-2/Rule 04/Recursion.ql

@@ -13,14 +13,14 @@
 import cpp
 
 class RecursiveCall extends FunctionCall {
-  RecursiveCall() {
-    this.getTarget().calls*(this.getEnclosingFunction())
-  }
+  RecursiveCall() { this.getTarget().calls*(this.getEnclosingFunction()) }
 }
 
 from RecursiveCall call, string msg
-where if (call.getTarget() = call.getEnclosingFunction()) then
-         msg = "This call directly invokes its containing function $@."
-      else
-         msg = "The function " + call.getEnclosingFunction() + " is indirectly recursive via this call to $@."
+where
+  if call.getTarget() = call.getEnclosingFunction()
+  then msg = "This call directly invokes its containing function $@."
+  else
+    msg = "The function " + call.getEnclosingFunction() +
+        " is indirectly recursive via this call to $@."
 select call, msg, call.getTarget(), call.getTarget().getName()

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

@@ -23,12 +23,17 @@ class Initialization extends Function {
 class Allocation extends FunctionCall {
   Allocation() {
     exists(string name | name = this.getTarget().getName() |
-      name = "malloc" or name = "calloc" or name = "alloca" or
-      name = "sbrk" or name = "valloc")
+      name = "malloc" or
+      name = "calloc" or
+      name = "alloca" or
+      name = "sbrk" or
+      name = "valloc"
+    )
   }
 }
 
 from Function f, Allocation a
-where not f instanceof Initialization and
-      a.getEnclosingFunction() = f
+where
+  not f instanceof Initialization and
+  a.getEnclosingFunction() = f
 select a, "Dynamic memory allocation is only allowed during initialization."

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

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

cpp/ql/src/JPL_C/LOC-2/Rule 09/AvoidNestedSemaphores.ql

@@ -12,20 +12,22 @@
 import Semaphores
 
 LockOperation maybeLocked(Function f) {
-  result.getEnclosingFunction() = f or
-  exists(Function g | f.calls(g) |
-    result = maybeLocked(g)
-  )
+  result.getEnclosingFunction() = f
+  or
+  exists(Function g | f.calls(g) | result = maybeLocked(g))
 }
 
 predicate intraproc(LockOperation inner, string msg, LockOperation outer) {
-  inner = outer.getAReachedNode() and outer.getLocked() != inner.getLocked() and
+  inner = outer.getAReachedNode() and
+  outer.getLocked() != inner.getLocked() and
   msg = "This lock operation is nested in a $@."
 }
 
 predicate interproc(FunctionCall inner, string msg, LockOperation outer) {
   inner = outer.getAReachedNode() and
-  exists(LockOperation lock | lock = maybeLocked(inner.getTarget()) and lock.getLocked() != outer.getLocked() |
+  exists(LockOperation lock |
+    lock = maybeLocked(inner.getTarget()) and lock.getLocked() != outer.getLocked()
+  |
     msg = "This call may perform a " + lock.say() + " while under the effect of a $@."
   )
 }

cpp/ql/src/JPL_C/LOC-2/Rule 09/AvoidSemaphores.ql

@@ -11,6 +11,8 @@
 import Semaphores
 
 from FunctionCall call, string kind
-where (call instanceof SemaphoreCreation and kind = "semaphores") or
-      (call instanceof LockingPrimitive  and kind = "locking primitives")
+where
+  call instanceof SemaphoreCreation and kind = "semaphores"
+  or
+  call instanceof LockingPrimitive and kind = "locking primitives"
 select call, "Use of " + kind + " should be avoided."

cpp/ql/src/JPL_C/LOC-2/Rule 09/OutOfOrderLocks.ql

@@ -18,11 +18,15 @@ predicate lockOrder(LockOperation outer, LockOperation inner) {
 
 int orderCount(Declaration outerLock, Declaration innerLock) {
   result = strictcount(LockOperation outer, LockOperation inner |
-    outer.getLocked() = outerLock and inner.getLocked() = innerLock and
-    lockOrder(outer, inner))
+      outer.getLocked() = outerLock and
+      inner.getLocked() = innerLock and
+      lockOrder(outer, inner)
+    )
 }
 
 from LockOperation outer, LockOperation inner
-where lockOrder(outer, inner)
-  and orderCount(outer.getLocked(), inner.getLocked()) <= orderCount(inner.getLocked(), outer.getLocked())
+where
+  lockOrder(outer, inner) and
+  orderCount(outer.getLocked(), inner.getLocked()) <= orderCount(inner.getLocked(),
+      outer.getLocked())
 select inner, "Out-of-order locks: A " + inner.say() + " usually precedes a $@.", outer, outer.say()

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

@@ -4,29 +4,31 @@
 
 import cpp
 
-
 class SemaphoreCreation extends FunctionCall {
   SemaphoreCreation() {
     exists(string name | name = this.getTarget().getName() |
-      name = "semBCreate" or name = "semMCreate" or name = "semCCreate" or
+      name = "semBCreate" or
+      name = "semMCreate" or
+      name = "semCCreate" or
       name = "semRWCreate"
     )
   }
 
-  Variable getSemaphore() {
-    result.getAnAccess() = this.getParent().(Assignment).getLValue()
-  }
+  Variable getSemaphore() { result.getAnAccess() = this.getParent().(Assignment).getLValue() }
 }
 
 abstract class LockOperation extends FunctionCall {
   abstract UnlockOperation getMatchingUnlock();
+
   abstract Declaration getLocked();
+
   abstract string say();
 
   ControlFlowNode getAReachedNode() {
-    result = this or
+    result = this
+    or
     exists(ControlFlowNode mid | mid = getAReachedNode() |
-      not(mid != this.getMatchingUnlock()) and
+      not mid != this.getMatchingUnlock() and
       result = mid.getASuccessor()
     )
   }
@@ -39,24 +41,21 @@ abstract class UnlockOperation extends FunctionCall {
 class SemaphoreTake extends LockOperation {
   SemaphoreTake() {
     exists(string name | name = this.getTarget().getName() |
-      name = "semTake" or
+      name = "semTake"
+      or
       // '_' is a wildcard, so this matches calls like
       // semBTakeScalable or semMTake_inline.
       name.matches("sem_Take%")
     )
   }
 
-  override Variable getLocked() {
-    result.getAnAccess() = this.getArgument(0)
-  }
+  override Variable getLocked() { result.getAnAccess() = this.getArgument(0) }
 
   override UnlockOperation getMatchingUnlock() {
     result.(SemaphoreGive).getLocked() = this.getLocked()
   }
 
-  override string say() {
-    result = "semaphore take of " + getLocked().getName()
-  }
+  override string say() { result = "semaphore take of " + getLocked().getName() }
 }
 
 class SemaphoreGive extends UnlockOperation {
@@ -67,14 +66,9 @@ class SemaphoreGive extends UnlockOperation {
     )
   }
 
-  Variable getLocked() {
-    result.getAnAccess() = this.getArgument(0)
-  }
-
-  override LockOperation getMatchingLock() {
-    this = result.getMatchingUnlock()
-  }
+  Variable getLocked() { result.getAnAccess() = this.getArgument(0) }
 
+  override LockOperation getMatchingLock() { this = result.getMatchingUnlock() }
 }
 
 class LockingPrimitive extends FunctionCall, LockOperation {
@@ -84,18 +78,16 @@ class LockingPrimitive extends FunctionCall, LockOperation {
     )
   }
 
-  override Function getLocked() {
-    result = this.getTarget()
-  }
+  override Function getLocked() { result = this.getTarget() }
 
   override UnlockOperation getMatchingUnlock() {
-    result.(UnlockingPrimitive).getTarget().getName() =
-      this.getTarget().getName().replaceAll("Lock", "Unlock")
+    result.(UnlockingPrimitive).getTarget().getName() = this
+          .getTarget()
+          .getName()
+          .replaceAll("Lock", "Unlock")
   }
 
-  override string say() {
-    result = "call to " + getLocked().getName()
-  }
+  override string say() { result = "call to " + getLocked().getName() }
 }
 
 class UnlockingPrimitive extends FunctionCall, UnlockOperation {
@@ -105,11 +97,7 @@ class UnlockingPrimitive extends FunctionCall, UnlockOperation {
     )
   }
 
-  Function getLocked() {
-    result = getMatchingLock().getLocked()
-  }
+  Function getLocked() { result = getMatchingLock().getLocked() }
 
-  override LockOperation getMatchingLock() {
-    this = result.getMatchingUnlock()
-  }
+  override LockOperation getMatchingLock() { this = result.getMatchingUnlock() }
 }

cpp/ql/src/JPL_C/LOC-2/Rule 11/SimpleControlFlowJmp.ql

@@ -15,8 +15,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/JPL_C/LOC-2/Rule 12/EnumInitialization.ql

@@ -8,15 +8,14 @@
  *       readability
  *       external/jpl
  */
+
 import cpp
 
-predicate hasInitializer(EnumConstant c) {
-  c.getInitializer().fromSource()
-}
+predicate hasInitializer(EnumConstant c) { c.getInitializer().fromSource() }
 
 /** Does this have an initializer that is not just a ref to another constant in the same enum? */
 predicate hasNonReferenceInitializer(EnumConstant c) {
-  exists (Initializer init |
+  exists(Initializer init |
     init = c.getInitializer() and
     init.fromSource() and
     not init.getExpr().(EnumConstantAccess).getTarget().getDeclaringEnum() = c.getDeclaringEnum()
@@ -24,14 +23,13 @@ predicate hasNonReferenceInitializer(EnumConstant c) {
 }
 
 predicate hasReferenceInitializer(EnumConstant c) {
-  exists (Initializer init |
+  exists(Initializer init |
     init = c.getInitializer() and
     init.fromSource() and
     init.getExpr().(EnumConstantAccess).getTarget().getDeclaringEnum() = c.getDeclaringEnum()
   )
 }
 
-
 // There exists another constant whose value is implicit, but it's
 // not the last one: the last value is okay to use to get the highest
 // enum value automatically. It can be followed by aliases though.
@@ -48,15 +46,16 @@ predicate enumThatHasConstantWithImplicitValue(Enum e) {
 }
 
 from Enum e, int i
-where // e is at position i, and has an explicit value in the source - but
-      // not just a reference to another enum constant
-      hasNonReferenceInitializer(e.getEnumConstant(i)) and
-      // but e is not the first or the last constant of the enum
-      i != 0 and
-      exists(e.getEnumConstant(i+1)) and
-      // and there exists another constant whose value is implicit, but it's
-      // not the last one: the last value is okay to use to get the highest
-      // enum value automatically. It can be followed by aliases though.
-      enumThatHasConstantWithImplicitValue(e)
-
-select e, "In an enumerator list, the = construct should not be used to explicitly initialize members other than the first, unless all items are explicitly initialized."
+where
+  // e is at position i, and has an explicit value in the source - but
+  // not just a reference to another enum constant
+  hasNonReferenceInitializer(e.getEnumConstant(i)) and
+  // but e is not the first or the last constant of the enum
+  i != 0 and
+  exists(e.getEnumConstant(i + 1)) and
+  // and there exists another constant whose value is implicit, but it's
+  // not the last one: the last value is okay to use to get the highest
+  // enum value automatically. It can be followed by aliases though.
+  enumThatHasConstantWithImplicitValue(e)
+select e,
+  "In an enumerator list, the = construct should not be used to explicitly initialize members other than the first, unless all items are explicitly initialized."

cpp/ql/src/JPL_C/LOC-3/Rule 13/ExternDeclsInHeader.ql

@@ -11,7 +11,8 @@
 import cpp
 
 from VariableDeclarationEntry v
-where v.getVariable() instanceof GlobalVariable and
+where
+  v.getVariable() instanceof GlobalVariable and
   v.hasSpecifier("extern") and
   not v.getFile() instanceof HeaderFile
 select v, v.getName() + " should be declared only in a header file that is included as needed."

cpp/ql/src/JPL_C/LOC-3/Rule 13/LimitedScopeFile.ql

@@ -13,9 +13,11 @@
 import cpp
 
 from GlobalVariable v
-where forex(VariableAccess va | va.getTarget() = v | va.getFile() = v.getDefinitionLocation().getFile())
-      and not v.hasSpecifier("static")
-      and strictcount(v.getAnAccess().getEnclosingFunction()) > 1 // If = 1, variable should be function-scope.
-      and not v.getADeclarationEntry().getFile() instanceof HeaderFile // intended to be accessed elsewhere
-select v, "The global variable " + v.getName() + " is not accessed outside of " + v.getFile().getBaseName()
-      + " and could be made static."
+where
+  forex(VariableAccess va | va.getTarget() = v | va.getFile() = v.getDefinitionLocation().getFile()) and
+  not v.hasSpecifier("static") and
+  strictcount(v.getAnAccess().getEnclosingFunction()) > 1 and // If = 1, variable should be function-scope.
+  not v.getADeclarationEntry().getFile() instanceof HeaderFile // intended to be accessed elsewhere
+select v,
+  "The global variable " + v.getName() + " is not accessed outside of " + v.getFile().getBaseName() +
+    " and could be made static."

cpp/ql/src/JPL_C/LOC-3/Rule 13/LimitedScopeFunction.ql

@@ -12,8 +12,11 @@
 import cpp
 
 from GlobalVariable v, Function f
-where v.getAnAccess().getEnclosingFunction() = f and
-      strictcount(v.getAnAccess().getEnclosingFunction()) = 1 and
-      forall(VariableAccess a | a = v.getAnAccess() | exists(a.getEnclosingFunction())) and
-      not v.getADeclarationEntry().getFile() instanceof HeaderFile // intended to be accessed elsewhere
-select v, "The variable " + v.getName() + " is only accessed in $@ and should be scoped accordingly.", f, f.getName()
+where
+  v.getAnAccess().getEnclosingFunction() = f and
+  strictcount(v.getAnAccess().getEnclosingFunction()) = 1 and
+  forall(VariableAccess a | a = v.getAnAccess() | exists(a.getEnclosingFunction())) and
+  not v.getADeclarationEntry().getFile() instanceof HeaderFile // intended to be accessed elsewhere
+select v,
+  "The variable " + v.getName() + " is only accessed in $@ and should be scoped accordingly.", f,
+  f.getName()

cpp/ql/src/JPL_C/LOC-3/Rule 13/LimitedScopeLocalHidesGlobal.ql

@@ -8,27 +8,29 @@
  *       readability
  *       external/jpl
  */
+
 import cpp
 
 class LocalVariableOrParameter extends Variable {
   LocalVariableOrParameter() {
-    this instanceof LocalVariable or
+    this instanceof LocalVariable
+    or
     // A function declaration (i.e. "int foo(int bar);") doesn't usefully
     // shadow globals; the parameter should be on the version of the function
     // that has a body.
     exists(Parameter p | p = this |
-           p.getFunction().getDefinitionLocation().getFile() = this.getFile() and
-           exists(p.getFunction().getBlock()))
+      p.getFunction().getDefinitionLocation().getFile() = this.getFile() and
+      exists(p.getFunction().getBlock())
+    )
   }
 
   string type() {
-    if this instanceof Parameter
-    then result = "Parameter "
-    else result = "Local variable "
+    if this instanceof Parameter then result = "Parameter " else result = "Local variable "
   }
 }
 
 from LocalVariableOrParameter lv, GlobalVariable gv
-where lv.getName() = gv.getName() and
-      lv.getFile() = gv.getFile()
+where
+  lv.getName() = gv.getName() and
+  lv.getFile() = gv.getFile()
 select lv, lv.type() + lv.getName() + " hides the global variable $@.", gv, gv.getName()

cpp/ql/src/JPL_C/LOC-3/Rule 14/CheckingReturnValues.ql

@@ -11,7 +11,8 @@
 
 import cpp
 
-/** In its full generality, the rule applies to all functions that
+/**
+ * In its full generality, the rule applies to all functions that
  * return non-void, including things like 'printf' and 'close',
  * which are routinely not checked because the behavior on success
  * is the same as the behavior on failure. The recommendation is
@@ -27,13 +28,15 @@ predicate whitelist(Function f) {
 }
 
 from FunctionCall c, string msg
-where not c.getTarget().getType() instanceof VoidType
-      and not whitelist(c.getTarget())
-      and
-      (
-        (c instanceof ExprInVoidContext and msg = "The return value of non-void function $@ is not checked.")
-        or
-        (definition(_, c.getParent()) and not definitionUsePair(_, c.getParent(), _) and
-            msg = "$@'s return value is stored but not checked.")
-      )
+where
+  not c.getTarget().getType() instanceof VoidType and
+  not whitelist(c.getTarget()) and
+  (
+    c instanceof ExprInVoidContext and
+    msg = "The return value of non-void function $@ is not checked."
+    or
+    definition(_, c.getParent()) and
+    not definitionUsePair(_, c.getParent(), _) and
+    msg = "$@'s return value is stored but not checked."
+  )
 select c, msg, c.getTarget() as f, f.getName()

cpp/ql/src/JPL_C/LOC-3/Rule 15/CheckingParameterValues.ql

@@ -12,16 +12,18 @@
 import JPL_C.Tasks
 
 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 p.getFunction() instanceof PublicFunction and
-    not exists(Expr e | e.isCondition() | e.getAChild*() = va)
+where
+  va = firstAccess(p) and
+  p.getFunction() instanceof PublicFunction 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/JPL_C/LOC-3/Rule 16/UseOfAssertionsConstant.ql

@@ -12,9 +12,9 @@
 import semmle.code.cpp.commons.Assertions
 
 from Assertion a, string value, string msg
-where value = a.getAsserted().getValue() and
-    if value.toInt() = 0 then
-        msg = "This assertion is always false."
-    else
-        msg = "This assertion is always true."
+where
+  value = a.getAsserted().getValue() and
+  if value.toInt() = 0
+  then msg = "This assertion is always false."
+  else msg = "This assertion is always true."
 select a.getAsserted(), msg

cpp/ql/src/JPL_C/LOC-3/Rule 16/UseOfAssertionsDensity.ql

@@ -12,6 +12,7 @@
 import semmle.code.cpp.commons.Assertions
 
 from Function f
-where f.getMetrics().getNumberOfLinesOfCode() > 10 and
+where
+  f.getMetrics().getNumberOfLinesOfCode() > 10 and
   not exists(Assertion a | a.getAsserted().getEnclosingFunction() = f)
 select f, "All functions of more than 10 lines should have at least one assertion."

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

@@ -13,11 +13,16 @@ import cpp
 
 predicate allowedTypedefs(TypedefType t) {
   exists(string name | name = t.getName() |
-    name = "I64" or name = "U64" or
-    name = "I32" or name = "U32" or
-    name = "I16" or name = "U16" or
-    name = "I8" or name = "U8" or
-    name = "F64" or name = "F32"
+    name = "I64" or
+    name = "U64" or
+    name = "I32" or
+    name = "U32" or
+    name = "I16" or
+    name = "U16" or
+    name = "I8" or
+    name = "U8" or
+    name = "F64" or
+    name = "F32"
   )
 }
 
@@ -25,7 +30,8 @@ predicate allowedTypedefs(TypedefType t) {
  * Gets a type which appears literally in the declaration of `d`.
  */
 Type getAnImmediateUsedType(Declaration d) {
-  d.isDefined() and (
+  d.isDefined() and
+  (
     result = d.(Function).getType() or
     result = d.(Variable).getType()
   )
@@ -48,7 +54,11 @@ predicate problematic(IntegralType t) {
 }
 
 from Declaration d, Type usedType
-where usedType = getAUsedType*(getAnImmediateUsedType(d)) and problematic(usedType)
+where
+  usedType = getAUsedType*(getAnImmediateUsedType(d)) and
+  problematic(usedType) and
   // Ignore violations for which we do not have a valid location.
-  and not(d.getLocation() instanceof UnknownLocation)
-select d, d.getName() + " uses the basic integral type " + usedType.getName() + " rather than a typedef with size and signedness."
+  not d.getLocation() instanceof UnknownLocation
+select d,
+  d.getName() + " uses the basic integral type " + usedType.getName() +
+    " rather than a typedef with size and signedness."

cpp/ql/src/JPL_C/LOC-3/Rule 18/CompoundExpressions.ql

@@ -12,7 +12,9 @@
 import cpp
 
 from BinaryOperation parent, BinaryOperation child
-where parent.getAnOperand() = child and not child.isParenthesised() and
+where
+  parent.getAnOperand() = child and
+  not child.isParenthesised() and
   (parent instanceof BinaryBitwiseOperation or child instanceof BinaryBitwiseOperation) and
   // Some benign cases...
   not (parent instanceof BitwiseAndExpr and child instanceof BitwiseAndExpr) and

cpp/ql/src/JPL_C/LOC-3/Rule 19/NoBooleanSideEffects.ql

@@ -46,10 +46,9 @@ predicate inherentlyUnsafe(Function f) {
   exists(Variable v | v.getAnAssignedValue().getEnclosingFunction() = f |
     v instanceof GlobalVariable or
     v.isStatic()
-  ) or
-  exists(FunctionCall c | c.getEnclosingFunction() = f |
-    inherentlyUnsafe(c.getTarget())
   )
+  or
+  exists(FunctionCall c | c.getEnclosingFunction() = f | inherentlyUnsafe(c.getTarget()))
 }
 
 /**
@@ -59,7 +58,9 @@ predicate inherentlyUnsafe(Function f) {
  * not inherently unsafe.
  */
 predicate safeToCall(Function f) {
-  forall(PointerType paramPointerType | paramPointerType = getAPointerType(f.getAParameter().getType()) |
+  forall(PointerType paramPointerType |
+    paramPointerType = getAPointerType(f.getAParameter().getType())
+  |
     paramPointerType.getBaseType().isConst()
   ) and
   not inherentlyUnsafe(f)
@@ -78,12 +79,16 @@ class BooleanExpression extends Expr {
 }
 
 predicate hasSideEffect(Expr e) {
-  e instanceof Assignment or
-  e instanceof CrementOperation or
-  e instanceof ExprCall or
+  e instanceof Assignment
+  or
+  e instanceof CrementOperation
+  or
+  e instanceof ExprCall
+  or
   exists(Function f | f = e.(FunctionCall).getTarget() and not safeFunctionWhitelist(f) |
     inherentlyUnsafe(f) or not safeToCall(f)
-  ) or
+  )
+  or
   hasSideEffect(e.getAChild())
 }
 

cpp/ql/src/JPL_C/LOC-4/Rule 20/PreprocessorUse.ql

@@ -12,12 +12,13 @@
 import cpp
 
 from PreprocessorDirective p
-where not p instanceof Include and
-      not p instanceof Macro and
-      not p instanceof PreprocessorIf and
-      not p instanceof PreprocessorElif and
-      not p instanceof PreprocessorElse and
-      not p instanceof PreprocessorIfdef and
-      not p instanceof PreprocessorIfndef and
-      not p instanceof PreprocessorEndif
+where
+  not p instanceof Include and
+  not p instanceof Macro and
+  not p instanceof PreprocessorIf and
+  not p instanceof PreprocessorElif and
+  not p instanceof PreprocessorElse and
+  not p instanceof PreprocessorIfdef and
+  not p instanceof PreprocessorIfndef and
+  not p instanceof PreprocessorEndif
 select p, "This preprocessor directive is not allowed."

cpp/ql/src/JPL_C/LOC-4/Rule 20/PreprocessorUseIfdef.ql

@@ -12,6 +12,7 @@
 import cpp
 
 from PreprocessorDirective i
-where (i instanceof PreprocessorIf or i instanceof PreprocessorIfdef or i instanceof PreprocessorIfndef)
-      and not i.getFile() instanceof HeaderFile
+where
+  (i instanceof PreprocessorIf or i instanceof PreprocessorIfdef or i instanceof PreprocessorIfndef) and
+  not i.getFile() instanceof HeaderFile
 select i, "Use of conditional compilation must be kept to a minimum."

cpp/ql/src/JPL_C/LOC-4/Rule 20/PreprocessorUseUndisciplined.ql

@@ -12,6 +12,10 @@
 import cpp
 
 from Macro m, string msg
-where (m.getHead().matches("%...%") and msg = "The macro " + m.getHead() + " is variadic, and hence not allowed.") or
-      (m.getBody().matches("%##%") and msg = "The macro " + m.getHead() + " uses token pasting and is not allowed.")
+where
+  m.getHead().matches("%...%") and
+  msg = "The macro " + m.getHead() + " is variadic, and hence not allowed."
+  or
+  m.getBody().matches("%##%") and
+  msg = "The macro " + m.getHead() + " uses token pasting and is not allowed."
 select m, msg

cpp/ql/src/JPL_C/LOC-4/Rule 21/MacroInBlock.ql

@@ -12,8 +12,10 @@
 import cpp
 
 int lineInBlock(File f) {
-  exists(Block block, Location blockLocation | block.getFile() = f and blockLocation = block.getLocation()|
-    result in [blockLocation.getStartLine()..blockLocation.getEndLine()]
+  exists(Block block, Location blockLocation |
+    block.getFile() = f and blockLocation = block.getLocation()
+  |
+    result in [blockLocation.getStartLine() .. blockLocation.getEndLine()]
   )
 }
 

cpp/ql/src/JPL_C/LOC-4/Rule 23/MismatchedIfdefs.ql

@@ -12,43 +12,35 @@
 import cpp
 
 class FileWithDirectives extends File {
-  FileWithDirectives() {
-    exists(Directive d | d.getFile() = this)
-  }
+  FileWithDirectives() { exists(Directive d | d.getFile() = this) }
 
   int getDirectiveLine(Directive d) {
     d.getFile() = this and d.getLocation().getStartLine() = result
   }
 
   int getDirectiveIndex(Directive d) {
-    exists(int line | line = getDirectiveLine(d) |
-      line = rank[result](getDirectiveLine(_))
-    )
+    exists(int line | line = getDirectiveLine(d) | line = rank[result](getDirectiveLine(_)))
   }
 
   int depth(Directive d) {
     exists(int index | index = getDirectiveIndex(d) |
-      (index = 1 and result = d.depthChange()) or
-      exists(Directive prev | getDirectiveIndex(prev) = index-1 |
+      index = 1 and result = d.depthChange()
+      or
+      exists(Directive prev | getDirectiveIndex(prev) = index - 1 |
         result = d.depthChange() + depth(prev)
       )
     )
   }
 
-  Directive lastDirective() {
-    getDirectiveIndex(result) = max(getDirectiveIndex(_))
-  }
+  Directive lastDirective() { getDirectiveIndex(result) = max(getDirectiveIndex(_)) }
 }
 
 abstract class Directive extends PreprocessorDirective {
   abstract int depthChange();
+
   abstract predicate mismatched();
 
-  int depth() {
-    exists(FileWithDirectives f |
-      f.depth(this) = result
-    )
-  }
+  int depth() { exists(FileWithDirectives f | f.depth(this) = result) }
 }
 
 class IfDirective extends Directive {
@@ -59,6 +51,7 @@ class IfDirective extends Directive {
   }
 
   override int depthChange() { result = 1 }
+
   override predicate mismatched() { none() }
 }
 
@@ -69,24 +62,26 @@ class ElseDirective extends Directive {
   }
 
   override int depthChange() { result = 0 }
+
   override predicate mismatched() { depth() < 1 }
 }
 
 class EndifDirective extends Directive {
-  EndifDirective() {
-    this instanceof PreprocessorEndif
-  }
+  EndifDirective() { this instanceof PreprocessorEndif }
 
   override int depthChange() { result = -1 }
+
   override predicate mismatched() { depth() < 0 }
 }
 
 from FileWithDirectives f, Directive d, string msg
-where d.getFile() = f and
-  if d.mismatched() then (
-    msg = "'" + d + "' has no matching #if in file " + f.getBaseName() + "."
-  ) else (
-    d = f.lastDirective() and d.depth() > 0 and msg = "File " + f.getBaseName() +
-        " ends with " + d.depth() + " unterminated #if directives."
+where
+  d.getFile() = f and
+  if d.mismatched()
+  then msg = "'" + d + "' has no matching #if in file " + f.getBaseName() + "."
+  else (
+    d = f.lastDirective() and
+    d.depth() > 0 and
+    msg = "File " + f.getBaseName() + " ends with " + d.depth() + " unterminated #if directives."
   )
 select d, msg

cpp/ql/src/JPL_C/LOC-4/Rule 24/MultipleStmtsPerLine.ql

@@ -22,16 +22,16 @@ class OneLineStmt extends Stmt {
   }
 }
 
-int numStmt(File f, int line) {
-  result = strictcount(OneLineStmt o | o.onLine(f, line))
-}
+int numStmt(File f, int line) { result = strictcount(OneLineStmt o | o.onLine(f, line)) }
 
 from File f, int line, OneLineStmt o, int cnt
-where numStmt(f, line) = cnt
-  and cnt > 1
-  and o.onLine(f, line)
-  and o.getLocation().getStartColumn() =
-        min(OneLineStmt other, int toMin
-          | other.onLine(f, line) and toMin = other.getLocation().getStartColumn()
-          | toMin)
+where
+  numStmt(f, line) = cnt and
+  cnt > 1 and
+  o.onLine(f, line) and
+  o.getLocation().getStartColumn() = min(OneLineStmt other, int toMin |
+      other.onLine(f, line) and toMin = other.getLocation().getStartColumn()
+    |
+      toMin
+    )
 select o, "This line contains " + cnt + " statements; only one is allowed."

cpp/ql/src/JPL_C/LOC-4/Rule 24/MultipleVarDeclsPerLine.ql

@@ -12,7 +12,8 @@
 import cpp
 
 from DeclStmt d
-where exists(Variable v1, Variable v2 | v1 = d.getADeclaration() and v2 = d.getADeclaration() |
+where
+  exists(Variable v1, Variable v2 | v1 = d.getADeclaration() and v2 = d.getADeclaration() |
     v1 != v2 and
     v1.getLocation().getStartLine() = v2.getLocation().getStartLine()
   )

cpp/ql/src/JPL_C/LOC-4/Rule 25/FunctionSizeLimits.ql

@@ -26,6 +26,7 @@ string paramWarning(Function f) {
 }
 
 from Function f, string msg
-where msg = lengthWarning(f) or
-      msg = paramWarning(f)
+where
+  msg = lengthWarning(f) or
+  msg = paramWarning(f)
 select f, msg

cpp/ql/src/JPL_C/LOC-4/Rule 26/DeclarationPointerNesting.ql

@@ -15,7 +15,7 @@ string var(Variable v) {
   exists(int level | level = v.getType().getPointerIndirectionLevel() |
     level > 2 and
     result = "The type of " + v.getName() + " uses " + level +
-      " levels of pointer indirection -- maximum allowed is 2."
+        " levels of pointer indirection -- maximum allowed is 2."
   )
 }
 
@@ -23,7 +23,7 @@ string fun(Function f) {
   exists(int level | level = f.getType().getPointerIndirectionLevel() |
     level > 2 and
     result = "The return type of " + f.getName() + " uses " + level +
-      " levels of pointer indirection -- maximum allowed is 2."
+        " levels of pointer indirection -- maximum allowed is 2."
   )
 }
 

cpp/ql/src/JPL_C/LOC-4/Rule 27/PointerDereferenceInStmt.ql

@@ -12,7 +12,8 @@
 import cpp
 
 from PointerDereferenceExpr e, int n
-where not e.getParent+() instanceof PointerDereferenceExpr
-  and n = strictcount(PointerDereferenceExpr child | child.getParent+() = e)
-  and n > 1
+where
+  not e.getParent+() instanceof PointerDereferenceExpr and
+  n = strictcount(PointerDereferenceExpr child | child.getParent+() = e) and
+  n > 1
 select e, "This expression involves " + n + " levels of pointer dereference; 2 are allowed."

cpp/ql/src/JPL_C/LOC-4/Rule 28/HiddenPointerDereferenceMacro.ql

@@ -12,9 +12,11 @@
 import cpp
 
 from Macro m
-where forex(MacroInvocation mi | mi.getMacro() = m |
+where
+  forex(MacroInvocation mi | mi.getMacro() = m |
     exists(PointerDereferenceExpr e, Location miLoc, Location eLoc | e = mi.getAGeneratedElement() |
-      miLoc = mi.getLocation() and eLoc = e.getLocation() and
+      miLoc = mi.getLocation() and
+      eLoc = e.getLocation() and
       eLoc.getStartColumn() = miLoc.getStartColumn() and
       eLoc.getStartLine() = miLoc.getStartLine()
     )

cpp/ql/src/JPL_C/LOC-4/Rule 30/FunctionPointerConversions.ql

@@ -20,7 +20,9 @@ predicate permissibleConversion(Type t) {
 }
 
 from Expr e, Type converted
-where e.getType() instanceof FunctionPointerType and
+where
+  e.getType() instanceof FunctionPointerType and
   e.getFullyConverted().getType() = converted and
   not permissibleConversion(converted)
-select e, "Function pointer converted to " + converted.getName() + ", which is not an integral type."
+select e,
+  "Function pointer converted to " + converted.getName() + ", which is not an integral type."

cpp/ql/src/JPL_C/LOC-4/Rule 31/IncludesFirst.ql

@@ -12,8 +12,16 @@
 import cpp
 
 int firstCodeLine(File f) {
-  result = min(Declaration d, Location l, int toMin | (l = d.getLocation() and
-    l.getFile() = f and not d.isInMacroExpansion()) and (toMin = l.getStartLine()) | toMin)
+  result = min(Declaration d, Location l, int toMin |
+      (
+        l = d.getLocation() and
+        l.getFile() = f and
+        not d.isInMacroExpansion()
+      ) and
+      toMin = l.getStartLine()
+    |
+      toMin
+    )
 }
 
 int badIncludeLine(File f, Include i) {
@@ -23,7 +31,9 @@ int badIncludeLine(File f, Include i) {
 }
 
 from File f, Include i, int line
-where line = badIncludeLine(f, i) and
+where
+  line = badIncludeLine(f, i) and
   line = min(badIncludeLine(f, _))
-select i, "'" + i.toString() + "' is preceded by code -- it should be moved above line " +
-  firstCodeLine(f) + " in " + f.getBaseName() + "."
+select i,
+  "'" + i.toString() + "' is preceded by code -- it should be moved above line " + firstCodeLine(f) +
+    " in " + f.getBaseName() + "."

cpp/ql/src/JPL_C/Tasks.qll

@@ -22,7 +22,8 @@ class Task extends Function {
  */
 class PublicFunction extends Function {
   PublicFunction() {
-    not this.isStatic() and (
+    not this.isStatic() and
+    (
       strictcount(Task t | t.calls+(this)) > 1 or
       not exists(Task t | t.getFile() = this.getFile())
     )

cpp/ql/src/Likely Bugs/AmbiguouslySignedBitField.ql

@@ -14,17 +14,22 @@
  *       language-features
  *       external/cwe/cwe-190
  */
+
 import cpp
 
 from BitField bf
-where not bf.getUnspecifiedType().(IntegralType).isExplicitlySigned()
-  and not bf.getUnspecifiedType().(IntegralType).isExplicitlyUnsigned()
-  and not bf.getUnspecifiedType() instanceof Enum
-  and not bf.getUnspecifiedType() instanceof BoolType
+where
+  not bf.getUnspecifiedType().(IntegralType).isExplicitlySigned() and
+  not bf.getUnspecifiedType().(IntegralType).isExplicitlyUnsigned() and
+  not bf.getUnspecifiedType() instanceof Enum and
+  not bf.getUnspecifiedType() instanceof BoolType and
   // At least for C programs on Windows, BOOL is a common typedef for a type
   // representing BoolType.
-  and not bf.getType().hasName("BOOL")
+  not bf.getType().hasName("BOOL") and
   // If this is true, then there cannot be unsigned sign extension or overflow.
-  and not bf.getDeclaredNumBits() = bf.getType().getSize() * 8
-  and not bf.isAnonymous()
-select bf, "Bit field " + bf.getName() + " of type " +  bf.getUnderlyingType().getName() +  " should have explicitly unsigned integral, explicitly signed integral, or enumeration type."
+  not bf.getDeclaredNumBits() = bf.getType().getSize() * 8 and
+  not bf.isAnonymous() and
+  not bf.isFromUninstantiatedTemplate(_)
+select bf,
+  "Bit field " + bf.getName() + " of type " + bf.getUnderlyingType().getName() +
+    " should have explicitly unsigned integral, explicitly signed integral, or enumeration type."

cpp/ql/src/Likely Bugs/Arithmetic/BadAdditionOverflowCheck.qhelp

@@ -2,36 +2,39 @@
   "-//Semmle//qhelp//EN"
   "qhelp.dtd">
 <qhelp>
-  <overview>
-    <p>
-      Checking for overflow of integer addition needs to be done with
-      care, because automatic type promotion can prevent the check
-      from working correctly.
-    </p>
-  </overview>
-  <recommendation>
-    <p>
-      Use an explicit cast to make sure that the result of the addition is
-      not implicitly converted to a larger type.
-    </p>
-  </recommendation>
-  <example>
-    <sample src="BadAdditionOverflowCheckExample1.cpp" />
-    <p>
-      On a typical architecture where <tt>short</tt> is 16 bits
-      and <tt>int</tt> is 32 bits, the operands of the addition are
-      automatically promoted to <tt>int</tt>, so it cannot overflow
-      and the result of the comparison is always false.
-    </p>
-    <p>
-      The code below implements the check correctly, by using an
-      explicit cast to make sure that the result of the addition
-      is <tt>unsigned short</tt>.
-    </p>
-    <sample src="BadAdditionOverflowCheckExample2.cpp" />
-  </example>
-  <references>
-    <li><a href="http://c-faq.com/expr/preservingrules.html">Preserving Rules</a></li>
-    <li><a href="https://www.securecoding.cert.org/confluence/plugins/servlet/mobile#content/view/20086942">Understand integer conversion rules</a></li>
-  </references>
+
+<overview>
+<p>
+Checking for overflow of integer addition needs to be done with
+care, because automatic type promotion can prevent the check
+from working as intended, with the same value (<code>true</code>
+or <code>false</code>) always being returned.
+</p>
+</overview>
+<recommendation>
+<p>
+Use an explicit cast to make sure that the result of the addition is
+not implicitly converted to a larger type.
+</p>
+</recommendation>
+<example>
+<sample src="BadAdditionOverflowCheckExample1.cpp" />
+<p>
+On a typical architecture where <code>short</code> is 16 bits
+and <code>int</code> is 32 bits, the operands of the addition are
+automatically promoted to <code>int</code>, so it cannot overflow
+and the result of the comparison is always false.
+</p>
+<p>
+The code below implements the check correctly, by using an
+explicit cast to make sure that the result of the addition
+is <code>unsigned short</code> (which may overflow, in which case
+the comparison would evaluate to <code>true</code>).
+</p>
+<sample src="BadAdditionOverflowCheckExample2.cpp" />
+</example>
+<references>
+<li><a href="http://c-faq.com/expr/preservingrules.html">Preserving Rules</a></li>
+<li><a href="https://www.securecoding.cert.org/confluence/plugins/servlet/mobile#content/view/20086942">Understand integer conversion rules</a></li>
+</references>
 </qhelp>

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

@@ -13,8 +13,9 @@ private import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
  * swapping the operands both ways round.
  */
 private predicate addExpr(AddExpr plus, Expr a, Expr b) {
-  (a = plus.getLeftOperand() and b = plus.getRightOperand()) or
-  (b = plus.getLeftOperand() and a = plus.getRightOperand())
+  a = plus.getLeftOperand() and b = plus.getRightOperand()
+  or
+  b = plus.getLeftOperand() and a = plus.getRightOperand()
 }
 
 /**
@@ -29,12 +30,12 @@ private predicate addExpr(AddExpr plus, Expr a, Expr b) {
  * false.
  */
 predicate badAdditionOverflowCheck(RelationalOperation cmp, AddExpr plus) {
-  exists (Variable v, VariableAccess a1, VariableAccess a2, Expr b
-  | addExpr(plus, a1, b) and
+  exists(Variable v, VariableAccess a1, VariableAccess a2, Expr b |
+    addExpr(plus, a1, b) and
     a1 = v.getAnAccess() and
     a2 = v.getAnAccess() and
-    not exists (a1.getQualifier()) and // Avoid structure fields
-    not exists (a2.getQualifier()) and // Avoid structure fields
+    not exists(a1.getQualifier()) and // Avoid structure fields
+    not exists(a2.getQualifier()) and // Avoid structure fields
     // Simple type-based check that the addition cannot overflow.
     exprMinVal(plus) <= exprMinVal(a1) + exprMinVal(b) and
     exprMaxVal(plus) > exprMaxVal(a1) and
@@ -43,5 +44,6 @@ predicate badAdditionOverflowCheck(RelationalOperation cmp, AddExpr plus) {
     exprMinVal(plus.getExplicitlyConverted()) <= exprMinVal(plus) and
     exprMaxVal(plus.getExplicitlyConverted()) >= exprMaxVal(plus) and
     cmp.getAnOperand() = plus and
-    cmp.getAnOperand() = a2)
+    cmp.getAnOperand() = a2
+  )
 }

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

@@ -1,3 +1,4 @@
 bool checkOverflow(unsigned short x, unsigned short y) {
-  return (x + y < x);  // BAD: x and y are automatically promoted to int.
+  // BAD: comparison is always false due to type promotion
+  return (x + y < x);  
 }

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

@@ -10,12 +10,14 @@
  *       correctness
  *       types
  */
+
 import cpp
 
 from EqualityOperation t, RemExpr lhs, Literal rhs
-where t.getLeftOperand() = lhs and
-      t.getRightOperand() = rhs and
-      lhs.getLeftOperand().getUnspecifiedType().(IntegralType).isSigned() and
-      lhs.getRightOperand().getValue() = "2" and
-      rhs.getValue() = "1"
+where
+  t.getLeftOperand() = lhs and
+  t.getRightOperand() = rhs and
+  lhs.getLeftOperand().getUnspecifiedType().(IntegralType).isSigned() and
+  lhs.getRightOperand().getValue() = "2" and
+  rhs.getValue() = "1"
 select t, "Possibly invalid test for oddness. This will fail for negative numbers."

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

@@ -9,12 +9,21 @@
  * @tags reliability
  *       correctness
  */
+
 import cpp
 
 from RelationalOperation e, BinaryBitwiseOperation lhs
-where lhs = e.getLeftOperand() and
-      lhs.getActualType().(IntegralType).isSigned() and
-      forall(int op | op = lhs.(BitwiseAndExpr).getAnOperand().getValue().toInt() | op < 0) and
-      e.getRightOperand().getValue() = "0" and
-      not e.isAffectedByMacro()
+where
+  // `lhs > 0` (or `0 < lhs`)
+  // (note that `lhs < 0`, `lhs >= 0` or `lhs <= 0` all imply that the signedness of
+  //  `lhs` is understood, so should not be flagged).
+  (e instanceof GTExpr or e instanceof LTExpr) and
+  e.getGreaterOperand() = lhs and
+  e.getLesserOperand().getValue() = "0" and
+  // lhs is signed
+  lhs.getActualType().(IntegralType).isSigned() and
+  // if `lhs` has the form `x & c`, with constant `c`, `c` is negative
+  forall(int op | op = lhs.(BitwiseAndExpr).getAnOperand().getValue().toInt() | op < 0) and
+  // exception for cases involving macros
+  not e.isAffectedByMacro()
 select e, "Potential unsafe sign check of a bitwise operation."

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

@@ -10,9 +10,12 @@
  * @tags maintainability
  *       readability
  */
+
 import cpp
 
-
 from ComparisonOperation co, ComparisonOperation chco
-where co.getAChild() = chco and not chco.isParenthesised()
-select co, "Check the comparison operator precedence."
+where
+  co.getAChild() = chco and
+  not chco.isParenthesised() and
+  not co.isFromUninstantiatedTemplate(_)
+select co, "Comparison as an operand to another comparison."

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

@@ -25,11 +25,15 @@ import PointlessSelfComparison
  *     `parent = 2 * child`
  */
 private predicate linearChild(Expr parent, Expr child, float multiplier) {
-  (child = parent.(AddExpr).getAChild() and multiplier = 1.0) or
-  (child = parent.(SubExpr).getLeftOperand() and multiplier = 1.0) or
-  (child = parent.(SubExpr).getRightOperand() and multiplier = -1.0) or
-  (child = parent.(UnaryPlusExpr).getOperand() and multiplier = 1.0) or
-  (child = parent.(UnaryMinusExpr).getOperand() and multiplier = -1.0)
+  child = parent.(AddExpr).getAChild() and multiplier = 1.0
+  or
+  child = parent.(SubExpr).getLeftOperand() and multiplier = 1.0
+  or
+  child = parent.(SubExpr).getRightOperand() and multiplier = -1.0
+  or
+  child = parent.(UnaryPlusExpr).getOperand() and multiplier = 1.0
+  or
+  child = parent.(UnaryMinusExpr).getOperand() and multiplier = -1.0
 }
 
 /**
@@ -41,18 +45,19 @@ private predicate linearChild(Expr parent, Expr child, float multiplier) {
  * In this example, `x` has multiplier 4, `y` has multiplier -1, and `z`
  * has multiplier -3 (multipliers from the right hand child are negated).
  */
-private predicate cmpLinearSubExpr(
-  ComparisonOperation cmp, Expr child, float multiplier) {
-  not convertedExprMightOverflow(child)
-  and
-  ((child = cmp.getLeftOperand() and multiplier = 1.0)
-   or
-   (child = cmp.getRightOperand() and multiplier = -1.0)
-   or
-   exists (Expr parent, float m1, float m2
-   | cmpLinearSubExpr(cmp, parent, m1) and
-     linearChild(parent, child, m2) and
-     multiplier = m1 * m2))
+private predicate cmpLinearSubExpr(ComparisonOperation cmp, Expr child, float multiplier) {
+  not convertedExprMightOverflow(child) and
+  (
+    child = cmp.getLeftOperand() and multiplier = 1.0
+    or
+    child = cmp.getRightOperand() and multiplier = -1.0
+    or
+    exists(Expr parent, float m1, float m2 |
+      cmpLinearSubExpr(cmp, parent, m1) and
+      linearChild(parent, child, m2) and
+      multiplier = m1 * m2
+    )
+  )
 }
 
 /**
@@ -60,9 +65,10 @@ private predicate cmpLinearSubExpr(
  * `child` is an access of variable `v`.
  */
 private predicate cmpLinearSubVariable(
-  ComparisonOperation cmp, Variable v, VariableAccess child, float multiplier) {
+  ComparisonOperation cmp, Variable v, VariableAccess child, float multiplier
+) {
   v = child.getTarget() and
-  not exists (child.getQualifier()) and
+  not exists(child.getQualifier()) and
   cmpLinearSubExpr(cmp, child, multiplier)
 }
 
@@ -75,23 +81,19 @@ private predicate cmpLinearSubVariable(
  *     `v + x - v < y`
  *     `v + x + v < y + 2*v`
  */
-private predicate cancelingSubExprs(
-  ComparisonOperation cmp, VariableAccess a1, VariableAccess a2) {
-  exists (Variable v
-  | exists (float m | m < 0 and cmpLinearSubVariable(cmp, v, a1, m)) and
-    exists (float m | m > 0 and cmpLinearSubVariable(cmp, v, a2, m)))
+private predicate cancelingSubExprs(ComparisonOperation cmp, VariableAccess a1, VariableAccess a2) {
+  exists(Variable v |
+    exists(float m | m < 0 and cmpLinearSubVariable(cmp, v, a1, m)) and
+    exists(float m | m > 0 and cmpLinearSubVariable(cmp, v, a2, m))
+  )
 }
 
 from ComparisonOperation cmp, VariableAccess a1, VariableAccess a2
 where
-  cancelingSubExprs(cmp, a1, a2)
-
+  cancelingSubExprs(cmp, a1, a2) and
   // Most practical examples found by this query are instances of
   // BadAdditionOverflowCheck or PointlessSelfComparison.
-  and not badAdditionOverflowCheck(cmp, _)
-  and not pointlessSelfComparison(cmp)
-select
-  cmp,
-  "Comparison can be simplified by canceling $@ with $@.",
-  a1, a1.getTarget().getName(),
+  not badAdditionOverflowCheck(cmp, _) and
+  not pointlessSelfComparison(cmp)
+select cmp, "Comparison can be simplified by canceling $@ with $@.", a1, a1.getTarget().getName(),
   a2, a2.getTarget().getName()

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

@@ -10,11 +10,15 @@
  * @tags reliability
  *       correctness
  */
+
 import cpp
 
 from EqualityOperation ro, Expr left, Expr right
-where left = ro.getLeftOperand() and right = ro.getRightOperand() and
-      ro.getAnOperand().getExplicitlyConverted().getType().getUnderlyingType() instanceof FloatingPointType and
-      not ro.getAnOperand().isConstant() and // comparisons to constants generate too many false positives
-      not left.(VariableAccess).getTarget() = right.(VariableAccess).getTarget() // skip self comparison
+where
+  left = ro.getLeftOperand() and
+  right = ro.getRightOperand() and
+  ro.getAnOperand().getExplicitlyConverted().getType().getUnderlyingType() instanceof
+    FloatingPointType and
+  not ro.getAnOperand().isConstant() and // comparisons to constants generate too many false positives
+  not left.(VariableAccess).getTarget() = right.(VariableAccess).getTarget() // skip self comparison
 select ro, "Equality test on floating point values may not behave as expected."

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

@@ -15,6 +15,7 @@
  *       external/cwe/cwe-197
  *       external/cwe/cwe-681
  */
+
 import cpp
 import semmle.code.cpp.controlflow.SSA
 
@@ -28,9 +29,12 @@ import semmle.code.cpp.controlflow.SSA
  * controlled, so we consider it less likely to cause an overflow.
  */
 predicate likelySmall(Expr e) {
-  e.isConstant() or
-  e.getType().getSize() <= 1 or
-  e.(ArrayExpr).getArrayBase().getType().(ArrayType).getBaseType().isConst() or
+  e.isConstant()
+  or
+  e.getType().getSize() <= 1
+  or
+  e.(ArrayExpr).getArrayBase().getType().(ArrayType).getBaseType().isConst()
+  or
   exists(SsaDefinition def, Variable v |
     def.getAUse(v) = e and
     likelySmall(def.getDefiningValue(v))
@@ -41,9 +45,7 @@ predicate likelySmall(Expr e) {
  * Gets an operand of a multiply expression (we need the restriction
  * to multiply expressions to get the correct transitive closure).
  */
-Expr getMulOperand(MulExpr me) {
-  result = me.getAnOperand()
-}
+Expr getMulOperand(MulExpr me) { result = me.getAnOperand() }
 
 /**
  * Gets the number of non-constant operands of a multiply expression,
@@ -56,53 +58,50 @@ Expr getMulOperand(MulExpr me) {
  */
 int getEffectiveMulOperands(MulExpr me) {
   result = count(Expr op |
-    op = getMulOperand*(me) and
-    not op instanceof MulExpr and
-    not likelySmall(op)
-  )
+      op = getMulOperand*(me) and
+      not op instanceof MulExpr and
+      not likelySmall(op)
+    )
 }
 
 from MulExpr me, Type t1, Type t2
-where t1 = me.getType().getUnderlyingType() and
-      t2 = me.getConversion().getType().getUnderlyingType() and
-      t1.getSize() < t2.getSize() and
-      (
-        (
-          t1.getUnspecifiedType() instanceof IntegralType and
-          t2.getUnspecifiedType() instanceof IntegralType
-        ) or (
-          t1.getUnspecifiedType() instanceof FloatingPointType and
-          t2.getUnspecifiedType() instanceof FloatingPointType
-        )
-      ) and
-
-      // exclude explicit conversions
-      me.getConversion().isCompilerGenerated() and
-
-      // require the multiply to have two non-constant operands
-      // (the intuition here is that multiplying two unknowns is
-      // much more likely to produce a result that needs significantly
-      // more bits than the operands did, and thus requires a larger
-      // type).
-      getEffectiveMulOperands(me) >= 2 and
-
-      // exclude varargs promotions
-      not exists(FunctionCall fc, int vararg |
-        fc.getArgument(vararg) = me and
-        vararg >= fc.getTarget().getNumberOfParameters()
-      ) and
-
-      // exclude cases where the type was made bigger by a literal
-      // (compared to other cases such as assignment, this is more
-      // likely to be a trivial accident rather than suggesting a
-      // larger type is needed for the result).
-      not exists(Expr other, Expr e |
-        other = me.getParent().(BinaryOperation).getAnOperand() and
-        not other = me and
-        (
-          e = other or
-          e = other.(BinaryOperation).getAnOperand*()
-        ) and
-        e.(Literal).getType().getSize() = t2.getSize()
-      )
-select me, "Multiplication result may overflow '" + me.getType().toString() + "' before it is converted to '" + me.getFullyConverted().getType().toString() + "'."
+where
+  t1 = me.getType().getUnderlyingType() and
+  t2 = me.getConversion().getType().getUnderlyingType() and
+  t1.getSize() < t2.getSize() and
+  (
+    t1.getUnspecifiedType() instanceof IntegralType and
+    t2.getUnspecifiedType() instanceof IntegralType
+    or
+    t1.getUnspecifiedType() instanceof FloatingPointType and
+    t2.getUnspecifiedType() instanceof FloatingPointType
+  ) and
+  // exclude explicit conversions
+  me.getConversion().isCompilerGenerated() and
+  // require the multiply to have two non-constant operands
+  // (the intuition here is that multiplying two unknowns is
+  // much more likely to produce a result that needs significantly
+  // more bits than the operands did, and thus requires a larger
+  // type).
+  getEffectiveMulOperands(me) >= 2 and
+  // exclude varargs promotions
+  not exists(FunctionCall fc, int vararg |
+    fc.getArgument(vararg) = me and
+    vararg >= fc.getTarget().getNumberOfParameters()
+  ) and
+  // exclude cases where the type was made bigger by a literal
+  // (compared to other cases such as assignment, this is more
+  // likely to be a trivial accident rather than suggesting a
+  // larger type is needed for the result).
+  not exists(Expr other, Expr e |
+    other = me.getParent().(BinaryOperation).getAnOperand() and
+    not other = me and
+    (
+      e = other or
+      e = other.(BinaryOperation).getAnOperand*()
+    ) and
+    e.(Literal).getType().getSize() = t2.getSize()
+  )
+select me,
+  "Multiplication result may overflow '" + me.getType().toString() + "' before it is converted to '"
+    + me.getFullyConverted().getType().toString() + "'."

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

@@ -11,6 +11,7 @@
  * @tags maintainability
  *       readability
  */
+
 import cpp
 private import semmle.code.cpp.commons.Exclusions
 private import semmle.code.cpp.rangeanalysis.PointlessComparison
@@ -25,38 +26,40 @@ import UnsignedGEZero
 // So to reduce the number of false positives, we do not report a result if
 // the comparison is in a macro expansion. Similarly for template
 // instantiations.
-from
-  ComparisonOperation cmp, SmallSide ss,
-  float left, float right, boolean value,
-  string reason
+from ComparisonOperation cmp, SmallSide ss, float left, float right, boolean value, string reason
 where
   not cmp.isInMacroExpansion() and
   not cmp.isFromTemplateInstantiation(_) and
   not functionContainsDisabledCode(cmp.getEnclosingFunction()) and
   reachablePointlessComparison(cmp, left, right, value, ss) and
-
   // a comparison between an enum and zero is always valid because whether
   // the underlying type of an enum is signed is compiler-dependent
-  not exists (Expr e, ConstantZero z
-      | relOpWithSwap(cmp, e.getFullyConverted(), z, _, _) and
-        e.getUnderlyingType() instanceof Enum) and
-  
+  not exists(Expr e, ConstantZero z |
+    relOpWithSwap(cmp, e.getFullyConverted(), z, _, _) and
+    e.getUnderlyingType() instanceof Enum
+  ) and
   // Construct a reason for the message. Something like: x >= 5 and 3 >= y.
-  exists (string cmpOp, string leftReason, string rightReason
-  | ((ss = LeftIsSmaller()  and cmpOp = " <= ") or
-     (ss = RightIsSmaller() and cmpOp = " >= ")) and
+  exists(string cmpOp, string leftReason, string rightReason |
+    (
+      ss = LeftIsSmaller() and cmpOp = " <= "
+      or
+      ss = RightIsSmaller() and cmpOp = " >= "
+    ) and
     leftReason = cmp.getLeftOperand().toString() + cmpOp + left.toString() and
     rightReason = right.toString() + cmpOp + cmp.getRightOperand().toString() and
     // If either of the operands is constant, then don't include it.
-    (if cmp.getLeftOperand().isConstant()
-       then if cmp.getRightOperand().isConstant()
-              then none() // Both operands are constant so don't create a message.
-              else reason = rightReason
-       else if cmp.getRightOperand().isConstant()
-              then reason = leftReason
-              else reason = leftReason + " and " + rightReason)) and
-
+    (
+      if cmp.getLeftOperand().isConstant()
+      then
+        if cmp.getRightOperand().isConstant()
+        then none() // Both operands are constant so don't create a message.
+        else reason = rightReason
+      else
+        if cmp.getRightOperand().isConstant()
+        then reason = leftReason
+        else reason = leftReason + " and " + rightReason
+    )
+  ) and
   // Don't report results which have already been reported by UnsignedGEZero.
   not unsignedGEZero(cmp, _)
-select
-  cmp, "Comparison is always " + value.toString() + " because " + reason + "."
+select cmp, "Comparison is always " + value.toString() + " because " + reason + "."

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

@@ -1,8 +1,8 @@
 /**
  * @name Self comparison
  * @description Comparing a variable to itself always produces the
-                same result, unless the purpose is to check for
-                integer overflow or floating point NaN.
+ *              same result, unless the purpose is to check for
+ *              integer overflow or floating point NaN.
  * @kind problem
  * @problem.severity warning
  * @precision high
@@ -15,15 +15,15 @@ import cpp
 import PointlessSelfComparison
 
 from ComparisonOperation cmp
-where pointlessSelfComparison(cmp)
-  and not nanTest(cmp)
-  and not overflowTest(cmp)
-  and not exists(MacroInvocation mi | 
-    // cmp is in mi 
-    mi.getAnExpandedElement() = cmp and 
-  
-    // and cmp was apparently not passed in as a macro parameter 
-    cmp.getLocation().getStartLine() = mi.getLocation().getStartLine() and 
-    cmp.getLocation().getStartColumn() = mi.getLocation().getStartColumn() 
-  ) 
+where
+  pointlessSelfComparison(cmp) and
+  not nanTest(cmp) and
+  not overflowTest(cmp) and
+  not exists(MacroInvocation mi |
+    // cmp is in mi
+    mi.getAnExpandedElement() = cmp and
+    // and cmp was apparently not passed in as a macro parameter
+    cmp.getLocation().getStartLine() = mi.getLocation().getStartLine() and
+    cmp.getLocation().getStartColumn() = mi.getLocation().getStartColumn()
+  )
 select cmp, "Self comparison."

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

@@ -21,15 +21,16 @@ import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
  * `<=`, `>=`).
  */
 predicate pointlessSelfComparison(ComparisonOperation cmp) {
-  exists (Variable v, VariableAccess lhs, VariableAccess rhs
-  | lhs = cmp.getLeftOperand() and
+  exists(Variable v, VariableAccess lhs, VariableAccess rhs |
+    lhs = cmp.getLeftOperand() and
     rhs = cmp.getRightOperand() and
     lhs = v.getAnAccess() and
     rhs = v.getAnAccess() and
-    not exists (lhs.getQualifier()) and // Avoid structure fields
-    not exists (rhs.getQualifier()) and // Avoid structure fields
+    not exists(lhs.getQualifier()) and // Avoid structure fields
+    not exists(rhs.getQualifier()) and // Avoid structure fields
     not convertedExprMightOverflow(lhs) and
-    not convertedExprMightOverflow(rhs))
+    not convertedExprMightOverflow(rhs)
+  )
 }
 
 /**
@@ -44,10 +45,10 @@ predicate pointlessSelfComparison(ComparisonOperation cmp) {
  */
 predicate nanTest(EqualityOperation cmp) {
   pointlessSelfComparison(cmp) and
-  exists (Type t
-  | t = cmp.getLeftOperand().getUnspecifiedType()
-  | t instanceof FloatingPointType or
-    t instanceof TemplateParameter)
+  exists(Type t | t = cmp.getLeftOperand().getUnspecifiedType() |
+    t instanceof FloatingPointType or
+    t instanceof TemplateParameter
+  )
 }
 
 /**

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

@@ -10,6 +10,7 @@
  * @tags maintainability
  *       readability
  */
+
 import cpp
 import UnsignedGEZero
 

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

@@ -18,20 +18,16 @@ class ConstantZero extends Expr {
 class UnsignedGEZero extends GEExpr {
   UnsignedGEZero() {
     this.getRightOperand() instanceof ConstantZero and
-
     // left operand was a signed or unsigned IntegralType before conversions
     // (not a pointer, checking a pointer >= 0 is an entirely different mistake)
     // (not an enum, as the fully converted type of an enum is compiler dependent
     //  so checking an enum >= 0 is always reasonable)
     getLeftOperand().getUnderlyingType() instanceof IntegralType and
-
     exists(Expr ue |
       // ue is some conversion of the left operand
       ue = getLeftOperand().getConversion*() and
-
       // ue is unsigned
       ue.getUnderlyingType().(IntegralType).isUnsigned() and
-
       // ue may be converted to zero or more strictly larger possibly signed types
       // before it is fully converted
       forall(Expr following | following = ue.getConversion+() |
@@ -45,7 +41,6 @@ predicate unsignedGEZero(UnsignedGEZero ugez, string msg) {
   not exists(MacroInvocation mi |
     // ugez is in mi
     mi.getAnExpandedElement() = ugez and
-
     // and ugez was apparently not passed in as a macro parameter
     ugez.getLocation().getStartLine() = mi.getLocation().getStartLine() and
     ugez.getLocation().getStartColumn() = mi.getLocation().getStartColumn()

cpp/ql/src/Likely Bugs/ContinueInFalseLoop.qhelp

@@ -0,0 +1,24 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p>A <code>continue</code> statement only re-runs the loop if the loop condition is true. Therefore using <code>continue</code> in a loop with a constant false condition will never cause the loop body to be re-run, which is misleading.
+</p>
+
+</overview>
+<recommendation>
+
+<p>Replace the <code>continue</code> statement with a <code>break</code> statement if the intent is to break from the loop.
+</p>
+
+</recommendation>
+
+<references>
+<li>
+  Tutorialspoint - C Programming Language: <a href="https://www.tutorialspoint.com/cprogramming/c_continue_statement.htm">Continue Statement in C</a>.
+</li>
+</references>
+</qhelp>

cpp/ql/src/Likely Bugs/ContinueInFalseLoop.ql

@@ -6,28 +6,43 @@
  * @kind problem
  * @id cpp/continue-in-false-loop
  * @problem.severity warning
+ * @precision high
+ * @tags correctness
  */
 
 import cpp
 
-Loop getAFalseLoop() {
-  result.getControllingExpr().getValue() = "0"
-  and not result.getControllingExpr().isAffectedByMacro()
+/**
+ * Gets a `do` ... `while` loop with a constant false condition.
+ */
+DoStmt getAFalseLoop() {
+  result.getControllingExpr().getValue() = "0" and
+  not result.getControllingExpr().isAffectedByMacro()
 }
 
-Loop enclosingLoop(Stmt s) {
+/**
+ * Gets a `do` ... `while` loop surrounding a statement.  This is blocked by a
+ * `switch` statement, since a `continue` inside a `switch` inside a loop may be
+ * jusitifed (`continue` breaks out of the loop whereas `break` only escapes the
+ * `switch`).
+ */
+DoStmt enclosingLoop(Stmt s) {
   exists(Stmt parent |
     parent = s.getParent() and
-    if parent instanceof Loop then
+    (
+      parent instanceof Loop and
       result = parent
-    else
-      result = enclosingLoop(parent))
+      or
+      not parent instanceof Loop and
+      not parent instanceof SwitchStmt and
+      result = enclosingLoop(parent)
+    )
+  )
 }
 
-from Loop loop, ContinueStmt continue
-where loop = getAFalseLoop()
-  and loop = enclosingLoop(continue)
-select continue,
-  "This 'continue' never re-runs the loop - the $@ is always false.",
-  loop.getControllingExpr(),
-  "loop condition"
+from DoStmt loop, ContinueStmt continue
+where
+  loop = getAFalseLoop() and
+  loop = enclosingLoop(continue)
+select continue, "This 'continue' never re-runs the loop - the $@ is always false.",
+  loop.getControllingExpr(), "loop condition"

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

@@ -8,20 +8,21 @@
  * @precision high
  * @tags reliability
  */
+
 import cpp
 import semmle.code.cpp.commons.Buffer
 
 from Function f, FunctionCall c, int i, ArrayType argType, ArrayType paramType, int a, int b
-where f = c.getTarget() and
-        argType = c.getArgument(i).getType() and
-        paramType = f.getParameter(i).getType() and
-        a = argType.getArraySize() and
-        b = paramType.getArraySize() and
-        argType.getBaseType().getSize() = paramType.getBaseType().getSize() and
-        a < b and
-        not memberMayBeVarSize(_, c.getArgument(i).(VariableAccess).getTarget()) and
-        // filter out results for inconsistent declarations
-        strictcount(f.getParameter(i).getType().getSize()) = 1
-select c.getArgument(i), "Array of size " + a +
-        " passed to $@ which expects an array of size " + b + ".",
-        f, f.getName()
+where
+  f = c.getTarget() and
+  argType = c.getArgument(i).getType() and
+  paramType = f.getParameter(i).getType() and
+  a = argType.getArraySize() and
+  b = paramType.getArraySize() and
+  argType.getBaseType().getSize() = paramType.getBaseType().getSize() and
+  a < b and
+  not memberMayBeVarSize(_, c.getArgument(i).(VariableAccess).getTarget()) and
+  // filter out results for inconsistent declarations
+  strictcount(f.getParameter(i).getType().getSize()) = 1
+select c.getArgument(i),
+  "Array of size " + a + " passed to $@ which expects an array of size " + b + ".", f, f.getName()