Skip to content

Support using function pointers with PersistedAssemblyBuilder #115015

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

Closed
Sergio0694 opened this issue Apr 24, 2025 · 9 comments
Closed

Support using function pointers with PersistedAssemblyBuilder #115015

Sergio0694 opened this issue Apr 24, 2025 · 9 comments
Labels
area-System.Reflection needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration partner-impact This issue impacts a partner who needs to be kept updated

Comments

@Sergio0694
Copy link
Contributor

Note

Follow up from #106216

Not exactly sure how to structure this issue, as it's technically some kind of API proposal, but I also don't have an exact API shape yet. The general issue is that it doesn't seem to be possible to use function pointers when using PersistedAssemblyBuilder to create a .dll (though this seems to be a more general issue with reflection in general too). Suppose you want to define a type like this:

struct Foo
{
    delegate* unmanaged[MemberFunction]<void**, void*, int> Bar;
}

There is no way to get or create a Type instance that you can pass to TypeBuilder.DefineField to define that field.

Presumably we'd need some kind of API on ModuleBuilder (?) to synthesize a Type for a given function pointer.

This is impacting CsWinRT 3.0 in the sense that we realized we can't generate all the interop code we need in the sidecar .dll via PersistedAssemblyBuilder, because of this limitation (we can't emit the function pointer types, nor the calli invocations for them). We're thinking we could take a dependency on Roslyn and generate a separate .dll with just the vtable types, from C# sources, and reference that from the real sidecar .dll, built with PersistedAssemblyBuilder. It seems that if you do this, you can actually use function pointer types, as long as you "get them from somewhere", rather than synthesizing them yourself. Of course, this adds complexity and we'd rather just use PersistedAssemblyBuilder for everything, and just end up with a single sidecar .dll, instead of two.

This seems related to #111003, so possibly PersistedAssemblyBuilder also has other bugs that would still make it break with function pointers?

cc. @jkotas do you have any thoughts on this? From our side in CsWinRT, some not necessarily complete feature would also be fine, as long as we wouldn't be blocked from shipping (perhaps just fixing #111003 plus us using the two .dll-s would be enough for now?). I can also help put together some minimal examples of the kind of code we'd need to generate, so we could validate that things work enough for us in the meantime.

@Sergio0694 Sergio0694 added area-System.Reflection partner-impact This issue impacts a partner who needs to be kept updated labels Apr 24, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Apr 24, 2025
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

@Sergio0694
Copy link
Contributor Author

Mh, actually I just saw #75347 and #75348, currently in the .NET 10 milestone, so it seems a whole lot of work around function pointers is already planned and being done? Would those two also cover this scenario? Not sure I'm seeing an API to create a new function pointer type you can then use to define a field in a new type. Also, are those items likely to make .NET 10? 🤔

@jkotas
Copy link
Member

jkotas commented Apr 25, 2025

@jkotas do you have any thoughts on this?

Function pointers support in Reflection.Emit is not implemented as you can see from the issues that you have linked.

currently in the .NET 10 milestone, so it seems a whole lot of work around function pointers is already planned

Milestone field set to .NET 10 does not mean that the work is planned/committed for .NET 10.

@Sergio0694
Copy link
Contributor Author

I see. Looking at that issue, it seems that emitting calli instructions does work correctly on .NET Framework, and it's only broken on .NET 9. That makes me wonder whether to avoid getting blocked, we could just make our build task use .NET Framework instead of .NET 9? My understanding is that the framework in use by the tool doesn't matter anyway, so there should be no difference? 🤔

@jkotas
Copy link
Member

jkotas commented Apr 25, 2025

Reflection.Emit in .NET Framework is coupled with .NET Framework mscorlib that the process is running on. You would have hard time emitting references to things that do not exist in .NET Framework mscorlib.

@Sergio0694
Copy link
Contributor Author

Alright, let's scratch that then. I'm out of ideas 😆

@steveharter
Copy link
Contributor

Currently we have not added support for obtaining or declaring a new function pointer-based field, property or parameter.

However, emit does support calli against a native int \ intptr value. So a workaround is to use intptr-based fields, etc.

having a "partner-impact" status does increase the priority, so we'll see if we can put a plan together v10. cc @ericstj.

@steveharter steveharter added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Apr 25, 2025
@Sergio0694
Copy link
Contributor Author

Sergio0694 commented Apr 25, 2025

"So a workaround is to use intptr-based fields, etc."

I considered that, however we can't do that as it'd break ILC folding for vtables.

"having a "partner-impact" status does increase the priority"

Yup, to clarify I only added that tag for context, not to necessarily affect priority.

"so we'll see if we can put a plan together v10"

I think we're good, no need to prioritize this. We ended up going with https://github.com/Washi1337/AsmResolver, which seems extremely flexible and covers everything we need for both reading metadata and emitting the final assembly. We've already had a bunch of asks for the runtime team for things we'd like to get for CsWinRT 3.0 (see #114179), so I want to be respectful of everyone's time and make sure to only ask to prioritize things that are either actually blocking us, or that would be problematic for us if they missed the .NET 10 release. This is not one of them, now that we're going with another library to handle our IL analysis and generation. So just to clarify, feel free to not consider us (CsWinRT) with respect to planning for any of the new reflection work 🙂

(I also did not add this issue to that epic, to make sure it was clear it doesn't have to be considered in that same bucket)

Thanks again to both of you!

@steveharter
Copy link
Contributor

I think we're good, no need to prioritize this. We ended up going with https://github.com/Washi1337/AsmResolver, which seems extremely flexible and covers everything we need for both reading metadata and emitting the final assembly.

Thanks for the update. I think it makes sense to mark this as a duplicate of #111003 which we hope to get done for v10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Reflection needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration partner-impact This issue impacts a partner who needs to be kept updated
Projects
None yet
Development

No branches or pull requests

3 participants