Skip to content

Commit

Permalink
DataFlow/C++: Get rid of the dataflow hook to modify the 'join' and '…
Browse files Browse the repository at this point in the history
…branch' metrics in dataflow. These should no longer be needed now that we have second-level scopes.
  • Loading branch information
MathiasVP committed Apr 23, 2024
1 parent 3592e76 commit d396a0b
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 126 deletions.
Expand Up @@ -20,8 +20,6 @@ module CppDataFlow implements InputSig<Location> {

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

predicate getAdditionalFlowIntoCallNodeTerm = Private::getAdditionalFlowIntoCallNodeTerm/2;

predicate getSecondLevelScope = Private::getSecondLevelScope/1;

predicate validParameterAliasStep = Private::validParameterAliasStep/2;
Expand Down
Expand Up @@ -36,28 +36,6 @@ private module Cached {
not Ssa::ignoreOperand(op) and exists(Ssa::getIRRepresentationOfOperand(op))
}
}

/**
* Gets an additional term that is added to the `join` and `branch` computations to reflect
* an additional forward or backwards branching factor that is not taken into account
* when calculating the (virtual) dispatch cost.
*
* Argument `arg` is part of a path from a source to a sink, and `p` is the target parameter.
*/
pragma[nomagic]
cached
int getAdditionalFlowIntoCallNodeTerm(ArgumentNode arg, ParameterNode p) {
DataFlowImplCommon::forceCachingInSameStage() and
exists(
ParameterNode switchee, SwitchInstruction switch, ConditionOperand op, DataFlowCall call
|
DataFlowImplCommon::viableParamArg(call, p, arg) and
DataFlowImplCommon::viableParamArg(call, switchee, _) and
switch.getExpressionOperand() = op and
getAdditionalFlowIntoCallNodeTermStep+(switchee, operandNode(op)) and
result = countNumberOfBranchesUsingParameter(switch, p)
)
}
}

import Cached
Expand Down Expand Up @@ -1433,78 +1411,6 @@ private predicate localStepsToSwitch(Node node) {
)
}

/**
* Holds if `node` is part of a path from a `ParameterNode` to an operand
* of a `SwitchInstruction`.
*/
private predicate localStepsFromParameterToSwitch(Node node) {
localStepsToSwitch(node) and
(
node instanceof ParameterNode
or
exists(Node prev |
localStepsFromParameterToSwitch(prev) and
localFlowStepWithSummaries(prev, node)
)
)
}

/**
* The local flow relation `localFlowStepWithSummaries` pruned to only
* include steps that are part of a path from a `ParameterNode` to an
* operand of a `SwitchInstruction`.
*/
private predicate getAdditionalFlowIntoCallNodeTermStep(Node node1, Node node2) {
localStepsFromParameterToSwitch(node1) and
localStepsFromParameterToSwitch(node2) and
localFlowStepWithSummaries(node1, node2)
}

/** Gets the `IRVariable` associated with the parameter node `p`. */
pragma[nomagic]
private IRVariable getIRVariableForParameterNode(ParameterNode p) {
result = p.(InstructionDirectParameterNode).getIRVariable()
or
result.getAst() = p.(IndirectParameterNode).getParameter()
}

/** Holds if `v` is the source variable corresponding to the parameter represented by `p`. */
pragma[nomagic]
private predicate parameterNodeHasSourceVariable(ParameterNode p, Ssa::SourceVariable v) {
v.getIRVariable() = getIRVariableForParameterNode(p) and
exists(Position pos | p.isParameterOf(_, pos) |
pos instanceof DirectPosition and
v.getIndirection() = 1
or
pos.(IndirectionPosition).getIndirectionIndex() + 1 = v.getIndirection()
)
}

private EdgeKind caseOrDefaultEdge() {
result instanceof CaseEdge or
result instanceof DefaultEdge
}

/**
* Gets the number of switch branches that that read from (or write to) the parameter `p`.
*/
private int countNumberOfBranchesUsingParameter(SwitchInstruction switch, ParameterNode p) {
exists(Ssa::SourceVariable sv |
parameterNodeHasSourceVariable(p, sv) and
// Count the number of cases that use the parameter. We do this by finding the phi node
// that merges the uses/defs of the parameter. There might be multiple such phi nodes, so
// we pick the one with the highest edge count.
result =
max(SsaPhiNode phi |
switch.getSuccessor(caseOrDefaultEdge()).getBlock().dominanceFrontier() =
phi.getBasicBlock() and
phi.getSourceVariable() = sv
|
strictcount(phi.getAnInput())
)
)
}

pragma[nomagic]
private predicate isInputOutput(
DF::DataFlowFunction target, Node node1, Node node2, IO::FunctionInput input,
Expand Down
9 changes: 0 additions & 9 deletions shared/dataflow/codeql/dataflow/DataFlow.qll
Expand Up @@ -299,15 +299,6 @@ signature module InputSig<LocationSig Location> {
*/
default predicate neverSkipInPathGraph(Node n) { none() }

/**
* Gets an additional term that is added to the `join` and `branch` computations to reflect
* an additional forward or backwards branching factor that is not taken into account
* when calculating the (virtual) dispatch cost.
*
* Argument `arg` is part of a path from a source to a sink, and `p` is the target parameter.
*/
default int getAdditionalFlowIntoCallNodeTerm(ArgumentNode arg, ParameterNode p) { none() }

/**
* A second-level control-flow scope in a callable.
*
Expand Down
31 changes: 10 additions & 21 deletions shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll
Expand Up @@ -1102,17 +1102,6 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
not inBarrier(p)
}

/**
* Gets an additional term that is added to `branch` and `join` when deciding whether
* the amount of forward or backward branching is within the limit specified by the
* configuration.
*/
pragma[nomagic]
private int getLanguageSpecificFlowIntoCallNodeCand1(ArgNodeEx arg, ParamNodeEx p) {
flowIntoCallNodeCand1(_, arg, p) and
result = getAdditionalFlowIntoCallNodeTerm(arg.projectToNode(), p.projectToNode())
}

private module SndLevelScopeOption = Option<DataFlowSecondLevelScope>;

private class SndLevelScopeOption = SndLevelScopeOption::Option;
Expand Down Expand Up @@ -1177,11 +1166,11 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
private int branch(ArgNodeEx n1) {
result =
strictcount(DataFlowCallable c |
exists(NodeEx n |
flowIntoCallNodeCand1(_, n1, n) and
c = n.getEnclosingCallable()
)
) + sum(ParamNodeEx p1 | | getLanguageSpecificFlowIntoCallNodeCand1(n1, p1))
exists(NodeEx n |
flowIntoCallNodeCand1(_, n1, n) and
c = n.getEnclosingCallable()
)
)
}

/**
Expand All @@ -1193,11 +1182,11 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
private int join(ParamNodeEx n2) {
result =
strictcount(DataFlowCallable c |
exists(NodeEx n |
flowIntoCallNodeCand1(_, n, n2) and
c = n.getEnclosingCallable()
)
) + sum(ArgNodeEx arg2 | | getLanguageSpecificFlowIntoCallNodeCand1(arg2, n2))
exists(NodeEx n |
flowIntoCallNodeCand1(_, n, n2) and
c = n.getEnclosingCallable()
)
)
}

/**
Expand Down

0 comments on commit d396a0b

Please sign in to comment.