Merge commit '52d8acc1a198c5ea29c1dddceda1d6c0fb75de14' into dataflow-defbyref-to-field

This is a partial merge from master. In particular, it takes in github#3382
and github#3385.

Author: jbj

CODEOWNERS

@@ -1,8 +1,8 @@
-/cpp/ @Semmle/cpp-analysis
-/csharp/ @Semmle/cs
-/java/ @Semmle/java
-/javascript/ @Semmle/js
-/python/ @Semmle/python
+/cpp/ @github/codeql-c-analysis
+/csharp/ @github/codeql-csharp
+/java/ @github/codeql-java
+/javascript/ @github/codeql-javascript
+/python/ @github/codeql-python
 /cpp/**/*.qhelp @hubwriter
 /csharp/**/*.qhelp @jf205
 /java/**/*.qhelp @felicitymay

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

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

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

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

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

@@ -24,7 +24,7 @@ private predicate isCharSzPtrExpr(Expr e) {
 from Expr sizeofExpr, Expr e
 where
   // If we see an addWithSizeof then we expect the type of
-  // the pointer expression to be char* or void*. Otherwise it
+  // the pointer expression to be `char*` or `void*`. Otherwise it
   // is probably a mistake.
   addWithSizeof(e, sizeofExpr, _) and not isCharSzPtrExpr(e)
 select sizeofExpr,

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

@@ -103,6 +103,9 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
 
   /**
    * Holds if this function is declared to be `constexpr`.
+   *
+   * Note that this does not hold if the function has been declared
+   * `consteval`.
    */
   predicate isDeclaredConstexpr() { this.hasSpecifier("declared_constexpr") }
 
@@ -115,9 +118,16 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
    * template <typename T> constexpr int g(T x) { return f(x); }
    * ```
    * `g<int>` is declared constexpr, but is not constexpr.
+   *
+   * Will also hold if this function is `consteval`.
    */
   predicate isConstexpr() { this.hasSpecifier("is_constexpr") }
 
+  /**
+   * Holds if this function is declared to be `consteval`.
+   */
+  predicate isConsteval() { this.hasSpecifier("is_consteval") }
+
   /**
    * Holds if this function is declared with `__attribute__((naked))` or
    * `__declspec(naked)`.

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

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

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

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

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

@@ -183,11 +183,14 @@ private predicate referenceFromVariableAccess(VariableAccess va, Expr reference)
   )
 }
 
-private predicate valueMayEscapeAt(Expr e) {
+private predicate addressMayEscapeAt(Expr e) {
   exists(Call call |
     e = call.getAnArgument().getFullyConverted() and
     not stdIdentityFunction(call.getTarget()) and
     not stdAddressOf(call.getTarget())
+    or
+    e = call.getQualifier().getFullyConverted() and
+    e.getUnderlyingType() instanceof PointerType
   )
   or
   exists(AssignExpr assign | e = assign.getRValue().getFullyConverted())
@@ -205,8 +208,8 @@ private predicate valueMayEscapeAt(Expr e) {
   exists(AsmStmt asm | e = asm.getAChild().(Expr).getFullyConverted())
 }
 
-private predicate valueMayEscapeMutablyAt(Expr e) {
-  valueMayEscapeAt(e) and
+private predicate addressMayEscapeMutablyAt(Expr e) {
+  addressMayEscapeAt(e) and
   exists(Type t | t = e.getType().getUnderlyingType() |
     exists(PointerType pt |
       pt = t
@@ -225,6 +228,22 @@ private predicate valueMayEscapeMutablyAt(Expr e) {
   )
 }
 
+private predicate lvalueMayEscapeAt(Expr e) {
+  // A call qualifier, like `q` in `q.f()`, is special in that the address of
+  // `q` escapes even though `q` is not a pointer or a reference.
+  exists(Call call |
+    e = call.getQualifier().getFullyConverted() and
+    e.getType().getUnspecifiedType() instanceof Class
+  )
+}
+
+private predicate lvalueMayEscapeMutablyAt(Expr e) {
+  lvalueMayEscapeAt(e) and
+  // A qualifier of a call to a const member function is converted to a const
+  // class type.
+  not e.getType().isConst()
+}
+
 private predicate addressFromVariableAccess(VariableAccess va, Expr e) {
   pointerFromVariableAccess(va, e)
   or
@@ -271,8 +290,11 @@ private module EscapesTree_Cached {
    */
   cached
   predicate variableAddressEscapesTree(VariableAccess va, Expr e) {
-    valueMayEscapeAt(e) and
+    addressMayEscapeAt(e) and
     addressFromVariableAccess(va, e)
+    or
+    lvalueMayEscapeAt(e) and
+    lvalueFromVariableAccess(va, e)
   }
 
   /**
@@ -301,8 +323,11 @@ private module EscapesTree_Cached {
    */
   cached
   predicate variableAddressEscapesTreeNonConst(VariableAccess va, Expr e) {
-    valueMayEscapeMutablyAt(e) and
+    addressMayEscapeMutablyAt(e) and
     addressFromVariableAccess(va, e)
+    or
+    lvalueMayEscapeMutablyAt(e) and
+    lvalueFromVariableAccess(va, e)
   }
 
   /**

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

@@ -505,8 +505,6 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
   // Expr -> Expr
   exprToExprStep_nocfg(nodeFrom.asExpr(), nodeTo.asExpr())
   or
-  exprToExprStep_nocfg(nodeFrom.(PostUpdateNode).getPreUpdateNode().asExpr(), nodeTo.asExpr())
-  or
   // Node -> FlowVar -> VariableAccess
   exists(FlowVar var |
     (
@@ -674,7 +672,7 @@ private module FieldFlow {
     exists(FieldConfiguration cfg | cfg.hasFlow(node1, node2)) and
     // This configuration should not be able to cross function boundaries, but
     // we double-check here just to be sure.
-    node1.getFunction() = node2.getFunction()
+    node1.getEnclosingCallable() = node2.getEnclosingCallable()
   }
 }
 

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

@@ -9,9 +9,8 @@ private import semmle.code.cpp.dataflow.EscapesTree
  */
 abstract class Call extends Expr, NameQualifiableElement {
   /**
-   * Gets the number of actual parameters in this call; use
-   * `getArgument(i)` with `i` between `0` and `result - 1` to
-   * retrieve actuals.
+   * Gets the number of arguments (actual parameters) of this call. The count
+   * does _not_ include the qualifier of the call, if any.
    */
   int getNumberOfArguments() { result = count(this.getAnArgument()) }
 
@@ -32,21 +31,24 @@ abstract class Call extends Expr, NameQualifiableElement {
   Expr getQualifier() { result = this.getChild(-1) }
 
   /**
-   * Gets an argument for this call.
+   * Gets an argument for this call. To get the qualifier of this call, if
+   * any, use `getQualifier()`.
    */
   Expr getAnArgument() { exists(int i | result = this.getChild(i) and i >= 0) }
 
   /**
    * Gets the nth argument for this call.
    *
-   * The range of `n` is from `0` to `getNumberOfArguments() - 1`.
+   * The range of `n` is from `0` to `getNumberOfArguments() - 1`. To get the
+   * qualifier of this call, if any, use `getQualifier()`.
    */
   Expr getArgument(int n) { result = this.getChild(n) and n >= 0 }
 
   /**
-   * Gets a sub expression of the argument at position `index`. If the
+   * Gets a subexpression of the argument at position `index`. If the
    * argument itself contains calls, such calls will be considered
-   * leafs in the expression tree.
+   * leaves in the expression tree. The qualifier of the call, if any, is not
+   * considered to be an argument.
    *
    * Example: the call `f(2, 3 + 4, g(4 + 5))` has sub expression(s)
    * `2` at index 0; `3`, `4`, and `3 + 4` at index 1; and `g(4 + 5)`