Skip to content

Fix interpreter async suspend DiagnosticIP calculation#125282

Merged
davidwrighton merged 6 commits intodotnet:mainfrom
davidwrighton:add_interpreter_async_test
Mar 7, 2026
Merged

Fix interpreter async suspend DiagnosticIP calculation#125282
davidwrighton merged 6 commits intodotnet:mainfrom
davidwrighton:add_interpreter_async_test

Conversation

@davidwrighton
Copy link
Member

@davidwrighton davidwrighton commented Mar 6, 2026

Fix #124044

The interpreter was storing an absolute address for DiagnosticIP in INTOP_HANDLE_CONTINUATION_SUSPEND, but this field needs to be stored as an offset and then relocated during FinalizeMethodData (similar to other fields like methodStartIP).

This caused a crash in ToString_Async and other async exception handling tests on Apple mobile with CoreCLR R2R and interpreter fallback, because the diagnostic IP pointed to invalid memory.

Changes:

  • compiler.cpp: Store DiagnosticIP as an offset from method code start during emit, then add the final bytecode base address during finalization.
  • simple-eh.cs: Remove ActiveIssue attributes for the three tests that were disabled due to this bug.

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes interpreter async suspension diagnostics by changing how CORINFO_AsyncResumeInfo.DiagnosticIP is recorded/relocated, and re-enables/adds tests that cover async throw-after-yield scenarios.

Changes:

  • Interpreter compiler: store DiagnosticIP as an offset during emission and relocate it during method finalization.
  • Async simple EH tests: re-enable two tests previously disabled for the interpreter.
  • Interpreter test suite: add coverage for throw-after-yield async exception handling.

Reviewed changes

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

File Description
src/coreclr/interpreter/compiler.cpp Adjusts how async suspend DiagnosticIP is captured and relocated during finalization.
src/tests/async/simple-eh/simple-eh.cs Removes ActiveIssue gating so two async EH tests run again under interpreter.
src/tests/JIT/interpreter/Interpreter.cs Adds an async throw-after-yield scenario to the interpreter test driver.

Copilot AI review requested due to automatic review settings March 6, 2026 22:40
Copy link
Contributor

Copilot AI left a comment

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 2 out of 2 changed files in this pull request and generated 1 comment.

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash in ToString_Async test on Apple mobile with CoreCLR R2R and interpreter

3 participants