Skip to content

Commit 2e9087a

Browse files
authored
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 c29ebd8 commit 2e9087a

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
@@ -26,20 +26,20 @@
2626
#define ASSERTE_ALL_BUILDS(expr) _ASSERTE_ALL_BUILDS((expr))
2727

2828
#ifdef TARGET_UNIX
29-
#define NO_HOSTING_API_FRAME_ADDRESS ((void*)ULONG_PTR_MAX)
30-
void* g_hostingApiFrameAddress = NO_HOSTING_API_FRAME_ADDRESS;
29+
#define NO_HOSTING_API_RETURN_ADDRESS ((void*)ULONG_PTR_MAX)
30+
void* g_hostingApiReturnAddress = NO_HOSTING_API_RETURN_ADDRESS;
3131

3232
class HostingApiFrameHolder
3333
{
3434
public:
35-
HostingApiFrameHolder(void* frameAddress)
35+
HostingApiFrameHolder(void* returnAddress)
3636
{
37-
g_hostingApiFrameAddress = frameAddress;
37+
g_hostingApiReturnAddress = returnAddress;
3838
}
3939

4040
~HostingApiFrameHolder()
4141
{
42-
g_hostingApiFrameAddress = NO_HOSTING_API_FRAME_ADDRESS;
42+
g_hostingApiReturnAddress = NO_HOSTING_API_RETURN_ADDRESS;
4343
}
4444
};
4545
#endif // TARGET_UNIX
@@ -236,6 +236,7 @@ extern "C" int coreclr_create_delegate(void*, unsigned int, const char*, const c
236236
// HRESULT indicating status of the operation. S_OK if the assembly was successfully executed
237237
//
238238
extern "C"
239+
NOINLINE
239240
DLLEXPORT
240241
int coreclr_initialize(
241242
const char* exePath,
@@ -256,7 +257,7 @@ int coreclr_initialize(
256257
host_runtime_contract* hostContract = nullptr;
257258

258259
#ifdef TARGET_UNIX
259-
HostingApiFrameHolder apiFrameHolder(__builtin_frame_address(0));
260+
HostingApiFrameHolder apiFrameHolder(_ReturnAddress());
260261
#endif
261262

262263
ConvertConfigPropertiesToUnicode(
@@ -473,6 +474,7 @@ int coreclr_create_delegate(
473474
// HRESULT indicating status of the operation. S_OK if the assembly was successfully executed
474475
//
475476
extern "C"
477+
NOINLINE
476478
DLLEXPORT
477479
int coreclr_execute_assembly(
478480
void* hostHandle,
@@ -489,7 +491,7 @@ int coreclr_execute_assembly(
489491
*exitCode = -1;
490492

491493
#ifdef TARGET_UNIX
492-
HostingApiFrameHolder apiFrameHolder(__builtin_frame_address(0));
494+
HostingApiFrameHolder apiFrameHolder(_ReturnAddress());
493495
#endif
494496

495497
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
@@ -4537,7 +4537,7 @@ VOID UnwindManagedExceptionPass2(PAL_SEHException& ex, CONTEXT* unwindStartConte
45374537
EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE);
45384538
}
45394539

4540-
extern void* g_hostingApiFrameAddress;
4540+
extern void* g_hostingApiReturnAddress;
45414541

45424542
//---------------------------------------------------------------------------------------
45434543
//
@@ -4741,7 +4741,7 @@ VOID DECLSPEC_NORETURN UnwindManagedExceptionPass1(PAL_SEHException& ex, CONTEXT
47414741
STRESS_LOG2(LF_EH, LL_INFO100, "Processing exception at native frame: IP = %p, SP = %p \n", controlPc, sp);
47424742

47434743
// Consider the exception unhandled if the unwinding cannot proceed further or if it went past the coreclr_initialize or coreclr_execute_assembly
4744-
if ((controlPc == 0) || (sp > (UINT_PTR)g_hostingApiFrameAddress))
4744+
if ((controlPc == 0) || (controlPc == (UINT_PTR)g_hostingApiReturnAddress))
47454745
{
47464746
if (!GetThread()->HasThreadStateNC(Thread::TSNC_ProcessedUnhandledException))
47474747
{

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)