[wasm] Move "thread in wasm" flag handling out of compiled code

Instead of modifying this flag in compiled wasm code, we can just
change it in the caller / called code. This saves code space and
compilation time and fixes the referenced bug.

R=titzer@chromium.org, eholk@chromium.org

Bug: chromium:773631, v8:5277
Change-Id: I095158ac01eecd21a92649a3990e8d7c593db912
Reviewed-on: https://chromium-review.googlesource.com/712597
Reviewed-by: Ben Titzer <titzer@chromium.org>
Reviewed-by: Eric Holk <eholk@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48602}

src/compiler/wasm-compiler.cc

@@ -2796,8 +2796,8 @@ void WasmGraphBuilder::BuildJSToWasmWrapper(Handle<Code> wasm_code,
     // Throw a TypeError. Use the js_context of the calling javascript function
     // (passed as a parameter), such that the generated code is js_context
     // independent.
-    BuildCallToRuntimeWithContextFromJS(Runtime::kWasmThrowTypeError,
-                                        js_context, nullptr, 0);
+    BuildCallToRuntimeWithContext(Runtime::kWasmThrowTypeError, js_context,
+                                  nullptr, 0);
 
     // Add a dummy call to the wasm function so that the generated wrapper
     // contains a reference to the wrapped wasm function. Without this reference
@@ -3133,9 +3133,6 @@ void WasmGraphBuilder::BuildCWasmEntry(Address wasm_context_address) {
   Node* code_obj = Param(CWasmEntryParameters::kCodeObject + 1);
   Node* arg_buffer = Param(CWasmEntryParameters::kArgumentsBuffer + 1);
 
-  // Set the ThreadInWasm flag before we do the actual call.
-  BuildModifyThreadInWasmFlag(true);
-
   int wasm_arg_count = static_cast<int>(sig_->parameter_count());
   int arg_count = wasm_arg_count + 4;  // code, wasm_context, control, effect
   Node** args = Buffer(arg_count);
@@ -3165,9 +3162,6 @@ void WasmGraphBuilder::BuildCWasmEntry(Address wasm_context_address) {
       graph()->NewNode(jsgraph()->common()->Call(desc), arg_count, args);
   *effect_ = call;
 
-  // Clear the ThreadInWasmFlag
-  BuildModifyThreadInWasmFlag(false);
-
   // Store the return value.
   DCHECK_GE(1, sig_->return_count());
   if (sig_->return_count() == 1) {
@@ -3320,23 +3314,6 @@ Node* WasmGraphBuilder::BuildCallToRuntimeWithContext(Runtime::FunctionId f,
                                                       Node* js_context,
                                                       Node** parameters,
                                                       int parameter_count) {
-  // We're leaving Wasm code, so clear the flag.
-  *control_ = BuildModifyThreadInWasmFlag(false);
-  // Since the thread-in-wasm flag is clear, it is as if we are calling from JS.
-  Node* call = BuildCallToRuntimeWithContextFromJS(f, js_context, parameters,
-                                                   parameter_count);
-
-  // Restore the thread-in-wasm flag, since we have returned to Wasm.
-  *control_ = BuildModifyThreadInWasmFlag(true);
-
-  return call;
-}
-
-// This version of BuildCallToRuntime does not clear and set the thread-in-wasm
-// flag.
-Node* WasmGraphBuilder::BuildCallToRuntimeWithContextFromJS(
-    Runtime::FunctionId f, Node* js_context, Node* const* parameters,
-    int parameter_count) {
   const Runtime::Function* fun = Runtime::FunctionForId(f);
   CallDescriptor* desc = Linkage::GetRuntimeCallDescriptor(
       jsgraph()->zone(), f, fun->nargs, Operator::kNoProperties,

src/runtime/runtime-wasm.cc

@@ -41,6 +41,24 @@ Context* GetWasmContextOnStackTop(Isolate* isolate) {
       ->compiled_module()
       ->ptr_to_native_context();
 }
+
+class ClearThreadInWasmScope {
+ public:
+  explicit ClearThreadInWasmScope(bool coming_from_wasm)
+      : coming_from_wasm_(coming_from_wasm) {
+    DCHECK_EQ(trap_handler::UseTrapHandler() && coming_from_wasm,
+              trap_handler::IsThreadInWasm());
+    if (coming_from_wasm) trap_handler::ClearThreadInWasm();
+  }
+  ~ClearThreadInWasmScope() {
+    DCHECK(!trap_handler::IsThreadInWasm());
+    if (coming_from_wasm_) trap_handler::SetThreadInWasm();
+  }
+
+ private:
+  const bool coming_from_wasm_;
+};
+
 }  // namespace
 
 RUNTIME_FUNCTION(Runtime_WasmGrowMemory) {
@@ -50,6 +68,9 @@ RUNTIME_FUNCTION(Runtime_WasmGrowMemory) {
   Handle<WasmInstanceObject> instance(GetWasmInstanceOnStackTop(isolate),
                                       isolate);
 
+  // This runtime function is always being called from wasm code.
+  ClearThreadInWasmScope flag_scope(true);
+
   // Set the current isolate's context.
   DCHECK_NULL(isolate->context());
   isolate->set_context(instance->compiled_module()->ptr_to_native_context());
@@ -110,6 +131,7 @@ Object* ThrowRuntimeError(Isolate* isolate, int message_id, int byte_offset,
 RUNTIME_FUNCTION(Runtime_ThrowWasmErrorFromTrapIf) {
   DCHECK_EQ(1, args.length());
   CONVERT_SMI_ARG_CHECKED(message_id, 0);
+  ClearThreadInWasmScope clear_wasm_flag(isolate->context() == nullptr);
   return ThrowRuntimeError(isolate, message_id, 0, false);
 }
 
@@ -117,6 +139,7 @@ RUNTIME_FUNCTION(Runtime_ThrowWasmError) {
   DCHECK_EQ(2, args.length());
   CONVERT_SMI_ARG_CHECKED(message_id, 0);
   CONVERT_SMI_ARG_CHECKED(byte_offset, 1);
+  ClearThreadInWasmScope clear_wasm_flag(isolate->context() == nullptr);
   return ThrowRuntimeError(isolate, message_id, byte_offset, true);
 }
 
@@ -266,6 +289,8 @@ RUNTIME_FUNCTION(Runtime_WasmRunInterpreter) {
   CHECK(arg_buffer_obj->IsSmi());
   uint8_t* arg_buffer = reinterpret_cast<uint8_t*>(*arg_buffer_obj);
 
+  ClearThreadInWasmScope wasm_flag(true);
+
   // Set the current isolate's context.
   DCHECK_NULL(isolate->context());
   isolate->set_context(instance->compiled_module()->ptr_to_native_context());
@@ -297,11 +322,7 @@ RUNTIME_FUNCTION(Runtime_WasmStackGuard) {
   DCHECK_EQ(0, args.length());
   DCHECK(!trap_handler::UseTrapHandler() || trap_handler::IsThreadInWasm());
 
-  struct ClearAndRestoreThreadInWasm {
-    ClearAndRestoreThreadInWasm() { trap_handler::ClearThreadInWasm(); }
-
-    ~ClearAndRestoreThreadInWasm() { trap_handler::SetThreadInWasm(); }
-  } restore_thread_in_wasm;
+  ClearThreadInWasmScope wasm_flag(true);
 
   // Set the current isolate's context.
   DCHECK_NULL(isolate->context());

src/trap-handler/handler-inside.cc

@@ -102,13 +102,15 @@ bool TryHandleSignal(int signum, siginfo_t* info, ucontext_t* context) {
     if (TryFindLandingPad(fault_addr, &landing_pad)) {
       // Tell the caller to return to the landing pad.
       context->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;
     }
   }  // end signal mask scope
 
   // If we get here, it's not a recoverable wasm fault, so we go to the next
-  // handler.
-  g_thread_in_wasm_code = true;
+  // handler. Leave the g_thread_in_wasm_code flag unset since we do not return
+  // to wasm code.
   return false;
 }
 

src/wasm/wasm-interpreter.cc

@@ -2224,9 +2224,18 @@ class ThreadImpl {
     args[compiler::CWasmEntryParameters::kArgumentsBuffer] = arg_buffer_obj;
 
     Handle<Object> receiver = isolate->factory()->undefined_value();
+    trap_handler::SetThreadInWasm();
     MaybeHandle<Object> maybe_retval =
         Execution::Call(isolate, wasm_entry, receiver, arraysize(args), args);
-    if (maybe_retval.is_null()) return TryHandleException(isolate);
+    TRACE("  => External wasm function returned%s\n",
+          maybe_retval.is_null() ? " with exception" : "");
+
+    if (maybe_retval.is_null()) {
+      DCHECK(!trap_handler::IsThreadInWasm());
+      return TryHandleException(isolate);
+    }
+
+    trap_handler::ClearThreadInWasm();
 
     // Pop arguments off the stack.
     sp_ -= num_args;