C++: Local field flow using global library

This commit removes fields from the responsibilities of `FlowVar.qll`.
The treatment of fields in that file was slow and imprecise.

It then adds another copy of the shared global data flow library, used
only to find local field flow, and it exposes that local field flow
through `localFlow` and `localFlowStep`.

This has a performance cost. It adds two cached stages to any query that
uses `localFlow`: the stage from `DataFlowImplCommon`, which is shared
with all queries that use global data flow, and a new stage just for
`localFlowStep`.

config/identical-files.json

@@ -9,6 +9,7 @@
     "cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll",
     "cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll",
     "cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll",
+    "cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll",
     "cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll",
     "cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll",
     "cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll",

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

@@ -443,8 +443,14 @@ private module ThisFlow {
  * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local
  * (intra-procedural) step.
  */
+cached
 predicate localFlowStep(Node nodeFrom, Node nodeTo) {
   simpleLocalFlowStep(nodeFrom, nodeTo)
+  or
+  // Field flow is not strictly a "step" but covers the whole function
+  // transitively. There's no way to get a step-like relation out of the global
+  // data flow library, so we just have to accept some big steps here.
+  FieldFlow::fieldFlow(nodeFrom, nodeTo)
 }
 
 /**
@@ -571,6 +577,51 @@ private predicate exprToDefinitionByReferenceStep(Expr exprIn, Expr argOut) {
   )
 }
 
+private module FieldFlow {
+  private import DataFlowImplLocal
+  private import DataFlowPrivate
+
+  /**
+   * A configuration for finding local-only flow through fields. This uses the
+   * `Configuration` class in the dedicated `DataFlowImplLocal` copy of the
+   * shared library that's not user-exposed directly.
+   *
+   * To keep the flow local to a single function, we put barriers on parameters
+   * and return statements. Sources and sinks are the values that go into and
+   * out of fields, respectively.
+   */
+  private class FieldConfiguration extends Configuration {
+    FieldConfiguration() { this = "FieldConfiguration" }
+
+    override predicate isSource(Node source) {
+      storeStep(source, _, _)
+    }
+
+    override predicate isSink(Node sink) {
+      readStep(_, _, sink)
+    }
+
+    override predicate isBarrier(Node node) {
+      node instanceof ParameterNode
+    }
+
+    override predicate isBarrierOut(Node node) {
+      node.asExpr().getParent() instanceof ReturnStmt
+      or
+      node.asExpr().getParent() instanceof ThrowExpr
+    }
+  }
+
+  predicate fieldFlow(Node node1, Node node2) {
+    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()
+  }
+}
+
 VariableAccess getAnAccessToAssignedVariable(Expr assign) {
   (
     assign instanceof Assignment

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

@@ -212,6 +212,7 @@ module FlowVar_internal {
     or
     TBlockVar(SubBasicBlock sbb, Variable v) {
       not fullySupportedSsaVariable(v) and
+      not v instanceof Field and // Fields are interprocedural data flow, not local
       reachable(sbb) and
       (
         initializer(sbb.getANode(), v, _)

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

@@ -137,7 +137,7 @@ void following_pointers(
   sink(sourceStruct1_ptr->m1); // flow
   sink(sourceStruct1_ptr->getFirst()); // flow [NOT DETECTED with IR]
   sink(sourceStruct1_ptr->m2); // no flow
-  sink(sourceStruct1.m1); // flow (due to lack of no-alias tracking)
+  sink(sourceStruct1.m1); // no flow
 
   twoIntFields s = { source(), source() };
 

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

@@ -22,7 +22,6 @@
 | test.cpp:126:8:126:19 | sourceArray1 | test.cpp:120:9:120:20 | sourceArray1 |
 | test.cpp:137:27:137:28 | m1 | test.cpp:136:27:136:32 | call to source |
 | test.cpp:138:27:138:34 | call to getFirst | test.cpp:136:27:136:32 | call to source |
-| test.cpp:140:22:140:23 | m1 | test.cpp:136:27:136:32 | call to source |
 | test.cpp:145:10:145:11 | m2 | test.cpp:142:32:142:37 | call to source |
 | test.cpp:153:17:153:18 | m2 | test.cpp:151:35:151:40 | call to source |
 | test.cpp:188:8:188:8 | y | test.cpp:186:27:186:32 | call to source |

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

@@ -9,7 +9,6 @@
 | test.cpp:109:9:109:14 | test.cpp:110:10:110:12 | IR only |
 | test.cpp:136:27:136:32 | test.cpp:137:27:137:28 | AST only |
 | test.cpp:136:27:136:32 | test.cpp:138:27:138:34 | AST only |
-| test.cpp:136:27:136:32 | test.cpp:140:22:140:23 | AST only |
 | test.cpp:147:42:147:47 | test.cpp:149:18:149:19 | IR only |
 | test.cpp:395:17:395:22 | test.cpp:397:10:397:18 | AST only |
 | test.cpp:407:13:407:18 | test.cpp:413:10:413:14 | AST only |

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

@@ -35,24 +35,24 @@ void assignAfterAlias() {
   S s1 = { 0, 0 };
   S &ref1 = s1;
   ref1.m1 = user_input();
-  sink(s1.m1); // flow
+  sink(s1.m1); // flow [FALSE NEGATIVE]
 
   S s2 = { 0, 0 };
   S &ref2 = s2;
   s2.m1 = user_input();
-  sink(ref2.m1); // flow
+  sink(ref2.m1); // flow [FALSE NEGATIVE]
 }
 
 void assignAfterCopy() {
   S s1 = { 0, 0 };
   S copy1 = s1;
   copy1.m1 = user_input();
-  sink(s1.m1); // no flow [FALSE POSITIVE]
+  sink(s1.m1); // no flow
 
   S s2 = { 0, 0 };
   S copy2 = s2;
   s2.m1 = user_input();
-  sink(copy2.m1); // no flow [FALSE POSITIVE]
+  sink(copy2.m1); // no flow
 }
 
 void assignBeforeCopy() {

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

@@ -98,14 +98,9 @@ edges
 | aliasing.cpp:26:19:26:20 | ref arg s2 [m1, ... (1)] | aliasing.cpp:30:8:30:9 | s2 [m1, ... (1)] |
 | aliasing.cpp:29:8:29:9 | s1 [m1, ... (1)] | aliasing.cpp:29:11:29:12 | m1 |
 | aliasing.cpp:30:8:30:9 | s2 [m1, ... (1)] | aliasing.cpp:30:11:30:12 | m1 |
-| aliasing.cpp:37:13:37:22 | call to user_input [void] | aliasing.cpp:38:11:38:12 | m1 |
-| aliasing.cpp:42:11:42:20 | call to user_input [void] | aliasing.cpp:43:13:43:14 | m1 |
-| aliasing.cpp:49:14:49:23 | call to user_input [void] | aliasing.cpp:50:11:50:12 | m1 |
-| aliasing.cpp:54:11:54:20 | call to user_input [void] | aliasing.cpp:55:14:55:15 | m1 |
 | aliasing.cpp:60:3:60:4 | s2 [post update] [m1, ... (1)] | aliasing.cpp:62:8:62:12 | copy2 [m1, ... (1)] |
 | aliasing.cpp:60:3:60:22 | ... = ... [void] | aliasing.cpp:60:3:60:4 | s2 [post update] [m1, ... (1)] |
 | aliasing.cpp:60:11:60:20 | call to user_input [void] | aliasing.cpp:60:3:60:22 | ... = ... [void] |
-| aliasing.cpp:60:11:60:20 | call to user_input [void] | aliasing.cpp:62:14:62:15 | m1 |
 | aliasing.cpp:62:8:62:12 | copy2 [m1, ... (1)] | aliasing.cpp:62:14:62:15 | m1 |
 | constructors.cpp:26:15:26:15 | f [a_, ... (1)] | constructors.cpp:28:10:28:10 | f [a_, ... (1)] |
 | constructors.cpp:26:15:26:15 | f [b_, ... (1)] | constructors.cpp:29:10:29:10 | f [b_, ... (1)] |
@@ -169,10 +164,6 @@ edges
 | C.cpp:31:10:31:11 | s3 | C.cpp:24:16:24:25 | new [void] | C.cpp:31:10:31:11 | s3 | s3 flows from $@ | C.cpp:24:16:24:25 | new [void] | new [void] |
 | aliasing.cpp:29:11:29:12 | m1 | aliasing.cpp:9:11:9:20 | call to user_input [void] | aliasing.cpp:29:11:29:12 | m1 | m1 flows from $@ | aliasing.cpp:9:11:9:20 | call to user_input [void] | call to user_input [void] |
 | aliasing.cpp:30:11:30:12 | m1 | aliasing.cpp:13:10:13:19 | call to user_input [void] | aliasing.cpp:30:11:30:12 | m1 | m1 flows from $@ | aliasing.cpp:13:10:13:19 | call to user_input [void] | call to user_input [void] |
-| aliasing.cpp:38:11:38:12 | m1 | aliasing.cpp:37:13:37:22 | call to user_input [void] | aliasing.cpp:38:11:38:12 | m1 | m1 flows from $@ | aliasing.cpp:37:13:37:22 | call to user_input [void] | call to user_input [void] |
-| aliasing.cpp:43:13:43:14 | m1 | aliasing.cpp:42:11:42:20 | call to user_input [void] | aliasing.cpp:43:13:43:14 | m1 | m1 flows from $@ | aliasing.cpp:42:11:42:20 | call to user_input [void] | call to user_input [void] |
-| aliasing.cpp:50:11:50:12 | m1 | aliasing.cpp:49:14:49:23 | call to user_input [void] | aliasing.cpp:50:11:50:12 | m1 | m1 flows from $@ | aliasing.cpp:49:14:49:23 | call to user_input [void] | call to user_input [void] |
-| aliasing.cpp:55:14:55:15 | m1 | aliasing.cpp:54:11:54:20 | call to user_input [void] | aliasing.cpp:55:14:55:15 | m1 | m1 flows from $@ | aliasing.cpp:54:11:54:20 | call to user_input [void] | call to user_input [void] |
 | aliasing.cpp:62:14:62:15 | m1 | aliasing.cpp:60:11:60:20 | call to user_input [void] | aliasing.cpp:62:14:62:15 | m1 | m1 flows from $@ | aliasing.cpp:60:11:60:20 | call to user_input [void] | call to user_input [void] |
 | constructors.cpp:28:12:28:12 | call to a | constructors.cpp:34:11:34:20 | call to user_input [void] | constructors.cpp:28:12:28:12 | call to a | call to a flows from $@ | constructors.cpp:34:11:34:20 | call to user_input [void] | call to user_input [void] |
 | constructors.cpp:28:12:28:12 | call to a | constructors.cpp:36:11:36:20 | call to user_input [void] | constructors.cpp:28:12:28:12 | call to a | call to a flows from $@ | constructors.cpp:36:11:36:20 | call to user_input [void] | call to user_input [void] |