Skip to content

Commit 6b851d0

Browse files
MathiasVPMathiasVP
committed
C++: Fix an inconsistency with too many out nodes.
1 parent 7439de3 commit 6b851d0

4 files changed

Lines changed: 39 additions & 32 deletions

File tree

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll‎

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -367,14 +367,18 @@ class ReturnIndirectionNode extends IndirectReturnNode, ReturnNode {
367367
}
368368
}
369369

370-
private Operand fullyConvertedCallStep(Operand op) {
370+
private Operand fullyConvertedCallStepImpl(Operand op) {
371371
not exists(getANonConversionUse(op)) and
372372
exists(Instruction instr |
373373
conversionFlow(op, instr, _, _) and
374374
result = getAUse(instr)
375375
)
376376
}
377377

378+
private Operand fullyConvertedCallStep(Operand op) {
379+
result = unique( | | fullyConvertedCallStepImpl(op))
380+
}
381+
378382
/**
379383
* Gets the instruction that uses this operand, if the instruction is not
380384
* ignored for dataflow purposes.
@@ -401,10 +405,21 @@ private Instruction getANonConversionUse(Operand operand) {
401405
}
402406

403407
/**
404-
* Gets the operand that represents the first use of the value of `call` following
408+
* Gets an operand that represents the use of the value of `call` following
405409
* a sequence of conversion-like instructions.
410+
*
411+
* Note that `operand` is not functionally determined by `call` since there
412+
* can be multiple sequences of disjoint conversions following a call. For example,
413+
* consider an example like:
414+
* ```cpp
415+
* long f();
416+
* int y;
417+
* long x = (long)(y = (int)f());
418+
* ```
419+
* in this case, there'll be a long-to-int conversion on `f()` before the value is assigned to `y`,
420+
* and there will be an int-to-long conversion on `(int)f()` before the value is assigned to `x`.
406421
*/
407-
predicate operandForFullyConvertedCall(Operand operand, CallInstruction call) {
422+
private predicate operandForFullyConvertedCallImpl(Operand operand, CallInstruction call) {
408423
exists(getANonConversionUse(operand)) and
409424
(
410425
operand = getAUse(call)
@@ -413,6 +428,25 @@ predicate operandForFullyConvertedCall(Operand operand, CallInstruction call) {
413428
)
414429
}
415430

431+
/**
432+
* Gets the operand that represents the use of the value of `call` following
433+
* a sequence of conversion-like instructions, if a unique operand exists.
434+
*/
435+
predicate operandForFullyConvertedCall(Operand operand, CallInstruction call) {
436+
operand = unique(Operand cand | operandForFullyConvertedCallImpl(cand, call))
437+
}
438+
439+
private predicate instructionForFullyConvertedCallWithConversions(
440+
Instruction instr, CallInstruction call
441+
) {
442+
// Otherwise, flow to the first non-conversion use.
443+
instr =
444+
getUse(unique(Operand operand |
445+
operand = fullyConvertedCallStep*(getAUse(call)) and
446+
not exists(fullyConvertedCallStep(operand))
447+
))
448+
}
449+
416450
/**
417451
* Gets the instruction that represents the first use of the value of `call` following
418452
* a sequence of conversion-like instructions.
@@ -424,13 +458,10 @@ predicate instructionForFullyConvertedCall(Instruction instr, CallInstruction ca
424458
not operandForFullyConvertedCall(_, call) and
425459
(
426460
// If there is no use of the call then we pick the call instruction
427-
not exists(getAUse(call)) and
461+
not instructionForFullyConvertedCallWithConversions(_, call) and
428462
instr = call
429463
or
430-
// Otherwise, flow to the first non-conversion use.
431-
exists(Operand operand | operand = fullyConvertedCallStep*(getAUse(call)) |
432-
instr = getANonConversionUse(operand)
433-
)
464+
instructionForFullyConvertedCallWithConversions(instr, call)
434465
)
435466
}
436467

‎cpp/ql/test/library-tests/dataflow/dataflow-tests/test-number-of-outnodes.expected‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +0,0 @@
1-
| dispatch.cpp:60:18:60:29 | Call: new | Unexpected result: ir-count(indirect return)=3 |
2-
| dispatch.cpp:61:18:61:29 | Call: new | Unexpected result: ir-count(indirect return)=3 |
3-
| dispatch.cpp:65:10:65:21 | Call: new | Unexpected result: ir-count(indirect return)=3 |

‎cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,6 @@ compatibleTypesReflexive
1212
unreachableNodeCCtx
1313
localCallNodes
1414
postIsNotPre
15-
| A.cpp:98:12:98:18 | new indirection | PostUpdateNode should not equal its pre-update node. |
16-
| B.cpp:6:15:6:24 | new indirection | PostUpdateNode should not equal its pre-update node. |
17-
| B.cpp:15:15:15:27 | new indirection | PostUpdateNode should not equal its pre-update node. |
18-
| C.cpp:22:12:22:21 | new indirection | PostUpdateNode should not equal its pre-update node. |
19-
| C.cpp:24:16:24:25 | new indirection | PostUpdateNode should not equal its pre-update node. |
20-
| D.cpp:28:15:28:24 | new indirection | PostUpdateNode should not equal its pre-update node. |
21-
| D.cpp:35:15:35:24 | new indirection | PostUpdateNode should not equal its pre-update node. |
22-
| D.cpp:42:15:42:24 | new indirection | PostUpdateNode should not equal its pre-update node. |
23-
| D.cpp:49:15:49:24 | new indirection | PostUpdateNode should not equal its pre-update node. |
24-
| D.cpp:56:15:56:24 | new indirection | PostUpdateNode should not equal its pre-update node. |
2515
postHasUniquePre
2616
uniquePostUpdate
2717
| aliasing.cpp:70:11:70:11 | definition of w indirection | Node has multiple PostUpdateNodes. |
@@ -42,16 +32,6 @@ postIsInSameCallable
4232
reverseRead
4333
argHasPostUpdate
4434
postWithInFlow
45-
| A.cpp:98:12:98:18 | new indirection | PostUpdateNode should not be the target of local flow. |
46-
| B.cpp:6:15:6:24 | new indirection | PostUpdateNode should not be the target of local flow. |
47-
| B.cpp:15:15:15:27 | new indirection | PostUpdateNode should not be the target of local flow. |
48-
| C.cpp:22:12:22:21 | new indirection | PostUpdateNode should not be the target of local flow. |
49-
| C.cpp:24:16:24:25 | new indirection | PostUpdateNode should not be the target of local flow. |
50-
| D.cpp:28:15:28:24 | new indirection | PostUpdateNode should not be the target of local flow. |
51-
| D.cpp:35:15:35:24 | new indirection | PostUpdateNode should not be the target of local flow. |
52-
| D.cpp:42:15:42:24 | new indirection | PostUpdateNode should not be the target of local flow. |
53-
| D.cpp:49:15:49:24 | new indirection | PostUpdateNode should not be the target of local flow. |
54-
| D.cpp:56:15:56:24 | new indirection | PostUpdateNode should not be the target of local flow. |
5535
| realistic.cpp:54:16:54:47 | memcpy output argument | PostUpdateNode should not be the target of local flow. |
5636
| realistic.cpp:60:16:60:18 | memcpy output argument | PostUpdateNode should not be the target of local flow. |
5737
viableImplInCallContextTooLarge

‎cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ edges
3131
| A.cpp:57:11:57:24 | call to B [c] | A.cpp:57:11:57:24 | new indirection [c] |
3232
| A.cpp:57:11:57:24 | new indirection [c] | A.cpp:28:8:28:10 | this indirection [c] |
3333
| A.cpp:57:11:57:24 | new indirection [c] | A.cpp:57:10:57:32 | call to get |
34-
| A.cpp:57:11:57:24 | new indirection [c] | A.cpp:57:11:57:24 | new indirection [c] |
3534
| A.cpp:57:17:57:23 | new | A.cpp:23:10:23:10 | c |
3635
| A.cpp:57:17:57:23 | new | A.cpp:57:11:57:24 | call to B [c] |
3736
| A.cpp:57:17:57:23 | new | A.cpp:57:17:57:23 | new |

0 commit comments

Comments
 (0)