Skip to content

Commit 69f6e1e

Browse files
authored
Merge pull request #16010 from RasmusWL/perf
Python: Two small join-order fixes
2 parents fb4ed39 + 93f940a commit 69f6e1e

File tree

2 files changed

+29
-13
lines changed

2 files changed

+29
-13
lines changed

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -876,7 +876,7 @@ private module TrackAttrReadInput implements CallGraphConstruction::Simple::Inpu
876876

877877
predicate start(Node start, AttrRead attr) {
878878
start = attr and
879-
attr.getObject() in [
879+
pragma[only_bind_into](attr.getObject()) in [
880880
classTracker(_), classInstanceTracker(_), selfTracker(_), clsArgumentTracker(_),
881881
superCallNoArgumentTracker(_), superCallTwoArgumentTracker(_, _)
882882
]
@@ -1302,19 +1302,15 @@ predicate getCallArg(CallNode call, Function target, CallType type, Node arg, Ar
13021302
//
13031303
// call_func(my_obj.some_method)
13041304
// ```
1305-
exists(CfgNode cfgNode | cfgNode.getNode() = call |
1306-
cfgNode.getEnclosingCallable() = arg.getEnclosingCallable()
1307-
)
1305+
exists(CfgNode cfgNode | cfgNode.getNode() = call | sameEnclosingCallable(cfgNode, arg))
13081306
or
13091307
// cls argument for classmethod calls -- see note above about bound methods
13101308
type instanceof CallTypeClassMethod and
13111309
apos.isSelf() and
13121310
resolveMethodCall(call, target, type, arg) and
13131311
(arg = classTracker(_) or arg = clsArgumentTracker(_)) and
13141312
// dataflow lib has requirement that arguments and calls are in same enclosing callable.
1315-
exists(CfgNode cfgNode | cfgNode.getNode() = call |
1316-
cfgNode.getEnclosingCallable() = arg.getEnclosingCallable()
1317-
)
1313+
exists(CfgNode cfgNode | cfgNode.getNode() = call | sameEnclosingCallable(cfgNode, arg))
13181314
or
13191315
// normal arguments for method calls
13201316
(
@@ -1365,6 +1361,16 @@ predicate getCallArg(CallNode call, Function target, CallType type, Node arg, Ar
13651361
)
13661362
}
13671363

1364+
/**
1365+
* join-order helper for getCallArg, since otherwise we would do cartesian product of
1366+
* the enclosing callables
1367+
*/
1368+
bindingset[node1, node2]
1369+
pragma[inline_late]
1370+
private predicate sameEnclosingCallable(Node node1, Node node2) {
1371+
node1.getEnclosingCallable() = node2.getEnclosingCallable()
1372+
}
1373+
13681374
// =============================================================================
13691375
// DataFlowCall
13701376
// =============================================================================

python/ql/lib/semmle/python/frameworks/SqlAlchemy.qll

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,15 +113,25 @@ module SqlAlchemy {
113113
*/
114114
abstract class InstanceSource extends DataFlow::LocalSourceNode { }
115115

116+
/**
117+
* join-ordering helper for ConnectionConstruction char-pred -- without this would
118+
* start with _all_ `CallCfgNode` and join those with `MethodCallNode` .. which is
119+
* silly
120+
*/
121+
pragma[noinline]
122+
private DataFlow::MethodCallNode connectionConstruction_helper() {
123+
result.calls(Engine::instance(), ["begin", "connect"])
124+
or
125+
result.calls(instance(), ["connect", "execution_options"])
126+
}
127+
116128
private class ConnectionConstruction extends InstanceSource, DataFlow::CallCfgNode {
117129
ConnectionConstruction() {
118-
this = classRef().getACall()
119-
or
120-
this.(DataFlow::MethodCallNode).calls(Engine::instance(), ["begin", "connect"])
130+
// without the `pragma[only_bind_out]` we would start with joining
131+
// `API::Node.getACall` with `CallCfgNode` which is not optimal
132+
this = pragma[only_bind_out](classRef()).getACall()
121133
or
122-
this.(DataFlow::MethodCallNode).calls(instance(), "connect")
123-
or
124-
this.(DataFlow::MethodCallNode).calls(instance(), "execution_options")
134+
this = connectionConstruction_helper()
125135
}
126136
}
127137

0 commit comments

Comments
 (0)