Skip to content

Commit b154c93

Browse files
committed
Improve performance of ExprChildMapping::reachesBasicBlock()
Since all expressions are now post-order, the logic of `reachesBasicBlock` can be simplified, and performance can be improved as well.
1 parent 88fb3c7 commit b154c93

1 file changed

Lines changed: 14 additions & 40 deletions

File tree

ql/src/codeql_ruby/controlflow/CfgNodes.qll

Lines changed: 14 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,13 @@ class ReturningCfgNode extends AstCfgNode {
113113
}
114114
}
115115

116+
private Expr desugar(Expr n) {
117+
result = n.getDesugared()
118+
or
119+
not exists(n.getDesugared()) and
120+
result = n
121+
}
122+
116123
/**
117124
* A class for mapping parent-child AST nodes to parent-child CFG nodes.
118125
*/
@@ -123,43 +130,19 @@ abstract private class ExprChildMapping extends Expr {
123130
*/
124131
abstract predicate relevantChild(Expr child);
125132

126-
pragma[noinline]
127-
private BasicBlock getABasicBlockInScope() {
128-
result.getANode() = TAstCfgNode(this.getAChild*(), _)
129-
}
130-
131133
pragma[nomagic]
132-
private predicate reachesBasicBlockBase(Expr child, CfgNode cfn, BasicBlock bb) {
134+
private predicate reachesBasicBlock(Expr child, CfgNode cfn, BasicBlock bb) {
133135
this.relevantChild(child) and
134136
cfn = this.getAControlFlowNode() and
135137
bb.getANode() = cfn
136-
}
137-
138-
pragma[nomagic]
139-
private predicate reachesBasicBlock(Expr child, CfgNode cfn, BasicBlock bb) {
140-
this.reachesBasicBlockBase(child, cfn, bb)
141138
or
142-
this.relevantChild(child) and
143-
this.reachesBasicBlockRec(child, cfn, bb) and
144-
bb = this.getABasicBlockInScope()
145-
}
146-
147-
pragma[nomagic]
148-
private predicate reachesBasicBlockRec(Expr child, CfgNode cfn, BasicBlock bb) {
149-
exists(BasicBlock mid | this.reachesBasicBlock(child, cfn, mid) |
150-
bb = mid.getASuccessor()
151-
or
152-
bb = mid.getAPredecessor()
139+
exists(BasicBlock mid |
140+
this.reachesBasicBlock(child, cfn, mid) and
141+
bb = mid.getAPredecessor() and
142+
not mid.getANode().getNode() = child
153143
)
154144
}
155145

156-
private Expr desugar(Expr n) {
157-
result = n.getDesugared()
158-
or
159-
not exists(n.getDesugared()) and
160-
result = n
161-
}
162-
163146
/**
164147
* Holds if there is a control-flow path from `cfn` to `cfnChild`, where `cfn`
165148
* is a control-flow node for this expression, and `cfnChild` is a control-flow
@@ -169,17 +152,8 @@ abstract private class ExprChildMapping extends Expr {
169152
*/
170153
cached
171154
predicate hasCfgChild(Expr child, CfgNode cfn, CfgNode cfnChild) {
172-
exists(BasicBlock bb |
173-
this.reachesBasicBlockBase(child, cfn, bb) and
174-
cfnChild = bb.getANode() and
175-
cfnChild = desugar(child).getAControlFlowNode()
176-
)
177-
or
178-
exists(BasicBlock bb |
179-
this.reachesBasicBlockRec(child, cfn, bb) and
180-
cfnChild = bb.getANode() and
181-
cfnChild = desugar(child).getAControlFlowNode()
182-
)
155+
this.reachesBasicBlock(child, cfn, cfnChild.getBasicBlock()) and
156+
cfnChild = desugar(child).getAControlFlowNode()
183157
}
184158
}
185159

0 commit comments

Comments
 (0)