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.

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

Fix exception handling in the prestub worker #111937

Merged
merged 10 commits into from
Feb 7, 2025
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
17 changes: 10 additions & 7 deletions src/coreclr/vm/excep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3053,14 +3053,17 @@ void StackTraceInfo::AppendElement(OBJECTHANDLE hThrowable, UINT_PTR currentIP,
// This is a workaround to fix the generation of stack traces from exception objects so that
// they point to the line that actually generated the exception instead of the line
// following.
if (pCf->IsIPadjusted())
if (pCf != NULL)
{
stackTraceElem.flags |= STEF_IP_ADJUSTED;
}
else if (!pCf->HasFaulted() && stackTraceElem.ip != 0)
{
stackTraceElem.ip -= STACKWALK_CONTROLPC_ADJUST_OFFSET;
stackTraceElem.flags |= STEF_IP_ADJUSTED;
if (pCf->IsIPadjusted())
{
stackTraceElem.flags |= STEF_IP_ADJUSTED;
}
else if (!pCf->HasFaulted() && stackTraceElem.ip != 0)
{
stackTraceElem.ip -= STACKWALK_CONTROLPC_ADJUST_OFFSET;
stackTraceElem.flags |= STEF_IP_ADJUSTED;
}
}

#ifndef TARGET_UNIX // Watson is supported on Windows only
Expand Down
12 changes: 8 additions & 4 deletions src/coreclr/vm/exceptionhandling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -932,7 +932,12 @@ ProcessCLRExceptionNew(IN PEXCEPTION_RECORD pExceptionRecord,

Thread* pThread = GetThread();

if (pThread->HasThreadStateNC(Thread::TSNC_ProcessedUnhandledException))
// Skip native frames of asm helpers that have the ProcessCLRException set as their personality routine.
// There is nothing to do for those with the new exception handling.
// Also skip all frames when processing unhandled exceptions. That allows them to reach the host app
// level and let 3rd party the chance to handle them.
if (!ExecutionManager::IsManagedCode((PCODE)pDispatcherContext->ControlPc) ||
pThread->HasThreadStateNC(Thread::TSNC_ProcessedUnhandledException))
{
if ((pExceptionRecord->ExceptionFlags & EXCEPTION_UNWINDING))
{
Expand Down Expand Up @@ -8520,7 +8525,7 @@ static StackWalkAction MoveToNextNonSkippedFrame(StackFrameIterator* pStackFrame
return retVal;
}

extern "C" size_t CallDescrWorkerInternalReturnAddressOffset;
bool IsCallDescrWorkerInternalReturnAddress(PCODE pCode);

extern "C" bool QCALLTYPE SfiNext(StackFrameIterator* pThis, uint* uExCollideClauseIdx, bool* fUnwoundReversePInvoke, bool* pfIsExceptionIntercepted)
{
Expand Down Expand Up @@ -8575,8 +8580,7 @@ extern "C" bool QCALLTYPE SfiNext(StackFrameIterator* pThis, uint* uExCollideCla
}
else
{
size_t CallDescrWorkerInternalReturnAddress = (size_t)CallDescrWorkerInternal + CallDescrWorkerInternalReturnAddressOffset;
if (GetIP(pThis->m_crawl.GetRegisterSet()->pCallerContext) == CallDescrWorkerInternalReturnAddress)
if (IsCallDescrWorkerInternalReturnAddress(GetIP(pThis->m_crawl.GetRegisterSet()->pCallerContext)))
{
invalidRevPInvoke = true;
}
Expand Down
16 changes: 14 additions & 2 deletions src/coreclr/vm/exceptmacros.h
Original file line number Diff line number Diff line change
Expand Up @@ -277,15 +277,22 @@ VOID DECLSPEC_NORETURN UnwindAndContinueRethrowHelperAfterCatch(Frame* pEntryFra
#ifdef TARGET_UNIX
VOID DECLSPEC_NORETURN DispatchManagedException(PAL_SEHException& ex, bool isHardwareException);

#define INSTALL_MANAGED_EXCEPTION_DISPATCHER \
#define INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX \
PAL_SEHException exCopy; \
bool hasCaughtException = false; \
try {

#define UNINSTALL_MANAGED_EXCEPTION_DISPATCHER \
#define INSTALL_MANAGED_EXCEPTION_DISPATCHER \
INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX

#define UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(nativeRethrow) \
} \
catch (PAL_SEHException& ex) \
{ \
if (nativeRethrow) \
{ \
throw; \
} \
exCopy = std::move(ex); \
hasCaughtException = true; \
} \
Expand All @@ -294,6 +301,9 @@ VOID DECLSPEC_NORETURN DispatchManagedException(PAL_SEHException& ex, bool isHar
DispatchManagedException(exCopy, false);\
}

#define UNINSTALL_MANAGED_EXCEPTION_DISPATCHER \
UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(false)

// Install trap that catches unhandled managed exception and dumps its stack
#define INSTALL_UNHANDLED_MANAGED_EXCEPTION_TRAP \
try {
Expand All @@ -315,7 +325,9 @@ VOID DECLSPEC_NORETURN DispatchManagedException(PAL_SEHException& ex, bool isHar
#else // TARGET_UNIX

#define INSTALL_MANAGED_EXCEPTION_DISPATCHER
#define INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX
#define UNINSTALL_MANAGED_EXCEPTION_DISPATCHER
#define UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX

#define INSTALL_UNHANDLED_MANAGED_EXCEPTION_TRAP
#define UNINSTALL_UNHANDLED_MANAGED_EXCEPTION_TRAP
Expand Down
114 changes: 73 additions & 41 deletions src/coreclr/vm/prestub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2523,6 +2523,20 @@ Stub * MakeInstantiatingStubWorker(MethodDesc *pMD)
}
#endif // defined(FEATURE_SHARE_GENERIC_CODE)

extern "C" size_t CallDescrWorkerInternalReturnAddressOffset;

bool IsCallDescrWorkerInternalReturnAddress(PCODE pCode)
{
LIMITED_METHOD_CONTRACT;
#ifdef FEATURE_EH_FUNCLETS
size_t CallDescrWorkerInternalReturnAddress = (size_t)CallDescrWorkerInternal + CallDescrWorkerInternalReturnAddressOffset;

return pCode == CallDescrWorkerInternalReturnAddress;
#else // FEATURE_EH_FUNCLETS
return false;
#endif // FEATURE_EH_FUNCLETS
}

//=============================================================================
// This function generates the real code when from Preemptive mode.
// It is specifically designed to work with the UnmanagedCallersOnlyAttribute.
Expand Down Expand Up @@ -2618,60 +2632,76 @@ extern "C" PCODE STDCALL PreStubWorker(TransitionBlock* pTransitionBlock, Method

pPFrame->Push(CURRENT_THREAD);

INSTALL_MANAGED_EXCEPTION_DISPATCHER;
INSTALL_UNWIND_AND_CONTINUE_HANDLER;
EX_TRY
{
bool propagateExceptionToNativeCode = IsCallDescrWorkerInternalReturnAddress(pTransitionBlock->m_ReturnAddress);

// Make sure the method table is restored, and method instantiation if present
pMD->CheckRestore();
CONSISTENCY_CHECK(GetAppDomain()->CheckCanExecuteManagedCode(pMD));
INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX;
INSTALL_UNWIND_AND_CONTINUE_HANDLER_EX;

MethodTable* pDispatchingMT = NULL;
if (pMD->IsVtableMethod())
{
OBJECTREF curobj = pPFrame->GetThis();
// Make sure the method table is restored, and method instantiation if present
pMD->CheckRestore();
CONSISTENCY_CHECK(GetAppDomain()->CheckCanExecuteManagedCode(pMD));

if (curobj != NULL) // Check for virtual function called non-virtually on a NULL object
MethodTable* pDispatchingMT = NULL;
if (pMD->IsVtableMethod())
{
pDispatchingMT = curobj->GetMethodTable();
OBJECTREF curobj = pPFrame->GetThis();

if (pDispatchingMT->IsIDynamicInterfaceCastable())
if (curobj != NULL) // Check for virtual function called non-virtually on a NULL object
{
MethodTable* pMDMT = pMD->GetMethodTable();
TypeHandle objectType(pDispatchingMT);
TypeHandle methodType(pMDMT);
pDispatchingMT = curobj->GetMethodTable();

GCStress<cfg_any>::MaybeTrigger();
INDEBUG(curobj = NULL); // curobj is unprotected and CanCastTo() can trigger GC
if (!objectType.CanCastTo(methodType))
if (pDispatchingMT->IsIDynamicInterfaceCastable())
{
// Apparently IDynamicInterfaceCastable magic was involved when we chose this method to be called
// that's why we better stick to the MethodTable it belongs to, otherwise
// DoPrestub() will fail not being able to find implementation for pMD in pDispatchingMT.
MethodTable* pMDMT = pMD->GetMethodTable();
TypeHandle objectType(pDispatchingMT);
TypeHandle methodType(pMDMT);

GCStress<cfg_any>::MaybeTrigger();
INDEBUG(curobj = NULL); // curobj is unprotected and CanCastTo() can trigger GC
if (!objectType.CanCastTo(methodType))
{
// Apparently IDynamicInterfaceCastable magic was involved when we chose this method to be called
// that's why we better stick to the MethodTable it belongs to, otherwise
// DoPrestub() will fail not being able to find implementation for pMD in pDispatchingMT.

pDispatchingMT = pMDMT;
pDispatchingMT = pMDMT;
}
}
}

// For value types, the only virtual methods are interface implementations.
// Thus pDispatching == pMT because there
// is no inheritance in value types. Note the BoxedEntryPointStubs are shared
// between all sharable generic instantiations, so the == test is on
// canonical method tables.
// For value types, the only virtual methods are interface implementations.
// Thus pDispatching == pMT because there
// is no inheritance in value types. Note the BoxedEntryPointStubs are shared
// between all sharable generic instantiations, so the == test is on
// canonical method tables.
#ifdef _DEBUG
MethodTable* pMDMT = pMD->GetMethodTable(); // put this here to see what the MT is in debug mode
_ASSERTE(!pMD->GetMethodTable()->IsValueType() ||
(pMD->IsUnboxingStub() && (pDispatchingMT->GetCanonicalMethodTable() == pMDMT->GetCanonicalMethodTable())));
MethodTable* pMDMT = pMD->GetMethodTable(); // put this here to see what the MT is in debug mode
_ASSERTE(!pMD->GetMethodTable()->IsValueType() ||
(pMD->IsUnboxingStub() && (pDispatchingMT->GetCanonicalMethodTable() == pMDMT->GetCanonicalMethodTable())));
#endif // _DEBUG
}
}
}

GCX_PREEMP_THREAD_EXISTS(CURRENT_THREAD);
GCX_PREEMP_THREAD_EXISTS(CURRENT_THREAD);
{
pbRetVal = pMD->DoPrestub(pDispatchingMT, CallerGCMode::Coop);
}

UNINSTALL_UNWIND_AND_CONTINUE_HANDLER_EX(propagateExceptionToNativeCode);
UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(propagateExceptionToNativeCode);
}
EX_CATCH
{
pbRetVal = pMD->DoPrestub(pDispatchingMT, CallerGCMode::Coop);
if (g_isNewExceptionHandlingEnabled)
{
OBJECTHANDLE ohThrowable = CURRENT_THREAD->LastThrownObjectHandle();
_ASSERTE(ohThrowable);
StackTraceInfo::AppendElement(ohThrowable, 0, (UINT_PTR)pTransitionBlock, pMD, NULL);
}
EX_RETHROW;
}

UNINSTALL_UNWIND_AND_CONTINUE_HANDLER;
UNINSTALL_MANAGED_EXCEPTION_DISPATCHER;
EX_END_CATCH(SwallowAllExceptions)

{
HardwareExceptionHolder;
Expand Down Expand Up @@ -3127,8 +3157,10 @@ EXTERN_C PCODE STDCALL ExternalMethodFixupWorker(TransitionBlock * pTransitionBl

pEMFrame->Push(CURRENT_THREAD); // Push the new ExternalMethodFrame onto the frame stack

INSTALL_MANAGED_EXCEPTION_DISPATCHER;
INSTALL_UNWIND_AND_CONTINUE_HANDLER;
bool propagateExceptionToNativeCode = IsCallDescrWorkerInternalReturnAddress(pTransitionBlock->m_ReturnAddress);

INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX;
INSTALL_UNWIND_AND_CONTINUE_HANDLER_EX;

bool fVirtual = false;
MethodDesc * pMD = NULL;
Expand Down Expand Up @@ -3370,8 +3402,8 @@ EXTERN_C PCODE STDCALL ExternalMethodFixupWorker(TransitionBlock * pTransitionBl
}
// Ready to return

UNINSTALL_UNWIND_AND_CONTINUE_HANDLER;
UNINSTALL_MANAGED_EXCEPTION_DISPATCHER;
UNINSTALL_UNWIND_AND_CONTINUE_HANDLER_EX(propagateExceptionToNativeCode);
UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(propagateExceptionToNativeCode);

pEMFrame->Pop(CURRENT_THREAD); // Pop the ExternalMethodFrame from the frame stack

Expand Down
21 changes: 13 additions & 8 deletions src/coreclr/vm/virtualcallstub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1262,6 +1262,8 @@ ResolveCacheElem* __fastcall VirtualCallStubManager::PromoteChainEntry(ResolveCa
}
#endif // CHAIN_LOOKUP

bool IsCallDescrWorkerInternalReturnAddress(PCODE pCode);

/* Resolve to a method and return its address or NULL if there is none.
Our return value is the target address that control should continue to. Our caller will
enter the target address as if a direct call with the original stack frame had been made from
Expand Down Expand Up @@ -1309,14 +1311,16 @@ PCODE VSD_ResolveWorker(TransitionBlock * pTransitionBlock,

PCODE target = (PCODE)NULL;

bool propagateExceptionToNativeCode = IsCallDescrWorkerInternalReturnAddress(pTransitionBlock->m_ReturnAddress);

if (pObj == NULL) {
pSDFrame->SetForNullReferenceException();
pSDFrame->Push(CURRENT_THREAD);
INSTALL_MANAGED_EXCEPTION_DISPATCHER;
INSTALL_UNWIND_AND_CONTINUE_HANDLER;
INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX;
INSTALL_UNWIND_AND_CONTINUE_HANDLER_EX;
COMPlusThrow(kNullReferenceException);
UNINSTALL_UNWIND_AND_CONTINUE_HANDLER;
UNINSTALL_MANAGED_EXCEPTION_DISPATCHER;
UNINSTALL_UNWIND_AND_CONTINUE_HANDLER_EX(propagateExceptionToNativeCode);
UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(propagateExceptionToNativeCode);
_ASSERTE(!"Throw returned");
}

Expand Down Expand Up @@ -1351,8 +1355,9 @@ PCODE VSD_ResolveWorker(TransitionBlock * pTransitionBlock,

pSDFrame->SetRepresentativeSlot(pRepresentativeMT, representativeToken.GetSlotNumber());
pSDFrame->Push(CURRENT_THREAD);
INSTALL_MANAGED_EXCEPTION_DISPATCHER;
INSTALL_UNWIND_AND_CONTINUE_HANDLER;

INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX;
INSTALL_UNWIND_AND_CONTINUE_HANDLER_EX;

// For Virtual Delegates the m_siteAddr is a field of a managed object
// Thus we have to report it as an interior pointer,
Expand Down Expand Up @@ -1388,8 +1393,8 @@ PCODE VSD_ResolveWorker(TransitionBlock * pTransitionBlock,

GCPROTECT_END();

UNINSTALL_UNWIND_AND_CONTINUE_HANDLER;
UNINSTALL_MANAGED_EXCEPTION_DISPATCHER;
UNINSTALL_UNWIND_AND_CONTINUE_HANDLER_EX(propagateExceptionToNativeCode);
UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(propagateExceptionToNativeCode);
pSDFrame->Pop(CURRENT_THREAD);

return target;
Expand Down
14 changes: 14 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;

namespace Dependency
{
public class DependencyClass
{
public static void Test()
{
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<CLRTestKind>BuildOnly</CLRTestKind>
<OutputType>Library</OutputType>
</PropertyGroup>
<ItemGroup>
<Compile Include="dependencytodelete.cs" />
</ItemGroup>
</Project>
17 changes: 17 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.il
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

.assembly extern legacy library mscorlib {}
.assembly extern dependencytodelete {}
.assembly 'tailcallinvoker' {}

.class public sequential ansi sealed beforefieldinit TailCallInvoker
extends [mscorlib]System.Object
{
.method public static void Test() cil managed noinlining
{
.maxstack 1
tail. call void [dependencytodelete]Dependency.DependencyClass::Test()
ret
} // end of method TailCallInvoker.Test
} // end of class TailCallInvoker
10 changes: 10 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.ilproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk.IL">
<PropertyGroup>
<CLRTestKind>BuildOnly</CLRTestKind>
<OutputType>Library</OutputType>
</PropertyGroup>
<ItemGroup>
<Compile Include="tailcallinvoker.il" />
<ProjectReference Include="dependencytodelete.csproj" />
</ItemGroup>
</Project>
Loading