[asm.js] Ensure coercion of imports is non-observable.

This makes sure that the coercion of global import values to numbers
remains non-observable to JavaScript. It allows instantiation failures
to fall back to JavaScript proper without accidentally causing some
side-effect to happen twice. Also coercions might invalidate previous
checks done during linking or throw exceptions.

R=clemensh@chromium.org
TEST=mjsunit/regress/regress-6431
BUG=v8:6431

Change-Id: Ibe2f7a336bc0fb25532d526746ecc802e04bbd5c
Reviewed-on: https://chromium-review.googlesource.com/512544
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46051}

src/wasm/module-compiler.cc

@@ -1446,10 +1446,19 @@ int InstanceBuilder::ProcessImports(Handle<FixedArray> code_table,
           return -1;
         }
         if (module_->is_asm_js()) {
-          if (module_->globals[import.index].type == kWasmI32) {
-            value = Object::ToInt32(isolate_, value).ToHandleChecked();
-          } else {
-            value = Object::ToNumber(value).ToHandleChecked();
+          // Accepting {JSFunction} on top of just primitive values here is a
+          // workaround to support legacy asm.js code with broken binding. Note
+          // that using {NaN} (or Smi::kZero) here is what using the observable
+          // conversion via {ToPrimitive} would produce as well.
+          // TODO(mstarzinger): Still observable if Function.prototype.valueOf
+          // or friends are patched, we might need to check for that as well.
+          if (value->IsJSFunction()) value = isolate_->factory()->nan_value();
+          if (value->IsPrimitive() && !value->IsSymbol()) {
+            if (module_->globals[import.index].type == kWasmI32) {
+              value = Object::ToInt32(isolate_, value).ToHandleChecked();
+            } else {
+              value = Object::ToNumber(value).ToHandleChecked();
+            }
           }
         }
         if (!value->IsNumber()) {

test/mjsunit/asm/global-imports.js

@@ -74,6 +74,7 @@ Run("var x = stdlib.Math.E",    "(x = 3.1415, 1) | 0",      {},      1,  false);
 Run("var x = stdlib.Math.imul", "(x = fround, 1) | 0",      {},      1,  false);
 
 // Imports missing or causing side-effects during lookup.
-Run("var x = +foreign.x", "+x", { no_x_present:0 },        NaN, true);
-Run("var x = +foreign.x", "+x", { get x() { return 23 } }, 23,  false);
-Run("var x = +foreign.x", "+x", new Proxy({ x:42 }, {}),   42,  false);
+Run("var x = +foreign.x", "+x", { no_x_present:0 },             NaN, true);
+Run("var x = +foreign.x", "+x", { get x() { return 23 } },      23,  false);
+Run("var x = +foreign.x", "+x", new Proxy({ x:42 }, {}),        42,  false);
+Run("var x = +foreign.x", "+x", { x : { valueOf : () => 65 } }, 65,  false);

test/mjsunit/regress/regress-6431.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.
+
+// Flags: --allow-natives-syntax
+
+(function TestImportSymbolValue() {
+  function Module(stdlib, foreign) {
+    "use asm";
+    var x = +foreign.x;
+    function f() {}
+    return { f:f };
+  }
+  var foreign = { x : Symbol("boom") };
+  assertThrows(() => Module(this, foreign));
+  assertFalse(%IsAsmWasmCode(Module));
+})();
+
+(function TestImportMutatingObject() {
+  function Module(stdlib, foreign) {
+    "use asm";
+    var x = +foreign.x;
+    var PI = stdlib.Math.PI;
+    function f() { return +(PI + x) }
+    return { f:f };
+  }
+  var stdlib = { Math : { PI : Math.PI } };
+  var foreign = { x : { valueOf : () => (stdlib.Math.PI = 23, 42) } };
+  var m = Module(stdlib, foreign);
+  assertFalse(%IsAsmWasmCode(Module));
+  assertEquals(65, m.f());
+})();

test/mjsunit/wasm/asm-wasm.js

@@ -1153,19 +1153,7 @@ function TestForeignVariables() {
   TestCase({foo: null, bar: null, baz: null}, 0, 0, 0, 0);
   // Check that string values are converted properly.
   TestCase({foo: 'hi', bar: 'there', baz: 'dude'}, 0, NaN, 0, NaN);
-  TestCase({foo: '0xff', bar: '234', baz: '456.1'}, 255, 234, 456, 456.1, 456);
-  // Check that Date values are converted properly.
-  TestCase({foo: new Date(123), bar: new Date(456),
-            baz: new Date(789)}, 123, 456, 789, 789);
-  // Check that list values are converted properly.
-  TestCase({foo: [], bar: [], baz: []}, 0, 0, 0, 0);
-  // Check that object values are converted properly.
-  TestCase({foo: {}, bar: {}, baz: {}}, 0, NaN, 0, NaN);
-  // Check that objects with valueOf are converted properly.
-  var o = {
-    valueOf: function() { return 99; }
-  };
-  TestCase({foo: o, bar: o, baz: o}, 99, 99, 99, 99);
+  TestCase({foo: '0xff', bar: '234', baz: '456.1'}, 255, 234, 456, 456.1);
   // Check that function values are converted properly.
   TestCase({foo: TestCase, bar: TestCase, qux: TestCase}, 0, NaN, 0, NaN);
 }