diff --git a/eng/native/configurecompiler.cmake b/eng/native/configurecompiler.cmake index 48972cc67af02..20fe248b0a741 100644 --- a/eng/native/configurecompiler.cmake +++ b/eng/native/configurecompiler.cmake @@ -159,6 +159,20 @@ elseif (CLR_CMAKE_HOST_UNIX) endif(MSVC) if (CLR_CMAKE_ENABLE_SANITIZERS) + enable_language(C) + if (CMAKE_C_COMPILER_ID MATCHES "Clang") + execute_process( + COMMAND ${CMAKE_C_COMPILER} -print-resource-dir + OUTPUT_VARIABLE compilerResourceDir + OUTPUT_STRIP_TRAILING_WHITESPACE) + include_directories(SYSTEM ${compilerResourceDir}/include) + + # Work around https://gitlab.kitware.com/cmake/cmake/-/issues/19227 since CoreCLR passes -nostdinc + # and CMake will filter out the resource dir if -nostdinc is passed. + list(REMOVE_ITEM CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES "${compilerResourceDir}/include") + list(REMOVE_ITEM CMAKE_C_IMPLICIT_INCLUDE_DIRECTORIES "${compilerResourceDir}/include") + endif() + set (CLR_CMAKE_BUILD_SANITIZERS "") set (CLR_CMAKE_SANITIZER_RUNTIMES "") string(FIND "${CLR_CMAKE_ENABLE_SANITIZERS}" "address" __ASAN_POS) @@ -176,12 +190,11 @@ if (CLR_CMAKE_ENABLE_SANITIZERS) # The rest of our platforms use statically-linked ASAN so this isn't a concern for those platforms. if (CLR_CMAKE_TARGET_OSX OR CLR_CMAKE_TARGET_MACCATALYST) function(getSanitizerRuntimeDirectory output) - enable_language(C) execute_process( - COMMAND ${CMAKE_C_COMPILER} -print-resource-dir - OUTPUT_VARIABLE compilerResourceDir + COMMAND ${CMAKE_C_COMPILER} -print-runtime-dir + OUTPUT_VARIABLE compilerRuntimeDir OUTPUT_STRIP_TRAILING_WHITESPACE) - set(${output} "${compilerResourceDir}/lib/darwin/" PARENT_SCOPE) + set(${output} "${compilerResourceDir}" PARENT_SCOPE) endfunction() getSanitizerRuntimeDirectory(sanitizerRuntimeDirectory) find_library(ASAN_RUNTIME clang_rt.asan_osx_dynamic PATHS ${sanitizerRuntimeDirectory}) @@ -214,11 +227,13 @@ if (CLR_CMAKE_ENABLE_SANITIZERS) # define the preprocessor define ourselves. add_compile_definitions($<$:HAS_ADDRESS_SANITIZER>) - # Disable the use-after-return check for ASAN on Clang. This is because we have a lot of code that - # depends on the fact that our locals are not saved in a parallel stack, so we can't enable this today. - # If we ever have a way to detect a parallel stack and track its bounds, we can re-enable this check. - add_compile_options($<$:-fsanitize-address-use-after-return=never>) - add_compile_options($<$:-fsanitize-address-use-after-return=never>) + # Disable the use-after-return check for ASAN on release builds. We track the fake-stack used by use-after-return + # checking only on Debug/Checked builds. + # On Debug and Checked builds, always enable the use-after-return check to reduce code-size. + add_compile_options($<$,$>:-fsanitize-address-use-after-return=never>) + add_compile_options($<$,$>:-fsanitize-address-use-after-return=never>) + add_compile_options($<$,$>:-fsanitize-address-use-after-return=always>) + add_compile_options($<$,$>:-fsanitize-address-use-after-return=always>) endif() endif() diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h index e4a520c1dcf5c..b5c4bb7f25614 100644 --- a/src/coreclr/pal/inc/pal.h +++ b/src/coreclr/pal/inc/pal.h @@ -85,7 +85,6 @@ extern "C" { #include #include -#include // Native system libray handle. // On Unix systems, NATIVE_LIBRARY_HANDLE type represents a library handle not registered with the PAL. diff --git a/src/coreclr/pal/src/exception/machexception.cpp b/src/coreclr/pal/src/exception/machexception.cpp index 5f1bf97605784..3593ed72fdc8f 100644 --- a/src/coreclr/pal/src/exception/machexception.cpp +++ b/src/coreclr/pal/src/exception/machexception.cpp @@ -366,14 +366,16 @@ RestoreCompleteContext( ); #endif -__attribute__((noinline)) +__attribute__((noinline)) DISABLE_ASAN static void PAL_DispatchExceptionInner(PCONTEXT pContext, PEXCEPTION_RECORD pExRecord) { // Stash the inner context record into a local in a frame other than PAL_DispatchException // to ensure we have a compiler-defined callee stack frame state to record the context record // local before we call SEHProcessException. The instrumentation introduced by native sanitizers // doesn't interface that well with the fake caller frames we define for PAL_DispatchException, - // but they work fine for any callees of PAL_DispatchException. + // but they work fine for any callees of PAL_DispatchException. However, we still need to disable + // sanitizers for this function to avoid using any "fake stack" features + // that would break the "frame offset" calculations done below. CONTEXT *contextRecord = pContext; g_hardware_exception_context_locvar_offset = (int)((char*)&contextRecord - (char*)__builtin_frame_address(0)); diff --git a/src/coreclr/pal/src/exception/seh-unwind.cpp b/src/coreclr/pal/src/exception/seh-unwind.cpp index 71e94f929cf33..19386bc8b8aff 100644 --- a/src/coreclr/pal/src/exception/seh-unwind.cpp +++ b/src/coreclr/pal/src/exception/seh-unwind.cpp @@ -674,7 +674,7 @@ BOOL PAL_VirtualUnwind(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *contextP // cannot cross on some systems. if ((void*)curPc == g_InvokeActivationHandlerReturnAddress) { - CONTEXT* activationContext = (CONTEXT*)(CONTEXTGetFP(context) + g_inject_activation_context_locvar_offset); + CONTEXT* activationContext = *(CONTEXT**)(CONTEXTGetFP(context) + g_inject_activation_context_locvar_offset); memcpy_s(context, sizeof(CONTEXT), activationContext, sizeof(CONTEXT)); return TRUE; diff --git a/src/coreclr/pal/src/exception/signal.cpp b/src/coreclr/pal/src/exception/signal.cpp index 28ddb0dbee331..aaf1426511850 100644 --- a/src/coreclr/pal/src/exception/signal.cpp +++ b/src/coreclr/pal/src/exception/signal.cpp @@ -847,6 +847,19 @@ static void InvokeActivationHandler(CONTEXT *pWinContext) g_activationFunction(pWinContext); } +__attribute__((noinline)) +DISABLE_ASAN +static void HoldContextAndInvokeActivationHandler(CONTEXT* pWinContext) +{ + // We need to disable ASAN for this function as we use the frame offset to find the context when stackwalking. + // Some ASAN features use fake stacks, which is incompatible with this design. + CONTEXT* winContext = pWinContext; + g_inject_activation_context_locvar_offset = (int)((char*)&winContext - (char*)__builtin_frame_address(0)); + int savedErrNo = errno; // Make sure that errno is not modified + InvokeActivationHandler(winContext); + errno = savedErrNo; +} + /*++ Function : inject_activation_handler @@ -892,11 +905,7 @@ static void inject_activation_handler(int code, siginfo_t *siginfo, void *contex if (g_safeActivationCheckFunction(CONTEXTGetPC(&winContext))) { - g_inject_activation_context_locvar_offset = (int)((char*)&winContext - (char*)__builtin_frame_address(0)); - int savedErrNo = errno; // Make sure that errno is not modified - InvokeActivationHandler(&winContext); - errno = savedErrNo; - + HoldContextAndInvokeActivationHandler(&winContext); // Activation function may have modified the context, so update it. CONTEXTToNativeContext(&winContext, ucontext); } @@ -1011,6 +1020,22 @@ void PAL_IgnoreProfileSignal(int signalNum) #endif } +__attribute__((noinline)) +DISABLE_ASAN +static bool CallSEHProcessException(CONTEXT* pContext, PAL_SEHException* pException, ucontext_t* ucontext) +{ + // We need to disable ASAN for this function as we use the frame offset to find the context when stackwalking. + // Some ASAN features use fake stacks, which is incompatible with this design. + CONTEXT* winContext = pContext; + g_hardware_exception_context_locvar_offset = (int)((char*)&winContext - (char*)__builtin_frame_address(0)); + if (SEHProcessException(pException)) + { + // Exception handling may have modified the context, so update it. + CONTEXTToNativeContext(winContext, ucontext); + return true; + } + return false; +} /*++ Function : @@ -1036,12 +1061,10 @@ static bool common_signal_handler(int code, siginfo_t *siginfo, void *sigcontext #if !HAVE_MACH_EXCEPTIONS sigset_t signal_set; CONTEXT signalContextRecord; - CONTEXT* signalContextRecordPtr = &signalContextRecord; EXCEPTION_RECORD exceptionRecord; native_context_t *ucontext; ucontext = (native_context_t *)sigcontext; - g_hardware_exception_context_locvar_offset = (int)((char*)&signalContextRecordPtr - (char*)__builtin_frame_address(0)); if (code == (SIGSEGV | StackOverflowFlag)) { @@ -1094,14 +1117,10 @@ static bool common_signal_handler(int code, siginfo_t *siginfo, void *sigcontext // The exception object takes ownership of the exceptionRecord and contextRecord PAL_SEHException exception(&exceptionRecord, &signalContextRecord, true); - if (SEHProcessException(&exception)) - { - // Exception handling may have modified the context, so update it. - CONTEXTToNativeContext(exception.ExceptionPointers.ContextRecord, ucontext); - return true; - } -#endif // !HAVE_MACH_EXCEPTIONS + return CallSEHProcessException(&signalContextRecord, &exception, ucontext); +#else // !HAVE_MACH_EXCEPTIONS return false; +#endif // !HAVE_MACH_EXCEPTIONS } /*++ diff --git a/src/coreclr/pal/src/include/pal/palinternal.h b/src/coreclr/pal/src/include/pal/palinternal.h index 3fa16f38cfbe7..4eea43846c456 100644 --- a/src/coreclr/pal/src/include/pal/palinternal.h +++ b/src/coreclr/pal/src/include/pal/palinternal.h @@ -205,6 +205,7 @@ function_name() to call the system's implementation // https://gcc.gnu.org/ml/libstdc++/2016-01/msg00025.html #define _GLIBCXX_INCLUDE_NEXT_C_HEADERS 1 +#include #ifdef __APPLE__ #undef GetCurrentThread diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index 8d880b224f60c..ee436f4772e61 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -3182,7 +3182,7 @@ void UnwindFrameChain(Thread* pThread, LPVOID pvLimitSP) CONTRACTL_END; Frame* pFrame = pThread->m_pFrame; - if (pFrame < pvLimitSP) + if (pThread->IsStackPointerBefore(dac_cast(pFrame), dac_cast(pvLimitSP))) { GCX_COOP_THREAD_EXISTS(pThread); diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp index a30163e508f77..ffe025784dd02 100644 --- a/src/coreclr/vm/exceptionhandling.cpp +++ b/src/coreclr/vm/exceptionhandling.cpp @@ -339,7 +339,8 @@ StackWalkAction UpdateObjectRefInResumeContextCallback(CrawlFrame* pCF, LPVOID p if (pFrame->NeedsUpdateRegDisplay()) { - CONSISTENCY_CHECK(pFrame >= pState->pHighestFrameWithRegisters); + CONSISTENCY_CHECK(pCF->GetThread()->IsStackPointerBefore(dac_cast(pState->pHighestFrameWithRegisters), dac_cast(pFrame)) || + pState->pHighestFrameWithRegisters == pFrame); pState->pHighestFrameWithRegisters = pFrame; // Is this an InlinedCallFrame? @@ -395,7 +396,7 @@ bool ExceptionTracker::FindNonvolatileRegisterPointers(Thread* pThread, UINT_PTR Frame *pHighestFrameWithRegisters = NULL; Frame *pFrame = pThread->GetFrame(); - while ((UINT_PTR)pFrame < uOriginalSP) + while (pThread->IsStackPointerBefore(dac_cast(pFrame), uOriginalSP)) { if (pFrame->NeedsUpdateRegDisplay()) pHighestFrameWithRegisters = pFrame; @@ -1310,12 +1311,14 @@ ProcessCLRException(IN PEXCEPTION_RECORD pExceptionRecord, // INDEBUG(pTracker = (ExceptionTracker*)POISONC); + INDEBUG(DWORD exceptionCode = pExceptionRecord->ExceptionCode); + // Note that we should only fail to fix up for SO. bool fFixedUp = FixNonvolatileRegisters(uOriginalSP, pThread, pContextRecord, fAborting); - _ASSERTE(fFixedUp || (pExceptionRecord->ExceptionCode == STATUS_STACK_OVERFLOW)); + _ASSERTE(fFixedUp || (exceptionCode == STATUS_STACK_OVERFLOW)); - CONSISTENCY_CHECK(pLimitFrame > dac_cast(GetSP(pContextRecord))); + CONSISTENCY_CHECK(pThread->IsStackPointerBefore(dac_cast(GetSP(pContextRecord)), dac_cast(pLimitFrame))); if (pICFSetAsLimitFrame != NULL) { _ASSERTE(pICFSetAsLimitFrame == pLimitFrame); @@ -1910,7 +1913,7 @@ CLRUnwindStatus ExceptionTracker::ProcessOSExceptionNotification( cfThisFrame.SetCurGSCookie(Frame::SafeGetGSCookiePtr(pFrame)); } - while (((UINT_PTR)pFrame) < uCallerSP) + while (pThread->IsStackPointerBefore(dac_cast(pFrame), uCallerSP)) { // InlinedCallFrames (ICF) are allocated, initialized and linked to the Frame chain // by the code generated by the JIT for a method containing a PInvoke. @@ -1974,8 +1977,8 @@ CLRUnwindStatus ExceptionTracker::ProcessOSExceptionNotification( // // 1) ICF address is higher than the current frame's SP (which we get from DispatcherContext), AND // 2) ICF address is below callerSP. - if ((GetSP(pDispatcherContext->ContextRecord) < (TADDR)pICF) && - ((UINT_PTR)pICF < uCallerSP)) + if (pThread->IsStackPointerBefore(GetSP(pDispatcherContext->ContextRecord), (TADDR)pICF) && + pThread->IsStackPointerBefore((TADDR)pICF, uCallerSP)) { pICFForUnwindTarget = pFrame; @@ -3422,7 +3425,7 @@ CLRUnwindStatus ExceptionTracker::ProcessManagedCallFrame( #undef OPTIONAL_SO_CLEANUP_UNWIND -#define OPTIONAL_SO_CLEANUP_UNWIND(pThread, pFrame) if (pThread->GetFrame() < pFrame) { UnwindFrameChain(pThread, pFrame); } +#define OPTIONAL_SO_CLEANUP_UNWIND(pThread, pFrame) if (pThread->IsStackPointerBefore(dac_cast(pThread->GetFrame()), dac_cast(pFrame))) { UnwindFrameChain(pThread, pFrame); } typedef DWORD_PTR (HandlerFn)(UINT_PTR uStackFrame, Object* pExceptionObj); @@ -4116,7 +4119,7 @@ ExceptionTracker* ExceptionTracker::GetOrCreateTracker( { // In the second pass, there's a possibility that UMThunkUnwindFrameChainHandler() has // popped some frames off the frame chain underneath us. Check for this case here. - if (pTracker->m_pLimitFrame < pThread->GetFrame()) + if (pThread->IsStackPointerBefore(dac_cast(pTracker->m_pLimitFrame), dac_cast(pThread->GetFrame()))) { pTracker->ResetLimitFrame(); } @@ -4124,7 +4127,7 @@ ExceptionTracker* ExceptionTracker::GetOrCreateTracker( } } - _ASSERTE(pTracker->m_pLimitFrame >= pThread->GetFrame()); + _ASSERTE(!pThread->IsStackPointerBefore(dac_cast(pTracker->m_pLimitFrame), dac_cast(pThread->GetFrame()))); RETURN pTracker; } @@ -6123,7 +6126,7 @@ BOOL IsSafeToUnwindFrameChain(Thread* pThread, LPVOID MemoryStackFpForFrameChain { // Look for the last Frame to be removed that marks a managed-to-unmanaged transition Frame* pLastFrameOfInterest = FRAME_TOP; - for (Frame* pf = pThread->m_pFrame; pf < MemoryStackFpForFrameChain; pf = pf->PtrNextFrame()) + for (Frame* pf = pThread->m_pFrame; pThread->IsStackPointerBefore(dac_cast(pf), dac_cast(MemoryStackFpForFrameChain)); pf = pf->PtrNextFrame()) { PCODE retAddr = pf->GetReturnAddress(); if (retAddr != (PCODE)NULL && ExecutionManager::IsManagedCode(retAddr)) @@ -6178,9 +6181,9 @@ void CleanUpForSecondPass(Thread* pThread, bool fIsSO, LPVOID MemoryStackFpForFr // incorrectly removed a transition frame (more details in IsSafeToUnwindFrameChain) // [Do not perform the IsSafeToUnwindFrameChain() check in the SO case, since // IsSafeToUnwindFrameChain() requires a large amount of stack space.] - _ASSERTE(fIsSO || IsSafeToUnwindFrameChain(pThread, (Frame*)MemoryStackFpForFrameChain)); + _ASSERTE(fIsSO || IsSafeToUnwindFrameChain(pThread, MemoryStackFpForFrameChain)); - UnwindFrameChain(pThread, (Frame*)MemoryStackFpForFrameChain); + UnwindFrameChain(pThread, MemoryStackFpForFrameChain); // Only pop the trackers if this is not an SO. It's not safe to pop the trackers during EH for an SO. // Instead, we rely on the END_SO_TOLERANT_CODE macro to call ClearExceptionStateAfterSO(). Of course, diff --git a/src/coreclr/vm/frames.cpp b/src/coreclr/vm/frames.cpp index e6afc8995ef8e..0953d7b49c9d2 100644 --- a/src/coreclr/vm/frames.cpp +++ b/src/coreclr/vm/frames.cpp @@ -390,7 +390,7 @@ VOID Frame::Push(Thread *pThread) // with multiple Frames in coreclr.dll _ASSERTE((pThread->IsExecutingOnAltStack() || (m_Next == FRAME_TOP) || - (PBYTE(m_Next) + (2 * GetOsPageSize())) > PBYTE(this)) && + dac_cast(pThread->GetRealStackPointer(dac_cast(this))) < dac_cast(pThread->GetRealStackPointer(dac_cast(m_Next))) + (2 * GetOsPageSize())) && "Pushing a frame out of order ?"); _ASSERTE(// If AssertOnFailFast is set, the test expects to do stack overrun @@ -1004,7 +1004,7 @@ void GCFrame::Push(Thread* pThread) // So GetOsPageSize() is a guess of the maximum stack frame size of any method // with multiple GCFrames in coreclr.dll _ASSERTE(((m_Next == NULL) || - (PBYTE(m_Next) + (2 * GetOsPageSize())) > PBYTE(this)) && + dac_cast(pThread->GetRealStackPointer(dac_cast(this))) < dac_cast(pThread->GetRealStackPointer(dac_cast(m_Next))) + (2 * GetOsPageSize())) && "Pushing a GCFrame out of order ?"); pThread->SetGCFrame(this); diff --git a/src/coreclr/vm/stackwalk.cpp b/src/coreclr/vm/stackwalk.cpp index b333249637d37..fb960ff355618 100644 --- a/src/coreclr/vm/stackwalk.cpp +++ b/src/coreclr/vm/stackwalk.cpp @@ -1180,7 +1180,7 @@ BOOL StackFrameIterator::Init(Thread * pThread, // Walk the ExInfo chain, past any specified starting frame. m_exInfoWalk.Init(&(pThread->GetExceptionState()->m_currentExInfo)); // false means don't reset UseExInfoForStackwalk - m_exInfoWalk.WalkToPosition(dac_cast(m_pStartFrame), false); + m_exInfoWalk.WalkToPosition(dac_cast(pThread->GetRealStackPointer(m_pStartFrame)), false); #endif // ELIMINATE_FEF #ifdef FEATURE_EH_FUNCLETS @@ -1339,7 +1339,7 @@ BOOL StackFrameIterator::ResetRegDisp(PREGDISPLAY pRegDisp, while (m_crawl.pFrame != FRAME_TOP) { // this check is sufficient on WIN64 - if (dac_cast(m_crawl.pFrame) >= curSP) + if (m_pThread->IsStackPointerBefore(curSP, dac_cast(m_crawl.pFrame))) { #if defined(TARGET_X86) // check the IP @@ -2599,7 +2599,7 @@ StackWalkAction StackFrameIterator::NextRaw(void) // FaultingExceptionFrame is special case where it gets // pushed on the stack after the frame is running _ASSERTE((m_crawl.pFrame == FRAME_TOP) || - ((TADDR)GetRegdisplaySP(m_crawl.pRD) < dac_cast(m_crawl.pFrame)) || + m_pThread->IsStackPointerBefore((TADDR)GetRegdisplaySP(m_crawl.pRD), dac_cast(m_crawl.pFrame)) || (m_crawl.pFrame->GetVTablePtr() == FaultingExceptionFrame::GetMethodFrameVPtr())); #endif // !defined(ELIMINATE_FEF) @@ -2646,9 +2646,9 @@ StackWalkAction StackFrameIterator::NextRaw(void) PTR_VOID newSP = PTR_VOID((TADDR)GetRegdisplaySP(m_crawl.pRD)); #ifndef NO_FIXED_STACK_LIMIT - FAIL_IF_SPECULATIVE_WALK(m_crawl.pThread->IsExecutingOnAltStack() || newSP >= m_crawl.pThread->GetCachedStackLimit()); + FAIL_IF_SPECULATIVE_WALK(m_crawl.pThread->IsExecutingOnAltStack() || m_crawl.pThread->IsStackPointerBefore(dac_cast(m_crawl.pThread->GetCachedStackLimit()), dac_cast(newSP))); #endif // !NO_FIXED_STACK_LIMIT - FAIL_IF_SPECULATIVE_WALK(m_crawl.pThread->IsExecutingOnAltStack() || newSP < m_crawl.pThread->GetCachedStackBase()); + FAIL_IF_SPECULATIVE_WALK(m_crawl.pThread->IsExecutingOnAltStack() || m_crawl.pThread->IsStackPointerBefore(dac_cast(newSP), dac_cast(m_crawl.pThread->GetCachedStackBase()))); #undef FAIL_IF_SPECULATIVE_WALK @@ -2943,7 +2943,7 @@ void StackFrameIterator::ProcessCurrentFrame(void) // the pContext. // There are still a few cases in which a FaultingExceptionFrame is linked in. If // the next frame is one of them, we don't want to override it. THIS IS PROBABLY BAD!!! - if ( (pContextSP < dac_cast(m_crawl.pFrame)) && + if ( m_pThread->IsStackPointerBefore(pContextSP, dac_cast(m_crawl.pFrame)) && ((m_crawl.GetFrame() == FRAME_TOP) || (m_crawl.GetFrame()->GetVTablePtr() != FaultingExceptionFrame::GetMethodFrameVPtr() ) ) ) { @@ -3084,7 +3084,7 @@ BOOL StackFrameIterator::CheckForSkippedFrames(void) #endif // PROCESS_EXPLICIT_FRAME_BEFORE_MANAGED_FRAME if ( !( (m_crawl.pFrame != FRAME_TOP) && - (dac_cast(m_crawl.pFrame) < pvReferenceSP) ) + m_pThread->IsStackPointerBefore(dac_cast(m_crawl.pFrame), pvReferenceSP) ) ) { return FALSE; @@ -3095,7 +3095,7 @@ BOOL StackFrameIterator::CheckForSkippedFrames(void) // We might have skipped past some Frames. // This happens with InlinedCallFrames. while ( (m_crawl.pFrame != FRAME_TOP) && - (dac_cast(m_crawl.pFrame) < pvReferenceSP) + m_pThread->IsStackPointerBefore(dac_cast(m_crawl.pFrame), pvReferenceSP) ) { BOOL fReportInteropMD = diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 8c60b2b5a7982..0029d969b4963 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -48,6 +48,8 @@ #include #endif +#include + static const PortableTailCallFrame g_sentinelTailCallFrame = { NULL, NULL }; TailCallTls::TailCallTls() @@ -225,7 +227,7 @@ void Thread::SetFrame(Frame *pFrame) if (g_pConfig->fAssertOnFailFast() == false) return; - Frame* espVal = (Frame*)GetCurrentSP(); + void* espVal = (void*)GetCurrentSP(); while (pFrame != (Frame*) -1) { @@ -233,8 +235,14 @@ void Thread::SetFrame(Frame *pFrame) if (pFrame == stopFrame) _ASSERTE(!"SetFrame frame == stopFrame"); - _ASSERTE(IsExecutingOnAltStack() || espVal < pFrame); - _ASSERTE(IsExecutingOnAltStack() || pFrame < m_CacheStackBase); + if (!IsExecutingOnAltStack()) + { +#if defined(DEBUG) && defined(HAS_ADDRESS_SANITIZER) + _ASSERTE(__asan_addr_is_in_fake_stack(m_fakeStack, pFrame, nullptr, nullptr) || (espVal < pFrame && pFrame < m_CacheStackBase)); +#else + _ASSERTE(espVal < pFrame && pFrame < m_CacheStackBase); +#endif + } _ASSERTE(pFrame->GetFrameType() < Frame::TYPE_COUNT); pFrame = pFrame->m_Next; @@ -6232,6 +6240,10 @@ BOOL Thread::SetStackLimits(SetStackLimitScope scope) } CONTRACTL_END; +#if defined(DEBUG) && defined(HAS_ADDRESS_SANITIZER) + m_fakeStack = __asan_get_current_fake_stack(); +#endif + if (scope == fAll) { m_CacheStackBase = GetStackUpperBound(); @@ -7558,7 +7570,7 @@ Frame * Thread::NotifyFrameChainOfExceptionUnwind(Frame* pStartFrame, LPVOID pvL while (pFrame < pvLimitSP) { CONSISTENCY_CHECK(pFrame != PTR_NULL); - CONSISTENCY_CHECK((pFrame) > static_cast((LPVOID)GetCurrentSP())); + CONSISTENCY_CHECK(IsStackPointerBefore(reinterpret_cast(static_cast((LPVOID)GetCurrentSP())), reinterpret_cast(pFrame))); pFrame->ExceptionUnwind(); pFrame = pFrame->Next(); } diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h index 6aa3e04b00465..6d62483f84b76 100644 --- a/src/coreclr/vm/threads.h +++ b/src/coreclr/vm/threads.h @@ -1182,9 +1182,7 @@ class Thread WRAPPER_NO_CONTRACT; if (this == GetThreadNULLOk()) { - void* curSP; - curSP = (void *)GetCurrentSP(); - _ASSERTE(IsExecutingOnAltStack() || (curSP <= m_pFrame && m_pFrame < m_CacheStackBase) || m_pFrame == (Frame*) -1); + _ASSERTE(IsExecutingOnAltStack() || IsAddressInCurrentStack(m_pFrame) || m_pFrame == (Frame*) -1); } #endif @@ -1202,9 +1200,7 @@ class Thread WRAPPER_NO_CONTRACT; if (this == GetThreadNULLOk()) { - void* curSP; - curSP = (void *)GetCurrentSP(); - _ASSERTE((m_pGCFrame == NULL) || (curSP <= m_pGCFrame && m_pGCFrame < m_CacheStackBase)); + _ASSERTE((m_pGCFrame == NULL) || IsAddressInCurrentStack(m_pGCFrame)); } #endif @@ -2439,6 +2435,13 @@ class Thread PTR_VOID m_CacheStackLimit; UINT_PTR m_CacheStackSufficientExecutionLimit; UINT_PTR m_CacheStackStackAllocNonRiskyExecutionLimit; +#ifdef DEBUG + // A pointer that represents the fake stack for the current thread. + // Currently used only by AddressSanitizer when use-after-return validation is enabled. + // We include it on all debug builds to ensure that the DAC's implementation of Thread has the same layout + // as the runtime's implementation. + PTR_VOID m_fakeStack; +#endif #define HARD_GUARD_REGION_SIZE GetOsPageSize() @@ -2494,6 +2497,12 @@ class Thread BOOL IsAddressInStack (PTR_VOID addr) const { +#if defined(DEBUG) && defined(HAS_ADDRESS_SANITIZER) + if (__asan_addr_is_in_fake_stack((void*)PTR_TO_TADDR(m_fakeStack), (void*)PTR_TO_TADDR(addr), nullptr, nullptr)) + { + return TRUE; + } +#endif // HAS_ADDRESS_SANITIZER LIMITED_METHOD_DAC_CONTRACT; _ASSERTE(m_CacheStackBase != NULL); _ASSERTE(m_CacheStackLimit != NULL); @@ -2510,12 +2519,36 @@ class Thread return FALSE; } +#if defined(DEBUG) && defined(HAS_ADDRESS_SANITIZER) + if (__asan_addr_is_in_fake_stack((void*)PTR_TO_TADDR(currentThread->m_fakeStack), (void*)PTR_TO_TADDR(addr), nullptr, nullptr)) + { + return TRUE; + } +#endif // HAS_ADDRESS_SANITIZER + PTR_VOID sp = dac_cast(GetCurrentSP()); _ASSERTE(currentThread->m_CacheStackBase != NULL); _ASSERTE(sp < currentThread->m_CacheStackBase); return sp < addr && addr <= currentThread->m_CacheStackBase; } + BOOL IsStackPointerBefore(TADDR sp1, TADDR sp2) + { + sp1 = dac_cast(GetRealStackPointer(dac_cast(sp1))); + sp2 = dac_cast(GetRealStackPointer(dac_cast(sp2))); + return sp1 < sp2; + } + + PTR_VOID GetRealStackPointer(PTR_VOID addr) + { +#if defined(DEBUG) && defined(HAS_ADDRESS_SANITIZER) + void* realAddr = __asan_addr_is_in_fake_stack((void*)PTR_TO_TADDR(m_fakeStack), (void*)PTR_TO_TADDR(addr), nullptr, nullptr); + return realAddr ? dac_cast(realAddr) : addr; +#else + return addr; +#endif + } + // DetermineIfGuardPagePresent returns TRUE if the thread's stack contains a proper guard page. This function // makes a physical check of the stack, rather than relying on whether or not the CLR is currently processing a // stack overflow exception. diff --git a/src/native/minipal/utils.h b/src/native/minipal/utils.h index 768de9e48b4b2..30a605fde10b9 100644 --- a/src/native/minipal/utils.h +++ b/src/native/minipal/utils.h @@ -74,14 +74,7 @@ #endif #if defined(HAS_ADDRESS_SANITIZER) -# ifdef __cplusplus - extern "C" - { -# endif - void SANITIZER_INTERFACE_CALLCONV __asan_handle_no_return(void); -# ifdef __cplusplus - } -# endif +#include #elif defined(__llvm__) # pragma clang diagnostic push # ifdef COMPILER_SUPPORTS_W_RESERVED_IDENTIFIER