Skip to content

Commit 03e91a2

Browse files
committed
API graphs: Performance fixes
1 parent ae70af0 commit 03e91a2

File tree

1 file changed

+21
-12
lines changed

1 file changed

+21
-12
lines changed

ql/lib/codeql/ruby/ApiGraphs.qll

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -277,50 +277,56 @@ module API {
277277
not exists(read.getScopeExpr())
278278
)
279279
or
280-
exists(DataFlow::LocalSourceNode src, DataFlow::LocalSourceNode pred |
280+
exists(ExprCfgNode node |
281281
// First, we find a predecessor of the node `ref` that we want to determine. The predecessor
282282
// is any node that is a type-tracked use of a data flow node (`src`), which is itself a
283283
// reference to the API node `base`. Thus, `pred` and `src` both represent uses of `base`.
284284
//
285285
// Once we have identified the predecessor, we define its relation to the successor `ref` as
286286
// well as the label on the edge from `pred` to `ref`. This label describes the nature of
287287
// the relationship between `pred` and `ref`.
288-
use(base, src) and pred = trackUseNode(src)
288+
useExpr(node, base)
289289
|
290290
// // Referring to an attribute on a node that is a use of `base`:
291291
// pred = `Rails` part of `Rails::Whatever`
292292
// lbl = `Whatever`
293293
// ref = `Rails::Whatever`
294-
exists(ExprNodes::ConstantAccessCfgNode c, DataFlow::Node node, ConstantReadAccess read |
294+
exists(ExprNodes::ConstantAccessCfgNode c, ConstantReadAccess read |
295295
not exists(resolveTopLevel(read)) and
296-
pred.flowsTo(node) and
297-
node.asExpr() = c.getScopeExpr() and
296+
node = c.getScopeExpr() and
298297
lbl = Label::member(read.getName()) and
299298
ref.asExpr() = c and
300299
read = c.getExpr()
301300
)
302301
or
303302
// Calling a method on a node that is a use of `base`
304-
exists(ExprNodes::MethodCallCfgNode call, DataFlow::Node node, string name |
305-
pred.flowsTo(node) and
306-
node.asExpr() = call.getReceiver() and
303+
exists(ExprNodes::MethodCallCfgNode call, string name |
304+
node = call.getReceiver() and
307305
name = call.getExpr().getMethodName() and
308306
lbl = Label::return(name) and
309307
name != "new" and
310308
ref.asExpr() = call
311309
)
312310
or
313311
// Calling the `new` method on a node that is a use of `base`, which creates a new instance
314-
exists(ExprNodes::MethodCallCfgNode call, DataFlow::Node node |
315-
pred.flowsTo(node) and
316-
node.asExpr() = call.getReceiver() and
312+
exists(ExprNodes::MethodCallCfgNode call |
313+
node = call.getReceiver() and
317314
lbl = Label::instance() and
318315
call.getExpr().getMethodName() = "new" and
319316
ref.asExpr() = call
320317
)
321318
)
322319
}
323320

321+
pragma[nomagic]
322+
private predicate useExpr(ExprCfgNode node, TApiNode base) {
323+
exists(DataFlow::LocalSourceNode src, DataFlow::LocalSourceNode pred |
324+
use(base, src) and
325+
pred = trackUseNode(src) and
326+
pred.flowsTo(any(DataFlow::ExprNode n | n.getExprNode() = node))
327+
)
328+
}
329+
324330
/**
325331
* Holds if `ref` is a use of node `nd`.
326332
*/
@@ -332,7 +338,10 @@ module API {
332338
*
333339
* The flow from `src` to that node may be inter-procedural.
334340
*/
335-
private DataFlow::LocalSourceNode trackUseNode(DataFlow::LocalSourceNode src, TypeTracker t) {
341+
private DataFlow::LocalSourceNode trackUseNode(DataFlow::Node src, TypeTracker t) {
342+
// Declaring `src` to be a `SourceNode` currently causes a redundant check in the
343+
// recursive case, so instead we check it explicitly here.
344+
src instanceof DataFlow::LocalSourceNode and
336345
t.start() and
337346
use(_, src) and
338347
result = src

0 commit comments

Comments
 (0)