Skip to content

Commit e495805

Browse files
committed
Fix iteration in redundant goto elimination
This loop expected that, for a given exitTreeTop, the next tree exitTreeTop->getNextTreeTop() would remain fixed even if the current block is removed from the trees. However, we shouldn't assume anything about the next/prev pointers of trees that have been deleted, since it's highly unclear which trees are supposed to be before/after a deleted tree. And indeed, the removal of the current block can sometimes update the next pointer. In particular, this happens when removing an empty block (in the following example, block_X) with two predecessors, one of which falls through (block_F) and one of which branches (block_B): BBStart <block_B> ... if... --> block_X BBEnd </block_B> ... BBStart <block_F> ... (can fall through) BBEnd </block_F> BBStart <block_X> BBEnd </block_X> BBStart <block_N> ... BBEnd </block_N> BBStart <block_N'> ... BBEnd </block_N'> When removing block_X, redundant goto elimination (indirectly) calls TR::Block::insertBlockAsFallThrough() to make block_F fall through directly to block_N. This effectively clips out block_N's trees and then reinserts them immediately after block_F. As a result, before block_X is deleted, the trees are temporarily in the order shown below, where the tree immediately following the exit of block_X is the entry of block_N', not block_N: BBStart <block_F> ... (can fall through) BBEnd </block_F> BBStart <block_N> ... BBEnd </block_N> BBStart <block_X> BBEnd </block_X> BBStart <block_N'> ... BBEnd </block_N'> The typical consequence of the mistaken assumption that the next tree remains stable would be just that processing skips block_N and goes directly to block_N'. However, this assumption can cause a crash when redundant goto elimination is requested on block_X specifically. In that case, endTree is the entry of block_N, and because block_N is skipped, the loop doesn't terminate properly. Eventually it runs past the last block and tries to do treeTop->getNode() when treeTop is null. The crash went unnoticed because the code in dead trees elimination that requests redundant goto elimination on particular blocks was instead mistakenly requesting a pass over the entire method. To prevent these issues, redundant goto elimination now gets the entry of the next block before attempting to remove the current block.
1 parent c086f16 commit e495805

File tree

1 file changed

+3
-3
lines changed

1 file changed

+3
-3
lines changed

compiler/optimizer/LocalOpts.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -3338,16 +3338,16 @@ int32_t TR_EliminateRedundantGotos::process(TR::TreeTop *startTree, TR::TreeTop
33383338
bool gotosWereEliminated = false;
33393339

33403340

3341-
for (TR::TreeTop *treeTop = startTree, *exitTreeTop;
3341+
for (TR::TreeTop *treeTop = startTree, *nextBlockStartTree = NULL;
33423342
(treeTop != endTree);
3343-
treeTop = exitTreeTop->getNextTreeTop())
3343+
treeTop = nextBlockStartTree)
33443344
{
33453345
// Get information about this block
33463346
//
33473347
TR::Node *node = treeTop->getNode();
33483348
TR_ASSERT(node->getOpCodeValue() == TR::BBStart, "Local Opts, expected BBStart treetop");
33493349
TR::Block *block = node->getBlock();
3350-
exitTreeTop = block->getExit();
3350+
nextBlockStartTree = block->getExit()->getNextTreeTop();
33513351

33523352
if (block->hasExceptionPredecessors())
33533353
continue;

0 commit comments

Comments
 (0)