From 81addc0992dd02b3689fe36e8cba7961af13d299 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 22 Feb 2018 01:31:50 +0100 Subject: [PATCH] Revert "Prevent memory allocation in signal handler (#16384)" This reverts commit d9753f42e8c77688f2e560febf378b2c190d8b02. --- src/pal/inc/pal.h | 8 ++------ src/pal/src/exception/seh.cpp | 34 -------------------------------- src/pal/src/exception/signal.cpp | 30 +++++++++++++++------------- 3 files changed, 18 insertions(+), 54 deletions(-) diff --git a/src/pal/inc/pal.h b/src/pal/inc/pal.h index affe5556ddc3..6585435bbb50 100644 --- a/src/pal/inc/pal.h +++ b/src/pal/inc/pal.h @@ -5691,14 +5691,13 @@ struct PAL_SEHException ExceptionPointers.ExceptionRecord = ex.ExceptionPointers.ExceptionRecord; ExceptionPointers.ContextRecord = ex.ExceptionPointers.ContextRecord; TargetFrameSp = ex.TargetFrameSp; - RecordsOnStack = ex.RecordsOnStack; ex.Clear(); } void FreeRecords() { - if (ExceptionPointers.ExceptionRecord != NULL && !RecordsOnStack ) + if (ExceptionPointers.ExceptionRecord != NULL) { PAL_FreeExceptionRecords(ExceptionPointers.ExceptionRecord, ExceptionPointers.ContextRecord); ExceptionPointers.ExceptionRecord = NULL; @@ -5710,14 +5709,12 @@ struct PAL_SEHException EXCEPTION_POINTERS ExceptionPointers; // Target frame stack pointer set before the 2nd pass. SIZE_T TargetFrameSp; - bool RecordsOnStack; - PAL_SEHException(EXCEPTION_RECORD *pExceptionRecord, CONTEXT *pContextRecord, bool onStack = false) + PAL_SEHException(EXCEPTION_RECORD *pExceptionRecord, CONTEXT *pContextRecord) { ExceptionPointers.ExceptionRecord = pExceptionRecord; ExceptionPointers.ContextRecord = pContextRecord; TargetFrameSp = NoTargetFrameSp; - RecordsOnStack = onStack; } PAL_SEHException() @@ -5753,7 +5750,6 @@ struct PAL_SEHException ExceptionPointers.ExceptionRecord = NULL; ExceptionPointers.ContextRecord = NULL; TargetFrameSp = NoTargetFrameSp; - RecordsOnStack = false; } CONTEXT* GetContextRecord() diff --git a/src/pal/src/exception/seh.cpp b/src/pal/src/exception/seh.cpp index b05707076b8a..73d8ce505b13 100644 --- a/src/pal/src/exception/seh.cpp +++ b/src/pal/src/exception/seh.cpp @@ -202,38 +202,6 @@ void ThrowExceptionHelper(PAL_SEHException* ex) throw std::move(*ex); } -/*++ -Function: - EnsureExceptionRecordsOnHeap - - Helper function to move records from stack to heap. - -Parameters: - PAL_SEHException* exception ---*/ -static void EnsureExceptionRecordsOnHeap(PAL_SEHException* exception) -{ - if( !exception->RecordsOnStack || - exception->ExceptionPointers.ExceptionRecord == NULL ) - { - return; - } - - CONTEXT* contextRecord = exception->ExceptionPointers.ContextRecord; - EXCEPTION_RECORD* exceptionRecord = exception->ExceptionPointers.ExceptionRecord; - - CONTEXT* contextRecordCopy; - EXCEPTION_RECORD* exceptionRecordCopy; - AllocateExceptionRecords(&exceptionRecordCopy, &contextRecordCopy); - - *exceptionRecordCopy = *exceptionRecord; - *contextRecordCopy = *contextRecord; - - exception->ExceptionPointers.ExceptionRecord = exceptionRecordCopy; - exception->ExceptionPointers.ContextRecord = contextRecordCopy; - exception->RecordsOnStack = false; -} - /*++ Function: SEHProcessException @@ -282,7 +250,6 @@ SEHProcessException(PAL_SEHException* exception) } } - EnsureExceptionRecordsOnHeap(exception); if (g_hardwareExceptionHandler(exception)) { // The exception happened in managed code and the execution should continue. @@ -295,7 +262,6 @@ SEHProcessException(PAL_SEHException* exception) if (CatchHardwareExceptionHolder::IsEnabled()) { - EnsureExceptionRecordsOnHeap(exception); PAL_ThrowExceptionFromContext(exception->GetContextRecord(), exception); } } diff --git a/src/pal/src/exception/signal.cpp b/src/pal/src/exception/signal.cpp index 6748d54f0fe4..96dca507063f 100644 --- a/src/pal/src/exception/signal.cpp +++ b/src/pal/src/exception/signal.cpp @@ -845,30 +845,32 @@ static bool common_signal_handler(int code, siginfo_t *siginfo, void *sigcontext { sigset_t signal_set; CONTEXT signalContextRecord; - CONTEXT contextRecord; - EXCEPTION_RECORD exceptionRecord; + CONTEXT *contextRecord; + EXCEPTION_RECORD *exceptionRecord; native_context_t *ucontext; ucontext = (native_context_t *)sigcontext; g_common_signal_handler_context_locvar_offset = (int)((char*)&signalContextRecord - (char*)__builtin_frame_address(0)); - exceptionRecord.ExceptionCode = CONTEXTGetExceptionCodeForSignal(siginfo, ucontext); - exceptionRecord.ExceptionFlags = EXCEPTION_IS_SIGNAL; - exceptionRecord.ExceptionRecord = NULL; - exceptionRecord.ExceptionAddress = GetNativeContextPC(ucontext); - exceptionRecord.NumberParameters = numParams; + AllocateExceptionRecords(&exceptionRecord, &contextRecord); + + exceptionRecord->ExceptionCode = CONTEXTGetExceptionCodeForSignal(siginfo, ucontext); + exceptionRecord->ExceptionFlags = EXCEPTION_IS_SIGNAL; + exceptionRecord->ExceptionRecord = NULL; + exceptionRecord->ExceptionAddress = GetNativeContextPC(ucontext); + exceptionRecord->NumberParameters = numParams; va_list params; va_start(params, numParams); for (int i = 0; i < numParams; i++) { - exceptionRecord.ExceptionInformation[i] = va_arg(params, size_t); + exceptionRecord->ExceptionInformation[i] = va_arg(params, size_t); } // Pre-populate context with data from current frame, because ucontext doesn't have some data (e.g. SS register) // which is required for restoring context - RtlCaptureContext(&contextRecord); + RtlCaptureContext(contextRecord); ULONG contextFlags = CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_FLOATING_POINT; @@ -879,7 +881,7 @@ static bool common_signal_handler(int code, siginfo_t *siginfo, void *sigcontext // Fill context record with required information. from pal.h: // On non-Win32 platforms, the CONTEXT pointer in the // PEXCEPTION_POINTERS will contain at least the CONTEXT_CONTROL registers. - CONTEXTFromNativeContext(ucontext, &contextRecord, contextFlags); + CONTEXTFromNativeContext(ucontext, contextRecord, contextFlags); /* Unmask signal so we can receive it again */ sigemptyset(&signal_set); @@ -890,17 +892,17 @@ static bool common_signal_handler(int code, siginfo_t *siginfo, void *sigcontext ASSERT("pthread_sigmask failed; error number is %d\n", sigmaskRet); } - contextRecord.ContextFlags |= CONTEXT_EXCEPTION_ACTIVE; + contextRecord->ContextFlags |= CONTEXT_EXCEPTION_ACTIVE; - memcpy_s(&signalContextRecord, sizeof(CONTEXT), &contextRecord, sizeof(CONTEXT)); + memcpy_s(&signalContextRecord, sizeof(CONTEXT), contextRecord, sizeof(CONTEXT)); // The exception object takes ownership of the exceptionRecord and contextRecord - PAL_SEHException exception(&exceptionRecord, &contextRecord, true); + PAL_SEHException exception(exceptionRecord, contextRecord); if (SEHProcessException(&exception)) { // Exception handling may have modified the context, so update it. - CONTEXTToNativeContext(&contextRecord, ucontext); + CONTEXTToNativeContext(contextRecord, ucontext); return true; }