JIT: Check for inline candidates in gtTreeContainsAsyncCall#124502
JIT: Check for inline candidates in gtTreeContainsAsyncCall#124502jakobbotsch merged 1 commit intodotnet:mainfrom
Conversation
Inline candidates can later be substituted for the original call. That call may contain async calls, in which case the standard spilling behavior should kick.
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
This PR fixes a JIT code generation bug where async calls within inline candidates were not properly detected, causing incorrect register/memory handling that led to null reference crashes in Native AOT scenarios with runtime async enabled.
Changes:
- Enhanced
gtTreeContainsAsyncCallto recursively check inline candidates withinGT_RET_EXPRnodes for async calls - Updated lambda capture from
[]to[=]to enable the recursive member function call
|
cc @dotnet/jit-contrib PTAL @EgorBo |
|
Do we understand why only native AOT was hitting this? Is this not hittable with the VM-based runtime? (Asking because I see there's no targeted regression test) |
I would expect regular coreclr to be able to hit this too, but we aren't building the libraries (like Microsoft.Extensions.Caching.Memory) with runtime async in the regular coreclr yet. Plus, we only very recently enabled the inline candidates that are required for this to reproduce in #124183 -- before that PR I wouldn't expect this to repro. Furthermore to hit this problem requires boxing the result of a late rejected async inline candidate which is probably a rare situation.
So far I haven't been adding regression tests for async specific issues that we are finding in our existing testing. |
|
Although it seems like we would expect to hit the issue in #124527. There is probably some element of randomness in the test itself around where the actual suspensions happen (that would explain why it didn't reproduce consistently for us). |
Yep, that's where I would expect to see this too and why I asked! |
|
I took a closer look and:
|
|
I see, thank you for checking! I assume we're going to get coverage for these (tiering disabled, etc.) with CoreCLR in our various outerloops once runtime async is enabled by default. |
Yes, I would expect so. If anything libraries-jitstress runs all the libraries tests in a number of different configurations, including with and without tiered compilation. |
Inline candidates can later be substituted for the original call. That call may contain async calls, in which case the same spilling behavior should kick in.
Fix #124496