Skip to content
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

JIT: Add bounds check for ordinals in ThreeOptLayout::ConsiderEdge #111719

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

amanasifkhalid
Copy link
Member

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.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jan 22, 2025
@amanasifkhalid
Copy link
Member Author

cc @dotnet/jit-contrib. @jakobbotsch thanks for tracking this down; could you PTAL?

@amanasifkhalid
Copy link
Member Author

No diffs.

@amanasifkhalid
Copy link
Member Author

/ba-g blocked by build timeouts

@amanasifkhalid amanasifkhalid merged commit c5bb505 into dotnet:main Jan 23, 2025
103 of 109 checks passed
@amanasifkhalid amanasifkhalid deleted the 3-opt-ordinal-check branch January 23, 2025 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Segfault in Compiler::fgGetPredForBlock on MacOS x64
2 participants