Skip to content

Commit 72384e5

Browse files
committed
C#: Speedup Assertion::strictlyDominates()
1 parent baa596c commit 72384e5

File tree

1 file changed

+62
-22
lines changed

1 file changed

+62
-22
lines changed

csharp/ql/src/semmle/code/csharp/commons/Assertions.qll

Lines changed: 62 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -42,41 +42,81 @@ class Assertion extends MethodCall {
4242
/** Gets the expression that this assertion pertains to. */
4343
Expr getExpr() { result = this.getArgumentForParameter(target.getAssertedParameter()) }
4444

45+
/**
46+
* Holds if basic block `succ` is immediately dominated by this assertion.
47+
* That is, `succ` can only be reached from the callable entry point by
48+
* going via *some* basic block `pred` containing this assertion, and `pred`
49+
* is an immediate predecessor of `succ`.
50+
*
51+
* Moreover, this assertion corresponds to multiple control flow nodes,
52+
* which is why
53+
*
54+
* ```
55+
* exists(BasicBlock bb |
56+
* bb.getANode() = this.getAControlFlowNode() |
57+
* cb.immediatelyDominates(succ)
58+
* )
59+
* ```
60+
*
61+
* does not work.
62+
*/
4563
pragma[nomagic]
46-
private JoinBlockPredecessor getAPossiblyDominatedPredecessor(JoinBlock jb) {
64+
private predicate immediatelyDominatesBlockSplit(BasicBlock succ) {
4765
// Only calculate dominance by explicit recursion for split nodes;
4866
// all other nodes can use regular CFG dominance
4967
this instanceof ControlFlow::Internal::SplitControlFlowElement and
50-
exists(BasicBlock bb | bb = this.getAControlFlowNode().getBasicBlock() |
51-
result = bb.getASuccessor*()
52-
) and
53-
result.getASuccessor() = jb and
54-
not jb.dominates(result)
68+
exists(BasicBlock bb | bb.getANode() = this.getAControlFlowNode() |
69+
succ = bb.getASuccessor() and
70+
forall(BasicBlock pred | pred = succ.getAPredecessor() and pred != bb |
71+
succ.dominates(pred)
72+
or
73+
// `pred` might be another split of this element
74+
pred.getANode().getElement() = this
75+
)
76+
)
5577
}
5678

57-
pragma[nomagic]
58-
private predicate isPossiblyDominatedJoinBlock(JoinBlock jb) {
59-
exists(this.getAPossiblyDominatedPredecessor(jb)) and
60-
forall(BasicBlock pred | pred = jb.getAPredecessor() |
61-
pred = this.getAPossiblyDominatedPredecessor(jb)
79+
pragma[noinline]
80+
private predicate strictlyDominatesJoinBlockPredecessor(JoinBlock jb, int i) {
81+
this.strictlyDominatesSplit(jb.getJoinBlockPredecessor(i))
82+
}
83+
84+
private predicate strictlyDominatesJoinBlockSplit(JoinBlock jb, int i) {
85+
i = -1 and
86+
this.strictlyDominatesJoinBlockPredecessor(jb, _)
87+
or
88+
this.strictlyDominatesJoinBlockSplit(jb, i - 1) and
89+
(
90+
this.strictlyDominatesJoinBlockPredecessor(jb, i)
6291
or
63-
jb.dominates(pred)
92+
this.getAControlFlowNode().getBasicBlock() = jb.getJoinBlockPredecessor(i)
6493
)
6594
}
6695

6796
pragma[nomagic]
6897
private predicate strictlyDominatesSplit(BasicBlock bb) {
69-
this.getAControlFlowNode().getBasicBlock().immediatelyDominates(bb)
98+
this.immediatelyDominatesBlockSplit(bb)
7099
or
71-
if bb instanceof JoinBlock
72-
then
73-
this.isPossiblyDominatedJoinBlock(bb) and
74-
forall(BasicBlock pred | pred = this.getAPossiblyDominatedPredecessor(bb) |
75-
this.strictlyDominatesSplit(pred)
76-
or
77-
this.getAControlFlowNode().getBasicBlock() = pred
78-
)
79-
else this.strictlyDominatesSplit(bb.getAPredecessor())
100+
// Equivalent with
101+
//
102+
// ```
103+
// exists(JoinBlockPredecessor pred | pred = bb.getAPredecessor() |
104+
// this.strictlyDominatesSplit(pred)
105+
// ) and
106+
// forall(JoinBlockPredecessor pred | pred = bb.getAPredecessor() |
107+
// this.strictlyDominatesSplit(pred)
108+
// or
109+
// this.getAControlFlowNode().getBasicBlock() = pred
110+
// )
111+
// ```
112+
//
113+
// but uses no universal recursion for better performance.
114+
exists(int last | last = max(int i | exists(bb.(JoinBlock).getJoinBlockPredecessor(i))) |
115+
this.strictlyDominatesJoinBlockSplit(bb, last)
116+
)
117+
or
118+
not bb instanceof JoinBlock and
119+
this.strictlyDominatesSplit(bb.getAPredecessor())
80120
}
81121

82122
/**

0 commit comments

Comments
 (0)