Skip to content
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.

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

DataFlow/C++: fieldFlowBranchLimit follow-up (2) #16303

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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