Skip to content

Conversation

jgh07
Copy link

@jgh07 jgh07 commented Sep 21, 2025

This fixes #111003. The current implementation is a work in progress, only managed function pointers with no modopts are supported.

I imagine adding support for unmanaged calling conventions and custom modifiers will be a bit more challenging, since I cannot use Type.GetFunctionPointerCallingConventions et al.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Sep 21, 2025
Copy link
Contributor

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

@jgh07
Copy link
Author

jgh07 commented Sep 21, 2025

@jgh07 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@dotnet-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree

@jgh07
Copy link
Author

jgh07 commented Oct 13, 2025

I've now added support for unmanaged calling conventions and custom modifiers, altough there are still some limitations which I outlined in the original issue (#111003).

Fully supporting all possible variants of function pointer types will require updates of the reflection API, as far as I'm concerned. Still, my implementation should support everything that C# currently supports, which is why I am opening the PR for review.

I understand if this PR cannot be merged until there is comprehensive support for the feature, altough in my opinion it is already 90% of the way there in terms of support for real-life scenarios.

@jgh07 jgh07 marked this pull request as ready for review October 13, 2025 16:56
@Copilot Copilot AI review requested due to automatic review settings October 13, 2025 16:56
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for function pointer types in System.Reflection.Emit by implementing signature generation for function pointers. The implementation currently supports managed function pointers without custom modifiers (modopt), with unmanaged calling conventions and custom modifiers planned for future work.

Key changes:

  • Added function pointer type detection and handling in signature generation
  • Implemented calling convention mapping for function pointers
  • Added support for custom modifiers on function pointer return types and parameters

@jgh07 jgh07 requested a review from Copilot October 13, 2025 17:00
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

@jkotas
Copy link
Member

jkotas commented Oct 13, 2025

Could you please add tests as well?

@jgh07
Copy link
Author

jgh07 commented Oct 13, 2025

Could you please add tests as well?

Done.

@jgh07 jgh07 requested a review from jkotas October 16, 2025 16:35
@jkotas
Copy link
Member

jkotas commented Oct 17, 2025

The new test is failing:

   System.Reflection.Emit.Tests.AssemblySaveTypeBuilderTests.ConsumeFunctionPointerFields [FAIL]
      System.IO.FileNotFoundException : Could not load file or assembly 'Assembly1, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.
      
      Stack Trace:
           at Program.Main()
           at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)
           at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)

@jkotas
Copy link
Member

jkotas commented Oct 18, 2025

I have pushed commit with a few more test cases for field references. Some of the do not work. Could you please take a look?

@jgh07
Copy link
Author

jgh07 commented Oct 18, 2025

My implementation should be correct now. The C# field references still fail when they contain unmanaged calling conventions or other custom modifiers, but as far as I can tell that's an issue with GetMemberReferenceHandle which is called by ILGenerator.Emit.

Here is the relevant code section (starting at line 737 in ModuleBuilderImpl.cs):

case FieldInfo field:
    Type declaringType = field.DeclaringType!;
    if (declaringType.IsGenericTypeDefinition)
    {
        //The type of the field has to be fully instantiated type.
        declaringType = declaringType.MakeGenericType(declaringType.GetGenericArguments());
    }

    Type fieldType = ((FieldInfo)GetOriginalMemberIfConstructedType(field)).FieldType; // <- This loses all calling conventions and modifiers
    memberHandle = AddMemberReference(field.Name, GetTypeHandle(declaringType),
        MetadataSignatureHelper.GetFieldSignature(fieldType, field.GetRequiredCustomModifiers(), field.GetOptionalCustomModifiers(), this));

    break;

Using FieldInfo.FieldType strips away all modifiers, causing wrong signatures to be emitted which cannot be matched with any fields. I tried replacing it with FieldInfo.GetModifiedFieldType() and I actually managed to get this specific test to work, but this causes a lot of other issues since there are some methods that are not supported on modified types.

This is also an important issue that should be fixed, but it goes beyond the scope of this PR, in my opinion.

@jkotas
Copy link
Member

jkotas commented Oct 19, 2025

This is also an important issue that should be fixed, but it goes beyond the scope of this PR, in my opinion.

Sounds good. We can use the existing issue to track this or create a new one - either way is fine with me.

@jgh07
Copy link
Author

jgh07 commented Oct 19, 2025

Sounds good. We can use the existing issue to track this or create a new one - either way is fine with me.

In my opinion it should be a separate issue. I don't know if this is at all feasible, but I think it would be great if function pointers were treated specially in the sense that they would always preserve calling conventions and modifiers in their Type representation, even without explicit field references.

il.Emit(OpCodes.Ldsfld, typeof(ClassWithFunctionPointerFields).GetField("field1"));
il.Emit(OpCodes.Pop);
// References to fields with unmanaged calling convention are broken
// [ActiveIssue("https://github.com/dotnet/runtime/issues/111003")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you would like this to be tracked by a new issue, could you please create one and point this comment to it instead of the existing one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Reflection.Emit 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.

ILGenerator does not properly handle function pointers

2 participants