diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index cc4ae90faf9..7a01010343b 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -404,7 +404,7 @@ class ReturnIndirectionNode extends IndirectReturnNode, ReturnNode { } } -private Operand fullyConvertedCallStep(Operand op) { +private Operand fullyConvertedCallStepImpl(Operand op) { not exists(getANonConversionUse(op)) and exists(Instruction instr | conversionFlow(op, instr, _, _) and @@ -412,6 +412,10 @@ private Operand fullyConvertedCallStep(Operand op) { ) } +private Operand fullyConvertedCallStep(Operand op) { + result = unique( | | fullyConvertedCallStepImpl(op)) +} + /** * Gets the instruction that uses this operand, if the instruction is not * ignored for dataflow purposes. @@ -438,10 +442,21 @@ private Instruction getANonConversionUse(Operand operand) { } /** - * Gets the operand that represents the first use of the value of `call` following + * Gets an operand that represents the use of the value of `call` following * a sequence of conversion-like instructions. + * + * Note that `operand` is not functionally determined by `call` since there + * can be multiple sequences of disjoint conversions following a call. For example, + * consider an example like: + * ```cpp + * long f(); + * int y; + * long x = (long)(y = (int)f()); + * ``` + * in this case, there'll be a long-to-int conversion on `f()` before the value is assigned to `y`, + * and there will be an int-to-long conversion on `(int)f()` before the value is assigned to `x`. */ -predicate operandForFullyConvertedCall(Operand operand, CallInstruction call) { +private predicate operandForFullyConvertedCallImpl(Operand operand, CallInstruction call) { exists(getANonConversionUse(operand)) and ( operand = getAUse(call) @@ -450,6 +465,24 @@ predicate operandForFullyConvertedCall(Operand operand, CallInstruction call) { ) } +/** + * Gets the operand that represents the use of the value of `call` following + * a sequence of conversion-like instructions, if a unique operand exists. + */ +predicate operandForFullyConvertedCall(Operand operand, CallInstruction call) { + operand = unique(Operand cand | operandForFullyConvertedCallImpl(cand, call)) +} + +private predicate instructionForFullyConvertedCallWithConversions( + Instruction instr, CallInstruction call +) { + instr = + getUse(unique(Operand operand | + operand = fullyConvertedCallStep*(getAUse(call)) and + not exists(fullyConvertedCallStep(operand)) + )) +} + /** * Gets the instruction that represents the first use of the value of `call` following * a sequence of conversion-like instructions. @@ -458,16 +491,15 @@ predicate operandForFullyConvertedCall(Operand operand, CallInstruction call) { * conversion instruction) to use to represent the value of `call` after conversions. */ predicate instructionForFullyConvertedCall(Instruction instr, CallInstruction call) { + // Only pick an instruction for the call if we cannot pick a unique operand. not operandForFullyConvertedCall(_, call) and ( // If there is no use of the call then we pick the call instruction - not exists(getAUse(call)) and + not instructionForFullyConvertedCallWithConversions(_, call) and instr = call or - // Otherwise, flow to the first non-conversion use. - exists(Operand operand | operand = fullyConvertedCallStep*(getAUse(call)) | - instr = getANonConversionUse(operand) - ) + // Otherwise, flow to the first instruction that defines multiple operands. + instructionForFullyConvertedCallWithConversions(instr, call) ) } diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-number-of-outnodes.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-number-of-outnodes.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-number-of-outnodes.ql b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-number-of-outnodes.ql new file mode 100644 index 00000000000..dbaf015630c --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-number-of-outnodes.ql @@ -0,0 +1,46 @@ +import TestUtilities.InlineExpectationsTest +import cpp + +module AstTest { + private import semmle.code.cpp.dataflow.old.DataFlow::DataFlow + private import semmle.code.cpp.dataflow.old.internal.DataFlowPrivate + + class ASTMultipleOutNodesTest extends InlineExpectationsTest { + ASTMultipleOutNodesTest() { this = "ASTMultipleOutNodesTest" } + + override string getARelevantTag() { result = "ast-count(" + any(ReturnKind k).toString() + ")" } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + exists(DataFlowCall call, int n, ReturnKind kind | + call.getLocation() = location and + n = strictcount(getAnOutNode(call, kind)) and + n > 1 and + element = call.toString() and + tag = "ast-count(" + kind.toString() + ")" and + value = n.toString() + ) + } + } +} + +module IRTest { + private import semmle.code.cpp.ir.dataflow.DataFlow + private import semmle.code.cpp.ir.dataflow.internal.DataFlowPrivate + + class IRMultipleOutNodesTest extends InlineExpectationsTest { + IRMultipleOutNodesTest() { this = "IRMultipleOutNodesTest" } + + override string getARelevantTag() { result = "ir-count(" + any(ReturnKind k).toString() + ")" } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + exists(DataFlowCall call, int n, ReturnKind kind | + call.getLocation() = location and + n = strictcount(getAnOutNode(call, kind)) and + n > 1 and + element = call.toString() and + tag = "ir-count(" + kind.toString() + ")" and + value = n.toString() + ) + } + } +} diff --git a/cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected b/cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected index 1e9c150b01b..f5e53c2d7a3 100644 --- a/cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected +++ b/cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected @@ -12,16 +12,6 @@ compatibleTypesReflexive unreachableNodeCCtx localCallNodes postIsNotPre -| A.cpp:98:12:98:18 | new indirection | PostUpdateNode should not equal its pre-update node. | -| B.cpp:6:15:6:24 | new indirection | PostUpdateNode should not equal its pre-update node. | -| B.cpp:15:15:15:27 | new indirection | PostUpdateNode should not equal its pre-update node. | -| C.cpp:22:12:22:21 | new indirection | PostUpdateNode should not equal its pre-update node. | -| C.cpp:24:16:24:25 | new indirection | PostUpdateNode should not equal its pre-update node. | -| D.cpp:28:15:28:24 | new indirection | PostUpdateNode should not equal its pre-update node. | -| D.cpp:35:15:35:24 | new indirection | PostUpdateNode should not equal its pre-update node. | -| D.cpp:42:15:42:24 | new indirection | PostUpdateNode should not equal its pre-update node. | -| D.cpp:49:15:49:24 | new indirection | PostUpdateNode should not equal its pre-update node. | -| D.cpp:56:15:56:24 | new indirection | PostUpdateNode should not equal its pre-update node. | postHasUniquePre uniquePostUpdate | aliasing.cpp:70:11:70:11 | definition of w indirection | Node has multiple PostUpdateNodes. | @@ -42,16 +32,6 @@ postIsInSameCallable reverseRead argHasPostUpdate postWithInFlow -| A.cpp:98:12:98:18 | new indirection | PostUpdateNode should not be the target of local flow. | -| B.cpp:6:15:6:24 | new indirection | PostUpdateNode should not be the target of local flow. | -| B.cpp:15:15:15:27 | new indirection | PostUpdateNode should not be the target of local flow. | -| C.cpp:22:12:22:21 | new indirection | PostUpdateNode should not be the target of local flow. | -| C.cpp:24:16:24:25 | new indirection | PostUpdateNode should not be the target of local flow. | -| D.cpp:28:15:28:24 | new indirection | PostUpdateNode should not be the target of local flow. | -| D.cpp:35:15:35:24 | new indirection | PostUpdateNode should not be the target of local flow. | -| D.cpp:42:15:42:24 | new indirection | PostUpdateNode should not be the target of local flow. | -| D.cpp:49:15:49:24 | new indirection | PostUpdateNode should not be the target of local flow. | -| D.cpp:56:15:56:24 | new indirection | PostUpdateNode should not be the target of local flow. | | realistic.cpp:54:16:54:47 | memcpy output argument | PostUpdateNode should not be the target of local flow. | | realistic.cpp:60:16:60:18 | memcpy output argument | PostUpdateNode should not be the target of local flow. | viableImplInCallContextTooLarge diff --git a/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected b/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected index c238e7aac96..ddc7c397f1e 100644 --- a/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected +++ b/cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected @@ -31,7 +31,6 @@ edges | A.cpp:57:11:57:24 | call to B [c] | A.cpp:57:11:57:24 | new indirection [c] | | A.cpp:57:11:57:24 | new indirection [c] | A.cpp:28:8:28:10 | this indirection [c] | | A.cpp:57:11:57:24 | new indirection [c] | A.cpp:57:10:57:32 | call to get | -| A.cpp:57:11:57:24 | new indirection [c] | A.cpp:57:11:57:24 | new indirection [c] | | A.cpp:57:17:57:23 | new | A.cpp:23:10:23:10 | c | | A.cpp:57:17:57:23 | new | A.cpp:57:11:57:24 | call to B [c] | | A.cpp:57:17:57:23 | new | A.cpp:57:17:57:23 | new | diff --git a/cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected b/cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected index 2d9721e86a5..5e3dcaba931 100644 --- a/cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected +++ b/cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected @@ -223,11 +223,6 @@ compatibleTypesReflexive unreachableNodeCCtx localCallNodes postIsNotPre -| condition_decls.cpp:3:13:3:22 | new indirection | PostUpdateNode should not equal its pre-update node. | -| defdestructordeleteexpr.cpp:4:9:4:15 | new indirection | PostUpdateNode should not equal its pre-update node. | -| deleteexpr.cpp:7:9:7:15 | new indirection | PostUpdateNode should not equal its pre-update node. | -| ir.cpp:943:3:943:11 | new indirection | PostUpdateNode should not equal its pre-update node. | -| ir.cpp:947:3:947:25 | new indirection | PostUpdateNode should not equal its pre-update node. | postHasUniquePre uniquePostUpdate | cpp11.cpp:82:17:82:17 | this indirection | Node has multiple PostUpdateNodes. | @@ -251,19 +246,14 @@ postIsInSameCallable reverseRead argHasPostUpdate postWithInFlow -| condition_decls.cpp:3:13:3:22 | new indirection | PostUpdateNode should not be the target of local flow. | | cpp11.cpp:77:19:77:21 | call to Val | PostUpdateNode should not be the target of local flow. | | cpp11.cpp:82:11:82:14 | call to Val | PostUpdateNode should not be the target of local flow. | | cpp11.cpp:82:45:82:48 | call to Val | PostUpdateNode should not be the target of local flow. | | cpp11.cpp:82:51:82:51 | call to Val | PostUpdateNode should not be the target of local flow. | -| defdestructordeleteexpr.cpp:4:9:4:15 | new indirection | PostUpdateNode should not be the target of local flow. | -| deleteexpr.cpp:7:9:7:15 | new indirection | PostUpdateNode should not be the target of local flow. | | ir.cpp:809:7:809:13 | call to Base | PostUpdateNode should not be the target of local flow. | | ir.cpp:810:7:810:26 | call to Base | PostUpdateNode should not be the target of local flow. | | ir.cpp:823:7:823:13 | call to Base | PostUpdateNode should not be the target of local flow. | | ir.cpp:824:7:824:26 | call to Base | PostUpdateNode should not be the target of local flow. | -| ir.cpp:943:3:943:11 | new indirection | PostUpdateNode should not be the target of local flow. | -| ir.cpp:947:3:947:25 | new indirection | PostUpdateNode should not be the target of local flow. | | try_catch.cpp:7:8:7:8 | call to exception | PostUpdateNode should not be the target of local flow. | viableImplInCallContextTooLarge uniqueParameterNodeAtPosition