Skip to content

C++: Deduplicate OutNodes #12141

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MathiasVP merged 4 commits into from
Feb 10, 2023
Merged

C++: Deduplicate OutNodes #12141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7439de3
C++: Add a new test that demonstrates multiple out nodes.
MathiasVP Feb 9, 2023
6b851d0
C++: Fix an inconsistency with too many out nodes.
MathiasVP Feb 9, 2023
981c976
C++: Expand comments.
MathiasVP Feb 9, 2023
4719fd5
C++: Accept more test changes.
MathiasVP Feb 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 40 additions & 8 deletions cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,14 +367,18 @@ 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
result = getAUse(instr)
)
}

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.
Expand All @@ -401,10 +405,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)
Expand All @@ -413,6 +428,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.
Expand All @@ -421,16 +454,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)
)
}

Expand Down
Empty file added cpp/ql/test/library-tests/dataflow/dataflow-tests/test-number-of-outnodes.expected
View file
Open in desktop
Empty file.
46 changes: 46 additions & 0 deletions cpp/ql/test/library-tests/dataflow/dataflow-tests/test-number-of-outnodes.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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()
)
}
}
}
20 changes: 0 additions & 20 deletions cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
10 changes: 0 additions & 10 deletions cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand All @@ -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
Expand Down