From ebfc562973565c7936a2646116239f5dbc6ed823 Mon Sep 17 00:00:00 2001 From: Tomas Rylek Date: Thu, 4 Aug 2022 01:45:41 +0200 Subject: [PATCH 1/2] Fix missing case for function pointer signatures in Crossgen2 As Andy Ayers suggested in the issue #72822 and I confirmed, Crossgen2 signature emitter was missing the code path for emitting function pointer signatures. This change fixes this deficiency. The change also removes the issues.targets exclusion of the affected test that Andy originally merged in to mitigate the issue. Thanks Tomas --- .../Common/Internal/Runtime/CorConstants.cs | 32 ++++++++++++++++++ .../ReadyToRun/SignatureBuilder.cs | 33 +++++++++++++++++++ src/tests/issues.targets | 3 -- 3 files changed, 65 insertions(+), 3 deletions(-) diff --git a/src/coreclr/tools/Common/Internal/Runtime/CorConstants.cs b/src/coreclr/tools/Common/Internal/Runtime/CorConstants.cs index 5591b062f551ea..a7d99ffd0107f3 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/CorConstants.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/CorConstants.cs @@ -95,4 +95,36 @@ public enum CorTokenType mdtName = 0x71000000, mdtBaseType = 0x72000000, } + + [Flags] + public enum CorUnmanagedCallingConvention + { + IMAGE_CEE_UNMANAGED_CALLCONV_C = 0x1, + IMAGE_CEE_UNMANAGED_CALLCONV_STDCALL = 0x2, + IMAGE_CEE_UNMANAGED_CALLCONV_THISCALL = 0x3, + IMAGE_CEE_UNMANAGED_CALLCONV_FASTCALL = 0x4, + } + + [Flags] + public enum CorCallingConvention + { + IMAGE_CEE_CS_CALLCONV_DEFAULT = 0x0, + IMAGE_CEE_CS_CALLCONV_C = (int)CorUnmanagedCallingConvention.IMAGE_CEE_UNMANAGED_CALLCONV_C, + IMAGE_CEE_CS_CALLCONV_STDCALL = (int)CorUnmanagedCallingConvention.IMAGE_CEE_UNMANAGED_CALLCONV_STDCALL, + IMAGE_CEE_CS_CALLCONV_THISCALL = (int)CorUnmanagedCallingConvention.IMAGE_CEE_UNMANAGED_CALLCONV_THISCALL, + IMAGE_CEE_CS_CALLCONV_FASTCALL = (int)CorUnmanagedCallingConvention.IMAGE_CEE_UNMANAGED_CALLCONV_FASTCALL, + IMAGE_CEE_CS_CALLCONV_VARARG = 0x5, + IMAGE_CEE_CS_CALLCONV_FIELD = 0x6, + IMAGE_CEE_CS_CALLCONV_LOCAL_SIG = 0x7, + IMAGE_CEE_CS_CALLCONV_PROPERTY = 0x8, + IMAGE_CEE_CS_CALLCONV_UNMANAGED = 0x9, // Unmanaged calling convention encoded as modopts + IMAGE_CEE_CS_CALLCONV_GENERICINST = 0xa, // generic method instantiation + IMAGE_CEE_CS_CALLCONV_NATIVEVARARG = 0xb, // used ONLY for 64bit vararg PInvoke calls + IMAGE_CEE_CS_CALLCONV_MAX = 0xc, // first invalid calling convention + + IMAGE_CEE_CS_CALLCONV_MASK = 0x0f, // Calling convention is bottom 4 bits + IMAGE_CEE_CS_CALLCONV_HASTHIS = 0x20, // Top bit indicates a 'this' parameter + IMAGE_CEE_CS_CALLCONV_EXPLICITTHIS = 0x40, // This parameter is explicitly in the signature + IMAGE_CEE_CS_CALLCONV_GENERIC = 0x10, // Generic method sig with explicit number of type arguments (precedes ordinary parameter count) + } } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureBuilder.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureBuilder.cs index 6632e7439655fc..f89e9ccc24b184 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureBuilder.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureBuilder.cs @@ -221,6 +221,10 @@ public void EmitTypeSignature(TypeDesc typeDesc, SignatureContext context) EmitPointerTypeSignature((PointerType)typeDesc, context); return; + case TypeFlags.FunctionPointer: + EmitFunctionPointerTypeSignature((FunctionPointerType)typeDesc, context); + return; + case TypeFlags.ByRef: EmitByRefTypeSignature((ByRefType)typeDesc, context); break; @@ -366,6 +370,35 @@ private void EmitPointerTypeSignature(PointerType type, SignatureContext context EmitTypeSignature(type.ParameterType, context); } + private void EmitFunctionPointerTypeSignature(FunctionPointerType type, SignatureContext context) + { + CorCallingConvention callingConvention = (type.Signature.Flags & MethodSignatureFlags.UnmanagedCallingConventionMask) switch + { + MethodSignatureFlags.None => CorCallingConvention.IMAGE_CEE_CS_CALLCONV_DEFAULT, + MethodSignatureFlags.UnmanagedCallingConventionCdecl => CorCallingConvention.IMAGE_CEE_CS_CALLCONV_C, + MethodSignatureFlags.UnmanagedCallingConventionStdCall => CorCallingConvention.IMAGE_CEE_CS_CALLCONV_STDCALL, + MethodSignatureFlags.UnmanagedCallingConventionThisCall => CorCallingConvention.IMAGE_CEE_CS_CALLCONV_THISCALL, + MethodSignatureFlags.CallingConventionVarargs => CorCallingConvention.IMAGE_CEE_CS_CALLCONV_VARARG, + MethodSignatureFlags.UnmanagedCallingConvention => CorCallingConvention.IMAGE_CEE_CS_CALLCONV_UNMANAGED, + _ => throw new NotSupportedException() + }; + + if ((type.Signature.Flags & MethodSignatureFlags.Static) == 0) + { + callingConvention |= CorCallingConvention.IMAGE_CEE_CS_CALLCONV_HASTHIS | CorCallingConvention.IMAGE_CEE_CS_CALLCONV_EXPLICITTHIS; + } + + EmitElementType(CorElementType.ELEMENT_TYPE_FNPTR); + EmitUInt((byte)callingConvention); + EmitUInt((uint)type.Signature.Length); + + EmitTypeSignature(type.Signature.ReturnType, context); + for (int argIndex = 0; argIndex < type.Signature.Length; argIndex++) + { + EmitTypeSignature(type.Signature[argIndex], context); + } + } + private void EmitByRefTypeSignature(ByRefType type, SignatureContext context) { EmitElementType(CorElementType.ELEMENT_TYPE_BYREF); diff --git a/src/tests/issues.targets b/src/tests/issues.targets index a720de9c70abc5..56a24fa0786db1 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -925,9 +925,6 @@ https://github.com/dotnet/runtime/issues/63856 - - https://github.com/dotnet/runtime/issues/72822 - From 9a4cc483569d1970e1d50ef31fe0b1566b62d1b5 Mon Sep 17 00:00:00 2001 From: Tomas Rylek Date: Thu, 4 Aug 2022 02:47:51 +0200 Subject: [PATCH 2/2] Address JanK's PR feedback I have reverted the additions to CorConstants and modified the code to use the constants in System.Reflection.Metadata. I have removed the flag translation based on Jan's confirmation that the calling convention part of MethodSignatureFlags is indeed kept in sync with the CoreCLR native runtime calling conventions and I dropped the explicit this flag. Thanks Tomas --- .../Common/Internal/Runtime/CorConstants.cs | 32 ------------------- .../ReadyToRun/SignatureBuilder.cs | 19 ++--------- 2 files changed, 3 insertions(+), 48 deletions(-) diff --git a/src/coreclr/tools/Common/Internal/Runtime/CorConstants.cs b/src/coreclr/tools/Common/Internal/Runtime/CorConstants.cs index a7d99ffd0107f3..5591b062f551ea 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/CorConstants.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/CorConstants.cs @@ -95,36 +95,4 @@ public enum CorTokenType mdtName = 0x71000000, mdtBaseType = 0x72000000, } - - [Flags] - public enum CorUnmanagedCallingConvention - { - IMAGE_CEE_UNMANAGED_CALLCONV_C = 0x1, - IMAGE_CEE_UNMANAGED_CALLCONV_STDCALL = 0x2, - IMAGE_CEE_UNMANAGED_CALLCONV_THISCALL = 0x3, - IMAGE_CEE_UNMANAGED_CALLCONV_FASTCALL = 0x4, - } - - [Flags] - public enum CorCallingConvention - { - IMAGE_CEE_CS_CALLCONV_DEFAULT = 0x0, - IMAGE_CEE_CS_CALLCONV_C = (int)CorUnmanagedCallingConvention.IMAGE_CEE_UNMANAGED_CALLCONV_C, - IMAGE_CEE_CS_CALLCONV_STDCALL = (int)CorUnmanagedCallingConvention.IMAGE_CEE_UNMANAGED_CALLCONV_STDCALL, - IMAGE_CEE_CS_CALLCONV_THISCALL = (int)CorUnmanagedCallingConvention.IMAGE_CEE_UNMANAGED_CALLCONV_THISCALL, - IMAGE_CEE_CS_CALLCONV_FASTCALL = (int)CorUnmanagedCallingConvention.IMAGE_CEE_UNMANAGED_CALLCONV_FASTCALL, - IMAGE_CEE_CS_CALLCONV_VARARG = 0x5, - IMAGE_CEE_CS_CALLCONV_FIELD = 0x6, - IMAGE_CEE_CS_CALLCONV_LOCAL_SIG = 0x7, - IMAGE_CEE_CS_CALLCONV_PROPERTY = 0x8, - IMAGE_CEE_CS_CALLCONV_UNMANAGED = 0x9, // Unmanaged calling convention encoded as modopts - IMAGE_CEE_CS_CALLCONV_GENERICINST = 0xa, // generic method instantiation - IMAGE_CEE_CS_CALLCONV_NATIVEVARARG = 0xb, // used ONLY for 64bit vararg PInvoke calls - IMAGE_CEE_CS_CALLCONV_MAX = 0xc, // first invalid calling convention - - IMAGE_CEE_CS_CALLCONV_MASK = 0x0f, // Calling convention is bottom 4 bits - IMAGE_CEE_CS_CALLCONV_HASTHIS = 0x20, // Top bit indicates a 'this' parameter - IMAGE_CEE_CS_CALLCONV_EXPLICITTHIS = 0x40, // This parameter is explicitly in the signature - IMAGE_CEE_CS_CALLCONV_GENERIC = 0x10, // Generic method sig with explicit number of type arguments (precedes ordinary parameter count) - } } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureBuilder.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureBuilder.cs index f89e9ccc24b184..01bb5fe766d78d 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureBuilder.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureBuilder.cs @@ -372,24 +372,11 @@ private void EmitPointerTypeSignature(PointerType type, SignatureContext context private void EmitFunctionPointerTypeSignature(FunctionPointerType type, SignatureContext context) { - CorCallingConvention callingConvention = (type.Signature.Flags & MethodSignatureFlags.UnmanagedCallingConventionMask) switch - { - MethodSignatureFlags.None => CorCallingConvention.IMAGE_CEE_CS_CALLCONV_DEFAULT, - MethodSignatureFlags.UnmanagedCallingConventionCdecl => CorCallingConvention.IMAGE_CEE_CS_CALLCONV_C, - MethodSignatureFlags.UnmanagedCallingConventionStdCall => CorCallingConvention.IMAGE_CEE_CS_CALLCONV_STDCALL, - MethodSignatureFlags.UnmanagedCallingConventionThisCall => CorCallingConvention.IMAGE_CEE_CS_CALLCONV_THISCALL, - MethodSignatureFlags.CallingConventionVarargs => CorCallingConvention.IMAGE_CEE_CS_CALLCONV_VARARG, - MethodSignatureFlags.UnmanagedCallingConvention => CorCallingConvention.IMAGE_CEE_CS_CALLCONV_UNMANAGED, - _ => throw new NotSupportedException() - }; - - if ((type.Signature.Flags & MethodSignatureFlags.Static) == 0) - { - callingConvention |= CorCallingConvention.IMAGE_CEE_CS_CALLCONV_HASTHIS | CorCallingConvention.IMAGE_CEE_CS_CALLCONV_EXPLICITTHIS; - } + SignatureCallingConvention callingConvention = (SignatureCallingConvention)(type.Signature.Flags & MethodSignatureFlags.UnmanagedCallingConventionMask); + SignatureAttributes callingConventionAttributes = ((type.Signature.Flags & MethodSignatureFlags.Static) != 0 ? SignatureAttributes.None : SignatureAttributes.Instance); EmitElementType(CorElementType.ELEMENT_TYPE_FNPTR); - EmitUInt((byte)callingConvention); + EmitUInt((uint)((byte)callingConvention | (byte)callingConventionAttributes)); EmitUInt((uint)type.Signature.Length); EmitTypeSignature(type.Signature.ReturnType, context);