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

Type.GetInterfaceMap doesn't work for all cases #89157

Open
MichalStrehovsky opened this issue Mar 26, 2021 · 5 comments
Open

Type.GetInterfaceMap doesn't work for all cases #89157

MichalStrehovsky opened this issue Mar 26, 2021 · 5 comments

Comments

@MichalStrehovsky
Copy link
Member

It only works for the most straightforward cases. See failing System.Runtime tests blocked on this issue.

@Suchiman
Copy link
Contributor

I've looked into this and discovered two issues:

  1. For generic methods with unbound generic parameters it panics with (System.Reflection.MissingRuntimeArtifactException): This object cannot be invoked because no code was generated for it: 'InterfaceMapTest.SimpleType.GenericMethod()' here, also discovered Reflection Invoke does not check ContainsGenericParameters #95245 while looking into this. One workaround that could get us closer but still not all the way would be calling MakeGenericMethod(typeof(object)) on them if they don't have incompatible generic constraints before calling GetMethodInvoker and then using GetGenericMethodDefinition on the result.

  2. The MethodBase returned from RuntimeAugments.Callbacks.GetMethodBaseFromStartAddressIfAvailable(classRtMethodHandle) is Void Method(T) instead of Void Method(System.Object) when T is object, if T is string then it correctly returns Void Method(System.String) though. I'd suspect it's some issue around object ~ System.__Canon in GetMethodBaseFromStartAddressIfAvailable? If GetMethodBaseFromStartAddressIfAvailable cannot be fixed one could use MakeGenericMethod(typeof(object)) for this special case.

@MichalStrehovsky
Copy link
Member Author

For 2, I'm surprised it doesn't just crash - we're asking for a method invoker of an uninstantiated method, basically (the interface methods returned from GetMethods are uninstantiated). I don't know if there's a easy way to solve this. We could do MakeGeneric, but that might end up throwing if the code doesn't exist.

@Suchiman
Copy link
Contributor

Suchiman commented Mar 28, 2021

we're asking for a method invoker of an uninstantiated method

That's what's happening in 1), in 2) we're having actually instantiated methods because they're being retrieved from

interface IGenericInterface<T>
{
    void Method(T arg);
}

var ifaceMethod = typeof(IGenericInterface<object>).GetMethod("Method"); // Void Method(System.Object)
var invoker = (VirtualMethodInvoker)GetMethodInvoker(ifaceMethod);
IntPtr classRtMethodHandle = invoker.ResolveTarget(instanceType.TypeHandle);
MethodBase methodBase = RuntimeAugments.Callbacks.GetMethodBaseFromStartAddressIfAvailable(classRtMethodHandle); // Void Method(T)

@MichalStrehovsky
Copy link
Member Author

MethodBase.GetMethodFromHandle has two overloads to combat the issue with generic sharing - maybe there's something in the implementation of it that we could leverage (I haven't actually looked though).

@MichalStrehovsky
Copy link
Member Author

MichalStrehovsky commented Jun 15, 2023

If/when this is fully implemented, we also need to make sure reflection-visibility of the interface method also ensures reflection-visibility of the implementation method. There won't be anything in the compiler to ensure the target methods are reflection visible. Note that doing this for everything is a size regression so we should make sure compiler only makes targets reflection-visible if GetInterfaceMap is actually called. Nobody actually calls this API.

@MichalStrehovsky MichalStrehovsky transferred this issue from dotnet/runtimelab Jul 19, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 19, 2023
@MichalStrehovsky MichalStrehovsky added this to the Future milestone Jul 19, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

2 participants