Skip to content
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

[mono] Fix crash when GetDelegateForFunctionPointer is passed non delegate type #79744

Merged
merged 1 commit into from
Jan 10, 2023

Conversation

giritrivedi
Copy link
Contributor

When GetDelegateForFunctionPointer is invoked with non-delegate type, it iterates over all the methods available to check if these methods are present. "Invoke" method is one such method which is executed by the coreCLR for any type of delegates. Having said that with non-delegate's "Invoke" method is not supported hence "mono_class_get_method_from_name_checked" returns NULL. Handle this case whenever mono_class_get_method_from_name_checked returns NULL for any methods which are not supported.

…egate type

When GetDelegateForFunctionPointer is invoked with non delegate type.
It iterates over all the methods available to check if these
methods are present. "Invoke" method is one such method which is
executed by the coreCLR for any type of delegates. Having said that
with non delegate "Invoke" method is not supported hence
"mono_class_get_method_from_name_checked" returns NULL.
Handle this case whenever mono_class_get_method_from_name_checked
returns NULL for any methods which are not supported.
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Dec 16, 2022
@vargaz
Copy link
Contributor

vargaz commented Dec 17, 2022

How can this happen ? Marshal.GetDelegateForFunctionPointer () checks that the type is a delegate type.

@giritrivedi
Copy link
Contributor Author

This is specifically crashed when multicastDelegate type is passed to Marshal.GetDelegateForFunctionPointer().

@giritrivedi
Copy link
Contributor Author

@lambdageek , @thaystg Can you check the changes and review please. ?

@marek-safar
Copy link
Contributor

Could you please add/enable a test for this change?

@vargaz
Copy link
Contributor

vargaz commented Dec 21, 2022

Apparently, passing MulticastDelegate is allowed according to this comment in Marshal.cs:

            // For backward compatibility, we allow lookup of existing delegate to
            // function pointer mappings using abstract MulticastDelegate type. We will check
            // for the non-abstract delegate type later if no existing mapping is found.

@giritrivedi
Copy link
Contributor Author

There is already an existing test case RunGetDelForFcnPtrTest available in file FunctionPointer.cs which tests this scenario as Marshal.GetDelegateForFunctionPointer(fcnptr, typeof(MulticastDelegate)).
This test was crashing on s390x architecture.

@giritrivedi
Copy link
Contributor Author

Hi,
Is there anything pending for this PR to be merged ?

@vargaz vargaz merged commit a10ee9e into dotnet:main Jan 10, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 9, 2023
@giritrivedi giritrivedi deleted the non_delegate_type_fix branch November 22, 2023 09:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM-meta-mono community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants