Skip to content

Preserve last error for patchpoint helpers #75922

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

Merged
AndyAyersMS merged 3 commits into from
Sep 22, 2022
Merged

Preserve last error for patchpoint helpers #75922

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
113ead3
Preserve last error for patchpoint helpers
AndyAyersMS Sep 20, 2022
4cc615b
fix build issues, review feedback
AndyAyersMS Sep 20, 2022
05957af
add test case
AndyAyersMS Sep 21, 2022
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
Next Next commit
Preserve last error for patchpoint helpers 
In stress modes (and eventually perhaps in normal uses) the jit may insert
patchpoint helper calls in regions where last error is live. So the helpers
need to preserve last error.

Because some invocations of the helpers may transition to OSR methods instead
of returning, we can't use the normal macros for this.

Fixes #75828.
  • Loading branch information
@AndyAyersMS
AndyAyersMS committed Sep 20, 2022
commit 113ead3a88933bddfcc655f8f37ffc9b31d63d4b
192 changes: 107 additions & 85 deletions src/coreclr/vm/jithelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4916,6 +4916,9 @@ HCIMPLEND

void JIT_Patchpoint(int* counter, int ilOffset)
{
// BEGIN_PRESERVE_LAST_ERROR;
DWORD __dwLastError = ::GetLastError();
Copy link
Member

@jkotas jkotas Sep 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DWORD __dwLastError = ::GetLastError();
DWORD dwLastError = ::GetLastError();


// This method may not return normally
STATIC_CONTRACT_GC_NOTRIGGER;
STATIC_CONTRACT_MODE_COOPERATIVE;
Expand Down Expand Up @@ -4955,7 +4958,8 @@ void JIT_Patchpoint(int* counter, int ilOffset)
{
LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_Patchpoint: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n",
ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset));
return;

goto DONE;
}

// See if we have an OSR method for this patchpoint.
Expand Down Expand Up @@ -4983,7 +4987,7 @@ void JIT_Patchpoint(int* counter, int ilOffset)
{
LOG((LF_TIEREDCOMPILATION, LL_INFO10, "Jit_Patchpoint: ignoring patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n",
ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset));
return;
goto DONE;
}
#endif

Expand Down Expand Up @@ -5024,15 +5028,15 @@ void JIT_Patchpoint(int* counter, int ilOffset)
// Defer, if we haven't yet reached the limit
if (hitCount < hitLimit)
{
return;
goto DONE;
}

// Third, make sure no other thread is trying to create the OSR method.
LONG oldFlags = ppInfo->m_flags;
if ((oldFlags & PerPatchpointInfo::patchpoint_triggered) == PerPatchpointInfo::patchpoint_triggered)
{
LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_Patchpoint: AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip));
return;
goto DONE;
}

LONG newFlags = oldFlags | PerPatchpointInfo::patchpoint_triggered;
Expand All @@ -5041,7 +5045,7 @@ void JIT_Patchpoint(int* counter, int ilOffset)
if (!triggerTransition)
{
LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_Patchpoint: (lost race) AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip));
return;
goto DONE;
}

// Time to create the OSR method.
Expand Down Expand Up @@ -5071,7 +5075,7 @@ void JIT_Patchpoint(int* counter, int ilOffset)
" marking patchpoint invalid for Method=0x%pM il offset %d\n", ip, pMD, ilOffset);

InterlockedOr(&ppInfo->m_flags, (LONG)PerPatchpointInfo::patchpoint_invalid);
return;
goto DONE;
}

// We've successfully created the osr method; make it available.
Expand All @@ -5083,115 +5087,126 @@ void JIT_Patchpoint(int* counter, int ilOffset)
// If we get here, we have code to transition to...
_ASSERTE(osrMethodCode != NULL);

Thread *pThread = GetThread();
{
Thread *pThread = GetThread();

#ifdef FEATURE_HIJACK
// We can't crawl the stack of a thread that currently has a hijack pending
// (since the hijack routine won't be recognized by any code manager). So we
// Undo any hijack, the EE will re-attempt it later.
pThread->UnhijackThread();
// We can't crawl the stack of a thread that currently has a hijack pending
// (since the hijack routine won't be recognized by any code manager). So we
// Undo any hijack, the EE will re-attempt it later.
pThread->UnhijackThread();
#endif

// Find context for the original method
CONTEXT *pFrameContext = NULL;
// Find context for the original method
CONTEXT *pFrameContext = NULL;
#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64)
DWORD contextSize = 0;
ULONG64 xStateCompactionMask = 0;
DWORD contextFlags = CONTEXT_FULL;
if (Thread::AreCetShadowStacksEnabled())
{
xStateCompactionMask = XSTATE_MASK_CET_U;
contextFlags |= CONTEXT_XSTATE;
}
DWORD contextSize = 0;
ULONG64 xStateCompactionMask = 0;
DWORD contextFlags = CONTEXT_FULL;
if (Thread::AreCetShadowStacksEnabled())
{
xStateCompactionMask = XSTATE_MASK_CET_U;
contextFlags |= CONTEXT_XSTATE;
}

// The initialize call should fail but return contextSize
BOOL success = g_pfnInitializeContext2 ?
g_pfnInitializeContext2(NULL, contextFlags, NULL, &contextSize, xStateCompactionMask) :
InitializeContext(NULL, contextFlags, NULL, &contextSize);
// The initialize call should fail but return contextSize
BOOL success = g_pfnInitializeContext2 ?
g_pfnInitializeContext2(NULL, contextFlags, NULL, &contextSize, xStateCompactionMask) :
InitializeContext(NULL, contextFlags, NULL, &contextSize);

_ASSERTE(!success && (GetLastError() == ERROR_INSUFFICIENT_BUFFER));
_ASSERTE(!success && (GetLastError() == ERROR_INSUFFICIENT_BUFFER));

PVOID pBuffer = _alloca(contextSize);
success = g_pfnInitializeContext2 ?
g_pfnInitializeContext2(pBuffer, contextFlags, &pFrameContext, &contextSize, xStateCompactionMask) :
InitializeContext(pBuffer, contextFlags, &pFrameContext, &contextSize);
_ASSERTE(success);
PVOID pBuffer = _alloca(contextSize);
success = g_pfnInitializeContext2 ?
g_pfnInitializeContext2(pBuffer, contextFlags, &pFrameContext, &contextSize, xStateCompactionMask) :
InitializeContext(pBuffer, contextFlags, &pFrameContext, &contextSize);
_ASSERTE(success);
#else // TARGET_WINDOWS && TARGET_AMD64
CONTEXT frameContext;
frameContext.ContextFlags = CONTEXT_FULL;
pFrameContext = &frameContext;
CONTEXT frameContext;
frameContext.ContextFlags = CONTEXT_FULL;
pFrameContext = &frameContext;
#endif // TARGET_WINDOWS && TARGET_AMD64

// Find context for the original method
RtlCaptureContext(pFrameContext);
// Find context for the original method
RtlCaptureContext(pFrameContext);

#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64)
if (Thread::AreCetShadowStacksEnabled())
{
pFrameContext->ContextFlags |= CONTEXT_XSTATE;
SetXStateFeaturesMask(pFrameContext, xStateCompactionMask);
SetSSP(pFrameContext, _rdsspq());
}
if (Thread::AreCetShadowStacksEnabled())
{
pFrameContext->ContextFlags |= CONTEXT_XSTATE;
SetXStateFeaturesMask(pFrameContext, xStateCompactionMask);
SetSSP(pFrameContext, _rdsspq());
}
#endif // TARGET_WINDOWS && TARGET_AMD64

// Walk back to the original method frame
pThread->VirtualUnwindToFirstManagedCallFrame(pFrameContext);
// Walk back to the original method frame
pThread->VirtualUnwindToFirstManagedCallFrame(pFrameContext);

// Remember original method FP and SP because new method will inherit them.
UINT_PTR currentSP = GetSP(pFrameContext);
UINT_PTR currentFP = GetFP(pFrameContext);
// Remember original method FP and SP because new method will inherit them.
UINT_PTR currentSP = GetSP(pFrameContext);
UINT_PTR currentFP = GetFP(pFrameContext);

// We expect to be back at the right IP
if ((UINT_PTR)ip != GetIP(pFrameContext))
{
// Should be fatal
STRESS_LOG2(LF_TIEREDCOMPILATION, LL_FATALERROR, "Jit_Patchpoint: patchpoint (0x%p) TRANSITION"
" unexpected context IP 0x%p\n", ip, GetIP(pFrameContext));
EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE);
}
// We expect to be back at the right IP
if ((UINT_PTR)ip != GetIP(pFrameContext))
{
// Should be fatal
STRESS_LOG2(LF_TIEREDCOMPILATION, LL_FATALERROR, "Jit_Patchpoint: patchpoint (0x%p) TRANSITION"
" unexpected context IP 0x%p\n", ip, GetIP(pFrameContext));
EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE);
}

// Now unwind back to the original method caller frame.
EECodeInfo callerCodeInfo(GetIP(pFrameContext));
ULONG_PTR establisherFrame = 0;
PVOID handlerData = NULL;
RtlVirtualUnwind(UNW_FLAG_NHANDLER, callerCodeInfo.GetModuleBase(), GetIP(pFrameContext), callerCodeInfo.GetFunctionEntry(),
pFrameContext, &handlerData, &establisherFrame, NULL);
// Now unwind back to the original method caller frame.
EECodeInfo callerCodeInfo(GetIP(pFrameContext));
ULONG_PTR establisherFrame = 0;
PVOID handlerData = NULL;
RtlVirtualUnwind(UNW_FLAG_NHANDLER, callerCodeInfo.GetModuleBase(), GetIP(pFrameContext), callerCodeInfo.GetFunctionEntry(),
pFrameContext, &handlerData, &establisherFrame, NULL);

// Now, set FP and SP back to the values they had just before this helper was called,
// since the new method must have access to the original method frame.
//
// TODO: if we access the patchpointInfo here, we can read out the FP-SP delta from there and
// use that to adjust the stack, likely saving some stack space.
// Now, set FP and SP back to the values they had just before this helper was called,
// since the new method must have access to the original method frame.
//
// TODO: if we access the patchpointInfo here, we can read out the FP-SP delta from there and
// use that to adjust the stack, likely saving some stack space.

#if defined(TARGET_AMD64)
// If calls push the return address, we need to simulate that here, so the OSR
// method sees the "expected" SP misalgnment on entry.
_ASSERTE(currentSP % 16 == 0);
currentSP -= 8;
// If calls push the return address, we need to simulate that here, so the OSR
// method sees the "expected" SP misalgnment on entry.
_ASSERTE(currentSP % 16 == 0);
currentSP -= 8;

#if defined(TARGET_WINDOWS)
DWORD64 ssp = GetSSP(pFrameContext);
if (ssp != 0)
{
SetSSP(pFrameContext, ssp - 8);
}
DWORD64 ssp = GetSSP(pFrameContext);
if (ssp != 0)
{
SetSSP(pFrameContext, ssp - 8);
}
#endif // TARGET_WINDOWS

pFrameContext->Rbp = currentFP;
pFrameContext->Rbp = currentFP;
#endif // TARGET_AMD64

SetSP(pFrameContext, currentSP);
SetSP(pFrameContext, currentSP);

// Note we can get here w/o triggering, if there is an existing OSR method and
// we hit the patchpoint.
const int transitionLogLevel = isNewMethod ? LL_INFO10 : LL_INFO1000;
LOG((LF_TIEREDCOMPILATION, transitionLogLevel, "Jit_Patchpoint: patchpoint [%d] (0x%p) TRANSITION to ip 0x%p\n", ppId, ip, osrMethodCode));
// Note we can get here w/o triggering, if there is an existing OSR method and
// we hit the patchpoint.
const int transitionLogLevel = isNewMethod ? LL_INFO10 : LL_INFO1000;
LOG((LF_TIEREDCOMPILATION, transitionLogLevel, "Jit_Patchpoint: patchpoint [%d] (0x%p) TRANSITION to ip 0x%p\n", ppId, ip, osrMethodCode));

// Install new entry point as IP
SetIP(pFrameContext, osrMethodCode);
// Install new entry point as IP
SetIP(pFrameContext, osrMethodCode);

// Transition!
ClrRestoreNonvolatileContext(pFrameContext);
// Restore last error (since call below does not return)
// END_PRESERVE_LAST_ERROR;
::SetLastError(__dwLastError);

// Transition!
ClrRestoreNonvolatileContext(pFrameContext);
}

DONE:

// END_PRESERVE_LAST_ERROR;
::SetLastError(__dwLastError);
}

// Jit helper invoked at a partial compilation patchpoint.
Expand All @@ -5205,6 +5220,9 @@ void JIT_Patchpoint(int* counter, int ilOffset)
//
void JIT_PartialCompilationPatchpoint(int ilOffset)
{
// BEGIN_PRESERVE_LAST_ERROR;
DWORD __dwLastError = ::GetLastError();
Copy link
Member

@jkotas jkotas Sep 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DWORD __dwLastError = ::GetLastError();
DWORD dwLastError = ::GetLastError();


// This method will not return normally
STATIC_CONTRACT_GC_NOTRIGGER;
STATIC_CONTRACT_MODE_COOPERATIVE;
Expand Down Expand Up @@ -5356,6 +5374,10 @@ void JIT_PartialCompilationPatchpoint(int ilOffset)
// Install new entry point as IP
SetIP(&frameContext, osrMethodCode);

// Restore last error (since call below does not return)
// END_PRESERVE_LAST_ERROR;
::SetLastError(__dwLastError);

// Transition!
RtlRestoreContext(&frameContext, NULL);
}
Expand Down