[wasm] set thread-in-wasm flag after converting arguments

In JS to Wasm wrappers, arguments have to be converted from JavaScript's
representation to Wasm's representation. Because of property accessors, this can
result in JavaScript or even asm.js/Wasm code being run. We were previously
setting this flag before doing the parameter conversions, and if these
conversions triggered a Wasm property getter then we would try to set the flag
twice.

With this change, we wait until after all argument conversions are done to set
the flag.

Bug: chromium:769846

R=bradnelson@chromium.org

Change-Id: Ia4b56df45619dcad69f3750bb33cacfedcaeb5b2
Reviewed-on: https://chromium-review.googlesource.com/693414
Commit-Queue: Brad Nelson <bradnelson@chromium.org>
Reviewed-by: Brad Nelson <bradnelson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48244}

src/compiler/wasm-compiler.cc

@@ -2796,15 +2796,12 @@ void WasmGraphBuilder::BuildJSToWasmWrapper(Handle<Code> wasm_code,
       reinterpret_cast<uintptr_t>(wasm_context_address),
       RelocInfo::WASM_CONTEXT_REFERENCE);
 
-  // Set the ThreadInWasm flag before we do the actual call.
-  BuildModifyThreadInWasmFlag(true);
-
   if (!wasm::IsJSCompatibleSignature(sig_)) {
     // 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.
-    BuildCallToRuntimeWithContext(Runtime::kWasmThrowTypeError, js_context,
-                                  nullptr, 0);
+    BuildCallToRuntimeWithContextFromJS(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
@@ -2835,6 +2832,9 @@ void WasmGraphBuilder::BuildJSToWasmWrapper(Handle<Code> wasm_code,
     args[pos++] = wasm_param;
   }
 
+  // Set the ThreadInWasm flag before we do the actual call.
+  BuildModifyThreadInWasmFlag(true);
+
   args[pos++] = *effect_;
   args[pos++] = *control_;
 
@@ -3313,7 +3313,21 @@ Node* WasmGraphBuilder::BuildCallToRuntimeWithContext(Runtime::FunctionId f,
   DCHECK_NE(f, Runtime::kClearThreadInWasm);
   // 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,
@@ -3342,9 +3356,6 @@ Node* WasmGraphBuilder::BuildCallToRuntimeWithContext(Runtime::FunctionId f,
                                            count, inputs);
   *effect_ = node;
 
-  // Restore the thread-in-wasm flag, since we have returned to Wasm.
-  *control_ = BuildModifyThreadInWasmFlag(true);
-
   return node;
 }
 

src/compiler/wasm-compiler.h

@@ -522,7 +522,10 @@ class WasmGraphBuilder {
 
   Node* BuildCallToRuntimeWithContext(Runtime::FunctionId f, Node* js_context,
                                       Node** parameters, int parameter_count);
-
+  Node* BuildCallToRuntimeWithContextFromJS(Runtime::FunctionId f,
+                                            Node* js_context,
+                                            Node* const* parameters,
+                                            int parameter_count);
   Node* BuildModifyThreadInWasmFlag(bool new_value);
   Builtins::Name GetBuiltinIdForTrap(wasm::TrapReason reason);
 };

test/mjsunit/regress/wasm/regression-769846.js

+// Copyright 2017 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.
+
+function Module() {
+  "use asm";
+  function div_(__v_6) {
+    __v_6 = __v_6 | 0;
+  }
+  return { f: div_}
+};
+var __f_0 = Module().f;
+__v_8 = [0];
+__v_8.__defineGetter__(0, function() { return __f_0(__v_8); });
+__v_8[0];