-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[wasm][coreclr] Fix DoPrestub for delegates #122596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[wasm][coreclr] Fix DoPrestub for delegates #122596
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.
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 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 <[email protected]>
| #ifdef FEATURE_PORTABLE_ENTRYPOINTS | ||
| if (pMT->IsDelegate()) | ||
| { | ||
| return pCode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.