-
Notifications
You must be signed in to change notification settings - Fork 5.3k
JIT: Fix another issue when optimizing out async calls late #121550
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
Conversation
`genAsyncResumeInfoTable` may be null if we optimize out all async calls, so handle this case too.
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.
Pull Request Overview
This PR fixes a potential null reference crash when async calls are optimized out late in the JIT compilation process. The issue occurs when compSuspensionPoints exists (created during early async transformation) but genAsyncResumeInfoTable remains null because all GT_RECORD_ASYNC_RESUME nodes were removed by late optimizations.
- Removes an assertion that incorrectly assumed
genAsyncResumeInfoTablewould always be non-null - Adds null check before accessing
genAsyncResumeInfoTableto prevent crashes - Sets diagnostic native offsets to 0 when async resume info table doesn't exist
|
PTAL @dotnet/jit-contrib |
|
@jakobbotsch what are the typical symptoms of this bug? Is it a JIT-time crash or run time NRE? |
In checked builds you would see assert, and in release builds like JIT-time crashes. I found this issue via Fuzzlyn. It's probably unlikely any of our tests are hitting it. |
|
I am looking at something that may be a Linux-specific GC hole. At least it has symptoms of a GC hole (a variable becomes Sounds like it cannot be possibly related to this. |
|
/ba-g Timeouts |
genAsyncResumeInfoTablemay be null if we optimize out all async calls, so handle this case too.