[release/5.0] Take SuppressGCTransition into account in IL stub caching

src/coreclr/src/vm/dllimport.cpp

@@ -260,7 +260,8 @@ class ILStubState : public StubState
  DWORD dwStubFlags,
  int iLCIDParamIdx,
  MethodDesc* pTargetMD)
- : m_slIL(dwStubFlags, pStubModule, signature, pTypeContext, pTargetMD, iLCIDParamIdx, fTargetHasThis, fStubHasThis)
+ : m_slIL(dwStubFlags, pStubModule, signature, pTypeContext, pTargetMD, iLCIDParamIdx, fTargetHasThis, fStubHasThis)
+ , m_dwStubFlags(dwStubFlags)
  {
  STANDARD_VM_CONTRACT;
 
@@ -292,7 +293,7 @@ class ILStubState : public StubState
  {
  WRAPPER_NO_CONTRACT;
  m_slIL.Begin(dwStubFlags);
- m_dwStubFlags = dwStubFlags;
+ _ASSERTE(m_dwStubFlags == dwStubFlags);
  }
 
  void MarshalReturn(MarshalInfo* pInfo, int argOffset)
@@ -1204,6 +1205,8 @@ class ILStubState : public StubState
 
  TokenLookupMap* GetTokenLookupMap() { WRAPPER_NO_CONTRACT; return m_slIL.GetTokenLookupMap(); }
 
+ DWORD GetFlags() const { return m_dwStubFlags; }
+
 protected:
  CQuickBytes m_qbNativeFnSigBuffer;
  NDirectStubLinker m_slIL;
@@ -1350,7 +1353,7 @@ class PInvoke_ILStubState : public ILStubState
  pTypeContext,
  TargetHasThis(dwStubFlags),
  StubHasThis(dwStubFlags),
- dwStubFlags,
+ UpdateStubFlags(dwStubFlags, pTargetMD),
  iLCIDParamIdx,
  pTargetMD)
  {
@@ -1363,6 +1366,15 @@ class PInvoke_ILStubState : public ILStubState
  }
 
 private:
+ static DWORD UpdateStubFlags(DWORD dwStubFlags, MethodDesc* pTargetMD)
+ {
+ if (TargetSuppressGCTransition(dwStubFlags, pTargetMD))
+ {
+ dwStubFlags |= NDIRECTSTUB_FL_SUPPRESSGCTRANSITION;
+ }
+ return dwStubFlags;
+ }
+
  static BOOL TargetHasThis(DWORD dwStubFlags)
  {
  //
@@ -1381,6 +1393,11 @@ class PInvoke_ILStubState : public ILStubState
  //
  return SF_IsForwardDelegateStub(dwStubFlags);
  }
+
+ static BOOL TargetSuppressGCTransition(DWORD dwStubFlags, MethodDesc* pTargetMD)
+ {
+ return SF_IsForwardStub(dwStubFlags) && pTargetMD && pTargetMD->ShouldSuppressGCTransition();
+ }
 };
 
 #ifdef FEATURE_COMINTEROP
@@ -4642,7 +4659,6 @@ MethodDesc* CreateInteropILStub(
  CorNativeLinkType nlType,
  CorNativeLinkFlags nlFlags,
  CorPinvokeMap unmgdCallConv,
- DWORD dwStubFlags, // NDirectStubFlags
  int nParamTokens,
  mdParamDef* pParamTokenArray,
  int iLCIDArg,
@@ -4676,6 +4692,8 @@ MethodDesc* CreateInteropILStub(
  // and vararg pinvoke.
  //
 
+ DWORD dwStubFlags = pss->GetFlags();
+
 #ifdef FEATURE_COMINTEROP
  //
  // Try to locate predefined IL stub either defined in user code or hardcoded in CLR
@@ -5018,7 +5036,6 @@ MethodDesc* NDirect::CreateCLRToNativeILStub(
  nlType,
  nlFlags,
  unmgdCallConv,
- dwStubFlags,
  numParamTokens,
  pParamTokenArray,
  iLCIDArg);
@@ -5092,7 +5109,6 @@ MethodDesc* NDirect::CreateFieldAccessILStub(
  (CorNativeLinkType)0,
  (CorNativeLinkFlags)0,
  (CorPinvokeMap)0,
- dwStubFlags,
  numParamTokens,
  pParamTokenArray,
  -1);
@@ -5201,7 +5217,6 @@ MethodDesc* NDirect::CreateStructMarshalILStub(MethodTable* pMT)
  (CorNativeLinkType)0,
  (CorNativeLinkFlags)0,
  (CorPinvokeMap)0,
- dwStubFlags,
  numParamTokens,
  pParamTokenArray,
  -1,

src/coreclr/src/vm/dllimport.h

@@ -150,15 +150,15 @@ enum NDirectStubFlags
 #ifdef FEATURE_COMINTEROP
  NDIRECTSTUB_FL_FIELDGETTER = 0x00002000, // COM->CLR field getter
  NDIRECTSTUB_FL_FIELDSETTER = 0x00004000, // COM->CLR field setter
- // unused = 0x00008000,
+#endif // FEATURE_COMINTEROP
+ NDIRECTSTUB_FL_SUPPRESSGCTRANSITION = 0x00008000,
  // unused = 0x00010000,
  // unused = 0x00020000,
  // unused = 0x00080000,
  // unused = 0x00100000,
  // unused = 0x00200000,
  // unused = 0x00400000,
  // unused = 0x00800000,
-#endif // FEATURE_COMINTEROP
 
  // internal flags -- these won't ever show up in an NDirectStubHashBlob
  NDIRECTSTUB_FL_FOR_NUMPARAMBYTES = 0x10000000, // do just enough to return the right value from Marshal.NumParamBytes

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

@@ -21,3 +21,15 @@ BOOL DLL_EXPORT NextUInt(/* out */ uint32_t *n)
  *n = (++_n);
  return TRUE;
 }
+
+typedef int (STDMETHODVCALLTYPE *CALLBACKPROC)(int n);
+
+extern "C"
+BOOL DLL_EXPORT STDMETHODVCALLTYPE InvokeCallback(CALLBACKPROC cb, int* n)
+{
+ if (cb == nullptr || n == nullptr)
+ return FALSE;
+
+ *n = cb((++_n));
+ return TRUE;
+}

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

@@ -24,6 +24,34 @@ static class SuppressGCTransitionNative
  [DllImport(nameof(SuppressGCTransitionNative), EntryPoint = "NextUInt")]
  public static extern unsafe bool NextUInt_NoInline_GCTransition(int* n);
 
+ [DllImport(nameof(SuppressGCTransitionNative), CallingConvention=CallingConvention.Cdecl, EntryPoint = "InvokeCallback")]
+ [SuppressGCTransition]
+ public static extern unsafe int InvokeCallbackFuncPtr_Inline_NoGCTransition(delegate* unmanaged[Cdecl]<int, int> cb, int* n);
+
+ [DllImport(nameof(SuppressGCTransitionNative), CallingConvention=CallingConvention.Cdecl, EntryPoint = "InvokeCallback")]
+ public static extern unsafe int InvokeCallbackFuncPtr_Inline_GCTransition(delegate* unmanaged[Cdecl]<int, int> cb, int* n);
+
+ [DllImport(nameof(SuppressGCTransitionNative), CallingConvention=CallingConvention.Cdecl, EntryPoint = "InvokeCallback")]
+ [SuppressGCTransition]
+ public static extern unsafe bool InvokeCallbackFuncPtr_NoInline_NoGCTransition(delegate* unmanaged[Cdecl]<int, int> cb, int* n);
+
+ [DllImport(nameof(SuppressGCTransitionNative), CallingConvention=CallingConvention.Cdecl, EntryPoint = "InvokeCallback")]
+ public static extern unsafe bool InvokeCallbackFuncPtr_NoInline_GCTransition(delegate* unmanaged[Cdecl]<int, int> cb, int* n);
+
+ [DllImport(nameof(SuppressGCTransitionNative), CallingConvention=CallingConvention.Cdecl, EntryPoint = "InvokeCallback")]
+ [SuppressGCTransition]
+ public static extern unsafe int InvokeCallbackVoidPtr_Inline_NoGCTransition(void* cb, int* n);
+
+ [DllImport(nameof(SuppressGCTransitionNative), CallingConvention=CallingConvention.Cdecl, EntryPoint = "InvokeCallback")]
+ public static extern unsafe int InvokeCallbackVoidPtr_Inline_GCTransition(void* cb, int* n);
+
+ [DllImport(nameof(SuppressGCTransitionNative), CallingConvention=CallingConvention.Cdecl, EntryPoint = "InvokeCallback")]
+ [SuppressGCTransition]
+ public static extern unsafe bool InvokeCallbackVoidPtr_NoInline_NoGCTransition(void* cb, int* n);
+
+ [DllImport(nameof(SuppressGCTransitionNative), CallingConvention=CallingConvention.Cdecl, EntryPoint = "InvokeCallback")]
+ public static extern unsafe bool InvokeCallbackVoidPtr_NoInline_GCTransition(void* cb, int* n);
+
  public static IntPtr GetNextUIntFunctionPointer()
  {
  IntPtr mod = GetNativeLibrary();
@@ -146,8 +174,79 @@ private static int CallAsFunctionPointer(int expected)
  Assert.AreEqual(expected, n);
  return n + 1;
  }
+ [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvCdecl) })]
+ private static int ReturnInt(int value)
+ {
+ return value;
+ }
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static int ILStubCache_NoGCTransition_GCTransition(int expected)
+ {
+ // This test uses a callback marked UnmanagedCallersOnly as a way to verify that
+ // SuppressGCTransition is taken into account when caching IL stubs.
+ // It calls functions with the same signature, differing only in SuppressGCTransition.
+ // When calling an UnmanagedCallersOnly method, the runtime validates that the GC is in
+ // pre-emptive mode. If not, it throws a fatal error that cannot be caught and crashes.
+ // If the stub for the p/invoke with the transition suppressed is incorrectly reused for
+ // the p/invoke without the suppression, invoking the callback would produce a fatal error.
+ Console.WriteLine($"{nameof(ILStubCache_NoGCTransition_GCTransition)} ({expected}) ...");
+
+ int n;
 
- public static int Main()
+ // Call function that has SuppressGCTransition
+ SuppressGCTransitionNative.InvokeCallbackFuncPtr_Inline_NoGCTransition(null, null);
+
+ // Call function with same (blittable) signature, but without SuppressGCTransition.
+ // IL stub should not be re-used, GC transition should occur, and callback should be invoked.
+ SuppressGCTransitionNative.InvokeCallbackFuncPtr_Inline_GCTransition(&ReturnInt, &n);
+ Assert.AreEqual(expected++, n);
+
+ // Call function that has SuppressGCTransition
+ SuppressGCTransitionNative.InvokeCallbackFuncPtr_NoInline_NoGCTransition(null, null);
+
+ // Call function with same (non-blittable) signature, but without SuppressGCTransition
+ // IL stub should not be re-used, GC transition should occur, and callback should be invoked.
+ SuppressGCTransitionNative.InvokeCallbackFuncPtr_NoInline_GCTransition(&ReturnInt, &n);
+ Assert.AreEqual(expected++, n);
+
+ return n + 1;
+ }
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private static int ILStubCache_GCTransition_NoGCTransition(int expected)
+ {
+ // This test uses a callback marked UnmanagedCallersOnly as a way to verify that
+ // SuppressGCTransition is taken into account when caching IL stubs.
+ // It calls functions with the same signature, differing only in SuppressGCTransition.
+ // When calling an UnmanagedCallersOnly method, the runtime validates that the GC is in
+ // pre-emptive mode. If not, it throws a fatal error that cannot be caught and crashes.
+ Console.WriteLine($"{nameof(ILStubCache_GCTransition_NoGCTransition)} ({expected}) ...");
+
+ int n;
+
+ void* cb = (delegate* unmanaged[Cdecl]<int, int>)&ReturnInt;
+
+ // Call function that does not have SuppressGCTransition
+ SuppressGCTransitionNative.InvokeCallbackVoidPtr_Inline_GCTransition(cb, &n);
+ Assert.AreEqual(expected++, n);
+
+ // Call function with same (blittable) signature, but with SuppressGCTransition.
+ // IL stub should not be re-used, GC transition not should occur, and callback invocation should fail.
+ SuppressGCTransitionNative.InvokeCallbackVoidPtr_Inline_NoGCTransition(cb, &n);
+ Assert.AreEqual(expected++, n);
+
+ // Call function that does not have SuppressGCTransition
+ SuppressGCTransitionNative.InvokeCallbackVoidPtr_NoInline_GCTransition(cb, &n);
+ Assert.AreEqual(expected++, n);
+
+ // Call function with same (non-blittable) signature, but with SuppressGCTransition
+ // IL stub should not be re-used, GC transition not should occur, and callback invocation should fail.
+ expected = n + 1;
+ SuppressGCTransitionNative.InvokeCallbackVoidPtr_NoInline_NoGCTransition(cb, &n);
+ Assert.AreEqual(expected++, n);
+
+ return n + 1;
+ }
+ public static int Main(string[] args)
  {
  try
  {
@@ -159,6 +258,13 @@ public static int Main()
  n = Mixed(n);
  n = Mixed_TightLoop(n);
  n = CallAsFunctionPointer(n);
+ n = ILStubCache_NoGCTransition_GCTransition(n);
+
+ if (args.Length != 0 && args[0].Equals("ILStubCache", StringComparison.OrdinalIgnoreCase))
+ {
+ // This test intentionally results in a fatal error, so only run when manually specified
+ n = ILStubCache_GCTransition_NoGCTransition(n);
+ }
  }
  catch (Exception e)
  {