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

Skip putting patches in certain stubs to prevent crashing on step-in on M1 with JMC disabled #106105

Merged
merged 4 commits into from
Aug 12, 2024
Merged

Skip putting patches in certain stubs to prevent crashing on step-in on M1 with JMC disabled #106105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7305ddf
skip putting patches in certain stubs to prevent crashing on M1
mikelle-rogers Aug 8, 2024
4b55816
fixing indents and comments
mikelle-rogers Aug 8, 2024
835d83c
fixing Jit_TailCall check
mikelle-rogers Aug 8, 2024
f80157b
remove tail call only defined in x86
mikelle-rogers Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 24 additions & 21 deletions src/coreclr/debug/ee/controller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7278,6 +7278,9 @@ void DebuggerStepper::TriggerMethodEnter(Thread * thread,
// the assert if we end up in the method we started in (which could happen if we trace call
// instructions before the JMC probe).
// m_StepInStartMethod may be null (if this step-in didn't start from managed code).
#if defined(TARGET_ARM64) && defined(__APPLE__)
LOG((LF_CORDB, LL_INFO10000, "DebuggerStepper::TriggerMethodEnter: Consistency_check_MSGF not needed because we skip setting breakpoints in certain patches on arm64-macOS\n"));
#else
if ((m_StepInStartMethod != pDesc) &&
(!m_StepInStartMethod->IsLCGMethod()))
{
Expand All @@ -7288,27 +7291,27 @@ void DebuggerStepper::TriggerMethodEnter(Thread * thread,

SString sLog;
StubManager::DbgGetLog(&sLog);

// Assert b/c the Stub-manager should have caught us first.
// We don't want people relying on TriggerMethodEnter as the real implementation for Traditional Step-in
// (see above for reasons why). However, using TME will provide a bandage for the final retail product
// in cases where we are missing a stub-manager.
CONSISTENCY_CHECK_MSGF(false, (
"\nThe Stubmanagers failed to identify and trace a stub on step-in. The stub-managers for this code-path path need to be fixed.\n"
"See http://team/sites/clrdev/Devdocs/StubManagers.rtf for more information on StubManagers.\n"
"Stepper this=0x%p, startMethod='%s::%s'\n"
"---------------------------------\n"
"Stub manager log:\n%s"
"\n"
"The thread is now in managed method '%s::%s'.\n"
"---------------------------------\n",
this,
((m_StepInStartMethod == NULL) ? "unknown" : m_StepInStartMethod->m_pszDebugClassName),
((m_StepInStartMethod == NULL) ? "unknown" : m_StepInStartMethod->m_pszDebugMethodName),
sLog.GetUTF8(),
pDesc->m_pszDebugClassName, pDesc->m_pszDebugMethodName
));
}
// Assert b/c the Stub-manager should have caught us first.
// We don't want people relying on TriggerMethodEnter as the real implementation for Traditional Step-in
// (see above for reasons why). However, using TME will provide a bandage for the final retail product
// in cases where we are missing a stub-manager.
CONSISTENCY_CHECK_MSGF(false, (
"\nThe Stubmanagers failed to identify and trace a stub on step-in. The stub-managers for this code-path path need to be fixed.\n"
"See http://team/sites/clrdev/Devdocs/StubManagers.rtf for more information on StubManagers.\n"
"Stepper this=0x%p, startMethod='%s::%s'\n"
"---------------------------------\n"
"Stub manager log:\n%s"
"\n"
"The thread is now in managed method '%s::%s'.\n"
"---------------------------------\n",
this,
((m_StepInStartMethod == NULL) ? "unknown" : m_StepInStartMethod->m_pszDebugClassName),
((m_StepInStartMethod == NULL) ? "unknown" : m_StepInStartMethod->m_pszDebugMethodName),
sLog.GetUTF8(),
pDesc->m_pszDebugClassName, pDesc->m_pszDebugMethodName
));
}
#endif //defined(TARGET_ARM64) && defined(__APPLE__)
#endif

// Place a patch to stop us.
Expand Down
41 changes: 38 additions & 3 deletions src/coreclr/vm/stubmgr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -938,10 +938,15 @@ BOOL ThePreStubManager::DoTraceStub(PCODE stubStartAddress, TraceDestination *tr
// We cannot tell where the stub will end up
// until after the prestub worker has been run.
//

#if defined(TARGET_ARM64) && defined(__APPLE__)
// On ARM64 Mac, we cannot put a breakpoint inside of ThePreStubPatchLabel
LOG((LF_CORDB, LL_INFO10000, "TPSM::DoTraceStub: Skipping on arm64-macOS\n"));
return FALSE;
#else
trace->InitForFramePush(GetEEFuncEntryPoint(ThePreStubPatchLabel));

return TRUE;
#endif //defined(TARGET_ARM64) && defined(__APPLE__)
}

//-----------------------------------------------------------
Expand Down Expand Up @@ -1028,7 +1033,13 @@ BOOL PrecodeStubManager::DoTraceStub(PCODE stubStartAddress,
#ifdef HAS_NDIRECT_IMPORT_PRECODE
case PRECODE_NDIRECT_IMPORT:
#ifndef DACCESS_COMPILE
#if defined(TARGET_ARM64) && defined(__APPLE__)
// On ARM64 Mac, we cannot put a breakpoint inside of NDirectImportThunk
LOG((LF_CORDB, LL_INFO10000, "PSM::DoTraceStub: Skipping on arm64-macOS\n"));
return FALSE;
#else
trace->InitForUnmanaged(GetEEFuncEntryPoint(NDirectImportThunk));
#endif //defined(TARGET_ARM64) && defined(__APPLE__)
#else
trace->InitForOther((PCODE)NULL);
#endif
Expand Down Expand Up @@ -1656,8 +1667,14 @@ BOOL RangeSectionStubManager::DoTraceStub(PCODE stubStartAddress, TraceDestinati
case STUB_CODE_BLOCK_METHOD_CALL_THUNK:
#ifdef DACCESS_COMPILE
DacNotImpl();
#else
#if defined(TARGET_ARM64) && defined(__APPLE__)
// On ARM64 Mac, we cannot put a breakpoint inside of ExternalMethodFixupPatchLabel
LOG((LF_CORDB, LL_INFO10000, "RSM::DoTraceStub: Skipping on arm64-macOS\n"));
return FALSE;
#else
trace->InitForManagerPush(GetEEFuncEntryPoint(ExternalMethodFixupPatchLabel), this);
#endif //defined(TARGET_ARM64) && defined(__APPLE__)
#endif
return TRUE;

Expand Down Expand Up @@ -1801,7 +1818,13 @@ BOOL ILStubManager::DoTraceStub(PCODE stubStartAddress,
MethodDesc* pStubMD = ExecutionManager::GetCodeMethodDesc(stubStartAddress);
if (pStubMD != NULL && pStubMD->AsDynamicMethodDesc()->IsMulticastStub())
{
#if defined(TARGET_ARM64) && defined(__APPLE__)
//On ARM64 Mac, we cannot put a breakpoint inside of MulticastDebuggerTraceHelper
LOG((LF_CORDB, LL_INFO10000, "ILSM::DoTraceStub: skipping on arm64-macOS\n"));
return FALSE;
#else
traceDestination = GetEEFuncEntryPoint(StubHelpers::MulticastDebuggerTraceHelper);
#endif //defined(TARGET_ARM64) && defined(__APPLE__)
}
else
{
Expand Down Expand Up @@ -2104,18 +2127,30 @@ BOOL InteropDispatchStubManager::TraceManager(Thread *thread,

if (IsVarargPInvokeStub(GetIP(pContext)))
{
#if defined(TARGET_ARM64) && defined(__APPLE__)
//On ARM64 Mac, we cannot put a breakpoint inside of VarargPInvokeStub
LOG((LF_CORDB, LL_INFO10000, "IDSM::TraceManager: Skipping on arm64-macOS\n"));
return FALSE;
#else
NDirectMethodDesc *pNMD = (NDirectMethodDesc *)arg;
_ASSERTE(pNMD->IsNDirect());
PCODE target = (PCODE)pNMD->GetNDirectTarget();

LOG((LF_CORDB, LL_INFO10000, "IDSM::TraceManager: Vararg P/Invoke case 0x%p\n", target));
LOG((LF_CORDB, LL_INFO10000, "IDSM::TraceManager: Vararg P/Invoke case %p\n", target));
trace->InitForUnmanaged(target);
#endif //defined(TARGET_ARM64) && defined(__APPLE__)
}
else if (GetIP(pContext) == GetEEFuncEntryPoint(GenericPInvokeCalliHelper))
{
#if defined(TARGET_ARM64) && defined(__APPLE__)
//On ARM64 Mac, we cannot put a breakpoint inside of GenericPInvokeCalliHelper
LOG((LF_CORDB, LL_INFO10000, "IDSM::TraceManager: Skipping on arm64-macOS\n"));
return FALSE;
#else
PCODE target = (PCODE)arg;
LOG((LF_CORDB, LL_INFO10000, "IDSM::TraceManager: Unmanaged CALLI case 0x%p\n", target));
LOG((LF_CORDB, LL_INFO10000, "IDSM::TraceManager: Unmanaged CALLI case %p\n", target));
trace->InitForUnmanaged(target);
#endif //defined(TARGET_ARM64) && defined(__APPLE__)
}
#ifdef FEATURE_COMINTEROP
else
Expand Down