Skip to content

Commit

Permalink
Fix preventing memory allocation in signal handler
Browse files Browse the repository at this point in the history
There was a subtle bug. When the hardware exception handler returns back
to the signal handler, the exception's CONTEXT record may contain
modified registers and so the changes need to be propagated back to the
signal context. But the recent change dotnet#16384 was restoring the signal
context from the originally grabbed context instead of the one that's
pointed to by the exception, which is different.

I have also added a little optimization - the contextRecord that was
added is not needed, since the signalContextRecord can be used as the
initial context record for the exception. So we can save the
contextRecord and also copying to the signalContextRecord from it.
  • Loading branch information
janvorli committed Feb 22, 2018
1 parent ba1d5b2 commit 0ee2850
Showing 1 changed file with 3 additions and 6 deletions.
9 changes: 3 additions & 6 deletions src/pal/src/exception/signal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,6 @@ static bool common_signal_handler(int code, siginfo_t *siginfo, void *sigcontext
{
sigset_t signal_set;
CONTEXT signalContextRecord;
CONTEXT contextRecord;
EXCEPTION_RECORD exceptionRecord;
native_context_t *ucontext;

Expand Down Expand Up @@ -890,17 +889,15 @@ 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;

memcpy_s(&signalContextRecord, sizeof(CONTEXT), &contextRecord, sizeof(CONTEXT));
signalContextRecord.ContextFlags |= CONTEXT_EXCEPTION_ACTIVE;

// The exception object takes ownership of the exceptionRecord and contextRecord
PAL_SEHException exception(&exceptionRecord, &contextRecord, true);
PAL_SEHException exception(&exceptionRecord, &signalContextRecord, true);

if (SEHProcessException(&exception))
{
// Exception handling may have modified the context, so update it.
CONTEXTToNativeContext(&contextRecord, ucontext);
CONTEXTToNativeContext(exception.ExceptionPointers.contextRecord, ucontext);
return true;
}

Expand Down

0 comments on commit 0ee2850

Please sign in to comment.