C++: Fix missing result in 'ModelUtil'. The problem was that 'n.asInstruction()' on line 81 wasn't necessarily a 'CallInstruction' (it could be a conversion).

Author: MathiasVP

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

@@ -32,25 +32,34 @@ DataFlow::Node callInput(CallInstruction call, FunctionInput input) {
 }
 
 /**
- * Gets the instruction that holds the `output` for `call`.
+ * Gets the node that represents the output of `call` with kind `output` at
+ * indirection index `indirectionIndex`.
  */
-Node callOutput(CallInstruction call, FunctionOutput output) {
+private Node callOutputWithIndirectionIndex(
+  CallInstruction call, FunctionOutput output, int indirectionIndex
+) {
   // The return value
   simpleOutNode(result, call) and
-  output.isReturnValue()
+  output.isReturnValue() and
+  indirectionIndex = 0
   or
   // The side effect of a call on the value pointed to by an argument or qualifier
-  exists(int index, int indirectionIndex |
+  exists(int index |
     result.(IndirectArgumentOutNode).getArgumentIndex() = index and
-    result.(IndirectArgumentOutNode).getIndirectionIndex() = indirectionIndex and
+    result.(IndirectArgumentOutNode).getIndirectionIndex() = indirectionIndex - 1 and
     result.(IndirectArgumentOutNode).getCallInstruction() = call and
-    output.isParameterDerefOrQualifierObject(index, indirectionIndex)
+    output.isParameterDerefOrQualifierObject(index, indirectionIndex - 1)
   )
   or
-  exists(int ind |
-    result = getIndirectReturnOutNode(call, ind) and
-    output.isReturnValueDeref(ind)
-  )
+  result = getIndirectReturnOutNode(call, indirectionIndex) and
+  output.isReturnValueDeref(indirectionIndex)
+}
+
+/**
+ * Gets the instruction that holds the `output` for `call`.
+ */
+Node callOutput(CallInstruction call, FunctionOutput output) {
+  result = callOutputWithIndirectionIndex(call, output, _)
 }
 
 DataFlow::Node callInput(CallInstruction call, FunctionInput input, int d) {
@@ -76,19 +85,16 @@ private IndirectReturnOutNode getIndirectReturnOutNode(CallInstruction call, int
  */
 bindingset[d]
 Node callOutput(CallInstruction call, FunctionOutput output, int d) {
-  exists(DataFlow::Node n | n = callOutput(call, output) and d > 0 |
+  exists(DataFlow::Node n, int indirectionIndex |
+    n = callOutputWithIndirectionIndex(call, output, indirectionIndex) and d > 0
+  |
     // The return value
-    result = getIndirectReturnOutNode(n.asInstruction(), d)
+    result = callOutputWithIndirectionIndex(call, output, indirectionIndex + d)
     or
+    n = callOutputWithIndirectionIndex(call, output, indirectionIndex) and
     // If there isn't an indirect out node for the call with indirection `d` then
     // we conflate this with the underlying `CallInstruction`.
-    not exists(getIndirectReturnOutNode(call, d)) and
+    not exists(getIndirectReturnOutNode(call, indirectionIndex + d)) and
     n = result
-    or
-    // The side effect of a call on the value pointed to by an argument or qualifier
-    exists(Operand operand, int indirectionIndex |
-      Ssa::outNodeHasAddressAndIndex(n, operand, indirectionIndex) and
-      Ssa::outNodeHasAddressAndIndex(result, operand, indirectionIndex + d)
-    )
   )
 }

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

@@ -447,5 +447,5 @@ void test_indirect_taint() {
   int* p = indirect_source();
   m[1] = p;
   int* q = m[1];
-  sink(q); // $ MISSING: ast ir
+  sink(q); // $ ir MISSING: ast
 }

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

@@ -85,11 +85,11 @@ struct B {
 
 void test_operator_arrow(std::unique_ptr<A> p, std::unique_ptr<B> q) {
   p->x = source();
-  sink(p->x); // $ ast,ir
+  sink(p->x); // $ ast MISSING: ir
   sink(p->y);
 
   q->a1.x = source();
-  sink(q->a1.x); // $ ast,ir
+  sink(q->a1.x); // $ ast MISSING: ir
   sink(q->a1.y);
   sink(q->a2.x);
 }
@@ -101,7 +101,7 @@ void taint_x(A* pa) {
 void reverse_taint_smart_pointer() {
   std::unique_ptr<A> p = std::unique_ptr<A>(new A);
   taint_x(p.get());
-  sink(p->x); // $ ast,ir
+  sink(p->x); // $ ast MISSING: ir
 }
 
 struct C {
@@ -131,7 +131,7 @@ int nested_shared_ptr_taint(std::shared_ptr<C> p1, std::unique_ptr<std::shared_p
 
 int nested_shared_ptr_taint_cref(std::shared_ptr<C> p1, std::unique_ptr<std::shared_ptr<int>> p2) {
   taint_x_shared_cref(p1->q);
-  sink(p1->q->x); // $ ast,ir
+  sink(p1->q->x); // $ ast MISSING: ir
 
   getNumberCRef(*p2);
   sink(**p2); // $ ast,ir