[wasm][coreclr] Fix DoPrestub for delegates#122596
[wasm][coreclr] Fix DoPrestub for delegates#122596radekdoulik wants to merge 2 commits intodotnet:mainfrom
Conversation
Return early for delegates to avoid setting the slot in `SetCodeEntryPoint` and to skip backpatch. The delegates are special case, where we have single implementation for all of them, and we don't want to break other parts, which depend on 1:1 mapping between implementation and method desc. This fixes dotnet#121955, which was still happening a lot in the runtime tests (cca 90 occurences in the out of process tests on wasm) We were getting the exception, because during the access checks we got private CreateDelegate method from the MT slot.
|
Tagging subscribers to this area: @mangod9 |
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #121955 by adding special handling for delegates in the DoPrestub function to preserve the 1:1 mapping between implementation and method descriptors. The fix prevents delegates from having their code entry point set in the vtable slot and from being backpatched, which was causing access check failures where private delegate methods were incorrectly retrieved from MT slots.
Key Changes
- Adds early return for delegates under
FEATURE_PORTABLE_ENTRYPOINTSto skipSetCodeEntryPointandDoBackpatchcalls - Targets edge cases where delegate method descriptors have native code already set (
pCode != NULL)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| #ifdef FEATURE_PORTABLE_ENTRYPOINTS | ||
| if (pMT->IsDelegate()) | ||
| { | ||
| return pCode; |
There was a problem hiding this comment.
If we early out here, does it mean that we will be going through PreStub every time the delegate constructor is called? We should not be going through PreStub in steady state as we have discussed...
There was a problem hiding this comment.
We are not going through PreStub again as the method already has interpreter code after 1st PreStub pass and so we don't call DoPreStub again. OTOH it will leave the method in different state. I will try to better understand the conditions under which we reach the issue and possibly find better solution.
|
I am unable to reproduce it anymore with main. I will rerun runtime tests to see if it still happens in them. @pavelsavara, do you still get |
Yes in any test, for example |
|
I would like to put workaround in place. Please remove it when this is resolved |
3 participants
Return early for delegates to avoid setting the slot in
SetCodeEntryPointand to skip backpatch. The delegates are special case, where we have single implementation for all of them, and we don't want to break other parts, which depend on 1:1 mapping between implementation and method desc.This fixes #121955, which was still happening a lot in the runtime tests (cca 90 occurences in the out of process tests on wasm)
We were getting the exception, because during the access checks we got private DelegateConstruct method from the MT slot.