-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Java: Use the shared BasicBlocks library. #19505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f202586
13c5906
db01828
6b830fa
a98d93b
3fde675
10efea1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
--- | ||
category: deprecated | ||
--- | ||
* Java now uses the shared `BasicBlock` library. This means that the names of several member predicates have been changed to align with the names used in other languages. The old predicates have been deprecated. The `BasicBlock` class itself no longer extends `ControlFlowNode` - the predicate `getFirstNode` can be used to fix any QL code that somehow relied on this. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,91 +8,75 @@ | |
* Predicates for basic-block-level dominance. | ||
*/ | ||
|
||
/** Entry points for control-flow. */ | ||
private predicate flowEntry(BasicBlock entry) { | ||
exists(Stmt entrystmt | entrystmt = entry.getFirstNode().asStmt() | | ||
exists(Callable c | entrystmt = c.getBody()) | ||
or | ||
// This disjunct is technically superfluous, but safeguards against extractor problems. | ||
entrystmt instanceof BlockStmt and | ||
not exists(entry.getEnclosingCallable()) and | ||
not entrystmt.getParent() instanceof Stmt | ||
) | ||
} | ||
|
||
/** The successor relation for basic blocks. */ | ||
private predicate bbSucc(BasicBlock pre, BasicBlock post) { post = pre.getABBSuccessor() } | ||
|
||
/** The immediate dominance relation for basic blocks. */ | ||
cached | ||
predicate bbIDominates(BasicBlock dom, BasicBlock node) = | ||
idominance(flowEntry/1, bbSucc/2)(_, dom, node) | ||
|
||
/** Holds if the dominance relation is calculated for `bb`. */ | ||
predicate hasDominanceInformation(BasicBlock bb) { | ||
exists(BasicBlock entry | flowEntry(entry) and bbSucc*(entry, bb)) | ||
/** | ||
* DEPRECATED: Use `BasicBlock::immediatelyDominates` instead. | ||
* | ||
* The immediate dominance relation for basic blocks. | ||
*/ | ||
deprecated predicate bbIDominates(BasicBlock dom, BasicBlock node) { | ||
dom.immediatelyDominates(node) | ||
} | ||
|
||
/** Exit points for basic-block control-flow. */ | ||
private predicate bbSink(BasicBlock exit) { exit.getLastNode() instanceof ControlFlow::ExitNode } | ||
|
||
/** Reversed `bbSucc`. */ | ||
private predicate bbPred(BasicBlock post, BasicBlock pre) { post = pre.getABBSuccessor() } | ||
private predicate bbPred(BasicBlock post, BasicBlock pre) { post = pre.getASuccessor() } | ||
Check warningCode scanning / CodeQL Missing QLDoc for parameter Warning
The QLDoc has no documentation for post, or pre, but the QLDoc mentions bbSucc
|
||
|
||
/** The immediate post-dominance relation on basic blocks. */ | ||
cached | ||
predicate bbIPostDominates(BasicBlock dominator, BasicBlock node) = | ||
deprecated predicate bbIPostDominates(BasicBlock dominator, BasicBlock node) = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Immediate post-dominance is not exposed in the shared lib - I'm not sure we've ever seen any use-cases for it, though, so adding it is probably of little value - hence why I simply opted to discretely deprecate this single public instance. |
||
idominance(bbSink/1, bbPred/2)(_, dominator, node) | ||
|
||
/** Holds if `dom` strictly dominates `node`. */ | ||
predicate bbStrictlyDominates(BasicBlock dom, BasicBlock node) { bbIDominates+(dom, node) } | ||
|
||
/** Holds if `dom` dominates `node`. (This is reflexive.) */ | ||
predicate bbDominates(BasicBlock dom, BasicBlock node) { | ||
bbStrictlyDominates(dom, node) or dom = node | ||
/** | ||
* DEPRECATED: Use `BasicBlock::strictlyDominates` instead. | ||
* | ||
* Holds if `dom` strictly dominates `node`. | ||
*/ | ||
deprecated predicate bbStrictlyDominates(BasicBlock dom, BasicBlock node) { | ||
dom.strictlyDominates(node) | ||
} | ||
|
||
/** Holds if `dom` strictly post-dominates `node`. */ | ||
predicate bbStrictlyPostDominates(BasicBlock dom, BasicBlock node) { bbIPostDominates+(dom, node) } | ||
/** | ||
* DEPRECATED: Use `BasicBlock::dominates` instead. | ||
* | ||
* Holds if `dom` dominates `node`. (This is reflexive.) | ||
*/ | ||
deprecated predicate bbDominates(BasicBlock dom, BasicBlock node) { dom.dominates(node) } | ||
|
||
/** Holds if `dom` post-dominates `node`. (This is reflexive.) */ | ||
predicate bbPostDominates(BasicBlock dom, BasicBlock node) { | ||
bbStrictlyPostDominates(dom, node) or dom = node | ||
/** | ||
* DEPRECATED: Use `BasicBlock::strictlyPostDominates` instead. | ||
* | ||
* Holds if `dom` strictly post-dominates `node`. | ||
*/ | ||
deprecated predicate bbStrictlyPostDominates(BasicBlock dom, BasicBlock node) { | ||
dom.strictlyPostDominates(node) | ||
} | ||
|
||
/** | ||
* DEPRECATED: Use `BasicBlock::postDominates` instead. | ||
* | ||
* Holds if `dom` post-dominates `node`. (This is reflexive.) | ||
*/ | ||
deprecated predicate bbPostDominates(BasicBlock dom, BasicBlock node) { dom.postDominates(node) } | ||
|
||
/** | ||
* The dominance frontier relation for basic blocks. | ||
* | ||
* This is equivalent to: | ||
* | ||
* ``` | ||
* bbDominates(x, w.getABBPredecessor()) and not bbStrictlyDominates(x, w) | ||
* x.dominates(w.getAPredecessor()) and not x.strictlyDominates(w) | ||
* ``` | ||
*/ | ||
predicate dominanceFrontier(BasicBlock x, BasicBlock w) { | ||
x = w.getABBPredecessor() and not bbIDominates(x, w) | ||
x = w.getAPredecessor() and not x.immediatelyDominates(w) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about deprecating this in favor of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think dominance frontier is used for anything else than SSA construction, and the shared SSA lib does its own calculation, so perhaps we should deprecate both. In any case, I think that's fine to postpone for now. |
||
or | ||
exists(BasicBlock prev | dominanceFrontier(prev, w) | | ||
bbIDominates(x, prev) and | ||
not bbIDominates(x, w) | ||
x.immediatelyDominates(prev) and | ||
not x.immediatelyDominates(w) | ||
) | ||
} | ||
|
||
/** | ||
* Holds if `(bb1, bb2)` is an edge that dominates `bb2`, that is, all other | ||
* predecessors of `bb2` are dominated by `bb2`. This implies that `bb1` is the | ||
* immediate dominator of `bb2`. | ||
* | ||
* This is a necessary and sufficient condition for an edge to dominate anything, | ||
* and in particular `dominatingEdge(bb1, bb2) and bb2.bbDominates(bb3)` means | ||
* that the edge `(bb1, bb2)` dominates `bb3`. | ||
*/ | ||
predicate dominatingEdge(BasicBlock bb1, BasicBlock bb2) { | ||
bbIDominates(bb1, bb2) and | ||
bb1.getABBSuccessor() = bb2 and | ||
forall(BasicBlock pred | pred = bb2.getABBPredecessor() and pred != bb1 | bbDominates(bb2, pred)) | ||
} | ||
|
||
/* | ||
* Predicates for expression-level dominance. | ||
*/ | ||
|
@@ -102,7 +86,7 @@ | |
exists(BasicBlock bb, int i | dominator = bb.getNode(i) and node = bb.getNode(i + 1)) | ||
or | ||
exists(BasicBlock dom, BasicBlock bb | | ||
bbIDominates(dom, bb) and | ||
dom.immediatelyDominates(bb) and | ||
dominator = dom.getLastNode() and | ||
node = bb.getFirstNode() | ||
) | ||
|
@@ -112,7 +96,7 @@ | |
pragma[inline] | ||
predicate strictlyDominates(ControlFlowNode dom, ControlFlowNode node) { | ||
// This predicate is gigantic, so it must be inlined. | ||
bbStrictlyDominates(dom.getBasicBlock(), node.getBasicBlock()) | ||
dom.getBasicBlock().strictlyDominates(node.getBasicBlock()) | ||
or | ||
exists(BasicBlock b, int i, int j | dom = b.getNode(i) and node = b.getNode(j) and i < j) | ||
} | ||
|
@@ -121,7 +105,7 @@ | |
pragma[inline] | ||
predicate dominates(ControlFlowNode dom, ControlFlowNode node) { | ||
// This predicate is gigantic, so it must be inlined. | ||
bbStrictlyDominates(dom.getBasicBlock(), node.getBasicBlock()) | ||
dom.getBasicBlock().strictlyDominates(node.getBasicBlock()) | ||
or | ||
exists(BasicBlock b, int i, int j | dom = b.getNode(i) and node = b.getNode(j) and i <= j) | ||
} | ||
|
@@ -130,7 +114,7 @@ | |
pragma[inline] | ||
predicate strictlyPostDominates(ControlFlowNode dom, ControlFlowNode node) { | ||
// This predicate is gigantic, so it must be inlined. | ||
bbStrictlyPostDominates(dom.getBasicBlock(), node.getBasicBlock()) | ||
dom.getBasicBlock().strictlyPostDominates(node.getBasicBlock()) | ||
or | ||
exists(BasicBlock b, int i, int j | dom = b.getNode(i) and node = b.getNode(j) and i > j) | ||
} | ||
|
@@ -139,7 +123,7 @@ | |
pragma[inline] | ||
predicate postDominates(ControlFlowNode dom, ControlFlowNode node) { | ||
// This predicate is gigantic, so it must be inlined. | ||
bbStrictlyPostDominates(dom.getBasicBlock(), node.getBasicBlock()) | ||
dom.getBasicBlock().strictlyPostDominates(node.getBasicBlock()) | ||
or | ||
exists(BasicBlock b, int i, int j | dom = b.getNode(i) and node = b.getNode(j) and i >= j) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed offline it might be nice to agree on a name for this predicate that every language can use. Right now Java's basic block will have both
getScope
andgetEnclosingCallable
that does the exact same thing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I think that's best left as a follow-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, absolutely :)