[wasm][asm.js] Fix and enable several asm.js tests with the new parser.

Fix a few items broken during review of scanner + parser:
* Make the scanner retain stale newline state on a rewind (as otherwise it must be able to correctly rewind that too, though it doesn't need it). (Probably should revisit).
* Change StashCode in the builder skip to the zero case, as it crashes for some reason (added TODO).

Also fix:
* Drop test based on constant expression evaluation in main parser
* Support constant defined based on existing constant.
* Type constants as signed.
* Added a check that all used functions are defined eventually.
* Zone allocate strings for simplicity (TODOs to refactor better).

BUG=v8:6090
BUG=v8:4203
R=mstarzinger@chromium.org,marja@chromium.org,vogelheim@chromium.org

Review-Url: https://codereview.chromium.org/2771183002
Cr-Original-Commit-Position: refs/heads/master@{#44200}
Committed: https://chromium.googlesource.com/v8/v8/+/be0dbdd679b60c31d480d7635e579787a6a218df
Review-Url: https://codereview.chromium.org/2771183002
Cr-Commit-Position: refs/heads/master@{#44203}

src/asmjs/asm-parser.cc

@@ -71,12 +71,11 @@ namespace wasm {
 
 #define RECURSE_OR_RETURN(ret, call)                                       \
   do {                                                                     \
-    DCHECK(GetCurrentStackPosition() >= stack_limit_);                     \
     DCHECK(!failed_);                                                      \
-    call;                                                                  \
     if (GetCurrentStackPosition() < stack_limit_) {                        \
       FAIL_AND_RETURN(ret, "Stack overflow while parsing asm.js module."); \
     }                                                                      \
+    call;                                                                  \
     if (failed_) return ret;                                               \
   } while (false);
 
@@ -238,21 +237,25 @@ uint32_t AsmJsParser::VarIndex(VarInfo* info) {
 void AsmJsParser::AddGlobalImport(std::string name, AsmType* type,
                                   ValueType vtype, bool mutable_variable,
                                   VarInfo* info) {
+  // TODO(bradnelson): Refactor memory management here.
+  // 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, 0, info->index, true});
+    global_imports_.push_back({name_data, name.size(), 0, info->index, true});
   } else {
     // Just use the import directly.
-    global_imports_.push_back({name, 0, info->index, false});
+    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);
+      name_data, static_cast<int>(name.size()), vtype);
   if (!mutable_variable) {
     info->DeclareGlobalImport(type, gi.import_index);
   }
@@ -362,6 +365,16 @@ void AsmJsParser::ValidateModule() {
   }
   RECURSE(ValidateExport());
 
+  // Check that all functions were eventually defined.
+  for (auto info : global_var_info_) {
+    if (info.kind != VarKind::kFunction) {
+      continue;
+    }
+    if (!info.function_defined) {
+      FAIL("Undefined function");
+    }
+  }
+
   // Add start function to init things.
   WasmFunctionBuilder* start = module_builder_->AddFunction();
   module_builder_->MarkStartFunction(start);
@@ -447,8 +460,9 @@ void AsmJsParser::ValidateModuleVar(bool mutable_variable) {
     if (uvalue > 0x7fffffff) {
       FAIL("Numeric literal out of range");
     }
-    DeclareGlobal(info, mutable_variable, AsmType::Int(), kWasmI32,
-                  WasmInitExpr(static_cast<int32_t>(uvalue)));
+    DeclareGlobal(info, mutable_variable,
+                  mutable_variable ? AsmType::Int() : AsmType::Signed(),
+                  kWasmI32, WasmInitExpr(static_cast<int32_t>(uvalue)));
   } else if (Check('-')) {
     if (CheckForDouble(&dvalue)) {
       DeclareGlobal(info, mutable_variable, AsmType::Double(), kWasmF64,
@@ -457,8 +471,9 @@ void AsmJsParser::ValidateModuleVar(bool mutable_variable) {
       if (uvalue > 0x7fffffff) {
         FAIL("Numeric literal out of range");
       }
-      DeclareGlobal(info, mutable_variable, AsmType::Int(), kWasmI32,
-                    WasmInitExpr(-static_cast<int32_t>(uvalue)));
+      DeclareGlobal(info, mutable_variable,
+                    mutable_variable ? AsmType::Int() : AsmType::Signed(),
+                    kWasmI32, WasmInitExpr(-static_cast<int32_t>(uvalue)));
     } else {
       FAIL("Expected numeric literal");
     }
@@ -470,16 +485,33 @@ void AsmJsParser::ValidateModuleVar(bool mutable_variable) {
   } else if (ValidateModuleVarImport(info, mutable_variable)) {
     // Handled inside.
   } else if (scanner_.IsGlobal()) {
-    RECURSE(ValidateModuleVarFloat(info, mutable_variable));
+    RECURSE(ValidateModuleVarFromGlobal(info, mutable_variable));
   } else {
     FAIL("Bad variable declaration");
   }
 }
 
 // 6.1 ValidateModule - global float declaration
-void AsmJsParser::ValidateModuleVarFloat(VarInfo* info, bool mutable_variable) {
-  if (!GetVarInfo(Consume())->type->IsA(stdlib_fround_)) {
-    FAIL("Expected fround");
+void AsmJsParser::ValidateModuleVarFromGlobal(VarInfo* info,
+                                              bool mutable_variable) {
+  VarInfo* src_info = GetVarInfo(Consume());
+  if (!src_info->type->IsA(stdlib_fround_)) {
+    if (src_info->mutable_variable) {
+      FAIL("Can only use immutable variables in global definition");
+    }
+    if (mutable_variable) {
+      FAIL("Can only define immutable variables with other immutables");
+    }
+    if (!src_info->type->IsA(AsmType::Int()) &&
+        !src_info->type->IsA(AsmType::Float()) &&
+        !src_info->type->IsA(AsmType::Double())) {
+      FAIL("Expected int, float, double, or fround for global definition");
+    }
+    info->kind = VarKind::kGlobal;
+    info->type = src_info->type;
+    info->index = src_info->index;
+    info->mutable_variable = false;
+    return;
   }
   EXPECT_TOKEN('(');
   bool negate = false;
@@ -532,7 +564,11 @@ bool AsmJsParser::ValidateModuleVarImport(VarInfo* info,
     info->kind = VarKind::kImportedFunction;
     function_import_info_.resize(function_import_info_.size() + 1);
     info->import = &function_import_info_.back();
-    info->import->name = import_name;
+    // TODO(bradnelson): Refactor memory management here.
+    // AsmModuleBuilder should really own import names.
+    info->import->function_name = zone()->NewArray<char>(import_name.size());
+    memcpy(info->import->function_name, import_name.data(), import_name.size());
+    info->import->function_name_size = import_name.size();
     return true;
   }
   return false;
@@ -569,6 +605,7 @@ void AsmJsParser::ValidateModuleVarStdlib(VarInfo* info) {
   case TOK(name):                                           \
     DeclareGlobal(info, false, AsmType::Double(), kWasmF64, \
                   WasmInitExpr(M_##name));                  \
+    stdlib_uses_.insert(AsmTyper::kMath##name);             \
     break;
       STDLIB_MATH_VALUE_LIST(V)
 #undef V
@@ -585,9 +622,11 @@ void AsmJsParser::ValidateModuleVarStdlib(VarInfo* info) {
   } else if (Check(TOK(Infinity))) {
     DeclareGlobal(info, false, AsmType::Double(), kWasmF64,
                   WasmInitExpr(std::numeric_limits<double>::infinity()));
+    stdlib_uses_.insert(AsmTyper::kInfinity);
   } else if (Check(TOK(NaN))) {
     DeclareGlobal(info, false, AsmType::Double(), kWasmF64,
                   WasmInitExpr(std::numeric_limits<double>::quiet_NaN()));
+    stdlib_uses_.insert(AsmTyper::kNaN);
   } else {
     FAIL("Invalid member of stdlib");
   }
@@ -695,13 +734,18 @@ void AsmJsParser::ValidateFunction() {
   if (function_info->kind == VarKind::kUnused) {
     function_info->kind = VarKind::kFunction;
     function_info->function_builder = module_builder_->AddFunction();
+    // TODO(bradnelson): Cleanup memory management here.
+    // WasmModuleBuilder should own these.
+    char* function_name = zone()->NewArray<char>(function_name_raw.size());
+    memcpy(function_name, function_name_raw.data(), function_name_raw.size());
     function_info->function_builder->SetName(
-        {function_name_raw.c_str(),
-         static_cast<int>(function_name_raw.size())});
+        {function_name, static_cast<int>(function_name_raw.size())});
     function_info->index = function_info->function_builder->func_index();
     function_info->function_defined = true;
   } else if (function_info->function_defined) {
     FAIL("Function redefined");
+  } else {
+    function_info->function_defined = true;
   }
   current_function_builder_ = function_info->function_builder;
   return_type_ = nullptr;
@@ -855,7 +899,7 @@ void AsmJsParser::ValidateFunctionLocals(
           info->type = AsmType::Double();
           info->index = static_cast<uint32_t>(param_count + locals->size());
           locals->push_back(kWasmF64);
-          byte code[] = {WASM_F64(dvalue)};
+          byte code[] = {WASM_F64(-dvalue)};
           current_function_builder_->EmitCode(code, sizeof(code));
           current_function_builder_->EmitSetLocal(info->index);
         } else if (CheckForUnsigned(&uvalue)) {
@@ -2063,8 +2107,9 @@ AsmType* AsmJsParser::ValidateCall() {
     uint32_t index;
     if (cache_index >= function_info->import->cache_index.size()) {
       index = module_builder_->AddImport(
-          function_info->import->name.data(),
-          static_cast<uint32_t>(function_info->import->name.size()), sig);
+          function_info->import->function_name,
+          static_cast<uint32_t>(function_info->import->function_name_size),
+          sig);
       function_info->import->cache_index.push_back(index);
     } else {
       index = function_info->import->cache_index[cache_index];

src/asmjs/asm-parser.h

@@ -59,7 +59,8 @@ class AsmJsParser {
   // clang-format on
 
   struct FunctionImportInfo {
-    std::string name;
+    char* function_name;
+    size_t function_name_size;
     SignatureMap cache;
     std::vector<uint32_t> cache_index;
   };
@@ -80,7 +81,8 @@ class AsmJsParser {
   };
 
   struct GlobalImport {
-    std::string import_name;
+    char* import_name;
+    size_t import_name_size;
     uint32_t import_index;
     uint32_t global_index;
     bool needs_init;
@@ -248,7 +250,7 @@ class AsmJsParser {
   bool ValidateModuleVarImport(VarInfo* info, bool mutable_variable);
   void ValidateModuleVarStdlib(VarInfo* info);
   void ValidateModuleVarNewStdlib(VarInfo* info);
-  void ValidateModuleVarFloat(VarInfo* info, bool mutable_variable);
+  void ValidateModuleVarFromGlobal(VarInfo* info, bool mutable_variable);
 
   void ValidateExport();         // 6.2 ValidateExport
   void ValidateFunctionTable();  // 6.3 ValidateFunctionTable

src/asmjs/asm-scanner.cc

@@ -145,12 +145,14 @@ void AsmJsScanner::Next() {
 }
 
 void AsmJsScanner::Rewind() {
+  // TODO(bradnelson): Currently rewinding needs to leave in place the
+  // preceding newline state (in case a |0 ends a line).
+  // This is weird and stateful, fix me.
   DCHECK(!rewind_);
   next_token_ = token_;
   token_ = preceding_token_;
   preceding_token_ = kUninitialized;
   rewind_ = true;
-  preceded_by_newline_ = false;
   identifier_string_.clear();
 }
 

src/wasm/wasm-module-builder.cc

@@ -190,9 +190,10 @@ void WasmFunctionBuilder::StashCode(std::vector<byte>* dst, size_t position) {
     body_.resize(position);
     return;
   }
+  DCHECK_LE(position, body_.size());
   size_t len = body_.size() - position;
   dst->resize(len);
-  memcpy(dst->data(), &body_[position], len);
+  memcpy(dst->data(), body_.data() + position, len);
   body_.resize(position);
 }
 

src/wasm/wasm-module.cc

@@ -1835,7 +1835,7 @@ class InstantiationHelper {
                             module_name, import_name);
             return -1;
           }
-          if (FLAG_fast_validate_asm) {
+          if (module_->is_asm_js() && FLAG_fast_validate_asm) {
             if (module_->globals[import.index].type == kWasmI32) {
               value = Object::ToInt32(isolate_, value).ToHandleChecked();
             } else {

test/mjsunit/asm/asm-validation.js

@@ -339,23 +339,6 @@ function assertValidAsm(func) {
   assertFalse(%IsAsmWasmCode(Module));
 })();
 
-(function TestBadishBooleanExprAnnotation() {
-  function Module() {
-    "use asm";
-    function foo(x) {
-      x = x | 0;
-      x = (x + 1) | false;
-      return x | 0;
-    }
-    return { foo: foo };
-  }
-  var m = Module();
-  // We all false here because the parser optimizes expressons like:
-  // !123 to false.
-  assertTrue(%IsAsmWasmCode(Module));
-  assertEquals(4, m.foo(3));
-})();
-
 (function TestBadFroundTrue() {
   function Module(stdlib) {
     "use asm";

test/mjsunit/mjsunit.status

@@ -662,44 +662,19 @@
 ['variant == asm_wasm', {
   # Issue 6127: We won't fix these in the "old" validator. But we definitely
   # need to re-enable these for the "new" validator.
-  'code-coverage-precise': [SKIP],
-  'object-freeze': [SKIP],
-  'asm/asm-validation': [SKIP],
-  'asm/infinite-loops': [SKIP],
-  'es6/completion': [SKIP],
-  'es6/function-length-configurable': [SKIP],
-  'es6/object-assign': [SKIP],
-  'es6/proxies-cross-realm-exception': [SKIP],
-  'harmony/function-tostring': [SKIP],
   'compiler/regress-445859': [SKIP],
   'compiler/regress-452427': [SKIP],
-  'regress/regress-353058': [SKIP],
   'regress/regress-458987': [SKIP],
-  'regress/regress-599068-func-bindings': [SKIP],
   'regress/regress-599719': [SKIP],
   'regress/regress-618608': [SKIP],
-  'regress/regress-619382': [SKIP],
-  'regress/regress-632289': [SKIP],
   'regress/regress-670808': [SKIP],
-  'regress/regress-crbug-530598': [SKIP],
-  'wasm/asm-wasm': [SKIP],
   'wasm/asm-wasm-exception-in-tonumber': [SKIP],
   'wasm/asm-wasm-stack': [SKIP],
-  'wasm/errors': [SKIP],
 
   # Issue 6127: Tiggers ASAN reports with "new" validator.
   'asm/b5528-comma': [SKIP],
   'asm/if-folding': [SKIP],
   'asm/if-reduction': [SKIP],
-  'wasm/embenchen/box2d': [SKIP],
-  'wasm/embenchen/copy': [SKIP],
-  'wasm/embenchen/corrections': [SKIP],
-  'wasm/embenchen/fannkuch': [SKIP],
-  'wasm/embenchen/fasta': [SKIP],
-  'wasm/embenchen/lua_binarytrees': [SKIP],
-  'wasm/embenchen/memops': [SKIP],
-  'wasm/embenchen/primes': [SKIP],
-  'wasm/embenchen/zlib': [SKIP],
 
   # Issue 6127: Breaks on no-snap bots.
   'es8/regress/regress-624300': [SKIP],