Reland: [wasm] Add more unit tests for trap handler

The problem were missing V8_EXPORT_PRIVATE and V8_EXPORT.

The unittests test if the trap handler only handles those traps it
is supposed to handle:
* Only handle traps when the thread-in-wasm flag is set.
* Only handle traps of the right type, i.e. memory access violations.
* Only handle traps at recorded instructions.

The tests also test the consistency of the thread-in-wasm flag. I made
one change in the trap handler where that consistency could be
violated.

All tests are executed with the default trap handler provided by V8,
and with the trap handler callback installed in a test signal/exception
handler.

Patchset 1 is the original CL.

R=mstarzinger@chromium.org

Change-Id: I172d94f24cdba4c3a1f7f344825b059dbb59da79
Reviewed-on: https://chromium-review.googlesource.com/c/1351024Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57947}

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

@@ -6,6 +6,7 @@
 #define V8_WASM_TRAP_HANDLER_WIN_H_
 
 #include <windows.h>
+#include "include/v8config.h"
 
 namespace v8 {
 /**
@@ -20,7 +21,7 @@ namespace v8 {
  *
  * \param exception An EXCEPTION_POINTERS* as provided to the exception handler.
  */
-bool TryHandleWebAssemblyTrapWindows(EXCEPTION_POINTERS* exception);
+V8_EXPORT bool TryHandleWebAssemblyTrapWindows(EXCEPTION_POINTERS* exception);
 
 }  // namespace v8
 #endif  // V8_WASM_TRAP_HANDLER_WIN_H_

src/api.cc

@@ -107,11 +107,13 @@
 
 #if V8_OS_LINUX || V8_OS_MACOSX
 #include <signal.h>
+#include "include/v8-wasm-trap-handler-posix.h"
 #include "src/trap-handler/handler-inside-posix.h"
 #endif
 
 #if V8_OS_WIN
 #include <windows.h>
+#include "include/v8-wasm-trap-handler-win.h"
 #include "src/trap-handler/handler-inside-win.h"
 #endif
 
@@ -5927,7 +5929,6 @@ void v8::V8::SetReturnAddressLocationResolver(
   i::StackFrame::SetReturnAddressLocationResolver(return_address_resolver);
 }
 
-
 bool v8::V8::Dispose() {
   i::V8::TearDown();
 #ifdef V8_USE_EXTERNAL_STARTUP_DATA

src/arm/macro-assembler-arm.h

@@ -579,7 +579,7 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase {
 };
 
 // MacroAssembler implements a collection of frequently used macros.
-class MacroAssembler : public TurboAssembler {
+class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
  public:
   MacroAssembler(const AssemblerOptions& options, void* buffer, int size)
       : TurboAssembler(options, buffer, size) {}

src/arm64/macro-assembler-arm64.h

@@ -1281,7 +1281,7 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase {
                            Address wasm_target);
 };
 
-class MacroAssembler : public TurboAssembler {
+class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
  public:
   MacroAssembler(const AssemblerOptions& options, void* buffer, int size)
       : TurboAssembler(options, buffer, size) {}

src/external-reference.h

@@ -302,7 +302,7 @@ class ExternalReference {
 #undef DECL_EXTERNAL_REFERENCE
 
 #define DECL_EXTERNAL_REFERENCE(name, desc) \
-  static ExternalReference name(Isolate* isolate);
+  static V8_EXPORT_PRIVATE ExternalReference name(Isolate* isolate);
   EXTERNAL_REFERENCE_LIST_WITH_ISOLATE(DECL_EXTERNAL_REFERENCE)
 #undef DECL_EXTERNAL_REFERENCE
 

src/ia32/macro-assembler-ia32.h

@@ -470,7 +470,7 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase {
 };
 
 // MacroAssembler implements a collection of frequently used macros.
-class MacroAssembler : public TurboAssembler {
+class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
  public:
   MacroAssembler(const AssemblerOptions& options, void* buffer, int size)
       : TurboAssembler(options, buffer, size) {}

src/mips/macro-assembler-mips.h

@@ -916,7 +916,7 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase {
 };
 
 // MacroAssembler implements a collection of frequently used macros.
-class MacroAssembler : public TurboAssembler {
+class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
  public:
   MacroAssembler(const AssemblerOptions& options, void* buffer, int size)
       : TurboAssembler(options, buffer, size) {}

src/mips64/macro-assembler-mips64.h

@@ -928,7 +928,7 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase {
 };
 
 // MacroAssembler implements a collection of frequently used macros.
-class MacroAssembler : public TurboAssembler {
+class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
  public:
   MacroAssembler(const AssemblerOptions& options, void* buffer, int size)
       : TurboAssembler(options, buffer, size) {}

src/ppc/macro-assembler-ppc.h

@@ -668,7 +668,7 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase {
 };
 
 // MacroAssembler implements a collection of frequently used acros.
-class MacroAssembler : public TurboAssembler {
+class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
  public:
   MacroAssembler(const AssemblerOptions& options, void* buffer, int size)
       : TurboAssembler(options, buffer, size) {}

src/s390/macro-assembler-s390.h

@@ -1024,7 +1024,7 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase {
 };
 
 // MacroAssembler implements a collection of frequently used macros.
-class MacroAssembler : public TurboAssembler {
+class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
  public:
   MacroAssembler(const AssemblerOptions& options, void* buffer, int size)
       : TurboAssembler(options, buffer, size) {}

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

@@ -72,6 +72,17 @@ class SigUnmaskStack {
 };
 
 bool TryHandleSignal(int signum, siginfo_t* info, void* context) {
+  // Ensure the faulting thread was actually running Wasm code. This should be
+  // 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;
+  }
+
+  // Clear g_thread_in_wasm_code, primarily to protect against nested faults.
+  g_thread_in_wasm_code = false;
+
   // Bail out early in case we got called for the wrong kind of signal.
 
   if (signum != kOobSignal) {
@@ -83,14 +94,6 @@ bool TryHandleSignal(int signum, siginfo_t* info, void* context) {
     return false;
   }
 
-  // Ensure the faulting thread was actually running Wasm code.
-  if (!IsThreadInWasm()) {
-    return false;
-  }
-
-  // Clear g_thread_in_wasm_code, primarily to protect against nested faults.
-  g_thread_in_wasm_code = 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.
   {

src/trap-handler/trap-handler.h

@@ -44,14 +44,14 @@ const int kInvalidIndex = -1;
 ///
 /// This returns a number that can be used to identify the handler data to
 /// ReleaseHandlerData, or -1 on failure.
-int RegisterHandlerData(Address base, size_t size,
-                        size_t num_protected_instructions,
-                        const ProtectedInstructionData* protected_instructions);
+int V8_EXPORT_PRIVATE RegisterHandlerData(
+    Address base, size_t size, size_t num_protected_instructions,
+    const ProtectedInstructionData* protected_instructions);
 
 /// Removes the data from the master list and frees any memory, if necessary.
 /// TODO(mtrofin): We can switch to using size_t for index and not need
 /// kInvalidIndex.
-void ReleaseHandlerData(int index);
+void V8_EXPORT_PRIVATE ReleaseHandlerData(int index);
 
 #if V8_OS_WIN
 #define THREAD_LOCAL __declspec(thread)
@@ -79,7 +79,7 @@ extern THREAD_LOCAL int g_thread_in_wasm_code;
 // Return the address of the thread-local {g_thread_in_wasm_code} variable. This
 // pointer can be accessed and modified as long as the thread calling this
 // function exists. Only use if from the same thread do avoid race conditions.
-inline int* GetThreadInWasmThreadLocalAddress() {
+inline V8_EXPORT_PRIVATE int* GetThreadInWasmThreadLocalAddress() {
   return &g_thread_in_wasm_code;
 }
 

src/wasm/wasm-memory.cc

@@ -154,6 +154,20 @@ WasmMemoryTracker::~WasmMemoryTracker() {
   DCHECK_EQ(allocated_address_space_, 0u);
 }
 
+void* WasmMemoryTracker::TryAllocateBackingStoreForTesting(
+    Heap* heap, size_t size, void** allocation_base,
+    size_t* allocation_length) {
+  return TryAllocateBackingStore(this, heap, size, allocation_base,
+                                 allocation_length);
+}
+
+void WasmMemoryTracker::FreeBackingStoreForTesting(base::AddressRegion memory,
+                                                   void* buffer_start) {
+  ReleaseAllocation(nullptr, buffer_start);
+  CHECK(FreePages(GetPlatformPageAllocator(),
+                  reinterpret_cast<void*>(memory.begin()), memory.size()));
+}
+
 bool WasmMemoryTracker::ReserveAddressSpace(size_t num_bytes,
                                             ReservationLimit limit) {
   size_t reservation_limit =

src/wasm/wasm-memory.h

@@ -66,6 +66,18 @@ class WasmMemoryTracker {
     friend WasmMemoryTracker;
   };
 
+  // Allow tests to allocate a backing store the same way as we do it for
+  // WebAssembly memory. This is used in unit tests for trap handler to
+  // generate the same signals/exceptions for invalid memory accesses as
+  // we would get with WebAssembly memory.
+  V8_EXPORT_PRIVATE void* TryAllocateBackingStoreForTesting(
+      Heap* heap, size_t size, void** allocation_base,
+      size_t* allocation_length);
+
+  // Free memory allocated with TryAllocateBackingStoreForTesting.
+  V8_EXPORT_PRIVATE void FreeBackingStoreForTesting(base::AddressRegion memory,
+                                                    void* buffer_start);
+
   // Decreases the amount of reserved address space.
   void ReleaseReservation(size_t num_bytes);
 

src/x64/assembler-x64.h

@@ -311,7 +311,7 @@ enum ScaleFactor : int8_t {
   times_pointer_size = (kPointerSize == 8) ? times_8 : times_4
 };
 
-class Operand {
+class V8_EXPORT_PRIVATE Operand {
  public:
   struct Data {
     byte rex = 0;

src/x64/macro-assembler-x64.h

@@ -537,7 +537,7 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase {
 };
 
 // MacroAssembler implements a collection of frequently used macros.
-class MacroAssembler : public TurboAssembler {
+class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
  public:
   MacroAssembler(const AssemblerOptions& options, void* buffer, int size)
       : TurboAssembler(options, buffer, size) {}

test/cctest/test-assembler-arm64.cc

@@ -209,8 +209,7 @@ static void InitializeVM() {
   __ Ret();                     \
   __ GetCode(masm.isolate(), nullptr);
 
-#define TEARDOWN() \
-  CHECK(v8::internal::FreePages(GetPlatformPageAllocator(), buf, allocated));
+#define TEARDOWN() FreeAssemblerBuffer(buf, allocated);
 
 #endif  // ifdef USE_SIMULATOR.
 

test/common/assembler-tester.h

@@ -23,6 +23,10 @@ static inline uint8_t* AllocateAssemblerBuffer(
   return static_cast<uint8_t*>(result);
 }
 
+static inline void FreeAssemblerBuffer(uint8_t* buffer, size_t size) {
+  CHECK(FreePages(GetPlatformPageAllocator(), buffer, size));
+}
+
 static inline void MakeAssemblerBufferExecutable(uint8_t* buffer,
                                                  size_t allocated) {
   // Flush the instruction cache as part of making the buffer executable.

test/unittests/BUILD.gn

@@ -255,6 +255,7 @@ v8_source_set("unittests_sources") {
     sources += [
       "assembler/turbo-assembler-x64-unittest.cc",
       "compiler/x64/instruction-selector-x64-unittest.cc",
+      "wasm/trap-handler-x64-unittest.cc",
     ]
   } else if (v8_current_cpu == "ppc" || v8_current_cpu == "ppc64") {
     sources += [