Repository navigation
Encode function pointer types in DynamicMethod signatures again - #136454
caraioniurie47 wants to merge 3 commits into
Conversation
Since the new function pointer APIs, SignatureHelper throws
NotSupportedException ("Function pointer signature can only be encoded
by dynamic ILGenerator.") for a function pointer type in a signature
built without a DynamicScope. Signatures without a module take that
path: a DynamicMethod's own signature (GetILGenerator,
GetDynamicILInfo), its locals (DeclareLocal), the return type of a
method passed to DynamicILGenerator.EmitCall and of EmitCalli, and
SignatureHelper.GetMethodSigHelper / GetLocalVarSigHelper called
without a module. .NET 10 encoded a function pointer runtime type there
as ELEMENT_TYPE_INTERNAL with its type handle, and such a DynamicMethod
ran.
Encode a function pointer RuntimeType that way again when the signature
has neither a dynamic scope nor a module. A function pointer in a
signature with a module (TypeBuilder), and a function pointer type that
is not a RuntimeType (a signature type), still throw.
Fix dotnet#136453
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @steveisok, @dotnet/area-system-reflection |
|
New test is crashing: Have you verified that the runtime is able to deal with function pointers encoded as |
There was a problem hiding this comment.
🟡 Changes recommended
The unmanaged return-type test reaches an unsupported marshalling path in the CoreCLR interpreter and needs a runtime fix or narrowly scoped exclusion.
1 open issue
What changed in this PR
Restores runtime function-pointer encoding in module-less signatures, addressing the DynamicMethod regression introduced by #123819 while preserving rejection of unsupported signatures.
Changes:
- Uses runtime type handles for function pointers when neither a module nor a dynamic scope is present.
- Adds regression coverage for parameters, locals, return types, and indirect calls through both dynamic IL APIs.
- Verifies that module-backed function-pointer signatures still throw.
Validation here was source-only; no builds or tests were run.
| File | Description |
|---|---|
src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs |
Tests function-pointer parameters and locals through DynamicILInfo. |
src/libraries/System.Reflection.Emit.ILGeneration/tests/SignatureHelper/SignatureHelperGetMethodSigHelper.cs |
Checks continued rejection of module-backed function-pointer signatures. |
src/libraries/System.Reflection.Emit.ILGeneration/tests/ILGenerator/Emit4Tests.cs |
Exercises function-pointer signatures and calls in dynamic methods. |
src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/SignatureHelper.cs |
Restores handle-based encoding for module-less runtime function-pointer types. |
Review effort: Balanced
| ILGenerator il = dynamicMethod.GetILGenerator(); | ||
| il.Emit(OpCodes.Ldarg_0); | ||
| if (unmanaged) | ||
| il.EmitCalli(OpCodes.Calli, CallingConvention.Cdecl, typeof(delegate*<int, int, int>), Type.EmptyTypes); |
There was a problem hiding this comment.
Fixed in 37815b3: a function pointer in a signature without a module is now written as ELEMENT_TYPE_FNPTR with its own signature, which MarshalingRequired handles like primitive and pointer types, so the unmanaged: true case stays enabled on the interpreter. Details: #136454 (comment).
ELEMENT_TYPE_INTERNAL stands in for ELEMENT_TYPE_CLASS or ELEMENT_TYPE_VALUETYPE, and readers such as PInvoke::MarshalingRequired treat its type handle as one: the CoreCLR interpreter rejected an unmanaged calli returning a function pointer. Write a function pointer in a signature without a module as ELEMENT_TYPE_FNPTR followed by its own signature, with its return and parameter types encoded as in the outer signature. That is the shape SigPointer::ConvertToInternalExactlyOne produces for module-independent signatures. Only the default and unmanaged calling conventions are written, so a function pointer type with another one (vararg or instance) throws NotSupportedException instead of loading as a different type. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No, I hadn't verified that; my CoreCLR runs used only the JIT. 37815b3 writes a function pointer in a signature without a module as Only the default and unmanaged calling conventions are written, so a vararg or instance function pointer type now throws |
| { | ||
| // Rejects a calling convention other than the default or unmanaged one (vararg, instance). | ||
| if (retType.MakeFunctionPointerType(paramTypes, functionPointerType.IsUnmanagedFunctionPointer) != functionPointerType) | ||
| throw new NotSupportedException(SR.NotSupported_FunctionPointerSignature); |
There was a problem hiding this comment.
This exception ("Function pointer signature can only be encoded by dynamic ILGenerator.") seems unrelated at best and misleading at worst. I don't think DynamicILGenerator supports encoding vararg or instance function pointer signatures either.
There was a problem hiding this comment.
Agreed. I reused the existing message to keep the regression fix small, but it doesn't fit here. Options: a new message for this throw only, or that plus the same throw in EmitCalli(Type), which currently drops their vararg or instance convention. Which one fits?
There was a problem hiding this comment.
I'm not sure this check is necessary for this PR anyway. I can hardly think of a scenario where this would even come up. Type.MakeFunctionPointerType and Type.MakeFunctionPointerSignatureType never produce hasthis or vararg function pointer types, and the C# compiler never emits them either.
I guess there are niche scenarios where such types could occur, but it seems unrelated to this regression fix. @jkotas What do you think?
There was a problem hiding this comment.
The check is there because this path, like EmitCalli(Type), never writes a vararg or instance convention. Without it, such a function pointer type read from IL metadata (for example a field type) would silently load as a different type; .NET 10 wrote the exact type handle, so the check guards the encoding this PR adds. Removing it means deleting the check and its test and rewording the description where it says such types throw. If it stays, it gets its own message, as offered above.
The same message also backs the throw for a signature with a module or a function pointer signature type (SignatureHelper.cs:492-493). With this PR, signatures without a module encode function pointers too (test), so "can only be encoded by dynamic ILGenerator" no longer holds there either. Should the check stay, and should that throw get its own message as well?
There was a problem hiding this comment.
I agree that this error message is confusing. Can it be fixed?
There was a problem hiding this comment.
Fixed in 8128bf7. The check stays; both throws now name the type and list the causes: "This signature cannot encode function pointer type '{0}': it has a module, the type is not a runtime type, or its calling convention is vararg or instance." The same push rewrites the code comments to give the reasons the code can't show.
| throw new NotSupportedException(SR.NotSupported_FunctionPointerSignature); | ||
|
|
||
| sig.AddOneArgTypeHelper(retType); | ||
| sig.AddArguments(paramTypes, null, null); |
There was a problem hiding this comment.
What happens with modopts and modreqs on this path? Are they silently ignored?
If they are ignored, does it mean that this does not respect the unmanaged calling convention from the function pointer?
There was a problem hiding this comment.
Nothing is dropped there. Only a RuntimeType reaches this path (SignatureHelper.cs:492-493), and a RuntimeType function pointer has no custom modifiers and, of the unmanaged calling conventions, keeps only unmanaged (RuntimeType.CoreCLR.cs:3776-3786); a vararg or instance one throws (SignatureHelper.cs:154-155). The loader builds the type from the component types and normalizes Cdecl, Stdcall, Thiscall and Fastcall to unmanaged (siginfo.cpp:1894-1897). On this PR's build, delegate* unmanaged[Cdecl, SuppressGCTransition]<int>, delegate* unmanaged[Stdcall]<int> and delegate* unmanaged<int> are the same type; this path writes a signature that loads back to it, the type .NET 10's ELEMENT_TYPE_INTERNAL handle named.
A modified type passed to the DynamicMethod constructor loses its modifiers there, as in .NET 10 (DynamicMethod.cs:238); DeclareLocal and module-less SignatureHelper reject it.
The convention for a call comes from the calli signature; EmitCalli(Type) writes the modifiers through the scope path (SignatureHelper.cs:162-163).
The message thrown for a function pointer type that a signature can't encode said only a dynamic ILGenerator could encode it, which no longer holds: signatures without a module now encode runtime function pointer types with the default or unmanaged calling convention. Name the type and list the causes. Give the reasons the code can't show: why a signature without a scope writes the runtime's module-independent shape and no modifiers, why it accepts only the default and unmanaged calling conventions, why a module's signature and a type other than a RuntimeType still throw, why the unmanaged test helper returns nint, and why the vararg and instance test helper reads a field type. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

On CoreCLR, since #123819,
SignatureHelperthrowsNotSupportedException("Function pointer signature can only be encoded by dynamic ILGenerator.") for a function pointer type in any signature built without aDynamicScope, so aDynamicMethodwith a function pointer parameter, local or return type fails; on .NET 10 it ran. Signatures without a module take that path: aDynamicMethod's own signature and locals, the return types ofEmitCallandEmitCalli, the optional parameter types of a vararg call, and theSignatureHelpermethods called without a module.The fix: in a signature with neither a
DynamicScopenor a module, a function pointerRuntimeTypeis written asELEMENT_TYPE_FNPTRfollowed by its own signature, with its return and parameter types encoded as in the rest of the signature, soELEMENT_TYPE_INTERNALagain stands only for a class or value type. That is the shapeSigPointer::ConvertToInternalExactlyOneproduces for module-independent signatures. Only the default and unmanaged calling conventions are written, so a function pointer type with another one (vararg or instance) throwsNotSupportedExceptioninstead of loading as a different type. #123819's review asked for the throw (r2808030756): "It is better to throw than produce bad binary silently." Signatures without a module now get the encoding the runtime itself uses for them; the throw stays for a signature with a module, a function pointer signature type, and the other calling conventions.This PR's first commit wrote
ELEMENT_TYPE_INTERNALwith the function pointer's own type handle, as .NET 10 did; readers such asPInvoke::MarshalingRequiredtake that handle for a class or value type, and the CoreCLR interpreter rejected an unmanagedcallireturning a function pointer.Tests in
ILGeneratorEmit4,SignatureHelperGetMethodSigHelperandDynamicILInfoTestscover function pointer parameters, locals and return types of aDynamicMethod(nested, unmanaged, with class and value-type components), the signature bytes written, and the throws for a signature with a module and for vararg and instance function pointer types.Resolves #136453
Note
AI-generated, written at my direction and reviewed by me before posting.
Verified on this PR's head over f0c7a08, Windows 11 x64: the System.Reflection.Emit, ILGeneration and Lightweight tests on CoreCLR (each new test expecting success fails without the fix), the last two also with the new tests' dynamic methods on the CoreCLR interpreter (Checked), and on Mono, JIT and interpreter.