NativeCallable bugfixes (#34335)

- Fix race condition in JIT_ReversePInvokeEnter - Disable R2R for x86 on all platforms
dotnet · Mar 31, 2020 · a9ace0f · a9ace0f
1 parent 57eb3bd
commit a9ace0f
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 35 deletions.
diff --git a/src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs
@@ -2921,8 +2921,7 @@ private uint getJitFlags(ref CORJIT_FLAGS flags, uint sizeInBytes)
  if (this.MethodBeingCompiled.IsNativeCallable)
  {
 #if READYTORUN
- if (targetArchitecture == TargetArchitecture.X86
- && _compilation.TypeSystemContext.Target.OperatingSystem == TargetOS.Windows)
+ if (targetArchitecture == TargetArchitecture.X86)
  {
  throw new RequiresRuntimeJitException("ReadyToRun: Methods with NativeCallableAttribute not implemented");
  }

diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs
@@ -1661,9 +1661,7 @@ private void getCallInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_RESO
  pResult->methodFlags = FilterNamedIntrinsicMethodAttribs(pResult->methodFlags, methodToCall);
 
  var targetDetails = _compilation.TypeSystemContext.Target;
- if (targetDetails.Architecture == TargetArchitecture.X86
- && targetDetails.OperatingSystem == TargetOS.Windows
- && targetMethod.IsNativeCallable)
+ if (targetDetails.Architecture == TargetArchitecture.X86 && targetMethod.IsNativeCallable)
  {
  throw new RequiresRuntimeJitException("ReadyToRun: References to methods with NativeCallableAttribute not implemented");
  }

diff --git a/src/coreclr/src/vm/i386/asmhelpers.S b/src/coreclr/src/vm/i386/asmhelpers.S
@@ -555,22 +555,6 @@ LEAF_ENTRY PrecodeFixupThunk, _TEXT
  jmp C_FUNC(ThePreStub)
 LEAF_END PrecodeFixupThunk, _TEXT
 
-NESTED_ENTRY UMThunkStubRareDisable, _TEXT, NoHandler
- push eax
- push ecx
-
- sub esp, 12
- push eax // Push the UMEntryThunk
- push ecx // Push thread
- CHECK_STACK_ALIGNMENT
- call C_FUNC(UMThunkStubRareDisableWorker)
- add esp, 12
-
- pop ecx
- pop eax
- ret
-NESTED_END UMThunkStubRareDisable, _TEXT
-
 //
 // Used to get the current instruction pointer value
 //

diff --git a/src/coreclr/src/vm/jithelpers.cpp b/src/coreclr/src/vm/jithelpers.cpp
@@ -5383,8 +5383,14 @@ NOINLINE static void JIT_ReversePInvokeEnterRare(ReversePInvokeFrame* frame)
  if (thread->PreemptiveGCDisabled())
  ReversePInvokeBadTransition();
 
- thread->DisablePreemptiveGC();
  frame->currentThread = thread;
+
+ thread->DisablePreemptiveGC();
+}
+
+NOINLINE static void JIT_ReversePInvokeEnterRare2(ReversePInvokeFrame* frame)
+{
+ frame->currentThread->RareDisablePreemptiveGC();
 }
 
 EXTERN_C void JIT_ReversePInvokeEnter(ReversePInvokeFrame* frame)
@@ -5397,13 +5403,17 @@ EXTERN_C void JIT_ReversePInvokeEnter(ReversePInvokeFrame* frame)
  if (thread != NULL
  && !thread->PreemptiveGCDisabled())
  {
+ frame->currentThread = thread;
+
  // Manually inline the fast path in Thread::DisablePreemptiveGC().
  thread->m_fPreemptiveGCDisabled.StoreWithoutBarrier(1);
  if (g_TrapReturningThreads.LoadWithoutBarrier() == 0)
  {
- frame->currentThread = thread;
  return;
  }
+
+ JIT_ReversePInvokeEnterRare2(frame);
+ return;
  }
 
  JIT_ReversePInvokeEnterRare(frame);

diff --git a/src/coreclr/src/vm/jitinterface.cpp b/src/coreclr/src/vm/jitinterface.cpp
@@ -9209,10 +9209,10 @@ void CEEInfo::getFunctionFixedEntryPoint(CORINFO_METHOD_HANDLE ftn,
 
  pResult->accessType = IAT_VALUE;
 
-// Also see GetBaseCompileFlags() below for an additional check.
-#if defined(TARGET_X86) && defined(TARGET_WINDOWS) && !defined(CROSSGEN_COMPILE)
+#if defined(TARGET_X86) && !defined(CROSSGEN_COMPILE)
  // Deferring X86 support until a need is observed or
  // time permits investigation into all the potential issues.
+ // https:/dotnet/runtime/issues/33582
  if (pMD->HasNativeCallableAttribute())
  {
  pResult->addr = (void*)COMDelegate::ConvertToCallback(pMD);
@@ -9221,12 +9221,9 @@ void CEEInfo::getFunctionFixedEntryPoint(CORINFO_METHOD_HANDLE ftn,
  {
  pResult->addr = (void*)pMD->GetMultiCallableAddrOfCode();
  }
-
 #else
-
  pResult->addr = (void*)pMD->GetMultiCallableAddrOfCode();
-
-#endif // !(TARGET_X86 && TARGET_WINDOWS) || CROSSGEN_COMPILE
+#endif
 
  EE_TO_JIT_TRANSITION();
 }
@@ -12441,10 +12438,10 @@ CorJitResult CallCompileMethodWithSEHWrapper(EEJitManager *jitMgr,
  }
  }
 
-#if !defined(TARGET_X86) || !defined(TARGET_WINDOWS)
+#if !defined(TARGET_X86)
  if (ftn->HasNativeCallableAttribute())
  flags.Set(CORJIT_FLAGS::CORJIT_FLAG_REVERSE_PINVOKE);
-#endif // !TARGET_X86 || !TARGET_WINDOWS
+#endif // !TARGET_X86
 
  return flags;
 }

diff --git a/src/coreclr/src/zap/zapinfo.cpp b/src/coreclr/src/zap/zapinfo.cpp
@@ -482,14 +482,14 @@ void ZapInfo::CompileMethod()
  }
 #endif
 
-#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#ifdef TARGET_X86
  if (GetCompileInfo()->IsNativeCallableMethod(m_currentMethodHandle))
  {
  if (m_zapper->m_pOpt->m_verbose)
  m_zapper->Warning(W("ReadyToRun: Methods with NativeCallableAttribute not implemented\n"));
  ThrowHR(E_NOTIMPL);
  }
-#endif // (TARGET_X86) && defined(TARGET_WINDOWS)
+#endif // TARGET_X86
 
  if (m_pImage->m_stats)
  {
@@ -2285,14 +2285,14 @@ void ZapInfo::getCallInfo(CORINFO_RESOLVED_TOKEN * pResolvedToken,
  }
 #endif
 
-#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+#ifdef TARGET_X86
  if (GetCompileInfo()->IsNativeCallableMethod(pResult->hMethod))
  {
  if (m_zapper->m_pOpt->m_verbose)
  m_zapper->Warning(W("ReadyToRun: References to methods with NativeCallableAttribute not implemented\n"));
  ThrowHR(E_NOTIMPL);
  }
-#endif // (TARGET_X86) && defined(TARGET_WINDOWS)
+#endif // TARGET_X86
 
  if (flags & CORINFO_CALLINFO_KINDONLY)
  return;