diff --git a/src/coreclr/debug/createdump/createdumppal.cpp b/src/coreclr/debug/createdump/createdumppal.cpp index 4dd7204ce91fd5..9e25a305869a70 100644 --- a/src/coreclr/debug/createdump/createdumppal.cpp +++ b/src/coreclr/debug/createdump/createdumppal.cpp @@ -24,7 +24,8 @@ typedef BOOL (*PFN_PAL_VirtualUnwindOutOfProc)( KNONVOLATILE_CONTEXT_POINTERS *contextPointers, PULONG64 functionStart, SIZE_T baseAddress, - UnwindReadMemoryCallback readMemoryCallback); + UnwindReadMemoryCallback readMemoryCallback, + bool *isSignalFrame); typedef BOOL (*PFN_PAL_GetUnwindInfoSize)( SIZE_T baseAddress, @@ -177,13 +178,14 @@ PAL_VirtualUnwindOutOfProc( KNONVOLATILE_CONTEXT_POINTERS *contextPointers, PULONG64 functionStart, SIZE_T baseAddress, - UnwindReadMemoryCallback readMemoryCallback) + UnwindReadMemoryCallback readMemoryCallback, + bool *isSignalFrame) { if (!InitializePAL() || g_PAL_VirtualUnwindOutOfProc == nullptr) { return FALSE; } - return g_PAL_VirtualUnwindOutOfProc(context, contextPointers, functionStart, baseAddress, readMemoryCallback); + return g_PAL_VirtualUnwindOutOfProc(context, contextPointers, functionStart, baseAddress, readMemoryCallback, isSignalFrame); } BOOL diff --git a/src/coreclr/debug/createdump/threadinfo.cpp b/src/coreclr/debug/createdump/threadinfo.cpp index bd13736846ddc1..2da1695113c58c 100644 --- a/src/coreclr/debug/createdump/threadinfo.cpp +++ b/src/coreclr/debug/createdump/threadinfo.cpp @@ -45,6 +45,8 @@ ThreadInfo::UnwindNativeFrames(CONTEXT* pContext) uint64_t previousSp = 0; uint64_t previousIp = 0; int ipMatchCount = 0; + bool isSignalFrame = false; + bool crossedSignalTrampoline = false; // For each native frame, add a page around the IP and any unwind info not already // added in VisitProgramHeader (Linux) and VisitSection (MacOS) to the dump. @@ -63,10 +65,19 @@ ThreadInfo::UnwindNativeFrames(CONTEXT* pContext) sp++; } #endif - if (ip == 0 || sp <= previousSp) { - TRACE_VERBOSE("Unwind: sp not increasing or ip == 0 sp %p ip %p\n", (void*)sp, (void*)ip); + // When a signal handler uses SA_ONSTACK (alternate signal stack), the SP can legitimately + // decrease when unwinding crosses the signal trampoline back to the original thread stack. + // Allow the SP decrease if the current frame is a signal trampoline (detected by the + // previous unwind call) and we haven't already crossed one (limit to one crossing to + // bound corruption damage). + if (ip == 0 || sp == 0 || (sp <= previousSp && (!isSignalFrame || crossedSignalTrampoline))) { + TRACE("Unwind: STOPPED sp not increasing or ip == 0 sp %p ip %p prev_sp %p\n", (void*)sp, (void*)ip, (void*)previousSp); break; } + if (sp < previousSp) + { + crossedSignalTrampoline = true; + } // Break out of the endless loop if the IP matches over a 1000 times. This is a fallback // behavior of libunwind when the module the IP is in doesn't have unwind info and for // simple stack overflows. The stack memory is added to the dump in GetThreadStack and @@ -98,8 +109,9 @@ ThreadInfo::UnwindNativeFrames(CONTEXT* pContext) // Unwind the native frame adding all the memory accessed to the core dump via the read memory adapter. ULONG64 functionStart; - if (!PAL_VirtualUnwindOutOfProc(pContext, nullptr, &functionStart, baseAddress, ReadMemoryAdapter)) { - TRACE("Unwind: PAL_VirtualUnwindOutOfProc returned false\n"); + isSignalFrame = false; + if (!PAL_VirtualUnwindOutOfProc(pContext, nullptr, &functionStart, baseAddress, ReadMemoryAdapter, &isSignalFrame)) { + TRACE("Unwind: PAL_VirtualUnwindOutOfProc returned false ip %p base %p sp %p\n", (void*)ip, (void*)baseAddress, (void*)sp); break; } diff --git a/src/coreclr/debug/daccess/dacfn.cpp b/src/coreclr/debug/daccess/dacfn.cpp index c3131e2d571efe..16f302ecd220a7 100644 --- a/src/coreclr/debug/daccess/dacfn.cpp +++ b/src/coreclr/debug/daccess/dacfn.cpp @@ -248,7 +248,7 @@ typedef BOOL(*UnwindReadMemoryCallback)(PVOID address, PVOID buffer, SIZE_T size extern BOOL -PAL_VirtualUnwindOutOfProc(PT_CONTEXT context, PT_KNONVOLATILE_CONTEXT_POINTERS contextPointers, PULONG64 functionStart, SIZE_T baseAddress, UnwindReadMemoryCallback readMemoryCallback); +PAL_VirtualUnwindOutOfProc(PT_CONTEXT context, PT_KNONVOLATILE_CONTEXT_POINTERS contextPointers, PULONG64 functionStart, SIZE_T baseAddress, UnwindReadMemoryCallback readMemoryCallback, bool *isSignalFrame); #endif HRESULT @@ -282,7 +282,7 @@ DacVirtualUnwind(ULONG32 threadId, PT_CONTEXT context, PT_KNONVOLATILE_CONTEXT_P hr = S_OK; SIZE_T baseAddress = DacGlobalBase(); - if (baseAddress == 0 || !PAL_VirtualUnwindOutOfProc(context, contextPointers, nullptr, baseAddress, DacReadAllAdapter)) + if (baseAddress == 0 || !PAL_VirtualUnwindOutOfProc(context, contextPointers, nullptr, baseAddress, DacReadAllAdapter, nullptr)) { hr = E_FAIL; } diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h index e691ee5e06d352..d044bc3e6df96d 100644 --- a/src/coreclr/pal/inc/pal.h +++ b/src/coreclr/pal/inc/pal.h @@ -2689,7 +2689,7 @@ typedef BOOL(*UnwindReadMemoryCallback)(PVOID address, PVOID buffer, SIZE_T size PALIMPORT BOOL PALAPI PAL_VirtualUnwind(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *contextPointers); -PALIMPORT BOOL PALAPI PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *contextPointers, PULONG64 functionStart, SIZE_T baseAddress, UnwindReadMemoryCallback readMemoryCallback); +PALIMPORT BOOL PALAPI PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *contextPointers, PULONG64 functionStart, SIZE_T baseAddress, UnwindReadMemoryCallback readMemoryCallback, bool *isSignalFrame); PALIMPORT BOOL PALAPI PAL_GetUnwindInfoSize(SIZE_T baseAddress, ULONG64 ehFrameHdrAddr, UnwindReadMemoryCallback readMemoryCallback, PULONG64 ehFrameStart, PULONG64 ehFrameSize); diff --git a/src/coreclr/pal/src/exception/remote-unwind.cpp b/src/coreclr/pal/src/exception/remote-unwind.cpp index 0a6aba1fa3fc05..3963578eff0afe 100644 --- a/src/coreclr/pal/src/exception/remote-unwind.cpp +++ b/src/coreclr/pal/src/exception/remote-unwind.cpp @@ -2460,10 +2460,11 @@ static unw_accessors_t unwind_accessors = init_unwind_accessors(); functionStart - the pointer to return the starting address of the function or nullptr baseAddress - base address of the module to find the unwind info readMemoryCallback - reads memory from the target + isSignalFrame - output parameter: set to true if the unwound-to frame is a signal trampoline --*/ BOOL PALAPI -PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *contextPointers, PULONG64 functionStart, SIZE_T baseAddress, UnwindReadMemoryCallback readMemoryCallback) +PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *contextPointers, PULONG64 functionStart, SIZE_T baseAddress, UnwindReadMemoryCallback readMemoryCallback, bool *isSignalFrame) { unw_addr_space_t addrSpace = 0; unw_cursor_t cursor; @@ -2471,6 +2472,11 @@ PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *cont BOOL result = FALSE; int st; + if (isSignalFrame) + { + *isSignalFrame = false; + } + info.BaseAddress = baseAddress; info.Context = context; info.FunctionStart = 0; @@ -2542,6 +2548,14 @@ PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *cont goto exit; } + // Check if the frame we landed on is a signal trampoline. When a signal handler uses + // SA_ONSTACK, stepping from a signal frame crosses from the alternate signal stack to the + // original thread stack, which can cause the SP to decrease. + if (isSignalFrame && unw_is_signal_frame(&cursor) > 0) + { + *isSignalFrame = true; + } + UnwindContextToContext(&cursor, context); if (contextPointers != NULL) @@ -2708,7 +2722,7 @@ PAL_GetUnwindInfoSize(SIZE_T baseAddress, ULONG64 ehFrameHdrAddr, UnwindReadMemo BOOL PALAPI -PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *contextPointers, PULONG64 functionStart, SIZE_T baseAddress, UnwindReadMemoryCallback readMemoryCallback) +PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *contextPointers, PULONG64 functionStart, SIZE_T baseAddress, UnwindReadMemoryCallback readMemoryCallback, bool *isSignalFrame) { return FALSE; }