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

src/asmjs/asm-wasm-builder.cc

 // Copyright 2015 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.
 
 #include "src/v8.h"
 
 // Required to get M_E etc. in MSVC.
 #if defined(_WIN32)
 #define _USE_MATH_DEFINES
 #endif
@@ skipping 17 matching lines @@
 namespace internal {
 namespace wasm {
 
 #define RECURSE(call)               \
   do {                              \
     DCHECK(!HasStackOverflow());    \
     call;                           \
     if (HasStackOverflow()) return; \
   } while (false)
 
+#ifdef DEBUG
+namespace {
+bool IsChild(Expression* parent, Expression* child) {

[bradnelson, 2016/12/07 19:19:06]

IsBinOpChild ?

[Clemens Hammacher, 2016/12/08 10:50:23]

I just removed the function completely :)
By storing a BinaryOperation* as parent, the condition is simple enough to be
inlined.

+  DCHECK_NOT_NULL(parent);
+  if (parent->IsBinaryOperation()) {
+    BinaryOperation* bin_op = static_cast<BinaryOperation*>(parent);
+    return bin_op->left() == child || bin_op->right() == child;
+  }
+  UNREACHABLE();
+  return false;
+}
+}  // namespace
+#endif
+
 enum AsmScope { kModuleScope, kInitScope, kFuncScope, kExportScope };
 enum ValueFate { kDrop, kLeaveOnStack };
 
 struct ForeignVariable {
   Handle<Name> name;
   Variable* var;
   LocalType type;
 };
 
 class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
@@ skipping 16 matching lines @@
         ast_value_factory_(ast_value_factory),
         script_(script),
         typer_(typer),
         typer_failed_(false),
         breakable_blocks_(zone),
         foreign_variables_(zone),
         init_function_(nullptr),
         foreign_init_function_(nullptr),
         function_tables_(ZoneHashMap::kDefaultHashMapCapacity,
                          ZoneAllocationPolicy(zone)),
-        imported_function_table_(this) {
+        imported_function_table_(this),
+        parent_(nullptr) {
     InitializeAstVisitor(isolate);
   }
 
   void InitializeInitFunction() {
     FunctionSig::Builder b(zone(), 0, 0);
     init_function_ = builder_->AddFunction(b.Build());
     builder_->MarkStartFunction(init_function_);
   }
 
   void BuildForeignInitFunction() {
@@ skipping 1304 matching lines @@
       Expression* arg = args->at(i);
       RECURSE(Visit(arg));
     }
   }
 
   void VisitCall(Call* expr) { VisitCallExpression(expr); }
 
   bool VisitCallExpression(Call* expr) {
     Call::CallType call_type = expr->GetCallType();
     bool returns_value = true;
+
+    // Save the parent now, it might be overwritten in VisitCallArgs.
+    Expression* this_parent = parent_;
+
     switch (call_type) {
       case Call::OTHER_CALL: {
         VariableProxy* proxy = expr->expression()->AsVariableProxy();
         if (proxy != nullptr) {
           DCHECK(kFuncScope == scope_ ||
                  typer_->VariableAsStandardMember(proxy->var()) ==
                      AsmTyper::kMathFround);
           if (VisitStdlibFunction(expr, proxy)) {
             return true;
           }
@@ skipping 10 matching lines @@
             sig.AddReturn(return_type);
           } else {
             returns_value = false;
           }
           for (int i = 0; i < args->length(); ++i) {
             sig.AddParam(TypeOf(args->at(i)));
           }
           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, IsChild(this_parent, expr));
+          int pos = expr->position();
+          int parent_pos = returns_value ? this_parent->position() : pos;
+          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());
           returns_value = function->signature()->return_count() > 0;
         }
         break;
       }
       case Call::KEYED_PROPERTY_CALL: {
         DCHECK_EQ(kFuncScope, scope_);
         Property* p = expr->expression()->AsProperty();
         DCHECK_NOT_NULL(p);
         VariableProxy* var = p->obj()->AsVariableProxy();
         DCHECK_NOT_NULL(var);
         FunctionTableIndices* indices = LookupOrAddFunctionTable(var, p);
         Visit(p->key());  // TODO(titzer): should use RECURSE()
 
         // We have to use a temporary for the correct order of evaluation.
         current_function_builder_->EmitI32Const(indices->start_index);
         current_function_builder_->Emit(kExprI32Add);
         WasmTemporary tmp(current_function_builder_, kAstI32);
         current_function_builder_->EmitSetLocal(tmp.index());
 
         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
         returns_value =
             builder_->GetSignature(indices->signature_index)->return_count() >
             0;
         break;
       }
       default:
         UNREACHABLE();
@@ skipping 151 matching lines @@
     if (expr->op() == Token::BIT_XOR) {
       return expr->left()->AsBinaryOperation()->left();
     } else {
       return expr->left();
     }
   }
 
   void VisitBinaryOperation(BinaryOperation* expr) {
     ConvertOperation convertOperation = MatchBinaryOperation(expr);
     static const bool kDontIgnoreSign = false;
+    parent_ = expr;
     if (convertOperation == kToDouble) {
       RECURSE(Visit(expr->left()));
       TypeIndex type = TypeIndexOf(expr->left(), kDontIgnoreSign);
       if (type == kInt32 || type == kFixnum) {
         current_function_builder_->Emit(kExprF64SConvertI32);
       } else if (type == kUint32) {
         current_function_builder_->Emit(kExprF64UConvertI32);
       } else if (type == kFloat32) {
         current_function_builder_->Emit(kExprF64ConvertF32);
       } else {
@@ skipping 283 matching lines @@
   Handle<Script> script_;
   AsmTyper* typer_;
   bool typer_failed_;
   ZoneVector<std::pair<BreakableStatement*, bool>> breakable_blocks_;
   ZoneVector<ForeignVariable> foreign_variables_;
   WasmFunctionBuilder* init_function_;
   WasmFunctionBuilder* foreign_init_function_;
   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.
+  Expression* parent_;
 
   DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
 
  private:
   DISALLOW_COPY_AND_ASSIGN(AsmWasmBuilderImpl);
 };
 
 AsmWasmBuilder::AsmWasmBuilder(Isolate* isolate, Zone* zone,
                                AstValueFactory* ast_value_factory,
                                Handle<Script> script, FunctionLiteral* literal)
@@ skipping 17 matching lines @@
   impl.builder_->WriteAsmJsOffsetTable(*asm_offsets_buffer);
   return {module_buffer, asm_offsets_buffer, success};
 }
 
 const char* AsmWasmBuilder::foreign_init_name = "__foreign_init__";
 const char* AsmWasmBuilder::single_function_name = "__single_function__";
 
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8