[wasm] Fix location for error in asm.js ToNumber conversion

src/asmjs/asm-wasm-builder.cc

Index: src/asmjs/asm-wasm-builder.cc
diff --git a/src/asmjs/asm-wasm-builder.cc b/src/asmjs/asm-wasm-builder.cc
index 9415a84521e2c276ff8dc319fd46fab9ff2da274..6a98d504fe444707a5c2d08627032629172fdc8c 100644
--- a/src/asmjs/asm-wasm-builder.cc
+++ b/src/asmjs/asm-wasm-builder.cc
@@ -71,7 +71,8 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
         foreign_init_function_(nullptr),
         function_tables_(ZoneHashMap::kDefaultHashMapCapacity,
                          ZoneAllocationPolicy(zone)),
-        imported_function_table_(this) {
+        imported_function_table_(this),
+        bin_op_parent_(nullptr) {
     InitializeAstVisitor(isolate);
   }
 
@@ -1398,6 +1399,10 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
   bool VisitCallExpression(Call* expr) {
     Call::CallType call_type = expr->GetCallType();
     bool returns_value = true;
+
+    // Save the parent now, it might be overwritten in VisitCallArgs.
+    BinaryOperation* this_parent = bin_op_parent_;

[titzer, 2016/12/08 14:08:50]

name this parent_binop?

[Clemens Hammacher, 2016/12/08 15:26:21]

Done.

+
     switch (call_type) {
       case Call::OTHER_CALL: {
         VariableProxy* proxy = expr->expression()->AsVariableProxy();
@@ -1428,13 +1433,20 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
           uint32_t index = imported_function_table_.LookupOrInsertImportUse(
               vp->var(), sig.Build());
           VisitCallArgs(expr);
-          current_function_builder_->AddAsmWasmOffset(expr->position());
+          // For non-void functions, we must know the parent node.
+          DCHECK_IMPLIES(returns_value, this_parent != nullptr);
+          DCHECK_IMPLIES(returns_value, this_parent->left() == expr ||
+                                            this_parent->right() == expr);
+          int pos = expr->position();
+          int parent_pos = returns_value ? this_parent->position() : pos;

[titzer, 2016/12/08 14:08:50]

Why not just

int parent_pos = this_parent ? this_parent->position() : pos;

[Clemens Hammacher, 2016/12/08 15:26:21]

The AsmWasmOffsetTable is delta-encoded, so it's actually better to emit the
position of the call expression twice instead of emitting an arbitrary position
(of some previously visited binary operation).
Note that this_parent might also be set for void functions, but then it's not
guaranteed that it's the immediate parent.

+          current_function_builder_->AddAsmWasmOffset(pos, parent_pos);
           current_function_builder_->Emit(kExprCallFunction);
           current_function_builder_->EmitVarInt(index);
         } else {
           WasmFunctionBuilder* function = LookupOrInsertFunction(vp->var());
           VisitCallArgs(expr);
-          current_function_builder_->AddAsmWasmOffset(expr->position());
+          current_function_builder_->AddAsmWasmOffset(expr->position(),
+                                                      expr->position());
           current_function_builder_->Emit(kExprCallFunction);
           current_function_builder_->EmitDirectCallIndex(
               function->func_index());
@@ -1460,7 +1472,8 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
         VisitCallArgs(expr);
 
         current_function_builder_->EmitGetLocal(tmp.index());
-        current_function_builder_->AddAsmWasmOffset(expr->position());
+        current_function_builder_->AddAsmWasmOffset(expr->position(),
+                                                    expr->position());
         current_function_builder_->Emit(kExprCallIndirect);
         current_function_builder_->EmitVarInt(indices->signature_index);
         current_function_builder_->EmitVarInt(0);  // table index
@@ -1632,6 +1645,7 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
   void VisitBinaryOperation(BinaryOperation* expr) {
     ConvertOperation convertOperation = MatchBinaryOperation(expr);
     static const bool kDontIgnoreSign = false;
+    bin_op_parent_ = expr;

[titzer, 2016/12/08 14:08:50]

parent_binop_?

[Clemens Hammacher, 2016/12/08 15:26:21]

Done.

     if (convertOperation == kToDouble) {
       RECURSE(Visit(expr->left()));
       TypeIndex type = TypeIndexOf(expr->left(), kDontIgnoreSign);
@@ -1935,6 +1949,9 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
   uint32_t next_table_index_;
   ZoneHashMap function_tables_;
   ImportedFunctionTable imported_function_table_;
+  // Remember the parent node for reporting the correct location for ToNumber
+  // conversions after calls.
+  BinaryOperation* bin_op_parent_;
 
   DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();