Skip to content

[6000.0] Fix Crash on Default Interface Methods Call #2148

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

Open
wants to merge 1 commit into
base: unity-6000.0-mbe
Choose a base branch
from

Conversation

scott-ferguson-unity
Copy link
Collaborator

This is a port of dotnet/runtime#115306

In case of a generic interface, build_imt_slots calculate the vt_slot to find the index in the vtable for the implementation. Currently implementation assumed interface methods are either static and/or virtual, but it's possible to have private/sealed methods that won't end up as virtual methods in vtable. This case was handled when building the vtable, but we didn't handle that scenario when building the IMT slots, ending up with wrong vtable slot, potentially outside of allocated memory.

In repro provided by dotnet/runtime#113958, the created vtable will have 5 slots, 4 from implementing Object and 1 from the implemented interface. When calculating the vt_slot in build_imt_slots we did however include the private DIM methods when calculating vtable slot, ending up with 4 + 3, slot 7, but vtable only allocated 5, so it will read outside allocated memory, and if it this turns out to be NULL, that will trigger assert, otherwise it will use random value, but since IMT slot will be patched with compiled method, the issue wouldn't manifest, at least not in the specific repro scenario.

Fix makes sure we only count virtual methods in build_imt_slots vt_slot index, similar to previous fixes for static and non-virtual methods in non-generic interfaces:

  • Should this pull request have release notes?
    • Yes
    • No
  • Do these changes need to be back ported?
    • Yes
    • No
  • Do these changes need to be upstreamed to mono/mono or dotnet/runtime repositories?
    • Yes
    • No

Reviewers: please consider these questions as well! ❤️

Release notes

Fixed UUM-99049 @scott-ferguson-unity
Mono: Fix crash on some default interface method calls

This is a port of dotnet/runtime#115306:

In case of a generic interface, build_imt_slots calculate the vt_slot to find the index in the vtable for the implementation. Currently implementation assumed interface methods are either static and/or virtual, but it's possible to have private/sealed methods that won't end up as virtual methods in vtable. This case was handled when building the vtable, but we didn't handle that scenario when building the IMT slots, ending up with wrong vtable slot, potentially outside of allocated memory.

In repro provided by dotnet/runtime#113958, the created vtable will have 5 slots, 4 from implementing Object and 1 from the implemented interface. When calculating the vt_slot in build_imt_slots we did however include the private DIM methods when calculating vtable slot, ending up with 4 + 3, slot 7, but vtable only allocated 5, so it will read outside allocated memory, and if it this turns out to be NULL, that will trigger assert, otherwise it will use random value, but since IMT slot will be patched with compiled method, the issue wouldn't manifest, at least not in the specific repro scenario.

Fix makes sure we only count virtual methods in build_imt_slots vt_slot index, similar to previous fixes for static and non-virtual methods in non-generic interfaces:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant