Skip to content

Commit aa4290f

Browse files
authored
Fix behavior of inlined NDirect methods with MulticoreJit (#55185)
* 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
1 parent 749fa13 commit aa4290f

File tree

9 files changed

+55
-21
lines changed

9 files changed

+55
-21
lines changed

src/coreclr/debug/daccess/nidump.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,7 +1623,8 @@ NativeImageDumper::PrintManifestTokenName(mdToken token,
16231623

16241624
BOOL NativeImageDumper::HandleFixupForHistogram(PTR_CORCOMPILE_IMPORT_SECTION pSection,
16251625
SIZE_T fixupIndex,
1626-
SIZE_T *fixupCell)
1626+
SIZE_T *fixupCell,
1627+
BOOL mayUsePrecompiledNDirectMethods)
16271628
{
16281629
COUNT_T nImportSections;
16291630
PTR_CORCOMPILE_IMPORT_SECTION pImportSections = m_decoder.GetNativeImportSections(&nImportSections);
@@ -3577,7 +3578,7 @@ size_t NativeImageDumper::TranslateSymbol(IXCLRDisassemblySupport *dis,
35773578
return 0;
35783579
}
35793580

3580-
BOOL NativeImageDumper::HandleFixupForMethodDump(PTR_CORCOMPILE_IMPORT_SECTION pSection, SIZE_T fixupIndex, SIZE_T *fixupCell)
3581+
BOOL NativeImageDumper::HandleFixupForMethodDump(PTR_CORCOMPILE_IMPORT_SECTION pSection, SIZE_T fixupIndex, SIZE_T *fixupCell, BOOL mayUsePrecompiledNDirectMethods)
35813582
{
35823583
PTR_SIZE_T fixupPtr(TO_TADDR(fixupCell));
35833584
m_display->StartElement( "Fixup" );

src/coreclr/debug/daccess/nidump.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,10 +283,10 @@ class NativeImageDumper
283283
IMetaDataAssemblyImport *m_manifestAssemblyImport;
284284

285285
//helper for ComputeMethodFixupHistogram
286-
BOOL HandleFixupForHistogram(PTR_CORCOMPILE_IMPORT_SECTION pSection, SIZE_T fixupIndex, SIZE_T *fixupCell);
286+
BOOL HandleFixupForHistogram(PTR_CORCOMPILE_IMPORT_SECTION pSection, SIZE_T fixupIndex, SIZE_T *fixupCell, BOOL mayUsePrecompiledNDirectMethods = TRUE);
287287

288288
//helper for DumpMethodFixups
289-
BOOL HandleFixupForMethodDump(PTR_CORCOMPILE_IMPORT_SECTION pSection, SIZE_T fixupIndex, SIZE_T *fixupCell);
289+
BOOL HandleFixupForMethodDump(PTR_CORCOMPILE_IMPORT_SECTION pSection, SIZE_T fixupIndex, SIZE_T *fixupCell, BOOL mayUsePrecompiledNDirectMethods = TRUE);
290290

291291
// Dependencies
292292

src/coreclr/vm/ceeload.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10262,7 +10262,7 @@ PTR_BYTE Module::GetNativeDebugInfo(MethodDesc * pMD)
1026210262

1026310263
//-----------------------------------------------------------------------------
1026410264

10265-
BOOL Module::FixupNativeEntry(CORCOMPILE_IMPORT_SECTION* pSection, SIZE_T fixupIndex, SIZE_T* fixupCell)
10265+
BOOL Module::FixupNativeEntry(CORCOMPILE_IMPORT_SECTION* pSection, SIZE_T fixupIndex, SIZE_T* fixupCell, BOOL mayUsePrecompiledNDirectMethods)
1026610266
{
1026710267
CONTRACTL
1026810268
{
@@ -10280,7 +10280,7 @@ BOOL Module::FixupNativeEntry(CORCOMPILE_IMPORT_SECTION* pSection, SIZE_T fixupI
1028010280
{
1028110281
PTR_DWORD pSignatures = dac_cast<PTR_DWORD>(GetNativeOrReadyToRunImage()->GetRvaData(pSection->Signatures));
1028210282

10283-
if (!LoadDynamicInfoEntry(this, pSignatures[fixupIndex], fixupCell))
10283+
if (!LoadDynamicInfoEntry(this, pSignatures[fixupIndex], fixupCell, mayUsePrecompiledNDirectMethods))
1028410284
return FALSE;
1028510285

1028610286
_ASSERTE(*fixupCell != NULL);
@@ -10291,7 +10291,7 @@ BOOL Module::FixupNativeEntry(CORCOMPILE_IMPORT_SECTION* pSection, SIZE_T fixupI
1029110291
if (CORCOMPILE_IS_FIXUP_TAGGED(fixup, pSection))
1029210292
{
1029310293
// Fixup has not been fixed up yet
10294-
if (!LoadDynamicInfoEntry(this, (RVA)CORCOMPILE_UNTAG_TOKEN(fixup), fixupCell))
10294+
if (!LoadDynamicInfoEntry(this, (RVA)CORCOMPILE_UNTAG_TOKEN(fixup), fixupCell, mayUsePrecompiledNDirectMethods))
1029510295
return FALSE;
1029610296

1029710297
_ASSERTE(!CORCOMPILE_IS_FIXUP_TAGGED(*fixupCell, pSection));

src/coreclr/vm/ceeload.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2679,17 +2679,17 @@ class Module
26792679
IMDInternalImport *GetNativeAssemblyImport(BOOL loadAllowed = TRUE);
26802680
IMDInternalImport *GetNativeAssemblyImportIfLoaded();
26812681

2682-
BOOL FixupNativeEntry(CORCOMPILE_IMPORT_SECTION * pSection, SIZE_T fixupIndex, SIZE_T *fixup);
2682+
BOOL FixupNativeEntry(CORCOMPILE_IMPORT_SECTION * pSection, SIZE_T fixupIndex, SIZE_T *fixup, BOOL mayUsePrecompiledNDirectMethods = TRUE);
26832683

26842684
//this split exists to support new CLR Dump functionality in DAC. The
26852685
//template removes any indirections.
2686-
BOOL FixupDelayList(TADDR pFixupList);
2686+
BOOL FixupDelayList(TADDR pFixupList, BOOL mayUsePrecompiledNDirectMethods = TRUE);
26872687

26882688
template<typename Ptr, typename FixupNativeEntryCallback>
26892689
BOOL FixupDelayListAux(TADDR pFixupList,
26902690
Ptr pThis, FixupNativeEntryCallback pfnCB,
26912691
PTR_CORCOMPILE_IMPORT_SECTION pImportSections, COUNT_T nImportSections,
2692-
PEDecoder * pNativeImage);
2692+
PEDecoder * pNativeImage, BOOL mayUsePrecompiledNDirectMethods = TRUE);
26932693
void RunEagerFixups();
26942694
void RunEagerFixupsUnlocked();
26952695

src/coreclr/vm/ceeload.inl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -463,21 +463,21 @@ FORCEINLINE PTR_DomainLocalModule Module::GetDomainLocalModule()
463463

464464
#include "nibblestream.h"
465465

466-
FORCEINLINE BOOL Module::FixupDelayList(TADDR pFixupList)
466+
FORCEINLINE BOOL Module::FixupDelayList(TADDR pFixupList, BOOL mayUsePrecompiledNDirectMethods)
467467
{
468468
WRAPPER_NO_CONTRACT;
469469

470470
COUNT_T nImportSections;
471471
PTR_CORCOMPILE_IMPORT_SECTION pImportSections = GetImportSections(&nImportSections);
472472

473-
return FixupDelayListAux(pFixupList, this, &Module::FixupNativeEntry, pImportSections, nImportSections, GetNativeOrReadyToRunImage());
473+
return FixupDelayListAux(pFixupList, this, &Module::FixupNativeEntry, pImportSections, nImportSections, GetNativeOrReadyToRunImage(), mayUsePrecompiledNDirectMethods);
474474
}
475475

476476
template<typename Ptr, typename FixupNativeEntryCallback>
477477
BOOL Module::FixupDelayListAux(TADDR pFixupList,
478478
Ptr pThis, FixupNativeEntryCallback pfnCB,
479479
PTR_CORCOMPILE_IMPORT_SECTION pImportSections, COUNT_T nImportSections,
480-
PEDecoder * pNativeImage)
480+
PEDecoder * pNativeImage, BOOL mayUsePrecompiledNDirectMethods)
481481
{
482482
CONTRACTL
483483
{
@@ -567,7 +567,7 @@ BOOL Module::FixupDelayListAux(TADDR pFixupList,
567567
{
568568
CONSISTENCY_CHECK(fixupIndex * sizeof(TADDR) < cbData);
569569

570-
if (!(pThis->*pfnCB)(pImportSection, fixupIndex, dac_cast<PTR_SIZE_T>(pData + fixupIndex * sizeof(TADDR))))
570+
if (!(pThis->*pfnCB)(pImportSection, fixupIndex, dac_cast<PTR_SIZE_T>(pData + fixupIndex * sizeof(TADDR)), mayUsePrecompiledNDirectMethods))
571571
return FALSE;
572572

573573
int delta = reader.ReadEncodedU32();

src/coreclr/vm/jitinterface.cpp

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10162,6 +10162,20 @@ bool CEEInfo::pInvokeMarshalingRequired(CORINFO_METHOD_HANDLE method, CORINFO_SI
1016210162
#endif
1016310163
}
1016410164

10165+
PrepareCodeConfig *config = GetThread()->GetCurrentPrepareCodeConfig();
10166+
if (config != nullptr && config->IsForMulticoreJit())
10167+
{
10168+
bool suppressGCTransition = false;
10169+
CorInfoCallConvExtension unmanagedCallConv = getUnmanagedCallConv(method, callSiteSig, &suppressGCTransition);
10170+
10171+
if (suppressGCTransition)
10172+
{
10173+
// MultiCoreJit thread can't inline PInvoke with SuppressGCTransitionAttribute,
10174+
// because it can't be resolved in mcj thread
10175+
result = TRUE;
10176+
}
10177+
}
10178+
1016510179
EE_TO_JIT_TRANSITION();
1016610180

1016710181
return result;
@@ -13777,7 +13791,8 @@ bool IsInstructionSetSupported(CORJIT_FLAGS jitFlags, ReadyToRunInstructionSet r
1377713791

1377813792
BOOL LoadDynamicInfoEntry(Module *currentModule,
1377913793
RVA fixupRva,
13780-
SIZE_T *entry)
13794+
SIZE_T *entry,
13795+
BOOL mayUsePrecompiledNDirectMethods)
1378113796
{
1378213797
STANDARD_VM_CONTRACT;
1378313798

@@ -14005,10 +14020,17 @@ BOOL LoadDynamicInfoEntry(Module *currentModule,
1400514020

1400614021
case ENCODE_PINVOKE_TARGET:
1400714022
{
14008-
MethodDesc *pMethod = ZapSig::DecodeMethod(currentModule, pInfoModule, pBlob);
14023+
if (mayUsePrecompiledNDirectMethods)
14024+
{
14025+
MethodDesc *pMethod = ZapSig::DecodeMethod(currentModule, pInfoModule, pBlob);
1400914026

14010-
_ASSERTE(pMethod->IsNDirect());
14011-
result = (size_t)(LPVOID)NDirectMethodDesc::ResolveAndSetNDirectTarget((NDirectMethodDesc*)pMethod);
14027+
_ASSERTE(pMethod->IsNDirect());
14028+
result = (size_t)(LPVOID)NDirectMethodDesc::ResolveAndSetNDirectTarget((NDirectMethodDesc*)pMethod);
14029+
}
14030+
else
14031+
{
14032+
return FALSE;
14033+
}
1401214034
}
1401314035
break;
1401414036

src/coreclr/vm/jitinterface.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ void getMethodInfoILMethodHeaderHelper(
8989

9090
BOOL LoadDynamicInfoEntry(Module *currentModule,
9191
RVA fixupRva,
92-
SIZE_T *entry);
92+
SIZE_T *entry,
93+
BOOL mayUsePrecompiledNDirectMethods = TRUE);
9394

9495
//
9596
// The legacy x86 monitor helpers do not need a state argument
@@ -1163,4 +1164,3 @@ FCDECL1(INT64, GetCompiledMethodCount, CLR_BOOL currentThread);
11631164
FCDECL1(INT64, GetCompilationTimeInTicks, CLR_BOOL currentThread);
11641165

11651166
#endif // JITINTERFACE_H
1166-

src/coreclr/vm/prestub.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,12 @@ PCODE MethodDesc::PrepareILBasedCode(PrepareCodeConfig* pConfig)
426426
#endif
427427
}
428428

429+
if (pConfig->IsForMulticoreJit() && pCode == NULL && pConfig->ReadyToRunRejectedPrecompiledCode())
430+
{
431+
// Was unable to load code from r2r image in mcj thread, don't try to jit it, this method will be loaded later
432+
return NULL;
433+
}
434+
429435
if (pCode == NULL)
430436
{
431437
LOG((LF_CLASSLOADER, LL_INFO1000000,

src/coreclr/vm/readytoruninfo.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -978,7 +978,12 @@ PCODE ReadyToRunInfo::GetEntryPoint(MethodDesc * pMD, PrepareCodeConfig* pConfig
978978

979979
if (fFixups)
980980
{
981-
if (!m_pModule->FixupDelayList(dac_cast<TADDR>(GetImage()->GetBase()) + offset))
981+
BOOL mayUsePrecompiledNDirectMethods = TRUE;
982+
#ifndef CROSSGEN_COMPILE
983+
mayUsePrecompiledNDirectMethods = !pConfig->IsForMulticoreJit();
984+
#endif // CROSSGEN_COMPILE
985+
986+
if (!m_pModule->FixupDelayList(dac_cast<TADDR>(GetImage()->GetBase()) + offset, mayUsePrecompiledNDirectMethods))
982987
{
983988
#ifndef CROSSGEN_COMPILE
984989
pConfig->SetReadyToRunRejectedPrecompiledCode();

0 commit comments

Comments
 (0)