[wasm] Refactor trap-handler to allow an extension to windows

This CL refactors the existing trap handler code for Linux to allow a
cleaner extension to Windows.

1) The CL extracts platform-specific code into separate files, see
https://docs.google.com/document/d/1HCgKIpdjy_CEodTLvZ5VuykDI6gGTHrTtau2j0zwm28.
Specifically this means:
* Move posix-specific API functions from v8.h to v8-wasm-trap-handler-posix.h.
  Deprecate the existing TryHandleSignal API function.
* Move posix-specific function declarations from trap-handler-internal.h to
  handler-inside-posix.h
* Move posix-specific function definitions from handler-shared.cc to
  handler-outside-posix.cc

2) The CL changes filenames from *-linux.* to *-posix.*. I expect that
most of the implementation for MacOS will be the same as for Linux.

Bug: v8:6743
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I4bb7f199564a2f01042084d15a82311d11a93c7b
Reviewed-on: https://chromium-review.googlesource.com/c/1280324
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57028}

BUILD.gn

@@ -1524,6 +1524,10 @@ v8_header_set("v8_headers") {
     "include/v8config.h",
   ]
 
+  if (is_linux) {
+    sources += [ "include/v8-wasm-trap-handler-posix.h" ]
+  }
+
   deps = [
     ":v8_version",
   ]
@@ -1547,6 +1551,7 @@ v8_source_set("v8_base") {
     "include/v8-profiler.h",
     "include/v8-testing.h",
     "include/v8-util.h",
+    "include/v8-wasm-trap-handler-posix.h",
     "include/v8.h",
     "include/v8config.h",
     "src/accessors.cc",
@@ -2743,8 +2748,9 @@ v8_source_set("v8_base") {
     ]
     if (is_linux) {
       sources += [
-        "src/trap-handler/handler-inside-linux.cc",
-        "src/trap-handler/handler-outside-linux.cc",
+        "src/trap-handler/handler-inside-posix.cc",
+        "src/trap-handler/handler-inside-posix.h",
+        "src/trap-handler/handler-outside-posix.cc",
       ]
     }
     if (is_win) {

include/v8-wasm-trap-handler-posix.h

@@ -0,0 +1,29 @@
+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_WASM_TRAP_HANDLER_POSIX_H_
+#define V8_WASM_TRAP_HANDLER_POSIX_H_
+
+#include <signal.h>
+
+namespace v8 {
+/**
+ * This function determines whether a memory access violation has been an
+ * out-of-bounds memory access in WebAssembly. If so, it will modify the context
+ * parameter and add a return address where the execution can continue after the
+ * signal handling, and return true. Otherwise, false will be returned.
+ *
+ * The parameters to this function correspond to those passed to a Posix signal
+ * handler. Use this function only on Linux and Mac.
+ *
+ * \param sig_code The signal code, e.g. SIGSEGV.
+ * \param info A pointer to the siginfo_t struct provided to the signal handler.
+ * \param context A pointer to a ucontext_t struct provided to the signal
+ * handler.
+ */
+bool TryHandleWebAssemblyTrapPosix(int sig_code, siginfo_t* info,
+                                   void* context);
+
+}  // namespace v8
+#endif  // V8_WASM_TRAP_HANDLER_POSIX_H_

include/v8.h

@@ -8460,7 +8460,9 @@ class V8_EXPORT V8 {
    * \param context The third argument passed to the Linux signal handler, which
    * points to a ucontext_t structure.
    */
-  static bool TryHandleSignal(int signal_number, void* info, void* context);
+  V8_DEPRECATE_SOON("Use TryHandleWebAssemblyTrapPosix",
+                    static bool TryHandleSignal(int signal_number, void* info,
+                                                void* context));
 #endif  // V8_OS_POSIX
 
   /**

src/DEPS

@@ -27,6 +27,7 @@ include_rules = [
   "+src/interpreter/interpreter-generator.h",
   "+src/interpreter/setup-interpreter.h",
   "-src/trap-handler",
+  "+src/trap-handler/handler-inside-posix.h",
   "+src/trap-handler/trap-handler.h",
   "+testing/gtest/include/gtest/gtest_prod.h",
   "-src/libplatform",

src/api.cc

@@ -101,6 +101,11 @@
 #include "src/wasm/wasm-result.h"
 #include "src/wasm/wasm-serialization.h"
 
+#ifdef V8_OS_POSIX
+#include <signal.h>
+#include "src/trap-handler/handler-inside-posix.h"
+#endif
+
 namespace v8 {
 
 /*
@@ -5887,14 +5892,19 @@ bool v8::V8::Initialize() {
 }
 
 #if V8_OS_POSIX
-bool V8::TryHandleSignal(int signum, void* info, void* context) {
+bool TryHandleWebAssemblyTrapPosix(int sig_code, siginfo_t* info,
+                                   void* context) {
 #if V8_OS_LINUX && V8_TARGET_ARCH_X64 && !V8_OS_ANDROID
-  return v8::internal::trap_handler::TryHandleSignal(
-      signum, static_cast<siginfo_t*>(info), static_cast<ucontext_t*>(context));
+  return i::trap_handler::TryHandleSignal(sig_code, info, context);
 #else  // V8_OS_LINUX && V8_TARGET_ARCH_X64 && !V8_OS_ANDROID
   return false;
 #endif
 }
+
+bool V8::TryHandleSignal(int signum, void* info, void* context) {
+  return TryHandleWebAssemblyTrapPosix(
+      signum, reinterpret_cast<siginfo_t*>(info), context);
+}
 #endif
 
 bool V8::RegisterDefaultSignalHandler() {

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

@@ -23,6 +23,8 @@
 // context. Some additional code is used both inside and outside the signal
 // handler. This code can be found in handler-shared.cc.
 
+#include "src/trap-handler/handler-inside-posix.h"
+
 #include <signal.h>
 #include <stddef.h>
 #include <stdlib.h>
@@ -59,7 +61,7 @@ class SigUnmaskStack {
   void operator=(const SigUnmaskStack&) = delete;
 };
 
-bool TryHandleSignal(int signum, siginfo_t* info, ucontext_t* context) {
+bool TryHandleSignal(int signum, siginfo_t* info, void* context) {
   // Bail out early in case we got called for the wrong kind of signal.
   if (signum != SIGSEGV) {
     return false;
@@ -91,11 +93,12 @@ bool TryHandleSignal(int signum, siginfo_t* info, ucontext_t* context) {
     sigaddset(&sigs, SIGSEGV);
     SigUnmaskStack unmask(sigs);
 
-    uintptr_t fault_addr = context->uc_mcontext.gregs[REG_RIP];
+    ucontext_t* uc = reinterpret_cast<ucontext_t*>(context);
+    uintptr_t fault_addr = uc->uc_mcontext.gregs[REG_RIP];
     uintptr_t landing_pad = 0;
     if (TryFindLandingPad(fault_addr, &landing_pad)) {
       // Tell the caller to return to the landing pad.
-      context->uc_mcontext.gregs[REG_RIP] = landing_pad;
+      uc->uc_mcontext.gregs[REG_RIP] = 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;
@@ -109,9 +112,7 @@ bool TryHandleSignal(int signum, siginfo_t* info, ucontext_t* context) {
 }
 
 void HandleSignal(int signum, siginfo_t* info, void* context) {
-  ucontext_t* uc = reinterpret_cast<ucontext_t*>(context);
-
-  if (!TryHandleSignal(signum, info, uc)) {
+  if (!TryHandleSignal(signum, info, context)) {
     // Since V8 didn't handle this signal, we want to re-raise the same signal.
     // For kernel-generated SEGV signals, we do this by restoring the original
     // SEGV handler and then returning. The fault will happen again and the
@@ -120,7 +121,7 @@ void HandleSignal(int signum, siginfo_t* info, void* context) {
     // We handle user-generated signals by calling raise() instead. This is for
     // completeness. We should never actually see one of these, but just in
     // case, we do the right thing.
-    RestoreOriginalSignalHandler();
+    RemoveTrapHandler();
     if (!IsKernelGeneratedSignal(info)) {
       raise(signum);
     }

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

@@ -0,0 +1,22 @@
+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_TRAP_HANDLER_HANDLER_INSIDE_POSIX_H_
+#define V8_TRAP_HANDLER_HANDLER_INSIDE_POSIX_H_
+
+#include <signal.h>
+
+namespace v8 {
+namespace internal {
+namespace trap_handler {
+
+void HandleSignal(int signum, siginfo_t* info, void* context);
+
+bool TryHandleSignal(int signum, siginfo_t* info, void* context);
+
+}  // namespace trap_handler
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_TRAP_HANDLER_HANDLER_INSIDE_POSIX_H_

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

@@ -21,14 +21,23 @@
 
 #include <signal.h>
 
+#include "src/trap-handler/handler-inside-posix.h"
 #include "src/trap-handler/trap-handler-internal.h"
-#include "src/trap-handler/trap-handler.h"
 
 namespace v8 {
 namespace internal {
 namespace trap_handler {
 
 #if V8_TRAP_HANDLER_SUPPORTED
+namespace {
+struct sigaction g_old_handler;
+
+// When using the default signal handler, we save the old one to restore in case
+// V8 chooses not to handle the signal.
+bool g_is_default_signal_handler_registered;
+
+}  // namespace
+
 bool RegisterDefaultTrapHandler() {
   CHECK(!g_is_default_signal_handler_registered);
 
@@ -66,7 +75,17 @@ bool RegisterDefaultTrapHandler() {
   g_is_default_signal_handler_registered = true;
   return true;
 }
+#endif  // V8_TRAP_HANDLER_SUPPORTED
+
+void RemoveTrapHandler() {
+#if V8_TRAP_HANDLER_SUPPORTED
+  if (g_is_default_signal_handler_registered) {
+    if (sigaction(SIGSEGV, &g_old_handler, nullptr) == 0) {
+      g_is_default_signal_handler_registered = false;
+    }
+  }
 #endif
+}
 
 }  // namespace trap_handler
 }  // namespace internal

src/trap-handler/handler-outside.cc

@@ -19,7 +19,6 @@
 //
 // For the code that runs in the signal handler itself, see handler-inside.cc.
 
-#include <signal.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>

src/trap-handler/handler-shared.cc

@@ -28,21 +28,6 @@ namespace trap_handler {
 // 1 byte in size; see https://sourceware.org/bugzilla/show_bug.cgi?id=14898.
 THREAD_LOCAL int g_thread_in_wasm_code;
 
-#if V8_TRAP_HANDLER_SUPPORTED
-// When using the default signal handler, we save the old one to restore in case
-// V8 chooses not to handle the signal.
-struct sigaction g_old_handler;
-bool g_is_default_signal_handler_registered;
-#endif
-
-V8_EXPORT_PRIVATE void RestoreOriginalSignalHandler() {
-#if V8_TRAP_HANDLER_SUPPORTED
-  if (sigaction(SIGSEGV, &g_old_handler, nullptr) == 0) {
-    g_is_default_signal_handler_registered = false;
-  }
-#endif
-}
-
 static_assert(sizeof(g_thread_in_wasm_code) > 1,
               "sizeof(thread_local_var) must be > 1, see "
               "https://sourceware.org/bugzilla/show_bug.cgi?id=14898");

src/trap-handler/trap-handler-internal.h

@@ -41,10 +41,6 @@ class MetadataLock {
   void operator=(const MetadataLock&) = delete;
 };
 
-#if V8_TRAP_HANDLER_SUPPORTED
-void HandleSignal(int signum, siginfo_t* info, void* context);
-#endif
-
 // To enable constant time registration of handler data, we keep a free list of
 // entries in the gCodeObjects table. Each entry contains a {next_free} field,
 // which can be used to figure out where the next entry should be inserted.
@@ -68,13 +64,6 @@ extern std::atomic_size_t gRecoveredTrapCount;
 // unchanged.
 bool TryFindLandingPad(uintptr_t fault_addr, uintptr_t* landing_pad);
 
-#if V8_TRAP_HANDLER_SUPPORTED
-// When using the default signal handler, we save the old one to restore in case
-// V8 chooses not to handle the signal.
-extern struct sigaction g_old_handler;
-extern bool g_is_default_signal_handler_registered;
-#endif
-
 }  // namespace trap_handler
 }  // namespace internal
 }  // namespace v8

src/trap-handler/trap-handler.h

@@ -5,7 +5,6 @@
 #ifndef V8_TRAP_HANDLER_TRAP_HANDLER_H_
 #define V8_TRAP_HANDLER_TRAP_HANDLER_H_
 
-#include <signal.h>
 #include <stdint.h>
 #include <stdlib.h>
 
@@ -13,10 +12,6 @@
 #include "src/flags.h"
 #include "src/globals.h"
 
-#if V8_OS_LINUX
-#include <ucontext.h>
-#endif
-
 namespace v8 {
 namespace internal {
 namespace trap_handler {
@@ -101,11 +96,7 @@ inline void ClearThreadInWasm() {
 }
 
 bool RegisterDefaultTrapHandler();
-V8_EXPORT_PRIVATE void RestoreOriginalSignalHandler();
-
-#if V8_OS_LINUX
-bool TryHandleSignal(int signum, siginfo_t* info, ucontext_t* context);
-#endif  // V8_OS_LINUX
+V8_EXPORT_PRIVATE void RemoveTrapHandler();
 
 size_t GetRecoveredTrapCount();
 

test/unittests/wasm/trap-handler-unittest.cc

@@ -58,7 +58,7 @@ TEST_F(SignalHandlerFallbackTest, DoTest) {
     FAIL();
   } else {
     // Our signal handler ran.
-    v8::internal::trap_handler::RestoreOriginalSignalHandler();
+    v8::internal::trap_handler::RemoveTrapHandler();
     SUCCEED();
     return;
   }