Merge pull request #774 from jbj/dataflow-asExpr-conversion

Approved by dave-bartolomeo

Author: semmle-qlci

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

@@ -24,8 +24,22 @@ class Node extends Instruction {
     result = this.getResultType()
   }
 
-  /** Gets the expression corresponding to this node, if any. */
-  Expr asExpr() { result = this.getConvertedResultExpression() }
+  /**
+   * Gets the non-conversion expression corresponding to this node, if any. If
+   * this node strictly (in the sense of `asConvertedExpr`) corresponds to a
+   * `Conversion`, then the result is that `Conversion`'s non-`Conversion` base
+   * expression.
+   */
+  Expr asExpr() {
+    result.getConversion*() = this.getConvertedResultExpression() and
+    not result instanceof Conversion
+  }
+
+  /**
+   * Gets the expression corresponding to this node, if any. The returned
+   * expression may be a `Conversion`.
+   */
+  Expr asConvertedExpr() { result = this.getConvertedResultExpression() }
 
   /** Gets the parameter corresponding to this node, if any. */
   Parameter asParameter() { result = this.(InitializeParameterInstruction).getParameter() }
@@ -48,10 +62,21 @@ class Node extends Instruction {
  * An expression, viewed as a node in a data flow graph.
  */
 class ExprNode extends Node {
-  Expr expr;
+  ExprNode() { exists(this.asExpr()) }
+
+  /**
+   * Gets the non-conversion expression corresponding to this node, if any. If
+   * this node strictly (in the sense of `getConvertedExpr`) corresponds to a
+   * `Conversion`, then the result is that `Conversion`'s non-`Conversion` base
+   * expression.
+   */
+  Expr getExpr() { result = this.asExpr() }
 
-  ExprNode() { expr = this.asExpr() }
-  Expr getExpr() { result = expr }
+  /**
+   * Gets the expression corresponding to this node, if any. The returned
+   * expression may be a `Conversion`.
+   */
+  Expr getConvertedExpr() { result = this.asConvertedExpr() }
 }
 
 /**
@@ -98,10 +123,17 @@ abstract class PostUpdateNode extends Node {
 }
 
 /**
- * Gets the `Node` corresponding to `e`.
+ * Gets a `Node` corresponding to `e` or any of its conversions. There is no
+ * result if `e` is a `Conversion`.
  */
 ExprNode exprNode(Expr e) { result.getExpr() = e }
 
+/**
+ * Gets the `Node` corresponding to `e`, if any. Here, `e` may be a
+ * `Conversion`.
+ */
+ExprNode convertedExprNode(Expr e) { result.getExpr() = e }
+
 /**
  * Gets the `Node` corresponding to the value of `p` at function entry.
  */

cpp/ql/test/library-tests/dataflow/dataflow-tests/IRDataflowTestCommon.qll

@@ -19,7 +19,7 @@ class TestAllocationConfig extends DataFlow::Configuration {
   override predicate isSink(DataFlow::Node sink) {
     exists(FunctionCall call |
       call.getTarget().getName() = "sink" and
-      sink.asExpr() = call.getAnArgument().getFullyConverted()
+      sink.asExpr() = call.getAnArgument()
     )
   }
 

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

@@ -1,5 +1,5 @@
 int source();
-void sink(...);
+void sink(int); void sink(const int *); void sink(int **);
 
 void intraprocedural_with_local_flow() {
   int t2;

cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected

@@ -1,5 +1,5 @@
 | test.cpp:66:30:66:36 | test.cpp:71:8:71:9 | AST only |
-| test.cpp:89:28:89:34 | test.cpp:90:8:90:14 | AST only |
+| test.cpp:89:28:89:34 | test.cpp:92:8:92:14 | IR only |
 | test.cpp:100:13:100:18 | test.cpp:103:10:103:12 | AST only |
 | test.cpp:120:9:120:20 | test.cpp:126:8:126:19 | AST only |
 | test.cpp:122:18:122:30 | test.cpp:132:22:132:23 | IR only |

cpp/ql/test/library-tests/dataflow/dataflow-tests/test_ir.expected

@@ -9,6 +9,8 @@
 | test.cpp:76:8:76:9 | Load: u1 | test.cpp:75:7:75:8 | Uninitialized: definition of u1 |
 | test.cpp:84:8:84:18 | Load: ... ? ... : ... | test.cpp:83:7:83:8 | Uninitialized: definition of u2 |
 | test.cpp:86:8:86:9 | Load: i1 | test.cpp:83:7:83:8 | Uninitialized: definition of u2 |
+| test.cpp:90:8:90:14 | Load: source1 | test.cpp:89:28:89:34 | InitializeParameter: source1 |
+| test.cpp:92:8:92:14 | Load: source1 | test.cpp:89:28:89:34 | InitializeParameter: source1 |
 | test.cpp:132:22:132:23 | Load: m1 | test.cpp:122:18:122:30 | InitializeParameter: sourceStruct1 |
 | test.cpp:140:22:140:23 | Load: m1 | test.cpp:122:18:122:30 | InitializeParameter: sourceStruct1 |
 | test.cpp:188:8:188:8 | Load: y | test.cpp:186:27:186:32 | Call: call to source |