Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support sigaction with SIG_DFL and SA_SIGINFO. #55673

Merged
merged 1 commit into from
Jul 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 46 additions & 29 deletions src/coreclr/pal/src/exception/signal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,26 @@ bool IsRunningOnAlternateStack(void *context)
#endif // HAVE_MACH_EXCEPTIONS
}

static bool IsSaSigInfo(struct sigaction* action)
{
return (action->sa_flags & SA_SIGINFO) != 0;
}

static bool IsSigDfl(struct sigaction* action)
{
// macOS can return sigaction with SIG_DFL and SA_SIGINFO.
// SA_SIGINFO means we should use sa_sigaction, but here we want to check sa_handler.
// So we ignore SA_SIGINFO when sa_sigaction and sa_handler are at the same address.
return (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nit - can you please add ( ) to explicitly express the precedence in all the composite conditions? It is a preferred style in this codebase. Like this:

return ((&action->sa_handler == (void*)&action->sa_sigaction) || !IsSaSigInfo(action)) &&
(action->sa_handler == SIG_DFL));

action->sa_handler == SIG_DFL;
}

static bool IsSigIgn(struct sigaction* action)
{
return (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) &&
action->sa_handler == SIG_IGN;
}

/*++
Function :
invoke_previous_action
Expand All @@ -363,44 +383,41 @@ static void invoke_previous_action(struct sigaction* action, int code, siginfo_t
{
_ASSERTE(action != NULL);

if (action->sa_flags & SA_SIGINFO)
if (IsSigIgn(action))
{
// Directly call the previous handler.
_ASSERTE(action->sa_sigaction != NULL);
action->sa_sigaction(code, siginfo, context);
}
else
{
if (action->sa_handler == SIG_IGN)
if (signalRestarts)
{
if (signalRestarts)
{
// This signal mustn't be ignored because it will be restarted.
PROCAbort(code);
}
return;
// This signal mustn't be ignored because it will be restarted.
PROCAbort(code);
}
else if (action->sa_handler == SIG_DFL)
return;
}
else if (IsSigDfl(action))
{
if (signalRestarts)
{
if (signalRestarts)
{
// Restore the original and restart h/w exception.
restore_signal(code, action);
}
else
{
// We can't invoke the original handler because returning from the
// handler doesn't restart the exception.
PROCAbort(code);
}
// Restore the original and restart h/w exception.
restore_signal(code, action);
}
else
{
// Directly call the previous handler.
_ASSERTE(action->sa_handler != NULL);
action->sa_handler(code);
// We can't invoke the original handler because returning from the
// handler doesn't restart the exception.
PROCAbort(code);
}
}
else if (IsSaSigInfo(action))
{
// Directly call the previous handler.
_ASSERTE(action->sa_sigaction != NULL);
action->sa_sigaction(code, siginfo, context);
}
else
{
// Directly call the previous handler.
_ASSERTE(action->sa_handler != NULL);
action->sa_handler(code);
}

PROCNotifyProcessShutdown(IsRunningOnAlternateStack(context));

Expand Down
33 changes: 21 additions & 12 deletions src/libraries/Native/Unix/System.Native/pal_signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,21 @@ static bool IsSaSigInfo(struct sigaction* action)
#pragma clang diagnostic pop
}

static bool IsSigIgn(struct sigaction* action)
static bool IsSigDfl(struct sigaction* action)
{
assert(action);
return !IsSaSigInfo(action) && action->sa_handler == SIG_IGN;
// macOS can return sigaction with SIG_DFL and SA_SIGINFO.
// SA_SIGINFO means we should use sa_sigaction, but here we want to check sa_handler.
// So we ignore SA_SIGINFO when sa_sigaction and sa_handler are at the same address.
return (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) &&
action->sa_handler == SIG_DFL;
}

static bool IsSigDfl(struct sigaction* action)
static bool IsSigIgn(struct sigaction* action)
{
assert(action);
return !IsSaSigInfo(action) && action->sa_handler == SIG_DFL;
return (&action->sa_handler == (void*)&action->sa_sigaction || !IsSaSigInfo(action)) &&
action->sa_handler == SIG_IGN;
}

static bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal)
Expand Down Expand Up @@ -209,15 +214,19 @@ static void SignalHandler(int sig, siginfo_t* siginfo, void* context)
if (!IsCancelableTerminationSignal(sig))
{
struct sigaction* origHandler = OrigActionFor(sig);
if (IsSaSigInfo(origHandler))
{
assert(origHandler->sa_sigaction);
origHandler->sa_sigaction(sig, siginfo, context);
}
else if (origHandler->sa_handler != SIG_IGN &&
origHandler->sa_handler != SIG_DFL)
if (!IsSigDfl(origHandler) && !IsSigIgn(origHandler))
{
origHandler->sa_handler(sig);
if (IsSaSigInfo(origHandler))
{
assert(origHandler->sa_sigaction);
origHandler->sa_sigaction(sig, siginfo, context);
}
else
{
assert(origHandler->sa_handler);
origHandler->sa_handler(sig);
}

}
}

Expand Down