Skip to content

Commit e11a4b6

Browse files
hvitvedhmac
hvitved
authored and
hmac
committed
Ruby: Remove SynthSplatArgParameterNode 
1 parent 5a6a52b commit e11a4b6

File tree

6 files changed

+279
-869
lines changed

6 files changed

+279
-869
lines changed

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

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,6 @@ private module Cached {
473473
exists(Parameter p | p.getPosition() = pos and p instanceof SplatParameter)
474474
} or
475475
TSynthSplatParameterPosition() or
476-
TSynthArgSplatParameterPosition() or
477476
TAnyParameterPosition() or
478477
TAnyKeywordParameterPosition()
479478
}
@@ -1303,9 +1302,6 @@ class ParameterPosition extends TParameterPosition {
13031302

13041303
predicate isSynthSplat() { this = TSynthSplatParameterPosition() }
13051304

1306-
// A fake position to indicate that this parameter node holds content from a synth arg splat node
1307-
predicate isSynthArgSplat() { this = TSynthArgSplatParameterPosition() }
1308-
13091305
predicate isSplat(int n) { this = TSplatParameterPosition(n) }
13101306

13111307
/**
@@ -1341,8 +1337,6 @@ class ParameterPosition extends TParameterPosition {
13411337
or
13421338
this.isSynthSplat() and result = "synthetic *"
13431339
or
1344-
this.isSynthArgSplat() and result = "synthetic * (from *args)"
1345-
or
13461340
exists(int pos | this.isSplat(pos) and result = "* (position " + pos + ")")
13471341
}
13481342
}
@@ -1442,11 +1436,8 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
14421436
or
14431437
ppos.isSplat(0) and apos.isSynthSplat()
14441438
or
1445-
ppos.isSynthSplat() and apos.isSynthSplat()
1446-
or
1447-
ppos.isSynthSplat() and apos.isSplat(0)
1448-
or
1449-
apos.isSynthSplat() and ppos.isSynthArgSplat()
1439+
ppos.isSynthSplat() and
1440+
(apos.isSynthSplat() or apos.isSplat(0))
14501441
or
14511442
// Exact splat match
14521443
exists(int n | apos.isSplat(n) and ppos.isSplat(n))

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

Lines changed: 8 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -452,9 +452,6 @@ private module Cached {
452452
exists(c.asCallable()) and // exclude library callables
453453
isParameterNode(_, c, any(ParameterPosition p | p.isPositional(_)))
454454
} or
455-
TSynthSplatArgParameterNode(DataFlowCallable c) {
456-
exists(c.asCallable()) // exclude library callables
457-
} or
458455
TSynthSplatParameterElementNode(DataFlowCallable c, int n) {
459456
exists(c.asCallable()) and // exclude library callables
460457
isParameterNode(_, c, any(ParameterPosition p | p.isSplat(_))) and
@@ -486,7 +483,7 @@ private module Cached {
486483

487484
class TSourceParameterNode =
488485
TNormalParameterNode or TBlockParameterNode or TSelfParameterNode or
489-
TSynthHashSplatParameterNode or TSynthSplatParameterNode or TSynthSplatArgParameterNode;
486+
TSynthHashSplatParameterNode or TSynthSplatParameterNode;
490487

491488
cached
492489
Location getLocation(NodeImpl n) { result = n.getLocationImpl() }
@@ -694,8 +691,6 @@ predicate nodeIsHidden(Node n) {
694691
or
695692
n instanceof SynthSplatArgumentNode
696693
or
697-
n instanceof SynthSplatArgParameterNode
698-
or
699694
n instanceof SynthSplatParameterElementNode
700695
or
701696
n instanceof LambdaSelfReferenceNode
@@ -1087,31 +1082,6 @@ private module ParameterNodes {
10871082
final override string toStringImpl() { result = "synthetic *args" }
10881083
}
10891084

1090-
/**
1091-
* A node that holds all positional arguments passed in a call to `c`.
1092-
* This is a mirror of the `SynthSplatArgumentNode` on the callable side.
1093-
* See `SynthSplatArgumentNode` for more information.
1094-
*/
1095-
class SynthSplatArgParameterNode extends ParameterNodeImpl, TSynthSplatArgParameterNode {
1096-
private DataFlowCallable callable;
1097-
1098-
SynthSplatArgParameterNode() { this = TSynthSplatArgParameterNode(callable) }
1099-
1100-
final override Parameter getParameter() { none() }
1101-
1102-
final override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
1103-
c = callable and pos.isSynthArgSplat()
1104-
}
1105-
1106-
final override CfgScope getCfgScope() { result = callable.asCallable() }
1107-
1108-
final override DataFlowCallable getEnclosingCallable() { result = callable }
1109-
1110-
final override Location getLocationImpl() { result = callable.getLocation() }
1111-
1112-
final override string toStringImpl() { result = "synthetic *args" }
1113-
}
1114-
11151085
/**
11161086
* A node that holds the content of a specific positional argument.
11171087
* See `SynthSplatArgumentNode` for more information.
@@ -1285,9 +1255,9 @@ module ArgumentNodes {
12851255
*
12861256
* 1. We want `3` to flow to `z[0]` and `4` to flow to `z[1]`. We model this by first storing all arguments
12871257
* in a synthetic argument node `SynthSplatArgumentNode` (see `storeStepCommon`).
1288-
* 2. We match this to an analogous parameter node `SynthSplatArgParameterNode` on the callee side
1258+
* 2. We match this to an analogous parameter node `SynthSplatParameterNode` on the callee side
12891259
* (see `parameterMatch`).
1290-
* 3. For each content element stored in the `SynthSplatArgParameterNode`, we add a read step to a separate
1260+
* 3. For each content element stored in the `SynthSplatParameterNode`, we add a read step to a separate
12911261
* `SynthSplatParameterElementNode`, which is parameterized by the element index (see `readStep`).
12921262
* 4. Finally, we add store steps from these `SynthSplatParameterElementNode`s to the real splat parameter node
12931263
* (see `storeStep`).
@@ -1638,12 +1608,12 @@ predicate readStepCommon(Node node1, ContentSet c, Node node2) {
16381608
// TODO: convert into the above form
16391609
synthSplatArgumentElementReadStep(node1, c, node2)
16401610
or
1641-
// read from SynthSplatArgParameterNode[n] to nth positional parameter
1642-
exists(SynthSplatArgParameterNode argParamNode, NormalParameterNode posNode, int n |
1643-
argParamNode = node1 and
1611+
// read from SynthSplatParameterNode[n] to nth positional parameter
1612+
exists(SynthSplatParameterNode paramNode, NormalParameterNode posNode, int n |
1613+
paramNode = node1 and
16441614
posNode = node2 and
16451615
posNode
1646-
.isParameterOf(argParamNode.getEnclosingCallable(),
1616+
.isParameterOf(paramNode.getEnclosingCallable(),
16471617
any(ParameterPosition p | p.isPositional(n))) and
16481618
c = getPositionalContent(n)
16491619
)
@@ -1655,6 +1625,7 @@ predicate synthSplatArgumentElementReadStep(
16551625
) {
16561626
exists(int splatPos, CfgNodes::ExprNodes::CallCfgNode call |
16571627
node1.asExpr().(Argument).isArgumentOf(call, any(ArgumentPosition p | p.isSplat(splatPos))) and
1628+
splatPos > 0 and
16581629
node2.getCall() = call and
16591630
(
16601631
exists(int n |
@@ -1698,17 +1669,9 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
16981669
FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c,
16991670
node2.(FlowSummaryNode).getSummaryNode())
17001671
or
1701-
// Read from SynthSplatArgParameterNode into SynthSplatParameterElementNode
1702-
node2 =
1703-
any(SynthSplatParameterElementNode e |
1704-
node1.(SynthSplatArgParameterNode).isParameterOf(e.getEnclosingCallable(), _) and
1705-
c = getPositionalContent(e.getReadPosition())
1706-
)
1707-
or
17081672
VariableCapture::readStep(node1, any(Content::CapturedVariableContent v | c.isSingleton(v)), node2)
17091673
or
17101674
// 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.
17121675
node2 =
17131676
any(SynthSplatParameterElementNode e |
17141677
node1.(SynthSplatParameterNode).isParameterOf(e.getEnclosingCallable(), _) and

‎ruby/ql/lib/codeql/ruby/typetracking/TypeTrackerSpecific.qll‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -334,9 +334,7 @@ predicate readStoreStepIntoSourceNode(
334334
Node nodeFrom, Node nodeTo, DataFlow::ContentSet loadContent, DataFlow::ContentSet storeContent
335335
) {
336336
exists(DataFlowPrivate::SynthSplatParameterElementNode mid |
337-
nodeFrom
338-
.(DataFlowPrivate::SynthSplatArgParameterNode)
339-
.isParameterOf(mid.getEnclosingCallable(), _) and
337+
nodeFrom.(DataFlowPrivate::SynthSplatParameterNode).isParameterOf(mid.getEnclosingCallable(), _) and
340338
loadContent = DataFlowPrivate::getPositionalContent(mid.getReadPosition()) and
341339
nodeTo = mid.getSplatParameterNode(_) and
342340
storeContent = DataFlowPrivate::getPositionalContent(mid.getStorePosition())

‎ruby/ql/test/library-tests/dataflow/local/TaintStep.expected‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2797,7 +2797,6 @@
27972797
| UseUseExplosion.rb:21:3686:21:3696 | else ... | UseUseExplosion.rb:21:9:21:3700 | if ... |
27982798
| UseUseExplosion.rb:21:3691:21:3696 | call to use | UseUseExplosion.rb:21:3686:21:3696 | else ... |
27992799
| UseUseExplosion.rb:24:5:25:7 | synthetic *args | UseUseExplosion.rb:24:13:24:13 | i |
2800-
| UseUseExplosion.rb:24:5:25:7 | synthetic *args | UseUseExplosion.rb:24:13:24:13 | i |
28012800
| UseUseExplosion.rb:24:5:25:7 | use | UseUseExplosion.rb:1:1:26:3 | C |
28022801
| file://:0:0:0:0 | [summary param] position 0 in & | file://:0:0:0:0 | [summary] read: Argument[0].Element[any] in & |
28032802
| file://:0:0:0:0 | [summary param] position 0 in + | file://:0:0:0:0 | [summary] read: Argument[0].Element[any] in + |
@@ -2843,7 +2842,6 @@
28432842
| local_dataflow.rb:1:1:7:3 | self (foo) | local_dataflow.rb:3:8:3:10 | self |
28442843
| local_dataflow.rb:1:1:7:3 | self in foo | local_dataflow.rb:1:1:7:3 | self (foo) |
28452844
| local_dataflow.rb:1:1:7:3 | synthetic *args | local_dataflow.rb:1:9:1:9 | a |
2846-
| local_dataflow.rb:1:1:7:3 | synthetic *args | local_dataflow.rb:1:9:1:9 | a |
28472845
| local_dataflow.rb:1:1:150:3 | <uninitialized> | local_dataflow.rb:10:9:10:9 | x |
28482846
| local_dataflow.rb:1:1:150:3 | self (local_dataflow.rb) | local_dataflow.rb:49:1:53:3 | self |
28492847
| local_dataflow.rb:1:9:1:9 | a | local_dataflow.rb:1:9:1:9 | a |
@@ -2882,7 +2880,6 @@
28822880
| local_dataflow.rb:10:5:13:3 | __synth__0__1 | local_dataflow.rb:10:9:10:9 | x |
28832881
| local_dataflow.rb:10:5:13:3 | call to each | local_dataflow.rb:10:5:13:3 | ... |
28842882
| local_dataflow.rb:10:5:13:3 | synthetic *args | local_dataflow.rb:10:5:13:3 | __synth__0__1 |
2885-
| local_dataflow.rb:10:5:13:3 | synthetic *args | local_dataflow.rb:10:5:13:3 | __synth__0__1 |
28862883
| local_dataflow.rb:10:9:10:9 | ... = ... | local_dataflow.rb:10:9:10:9 | if ... |
28872884
| local_dataflow.rb:10:9:10:9 | defined? ... | local_dataflow.rb:10:9:10:9 | [false] ! ... |
28882885
| local_dataflow.rb:10:9:10:9 | defined? ... | local_dataflow.rb:10:9:10:9 | [true] ! ... |
@@ -2902,7 +2899,6 @@
29022899
| local_dataflow.rb:15:1:17:3 | __synth__0__1 | local_dataflow.rb:15:5:15:5 | x |
29032900
| local_dataflow.rb:15:1:17:3 | call to each | local_dataflow.rb:15:1:17:3 | ... |
29042901
| local_dataflow.rb:15:1:17:3 | synthetic *args | local_dataflow.rb:15:1:17:3 | __synth__0__1 |
2905-
| local_dataflow.rb:15:1:17:3 | synthetic *args | local_dataflow.rb:15:1:17:3 | __synth__0__1 |
29062902
| local_dataflow.rb:15:5:15:5 | ... = ... | local_dataflow.rb:15:5:15:5 | if ... |
29072903
| local_dataflow.rb:15:5:15:5 | defined? ... | local_dataflow.rb:15:5:15:5 | [false] ! ... |
29082904
| local_dataflow.rb:15:5:15:5 | defined? ... | local_dataflow.rb:15:5:15:5 | [true] ! ... |
@@ -2919,7 +2915,6 @@
29192915
| local_dataflow.rb:19:1:21:3 | __synth__0__1 | local_dataflow.rb:19:5:19:5 | x |
29202916
| local_dataflow.rb:19:1:21:3 | call to each | local_dataflow.rb:19:1:21:3 | ... |
29212917
| local_dataflow.rb:19:1:21:3 | synthetic *args | local_dataflow.rb:19:1:21:3 | __synth__0__1 |
2922-
| local_dataflow.rb:19:1:21:3 | synthetic *args | local_dataflow.rb:19:1:21:3 | __synth__0__1 |
29232918
| local_dataflow.rb:19:5:19:5 | ... = ... | local_dataflow.rb:19:5:19:5 | if ... |
29242919
| local_dataflow.rb:19:5:19:5 | defined? ... | local_dataflow.rb:19:5:19:5 | [false] ! ... |
29252920
| local_dataflow.rb:19:5:19:5 | defined? ... | local_dataflow.rb:19:5:19:5 | [true] ! ... |
@@ -2939,14 +2934,12 @@
29392934
| local_dataflow.rb:32:5:32:25 | bar | local_dataflow.rb:32:1:32:1 | x |
29402935
| local_dataflow.rb:32:5:32:25 | bar | local_dataflow.rb:32:1:32:25 | ... = ... |
29412936
| local_dataflow.rb:34:1:39:3 | synthetic *args | local_dataflow.rb:34:7:34:7 | x |
2942-
| local_dataflow.rb:34:1:39:3 | synthetic *args | local_dataflow.rb:34:7:34:7 | x |
29432937
| local_dataflow.rb:34:7:34:7 | x | local_dataflow.rb:34:7:34:7 | x |
29442938
| local_dataflow.rb:34:7:34:7 | x | local_dataflow.rb:35:6:35:6 | x |
29452939
| local_dataflow.rb:35:6:35:6 | x | local_dataflow.rb:35:6:35:11 | ... == ... |
29462940
| local_dataflow.rb:35:11:35:11 | 4 | local_dataflow.rb:35:6:35:11 | ... == ... |
29472941
| local_dataflow.rb:36:13:36:13 | 7 | local_dataflow.rb:36:6:36:13 | return |
29482942
| local_dataflow.rb:41:1:47:3 | synthetic *args | local_dataflow.rb:41:7:41:7 | x |
2949-
| local_dataflow.rb:41:1:47:3 | synthetic *args | local_dataflow.rb:41:7:41:7 | x |
29502943
| local_dataflow.rb:41:7:41:7 | x | local_dataflow.rb:41:7:41:7 | x |
29512944
| local_dataflow.rb:41:7:41:7 | x | local_dataflow.rb:42:6:42:6 | x |
29522945
| local_dataflow.rb:42:6:42:6 | x | local_dataflow.rb:42:6:42:11 | ... == ... |
@@ -2966,11 +2959,9 @@
29662959
| local_dataflow.rb:51:24:51:24 | 9 | local_dataflow.rb:51:20:51:24 | ... < ... |
29672960
| local_dataflow.rb:55:5:55:13 | Array | local_dataflow.rb:55:5:55:13 | call to [] |
29682961
| local_dataflow.rb:57:1:58:3 | synthetic *args | local_dataflow.rb:57:9:57:9 | x |
2969-
| local_dataflow.rb:57:1:58:3 | synthetic *args | local_dataflow.rb:57:9:57:9 | x |
29702962
| local_dataflow.rb:60:1:90:3 | self (test_case) | local_dataflow.rb:78:12:78:20 | self |
29712963
| local_dataflow.rb:60:1:90:3 | self in test_case | local_dataflow.rb:60:1:90:3 | self (test_case) |
29722964
| local_dataflow.rb:60:1:90:3 | synthetic *args | local_dataflow.rb:60:15:60:15 | x |
2973-
| local_dataflow.rb:60:1:90:3 | synthetic *args | local_dataflow.rb:60:15:60:15 | x |
29742965
| local_dataflow.rb:60:15:60:15 | x | local_dataflow.rb:60:15:60:15 | x |
29752966
| local_dataflow.rb:60:15:60:15 | x | local_dataflow.rb:61:12:61:12 | x |
29762967
| local_dataflow.rb:61:7:68:5 | SSA phi read(x) | local_dataflow.rb:69:12:69:12 | x |
@@ -3144,7 +3135,6 @@
31443135
| local_dataflow.rb:118:3:118:11 | self | local_dataflow.rb:119:3:119:31 | self |
31453136
| local_dataflow.rb:118:17:118:31 | <captured entry> self | local_dataflow.rb:118:23:118:29 | self |
31463137
| local_dataflow.rb:118:17:118:31 | synthetic *args | local_dataflow.rb:118:20:118:20 | x |
3147-
| local_dataflow.rb:118:17:118:31 | synthetic *args | local_dataflow.rb:118:20:118:20 | x |
31483138
| local_dataflow.rb:118:20:118:20 | x | local_dataflow.rb:118:20:118:20 | x |
31493139
| local_dataflow.rb:118:20:118:20 | x | local_dataflow.rb:118:28:118:28 | x |
31503140
| local_dataflow.rb:119:3:119:31 | [post] self | local_dataflow.rb:119:8:119:16 | self |
@@ -3160,11 +3150,9 @@
31603150
| local_dataflow.rb:123:8:123:45 | call to tap | local_dataflow.rb:123:8:123:49 | call to dup |
31613151
| local_dataflow.rb:123:26:123:45 | <captured entry> self | local_dataflow.rb:123:32:123:43 | self |
31623152
| local_dataflow.rb:123:26:123:45 | synthetic *args | local_dataflow.rb:123:29:123:29 | x |
3163-
| local_dataflow.rb:123:26:123:45 | synthetic *args | local_dataflow.rb:123:29:123:29 | x |
31643153
| local_dataflow.rb:126:1:128:3 | self (use) | local_dataflow.rb:127:3:127:8 | self |
31653154
| local_dataflow.rb:126:1:128:3 | self in use | local_dataflow.rb:126:1:128:3 | self (use) |
31663155
| local_dataflow.rb:126:1:128:3 | synthetic *args | local_dataflow.rb:126:9:126:9 | x |
3167-
| local_dataflow.rb:126:1:128:3 | synthetic *args | local_dataflow.rb:126:9:126:9 | x |
31683156
| local_dataflow.rb:130:1:150:3 | self (use_use_madness) | local_dataflow.rb:132:6:132:11 | self |
31693157
| local_dataflow.rb:130:1:150:3 | self in use_use_madness | local_dataflow.rb:130:1:150:3 | self (use_use_madness) |
31703158
| local_dataflow.rb:131:3:131:3 | x | local_dataflow.rb:132:10:132:10 | x |

0 commit comments

Comments
 (0)