From 3b585b4196aa6534ad28102eaa660c6910ecf0c7 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 21 Jun 2024 13:19:49 +0100 Subject: [PATCH 1/3] C++: Add test with missing flow. --- .../library-tests/dataflow/dataflow-tests/TestBase.qll | 3 +++ cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/TestBase.qll b/cpp/ql/test/library-tests/dataflow/dataflow-tests/TestBase.qll index 426098084ca..b9213a71549 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/TestBase.qll +++ b/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 ) } diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp index c1c84c71e3b..cd8dab9e791 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/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 +void indirect_sink_const_ref(const T&); + +void test_temp_with_conversion_from_materialization() { + indirect_sink_const_ref(source()); // $ MISSING: ast,ir +} \ No newline at end of file From 7d41e8ef730f62fc40597f087d142b426152f737 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 21 Jun 2024 13:27:39 +0100 Subject: [PATCH 2/3] C++: Perform a TC to skip conversions when special-casing materialization of temporaries. --- .../cpp/ir/dataflow/internal/ExprNodes.qll | 45 +++++++++++++++++-- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll index 8024c7973f6..c2b89a67f69 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll +++ b/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) ) } From 1bb762bea94e75c7b422ff32a2b3636ff44c69dd Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 21 Jun 2024 13:34:39 +0100 Subject: [PATCH 3/3] C++: Accept test changes. --- .../dataflow/dataflow-tests/test-source-sink.expected | 1 + cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected index 808977af972..ac26a0c76ec 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected +++ b/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 | diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp index cd8dab9e791..ae6b76b88ca 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp @@ -1078,5 +1078,5 @@ template void indirect_sink_const_ref(const T&); void test_temp_with_conversion_from_materialization() { - indirect_sink_const_ref(source()); // $ MISSING: ast,ir + indirect_sink_const_ref(source()); // $ ir MISSING: ast } \ No newline at end of file