Fix behavior of inlined NDirect methods with MulticoreJit (#55185)
authorGleb Balykov <g.balykov@samsung.com>
Mon, 2 Aug 2021 14:13:40 +0000 (17:13 +0300)
committerGitHub <noreply@github.com>
Mon, 2 Aug 2021 14:13:40 +0000 (07:13 -0700)
* Disable loading of methods from r2r images in MulticoreJit thread, if they have NDirect methods inlined in them

NDirect methods can be inlined in other methods during aot compilation.
If such method is loaded in mcj thread, it results in NDirect method being loaded.
Additionally, there's no way to control inlining during aot from runtime side, because r2r ni.dll might already exist.
In order to fix this, runtime aborts loading if method has NDirect methods inlined in it and this all happens in mcj thread.

* Forbid inlining of pinvoke methods with SuppressGCTransitionAttribute in MulticoreJit thread

src/coreclr/debug/daccess/nidump.cpp
src/coreclr/debug/daccess/nidump.h
src/coreclr/vm/ceeload.cpp
src/coreclr/vm/ceeload.h
src/coreclr/vm/ceeload.inl
src/coreclr/vm/jitinterface.cpp
src/coreclr/vm/jitinterface.h
src/coreclr/vm/prestub.cpp
src/coreclr/vm/readytoruninfo.cpp

index e421e15..06f02a3 100644 (file)
@@ -1623,7 +1623,8 @@ NativeImageDumper::PrintManifestTokenName(mdToken token,
 
 BOOL NativeImageDumper::HandleFixupForHistogram(PTR_CORCOMPILE_IMPORT_SECTION pSection,
                                                 SIZE_T fixupIndex,
-                                                SIZE_T *fixupCell)
+                                                SIZE_T *fixupCell,
+                                                BOOL mayUsePrecompiledNDirectMethods)
 {
     COUNT_T nImportSections;
     PTR_CORCOMPILE_IMPORT_SECTION pImportSections = m_decoder.GetNativeImportSections(&nImportSections);
@@ -3577,7 +3578,7 @@ size_t NativeImageDumper::TranslateSymbol(IXCLRDisassemblySupport *dis,
     return 0;
 }
 
-BOOL NativeImageDumper::HandleFixupForMethodDump(PTR_CORCOMPILE_IMPORT_SECTION pSection, SIZE_T fixupIndex, SIZE_T *fixupCell)
+BOOL NativeImageDumper::HandleFixupForMethodDump(PTR_CORCOMPILE_IMPORT_SECTION pSection, SIZE_T fixupIndex, SIZE_T *fixupCell, BOOL mayUsePrecompiledNDirectMethods)
 {
     PTR_SIZE_T fixupPtr(TO_TADDR(fixupCell));
     m_display->StartElement( "Fixup" );
index a6e9461..f39b4db 100644 (file)
@@ -283,10 +283,10 @@ private:
     IMetaDataAssemblyImport *m_manifestAssemblyImport;
 
     //helper for ComputeMethodFixupHistogram
-    BOOL HandleFixupForHistogram(PTR_CORCOMPILE_IMPORT_SECTION pSection, SIZE_T fixupIndex, SIZE_T *fixupCell);
+    BOOL HandleFixupForHistogram(PTR_CORCOMPILE_IMPORT_SECTION pSection, SIZE_T fixupIndex, SIZE_T *fixupCell, BOOL mayUsePrecompiledNDirectMethods = TRUE);
 
     //helper for DumpMethodFixups
-    BOOL HandleFixupForMethodDump(PTR_CORCOMPILE_IMPORT_SECTION pSection, SIZE_T fixupIndex, SIZE_T *fixupCell);
+    BOOL HandleFixupForMethodDump(PTR_CORCOMPILE_IMPORT_SECTION pSection, SIZE_T fixupIndex, SIZE_T *fixupCell, BOOL mayUsePrecompiledNDirectMethods = TRUE);
 
     // Dependencies
 
index c5a9d98..e0944f0 100644 (file)
@@ -10262,7 +10262,7 @@ PTR_BYTE Module::GetNativeDebugInfo(MethodDesc * pMD)
 
 //-----------------------------------------------------------------------------
 
-BOOL Module::FixupNativeEntry(CORCOMPILE_IMPORT_SECTION* pSection, SIZE_T fixupIndex, SIZE_T* fixupCell)
+BOOL Module::FixupNativeEntry(CORCOMPILE_IMPORT_SECTION* pSection, SIZE_T fixupIndex, SIZE_T* fixupCell, BOOL mayUsePrecompiledNDirectMethods)
 {
     CONTRACTL
     {
@@ -10280,7 +10280,7 @@ BOOL Module::FixupNativeEntry(CORCOMPILE_IMPORT_SECTION* pSection, SIZE_T fixupI
         {
             PTR_DWORD pSignatures = dac_cast<PTR_DWORD>(GetNativeOrReadyToRunImage()->GetRvaData(pSection->Signatures));
 
-            if (!LoadDynamicInfoEntry(this, pSignatures[fixupIndex], fixupCell))
+            if (!LoadDynamicInfoEntry(this, pSignatures[fixupIndex], fixupCell, mayUsePrecompiledNDirectMethods))
                 return FALSE;
 
             _ASSERTE(*fixupCell != NULL);
@@ -10291,7 +10291,7 @@ BOOL Module::FixupNativeEntry(CORCOMPILE_IMPORT_SECTION* pSection, SIZE_T fixupI
         if (CORCOMPILE_IS_FIXUP_TAGGED(fixup, pSection))
         {
             // Fixup has not been fixed up yet
-            if (!LoadDynamicInfoEntry(this, (RVA)CORCOMPILE_UNTAG_TOKEN(fixup), fixupCell))
+            if (!LoadDynamicInfoEntry(this, (RVA)CORCOMPILE_UNTAG_TOKEN(fixup), fixupCell, mayUsePrecompiledNDirectMethods))
                 return FALSE;
 
             _ASSERTE(!CORCOMPILE_IS_FIXUP_TAGGED(*fixupCell, pSection));
index 8111610..963ee16 100644 (file)
@@ -2679,17 +2679,17 @@ public:
     IMDInternalImport *GetNativeAssemblyImport(BOOL loadAllowed = TRUE);
     IMDInternalImport *GetNativeAssemblyImportIfLoaded();
 
-    BOOL FixupNativeEntry(CORCOMPILE_IMPORT_SECTION * pSection, SIZE_T fixupIndex, SIZE_T *fixup);
+    BOOL FixupNativeEntry(CORCOMPILE_IMPORT_SECTION * pSection, SIZE_T fixupIndex, SIZE_T *fixup, BOOL mayUsePrecompiledNDirectMethods = TRUE);
 
     //this split exists to support new CLR Dump functionality in DAC.  The
     //template removes any indirections.
-    BOOL FixupDelayList(TADDR pFixupList);
+    BOOL FixupDelayList(TADDR pFixupList, BOOL mayUsePrecompiledNDirectMethods = TRUE);
 
     template<typename Ptr, typename FixupNativeEntryCallback>
     BOOL FixupDelayListAux(TADDR pFixupList,
                            Ptr pThis, FixupNativeEntryCallback pfnCB,
                            PTR_CORCOMPILE_IMPORT_SECTION pImportSections, COUNT_T nImportSections,
-                           PEDecoder * pNativeImage);
+                           PEDecoder * pNativeImage, BOOL mayUsePrecompiledNDirectMethods = TRUE);
     void RunEagerFixups();
     void RunEagerFixupsUnlocked();
 
index d6dbe8e..f71511c 100644 (file)
@@ -463,21 +463,21 @@ FORCEINLINE PTR_DomainLocalModule Module::GetDomainLocalModule()
 
 #include "nibblestream.h"
 
-FORCEINLINE BOOL Module::FixupDelayList(TADDR pFixupList)
+FORCEINLINE BOOL Module::FixupDelayList(TADDR pFixupList, BOOL mayUsePrecompiledNDirectMethods)
 {
     WRAPPER_NO_CONTRACT;
 
     COUNT_T nImportSections;
     PTR_CORCOMPILE_IMPORT_SECTION pImportSections = GetImportSections(&nImportSections);
 
-    return FixupDelayListAux(pFixupList, this, &Module::FixupNativeEntry, pImportSections, nImportSections, GetNativeOrReadyToRunImage());
+    return FixupDelayListAux(pFixupList, this, &Module::FixupNativeEntry, pImportSections, nImportSections, GetNativeOrReadyToRunImage(), mayUsePrecompiledNDirectMethods);
 }
 
 template<typename Ptr, typename FixupNativeEntryCallback>
 BOOL Module::FixupDelayListAux(TADDR pFixupList,
                                Ptr pThis, FixupNativeEntryCallback pfnCB,
                                PTR_CORCOMPILE_IMPORT_SECTION pImportSections, COUNT_T nImportSections,
-                               PEDecoder * pNativeImage)
+                               PEDecoder * pNativeImage, BOOL mayUsePrecompiledNDirectMethods)
 {
     CONTRACTL
     {
@@ -567,7 +567,7 @@ BOOL Module::FixupDelayListAux(TADDR pFixupList,
         {
             CONSISTENCY_CHECK(fixupIndex * sizeof(TADDR) < cbData);
 
-            if (!(pThis->*pfnCB)(pImportSection, fixupIndex, dac_cast<PTR_SIZE_T>(pData + fixupIndex * sizeof(TADDR))))
+            if (!(pThis->*pfnCB)(pImportSection, fixupIndex, dac_cast<PTR_SIZE_T>(pData + fixupIndex * sizeof(TADDR)), mayUsePrecompiledNDirectMethods))
                 return FALSE;
 
             int delta = reader.ReadEncodedU32();
index 51c87fa..692c35e 100644 (file)
@@ -10162,6 +10162,20 @@ bool CEEInfo::pInvokeMarshalingRequired(CORINFO_METHOD_HANDLE method, CORINFO_SI
 #endif
     }
 
+    PrepareCodeConfig *config = GetThread()->GetCurrentPrepareCodeConfig();
+    if (config != nullptr && config->IsForMulticoreJit())
+    {
+        bool suppressGCTransition = false;
+        CorInfoCallConvExtension unmanagedCallConv = getUnmanagedCallConv(method, callSiteSig, &suppressGCTransition);
+
+        if (suppressGCTransition)
+        {
+            // MultiCoreJit thread can't inline PInvoke with SuppressGCTransitionAttribute,
+            // because it can't be resolved in mcj thread
+            result = TRUE;
+        }
+    }
+
     EE_TO_JIT_TRANSITION();
 
     return result;
@@ -13777,7 +13791,8 @@ bool IsInstructionSetSupported(CORJIT_FLAGS jitFlags, ReadyToRunInstructionSet r
 
 BOOL LoadDynamicInfoEntry(Module *currentModule,
                           RVA fixupRva,
-                          SIZE_T *entry)
+                          SIZE_T *entry,
+                          BOOL mayUsePrecompiledNDirectMethods)
 {
     STANDARD_VM_CONTRACT;
 
@@ -14005,10 +14020,17 @@ BOOL LoadDynamicInfoEntry(Module *currentModule,
 
     case ENCODE_PINVOKE_TARGET:
         {
-            MethodDesc *pMethod = ZapSig::DecodeMethod(currentModule, pInfoModule, pBlob);
+            if (mayUsePrecompiledNDirectMethods)
+            {
+                MethodDesc *pMethod = ZapSig::DecodeMethod(currentModule, pInfoModule, pBlob);
 
-            _ASSERTE(pMethod->IsNDirect());
-            result = (size_t)(LPVOID)NDirectMethodDesc::ResolveAndSetNDirectTarget((NDirectMethodDesc*)pMethod);
+                _ASSERTE(pMethod->IsNDirect());
+                result = (size_t)(LPVOID)NDirectMethodDesc::ResolveAndSetNDirectTarget((NDirectMethodDesc*)pMethod);
+            }
+            else
+            {
+                return FALSE;
+            }
         }
         break;
 
index feb9ed9..c231cc8 100644 (file)
@@ -89,7 +89,8 @@ void getMethodInfoILMethodHeaderHelper(
 
 BOOL LoadDynamicInfoEntry(Module *currentModule,
                           RVA fixupRva,
-                          SIZE_T *entry);
+                          SIZE_T *entry,
+                          BOOL mayUsePrecompiledNDirectMethods = TRUE);
 
 //
 // The legacy x86 monitor helpers do not need a state argument
@@ -1163,4 +1164,3 @@ FCDECL1(INT64, GetCompiledMethodCount, CLR_BOOL currentThread);
 FCDECL1(INT64, GetCompilationTimeInTicks, CLR_BOOL currentThread);
 
 #endif // JITINTERFACE_H
-
index 20f5cbf..75f78f5 100644 (file)
@@ -426,6 +426,12 @@ PCODE MethodDesc::PrepareILBasedCode(PrepareCodeConfig* pConfig)
 #endif
     }
 
+    if (pConfig->IsForMulticoreJit() && pCode == NULL && pConfig->ReadyToRunRejectedPrecompiledCode())
+    {
+        // Was unable to load code from r2r image in mcj thread, don't try to jit it, this method will be loaded later
+        return NULL;
+    }
+
     if (pCode == NULL)
     {
         LOG((LF_CLASSLOADER, LL_INFO1000000,
index cf2803d..0253d59 100644 (file)
@@ -978,7 +978,12 @@ PCODE ReadyToRunInfo::GetEntryPoint(MethodDesc * pMD, PrepareCodeConfig* pConfig
 
         if (fFixups)
         {
-            if (!m_pModule->FixupDelayList(dac_cast<TADDR>(GetImage()->GetBase()) + offset))
+            BOOL mayUsePrecompiledNDirectMethods = TRUE;
+#ifndef CROSSGEN_COMPILE
+            mayUsePrecompiledNDirectMethods = !pConfig->IsForMulticoreJit();
+#endif // CROSSGEN_COMPILE
+
+            if (!m_pModule->FixupDelayList(dac_cast<TADDR>(GetImage()->GetBase()) + offset, mayUsePrecompiledNDirectMethods))
             {
 #ifndef CROSSGEN_COMPILE
                 pConfig->SetReadyToRunRejectedPrecompiledCode();