Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

superpmi collection crash #53440

Closed
BruceForstall opened this issue May 28, 2021 · 0 comments · Fixed by #53442
Closed

superpmi collection crash #53440

BruceForstall opened this issue May 28, 2021 · 0 comments · Fixed by #53442
Assignees
@BruceForstall
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone
6.0.0

Comments

@BruceForstall
Copy link
Member

Crash doing superpmi PMI collection using:

py c:\gh\runtime\src\coreclr\scripts\superpmi.py collect --pmi -assemblies c:\gh\runtime\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root -jit_name c:\gh\runtime\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\clrjit.dll -output_mch_path c:\bugs\spmicollect10\libraries.pmi.windows.x64.checked.mch -pmi_location c:\gh\jitutils\bin\pmi.dll --use_zapdisable

stack:

 	KernelBase.dll!RaiseFailFastException�()	Unknown
 	coreclr.dll!_DbgBreakCheck(const char * szFile, int iLine, const char * szExpr, int fConstrained) Line 427	C++
 	coreclr.dll!_DbgBreakCheckNoThrow(const char * szFile, int iLine, const char * szExpr, int fConstrained) Line 534	C++
 	coreclr.dll!DbgAssertDialog(const char * szFile, int iLine, const char * szExpr) Line 695	C++
 	coreclr.dll!EEFilterException(_EXCEPTION_POINTERS * pExceptionPointers, void * unused) Line 10614	C++
 	[Inline Frame] coreclr.dll!RunWithErrorTrapFilter(_EXCEPTION_POINTERS * exceptionPointers, void *) Line 10907	C++
 	coreclr.dll!`CEEInfo::runWithErrorTrap'::`1'::filt$0() Line 10937	C++
 	coreclr.dll!__C_specific_handler(_EXCEPTION_RECORD * ExceptionRecord, void * EstablisherFrame, _CONTEXT * ContextRecord, _DISPATCHER_CONTEXT * DispatcherContext) Line 376	C++
 	ntdll.dll!RtlpExecuteHandlerForException�()	Unknown
 	ntdll.dll!RtlDispatchException()	Unknown
 	ntdll.dll!KiUserExceptionDispatch�()	Unknown
 	superpmi-shim-collector.dll!CompileResult::AddCall(const char * name) Line 92	C++
 	superpmi-shim-collector.dll!JitHost::getIntConfigValue(const wchar_t * key, int defaultValue) Line 60	C++
 	clrjit.dll!Compiler::compInit(ArenaAllocator * pAlloc, CORINFO_METHOD_STRUCT_ * methodHnd, ICorJitInfo * compHnd, CORINFO_METHOD_INFO * methodInfo, InlineInfo * inlineInfo) Line 1862	C++
 	[Inline Frame] clrjit.dll!jitNativeCode::__l8::__Body::Run::__l6::__Body::Run(jitNativeCode::__l8::__Body::Run::__l5::__JITParam *) Line 7090	C++
 	clrjit.dll!`jitNativeCode'::`8'::__Body::Run(jitNativeCode::__l2::__JITParam * __JITpParam) Line 7101	C++
 	clrjit.dll!jitNativeCode(CORINFO_METHOD_STRUCT_ * methodHnd, CORINFO_MODULE_STRUCT_ * classPtr, ICorJitInfo * compHnd, CORINFO_METHOD_INFO * methodInfo, void * * methodCodePtr, unsigned int * methodCodeSize, JitFlags * compileFlags, void * inlineInfoPtr) Line 7125	C++
 	clrjit.dll!Compiler::fgInvokeInlineeCompiler::__l2::<lambda>(Compiler::fgInvokeInlineeCompiler::__l2::Param * pParam) Line 979	C++
 	coreclr.dll!`CEEInfo::runWithErrorTrap'::`6'::__Body::Run(RunWithErrorTrapFilterParam * pTrapParam) Line 10935	C++
 	coreclr.dll!CEEInfo::runWithErrorTrap(void(*)(void *) function, void * param) Line 10937	C++
 	[Inline Frame] clrjit.dll!Compiler::eeRunWithErrorTrap(void(*)(Compiler::fgInvokeInlineeCompiler::__l2::Param *)) Line 7847	C++
 	clrjit.dll!Compiler::fgInvokeInlineeCompiler(GenTreeCall * call, InlineResult * inlineResult) Line 993	C++
 	clrjit.dll!Compiler::fgMorphCallInlineHelper(GenTreeCall * call, InlineResult * result) Line 6654	C++
 	clrjit.dll!Compiler::fgMorphCallInline(GenTreeCall * call, InlineResult * inlineResult) Line 6537	C++
 	clrjit.dll!Compiler::fgInline() Line 158	C++
 	clrjit.dll!Phase::Run() Line 62	C++
 	[Inline Frame] clrjit.dll!DoPhase(Compiler *) Line 136	C++
 	clrjit.dll!Compiler::compCompile(void * * methodCodePtr, unsigned int * methodCodeSize, JitFlags * compileFlags) Line 4664	C++
 	clrjit.dll!Compiler::compCompileHelper(CORINFO_MODULE_STRUCT_ * classPtr, ICorJitInfo * compHnd, CORINFO_METHOD_INFO * methodInfo, void * * methodCodePtr, unsigned int * methodCodeSize, JitFlags * compileFlags) Line 6455	C++
 	[Inline Frame] clrjit.dll!Compiler::compCompile::__l73::__Body::Run(Compiler::compCompile::__l2::__JITParam *) Line 5748	C++
>	clrjit.dll!Compiler::compCompile(CORINFO_MODULE_STRUCT_ * classPtr, void * * methodCodePtr, unsigned int * methodCodeSize, JitFlags * compileFlags) Line 5752	C++
 	[Inline Frame] clrjit.dll!jitNativeCode::__l8::__Body::Run::__l6::__Body::Run(jitNativeCode::__l8::__Body::Run::__l5::__JITParam *) Line 7098	C++
 	clrjit.dll!`jitNativeCode'::`8'::__Body::Run(jitNativeCode::__l2::__JITParam * __JITpParam) Line 7101	C++
 	clrjit.dll!jitNativeCode(CORINFO_METHOD_STRUCT_ * methodHnd, CORINFO_MODULE_STRUCT_ * classPtr, ICorJitInfo * compHnd, CORINFO_METHOD_INFO * methodInfo, void * * methodCodePtr, unsigned int * methodCodeSize, JitFlags * compileFlags, void * inlineInfoPtr) Line 7125	C++
 	clrjit.dll!CILJit::compileMethod(ICorJitInfo * compHnd, CORINFO_METHOD_INFO * methodInfo, unsigned int flags, unsigned char * * entryAddress, unsigned int * nativeSizeOfCode) Line 279	C++
 	superpmi-shim-collector.dll!interceptor_ICJC::compileMethod(ICorJitInfo * comp, CORINFO_METHOD_INFO * info, unsigned int flags, unsigned char * * nativeEntry, unsigned int * nativeSizeOfCode) Line 62	C++
 	coreclr.dll!invokeCompileMethodHelper(EEJitManager * jitMgr, CEEInfo * comp, CORINFO_METHOD_INFO * info, CORJIT_FLAGS jitFlags, unsigned char * * nativeEntry, unsigned int * nativeSizeOfCode) Line 12659	C++
 	coreclr.dll!invokeCompileMethod(EEJitManager * jitMgr, CEEInfo * comp, CORINFO_METHOD_INFO * info, CORJIT_FLAGS jitFlags, unsigned char * * nativeEntry, unsigned int * nativeSizeOfCode) Line 12724	C++
 	coreclr.dll!`CallCompileMethodWithSEHWrapper'::`10'::__Body::Run(CallCompileMethodWithSEHWrapper::__l2::Param * pParam) Line 12778	C++
 	coreclr.dll!CallCompileMethodWithSEHWrapper(EEJitManager * jitMgr, CEEInfo * comp, CORINFO_METHOD_INFO * info, CORJIT_FLAGS flags, unsigned char * * nativeEntry, unsigned int * nativeSizeOfCode, NativeCodeVersion nativeCodeVersion) Line 12785	C++
 	coreclr.dll!UnsafeJitFunction(PrepareCodeConfig * config, COR_ILMETHOD_DECODER * ILHeader, CORJIT_FLAGS flags, unsigned long * pSizeOfCode) Line 13314	C++
 	coreclr.dll!MethodDesc::JitCompileCodeLocked(PrepareCodeConfig * pConfig, ListLockEntryBase<NativeCodeVersion> * pEntry, unsigned long * pSizeOfCode, CORJIT_FLAGS * pFlags) Line 1023	C++
 	coreclr.dll!MethodDesc::JitCompileCodeLockedEventWrapper(PrepareCodeConfig * pConfig, ListLockEntryBase<NativeCodeVersion> * pEntry) Line 892	C++
 	coreclr.dll!MethodDesc::JitCompileCode(PrepareCodeConfig * pConfig) Line 832	C++
 	coreclr.dll!MethodDesc::PrepareILBasedCode(PrepareCodeConfig * pConfig) Line 437	C++
 	coreclr.dll!MethodDesc::PrepareCode(PrepareCodeConfig * pConfig) Line 336	C++
 	coreclr.dll!CodeVersionManager::PublishVersionableCodeIfNecessary(MethodDesc * pMethodDesc, CallerGCMode callerGCMode, bool * doBackpatchRef, bool * doFullBackpatchRef) Line 1701	C++
 	coreclr.dll!MethodDesc::DoPrestub(MethodTable * pDispatchingMT, CallerGCMode callerGCMode) Line 2172	C++
 	coreclr.dll!PreStubWorker(TransitionBlock * pTransitionBlock, MethodDesc * pMD) Line 1998	C++
 	coreclr.dll!ThePreStub�()	Unknown
 	00007ffdec13f519()	Unknown
...

This is due to #52427, which introduced this call to compInit():

 info.compMethodSuperPMIIndex = g_jitHost->getIntConfigValue(W("SuperPMIMethodContextNumber"), -1);

Previously, the get*ConfigValue APIs had only been called by the JIT during jitStartup, which is called only once ever for the load of the JIT. Now, it's being called per compile. This exposes a race condition in the superpmi collection shim jithost implementation, which swaps out the stored MethodContext on a per-compile basis, in the shim compileMethod function. But the jithost is shared amongst threads, so one thread can trash the other thread's stashed MethodContext. Previously, it didn't matter because it was never used except for allocateMemory/freeMemory, which doesn't use the stashed MethodContext.
@BruceForstall BruceForstall added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 28, 2021
@BruceForstall BruceForstall added this to the 6.0.0 milestone May 28, 2021
@BruceForstall BruceForstall self-assigned this May 28, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label May 28, 2021
@BruceForstall BruceForstall removed the untriaged New issue has not been triaged by the area owner label May 28, 2021
@BruceForstall BruceForstall mentioned this issue May 28, 2021
Closed
BruceForstall added a commit to BruceForstall/runtime that referenced this issue May 28, 2021
@BruceForstall
Fix SuperPMI collect with getIntConfigValue
4f36bcd 
PR dotnet#52427 introduced a per-compilation call to getIntConfigValue
on "SuperPMIMethodContextNumber". This pseudo-config should not
be collected. It exposed a pre-existing multi-threading issue in
the superpmi collector shim. I got rid of the per-compilation
override of the jithost MethodContext, which is problematic, and
currently unneeded, but documented the considerations around collecting
per-compilation data.

Fixes dotnet#53440
@BruceForstall BruceForstall mentioned this issue May 28, 2021
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 28, 2021
BruceForstall added a commit that referenced this issue May 28, 2021
@BruceForstall @kunalspathak
Fix SuperPMI collect with getIntConfigValue (#53442)
1ffa574 
* Fix SuperPMI collect with getIntConfigValue

PR #52427 introduced a per-compilation call to getIntConfigValue
on "SuperPMIMethodContextNumber". This pseudo-config should not
be collected. It exposed a pre-existing multi-threading issue in
the superpmi collector shim. I got rid of the per-compilation
override of the jithost MethodContext, which is problematic, and
currently unneeded, but documented the considerations around collecting
per-compilation data.

Fixes #53440

* Update src/coreclr/ToolBox/superpmi/superpmi-shim-collector/jithost.cpp

Co-authored-by: Kunal Pathak <Kunal.Pathak@microsoft.com>

Co-authored-by: Kunal Pathak <Kunal.Pathak@microsoft.com>
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 28, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@BruceForstall BruceForstall

Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Milestone
6.0.0
Development

Successfully merging a pull request may close this issue.

Fix SuperPMI collect with getIntConfigValue BruceForstall/runtime
1 participant
@BruceForstall