Skip to content

Fix unhandled exception in tiered compilation thread #116716

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/coreclr/vm/dispatchinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2168,6 +2168,7 @@ HRESULT DispatchInfo::InvokeMember(SimpleComCallWrapper *pSimpleWrap, DISPID id,
// which may swallow managed exceptions. The debugger needs this in order to send a
// CatchHandlerFound (CHF) notification.
DebuggerU2MCatchHandlerFrame catchFrame(true /* catchesAllExceptions */);

EX_TRY
{
InvokeMemberDebuggerWrapper(pDispMemberInfo,
Expand Down
20 changes: 10 additions & 10 deletions src/coreclr/vm/exceptionhandling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3910,7 +3910,7 @@ extern "C" CLR_BOOL QCALLTYPE SfiNext(StackFrameIterator* pThis, uint* uExCollid
QCALL_CONTRACT;

StackWalkAction retVal = SWA_FAILED;
CLR_BOOL invalidRevPInvoke = FALSE;
CLR_BOOL isPropagatingToNativeCode = FALSE;
Thread* pThread = GET_THREAD();
ExInfo* pTopExInfo = (ExInfo*)pThread->GetExceptionState()->GetCurrentExceptionTracker();

Expand Down Expand Up @@ -3958,18 +3958,18 @@ extern "C" CLR_BOOL QCALLTYPE SfiNext(StackFrameIterator* pThis, uint* uExCollid
EECodeInfo codeInfo(preUnwindControlPC);
#ifdef USE_GC_INFO_DECODER
GcInfoDecoder gcInfoDecoder(codeInfo.GetGCInfoToken(), DECODE_REVERSE_PINVOKE_VAR);
invalidRevPInvoke = gcInfoDecoder.GetReversePInvokeFrameStackSlot() != NO_REVERSE_PINVOKE_FRAME;
isPropagatingToNativeCode = gcInfoDecoder.GetReversePInvokeFrameStackSlot() != NO_REVERSE_PINVOKE_FRAME;
#else // USE_GC_INFO_DECODER
hdrInfo *hdrInfoBody;
codeInfo.DecodeGCHdrInfo(&hdrInfoBody);
invalidRevPInvoke = hdrInfoBody->revPInvokeOffset != INVALID_REV_PINVOKE_OFFSET;
isPropagatingToNativeCode = hdrInfoBody->revPInvokeOffset != INVALID_REV_PINVOKE_OFFSET;
#endif // USE_GC_INFO_DECODER
bool isPropagatingToExternalNativeCode = false;

EH_LOG((LL_INFO100, "SfiNext: reached native frame at IP=%p, SP=%p, invalidRevPInvoke=%d\n",
GetIP(pThis->m_crawl.GetRegisterSet()->pCurrentContext), GetSP(pThis->m_crawl.GetRegisterSet()->pCurrentContext), invalidRevPInvoke));
EH_LOG((LL_INFO100, "SfiNext: reached native frame at IP=%p, SP=%p, isPropagatingToNativeCode=%d\n",
GetIP(pThis->m_crawl.GetRegisterSet()->pCurrentContext), GetSP(pThis->m_crawl.GetRegisterSet()->pCurrentContext), isPropagatingToNativeCode));
Comment on lines +3969 to +3970
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
EH_LOG((LL_INFO100, "SfiNext: reached native frame at IP=%p, SP=%p, isPropagatingToNativeCode=%d\n",
GetIP(pThis->m_crawl.GetRegisterSet()->pCurrentContext), GetSP(pThis->m_crawl.GetRegisterSet()->pCurrentContext), isPropagatingToNativeCode));
EH_LOG((LL_INFO100, "SfiNext: reached native frame at IP=%p, SP=%p, isPropagatingToNativeCode=%s\n",
GetIP(pThis->m_crawl.GetRegisterSet()->pCurrentContext), GetSP(pThis->m_crawl.GetRegisterSet()->pCurrentContext), (isPropagatingToNativeCode ? "true" : "false")));


if (invalidRevPInvoke)
if (isPropagatingToNativeCode)
{
#ifdef HOST_UNIX
void* callbackCxt = NULL;
Expand All @@ -3994,20 +3994,20 @@ extern "C" CLR_BOOL QCALLTYPE SfiNext(StackFrameIterator* pThis, uint* uExCollid
if (IsCallDescrWorkerInternalReturnAddress(GetIP(pThis->m_crawl.GetRegisterSet()->pCurrentContext)))
{
EH_LOG((LL_INFO100, "SfiNext: the native frame is CallDescrWorkerInternal"));
invalidRevPInvoke = TRUE;
isPropagatingToNativeCode = TRUE;
}
else if (doingFuncletUnwind && codeInfo.GetJitManager()->IsFilterFunclet(&codeInfo))
{
EH_LOG((LL_INFO100, "SfiNext: current frame is filter funclet"));
invalidRevPInvoke = TRUE;
isPropagatingToNativeCode = TRUE;
}
else
{
isPropagatingToExternalNativeCode = true;
}
}

if (invalidRevPInvoke)
if (isPropagatingToNativeCode)
{
pFrame = pThis->m_crawl.GetFrame();

Expand Down Expand Up @@ -4167,7 +4167,7 @@ Exit:;

if (fUnwoundReversePInvoke)
{
*fUnwoundReversePInvoke = invalidRevPInvoke;
*fUnwoundReversePInvoke = isPropagatingToNativeCode;
}

if (pThis->GetFrameState() == StackFrameIterator::SFITER_FRAMELESS_METHOD)
Expand Down
146 changes: 12 additions & 134 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10271,90 +10271,6 @@ int32_t * CEEInfo::getAddrOfCaptureThreadGlobal(void **ppIndirection)
return result;
}


// This method is called from CEEInfo::FilterException which
// is run as part of the SEH filter clause for the JIT.
// It is fatal to throw an exception while running a SEH filter clause
// so our contract is NOTHROW, NOTRIGGER.
//
LONG EEFilterException(struct _EXCEPTION_POINTERS *pExceptionPointers, void *unused)
{
CONTRACTL {
NOTHROW;
GC_NOTRIGGER;
} CONTRACTL_END;

int result = 0;

JIT_TO_EE_TRANSITION_LEAF();

unsigned code = pExceptionPointers->ExceptionRecord->ExceptionCode;

#ifdef _DEBUG
if (code == EXCEPTION_ACCESS_VIOLATION)
{
static int hit = 0;
if (hit++ == 0)
{
_ASSERTE(!"Access violation while Jitting!");
// If you set the debugger to catch access violations and 'go'
// you will get back to the point at which the access violation occurred
result = EXCEPTION_CONTINUE_EXECUTION;
}
else
{
result = EXCEPTION_CONTINUE_SEARCH;
}
}
else
#endif // _DEBUG
// No one should be catching breakpoint
// Similarly the JIT doesn't know how to reset the guard page, so it shouldn't
// be catching a hard stack overflow
if (code == EXCEPTION_BREAKPOINT || code == EXCEPTION_SINGLE_STEP || code == EXCEPTION_STACK_OVERFLOW)
{
result = EXCEPTION_CONTINUE_SEARCH;
}
else if (!IsComPlusException(pExceptionPointers->ExceptionRecord))
{
result = EXCEPTION_EXECUTE_HANDLER;
}
else
{
GCX_COOP();

// This is actually the LastThrown exception object.
OBJECTREF throwable = CLRException::GetThrowableFromExceptionRecord(pExceptionPointers->ExceptionRecord);

if (throwable != NULL)
{
struct
{
OBJECTREF oLastThrownObject;
} _gc;

ZeroMemory(&_gc, sizeof(_gc));

// Setup the throwables
_gc.oLastThrownObject = throwable;

GCPROTECT_BEGIN(_gc);

// Don't catch ThreadAbort and other uncatchable exceptions
if (IsUncatchable(&_gc.oLastThrownObject))
result = EXCEPTION_CONTINUE_SEARCH;
else
result = EXCEPTION_EXECUTE_HANDLER;

GCPROTECT_END();
}
}

EE_TO_JIT_TRANSITION_LEAF();

return result;
}

// This code is called if FilterException chose to handle the exception.
void CEEInfo::HandleException(struct _EXCEPTION_POINTERS *pExceptionPointers)
{
Expand Down Expand Up @@ -10533,26 +10449,6 @@ uint32_t CEEInfo::getJitFlags(CORJIT_FLAGS* jitFlags, uint32_t sizeInBytes)
}

/*********************************************************************/
#if !defined(TARGET_UNIX)

struct RunWithErrorTrapFilterParam
{
ICorDynamicInfo* m_corInfo;
void (*m_function)(void*);
void* m_param;
EXCEPTION_POINTERS m_exceptionPointers;
};

static LONG RunWithErrorTrapFilter(struct _EXCEPTION_POINTERS* exceptionPointers, void* theParam)
{
WRAPPER_NO_CONTRACT;

auto* param = reinterpret_cast<RunWithErrorTrapFilterParam*>(theParam);
param->m_exceptionPointers = *exceptionPointers;
return EEFilterException(exceptionPointers, nullptr);
}

#endif // !defined(TARGET_UNIX)

bool CEEInfo::runWithSPMIErrorTrap(void (*function)(void*), void* param)
{
Expand All @@ -10573,43 +10469,25 @@ bool CEEInfo::runWithSPMIErrorTrap(void (*function)(void*), void* param)

bool CEEInfo::runWithErrorTrap(void (*function)(void*), void* param)
{
// No dynamic contract here because SEH is used
STATIC_CONTRACT_THROWS;
STATIC_CONTRACT_GC_TRIGGERS;
STATIC_CONTRACT_MODE_PREEMPTIVE;
CONTRACTL {
THROWS;
GC_TRIGGERS;
MODE_PREEMPTIVE;
}
CONTRACTL_END;

// NOTE: the lack of JIT/EE transition markers in this method is intentional. Any
// transitions into the EE proper should occur either via the call to
// `EEFilterException` (which is appropriately marked) or via JIT/EE
// interface calls made by `function`.
// transitions into the EE proper should occur via JIT/EE interface calls
// made by `function`.

bool success = true;

#if !defined(TARGET_UNIX)

RunWithErrorTrapFilterParam trapParam;
trapParam.m_corInfo = this;
trapParam.m_function = function;
trapParam.m_param = param;

PAL_TRY(RunWithErrorTrapFilterParam*, pTrapParam, &trapParam)
{
pTrapParam->m_function(pTrapParam->m_param);
}
PAL_EXCEPT_FILTER(RunWithErrorTrapFilter)
{
HandleException(&trapParam.m_exceptionPointers);
success = false;
}
PAL_ENDTRY

#else // !defined(TARGET_UNIX)
GCX_COOP();
DebuggerU2MCatchHandlerFrame catchFrame(true /* catchesAllExceptions */);

// We shouldn't need PAL_TRY on *nix: any exceptions that we are able to catch
// ought to originate from the runtime itself and should be catchable inside of
// EX_TRY/EX_CATCH, including emulated SEH exceptions.
EX_TRY
{
GCX_PREEMP();
function(param);
}
EX_CATCH
Expand All @@ -10618,7 +10496,7 @@ bool CEEInfo::runWithErrorTrap(void (*function)(void*), void* param)
}
EX_END_CATCH(RethrowTerminalExceptions);

#endif
catchFrame.Pop();

return success;
}
Expand Down
30 changes: 30 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_116676/test116676.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
using System;
using System.Threading;
using System.Runtime.CompilerServices;
using Xunit;

public class Test116676
{
[UnsafeAccessor(UnsafeAccessorKind.Method)]
extern static void DoesNotExist([UnsafeAccessorType("DoesNotExist")] object a);

[MethodImpl(MethodImplOptions.NoInlining)]
static void Work(bool f)
{
if (f)
DoesNotExist(null);
}

[Fact]
public static void TestEntryPoint()
{
for (int i = 0; i < 200; i++)
{
Thread.Sleep(10);
Work(false);
}
}
}

12 changes: 12 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_116676/test116676.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<CLRTestPriority>1</CLRTestPriority>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="test116676.cs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="$(TestSourceDir)Common/CoreCLRTestLibrary/CoreCLRTestLibrary.csproj" />
</ItemGroup>
</Project>
Loading