[wasm][coreCLR] Dispatch R2R-compiled methods from reflection and UnmanagedCallersOnly paths#129766
Open
pavelsavara wants to merge 13 commits into
Open
[wasm][coreCLR] Dispatch R2R-compiled methods from reflection and UnmanagedCallersOnly paths#129766pavelsavara wants to merge 13 commits into
pavelsavara wants to merge 13 commits into
Conversation
…thods CallDescrWorkerInternal (the wasm reflection/CallDescr invoke path) resolved the interpreter byte code via MethodDesc::GetInterpreterCode() and, after a DoPrestub retry, passed the pointer straight to ExecuteInterpretedMethodWithArgs. When the target method has no interpreter byte code because it was compiled to native (R2R) code, GetInterpreterCode() returns NULL. On wasm a NULL pointer reads zeroed low linear memory instead of faulting, so the interpreter dispatched the NULL byte code as INTOP_INVALID and aborted with Unimplemented or invalid interpreter opcode plus a secondary stackwalk assert. Add the InvokeManagedMethod fallback (the interpreter->R2R thunk) when there is no interpreter code, mirroring the existing pattern in ExecuteInterpretedMethodWithArgs_PortableEntryPoint_Complex and the CALL_INTERP_METHOD path in InterpExecMethod. Also harden the sibling entry points: ExecuteInterpretedMethodFromUnmanaged now takes the same fallback, and ExecuteInterpretedMethodWithArgs asserts a non-NULL targetIp so future misuse fails loudly at the point of error.
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg |
jkotas
reviewed
Jun 23, 2026
Address PR dotnet#129766 review feedback: resolve R2R-compiled native code for UnmanagedCallersOnly methods directly instead of going through the interpreter reverse-thunk fallback. - GetUnmanagedCallersOnlyThunk (wasm/helpers.cpp): run DoPrestub under GCX_PREEMP to publish code, then return the native (R2R) entrypoint via PortableEntryPoint when present; the g_ReverseThunks fallback is only for interpreted methods. - ExecuteInterpretedMethodFromUnmanaged (prestub.cpp): R2R methods are now dispatched directly and never reach this path, so replace the InvokeManagedMethod fallback with an assert that the interpreter byte code exists. - Widen contracts for the prestub-on-resolve path: EnsureCodeForUnmanagedCallersOnly (precode_portable.cpp) is THROWS/GC_TRIGGERS/MODE_PREEMPTIVE; GetSingleCallableAddrOfCodeForUnmanagedCallersOnly (method.cpp) is GC_TRIGGERS under FEATURE_PORTABLE_ENTRYPOINTS.
pavelsavara
added a commit
to pavelsavara/runtime
that referenced
this pull request
Jun 24, 2026
Address PR dotnet#129766 review feedback: resolve R2R-compiled native code for UnmanagedCallersOnly methods directly instead of going through the interpreter reverse-thunk fallback. - GetUnmanagedCallersOnlyThunk (wasm/helpers.cpp): run DoPrestub under GCX_PREEMP to publish code, then return the native (R2R) entrypoint via PortableEntryPoint when present; the g_ReverseThunks fallback is only for interpreted methods. - ExecuteInterpretedMethodFromUnmanaged (prestub.cpp): R2R methods are now dispatched directly and never reach this path, so replace the InvokeManagedMethod fallback with an assert that the interpreter byte code exists. - Widen contracts for the prestub-on-resolve path: EnsureCodeForUnmanagedCallersOnly (precode_portable.cpp) is THROWS/GC_TRIGGERS/MODE_PREEMPTIVE; GetSingleCallableAddrOfCodeForUnmanagedCallersOnly (method.cpp) is GC_TRIGGERS under FEATURE_PORTABLE_ENTRYPOINTS.
Member
|
UCO -> R2R seems to be needed to run against an R2R'd SPC, something in the shutdown path calls back into the runtime this way. |
jkotas
reviewed
Jun 25, 2026
jkotas
reviewed
Jun 25, 2026
jkotas
reviewed
Jun 25, 2026
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
jkotas
reviewed
Jun 25, 2026
3 tasks
Member
Author
|
I'm OOO till Wed, whoever is blocked by this, feel free to take over. |
jkotas
reviewed
Jun 26, 2026
jkotas
reviewed
Jun 26, 2026
Comment on lines
+41
to
+51
| if (targetIp == NULL) | ||
| { | ||
| // The target method has no interpreter code because it was compiled to native (R2R) code. | ||
| // Invoke it as a compiled managed method through the interpreter->R2R thunk, mirroring the | ||
| // fallback already present in ExecuteInterpretedMethodWithArgs_PortableEntryPoint_Complex and | ||
| // the CALL_INTERP_METHOD path in InterpExecMethod. Without this, the NULL bytecode pointer | ||
| // would be handed to the interpreter and dispatched as INTOP_INVALID. | ||
| ManagedMethodParam param = { pMethod, args, (int8_t*)retBuff, (PCODE)NULL, nullptr }; | ||
| InvokeManagedMethod(¶m); | ||
| return; | ||
| } |
Member
There was a problem hiding this comment.
Yes, the whole CallDescrWorker is not setup correctly wrt GC on wasm. Our plan is to delete it.
jkotas
reviewed
Jun 26, 2026
Comment on lines
+41
to
+50
| if (targetIp == NULL) | ||
| { | ||
| // The target method has no interpreter code because it was compiled to native (R2R) code. | ||
| // Invoke it as a compiled managed method through the interpreter->R2R thunk, mirroring the | ||
| // fallback already present in ExecuteInterpretedMethodWithArgs_PortableEntryPoint_Complex and | ||
| // the CALL_INTERP_METHOD path in InterpExecMethod. Without this, the NULL bytecode pointer | ||
| // would be handed to the interpreter and dispatched as INTOP_INVALID. | ||
| ManagedMethodParam param = { pMethod, args, (int8_t*)retBuff, (PCODE)NULL, nullptr }; | ||
| InvokeManagedMethod(¶m); | ||
| return; |
Comment on lines
+48
to
+50
| ManagedMethodParam param = { pMethod, args, (int8_t*)retBuff, (PCODE)NULL, nullptr }; | ||
| InvokeManagedMethod(¶m); | ||
| return; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On wasm (CoreCLR interpreter,
FEATURE_PORTABLE_ENTRYPOINTS), invoking a method that was compiled to native R2R code crashed at xUnit discovery with:followed by a secondary
m_crawl.GetCodeInfo()->IsValid()stackwalk assert.Root cause
The wasm invoke paths resolve a method's interpreter byte code via
MethodDesc::GetInterpreterCode()and pass the pointer to the interpreter. When the target method has no interpreter byte code because it was compiled to native (R2R) code,GetInterpreterCode()returnsNULL. On wasm aNULLpointer reads zeroed low linear memory instead of faulting, so the interpreter setstartIp = NULL, fetchedip[0] == 0, and dispatched it asINTOP_INVALID— aborting the process.This only reproduced with R2R builds (native methods have no interpreter byte code). Pure-IL builds give every method interpreter byte code, so the byte code pointer was never
NULL.Two distinct entry paths reach a native (R2R) method without interpreter byte code:
CallDescr(e.g. xUnit discovery and invoke) →CallDescrWorkerInternal.UnmanagedCallersOnlyreverse calls from native code →GetUnmanagedCallersOnlyThunk/ExecuteInterpretedMethodFromUnmanaged.Fix
Reflection /
CallDescrpath (calldescrworkerwasm.cpp): when the resolved interpreter byte code isNULL, fall back toInvokeManagedMethod(the interpreter → R2R thunk), mirroring the established pattern inExecuteInterpretedMethodWithArgs_PortableEntryPoint_Complexand theCALL_INTERP_METHODpath inInterpExecMethod.UnmanagedCallersOnlypath (wasm/helpers.cpp): resolve the method's code first and, if it has native (R2R) code, return that R2R entrypoint directly. AnUnmanagedCallersOnlymethod compiled by crossgen2 is emitted with the native calling convention, so its R2R code is itself the directly-callable unmanaged entrypoint. Theg_ReverseThunksinterpreter fallback is only needed for interpreted methods, so R2R methods are now dispatched directly and never go through the interpreter reverse thunk.Because the
UnmanagedCallersOnlypath is fixed at the source, the downstream interpreter helpers now assert instead of silently dispatching aNULL:ExecuteInterpretedMethodFromUnmanagedasserts a non-NULLbyte code pointer (R2R methods no longer reach it).ExecuteInterpretedMethodWithArgsasserts a non-NULLtargetIp, so any future misuse fails loudly at the actual point of error instead of as a crypticINTOP_INVALID.Contract widening: resolving an
UnmanagedCallersOnlyentrypoint may now run the prestub to publish R2R native code, which can throw and trigger a GC. Contracts are widened accordingly:MethodDesc::GetSingleCallableAddrOfCodeForUnmanagedCallersOnlyisGC_TRIGGERSunderFEATURE_PORTABLE_ENTRYPOINTS.PortableEntryPoint::EnsureCodeForUnmanagedCallersOnlyisTHROWS; GC_TRIGGERS; MODE_PREEMPTIVE.Validation
System.Drawing.Primitivestests built with R2R, run under V8:Identical to the IL baseline.
Note
This pull request was authored with the assistance of GitHub Copilot.