Skip to content

Commit 5a6a52b

Browse files
committed
Ruby: Use fewer SynthSplatArgumentElementNodes
In cases such as def f(x, *y); end f(*[1, 2]) we don't need any `SynthSplatArgumentElementNodes`. We get flow from the splat argument to a `SynthSplatParameterNode` via `parameterMatch`, then from element 0 of the synth splat to the positional param `x` via a read step. We add a read step from element 1 to `SynthSplatParameterElementNode(1)`. From there we get flow to element 0 of `*y` via an existing store step.
1 parent 4c1beea commit 5a6a52b

File tree

3 files changed

+22
-33
lines changed

3 files changed

+22
-33
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1444,6 +1444,8 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
14441444
or
14451445
ppos.isSynthSplat() and apos.isSynthSplat()
14461446
or
1447+
ppos.isSynthSplat() and apos.isSplat(0)
1448+
or
14471449
apos.isSynthSplat() and ppos.isSynthArgSplat()
14481450
or
14491451
// Exact splat match

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,9 @@ private module Cached {
478478
TSynthSplatArgumentNode(CfgNodes::ExprNodes::CallCfgNode c) or
479479
TSynthSplatArgumentElementNode(CfgNodes::ExprNodes::CallCfgNode c, int n) {
480480
n in [-1 .. 10] and
481-
exists(Argument arg, ArgumentPosition pos | pos.isSplat(_) and arg.isArgumentOf(c, pos))
481+
exists(Argument arg, ArgumentPosition pos |
482+
pos.isSplat(any(int p | p > 0)) and arg.isArgumentOf(c, pos)
483+
)
482484
} or
483485
TCaptureNode(VariableCapture::Flow::SynthesizedCaptureNode cn)
484486

@@ -1666,10 +1668,6 @@ predicate synthSplatArgumentElementReadStep(
16661668
)
16671669
}
16681670

1669-
predicate readStepFromSynthSplatParam(SynthSplatParameterNode node1, ContentSet c, Node node2) {
1670-
readStep(node1, c, node2)
1671-
}
1672-
16731671
/**
16741672
* Holds if there is a read step of content `c` from `node1` to `node2`.
16751673
*/
@@ -1709,6 +1707,14 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
17091707
or
17101708
VariableCapture::readStep(node1, any(Content::CapturedVariableContent v | c.isSingleton(v)), node2)
17111709
or
1710+
// Read from SynthSplatParameterNode into SynthSplatParameterElementNode
1711+
// This models flow from elements in a splat argument to elements in a splat parameter, where there are preceding positional parameters.
1712+
node2 =
1713+
any(SynthSplatParameterElementNode e |
1714+
node1.(SynthSplatParameterNode).isParameterOf(e.getEnclosingCallable(), _) and
1715+
c = getPositionalContent(e.getReadPosition())
1716+
)
1717+
or
17121718
readStepCommon(node1, c, node2)
17131719
}
17141720

ruby/ql/test/library-tests/dataflow/params/params-flow.expected

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
testFailures
2+
failures
23
edges
34
| params_flow.rb:9:16:9:17 | p1 | params_flow.rb:10:10:10:11 | p1 |
45
| params_flow.rb:9:20:9:21 | p2 | params_flow.rb:11:10:11:11 | p2 |
@@ -50,10 +51,8 @@ edges
5051
| params_flow.rb:46:1:46:4 | args [element 1] | params_flow.rb:47:13:47:16 | args [element 1] |
5152
| params_flow.rb:46:9:46:17 | call to taint | params_flow.rb:46:1:46:4 | args [element 0] |
5253
| params_flow.rb:46:20:46:28 | call to taint | params_flow.rb:46:1:46:4 | args [element 1] |
53-
| params_flow.rb:47:1:47:17 | *[0] | params_flow.rb:9:16:9:17 | p1 |
54-
| params_flow.rb:47:1:47:17 | *[1] | params_flow.rb:9:20:9:21 | p2 |
55-
| params_flow.rb:47:12:47:16 | * ... [element 0] | params_flow.rb:47:1:47:17 | *[0] |
56-
| params_flow.rb:47:12:47:16 | * ... [element 1] | params_flow.rb:47:1:47:17 | *[1] |
54+
| params_flow.rb:47:12:47:16 | * ... [element 0] | params_flow.rb:9:16:9:17 | p1 |
55+
| params_flow.rb:47:12:47:16 | * ... [element 1] | params_flow.rb:9:20:9:21 | p2 |
5756
| params_flow.rb:47:13:47:16 | args [element 0] | params_flow.rb:47:12:47:16 | * ... [element 0] |
5857
| params_flow.rb:47:13:47:16 | args [element 1] | params_flow.rb:47:12:47:16 | * ... [element 1] |
5958
| params_flow.rb:49:13:49:14 | p1 | params_flow.rb:50:10:50:11 | p1 |
@@ -73,19 +72,15 @@ edges
7372
| params_flow.rb:60:1:60:4 | args [element 1] | params_flow.rb:61:10:61:13 | args [element 1] |
7473
| params_flow.rb:60:9:60:17 | call to taint | params_flow.rb:60:1:60:4 | args [element 0] |
7574
| params_flow.rb:60:20:60:28 | call to taint | params_flow.rb:60:1:60:4 | args [element 1] |
76-
| params_flow.rb:61:1:61:14 | *[0] | params_flow.rb:49:13:49:14 | p1 |
77-
| params_flow.rb:61:1:61:14 | *[1] | params_flow.rb:49:17:49:24 | *posargs [element 0] |
78-
| params_flow.rb:61:9:61:13 | * ... [element 0] | params_flow.rb:61:1:61:14 | *[0] |
79-
| params_flow.rb:61:9:61:13 | * ... [element 1] | params_flow.rb:61:1:61:14 | *[1] |
75+
| params_flow.rb:61:9:61:13 | * ... [element 0] | params_flow.rb:49:13:49:14 | p1 |
76+
| params_flow.rb:61:9:61:13 | * ... [element 1] | params_flow.rb:49:17:49:24 | *posargs [element 0] |
8077
| params_flow.rb:61:10:61:13 | args [element 0] | params_flow.rb:61:9:61:13 | * ... [element 0] |
8178
| params_flow.rb:61:10:61:13 | args [element 1] | params_flow.rb:61:9:61:13 | * ... [element 1] |
8279
| params_flow.rb:63:1:63:4 | args | params_flow.rb:67:13:67:16 | args |
8380
| params_flow.rb:63:8:63:16 | call to taint | params_flow.rb:63:1:63:4 | args |
8481
| params_flow.rb:64:16:64:17 | *x [element 0] | params_flow.rb:65:10:65:10 | x [element 0] |
8582
| params_flow.rb:65:10:65:10 | x [element 0] | params_flow.rb:65:10:65:13 | ...[...] |
86-
| params_flow.rb:67:1:67:17 | *[0] | params_flow.rb:64:16:64:17 | *x [element 0] |
8783
| params_flow.rb:67:12:67:16 | * ... [element 0] | params_flow.rb:64:16:64:17 | *x [element 0] |
88-
| params_flow.rb:67:12:67:16 | * ... [element 0] | params_flow.rb:67:1:67:17 | *[0] |
8984
| params_flow.rb:67:13:67:16 | args | params_flow.rb:67:12:67:16 | * ... [element 0] |
9085
| params_flow.rb:69:14:69:14 | x | params_flow.rb:70:10:70:10 | x |
9186
| params_flow.rb:69:17:69:17 | y | params_flow.rb:71:10:71:10 | y |
@@ -167,26 +162,21 @@ edges
167162
| params_flow.rb:114:58:114:66 | call to taint | params_flow.rb:108:44:108:44 | c |
168163
| params_flow.rb:117:1:117:1 | [post] x [element] | params_flow.rb:118:13:118:13 | x [element] |
169164
| params_flow.rb:117:19:117:27 | call to taint | params_flow.rb:117:1:117:1 | [post] x [element] |
170-
| params_flow.rb:118:1:118:14 | *[-1] | params_flow.rb:9:16:9:17 | p1 |
171-
| params_flow.rb:118:1:118:14 | *[-1] | params_flow.rb:9:20:9:21 | p2 |
172-
| params_flow.rb:118:12:118:13 | * ... [element] | params_flow.rb:118:1:118:14 | *[-1] |
165+
| params_flow.rb:118:12:118:13 | * ... [element] | params_flow.rb:9:16:9:17 | p1 |
166+
| params_flow.rb:118:12:118:13 | * ... [element] | params_flow.rb:9:20:9:21 | p2 |
173167
| params_flow.rb:118:13:118:13 | x [element] | params_flow.rb:118:12:118:13 | * ... [element] |
174168
| params_flow.rb:130:1:130:4 | args [element 0] | params_flow.rb:131:11:131:14 | args [element 0] |
175169
| params_flow.rb:130:1:130:4 | args [element 1] | params_flow.rb:131:11:131:14 | args [element 1] |
176170
| params_flow.rb:130:9:130:17 | call to taint | params_flow.rb:130:1:130:4 | args [element 0] |
177171
| params_flow.rb:130:20:130:28 | call to taint | params_flow.rb:130:1:130:4 | args [element 1] |
178-
| params_flow.rb:131:1:131:46 | *[0] | params_flow.rb:83:14:83:14 | t |
179-
| params_flow.rb:131:1:131:46 | *[1] | params_flow.rb:83:17:83:17 | u |
180-
| params_flow.rb:131:10:131:14 | * ... [element 0] | params_flow.rb:131:1:131:46 | *[0] |
181-
| params_flow.rb:131:10:131:14 | * ... [element 1] | params_flow.rb:131:1:131:46 | *[1] |
172+
| params_flow.rb:131:10:131:14 | * ... [element 0] | params_flow.rb:83:14:83:14 | t |
173+
| params_flow.rb:131:10:131:14 | * ... [element 1] | params_flow.rb:83:17:83:17 | u |
182174
| params_flow.rb:131:11:131:14 | args [element 0] | params_flow.rb:131:10:131:14 | * ... [element 0] |
183175
| params_flow.rb:131:11:131:14 | args [element 1] | params_flow.rb:131:10:131:14 | * ... [element 1] |
184176
| params_flow.rb:131:17:131:25 | call to taint | params_flow.rb:83:17:83:17 | u |
185177
| params_flow.rb:133:14:133:18 | *args [element 1] | params_flow.rb:134:10:134:13 | args [element 1] |
186178
| params_flow.rb:134:10:134:13 | args [element 1] | params_flow.rb:134:10:134:16 | ...[...] |
187-
| params_flow.rb:137:1:137:44 | *[1] | params_flow.rb:133:14:133:18 | *args [element 1] |
188179
| params_flow.rb:137:10:137:43 | * ... [element 1] | params_flow.rb:133:14:133:18 | *args [element 1] |
189-
| params_flow.rb:137:10:137:43 | * ... [element 1] | params_flow.rb:137:1:137:44 | *[1] |
190180
| params_flow.rb:137:23:137:31 | call to taint | params_flow.rb:137:10:137:43 | * ... [element 1] |
191181
nodes
192182
| params_flow.rb:9:16:9:17 | p1 | semmle.label | p1 |
@@ -246,8 +236,6 @@ nodes
246236
| params_flow.rb:46:1:46:4 | args [element 1] | semmle.label | args [element 1] |
247237
| params_flow.rb:46:9:46:17 | call to taint | semmle.label | call to taint |
248238
| params_flow.rb:46:20:46:28 | call to taint | semmle.label | call to taint |
249-
| params_flow.rb:47:1:47:17 | *[0] | semmle.label | *[0] |
250-
| params_flow.rb:47:1:47:17 | *[1] | semmle.label | *[1] |
251239
| params_flow.rb:47:12:47:16 | * ... [element 0] | semmle.label | * ... [element 0] |
252240
| params_flow.rb:47:12:47:16 | * ... [element 1] | semmle.label | * ... [element 1] |
253241
| params_flow.rb:47:13:47:16 | args [element 0] | semmle.label | args [element 0] |
@@ -270,8 +258,6 @@ nodes
270258
| params_flow.rb:60:1:60:4 | args [element 1] | semmle.label | args [element 1] |
271259
| params_flow.rb:60:9:60:17 | call to taint | semmle.label | call to taint |
272260
| params_flow.rb:60:20:60:28 | call to taint | semmle.label | call to taint |
273-
| params_flow.rb:61:1:61:14 | *[0] | semmle.label | *[0] |
274-
| params_flow.rb:61:1:61:14 | *[1] | semmle.label | *[1] |
275261
| params_flow.rb:61:9:61:13 | * ... [element 0] | semmle.label | * ... [element 0] |
276262
| params_flow.rb:61:9:61:13 | * ... [element 1] | semmle.label | * ... [element 1] |
277263
| params_flow.rb:61:10:61:13 | args [element 0] | semmle.label | args [element 0] |
@@ -281,7 +267,6 @@ nodes
281267
| params_flow.rb:64:16:64:17 | *x [element 0] | semmle.label | *x [element 0] |
282268
| params_flow.rb:65:10:65:10 | x [element 0] | semmle.label | x [element 0] |
283269
| params_flow.rb:65:10:65:13 | ...[...] | semmle.label | ...[...] |
284-
| params_flow.rb:67:1:67:17 | *[0] | semmle.label | *[0] |
285270
| params_flow.rb:67:12:67:16 | * ... [element 0] | semmle.label | * ... [element 0] |
286271
| params_flow.rb:67:13:67:16 | args | semmle.label | args |
287272
| params_flow.rb:69:14:69:14 | x | semmle.label | x |
@@ -379,15 +364,12 @@ nodes
379364
| params_flow.rb:114:58:114:66 | call to taint | semmle.label | call to taint |
380365
| params_flow.rb:117:1:117:1 | [post] x [element] | semmle.label | [post] x [element] |
381366
| params_flow.rb:117:19:117:27 | call to taint | semmle.label | call to taint |
382-
| params_flow.rb:118:1:118:14 | *[-1] | semmle.label | *[-1] |
383367
| params_flow.rb:118:12:118:13 | * ... [element] | semmle.label | * ... [element] |
384368
| params_flow.rb:118:13:118:13 | x [element] | semmle.label | x [element] |
385369
| params_flow.rb:130:1:130:4 | args [element 0] | semmle.label | args [element 0] |
386370
| params_flow.rb:130:1:130:4 | args [element 1] | semmle.label | args [element 1] |
387371
| params_flow.rb:130:9:130:17 | call to taint | semmle.label | call to taint |
388372
| params_flow.rb:130:20:130:28 | call to taint | semmle.label | call to taint |
389-
| params_flow.rb:131:1:131:46 | *[0] | semmle.label | *[0] |
390-
| params_flow.rb:131:1:131:46 | *[1] | semmle.label | *[1] |
391373
| params_flow.rb:131:10:131:14 | * ... [element 0] | semmle.label | * ... [element 0] |
392374
| params_flow.rb:131:10:131:14 | * ... [element 1] | semmle.label | * ... [element 1] |
393375
| params_flow.rb:131:11:131:14 | args [element 0] | semmle.label | args [element 0] |
@@ -396,7 +378,6 @@ nodes
396378
| params_flow.rb:133:14:133:18 | *args [element 1] | semmle.label | *args [element 1] |
397379
| params_flow.rb:134:10:134:13 | args [element 1] | semmle.label | args [element 1] |
398380
| params_flow.rb:134:10:134:16 | ...[...] | semmle.label | ...[...] |
399-
| params_flow.rb:137:1:137:44 | *[1] | semmle.label | *[1] |
400381
| params_flow.rb:137:10:137:43 | * ... [element 1] | semmle.label | * ... [element 1] |
401382
| params_flow.rb:137:23:137:31 | call to taint | semmle.label | call to taint |
402383
subpaths

0 commit comments

Comments
 (0)