C++: Use the 'StoreInstruction' instead of the 'ReturnValueInstruction' when detecting return expressions.

Author: MathiasVP

cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll

@@ -361,9 +361,30 @@ module GuardsInput implements SharedGuards::InputSig<Cpp::Location, Instruction,
 
     /** Gets an expression returned from this function. */
     GuardsInput::Expr getAReturnExpr() {
-      exists(ReturnValueInstruction ret |
-        ret.getEnclosingFunction() = this and
-        result = ret.getReturnValue()
+      exists(StoreInstruction store |
+        // We use the `Store` instruction that writes the return value instead of the
+        // `ReturnValue` instruction since the `ReturnValue` instruction is not always
+        // dominated by certain guards. For example:
+        // ```
+        // if(b) {
+        //   return true;
+        // } else {
+        //   return false;
+        // }
+        // ```
+        // this will be translated into IR like:
+        // ```
+        // if(b) {
+        //   x = true;
+        // } else {
+        //   x = false;
+        // }
+        // return x;
+        // ```
+        store.getDestinationAddress().(VariableAddressInstruction).getIRVariable() instanceof
+          IRReturnVariable and
+        store.getEnclosingFunction() = this and
+        result = store
       )
     }
   }

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

@@ -139,7 +139,7 @@ void test_guarded_wrapper() {
   int x = source();
 
   if(guarded_wrapper(x)) {
-    sink(x); // $ SPURIOUS: ast,ir
+    sink(x); // $ SPURIOUS: ast
   } else {
     sink(x); // $ ast,ir
   }

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

@@ -158,7 +158,6 @@ irFlow
 | BarrierGuard.cpp:49:10:49:15 | call to source | BarrierGuard.cpp:55:13:55:13 | x |
 | BarrierGuard.cpp:60:11:60:16 | call to source | BarrierGuard.cpp:64:14:64:14 | x |
 | BarrierGuard.cpp:60:11:60:16 | call to source | BarrierGuard.cpp:66:14:66:14 | x |
-| BarrierGuard.cpp:139:11:139:16 | call to source | BarrierGuard.cpp:142:10:142:10 | x |
 | BarrierGuard.cpp:139:11:139:16 | call to source | BarrierGuard.cpp:144:10:144:10 | x |
 | acrossLinkTargets.cpp:19:27:19:32 | call to source | acrossLinkTargets.cpp:12:8:12:8 | x |
 | clang.cpp:12:9:12:20 | sourceArray1 | clang.cpp:18:8:18:19 | sourceArray1 |