[wasm] Load thread-in-wasm flag from the isolate

The existing implementation embedded an isolate-specific pointer to the
thread-in-wasm flag in the wrapper code. However, when the module code
is shared among multiple workers, this can mean that the workers
share the same thread-in-wasm flag.

With this change we load the pointer to the flag at runtime from the
current isolate. Thereby the correct flag is used even when the same
code is executed on different workers.

Note that we could access the right flag address by going through the
root register. However, changing the code generation to use the root
register requires some inconvenient steps:
* Pass the isolate to the pipeline again, which we don't want.
* Change the WasmCallDescriptor to allow the use of the root register
  for wrappers but not for other code.
To avoid these issues, and allow the CL to be easy to merge back, we
got for the changes proposed here.

R=mstarzinger@chromium.org, ishell@chromium.org

Bug: v8:8533
Change-Id: If15565a7ad7cba835cfc1628e7a4d3fdef90a5c0
Reviewed-on: https://chromium-review.googlesource.com/c/1358518
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58044}

src/compiler/wasm-compiler.cc

@@ -3065,8 +3065,7 @@ Node* WasmGraphBuilder::CurrentMemoryPages() {
 
 Node* WasmGraphBuilder::BuildLoadBuiltinFromInstance(int builtin_index) {
   DCHECK(Builtins::IsBuiltinId(builtin_index));
-  Node* isolate_root =
-      LOAD_INSTANCE_FIELD(IsolateRoot, MachineType::TaggedPointer());
+  Node* isolate_root = LOAD_INSTANCE_FIELD(IsolateRoot, MachineType::Pointer());
   return LOAD_TAGGED_POINTER(isolate_root,
                              IsolateData::builtin_slot_offset(builtin_index));
 }
@@ -4540,14 +4539,13 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
 
   void BuildModifyThreadInWasmFlag(bool new_value) {
     if (!trap_handler::IsTrapHandlerEnabled()) return;
-    Node* thread_in_wasm_flag_address_address =
-        graph()->NewNode(mcgraph()->common()->ExternalConstant(
-            ExternalReference::wasm_thread_in_wasm_flag_address_address(
-                isolate_)));
-    Node* thread_in_wasm_flag_address = SetEffect(graph()->NewNode(
-        mcgraph()->machine()->Load(LoadRepresentation(MachineType::Pointer())),
-        thread_in_wasm_flag_address_address, mcgraph()->Int32Constant(0),
-        Effect(), Control()));
+    Node* isolate_root =
+        LOAD_INSTANCE_FIELD(IsolateRoot, MachineType::Pointer());
+
+    Node* thread_in_wasm_flag_address =
+        LOAD_RAW(isolate_root, Isolate::thread_in_wasm_flag_address_offset(),
+                 MachineType::Pointer());
+
     SetEffect(graph()->NewNode(
         mcgraph()->machine()->Store(StoreRepresentation(
             MachineRepresentation::kWord32, kNoWriteBarrier)),

src/external-reference.cc

@@ -826,12 +826,6 @@ ExternalReference ExternalReference::debug_restart_fp_address(
   return ExternalReference(isolate->debug()->restart_fp_address());
 }
 
-ExternalReference ExternalReference::wasm_thread_in_wasm_flag_address_address(
-    Isolate* isolate) {
-  return ExternalReference(reinterpret_cast<Address>(
-      &isolate->thread_local_top()->thread_in_wasm_flag_address_));
-}
-
 ExternalReference ExternalReference::fast_c_call_caller_fp_address(
     Isolate* isolate) {
   return ExternalReference(

src/external-reference.h

@@ -70,8 +70,6 @@ class StatsCounter;
   V(debug_suspended_generator_address,                                         \
     "Debug::step_suspended_generator_address()")                               \
   V(debug_restart_fp_address, "Debug::restart_fp_address()")                   \
-  V(wasm_thread_in_wasm_flag_address_address,                                  \
-    "&Isolate::thread_in_wasm_flag_address")                                   \
   V(fast_c_call_caller_fp_address,                                             \
     "IsolateData::fast_c_call_caller_fp_address")                              \
   V(fast_c_call_caller_pc_address,                                             \

src/isolate.h

@@ -1030,6 +1030,18 @@ class Isolate final : private HiddenFactory {
     deoptimizer_lazy_throw_ = value;
   }
   ThreadLocalTop* thread_local_top() { return &thread_local_top_; }
+
+  static uint32_t thread_in_wasm_flag_address_offset() {
+    // For WebAssembly trap handlers there is a flag in thread-local storage
+    // which indicates that the executing thread executes WebAssembly code. To
+    // access this flag directly from generated code, we store a pointer to the
+    // flag in ThreadLocalTop in thread_in_wasm_flag_address_. This function
+    // here returns the offset of that member from {isolate_root()}.
+    return static_cast<uint32_t>(
+        OFFSET_OF(Isolate, thread_local_top_.thread_in_wasm_flag_address_) -
+        isolate_root_bias());
+  }
+
   MaterializedObjectStore* materialized_object_store() {
     return materialized_object_store_;
   }

src/runtime/runtime-test.cc

@@ -843,6 +843,12 @@ RUNTIME_FUNCTION(Runtime_IsWasmTrapHandlerEnabled) {
   return isolate->heap()->ToBoolean(trap_handler::IsTrapHandlerEnabled());
 }
 
+RUNTIME_FUNCTION(Runtime_IsThreadInWasm) {
+  DisallowHeapAllocation no_gc;
+  DCHECK_EQ(0, args.length());
+  return isolate->heap()->ToBoolean(trap_handler::IsThreadInWasm());
+}
+
 RUNTIME_FUNCTION(Runtime_GetWasmRecoveredTrapCount) {
   HandleScope scope(isolate);
   DCHECK_EQ(0, args.length());

src/runtime/runtime.h

@@ -491,6 +491,7 @@ namespace internal {
   F(IsLiftoffFunction, 1, 1)                  \
   F(IsWasmCode, 1, 1)                         \
   F(IsWasmTrapHandlerEnabled, 0, 1)           \
+  F(IsThreadInWasm, 0, 1)                     \
   F(NeverOptimizeFunction, 1, 1)              \
   F(NotifyContextDisposed, 0, 1)              \
   F(OptimizeFunctionOnNextCall, -1, 1)        \

test/mjsunit/regress/wasm/regress-8533.js

+// 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.
+
+// Flags: --wasm-shared-engine --no-wasm-disable-structured-cloning --allow-natives-syntax --experimental-wasm-threads
+
+load('test/mjsunit/wasm/wasm-constants.js');
+load('test/mjsunit/wasm/wasm-module-builder.js');
+
+
+// In  this test we start a worker which enters wasm and stays there in a loop.
+// The main thread stays in JS and checks that its thread-in-wasm flag is not
+// set. The main thread calls setTimeout after every check to give the worker a
+// chance to be scheculed.
+const sync_address = 12;
+(function TestPostModule() {
+  let builder = new WasmModuleBuilder();
+  let sig_index = builder.addType(kSig_v_v);
+  let import_id = builder.addImport('m', 'func', sig_index);
+  builder.addFunction('wait', kSig_v_v)
+      .addBody([
+        // Calling the imported function sets the thread-in-wasm flag of the
+        // main thread.
+        kExprCallFunction, import_id,  // --
+        kExprLoop, kWasmStmt,          // --
+        kExprI32Const, sync_address,   // --
+        kExprI32LoadMem, 0, 0,         // --
+        kExprI32Eqz,
+        kExprBrIf, 0,                  // --
+        kExprEnd,
+      ])
+      .exportFunc();
+
+  builder.addFunction('signal', kSig_v_v)
+      .addBody([
+        kExprI32Const, sync_address,  // --
+        kExprI32Const, 1,             // --
+        kExprI32StoreMem, 0, 0,       // --
+        ])
+      .exportFunc();
+  builder.addImportedMemory("m", "imported_mem", 0, 1, "shared");
+
+  let module = builder.toModule();
+  let memory = new WebAssembly.Memory({initial: 1, maximum: 1, shared: true});
+
+  let workerScript = `
+    onmessage = function(msg) {
+      try {
+        let worker_instance = new WebAssembly.Instance(msg.module,
+            {m: {imported_mem: msg.memory,
+                 func: _ => 5}});
+        postMessage("start running");
+        worker_instance.exports.wait();
+        postMessage("finished");
+      } catch(e) {
+        postMessage('ERROR: ' + e);
+      }
+    }
+  `;
+
+  let worker = new Worker(workerScript, {type: 'string'});
+  worker.postMessage({module: module, memory: memory});
+
+  let main_instance = new WebAssembly.Instance(
+      module, {m: {imported_mem: memory, func: _ => 7}});
+
+  let counter = 0;
+  function CheckThreadNotInWasm() {
+    // We check the thread-in-wasm flag many times and reschedule ourselves in
+    // between to increase the chance that we read the flag set by the worker.
+    assertFalse(%IsThreadInWasm());
+    counter++;
+    if (counter < 100) {
+      setTimeout(CheckThreadNotInWasm, 0);
+    } else {
+      main_instance.exports.signal(sync_address);
+      assertEquals('finished', worker.getMessage());
+      worker.terminate();
+    }
+  }
+
+  assertFalse(%IsThreadInWasm());
+  assertEquals('start running', worker.getMessage());
+  CheckThreadNotInWasm();
+})();

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

@@ -215,17 +215,15 @@ class TrapHandlerTest : public TestWithIsolate,
 
   void GenerateSetThreadInWasmFlagCode(MacroAssembler* masm) {
     masm->Move(scratch,
-               ExternalReference::wasm_thread_in_wasm_flag_address_address(
-                   i_isolate()));
-    masm->movp(scratch, MemOperand(scratch, 0));
+               i_isolate()->thread_local_top()->thread_in_wasm_flag_address_,
+               RelocInfo::NONE);
     masm->movl(MemOperand(scratch, 0), Immediate(1));
   }
 
   void GenerateResetThreadInWasmFlagCode(MacroAssembler* masm) {
     masm->Move(scratch,
-               ExternalReference::wasm_thread_in_wasm_flag_address_address(
-                   i_isolate()));
-    masm->movp(scratch, MemOperand(scratch, 0));
+               i_isolate()->thread_local_top()->thread_in_wasm_flag_address_,
+               RelocInfo::NONE);
     masm->movl(MemOperand(scratch, 0), Immediate(0));
   }
 
@@ -240,13 +238,17 @@ class TrapHandlerTest : public TestWithIsolate,
     GeneratedCode<void>::FromAddress(i_isolate(),
                                      reinterpret_cast<Address>(buffer.start()))
         .Call();
+    CHECK(!g_test_handler_executed);
   }
 
   // Execute the code in buffer. We expect a crash which we recover from in the
   // test handler.
   void ExecuteExpectCrash(Vector<byte> buffer, bool check_wasm_flag = true) {
     CHECK(!g_test_handler_executed);
-    ExecuteBuffer(buffer);
+    MakeAssemblerBufferExecutable(buffer.start(), buffer.size());
+    GeneratedCode<void>::FromAddress(i_isolate(),
+                                     reinterpret_cast<Address>(buffer.start()))
+        .Call();
     CHECK(g_test_handler_executed);
     g_test_handler_executed = false;
     if (check_wasm_flag) CHECK(!GetThreadInWasmFlag());