Merge pull request github#5196 from MathiasVP/fix-dataflow-regression-const-member-function

C++: Fix missing dataflow "out of" const member functions

Author: geoffw0

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

@@ -131,7 +131,22 @@ private predicate lvalueToUpdate(Expr lvalue, Expr outer, ControlFlowNode node)
     exists(Call call | node = call |
       outer = call.getQualifier().getFullyConverted() and
       outer.getUnspecifiedType() instanceof Class and
-      not call.getTarget().hasSpecifier("const")
+      not (
+        call.getTarget().hasSpecifier("const") and
+        // Given the following program:
+        // ```
+        // struct C {
+        //   void* data_;
+        //   void* data() const { return data; }
+        // };
+        // C c;
+        // memcpy(c.data(), source, 16)
+        // ```
+        // the data pointed to by `c.data_` is potentially modified by the call to `memcpy` even though
+        // `C::data` has a const specifier. So we further place the restriction that the type returned
+        // by `call` should not be of the form `const T*` (for some deeply const type `T`).
+        call.getType().isDeeplyConstBelow()
+      )
     )
     or
     assignmentTo(outer, node)
@@ -170,7 +185,11 @@ private predicate pointerToUpdate(Expr pointer, Expr outer, ControlFlowNode node
       or
       outer = call.getQualifier().getFullyConverted() and
       outer.getUnspecifiedType() instanceof PointerType and
-      not call.getTarget().hasSpecifier("const")
+      not (
+        call.getTarget().hasSpecifier("const") and
+        // See the `lvalueToUpdate` case for an explanation of this conjunct.
+        call.getType().isDeeplyConstBelow()
+      )
     )
     or
     exists(PointerFieldAccess fa |

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

@@ -36,9 +36,6 @@ argHasPostUpdate
 | arrays.cpp:10:8:10:15 | * ... | ArgumentNode is missing PostUpdateNode. |
 | arrays.cpp:16:8:16:13 | access to array | ArgumentNode is missing PostUpdateNode. |
 | arrays.cpp:17:8:17:13 | access to array | ArgumentNode is missing PostUpdateNode. |
-| by_reference.cpp:51:8:51:8 | s | ArgumentNode is missing PostUpdateNode. |
-| by_reference.cpp:57:8:57:8 | s | ArgumentNode is missing PostUpdateNode. |
-| by_reference.cpp:63:8:63:8 | s | ArgumentNode is missing PostUpdateNode. |
 postWithInFlow
 | A.cpp:25:13:25:13 | c [post update] | PostUpdateNode should not be the target of local flow. |
 | A.cpp:27:28:27:28 | c [post update] | PostUpdateNode should not be the target of local flow. |

cpp/ql/test/library-tests/dataflow/fields/partial-definition-diff.expected

@@ -227,14 +227,18 @@
 | by_reference.cpp:20:23:20:27 | value | AST only |
 | by_reference.cpp:24:19:24:22 | this | AST only |
 | by_reference.cpp:24:25:24:29 | value | AST only |
+| by_reference.cpp:40:12:40:15 | this | AST only |
 | by_reference.cpp:50:3:50:3 | s | AST only |
 | by_reference.cpp:50:17:50:26 | call to user_input | AST only |
+| by_reference.cpp:51:8:51:8 | s | AST only |
 | by_reference.cpp:51:10:51:20 | call to getDirectly | AST only |
 | by_reference.cpp:56:3:56:3 | s | AST only |
 | by_reference.cpp:56:19:56:28 | call to user_input | AST only |
+| by_reference.cpp:57:8:57:8 | s | AST only |
 | by_reference.cpp:57:10:57:22 | call to getIndirectly | AST only |
 | by_reference.cpp:62:3:62:3 | s | AST only |
 | by_reference.cpp:62:25:62:34 | call to user_input | AST only |
+| by_reference.cpp:63:8:63:8 | s | AST only |
 | by_reference.cpp:63:10:63:28 | call to getThroughNonMember | AST only |
 | by_reference.cpp:68:17:68:18 | & ... | AST only |
 | by_reference.cpp:68:21:68:30 | call to user_input | AST only |

cpp/ql/test/library-tests/dataflow/fields/partial-definition.expected

@@ -266,14 +266,18 @@
 | by_reference.cpp:20:23:20:27 | value |
 | by_reference.cpp:24:19:24:22 | this |
 | by_reference.cpp:24:25:24:29 | value |
+| by_reference.cpp:40:12:40:15 | this |
 | by_reference.cpp:50:3:50:3 | s |
 | by_reference.cpp:50:17:50:26 | call to user_input |
+| by_reference.cpp:51:8:51:8 | s |
 | by_reference.cpp:51:10:51:20 | call to getDirectly |
 | by_reference.cpp:56:3:56:3 | s |
 | by_reference.cpp:56:19:56:28 | call to user_input |
+| by_reference.cpp:57:8:57:8 | s |
 | by_reference.cpp:57:10:57:22 | call to getIndirectly |
 | by_reference.cpp:62:3:62:3 | s |
 | by_reference.cpp:62:25:62:34 | call to user_input |
+| by_reference.cpp:63:8:63:8 | s |
 | by_reference.cpp:63:10:63:28 | call to getThroughNonMember |
 | by_reference.cpp:68:17:68:18 | & ... |
 | by_reference.cpp:68:21:68:30 | call to user_input |