[asm.js] Fix source positions of ToNumber conversions.

This extends the test coverage for source position tracking of ToNumber
conversion to also test conversion to "double" type. It also fixes the
discovered inconsistencies. Note that the conversion to "float" remains
untested as imported functions are not allowed have "float" return type.

R=clemensh@chromium.org
TEST=mjsunit/wasm/asm-wasm-exception-in-tonumber
BUG=v8:6127

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

src/asmjs/asm-parser.cc

@@ -1576,8 +1576,10 @@ AsmType* AsmJsParser::UnaryExpression() {
         FAILn("expected int/double?/float?");
       }
     }
-  } else if (Check('+')) {
+  } else if (Peek('+')) {
     call_coercion_ = AsmType::Double();
+    call_coercion_position_ = scanner_.Position();
+    scanner_.Next();  // Done late for correct position.
     RECURSEn(ret = UnaryExpression());
     // TODO(bradnelson): Generalize.
     if (ret->IsA(AsmType::Signed())) {
@@ -2016,7 +2018,8 @@ AsmType* AsmJsParser::ParenthesizedExpression() {
 AsmType* AsmJsParser::ValidateCall() {
   AsmType* return_type = call_coercion_;
   call_coercion_ = nullptr;
-  int pos = static_cast<int>(scanner_.Position());
+  int call_pos = static_cast<int>(scanner_.Position());
+  int to_number_pos = static_cast<int>(call_coercion_position_);
   AsmJsScanner::token_t function_name = Consume();
   // TODO(mstarzinger): Consider using Chromiums base::Optional instead.
   std::unique_ptr<TemporaryVariableScope> tmp;
@@ -2056,7 +2059,7 @@ AsmType* AsmJsParser::ValidateCall() {
     tmp.reset(new TemporaryVariableScope(this));
     current_function_builder_->EmitSetLocal(tmp.get()->get());
     // The position of function table calls is after the table lookup.
-    pos = static_cast<int>(scanner_.Position());
+    call_pos = static_cast<int>(scanner_.Position());
   }
   std::vector<AsmType*> param_types;
   ZoneVector<AsmType*> param_specific_types(zone());
@@ -2083,8 +2086,10 @@ AsmType* AsmJsParser::ValidateCall() {
   // TODO(bradnelson): clarify how this binds, and why only float?
   if (Peek('|') &&
       (return_type == nullptr || return_type->IsA(AsmType::Float()))) {
+    to_number_pos = static_cast<int>(scanner_.Position());
     return_type = AsmType::Signed();
   } else if (return_type == nullptr) {
+    to_number_pos = call_pos;  // No conversion.
     return_type = AsmType::Void();
   }
   AsmType* function_type = AsmType::Function(zone(), return_type);
@@ -2106,8 +2111,7 @@ AsmType* AsmJsParser::ValidateCall() {
     function_info->function_builder = module_builder_->AddFunction();
     function_info->index = function_info->function_builder->func_index();
     function_info->type = function_type;
-    // TODO(mstarzinger): Fix the {to_number_position} and test it.
-    current_function_builder_->AddAsmWasmOffset(pos, pos);
+    current_function_builder_->AddAsmWasmOffset(call_pos, to_number_pos);
     current_function_builder_->Emit(kExprCallFunction);
     current_function_builder_->EmitDirectCallIndex(function_info->index);
   } else if (function_info->kind == VarKind::kImportedFunction) {
@@ -2132,22 +2136,19 @@ AsmType* AsmJsParser::ValidateCall() {
     } else {
       index = function_info->import->cache_index[cache_index];
     }
-    // TODO(mstarzinger): Fix the {to_number_position} and test it.
-    current_function_builder_->AddAsmWasmOffset(pos, pos);
+    current_function_builder_->AddAsmWasmOffset(call_pos, to_number_pos);
     current_function_builder_->Emit(kExprCallFunction);
     current_function_builder_->EmitVarUint(index);
   } else if (function_info->type->IsA(AsmType::None())) {
     function_info->type = function_type;
     if (function_info->kind == VarKind::kTable) {
       current_function_builder_->EmitGetLocal(tmp.get()->get());
-      // TODO(mstarzinger): Fix the {to_number_position} and test it.
-      current_function_builder_->AddAsmWasmOffset(pos, pos);
+      current_function_builder_->AddAsmWasmOffset(call_pos, to_number_pos);
       current_function_builder_->Emit(kExprCallIndirect);
       current_function_builder_->EmitVarUint(signature_index);
       current_function_builder_->EmitVarUint(0);  // table index
     } else {
-      // TODO(mstarzinger): Fix the {to_number_position} and test it.
-      current_function_builder_->AddAsmWasmOffset(pos, pos);
+      current_function_builder_->AddAsmWasmOffset(call_pos, to_number_pos);
       current_function_builder_->Emit(kExprCallFunction);
       current_function_builder_->EmitDirectCallIndex(function_info->index);
     }
@@ -2288,7 +2289,7 @@ AsmType* AsmJsParser::ValidateCall() {
       current_function_builder_->EmitVarUint(signature_index);
       current_function_builder_->EmitVarUint(0);  // table index
     } else {
-      current_function_builder_->AddAsmWasmOffset(pos, pos);
+      current_function_builder_->AddAsmWasmOffset(call_pos, to_number_pos);
       current_function_builder_->Emit(kExprCallFunction);
       current_function_builder_->EmitDirectCallIndex(function_info->index);
     }
@@ -2379,6 +2380,9 @@ void AsmJsParser::ValidateFloatCoercion() {
   scanner_.Next();
   EXPECT_TOKEN('(');
   call_coercion_ = AsmType::Float();
+  // NOTE: The coercion position to float is not observable from JavaScript,
+  // because imported functions are not allowed to have float return type.
+  call_coercion_position_ = scanner_.Position();
   AsmType* ret;
   RECURSE(ret = ValidateExpression());
   if (ret->IsA(AsmType::Floatish())) {

src/asmjs/asm-parser.h

@@ -147,6 +147,9 @@ class AsmJsParser {
   // along the coercion.
   AsmType* call_coercion_;
 
+  // The source position associated with the above {call_coercion}.
+  size_t call_coercion_position_;
+
   // Used to track the last label we've seen so it can be matched to later
   // statements it's attached to.
   AsmJsScanner::token_t pending_label_;

test/mjsunit/mjsunit.status

@@ -660,7 +660,6 @@
   # Issue 6127: We won't fix these in the "old" validator. But we definitely
   # need to re-enable these for the "new" validator.
   'regress/regress-618608': [SKIP],
-  'wasm/asm-wasm-exception-in-tonumber': [SKIP],
 
   # Issue 6127: Currently {StashCode} breaks the source position table.
   'wasm/asm-wasm-expr': [SKIP],

test/mjsunit/wasm/asm-wasm-exception-in-tonumber.js

@@ -33,7 +33,7 @@ function sym(return_sym) {
   throw Error("user-thrown");
 }
 
-function generateAsmJs(stdlib, foreign) {
+function generateAsmJsInteger(stdlib, foreign) {
   'use asm';
   var sym = foreign.sym;
   function callSym(i) {
@@ -43,14 +43,24 @@ function generateAsmJs(stdlib, foreign) {
   return callSym;
 }
 
-function testHelper(use_asm_js, check_detailed, expected, input) {
+function generateAsmJsDouble(stdlib, foreign) {
+  'use asm';
+  var sym = foreign.sym;
+  function callSym(i) {
+    i=i|0;
+    return +sym(i|0);
+  }
+  return callSym;
+}
+
+function testHelper(use_asm_js, check_detailed, expected, input, source) {
   if (check_detailed) {
     Error.prepareStackTrace = (error, frames) => frames;
   } else {
     delete Error.prepareStackTrace;
   }
 
-  var fn_code = '(' + generateAsmJs.toString() + ')({}, {sym: sym})';
+  var fn_code = '(' + source.toString() + ')({}, {sym: sym})';
   if (!use_asm_js) fn_code = fn_code.replace('use asm', '');
   //print('executing:\n' + fn_code);
   var asm_js_fn = eval(fn_code);
@@ -65,18 +75,18 @@ function testHelper(use_asm_js, check_detailed, expected, input) {
   }
 }
 
-function testAll(expected_stack, expected_frames, input) {
+function testAll(expected_stack, expected_frames, input, source) {
   for (use_asm_js = 0; use_asm_js <= 1; ++use_asm_js) {
     for (test_detailed = 0; test_detailed <= 1; ++test_detailed) {
       print('\nConfig: asm ' + use_asm_js + '; detailed ' + test_detailed);
       testHelper(
           use_asm_js, test_detailed,
-          test_detailed ? expected_frames : expected_stack, input);
+          test_detailed ? expected_frames : expected_stack, input, source);
     }
   }
 }
 
-(function testStackForThrowAtCall() {
+(function testStackForThrowAtIntegerCall() {
   var expected_stack = [
     '^Error: user-thrown$',
     '^ *at sym \\(' + filename + ':\\d+:9\\)$',
@@ -89,10 +99,10 @@ function testAll(expected_stack, expected_frames, input) {
       [  "callSym",   12],
   ];
 
-  testAll(expected_stack, expected_frames, 0);
+  testAll(expected_stack, expected_frames, 0, generateAsmJsInteger);
 })();
 
-(function testStackForThrowAtConversion() {
+(function testStackForThrowAtIntegerConversion() {
   var expected_stack = [
     '^TypeError: Cannot convert a Symbol value to a number$',
     '^ *at callSym \\(eval at testHelper \\(' + filename +
@@ -103,5 +113,35 @@ function testAll(expected_stack, expected_frames, input) {
       [  "callSym",   21],
   ];
 
-  testAll(expected_stack, expected_frames, 1);
+  testAll(expected_stack, expected_frames, 1, generateAsmJsInteger);
+})();
+
+(function testStackForThrowAtDoubleCall() {
+  var expected_stack = [
+    '^Error: user-thrown$',
+    '^ *at sym \\(' + filename + ':\\d+:9\\)$',
+    '^ *at callSym \\(eval at testHelper \\(' + filename +
+        ':\\d+:19\\), <anonymous>:\\d+:13\\)$',
+  ];
+  var expected_frames = [
+      //  function   pos
+      [      "sym",    9],
+      [  "callSym",   13],
+  ];
+
+  testAll(expected_stack, expected_frames, 0, generateAsmJsDouble);
+})();
+
+(function testStackForThrowAtDoubleConversion() {
+  var expected_stack = [
+    '^TypeError: Cannot convert a Symbol value to a number$',
+    '^ *at callSym \\(eval at testHelper \\(' + filename +
+        ':\\d+:19\\), <anonymous>:\\d+:12\\)$',
+  ];
+  var expected_frames = [
+      //  function   pos
+      [  "callSym",   12],
+  ];
+
+  testAll(expected_stack, expected_frames, 1, generateAsmJsDouble);
 })();