Revert "Prevent memory allocation in signal handler (dotnet#16384)"

This reverts commit d9753f4.
janvorli · Feb 22, 2018 · 81addc0 · 81addc0
1 parent ba1d5b2
commit 81addc0
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 54 deletions.
diff --git 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
@@ -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
@@ -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;
     }