[wasm] Initial signal handler

src/trap-handler/handler-outside.cc

Index: src/trap-handler/handler-outside.cc
diff --git a/src/trap-handler/handler-outside.cc b/src/trap-handler/handler-outside.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a642e2c46a4deabd285612ce8a51077a89e9485e
--- /dev/null
+++ b/src/trap-handler/handler-outside.cc
@@ -0,0 +1,173 @@
+// Copyright 2016 the V8 project authors. All rights reserved.

[jochen (gone - plz use gerrit), 2017/02/20 09:23:04]

nit. 2017

[Eric Holk, 2017/02/23 02:16:56]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// PLEASE READ BEFORE CHANGING THIS FILE!

[jochen (gone - plz use gerrit), 2017/02/20 09:23:04]

maybe move that to a README.md?

[Eric Holk, 2017/02/23 02:16:56]

I'd rather leave it here so that people are more likely to see it before
changing this file.

I'd be happy to add a README.md with more information about the layout of this
directory, if you'd like.

+//
+// This file implements the support code for the out of bounds signal handler.
+// Nothing in here actually runs in the signal handler, but the code here
+// manipulates data structures used by the signal handler so we still need to be
+// careful. In order to minimize this risk, here are some rules to follow.
+//
+// 1. Avoid introducing new external dependencies. The files in src/trap-handler
+//    should be as self-contained as possible to make it easy to audit the code.
+//
+// 2. Any changes must be reviewed by someone from the crash reporting
+//    or security team. Se OWNERS for suggested reviewers.
+//
+// For more information, see https://goo.gl/yMeyUY.
+//
+// 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>
+#include <string.h>
+
+#include <atomic>
+
+#include "src/flags.h"
+#include "src/trap-handler/trap-handler-internal.h"
+#include "src/trap-handler/trap-handler.h"
+
+#define TRACE(...)

[Mark Seaborn, 2017/02/17 21:41:12]

I think you said you were removing the TRACE() logging.

[Eric Holk, 2017/02/23 02:16:56]

I must have missed some. This is removed now too.

+
+namespace v8 {
+namespace internal {
+namespace trap_handler {
+
+const size_t kInitialCodeObjectSize = 1024;
+const size_t kCodeObjectGrowthFactor = 2;
+
+constexpr size_t HandlerDataSize(size_t num_protected_instructions) {
+  return offsetof(CodeProtectionInfo, instructions) +
+         num_protected_instructions * sizeof(ProtectedInstructionData);
+}
+
+CodeProtectionInfo* CreateHandlerData(
+    void* base, size_t size, size_t num_protected_instructions,
+    ProtectedInstructionData* protected_instructions) {
+  const size_t alloc_size = HandlerDataSize(num_protected_instructions);
+  CodeProtectionInfo* data =
+      reinterpret_cast<CodeProtectionInfo*>(malloc(alloc_size));
+
+  if (data == nullptr) {
+    return nullptr;
+  }
+
+  data->base = base;
+  data->size = size;
+  data->num_protected_instructions = num_protected_instructions;
+
+  memcpy(data->instructions, protected_instructions,
+         num_protected_instructions * sizeof(ProtectedInstructionData));
+
+  return data;
+}
+
+void UpdateCodePointer(int index, void* base) {
+  TRACE("Updating code pointer at index %d\n", index);
+  MetadataLock lock;
+  if (static_cast<size_t>(index) >= gNumCodeObjects) {
+    TRACE("Out of bounds\n");
+    abort();
+  }
+  CodeProtectionInfo* data = gCodeObjects[index];
+  data->base = base;
+}
+
+int RegisterHandlerData(void* base, size_t size,
+                        size_t num_protected_instructions,
+                        ProtectedInstructionData* protected_instructions) {
+  // TODO(eholk): in debug builds, make sure this data isn't already registered.
+
+  CodeProtectionInfo* data = CreateHandlerData(
+      base, size, num_protected_instructions, protected_instructions);
+
+  if (data == nullptr) {
+    TRACE("Could not allocate handler data\n");
+    abort();
+  }
+
+  MetadataLock lock;
+
+  size_t i;
+  for (i = 0; i < gNumCodeObjects; ++i) {

[titzer, 2017/02/20 09:50:08]

Oh, this is kinda bad. It's gonna be linear time to insert a code object into
this list. I think we should introduce a free list to make it constant time.

[Eric Holk, 2017/02/23 02:16:56]

Good call. This basically makes instantiation quadratic in the number of
functions a wasm module has.

I reworked this to maintain a lazily-initialized free list. Fortunately, it only
required a trivial one line change in handler-inside.cc.

Done.

+    if (gCodeObjects[i] == nullptr) {
+      break;
+    }
+  }
+
+  // We didn't find an opening in the available space, so grow.
+  if (i == gNumCodeObjects) {
+    size_t new_size = gNumCodeObjects > 0
+                          ? gNumCodeObjects * kCodeObjectGrowthFactor
+                          : kInitialCodeObjectSize;
+    gCodeObjects = static_cast<CodeProtectionInfo**>(
+        realloc(gCodeObjects, sizeof(*gCodeObjects) * new_size));
+
+    if (gCodeObjects == nullptr) {
+      abort();
+    }
+
+    memset(gCodeObjects + gNumCodeObjects, 0,
+           sizeof(*gCodeObjects) * (new_size - gNumCodeObjects));
+    gNumCodeObjects = new_size;
+  }
+
+  gCodeObjects[i] = data;
+  TRACE("Registered handler data %p at index %zu\n", data, i);
+  return static_cast<int>(i);

[Mark Seaborn, 2017/02/17 21:41:12]

Please check that this doesn't overflow/wrap, as we talked about earlier.

[Eric Holk, 2017/02/23 02:16:57]

Done.

I also changed the new_size calculation above to clamp it at
std::numeric_limits<int>::max() so we don't allocate memory that can never be
used.

+}
+
+void ReleaseHandlerData(int index) {
+  // Remove the data from the global list if it's there.
+
+  MetadataLock lock;
+
+  CodeProtectionInfo* data = gCodeObjects[index];
+  gCodeObjects[index] = nullptr;
+
+  TRACE("Released handler data %p at index %d\n", data, index);
+
+  // TODO(eholk): on debug builds, ensure there are no more copies in
+  // the list.
+  free(data);

[Mark Seaborn, 2017/02/17 21:41:12]

You could do this outside of the lock.

[Eric Holk, 2017/02/23 02:16:56]

Done.

+}
+
+void SetThreadInWasm() {
+  TRACE("Transitioning to Wasm\n");
+  g_thread_in_wasm_code = true;

[Mark Seaborn, 2017/02/17 21:41:12]

This could first assert that g_thread_in_wasm_code is false, and the opposite
for ClearThreadInWasm().

That would be instead of the checks in src/runtime/runtime-wasm.cc.

[Eric Holk, 2017/02/23 02:16:56]

Done.

+}
+
+void ClearThreadInWasm() {
+  TRACE("Transitioning from Wasm\n");
+  g_thread_in_wasm_code = false;
+}
+
+bool IsThreadInWasm() { return g_thread_in_wasm_code; }
+
+V8_EXPORT_PRIVATE bool ShouldEnableTrapHandler() {
+  return FLAG_wasm_trap_handler && kTrapHandlerSupported;
+}
+
+bool RegisterDefaultSignalHandler() {
+#if V8_TRAP_HANDLER_SUPPORTED
+  struct sigaction action;
+  action.sa_sigaction = HandleSignal;
+  action.sa_flags = SA_SIGINFO;
+  sigemptyset(&action.sa_mask);
+  if (sigaction(SIGSEGV, &action, nullptr) != 0) {

[ahaas, 2017/02/20 09:27:24]

Could you add a comment for this code?

[Eric Holk, 2017/02/23 02:16:56]

Done.

+    return false;
+  }
+
+  return true;
+#else
+  return false;
+#endif
+}
+
+}  // namespace trap_handler
+}  // namespace internal
+}  // namespace v8