Skip to content

Commit f60800e

Browse files
janvorlicarlossanlop
authored andcommitted
Fix bug introduced by preventing unwind through stack bottom (#81956)
The irecent fix to prevent unwinding through stack bottom was incorrect for secondary threads, as it just compared the SP being above the frame of the hosting API. However, other threads can have their stacks anywhere in the memory, thus this sometimes broke exception handling on secondary threads. I have also found that there was one more case where the unwind through the hosting API need to be checked - the Thread::VirtualUnwindToFirstManagedCallFrame. I have decided to use the return address of the hosting API for the checks instead of the frame address. That makes the check work properly.
1 parent 218df16 commit f60800e

File tree

3 files changed

+15
-10
lines changed

3 files changed

+15
-10
lines changed

src/coreclr/dlls/mscoree/exports.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,20 +24,20 @@
2424
#define ASSERTE_ALL_BUILDS(expr) _ASSERTE_ALL_BUILDS((expr))
2525

2626
#ifdef TARGET_UNIX
27-
#define NO_HOSTING_API_FRAME_ADDRESS ((void*)ULONG_PTR_MAX)
28-
void* g_hostingApiFrameAddress = NO_HOSTING_API_FRAME_ADDRESS;
27+
#define NO_HOSTING_API_RETURN_ADDRESS ((void*)ULONG_PTR_MAX)
28+
void* g_hostingApiReturnAddress = NO_HOSTING_API_RETURN_ADDRESS;
2929

3030
class HostingApiFrameHolder
3131
{
3232
public:
33-
HostingApiFrameHolder(void* frameAddress)
33+
HostingApiFrameHolder(void* returnAddress)
3434
{
35-
g_hostingApiFrameAddress = frameAddress;
35+
g_hostingApiReturnAddress = returnAddress;
3636
}
3737

3838
~HostingApiFrameHolder()
3939
{
40-
g_hostingApiFrameAddress = NO_HOSTING_API_FRAME_ADDRESS;
40+
g_hostingApiReturnAddress = NO_HOSTING_API_RETURN_ADDRESS;
4141
}
4242
};
4343
#endif // TARGET_UNIX
@@ -213,6 +213,7 @@ extern "C" int coreclr_create_delegate(void*, unsigned int, const char*, const c
213213
// HRESULT indicating status of the operation. S_OK if the assembly was successfully executed
214214
//
215215
extern "C"
216+
NOINLINE
216217
DLLEXPORT
217218
int coreclr_initialize(
218219
const char* exePath,
@@ -232,7 +233,7 @@ int coreclr_initialize(
232233
PInvokeOverrideFn* pinvokeOverride = nullptr;
233234

234235
#ifdef TARGET_UNIX
235-
HostingApiFrameHolder apiFrameHolder(__builtin_frame_address(0));
236+
HostingApiFrameHolder apiFrameHolder(_ReturnAddress());
236237
#endif
237238

238239
ConvertConfigPropertiesToUnicode(
@@ -443,6 +444,7 @@ int coreclr_create_delegate(
443444
// HRESULT indicating status of the operation. S_OK if the assembly was successfully executed
444445
//
445446
extern "C"
447+
NOINLINE
446448
DLLEXPORT
447449
int coreclr_execute_assembly(
448450
void* hostHandle,
@@ -459,7 +461,7 @@ int coreclr_execute_assembly(
459461
*exitCode = -1;
460462

461463
#ifdef TARGET_UNIX
462-
HostingApiFrameHolder apiFrameHolder(__builtin_frame_address(0));
464+
HostingApiFrameHolder apiFrameHolder(_ReturnAddress());
463465
#endif
464466

465467
ICLRRuntimeHost4* host = reinterpret_cast<ICLRRuntimeHost4*>(hostHandle);

src/coreclr/vm/exceptionhandling.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4523,7 +4523,7 @@ VOID UnwindManagedExceptionPass2(PAL_SEHException& ex, CONTEXT* unwindStartConte
45234523
EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE);
45244524
}
45254525

4526-
extern void* g_hostingApiFrameAddress;
4526+
extern void* g_hostingApiReturnAddress;
45274527

45284528
//---------------------------------------------------------------------------------------
45294529
//
@@ -4727,7 +4727,7 @@ VOID DECLSPEC_NORETURN UnwindManagedExceptionPass1(PAL_SEHException& ex, CONTEXT
47274727
STRESS_LOG2(LF_EH, LL_INFO100, "Processing exception at native frame: IP = %p, SP = %p \n", controlPc, sp);
47284728

47294729
// Consider the exception unhandled if the unwinding cannot proceed further or if it went past the coreclr_initialize or coreclr_execute_assembly
4730-
if ((controlPc == 0) || (sp > (UINT_PTR)g_hostingApiFrameAddress))
4730+
if ((controlPc == 0) || (controlPc == (UINT_PTR)g_hostingApiReturnAddress))
47314731
{
47324732
if (!GetThread()->HasThreadStateNC(Thread::TSNC_ProcessedUnhandledException))
47334733
{

src/coreclr/vm/stackwalk.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,8 @@ PCODE Thread::VirtualUnwindNonLeafCallFrame(T_CONTEXT* pContext, KNONVOLATILE_CO
709709
return uControlPc;
710710
}
711711

712+
extern void* g_hostingApiReturnAddress;
713+
712714
// static
713715
UINT_PTR Thread::VirtualUnwindToFirstManagedCallFrame(T_CONTEXT* pContext)
714716
{
@@ -751,8 +753,9 @@ UINT_PTR Thread::VirtualUnwindToFirstManagedCallFrame(T_CONTEXT* pContext)
751753

752754
uControlPc = GetIP(pContext);
753755

754-
if (uControlPc == 0)
756+
if ((uControlPc == 0) || (uControlPc == (PCODE)g_hostingApiReturnAddress))
755757
{
758+
uControlPc = 0;
756759
break;
757760
}
758761
#endif // !TARGET_UNIX

0 commit comments

Comments
 (0)