Skip to content

Commit 4f6656a

Browse files
hvitvedhvitved
committed
Ruby: Fix join orders following DB stats removal
1 parent 750f1ae commit 4f6656a

7 files changed

Lines changed: 99 additions & 66 deletions

File tree

‎ruby/ql/lib/codeql/ruby/ast/internal/Module.qll‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,7 @@ private TMethodOrExpr lookupMethodOrConst(Module m, string name) {
639639
// For now, we restrict the scope of top-level declarations to their file.
640640
// This may remove some plausible targets, but also removes a lot of
641641
// implausible targets
642-
if getNode(result).getEnclosingModule() instanceof Toplevel
643-
then getNode(result).getFile() = m.getADeclaration().getFile()
644-
else any()
642+
forall(File file | file = getNode(result).getEnclosingModule().(Toplevel).getFile() |
643+
file = m.getADeclaration().getFile()
644+
)
645645
}

‎ruby/ql/lib/codeql/ruby/ast/internal/Scope.qll‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,11 @@ cached
149149
private module Cached {
150150
/** Gets the enclosing scope of a node */
151151
cached
152-
Scope::Range scopeOf(Ruby::AstNode n) {
152+
Scope::Range scopeOfImpl(Ruby::AstNode n) {
153153
exists(Ruby::AstNode p | p = parentOf(n) |
154154
p = result
155155
or
156-
not p instanceof Scope::Range and result = scopeOf(p)
156+
not p instanceof Scope::Range and result = scopeOfImpl(p)
157157
)
158158
}
159159

@@ -163,16 +163,22 @@ private module Cached {
163163
* and synthesized scopes into account.
164164
*/
165165
cached
166-
Scope scopeOfInclSynth(AstNode n) {
166+
Scope scopeOfInclSynthImpl(AstNode n) {
167167
exists(AstNode p | p = parentOfInclSynth(n) |
168168
p = result
169169
or
170-
not p instanceof Scope and result = scopeOfInclSynth(p)
170+
not p instanceof Scope and result = scopeOfInclSynthImpl(p)
171171
)
172172
}
173173
}
174174

175-
import Cached
175+
bindingset[n]
176+
pragma[inline_late]
177+
Scope::Range scopeOf(Ruby::AstNode n) { result = Cached::scopeOfImpl(n) }
178+
179+
bindingset[n]
180+
pragma[inline_late]
181+
Scope scopeOfInclSynth(AstNode n) { result = Cached::scopeOfInclSynthImpl(n) }
176182

177183
abstract class ScopeImpl extends AstNode, TScopeType {
178184
final Scope getOuterScopeImpl() { result = scopeOfInclSynth(this) }

‎ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll‎

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -687,26 +687,30 @@ pragma[nomagic]
687687
private CfgScope getTargetInstance(DataFlowCall call, string method) {
688688
exists(boolean exact |
689689
result = lookupInstanceMethodCall(call, method, exact) and
690-
(
691-
if result.(Method).isPrivate()
692-
then
693-
call.asCall().getReceiver().getExpr() instanceof SelfVariableAccess and
694-
// For now, we restrict the scope of top-level declarations to their file.
695-
// This may remove some plausible targets, but also removes a lot of
696-
// implausible targets
697-
(
698-
isToplevelMethodInFile(result, call.asCall().getFile()) or
699-
not isToplevelMethodInFile(result, _)
700-
)
701-
else any()
702-
) and
703-
if result.(Method).isProtected()
704-
then
705-
result = lookupMethod(call.asCall().getExpr().getEnclosingModule().getModule(), method, exact)
706-
else any()
690+
(if result.(Method).isPrivate() then result = privateFilter(call) else any()) and
691+
if result.(Method).isProtected() then result = protectedFilter(call, method, exact) else any()
707692
)
708693
}
709694

695+
bindingset[call, result]
696+
pragma[inline_late]
697+
private CfgScope privateFilter(DataFlowCall call) {
698+
call.asCall().getReceiver().getExpr() instanceof SelfVariableAccess and
699+
// For now, we restrict the scope of top-level declarations to their file.
700+
// This may remove some plausible targets, but also removes a lot of
701+
// implausible targets
702+
(
703+
isToplevelMethodInFile(result, call.asCall().getFile()) or
704+
not isToplevelMethodInFile(result, _)
705+
)
706+
}
707+
708+
bindingset[call, method, exact, result]
709+
pragma[inline_late]
710+
private CfgScope protectedFilter(DataFlowCall call, string method, boolean exact) {
711+
result = lookupMethod(call.asCall().getExpr().getEnclosingModule().getModule(), method, exact)
712+
}
713+
710714
private module TrackBlockInput implements CallGraphConstruction::Simple::InputSig {
711715
class State = Block;
712716

‎ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll‎

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,13 @@ abstract class NodeImpl extends Node {
2828
DataFlowCallable getEnclosingCallable() { result = TCfgScope(this.getCfgScope()) }
2929

3030
/** Do not call: use `getEnclosingCallable()` instead. */
31-
abstract CfgScope getCfgScope();
31+
abstract CfgScope getCfgScopeImpl();
32+
33+
/** Do not call: use `getEnclosingCallable()` instead. */
34+
pragma[inline]
35+
final CfgScope getCfgScope() {
36+
pragma[only_bind_into](result) = pragma[only_bind_out](this).getCfgScopeImpl()
37+
}
3238

3339
/** Do not call: use `getLocation()` instead. */
3440
abstract Location getLocationImpl();
@@ -38,7 +44,7 @@ abstract class NodeImpl extends Node {
3844
}
3945

4046
private class ExprNodeImpl extends ExprNode, NodeImpl {
41-
override CfgScope getCfgScope() { result = this.getExprNode().getExpr().getCfgScope() }
47+
override CfgScope getCfgScopeImpl() { result = this.getExprNode().getExpr().getCfgScope() }
4248

4349
override Location getLocationImpl() { result = this.getExprNode().getLocation() }
4450

@@ -780,7 +786,7 @@ class SsaNode extends NodeImpl, TSsaNode {
780786
/** Holds if this node should be hidden from path explanations. */
781787
predicate isHidden() { any() }
782788

783-
override CfgScope getCfgScope() { result = node.getBasicBlock().getScope() }
789+
override CfgScope getCfgScopeImpl() { result = node.getBasicBlock().getScope() }
784790

785791
override Location getLocationImpl() { result = node.getLocation() }
786792

@@ -827,7 +833,7 @@ class CapturedVariableNode extends NodeImpl, TCapturedVariableNode {
827833
/** Gets the captured variable represented by this node. */
828834
VariableCapture::CapturedVariable getVariable() { result = variable }
829835

830-
override CfgScope getCfgScope() { result = variable.getCallable() }
836+
override CfgScope getCfgScopeImpl() { result = variable.getCallable() }
831837

832838
override Location getLocationImpl() { result = variable.getLocation() }
833839

@@ -849,7 +855,7 @@ class ReturningStatementNode extends NodeImpl, TReturningNode {
849855
/** Gets the expression corresponding to this node. */
850856
CfgNodes::ReturningCfgNode getReturningNode() { result = n }
851857

852-
override CfgScope getCfgScope() { result = n.getScope() }
858+
override CfgScope getCfgScopeImpl() { result = n.getScope() }
853859

854860
override Location getLocationImpl() { result = n.getLocation() }
855861

@@ -867,7 +873,7 @@ class CaptureNode extends NodeImpl, TCaptureNode {
867873

868874
VariableCapture::Flow::SynthesizedCaptureNode getSynthesizedCaptureNode() { result = cn }
869875

870-
override CfgScope getCfgScope() { result = cn.getEnclosingCallable() }
876+
override CfgScope getCfgScopeImpl() { result = cn.getEnclosingCallable() }
871877

872878
override Location getLocationImpl() { result = cn.getLocation() }
873879

@@ -935,7 +941,7 @@ private module ParameterNodes {
935941
)
936942
}
937943

938-
override CfgScope getCfgScope() { result = parameter.getCallable() }
944+
override CfgScope getCfgScopeImpl() { result = parameter.getCallable() }
939945

940946
override Location getLocationImpl() { result = parameter.getLocation() }
941947

@@ -979,7 +985,7 @@ private module ParameterNodes {
979985

980986
final override SelfVariable getSelfVariable() { result.getDeclaringScope() = method }
981987

982-
override CfgScope getCfgScope() { result = method }
988+
override CfgScope getCfgScopeImpl() { result = method }
983989

984990
override Location getLocationImpl() { result = method.getLocation() }
985991
}
@@ -1001,7 +1007,7 @@ private module ParameterNodes {
10011007

10021008
final override SelfVariable getSelfVariable() { result.getDeclaringScope() = t }
10031009

1004-
override CfgScope getCfgScope() { result = t }
1010+
override CfgScope getCfgScopeImpl() { result = t }
10051011

10061012
override Location getLocationImpl() { result = t.getLocation() }
10071013
}
@@ -1028,7 +1034,7 @@ private module ParameterNodes {
10281034
callable = c.asCfgScope() and pos.isLambdaSelf()
10291035
}
10301036

1031-
override CfgScope getCfgScope() { result = callable }
1037+
override CfgScope getCfgScopeImpl() { result = callable }
10321038

10331039
override Location getLocationImpl() { result = callable.getLocation() }
10341040

@@ -1071,7 +1077,7 @@ private module ParameterNodes {
10711077
c.asCfgScope() = method and pos.isBlock()
10721078
}
10731079

1074-
override CfgScope getCfgScope() { result = method }
1080+
override CfgScope getCfgScopeImpl() { result = method }
10751081

10761082
override Location getLocationImpl() {
10771083
result = this.getParameter().getLocation()
@@ -1130,7 +1136,7 @@ private module ParameterNodes {
11301136
c = callable and pos.isSynthHashSplat()
11311137
}
11321138

1133-
final override CfgScope getCfgScope() { result = callable.asCfgScope() }
1139+
final override CfgScope getCfgScopeImpl() { result = callable.asCfgScope() }
11341140

11351141
final override DataFlowCallable getEnclosingCallable() { result = callable }
11361142

@@ -1193,7 +1199,7 @@ private module ParameterNodes {
11931199
)
11941200
}
11951201

1196-
final override CfgScope getCfgScope() { result = callable.asCfgScope() }
1202+
final override CfgScope getCfgScopeImpl() { result = callable.asCfgScope() }
11971203

11981204
final override DataFlowCallable getEnclosingCallable() { result = callable }
11991205

@@ -1240,7 +1246,7 @@ private module ParameterNodes {
12401246
cs = getArrayContent(pos)
12411247
}
12421248

1243-
final override CfgScope getCfgScope() { result = callable.asCfgScope() }
1249+
final override CfgScope getCfgScopeImpl() { result = callable.asCfgScope() }
12441250

12451251
final override DataFlowCallable getEnclosingCallable() { result = callable }
12461252

@@ -1278,7 +1284,7 @@ class FlowSummaryNode extends NodeImpl, TFlowSummaryNode {
12781284
result = this.getSummaryNode().getSummarizedCallable()
12791285
}
12801286

1281-
override CfgScope getCfgScope() { none() }
1287+
override CfgScope getCfgScopeImpl() { none() }
12821288

12831289
override DataFlowCallable getEnclosingCallable() {
12841290
result.asLibraryCallable() = this.getSummarizedCallable()
@@ -1349,7 +1355,7 @@ module ArgumentNodes {
13491355
this.sourceArgumentOf(call.asCall(), pos)
13501356
}
13511357

1352-
override CfgScope getCfgScope() { result = yield.getScope() }
1358+
override CfgScope getCfgScopeImpl() { result = yield.getScope() }
13531359

13541360
override Location getLocationImpl() { result = yield.getLocation() }
13551361
}
@@ -1379,7 +1385,7 @@ module ArgumentNodes {
13791385
this.sourceArgumentOf(call.asCall(), pos)
13801386
}
13811387

1382-
override CfgScope getCfgScope() { result = sup.getScope() }
1388+
override CfgScope getCfgScopeImpl() { result = sup.getScope() }
13831389

13841390
override Location getLocationImpl() { result = sup.getLocation() }
13851391
}
@@ -1415,7 +1421,7 @@ module ArgumentNodes {
14151421
this.sourceArgumentOf(call.asCall(), pos)
14161422
}
14171423

1418-
final override CfgScope getCfgScope() { result = call_.getExpr().getCfgScope() }
1424+
final override CfgScope getCfgScopeImpl() { result = call_.getExpr().getCfgScope() }
14191425

14201426
final override Location getLocationImpl() { result = call_.getLocation() }
14211427
}
@@ -1563,7 +1569,7 @@ module ArgumentNodes {
15631569
)
15641570
}
15651571

1566-
override CfgScope getCfgScope() { result = c.getExpr().getCfgScope() }
1572+
override CfgScope getCfgScopeImpl() { result = c.getExpr().getCfgScope() }
15671573

15681574
override Location getLocationImpl() { result = c.getLocation() }
15691575

@@ -2037,13 +2043,18 @@ private predicate compatibleTypesNonSymRefl(DataFlowType t1, DataFlowType t2) {
20372043
}
20382044

20392045
pragma[nomagic]
2046+
private predicate compatibleModules(Module m1, Module m2) {
2047+
exists(Module m3 |
2048+
m3.getAnAncestor() = m1 and
2049+
m3.getAnAncestor() = m2
2050+
)
2051+
}
2052+
20402053
private predicate compatibleModuleTypes(TModuleDataFlowType t1, TModuleDataFlowType t2) {
2041-
exists(Module m1, Module m2, Module m3 |
2054+
exists(Module m1, Module m2 |
2055+
compatibleModules(m1, m2) and
20422056
t1 = TModuleDataFlowType(m1) and
20432057
t2 = TModuleDataFlowType(m2)
2044-
|
2045-
m3.getAnAncestor() = m1 and
2046-
m3.getAnAncestor() = m2
20472058
)
20482059
}
20492060

@@ -2074,7 +2085,7 @@ private module PostUpdateNodes {
20742085

20752086
override ExprNode getPreUpdateNode() { e = result.getExprNode() }
20762087

2077-
override CfgScope getCfgScope() { result = e.getExpr().getCfgScope() }
2088+
override CfgScope getCfgScopeImpl() { result = e.getExpr().getCfgScope() }
20782089

20792090
override Location getLocationImpl() { result = e.getLocation() }
20802091

‎ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,7 @@ private module Persistence {
500500
class ActiveRecordAssociation extends DataFlow::CallNode {
501501
private ActiveRecordModelClass modelClass;
502502

503+
pragma[nomagic]
503504
ActiveRecordAssociation() {
504505
not exists(this.asExpr().getExpr().getEnclosingMethod()) and
505506
this.asExpr().getExpr().getEnclosingModule() = modelClass and
@@ -583,6 +584,14 @@ private string pluralize(string input) {
583584
exists(string stem | stem + "s" = input) and result = input
584585
}
585586

587+
pragma[nomagic]
588+
private predicate activeRecordAssociationMethodCall(
589+
DataFlow::CallNode call, ActiveRecordAssociation assoc, string model
590+
) {
591+
call.getReceiver().(ActiveRecordInstance).getClass() = assoc.getSourceClass() and
592+
model = assoc.getTargetModelName()
593+
}
594+
586595
/**
587596
* A call to a method generated by an ActiveRecord association.
588597
* These yield ActiveRecord collection proxies, which act like collections but
@@ -595,24 +604,21 @@ private class ActiveRecordAssociationMethodCall extends DataFlow::CallNode {
595604
ActiveRecordAssociation assoc;
596605

597606
ActiveRecordAssociationMethodCall() {
598-
exists(string model | model = assoc.getTargetModelName() |
599-
this.getReceiver().(ActiveRecordInstance).getClass() = assoc.getSourceClass() and
607+
exists(string model | activeRecordAssociationMethodCall(this, assoc, model) |
608+
assoc.isCollection() and
600609
(
601-
assoc.isCollection() and
602-
(
603-
this.getMethodName() = pluralize(model) + ["", "="]
604-
or
605-
this.getMethodName() = "<<"
606-
or
607-
this.getMethodName() = model + ["_ids", "_ids="]
608-
)
610+
this.getMethodName() = pluralize(model) + ["", "="]
609611
or
610-
assoc.isSingular() and
611-
(
612-
this.getMethodName() = model + ["", "="] or
613-
this.getMethodName() = ["build_", "reload_"] + model or
614-
this.getMethodName() = "create_" + model + ["!", ""]
615-
)
612+
this.getMethodName() = "<<"
613+
or
614+
this.getMethodName() = model + ["_ids", "_ids="]
615+
)
616+
or
617+
assoc.isSingular() and
618+
(
619+
this.getMethodName() = model + ["", "="] or
620+
this.getMethodName() = ["build_", "reload_"] + model or
621+
this.getMethodName() = "create_" + model + ["!", ""]
616622
)
617623
)
618624
}

‎ruby/ql/lib/codeql/ruby/frameworks/actioncontroller/Filters.qll‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ module Filters {
4646
)
4747
}
4848

49+
bindingset[m, name]
50+
pragma[inline_late]
51+
private Method lookupMethodInlineLate(Module m, string name) { result = lookupMethod(m, name) }
52+
4953
/**
5054
* A call to a class method that adds or removes a filter from the callback chain.
5155
* This class exists to encapsulate common behavior between calls that
@@ -140,7 +144,8 @@ module Filters {
140144
*/
141145
Callable getAFilterCallable() {
142146
result =
143-
lookupMethod(this.getExpr().getEnclosingModule().getModule(), this.getFilterArgumentName())
147+
lookupMethodInlineLate(this.getExpr().getEnclosingModule().getModule(),
148+
this.getFilterArgumentName())
144149
}
145150
}
146151

‎ruby/ql/lib/codeql/ruby/frameworks/actiondispatch/internal/Routing.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,7 @@ module Routing {
943943
* Note: All-uppercase words like `CONSTANT` are not handled correctly.
944944
*/
945945
bindingset[base]
946+
pragma[inline_late]
946947
string underscore(string base) {
947948
base = "" and result = ""
948949
or

0 commit comments

Comments
 (0)