Skip to content

Fix async resumption stub with byref parameters - #129999

Merged
jakobbotsch merged 3 commits into
dotnet:mainfrom
jakobbotsch:fix-129990
Jun 30, 2026
Merged

Fix async resumption stub with byref parameters#129999
jakobbotsch merged 3 commits into
dotnet:mainfrom
jakobbotsch:fix-129990

Conversation

@jakobbotsch

@jakobbotschjakobbotsch commented Jun 29, 2026•
edited
Loading

Copy link
Copy Markdown
Member

It is not valid IL to initialize a byref local with initobj. Just load the value of these directly.

We had a test for byref parameters, but it did not suspend and did not result in creation of a resumption stub. The test case requires suspension + NoInlining to guarantee a suspension point is created.

In ASP.NET's case it is returning a Task<VerifyResult> from a Task returning function. This also requires a suspension point and resumption stub.

Fix#129990

@jakobbotsch
Fix async resumption stub with byref parameters
34cdc1d
It is not valid IL to initialize a byref local with `initobj`. Just load
the value of these directly.
We had a test for byref parameters, but it did not suspend and did not
result in creation of a resumption stub. The test case requires
suspension + NoInlining to guarantee a suspension point is created.
CopilotAI review requested due to automatic review settings June 29, 2026 20:18
@dotnet-policy-service

dotnet-policy-serviceBot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

CopilotAI reviewed Jun 29, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 IL generation in the async resumption stub path to avoid emitting invalid IL for byref parameters, and adds a regression test that reliably exercises the resumption-stub creation path for out parameters by forcing an actual suspension.

Changes:

  • Update CEEJitInfo::getAsyncResumptionStub to avoid initobj on byref locals by directly emitting a null managed pointer (ldc.0; conv.u) for byref arguments.
  • Strengthen the existing async/byref test by adding a suspension-producing scenario (and preventing inlining) so the runtime actually creates a resumption stub in the byref-parameter case.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

FileDescription
src/tests/async/byref-param/byref-param.csAdds a suspension-based byref/out test case to ensure a resumption stub is produced.
src/coreclr/vm/jitinterface.cppFixes resumption-stub IL emission for byref parameters to avoid invalid initobj usage.

Comment threadsrc/tests/async/byref-param/byref-param.cs Outdated
Comment threadsrc/coreclr/vm/jitinterface.cpp Outdated
CopilotAI review requested due to automatic review settings June 29, 2026 20:22
CopilotAI reviewed Jun 29, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment threadsrc/tests/async/byref-param/byref-param.cs
CopilotAI review requested due to automatic review settings June 29, 2026 20:28
CopilotAI reviewed Jun 29, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@jakobbotsch

jakobbotsch commented Jun 29, 2026

Copy link
Copy Markdown
MemberAuthor

PTAL @VSadov

@jakobbotsch
jakobbotsch requested a review from VSadovJune 29, 2026 20:40
@wtgodbe

wtgodbe commented Jun 29, 2026

Copy link
Copy Markdown
Member

We probably need this in p6 as well, aspnetcore sees the same test failures there

@wtgodbewtgodbe mentioned this pull request Jun 29, 2026
Closed
@JulieLeeMSFTJulieLeeMSFT mentioned this pull request Jun 29, 2026
Merged
4 tasks
VSadov
VSadov approved these changes Jun 29, 2026
View reviewed changes

@VSadovVSadov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@jakobbotsch

jakobbotsch commented Jun 29, 2026

Copy link
Copy Markdown
MemberAuthor

We probably need this in p6 as well, aspnetcore sees the same test failures there

We can backport this to preview6 too, but as a workaround you can change
https://github.com/dotnet/aspnetcore/blob/a6732b48f8e65c718cadbd2ee50d88222bcdf72e/src/Validation/test/Microsoft.Extensions.Validation.GeneratorTests/ValidationsGeneratorTestBase.cs#L40
to return Task<VerifyResult>. It should work around this issue.

@wtgodbe

wtgodbe commented Jun 29, 2026

Copy link
Copy Markdown
Member

Up to you on whether or not to backport, aspnetcore isn't actually blocked in p6, backflow there is purely for test verification

wtgodbe added a commit to dotnet/aspnetcore that referenced this pull request Jun 30, 2026
@wtgodbe
Work around preview6 runtime-async InvalidProgramException in Validat…
853e3e5
…ionsGenerator tests
The preview6 runtime miscompiles awaiting a synchronous Task-returning method
that has a byref (out) parameter, throwing InvalidProgramException at runtime
(dotnet/runtime#129999). Change ValidationsGeneratorTestBase.Verify to return
Task<VerifyResult> instead of Task, which avoids the non-generic Task await path
that triggers the bug.
This is a preview6-only workaround; main ingests the real runtime fix.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jakobbotsch
jakobbotsch merged commit db5dce5 into dotnet:mainJun 30, 2026
118 checks passed
@jakobbotsch
jakobbotsch deleted the fix-129990 branch June 30, 2026 07:28
@jakobbotsch

jakobbotsch commented Jun 30, 2026

Copy link
Copy Markdown
MemberAuthor

/backport to release/11.0-preview6

@github-actions

github-actionsBot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Started backporting to release/11.0-preview6 (link to workflow run)

@github-actionsgithub-actionsBot mentioned this pull request Jun 30, 2026
Merged
4 tasks
@dotnet-maestrodotnet-maestroBot mentioned this pull request Jun 30, 2026
Merged
@dotnet-milestone-botdotnet-milestone-botBot added this to the 11.0-preview7 milestone Jun 30, 2026
svick pushed a commit that referenced this pull request Jul 1, 2026
@github-actions@jakobbotsch
[release/11.0-preview6] Fix async resumption stub with byref paramete…
b492b5e
…rs (#130022)
Backport of #129999 to release/11.0-preview6
/cc @jakobbotsch
## Customer Impact
- [X] Customer reported
- [ ] Found internally
A Task-returning method `A` with a byref parameter may cause the runtime
to throw an `InvalidProgramException` if directly returning the result
of another Task-returning method `B()`, the task returned by `B`
suspends and also one of the following is true:
- `A` is marked with `[MethodImpl(MethodImplOptions.NoInlining)]`
- `B` returns `Task<T>` and `A` returns `Task`
In these cases the runtime produces an internal IL stub with invalid IL.
The invalid IL tries to zero a byref local using `initobj`, which is not
legal IL. Reported by ASP.NET in #129990.
## Regression
- [X] Yes
- [ ] No
This issue exists since runtime async was introduced, but #128384 made
it possible to hit this issue from C# code.
## Testing
Unit test introduced.
## Risk
Low. Adjust IL emission to load a `byref` zero without `initobj`.
---------
Co-authored-by: Jakob Botsch Nielsen <jakob.botsch.nielsen@gmail.com>
eiriktsarpalis pushed a commit that referenced this pull request Jul 15, 2026
@jakobbotsch@eiriktsarpalis
Fix async resumption stub with byref parameters (#129999)
49bf32d
It is not valid IL to initialize a byref local with `initobj`. Just load
the value of these directly.
We had a test for byref parameters, but it did not suspend and did not
result in creation of a resumption stub. The test case requires
suspension + NoInlining to guarantee a suspension point is created.
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jul 31, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@VSadovVSadovVSadov approved these changes
@wtgodbewtgodbewtgodbe approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

Assignees

@jakobbotschjakobbotsch

Labels

area-VM-coreclr

Projects

None yet

Milestone

11.0-preview7

Development

Successfully merging this pull request may close these issues.

Runtime-async miscompiles synchronous Task-returning methods into InvalidProgramException (regression from #128384, not fixed by #129884)

4 participants

@jakobbotsch@wtgodbe@VSadov