[asm.js] Test and fix global variable imports.

This also removes a broken optimization regarding immutable (i.e. const)
global variables. For now mutable and immutable global variables are
treated the same and hence copied during module initialization.

R=rossberg@chromium.org
TEST=mjsunit/asm/global-imports
BUG=v8:6279

Change-Id: I020fc12036dc534f5a62fb43f5c6fdb252314e62
Reviewed-on: https://chromium-review.googlesource.com/483360Reviewed-by: Andreas Rossberg <rossberg@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44763}

src/asmjs/asm-parser.cc

@@ -231,11 +231,8 @@ wasm::AsmJsParser::VarInfo* AsmJsParser::GetVarInfo(
 }
 
 uint32_t AsmJsParser::VarIndex(VarInfo* info) {
-  if (info->import != nullptr) {
-    return info->index;
-  } else {
-    return info->index + static_cast<uint32_t>(global_imports_.size());
-  }
+  DCHECK(info->kind == VarKind::kGlobal);
+  return info->index + static_cast<uint32_t>(global_imports_.size());
 }
 
 void AsmJsParser::AddGlobalImport(std::string name, AsmType* type,
@@ -245,38 +242,14 @@ void AsmJsParser::AddGlobalImport(std::string name, AsmType* type,
   // AsmModuleBuilder should really own import names.
   char* name_data = zone()->NewArray<char>(name.size());
   memcpy(name_data, name.data(), name.size());
-  if (mutable_variable) {
-    // Allocate a separate variable for the import.
-    DeclareGlobal(info, true, type, vtype);
-    // Record the need to initialize the global from the import.
-    global_imports_.push_back({name_data, name.size(), 0, info->index, true});
-  } else {
-    // Just use the import directly.
-    global_imports_.push_back({name_data, name.size(), 0, info->index, false});
-  }
-  GlobalImport& gi = global_imports_.back();
-  // TODO(bradnelson): Reuse parse buffer memory / make wasm-module-builder
-  // managed the memory for the import name (currently have to keep our
-  // own memory for it).
-  gi.import_index = module_builder_->AddGlobalImport(
-      name_data, static_cast<int>(name.size()), vtype);
-  if (!mutable_variable) {
-    info->DeclareGlobalImport(type, gi.import_index);
-  }
-}
 
-void AsmJsParser::VarInfo::DeclareGlobalImport(AsmType* type, uint32_t index) {
-  kind = VarKind::kGlobal;
-  this->type = type;
-  this->index = index;
-  mutable_variable = false;
-}
+  // Allocate a separate variable for the import.
+  // TODO(mstarzinger): Consider using the imported global directly instead of
+  // allocating a separate global variable for immutable (i.e. const) imports.
+  DeclareGlobal(info, mutable_variable, type, vtype);
 
-void AsmJsParser::VarInfo::DeclareStdlibFunc(VarKind kind, AsmType* type) {
-  this->kind = kind;
-  this->type = type;
-  index = 0;  // unused
-  mutable_variable = false;
+  // Record the need to initialize the global from the import.
+  global_imports_.push_back({name_data, name.size(), vtype, info});
 }
 
 void AsmJsParser::DeclareGlobal(VarInfo* info, bool mutable_variable,
@@ -288,6 +261,14 @@ void AsmJsParser::DeclareGlobal(VarInfo* info, bool mutable_variable,
   info->mutable_variable = mutable_variable;
 }
 
+void AsmJsParser::DeclareStdlibFunc(VarInfo* info, VarKind kind,
+                                    AsmType* type) {
+  info->kind = kind;
+  info->type = type;
+  info->index = 0;  // unused
+  info->mutable_variable = false;
+}
+
 uint32_t AsmJsParser::TempVariable(int index) {
   if (index + 1 > function_temp_locals_used_) {
     function_temp_locals_used_ = index + 1;
@@ -383,12 +364,15 @@ void AsmJsParser::ValidateModule() {
   WasmFunctionBuilder* start = module_builder_->AddFunction();
   module_builder_->MarkStartFunction(start);
   for (auto global_import : global_imports_) {
-    if (global_import.needs_init) {
-      start->EmitWithVarInt(kExprGetGlobal, global_import.import_index);
-      start->EmitWithVarInt(kExprSetGlobal,
-                            static_cast<uint32_t>(global_import.global_index +
-                                                  global_imports_.size()));
-    }
+    // TODO(bradnelson): Reuse parse buffer memory / make wasm-module-builder
+    // managed the memory for the import name (currently have to keep our
+    // own memory for it).
+    uint32_t import_index = module_builder_->AddGlobalImport(
+        global_import.import_name,
+        static_cast<int>(global_import.import_name_size),
+        global_import.value_type);
+    start->EmitWithVarInt(kExprGetGlobal, import_index);
+    start->EmitWithVarInt(kExprSetGlobal, VarIndex(global_import.var_info));
   }
   start->Emit(kExprEnd);
   FunctionSig::Builder b(zone(), 0, 0);
@@ -586,7 +570,7 @@ void AsmJsParser::ValidateModuleVarNewStdlib(VarInfo* info) {
   switch (Consume()) {
 #define V(name, _junk1, _junk2, _junk3)                          \
   case TOK(name):                                                \
-    info->DeclareStdlibFunc(VarKind::kSpecial, AsmType::name()); \
+    DeclareStdlibFunc(info, VarKind::kSpecial, AsmType::name()); \
     break;
     STDLIB_ARRAY_TYPE_LIST(V)
 #undef V
@@ -615,7 +599,7 @@ void AsmJsParser::ValidateModuleVarStdlib(VarInfo* info) {
 #undef V
 #define V(name, Name, op, sig)                                      \
   case TOK(name):                                                   \
-    info->DeclareStdlibFunc(VarKind::kMath##Name, stdlib_##sig##_); \
+    DeclareStdlibFunc(info, VarKind::kMath##Name, stdlib_##sig##_); \
     stdlib_uses_.insert(AsmTyper::kMath##Name);                     \
     break;
       STDLIB_MATH_FUNCTION_LIST(V)
@@ -1531,6 +1515,9 @@ AsmType* AsmJsParser::AssignmentExpression() {
       if (info->kind == VarKind::kLocal) {
         current_function_builder_->EmitTeeLocal(info->index);
       } else if (info->kind == VarKind::kGlobal) {
+        if (!info->mutable_variable) {
+          FAILn("Expected mutable variable in assignment");
+        }
         current_function_builder_->EmitWithVarUint(kExprSetGlobal,
                                                    VarIndex(info));
         current_function_builder_->EmitWithVarUint(kExprGetGlobal,

src/asmjs/asm-parser.h

@@ -76,16 +76,13 @@ class AsmJsParser {
     bool function_defined;
 
     VarInfo();
-    void DeclareGlobalImport(AsmType* type, uint32_t index);
-    void DeclareStdlibFunc(VarKind kind, AsmType* type);
   };
 
   struct GlobalImport {
     char* import_name;
     size_t import_name_size;
-    uint32_t import_index;
-    uint32_t global_index;
-    bool needs_init;
+    ValueType value_type;
+    VarInfo* var_info;
   };
 
   enum class BlockKind { kRegular, kLoop, kOther };
@@ -234,6 +231,7 @@ class AsmJsParser {
   void DeclareGlobal(VarInfo* info, bool mutable_variable, AsmType* type,
                      ValueType vtype,
                      const WasmInitExpr& init = WasmInitExpr());
+  void DeclareStdlibFunc(VarInfo* info, VarKind kind, AsmType* type);
 
   // Allocates a temporary local variable. The given {index} is absolute within
   // the function body, consider using {TemporaryVariableScope} when nesting.

test/mjsunit/asm/global-imports.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 --validate-asm --fast-validate-asm
+
+function MODULE_TEMPLATE(stdlib, foreign, buffer) {
+  "use asm";
+  var fround = stdlib.Math.fround;
+  IMPORT;
+  function f(int, flt, dbl) {
+    int = int | 0;
+    flt = fround(flt);
+    dbl = +dbl;
+    return EXPRESSION;
+  }
+  return { f:f };
+}
+
+var throws = {};
+var test_count = 0;
+const stdlib = this;
+const buffer = new ArrayBuffer(1024);
+
+function assertThrowsOrEquals(result, fun) {
+  if (result === throws) {
+    assertThrows(fun, TypeError);
+  } else {
+    assertEquals(result, fun(1, 2.3, 4.2));
+  }
+}
+
+function RunAsmJsTest(asm_source, imports, result, valid) {
+  var nonasm_source = asm_source.replace(new RegExp("use asm"), "");
+
+  var js_module = eval("(" + nonasm_source + ")")
+  var js_instance = js_module(stdlib, imports, buffer);
+  assertThrowsOrEquals(result, js_instance.f);
+
+  var asm_module = eval("(" + asm_source + ")");
+  var asm_instance = asm_module(stdlib, imports, buffer);
+  assertEquals(valid, %IsAsmWasmCode(asm_module));
+  assertThrowsOrEquals(result, asm_instance.f);
+}
+
+function Run(imp, exp, imports, result, valid) {
+  var name = "test" + (++test_count);
+  var src = MODULE_TEMPLATE.toString();
+  src = src.replace("IMPORT", imp);
+  src = src.replace("EXPRESSION", exp);
+  src = src.replace("MODULE_TEMPLATE", name);
+  RunAsmJsTest(src, imports, result, valid);
+}
+
+Run("var x = foreign.x | 0",   "(x + int) | 0", {x:12},  13,     true);
+Run("var x = foreign.x | 0",   "(x = int) | 0", {x:12},  1,      true);
+Run("var x = foreign.x | 0",   "+(x + dbl)",    {x:12},  16.2,   false);
+Run("var x = +foreign.x",      "+(x + dbl)",    {x:1.2}, 5.4,    true);
+Run("var x = +foreign.x",      "+(x = dbl)",    {x:1.2}, 4.2,    true);
+Run("var x = +foreign.x",      "(x + int) | 0", {x:1.2}, 2,      false);
+Run("const x = foreign.x | 0", "(x + int) | 0", {x:12},  13,     true);
+Run("const x = foreign.x | 0", "(x = int) | 0", {x:12},  throws, false);
+Run("const x = foreign.x | 0", "+(x + dbl)",    {x:12},  16.2,   false);
+Run("const x = +foreign.x",    "+(x + dbl)",    {x:1.2}, 5.4,    true);
+Run("const x = +foreign.x",    "+(x = dbl)",    {x:1.2}, throws, false);
+Run("const x = +foreign.x",    "(x + int) | 0", {x:1.2}, 2,      false);