Skip to content

Commit 220ad54

Browse files
committed
DataFlow/C++: Get rid of the dataflow hook to modify the 'join' and 'branch' metrics in dataflow. These should no longer be needed now that we have second-level scopes.
1 parent a39d8b7 commit 220ad54

File tree

4 files changed

+10
-126
lines changed

4 files changed

+10
-126
lines changed

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplSpecific.qll

-2
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ module CppDataFlow implements InputSig<Location> {
2020

2121
Node exprNode(DataFlowExpr e) { result = Public::exprNode(e) }
2222

23-
predicate getAdditionalFlowIntoCallNodeTerm = Private::getAdditionalFlowIntoCallNodeTerm/2;
24-
2523
predicate getSecondLevelScope = Private::getSecondLevelScope/1;
2624

2725
predicate validParameterAliasStep = Private::validParameterAliasStep/2;

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

-94
Original file line numberDiff line numberDiff line change
@@ -36,28 +36,6 @@ private module Cached {
3636
not Ssa::ignoreOperand(op) and exists(Ssa::getIRRepresentationOfOperand(op))
3737
}
3838
}
39-
40-
/**
41-
* Gets an additional term that is added to the `join` and `branch` computations to reflect
42-
* an additional forward or backwards branching factor that is not taken into account
43-
* when calculating the (virtual) dispatch cost.
44-
*
45-
* Argument `arg` is part of a path from a source to a sink, and `p` is the target parameter.
46-
*/
47-
pragma[nomagic]
48-
cached
49-
int getAdditionalFlowIntoCallNodeTerm(ArgumentNode arg, ParameterNode p) {
50-
DataFlowImplCommon::forceCachingInSameStage() and
51-
exists(
52-
ParameterNode switchee, SwitchInstruction switch, ConditionOperand op, DataFlowCall call
53-
|
54-
DataFlowImplCommon::viableParamArg(call, p, arg) and
55-
DataFlowImplCommon::viableParamArg(call, switchee, _) and
56-
switch.getExpressionOperand() = op and
57-
getAdditionalFlowIntoCallNodeTermStep+(switchee, operandNode(op)) and
58-
result = countNumberOfBranchesUsingParameter(switch, p)
59-
)
60-
}
6139
}
6240

6341
import Cached
@@ -1433,78 +1411,6 @@ private predicate localStepsToSwitch(Node node) {
14331411
)
14341412
}
14351413

1436-
/**
1437-
* Holds if `node` is part of a path from a `ParameterNode` to an operand
1438-
* of a `SwitchInstruction`.
1439-
*/
1440-
private predicate localStepsFromParameterToSwitch(Node node) {
1441-
localStepsToSwitch(node) and
1442-
(
1443-
node instanceof ParameterNode
1444-
or
1445-
exists(Node prev |
1446-
localStepsFromParameterToSwitch(prev) and
1447-
localFlowStepWithSummaries(prev, node)
1448-
)
1449-
)
1450-
}
1451-
1452-
/**
1453-
* The local flow relation `localFlowStepWithSummaries` pruned to only
1454-
* include steps that are part of a path from a `ParameterNode` to an
1455-
* operand of a `SwitchInstruction`.
1456-
*/
1457-
private predicate getAdditionalFlowIntoCallNodeTermStep(Node node1, Node node2) {
1458-
localStepsFromParameterToSwitch(node1) and
1459-
localStepsFromParameterToSwitch(node2) and
1460-
localFlowStepWithSummaries(node1, node2)
1461-
}
1462-
1463-
/** Gets the `IRVariable` associated with the parameter node `p`. */
1464-
pragma[nomagic]
1465-
private IRVariable getIRVariableForParameterNode(ParameterNode p) {
1466-
result = p.(InstructionDirectParameterNode).getIRVariable()
1467-
or
1468-
result.getAst() = p.(IndirectParameterNode).getParameter()
1469-
}
1470-
1471-
/** Holds if `v` is the source variable corresponding to the parameter represented by `p`. */
1472-
pragma[nomagic]
1473-
private predicate parameterNodeHasSourceVariable(ParameterNode p, Ssa::SourceVariable v) {
1474-
v.getIRVariable() = getIRVariableForParameterNode(p) and
1475-
exists(Position pos | p.isParameterOf(_, pos) |
1476-
pos instanceof DirectPosition and
1477-
v.getIndirection() = 1
1478-
or
1479-
pos.(IndirectionPosition).getIndirectionIndex() + 1 = v.getIndirection()
1480-
)
1481-
}
1482-
1483-
private EdgeKind caseOrDefaultEdge() {
1484-
result instanceof CaseEdge or
1485-
result instanceof DefaultEdge
1486-
}
1487-
1488-
/**
1489-
* Gets the number of switch branches that that read from (or write to) the parameter `p`.
1490-
*/
1491-
private int countNumberOfBranchesUsingParameter(SwitchInstruction switch, ParameterNode p) {
1492-
exists(Ssa::SourceVariable sv |
1493-
parameterNodeHasSourceVariable(p, sv) and
1494-
// Count the number of cases that use the parameter. We do this by finding the phi node
1495-
// that merges the uses/defs of the parameter. There might be multiple such phi nodes, so
1496-
// we pick the one with the highest edge count.
1497-
result =
1498-
max(SsaPhiNode phi |
1499-
switch.getSuccessor(caseOrDefaultEdge()).getBlock().dominanceFrontier() =
1500-
phi.getBasicBlock() and
1501-
phi.getSourceVariable() = sv
1502-
|
1503-
strictcount(phi.getAnInput())
1504-
)
1505-
)
1506-
}
1507-
15081414
pragma[nomagic]
15091415
private predicate isInputOutput(
15101416
DF::DataFlowFunction target, Node node1, Node node2, IO::FunctionInput input,

shared/dataflow/codeql/dataflow/DataFlow.qll

-9
Original file line numberDiff line numberDiff line change
@@ -299,15 +299,6 @@ signature module InputSig<LocationSig Location> {
299299
*/
300300
default predicate neverSkipInPathGraph(Node n) { none() }
301301

302-
/**
303-
* Gets an additional term that is added to the `join` and `branch` computations to reflect
304-
* an additional forward or backwards branching factor that is not taken into account
305-
* when calculating the (virtual) dispatch cost.
306-
*
307-
* Argument `arg` is part of a path from a source to a sink, and `p` is the target parameter.
308-
*/
309-
default int getAdditionalFlowIntoCallNodeTerm(ArgumentNode arg, ParameterNode p) { none() }
310-
311302
/**
312303
* A second-level control-flow scope in a callable.
313304
*

shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll

+10-21
Original file line numberDiff line numberDiff line change
@@ -1102,17 +1102,6 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
11021102
not inBarrier(p)
11031103
}
11041104

1105-
/**
1106-
* Gets an additional term that is added to `branch` and `join` when deciding whether
1107-
* the amount of forward or backward branching is within the limit specified by the
1108-
* configuration.
1109-
*/
1110-
pragma[nomagic]
1111-
private int getLanguageSpecificFlowIntoCallNodeCand1(ArgNodeEx arg, ParamNodeEx p) {
1112-
flowIntoCallNodeCand1(_, arg, p) and
1113-
result = getAdditionalFlowIntoCallNodeTerm(arg.projectToNode(), p.projectToNode())
1114-
}
1115-
11161105
private module SndLevelScopeOption = Option<DataFlowSecondLevelScope>;
11171106

11181107
private class SndLevelScopeOption = SndLevelScopeOption::Option;
@@ -1177,11 +1166,11 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
11771166
private int branch(ArgNodeEx n1) {
11781167
result =
11791168
strictcount(DataFlowCallable c |
1180-
exists(NodeEx n |
1181-
flowIntoCallNodeCand1(_, n1, n) and
1182-
c = n.getEnclosingCallable()
1183-
)
1184-
) + sum(ParamNodeEx p1 | | getLanguageSpecificFlowIntoCallNodeCand1(n1, p1))
1169+
exists(NodeEx n |
1170+
flowIntoCallNodeCand1(_, n1, n) and
1171+
c = n.getEnclosingCallable()
1172+
)
1173+
)
11851174
}
11861175

11871176
/**
@@ -1193,11 +1182,11 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
11931182
private int join(ParamNodeEx n2) {
11941183
result =
11951184
strictcount(DataFlowCallable c |
1196-
exists(NodeEx n |
1197-
flowIntoCallNodeCand1(_, n, n2) and
1198-
c = n.getEnclosingCallable()
1199-
)
1200-
) + sum(ArgNodeEx arg2 | | getLanguageSpecificFlowIntoCallNodeCand1(arg2, n2))
1185+
exists(NodeEx n |
1186+
flowIntoCallNodeCand1(_, n, n2) and
1187+
c = n.getEnclosingCallable()
1188+
)
1189+
)
12011190
}
12021191

12031192
/**

0 commit comments

Comments
 (0)