-
Notifications
You must be signed in to change notification settings - Fork 5.3k
JIT: SpanHelpers.Fill may escape its last argument #122902
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
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
@EgorBo PTAL We will want to backport this to .NET 10. |
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 critical bug in the JIT compiler's object stack allocation optimization where the SpanHelpers.Fill<T> intrinsic was incorrectly treating all its arguments as non-escaping, when in fact the last argument (the value of type T) can escape if T is a reference type. This caused stack-allocated objects passed as the value parameter to be incorrectly optimized, leading to heap corruption and NullReferenceExceptions.
Key changes:
- Updated the escape analysis in
objectalloc.cppto correctly identify that only the first argument (byref to span data) ofSpanHelpers.Filldoesn't escape - Added a regression test that reproduces the original issue
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/jit/objectalloc.cpp | Fixed escape analysis for SpanHelpers.Fill to only mark the first argument (byref span) as non-escaping, allowing the value parameter to escape |
| src/tests/JIT/opt/ObjectStackAllocation/Runtime_122879.cs | Added regression test that reproduces the bug by using Span.Fill with a reference type and stack trashing |
| src/tests/JIT/opt/ObjectStackAllocation/Runtime_122879.csproj | Test project configuration with optimization enabled |
Yeah, it's the fact that the filled value may be a GC ref that causes problems. |
|
/ba-g IOS dead letter |
|
/backport to release/10.0 |
|
Started backporting to |
…22919) Backport of #122902 to release/10.0 /cc @AndyAyersMS ## Customer Impact - [X] Customer reported - [ ] Found internally Customer reported in #122879 Silent bad code in some cases involving calls to `SpanHelpers.Fill` where the fill value is a GC ref coming from a locally allocated object. ## Regression - [ ] Yes - [x] No This is a bug in a new optimization in .NET 10. ## Testing Verified fix on customer reported case; also added this as regression test. Bug was an oversight; the first argument to `SpanHelpers.Fill` does not escape, but the other two might. The JIT was assuming none of the arguments escaped. ## Risk Low, we now disable stack allocation in this case. Co-authored-by: Andy Ayers <[email protected]>

Fixes #122879.