Revert "[wasm] Disallow late enabling of trap handlers"

This reverts commit bcb0a7c5.

Reason for revert: TSAN failure (https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20TSAN/33868)

Original change's description:
> [wasm] Disallow late enabling of trap handlers
>
> It's dangerous if trap handlers are enabled after we already used the
> information whether they are enabled or not.
> This CL checks for such misbehaviour by remembering whether
> {IsTrapHandlerEnabled} was already called, and disallowing
> {EnableTrapHandler} afterwards. Also, calling {EnableTrapHandler}
> multiple times is disallowed now.
>
> The trap handler tests are changed to only enable trap handlers once,
> and to do that before allocating wasm memory or generating code.
>
> R=​ahaas@chromium.org
>
> Bug: v8:11017
> Change-Id: Ib2256bb8435efd914c12769cedd4a0051052aeef
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2494935
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70750}

TBR=ahaas@chromium.org,clemensb@chromium.org

Change-Id: I1d93dcb399e2a0b5b0543aa60d34087317c01cb3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:11017
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2497176Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70751}

src/trap-handler/handler-outside.cc

@@ -249,19 +249,9 @@ bool RegisterDefaultTrapHandler() { return false; }
 void RemoveTrapHandler() {}
 #endif
 
-bool g_is_trap_handler_enabled = false;
-bool g_can_enable_trap_handler = true;
+bool g_is_trap_handler_enabled;
 
 bool EnableTrapHandler(bool use_v8_handler) {
-  if (!g_can_enable_trap_handler) {
-    // Enabling the trap handler after IsTrapHandlerEnabled was called can lead
-    // to problems because code or objects might have been generated under the
-    // assumption that trap handlers are disabled.
-    FATAL("EnableTrapHandler called after IsTrapHandlerEnabled");
-  }
-  // We should only enable the trap handler once.
-  g_can_enable_trap_handler = false;
-  CHECK(!g_is_trap_handler_enabled);
   if (!V8_TRAP_HANDLER_SUPPORTED) {
     return false;
   }

src/trap-handler/trap-handler.h

@@ -65,8 +65,6 @@ void V8_EXPORT_PRIVATE ReleaseHandlerData(int index);
 #endif
 
 extern bool g_is_trap_handler_enabled;
-extern bool g_can_enable_trap_handler;
-
 // Enables trap handling for WebAssembly bounds checks.
 //
 // use_v8_handler indicates that V8 should install its own handler
@@ -75,10 +73,6 @@ V8_EXPORT_PRIVATE bool EnableTrapHandler(bool use_v8_handler);
 
 inline bool IsTrapHandlerEnabled() {
   DCHECK_IMPLIES(g_is_trap_handler_enabled, V8_TRAP_HANDLER_SUPPORTED);
-  // Disallow enabling the trap handler after retrieving the current value.
-  // Re-enabling them late can produce issues because code or objects might have
-  // been generated under the assumption that trap handlers are disabled.
-  g_can_enable_trap_handler = false;
   return g_is_trap_handler_enabled;
 }
 

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

@@ -80,8 +80,7 @@ class TrapHandlerTest : public TestWithIsolate,
                         public ::testing::WithParamInterface<TrapHandlerStyle> {
  protected:
   void SetUp() override {
-    InstallFallbackHandler();
-    SetupTrapHandler(GetParam());
+    CHECK(trap_handler::EnableTrapHandler(false));
     backing_store_ = BackingStore::AllocateWasmMemory(i_isolate(), 1, 1,
                                                       SharedFlag::kNotShared);
     CHECK(backing_store_);
@@ -94,9 +93,7 @@ class TrapHandlerTest : public TestWithIsolate,
                                       GetRandomMmapAddr());
 
     InitRecoveryCode();
-  }
 
-  void InstallFallbackHandler() {
 #if V8_OS_LINUX || V8_OS_MACOSX || V8_OS_FREEBSD
     // Set up a signal handler to recover from the expected crash.
     struct sigaction action;
@@ -200,13 +197,13 @@ class TrapHandlerTest : public TestWithIsolate,
   }
 #endif
 
+ public:
   void SetupTrapHandler(TrapHandlerStyle style) {
     bool use_default_handler = style == kDefault;
     g_use_as_first_chance_handler = !use_default_handler;
     CHECK(v8::V8::EnableWebAssemblyTrapHandler(use_default_handler));
   }
 
- public:
   void GenerateSetThreadInWasmFlagCode(MacroAssembler* masm) {
     masm->Move(scratch,
                i_isolate()->thread_local_top()->thread_in_wasm_flag_address_,
@@ -285,6 +282,7 @@ TEST_P(TrapHandlerTest, TestTrapHandlerRecovery) {
   CodeDesc desc;
   masm.GetCode(nullptr, &desc);
 
+  SetupTrapHandler(GetParam());
   trap_handler::ProtectedInstructionData protected_instruction{crash_offset,
                                                                recovery_offset};
   trap_handler::RegisterHandlerData(reinterpret_cast<Address>(desc.buffer),
@@ -316,6 +314,8 @@ TEST_P(TrapHandlerTest, TestReleaseHandlerData) {
       reinterpret_cast<Address>(desc.buffer), desc.instr_size, 1,
       &protected_instruction);
 
+  SetupTrapHandler(GetParam());
+
   ExecuteBuffer();
 
   // Deregister from the trap handler. The trap handler should not do the
@@ -345,6 +345,8 @@ TEST_P(TrapHandlerTest, TestNoThreadInWasmFlag) {
   trap_handler::RegisterHandlerData(reinterpret_cast<Address>(desc.buffer),
                                     desc.instr_size, 1, &protected_instruction);
 
+  SetupTrapHandler(GetParam());
+
   ExecuteExpectCrash(buffer_.get());
 }
 
@@ -371,6 +373,8 @@ TEST_P(TrapHandlerTest, TestCrashInWasmNoProtectedInstruction) {
   trap_handler::RegisterHandlerData(reinterpret_cast<Address>(desc.buffer),
                                     desc.instr_size, 1, &protected_instruction);
 
+  SetupTrapHandler(GetParam());
+
   ExecuteExpectCrash(buffer_.get());
 }
 
@@ -397,6 +401,8 @@ TEST_P(TrapHandlerTest, TestCrashInWasmWrongCrashType) {
   trap_handler::RegisterHandlerData(reinterpret_cast<Address>(desc.buffer),
                                     desc.instr_size, 1, &protected_instruction);
 
+  SetupTrapHandler(GetParam());
+
 #if V8_OS_POSIX
   // On Posix, the V8 default trap handler does not register for SIGFPE,
   // therefore the thread-in-wasm flag is never reset in this test. We
@@ -456,6 +462,8 @@ TEST_P(TrapHandlerTest, TestCrashInOtherThread) {
   trap_handler::RegisterHandlerData(reinterpret_cast<Address>(desc.buffer),
                                     desc.instr_size, 1, &protected_instruction);
 
+  SetupTrapHandler(GetParam());
+
   CodeRunner runner(this, buffer_.get());
   CHECK(!GetThreadInWasmFlag());
   // Set the thread-in-wasm flag manually in this thread.