Skip to content

Commit 1496c4f

Browse files
hvitvedhvitved
authored
Merge pull request #10686 from hvitved/ruby/remove-value-pair-content
Ruby: Remove `PairValueContent`
2 parents ecfbd5e + aae9a58 commit 1496c4f

File tree

6 files changed

+24
-113
lines changed

6 files changed

+24
-113
lines changed

‎ruby/ql/lib/codeql/ruby/dataflow/FlowSummary.qll

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,16 +84,6 @@ module SummaryComponent {
8484
result = SC::content(TElementLowerBoundContent(lower, true))
8585
}
8686

87-
/** Gets a summary component that represents a value in a pair at an unknown key. */
88-
SummaryComponent pairValueUnknown() {
89-
result = SC::content(TSingletonContent(TUnknownPairValueContent()))
90-
}
91-
92-
/** Gets a summary component that represents a value in a pair at a known key. */
93-
SummaryComponent pairValueKnown(ConstantValue key) {
94-
result = SC::content(TSingletonContent(DataFlow::Content::getPairValueContent(key)))
95-
}
96-
9787
/** Gets a summary component that represents the return value of a call. */
9888
SummaryComponent return() { result = SC::return(any(NormalReturnKind rk)) }
9989
}

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

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,8 @@ private module Cached {
277277
} or
278278
TSynthHashSplatArgumentNode(CfgNodes::ExprNodes::CallCfgNode c) {
279279
exists(Argument arg | arg.isArgumentOf(c, any(ArgumentPosition pos | pos.isKeyword(_))))
280+
or
281+
c.getAnArgument() instanceof CfgNodes::ExprNodes::PairCfgNode
280282
}
281283

282284
class TParameterNode =
@@ -411,8 +413,6 @@ private module Cached {
411413
newtype TContent =
412414
TKnownElementContent(ConstantValue cv) { trackKnownValue(cv) } or
413415
TUnknownElementContent() or
414-
TKnownPairValueContent(ConstantValue cv) { trackKnownValue(cv) } or
415-
TUnknownPairValueContent() or
416416
TFieldContent(string name) {
417417
name = any(InstanceVariable v).getName()
418418
or
@@ -449,8 +449,6 @@ private module Cached {
449449

450450
class TElementContent = TKnownElementContent or TUnknownElementContent;
451451

452-
class TPairValueContent = TKnownPairValueContent or TUnknownPairValueContent;
453-
454452
import Cached
455453

456454
/** Holds if `n` should be hidden from path explanations. */
@@ -1042,25 +1040,24 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
10421040
or
10431041
FlowSummaryImpl::Private::Steps::summaryStoreStep(node1, c, node2)
10441042
or
1045-
// Needed for pairs passed into method calls where the key is not a symbol,
1046-
// that is, where it is not a keyword argument.
1047-
node2.asExpr() =
1048-
any(CfgNodes::ExprNodes::PairCfgNode pair |
1049-
exists(CfgNodes::ExprCfgNode key, ConstantValue cv |
1050-
key = pair.getKey() and
1051-
pair.getValue() = node1.asExpr() and
1052-
cv = key.getConstantValue() and
1053-
not cv.isSymbol(_) and // handled as a keyword argument
1054-
c.isSingleton(Content::getPairValueContent(cv))
1055-
)
1043+
// Wrap all key-value arguments in a synthesized hash-splat argument node
1044+
exists(CfgNodes::ExprNodes::CallCfgNode call | node2 = TSynthHashSplatArgumentNode(call) |
1045+
// symbol key
1046+
exists(ArgumentPosition keywordPos, string name |
1047+
node1.asExpr().(Argument).isArgumentOf(call, keywordPos) and
1048+
keywordPos.isKeyword(name) and
1049+
c = getKeywordContent(name)
1050+
)
1051+
or
1052+
// non-symbol key
1053+
exists(CfgNodes::ExprNodes::PairCfgNode pair, CfgNodes::ExprCfgNode key, ConstantValue cv |
1054+
node1.asExpr() = pair.getValue() and
1055+
pair = call.getAnArgument() and
1056+
key = pair.getKey() and
1057+
cv = key.getConstantValue() and
1058+
not cv.isSymbol(_) and
1059+
c.isSingleton(TKnownElementContent(cv))
10561060
)
1057-
or
1058-
// Wrap all keyword arguments in a synthesized hash-splat argument node
1059-
exists(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition keywordPos, string name |
1060-
node2 = TSynthHashSplatArgumentNode(call) and
1061-
node1.asExpr().(Argument).isArgumentOf(call, keywordPos) and
1062-
keywordPos.isKeyword(name) and
1063-
c = getKeywordContent(name)
10641061
)
10651062
}
10661063

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

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -295,34 +295,6 @@ module Content {
295295
result = getElementContent(e.getConstantValue()).(KnownElementContent).getIndex()
296296
}
297297

298-
/** A value in a pair with a known or unknown key. */
299-
class PairValueContent extends Content, TPairValueContent { }
300-
301-
/** A value in a pair with a known key. */
302-
class KnownPairValueContent extends PairValueContent, TKnownPairValueContent {
303-
private ConstantValue key;
304-
305-
KnownPairValueContent() { this = TKnownPairValueContent(key) }
306-
307-
/** Gets the index in the collection. */
308-
ConstantValue getIndex() { result = key }
309-
310-
override string toString() { result = "pair " + key }
311-
}
312-
313-
/** A value in a pair with an unknown key. */
314-
class UnknownPairValueContent extends PairValueContent, TUnknownPairValueContent {
315-
override string toString() { result = "pair" }
316-
}
317-
318-
/** Gets the content representing a value in a pair corresponding to constant value `cv`. */
319-
PairValueContent getPairValueContent(ConstantValue cv) {
320-
result = TKnownPairValueContent(cv)
321-
or
322-
not exists(TKnownPairValueContent(cv)) and
323-
result = TUnknownPairValueContent()
324-
}
325-
326298
/**
327299
* A value stored behind a getter/setter pair.
328300
*

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -135,16 +135,6 @@ SummaryComponent interpretComponentSpecific(AccessPathToken c) {
135135
interpretElementArg(c.getAnArgument("WithoutElement")) and
136136
result = FlowSummary::SummaryComponent::withoutContent(cs)
137137
)
138-
or
139-
exists(string arg | arg = c.getAnArgument("PairValue") |
140-
arg = "?" and
141-
result = FlowSummary::SummaryComponent::pairValueUnknown()
142-
or
143-
exists(ConstantValue cv |
144-
result = FlowSummary::SummaryComponent::pairValueKnown(cv) and
145-
cv.serialize() = arg
146-
)
147-
)
148138
}
149139

150140
/** Gets the textual representation of a summary component in the format used for flow summaries. */

‎ruby/ql/lib/codeql/ruby/frameworks/core/Hash.qll

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,25 +17,6 @@ private import codeql.ruby.ast.internal.Module
1717
* in `Array.qll`.
1818
*/
1919
module Hash {
20-
/**
21-
* Holds if `key` is used as the non-symbol key in a hash literal. For example
22-
*
23-
* ```rb
24-
* {
25-
* :a => 1, # symbol
26-
* "b" => 2 # non-symbol, "b" is the key
27-
* }
28-
* ```
29-
*/
30-
private predicate isHashLiteralNonSymbolKey(ConstantValue key) {
31-
exists(Pair pair |
32-
key = DataFlow::Content::getKnownElementIndex(pair.getKey()) and
33-
// cannot use API graphs due to negative recursion
34-
pair = any(MethodCall mc | mc.getMethodName() = "[]").getAnArgument() and
35-
not key.isSymbol(_)
36-
)
37-
}
38-
3920
/**
4021
* Gets a call to the method `name` invoked on the `Hash` object
4122
* (not on a hash instance).
@@ -51,15 +32,6 @@ module Hash {
5132
final override MethodCall getACallSimple() { result = getAStaticHashCall("[]") }
5233

5334
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
54-
// { 'nonsymbol' => x }
55-
exists(ConstantValue key |
56-
isHashLiteralNonSymbolKey(key) and
57-
input = "Argument[0..].PairValue[" + key.serialize() + "]" and
58-
output = "ReturnValue.Element[" + key.serialize() + "]" and
59-
preservesValue = true
60-
)
61-
or
62-
// { symbol: x }
6335
// we make use of the special `hash-splat` argument kind, which contains all keyword
6436
// arguments wrapped in an implicit hash, as well as explicit hash splat arguments
6537
input = "Argument[hash-splat].WithElement[any]" and

‎ruby/ql/test/library-tests/dataflow/hash-flow/hash-flow.expected

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,9 @@ failures
22
edges
33
| hash_flow.rb:11:15:11:24 | call to taint : | hash_flow.rb:22:10:22:13 | hash [element :a] : |
44
| hash_flow.rb:13:12:13:21 | call to taint : | hash_flow.rb:24:10:24:13 | hash [element :c] : |
5-
| hash_flow.rb:15:9:15:23 | Pair [pair e] : | hash_flow.rb:26:10:26:13 | hash [element e] : |
6-
| hash_flow.rb:15:14:15:23 | call to taint : | hash_flow.rb:15:9:15:23 | Pair [pair e] : |
7-
| hash_flow.rb:17:9:17:25 | Pair [pair g] : | hash_flow.rb:28:10:28:13 | hash [element g] : |
8-
| hash_flow.rb:17:16:17:25 | call to taint : | hash_flow.rb:17:9:17:25 | Pair [pair g] : |
9-
| hash_flow.rb:19:9:19:23 | Pair [pair 0] : | hash_flow.rb:30:10:30:13 | hash [element 0] : |
10-
| hash_flow.rb:19:14:19:23 | call to taint : | hash_flow.rb:19:9:19:23 | Pair [pair 0] : |
5+
| hash_flow.rb:15:14:15:23 | call to taint : | hash_flow.rb:26:10:26:13 | hash [element e] : |
6+
| hash_flow.rb:17:16:17:25 | call to taint : | hash_flow.rb:28:10:28:13 | hash [element g] : |
7+
| hash_flow.rb:19:14:19:23 | call to taint : | hash_flow.rb:30:10:30:13 | hash [element 0] : |
118
| hash_flow.rb:22:10:22:13 | hash [element :a] : | hash_flow.rb:22:10:22:17 | ...[...] |
129
| hash_flow.rb:24:10:24:13 | hash [element :c] : | hash_flow.rb:24:10:24:17 | ...[...] |
1310
| hash_flow.rb:26:10:26:13 | hash [element e] : | hash_flow.rb:26:10:26:18 | ...[...] |
@@ -56,12 +53,10 @@ edges
5653
| hash_flow.rb:68:22:68:31 | call to taint : | hash_flow.rb:68:13:68:39 | ...[...] [element :a] : |
5754
| hash_flow.rb:69:10:69:14 | hash4 [element :a] : | hash_flow.rb:69:10:69:18 | ...[...] |
5855
| hash_flow.rb:72:13:72:45 | ...[...] [element a] : | hash_flow.rb:73:10:73:14 | hash5 [element a] : |
59-
| hash_flow.rb:72:18:72:34 | Pair [pair a] : | hash_flow.rb:72:13:72:45 | ...[...] [element a] : |
60-
| hash_flow.rb:72:25:72:34 | call to taint : | hash_flow.rb:72:18:72:34 | Pair [pair a] : |
56+
| hash_flow.rb:72:25:72:34 | call to taint : | hash_flow.rb:72:13:72:45 | ...[...] [element a] : |
6157
| hash_flow.rb:73:10:73:14 | hash5 [element a] : | hash_flow.rb:73:10:73:19 | ...[...] |
6258
| hash_flow.rb:76:13:76:47 | ...[...] [element a] : | hash_flow.rb:77:10:77:14 | hash6 [element a] : |
63-
| hash_flow.rb:76:19:76:35 | Pair [pair a] : | hash_flow.rb:76:13:76:47 | ...[...] [element a] : |
64-
| hash_flow.rb:76:26:76:35 | call to taint : | hash_flow.rb:76:19:76:35 | Pair [pair a] : |
59+
| hash_flow.rb:76:26:76:35 | call to taint : | hash_flow.rb:76:13:76:47 | ...[...] [element a] : |
6560
| hash_flow.rb:77:10:77:14 | hash6 [element a] : | hash_flow.rb:77:10:77:19 | ...[...] |
6661
| hash_flow.rb:84:13:84:42 | call to [] [element :a] : | hash_flow.rb:85:10:85:14 | hash1 [element :a] : |
6762
| hash_flow.rb:84:26:84:35 | call to taint : | hash_flow.rb:84:13:84:42 | call to [] [element :a] : |
@@ -533,11 +528,8 @@ edges
533528
nodes
534529
| hash_flow.rb:11:15:11:24 | call to taint : | semmle.label | call to taint : |
535530
| hash_flow.rb:13:12:13:21 | call to taint : | semmle.label | call to taint : |
536-
| hash_flow.rb:15:9:15:23 | Pair [pair e] : | semmle.label | Pair [pair e] : |
537531
| hash_flow.rb:15:14:15:23 | call to taint : | semmle.label | call to taint : |
538-
| hash_flow.rb:17:9:17:25 | Pair [pair g] : | semmle.label | Pair [pair g] : |
539532
| hash_flow.rb:17:16:17:25 | call to taint : | semmle.label | call to taint : |
540-
| hash_flow.rb:19:9:19:23 | Pair [pair 0] : | semmle.label | Pair [pair 0] : |
541533
| hash_flow.rb:19:14:19:23 | call to taint : | semmle.label | call to taint : |
542534
| hash_flow.rb:22:10:22:13 | hash [element :a] : | semmle.label | hash [element :a] : |
543535
| hash_flow.rb:22:10:22:17 | ...[...] | semmle.label | ...[...] |
@@ -599,12 +591,10 @@ nodes
599591
| hash_flow.rb:69:10:69:14 | hash4 [element :a] : | semmle.label | hash4 [element :a] : |
600592
| hash_flow.rb:69:10:69:18 | ...[...] | semmle.label | ...[...] |
601593
| hash_flow.rb:72:13:72:45 | ...[...] [element a] : | semmle.label | ...[...] [element a] : |
602-
| hash_flow.rb:72:18:72:34 | Pair [pair a] : | semmle.label | Pair [pair a] : |
603594
| hash_flow.rb:72:25:72:34 | call to taint : | semmle.label | call to taint : |
604595
| hash_flow.rb:73:10:73:14 | hash5 [element a] : | semmle.label | hash5 [element a] : |
605596
| hash_flow.rb:73:10:73:19 | ...[...] | semmle.label | ...[...] |
606597
| hash_flow.rb:76:13:76:47 | ...[...] [element a] : | semmle.label | ...[...] [element a] : |
607-
| hash_flow.rb:76:19:76:35 | Pair [pair a] : | semmle.label | Pair [pair a] : |
608598
| hash_flow.rb:76:26:76:35 | call to taint : | semmle.label | call to taint : |
609599
| hash_flow.rb:77:10:77:14 | hash6 [element a] : | semmle.label | hash6 [element a] : |
610600
| hash_flow.rb:77:10:77:19 | ...[...] | semmle.label | ...[...] |

0 commit comments

Comments
 (0)