Skip to content

Commit c5bb505

Browse files
JIT: Add bounds check for ordinals in ThreeOptLayout::ConsiderEdge (#111719)
When setting up the data structures needed for 3-opt layout, we allocate an array of basic blocks that is large enough to fit everything reachable via a RPO traversal. However, we only initialize as much of the array as needed to fit the blocks hot enough to be considered for reordering, so we frequently have uninitialized space at the end of the array. Thus, our current strategy to check if a block is in the set of candidates takes two steps: Check if the block's ordinal is less than the number of hot blocks. If it isn't, then it cannot possibly be in the set. If the ordinal is valid, check if the block in the array matches the block in question. We were neglecting to do the first step in ThreeOptLayout::ConsiderEdge, which meant in rare cases, we would compare a block outside the candidate set to some uninitialized part of the array, and the comparison would happen to be equal. Thus, we would later consider creating fallthrough along a flow edge that breaks 3-opt's invariants. To solve this, we can either add the bounds check mentioned above, or we can default-initialize the entire block array to ensure comparisons outside the candidate set are with nullptr. I've decided to pursue the former. Fixes #111563.
1 parent e20ee22 commit c5bb505

File tree

1 file changed

+6
-1
lines changed

1 file changed

+6
-1
lines changed

src/coreclr/jit/fgopt.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5173,7 +5173,12 @@ void Compiler::ThreeOptLayout::ConsiderEdge(FlowEdge* edge)
51735173
assert(dstPos < compiler->m_dfsTree->GetPostOrderCount());
51745174

51755175
// Don't consider edges to or from outside the hot range (i.e. ordinal doesn't match 'blockOrder' position).
5176-
if ((srcBlk != blockOrder[srcPos]) || (dstBlk != blockOrder[dstPos]))
5176+
if ((srcPos >= numCandidateBlocks) || (srcBlk != blockOrder[srcPos]))
5177+
{
5178+
return;
5179+
}
5180+
5181+
if ((dstPos >= numCandidateBlocks) || (dstBlk != blockOrder[dstPos]))
51775182
{
51785183
return;
51795184
}

0 commit comments

Comments
 (0)