Merge pull request #115 from github/aibaars/dataflow

My first dataflow step

ql/src/codeql_ruby/controlflow/CfgNodes.qll

@@ -193,6 +193,11 @@ module ExprNodes {
     final ExprCfgNode getRhs() { e.hasCfgChild(e.getRhs(), this, result) }
   }
 
+  /** A control-flow node that wraps an `AssignExpr` AST expression. */
+  class AssignExprCfgNode extends AssignmentCfgNode {
+    AssignExprCfgNode() { this.getExpr() instanceof AssignExpr }
+  }
+
   private class BinaryOperationExprChildMapping extends ExprChildMapping, BinaryOperation {
     override predicate relevantChild(Expr e) { e = this.getAnOperand() }
   }
@@ -244,6 +249,11 @@ module ExprNodes {
     final ExprCfgNode getExpr(int n) { e.hasCfgChild(e.getExpr(n), this, result) }
   }
 
+  /** A control-flow node that wraps an `ExprSequence` AST expression. */
+  class ParenthesizedExprCfgNode extends ExprSequenceCfgNode {
+    ParenthesizedExprCfgNode() { this.getExpr() instanceof ParenthesizedExpr }
+  }
+
   /** A control-flow node that wraps a `VariableReadAccess` AST expression. */
   class VariableReadAccessCfgNode extends ExprCfgNode {
     override VariableReadAccess e;

ql/src/codeql_ruby/dataflow/internal/DataFlowDispatch.qll

@@ -39,8 +39,8 @@ DataFlowCallable viableCallable(DataFlowCall call) { none() }
 /**
  * Holds if the set of viable implementations that can be called by `call`
  * might be improved by knowing the call context. This is the case if the
- * call is a delegate call, or if the qualifier accesses a parameter of
- * the enclosing callable `c` (including the implicit `this` parameter).
+ * qualifier accesses a parameter of the enclosing callable `c` (including
+ * the implicit `self` parameter).
  */
 predicate mayBenefitFromCallContext(DataFlowCall call, Callable c) { none() }
 

ql/src/codeql_ruby/dataflow/internal/DataFlowPrivate.qll

@@ -123,7 +123,10 @@ private module Cached {
   predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
     exists(Ssa::Definition def | LocalFlow::localSsaFlowStep(def, nodeFrom, nodeTo))
     or
-    nodeFrom.asExpr() = nodeTo.asExpr().(CfgNodes::ExprNodes::AssignmentCfgNode).getRhs()
+    nodeFrom.asExpr() = nodeTo.asExpr().(CfgNodes::ExprNodes::AssignExprCfgNode).getRhs()
+    or
+    nodeFrom.asExpr() =
+      nodeTo.asExpr().(CfgNodes::ExprNodes::ParenthesizedExprCfgNode).getLastExpr()
   }
 
   cached
@@ -217,7 +220,7 @@ abstract class ReturnNode extends Node {
 private module ReturnNodes {
   /**
    * A data-flow node that represents an expression returned by a callable,
-   * either using a (`yield`) `return` statement or an expression body (`=>`).
+   * either using an explict `return` statement or as the expression of a method body.
    */
   class ExprReturnNode extends ReturnNode, ExprNode {
     ExprReturnNode() {
@@ -240,7 +243,7 @@ abstract class OutNode extends Node {
 private module OutNodes {
   /**
    * A data-flow node that reads a value returned directly by a callable,
-   * either via a C# call or a CIL call.
+   * either via a call or a `yield` of a block.
    */
   class ExprOutNode extends OutNode, ExprNode {
     private DataFlowCall call;
@@ -297,8 +300,7 @@ predicate compatibleTypes(DataFlowType t1, DataFlowType t2) { any() }
  * an update to the field.
  *
  * Nodes corresponding to AST elements, for example `ExprNode`, usually refer
- * to the value before the update with the exception of `ObjectCreation`,
- * which represents the value after the constructor has run.
+ * to the value before the update.
  */
 abstract class PostUpdateNode extends Node {
   /** Gets the node before the state update. */
@@ -355,7 +357,10 @@ predicate isImmutableOrUnobservable(Node n) { none() }
  */
 predicate isUnreachableInCall(Node n, DataFlowCall call) { none() }
 
-class BarrierGuard extends AstNode {
+/**
+ * A guard that validates some expression.
+ */
+class BarrierGuard extends Expr {
   BarrierGuard() { none() }
 
   Node getAGuardedNode() { none() }

ql/test/library-tests/dataflow/local/DataflowStep.expected

@@ -0,0 +1,19 @@
+| local_dataflow.rb:1:9:1:9 | a | local_dataflow.rb:2:7:2:7 | a |
+| local_dataflow.rb:2:3:2:7 | ... = ... | local_dataflow.rb:3:13:3:13 | b |
+| local_dataflow.rb:2:7:2:7 | a | local_dataflow.rb:2:3:2:7 | ... = ... |
+| local_dataflow.rb:2:7:2:7 | a | local_dataflow.rb:2:3:2:7 | ... = ... |
+| local_dataflow.rb:2:7:2:7 | a | local_dataflow.rb:3:10:3:10 | a |
+| local_dataflow.rb:3:7:3:14 | (...; ...) | local_dataflow.rb:3:3:3:14 | ... = ... |
+| local_dataflow.rb:3:10:3:10 | [post] a | local_dataflow.rb:4:11:4:11 | a |
+| local_dataflow.rb:3:10:3:10 | a | local_dataflow.rb:4:11:4:11 | a |
+| local_dataflow.rb:3:13:3:13 | b | local_dataflow.rb:3:7:3:14 | (...; ...) |
+| local_dataflow.rb:3:13:3:13 | b | local_dataflow.rb:6:13:6:13 | b |
+| local_dataflow.rb:4:7:4:11 | ... = ... | local_dataflow.rb:4:3:4:11 | ... = ... |
+| local_dataflow.rb:4:11:4:11 | a | local_dataflow.rb:4:7:4:11 | ... = ... |
+| local_dataflow.rb:4:11:4:11 | a | local_dataflow.rb:5:12:5:12 | a |
+| local_dataflow.rb:5:7:5:13 | (... = ...) | local_dataflow.rb:5:3:5:13 | ... = ... |
+| local_dataflow.rb:5:8:5:12 | ... = ... | local_dataflow.rb:5:7:5:13 | (... = ...) |
+| local_dataflow.rb:5:12:5:12 | a | local_dataflow.rb:5:8:5:12 | ... = ... |
+| local_dataflow.rb:5:12:5:12 | a | local_dataflow.rb:6:8:6:8 | a |
+| local_dataflow.rb:6:7:6:14 | (... += ...) | local_dataflow.rb:6:3:6:14 | ... = ... |
+| local_dataflow.rb:6:8:6:13 | ... += ... | local_dataflow.rb:6:7:6:14 | (... += ...) |

ql/test/library-tests/dataflow/local/DataflowStep.ql

@@ -0,0 +1,6 @@
+import ruby
+import codeql_ruby.DataFlow
+
+from DataFlow::Node pred, DataFlow::Node succ
+where DataFlow::localFlowStep(pred, succ)
+select pred, succ

ql/test/library-tests/dataflow/local/local_dataflow.rb

@@ -0,0 +1,7 @@
+def foo(a)
+  b = a
+  c = (p a; b)
+  d = c = a
+  d = (c = a)
+  e = (a += b)
+end