[trap-handler] Clean up code to prepare additions

This CL cleans up the trap handler code on POSIX before making additions
for arm64 simulator support.
In particular,
  - it extends a comment about restoring the signal mask
    before restoring the "thread in wasm" flag, and fixes the code to
    actually implement that again;
  - it renames "SigUnmaskStack" to "UnmaskOobSignalScope", to make the
    intent clear, and it moves the signal masking code to the
    constructor of that class;
  - it replaces a call to "IsThreadInWasm" by just reading
    "g_thread_in_wasm_code" to make it more transparent what is
    happening (note that the next instruction will just write to that
    flag);
  - it replaces an if block by another early exit for consistency; and
    lastly
  - it avoids curly braces for single-line conditions, to increase
    readability and to match the rest of V8.

R=ahaas@chromium.org, mseaborn@chromium.org

Bug: v8:11955
Change-Id: I023381f8b8e4640e2b21ac617fe301ec9f130783
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3015562
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75697}

src/trap-handler/handler-inside-posix.cc

@@ -52,18 +52,21 @@ bool IsKernelGeneratedSignal(siginfo_t* info) {
          info->si_code != SI_ASYNCIO && info->si_code != SI_MESGQ;
 }
 
-class SigUnmaskStack {
+class UnmaskOobSignalScope {
  public:
-  explicit SigUnmaskStack(sigset_t sigs) {
-    // TODO(eholk): consider using linux-syscall-support for calling this
-    // syscall.
+  UnmaskOobSignalScope() {
+    sigset_t sigs;
+    // Fortunately, sigemptyset and sigaddset are async-signal-safe according to
+    // the POSIX standard.
+    sigemptyset(&sigs);
+    sigaddset(&sigs, kOobSignal);
     pthread_sigmask(SIG_UNBLOCK, &sigs, &old_mask_);
   }
 
-  SigUnmaskStack(const SigUnmaskStack&) = delete;
-  void operator=(const SigUnmaskStack&) = delete;
+  UnmaskOobSignalScope(const UnmaskOobSignalScope&) = delete;
+  void operator=(const UnmaskOobSignalScope&) = delete;
 
-  ~SigUnmaskStack() { pthread_sigmask(SIG_SETMASK, &old_mask_, nullptr); }
+  ~UnmaskOobSignalScope() { pthread_sigmask(SIG_SETMASK, &old_mask_, nullptr); }
 
  private:
   sigset_t old_mask_;
@@ -74,36 +77,29 @@ bool TryHandleSignal(int signum, siginfo_t* info, void* context) {
   // the first check in the trap handler to guarantee that the IsThreadInWasm
   // flag is only set in wasm code. Otherwise a later signal handler is executed
   // with the flag set.
-  if (!IsThreadInWasm()) {
-    return false;
-  }
+  if (!g_thread_in_wasm_code) return false;
 
   // Clear g_thread_in_wasm_code, primarily to protect against nested faults.
+  // The only path that resets the flag to true is if we find a landing pad (in
+  // which case this function returns true). Otherwise we leave the flag unset
+  // since we do not return to wasm code.
   g_thread_in_wasm_code = false;
 
   // Bail out early in case we got called for the wrong kind of signal.
-
-  if (signum != kOobSignal) {
-    return false;
-  }
+  if (signum != kOobSignal) return false;
 
   // Make sure the signal was generated by the kernel and not some other source.
-  if (!IsKernelGeneratedSignal(info)) {
-    return false;
-  }
-
-  // Begin signal mask scope. We need to be sure to restore the signal mask
-  // before we restore the g_thread_in_wasm_code flag.
+  if (!IsKernelGeneratedSignal(info)) return false;
+
+  // Unmask the oob signal, which is automatically masked during the execution
+  // of this handler. This ensures that crashes generated in this function will
+  // be handled by the crash reporter. Otherwise, the process might be killed
+  // with the crash going unreported. The scope object makes sure to restore the
+  // signal mask on return from this function. We put the scope object in a
+  // separate block to ensure that we restore the signal mask before we restore
+  // the g_thread_in_wasm_code flag.
   {
-    // Unmask the signal so that if this signal handler crashes, the crash will
-    // be handled by the crash reporter.  Otherwise, the process might be killed
-    // with the crash going unreported.
-    sigset_t sigs;
-    // Fortunately, sigemptyset and sigaddset are async-signal-safe according to
-    // the POSIX standard.
-    sigemptyset(&sigs);
-    sigaddset(&sigs, kOobSignal);
-    SigUnmaskStack unmask(sigs);
+    UnmaskOobSignalScope unmask_oob_signal;
 
     ucontext_t* uc = reinterpret_cast<ucontext_t*>(context);
 #if V8_OS_LINUX && V8_TARGET_ARCH_X64
@@ -119,19 +115,17 @@ bool TryHandleSignal(int signum, siginfo_t* info, void* context) {
 #endif
     uintptr_t fault_addr = *context_ip;
     uintptr_t landing_pad = 0;
-    if (TryFindLandingPad(fault_addr, &landing_pad)) {
-      // Tell the caller to return to the landing pad.
-      *context_ip = landing_pad;
-      // We will return to wasm code, so restore the g_thread_in_wasm_code flag.
-      g_thread_in_wasm_code = true;
-      return true;
-    }
-  }  // end signal mask scope
+    if (!TryFindLandingPad(fault_addr, &landing_pad)) return false;
 
-  // If we get here, it's not a recoverable wasm fault, so we go to the next
-  // handler. Leave the g_thread_in_wasm_code flag unset since we do not return
-  // to wasm code.
-  return false;
+    // Tell the caller to return to the landing pad.
+    *context_ip = landing_pad;
+  }
+  // We will return to wasm code, so restore the g_thread_in_wasm_code flag.
+  // This should only be done once the signal is blocked again (outside the
+  // {UnmaskOobSignalScope}) to ensure that we do not catch a signal we raise
+  // inside of the handler.
+  g_thread_in_wasm_code = true;
+  return true;
 }
 
 void HandleSignal(int signum, siginfo_t* info, void* context) {