Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(496)

Unified Diff: src/asmjs/asm-wasm-builder.cc

Issue 2555243002: [wasm] Fix location for error in asm.js ToNumber conversion (Closed)
Patch Set: No need to store parent in VisitCall Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | src/compiler/pipeline.h » ('j') | src/wasm/wasm-objects.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/asmjs/asm-wasm-builder.cc
diff --git a/src/asmjs/asm-wasm-builder.cc b/src/asmjs/asm-wasm-builder.cc
index 30eb0274f925820ed0e2be154ad9535a69323307..e11b31ef24cd33a846aa86e342efe18fc8ea29fe 100644
--- a/src/asmjs/asm-wasm-builder.cc
+++ b/src/asmjs/asm-wasm-builder.cc
@@ -35,6 +35,20 @@ namespace wasm {
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 stori
+ 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 };
@@ -71,7 +85,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),
+ parent_(nullptr) {
InitializeAstVisitor(isolate);
}
@@ -1396,6 +1411,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.
+ Expression* this_parent = parent_;
+
switch (call_type) {
case Call::OTHER_CALL: {
VariableProxy* proxy = expr->expression()->AsVariableProxy();
@@ -1426,13 +1445,18 @@ 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, 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());
@@ -1458,7 +1482,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
@@ -1630,6 +1655,7 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
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);
@@ -1933,6 +1959,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.
+ Expression* parent_;
DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
« no previous file with comments | « no previous file | src/compiler/pipeline.h » ('j') | src/wasm/wasm-objects.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698