[asm.js] Fix source positions in for-statement parsing.

This fixes source position tracking within the "increment" expression of
a for-statement. The old {StashCode} method was inherently incompatible
with side tables like the source position table, as it would leave them
untouched while mutating the bytecode stream. It was hence trimmed down
to {DeleteCode}.

R=bradnelson@chromium.org
BUG=v8:6127

Change-Id: I7a5ff60cd5334208c44b165c8b54144d9ae83209
Reviewed-on: https://chromium-review.googlesource.com/480301
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Brad Nelson <bradnelson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44714}

src/asmjs/asm-parser.cc

@@ -1219,20 +1219,20 @@ void AsmJsParser::ForStatement() {
     current_function_builder_->EmitWithU8(kExprBrIf, 1);
   }
   EXPECT_TOKEN(';');
-  // Stash away INCREMENT
-  size_t increment_position = current_function_builder_->GetPosition();
-  if (!Peek(')')) {
-    RECURSE(Expression(nullptr));
-  }
-  std::vector<byte> increment_code;
-  current_function_builder_->StashCode(&increment_code, increment_position);
+  // Race past INCREMENT
+  size_t increment_position = scanner_.Position();
+  ScanToClosingParenthesis();
   EXPECT_TOKEN(')');
   //       BODY
   RECURSE(ValidateStatement());
   //       INCREMENT
-  current_function_builder_->EmitCode(
-      increment_code.data(), static_cast<uint32_t>(increment_code.size()));
+  size_t end_position = scanner_.Position();
+  scanner_.Seek(increment_position);
+  if (!Peek(')')) {
+    RECURSE(Expression(nullptr));
+  }
   current_function_builder_->EmitWithU8(kExprBr, 0);
+  scanner_.Seek(end_position);
   //   }
   End();
   // }
@@ -1970,7 +1970,7 @@ AsmType* AsmJsParser::BitwiseORExpression() {
     RECURSEn(b = BitwiseXORExpression());
     // Handle |0 specially.
     if (zero && old_pos == scanner_.Position()) {
-      current_function_builder_->StashCode(nullptr, old_code);
+      current_function_builder_->DeleteCodeAfter(old_code);
       a = AsmType::Signed();
       continue;
     }
@@ -2440,6 +2440,23 @@ void AsmJsParser::ValidateFloatCoercion() {
   EXPECT_TOKEN(')');
 }
 
+void AsmJsParser::ScanToClosingParenthesis() {
+  int depth = 0;
+  for (;;) {
+    if (Peek('(')) {
+      ++depth;
+    } else if (Peek(')')) {
+      --depth;
+      if (depth < 0) {
+        break;
+      }
+    } else if (Peek(AsmJsScanner::kEndOfInput)) {
+      break;
+    }
+    scanner_.Next();
+  }
+}
+
 void AsmJsParser::GatherCases(std::vector<int32_t>* cases) {
   size_t start = scanner_.Position();
   int depth = 0;

src/asmjs/asm-parser.h

@@ -315,6 +315,14 @@ class AsmJsParser {
   void ValidateHeapAccess();             // 6.10 ValidateHeapAccess
   void ValidateFloatCoercion();          // 6.11 ValidateFloatCoercion
 
+  // Used as part of {ForStatement}. Scans forward to the next `)` in order to
+  // skip over the third expression in a for-statement. This is one piece that
+  // makes this parser not be a pure single-pass.
+  void ScanToClosingParenthesis();
+
+  // Used as part of {SwitchStatement}. Collects all case labels in the current
+  // switch-statement, then resets the scanner position. This is one piece that
+  // makes this parser not be a pure single-pass.
   void GatherCases(std::vector<int32_t>* cases);
 };
 

src/wasm/wasm-module-builder.cc

@@ -185,15 +185,8 @@ void WasmFunctionBuilder::SetAsmFunctionStartPosition(int position) {
   last_asm_source_position_ = position;
 }
 
-void WasmFunctionBuilder::StashCode(std::vector<byte>* dst, size_t position) {
-  if (dst == nullptr) {
-    body_.resize(position);
-    return;
-  }
+void WasmFunctionBuilder::DeleteCodeAfter(size_t position) {
   DCHECK_LE(position, body_.size());
-  size_t len = body_.size() - position;
-  dst->resize(len);
-  memcpy(dst->data(), body_.data() + position, len);
   body_.resize(position);
 }
 

src/wasm/wasm-module-builder.h

@@ -141,7 +141,7 @@ class V8_EXPORT_PRIVATE WasmFunctionBuilder : public ZoneObject {
 
   size_t GetPosition() const { return body_.size(); }
   void FixupByte(size_t position, byte value) { body_[position] = value; }
-  void StashCode(std::vector<byte>* dst, size_t position);
+  void DeleteCodeAfter(size_t position);
 
   void WriteSignature(ZoneBuffer& buffer) const;
   void WriteExports(ZoneBuffer& buffer) const;

test/mjsunit/mjsunit.status

@@ -669,11 +669,6 @@
 }],  # 'gcov_coverage'
 
 ##############################################################################
-['variant == asm_wasm', {
-  # Issue 6127: Currently {StashCode} breaks the source position table.
-  'wasm/asm-wasm-expr': [SKIP],
-}],  # variant == asm_wasm
-
 ['variant == wasm_traps', {
   # Skip stuff uninteresting for wasm traps
   'bugs/*': [SKIP],

test/mjsunit/wasm/asm-wasm-expr.js

@@ -23,7 +23,7 @@ const assign_in_stmt = [
   "do { S } while (=)",
 ];
 const assign_in_expr = [
-  "i32_func(=)",
+  "i32_func(=) | 0",
   "(=) ? E : E",
   "E ? (=) : E",
   "E ? E : (=)",
@@ -108,9 +108,6 @@ function DoTheTests(expr, assign, stmt) {
     e = e.replace(/S/g, stmt);
     var str = main.toString().replace("FUNC_BODY", "return (" + e + ") | 0;");
     var asm_source = MODULE_TEMPLATE.toString().replace("FUNC_DECL", str);
-    // TODO(titzer): a verbosity API for these kinds of tests?
-    //    print(asm_source);
-
     doTest(asm_source, "(" + test + ") " + e);
   }
 
@@ -123,8 +120,6 @@ function DoTheTests(expr, assign, stmt) {
     e = e.replace(/S/g, stmt);
     var str = main.toString().replace("FUNC_BODY",  e + "; return 0;");
     var asm_source = MODULE_TEMPLATE.toString().replace("FUNC_DECL", str);
-//    print(asm_source);
-
     doTest(asm_source, "(" + test + ") " + e);
   }
 
@@ -134,9 +129,8 @@ function DoTheTests(expr, assign, stmt) {
     var js_module = eval("(" + nonasm_source + ")")(stdlib, {}, buffer);
     expect(js_module);
 
-    var asmfunc = eval("(" + asm_source + ")");
-
     print("Testing ASMJS: " + orig);
+    var asmfunc = eval("(" + asm_source + ")");
     var asm_module = asmfunc(stdlib, {}, buffer);
     assertTrue(%IsAsmWasmCode(asmfunc));
     expect(asm_module);