Go: implement conservative cross-thread dataflow

Steps into captured variables are moved into jumpStep where they always should have been, and the store/load step implementation for channels is completed.

For the time being this takes a very conservative approach to identify channels that are likely connected: if there is exactly one receive site and one send site for a field, the two are presumed connected.

Author: smowton

go/ql/lib/semmle/go/controlflow/ControlFlowGraph.qll

@@ -166,11 +166,21 @@ module ControlFlow {
       )
     }
 
+    /**
+     * Holds if this node writes `rhs` to `channel`.
+     */
+    predicate writesToChannel(DataFlow::Node channel, DataFlow::Node rhs) {
+      exists(SendStmt send |
+        send.getChannel() = channel.(DataFlow::ExprNode).asExpr() and
+        send.getValue() = rhs.(DataFlow::ExprNode).asExpr()
+      )
+    }
+
     /**
      * Holds if this node sets any field or element of `base` to `rhs`.
      */
     predicate writesComponent(DataFlow::Node base, DataFlow::Node rhs) {
-      writesElement(base, _, rhs) or writesField(base, _, rhs)
+      writesElement(base, _, rhs) or writesField(base, _, rhs) or writesToChannel(base, rhs)
     }
   }
 

go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll

@@ -24,9 +24,7 @@ predicate containerStoreStep(Node node1, Node node2, Content c) {
   )
   or
   c instanceof CollectionContent and
-  exists(SendStmt send |
-    send.getChannel() = node2.(ExprNode).asExpr() and send.getValue() = node1.(ExprNode).asExpr()
-  )
+  exists(Write w | w.writesToChannel(node2, node1))
   or
   c instanceof MapKeyContent and
   node2.getType() instanceof MapType and

go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll

@@ -634,9 +634,7 @@ module Public {
     predicate isReceiverOf(MethodDecl m) { parm.isReceiverOf(m) }
   }
 
-  private Node getADirectlyWrittenNode() {
-    exists(Write w | w.writesField(result, _, _) or w.writesElement(result, _, _))
-  }
+  private Node getADirectlyWrittenNode() { exists(Write w | w.writesComponent(result, _)) }
 
   private DataFlow::Node getAccessPathPredecessor(DataFlow::Node node) {
     result = node.(PointerDereferenceNode).getOperand()

go/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll

@@ -70,10 +70,7 @@ predicate basicLocalFlowStep(Node nodeFrom, Node nodeTo) {
   )
   or
   // SSA -> SSA
-  exists(SsaDefinition pred, SsaDefinition succ |
-    succ.(SsaVariableCapture).getSourceVariable() = pred.(SsaExplicitDefinition).getSourceVariable() or
-    succ.(SsaPseudoDefinition).getAnInput() = pred
-  |
+  exists(SsaDefinition pred, SsaPseudoDefinition succ | succ.getAnInput() = pred |
     nodeFrom = ssaNode(pred) and
     nodeTo = ssaNode(succ)
   )
@@ -90,6 +87,12 @@ predicate basicLocalFlowStep(Node nodeFrom, Node nodeTo) {
     any(GlobalFunctionNode fn | fn.getFunction() = nodeTo.asExpr().(FunctionName).getTarget())
 }
 
+pragma[noinline]
+private Field getASparselyUsedChannelTypedField() {
+  result.getType() instanceof ChanType and
+  count(result.getARead()) = 2
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` in a way that loses the
  * calling context. For example, this would happen with flow through a
@@ -102,6 +105,27 @@ predicate jumpStep(Node n1, Node n2) {
     w.writes(v, n1) and
     n2 = v.getARead()
   )
+  or
+  exists(SsaDefinition pred, SsaDefinition succ |
+    succ.(SsaVariableCapture).getSourceVariable() = pred.(SsaExplicitDefinition).getSourceVariable()
+  |
+    n1 = ssaNode(pred) and
+    n2 = ssaNode(succ)
+  )
+  or
+  // If a channel-typed field is referenced exactly once in the context of
+  // a send statement and once in a receive expression, assume the two are linked.
+  exists(
+    Field f, DataFlow::ReadNode recvRead, DataFlow::ReadNode sendRead, RecvExpr re, SendStmt ss
+  |
+    f = getASparselyUsedChannelTypedField() and
+    recvRead = f.getARead() and
+    sendRead = f.getARead() and
+    recvRead.asExpr() = re.getOperand() and
+    sendRead.asExpr() = ss.getChannel() and
+    n1.(DataFlow::PostUpdateNode).getPreUpdateNode() = sendRead and
+    n2 = recvRead
+  )
 }
 
 /**