Objective-C msgSend* support for pending exceptions in Release

src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs

@@ -797,5 +797,23 @@ private static MarshallerKind GetArrayElementMarshallerKind(
                 return MarshallerKind.Invalid;
             }
         }
+
+        internal static bool ShouldCheckForPendingException(TargetDetails target, PInvokeMetadata metadata)
+        {
+            if (!target.IsOSX)
+                return false;
+
+            const string ObjectiveCLibrary = "/usr/lib/libobjc.dylib";
+            const string ObjectiveCMsgSend = "objc_msgSend";
+
+            // This is for the objc_msgSend suite of functions.
+            //   objc_msgSend
+            //   objc_msgSend_fpret
+            //   objc_msgSend_stret
+            //   objc_msgSendSuper
+            //   objc_msgSendSuper_stret
+            return metadata.Module.Equals(ObjectiveCLibrary)
+                && metadata.Name.StartsWith(ObjectiveCMsgSend);
+        }
     }
 }

src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/Stubs/PInvokeILEmitter.cs

@@ -43,7 +43,7 @@ private void EmitPInvokeCall(PInvokeILCodeStreams ilCodeStreams)
 
             MetadataType stubHelpersType = InteropTypes.GetStubHelpers(context);
 
-            // if the SetLastError flag is set in DllImport, clear the error code before doing P/Invoke 
+            // if the SetLastError flag is set in DllImport, clear the error code before doing P/Invoke
             if (_importMetadata.Flags.SetLastError)
             {
                 callsiteSetupCodeStream.Emit(ILOpcode.call, emitter.NewToken(
@@ -78,6 +78,9 @@ private MethodIL EmitIL()
             if (!_importMetadata.Flags.PreserveSig)
                 throw new NotSupportedException();
 
+            if (MarshalHelpers.ShouldCheckForPendingException(_targetMethod.Context.Target, _importMetadata))
+                throw new NotSupportedException();
+
             if (_targetMethod.IsUnmanagedCallersOnly)
                 throw new NotSupportedException();
 

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Interop/IL/Marshaller.ReadyToRun.cs

@@ -93,14 +93,18 @@ public static bool IsMarshallingRequired(MethodDesc targetMethod)
             if (targetMethod.IsUnmanagedCallersOnly)
                 return true;
 
-            PInvokeFlags flags = targetMethod.GetPInvokeMethodMetadata().Flags;
+            PInvokeMetadata metadata = targetMethod.GetPInvokeMethodMetadata();
+            PInvokeFlags flags = metadata.Flags;
 
             if (flags.SetLastError)
                 return true;
 
             if (!flags.PreserveSig)
                 return true;
 
+            if (MarshalHelpers.ShouldCheckForPendingException(targetMethod.Context.Target, metadata))
+                return true;
+
             var marshallers = GetMarshallersForMethod(targetMethod);
             for (int i = 0; i < marshallers.Length; i++)
             {

src/coreclr/vm/corelib.h

@@ -997,7 +997,7 @@ DEFINE_METHOD(STUBHELPERS,          ADD_TO_CLEANUP_LIST_SAFEHANDLE,    AddToClea
 DEFINE_METHOD(STUBHELPERS,          KEEP_ALIVE_VIA_CLEANUP_LIST,    KeepAliveViaCleanupList,       SM_RefCleanupWorkListElement_Obj_RetVoid)
 DEFINE_METHOD(STUBHELPERS,          DESTROY_CLEANUP_LIST,   DestroyCleanupList,         SM_RefCleanupWorkListElement_RetVoid)
 DEFINE_METHOD(STUBHELPERS,          GET_HR_EXCEPTION_OBJECT, GetHRExceptionObject,      SM_Int_RetException)
-DEFINE_METHOD(STUBHELPERS,          GET_PENDING_EXCEPTION_OBJECT, GetPendingExceptionObject,      SM_RetException)
+DEFINE_METHOD(STUBHELPERS,          GET_PENDING_EXCEPTION_OBJECT,   GetPendingExceptionObject,      SM_RetException)
 DEFINE_METHOD(STUBHELPERS,          CREATE_CUSTOM_MARSHALER_HELPER, CreateCustomMarshalerHelper, SM_IntPtr_Int_IntPtr_RetIntPtr)
 
 DEFINE_METHOD(STUBHELPERS,          CHECK_STRING_LENGTH,    CheckStringLength,          SM_Int_RetVoid)

src/coreclr/vm/dllimport.cpp

@@ -2657,7 +2657,9 @@ void PInvokeStaticSigInfo::PreInit(MethodDesc* pMD)
 }
 
 PInvokeStaticSigInfo::PInvokeStaticSigInfo(
-    _In_ MethodDesc* pMD, _Outptr_opt_ LPCUTF8 *pLibName, _Outptr_opt_ LPCUTF8 *pEntryPointName)
+    _In_ MethodDesc* pMD,
+    _Outptr_opt_ LPCUTF8* pLibName,
+    _Outptr_opt_ LPCUTF8* pEntryPointName)
 {
     CONTRACTL
     {
@@ -2755,7 +2757,10 @@ PInvokeStaticSigInfo::PInvokeStaticSigInfo(
     InitCallConv(CallConvWinApiSentinel, FALSE);
 }
 
-void PInvokeStaticSigInfo::DllImportInit(_In_ MethodDesc* pMD, _Outptr_opt_ LPCUTF8 *ppLibName, _Outptr_opt_ LPCUTF8 *ppEntryPointName)
+void PInvokeStaticSigInfo::DllImportInit(
+    _In_ MethodDesc* pMD,
+    _Outptr_opt_ LPCUTF8* ppLibName,
+    _Outptr_opt_ LPCUTF8* ppEntryPointName)
 {
     CONTRACTL
     {
@@ -2784,7 +2789,6 @@ void PInvokeStaticSigInfo::DllImportInit(_In_ MethodDesc* pMD, _Outptr_opt_ LPCU
         return;
     }
 
-    // out parameter pEntryPointName
     if (ppEntryPointName && *ppEntryPointName == NULL)
         *ppEntryPointName = pMD->GetName();
 
@@ -3098,6 +3102,11 @@ HRESULT NDirect::HasNAT_LAttribute(IMDInternalImport *pInternalImport, mdToken t
 
 
 // Either MD or signature & module must be given.
+//
+// N.B. This method can be called at a time when the associated NDirectMethodDesc
+// has not been fully populated. This means the optimized path for this call is to rely
+// on the most basic P/Invoke metadata. An example when this can happen is when the JIT
+// is compiling a method containing a P/Invoke that is being considered for inlining.
 /*static*/
 BOOL NDirect::MarshalingRequired(
     _In_opt_ MethodDesc* pMD,
@@ -3128,16 +3137,12 @@ BOOL NDirect::MarshalingRequired(
                 return TRUE;
         }
 
-        // SetLastError is handled by stub
-        PInvokeStaticSigInfo sigInfo(pMD);
-        if (sigInfo.GetLinkFlags() & nlfLastError)
-            return TRUE;
-
-        // LCID argument is handled by stub
-        if (GetLCIDParameterIndex(pMD) != -1)
-            return TRUE;
-
-        if (pMD->IsNDirect())
+        PInvokeStaticSigInfo sigInfo;
+        if (!pMD->IsNDirect())
+        {
+            new (&sigInfo) PInvokeStaticSigInfo(pMD);
+        }
+        else
         {
             // A P/Invoke marked with UnmanagedCallersOnlyAttribute
             // doesn't technically require marshalling. However, we
@@ -3154,13 +3159,23 @@ BOOL NDirect::MarshalingRequired(
             if (pNMD->IsClassConstructorTriggeredByILStub())
                 return TRUE;
 
+            InitializeSigInfoAndPopulateNDirectMethodDesc(pNMD, &sigInfo);
+
 #ifndef CROSSGEN_COMPILE
             // Pending exceptions are handled by stub
             if (Interop::ShouldCheckForPendingException(pNMD))
                 return TRUE;
 #endif // !CROSSGEN_COMPILE
         }
 
+        // SetLastError is handled by stub
+        if (sigInfo.GetLinkFlags() & nlfLastError)
+            return TRUE;
+
+        // LCID argument is handled by stub
+        if (GetLCIDParameterIndex(pMD) != -1)
+            return TRUE;
+
         callConv = sigInfo.GetCallConv();
     }
 
@@ -4225,7 +4240,11 @@ static void CreateNDirectStubAccessMetadata(
 
 namespace
 {
-    void PopulateNDirectMethodDescImpl(_Inout_ NDirectMethodDesc* pNMD, _In_ const PInvokeStaticSigInfo& sigInfo, _In_opt_z_ LPCUTF8 libName, _In_opt_z_ LPCUTF8 entryPointName)
+    void PopulateNDirectMethodDescImpl(
+        _Inout_ NDirectMethodDesc* pNMD,
+        _In_ const PInvokeStaticSigInfo& sigInfo,
+        _In_opt_z_ LPCUTF8 libName,
+        _In_opt_z_ LPCUTF8 entryPointName)
     {
         CONTRACTL
         {
@@ -4263,9 +4282,8 @@ namespace
             pNMD->ndirect.m_pszEntrypointName.SetValueMaybeNull(entryPointName);
         }
 
-        // Call this exactly ONCE per thread. Do not publish incomplete prestub flags
-        // or you will introduce a race condition.
-        pNMD->InterlockedSetNDirectFlags(ndirectflags);
+        // Do not publish incomplete prestub flags or you will introduce a race condition.
+        pNMD->InterlockedSetNDirectFlags(ndirectflags | NDirectMethodDesc::kNDirectPopulated);
     }
 }
 
@@ -4281,6 +4299,9 @@ void NDirect::PopulateNDirectMethodDesc(_Inout_ NDirectMethodDesc* pNMD)
     if (pNMD->IsSynchronized())
         COMPlusThrow(kTypeLoadException, IDS_EE_NOSYNCHRONIZED);
 
+    if (pNMD->IsPopulated())
+        return;
+
     LPCUTF8 szLibName = NULL, szEntryPointName = NULL;
     PInvokeStaticSigInfo sigInfo(pNMD, &szLibName, &szEntryPointName);
     PopulateNDirectMethodDescImpl(pNMD, sigInfo, szLibName, szEntryPointName);
@@ -4302,6 +4323,9 @@ void NDirect::InitializeSigInfoAndPopulateNDirectMethodDesc(_Inout_ NDirectMetho
     LPCUTF8 szLibName = NULL, szEntryPointName = NULL;
     new (pSigInfo) PInvokeStaticSigInfo(pNMD, &szLibName, &szEntryPointName);
 
+    if (pNMD->IsPopulated())
+        return;
+
     PopulateNDirectMethodDescImpl(pNMD, *pSigInfo, szLibName, szEntryPointName);
 }
 

src/coreclr/vm/dllimport.h

@@ -312,12 +312,12 @@ struct PInvokeStaticSigInfo
 
     PInvokeStaticSigInfo(_In_ MethodDesc* pMdDelegate);
 
-    PInvokeStaticSigInfo(_In_ MethodDesc* pMD, _Outptr_opt_ LPCUTF8 *pLibName, _Outptr_opt_ LPCUTF8 *pEntryPointName);
+    PInvokeStaticSigInfo(_In_ MethodDesc* pMD, _Outptr_opt_ LPCUTF8* pLibName, _Outptr_opt_ LPCUTF8* pEntryPointName);
 
 private:
     void ThrowError(WORD errorResourceID);
     void InitCallConv(CorInfoCallConvExtension callConv, BOOL bIsVarArg);
-    void DllImportInit(_In_ MethodDesc* pMD, _Outptr_opt_ LPCUTF8 *pLibName, _Outptr_opt_ LPCUTF8 *pEntryPointName);
+    void DllImportInit(_In_ MethodDesc* pMD, _Outptr_opt_ LPCUTF8* pLibName, _Outptr_opt_ LPCUTF8* pEntryPointName);
     void PreInit(Module* pModule, MethodTable *pClass);
     void PreInit(MethodDesc* pMD);
 

src/coreclr/vm/interoplibinterface.h

@@ -147,7 +147,7 @@ class ObjCMarshalNative
     static bool IsTrackedReference(_In_ OBJECTREF object, _Out_ bool* isReferenced);
 
 public: // Identification
-    static bool IsRuntimeMsgSendFunctionOverridden(
+    static bool IsRuntimeMessageSendFunction(
         _In_z_ const char* libraryName,
         _In_z_ const char* entrypointName);
 

src/coreclr/vm/interoplibinterface_objc.cpp

@@ -127,14 +127,16 @@ namespace
         OBJC_MSGSEND "Super_stret",
     };
 
-    const void* STDMETHODCALLTYPE MessageSendPInvokeOverride(_In_z_ const char* libraryName, _In_z_ const char* entrypointName)
+    bool IsObjectiveCMessageSendFunction(_In_z_ const char* libraryName, _In_z_ const char* entrypointName)
     {
-        // All overrides are in libobjc
-        if (strcmp(libraryName, ObjectiveCLibrary) != 0)
-            return nullptr;
+        // Is the function in libobjc and named appropriately.
+        return ((strcmp(libraryName, ObjectiveCLibrary) == 0)
+                && (strncmp(entrypointName, OBJC_MSGSEND, _countof(OBJC_MSGSEND) -1) == 0));
+    }
 
-        // All overrides start with objc_msgSend
-        if (strncmp(entrypointName, OBJC_MSGSEND, _countof(OBJC_MSGSEND) -1) != 0)
+    const void* STDMETHODCALLTYPE MessageSendPInvokeOverride(_In_z_ const char* libraryName, _In_z_ const char* entrypointName)
+    {
+        if (!IsObjectiveCMessageSendFunction(libraryName, entrypointName))
             return nullptr;
 
         for (int i = 0; i < _countof(MsgSendEntryPoints); ++i)
@@ -227,7 +229,7 @@ bool ObjCMarshalNative::IsTrackedReference(_In_ OBJECTREF object, _Out_ bool* is
     return true;
 }
 
-bool ObjCMarshalNative::IsRuntimeMsgSendFunctionOverridden(
+bool ObjCMarshalNative::IsRuntimeMessageSendFunction(
     _In_z_ const char* libraryName,
     _In_z_ const char* entrypointName)
 {
@@ -240,7 +242,7 @@ bool ObjCMarshalNative::IsRuntimeMsgSendFunctionOverridden(
     }
     CONTRACTL_END;
 
-    return MessageSendPInvokeOverride(libraryName, entrypointName) != NULL;
+    return IsObjectiveCMessageSendFunction(libraryName, entrypointName);
 }
 
 namespace

src/coreclr/vm/interoplibinterface_shared.cpp

@@ -3,6 +3,7 @@
 
 // Runtime headers
 #include "common.h"
+#include "dllimport.h"
 
 #include "interoplibinterface.h"
 
@@ -24,7 +25,7 @@ bool Interop::ShouldCheckForPendingException(_In_ NDirectMethodDesc* md)
     if (libraryName == NULL || entrypointName == NULL)
         return false;
 
-    if (ObjCMarshalNative::IsRuntimeMsgSendFunctionOverridden(libraryName, entrypointName))
+    if (ObjCMarshalNative::IsRuntimeMessageSendFunction(libraryName, entrypointName))
         return true;
 #endif // FEATURE_OBJCMARSHAL
 

src/coreclr/vm/method.hpp

@@ -3027,6 +3027,8 @@ class NDirectMethodDesc : public MethodDesc
         kIsQCall                        = 0x1000,
 
         kDefaultDllImportSearchPathsStatus = 0x2000, // either method has custom attribute or not.
+
+        kNDirectPopulated               = 0x8000, // Indicate if the NDirect has been fully populated.
     };
 
     // Resolve the import to the NDirect target and set it on the NDirectMethodDesc.
@@ -3159,6 +3161,12 @@ class NDirectMethodDesc : public MethodDesc
         return (ndirect.m_wFlags & kDefaultDllImportSearchPathsIsCached) != 0;
     }
 
+    BOOL IsPopulated()
+    {
+        LIMITED_METHOD_CONTRACT;
+        return (ndirect.m_wFlags & kNDirectPopulated) != 0;
+    }
+
     ULONG DefaultDllImportSearchPathsAttributeCachedValue()
     {
         LIMITED_METHOD_CONTRACT;

src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/ObjectiveC/PendingExceptionTests.cs

@@ -44,15 +44,6 @@ private static IntPtr SetPendingException([CallerMemberName] string callerName =
             return IntPtr.Zero;
         }
 
-        private static (MessageSendFunction MsgSend, IntPtr Func)[] msgSendOverrides =
-        {
-            (MessageSendFunction.MsgSend,           (IntPtr)(delegate* unmanaged<IntPtr, IntPtr, IntPtr>)&MsgSend),
-            (MessageSendFunction.MsgSendFpret,      (IntPtr)(delegate* unmanaged<IntPtr, IntPtr, IntPtr>)&MsgSendFpret),
-            (MessageSendFunction.MsgSendStret,      (IntPtr)(delegate* unmanaged<IntPtr*, IntPtr, IntPtr, void>)&MsgSendStret),
-            (MessageSendFunction.MsgSendSuper,      (IntPtr)(delegate* unmanaged<IntPtr, IntPtr, IntPtr>)&MsgSendSuper),
-            (MessageSendFunction.MsgSendSuperStret, (IntPtr)(delegate* unmanaged<IntPtr*, IntPtr, IntPtr, void>)&MsgSendSuperStret),
-        };
-
         [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         [InlineData(MessageSendFunction.MsgSend)]
         [InlineData(MessageSendFunction.MsgSendFpret)]
@@ -71,27 +62,27 @@ public void ValidateSetMessageSendPendingException(MessageSendFunction func)
             }, func.ToString()).Dispose();
         }
 
-        private static void ValidateSetMessageSendPendingExceptionImpl(MessageSendFunction funcToOverride)
+        private static void ValidateSetMessageSendPendingExceptionImpl(MessageSendFunction msgSend)
         {
-            foreach (var (msgSend, func) in msgSendOverrides)
+            IntPtr func = msgSend switch
             {
-                if (funcToOverride != msgSend)
-                {
-                    continue;
-                }
-
-                // Override message send function
-                ObjectiveCMarshal.SetMessageSendCallback(msgSend, func);
-
-                // Call message send function through P/Invoke
-                IntPtr inst = IntPtr.Zero;
-                IntPtr sel = IntPtr.Zero;
-
-                Exception ex = Assert.Throws<PendingException>(() => LibObjC.CallPInvoke(msgSend, inst, sel));
-                Assert.Equal(msgSend.ToString(), ex.Message);
-
-                break;
-            }
+                MessageSendFunction.MsgSend => (IntPtr)(delegate* unmanaged<IntPtr, IntPtr, IntPtr>)&MsgSend,
+                MessageSendFunction.MsgSendFpret => (IntPtr)(delegate* unmanaged<IntPtr, IntPtr, IntPtr>)&MsgSendFpret,
+                MessageSendFunction.MsgSendStret => (IntPtr)(delegate* unmanaged<IntPtr*, IntPtr, IntPtr, void>)&MsgSendStret,
+                MessageSendFunction.MsgSendSuper => (IntPtr)(delegate* unmanaged<IntPtr, IntPtr, IntPtr>)&MsgSendSuper,
+                MessageSendFunction.MsgSendSuperStret => (IntPtr)(delegate* unmanaged<IntPtr*, IntPtr, IntPtr, void>)&MsgSendSuperStret,
+                _ => throw new Exception($"Unknown {nameof(MessageSendFunction)}"),
+            };
+
+            // Override message send function
+            ObjectiveCMarshal.SetMessageSendCallback(msgSend, func);
+
+            // Call message send function through P/Invoke
+            IntPtr inst = IntPtr.Zero;
+            IntPtr sel = IntPtr.Zero;
+
+            Exception ex = Assert.Throws<PendingException>(() => LibObjC.CallPInvoke(msgSend, inst, sel));
+            Assert.Equal(msgSend.ToString(), ex.Message);
         }
     }
 }