[interpreter] Resolve open virtual delegates invoked through the shuffle thunk (wasm, Apple mobile) - #134699
[interpreter] Resolve open virtual delegates invoked through the shuffle thunk (wasm, Apple mobile)#134699lewing wants to merge 9 commits into
Conversation
…e shuffle thunk When compiled (R2R) code invokes an open virtual delegate on portable entry point platforms, it calls the IL delegate shuffle thunk, which does a calli on _methodPtrAux. For open virtual delegates that is CID_VirtualOpenDelegateDispatch, which is not a portable entry point, so treating it as one read a garbage MethodDesc and trapped in PrepareInterpreterCode. Resolve the virtual target in INTOP_CALLI the same way INTOP_CALLDELEGATE does. Re-enable System.Linq.Expressions.Tests on the browser CoreCLR R2R lane. Fixes #134261 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Extract ResolveOpenVirtualDelegateTarget for INTOP_CALLDELEGATE and INTOP_CALLI, use #else for the portable entry point branch, and add a DelegateTests case that invokes open virtual delegates (class override, boxed struct, enum, interface). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli |
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No blocking issues remain; only a documentation nit was noted.
Review effort: Lite
Findings: 1
What changed in this PR
Fixes browser-Wasm CoreCLR ReadyToRun crashes when invoking open virtual delegates.
Changes:
- Resolves open virtual delegate targets in interpreter
callihandling. - Shares delegate resolution logic and adds regression coverage.
- Re-enables LINQ Expressions tests for the affected lane.
| File | Summary |
|---|---|
src/libraries/tests.proj |
Re-enables the affected ReadyToRun test project. |
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/DelegateTests.cs |
Adds open virtual delegate dispatch tests. |
src/coreclr/vm/interpexec.cpp |
Resolves portable open virtual delegate targets; update the related preprocessor guard comments. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…Mono AOT Mono AOT on browser crashes invoking an open-instance interface delegate (#134707). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…-dispatch platforms CID_VirtualOpenDelegateDispatch needs the delegate (on arm64 it reads &_methodPtrAux from x11), so interpreted shuffle thunks cannot invoke it through calli. On maccatalyst/iOS/tvOS the interpreter called the native stub without x11 set and crashed in CID_VirtualOpenDelegateDispatchWorker (found by the new DelegateTests on maccatalyst-arm64 CoreCLR). Apply the INTOP_CALLI resolution under FEATURE_CACHED_INTERFACE_DISPATCH instead of only FEATURE_PORTABLE_ENTRYPOINTS. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to 'os-maccatalyst': @vitek-karas, @kotlarmilos, @steveisok, @akoeplinger |
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
|
The tests that were added showed that the problem was general to CID platforms so the PR has been updated to fix the apple mobile platforms as well. That means this is probably a candidate for a backport as well. |
…calli Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
LGTM otherwise |
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Invoking an open virtual delegate from compiled code crashes CoreCLR on platforms that use the interpreter with cached interface dispatch:
memory access out of boundsinPrepareInterpreterCode([browser][CoreCLR][R2R] System.Linq.Expressions tests trap in PrepareInterpreterCode #134261).CID_VirtualOpenDelegateDispatchWorker. In CI this showed up as a 75-minuteSystem.Runtime.Testshang onmaccatalyst-arm64 CoreCLR_Smoke, because a second fault while logging the crash spins inPAL_DispatchException([maccatalyst][CoreCLR] Fault while logging the fatal-error call stack spins in PAL_DispatchException instead of terminating #134720).Cause
For an open virtual delegate,
_methodPtris the delegate shuffle thunk and_methodPtrAuxisCID_VirtualOpenDelegateDispatch. The shuffle thunk is an IL stub that doescalli _methodPtrAux, and on these platforms it runs in the interpreter.CID_VirtualOpenDelegateDispatchcan't be invoked through a plaincalli, because it expects the address of_methodPtrAuxin a hidden argument thatcallicannot express:_methodPtrAuxinx11, which the interpreter'scallidoesn't set.PORTABILITY_ASSERTstub, not a portable entry point, soPortableEntryPoint::GetMethodDescreturns garbage.Interpreted callers don't hit this, because
INTOP_CALLDELEGATEalready recognizes the value and skips the shuffle thunk. Compiled (R2R) callers invoke_methodPtrdirectly and reach thecalli.Change
This is an interpreter-side workaround for #134733. The IL shuffle thunk's
callican't pass the hidden argument that open virtual delegate dispatch stubs expect, under both cached interface dispatch and virtual stub dispatch. The general fix belongs in the shuffle thunk and is tracked by that issue.INTOP_CALLI, underFEATURE_CACHED_INTERFACE_DISPATCH, recognizeCID_VirtualOpenDelegateDispatch. Resolve the virtual target from the delegate (the shuffle thunk'sthis) and the target'sthisargument, then continue throughCALL_INTERP_METHOD. That path falls back toInvokeManagedMethodwhen the target has compiled code.EmitCALLIinCreateILDelegateShuffleThunk.INTOP_CALLDELEGATEandINTOP_CALLIviaResolveOpenVirtualDelegateTarget.DelegateTestscoverage for invoking open virtual delegates: class override, boxed struct, enum and interface dispatch. The interface cases are disabled on browser Mono AOT, which has a separate crash tracked in [browser][Mono] Open-instance delegate to an interface method crashes in mono_class_interface_offset_with_variance #134707.System.Linq.Expressions.Testson the browser CoreCLR ReadyToRun lane.Validation (local)
PrepareInterpreterCodeDelegateTestsCID_VirtualOpenDelegateDispatchWorker)The browser Debug build compiles the new asserts.
Resolves #134261
Note
This PR description was generated with GitHub Copilot.