Merge pull request #16805 from MathiasVP/tc-in-temp-materialization

C++: Fix missing `asExpr` for temporary materializations with conversions

Author: MathiasVP

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll

@@ -193,6 +193,46 @@ private module Cached {
     )
   }
 
+  /** Holds if `operand`'s definition is a `VariableAddressInstruction` whose variable is a temporary */
+  private predicate isIRTempVariable(Operand operand) {
+    operand.getDef().(VariableAddressInstruction).getIRVariable() instanceof IRTempVariable
+  }
+
+  /**
+   * Holds if `node` is an indirect operand whose operand is an argument, and
+   * the `n`'th expression associated with the operand is `e`.
+   */
+  private predicate isIndirectOperandOfArgument(
+    IndirectOperand node, ArgumentOperand operand, Expr e, int n
+  ) {
+    node.hasOperandAndIndirectionIndex(operand, 1) and
+    e = getConvertedResultExpression(operand.getDef(), n)
+  }
+
+  /**
+   * Holds if `opFrom` is an operand to a conversion, and `opTo` is the unique
+   * use of the conversion.
+   */
+  private predicate isConversionStep(Operand opFrom, Operand opTo) {
+    exists(Instruction mid |
+      conversionFlow(opFrom, mid, false, false) and
+      opTo = unique( | | getAUse(mid))
+    )
+  }
+
+  /**
+   * Holds if an operand that satisfies `isIRTempVariable` flows to `op`
+   * through a (possibly empty) sequence of conversions.
+   */
+  private predicate irTempOperandConversionFlows(Operand op) {
+    isIRTempVariable(op)
+    or
+    exists(Operand mid |
+      irTempOperandConversionFlows(mid) and
+      isConversionStep(mid, op)
+    )
+  }
+
   /** Holds if `node` should be an `IndirectOperand` that maps `node.asExpr()` to `e`. */
   private predicate exprNodeShouldBeIndirectOperand(IndirectOperand node, Expr e, int n) {
     exists(ArgumentOperand operand |
@@ -203,9 +243,8 @@ private module Cached {
       // result. However, the instruction actually represents the _address_ of
       // the argument. So to fix this mismatch, we have the indirection of the
       // `VariableAddressInstruction` map to the expression.
-      node.hasOperandAndIndirectionIndex(operand, 1) and
-      e = getConvertedResultExpression(operand.getDef(), n) and
-      operand.getDef().(VariableAddressInstruction).getIRVariable() instanceof IRTempVariable
+      isIndirectOperandOfArgument(node, operand, e, n) and
+      irTempOperandConversionFlows(operand)
     )
   }
 

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

@@ -151,6 +151,9 @@ module IRTest {
         or
         call.getTarget().getName() = "indirect_sink" and
         sink.asIndirectExpr() = e
+        or
+        call.getTarget().getName() = "indirect_sink_const_ref" and
+        sink.asIndirectExpr() = e
       )
     }
 

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

@@ -313,6 +313,7 @@ irFlow
 | test.cpp:1021:18:1021:32 | *call to indirect_source | test.cpp:1027:19:1027:28 | *translated |
 | test.cpp:1021:18:1021:32 | *call to indirect_source | test.cpp:1031:19:1031:28 | *translated |
 | test.cpp:1045:14:1045:19 | call to source | test.cpp:1046:7:1046:10 | * ... |
+| test.cpp:1081:27:1081:34 | call to source | test.cpp:1081:27:1081:34 | call to source |
 | true_upon_entry.cpp:9:11:9:16 | call to source | true_upon_entry.cpp:13:8:13:8 | x |
 | true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |
 | true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x |

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

@@ -1073,3 +1073,10 @@ void single_object_in_both_cases(bool b, int x, int y) {
   *p = 0;
   sink(*p); // clean
 }
+
+template<typename T>
+void indirect_sink_const_ref(const T&);
+
+void test_temp_with_conversion_from_materialization() {
+  indirect_sink_const_ref(source()); // $ ir MISSING: ast
+}