C++: Move added flow from simpleLocalFlowStep to simpleInstructionLocalFlowStep and remove flow that could cause field conflation

Author: MathiasVP

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

@@ -385,13 +385,6 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) { simpleLocalFlowStep(nodeFr
  */
 predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
   simpleInstructionLocalFlowStep(nodeFrom.asInstruction(), nodeTo.asInstruction())
-  or
-  exists(LoadInstruction load, ChiInstruction chi |
-    not chi.isResultConflated() and
-    nodeTo.asInstruction() = load and
-    nodeFrom.asInstruction() = chi and
-    load.getSourceValueOperand().getAnyDef() = chi
-  )
 }
 
 private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction iTo) {
@@ -417,13 +410,13 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
   //
   // Flow through the partial operand belongs in the taint-tracking libraries
   // for now.
-  // TODO: To capture flow from a partial definition of an object (i.e., a field write) to the object
-  // we add dataflow through partial chi operands, but only if the chi node is not the chi node for all
-  // aliased memory.
-  iTo.getAnOperand().(ChiPartialOperand).getDef() = iFrom and not iTo.isResultConflated()
-  or
   iTo.getAnOperand().(ChiTotalOperand).getDef() = iFrom
   or
+  exists(ChiInstruction chi | iFrom = chi |
+    not chi.isResultConflated() and
+    iTo.(LoadInstruction).getSourceValueOperand().getAnyDef() = chi
+  )
+  or
   // Flow through modeled functions
   modelFlow(iFrom, iTo)
 }

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

@@ -2,7 +2,6 @@
 | BarrierGuard.cpp:60:11:60:16 | BarrierGuard.cpp:62:14:62:14 | AST only |
 | clang.cpp:12:9:12:20 | clang.cpp:22:8:22:20 | AST only |
 | clang.cpp:28:27:28:32 | clang.cpp:30:27:30:34 | AST only |
-| clang.cpp:28:27:28:32 | clang.cpp:31:27:31:28 | IR only |
 | clang.cpp:39:42:39:47 | clang.cpp:41:18:41:19 | IR only |
 | dispatch.cpp:16:37:16:42 | dispatch.cpp:32:16:32:24 | IR only |
 | dispatch.cpp:16:37:16:42 | dispatch.cpp:40:15:40:23 | IR only |
@@ -46,6 +45,9 @@
 | test.cpp:359:13:359:18 | test.cpp:365:10:365:14 | AST only |
 | test.cpp:373:13:373:18 | test.cpp:369:10:369:14 | AST only |
 | test.cpp:373:13:373:18 | test.cpp:375:10:375:14 | AST only |
+| test.cpp:382:48:382:54 | test.cpp:385:8:385:10 | AST only |
+| test.cpp:388:53:388:59 | test.cpp:392:8:392:10 | AST only |
+| test.cpp:388:53:388:59 | test.cpp:394:10:394:12 | AST only |
 | test.cpp:399:7:399:9 | test.cpp:401:8:401:10 | AST only |
 | test.cpp:405:7:405:9 | test.cpp:408:8:408:10 | AST only |
 | test.cpp:416:7:416:11 | test.cpp:418:8:418:12 | AST only |

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

@@ -12,7 +12,6 @@
 | clang.cpp:18:8:18:19 | (const int *)... | clang.cpp:12:9:12:20 | sourceArray1 |
 | clang.cpp:18:8:18:19 | sourceArray1 | clang.cpp:12:9:12:20 | sourceArray1 |
 | clang.cpp:29:27:29:28 | m1 | clang.cpp:28:27:28:32 | call to source |
-| clang.cpp:31:27:31:28 | m2 | clang.cpp:28:27:28:32 | call to source |
 | clang.cpp:37:10:37:11 | m2 | clang.cpp:34:32:34:37 | call to source |
 | clang.cpp:41:18:41:19 | m2 | clang.cpp:39:42:39:47 | call to source |
 | clang.cpp:45:17:45:18 | m2 | clang.cpp:43:35:43:40 | call to source |
@@ -62,9 +61,6 @@
 | test.cpp:266:12:266:12 | x | test.cpp:265:22:265:27 | call to source |
 | test.cpp:289:14:289:14 | x | test.cpp:305:17:305:22 | call to source |
 | test.cpp:318:7:318:7 | x | test.cpp:314:4:314:9 | call to source |
-| test.cpp:385:8:385:10 | tmp | test.cpp:382:48:382:54 | source1 |
-| test.cpp:392:8:392:10 | tmp | test.cpp:388:53:388:59 | source1 |
-| test.cpp:394:10:394:12 | tmp | test.cpp:388:53:388:59 | source1 |
 | test.cpp:450:9:450:22 | (statement expression) | test.cpp:449:26:449:32 | source1 |
 | test.cpp:461:8:461:12 | local | test.cpp:449:26:449:32 | source1 |
 | true_upon_entry.cpp:13:8:13:8 | x | true_upon_entry.cpp:9:11:9:16 | call to source |

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

@@ -17,15 +17,13 @@
 | taint.cpp:93:11:93:11 | taint.cpp:71:22:71:27 | AST only |
 | taint.cpp:94:11:94:11 | taint.cpp:72:7:72:12 | AST only |
 | taint.cpp:109:7:109:13 | taint.cpp:105:12:105:17 | IR only |
-| taint.cpp:110:7:110:13 | taint.cpp:105:12:105:17 | IR only |
-| taint.cpp:111:7:111:13 | taint.cpp:106:12:106:17 | IR only |
-| taint.cpp:112:7:112:13 | taint.cpp:106:12:106:17 | IR only |
 | taint.cpp:130:7:130:9 | taint.cpp:127:8:127:13 | IR only |
 | taint.cpp:137:7:137:9 | taint.cpp:120:11:120:16 | AST only |
 | taint.cpp:173:8:173:13 | taint.cpp:164:19:164:24 | AST only |
 | taint.cpp:181:8:181:9 | taint.cpp:185:11:185:16 | AST only |
 | taint.cpp:195:7:195:7 | taint.cpp:192:23:192:28 | AST only |
 | taint.cpp:195:7:195:7 | taint.cpp:193:6:193:6 | AST only |
+| taint.cpp:216:7:216:7 | taint.cpp:207:6:207:11 | AST only |
 | taint.cpp:229:3:229:6 | taint.cpp:223:10:223:15 | AST only |
 | taint.cpp:233:8:233:8 | taint.cpp:223:10:223:15 | AST only |
 | taint.cpp:236:3:236:6 | taint.cpp:223:10:223:15 | AST only |

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

@@ -2,9 +2,6 @@
 | taint.cpp:16:8:16:14 | source1 | taint.cpp:12:22:12:27 | call to source |
 | taint.cpp:17:8:17:16 | ++ ... | taint.cpp:12:22:12:27 | call to source |
 | taint.cpp:109:7:109:13 | access to array | taint.cpp:105:12:105:17 | call to source |
-| taint.cpp:110:7:110:13 | access to array | taint.cpp:105:12:105:17 | call to source |
-| taint.cpp:111:7:111:13 | access to array | taint.cpp:106:12:106:17 | call to source |
-| taint.cpp:112:7:112:13 | access to array | taint.cpp:106:12:106:17 | call to source |
 | taint.cpp:129:7:129:9 | * ... | taint.cpp:120:11:120:16 | call to source |
 | taint.cpp:130:7:130:9 | * ... | taint.cpp:127:8:127:13 | call to source |
 | taint.cpp:134:7:134:9 | * ... | taint.cpp:120:11:120:16 | call to source |
@@ -13,7 +10,6 @@
 | taint.cpp:168:8:168:14 | tainted | taint.cpp:164:19:164:24 | call to source |
 | taint.cpp:210:7:210:7 | x | taint.cpp:207:6:207:11 | call to source |
 | taint.cpp:215:7:215:7 | x | taint.cpp:207:6:207:11 | call to source |
-| taint.cpp:216:7:216:7 | y | taint.cpp:207:6:207:11 | call to source |
 | taint.cpp:250:8:250:8 | a | taint.cpp:223:10:223:15 | call to source |
 | taint.cpp:280:7:280:7 | t | taint.cpp:275:6:275:11 | call to source |
 | taint.cpp:289:7:289:7 | t | taint.cpp:275:6:275:11 | call to source |

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

@@ -2,10 +2,10 @@
 | literals.c:3:13:3:16 | 1.0 |
 | literals.c:4:13:4:16 | 1.0 |
 | literals.c:5:13:5:16 | 1.0 |
-| literals.c:6:13:6:16 | (0.0,1.0i) |
-| literals.c:7:13:7:16 | (0.0,1.0i) |
-| literals.c:8:13:8:16 | (0.0,1.0i) |
-| literals.c:9:13:9:16 | (0.0,1.0i) |
+| literals.c:6:13:6:16 | (1.0,1.0i) |
+| literals.c:7:13:7:16 | (1.0,1.0i) |
+| literals.c:8:13:8:16 | (1.0,1.0i) |
+| literals.c:9:13:9:16 | (1.0,1.0i) |
 | literals.c:10:13:10:16 | 1.0 |
 | literals.c:11:13:11:16 | 1.0 |
 | literals.c:12:13:12:16 | 1.0 |