Disambiguate calli thunks based on full calling conventions (#73359)

* Disambiguate calli thunks based on full calling conventions Fixes dotnet/runtimelab#153 * Add smoke test Co-authored-by: Michal Strehovský <[email protected]>
dotnet · Aug 5, 2022 · 12dc44c · 12dc44c
1 parent 1f070bf
commit 12dc44c
Show file tree

Hide file tree

Showing 12 changed files with 168 additions and 51 deletions.
diff --git a/...oreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/TypeLoaderExceptionHelper.cs b/...oreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/TypeLoaderExceptionHelper.cs
@@ -88,6 +88,14 @@ private static string GetFormatString(ExceptionStringID id)
                     return SR.InvalidProgram_Vararg;
                 case ExceptionStringID.InvalidProgramCallVirtFinalize:
                     return SR.InvalidProgram_CallVirtFinalize;
+                case ExceptionStringID.InvalidProgramNonStaticMethod:
+                    return SR.InvalidProgram_NonStaticMethod;
+                case ExceptionStringID.InvalidProgramGenericMethod:
+                    return SR.InvalidProgram_GenericMethod;
+                case ExceptionStringID.InvalidProgramNonBlittableTypes:
+                    return SR.InvalidProgram_NonBlittableTypes;
+                case ExceptionStringID.InvalidProgramMultipleCallConv:
+                    return SR.InvalidProgram_MultipleCallConv;
                 case ExceptionStringID.MissingField:
                     return SR.EE_MissingField;
                 case ExceptionStringID.MissingMethod:

diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MethodDesc.cs b/src/coreclr/tools/Common/TypeSystem/Common/MethodDesc.cs
@@ -12,7 +12,6 @@ namespace Internal.TypeSystem
     public enum MethodSignatureFlags
     {
         None = 0x0000,
-        // TODO: Generic, etc.
 
         UnmanagedCallingConventionMask       = 0x000F,
         UnmanagedCallingConventionCdecl      = 0x0001,
@@ -203,7 +202,6 @@ public bool EqualsWithCovariantReturnType(MethodSignature otherSignature)
 
         private bool Equals(MethodSignature otherSignature, bool allowCovariantReturn)
         {
-            // TODO: Generics, etc.
             if (this._flags != otherSignature._flags)
                 return false;
 
@@ -249,12 +247,15 @@ private bool Equals(MethodSignature otherSignature, bool allowCovariantReturn)
 
                 for (int i = 0; i < this._embeddedSignatureData.Length; i++)
                 {
-                    if (this._embeddedSignatureData[i].index != otherSignature._embeddedSignatureData[i].index)
-                        return false;
-                    if (this._embeddedSignatureData[i].kind != otherSignature._embeddedSignatureData[i].kind)
-                        return false;
-                    if (this._embeddedSignatureData[i].type != otherSignature._embeddedSignatureData[i].type)
+                    ref EmbeddedSignatureData thisData = ref this._embeddedSignatureData[i];
+                    ref EmbeddedSignatureData otherData = ref otherSignature._embeddedSignatureData[i];
+
+                    if (thisData.index != otherData.index ||
+                        thisData.kind != otherData.kind ||
+                        thisData.type != otherData.type)
+                    {
                         return false;
+                    }
                 }
 
                 return true;
@@ -314,7 +315,7 @@ public struct MethodSignatureBuilder
         private int _genericParameterCount;
         private TypeDesc _returnType;
         private TypeDesc[] _parameters;
-        private EmbeddedSignatureData[] _customModifiers;
+        private EmbeddedSignatureData[] _embeddedSignatureData;
 
         public MethodSignatureBuilder(MethodSignature template)
         {
@@ -324,7 +325,7 @@ public MethodSignatureBuilder(MethodSignature template)
             _genericParameterCount = template._genericParameterCount;
             _returnType = template._returnType;
             _parameters = template._parameters;
-            _customModifiers = template._embeddedSignatureData;
+            _embeddedSignatureData = template._embeddedSignatureData;
         }
 
         public MethodSignatureFlags Flags
@@ -371,15 +372,21 @@ public int Length
             }
         }
 
+        public void SetEmbeddedSignatureData(EmbeddedSignatureData[] embeddedSignatureData)
+        {
+            _embeddedSignatureData = embeddedSignatureData;
+        }
+
         public MethodSignature ToSignature()
         {
             if (_template == null ||
                 _flags != _template._flags ||
                 _genericParameterCount != _template._genericParameterCount ||
                 _returnType != _template._returnType ||
-                _parameters != _template._parameters)
+                _parameters != _template._parameters ||
+                _embeddedSignatureData != _template._embeddedSignatureData)
             {
-                _template = new MethodSignature(_flags, _genericParameterCount, _returnType, _parameters, _customModifiers);
+                _template = new MethodSignature(_flags, _genericParameterCount, _returnType, _parameters, _embeddedSignatureData);
             }
 
             return _template;

diff --git a/src/coreclr/tools/Common/TypeSystem/Common/Properties/Resources.resx b/src/coreclr/tools/Common/TypeSystem/Common/Properties/Resources.resx
@@ -156,9 +156,6 @@
   <data name="InvalidProgramCallVirtFinalize" xml:space="preserve">
     <value>Callvirt of '{0}' not supported</value>
   </data>
-  <data name="InvalidProgramUnmanagedCallersOnly" xml:space="preserve">
-    <value>UnmanagedCallersOnly method '{0}' cannot be called from managed code</value>
-  </data>
   <data name="InvalidProgramCallAbstractMethod" xml:space="preserve">
     <value>Direct call to abstract method '{0}' not allowed</value>
   </data>

diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/CalliMarshallingMethodThunk.Mangling.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/CalliMarshallingMethodThunk.Mangling.cs
@@ -2,6 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Diagnostics;
+
 using Internal.TypeSystem;
 
 namespace Internal.IL.Stubs
@@ -20,7 +22,16 @@ string IPrefixMangledSignature.Prefix
         {
             get
             {
-                return RuntimeMarshallingEnabled ? "CalliWithRuntimeMarshalling" : "Calli";
+                string prefix = RuntimeMarshallingEnabled ? "CalliWithRuntimeMarshalling" : "Calli";
+
+                // The target signature is expected to be normalized as MethodSignatureFlags.UnmanagedCallingConvention
+                Debug.Assert((_targetSignature.Flags & MethodSignatureFlags.UnmanagedCallingConventionMask) == MethodSignatureFlags.UnmanagedCallingConvention);
+
+                // Append calling convention details to the prefix
+                if (_targetSignature.HasEmbeddedSignatureData)
+                    prefix += _targetSignature.GetStandaloneMethodSignatureCallingConventions().ToString("x");
+
+                return prefix;
             }
         }
     }

diff --git a/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs b/src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs
@@ -204,8 +204,7 @@ private void EmitDelegateCall(DelegateMarshallingMethodThunk delegateMethod, PIn
                 MethodDesc invokeMethod = delegateMethod.DelegateType.GetKnownMethod("Invoke", null);
                 callsiteSetupCodeStream.Emit(ILOpcode.callvirt, emitter.NewToken(invokeMethod));
             }
-            else if (delegateMethod.Kind == DelegateMarshallingMethodThunkKind
-                .ForwardNativeFunctionWrapper)
+            else if (delegateMethod.Kind == DelegateMarshallingMethodThunkKind.ForwardNativeFunctionWrapper)
             {
                 // if the SetLastError flag is set in UnmanagedFunctionPointerAttribute, clear the error code before doing P/Invoke
                 if (_flags.SetLastError)
@@ -309,27 +308,9 @@ private void EmitPInvokeCall(PInvokeILCodeStreams ilCodeStreams)
                 fnptrLoadStream.Emit(ILOpcode.call, emitter.NewToken(lazyHelperType
                     .GetKnownMethod("ResolvePInvoke", null)));
 
-                MethodSignatureFlags unmanagedCallingConvention = _flags.UnmanagedCallingConvention;
-                if (unmanagedCallingConvention == MethodSignatureFlags.None)
-                    unmanagedCallingConvention = MethodSignatureFlags.UnmanagedCallingConvention;
-
-                EmbeddedSignatureData[] embeddedSignatureData = null;
-                if (_targetMethod.HasCustomAttribute("System.Runtime.InteropServices", "SuppressGCTransitionAttribute"))
-                {
-                    embeddedSignatureData = new EmbeddedSignatureData[]
-                    {
-                        new EmbeddedSignatureData()
-                        {
-                            index = MethodSignature.IndexOfCustomModifiersOnReturnType,
-                            kind = EmbeddedSignatureDataKind.OptionalCustomModifier,
-                            type = context.SystemModule.GetKnownType("System.Runtime.CompilerServices", "CallConvSuppressGCTransition")
-                        }
-                    };
-                }
-
                 MethodSignature nativeSig = new MethodSignature(
-                    _targetMethod.Signature.Flags | unmanagedCallingConvention, 0, nativeReturnType,
-                    nativeParameterTypes, embeddedSignatureData);
+                    MethodSignatureFlags.Static | MethodSignatureFlags.UnmanagedCallingConvention, 0, nativeReturnType, nativeParameterTypes,
+                    _targetMethod.GetPInvokeMethodCallingConventions().EncodeAsEmbeddedSignatureData(context));
 
                 ILLocalVariable vNativeFunctionPointer = emitter.NewLocal(context
                     .GetWellKnownType(WellKnownType.IntPtr));

diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/InteropStateManager.cs b/src/coreclr/tools/Common/TypeSystem/Interop/InteropStateManager.cs
@@ -188,7 +188,11 @@ public FieldDesc GetPInvokeLazyFixupField(MethodDesc method)
 
         public MethodDesc GetPInvokeCalliStub(MethodSignature signature, ModuleDesc moduleContext)
         {
-            return _pInvokeCalliHashtable.GetOrCreateValue(new CalliMarshallingMethodThunkKey(signature, MarshalHelpers.IsRuntimeMarshallingEnabled(moduleContext)));
+            // Normalize calling convention details on the signature
+            var normalizedSignatureBuilder = new MethodSignatureBuilder(signature);
+            normalizedSignatureBuilder.Flags = (signature.Flags & MethodSignatureFlags.Static) | MethodSignatureFlags.UnmanagedCallingConvention;
+            normalizedSignatureBuilder.SetEmbeddedSignatureData(signature.GetStandaloneMethodSignatureCallingConventions().EncodeAsEmbeddedSignatureData(moduleContext.Context));
+            return _pInvokeCalliHashtable.GetOrCreateValue(new CalliMarshallingMethodThunkKey(normalizedSignatureBuilder.ToSignature(), MarshalHelpers.IsRuntimeMarshallingEnabled(moduleContext)));
         }
 
         private class NativeStructTypeHashtable : LockFreeReaderHashtable<MetadataType, NativeStructType>

diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/UnmanagedCallingConventions.cs b/src/coreclr/tools/Common/TypeSystem/Interop/UnmanagedCallingConventions.cs
@@ -3,12 +3,13 @@
 
 using System;
 using System.Collections.Immutable;
+using System.Diagnostics;
+using System.Numerics;
 using System.Reflection.Metadata;
 
+using Internal.IL;
 using Internal.TypeSystem.Ecma;
 
-using Debug = System.Diagnostics.Debug;
-
 namespace Internal.TypeSystem
 {
     /// <summary>
@@ -193,6 +194,53 @@ private static UnmanagedCallingConventions AccumulateCallingConventions(Unmanage
             return existing | addedCallConv.Value;
         }
 
+        public static EmbeddedSignatureData[] EncodeAsEmbeddedSignatureData(this UnmanagedCallingConventions callingConventions, TypeSystemContext context)
+        {
+            UnmanagedCallingConventions convention = (callingConventions & UnmanagedCallingConventions.CallingConventionMask);
+            UnmanagedCallingConventions modifiers = (callingConventions & UnmanagedCallingConventions.ModifiersMask);
+
+            UnmanagedCallingConventions platformDefault = GetPlatformDefaultUnmanagedCallingConvention(context);
+
+            int count = ((convention != platformDefault) ? 1 : 0) + BitOperations.PopCount((uint)modifiers);
+
+            if (count == 0)
+                return null;
+
+            EmbeddedSignatureData[] ret = new EmbeddedSignatureData[count];
+
+            int index = 0;
+
+            if (convention != platformDefault)
+            {
+                ret[index++] = CreateCallConvEmbeddedSignatureData(context, convention switch
+                {
+                    UnmanagedCallingConventions.Cdecl => "CallConvCdecl",
+                    UnmanagedCallingConventions.Stdcall => "CallConvStdcall",
+                    UnmanagedCallingConventions.Fastcall => "CallConvFastcall",
+                    UnmanagedCallingConventions.Thiscall => "CallConvThiscall",
+                    _ => throw new InvalidProgramException()
+                });
+            }
+
+            if ((modifiers & UnmanagedCallingConventions.IsMemberFunction) != 0)
+                ret[index++] = CreateCallConvEmbeddedSignatureData(context, "CallConvMemberFunction");
+
+            if ((modifiers & UnmanagedCallingConventions.IsSuppressGcTransition) != 0)
+                ret[index++] = CreateCallConvEmbeddedSignatureData(context, "CallConvSuppressGCTransition");
+
+            Debug.Assert(index == count);
+
+            return ret;
+
+            static EmbeddedSignatureData CreateCallConvEmbeddedSignatureData(TypeSystemContext context, string name)
+                => new()
+                {
+                    index = MethodSignature.IndexOfCustomModifiersOnReturnType,
+                    kind = EmbeddedSignatureDataKind.OptionalCustomModifier,
+                    type = context.SystemModule.GetKnownType("System.Runtime.CompilerServices", name)
+                };
+        }
+
         private static UnmanagedCallingConventions GetPlatformDefaultUnmanagedCallingConvention(TypeSystemContext context)
             => context.Target.IsWindows ? UnmanagedCallingConventions.Stdcall : UnmanagedCallingConventions.Cdecl;
     }

diff --git a/src/coreclr/tools/Common/TypeSystem/Sorting/MethodSignature.Sorting.cs b/src/coreclr/tools/Common/TypeSystem/Sorting/MethodSignature.Sorting.cs
@@ -8,15 +8,15 @@ partial class MethodSignature
     {
         internal int CompareTo(MethodSignature other, TypeSystemComparer comparer)
         {
-            int result = _parameters.Length - other._parameters.Length;
+            int result = _parameters.Length.CompareTo(other._parameters.Length);
             if (result != 0)
                 return result;
 
-            result = (int)_flags - (int)other._flags;
+            result = ((int)_flags).CompareTo((int)other._flags);
             if (result != 0)
                 return result;
 
-            result = _genericParameterCount - other._genericParameterCount;
+            result = _genericParameterCount.CompareTo(other._genericParameterCount);
             if (result != 0)
                 return result;
 
@@ -30,10 +30,35 @@ internal int CompareTo(MethodSignature other, TypeSystemComparer comparer)
             {
                 result = comparer.Compare(_parameters[i], other._parameters[i]);
                 if (result != 0)
-                    break;
+                    return result;
             }
 
-            return result;
+            if (_embeddedSignatureData == null || other._embeddedSignatureData == null)
+                return (_embeddedSignatureData?.Length ?? 0).CompareTo(other._embeddedSignatureData?.Length ?? 0);
+
+            result = _embeddedSignatureData.Length.CompareTo(other._embeddedSignatureData.Length);
+            if (result != 0)
+                return result;
+
+            for (int i = 0; i < _embeddedSignatureData.Length; i++)
+            {
+                ref EmbeddedSignatureData thisData = ref _embeddedSignatureData[i];
+                ref EmbeddedSignatureData otherData = ref other._embeddedSignatureData[i];
+
+                result = string.CompareOrdinal(thisData.index, otherData.index);
+                if (result != 0)
+                    return result;
+
+                result = ((int)thisData.kind).CompareTo((int)otherData.kind);
+                if (result != 0)
+                    return result;
+
+                result = comparer.Compare(thisData.type, otherData.type);
+                if (result != 0)
+                    return result;
+            }
+
+            return 0;
         }
     }
 }
diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx
@@ -3929,6 +3929,9 @@
   <data name="InvalidProgram_CallVirtFinalize" xml:space="preserve">
     <value>Object.Finalize() can not be called directly. It is only callable by the runtime.</value>
   </data>
+  <data name="InvalidProgram_MultipleCallConv" xml:space="preserve">
+    <value>Multiple unmanaged calling conventions are specified. Only a single calling convention is supported.</value>
+  </data>
   <data name="Delegate_GarbageCollected" xml:space="preserve">
     <value>The corresponding delegate has been garbage collected. Please make sure the delegate is still referenced by managed code when you are using the marshalled native function pointer.</value>
   </data>

diff --git a/src/tests/baseservices/callconvs/TestCallingConventions.cs b/src/tests/baseservices/callconvs/TestCallingConventions.cs
@@ -30,9 +30,7 @@ public static string GetFileName()
 
         public static string GetFullPath()
         {
-            Assembly assembly = Assembly.GetExecutingAssembly();
-            string directory = Path.GetDirectoryName(assembly.Location);
-            return Path.Combine(directory, GetFileName());
+            return Path.Combine(AppContext.BaseDirectory, GetFileName());
         }
     }
 

diff --git a/src/tests/issues.targets b/src/tests/issues.targets
@@ -938,9 +938,6 @@
         <ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/StaticVirtualMethods/DiamondShape/svm_diamondshape_r/*">
             <Issue>https://github.com/dotnet/runtime/issues/72589</Issue>
         </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/baseservices/callconvs/TestCallingConventions/*">
-            <Issue>https://github.com/dotnet/runtimelab/issues/153</Issue>
-        </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)/baseservices/RuntimeConfiguration/TestConfig/*">
             <Issue>Test expects being run with corerun</Issue>
         </ExcludeList>