Skip to content

Commit ae57ca7

Browse files
authored
Merge pull request #21907 from hvitved/ruby/implicit-local-fix
Ruby: Fix bug in `implicitAssignmentNode`
2 parents d287f0c + c319680 commit ae57ca7

7 files changed

Lines changed: 131 additions & 62 deletions

File tree

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

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,17 @@ private import codeql.ruby.ast.internal.Pattern
1111
private import codeql.ruby.ast.internal.Scope
1212
private import codeql.ruby.ast.internal.Synthesis
1313

14+
private Ruby::AstNode getAssignmentParent(Ruby::AstNode n) {
15+
result = n.getParent() and
16+
(
17+
result instanceof Ruby::DestructuredLeftAssignment
18+
or
19+
result instanceof Ruby::LeftAssignmentList
20+
or
21+
result instanceof Ruby::RestAssignment
22+
)
23+
}
24+
1425
/**
1526
* Holds if `n` is in the left-hand-side of an explicit assignment `assignment`.
1627
*/
@@ -19,16 +30,7 @@ predicate explicitAssignmentNode(Ruby::AstNode n, Ruby::AstNode assignment) {
1930
or
2031
n = assignment.(Ruby::OperatorAssignment).getLeft()
2132
or
22-
exists(Ruby::AstNode parent |
23-
parent = n.getParent() and
24-
explicitAssignmentNode(parent, assignment)
25-
|
26-
parent instanceof Ruby::DestructuredLeftAssignment
27-
or
28-
parent instanceof Ruby::LeftAssignmentList
29-
or
30-
parent instanceof Ruby::RestAssignment
31-
)
33+
explicitAssignmentNode(getAssignmentParent(n), assignment)
3234
}
3335

3436
/** Holds if `n` is inside an implicit assignment. */
@@ -49,7 +51,7 @@ predicate implicitAssignmentNode(Ruby::AstNode n) {
4951
or
5052
n = any(Ruby::For for).getPattern()
5153
or
52-
implicitAssignmentNode(n.getParent())
54+
implicitAssignmentNode(getAssignmentParent(n))
5355
}
5456

5557
/** Holds if `n` is inside a parameter. */

ruby/ql/test/library-tests/variables/parameter.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ parameterVariable
2929
| scopes.rb:2:14:2:14 | x | scopes.rb:2:14:2:14 | x |
3030
| scopes.rb:9:14:9:14 | x | scopes.rb:9:14:9:14 | x |
3131
| scopes.rb:69:15:69:15 | x | scopes.rb:69:15:69:15 | x |
32+
| scopes.rb:80:13:80:17 | value | scopes.rb:80:13:80:17 | value |
33+
| scopes.rb:84:11:84:13 | msg | scopes.rb:84:11:84:13 | msg |
3234
| ssa.rb:1:7:1:7 | b | ssa.rb:1:7:1:7 | b |
3335
| ssa.rb:18:8:18:8 | x | ssa.rb:18:8:18:8 | x |
3436
| ssa.rb:25:8:25:15 | elements | ssa.rb:25:8:25:15 | elements |

ruby/ql/test/library-tests/variables/scopes.rb

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,4 +70,20 @@ module ParameterShadowing
7070
puts x
7171
end
7272
puts x # prints `1`, not `3`
73-
end
73+
end
74+
75+
class RescueSetter
76+
def name
77+
@name
78+
end
79+
80+
def name=(value)
81+
@name = value
82+
end
83+
84+
def foo(msg)
85+
raise msg
86+
rescue => self.name # calls `name=`
87+
:caught
88+
end
89+
end

ruby/ql/test/library-tests/variables/ssa.expected

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,12 @@ definition
8686
| parameters.rb:59:20:59:20 | a | parameters.rb:59:20:59:20 | a |
8787
| parameters.rb:59:23:59:23 | b | parameters.rb:59:23:59:23 | b |
8888
| parameters.rb:59:25:59:25 | c | parameters.rb:59:25:59:25 | c |
89-
| scopes.rb:1:1:73:3 | self (scopes.rb) | scopes.rb:1:1:73:3 | self |
90-
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:73:3 | self |
89+
| scopes.rb:1:1:89:4 | self (scopes.rb) | scopes.rb:1:1:89:4 | self |
90+
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:89:4 | self |
9191
| scopes.rb:4:4:4:4 | a | scopes.rb:4:4:4:4 | a |
9292
| scopes.rb:7:1:7:1 | a | scopes.rb:7:1:7:1 | a |
9393
| scopes.rb:9:9:18:3 | <captured entry> a | scopes.rb:7:1:7:1 | a |
94-
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:73:3 | self |
94+
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:89:4 | self |
9595
| scopes.rb:11:4:11:4 | a | scopes.rb:7:1:7:1 | a |
9696
| scopes.rb:13:4:13:4 | a | scopes.rb:7:1:7:1 | a |
9797
| scopes.rb:13:7:13:7 | b | scopes.rb:13:7:13:7 | b |
@@ -111,6 +111,12 @@ definition
111111
| scopes.rb:68:3:68:4 | xs | scopes.rb:68:3:68:4 | xs |
112112
| scopes.rb:69:11:71:5 | <captured entry> self | scopes.rb:66:1:73:3 | self |
113113
| scopes.rb:69:15:69:15 | x | scopes.rb:69:15:69:15 | x |
114+
| scopes.rb:75:1:89:3 | self (RescueSetter) | scopes.rb:75:1:89:3 | self |
115+
| scopes.rb:76:3:78:5 | self (name) | scopes.rb:76:3:78:5 | self |
116+
| scopes.rb:80:3:82:5 | self (name=) | scopes.rb:80:3:82:5 | self |
117+
| scopes.rb:80:13:80:17 | value | scopes.rb:80:13:80:17 | value |
118+
| scopes.rb:84:3:88:5 | self (foo) | scopes.rb:84:3:88:5 | self |
119+
| scopes.rb:84:11:84:13 | msg | scopes.rb:84:11:84:13 | msg |
114120
| ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self |
115121
| ssa.rb:1:7:1:7 | b | ssa.rb:1:7:1:7 | b |
116122
| ssa.rb:2:3:2:3 | i | ssa.rb:2:3:2:3 | i |
@@ -267,20 +273,20 @@ read
267273
| parameters.rb:59:20:59:20 | a | parameters.rb:59:20:59:20 | a | parameters.rb:60:11:60:11 | a |
268274
| parameters.rb:59:23:59:23 | b | parameters.rb:59:23:59:23 | b | parameters.rb:60:16:60:16 | b |
269275
| parameters.rb:59:25:59:25 | c | parameters.rb:59:25:59:25 | c | parameters.rb:60:21:60:21 | c |
270-
| scopes.rb:1:1:73:3 | self (scopes.rb) | scopes.rb:1:1:73:3 | self | scopes.rb:8:1:8:6 | self |
271-
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:3:4:3:9 | self |
272-
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:3:9:3:9 | self |
273-
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:5:4:5:9 | self |
276+
| scopes.rb:1:1:89:4 | self (scopes.rb) | scopes.rb:1:1:89:4 | self | scopes.rb:8:1:8:6 | self |
277+
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:3:4:3:9 | self |
278+
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:3:9:3:9 | self |
279+
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:5:4:5:9 | self |
274280
| scopes.rb:4:4:4:4 | a | scopes.rb:4:4:4:4 | a | scopes.rb:5:9:5:9 | a |
275281
| scopes.rb:7:1:7:1 | a | scopes.rb:7:1:7:1 | a | scopes.rb:8:6:8:6 | a |
276282
| scopes.rb:9:9:18:3 | <captured entry> a | scopes.rb:7:1:7:1 | a | scopes.rb:10:9:10:9 | a |
277283
| scopes.rb:9:9:18:3 | <captured entry> a | scopes.rb:7:1:7:1 | a | scopes.rb:11:4:11:4 | a |
278-
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:10:4:10:9 | self |
279-
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:12:4:12:9 | self |
280-
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:14:4:14:9 | self |
281-
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:15:4:15:9 | self |
282-
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:16:4:16:9 | self |
283-
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:17:4:17:9 | self |
284+
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:10:4:10:9 | self |
285+
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:12:4:12:9 | self |
286+
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:14:4:14:9 | self |
287+
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:15:4:15:9 | self |
288+
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:16:4:16:9 | self |
289+
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:17:4:17:9 | self |
284290
| scopes.rb:11:4:11:4 | a | scopes.rb:7:1:7:1 | a | scopes.rb:12:9:12:9 | a |
285291
| scopes.rb:13:4:13:4 | a | scopes.rb:7:1:7:1 | a | scopes.rb:14:9:14:9 | a |
286292
| scopes.rb:13:7:13:7 | b | scopes.rb:13:7:13:7 | b | scopes.rb:15:9:15:9 | b |
@@ -311,6 +317,12 @@ read
311317
| scopes.rb:68:3:68:4 | xs | scopes.rb:68:3:68:4 | xs | scopes.rb:69:3:69:4 | xs |
312318
| scopes.rb:69:11:71:5 | <captured entry> self | scopes.rb:66:1:73:3 | self | scopes.rb:70:5:70:10 | self |
313319
| scopes.rb:69:15:69:15 | x | scopes.rb:69:15:69:15 | x | scopes.rb:70:10:70:10 | x |
320+
| scopes.rb:76:3:78:5 | self (name) | scopes.rb:76:3:78:5 | self | scopes.rb:77:5:77:9 | self |
321+
| scopes.rb:80:3:82:5 | self (name=) | scopes.rb:80:3:82:5 | self | scopes.rb:81:5:81:9 | self |
322+
| scopes.rb:80:13:80:17 | value | scopes.rb:80:13:80:17 | value | scopes.rb:81:13:81:17 | value |
323+
| scopes.rb:84:3:88:5 | self (foo) | scopes.rb:84:3:88:5 | self | scopes.rb:85:5:85:13 | self |
324+
| scopes.rb:84:3:88:5 | self (foo) | scopes.rb:84:3:88:5 | self | scopes.rb:86:13:86:16 | self |
325+
| scopes.rb:84:11:84:13 | msg | scopes.rb:84:11:84:13 | msg | scopes.rb:85:11:85:13 | msg |
314326
| ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:3:3:3:8 | self |
315327
| ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:4:3:4:12 | self |
316328
| ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:7:5:7:10 | self |
@@ -460,12 +472,12 @@ firstRead
460472
| parameters.rb:59:20:59:20 | a | parameters.rb:59:20:59:20 | a | parameters.rb:60:11:60:11 | a |
461473
| parameters.rb:59:23:59:23 | b | parameters.rb:59:23:59:23 | b | parameters.rb:60:16:60:16 | b |
462474
| parameters.rb:59:25:59:25 | c | parameters.rb:59:25:59:25 | c | parameters.rb:60:21:60:21 | c |
463-
| scopes.rb:1:1:73:3 | self (scopes.rb) | scopes.rb:1:1:73:3 | self | scopes.rb:8:1:8:6 | self |
464-
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:3:4:3:9 | self |
475+
| scopes.rb:1:1:89:4 | self (scopes.rb) | scopes.rb:1:1:89:4 | self | scopes.rb:8:1:8:6 | self |
476+
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:3:4:3:9 | self |
465477
| scopes.rb:4:4:4:4 | a | scopes.rb:4:4:4:4 | a | scopes.rb:5:9:5:9 | a |
466478
| scopes.rb:7:1:7:1 | a | scopes.rb:7:1:7:1 | a | scopes.rb:8:6:8:6 | a |
467479
| scopes.rb:9:9:18:3 | <captured entry> a | scopes.rb:7:1:7:1 | a | scopes.rb:10:9:10:9 | a |
468-
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:10:4:10:9 | self |
480+
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:10:4:10:9 | self |
469481
| scopes.rb:11:4:11:4 | a | scopes.rb:7:1:7:1 | a | scopes.rb:12:9:12:9 | a |
470482
| scopes.rb:13:4:13:4 | a | scopes.rb:7:1:7:1 | a | scopes.rb:14:9:14:9 | a |
471483
| scopes.rb:13:7:13:7 | b | scopes.rb:13:7:13:7 | b | scopes.rb:15:9:15:9 | b |
@@ -485,6 +497,11 @@ firstRead
485497
| scopes.rb:68:3:68:4 | xs | scopes.rb:68:3:68:4 | xs | scopes.rb:69:3:69:4 | xs |
486498
| scopes.rb:69:11:71:5 | <captured entry> self | scopes.rb:66:1:73:3 | self | scopes.rb:70:5:70:10 | self |
487499
| scopes.rb:69:15:69:15 | x | scopes.rb:69:15:69:15 | x | scopes.rb:70:10:70:10 | x |
500+
| scopes.rb:76:3:78:5 | self (name) | scopes.rb:76:3:78:5 | self | scopes.rb:77:5:77:9 | self |
501+
| scopes.rb:80:3:82:5 | self (name=) | scopes.rb:80:3:82:5 | self | scopes.rb:81:5:81:9 | self |
502+
| scopes.rb:80:13:80:17 | value | scopes.rb:80:13:80:17 | value | scopes.rb:81:13:81:17 | value |
503+
| scopes.rb:84:3:88:5 | self (foo) | scopes.rb:84:3:88:5 | self | scopes.rb:85:5:85:13 | self |
504+
| scopes.rb:84:11:84:13 | msg | scopes.rb:84:11:84:13 | msg | scopes.rb:85:11:85:13 | msg |
488505
| ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:3:3:3:8 | self |
489506
| ssa.rb:1:7:1:7 | b | ssa.rb:1:7:1:7 | b | ssa.rb:5:6:5:6 | b |
490507
| ssa.rb:2:3:2:3 | i | ssa.rb:2:3:2:3 | i | ssa.rb:3:8:3:8 | i |
@@ -557,14 +574,14 @@ adjacentReads
557574
| parameters.rb:25:1:28:3 | self (opt_param) | parameters.rb:25:1:28:3 | self | parameters.rb:26:3:26:11 | self | parameters.rb:27:3:27:11 | self |
558575
| parameters.rb:25:15:25:18 | name | parameters.rb:25:15:25:18 | name | parameters.rb:25:40:25:43 | name | parameters.rb:26:8:26:11 | name |
559576
| parameters.rb:54:9:57:3 | <captured entry> self | parameters.rb:1:1:62:1 | self | parameters.rb:55:4:55:9 | self | parameters.rb:56:4:56:9 | self |
560-
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:3:4:3:9 | self | scopes.rb:3:9:3:9 | self |
561-
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:3:9:3:9 | self | scopes.rb:5:4:5:9 | self |
577+
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:3:4:3:9 | self | scopes.rb:3:9:3:9 | self |
578+
| scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:3:9:3:9 | self | scopes.rb:5:4:5:9 | self |
562579
| scopes.rb:9:9:18:3 | <captured entry> a | scopes.rb:7:1:7:1 | a | scopes.rb:10:9:10:9 | a | scopes.rb:11:4:11:4 | a |
563-
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:10:4:10:9 | self | scopes.rb:12:4:12:9 | self |
564-
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:12:4:12:9 | self | scopes.rb:14:4:14:9 | self |
565-
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:14:4:14:9 | self | scopes.rb:15:4:15:9 | self |
566-
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:15:4:15:9 | self | scopes.rb:16:4:16:9 | self |
567-
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:73:3 | self | scopes.rb:16:4:16:9 | self | scopes.rb:17:4:17:9 | self |
580+
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:10:4:10:9 | self | scopes.rb:12:4:12:9 | self |
581+
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:12:4:12:9 | self | scopes.rb:14:4:14:9 | self |
582+
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:14:4:14:9 | self | scopes.rb:15:4:15:9 | self |
583+
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:15:4:15:9 | self | scopes.rb:16:4:16:9 | self |
584+
| scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:89:4 | self | scopes.rb:16:4:16:9 | self | scopes.rb:17:4:17:9 | self |
568585
| scopes.rb:13:10:13:15 | __synth__2__1 | scopes.rb:13:10:13:15 | __synth__2__1 | scopes.rb:13:11:13:11 | __synth__2__1 | scopes.rb:13:14:13:14 | __synth__2__1 |
569586
| scopes.rb:13:19:13:32 | __synth__3 | scopes.rb:13:4:13:32 | __synth__3 | scopes.rb:13:4:13:4 | __synth__3 | scopes.rb:13:7:13:7 | __synth__3 |
570587
| scopes.rb:13:19:13:32 | __synth__3 | scopes.rb:13:4:13:32 | __synth__3 | scopes.rb:13:7:13:7 | __synth__3 | scopes.rb:13:10:13:15 | __synth__3 |
@@ -576,6 +593,7 @@ adjacentReads
576593
| scopes.rb:51:1:64:3 | self (ExceptionVariable) | scopes.rb:51:1:64:3 | self | scopes.rb:59:5:59:21 | self | scopes.rb:61:5:61:10 | self |
577594
| scopes.rb:51:1:64:3 | self (ExceptionVariable) | scopes.rb:51:1:64:3 | self | scopes.rb:61:5:61:10 | self | scopes.rb:63:3:63:8 | self |
578595
| scopes.rb:60:25:60:25 | x | scopes.rb:55:3:55:3 | x | scopes.rb:61:10:61:10 | x | scopes.rb:63:8:63:8 | x |
596+
| scopes.rb:84:3:88:5 | self (foo) | scopes.rb:84:3:88:5 | self | scopes.rb:85:5:85:13 | self | scopes.rb:86:13:86:16 | self |
579597
| ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:3:3:3:8 | self | ssa.rb:4:3:4:12 | self |
580598
| ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:4:3:4:12 | self | ssa.rb:7:5:7:10 | self |
581599
| ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:4:3:4:12 | self | ssa.rb:11:5:11:10 | self |

0 commit comments

Comments
 (0)