From e1b1bcb1406853a06840210912076f64204536ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 14 Apr 2023 11:44:01 +0900 Subject: [PATCH 1/2] Adjust managed type system for new function pointer handling After #81006, the calling convention is no longer part of the type system identity of a function pointer type - it serves more like a modopt as far as the type system is concerned. The type system only cares whether the pointer is managed or not. Adjust the managed type system accordingly: * If we're reading/representing a standalone method signature, read it as usual. Calling convention is available in flags/modopts. * If we're reading/representing a function pointer type, collapse the calling convention information into the managed/unmanaged bit only. --- .../Common/TypeSystem/Common/MethodDesc.cs | 3 ++- .../TypeSystem/Common/TypeSystemContext.cs | 4 ++++ .../TypeSystem/Ecma/EcmaSignatureParser.cs | 14 ++++++++++++- .../CoreTestAssembly/CoreTestAssembly.csproj | 1 + .../CoreTestAssembly/Platform.cs | 10 +++++++++ .../VirtualFunctionOverride.cs | 18 ++++++++++++++++ .../VirtualFunctionOverrideTests.cs | 21 +++++++++++++++++++ 7 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MethodDesc.cs b/src/coreclr/tools/Common/TypeSystem/Common/MethodDesc.cs index 7802c85d56e6a..c0ba423ca927c 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MethodDesc.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MethodDesc.cs @@ -27,7 +27,8 @@ public enum EmbeddedSignatureDataKind { RequiredCustomModifier = 0, OptionalCustomModifier = 1, - ArrayShape = 2 + ArrayShape = 2, + UnmanagedCallConv = 3, } public struct EmbeddedSignatureData diff --git a/src/coreclr/tools/Common/TypeSystem/Common/TypeSystemContext.cs b/src/coreclr/tools/Common/TypeSystem/Common/TypeSystemContext.cs index 7d2f6cfffd0e7..233fedde33936 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/TypeSystemContext.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/TypeSystemContext.cs @@ -242,6 +242,10 @@ protected override FunctionPointerType CreateValueFromKey(MethodSignature key) public FunctionPointerType GetFunctionPointerType(MethodSignature signature) { + // The type system only distinguishes between unmanaged and managed signatures. + // The caller should have normalized the signature by modifying flags and stripping modopts. + Debug.Assert((signature.Flags & MethodSignatureFlags.UnmanagedCallingConventionMask) is 0 or MethodSignatureFlags.UnmanagedCallingConvention); + Debug.Assert(!signature.HasEmbeddedSignatureData); return _functionPointerTypes.GetOrCreateValue(signature); } diff --git a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaSignatureParser.cs b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaSignatureParser.cs index e49ce2768d0f7..318ca38f80021 100644 --- a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaSignatureParser.cs +++ b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaSignatureParser.cs @@ -371,7 +371,19 @@ private MethodSignature ParseMethodSignatureImpl(bool skipEmbeddedSignatureData) Debug.Assert((int)MethodSignatureFlags.CallingConventionVarargs == (int)SignatureCallingConvention.VarArgs); Debug.Assert((int)MethodSignatureFlags.UnmanagedCallingConvention == (int)SignatureCallingConvention.Unmanaged); - flags = (MethodSignatureFlags)signatureCallConv; + // If skipEmbeddedSignatureData is true, we're building the signature for the purposes of building a type. + // We normalize unmanaged calling convention into a single value - "unmanaged". + if (skipEmbeddedSignatureData) + { + flags = MethodSignatureFlags.UnmanagedCallingConvention; + + // But we still need to remember this signature is different, so add this to the EmbeddedSignatureData of the owner signature. + _embeddedSignatureDataList?.Add(new EmbeddedSignatureData { index = string.Join(".", _indexStack) + "|" + ((int)signatureCallConv).ToString(), kind = EmbeddedSignatureDataKind.UnmanagedCallConv, type = null }); + } + else + { + flags = (MethodSignatureFlags)signatureCallConv; + } } if (!header.IsInstance) diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/CoreTestAssembly.csproj b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/CoreTestAssembly.csproj index 871ffbcb5a204..90a8efbc32fac 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/CoreTestAssembly.csproj +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/CoreTestAssembly.csproj @@ -4,6 +4,7 @@ CoreTestAssembly false true + true true netstandard2.0 diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs index 75ade965f9318..c357cd1fb7866 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs @@ -60,6 +60,11 @@ public struct RuntimeTypeHandle { } public struct RuntimeMethodHandle { } public struct RuntimeFieldHandle { } + public class Type + { + public static Type GetTypeFromHandle(RuntimeTypeHandle handle) => null; + } + public class Attribute { } public class AttributeUsageAttribute : Attribute { @@ -159,9 +164,14 @@ public sealed class IsByRefLikeAttribute : Attribute { } + public class CallConvCdecl { } + public class CallConvStdcall { } + public class CallConvSuppressGCTransition { } + public static class RuntimeFeature { public const string ByRefFields = nameof(ByRefFields); + public const string UnmanagedSignatureCallingConvention = nameof(UnmanagedSignatureCallingConvention); public const string VirtualStaticsInInterfaces = nameof(VirtualStaticsInInterfaces); } diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/VirtualFunctionOverride.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/VirtualFunctionOverride.cs index c237f561a266f..8440dfc3cd00e 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/VirtualFunctionOverride.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/VirtualFunctionOverride.cs @@ -42,4 +42,22 @@ class ClassWithFinalizer } } + + unsafe class FunctionPointerOverloadBase + { + // Do not reorder these, the test assumes this order + public virtual Type Method(delegate* unmanaged[Cdecl] p) => typeof(delegate* unmanaged[Cdecl]); + public virtual Type Method(delegate* unmanaged[Stdcall] p) => typeof(delegate* unmanaged[Stdcall]); + public virtual Type Method(delegate* unmanaged[Stdcall, SuppressGCTransition] p) => typeof(delegate* unmanaged[Stdcall, SuppressGCTransition]); + public virtual Type Method(delegate* p) => typeof(delegate*); + } + + unsafe class FunctionPointerOverloadDerived : FunctionPointerOverloadBase + { + // Do not reorder these, the test assumes this order + public override Type Method(delegate* unmanaged[Cdecl] p) => null; + public override Type Method(delegate* unmanaged[Stdcall] p) => null; + public override Type Method(delegate* unmanaged[Stdcall, SuppressGCTransition] p) => null; + public override Type Method(delegate* p) => null; + } } diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/VirtualFunctionOverrideTests.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/VirtualFunctionOverrideTests.cs index e228993296ff5..d2f9d9576c631 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/VirtualFunctionOverrideTests.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/VirtualFunctionOverrideTests.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.Linq; using Internal.TypeSystem; @@ -268,5 +269,25 @@ public void TestExactMethodImplGenericDeclResolutionOnInterfaces() Assert.Contains("!0,!1", md1.Name); Assert.Contains("!1,!0", md2.Name); } + + [Fact] + public void TestFunctionPointerOverloads() + { + MetadataType baseClass = _testModule.GetType("VirtualFunctionOverride", "FunctionPointerOverloadBase"); + MetadataType derivedClass = _testModule.GetType("VirtualFunctionOverride", "FunctionPointerOverloadDerived"); + + var resolvedMethods = new List(); + foreach (MethodDesc baseMethod in baseClass.GetVirtualMethods()) + resolvedMethods.Add(derivedClass.FindVirtualFunctionTargetMethodOnObjectType(baseMethod)); + + var expectedMethods = new List(); + foreach (MethodDesc derivedMethod in derivedClass.GetVirtualMethods()) + expectedMethods.Add(derivedMethod); + + Assert.Equal(expectedMethods, resolvedMethods); + + Assert.Equal(expectedMethods[0].Signature[0], expectedMethods[1].Signature[0]); + Assert.NotEqual(expectedMethods[0].Signature[0], expectedMethods[3].Signature[0]); + } } } From 546b6a11ab349b68863e20736bd09e7925006080 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 14 Apr 2023 14:06:52 +0900 Subject: [PATCH 2/2] Make the encoding roundtrippable --- .../TypeSystemMetadataEmitter.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/coreclr/tools/Common/TypeSystem/MetadataEmitter/TypeSystemMetadataEmitter.cs b/src/coreclr/tools/Common/TypeSystem/MetadataEmitter/TypeSystemMetadataEmitter.cs index 217bdbe13a7e5..7f465939fc3fc 100644 --- a/src/coreclr/tools/Common/TypeSystem/MetadataEmitter/TypeSystemMetadataEmitter.cs +++ b/src/coreclr/tools/Common/TypeSystem/MetadataEmitter/TypeSystemMetadataEmitter.cs @@ -514,6 +514,27 @@ public void Push() } } + public void UpdateSignatureCallingConventionAtCurrentIndexStack(ref SignatureCallingConvention callConv) + { + if (!Complete) + { + if (_embeddedDataIndex < _embeddedData.Length) + { + if (_embeddedData[_embeddedDataIndex].kind == EmbeddedSignatureDataKind.UnmanagedCallConv) + { + string indexData = string.Join(".", _indexStack); + + var unmanagedCallConvPossibility = _embeddedData[_embeddedDataIndex].index.Split('|'); + if (unmanagedCallConvPossibility[0] == indexData) + { + callConv = (SignatureCallingConvention)int.Parse(unmanagedCallConvPossibility[1]); + _embeddedDataIndex++; + } + } + } + } + } + public void EmitArrayShapeAtCurrentIndexStack(BlobBuilder signatureBuilder, int rank) { var shapeEncoder = new ArrayShapeEncoder(signatureBuilder); @@ -665,6 +686,9 @@ private void EncodeMethodSignature(BlobBuilder signatureBuilder, MethodSignature break; } + if (sigCallingConvention != SignatureCallingConvention.Default) + signatureDataEmitter.UpdateSignatureCallingConventionAtCurrentIndexStack(ref sigCallingConvention); + signatureEncoder.MethodSignature(sigCallingConvention, genericParameterCount, isInstanceMethod); signatureBuilder.WriteCompressedInteger(sig.Length); EncodeType(signatureBuilder, sig.ReturnType, signatureDataEmitter);