summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorAndy Ayers <andya@microsoft.com>2018-06-27 16:49:54 (GMT)
committerGitHub <noreply@github.com>2018-06-27 16:49:54 (GMT)
commitaa00dad99dff3af496b245e6b10500a5e631a8ba (patch)
tree1275079defa8f857841f5ed3c537951f06c4ce72 /src
parent373b10e0edf959665d9fdaf312b3dceb8cedbe81 (diff)
downloadcoreclr-aa00dad99dff3af496b245e6b10500a5e631a8ba.zip
coreclr-aa00dad99dff3af496b245e6b10500a5e631a8ba.tar.gz
coreclr-aa00dad99dff3af496b245e6b10500a5e631a8ba.tar.bz2
JIT: Fix bug in finally cloning caused by unsound callfinally reordering
Port of #18348 to release/2.1 We need to make sure that if we reorder a callfinally during finally cloning that the callfinally is actually the one being targeted by the last block in the try range. Closes #18332. Linked issue has some more detailed notes.
Diffstat (limited to 'src')
-rw-r--r--src/jit/flowgraph.cpp49
1 files changed, 32 insertions, 17 deletions
diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp
index 68e67ca..395a1e0 100644
--- a/src/jit/flowgraph.cpp
+++ b/src/jit/flowgraph.cpp
@@ -24574,32 +24574,44 @@ void Compiler::fgCloneFinally()
// We better have found at least one call finally.
assert(firstCallFinallyBlock != nullptr);
- // If there is more than one callfinally, move the one we are
- // going to retarget to be first in the callfinally range.
+ // If there is more than one callfinally, we'd like to move
+ // the one we are going to retarget to be first in the callfinally,
+ // but only if it's targeted by the last block in the try range.
if (firstCallFinallyBlock != normalCallFinallyBlock)
{
- JITDUMP("Moving callfinally BB%02u to be first in line, before BB%02u\n", normalCallFinallyBlock->bbNum,
- firstCallFinallyBlock->bbNum);
-
- BasicBlock* const firstToMove = normalCallFinallyBlock;
- BasicBlock* const lastToMove = normalCallFinallyBlock->bbNext;
BasicBlock* const placeToMoveAfter = firstCallFinallyBlock->bbPrev;
- fgUnlinkRange(firstToMove, lastToMove);
- fgMoveBlocksAfter(firstToMove, lastToMove, placeToMoveAfter);
+ if ((placeToMoveAfter->bbJumpKind == BBJ_ALWAYS) &&
+ (placeToMoveAfter->bbJumpDest == normalCallFinallyBlock))
+ {
+ JITDUMP("Moving callfinally BB%02u to be first in line, before BB%02u\n",
+ normalCallFinallyBlock->bbNum, firstCallFinallyBlock->bbNum);
+
+ BasicBlock* const firstToMove = normalCallFinallyBlock;
+ BasicBlock* const lastToMove = normalCallFinallyBlock->bbNext;
+
+ fgUnlinkRange(firstToMove, lastToMove);
+ fgMoveBlocksAfter(firstToMove, lastToMove, placeToMoveAfter);
#ifdef DEBUG
- // Sanity checks
- fgDebugCheckBBlist(false, false);
- fgVerifyHandlerTab();
+ // Sanity checks
+ fgDebugCheckBBlist(false, false);
+ fgVerifyHandlerTab();
#endif // DEBUG
- assert(nextBlock == lastBlock->bbNext);
+ assert(nextBlock == lastBlock->bbNext);
- // Update where the callfinally range begins, since we might
- // have altered this with callfinally rearrangement, and/or
- // the range begin might have been pretty loose to begin with.
- firstCallFinallyRangeBlock = normalCallFinallyBlock;
+ // Update where the callfinally range begins, since we might
+ // have altered this with callfinally rearrangement, and/or
+ // the range begin might have been pretty loose to begin with.
+ firstCallFinallyRangeBlock = normalCallFinallyBlock;
+ }
+ else
+ {
+ JITDUMP("Can't move callfinally BB%02u to be first in line"
+ " -- last finally block BB%02u doesn't jump to it\n",
+ normalCallFinallyBlock->bbNum, placeToMoveAfter->bbNum);
+ }
}
}
@@ -24772,6 +24784,9 @@ void Compiler::fgCloneFinally()
{
// We can't retarget this call since it
// returns somewhere else.
+ JITDUMP("Can't retarget callfinally in BB%02u as it jumps to BB%02u, not BB%02u\n",
+ currentBlock->bbNum, postTryFinallyBlock->bbNum, normalCallFinallyReturn->bbNum);
+
retargetedAllCalls = false;
}
}