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

Activator.CreateInstance supports ByRefLike types #102636

Merged

Conversation

AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented May 23, 2024

This represents an update of Activator.CreateInstance to naturally support byreflike generics.
It is modeled off of the current native AOT implementation.

This now works for all runtimes - CoreCLR, mono and native AOT. Native AOT was a no-op as its implementation was ByRefLike ready.

Contributes to #65112

This represents an update of Activator.CreateInstance
to naturally support byreflike generics. It is modeled
off of the current native AOT implementation.
@AaronRobinsonMSFT AaronRobinsonMSFT marked this pull request as ready for review May 29, 2024 18:26
@AaronRobinsonMSFT
Copy link
Member Author

AaronRobinsonMSFT commented May 29, 2024

@AaronRobinsonMSFT AaronRobinsonMSFT changed the title Activator.CreateInstance similar to NAOT Activator.CreateInstance supports byreflike types May 29, 2024
@AaronRobinsonMSFT AaronRobinsonMSFT changed the title Activator.CreateInstance supports byreflike types Activator.CreateInstance supports ByRefLike types May 29, 2024
Wrap invocation in TargetInvocationException.
@lambdageek
Copy link
Member

Mono crashes look related. I felt pretty certain just compiling the ctor would return something with the correct calling convention, but perhaps I need to verify that

@lambdageek
Copy link
Member

@AaronRobinsonMSFT please cherrypick lambdageek@c4067c4

The issue was that when using the interpreter ldftn/calli operate on a MonoFtnDesc* not a native code pointer (which is what mono_compile_method_checked returns). RuntimeMethodHandle.GetFunctionPointer is an interpreter intrinsic that gets replaced by an ldftn.


there's a second crash in System.Runtime.Tests on Build linux-x64 Debug Mono_MiniJIT_LibrariesTests that looks like a crash while creating a stack trace after an xunit test failure. My patch apparently fixes that, too, although I'm unable to explain why - my best guess is that in some generic sharing situation we weren't passing the generic sharing context correctly (get_ftnptr_for_method in mini-runtime.c does a bit more work than just mono_compile_method_checked)

On the Mono interpreter, the ldftn opcode for a managed method returns a MonoFtnDesc*, not
a native code pointer, and the calli opcode expects a MonoFtnDesc*.

The MethodHandle.GetFunctionPointer() function is an interpreter
intrinsic that returns such a MonoFtnDesc*
@AaronRobinsonMSFT
Copy link
Member Author

@AaronRobinsonMSFT please cherrypick lambdageek@c4067c4

Thanks @lambdageek. I've applied the changes. Let's see if the CI is happy now.

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 26a1e72 into dotnet:main May 31, 2024
149 checks passed
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the createinstance_update branch May 31, 2024 16:07
@github-actions github-actions bot locked and limited conversation to collaborators Jul 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants