Merge pull request #14095 from jordan-woyak/sigsegv_handler-cleanup

MemTools: Clean up SIGSEGV handler.

1 files changed, 30 insertions, 52 deletions

diff --git a/Source/Core/Core/MemTools.cpp b/Source/Core/Core/MemTools.cpp
index 75d20dd6a9..f9e7eef64d 100644
--- a/Source/Core/Core/MemTools.cpp
+++ b/Source/Core/Core/MemTools.cpp
@@ -258,73 +258,51 @@ bool IsExceptionHandlerSupported()
 #elif defined(_POSIX_VERSION) && !defined(_M_GENERIC)
 
 static struct sigaction old_sa_segv;
+#if defined(__APPLE__)
 static struct sigaction old_sa_bus;
+#endif
 
 static void sigsegv_handler(int sig, siginfo_t* info, void* raw_context)
 {
-  if (sig != SIGSEGV && sig != SIGBUS)
+  if (sig != SIGSEGV
+#if defined(__APPLE__)
+      && sig != SIGBUS
+#endif
+  )
   {
     // We are not interested in other signals - handle it as usual.
     return;
   }
-  ucontext_t* context = (ucontext_t*)raw_context;
-  int sicode = info->si_code;
+  auto* const context = static_cast<ucontext_t*>(raw_context);
+  const int sicode = info->si_code;
   if (sicode != SEGV_MAPERR && sicode != SEGV_ACCERR)
   {
     // Huh? Return.
     return;
   }
-  uintptr_t bad_address = (uintptr_t)info->si_addr;
+  const auto bad_address = reinterpret_cast<uintptr_t>(info->si_addr);
 
 // Get all the information we can out of the context.
 #ifdef __OpenBSD__
-  ucontext_t* ctx = context;
+  SContext* const ctx = context;
+#elif defined(__APPLE__)
+  // `uc_mcontext` is already a pointer here.
+  SContext* const ctx = context->uc_mcontext;
 #else
-  mcontext_t* ctx = &context->uc_mcontext;
+  SContext* const ctx = &context->uc_mcontext;
 #endif
-  // assume it's not a write
-  if (!Core::System::GetInstance().GetJitInterface().HandleFault(bad_address,
-#ifdef __APPLE__
-                                                                 *ctx
-#else
-                                                                 ctx
+  if (Core::System::GetInstance().GetJitInterface().HandleFault(bad_address, ctx))
+    return;
+
+  // If JIT didn't handle the signal, restore the original handler and invoke it.
+  const auto& old_sa =
+#if defined(__APPLE__)
+      (sig == SIGBUS) ? old_sa_bus :
 #endif
-                                                                 ))
-  {
-    // retry and crash
-    // According to the sigaction man page, if sa_flags "SA_SIGINFO" is set to the sigaction
-    // function pointer, otherwise sa_handler contains one of:
-    // SIG_DEF: The 'default' action is performed
-    // SIG_IGN: The signal is ignored
-    // Any other value is a function pointer to a signal handler
-
-    struct sigaction* old_sa;
-    if (sig == SIGSEGV)
-    {
-      old_sa = &old_sa_segv;
-    }
-    else
-    {
-      old_sa = &old_sa_bus;
-    }
+                        old_sa_segv;
 
-    if (old_sa->sa_flags & SA_SIGINFO)
-    {
-      old_sa->sa_sigaction(sig, info, raw_context);
-      return;
-    }
-    if (old_sa->sa_handler == SIG_DFL)
-    {
-      signal(sig, SIG_DFL);
-      return;
-    }
-    if (old_sa->sa_handler == SIG_IGN)
-    {
-      // Ignore signal
-      return;
-    }
-    old_sa->sa_handler(sig);
-  }
+  sigaction(sig, &old_sa, nullptr);
+  raise(sig);
 }
 
 void InstallExceptionHandler()
@@ -337,10 +315,9 @@ void InstallExceptionHandler()
 #endif
   signal_stack.ss_size = SIGSTKSZ;
   signal_stack.ss_flags = 0;
-  if (sigaltstack(&signal_stack, nullptr))
-    PanicAlertFmt("sigaltstack failed");
-  struct sigaction sa;
-  sa.sa_handler = nullptr;
+  if (sigaltstack(&signal_stack, nullptr) != 0)
+    PanicAlertFmt("sigaltstack failed: {}", Common::LastStrerrorString());
+  struct sigaction sa{};
   sa.sa_sigaction = &sigsegv_handler;
   sa.sa_flags = SA_SIGINFO;
   sigemptyset(&sa.sa_mask);
@@ -352,7 +329,8 @@ void InstallExceptionHandler()
 
 void UninstallExceptionHandler()
 {
-  stack_t signal_stack, old_stack;
+  stack_t signal_stack;
+  stack_t old_stack;
   signal_stack.ss_flags = SS_DISABLE;
   if (!sigaltstack(&signal_stack, &old_stack) && !(old_stack.ss_flags & SS_DISABLE))
   {