Make SuppressGCTransition tests explicitly check GC mode on checked/d…

…ebug coreclr (dotnet#52713)

* Add hook for explicitly checking GC mode for testing (Checked/Debug only)

* Make SuppressGCTransition tests explicitly check GC mode when possible

* Move SuppressGCTransition tests out of PInvoke/Attributes subdirectories

* Update paths in issues.targets

src/coreclr/System.Private.CoreLib/CreateRuntimeRootILLinkDescriptorFile.targets

@@ -16,6 +16,8 @@
   <ItemGroup>
     <_ILLinkDescriptorsFilePaths Include="$(ILLinkDirectory)ILLink.Descriptors.Shared.xml" />
     <_ILLinkDescriptorsFilePaths Condition="'$(TargetsWindows)' == 'true'" Include="$(ILLinkDirectory)ILLink.Descriptors.Windows.xml" />
+    <_ILLinkDescriptorsFilePaths Include="$(ILLinkDirectory)ILLink.Descriptors.Debug.xml"
+                                 Condition="'$(Configuration)' == 'Debug' or '$(Configuration)' == 'Checked'" />
     <_ILLinkDescriptorsFilePaths Include="$(CoreLibSharedDir)ILLink\ILLink.Descriptors.Shared.xml" />
     <_ILLinkDescriptorsFilePaths Include="$(CoreLibSharedDir)ILLink\ILLink.Descriptors.EventSource.xml" />
   </ItemGroup>

src/coreclr/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.Debug.xml

@@ -0,0 +1,8 @@
+<linker>
+  <assembly fullname="System.Private.CoreLib">
+    <type fullname="System.Runtime.InteropServices.Marshal">
+      <!-- Used for testing suppress GC transition -->
+      <method name="GetIsInCooperativeGCModeFunctionPointer" />
+    </type>
+  </assembly>
+</linker>

src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs

@@ -805,5 +805,10 @@ public static object BindToMoniker(string monikerName)
 
         [MethodImpl(MethodImplOptions.InternalCall)]
         internal static extern IntPtr GetFunctionPointerForDelegateInternal(Delegate d);
+
+#if DEBUG // Used for testing in Checked or Debug
+        [DllImport(RuntimeHelpers.QCall)]
+        internal static unsafe extern delegate* unmanaged<int> GetIsInCooperativeGCModeFunctionPointer();
+#endif
     }
 }

src/coreclr/vm/dllimport.cpp

@@ -2648,7 +2648,6 @@ void PInvokeStaticSigInfo::PreInit(MethodDesc* pMD)
     CONTRACTL_END;
 
     PreInit(pMD->GetModule(), pMD->GetMethodTable());
-    SetIsStatic (pMD->IsStatic());
     m_sig = pMD->GetSignature();
     if (pMD->IsEEImpl())
     {
@@ -2753,7 +2752,6 @@ PInvokeStaticSigInfo::PInvokeStaticSigInfo(
 
     PreInit(pModule, NULL);
     m_sig = sig;
-    SetIsStatic(!(MetaSig::GetCallingConvention(sig) & IMAGE_CEE_CS_CALLCONV_HASTHIS));
     InitCallConv(CallConvWinApiSentinel, FALSE);
 }
 
@@ -5879,8 +5877,8 @@ EXTERN_C LPVOID STDCALL NDirectImportWorker(NDirectMethodDesc* pMD)
         //
         INDEBUG(Thread *pThread = GetThread());
         {
-            _ASSERTE(pMD->ShouldSuppressGCTransition()
-                || pThread->GetFrame()->GetVTablePtr() == InlinedCallFrame::GetMethodFrameVPtr());
+            _ASSERTE(pThread->GetFrame()->GetVTablePtr() == InlinedCallFrame::GetMethodFrameVPtr()
+                || pMD->ShouldSuppressGCTransition());
 
             CONSISTENCY_CHECK(pMD->IsNDirect());
             //

src/coreclr/vm/dllimport.h

@@ -338,7 +338,7 @@ struct PInvokeStaticSigInfo
     #define COR_NATIVE_LINK_TYPE_SHIFT 3 // Keep in synch with above mask
     #define COR_NATIVE_LINK_FLAGS_SHIFT 6  // Keep in synch with above mask
 
-public: // getters
+public: // public getters
     DWORD GetStubFlags() const
     {
         WRAPPER_NO_CONTRACT;
@@ -347,24 +347,17 @@ struct PInvokeStaticSigInfo
                (IsDelegateInterop() ? NDIRECTSTUB_FL_DELEGATE : 0);
     }
     Module* GetModule() const { LIMITED_METHOD_CONTRACT; return m_pModule; }
-    BOOL IsStatic() const { LIMITED_METHOD_CONTRACT; return m_wFlags & PINVOKE_STATIC_SIGINFO_IS_STATIC; }
-    BOOL GetThrowOnUnmappableChar() const { LIMITED_METHOD_CONTRACT; return m_wFlags & PINVOKE_STATIC_SIGINFO_THROW_ON_UNMAPPABLE_CHAR; }
-    BOOL GetBestFitMapping() const { LIMITED_METHOD_CONTRACT; return m_wFlags & PINVOKE_STATIC_SIGINFO_BEST_FIT; }
     BOOL IsDelegateInterop() const { LIMITED_METHOD_CONTRACT; return m_wFlags & PINVOKE_STATIC_SIGINFO_IS_DELEGATE_INTEROP; }
     CorInfoCallConvExtension GetCallConv() const { LIMITED_METHOD_CONTRACT; return m_callConv; }
     Signature GetSignature() const { LIMITED_METHOD_CONTRACT; return m_sig; }
     CorNativeLinkType GetCharSet() const { LIMITED_METHOD_CONTRACT; return (CorNativeLinkType)((m_wFlags & COR_NATIVE_LINK_TYPE_MASK) >> COR_NATIVE_LINK_TYPE_SHIFT); }
     CorNativeLinkFlags GetLinkFlags() const { LIMITED_METHOD_CONTRACT; return (CorNativeLinkFlags)((m_wFlags & COR_NATIVE_LINK_FLAGS_MASK) >> COR_NATIVE_LINK_FLAGS_SHIFT); }
 
+public: // private getters
+    BOOL GetThrowOnUnmappableChar() const { LIMITED_METHOD_CONTRACT; return m_wFlags & PINVOKE_STATIC_SIGINFO_THROW_ON_UNMAPPABLE_CHAR; }
+    BOOL GetBestFitMapping() const { LIMITED_METHOD_CONTRACT; return m_wFlags & PINVOKE_STATIC_SIGINFO_BEST_FIT; }
+
 private: // setters
-    void SetIsStatic(BOOL isStatic)
-    {
-        LIMITED_METHOD_CONTRACT;
-        if (isStatic)
-            m_wFlags |= PINVOKE_STATIC_SIGINFO_IS_STATIC;
-        else
-            m_wFlags &= ~PINVOKE_STATIC_SIGINFO_IS_STATIC;
-    }
     void SetThrowOnUnmappableChar(BOOL throwOnUnmappableChar)
     {
         LIMITED_METHOD_CONTRACT;

src/coreclr/vm/ecalllist.h

@@ -763,6 +763,10 @@ FCFuncStart(gInteropMarshalFuncs)
     FCFuncElement("GetDelegateForFunctionPointerInternal", MarshalNative::GetDelegateForFunctionPointerInternal)
     FCFuncElement("GetFunctionPointerForDelegateInternal", MarshalNative::GetFunctionPointerForDelegateInternal)
 
+#ifdef _DEBUG
+    QCFuncElement("GetIsInCooperativeGCModeFunctionPointer", MarshalNative::GetIsInCooperativeGCModeFunctionPointer)
+#endif // _DEBUG
+
 #ifdef FEATURE_COMINTEROP
     FCFuncElement("GetHRForException", MarshalNative::GetHRForException)
     FCFuncElement("IsComObject", MarshalNative::IsComObject)

src/coreclr/vm/marshalnative.cpp

@@ -425,6 +425,31 @@ FCIMPL1(LPVOID, MarshalNative::GetFunctionPointerForDelegateInternal, Object* re
 }
 FCIMPLEND
 
+#ifdef _DEBUG
+namespace
+{
+    BOOL STDMETHODCALLTYPE IsInCooperativeGCMode()
+    {
+        return GetThread()->PreemptiveGCDisabled();
+    }
+}
+
+MarshalNative::IsInCooperativeGCMode_fn QCALLTYPE MarshalNative::GetIsInCooperativeGCModeFunctionPointer()
+{
+    QCALL_CONTRACT;
+
+    MarshalNative::IsInCooperativeGCMode_fn ret = NULL;
+
+    BEGIN_QCALL;
+
+    ret = IsInCooperativeGCMode;
+
+    END_QCALL;
+
+    return ret;
+}
+#endif
+
 /************************************************************************
  * Marshal.GetLastPInvokeError
  */

src/coreclr/vm/marshalnative.h

@@ -49,6 +49,11 @@ class MarshalNative
     static FCDECL2(Object*, GetDelegateForFunctionPointerInternal, LPVOID FPtr, ReflectClassBaseObject* refTypeUNSAFE);
     static FCDECL1(LPVOID, GetFunctionPointerForDelegateInternal, Object* refDelegateUNSAFE);
 
+#ifdef _DEBUG
+    using IsInCooperativeGCMode_fn = BOOL(STDMETHODCALLTYPE*)(void);
+    static IsInCooperativeGCMode_fn QCALLTYPE GetIsInCooperativeGCModeFunctionPointer();
+#endif
+
 #ifdef FEATURE_COMINTEROP
     //====================================================================
     // return the IUnknown* for an Object

src/tests/Interop/CMakeLists.txt

@@ -12,7 +12,6 @@ list(APPEND LINK_LIBRARIES_ADDITIONAL platformdefines)
 SET(CLR_INTEROP_TEST_ROOT ${CMAKE_CURRENT_SOURCE_DIR})
 
 include_directories(common)
-add_subdirectory(PInvoke/Attributes/SuppressGCTransition)
 add_subdirectory(PInvoke/Decimal)
 add_subdirectory(PInvoke/ArrayWithOffset)
 add_subdirectory(PInvoke/BestFitMapping/Char)
@@ -46,6 +45,7 @@ add_subdirectory(SimpleStruct)
 add_subdirectory(StructMarshalling/PInvoke)
 add_subdirectory(StructMarshalling/ReversePInvoke/MarshalExpStruct)
 add_subdirectory(StructMarshalling/ReversePInvoke/MarshalSeqStruct)
+add_subdirectory(SuppressGCTransition)
 add_subdirectory(BestFitMapping)
 add_subdirectory(RefInt)
 add_subdirectory(RefCharArray)

src/tests/Interop/PInvoke/Attributes/SuppressGCTransition/SuppressGCTransitionNative.cpp → src/tests/Interop/SuppressGCTransition/SuppressGCTransitionNative.cpp

@@ -10,16 +10,35 @@
 namespace
 {
     std::atomic<uint32_t> _n{ 0 };
+
+    using IsInCooperativeMode_fn = BOOL(STDMETHODCALLTYPE*)(void);
+    IsInCooperativeMode_fn s_isInCooperativeMode = nullptr;
+}
+
+extern "C"
+void DLL_EXPORT STDMETHODVCALLTYPE SetIsInCooperativeModeFunction(IsInCooperativeMode_fn fn)
+{
+    s_isInCooperativeMode = fn;
 }
 
 extern "C"
-BOOL DLL_EXPORT STDMETHODVCALLTYPE NextUInt(/* out */ uint32_t *n)
+BOOL DLL_EXPORT STDMETHODVCALLTYPE NextUInt(/* out */ uint32_t* n)
 {
+    BOOL ret;
     if (n == nullptr)
-        return FALSE;
+    {
+        ret = FALSE;
+    }
+    else
+    {
+        *n = (++_n);
+        ret = TRUE;
+    }
 
-    *n = (++_n);
-    return TRUE;
+    if (s_isInCooperativeMode != nullptr)
+        ret = s_isInCooperativeMode();
+
+    return ret;
 }
 
 typedef int (STDMETHODVCALLTYPE *CALLBACKPROC)(int n);

src/tests/Interop/PInvoke/Attributes/SuppressGCTransition/SuppressGCTransitionTest.cs → src/tests/Interop/SuppressGCTransition/SuppressGCTransitionTest.cs

@@ -10,6 +10,9 @@
 
 unsafe static class SuppressGCTransitionNative
 {
+    [DllImport(nameof(SuppressGCTransitionNative), CallingConvention=CallingConvention.Cdecl)]
+    public static extern unsafe void SetIsInCooperativeModeFunction(delegate* unmanaged<int> fn);
+
     [DllImport(nameof(SuppressGCTransitionNative), CallingConvention=CallingConvention.Cdecl, EntryPoint = "NextUInt")]
     [SuppressGCTransition]
     public static extern unsafe int NextUInt_Inline_NoGCTransition(int* n);
@@ -106,55 +109,95 @@ private static IntPtr GetNativeLibrary()
 
 unsafe class SuppressGCTransitionTest
 {
+    private static bool ExplicitModeCheckEnabled = false;
+    private static void InitializeExplicitModeCheck()
+    {
+        // GetIsInCooperativeGCModeFunctionPointer is conditionally included based on the runtime build configuration,
+        // so we check for its existence and only do the explicit mode validation if it is available.
+        Type marshalType = typeof(object).Assembly.GetType(typeof(System.Runtime.InteropServices.Marshal).FullName);
+        MethodInfo getFunctionPtr = marshalType.GetMethod("GetIsInCooperativeGCModeFunctionPointer", BindingFlags.NonPublic | BindingFlags.Static);
+        if (getFunctionPtr != null)
+        {
+            var isInCooperativeModeFunc = (delegate* unmanaged<int>)(IntPtr)getFunctionPtr.Invoke(null, null);
+            SuppressGCTransitionNative.SetIsInCooperativeModeFunction(isInCooperativeModeFunc);
+            ExplicitModeCheckEnabled = true;
+            Console.WriteLine("Explicit GC mode check is enabled");
+        }
+    }
+
+    private static void ValidateMode(bool transitionSuppressed, int inCooperativeMode)
+        => ValidateMode(transitionSuppressed, inCooperativeMode != 0);
+
+    private static void ValidateMode(bool transitionSuppressed, bool inCooperativeMode)
+    {
+        if (!ExplicitModeCheckEnabled)
+            return;
+
+        Assert.AreEqual(transitionSuppressed, inCooperativeMode, $"GC transition should{(transitionSuppressed ? "" : " not")} have been suppressed");
+    }
+
     [MethodImpl(MethodImplOptions.NoInlining)]
     private static int Inline_NoGCTransition(int expected)
     {
         Console.WriteLine($"{nameof(Inline_NoGCTransition)} ({expected}) ...");
         int n;
-        SuppressGCTransitionNative.NextUInt_Inline_NoGCTransition(&n);
+        int ret = SuppressGCTransitionNative.NextUInt_Inline_NoGCTransition(&n);
         Assert.AreEqual(expected, n);
+        ValidateMode(transitionSuppressed: true, ret);
         return n + 1;
     }
     [MethodImpl(MethodImplOptions.NoInlining)]
     private static int Inline_GCTransition(int expected)
     {
         Console.WriteLine($"{nameof(Inline_GCTransition)} ({expected}) ...");
         int n;
-        SuppressGCTransitionNative.NextUInt_Inline_GCTransition(&n);
+        int ret = SuppressGCTransitionNative.NextUInt_Inline_GCTransition(&n);
         Assert.AreEqual(expected, n);
+        ValidateMode(transitionSuppressed: false, ret);
         return n + 1;
     }
     [MethodImpl(MethodImplOptions.NoInlining)]
     private static int NoInline_NoGCTransition(int expected)
     {
         Console.WriteLine($"{nameof(NoInline_NoGCTransition)} ({expected}) ...");
         int n;
-        SuppressGCTransitionNative.NextUInt_NoInline_NoGCTransition(&n);
+        bool ret = SuppressGCTransitionNative.NextUInt_NoInline_NoGCTransition(&n);
         Assert.AreEqual(expected, n);
+        ValidateMode(transitionSuppressed: true, ret);
         return n + 1;
     }
     [MethodImpl(MethodImplOptions.NoInlining)]
     private static int NoInline_GCTransition(int expected)
     {
         Console.WriteLine($"{nameof(NoInline_GCTransition)} ({expected}) ...");
         int n;
-        SuppressGCTransitionNative.NextUInt_NoInline_GCTransition(&n);
+        bool ret = SuppressGCTransitionNative.NextUInt_NoInline_GCTransition(&n);
         Assert.AreEqual(expected, n);
+        ValidateMode(transitionSuppressed: false, ret);
         return n + 1;
     }
     [MethodImpl(MethodImplOptions.NoInlining)]
     private static int Mixed(int expected)
     {
         Console.WriteLine($"{nameof(Mixed)} ({expected}) ...");
         int n;
-        SuppressGCTransitionNative.NextUInt_NoInline_GCTransition(&n);
-        Assert.AreEqual(expected++, n);
-        SuppressGCTransitionNative.NextUInt_NoInline_NoGCTransition(&n);
-        Assert.AreEqual(expected++, n);
-        SuppressGCTransitionNative.NextUInt_Inline_GCTransition(&n);
-        Assert.AreEqual(expected++, n);
-        SuppressGCTransitionNative.NextUInt_Inline_NoGCTransition(&n);
-        Assert.AreEqual(expected++, n);
+
+        {
+            bool ret = SuppressGCTransitionNative.NextUInt_NoInline_GCTransition(&n);
+            Assert.AreEqual(expected++, n);
+            ValidateMode(transitionSuppressed: false, ret);
+            ret = SuppressGCTransitionNative.NextUInt_NoInline_NoGCTransition(&n);
+            Assert.AreEqual(expected++, n);
+            ValidateMode(transitionSuppressed: true, ret);
+        }
+        {
+            int ret = SuppressGCTransitionNative.NextUInt_Inline_GCTransition(&n);
+            Assert.AreEqual(expected++, n);
+            ValidateMode(transitionSuppressed: false, ret);
+            ret = SuppressGCTransitionNative.NextUInt_Inline_NoGCTransition(&n);
+            Assert.AreEqual(expected++, n);
+            ValidateMode(transitionSuppressed: true, ret);
+        }
         return n + 1;
     }
     [MethodImpl(MethodImplOptions.NoInlining)]
@@ -179,35 +222,39 @@ private static int Inline_NoGCTransition_FunctionPointer(int expected)
     {
         Console.WriteLine($"{nameof(Inline_NoGCTransition)} ({expected}) ...");
         int n;
-        SuppressGCTransitionNative.GetNextUIntFunctionPointer_Inline_NoGCTransition()(&n);
+        int ret = SuppressGCTransitionNative.GetNextUIntFunctionPointer_Inline_NoGCTransition()(&n);
         Assert.AreEqual(expected, n);
+        ValidateMode(transitionSuppressed: true, ret);
         return n + 1;
     }
     [MethodImpl(MethodImplOptions.NoInlining)]
     private static int Inline_GCTransition_FunctionPointer(int expected)
     {
         Console.WriteLine($"{nameof(Inline_GCTransition)} ({expected}) ...");
         int n;
-        SuppressGCTransitionNative.GetNextUIntFunctionPointer_Inline_GCTransition()(&n);
+        int ret = SuppressGCTransitionNative.GetNextUIntFunctionPointer_Inline_GCTransition()(&n);
         Assert.AreEqual(expected, n);
+        ValidateMode(transitionSuppressed: false, ret);
         return n + 1;
     }
     [MethodImpl(MethodImplOptions.NoInlining)]
     private static int NoInline_NoGCTransition_FunctionPointer(int expected)
     {
         Console.WriteLine($"{nameof(NoInline_NoGCTransition)} ({expected}) ...");
         int n;
-        SuppressGCTransitionNative.GetNextUIntFunctionPointer_NoInline_NoGCTransition()(&n);
+        bool ret = SuppressGCTransitionNative.GetNextUIntFunctionPointer_NoInline_NoGCTransition()(&n);
         Assert.AreEqual(expected, n);
+        ValidateMode(transitionSuppressed: true, ret);
         return n + 1;
     }
     [MethodImpl(MethodImplOptions.NoInlining)]
     private static int NoInline_GCTransition_FunctionPointer(int expected)
     {
         Console.WriteLine($"{nameof(NoInline_GCTransition)} ({expected}) ...");
         int n;
-        SuppressGCTransitionNative.GetNextUIntFunctionPointer_NoInline_GCTransition()(&n);
+        bool ret = SuppressGCTransitionNative.GetNextUIntFunctionPointer_NoInline_GCTransition()(&n);
         Assert.AreEqual(expected, n);
+        ValidateMode(transitionSuppressed: false, ret);
         return n + 1;
     }
 
@@ -223,8 +270,9 @@ private static int CallAsFunctionPointer(int expected)
         object boxedN = Pointer.Box(pn, typeof(int*));
 
         MethodInfo callNextUInt = typeof(FunctionPointer).GetMethod("Call_NextUInt");
-        callNextUInt.Invoke(null, new object[] { fptr, boxedN });
+        int ret = (int)callNextUInt.Invoke(null, new object[] { fptr, boxedN });
         Assert.AreEqual(expected, n);
+        ValidateMode(transitionSuppressed: false, ret);
         return n + 1;
     }
     [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvCdecl) })]
@@ -303,6 +351,8 @@ public static int Main(string[] args)
     {
         try
         {
+            InitializeExplicitModeCheck();
+
             int n = 1;
             n = Inline_NoGCTransition(n);
             n = Inline_GCTransition(n);