Error on multiple calling conventions in modopts (#39809)
authorElinor Fung <47805090+elinor-fung@users.noreply.github.com>
Tue, 28 Jul 2020 17:21:56 +0000 (10:21 -0700)
committerGitHub <noreply@github.com>
Tue, 28 Jul 2020 17:21:56 +0000 (10:21 -0700)
12 files changed:
src/coreclr/src/dlls/mscorrc/mscorrc.rc
src/coreclr/src/dlls/mscorrc/resource.h
src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs
src/coreclr/src/tools/Common/TypeSystem/Common/ExceptionStringID.cs
src/coreclr/src/tools/Common/TypeSystem/Common/Properties/Resources.resx
src/coreclr/src/tools/Common/TypeSystem/Common/ThrowHelper.cs
src/coreclr/src/tools/Common/TypeSystem/Common/TypeSystemException.cs
src/coreclr/src/vm/dllimport.cpp
src/coreclr/src/vm/jitinterface.cpp
src/coreclr/src/vm/siginfo.cpp
src/coreclr/src/vm/siginfo.hpp
src/tests/baseservices/callconvs/TestCallingConventions.cs

index a50f742..3604420 100644 (file)
@@ -182,6 +182,7 @@ BEGIN
     IDS_EE_NDIRECT_UNSUPPORTED_SIG          "Method's type signature is not PInvoke compatible."
     IDS_EE_COM_UNSUPPORTED_SIG              "Method's type signature is not Interop compatible."
     IDS_EE_COM_UNSUPPORTED_TYPE             "The method returned a COM Variant type that is not Interop compatible."
+    IDS_EE_MULTIPLE_CALLCONV_UNSUPPORTED    "Multiple unmanaged calling conventions are specified. Only a single calling convention is supported."
     IDS_EE_NDIRECT_BADNATL                  "Invalid PInvoke or UnmanagedFunctionPointer metadata format."
     IDS_EE_NDIRECT_BADNATL_CALLCONV         "Invalid PInvoke or UnmanagedFunctionPointer calling convention."
     IDS_EE_NDIRECT_BADNATL_VARARGS_CALLCONV "Invalid PInvoke calling convention. Vararg functions must use the cdecl calling convention."
index 738b751..b78c5d8 100644 (file)
@@ -43,6 +43,7 @@
 #define IDS_EE_COM_UNSUPPORTED_SIG              0x170d
 #define IDS_EE_NOSYNCHRONIZED                   0x170f
 #define IDS_EE_NDIRECT_BADNATL_THISCALL         0x1710
+#define IDS_EE_MULTIPLE_CALLCONV_UNSUPPORTED    0x1711
 
 #define IDS_EE_LOAD_BAD_MAIN_SIG                0x1712
 #define IDS_EE_COM_UNSUPPORTED_TYPE             0x1713
index f549637..3d7dd28 100644 (file)
@@ -508,6 +508,7 @@ namespace Internal.JitInterface
             if (!signature.HasEmbeddedSignatureData || signature.GetEmbeddedSignatureData() == null)
                 return false;
 
+            bool found = false;
             foreach (EmbeddedSignatureData data in signature.GetEmbeddedSignatureData())
             {
                 if (data.kind != EmbeddedSignatureDataKind.OptionalCustomModifier)
@@ -524,25 +525,28 @@ namespace Internal.JitInterface
                 if (defType.Namespace != "System.Runtime.CompilerServices")
                     continue;
 
-                // Take the first recognized calling convention in metadata.
-                switch (defType.Name)
+                // Look for a recognized calling convention in metadata.
+                CorInfoCallConv? callConvLocal = defType.Name switch
                 {
-                    case "CallConvCdecl":
-                        callConv = CorInfoCallConv.CORINFO_CALLCONV_C;
-                        return true;
-                    case "CallConvStdcall":
-                        callConv = CorInfoCallConv.CORINFO_CALLCONV_STDCALL;
-                        return true;
-                    case "CallConvFastcall":
-                        callConv = CorInfoCallConv.CORINFO_CALLCONV_FASTCALL;
-                        return true;
-                    case "CallConvThiscall":
-                        callConv = CorInfoCallConv.CORINFO_CALLCONV_THISCALL;
-                        return true;
+                    "CallConvCdecl"     => CorInfoCallConv.CORINFO_CALLCONV_C,
+                    "CallConvStdcall"   => CorInfoCallConv.CORINFO_CALLCONV_STDCALL,
+                    "CallConvFastcall"  => CorInfoCallConv.CORINFO_CALLCONV_FASTCALL,
+                    "CallConvThiscall"  => CorInfoCallConv.CORINFO_CALLCONV_THISCALL,
+                    _ => null
+                };
+
+                if (callConvLocal.HasValue)
+                {
+                    // Error if there are multiple recognized calling conventions
+                    if (found)
+                        ThrowHelper.ThrowInvalidProgramException(ExceptionStringID.InvalidProgramMultipleCallConv, MethodBeingCompiled);
+                    callConv = callConvLocal.Value;
+                    found = true;
                 }
             }
 
-            return false;
+            return found;
         }
 
         private void Get_CORINFO_SIG_INFO(MethodSignature signature, CORINFO_SIG_INFO* sig)
index 00286ea..d9d3de4 100644 (file)
@@ -36,6 +36,7 @@ namespace Internal.TypeSystem
         InvalidProgramNonStaticMethod,
         InvalidProgramGenericMethod,
         InvalidProgramNonBlittableTypes,
+        InvalidProgramMultipleCallConv,
 
         // BadImageFormatException
         BadImageFormatGeneric,
index 3681459..b0efd67 100644 (file)
   <data name="InvalidProgramNonBlittableTypes" xml:space="preserve">
     <value>UnmanagedCallersOnly attribute specified on method with non-blittable parameters '{0}'</value>
   </data>
+  <data name="InvalidProgramMultipleCallConv" xml:space="preserve">
+    <value>Multiple unmanaged calling conventions are specified. Only a single calling convention is supported.</value>
+  </data>
   <data name="BadImageFormatGeneric" xml:space="preserve">
     <value>The format of a DLL or executable being loaded is invalid</value>
   </data>
-</root>
\ No newline at end of file
+</root>
index 9ab806e..094ade1 100644 (file)
@@ -42,6 +42,12 @@ namespace Internal.TypeSystem
         }
 
         [System.Diagnostics.DebuggerHidden]
+        public static void ThrowInvalidProgramException(ExceptionStringID id)
+        {
+            throw new TypeSystemException.InvalidProgramException(id);
+        }
+
+        [System.Diagnostics.DebuggerHidden]
         public static void ThrowInvalidProgramException(ExceptionStringID id, MethodDesc method)
         {
             throw new TypeSystemException.InvalidProgramException(id, Format.Method(method));
index 9311369..17836ca 100644 (file)
@@ -138,6 +138,11 @@ namespace Internal.TypeSystem
             {
             }
 
+            internal InvalidProgramException(ExceptionStringID id)
+                : base(id)
+            {
+            }
+
             internal InvalidProgramException()
                 : base(ExceptionStringID.InvalidProgramDefault)
             {
index 2fd2fef..752fbd0 100644 (file)
@@ -2955,7 +2955,8 @@ namespace
         _In_ Module *pModule,
         _In_ PCCOR_SIGNATURE pSig,
         _In_ ULONG cSig,
-        _Out_ CorPinvokeMap *pPinvokeMapOut)
+        _Out_ CorPinvokeMap *pPinvokeMapOut,
+        _Out_ UINT *errorResID)
     {
         CONTRACTL
         {
@@ -2966,7 +2967,7 @@ namespace
         CONTRACTL_END
 
         CorUnmanagedCallingConvention callConvMaybe;
-        HRESULT hr = MetaSig::TryGetUnmanagedCallingConventionFromModOpt(pModule, pSig, cSig, &callConvMaybe);
+        HRESULT hr = MetaSig::TryGetUnmanagedCallingConventionFromModOpt(pModule, pSig, cSig, &callConvMaybe, errorResID);
         if (hr != S_OK)
             return hr;
         
@@ -2992,10 +2993,12 @@ void PInvokeStaticSigInfo::InitCallConv(CorPinvokeMap callConv, BOOL bIsVarArg)
         callConv = GetDefaultCallConv(bIsVarArg);
 
     CorPinvokeMap sigCallConv = (CorPinvokeMap)0;
-    HRESULT hr = GetUnmanagedPInvokeCallingConvention(m_pModule, m_sig.GetRawSig(), m_sig.GetRawSigLen(), &sigCallConv);
+    UINT errorResID;
+    HRESULT hr = GetUnmanagedPInvokeCallingConvention(m_pModule, m_sig.GetRawSig(), m_sig.GetRawSigLen(), &sigCallConv, &errorResID);
     if (FAILED(hr))
     {
-        SetError(IDS_EE_NDIRECT_BADNATL); //Bad metadata format
+        // Set an error message specific to P/Invokes or UnmanagedFunction for bad format.
+        SetError(hr == COR_E_BADIMAGEFORMAT ? IDS_EE_NDIRECT_BADNATL : errorResID);
     }
 
     // Do the same WinAPI to StdCall or CDecl for the signature calling convention as well. We need
@@ -6859,7 +6862,12 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD)
         if (callConv == IMAGE_CEE_CS_CALLCONV_UNMANAGED)
         {
             CorUnmanagedCallingConvention callConvMaybe;
-            if (S_OK == MetaSig::TryGetUnmanagedCallingConventionFromModOpt(pVASigCookie->pModule, signature.GetRawSig(), signature.GetRawSigLen(), &callConvMaybe))
+            UINT errorResID;
+            HRESULT hr = MetaSig::TryGetUnmanagedCallingConventionFromModOpt(pVASigCookie->pModule, signature.GetRawSig(), signature.GetRawSigLen(), &callConvMaybe, &errorResID);
+            if (FAILED(hr))
+                COMPlusThrowHR(hr, errorResID);
+
+            if (hr == S_OK)
             {
                 callConv = callConvMaybe;
             }
index 09c0776..e955b3d 100644 (file)
@@ -515,7 +515,12 @@ CEEInfo::ConvToJitSig(
             static_assert_no_msg(CORINFO_CALLCONV_FASTCALL == (CorInfoCallConv)IMAGE_CEE_UNMANAGED_CALLCONV_FASTCALL);
 
             CorUnmanagedCallingConvention callConvMaybe;
-            if (S_OK == MetaSig::TryGetUnmanagedCallingConventionFromModOpt(module, pSig, cbSig, &callConvMaybe))
+            UINT errorResID;
+            HRESULT hr = MetaSig::TryGetUnmanagedCallingConventionFromModOpt(module, pSig, cbSig, &callConvMaybe, &errorResID);
+            if (FAILED(hr))
+                COMPlusThrowHR(hr, errorResID);
+
+            if (hr == S_OK)
             {
                 sigRet->callConv = (CorInfoCallConv)callConvMaybe;
             }
index 61bbbb8..ee42a4e 100644 (file)
@@ -5267,7 +5267,8 @@ MetaSig::TryGetUnmanagedCallingConventionFromModOpt(
     _In_ Module *pModule,
     _In_ PCCOR_SIGNATURE pSig,
     _In_ ULONG cSig,
-    _Out_ CorUnmanagedCallingConvention *callConvOut)
+    _Out_ CorUnmanagedCallingConvention *callConvOut,
+    _Out_ UINT *errorResID)
 {
     CONTRACTL
     {
@@ -5276,6 +5277,7 @@ MetaSig::TryGetUnmanagedCallingConventionFromModOpt(
         FORBID_FAULT;
         MODE_ANY;
         PRECONDITION(callConvOut != NULL);
+        PRECONDITION(errorResID != NULL);
     }
     CONTRACTL_END
 
@@ -5285,13 +5287,17 @@ MetaSig::TryGetUnmanagedCallingConventionFromModOpt(
     _ASSERTE(pWalk <= pSig + cSig);
 
     *callConvOut = (CorUnmanagedCallingConvention)0;
+    bool found = false;
     while ((pWalk < (pSig + cSig)) && ((*pWalk == ELEMENT_TYPE_CMOD_OPT) || (*pWalk == ELEMENT_TYPE_CMOD_REQD)))
     {
         BOOL fIsOptional = (*pWalk == ELEMENT_TYPE_CMOD_OPT);
 
         pWalk++;
         if (pWalk + CorSigUncompressedDataSize(pWalk) > pSig + cSig)
-            return E_FAIL; // Bad formatting
+        {
+            *errorResID = BFA_BAD_SIGNATURE;
+            return COR_E_BADIMAGEFORMAT; // Bad formatting
+        }
 
         mdToken tk;
         pWalk += CorSigUncompressToken(pWalk, &tk);
@@ -5320,16 +5326,23 @@ MetaSig::TryGetUnmanagedCallingConventionFromModOpt(
 
         for (const auto &callConv : knownCallConvs)
         {
-            // Take the first recognized calling convention in metadata.
+            // Look for a recognized calling convention in metadata.
             if (::strcmp(typeName, callConv.name) == 0)
             {
+                // Error if there are multiple recognized calling conventions
+                if (found)
+                {
+                    *errorResID = IDS_EE_MULTIPLE_CALLCONV_UNSUPPORTED;
+                    return COR_E_INVALIDPROGRAM;
+                }
+
                 *callConvOut = callConv.value;
-                return S_OK;
+                found = true;
             }
         }
     }
 
-    return S_FALSE;
+    return found ? S_OK : S_FALSE;
 }
 
 //---------------------------------------------------------------------------------------
index 64c9341..7f3fa8b 100644 (file)
@@ -781,20 +781,20 @@ class MetaSig
 
         //----------------------------------------------------------
         // Gets the unmanaged calling convention by reading any modopts.
-        // If there are multiple modopts specifying recognized calling
-        // conventions, the first one that is found in the metadata wins.
-        // Note: the order in the metadata is the reverse of that in IL.
         //
         // Returns:
-        //   E_FAIL - Signature had an invalid format
         //   S_OK - Calling convention was read from modopt
         //   S_FALSE - Calling convention was not read from modopt
+        //   COR_E_BADIMAGEFORMAT - Signature had an invalid format
+        //   COR_E_INVALIDPROGRAM - Program is considered invalid (more
+        //                          than one calling convention specified)
         //----------------------------------------------------------
         static HRESULT TryGetUnmanagedCallingConventionFromModOpt(
             _In_ Module *pModule,
             _In_ PCCOR_SIGNATURE pSig,
             _In_ ULONG cSig,
-            _Out_ CorUnmanagedCallingConvention *callConvOut);
+            _Out_ CorUnmanagedCallingConvention *callConvOut,
+            _Out_ UINT *errorResID);
 
         static CorUnmanagedCallingConvention GetDefaultUnmanagedCallingConvention()
         {
index 935e401..fe42b69 100644 (file)
@@ -89,8 +89,10 @@ unsafe class Program
         {
             // Multiple modopts with calling conventions
             Console.WriteLine($" -- unmanaged modopt(stdcall) modopt(cdecl)");
-            int b = CallFunctionPointers.CallUnmanagedIntInt_ModOptStdcall_ModOptCdecl(cbCdecl, a);
-            Assert.AreEqual(expected, b);
+            var ex = Assert.Throws<InvalidProgramException>(
+                () => CallFunctionPointers.CallUnmanagedIntInt_ModOptStdcall_ModOptCdecl(cbCdecl, a),
+                "Multiple modopts with calling conventions should fail");
+            Assert.AreEqual("Multiple unmanaged calling conventions are specified. Only a single calling convention is supported.", ex.Message);
         }
 
         {
@@ -165,8 +167,10 @@ unsafe class Program
         {
             // Multiple modopts with calling conventions
             Console.WriteLine($" -- unmanaged modopt(stdcall) modopt(cdecl)");
-            var b = CallFunctionPointers.CallUnmanagedCharChar_ModOptStdcall_ModOptCdecl(cbCdecl, a);
-            Assert.AreEqual(expected, b);
+            var ex = Assert.Throws<InvalidProgramException>(
+                () => CallFunctionPointers.CallUnmanagedCharChar_ModOptStdcall_ModOptCdecl(cbCdecl, a),
+                "Multiple modopts with calling conventions should fail");
+            Assert.AreEqual("Multiple unmanaged calling conventions are specified. Only a single calling convention is supported.", ex.Message);
         }
 
         {