C++: Autoformat everything

Some files that will change in #1736 have been spared.

    ./build -j4 target/jars/qlformat
    find ql/cpp/ql -name "*.ql"  -print0 | xargs -0 target/jars/qlformat --input
    find ql/cpp/ql -name "*.qll" -print0 | xargs -0 target/jars/qlformat --input
    (cd ql && git checkout 'cpp/ql/src/semmle/code/cpp/ir/implementation/**/*SSA*.qll')
    buildutils-internal/scripts/pr-checks/sync-identical-files.py --latest

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*\\[)", _, _)
+      annotation = text.regexpFind("(?i)(?<=^|;)\\s*lgtm(?!\\B|\\s*\\[)", _, _).trim()
-                       .trim()
     )
   }
 
-
   /** Gets the text in this comment, excluding the leading //. */
-  string getText() {
+  string getText() { result = text }
-    result = text
-  }
 
   /** Gets the suppression annotation in this comment. */
-  string getAnnotation() {
+  string getAnnotation() { result = annotation }
-    result = annotation
-  }
 
   /**
-  * Holds if this comment applies to the range from column `startcolumn` of line `startline`
+   * Holds if this comment applies to the range from column `startcolumn` of line `startline`
-  * to column `endcolumn` of line `endline` in file `filepath`.
+   * 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() {
+  SuppressionScope getScope() { result = this }
-    result = this
-  }
 }
 
 /**
  * The scope of an alert suppression comment.
  */
 class SuppressionScope extends ElementBase {
-  SuppressionScope() {
+  SuppressionScope() { this instanceof SuppressionComment }
-    this instanceof SuppressionComment
-  }
 
   /**
-  * Holds if this element is at the specified location.
+   * Holds if this element is at the specified location.
-  * The location spans column `startcolumn` of line `startline` to
+   * The location spans column `startcolumn` of line `startline` to
-  * column `endcolumn` of line `endline` in file `filepath`.
+   * column `endcolumn` of line `endline` in file `filepath`.
-  * For more information, see
+   * For more information, see
-  * [Locations](https://help.semmle.com/QL/learn-ql/ql/locations.html).
+   * [Locations](https://help.semmle.com/QL/learn-ql/ql/locations.html).
-  */
+   */
-  predicate hasLocationInfo(string filepath, int startline, int startcolumn, int endline, int endcolumn) {
+  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
+select c, // suppression comment
-       c.getText(),       // text of suppression comment (excluding delimiters)
+  c.getText(), // text of suppression comment (excluding delimiters)
-       c.getAnnotation(), // text of suppression annotation
+  c.getAnnotation(), // text of suppression annotation
-       c.getScope()       // scope of suppression
+  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/Magic Constants/JapaneseEraDate.ql

@@ -42,7 +42,6 @@ predicate eraDate(int year, int month, int day) {
   year = 2019 and month = 5 and day = 1
 }
 
-
 predicate badStructInitialization(Element target, string message) {
   exists(
     StructLikeClass s, YearFieldAccess year, MonthFieldAccess month, DayFieldAccess day,

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() |
+  exists(Class c | c = v.getUnspecifiedType() | c.hasConstructor() or c.hasDestructor())
-    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().
+  call.(VariableCall).getVariable().getAnAssignedValue().getAChild*().(FunctionAccess).getTarget() =
-    getAChild*().(FunctionAccess).getTarget() = f
+    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,
+    ControlFlowNode source, ControlFlowNode node, ControlFlowNode next, LocalScopeVariable v
-    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
+select def, "The file opened here may not be closed at $@.", ret, "this exit point"
-  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().
+  call.(VariableCall).getVariable().getAnAssignedValue().getAChild*().(FunctionAccess).getTarget() =
-    getAChild*().(FunctionAccess).getTarget() = f
+    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,
+    ControlFlowNode source, ControlFlowNode node, ControlFlowNode next, LocalScopeVariable v
-    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
+select def, "The memory allocated here may not be released at $@.", ret, "this exit point"
-  def, "The memory allocated here may not be released at $@.",
-  ret, "this exit point"

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 |

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
+class Options extends string {
-{
+  Options() { this = "Options" }
-  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.getTarget().hasGlobalName("select") and
-     fc.getArgument(0).getValue() = "0")
+    fc.getArgument(0).getValue() = "0"
     or
     CustomOptions::okToIgnoreReturnValue(fc) // old Options.qll
   }
 }
 
-Options getOptions()
+Options getOptions() { any() }
-{
-  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
+  (caption = "TODO" or caption = "FIXME") and
-    exists (string commentContents, string commentBody, int offset, string interestingSuffix, int endOfLine, string dontCare, string captionedLine, string followingLine
+  exists(
-           | commentContents = c.getContents()
+    string commentContents, string commentBody, int offset, string interestingSuffix, int endOfLine,
-         and commentContents.matches("%" + caption + "%")
+    string dontCare, string captionedLine, string followingLine
-         and // Add some '\n's so that any interesting line, and its
+  |
-             // following line, will definitely begin and end with '\n'.
+    commentContents = c.getContents() and
-             commentBody = commentContents.regexpReplaceAll("(?s)^/\\*(.*)\\*/$|^//(.*)$", "\n$1$2\n\n")
+    commentContents.matches("%" + caption + "%") and
-         and dontCare = commentBody.regexpFind("\\n[/* \\t\\x0B\\f\\r]*" + caption, _, offset)
+    // Add some '\n's so that any interesting line, and its
-         and interestingSuffix = commentBody.suffix(offset)
+    // following line, will definitely begin and end with '\n'.
-         and endOfLine = interestingSuffix.indexOf("\n", 1, 0)
+    commentBody = commentContents.regexpReplaceAll("(?s)^/\\*(.*)\\*/$|^//(.*)$", "\n$1$2\n\n") and
-         and captionedLine = interestingSuffix.prefix(endOfLine).regexpReplaceAll("^[/*\\s]*" + caption + "\\s*:?", "").trim()
+    dontCare = commentBody.regexpFind("\\n[/* \\t\\x0B\\f\\r]*" + caption, _, offset) and
-         and followingLine = interestingSuffix.prefix(interestingSuffix.indexOf("\n", 2, 0)).suffix(endOfLine).trim()
+    interestingSuffix = commentBody.suffix(offset) and
-         and if captionedLine = ""
+    endOfLine = interestingSuffix.indexOf("\n", 1, 0) and
-             then result = caption + " comment"
+    captionedLine = interestingSuffix
-             else if followingLine = ""
+          .prefix(endOfLine)
-             then result = caption + " comment: " + captionedLine
+          .regexpReplaceAll("^[/*\\s]*" + caption + "\\s*:?", "")
-             else result = caption + " comment: " + captionedLine + " [...]"
+          .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 ';'
+        // If this line looks like code because it ends with a closing
-        trimmed.regexpMatch(".*[{};]") and
+        // brace that's preceded by something other than whitespace ...
-        (
+        trimmed.regexpMatch(".*.\\}")
-          // If this line looks like code because it ends with a closing
+        implies
-          // brace that's preceded by something other than whitespace ...
+        // ... then there has to be ") {" (or some variation)
-          trimmed.regexpMatch(".*.\\}")
+        // on the line, suggesting it's a statement like `if`
-          implies
+        // or a function definition. Otherwise it's likely to be a
-          // ... then there has to be ") {" (or some variation)
+        // benign use of braces such as a JSON example or explanatory
-          // on the line, suggesting it's a statement like `if`
+        // pseudocode.
-          // or a function definition. Otherwise it's likely to be a
+        trimmed.regexpMatch(".*(\\)|const|volatile|override|final|noexcept|&)\\s*\\{.*")
-          // 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.*")
       )
-    ) and (
+      or
-      // Exclude lines that start with '>' or contain '@{' or '@}'.
+      // Match comment lines that look like preprocessor code
-      // To account for the code generated by protobuf, we also insist that the comment
+      trimmed
-      // does not begin with `optional` or `repeated` and end with a `;`, which would
+          .regexpMatch("#\\s*(include|define|undef|if|ifdef|ifndef|elif|else|endif|error|pragma)\\b.*")
-      // normally be a quoted bit of literal `.proto` specification above the associated
+    ) and
-      // declaration.
+    // Exclude lines that start with '>' or contain '@{' or '@}'.
-      // To account for emacs folding markers, we ignore any line containing
+    // To account for the code generated by protobuf, we also insist that the comment
-      // `{{{` or `}}}`.
+    // does not begin with `optional` or `repeated` and end with a `;`, which would
-      // Finally, some code tends to embed GUIDs in comments, so we also exclude those.
+    // normally be a quoted bit of literal `.proto` specification above the associated
-      not trimmed
+    // 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 (
+    ) and
-      // Ignore comments on the same line as a preprocessor directive.
+    // Ignore comments on the same line as a preprocessor directive.
-      not exists(Location l |
+    not exists(Location l |
-        l = this.getLocation() and
+      l = this.getLocation() and
-        preprocLine(l.getFile(), l.getStartLine())
+      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) {
+Call uses(File other, Function f) { result.getTarget() = f and result.getFile() = other }
-  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())
+where
-  and not isCommented(f.getADeclarationEntry())
+  numCalls = strictcount(File other | exists(uses(other, f)) and other != f.getFile()) and
-  and not f instanceof Constructor
+  not isCommented(f.getADeclarationEntry()) and
-  and not f instanceof Destructor
+  not f instanceof Constructor and
-  and not f.hasName("operator=")
+  not f instanceof Destructor and
-  and f.getMetrics().getNumberOfLinesOfCode() >= 5
+  not f.hasName("operator=") and
-  and numCalls > 1
+  f.getMetrics().getNumberOfLinesOfCode() >= 5 and
-  and use = uses(callerFile, f)
+  numCalls > 1 and
-  and callerFile != f.getFile()
+  use = uses(callerFile, f) and
-select f, "Functions called from other files should be documented (called from $@).", use, use.getFile().getRelativePath()
+  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
+where
-      f.getCommentRatio() <= 0.02 and
+  n = f.getNumberOfLines() and
-      not f.isMultiplyDefined()
+  n > 100 and
-select f, "Poorly documented function: fewer than 2% comments for a function of " + n.toString() + " lines."
+  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@%") |
+  exists(Comment c | c.getContents().matches("%@non-terminating@%") | c.getCommentedElement() = l)
-    c.getCommentedElement() = l
-  )
 }
 
 Stmt exitFrom(Loop l) {
   l.getAChild+() = result and
-  (result instanceof ReturnStmt or
+  (
-    exists(BreakStmt break | break = result |
+    result instanceof ReturnStmt
-      not l.getAChild*() = break.getTarget())
+    or
+    exists(BreakStmt break | break = result | not l.getAChild*() = break.getTarget())
   )
 }
 
 from Loop l, Stmt exit
-where markedAsNonterminating(l) and
+where
-      exit = exitFrom(l)
+  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

@@ -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() =
+      a.getRValue() = any(AddExpr ae |
-        any(AddExpr ae | ae.getAnOperand() = var.getAnAccess() and
+          ae.getAnOperand() = var.getAnAccess() and
-            ae.getAnOperand().getValue().toInt() > 0))
+          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() =
+      a.getRValue() = any(SubExpr ae |
-        any(SubExpr ae | ae.getLeftOperand() = var.getAnAccess() and
+          ae.getLeftOperand() = var.getAnAccess() and
-            ae.getRightOperand().getValue().toInt() > 0))
+          ae.getRightOperand().getValue().toInt() > 0
+        )
+    )
   )
 }
 
-predicate inScope(Loop l, Stmt s) {
+predicate inScope(Loop l, Stmt s) { l.getAChild*() = s }
-  l.getAChild*() = s
-}
 
 predicate reachesNoInc(VariableAccess source, ControlFlowNode target) {
-  (upperBoundCheck(_, source) and source.getASuccessor() = target) or
+  upperBoundCheck(_, source) and source.getASuccessor() = target
-  exists(ControlFlowNode mid | reachesNoInc(source, mid) and not mid = getAnIncrement(source.getTarget()) |
+  or
-                          target = mid.getASuccessor() and
+  exists(ControlFlowNode mid |
-                          inScope(source.getEnclosingStmt(), target.getEnclosingStmt()))
+    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
+  lowerBoundCheck(_, source) and source.getASuccessor() = target
-  exists(ControlFlowNode mid | reachesNoDec(source, mid) and not mid = getADecrement(source.getTarget()) |
+  or
-                          target = mid.getASuccessor() and
+  exists(ControlFlowNode mid |
-                          inScope(source.getEnclosingStmt(), target.getEnclosingStmt()))
+    reachesNoDec(source, mid) and not mid = getADecrement(source.getTarget())
-}
+  |
-
+    target = mid.getASuccessor() and
-predicate hasSafeBound(Loop l) {
+    inScope(source.getEnclosingStmt(), target.getEnclosingStmt())
-  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
   )
 }
 
+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
+where
-    not markedAsNonterminating(loop) and
+  not hasSafeBound(loop) and
-    (
+  not markedAsNonterminating(loop) and
-      (
+  (
-        not upperBoundCheck(loop, _) and
+    not upperBoundCheck(loop, _) and
-        not lowerBoundCheck(loop, _) and
+    not lowerBoundCheck(loop, _) and
-        msg = "This loop does not have a fixed bound."
+    msg = "This loop does not have a fixed bound."
-      ) or exists(VariableAccess bound | upperBoundCheck(loop, bound) and
+    or
-                    reachesNoInc(bound, bound) and
+    exists(VariableAccess bound |
-                    msg = "The loop counter " + bound.getTarget().getName() + " is not always incremented in the loop body."
+      upperBoundCheck(loop, bound) and
-      ) or exists(VariableAccess bound | lowerBoundCheck(loop, bound) and
+      reachesNoInc(bound, bound) and
-                    reachesNoDec(bound, bound) and
+      msg = "The loop counter " + bound.getTarget().getName() +
-                    msg = "The loop counter " + bound.getTarget().getName() + " is not always decremented in the loop body."
+          " 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() {
+  RecursiveCall() { this.getTarget().calls*(this.getEnclosingFunction()) }
-    this.getTarget().calls*(this.getEnclosingFunction())
-  }
 }
 
 from RecursiveCall call, string msg
-where if (call.getTarget() = call.getEnclosingFunction()) then
+where
-         msg = "This call directly invokes its containing function $@."
+  if call.getTarget() = call.getEnclosingFunction()
-      else
+  then msg = "This call directly invokes its containing function $@."
-         msg = "The function " + call.getEnclosingFunction() + " is indirectly recursive via this call to $@."
+  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 = "malloc" or
-      name = "sbrk" or name = "valloc")
+      name = "calloc" or
+      name = "alloca" or
+      name = "sbrk" or
+      name = "valloc"
+    )
   }
 }
 
 from Function f, Allocation a
-where not f instanceof Initialization and
+where
-      a.getEnclosingFunction() = f
+  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 = "task_delay" or
-      name = "sleep" or name = "nanosleep" 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
+  result.getEnclosingFunction() = f
-  exists(Function g | f.calls(g) |
+  or
-    result = maybeLocked(g)
+  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
+where
-      (call instanceof LockingPrimitive  and kind = "locking primitives")
+  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
+      outer.getLocked() = outerLock and
-    lockOrder(outer, inner))
+      inner.getLocked() = innerLock and
+      lockOrder(outer, inner)
+    )
 }
 
 from LockOperation outer, LockOperation inner
-where lockOrder(outer, inner)
+where
-  and orderCount(outer.getLocked(), inner.getLocked()) <= orderCount(inner.getLocked(), outer.getLocked())
+  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() {
+  Variable getSemaphore() { result.getAnAccess() = this.getParent().(Assignment).getLValue() }
-    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() {
+  override Variable getLocked() { result.getAnAccess() = this.getArgument(0) }
-    result.getAnAccess() = this.getArgument(0)
-  }
 
   override UnlockOperation getMatchingUnlock() {
     result.(SemaphoreGive).getLocked() = this.getLocked()
   }
 
-  override string say() {
+  override string say() { result = "semaphore take of " + getLocked().getName() }
-    result = "semaphore take of " + getLocked().getName()
-  }
 }
 
 class SemaphoreGive extends UnlockOperation {
@@ -67,14 +66,9 @@ class SemaphoreGive extends UnlockOperation {
     )
   }
 
-  Variable getLocked() {
+  Variable getLocked() { result.getAnAccess() = this.getArgument(0) }
-    result.getAnAccess() = this.getArgument(0)
-  }
-
-  override LockOperation getMatchingLock() {
-    this = result.getMatchingUnlock()
-  }
 
+  override LockOperation getMatchingLock() { this = result.getMatchingUnlock() }
 }
 
 class LockingPrimitive extends FunctionCall, LockOperation {
@@ -84,18 +78,16 @@ class LockingPrimitive extends FunctionCall, LockOperation {
     )
   }
 
-  override Function getLocked() {
+  override Function getLocked() { result = this.getTarget() }
-    result = this.getTarget()
-  }
 
   override UnlockOperation getMatchingUnlock() {
-    result.(UnlockingPrimitive).getTarget().getName() =
+    result.(UnlockingPrimitive).getTarget().getName() = this
-      this.getTarget().getName().replaceAll("Lock", "Unlock")
+          .getTarget()
+          .getName()
+          .replaceAll("Lock", "Unlock")
   }
 
-  override string say() {
+  override string say() { result = "call to " + getLocked().getName() }
-    result = "call to " + getLocked().getName()
-  }
 }
 
 class UnlockingPrimitive extends FunctionCall, UnlockOperation {
@@ -105,11 +97,7 @@ class UnlockingPrimitive extends FunctionCall, UnlockOperation {
     )
   }
 
-  Function getLocked() {
+  Function getLocked() { result = getMatchingLock().getLocked() }
-    result = getMatchingLock().getLocked()
-  }
 
-  override LockOperation getMatchingLock() {
+  override LockOperation getMatchingLock() { this = result.getMatchingUnlock() }
-    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 = "setjmp" or
-        name = "sigsetjmp" or name = "siglongjmp")
+      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) {
+predicate hasInitializer(EnumConstant c) { c.getInitializer().fromSource() }
-  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
+where
-      // not just a reference to another enum constant
+  // e is at position i, and has an explicit value in the source - but
-      hasNonReferenceInitializer(e.getEnumConstant(i)) and
+  // not just a reference to another enum constant
-      // but e is not the first or the last constant of the enum
+  hasNonReferenceInitializer(e.getEnumConstant(i)) and
-      i != 0 and
+  // but e is not the first or the last constant of the enum
-      exists(e.getEnumConstant(i+1)) and
+  i != 0 and
-      // and there exists another constant whose value is implicit, but it's
+  exists(e.getEnumConstant(i + 1)) and
-      // not the last one: the last value is okay to use to get the highest
+  // and there exists another constant whose value is implicit, but it's
-      // enum value automatically. It can be followed by aliases though.
+  // not the last one: the last value is okay to use to get the highest
-      enumThatHasConstantWithImplicitValue(e)
+  // 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."
+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())
+where
-      and not v.hasSpecifier("static")
+  forex(VariableAccess va | va.getTarget() = v | va.getFile() = v.getDefinitionLocation().getFile()) and
-      and strictcount(v.getAnAccess().getEnclosingFunction()) > 1 // If = 1, variable should be function-scope.
+  not v.hasSpecifier("static") and
-      and not v.getADeclarationEntry().getFile() instanceof HeaderFile // intended to be accessed elsewhere
+  strictcount(v.getAnAccess().getEnclosingFunction()) > 1 and // If = 1, variable should be function-scope.
-select v, "The global variable " + v.getName() + " is not accessed outside of " + v.getFile().getBaseName()
+  not v.getADeclarationEntry().getFile() instanceof HeaderFile // intended to be accessed elsewhere
-      + " and could be made static."
+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
+where
-      strictcount(v.getAnAccess().getEnclosingFunction()) = 1 and
+  v.getAnAccess().getEnclosingFunction() = f and
-      forall(VariableAccess a | a = v.getAnAccess() | exists(a.getEnclosingFunction())) and
+  strictcount(v.getAnAccess().getEnclosingFunction()) = 1 and
-      not v.getADeclarationEntry().getFile() instanceof HeaderFile // intended to be accessed elsewhere
+  forall(VariableAccess a | a = v.getAnAccess() | exists(a.getEnclosingFunction())) and
-select v, "The variable " + v.getName() + " is only accessed in $@ and should be scoped accordingly.", f, f.getName()
+  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
+      p.getFunction().getDefinitionLocation().getFile() = this.getFile() and
-           exists(p.getFunction().getBlock()))
+      exists(p.getFunction().getBlock())
+    )
   }
 
   string type() {
-    if this instanceof Parameter
+    if this instanceof Parameter then result = "Parameter " else result = "Local variable "
-    then result = "Parameter "
-    else result = "Local variable "
   }
 }
 
 from LocalVariableOrParameter lv, GlobalVariable gv
-where lv.getName() = gv.getName() and
+where
-      lv.getFile() = gv.getFile()
+  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
+where
-      and not whitelist(c.getTarget())
+  not c.getTarget().getType() instanceof VoidType and
-      and
+  not whitelist(c.getTarget()) and
-      (
+  (
-        (c instanceof ExprInVoidContext and msg = "The return value of non-void function $@ is not checked.")
+    c instanceof ExprInVoidContext and
-        or
+    msg = "The return value of non-void function $@ is not checked."
-        (definition(_, c.getParent()) and not definitionUsePair(_, c.getParent(), _) and
+    or
-            msg = "$@'s return value is stored but not checked.")
+    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(p.getAnAccess()) and n = p.getFunction().getBlock()
-  exists(ControlFlowNode mid | flow(p, mid) and not mid = p.getAnAccess() and n = mid.getASuccessor())
+  or
+  exists(ControlFlowNode mid |
+    flow(p, mid) and not mid = p.getAnAccess() and n = mid.getASuccessor()
+  )
 }
 
-VariableAccess firstAccess(Parameter p) {
+VariableAccess firstAccess(Parameter p) { flow(p, result) and result = p.getAnAccess() }
-  flow(p, result) and result = p.getAnAccess()
-}
 
 from Parameter p, VariableAccess va
-where va = firstAccess(p) and p.getFunction() instanceof PublicFunction and
+where
-    not exists(Expr e | e.isCondition() | e.getAChild*() = va)
+  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
+where
-    if value.toInt() = 0 then
+  value = a.getAsserted().getValue() and
-        msg = "This assertion is always false."
+  if value.toInt() = 0
-    else
+  then msg = "This assertion is always false."
-        msg = "This assertion is always true."
+  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 = "I64" or
-    name = "I32" or name = "U32" or
+    name = "U64" or
-    name = "I16" or name = "U16" or
+    name = "I32" or
-    name = "I8" or name = "U8" or
+    name = "U32" or
-    name = "F64" or name = "F32"
+    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)
+  not d.getLocation() instanceof UnknownLocation
-select d, d.getName() + " uses the basic integral type " + usedType.getName() + " rather than a typedef with size and signedness."
+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 Assignment
-  e instanceof CrementOperation or
+  or
-  e instanceof ExprCall 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
+where
-      not p instanceof Macro and
+  not p instanceof Include and
-      not p instanceof PreprocessorIf and
+  not p instanceof Macro and
-      not p instanceof PreprocessorElif and
+  not p instanceof PreprocessorIf and
-      not p instanceof PreprocessorElse and
+  not p instanceof PreprocessorElif and
-      not p instanceof PreprocessorIfdef and
+  not p instanceof PreprocessorElse and
-      not p instanceof PreprocessorIfndef and
+  not p instanceof PreprocessorIfdef and
-      not p instanceof PreprocessorEndif
+  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)
+where
-      and not i.getFile() instanceof HeaderFile
+  (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
+where
-      (m.getBody().matches("%##%") and msg = "The macro " + m.getHead() + " uses token pasting and is not allowed.")
+  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()|
+  exists(Block block, Location blockLocation |
-    result in [blockLocation.getStartLine()..blockLocation.getEndLine()]
+    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() {
+  FileWithDirectives() { exists(Directive d | d.getFile() = this) }
-    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) |
+    exists(int line | line = getDirectiveLine(d) | line = rank[result](getDirectiveLine(_)))
-      line = rank[result](getDirectiveLine(_))
-    )
   }
 
   int depth(Directive d) {
     exists(int index | index = getDirectiveIndex(d) |
-      (index = 1 and result = d.depthChange()) or
+      index = 1 and result = d.depthChange()
-      exists(Directive prev | getDirectiveIndex(prev) = index-1 |
+      or
+      exists(Directive prev | getDirectiveIndex(prev) = index - 1 |
         result = d.depthChange() + depth(prev)
       )
     )
   }
 
-  Directive lastDirective() {
+  Directive lastDirective() { getDirectiveIndex(result) = max(getDirectiveIndex(_)) }
-    getDirectiveIndex(result) = max(getDirectiveIndex(_))
-  }
 }
 
 abstract class Directive extends PreprocessorDirective {
   abstract int depthChange();
+
   abstract predicate mismatched();
 
-  int depth() {
+  int depth() { exists(FileWithDirectives f | f.depth(this) = result) }
-    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() {
+  EndifDirective() { this instanceof PreprocessorEndif }
-    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
+where
-  if d.mismatched() then (
+  d.getFile() = f and
-    msg = "'" + d + "' has no matching #if in file " + f.getBaseName() + "."
+  if d.mismatched()
-  ) else (
+  then msg = "'" + d + "' has no matching #if in file " + f.getBaseName() + "."
-    d = f.lastDirective() and d.depth() > 0 and msg = "File " + f.getBaseName() +
+  else (
-        " ends with " + d.depth() + " unterminated #if directives."
+    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) {
+int numStmt(File f, int line) { result = strictcount(OneLineStmt o | o.onLine(f, line)) }
-  result = strictcount(OneLineStmt o | o.onLine(f, line))
-}
 
 from File f, int line, OneLineStmt o, int cnt
-where numStmt(f, line) = cnt
+where
-  and cnt > 1
+  numStmt(f, line) = cnt and
-  and o.onLine(f, line)
+  cnt > 1 and
-  and o.getLocation().getStartColumn() =
+  o.onLine(f, line) and
-        min(OneLineStmt other, int toMin
+  o.getLocation().getStartColumn() = min(OneLineStmt other, int toMin |
-          | other.onLine(f, line) and toMin = other.getLocation().getStartColumn()
+      other.onLine(f, line) and toMin = other.getLocation().getStartColumn()
-          | toMin)
+    |
+      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
+where
-      msg = paramWarning(f)
+  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
+where
-  and n = strictcount(PointerDereferenceExpr child | child.getParent+() = e)
+  not e.getParent+() instanceof PointerDereferenceExpr and
-  and n > 1
+  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
+  result = min(Declaration d, Location l, int toMin |
-    l.getFile() = f and not d.isInMacroExpansion()) and (toMin = l.getStartLine()) | 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 " +
+select i,
-  firstCodeLine(f) + " in " + f.getBaseName() + "."
+  "'" + 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,21 @@
  *       language-features
  *       external/cwe/cwe-190
  */
+
 import cpp
 
 from BitField bf
-where not bf.getUnspecifiedType().(IntegralType).isExplicitlySigned()
+where
-  and not bf.getUnspecifiedType().(IntegralType).isExplicitlyUnsigned()
+  not bf.getUnspecifiedType().(IntegralType).isExplicitlySigned() and
-  and not bf.getUnspecifiedType() instanceof Enum
+  not bf.getUnspecifiedType().(IntegralType).isExplicitlyUnsigned() and
-  and not bf.getUnspecifiedType() instanceof BoolType
+  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
+  not bf.getDeclaredNumBits() = bf.getType().getSize() * 8 and
-  and not bf.isAnonymous()
+  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."
+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.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
+  a = plus.getLeftOperand() and b = plus.getRightOperand()
-  (b = plus.getLeftOperand() and a = 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
+  exists(Variable v, VariableAccess a1, VariableAccess a2, Expr b |
-  | addExpr(plus, a1, b) and
+    addExpr(plus, a1, b) and
     a1 = v.getAnAccess() and
     a2 = v.getAnAccess() and
-    not exists (a1.getQualifier()) and // Avoid structure fields
+    not exists(a1.getQualifier()) and // Avoid structure fields
-    not exists (a2.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/BadCheckOdd.ql

@@ -10,12 +10,14 @@
  *       correctness
  *       types
  */
+
 import cpp
 
 from EqualityOperation t, RemExpr lhs, Literal rhs
-where t.getLeftOperand() = lhs and
+where
-      t.getRightOperand() = rhs and
+  t.getLeftOperand() = lhs and
-      lhs.getLeftOperand().getUnspecifiedType().(IntegralType).isSigned() and
+  t.getRightOperand() = rhs and
-      lhs.getRightOperand().getValue() = "2" and
+  lhs.getLeftOperand().getUnspecifiedType().(IntegralType).isSigned() and
-      rhs.getValue() = "1"
+  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,14 @@
  * @tags reliability
  *       correctness
  */
+
 import cpp
 
 from RelationalOperation e, BinaryBitwiseOperation lhs
-where lhs = e.getGreaterOperand() and
+where
-      lhs.getActualType().(IntegralType).isSigned() and
+  lhs = e.getGreaterOperand() and
-      forall(int op | op = lhs.(BitwiseAndExpr).getAnOperand().getValue().toInt() | op < 0) and
+  lhs.getActualType().(IntegralType).isSigned() and
-      e.getLesserOperand().getValue() = "0" and
+  forall(int op | op = lhs.(BitwiseAndExpr).getAnOperand().getValue().toInt() | op < 0) and
-      not e.isAffectedByMacro()
+  e.getLesserOperand().getValue() = "0" and
+  not e.isAffectedByMacro()
 select e, "Potential unsafe sign check of a bitwise operation."

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

@@ -10,8 +10,8 @@
  * @tags maintainability
  *       readability
  */
-import cpp
 
+import cpp
 
 from ComparisonOperation co, ComparisonOperation chco
 where co.getAChild() = chco and not chco.isParenthesised()

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.(AddExpr).getAChild() and multiplier = 1.0
-  (child = parent.(SubExpr).getLeftOperand() and multiplier = 1.0) or
+  or
-  (child = parent.(SubExpr).getRightOperand() and multiplier = -1.0) or
+  child = parent.(SubExpr).getLeftOperand() and multiplier = 1.0
-  (child = parent.(UnaryPlusExpr).getOperand() and multiplier = 1.0) or
+  or
-  (child = parent.(UnaryMinusExpr).getOperand() and multiplier = -1.0)
+  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(
+private predicate cmpLinearSubExpr(ComparisonOperation cmp, Expr child, float multiplier) {
-  ComparisonOperation cmp, Expr child, float multiplier) {
+  not convertedExprMightOverflow(child) and
-  not convertedExprMightOverflow(child)
+  (
-  and
+    child = cmp.getLeftOperand() and multiplier = 1.0
-  ((child = cmp.getLeftOperand() and multiplier = 1.0)
+    or
-   or
+    child = cmp.getRightOperand() and multiplier = -1.0
-   (child = cmp.getRightOperand() and multiplier = -1.0)
+    or
-   or
+    exists(Expr parent, float m1, float m2 |
-   exists (Expr parent, float m1, float m2
+      cmpLinearSubExpr(cmp, parent, m1) and
-   | cmpLinearSubExpr(cmp, parent, m1) and
+      linearChild(parent, child, m2) and
-     linearChild(parent, child, m2) and
+      multiplier = m1 * m2
-     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(
+private predicate cancelingSubExprs(ComparisonOperation cmp, VariableAccess a1, VariableAccess a2) {
-  ComparisonOperation cmp, VariableAccess a1, VariableAccess a2) {
+  exists(Variable v |
-  exists (Variable v
+    exists(float m | m < 0 and cmpLinearSubVariable(cmp, v, a1, m)) and
-  | exists (float m | m < 0 and cmpLinearSubVariable(cmp, v, a1, m)) and
+    exists(float m | m > 0 and cmpLinearSubVariable(cmp, v, a2, m))
-    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, _)
+  not badAdditionOverflowCheck(cmp, _) and
-  and not pointlessSelfComparison(cmp)
+  not pointlessSelfComparison(cmp)
-select
+select cmp, "Comparison can be simplified by canceling $@ with $@.", a1, a1.getTarget().getName(),
-  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
+where
-      ro.getAnOperand().getExplicitlyConverted().getType().getUnderlyingType() instanceof FloatingPointType and
+  left = ro.getLeftOperand() and
-      not ro.getAnOperand().isConstant() and // comparisons to constants generate too many false positives
+  right = ro.getRightOperand() and
-      not left.(VariableAccess).getTarget() = right.(VariableAccess).getTarget() // skip self comparison
+  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.isConstant()
-  e.getType().getSize() <= 1 or
+  or
-  e.(ArrayExpr).getArrayBase().getType().(ArrayType).getBaseType().isConst() 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) {
+Expr getMulOperand(MulExpr me) { result = me.getAnOperand() }
-  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
+      op = getMulOperand*(me) and
-    not op instanceof MulExpr and
+      not op instanceof MulExpr and
-    not likelySmall(op)
+      not likelySmall(op)
-  )
+    )
 }
 
 from MulExpr me, Type t1, Type t2
-where t1 = me.getType().getUnderlyingType() and
+where
-      t2 = me.getConversion().getType().getUnderlyingType() and
+  t1 = me.getType().getUnderlyingType() and
-      t1.getSize() < t2.getSize() and
+  t2 = me.getConversion().getType().getUnderlyingType() and
-      (
+  t1.getSize() < t2.getSize() and
-        (
+  (
-          t1.getUnspecifiedType() instanceof IntegralType and
+    t1.getUnspecifiedType() instanceof IntegralType and
-          t2.getUnspecifiedType() instanceof IntegralType
+    t2.getUnspecifiedType() instanceof IntegralType
-        ) or (
+    or
-          t1.getUnspecifiedType() instanceof FloatingPointType and
+    t1.getUnspecifiedType() instanceof FloatingPointType and
-          t2.getUnspecifiedType() instanceof FloatingPointType
+    t2.getUnspecifiedType() instanceof FloatingPointType
-        )
+  ) and
-      ) and
+  // exclude explicit conversions
-
+  me.getConversion().isCompilerGenerated() and
-      // exclude explicit conversions
+  // require the multiply to have two non-constant operands
-      me.getConversion().isCompilerGenerated() and
+  // (the intuition here is that multiplying two unknowns is
-
+  // much more likely to produce a result that needs significantly
-      // require the multiply to have two non-constant operands
+  // more bits than the operands did, and thus requires a larger
-      // (the intuition here is that multiplying two unknowns is
+  // type).
-      // much more likely to produce a result that needs significantly
+  getEffectiveMulOperands(me) >= 2 and
-      // more bits than the operands did, and thus requires a larger
+  // exclude varargs promotions
-      // type).
+  not exists(FunctionCall fc, int vararg |
-      getEffectiveMulOperands(me) >= 2 and
+    fc.getArgument(vararg) = me and
-
+    vararg >= fc.getTarget().getNumberOfParameters()
-      // exclude varargs promotions
+  ) and
-      not exists(FunctionCall fc, int vararg |
+  // exclude cases where the type was made bigger by a literal
-        fc.getArgument(vararg) = me and
+  // (compared to other cases such as assignment, this is more
-        vararg >= fc.getTarget().getNumberOfParameters()
+  // likely to be a trivial accident rather than suggesting a
-      ) and
+  // larger type is needed for the result).
-
+  not exists(Expr other, Expr e |
-      // exclude cases where the type was made bigger by a literal
+    other = me.getParent().(BinaryOperation).getAnOperand() and
-      // (compared to other cases such as assignment, this is more
+    not other = me and
-      // likely to be a trivial accident rather than suggesting a
+    (
-      // larger type is needed for the result).
+      e = other or
-      not exists(Expr other, Expr e |
+      e = other.(BinaryOperation).getAnOperand*()
-        other = me.getParent().(BinaryOperation).getAnOperand() and
+    ) and
-        not other = me and
+    e.(Literal).getType().getSize() = t2.getSize()
-        (
+  )
-          e = other or
+select me,
-          e = other.(BinaryOperation).getAnOperand*()
+  "Multiplication result may overflow '" + me.getType().toString() + "' before it is converted to '"
-        ) and
+    + me.getFullyConverted().getType().toString() + "'."
-        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
+from ComparisonOperation cmp, SmallSide ss, float left, float right, boolean value, string reason
-  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
+  not exists(Expr e, ConstantZero z |
-      | relOpWithSwap(cmp, e.getFullyConverted(), z, _, _) and
+    relOpWithSwap(cmp, e.getFullyConverted(), z, _, _) and
-        e.getUnderlyingType() instanceof Enum) 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
+  exists(string cmpOp, string leftReason, string rightReason |
-  | ((ss = LeftIsSmaller()  and cmpOp = " <= ") or
+    (
-     (ss = RightIsSmaller() and cmpOp = " >= ")) and
+      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()
+      if cmp.getLeftOperand().isConstant()
-              then none() // Both operands are constant so don't create a message.
+      then
-              else reason = rightReason
+        if cmp.getRightOperand().isConstant()
-       else if cmp.getRightOperand().isConstant()
+        then none() // Both operands are constant so don't create a message.
-              then reason = leftReason
+        else reason = rightReason
-              else reason = leftReason + " and " + rightReason)) and
+      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
+select cmp, "Comparison is always " + value.toString() + " because " + reason + "."
-  cmp, "Comparison is always " + value.toString() + " because " + reason + "."

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

@@ -15,15 +15,15 @@ import cpp
 import PointlessSelfComparison
 
 from ComparisonOperation cmp
-where pointlessSelfComparison(cmp)
+where
-  and not nanTest(cmp)
+  pointlessSelfComparison(cmp) and
-  and not overflowTest(cmp)
+  not nanTest(cmp) and
-  and not exists(MacroInvocation mi | 
+  not overflowTest(cmp) and
-    // cmp is in mi 
+  not exists(MacroInvocation mi |
-    mi.getAnExpandedElement() = cmp and 
+    // cmp is in mi
-  
+    mi.getAnExpandedElement() = cmp and
-    // and cmp was apparently not passed in as a macro parameter 
+    // and cmp was apparently not passed in as a macro parameter
-    cmp.getLocation().getStartLine() = mi.getLocation().getStartLine() and 
+    cmp.getLocation().getStartLine() = mi.getLocation().getStartLine() and
-    cmp.getLocation().getStartColumn() = mi.getLocation().getStartColumn() 
+    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
+  exists(Variable v, VariableAccess lhs, VariableAccess rhs |
-  | lhs = cmp.getLeftOperand() and
+    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(lhs.getQualifier()) and // Avoid structure fields
-    not exists (rhs.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
+  exists(Type t | t = cmp.getLeftOperand().getUnspecifiedType() |
-  | t = cmp.getLeftOperand().getUnspecifiedType()
+    t instanceof FloatingPointType or
-  | t instanceof FloatingPointType or
+    t instanceof TemplateParameter
-    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.ql

@@ -16,8 +16,8 @@ import cpp
  * Gets a `do` ... `while` loop with a constant false condition.
  */
 DoStmt getAFalseLoop() {
-  result.getControllingExpr().getValue() = "0"
+  result.getControllingExpr().getValue() = "0" and
-  and not result.getControllingExpr().isAffectedByMacro()
+  not result.getControllingExpr().isAffectedByMacro()
 }
 
 /**
@@ -30,22 +30,19 @@ DoStmt enclosingLoop(Stmt s) {
   exists(Stmt parent |
     parent = s.getParent() and
     (
-      (
+      parent instanceof Loop and
-        parent instanceof Loop and
+      result = parent
-        result = parent
+      or
-      ) or (
+      not parent instanceof Loop and
-        not parent instanceof Loop and
+      not parent instanceof SwitchStmt and
-        not parent instanceof SwitchStmt and
+      result = enclosingLoop(parent)
-        result = enclosingLoop(parent)
-      )
     )
   )
 }
 
 from DoStmt loop, ContinueStmt continue
-where loop = getAFalseLoop()
+where
-  and loop = enclosingLoop(continue)
+  loop = getAFalseLoop() and
-select continue,
+  loop = enclosingLoop(continue)
-  "This 'continue' never re-runs the loop - the $@ is always false.",
+select continue, "This 'continue' never re-runs the loop - the $@ is always false.",
-  loop.getControllingExpr(),
+  loop.getControllingExpr(), "loop condition"
-  "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
+where
-        argType = c.getArgument(i).getType() and
+  f = c.getTarget() and
-        paramType = f.getParameter(i).getType() and
+  argType = c.getArgument(i).getType() and
-        a = argType.getArraySize() and
+  paramType = f.getParameter(i).getType() and
-        b = paramType.getArraySize() and
+  a = argType.getArraySize() and
-        argType.getBaseType().getSize() = paramType.getBaseType().getSize() and
+  b = paramType.getArraySize() and
-        a < b and
+  argType.getBaseType().getSize() = paramType.getBaseType().getSize() and
-        not memberMayBeVarSize(_, c.getArgument(i).(VariableAccess).getTarget()) and
+  a < b and
-        // filter out results for inconsistent declarations
+  not memberMayBeVarSize(_, c.getArgument(i).(VariableAccess).getTarget()) and
-        strictcount(f.getParameter(i).getType().getSize()) = 1
+  // filter out results for inconsistent declarations
-select c.getArgument(i), "Array of size " + a +
+  strictcount(f.getParameter(i).getType().getSize()) = 1
-        " passed to $@ which expects an array of size " + b + ".",
+select c.getArgument(i),
-        f, f.getName()
+  "Array of size " + a + " passed to $@ which expects an array of size " + b + ".", f, f.getName()

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

@@ -20,26 +20,17 @@ import semmle.code.cpp.dataflow.DataFlow
 import DataFlow::PathGraph
 
 class CastToPointerArithFlow extends DataFlow::Configuration {
-  CastToPointerArithFlow() {
+  CastToPointerArithFlow() { this = "CastToPointerArithFlow" }
-    this = "CastToPointerArithFlow"
-  }
 
   override predicate isSource(DataFlow::Node node) {
     not node.asExpr() instanceof Conversion and
-    introducesNewField(
+    introducesNewField(node.asExpr().getType().(DerivedType).getBaseType(),
-      node.asExpr().getType().(DerivedType).getBaseType(),
+      node.asExpr().getConversion*().getType().(DerivedType).getBaseType())
-      node.asExpr().getConversion*().getType().(DerivedType).getBaseType()
-      
-    )
   }
 
   override predicate isSink(DataFlow::Node node) {
-    exists(PointerAddExpr pae |
+    exists(PointerAddExpr pae | pae.getAnOperand() = node.asExpr()) or
-      pae.getAnOperand() = node.asExpr()
+    exists(ArrayExpr ae | ae.getArrayBase() = node.asExpr())
-    ) or
-    exists(ArrayExpr ae |
-      ae.getArrayBase() = node.asExpr()
-    )
   }
 }
 
@@ -52,12 +43,19 @@ predicate introducesNewField(Class derived, Class base) {
     exists(Field f |
       f.getDeclaringType() = derived and
       derived.getABaseClass+() = base
-    ) or
+    )
+    or
     introducesNewField(derived.getABaseClass(), base)
   )
 }
 
 from DataFlow::PathNode source, DataFlow::PathNode sink, CastToPointerArithFlow cfg
-where cfg.hasFlowPath(source, sink)
+where
-  and source.getNode().asExpr().getFullyConverted().getUnspecifiedType() = sink.getNode().asExpr().getFullyConverted().getUnspecifiedType()
+  cfg.hasFlowPath(source, sink) and
-select sink, source, sink, "Pointer arithmetic here may be done with the wrong type because of the cast $@.", source, "here"
+  source.getNode().asExpr().getFullyConverted().getUnspecifiedType() = sink
+        .getNode()
+        .asExpr()
+        .getFullyConverted()
+        .getUnspecifiedType()
+select sink, source, sink,
+  "Pointer arithmetic here may be done with the wrong type because of the cast $@.", source, "here"

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

@@ -6,18 +6,25 @@
  * @problem.severity warning
  * @tags reliability
  */
+
 import cpp
 
 from Expr e1, Cast e2, IntegralType it1, IntegralType it2
-where e2 = e1.getConversion() and
+where
-      e2.isImplicit() and
+  e2 = e1.getConversion() and
-      it1 = e1.getUnderlyingType() and
+  e2.isImplicit() and
-      it2 = e2.getUnderlyingType() and
+  it1 = e1.getUnderlyingType() and
-      (
+  it2 = e2.getUnderlyingType() and
-        it1.isUnsigned() and it2.isSigned() and it1.getSize() >= it2.getSize()
+  (
-        or it1.isSigned() and it2.isUnsigned()
+    it1.isUnsigned() and it2.isSigned() and it1.getSize() >= it2.getSize()
-      ) and
+    or
-      not (e1.isConstant() and 0 <= e1.getValue().toInt() and
+    it1.isSigned() and it2.isUnsigned()
-                         e1.getValue().toInt() <= ((it2.getSize()*8-1)*(2.log())).exp())
+  ) and
-      and not e1.isConstant()
+  not (
-select e1, "Conversion between signed and unsigned types "+it1.toString()+" and "+it2.toString()+"."
+    e1.isConstant() and
+    0 <= e1.getValue().toInt() and
+    e1.getValue().toInt() <= ((it2.getSize() * 8 - 1) * 2.log()).exp()
+  ) and
+  not e1.isConstant()
+select e1,
+  "Conversion between signed and unsigned types " + it1.toString() + " and " + it2.toString() + "."

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

@@ -14,10 +14,9 @@
 import cpp
 
 from BitField fi, VariableAccess va
-
+where
-where fi.getNumBits() > va.getFullyConverted().getType().getSize() * 8
+  fi.getNumBits() > va.getFullyConverted().getType().getSize() * 8 and
-  and va.getExplicitlyConverted().getType().getSize() > va.getFullyConverted().getType().getSize()
+  va.getExplicitlyConverted().getType().getSize() > va.getFullyConverted().getType().getSize() and
-  and va.getTarget() = fi
+  va.getTarget() = fi and
-  and not va.getActualType() instanceof BoolType
+  not va.getActualType() instanceof BoolType
-
 select va, "Implicit downcast of bitfield $@", fi, fi.toString()

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

@@ -44,15 +44,18 @@ predicate whitelistPow(FunctionCall fc) {
     fc.getTarget().getName() = "pow" or
     fc.getTarget().getName() = "powf" or
     fc.getTarget().getName() = "powl"
-  ) and exists(float value |
+  ) and
+  exists(float value |
     value = fc.getArgument(0).getValue().toFloat() and
     (value.floor() - value).abs() < 0.001
   )
 }
 
 predicate whiteListWrapped(FunctionCall fc) {
-  whitelist(fc.getTarget()) or
+  whitelist(fc.getTarget())
-  whitelistPow(fc) or
+  or
+  whitelistPow(fc)
+  or
   exists(Expr e, ReturnStmt rs |
     whiteListWrapped(e) and
     DataFlow::localExprFlow(e, rs.getExpr()) and
@@ -61,8 +64,11 @@ predicate whiteListWrapped(FunctionCall fc) {
 }
 
 from FunctionCall c, FloatingPointType t1, IntegralType t2
-where t1 = c.getTarget().getType().getUnderlyingType() and
+where
-      t2 = c.getActualType() and
+  t1 = c.getTarget().getType().getUnderlyingType() and
-      c.hasImplicitConversion() and
+  t2 = c.getActualType() and
-      not whiteListWrapped(c)
+  c.hasImplicitConversion() and
-select c, "Return value of type " + t1.toString() + " is implicitly converted to " + t2.toString() + " here."
+  not whiteListWrapped(c)
+select c,
+  "Return value of type " + t1.toString() + " is implicitly converted to " + t2.toString() +
+    " here."

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

@@ -11,6 +11,7 @@
  *       correctness
  *       types
  */
+
 import cpp
 
 predicate lossyPointerCast(Expr e, PointerType pt, IntegralType it) {
@@ -25,4 +26,4 @@ predicate lossyPointerCast(Expr e, PointerType pt, IntegralType it) {
 
 from Expr e, PointerType pt, IntegralType it
 where lossyPointerCast(e, pt, it)
-select e, "Converted from " + pt.getName() + " to smaller type "+it.getName()
+select e, "Converted from " + pt.getName() + " to smaller type " + it.getName()

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

@@ -9,22 +9,26 @@
  *       correctness
  *       types
  */
+
 import cpp
 
 predicate commonErrorCode(string value) {
-  value = "0" or value = "1" or value = "-1"
+  value = "0" or
-  or value = "18446744073709551615" // 2^64-1, i.e. -1 as an unsigned int64
+  value = "1" or
-  or value = "4294967295" // 2^32-1, i.e. -1 as an unsigned int32
+  value = "-1" or
-  or value = "3735928559" // 0xdeadbeef
+  value = "18446744073709551615" or // 2^64-1, i.e. -1 as an unsigned int64
-  or value = "3735929054" // 0xdeadc0de
+  value = "4294967295" or // 2^32-1, i.e. -1 as an unsigned int32
-  or value = "3405691582" // 0xcafebabe
+  value = "3735928559" or // 0xdeadbeef
+  value = "3735929054" or // 0xdeadc0de
+  value = "3405691582" // 0xcafebabe
 }
 
 from Expr e
-where e.isConstant() and
+where
-      not commonErrorCode(e.getValue()) and
+  e.isConstant() and
-      e.getFullyConverted().getType() instanceof PointerType and
+  not commonErrorCode(e.getValue()) and
-      not e.getType() instanceof ArrayType and
+  e.getFullyConverted().getType() instanceof PointerType and
-      not e.getType() instanceof PointerType and
+  not e.getType() instanceof ArrayType and
-      not e.isInMacroExpansion()
+  not e.getType() instanceof PointerType and
+  not e.isInMacroExpansion()
 select e, "Nonzero value " + e.getValueText() + " cast to pointer."

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

@@ -33,11 +33,9 @@ predicate flowsToExpr(Expr source, Expr sink, boolean pathMightOverflow) {
  * checking whether the current expression might overflow.
  */
 predicate flowsToExprImpl(Expr source, Expr sink, boolean pathMightOverflow) {
-  (
+  source = sink and
-    source = sink and
+  pathMightOverflow = false and
-    pathMightOverflow = false and
+  source.(FunctionCall).getTarget().(Snprintf).returnsFullFormatLength()
-    source.(FunctionCall).getTarget().(Snprintf).returnsFullFormatLength()
-  )
   or
   exists(RangeSsaDefinition def, LocalScopeVariable v |
     flowsToDef(source, def, v, pathMightOverflow) and

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

@@ -10,12 +10,14 @@
  * @tags reliability
  *       correctness
  */
+
 import cpp
 
 from FormatLiteral fl, FormattingFunctionCall ffc, int expected, int given
-where ffc = fl.getUse()
+where
-  and expected = fl.getNumArgNeeded()
+  ffc = fl.getUse() and
-  and given = ffc.getNumFormatArgument()
+  expected = fl.getNumArgNeeded() and
-  and expected < given
+  given = ffc.getNumFormatArgument() and
-  and fl.specsAreKnown()
+  expected < given and
-select ffc, "Format expects "+expected.toString()+" arguments but given "+given.toString()
+  fl.specsAreKnown()
+select ffc, "Format expects " + expected.toString() + " arguments but given " + given.toString()

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

@@ -11,12 +11,14 @@
  *       security
  *       external/cwe/cwe-685
  */
+
 import cpp
 
 from FormatLiteral fl, FormattingFunctionCall ffc, int expected, int given
-where ffc = fl.getUse()
+where
-  and expected = fl.getNumArgNeeded()
+  ffc = fl.getUse() and
-  and given = ffc.getNumFormatArgument()
+  expected = fl.getNumArgNeeded() and
-  and expected > given
+  given = ffc.getNumFormatArgument() and
-  and fl.specsAreKnown()
+  expected > given and
-select ffc, "Format expects "+expected.toString()+" arguments but given "+given.toString()
+  fl.specsAreKnown()
+select ffc, "Format expects " + expected.toString() + " arguments but given " + given.toString()

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

@@ -16,17 +16,19 @@ import cpp
 
 /**
  * Holds if the argument corresponding to the `pos` conversion specifier
- * of `ffc` is expected to have type `expected`. 
+ * of `ffc` is expected to have type `expected`.
  */
 pragma[noopt]
-private predicate formattingFunctionCallExpectedType(FormattingFunctionCall ffc, int pos, Type expected) {
+private predicate formattingFunctionCallExpectedType(
-    exists(FormattingFunction f, int i, FormatLiteral fl |
+  FormattingFunctionCall ffc, int pos, Type expected
-      ffc instanceof FormattingFunctionCall and
+) {
-      ffc.getTarget() = f and
+  exists(FormattingFunction f, int i, FormatLiteral fl |
-      f.getFormatParameterIndex() = i and
+    ffc instanceof FormattingFunctionCall and
-      ffc.getArgument(i) = fl and
+    ffc.getTarget() = f and
-      fl.getConversionType(pos) = expected
+    f.getFormatParameterIndex() = i and
-    )
+    ffc.getArgument(i) = fl and
+    fl.getConversionType(pos) = expected
+  )
 }
 
 /**
@@ -50,7 +52,9 @@ predicate formatArgType(FormattingFunctionCall ffc, int pos, Type expected, Expr
  * `expected` and the corresponding argument `arg` has type `actual`.
  */
 pragma[noopt]
-predicate formatOtherArgType(FormattingFunctionCall ffc, int pos, Type expected, Expr arg, Type actual) {
+predicate formatOtherArgType(
+  FormattingFunctionCall ffc, int pos, Type expected, Expr arg, Type actual
+) {
   exists(Expr argConverted |
     (arg = ffc.getMinFieldWidthArgument(pos) or arg = ffc.getPrecisionArgument(pos)) and
     argConverted = arg.getFullyConverted() and
@@ -63,14 +67,14 @@ predicate formatOtherArgType(FormattingFunctionCall ffc, int pos, Type expected,
  * A type that may be expected by a printf format parameter, or that may
  * be pointed to by such a type (e.g. `wchar_t`, from `wchar_t *`).
  */
-class ExpectedType extends Type
+class ExpectedType extends Type {
-{
   ExpectedType() {
-  	exists(Type t |
+    exists(Type t |
       (
         formatArgType(_, _, t, _, _) or
         formatOtherArgType(_, _, t, _, _)
-      ) and this = t.getUnspecifiedType()
+      ) and
+      this = t.getUnspecifiedType()
     )
   }
 }
@@ -78,7 +82,7 @@ class ExpectedType extends Type
 /**
  * Holds if it is safe to display a value of type `actual` when `printf`
  * expects a value of type `expected`.
- * 
+ *
  * Note that variadic arguments undergo default argument promotions before
  * they reach `printf`, notably `bool`, `char`, `short` and `enum` types
  * are promoted to `int` (or `unsigned int`, as appropriate) and `float`s
@@ -89,69 +93,71 @@ predicate trivialConversion(ExpectedType expected, Type actual) {
     formatArgType(_, _, exp, _, act) and
     expected = exp.getUnspecifiedType() and
     actual = act.getUnspecifiedType()
-  ) and (
+  ) and
-    (
+  (
-      // allow a pointer type to be displayed with `%p`
+    // allow a pointer type to be displayed with `%p`
-      expected instanceof VoidPointerType and actual instanceof PointerType
+    expected instanceof VoidPointerType and actual instanceof PointerType
-    ) or (
+    or
-      // allow a function pointer type to be displayed with `%p`
+    // allow a function pointer type to be displayed with `%p`
-      expected instanceof VoidPointerType and actual instanceof FunctionPointerType and expected.getSize() = actual.getSize()
+    expected instanceof VoidPointerType and
-    ) or (
+    actual instanceof FunctionPointerType and
-      // allow an `enum` type to be displayed with `%i`, `%c` etc
+    expected.getSize() = actual.getSize()
-      expected instanceof IntegralType and actual instanceof Enum
+    or
-    ) or (
+    // allow an `enum` type to be displayed with `%i`, `%c` etc
-      // allow any `char *` type to be displayed with `%s`
+    expected instanceof IntegralType and actual instanceof Enum
-      expected instanceof CharPointerType and actual instanceof CharPointerType
+    or
-    ) or (
+    // allow any `char *` type to be displayed with `%s`
-      // allow `wchar_t *`, or any pointer to an integral type of the same size, to be displayed
+    expected instanceof CharPointerType and actual instanceof CharPointerType
-      // with `%ws`
+    or
-      expected.(PointerType).getBaseType().hasName("wchar_t") and
+    // allow `wchar_t *`, or any pointer to an integral type of the same size, to be displayed
-      exists(Wchar_t t |
+    // with `%ws`
-        actual.getUnspecifiedType().(PointerType).getBaseType().(IntegralType).getSize() = t.getSize()
+    expected.(PointerType).getBaseType().hasName("wchar_t") and
-      )
+    exists(Wchar_t t |
-    ) or (
+      actual.getUnspecifiedType().(PointerType).getBaseType().(IntegralType).getSize() = t.getSize()
-      // allow an `int` (or anything promoted to `int`) to be displayed with `%c`
-      expected instanceof CharType and actual instanceof IntType
-    ) or (
-      // allow an `int` (or anything promoted to `int`) to be displayed with `%wc`
-      expected instanceof Wchar_t and actual instanceof IntType
-    ) or (
-      expected instanceof UnsignedCharType and actual instanceof IntType
-    ) or (
-      // allow any integral type of the same size 
-      // (this permits signedness changes)
-      expected.(IntegralType).getSize() = actual.(IntegralType).getSize()
-    ) or (
-      // allow a pointer to any integral type of the same size
-      // (this permits signedness changes)
-      expected.(PointerType).getBaseType().(IntegralType).getSize() = actual.(PointerType).getBaseType().(IntegralType).getSize()
-    ) or (
-      expected = actual
     )
+    or
+    // allow an `int` (or anything promoted to `int`) to be displayed with `%c`
+    expected instanceof CharType and actual instanceof IntType
+    or
+    // allow an `int` (or anything promoted to `int`) to be displayed with `%wc`
+    expected instanceof Wchar_t and actual instanceof IntType
+    or
+    expected instanceof UnsignedCharType and actual instanceof IntType
+    or
+    // allow any integral type of the same size
+    // (this permits signedness changes)
+    expected.(IntegralType).getSize() = actual.(IntegralType).getSize()
+    or
+    // allow a pointer to any integral type of the same size
+    // (this permits signedness changes)
+    expected.(PointerType).getBaseType().(IntegralType).getSize() = actual
+          .(PointerType)
+          .getBaseType()
+          .(IntegralType)
+          .getSize()
+    or
+    expected = actual
   )
 }
 
 /**
  * Gets the size of the `int` type.
  */
-int sizeof_IntType() {
+int sizeof_IntType() { exists(IntType it | result = it.getSize()) }
-  exists(IntType it | result = it.getSize())
-}
 
 from FormattingFunctionCall ffc, int n, Expr arg, Type expected, Type actual
-where (
+where
-        (
+  (
-          formatArgType(ffc, n, expected, arg, actual) and
+    formatArgType(ffc, n, expected, arg, actual) and
-          not exists(Type anyExpected |
+    not exists(Type anyExpected |
-            formatArgType(ffc, n, anyExpected, arg, actual) and
+      formatArgType(ffc, n, anyExpected, arg, actual) and
-            trivialConversion(anyExpected.getUnspecifiedType(), actual.getUnspecifiedType())
+      trivialConversion(anyExpected.getUnspecifiedType(), actual.getUnspecifiedType())
-          )
+    )
-        )
+    or
-        or
+    formatOtherArgType(ffc, n, expected, arg, actual) and
-        (
+    not actual.getUnspecifiedType().(IntegralType).getSize() = sizeof_IntType()
-          formatOtherArgType(ffc, n, expected, arg, actual) and
+  ) and
-          not actual.getUnspecifiedType().(IntegralType).getSize() = sizeof_IntType()
+  not arg.isAffectedByMacro()
-        )
+select arg,
-      )
+  "This argument should be of type '" + expected.getName() + "' but is of type '" +
-      and not arg.isAffectedByMacro()
+    actual.getUnspecifiedType().getName() + "'"
-select arg, "This argument should be of type '"+expected.getName()+"' but is of type '"+actual.getUnspecifiedType().getName() + "'"

cpp/ql/src/Likely Bugs/InconsistentCallOnResult.ql

@@ -11,6 +11,7 @@
  *       non-attributable
  *       external/cwe/cwe-252
  */
+
 import cpp
 
 predicate exclude(Function f) {
@@ -39,7 +40,7 @@ predicate checkExpr(Expr e, string operation, Variable v) {
 
 predicate checkedFunctionCall(FunctionCall fc, string operation) {
   relevantFunctionCall(fc, _) and
-  exists (Variable v, Expr check | v.getAnAssignedValue() = fc |
+  exists(Variable v, Expr check | v.getAnAssignedValue() = fc |
     checkExpr(check, operation, v) and
     check != fc
   )
@@ -47,13 +48,11 @@ predicate checkedFunctionCall(FunctionCall fc, string operation) {
 
 predicate relevantFunctionCall(FunctionCall fc, Function f) {
   fc.getTarget() = f and
-  exists (Variable v | v.getAnAssignedValue() = fc) and
+  exists(Variable v | v.getAnAssignedValue() = fc) and
   not okToIgnore(fc)
 }
 
-predicate okToIgnore(FunctionCall fc) {
+predicate okToIgnore(FunctionCall fc) { fc.isInMacroExpansion() }
-  fc.isInMacroExpansion()
-}
 
 predicate functionStats(Function f, string operation, int used, int total, int percentage) {
   exists(PointerType pt | pt.getATypeNameUse() = f.getADeclarationEntry()) and
@@ -67,7 +66,9 @@ where
   relevantFunctionCall(unchecked, f) and
   not checkedFunctionCall(unchecked, operation) and
   functionStats(f, operation, _, _, percent) and
-  percent >= 70
+  percent >= 70 and
-  and unchecked.getFile().getAbsolutePath().matches("%fbcode%")
+  unchecked.getFile().getAbsolutePath().matches("%fbcode%") and
-  and not unchecked.getFile().getAbsolutePath().matches("%\\_build%")
+  not unchecked.getFile().getAbsolutePath().matches("%\\_build%")
-select unchecked, "After " + percent.toString() + "% of calls to " + f.getName() + " there is a call to " + operation + " on the return value. The call may be missing in this case."
+select unchecked,
+  "After " + percent.toString() + "% of calls to " + f.getName() + " there is a call to " +
+    operation + " on the return value. The call may be missing in this case."

cpp/ql/src/Likely Bugs/InconsistentCheckReturnNull.ql

@@ -14,11 +14,10 @@
  *       non-attributable
  *       external/cwe/cwe-476
  */
+
 import cpp
 
-predicate assertMacro(Macro m) {
+predicate assertMacro(Macro m) { m.getHead().toLowerCase().matches("%assert%") }
-  m.getHead().toLowerCase().matches("%assert%")
-}
 
 predicate assertInvocation(File f, int line) {
   exists(MacroInvocation i, Location l | assertMacro(i.getMacro()) and l = i.getLocation() |
@@ -28,61 +27,92 @@ predicate assertInvocation(File f, int line) {
 
 predicate nullCheckAssert(Expr e, Variable v, Declaration qualifier) {
   nullCheckInCondition(e, v, qualifier) and
-  exists(File f, int i | e.getLocation().getStartLine() = i and e.getFile() = f and assertInvocation(f, i))
+  exists(File f, int i |
+    e.getLocation().getStartLine() = i and e.getFile() = f and assertInvocation(f, i)
+  )
 }
 
 VariableAccess qualifiedAccess(Variable v, Declaration qualifier) {
   result = v.getAnAccess() and
   (
-       result.getQualifier().(VariableAccess).getTarget() = qualifier
+    result.getQualifier().(VariableAccess).getTarget() = qualifier
-    or exists(PointerDereferenceExpr e, VariableAccess va
+    or
-             | result.getQualifier() = e
+    exists(PointerDereferenceExpr e, VariableAccess va | result.getQualifier() = e |
-             | e.getOperand() = va and va.getTarget() = qualifier)
+      e.getOperand() = va and va.getTarget() = qualifier
-    or (not exists(result.getQualifier()) and qualifier = result.getEnclosingFunction())
+    )
-    or (result.getQualifier() instanceof ThisExpr and qualifier = result.getEnclosingFunction())
+    or
+    not exists(result.getQualifier()) and qualifier = result.getEnclosingFunction()
+    or
+    result.getQualifier() instanceof ThisExpr and qualifier = result.getEnclosingFunction()
   )
 }
 
 predicate nullCheckInCondition(Expr e, Variable v, Declaration qualifier) {
-    // if(v)
+  // if(v)
-    exists(FunctionCall fc | relevantFunctionCall(fc, _) and fc = assignedValueForVariableAndQualifier(v, qualifier) |
+  exists(FunctionCall fc |
-        e = qualifiedAccess(v, qualifier)
+    relevantFunctionCall(fc, _) and fc = assignedValueForVariableAndQualifier(v, qualifier)
-    )
+  |
-    or exists(AssignExpr a | a = e and a.getLValue() = qualifiedAccess(v, qualifier))
+    e = qualifiedAccess(v, qualifier)
-    // if(v == NULL), if(v != NULL), if(NULL != v), if(NULL == v)
+  )
-    or exists(EqualityOperation eq | eq = e and nullCheckInCondition(eq.getAnOperand(), v, qualifier) and
+  or
-                                                eq.getAnOperand().getValue() = "0")
+  exists(AssignExpr a | a = e and a.getLValue() = qualifiedAccess(v, qualifier))
-    // if(v && something)
+  or
-    or exists(LogicalAndExpr exp | exp = e and nullCheckInCondition(exp.getAnOperand(), v, qualifier))
+  // if(v == NULL), if(v != NULL), if(NULL != v), if(NULL == v)
-    // if(v || something)
+  exists(EqualityOperation eq |
-    or exists(LogicalOrExpr exp | exp = e and nullCheckInCondition(exp.getAnOperand(), v, qualifier))
+    eq = e and
-    // if(!v)
+    nullCheckInCondition(eq.getAnOperand(), v, qualifier) and
-    or exists(NotExpr exp | exp = e and nullCheckInCondition(exp.getAnOperand(), v, qualifier))
+    eq.getAnOperand().getValue() = "0"
-    or exists(FunctionCall c | c = e and nullCheckInCondition(c.getAnArgument(), v, qualifier) and
+  )
-                                         c.getTarget().getName() = "__builtin_expect")
+  or
-    or exists(ConditionDeclExpr d | d = e and nullCheckInCondition(d.getVariableAccess(), v, qualifier))
+  // if(v && something)
+  exists(LogicalAndExpr exp | exp = e and nullCheckInCondition(exp.getAnOperand(), v, qualifier))
+  or
+  // if(v || something)
+  exists(LogicalOrExpr exp | exp = e and nullCheckInCondition(exp.getAnOperand(), v, qualifier))
+  or
+  // if(!v)
+  exists(NotExpr exp | exp = e and nullCheckInCondition(exp.getAnOperand(), v, qualifier))
+  or
+  exists(FunctionCall c |
+    c = e and
+    nullCheckInCondition(c.getAnArgument(), v, qualifier) and
+    c.getTarget().getName() = "__builtin_expect"
+  )
+  or
+  exists(ConditionDeclExpr d | d = e and nullCheckInCondition(d.getVariableAccess(), v, qualifier))
 }
 
 predicate hasNullCheck(Function enclosing, Variable v, Declaration qualifier) {
-  exists(Expr exp | nullCheckInCondition(exp, v, qualifier) and exp.getEnclosingFunction() = enclosing |
+  exists(Expr exp |
-       exists(ControlStructure s | exp = s.getControllingExpr())
+    nullCheckInCondition(exp, v, qualifier) and exp.getEnclosingFunction() = enclosing
-    or exists(ConditionalExpr e | exp = e.getCondition())
+  |
-    or exists(ReturnStmt s | exp = s.getExpr() and not exp instanceof VariableAccess)
+    exists(ControlStructure s | exp = s.getControllingExpr())
-    or exists(AssignExpr e | exp = e.getRValue() and not exp instanceof VariableAccess)
+    or
-    or exists(AggregateLiteral al | exp = al.getAChild() and not exp instanceof VariableAccess)
+    exists(ConditionalExpr e | exp = e.getCondition())
-    or exists(Variable other | exp = other.getInitializer().getExpr() and not exp instanceof VariableAccess)
+    or
+    exists(ReturnStmt s | exp = s.getExpr() and not exp instanceof VariableAccess)
+    or
+    exists(AssignExpr e | exp = e.getRValue() and not exp instanceof VariableAccess)
+    or
+    exists(AggregateLiteral al | exp = al.getAChild() and not exp instanceof VariableAccess)
+    or
+    exists(Variable other |
+      exp = other.getInitializer().getExpr() and not exp instanceof VariableAccess
+    )
   )
 }
 
 Expr assignedValueForVariableAndQualifier(Variable v, Declaration qualifier) {
-     (result = v.getInitializer().getExpr() and qualifier = result.getEnclosingFunction())
+  result = v.getInitializer().getExpr() and qualifier = result.getEnclosingFunction()
-  or exists(AssignExpr e | e.getLValue() = qualifiedAccess(v, qualifier) and result = e.getRValue())
+  or
+  exists(AssignExpr e | e.getLValue() = qualifiedAccess(v, qualifier) and result = e.getRValue())
 }
 
-
 predicate checkedFunctionCall(FunctionCall fc) {
   relevantFunctionCall(fc, _) and
-  exists (Variable v, Declaration qualifier | fc = assignedValueForVariableAndQualifier(v, qualifier) |
+  exists(Variable v, Declaration qualifier |
+    fc = assignedValueForVariableAndQualifier(v, qualifier)
+  |
     hasNullCheck(fc.getEnclosingFunction(), v, qualifier)
   )
 }
@@ -91,27 +121,33 @@ predicate uncheckedFunctionCall(FunctionCall fc) {
   relevantFunctionCall(fc, _) and
   not checkedFunctionCall(fc) and
   not exists(File f, int line | f = fc.getFile() and line = fc.getLocation().getEndLine() |
-    assertInvocation(f, line+1) or assertInvocation(f, line)
+    assertInvocation(f, line + 1) or assertInvocation(f, line)
   ) and
-  not exists(Variable v, Declaration qualifier
+  not exists(Variable v, Declaration qualifier |
-            | fc = assignedValueForVariableAndQualifier(v, qualifier)
+    fc = assignedValueForVariableAndQualifier(v, qualifier)
-            | nullCheckAssert(_, v, qualifier)) and
+  |
-  not exists(ControlStructure s
+    nullCheckAssert(_, v, qualifier)
-            | callResultNullCheckInCondition(s.getControllingExpr(), fc)) and
+  ) and
-  not exists(FunctionCall other, Variable v, Declaration qualifier, Expr arg
+  not exists(ControlStructure s | callResultNullCheckInCondition(s.getControllingExpr(), fc)) and
-            | fc = assignedValueForVariableAndQualifier(v, qualifier)
+  not exists(FunctionCall other, Variable v, Declaration qualifier, Expr arg |
-            | arg = other.getAnArgument() and
+    fc = assignedValueForVariableAndQualifier(v, qualifier)
-              nullCheckInCondition(arg, v, qualifier) and
+  |
-              not arg instanceof VariableAccess)
+    arg = other.getAnArgument() and
+    nullCheckInCondition(arg, v, qualifier) and
+    not arg instanceof VariableAccess
+  )
 }
 
-
 Declaration functionQualifier(FunctionCall fc) {
-     fc.getQualifier().(VariableAccess).getTarget() = result
+  fc.getQualifier().(VariableAccess).getTarget() = result
-  or exists(PointerDereferenceExpr e, VariableAccess va
+  or
-           | fc.getQualifier() = e and e.getOperand() = va and va.getTarget() = result)
+  exists(PointerDereferenceExpr e, VariableAccess va |
-  or (not exists(fc.getQualifier()) and result = fc.getEnclosingFunction())
+    fc.getQualifier() = e and e.getOperand() = va and va.getTarget() = result
-  or (fc.getQualifier() instanceof ThisExpr and result = fc.getEnclosingFunction())
+  )
+  or
+  not exists(fc.getQualifier()) and result = fc.getEnclosingFunction()
+  or
+  fc.getQualifier() instanceof ThisExpr and result = fc.getEnclosingFunction()
 }
 
 predicate callTargetAndEnclosing(FunctionCall fc, Function target, Function enclosing) {
@@ -123,28 +159,38 @@ predicate callArgumentVariable(FunctionCall fc, Variable v, int i) {
 }
 
 predicate callResultNullCheckInCondition(Expr e, FunctionCall fc) {
-    // if(v)
+  // if(v)
-    exists(FunctionCall other | e = other and
+  exists(FunctionCall other |
-      relevantFunctionCall(fc,_) and not checkedFunctionCall(fc)
+    e = other and
-      and exists(Function called, Function enclosing |
+    relevantFunctionCall(fc, _) and
-                 callTargetAndEnclosing(fc, called, enclosing) and
+    not checkedFunctionCall(fc) and
-                 callTargetAndEnclosing(other, called, enclosing)) and
+    exists(Function called, Function enclosing |
-      forall(Variable v, int i | callArgumentVariable(fc, v, i) | callArgumentVariable(other, v, i)) and
+      callTargetAndEnclosing(fc, called, enclosing) and
-      (
+      callTargetAndEnclosing(other, called, enclosing)
-        functionQualifier(fc) = functionQualifier(other)
+    ) and
-        or
+    forall(Variable v, int i | callArgumentVariable(fc, v, i) | callArgumentVariable(other, v, i)) and
-        (not exists(functionQualifier(fc)) and not exists(functionQualifier(other)))
+    (
-      )
+      functionQualifier(fc) = functionQualifier(other)
+      or
+      not exists(functionQualifier(fc)) and not exists(functionQualifier(other))
     )
-    // if(v == NULL), if(v != NULL), if(NULL != v), if(NULL == v)
+  )
-    or exists(EqualityOperation eq | eq = e and callResultNullCheckInCondition(eq.getAnOperand(), fc) and
+  or
-                                                eq.getAnOperand().getValue() = "0")
+  // if(v == NULL), if(v != NULL), if(NULL != v), if(NULL == v)
-    // if(v && something)
+  exists(EqualityOperation eq |
-    or exists(LogicalAndExpr exp | exp = e and callResultNullCheckInCondition(exp.getAnOperand(), fc))
+    eq = e and
-    // if(v || something)
+    callResultNullCheckInCondition(eq.getAnOperand(), fc) and
-    or exists(LogicalOrExpr exp | exp = e and callResultNullCheckInCondition(exp.getAnOperand(), fc))
+    eq.getAnOperand().getValue() = "0"
-    // if(!v)
+  )
-    or exists(NotExpr exp | exp = e and callResultNullCheckInCondition(exp.getAnOperand(), fc))
+  or
+  // if(v && something)
+  exists(LogicalAndExpr exp | exp = e and callResultNullCheckInCondition(exp.getAnOperand(), fc))
+  or
+  // if(v || something)
+  exists(LogicalOrExpr exp | exp = e and callResultNullCheckInCondition(exp.getAnOperand(), fc))
+  or
+  // if(!v)
+  exists(NotExpr exp | exp = e and callResultNullCheckInCondition(exp.getAnOperand(), fc))
 }
 
 predicate dereferenced(Variable v, Declaration qualifier, Function f) {
@@ -152,11 +198,13 @@ predicate dereferenced(Variable v, Declaration qualifier, Function f) {
     e.getOperand() = qualifiedAccess(v, qualifier) and
     e.getEnclosingFunction() = f and
     not exists(SizeofExprOperator s | s.getExprOperand() = e)
-  ) or
+  )
+  or
   exists(FunctionCall c |
     c.getQualifier() = qualifiedAccess(v, qualifier) and
     c.getEnclosingFunction() = f
-  ) or
+  )
+  or
   exists(VariableAccess va |
     va.getQualifier() = qualifiedAccess(v, qualifier) and
     va.getEnclosingFunction() = f
@@ -165,15 +213,15 @@ predicate dereferenced(Variable v, Declaration qualifier, Function f) {
 
 predicate relevantFunctionCall(FunctionCall fc, Function f) {
   fc.getTarget() = f and
-  exists (Variable v, Declaration qualifier
+  exists(Variable v, Declaration qualifier |
-         | fc = assignedValueForVariableAndQualifier(v, qualifier)
+    fc = assignedValueForVariableAndQualifier(v, qualifier)
-         | dereferenced(v, qualifier, fc.getEnclosingFunction())) and
+  |
+    dereferenced(v, qualifier, fc.getEnclosingFunction())
+  ) and
   not okToIgnore(fc)
 }
 
-predicate okToIgnore(FunctionCall fc) {
+predicate okToIgnore(FunctionCall fc) { fc.isInMacroExpansion() }
-  fc.isInMacroExpansion()
-}
 
 predicate functionStats(Function f, int percentage) {
   exists(int used, int total |
@@ -190,4 +238,6 @@ where
   uncheckedFunctionCall(unchecked) and
   functionStats(f, percent) and
   percent >= 70
-select unchecked, "The result of this call to " + f.getName() + " is not checked for null, but " + percent + "% of calls to " + f.getName() + " check for null."
+select unchecked,
+  "The result of this call to " + f.getName() + " is not checked for null, but " + percent +
+    "% of calls to " + f.getName() + " check for null."

cpp/ql/src/Likely Bugs/JapaneseEra/StructWithExactEraDate.ql

@@ -15,8 +15,7 @@
 import cpp
 import semmle.code.cpp.commons.DateTime
 
-predicate assignedYear(Struct s, YearFieldAccess year, int value)
+predicate assignedYear(Struct s, YearFieldAccess year, int value) {
-{
   exists(Operation yearAssignment |
     s.getAField().getAnAccess() = year and
     yearAssignment.getAnOperand() = year and
@@ -24,8 +23,7 @@ predicate assignedYear(Struct s, YearFieldAccess year, int value)
   )
 }
 
-predicate assignedMonth(Struct s, MonthFieldAccess month, int value)
+predicate assignedMonth(Struct s, MonthFieldAccess month, int value) {
-{
   exists(Operation monthAssignment |
     s.getAField().getAnAccess() = month and
     monthAssignment.getAnOperand() = month and
@@ -33,8 +31,7 @@ predicate assignedMonth(Struct s, MonthFieldAccess month, int value)
   )
 }
 
-predicate assignedDay(Struct s, DayFieldAccess day, int value)
+predicate assignedDay(Struct s, DayFieldAccess day, int value) {
-{
   exists(Operation dayAssignment |
     s.getAField().getAnAccess() = day and
     dayAssignment.getAnOperand() = day and
@@ -42,8 +39,7 @@ predicate assignedDay(Struct s, DayFieldAccess day, int value)
   )
 }
 
-from
+from StructLikeClass s, YearFieldAccess year, MonthFieldAccess month, DayFieldAccess day
-  StructLikeClass s, YearFieldAccess year, MonthFieldAccess month, DayFieldAccess day
 where
   assignedYear(s, year, 1989) and
   assignedMonth(s, month, 1) and

cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll

@@ -197,18 +197,14 @@ class StructTmLeapYearFieldAccess extends LeapYearFieldAccess {
  * `Function` that includes an operation that is checking for leap year.
  */
 class ChecksForLeapYearFunction extends Function {
-  ChecksForLeapYearFunction() {
+  ChecksForLeapYearFunction() { this = any(CheckForLeapYearOperation clyo).getEnclosingFunction() }
-    this = any(CheckForLeapYearOperation clyo).getEnclosingFunction()
-  }
 }
 
 /**
  * `FunctionCall` that includes an operation that is checking for leap year.
  */
 class ChecksForLeapYearFunctionCall extends FunctionCall {
-  ChecksForLeapYearFunctionCall() {
+  ChecksForLeapYearFunctionCall() { this.getTarget() instanceof ChecksForLeapYearFunction }
-    this.getTarget() instanceof ChecksForLeapYearFunction
-  }
 }
 
 /**

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

@@ -10,26 +10,25 @@
  *       correctness
  *       external/cwe/cwe-481
  */
+
 import cpp
 import semmle.code.cpp.controlflow.LocalScopeVariableReachability
 
 class UndefReachability extends LocalScopeVariableReachability {
-  UndefReachability() {
+  UndefReachability() { this = "UndefReachability" }
-    this = "UndefReachability"
+
-  }
-  
   override predicate isSource(ControlFlowNode node, LocalScopeVariable v) {
     candidateVariable(v) and
     node = v.getParentScope() and
     not v instanceof Parameter and
     not v.hasInitializer()
   }
-  
+
   override predicate isSink(ControlFlowNode node, LocalScopeVariable v) {
     candidateVariable(v) and
     node = v.getAnAccess()
   }
-  
+
   override predicate isBarrier(ControlFlowNode node, LocalScopeVariable v) {
     node.(AssignExpr).getLValue() = v.getAnAccess()
   }
@@ -41,27 +40,23 @@ abstract class BooleanControllingAssignment extends AssignExpr {
 
 class BooleanControllingAssignmentInExpr extends BooleanControllingAssignment {
   BooleanControllingAssignmentInExpr() {
-       this.getParent() instanceof UnaryLogicalOperation
+    this.getParent() instanceof UnaryLogicalOperation or
-    or this.getParent() instanceof BinaryLogicalOperation
+    this.getParent() instanceof BinaryLogicalOperation or
-    or exists(ConditionalExpr c | c.getCondition() = this)
+    exists(ConditionalExpr c | c.getCondition() = this)
   }
 
-  override predicate isWhitelisted() {
+  override predicate isWhitelisted() { this.getConversion().(ParenthesisExpr).isParenthesised() }
-    this.getConversion().(ParenthesisExpr).isParenthesised()
-  }
 }
 
 class BooleanControllingAssignmentInStmt extends BooleanControllingAssignment {
   BooleanControllingAssignmentInStmt() {
-       exists(IfStmt i | i.getCondition() = this)
+    exists(IfStmt i | i.getCondition() = this) or
-    or exists(ForStmt f | f.getCondition() = this)
+    exists(ForStmt f | f.getCondition() = this) or
-    or exists(WhileStmt w | w.getCondition() = this)
+    exists(WhileStmt w | w.getCondition() = this) or
-    or exists(DoStmt d | d.getCondition() = this)
+    exists(DoStmt d | d.getCondition() = this)
   }
 
-  override predicate isWhitelisted() {
+  override predicate isWhitelisted() { this.isParenthesised() }
-    this.isParenthesised()
-  }
 }
 
 /**
@@ -84,6 +79,7 @@ predicate candidateVariable(Variable v) {
 }
 
 from BooleanControllingAssignment ae, UndefReachability undef
-where candidateResult(ae)
+where
-  and not undef.reaches(_, ae.getLValue().(VariableAccess).getTarget(), ae.getLValue())
+  candidateResult(ae) and
+  not undef.reaches(_, ae.getLValue().(VariableAccess).getTarget(), ae.getLValue())
 select ae, "Use of '=' where '==' may have been intended."

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

@@ -39,8 +39,9 @@ predicate nonShortCircuitLogic2(BinaryBitwiseOperation op) {
 }
 
 from LogicalOperation o
-where o.getParent() instanceof BitwiseOperation and
+where
-    not nonShortCircuitLogic2(o.getParent()) and
+  o.getParent() instanceof BitwiseOperation and
-    not o.getParent().isInMacroExpansion() and // It's ok if o itself is in a macro expansion.
+  not nonShortCircuitLogic2(o.getParent()) and
-    not o.getParent().(LShiftExpr).getLeftOperand() = o // Common pattern for producing bit masks: "(a && b) << 16".
+  not o.getParent().isInMacroExpansion() and // It's ok if o itself is in a macro expansion.
+  not o.getParent().(LShiftExpr).getLeftOperand() = o // Common pattern for producing bit masks: "(a && b) << 16".
 select o, "The result of this expression is Boolean, but it is used in a bitwise context."

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

@@ -10,10 +10,12 @@
  *       correctness
  *       external/cwe/cwe-482
  */
+
 import cpp
 
 from ExprInVoidContext op
-where op instanceof EQExpr
+where
-      or
+  op instanceof EQExpr
-      op.(FunctionCall).getTarget().hasName("operator==")
+  or
+  op.(FunctionCall).getTarget().hasName("operator==")
 select op, "This '==' operator has no effect. The assignment ('=') operator was probably intended."

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

@@ -21,8 +21,10 @@ predicate zeroComparison(EqualityOperation e) {
 
 predicate inNullContext(AddressOfExpr e) {
   e.getFullyConverted().getUnderlyingType() instanceof BoolType
-  or exists(ControlStructure c | c.getControllingExpr() = e)
+  or
-  or exists(EqualityOperation cmp | zeroComparison(cmp) |
+  exists(ControlStructure c | c.getControllingExpr() = e)
+  or
+  exists(EqualityOperation cmp | zeroComparison(cmp) |
     e = cmp.getLeftOperand() or
     e = cmp.getRightOperand()
   )
@@ -34,11 +36,12 @@ FieldAccess chainedFields(FieldAccess fa) {
 }
 
 from AddressOfExpr addrof, FieldAccess fa, Variable v, int offset
-where fa = addrof.getOperand()
+where
-  and inNullContext(addrof)
+  fa = addrof.getOperand() and
-  and not addrof.isInMacroExpansion()
+  inNullContext(addrof) and
-  and v.getAnAccess() = chainedFields(fa).getQualifier()
+  not addrof.isInMacroExpansion() and
-  and not v instanceof MemberVariable
+  v.getAnAccess() = chainedFields(fa).getQualifier() and
-  and offset = strictsum(chainedFields(fa).getTarget().getByteOffset())
+  not v instanceof MemberVariable and
-  and offset != 0
+  offset = strictsum(chainedFields(fa).getTarget().getByteOffset()) and
+  offset != 0
 select addrof, "This will only be NULL if " + v.getName() + " == -" + offset + "."

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

@@ -10,6 +10,7 @@
  *       correctness
  *       external/cwe/cwe-561
  */
+
 import cpp
 private import semmle.code.cpp.commons.Exclusions
 
@@ -20,8 +21,10 @@ class PureExprInVoidContext extends ExprInVoidContext {
 // loop variable mentioned in the init stmt of a for
 predicate accessInInitOfForStmt(Expr e) {
   e instanceof Access and
-  exists(ForStmt f, ExprStmt s | f.getInitialization() = s and
+  exists(ForStmt f, ExprStmt s |
-                                 s.getExpr() = e)
+    f.getInitialization() = s and
+    s.getExpr() = e
+  )
 }
 
 /**
@@ -29,7 +32,8 @@ predicate accessInInitOfForStmt(Expr e) {
  * code excluded by the preprocessor.
  */
 predicate functionContainsDisabledCodeRecursive(Function f) {
-  functionContainsDisabledCode(f) or
+  functionContainsDisabledCode(f)
+  or
   // recurse into function calls
   exists(FunctionCall fc |
     fc.getEnclosingFunction() = f and
@@ -42,7 +46,8 @@ predicate functionContainsDisabledCodeRecursive(Function f) {
  * preprocessor branch that may have code in another arm
  */
 predicate functionDefinedInIfDefRecursive(Function f) {
-  functionDefinedInIfDef(f) or
+  functionDefinedInIfDef(f)
+  or
   // recurse into function calls
   exists(FunctionCall fc |
     fc.getEnclosingFunction() = f and
@@ -60,36 +65,41 @@ predicate functionDefinedInIfDefRecursive(Function f) {
  * break encapsulation.
  */
 predicate baseCall(FunctionCall call) {
-  call.getNameQualifier().getQualifyingElement() =
+  call.getNameQualifier().getQualifyingElement() = call
-    call.getEnclosingFunction().getDeclaringType().(Class).getABaseClass+()
+        .getEnclosingFunction()
+        .getDeclaringType()
+        .(Class)
+        .getABaseClass+()
 }
 
-from PureExprInVoidContext peivc, Locatable parent,
+from PureExprInVoidContext peivc, Locatable parent, Locatable info, string info_text, string tail
-  Locatable info, string info_text, string tail
+where
-where // EQExprs are covered by CompareWhereAssignMeant.ql
+  // EQExprs are covered by CompareWhereAssignMeant.ql
-      not peivc instanceof EQExpr and
+  not peivc instanceof EQExpr and
-      // as is operator==
+  // as is operator==
-      not peivc.(FunctionCall).getTarget().hasName("operator==") and
+  not peivc.(FunctionCall).getTarget().hasName("operator==") and
-      not baseCall(peivc) and
+  not baseCall(peivc) and
-      not accessInInitOfForStmt(peivc) and
+  not accessInInitOfForStmt(peivc) and
-      not peivc.isCompilerGenerated() and
+  not peivc.isCompilerGenerated() and
-      not peivc.getEnclosingFunction().isDefaulted() and
+  not peivc.getEnclosingFunction().isDefaulted() and
-      not exists(Macro m | peivc = m.getAnInvocation().getAnExpandedElement()) and
+  not exists(Macro m | peivc = m.getAnInvocation().getAnExpandedElement()) and
-      not peivc.isFromTemplateInstantiation(_) and
+  not peivc.isFromTemplateInstantiation(_) and
-      parent = peivc.getParent() and
+  parent = peivc.getParent() and
-      not parent.isInMacroExpansion() and
+  not parent.isInMacroExpansion() and
-      not parent instanceof PureExprInVoidContext and
+  not parent instanceof PureExprInVoidContext and
-      not peivc.getEnclosingFunction().isCompilerGenerated() and
+  not peivc.getEnclosingFunction().isCompilerGenerated() and
-      not peivc.getType() instanceof UnknownType and
+  not peivc.getType() instanceof UnknownType and
-      not functionContainsDisabledCodeRecursive(peivc.(FunctionCall).getTarget()) and
+  not functionContainsDisabledCodeRecursive(peivc.(FunctionCall).getTarget()) and
-      not functionDefinedInIfDefRecursive(peivc.(FunctionCall).getTarget()) and
+  not functionDefinedInIfDefRecursive(peivc.(FunctionCall).getTarget()) and
-      if peivc instanceof FunctionCall then
+  if peivc instanceof FunctionCall
-        exists(Function target |
+  then
-          target = peivc.(FunctionCall).getTarget() and
+    exists(Function target |
-          info = target and
+      target = peivc.(FunctionCall).getTarget() and
-          info_text = target.getName() and
+      info = target and
-          tail = " (because $@ has no external side effects).")
+      info_text = target.getName() and
-      else
+      tail = " (because $@ has no external side effects)."
-        (tail = "." and info = peivc and info_text = "")
+    )
-select peivc, "This expression has no effect" + tail,
+  else (
-       info, info_text
+    tail = "." and info = peivc and info_text = ""
+  )
+select peivc, "This expression has no effect" + tail, info, info_text

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

@@ -9,6 +9,7 @@
  * @tags reliability
  *       readability
  */
+
 import cpp
 
 predicate macroUse(Locatable l) {
@@ -28,12 +29,12 @@ predicate macroUseLocation(File f, int start, int end) {
 pragma[noopt]
 predicate emptyIf(IfStmt s, Block b, File f, int start, int end) {
   s instanceof IfStmt and
-  not exists (s.getElse()) and
+  not exists(s.getElse()) and
   b = s.getThen() and
   b instanceof Block and
   not exists(b.getAChild()) and
   f = b.getFile() and
-  exists (Location l |
+  exists(Location l |
     l = b.getLocation() and
     start = l.getStartLine() and
     end = l.getEndLine()
@@ -53,6 +54,7 @@ predicate query(IfStmt s, Block b) {
 }
 
 from IfStmt s, Block b
-where query(s, b) and
+where
-      not b.isInMacroExpansion()
+  query(s, b) and
+  not b.isInMacroExpansion()
 select s, "If-statement with an empty then-branch and no else-branch."

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

@@ -17,19 +17,19 @@ import cpp
 /**
  * It's common in some projects to use "a double negation" to normalize the boolean
  * result to either 1 or 0.
- * This predciate is intended to filter explicit usage of a double negation as it typically 
+ * This predciate is intended to filter explicit usage of a double negation as it typically
- * indicates the explicit purpose to normalize the result for bit-wise or arithmetic purposes. 
+ * indicates the explicit purpose to normalize the result for bit-wise or arithmetic purposes.
  */
-predicate doubleNegationNormalization( NotExpr notexpr ){
+predicate doubleNegationNormalization(NotExpr notexpr) { notexpr.getAnOperand() instanceof NotExpr }
-  notexpr.getAnOperand() instanceof NotExpr
-}
 
 from BinaryBitwiseOperation binbitwop
-where exists( NotExpr notexpr | 
+where
-  binbitwop.getAnOperand() = notexpr
+  exists(NotExpr notexpr |
-  and not doubleNegationNormalization(notexpr)
+    binbitwop.getAnOperand() = notexpr and
-  and ( binbitwop instanceof BitwiseAndExpr
+    not doubleNegationNormalization(notexpr) and
-    or binbitwop instanceof BitwiseOrExpr )
+    (
-)
+      binbitwop instanceof BitwiseAndExpr or
+      binbitwop instanceof BitwiseOrExpr
+    )
+  )
 select binbitwop, "Usage of a logical not (!) expression as a bitwise operator."
-

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

@@ -10,11 +10,13 @@
  *       correctness
  *       external/cwe/cwe-478
  */
+
 import cpp
 
 from EnumSwitch es, float missing, float total
-where not es.hasDefaultCase()
+where
-     and missing = count(es.getAMissingCase())
+  not es.hasDefaultCase() and
-     and total = missing + count(es.getASwitchCase())
+  missing = count(es.getAMissingCase()) and
-     and missing/total < 0.30
+  total = missing + count(es.getASwitchCase()) and
-select es, "Switch statement is missing case for "+es.getAMissingCase().getName()
+  missing / total < 0.3
+select es, "Switch statement is missing case for " + es.getAMissingCase().getName()

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

@@ -10,6 +10,7 @@
  *       correctness
  *       external/cwe/cwe-480
  */
+
 import cpp
 
 /**
@@ -21,12 +22,10 @@ float candidateExpr(Expr e) {
     e = blo.getAnOperand() and
     e.isConstant() and
     result = e.getValue().toFloat() and
-
     // exclusions
     not e.isFromTemplateInstantiation(_) and
     not e instanceof SizeofOperator and
     not inMacroExpansion(blo) and
-
     // exclude values 0 and 1
     result != 0.0 and
     result != 1.0
@@ -36,15 +35,21 @@ float candidateExpr(Expr e) {
 from Expr e, float v, int l, string msg
 where
   v = candidateExpr(e) and
-
   // before reporting an error, we check that the candidate is either a hex/octal
-  // literal, or its value is a power of two. 
+  // literal, or its value is a power of two.
   l = v.log2().floor() and
-  if v = 2.pow(l) then
+  if v = 2.pow(l)
-    msg = "Operand to short-circuiting operator looks like a flag ("+v+" = 2 ^ "+l+"), may be typo for bitwise operator."
+  then
-  else exists(string kind |
+    msg = "Operand to short-circuiting operator looks like a flag (" + v + " = 2 ^ " + l +
-    ((e instanceof HexLiteral and kind = "a hexadecimal literal") or
+        "), may be typo for bitwise operator."
-     (e instanceof OctalLiteral and kind = "an octal literal")) and
+  else
-    msg = "Operand to short-circuiting operator is " + kind + ", and therefore likely a flag; a bitwise operator may be intended."
+    exists(string kind |
-  )
+      (
+        e instanceof HexLiteral and kind = "a hexadecimal literal"
+        or
+        e instanceof OctalLiteral and kind = "an octal literal"
+      ) and
+      msg = "Operand to short-circuiting operator is " + kind +
+          ", and therefore likely a flag; a bitwise operator may be intended."
+    )
 select e, msg

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

@@ -70,10 +70,8 @@ predicate isStringCopyUsedInLogicalOperationOrCondition(FunctionCall func, Expr
 
 from FunctionCall func, Expr expr1, string msg
 where
-  (
+  isStringCopyCastedAsBoolean(func, expr1, msg) and
-    isStringCopyCastedAsBoolean(func, expr1, msg) and
+  not isStringCopyUsedInLogicalOperationOrCondition(func, _, _)
-    not isStringCopyUsedInLogicalOperationOrCondition(func, _, _)
-  )
   or
   isStringCopyUsedInLogicalOperationOrCondition(func, expr1, msg)
 select expr1, msg

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

@@ -6,10 +6,11 @@
  * @precision high
  * @id cpp/inconsistent-loop-direction
  * @tags correctness
- *       external/cwe/cwe-835 
+ *       external/cwe/cwe-835
  *       external/microsoft/6293
  * @msrc.severity important
  */
+
 import cpp
 import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
 import semmle.code.cpp.dataflow.DataFlow
@@ -17,17 +18,20 @@ import semmle.code.cpp.dataflow.DataFlow
 /**
  * A `for` statement whose update is a crement operation on a variable.
  */
-predicate candidateForStmt(ForStmt forStmt, Variable v, CrementOperation update, RelationalOperation rel) {
+predicate candidateForStmt(
+  ForStmt forStmt, Variable v, CrementOperation update, RelationalOperation rel
+) {
   update = forStmt.getUpdate() and
   update.getAnOperand() = v.getAnAccess() and
   rel = forStmt.getCondition()
 }
 
 pragma[noinline]
-predicate candidateDecrForStmt(ForStmt forStmt, Variable v, VariableAccess lesserOperand, Expr terminalCondition) {
+predicate candidateDecrForStmt(
+  ForStmt forStmt, Variable v, VariableAccess lesserOperand, Expr terminalCondition
+) {
   exists(DecrementOperation update, RelationalOperation rel |
     candidateForStmt(forStmt, v, update, rel) and
-
     // condition is `v < terminalCondition`
     terminalCondition = rel.getGreaterOperand() and
     lesserOperand = rel.getLesserOperand() and
@@ -35,29 +39,30 @@ predicate candidateDecrForStmt(ForStmt forStmt, Variable v, VariableAccess lesse
   )
 }
 
-predicate illDefinedDecrForStmt( ForStmt forstmt, Variable v, Expr initialCondition, Expr terminalCondition ) {
+predicate illDefinedDecrForStmt(
+  ForStmt forstmt, Variable v, Expr initialCondition, Expr terminalCondition
+) {
   exists(VariableAccess lesserOperand |
     // decrementing for loop
     candidateDecrForStmt(forstmt, v, lesserOperand, terminalCondition) and
-
     // `initialCondition` is a value of `v` in the for loop
     v.getAnAssignedValue() = initialCondition and
     DataFlow::localFlowStep(DataFlow::exprNode(initialCondition), DataFlow::exprNode(lesserOperand)) and
-
     // `initialCondition` < `terminalCondition`
     (
-      ( upperBound(initialCondition) < lowerBound(terminalCondition) )
+      upperBound(initialCondition) < lowerBound(terminalCondition)
       or
-      ( forstmt.conditionAlwaysFalse() or forstmt.conditionAlwaysTrue() )
+      (forstmt.conditionAlwaysFalse() or forstmt.conditionAlwaysTrue())
     )
   )
 }
 
 pragma[noinline]
-predicate candidateIncrForStmt(ForStmt forStmt, Variable v, VariableAccess greaterOperand, Expr terminalCondition) {
+predicate candidateIncrForStmt(
+  ForStmt forStmt, Variable v, VariableAccess greaterOperand, Expr terminalCondition
+) {
   exists(IncrementOperation update, RelationalOperation rel |
     candidateForStmt(forStmt, v, update, rel) and
-
     // condition is `v > terminalCondition`
     terminalCondition = rel.getLesserOperand() and
     greaterOperand = rel.getGreaterOperand() and
@@ -65,71 +70,64 @@ predicate candidateIncrForStmt(ForStmt forStmt, Variable v, VariableAccess great
   )
 }
 
-predicate illDefinedIncrForStmt( ForStmt forstmt, Variable v, Expr initialCondition, Expr terminalCondition ) {
+predicate illDefinedIncrForStmt(
+  ForStmt forstmt, Variable v, Expr initialCondition, Expr terminalCondition
+) {
   exists(VariableAccess greaterOperand |
     // incrementing for loop
     candidateIncrForStmt(forstmt, v, greaterOperand, terminalCondition) and
-
     // `initialCondition` is a value of `v` in the for loop
     v.getAnAssignedValue() = initialCondition and
     DataFlow::localFlowStep(DataFlow::exprNode(initialCondition), DataFlow::exprNode(greaterOperand)) and
-
     // `terminalCondition` < `initialCondition`
     (
-      ( upperBound(terminalCondition) < lowerBound(initialCondition) )
+      upperBound(terminalCondition) < lowerBound(initialCondition)
       or
-      ( forstmt.conditionAlwaysFalse() or forstmt.conditionAlwaysTrue() )
+      (forstmt.conditionAlwaysFalse() or forstmt.conditionAlwaysTrue())
     )
   )
 }
- 
+
-predicate illDefinedForStmtWrongDirection( ForStmt forstmt, Variable v, Expr initialCondition, Expr terminalCondition
+predicate illDefinedForStmtWrongDirection(
-  , boolean isIncr ) {
+  ForStmt forstmt, Variable v, Expr initialCondition, Expr terminalCondition, boolean isIncr
-  ( illDefinedDecrForStmt( forstmt, v, initialCondition, terminalCondition) and isIncr = false )
+) {
+  illDefinedDecrForStmt(forstmt, v, initialCondition, terminalCondition) and isIncr = false
   or
-  ( illDefinedIncrForStmt( forstmt, v, initialCondition, terminalCondition) and isIncr = true)
+  illDefinedIncrForStmt(forstmt, v, initialCondition, terminalCondition) and isIncr = true
-}
- 
-bindingset[b]
-private string forLoopdirection(boolean b){
-  if( b = true ) then  result = "upward" 
-  else result = "downward"
 }
 
 bindingset[b]
-private string forLoopTerminalConditionRelationship(boolean b){
+private string forLoopdirection(boolean b) {
-  if( b = true ) then  result = "lower" 
+  if b = true then result = "upward" else result = "downward"
-  else result = "higher"
 }
 
- predicate illDefinedForStmt( ForStmt for, string message ) {
+bindingset[b]
-  exists( 
+private string forLoopTerminalConditionRelationship(boolean b) {
-    boolean isIncr, 
+  if b = true then result = "lower" else result = "higher"
-    Variable v,
+}
-    Expr initialCondition,
+
-    Expr terminalCondition |
+predicate illDefinedForStmt(ForStmt for, string message) {
-    illDefinedForStmtWrongDirection(for, v, initialCondition, terminalCondition, isIncr)
+  exists(boolean isIncr, Variable v, Expr initialCondition, Expr terminalCondition |
-    and
+    illDefinedForStmtWrongDirection(for, v, initialCondition, terminalCondition, isIncr) and
-    if( for.conditionAlwaysFalse() ) then 
+    if for.conditionAlwaysFalse()
-    (
+    then
-      message = "Ill-defined for-loop: a loop using variable \"" + v + "\" counts " 
+      message = "Ill-defined for-loop: a loop using variable \"" + v + "\" counts " +
-      + forLoopdirection(isIncr) + " from a value ("+ initialCondition +"), but the terminal condition is always false."
+          forLoopdirection(isIncr) + " from a value (" + initialCondition +
-    ) 
+          "), but the terminal condition is always false."
-    else if 
+    else
-    ( 
+      if for.conditionAlwaysTrue()
-      for.conditionAlwaysTrue() ) then (
+      then
-      message = "Ill-defined for-loop: a loop using variable \"" + v + "\" counts " 
+        message = "Ill-defined for-loop: a loop using variable \"" + v + "\" counts " +
-      + forLoopdirection(isIncr) + " from a value ("+ initialCondition +"), but the terminal condition is always true."
+            forLoopdirection(isIncr) + " from a value (" + initialCondition +
-    ) 
+            "), but the terminal condition is always true."
-    else 
+      else
-    (
+        message = "Ill-defined for-loop: a loop using variable \"" + v + "\" counts " +
-      message = "Ill-defined for-loop: a loop using variable \"" + v + "\" counts " 
+            forLoopdirection(isIncr) + " from a value (" + initialCondition +
-        + forLoopdirection(isIncr) + " from a value ("+ initialCondition +"), but the terminal condition is "
+            "), but the terminal condition is " + forLoopTerminalConditionRelationship(isIncr) +
-        + forLoopTerminalConditionRelationship(isIncr) + " (" + terminalCondition + ")."
+            " (" + terminalCondition + ")."
-    )
   )
 }
- 
+
-from ForStmt forstmt, string message 
+from ForStmt forstmt, string message
- where illDefinedForStmt(forstmt, message)
+where illDefinedForStmt(forstmt, message)
 select forstmt, message

cpp/ql/src/Likely Bugs/Memory Management/AllocaInLoop.ql

@@ -328,5 +328,4 @@ where
   not l.isTightlyBounded() and
   alloc = l.getAnAllocaCall() and
   alloc.getASuccessor*() = l.(Loop).getStmt()
-select alloc, "Stack allocation is inside a $@ loop.", l,
+select alloc, "Stack allocation is inside a $@ loop.", l, l.toString()
-  l.toString()

cpp/ql/src/Likely Bugs/Memory Management/Padding/More64BitWaste.ql

@@ -11,8 +11,10 @@
 import semmle.code.cpp.padding.Padding
 
 from PaddedType t, ILP32 ilp32, LP64 lp64, int w32, int w64
-where w32 = t.wastedSpace(ilp32) - t.trailingPadding(ilp32) and
+where
+  w32 = t.wastedSpace(ilp32) - t.trailingPadding(ilp32) and
   w64 = t.wastedSpace(lp64) - t.trailingPadding(lp64) and
   w64 > w32 and
   t.isPrecise()
-select t, t.getName() + " includes " + w32 + " bits of padding on ILP32, but " + w64 + " bits on LP64."
+select t,
+  t.getName() + " includes " + w32 + " bits of padding on ILP32, but " + w64 + " bits on LP64."

cpp/ql/src/Likely Bugs/Memory Management/Padding/NonPortablePrintf.ql

@@ -12,7 +12,8 @@
 import cpp
 import semmle.code.cpp.padding.Padding
 
-/** Used to avoid reporting conflicts between a char
+/**
+ * Used to avoid reporting conflicts between a char
  * pointer type with specified signedness and an unspecified
  * char pointer (whose signedness is compiler-dependent).
  */
@@ -24,14 +25,16 @@ class SignedOrUnsignedCharPointerType extends CharPointerType {
 }
 
 pragma[noopt]
-private predicate formattingFunctionCallExpectedType(FormattingFunctionCall ffc, int pos, Type expected) {
+private predicate formattingFunctionCallExpectedType(
-    exists(FormattingFunction f, int i, FormatLiteral fl |
+  FormattingFunctionCall ffc, int pos, Type expected
-      ffc.getTarget() = f and
+) {
-      ffc instanceof FormattingFunctionCall and
+  exists(FormattingFunction f, int i, FormatLiteral fl |
-      f.getFormatParameterIndex() = i and
+    ffc.getTarget() = f and
-      ffc.getArgument(i) = fl and
+    ffc instanceof FormattingFunctionCall and
-      fl.getConversionType(pos) = expected
+    f.getFormatParameterIndex() = i and
-    )
+    ffc.getArgument(i) = fl and
+    fl.getConversionType(pos) = expected
+  )
 }
 
 pragma[noopt]
@@ -42,7 +45,9 @@ predicate formatArgType(FormattingFunctionCall ffc, int pos, Type expected, Expr
 }
 
 pragma[noopt]
-predicate formatOtherArgType(FormattingFunctionCall ffc, int pos, Type expected, Expr arg, Type actual) {
+predicate formatOtherArgType(
+  FormattingFunctionCall ffc, int pos, Type expected, Expr arg, Type actual
+) {
   (arg = ffc.getMinFieldWidthArgument(pos) or arg = ffc.getPrecisionArgument(pos)) and
   actual = arg.getActualType() and
   exists(IntType it | it instanceof IntType and it.isImplicitlySigned() and expected = it)
@@ -52,37 +57,38 @@ predicate trivialConversion(Type expected, Type actual) {
   formatArgType(_, _, expected, _, actual) and
   (
     expected instanceof VoidPointerType and actual instanceof PointerType
-  or
+    or
     expected instanceof IntegralType and actual instanceof Enum
-  or
+    or
     expected instanceof CharPointerType and actual instanceof SignedOrUnsignedCharPointerType
-  or
+    or
     expected instanceof SignedOrUnsignedCharPointerType and actual instanceof CharPointerType
-  or
+    or
     expected instanceof CharType and actual instanceof IntType
-  or
+    or
     expected instanceof UnsignedCharType and actual instanceof IntType
-  or
+    or
     expected.(IntegralType).getUnsigned() = actual.(IntegralType).getUnsigned()
-  or
+    or
     expected = actual
   )
 }
 
-from FormattingFunctionCall ffc, int n, Expr arg, Type expected, Type actual, ILP32 ilp32, LP64 lp64, int size32, int size64
+from
-where (
+  FormattingFunctionCall ffc, int n, Expr arg, Type expected, Type actual, ILP32 ilp32, LP64 lp64,
-        (
+  int size32, int size64
-          formatArgType(ffc, n, expected, arg, actual) and
+where
-          not trivialConversion(expected, actual)
+  (
-        )
+    formatArgType(ffc, n, expected, arg, actual) and
-        or
+    not trivialConversion(expected, actual)
-        (
+    or
-          formatOtherArgType(ffc, n, expected, arg, actual) and
+    formatOtherArgType(ffc, n, expected, arg, actual) and
-          not actual instanceof IntType
+    not actual instanceof IntType
-        )
+  ) and
-      )
+  not arg.isAffectedByMacro() and
-      and not arg.isAffectedByMacro()
+  size32 = ilp32.paddedSize(actual) and
-      and size32 = ilp32.paddedSize(actual) and size64 = lp64.paddedSize(actual)
+  size64 = lp64.paddedSize(actual) and
-      and size64 != size32
+  size64 != size32
-select arg, "This argument should be of type '" + expected.getName() + "' but is of type '" + actual.getName() +
+select arg,
-  "' (which changes size from " + size32 + " to " + size64 + " on 64-bit systems)."
+  "This argument should be of type '" + expected.getName() + "' but is of type '" + actual.getName()
+    + "' (which changes size from " + size32 + " to " + size64 + " on 64-bit systems)."

cpp/ql/src/Likely Bugs/Memory Management/Padding/Suboptimal64BitType.ql

@@ -10,15 +10,17 @@
 
 import semmle.code.cpp.padding.Padding
 
-from PaddedType t, Architecture arch, WideCharType wc, int holes, int size, int percentage, int optimum
+from
+  PaddedType t, Architecture arch, WideCharType wc, int holes, int size, int percentage, int optimum
 where
-  arch.pointerSize() = 64 and  // Select 64-bit architecture
+  arch.pointerSize() = 64 and // Select 64-bit architecture
-  arch.wideCharSize() = (wc.getSize() * 8) and  // Select Windows(sizeof(wchar_t == 2)) or non-Windows(sizeof(wchar_t == 4))
+  arch.wideCharSize() = (wc.getSize() * 8) and // Select Windows(sizeof(wchar_t == 2)) or non-Windows(sizeof(wchar_t == 4))
   t.isPrecise() and
   optimum = t.optimalSize(arch) and
   size = arch.paddedSize(t) and
   holes = size - optimum and
   holes > 0 and
-  percentage = (holes*100.0/(float)size).ceil()
+  percentage = (holes * 100.0 / size.(float)).ceil()
-select t, t.getName() + " could be optimized to save " + holes + "/" + t.wastedSpace(arch) +
+select t,
-  " bits of padding (or " + percentage + "% of its size)."
+  t.getName() + " could be optimized to save " + holes + "/" + t.wastedSpace(arch) +
+    " bits of padding (or " + percentage + "% of its size)."

cpp/ql/src/Likely Bugs/Memory Management/PotentialBufferOverflow.ql

@@ -14,29 +14,25 @@
  *             Potentially overrunning write with float to string conversion
  *             (`cpp/overrunning-write-with-float) instead.
  */
+
 import cpp
 import semmle.code.cpp.commons.Buffer
 
 class SprintfCall extends FunctionCall {
-  SprintfCall() {
+  SprintfCall() { this.getTarget().hasName("sprintf") or this.getTarget().hasName("vsprintf") }
-    this.getTarget().hasName("sprintf") or this.getTarget().hasName("vsprintf")
-  }
 
-  int getBufferSize() {
+  int getBufferSize() { result = getBufferSize(this.getArgument(0), _) }
-    result = getBufferSize(this.getArgument(0), _)
-  }
 
   int getMaxConvertedLength() {
     result = this.getArgument(1).(FormatLiteral).getMaxConvertedLength()
   }
 
-  predicate isDangerous() {
+  predicate isDangerous() { this.getMaxConvertedLength() > this.getBufferSize() }
-    this.getMaxConvertedLength() > this.getBufferSize()
-  }
 
   string getDescription() {
-    result = "This conversion may yield a string of length "+this.getMaxConvertedLength().toString()+
+    result = "This conversion may yield a string of length " +
-             ", which exceeds the allocated buffer size of "+this.getBufferSize().toString()
+        this.getMaxConvertedLength().toString() + ", which exceeds the allocated buffer size of " +
+        this.getBufferSize().toString()
   }
 }
 

cpp/ql/src/Likely Bugs/Memory Management/ReturnCstrOfLocalStdString.ql

@@ -21,13 +21,14 @@ class StdString extends Class {
   StdString() {
     // `std::string` is usually a typedef and the actual class
     // is called something like `string  std::__cxx11::basic_string`.
-    exists (Type stdstring, Namespace std
+    exists(Type stdstring, Namespace std |
-    | stdstring.getName() = "string" and
+      stdstring.getName() = "string" and
       this = stdstring.getUnspecifiedType() and
       // Make sure that the class is in the `std` namespace.
       std = this.getNamespace().getParentNamespace*() and
       std.getName() = "std" and
-      std.getParentNamespace() instanceof GlobalNamespace)
+      std.getParentNamespace() instanceof GlobalNamespace
+    )
   }
 }
 
@@ -36,17 +37,20 @@ class StdString extends Class {
  * allocated `std::string`.
  */
 predicate refToStdString(Expr e, ConstructorCall source) {
-  exists (StdString stdstring
+  exists(StdString stdstring |
-  | stdstring.getAMemberFunction() = source.getTarget() and
+    stdstring.getAMemberFunction() = source.getTarget() and
-    not exists(LocalVariable v
+    not exists(LocalVariable v |
-        | source = v.getInitializer().getExpr() and
+      source = v.getInitializer().getExpr() and
-          v.isStatic()) and
+      v.isStatic()
-    e = source)
+    ) and
+    e = source
+  )
   or
   // Indirect use.
-  exists(Expr prev
+  exists(Expr prev |
-  | refToStdString(prev, source) and
+    refToStdString(prev, source) and
-    DataFlow::localFlowStep(DataFlow::exprNode(prev), DataFlow::exprNode(e)))
+    DataFlow::localFlowStep(DataFlow::exprNode(prev), DataFlow::exprNode(e))
+  )
 }
 
 /**
@@ -58,12 +62,11 @@ predicate refToStdString(Expr e, ConstructorCall source) {
  * will also become invalid.
  */
 predicate flowFunction(Function fcn, int argIndex) {
-  (fcn.hasQualifiedName("", "_JNIEnv", "NewStringUTF") and argIndex = 0)
+  fcn.hasQualifiedName("", "_JNIEnv", "NewStringUTF") and argIndex = 0
   or
-  (fcn.hasQualifiedName("art", "JNI", "NewStringUTF") and argIndex = 1)
+  fcn.hasQualifiedName("art", "JNI", "NewStringUTF") and argIndex = 1
   or
-  (fcn.hasQualifiedName("art", "CheckJNI", "NewStringUTF") and argIndex = 1)
+  fcn.hasQualifiedName("art", "CheckJNI", "NewStringUTF") and argIndex = 1
-
   // Add other functions that behave like NewStringUTF here.
 }
 
@@ -72,27 +75,28 @@ predicate flowFunction(Function fcn, int argIndex) {
  * `c_str` on a locally allocated `std::string`.
  */
 predicate refToCStr(Expr e, ConstructorCall source) {
-  exists (MemberFunction f, FunctionCall call
+  exists(MemberFunction f, FunctionCall call |
-  | f.getName() = "c_str" and
+    f.getName() = "c_str" and
     call = e and
     call.getTarget() = f and
-    refToStdString(call.getQualifier(), source))
+    refToStdString(call.getQualifier(), source)
+  )
   or
   // Indirect use.
-  exists(Expr prev
+  exists(Expr prev |
-  | refToCStr(prev, source) and
+    refToCStr(prev, source) and
-    DataFlow::localFlowStep(DataFlow::exprNode(prev), DataFlow::exprNode(e)))
+    DataFlow::localFlowStep(DataFlow::exprNode(prev), DataFlow::exprNode(e))
+  )
   or
   // Some functions, such as `JNIEnv::NewStringUTF()` (from Java's JNI)
   // embed return a structure containing a reference to the C-style string.
-  exists (Function f, int argIndex
+  exists(Function f, int argIndex |
-  | flowFunction(f, argIndex) and
+    flowFunction(f, argIndex) and
     f = e.(Call).getTarget() and
-    refToCStr(e.(Call).getArgument(argIndex), source))
+    refToCStr(e.(Call).getArgument(argIndex), source)
+  )
 }
 
 from ReturnStmt r, ConstructorCall source
 where refToCStr(r.getExpr(), source)
-select
+select r, "Return value may contain a dangling pointer to $@.", source, "this local std::string"
-  r, "Return value may contain a dangling pointer to $@.",
-  source, "this local std::string"

cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.ql

@@ -32,8 +32,8 @@ predicate conservativeDataFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
  * data flow through such expressions.
  */
 predicate hasNontrivialConversion(Expr e) {
-  e instanceof Conversion and not
+  e instanceof Conversion and
-  (
+  not (
     e instanceof Cast
     or
     e instanceof ParenthesisExpr
@@ -61,10 +61,7 @@ where
     exists(Expr pointerToLocal |
       variableAddressEscapesTree(va, pointerToLocal.getFullyConverted()) and
       not hasNontrivialConversion(pointerToLocal) and
-      conservativeDataFlowStep+(
+      conservativeDataFlowStep+(DataFlow::exprNode(pointerToLocal), DataFlow::exprNode(r.getExpr()))
-        DataFlow::exprNode(pointerToLocal),
-        DataFlow::exprNode(r.getExpr())
-      )
     )
   )
 select r, "May return stack-allocated memory from $@.", va, va.toString()

cpp/ql/src/Likely Bugs/Memory Management/StackAddressEscapes.ql

@@ -9,6 +9,7 @@
  * @id cpp/stack-address-escape
  * @tags reliability
  */
+
 import cpp
 import semmle.code.cpp.dataflow.StackAddress
 
@@ -18,7 +19,7 @@ import semmle.code.cpp.dataflow.StackAddress
  * escape.
  */
 predicate stackAddressEscapes(AssignExpr assignExpr, Expr source, boolean isLocal) {
-  stackPointerFlowsToUse(assignExpr.getRValue(), _, source, isLocal)  and
+  stackPointerFlowsToUse(assignExpr.getRValue(), _, source, isLocal) and
   not stackReferenceFlowsToUse(assignExpr.getLValue(), _, _, _)
 }
 
@@ -26,9 +27,11 @@ from Expr use, Expr source, boolean isLocal, string msg, string srcStr
 where
   stackAddressEscapes(use, source, isLocal) and
   if isLocal = true
-    then (msg = "A stack address ($@) may be assigned to a non-local variable." and
+  then (
-          srcStr = "source")
+    msg = "A stack address ($@) may be assigned to a non-local variable." and
-    else (msg = "A stack address which arrived via a $@ may be assigned to a non-local variable." and
+    srcStr = "source"
-          srcStr = "parameter")
+  ) else (
-select
+    msg = "A stack address which arrived via a $@ may be assigned to a non-local variable." and
-  use, msg, source, srcStr
+    srcStr = "parameter"
+  )
+select use, msg, source, srcStr

cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.ql

@@ -13,16 +13,16 @@
  *       external/cwe/cwe-119
  *       external/cwe/cwe-251
  */
+
 import cpp
 import Buffer
 private import semmle.code.cpp.valuenumbering.GlobalValueNumbering
 
-predicate isSizePlus(Expr e, BufferSizeExpr baseSize, int plus)
+predicate isSizePlus(Expr e, BufferSizeExpr baseSize, int plus) {
-{
+  // baseSize
-  (
+  e = baseSize and plus = 0
-    // baseSize
+  or
-    e = baseSize and plus = 0
+  exists(AddExpr ae, Expr operand1, Expr operand2, int plusSub |
-  ) or exists(AddExpr ae, Expr operand1, Expr operand2, int plusSub |
     // baseSize + n or n + baseSize
     ae = e and
     operand1 = ae.getAnOperand() and
@@ -30,7 +30,9 @@ predicate isSizePlus(Expr e, BufferSizeExpr baseSize, int plus)
     operand1 != operand2 and
     isSizePlus(operand1, baseSize, plusSub) and
     plus = plusSub + operand2.getValue().toInt()
-  ) or exists(SubExpr se, int plusSub |
+  )
+  or
+  exists(SubExpr se, int plusSub |
     // baseSize - n
     se = e and
     isSizePlus(se.getLeftOperand(), baseSize, plusSub) and
@@ -38,45 +40,40 @@ predicate isSizePlus(Expr e, BufferSizeExpr baseSize, int plus)
   )
 }
 
-predicate strncpyFunction(Function f, int argDest, int argSrc, int argLimit)
+predicate strncpyFunction(Function f, int argDest, int argSrc, int argLimit) {
-{
   exists(string name | name = f.getName() |
     (
-      (
+      name = "strcpy_s" or // strcpy_s(dst, max_amount, src)
-        name = "strcpy_s" or    // strcpy_s(dst, max_amount, src)
+      name = "wcscpy_s" or // wcscpy_s(dst, max_amount, src)
-        name = "wcscpy_s" or    // wcscpy_s(dst, max_amount, src)
+      name = "_mbscpy_s" // _mbscpy_s(dst, max_amount, src)
-        name = "_mbscpy_s"      // _mbscpy_s(dst, max_amount, src)
+    ) and
-      ) and
+    argDest = 0 and
-      argDest = 0 and
+    argSrc = 2 and
-      argSrc = 2 and
+    argLimit = 1
-      argLimit = 1
+    or
-    ) or (
+    (
-      (
+      name = "strncpy" or // strncpy(dst, src, max_amount)
-        name = "strncpy" or     // strncpy(dst, src, max_amount)
+      name = "strncpy_l" or // strncpy_l(dst, src, max_amount, locale)
-        name = "strncpy_l" or   // strncpy_l(dst, src, max_amount, locale)
+      name = "wcsncpy" or // wcsncpy(dst, src, max_amount)
-        name = "wcsncpy" or     // wcsncpy(dst, src, max_amount)
+      name = "_wcsncpy_l" or // _wcsncpy_l(dst, src, max_amount, locale)
-        name = "_wcsncpy_l" or  // _wcsncpy_l(dst, src, max_amount, locale)
+      name = "_mbsncpy" or // _mbsncpy(dst, src, max_amount)
-        name = "_mbsncpy" or    // _mbsncpy(dst, src, max_amount)
+      name = "_mbsncpy_l" // _mbsncpy_l(dst, src, max_amount, locale)
-        name = "_mbsncpy_l"     // _mbsncpy_l(dst, src, max_amount, locale)
+    ) and
-      ) and
+    argDest = 0 and
-      argDest = 0 and
+    argSrc = 1 and
-      argSrc = 1 and
+    argLimit = 2
-      argLimit = 2
-    )
   )
 }
 
-string nthString (int num) {
+string nthString(int num) {
-  (
+  num = 0 and
-    num = 0 and
+  result = "first"
-    result = "first"
+  or
-  ) or (
+  num = 1 and
-    num = 1 and
+  result = "second"
-    result = "second"
+  or
-  ) or (
+  num = 2 and
-    num = 2 and 
+  result = "third"
-    result = "third"
-  )
 }
 
 /**
@@ -88,14 +85,16 @@ int arrayExprFixedSize(Expr e) {
   or
   result = e.(NewArrayExpr).getAllocatedType().(ArrayType).getSize()
   or
-  exists (SsaDefinition def, LocalVariable v
+  exists(SsaDefinition def, LocalVariable v |
-  | not (e.getUnspecifiedType() instanceof ArrayType) and
+    not e.getUnspecifiedType() instanceof ArrayType and
     e = def.getAUse(v) and
-    result = arrayExprFixedSize(def.getDefiningValue(v)))
+    result = arrayExprFixedSize(def.getDefiningValue(v))
+  )
 }
 
-from Function f, FunctionCall fc, int argDest, int argSrc, int argLimit, int charSize,
+from
-     Access copyDest, Access copySource, string name, string nth
+  Function f, FunctionCall fc, int argDest, int argSrc, int argLimit, int charSize, Access copyDest,
+  Access copySource, string name, string nth
 where
   f = fc.getTarget() and
   strncpyFunction(f, argDest, argSrc, argLimit) and
@@ -104,22 +103,27 @@ where
   // Some of the functions operate on a larger char type, like `wchar_t`, so we
   // need to take this into account in the fixed size case.
   charSize = f.getParameter(argDest).getUnspecifiedType().(PointerType).getBaseType().getSize() and
-  if exists(fc.getArgument(argLimit).getValue().toInt()) then (
+  (
-    // Fixed sized case
+    if exists(fc.getArgument(argLimit).getValue().toInt())
-    exists(int size |
+    then
-      size = arrayExprFixedSize(copyDest) and
+      // Fixed sized case
-      size < charSize * fc.getArgument(argLimit).getValue().toInt() and
+      exists(int size |
-      size != 0 // if the array has zero size, something special is going on
+        size = arrayExprFixedSize(copyDest) and
-    )
+        size < charSize * fc.getArgument(argLimit).getValue().toInt() and
-  ) else exists (Access takenSizeOf, BufferSizeExpr sizeExpr, int plus |
+        size != 0 // if the array has zero size, something special is going on
-    // Variable sized case
+      )
-    sizeExpr = fc.getArgument(argLimit).getAChild*() and
+    else
-    isSizePlus(fc.getArgument(argLimit), sizeExpr, plus) and
+      exists(Access takenSizeOf, BufferSizeExpr sizeExpr, int plus |
-    plus >= 0 and
+        // Variable sized case
-    takenSizeOf = sizeExpr.getArg() and
+        sizeExpr = fc.getArgument(argLimit).getAChild*() and
-    globalValueNumber(copySource) = globalValueNumber(takenSizeOf) and // e.g. strncpy(x, y, strlen(y))
+        isSizePlus(fc.getArgument(argLimit), sizeExpr, plus) and
-    globalValueNumber(copyDest) != globalValueNumber(takenSizeOf) // e.g. strncpy(y, y, strlen(y))
+        plus >= 0 and
-  )
+        takenSizeOf = sizeExpr.getArg() and
-  and name = fc.getTarget().getName()
+        globalValueNumber(copySource) = globalValueNumber(takenSizeOf) and // e.g. strncpy(x, y, strlen(y))
-  and nth = nthString(argLimit)
+        globalValueNumber(copyDest) != globalValueNumber(takenSizeOf) // e.g. strncpy(y, y, strlen(y))
-select fc, "Potentially unsafe call to " + name + "; " + nth + " argument should be size of destination."
+      )
+  ) and
+  name = fc.getTarget().getName() and
+  nth = nthString(argLimit)
+select fc,
+  "Potentially unsafe call to " + name + "; " + nth + " argument should be size of destination."