dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.45k stars 4.76k forks source link

Add implementations for SignatureHelper.AddSentinel() and/or other methods in mono runtime #96389

Open buyaa-n opened 11 months ago

buyaa-n commented 11 months ago

A test added for ILGeneratorImpl.Emit(OpCode opcode, SignatureHelper signature) failed with

System.NotImplementedException : The method or operation is not implemented.
      Stack Trace:
        /_/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/SignatureHelper.cs(277,0): at System.Reflection.Emit.SignatureHelper.AddSentinel()
        /_/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs(1847,0): at System.Reflection.Emit.Tests.AssemblySaveILGeneratorTests.Emit_CallBySignature()
        /_/src/mono/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.Mono.cs(22,0): at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
        /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs(57,0): at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

The same would happen for VarArg methods. Looks there is several other public APIs not implemented, it would be great to add implementations for all

We might be able to move the runtime implementation into share libraries instead of trying to add implementation for mono implementation.

buyaa-n commented 10 months ago

Mono runtime also doesn't support standalone signature:

 System.NotImplementedException : The method or operation is not implemented.

  Stack Trace: 
SignatureHelper.GetSignature() line 381
ModuleBuilderImpl.GetSignatureMetadataToken(SignatureHelper signature) line 1040
AssemblySaveModuleBuilderTests.GetABCMetadataToken_Validations() line 283
ghost commented 10 months ago

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

Issue Details
A [test](https://github.com/dotnet/runtime/blob/0ba232e70e824474baa10e29350bd5c5552a3f85/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs#L1779) added for `ILGeneratorImpl.Emit(OpCode opcode, SignatureHelper signature)` failed with ```log System.NotImplementedException : The method or operation is not implemented. Stack Trace: /_/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/SignatureHelper.cs(277,0): at System.Reflection.Emit.SignatureHelper.AddSentinel() /_/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs(1847,0): at System.Reflection.Emit.Tests.AssemblySaveILGeneratorTests.Emit_CallBySignature() /_/src/mono/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.Mono.cs(22,0): at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args) /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs(57,0): at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr) ``` The same would happen for VarArg methods. Looks there is several other public APIs not implemented, it would be great to add implementations for all We might be able to move the runtime implementation into share libraries instead of trying to add implementation for mono implementation.
Author: buyaa-n
Assignees: -
Labels: `area-System.Reflection`, `untriaged`, `runtime-mono`, `area-System.Reflection-mono`
Milestone: -
steveharter commented 8 months ago

cc @BrzVlad