Skip to content

Commit edde3de

Browse files
hvitvedhvitved
authored
Merge pull request #11153 from hvitved/ruby/basic-block-at-conditions
Ruby: Split basic blocks around constant conditionals
2 parents d813590 + f0b9ca4 commit edde3de

File tree

4 files changed

+98
-1
lines changed

4 files changed

+98
-1
lines changed

‎ruby/ql/lib/codeql/ruby/controlflow/BasicBlocks.qll

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,30 @@ private module Cached {
252252
cfn.isJoin()
253253
or
254254
cfn.getAPredecessor().isBranch()
255+
or
256+
/*
257+
* In cases such as
258+
*
259+
* ```rb
260+
* if x or y
261+
* foo
262+
* else
263+
* bar
264+
* ```
265+
*
266+
* we have a CFG that looks like
267+
*
268+
* x --false--> [false] x or y --false--> bar
269+
* \ |
270+
* --true--> y --false--
271+
* \
272+
* --true--> [true] x or y --true--> foo
273+
*
274+
* and we want to ensure that both `foo` and `bar` start a new basic block,
275+
* in order to get a `ConditionalBlock` out of the disjunction.
276+
*/
277+
278+
exists(cfn.getAPredecessor(any(SuccessorTypes::ConditionalSuccessor s)))
255279
}
256280

257281
/**

‎ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.expected

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
WARNING: Type BarrierGuard has been deprecated and may be removed in future (barrier-guards.ql:8,3-15)
1+
WARNING: Type BarrierGuard has been deprecated and may be removed in future (barrier-guards.ql:9,3-15)
22
oldStyleBarrierGuards
33
| barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:4:5:4:7 | foo | barrier-guards.rb:3:4:3:6 | foo | true |
44
| barrier-guards.rb:9:4:9:24 | call to include? | barrier-guards.rb:10:5:10:7 | foo | barrier-guards.rb:9:21:9:23 | foo | true |
@@ -20,3 +20,50 @@ newStyleBarrierGuards
2020
| barrier-guards.rb:71:5:71:7 | foo |
2121
| barrier-guards.rb:83:5:83:7 | foo |
2222
| barrier-guards.rb:91:5:91:7 | foo |
23+
controls
24+
| barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:4:5:4:7 | foo | true |
25+
| barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:6:5:6:7 | foo | false |
26+
| barrier-guards.rb:9:4:9:24 | call to include? | barrier-guards.rb:10:5:10:7 | foo | true |
27+
| barrier-guards.rb:9:4:9:24 | call to include? | barrier-guards.rb:12:5:12:7 | foo | false |
28+
| barrier-guards.rb:15:4:15:15 | ... != ... | barrier-guards.rb:16:5:16:7 | foo | true |
29+
| barrier-guards.rb:15:4:15:15 | ... != ... | barrier-guards.rb:18:5:18:7 | foo | false |
30+
| barrier-guards.rb:21:8:21:19 | ... == ... | barrier-guards.rb:22:5:22:7 | foo | false |
31+
| barrier-guards.rb:21:8:21:19 | ... == ... | barrier-guards.rb:24:5:24:7 | foo | true |
32+
| barrier-guards.rb:27:8:27:19 | ... != ... | barrier-guards.rb:28:5:28:7 | foo | false |
33+
| barrier-guards.rb:27:8:27:19 | ... != ... | barrier-guards.rb:30:5:30:7 | foo | true |
34+
| barrier-guards.rb:37:4:37:20 | call to include? | barrier-guards.rb:38:5:38:7 | foo | true |
35+
| barrier-guards.rb:37:4:37:20 | call to include? | barrier-guards.rb:40:5:40:7 | foo | false |
36+
| barrier-guards.rb:43:4:43:15 | ... == ... | barrier-guards.rb:44:5:46:5 | self | true |
37+
| barrier-guards.rb:49:4:49:15 | ... == ... | barrier-guards.rb:50:5:53:5 | self | true |
38+
| barrier-guards.rb:56:4:56:15 | ... == ... | barrier-guards.rb:57:5:57:13 | my_lambda | true |
39+
| barrier-guards.rb:70:4:70:21 | call to include? | barrier-guards.rb:71:5:71:7 | foo | true |
40+
| barrier-guards.rb:70:4:70:21 | call to include? | barrier-guards.rb:73:5:73:7 | foo | false |
41+
| barrier-guards.rb:76:4:76:21 | call to include? | barrier-guards.rb:77:5:77:7 | foo | true |
42+
| barrier-guards.rb:76:4:76:21 | call to include? | barrier-guards.rb:79:5:79:7 | foo | false |
43+
| barrier-guards.rb:82:4:82:25 | ... != ... | barrier-guards.rb:83:5:83:7 | foo | true |
44+
| barrier-guards.rb:82:4:82:25 | ... != ... | barrier-guards.rb:85:5:85:7 | foo | false |
45+
| barrier-guards.rb:88:4:88:25 | ... == ... | barrier-guards.rb:89:5:89:7 | foo | true |
46+
| barrier-guards.rb:88:4:88:25 | ... == ... | barrier-guards.rb:91:5:91:7 | foo | false |
47+
| barrier-guards.rb:96:4:96:12 | call to condition | barrier-guards.rb:97:5:97:8 | bars | true |
48+
| barrier-guards.rb:100:4:100:21 | call to include? | barrier-guards.rb:101:5:101:7 | foo | true |
49+
| barrier-guards.rb:100:4:100:21 | call to include? | barrier-guards.rb:103:5:103:7 | foo | false |
50+
| barrier-guards.rb:106:4:106:4 | call to x | barrier-guards.rb:106:4:106:9 | [false] ... or ... | false |
51+
| barrier-guards.rb:106:4:106:4 | call to x | barrier-guards.rb:106:9:106:9 | self | false |
52+
| barrier-guards.rb:106:4:106:4 | call to x | barrier-guards.rb:109:5:109:8 | bars | false |
53+
| barrier-guards.rb:106:4:106:9 | [false] ... or ... | barrier-guards.rb:109:5:109:8 | bars | false |
54+
| barrier-guards.rb:106:4:106:9 | [true] ... or ... | barrier-guards.rb:107:5:107:7 | foo | true |
55+
| barrier-guards.rb:106:9:106:9 | call to y | barrier-guards.rb:106:4:106:9 | [false] ... or ... | false |
56+
| barrier-guards.rb:106:9:106:9 | call to y | barrier-guards.rb:109:5:109:8 | bars | false |
57+
| barrier-guards.rb:112:4:112:4 | call to x | barrier-guards.rb:112:4:112:10 | [true] ... and ... | true |
58+
| barrier-guards.rb:112:4:112:4 | call to x | barrier-guards.rb:112:10:112:10 | self | true |
59+
| barrier-guards.rb:112:4:112:4 | call to x | barrier-guards.rb:113:5:113:7 | foo | true |
60+
| barrier-guards.rb:112:4:112:10 | [false] ... and ... | barrier-guards.rb:115:5:115:8 | bars | false |
61+
| barrier-guards.rb:112:4:112:10 | [true] ... and ... | barrier-guards.rb:113:5:113:7 | foo | true |
62+
| barrier-guards.rb:112:10:112:10 | call to y | barrier-guards.rb:112:4:112:10 | [true] ... and ... | true |
63+
| barrier-guards.rb:112:10:112:10 | call to y | barrier-guards.rb:113:5:113:7 | foo | true |
64+
| barrier-guards.rb:118:4:118:8 | [false] not ... | barrier-guards.rb:121:5:121:8 | bars | false |
65+
| barrier-guards.rb:118:4:118:8 | [true] not ... | barrier-guards.rb:119:5:119:7 | foo | true |
66+
| barrier-guards.rb:118:8:118:8 | call to x | barrier-guards.rb:118:4:118:8 | [false] not ... | true |
67+
| barrier-guards.rb:118:8:118:8 | call to x | barrier-guards.rb:118:4:118:8 | [true] not ... | false |
68+
| barrier-guards.rb:118:8:118:8 | call to x | barrier-guards.rb:119:5:119:7 | foo | false |
69+
| barrier-guards.rb:118:8:118:8 | call to x | barrier-guards.rb:121:5:121:8 | bars | true |

‎ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.ql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import codeql.ruby.dataflow.internal.DataFlowPublic
22
import codeql.ruby.dataflow.BarrierGuards
33
import codeql.ruby.controlflow.CfgNodes
44
import codeql.ruby.controlflow.ControlFlowGraph
5+
import codeql.ruby.controlflow.BasicBlocks
56
import codeql.ruby.DataFlow
67

78
query predicate oldStyleBarrierGuards(
@@ -14,3 +15,10 @@ query predicate newStyleBarrierGuards(DataFlow::Node n) {
1415
n instanceof StringConstCompareBarrier or
1516
n instanceof StringConstArrayInclusionCallBarrier
1617
}
18+
19+
query predicate controls(CfgNode condition, BasicBlock bb, SuccessorTypes::ConditionalSuccessor s) {
20+
exists(ConditionBlock cb |
21+
cb.controls(bb, s) and
22+
condition = cb.getLastNode()
23+
)
24+
}

‎ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,21 @@
102102
else
103103
foo
104104
end
105+
106+
if x or y then
107+
foo
108+
else
109+
bars
110+
end
111+
112+
if x and y then
113+
foo
114+
else
115+
bars
116+
end
117+
118+
if not x then
119+
foo
120+
else
121+
bars
122+
end

0 commit comments

Comments
 (0)