Merge remote-tracking branch 'upstream/master' into dataflow-indirect-args

Conflicts:
	cpp/ql/test/library-tests/dataflow/fields/ir-flow.expected

Author: jbj

change-notes/1.25/analysis-javascript.md

@@ -3,6 +3,7 @@
 ## General improvements
 
 * Support for the following frameworks and libraries has been improved:
+  - [fstream](https://www.npmjs.com/package/fstream)
   - [jGrowl](https://github.com/stanlemon/jGrowl)
   - [jQuery](https://jquery.com/)
 
@@ -21,6 +22,7 @@
 | Uncontrolled data used in path expression (`js/path-injection`) | More results | This query now recognizes additional file system calls. |
 | Uncontrolled command line (`js/command-line-injection`) | More results | This query now recognizes additional command execution calls. |
 | Expression has no effect (`js/useless-expression`) | Less results | This query no longer flags an expression when that expression is the only content of the containing file. |
+| Unknown directive (`js/unknown-directive`) | Less results | This query no longer flags directives generated by the Babel compiler. |
 
 ## Changes to libraries
 

config/identical-files.json

@@ -111,12 +111,12 @@
     "csharp/ql/src/semmle/code/csharp/ir/implementation/raw/IR.qll",
     "csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/IR.qll"
   ],
-  "IR IRSanity": [
-    "cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRSanity.qll",
-    "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRSanity.qll",
-    "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRSanity.qll",
-    "csharp/ql/src/semmle/code/csharp/ir/implementation/raw/IRSanity.qll",
-    "csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/IRSanity.qll"
+  "IR IRConsistency": [
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRConsistency.qll",
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRConsistency.qll",
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRConsistency.qll",
+    "csharp/ql/src/semmle/code/csharp/ir/implementation/raw/IRConsistency.qll",
+    "csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/IRConsistency.qll"
   ],
   "IR PrintIR": [
     "cpp/ql/src/semmle/code/cpp/ir/implementation/raw/PrintIR.qll",
@@ -157,10 +157,10 @@
     "cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll",
     "csharp/ql/src/semmle/code/csharp/ir/implementation/Opcode.qll"
   ],
-  "IR SSASanity": [
-    "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.qll",
-    "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.qll",
-    "csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSASanity.qll"
+  "IR SSAConsistency": [
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConsistency.qll",
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConsistency.qll",
+    "csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSAConsistency.qll"
   ],
   "C++ IR InstructionImports": [
     "cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/InstructionImports.qll",

cpp/ql/src/Documentation/CaptionedComments.qll

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

cpp/ql/src/Documentation/CommentedOutCode.qll

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

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

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

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

@@ -4,22 +4,28 @@
  * passed to a function, or similar.
  */
 
+/*
+ * Maintainer note: this file is one of several files that are similar but not
+ * identical. Many changes to this file will also apply to the others:
+ * - AddressConstantExpression.qll
+ * - AddressFlow.qll
+ * - EscapesTree.qll
+ */
+
 private import cpp
 
 /**
  * Holds if `f` is an instantiation of the `std::move` or `std::forward`
  * template functions, these functions are essentially casts, so we treat them
  * as such.
  */
-private predicate stdIdentityFunction(Function f) {
-  f.getNamespace().getParentNamespace() instanceof GlobalNamespace and
-  f.getNamespace().getName() = "std" and
-  (
-    f.getName() = "move"
-    or
-    f.getName() = "forward"
-  )
-}
+private predicate stdIdentityFunction(Function f) { f.hasQualifiedName("std", ["move", "forward"]) }
+
+/**
+ * Holds if `f` is an instantiation of `std::addressof`, which effectively
+ * converts a reference to a pointer.
+ */
+private predicate stdAddressOf(Function f) { f.hasQualifiedName("std", "addressof") }
 
 private predicate lvalueToLvalueStepPure(Expr lvalueIn, Expr lvalueOut) {
   lvalueIn = lvalueOut.(DotFieldAccess).getQualifier().getFullyConverted()
@@ -91,12 +97,17 @@ private predicate lvalueToReferenceStep(Expr lvalueIn, Expr referenceOut) {
 }
 
 private predicate referenceToLvalueStep(Expr referenceIn, Expr lvalueOut) {
-  // This probably cannot happen. It would require an expression to be
-  // converted to a reference and back again without an intermediate variable
-  // assignment.
   referenceIn.getConversion() = lvalueOut.(ReferenceDereferenceExpr)
 }
 
+private predicate referenceToPointerStep(Expr referenceIn, Expr pointerOut) {
+  pointerOut =
+    any(FunctionCall call |
+      stdAddressOf(call.getTarget()) and
+      referenceIn = call.getArgument(0).getFullyConverted()
+    )
+}
+
 private predicate referenceToReferenceStep(Expr referenceIn, Expr referenceOut) {
   referenceOut =
     any(FunctionCall call |
@@ -145,6 +156,12 @@ private predicate pointerFromVariableAccess(VariableAccess va, Expr pointer) {
     pointerToPointerStep(prev, pointer)
   )
   or
+  // reference -> pointer
+  exists(Expr prev |
+    referenceFromVariableAccess(va, prev) and
+    referenceToPointerStep(prev, pointer)
+  )
+  or
   // lvalue -> pointer
   exists(Expr prev |
     lvalueFromVariableAccess(va, prev) and
@@ -169,7 +186,8 @@ private predicate referenceFromVariableAccess(VariableAccess va, Expr reference)
 private predicate addressMayEscapeAt(Expr e) {
   exists(Call call |
     e = call.getAnArgument().getFullyConverted() and
-    not stdIdentityFunction(call.getTarget())
+    not stdIdentityFunction(call.getTarget()) and
+    not stdAddressOf(call.getTarget())
     or
     e = call.getQualifier().getFullyConverted() and
     e.getUnderlyingType() instanceof PointerType