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: Implement call context grouping to improve performance #16456

Merged
merged 25 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
486eaad
Shared: Add MakeSets module.
aschackmull Apr 25, 2024
bc8ca1a
Dataflow: Introduce NodeRegions for use in isUnreachableInCall.
aschackmull Apr 25, 2024
012e1b4
Dataflow: Remove duplicate definitions
aschackmull Apr 26, 2024
86e6d0b
Dataflow: Switch local call contexts to use canonical representative.
aschackmull Apr 26, 2024
eb0b923
Dataflow: Switch column order in viableImplCallContextReducedReverse.
aschackmull May 1, 2024
79b1cd7
Dataflow: Refactor getLocalCc to avoid reference to NodeEx.
aschackmull May 1, 2024
b2e3d78
Dataflow: Share getCallContextReturn in DataFlowImplCommon::CallConte…
aschackmull May 1, 2024
ace369f
Dataflow: Share getCallContextCall in DataFlowImplCommon::CallContext…
aschackmull May 1, 2024
aa87243
Dataflow: Rename prunedViableImplInCallContext to viableImplCallConte…
aschackmull May 1, 2024
0561c65
Dataflow: Rename noPrunedViableImplInCallContext to viableImplNotCall…
aschackmull May 1, 2024
740bb84
Dataflow: Rename prunedViableImplInCallContextReverse to viableImplCa…
aschackmull May 1, 2024
5ac96d0
Dataflow: Move viableImplNotCallContextReducedReverse to DataFlowImpl…
aschackmull May 1, 2024
f6eb82e
Dataflow: Simplify.
aschackmull May 2, 2024
e1e6cd9
Dataflow: Simplify: remove Level1CallContextInput module
aschackmull May 2, 2024
1dd1f12
Dataflow: Move Level1CallContext to DataFlowImplCommon
aschackmull May 2, 2024
947c2bf
Dataflow: Move two declarations.
aschackmull May 2, 2024
52a232e
Dataflow: Make CallContext type private to DataFlowImplCommon.
aschackmull May 2, 2024
b83416f
Dataflow: Make two predicates private.
aschackmull May 2, 2024
972b81b
Util: Allow best-effort total orders with a reasonable fallback.
aschackmull May 8, 2024
5a25984
Dataflow: Switch call context to a set representation.
aschackmull May 8, 2024
1432519
Dataflow: Add totalorder predicates to all languages.
aschackmull May 8, 2024
5c635e9
C++/C#/Java: Update expected output.
aschackmull May 15, 2024
8085460
C++/Shared: Fix join order issues.
aschackmull May 15, 2024
4ff37cc
Dataflow: Address review comments.
aschackmull May 24, 2024
51c48c7
Dataflow: Address review comments (take 2).
aschackmull May 24, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ private import DataFlowUtil
/**
* Gets a function that might be called by `call`.
*/
Function viableCallable(DataFlowCall call) {
DataFlowCallable viableCallable(DataFlowCall call) {
result = call.(Call).getTarget()
or
// If the target of the call does not have a body in the snapshot, it might
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,17 @@ class CastNode extends Node {
CastNode() { none() } // stub implementation
}

class DataFlowCallable = Function;
class DataFlowCallable extends Function {
/** Gets a best-effort total ordering. */
int totalorder() {
this =
rank[result](DataFlowCallable c, string file, int startline, int startcolumn |
c.getLocation().hasLocationInfo(file, startline, startcolumn, _, _)
|
c order by file, startline, startcolumn
)
}
}

class DataFlowExpr = Expr;

Expand All @@ -261,10 +271,28 @@ class DataFlowCall extends Expr instanceof Call {
ExprNode getNode() { result.getExpr() = this }

/** Gets the enclosing callable of this call. */
Function getEnclosingCallable() { result = this.getEnclosingFunction() }
DataFlowCallable getEnclosingCallable() { result = this.getEnclosingFunction() }

/** Gets a best-effort total ordering. */
int totalorder() {
this =
rank[result](DataFlowCall c, int startline, int startcolumn |
c.getLocation().hasLocationInfo(_, startline, startcolumn, _, _)
|
c order by startline, startcolumn
)
}
}

class NodeRegion instanceof Unit {
string toString() { result = "NodeRegion" }

predicate contains(Node n) { none() }

int totalOrder() { result = 1 }
}

predicate isUnreachableInCall(Node n, DataFlowCall call) { none() } // stub implementation
predicate isUnreachableInCall(NodeRegion nr, DataFlowCall call) { none() } // stub implementation

/**
* Holds if access paths with `c` at their head always should be tracked at high
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,16 @@ class DataFlowCallable extends TDataFlowCallable {
result = this.asSummarizedCallable() or // SummarizedCallable = Function (in CPP)
result = this.asSourceCallable()
}

/** Gets a best-effort total ordering. */
int totalorder() {
this =
rank[result](DataFlowCallable c, string file, int startline, int startcolumn |
c.getLocation().hasLocationInfo(file, startline, startcolumn, _, _)
|
c order by file, startline, startcolumn
)
}
}

/**
Expand Down Expand Up @@ -1159,6 +1169,16 @@ class DataFlowCall extends TDataFlowCall {
* Gets the location of this call.
*/
Location getLocation() { none() }

/** Gets a best-effort total ordering. */
int totalorder() {
this =
rank[result](DataFlowCall c, int startline, int startcolumn |
c.getLocation().hasLocationInfo(_, startline, startcolumn, _, _)
|
c order by startline, startcolumn
)
}
}

/**
Expand Down Expand Up @@ -1247,43 +1267,53 @@ module IsUnreachableInCall {
any(G::IRGuardCondition guard).ensuresLt(left, right, k, block, areEqual)
}

predicate isUnreachableInCall(Node n, DataFlowCall call) {
class NodeRegion instanceof IRBlock {
string toString() { result = "NodeRegion" }
Comment on lines +1270 to +1271
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final extends?


predicate contains(Node n) { this = n.getBasicBlock() }

int totalOrder() {
this =
rank[result](IRBlock b, int startline, int startcolumn |
b.getLocation().hasLocationInfo(_, startline, startcolumn, _, _)
|
b order by startline, startcolumn
)
}
}

predicate isUnreachableInCall(NodeRegion block, DataFlowCall call) {
exists(
InstructionDirectParameterNode paramNode, ConstantIntegralTypeArgumentNode arg,
IntegerConstantInstruction constant, int k, Operand left, Operand right, IRBlock block
IntegerConstantInstruction constant, int k, Operand left, Operand right, int argval
|
// arg flows into `paramNode`
DataFlowImplCommon::viableParamArg(call, paramNode, arg) and
DataFlowImplCommon::viableParamArg(call, pragma[only_bind_into](paramNode),
pragma[only_bind_into](arg)) and
left = constant.getAUse() and
right = valueNumber(paramNode.getInstruction()).getAUse() and
block = n.getBasicBlock()
argval = arg.getValue()
|
// and there's a guard condition which ensures that the result of `left == right + k` is `areEqual`
exists(boolean areEqual |
ensuresEq(pragma[only_bind_into](left), pragma[only_bind_into](right),
pragma[only_bind_into](k), pragma[only_bind_into](block), areEqual)
|
exists(boolean areEqual | ensuresEq(left, right, k, block, areEqual) |
// this block ensures that left = right + k, but it holds that `left != right + k`
areEqual = true and
constant.getValue().toInt() != arg.getValue() + k
constant.getValue().toInt() != argval + k
or
// this block ensures that or `left != right + k`, but it holds that `left = right + k`
areEqual = false and
constant.getValue().toInt() = arg.getValue() + k
constant.getValue().toInt() = argval + k
)
or
// or there's a guard condition which ensures that the result of `left < right + k` is `isLessThan`
exists(boolean isLessThan |
ensuresLt(pragma[only_bind_into](left), pragma[only_bind_into](right),
pragma[only_bind_into](k), pragma[only_bind_into](block), isLessThan)
|
exists(boolean isLessThan | ensuresLt(left, right, k, block, isLessThan) |
isLessThan = true and
// this block ensures that `left < right + k`, but it holds that `left >= right + k`
constant.getValue().toInt() >= arg.getValue() + k
constant.getValue().toInt() >= argval + k
or
// this block ensures that `left >= right + k`, but it holds that `left < right + k`
isLessThan = false and
constant.getValue().toInt() < arg.getValue() + k
constant.getValue().toInt() < argval + k
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ edges
| test.cpp:289:19:289:25 | buffer3 | test.cpp:277:35:277:35 | p | provenance | |
| test.cpp:289:19:289:25 | buffer3 | test.cpp:289:19:289:25 | buffer3 | provenance | |
| test.cpp:292:25:292:27 | arr | test.cpp:299:16:299:21 | access to array | provenance | Config |
| test.cpp:292:25:292:27 | arr | test.cpp:299:16:299:21 | access to array | provenance | Config |
| test.cpp:306:20:306:23 | arr1 | test.cpp:292:25:292:27 | arr | provenance | |
| test.cpp:306:20:306:23 | arr1 | test.cpp:306:20:306:23 | arr1 | provenance | |
| test.cpp:309:20:309:23 | arr2 | test.cpp:292:25:292:27 | arr | provenance | |
Expand Down Expand Up @@ -159,7 +158,6 @@ nodes
| test.cpp:289:19:289:25 | buffer3 | semmle.label | buffer3 |
| test.cpp:289:19:289:25 | buffer3 | semmle.label | buffer3 |
| test.cpp:292:25:292:27 | arr | semmle.label | arr |
| test.cpp:292:25:292:27 | arr | semmle.label | arr |
| test.cpp:299:16:299:21 | access to array | semmle.label | access to array |
| test.cpp:306:20:306:23 | arr1 | semmle.label | arr1 |
| test.cpp:306:20:306:23 | arr1 | semmle.label | arr1 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,16 @@ class DataFlowCallable extends TDataFlowCallable {
or
result = this.asCapturedVariable().getLocation()
}

/** Gets a best-effort total ordering. */
int totalorder() {
this =
rank[result](DataFlowCallable c, string file, int startline, int startcolumn |
c.getLocation().hasLocationInfo(file, startline, startcolumn, _, _)
|
c order by file, startline, startcolumn
)
}
}

/** A call relevant for data flow. */
Expand Down Expand Up @@ -234,6 +244,16 @@ abstract class DataFlowCall extends TDataFlowCall {
) {
this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
}

/** Gets a best-effort total ordering. */
int totalorder() {
this =
rank[result](DataFlowCall c, int startline, int startcolumn |
c.hasLocationInfo(_, startline, startcolumn, _, _)
|
c order by startline, startcolumn
)
}
}

/** A non-delegate C# call relevant for data flow. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2380,16 +2380,31 @@ predicate expectsContent(Node n, ContentSet c) {
n.asExpr() instanceof SpreadElementExpr and c instanceof ElementContent
}

class NodeRegion instanceof ControlFlow::BasicBlock {
string toString() { result = "NodeRegion" }
Comment on lines +2383 to +2384
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final extends?


predicate contains(Node n) { this = n.getControlFlowNode().getBasicBlock() }

int totalOrder() {
this =
rank[result](ControlFlow::BasicBlock b, int startline, int startcolumn |
b.getLocation().hasLocationInfo(_, startline, startcolumn, _, _)
|
b order by startline, startcolumn
)
}
}

/**
* Holds if the node `n` is unreachable when the call context is `call`.
* Holds if the nodes in `nr` are unreachable when the call context is `call`.
*/
predicate isUnreachableInCall(Node n, DataFlowCall call) {
predicate isUnreachableInCall(NodeRegion nr, DataFlowCall call) {
exists(
ExplicitParameterNode paramNode, Guard guard, ControlFlow::SuccessorTypes::BooleanSuccessor bs
|
viableConstantBooleanParamArg(paramNode, bs.getValue().booleanNot(), call) and
paramNode.getSsaDefinition().getARead() = guard and
guard.controlsBlock(n.getControlFlowNode().getBasicBlock(), bs, _)
guard.controlsBlock(nr, bs, _)
)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
edges
| CallSensitivityFlow.cs:7:38:7:38 | o : Object | CallSensitivityFlow.cs:11:20:11:20 | access to parameter o : Object | provenance | |
| CallSensitivityFlow.cs:7:38:7:38 | o : Object | CallSensitivityFlow.cs:11:20:11:20 | access to parameter o : Object | provenance | |
| CallSensitivityFlow.cs:19:39:19:39 | o : Object | CallSensitivityFlow.cs:23:18:23:18 | access to parameter o | provenance | |
| CallSensitivityFlow.cs:27:40:27:40 | o : Object | CallSensitivityFlow.cs:31:18:31:18 | access to parameter o | provenance | |
| CallSensitivityFlow.cs:27:40:27:40 | o : Object | CallSensitivityFlow.cs:31:18:31:18 | access to parameter o | provenance | |
| CallSensitivityFlow.cs:35:41:35:41 | o : Object | CallSensitivityFlow.cs:39:18:39:18 | [cond (line 35): true] access to parameter o | provenance | |
| CallSensitivityFlow.cs:43:45:43:45 | o : Object | CallSensitivityFlow.cs:45:16:45:17 | access to local variable o1 : Object | provenance | |
| CallSensitivityFlow.cs:45:16:45:17 | access to local variable o1 : Object | CallSensitivityFlow.cs:49:20:49:22 | access to local variable tmp : Object | provenance | |
Expand All @@ -13,23 +11,18 @@ edges
| CallSensitivityFlow.cs:56:46:56:46 | o : Object | CallSensitivityFlow.cs:58:16:58:17 | access to local variable o1 : Object | provenance | |
| CallSensitivityFlow.cs:56:46:56:46 | o : Object | CallSensitivityFlow.cs:58:16:58:17 | access to local variable o1 : Object | provenance | |
| CallSensitivityFlow.cs:56:46:56:46 | o : Object | CallSensitivityFlow.cs:58:16:58:17 | access to local variable o1 : Object | provenance | |
| CallSensitivityFlow.cs:56:46:56:46 | o : Object | CallSensitivityFlow.cs:58:16:58:17 | access to local variable o1 : Object | provenance | |
| CallSensitivityFlow.cs:58:16:58:17 | access to local variable o1 : Object | CallSensitivityFlow.cs:62:20:62:22 | access to local variable tmp : Object | provenance | |
| CallSensitivityFlow.cs:58:16:58:17 | access to local variable o1 : Object | CallSensitivityFlow.cs:62:20:62:22 | access to local variable tmp : Object | provenance | |
| CallSensitivityFlow.cs:58:16:58:17 | access to local variable o1 : Object | CallSensitivityFlow.cs:62:20:62:22 | access to local variable tmp : Object | provenance | |
| CallSensitivityFlow.cs:58:16:58:17 | access to local variable o1 : Object | CallSensitivityFlow.cs:62:20:62:22 | access to local variable tmp : Object | provenance | |
| CallSensitivityFlow.cs:62:20:62:22 | access to local variable tmp : Object | CallSensitivityFlow.cs:63:13:63:14 | access to local variable o2 : Object | provenance | |
| CallSensitivityFlow.cs:62:20:62:22 | access to local variable tmp : Object | CallSensitivityFlow.cs:63:13:63:14 | access to local variable o2 : Object | provenance | |
| CallSensitivityFlow.cs:62:20:62:22 | access to local variable tmp : Object | CallSensitivityFlow.cs:63:13:63:14 | access to local variable o2 : Object | provenance | |
| CallSensitivityFlow.cs:62:20:62:22 | access to local variable tmp : Object | CallSensitivityFlow.cs:63:13:63:14 | access to local variable o2 : Object | provenance | |
| CallSensitivityFlow.cs:63:13:63:14 | access to local variable o2 : Object | CallSensitivityFlow.cs:65:16:65:17 | access to local variable o3 : Object | provenance | |
| CallSensitivityFlow.cs:63:13:63:14 | access to local variable o2 : Object | CallSensitivityFlow.cs:65:16:65:17 | access to local variable o3 : Object | provenance | |
| CallSensitivityFlow.cs:63:13:63:14 | access to local variable o2 : Object | CallSensitivityFlow.cs:65:16:65:17 | access to local variable o3 : Object | provenance | |
| CallSensitivityFlow.cs:63:13:63:14 | access to local variable o2 : Object | CallSensitivityFlow.cs:65:16:65:17 | access to local variable o3 : Object | provenance | |
| CallSensitivityFlow.cs:65:16:65:17 | access to local variable o3 : Object | CallSensitivityFlow.cs:66:14:66:15 | access to local variable o3 | provenance | |
| CallSensitivityFlow.cs:65:16:65:17 | access to local variable o3 : Object | CallSensitivityFlow.cs:66:14:66:15 | access to local variable o3 | provenance | |
| CallSensitivityFlow.cs:65:16:65:17 | access to local variable o3 : Object | CallSensitivityFlow.cs:66:14:66:15 | access to local variable o3 | provenance | |
| CallSensitivityFlow.cs:65:16:65:17 | access to local variable o3 : Object | CallSensitivityFlow.cs:66:14:66:15 | access to local variable o3 | provenance | |
| CallSensitivityFlow.cs:78:24:78:35 | object creation of type Object : Object | CallSensitivityFlow.cs:19:39:19:39 | o : Object | provenance | |
| CallSensitivityFlow.cs:79:25:79:36 | object creation of type Object : Object | CallSensitivityFlow.cs:27:40:27:40 | o : Object | provenance | |
| CallSensitivityFlow.cs:80:26:80:37 | object creation of type Object : Object | CallSensitivityFlow.cs:35:41:35:41 | o : Object | provenance | |
Expand Down Expand Up @@ -68,13 +61,10 @@ edges
| CallSensitivityFlow.cs:205:40:205:40 | o : Object | CallSensitivityFlow.cs:208:18:208:18 | access to parameter o | provenance | |
nodes
| CallSensitivityFlow.cs:7:38:7:38 | o : Object | semmle.label | o : Object |
| CallSensitivityFlow.cs:7:38:7:38 | o : Object | semmle.label | o : Object |
| CallSensitivityFlow.cs:11:20:11:20 | access to parameter o : Object | semmle.label | access to parameter o : Object |
| CallSensitivityFlow.cs:11:20:11:20 | access to parameter o : Object | semmle.label | access to parameter o : Object |
| CallSensitivityFlow.cs:19:39:19:39 | o : Object | semmle.label | o : Object |
| CallSensitivityFlow.cs:23:18:23:18 | access to parameter o | semmle.label | access to parameter o |
| CallSensitivityFlow.cs:27:40:27:40 | o : Object | semmle.label | o : Object |
| CallSensitivityFlow.cs:27:40:27:40 | o : Object | semmle.label | o : Object |
| CallSensitivityFlow.cs:31:18:31:18 | access to parameter o | semmle.label | access to parameter o |
| CallSensitivityFlow.cs:35:41:35:41 | o : Object | semmle.label | o : Object |
| CallSensitivityFlow.cs:39:18:39:18 | [cond (line 35): true] access to parameter o | semmle.label | [cond (line 35): true] access to parameter o |
Expand All @@ -87,23 +77,18 @@ nodes
| CallSensitivityFlow.cs:56:46:56:46 | o : Object | semmle.label | o : Object |
| CallSensitivityFlow.cs:56:46:56:46 | o : Object | semmle.label | o : Object |
| CallSensitivityFlow.cs:56:46:56:46 | o : Object | semmle.label | o : Object |
| CallSensitivityFlow.cs:56:46:56:46 | o : Object | semmle.label | o : Object |
| CallSensitivityFlow.cs:58:16:58:17 | access to local variable o1 : Object | semmle.label | access to local variable o1 : Object |
| CallSensitivityFlow.cs:58:16:58:17 | access to local variable o1 : Object | semmle.label | access to local variable o1 : Object |
| CallSensitivityFlow.cs:58:16:58:17 | access to local variable o1 : Object | semmle.label | access to local variable o1 : Object |
| CallSensitivityFlow.cs:58:16:58:17 | access to local variable o1 : Object | semmle.label | access to local variable o1 : Object |
| CallSensitivityFlow.cs:62:20:62:22 | access to local variable tmp : Object | semmle.label | access to local variable tmp : Object |
| CallSensitivityFlow.cs:62:20:62:22 | access to local variable tmp : Object | semmle.label | access to local variable tmp : Object |
| CallSensitivityFlow.cs:62:20:62:22 | access to local variable tmp : Object | semmle.label | access to local variable tmp : Object |
| CallSensitivityFlow.cs:62:20:62:22 | access to local variable tmp : Object | semmle.label | access to local variable tmp : Object |
| CallSensitivityFlow.cs:63:13:63:14 | access to local variable o2 : Object | semmle.label | access to local variable o2 : Object |
| CallSensitivityFlow.cs:63:13:63:14 | access to local variable o2 : Object | semmle.label | access to local variable o2 : Object |
| CallSensitivityFlow.cs:63:13:63:14 | access to local variable o2 : Object | semmle.label | access to local variable o2 : Object |
| CallSensitivityFlow.cs:63:13:63:14 | access to local variable o2 : Object | semmle.label | access to local variable o2 : Object |
| CallSensitivityFlow.cs:65:16:65:17 | access to local variable o3 : Object | semmle.label | access to local variable o3 : Object |
| CallSensitivityFlow.cs:65:16:65:17 | access to local variable o3 : Object | semmle.label | access to local variable o3 : Object |
| CallSensitivityFlow.cs:65:16:65:17 | access to local variable o3 : Object | semmle.label | access to local variable o3 : Object |
| CallSensitivityFlow.cs:65:16:65:17 | access to local variable o3 : Object | semmle.label | access to local variable o3 : Object |
| CallSensitivityFlow.cs:66:14:66:15 | access to local variable o3 | semmle.label | access to local variable o3 |
| CallSensitivityFlow.cs:78:24:78:35 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
| CallSensitivityFlow.cs:79:25:79:36 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
Expand Down