Skip to content

Commit 5f17d83

Browse files
committed
Ruby: Small change to isArrayExpr
1 parent 63dcce9 commit 5f17d83

File tree

3 files changed

+34
-2
lines changed

3 files changed

+34
-2
lines changed

ruby/ql/lib/codeql/ruby/ast/internal/Constant.qll

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -552,5 +552,10 @@ predicate isArrayExpr(Expr e, ArrayLiteralCfgNode arr) {
552552
// We map from A to a via ConstantReadAccess::getValue, yielding the Expr a.
553553
// To get to [...] we need to go via getSource(ExprCfgNode e), so we find a
554554
// CFG node for a and call `isArrayConstant`.
555-
isArrayConstant(e.getAControlFlowNode(), arr)
555+
//
556+
// The use of `forex` is intended to ensure that a is an array constant in all
557+
// control flow paths.
558+
// Note(hmac): I don't think this is necessary, as `getSource` will not return
559+
// results if the source is a phi node.
560+
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isArrayConstant(n, arr))
556561
}

ruby/ql/test/library-tests/ast/constants/constants.expected

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,23 +78,41 @@ getConst
7878
| constants.rb:54:3:58:5 | ModuleA::ModuleB::ClassB | FOURTY_ONE | constants.rb:48:18:48:19 | 41 |
7979
| constants.rb:62:3:64:5 | Mod1::Mod3 | FOURTY_FIVE | constants.rb:63:19:63:20 | 45 |
8080
| constants.rb:70:3:72:5 | Mod1::Mod3::Mod5 | FOURTY_SIX | constants.rb:71:18:71:19 | 46 |
81+
| file://:0:0:0:0 | Object | A | constants.rb:79:5:79:13 | [...] |
82+
| file://:0:0:0:0 | Object | B | constants.rb:80:5:80:5 | a |
83+
| file://:0:0:0:0 | Object | C | constants.rb:81:5:81:5 | A |
8184
| file://:0:0:0:0 | Object | GREETING | constants.rb:17:12:17:64 | ... + ... |
8285
lookupConst
8386
| constants.rb:1:1:15:3 | ModuleA | CONST_B | constants.rb:6:15:6:23 | "const_b" |
8487
| constants.rb:1:1:15:3 | ModuleA | FOURTY_FOUR | constants.rb:53:17:53:29 | "fourty-four" |
88+
| constants.rb:2:5:4:7 | ModuleA::ClassA | A | constants.rb:79:5:79:13 | [...] |
89+
| constants.rb:2:5:4:7 | ModuleA::ClassA | B | constants.rb:80:5:80:5 | a |
90+
| constants.rb:2:5:4:7 | ModuleA::ClassA | C | constants.rb:81:5:81:5 | A |
8591
| constants.rb:2:5:4:7 | ModuleA::ClassA | CONST_A | constants.rb:3:19:3:27 | "const_a" |
8692
| constants.rb:2:5:4:7 | ModuleA::ClassA | GREETING | constants.rb:17:12:17:64 | ... + ... |
8793
| constants.rb:8:5:14:7 | ModuleA::ModuleB | MAX_SIZE | constants.rb:39:30:39:33 | 1024 |
94+
| constants.rb:12:9:13:11 | ModuleA::ModuleB::ClassC | A | constants.rb:79:5:79:13 | [...] |
95+
| constants.rb:12:9:13:11 | ModuleA::ModuleB::ClassC | B | constants.rb:80:5:80:5 | a |
96+
| constants.rb:12:9:13:11 | ModuleA::ModuleB::ClassC | C | constants.rb:81:5:81:5 | A |
8897
| constants.rb:12:9:13:11 | ModuleA::ModuleB::ClassC | GREETING | constants.rb:17:12:17:64 | ... + ... |
98+
| constants.rb:31:1:33:3 | ModuleA::ClassD | A | constants.rb:79:5:79:13 | [...] |
99+
| constants.rb:31:1:33:3 | ModuleA::ClassD | B | constants.rb:80:5:80:5 | a |
100+
| constants.rb:31:1:33:3 | ModuleA::ClassD | C | constants.rb:81:5:81:5 | A |
89101
| constants.rb:31:1:33:3 | ModuleA::ClassD | CONST_A | constants.rb:3:19:3:27 | "const_a" |
90102
| constants.rb:31:1:33:3 | ModuleA::ClassD | FOURTY_TWO | constants.rb:32:16:32:17 | 42 |
91103
| constants.rb:31:1:33:3 | ModuleA::ClassD | GREETING | constants.rb:17:12:17:64 | ... + ... |
92104
| constants.rb:35:1:37:3 | ModuleA::ModuleC | FOURTY_THREE | constants.rb:36:18:36:19 | 43 |
105+
| constants.rb:54:3:58:5 | ModuleA::ModuleB::ClassB | A | constants.rb:79:5:79:13 | [...] |
106+
| constants.rb:54:3:58:5 | ModuleA::ModuleB::ClassB | B | constants.rb:80:5:80:5 | a |
107+
| constants.rb:54:3:58:5 | ModuleA::ModuleB::ClassB | C | constants.rb:81:5:81:5 | A |
93108
| constants.rb:54:3:58:5 | ModuleA::ModuleB::ClassB | FOURTY_FOUR | constants.rb:56:19:56:20 | 44 |
94109
| constants.rb:54:3:58:5 | ModuleA::ModuleB::ClassB | FOURTY_ONE | constants.rb:48:18:48:19 | 41 |
95110
| constants.rb:54:3:58:5 | ModuleA::ModuleB::ClassB | GREETING | constants.rb:17:12:17:64 | ... + ... |
96111
| constants.rb:62:3:64:5 | Mod1::Mod3 | FOURTY_FIVE | constants.rb:63:19:63:20 | 45 |
97112
| constants.rb:70:3:72:5 | Mod1::Mod3::Mod5 | FOURTY_SIX | constants.rb:71:18:71:19 | 46 |
113+
| file://:0:0:0:0 | Object | A | constants.rb:79:5:79:13 | [...] |
114+
| file://:0:0:0:0 | Object | B | constants.rb:80:5:80:5 | a |
115+
| file://:0:0:0:0 | Object | C | constants.rb:81:5:81:5 | A |
98116
| file://:0:0:0:0 | Object | GREETING | constants.rb:17:12:17:64 | ... + ... |
99117
constantValue
100118
| constants.rb:17:22:17:45 | CONST_A | constants.rb:3:19:3:27 | "const_a" |
@@ -108,6 +126,8 @@ constantValue
108126
| constants.rb:57:21:57:31 | FOURTY_FOUR | constants.rb:53:17:53:29 | "fourty-four" |
109127
| constants.rb:57:21:57:31 | FOURTY_FOUR | constants.rb:56:19:56:20 | 44 |
110128
| constants.rb:65:19:65:35 | FOURTY_FIVE | constants.rb:63:19:63:20 | 45 |
129+
| constants.rb:81:5:81:5 | A | constants.rb:79:5:79:13 | [...] |
130+
| constants.rb:82:5:82:5 | B | constants.rb:80:5:80:5 | a |
111131
constantWriteAccessQualifiedName
112132
| constants.rb:1:1:15:3 | ModuleA | ModuleA |
113133
| constants.rb:2:5:4:7 | ClassA | ModuleA::ClassA |
@@ -149,3 +169,5 @@ arrayConstant
149169
| constants.rb:79:5:79:13 | call to [] | constants.rb:79:5:79:13 | call to [] |
150170
| constants.rb:80:5:80:5 | a | constants.rb:78:5:78:13 | call to [] |
151171
| constants.rb:81:5:81:5 | A | constants.rb:79:5:79:13 | call to [] |
172+
| constants.rb:82:5:82:5 | B | constants.rb:78:5:78:13 | call to [] |
173+
| constants.rb:85:7:85:7 | b | constants.rb:78:5:78:13 | call to [] |

ruby/ql/test/library-tests/ast/constants/constants.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,9 @@ module Mod3::Mod5
7979
A = [1, 2, 3]
8080
B = a
8181
C = A
82-
b = B
82+
b = B
83+
84+
if condition
85+
c = b
86+
end
87+
c # not recognised

0 commit comments

Comments
 (0)