From c5833e8596a40c88f3528791c14643156b73e350 Mon Sep 17 00:00:00 2001 From: mstarzinger Date: Wed, 28 Jan 2015 03:39:24 -0800 Subject: [PATCH] Add missing FrameState to JSToName nodes. R=jarin@chromium.org TEST=mjsunit/regress/regress-crbug-451770 BUG=chromium:451770 LOG=N Review URL: https://codereview.chromium.org/880963002 Cr-Commit-Position: refs/heads/master@{#26305} --- src/arm/full-codegen-arm.cc | 4 ++-- src/arm64/full-codegen-arm64.cc | 4 ++-- src/ast-numbering.cc | 4 ++-- src/ast.h | 15 +++++++++++++-- src/compiler/ast-graph-builder.cc | 12 ++++++++---- src/compiler/ast-graph-builder.h | 2 +- src/compiler/operator-properties.cc | 1 + src/full-codegen.cc | 4 +++- src/full-codegen.h | 2 +- src/ia32/full-codegen-ia32.cc | 4 ++-- src/mips/full-codegen-mips.cc | 4 ++-- src/mips64/full-codegen-mips64.cc | 4 ++-- src/x64/full-codegen-x64.cc | 4 ++-- src/x87/full-codegen-x87.cc | 4 ++-- test/mjsunit/regress/regress-crbug-451770.js | 15 +++++++++++++++ test/unittests/compiler/js-operator-unittest.cc | 2 +- 16 files changed, 59 insertions(+), 26 deletions(-) create mode 100644 test/mjsunit/regress/regress-crbug-451770.js diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index d18390d..dc663d9 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -1810,7 +1810,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { __ Drop(2); } } else { - EmitPropertyKey(property); + EmitPropertyKey(property, expr->GetIdForProperty(property_index)); VisitForStackValue(value); switch (property->kind()) { @@ -2550,7 +2550,7 @@ void FullCodeGenerator::EmitClassDefineProperties(ClassLiteral* lit) { __ ldr(scratch, MemOperand(sp, 0)); // prototype } __ push(scratch); - EmitPropertyKey(property); + EmitPropertyKey(property, lit->GetIdForProperty(i)); VisitForStackValue(value); EmitSetHomeObjectIfNeeded(value, 2); diff --git a/src/arm64/full-codegen-arm64.cc b/src/arm64/full-codegen-arm64.cc index c871d03..802179b 100644 --- a/src/arm64/full-codegen-arm64.cc +++ b/src/arm64/full-codegen-arm64.cc @@ -1791,7 +1791,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { __ Drop(2); } } else { - EmitPropertyKey(property); + EmitPropertyKey(property, expr->GetIdForProperty(property_index)); VisitForStackValue(value); switch (property->kind()) { @@ -2247,7 +2247,7 @@ void FullCodeGenerator::EmitClassDefineProperties(ClassLiteral* lit) { __ Peek(scratch, 0); // prototype } __ Push(scratch); - EmitPropertyKey(property); + EmitPropertyKey(property, lit->GetIdForProperty(i)); VisitForStackValue(value); EmitSetHomeObjectIfNeeded(value, 2); diff --git a/src/ast-numbering.cc b/src/ast-numbering.cc index fd74b2e..a2bc658 100644 --- a/src/ast-numbering.cc +++ b/src/ast-numbering.cc @@ -441,7 +441,7 @@ void AstNumberingVisitor::VisitForStatement(ForStatement* node) { void AstNumberingVisitor::VisitClassLiteral(ClassLiteral* node) { IncrementNodeCount(); DisableOptimization(kClassLiteral); - node->set_base_id(ReserveIdRange(ClassLiteral::num_ids())); + node->set_base_id(ReserveIdRange(node->num_ids())); if (node->extends()) Visit(node->extends()); if (node->constructor()) Visit(node->constructor()); if (node->class_variable_proxy()) { @@ -455,7 +455,7 @@ void AstNumberingVisitor::VisitClassLiteral(ClassLiteral* node) { void AstNumberingVisitor::VisitObjectLiteral(ObjectLiteral* node) { IncrementNodeCount(); - node->set_base_id(ReserveIdRange(ObjectLiteral::num_ids())); + node->set_base_id(ReserveIdRange(node->num_ids())); for (int i = 0; i < node->properties()->length(); i++) { VisitObjectLiteralProperty(node->properties()->at(i)); } diff --git a/src/ast.h b/src/ast.h index 4903928..6da9a5f 100644 --- a/src/ast.h +++ b/src/ast.h @@ -1530,7 +1530,12 @@ class ObjectLiteral FINAL : public MaterializedLiteral { BailoutId CreateLiteralId() const { return BailoutId(local_id(0)); } - static int num_ids() { return parent_num_ids() + 1; } + // Return an AST id for a property that is used in simulate instructions. + BailoutId GetIdForProperty(int i) { return BailoutId(local_id(i + 1)); } + + // Unlike other AST nodes, this number of bailout IDs allocated for an + // ObjectLiteral can vary, so num_ids() is not a static method. + int num_ids() const { return parent_num_ids() + 1 + properties()->length(); } protected: ObjectLiteral(Zone* zone, ZoneList* properties, int literal_index, @@ -2641,11 +2646,17 @@ class ClassLiteral FINAL : public Expression { int start_position() const { return position(); } int end_position() const { return end_position_; } - static int num_ids() { return parent_num_ids() + 3; } BailoutId EntryId() const { return BailoutId(local_id(0)); } BailoutId DeclsId() const { return BailoutId(local_id(1)); } BailoutId ExitId() { return BailoutId(local_id(2)); } + // Return an AST id for a property that is used in simulate instructions. + BailoutId GetIdForProperty(int i) { return BailoutId(local_id(i + 3)); } + + // Unlike other AST nodes, this number of bailout IDs allocated for an + // ClassLiteral can vary, so num_ids() is not a static method. + int num_ids() const { return parent_num_ids() + 3 + properties()->length(); } + protected: ClassLiteral(Zone* zone, const AstRawString* name, Scope* scope, VariableProxy* class_variable_proxy, Expression* extends, diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index 28378a5..d37ca7e 100644 --- a/src/compiler/ast-graph-builder.cc +++ b/src/compiler/ast-graph-builder.cc @@ -899,7 +899,8 @@ void AstGraphBuilder::VisitClassLiteralContents(ClassLiteral* expr) { environment()->Push(property->is_static() ? literal : proto); VisitForValue(property->key()); - environment()->Push(BuildToName(environment()->Pop())); + environment()->Push( + BuildToName(environment()->Pop(), expr->GetIdForProperty(i))); VisitForValue(property->value()); Node* value = environment()->Pop(); Node* key = environment()->Pop(); @@ -1131,7 +1132,8 @@ void AstGraphBuilder::VisitObjectLiteral(ObjectLiteral* expr) { environment()->Push(literal); // Duplicate receiver. VisitForValue(property->key()); - environment()->Push(BuildToName(environment()->Pop())); + environment()->Push(BuildToName(environment()->Pop(), + expr->GetIdForProperty(property_index))); // TODO(mstarzinger): For ObjectLiteral::Property::PROTOTYPE the key should // not be on the operand stack while the value is being evaluated. Come up // with a repro for this and fix it. Also find a nice way to do so. :) @@ -2324,11 +2326,13 @@ Node* AstGraphBuilder::BuildToBoolean(Node* input) { } -Node* AstGraphBuilder::BuildToName(Node* input) { +Node* AstGraphBuilder::BuildToName(Node* input, BailoutId bailout_id) { // TODO(turbofan): Possible optimization is to NOP on name constants. But the // same caveat as with BuildToBoolean applies, and it should be factored out // into a JSOperatorReducer. - return NewNode(javascript()->ToName(), input); + Node* name = NewNode(javascript()->ToName(), input); + PrepareFrameState(name, bailout_id); + return name; } diff --git a/src/compiler/ast-graph-builder.h b/src/compiler/ast-graph-builder.h index 98d7fb6..6830c98 100644 --- a/src/compiler/ast-graph-builder.h +++ b/src/compiler/ast-graph-builder.h @@ -99,7 +99,7 @@ class AstGraphBuilder : public StructuredGraphBuilder, public AstVisitor { // Builders for automatic type conversion. Node* BuildToBoolean(Node* value); - Node* BuildToName(Node* value); + Node* BuildToName(Node* value, BailoutId bailout_id); // Builders for error reporting at runtime. Node* BuildThrowReferenceError(Variable* var, BailoutId bailout_id); diff --git a/src/compiler/operator-properties.cc b/src/compiler/operator-properties.cc index 319ed97..81299e3 100644 --- a/src/compiler/operator-properties.cc +++ b/src/compiler/operator-properties.cc @@ -70,6 +70,7 @@ bool OperatorProperties::HasFrameStateInput(const Operator* op) { // Conversions case IrOpcode::kJSToObject: case IrOpcode::kJSToNumber: + case IrOpcode::kJSToName: // Properties case IrOpcode::kJSLoadNamed: diff --git a/src/full-codegen.cc b/src/full-codegen.cc index 42941d8..cc32c3e 100644 --- a/src/full-codegen.cc +++ b/src/full-codegen.cc @@ -1211,9 +1211,11 @@ void FullCodeGenerator::EmitUnwindBeforeReturn() { } -void FullCodeGenerator::EmitPropertyKey(ObjectLiteralProperty* property) { +void FullCodeGenerator::EmitPropertyKey(ObjectLiteralProperty* property, + BailoutId bailout_id) { VisitForStackValue(property->key()); __ InvokeBuiltin(Builtins::TO_NAME, CALL_FUNCTION); + PrepareForBailoutForId(bailout_id, NO_REGISTERS); __ Push(result_register()); } diff --git a/src/full-codegen.h b/src/full-codegen.h index cc24bb8..2c12da4 100644 --- a/src/full-codegen.h +++ b/src/full-codegen.h @@ -580,7 +580,7 @@ class FullCodeGenerator: public AstVisitor { void EmitClassDefineProperties(ClassLiteral* lit); // Pushes the property key as a Name on the stack. - void EmitPropertyKey(ObjectLiteralProperty* property); + void EmitPropertyKey(ObjectLiteralProperty* property, BailoutId bailout_id); // Apply the compound assignment operator. Expects the left operand on top // of the stack and the right one in the accumulator. diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index db405bc..3873c55 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -1732,7 +1732,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { __ Drop(2); } } else { - EmitPropertyKey(property); + EmitPropertyKey(property, expr->GetIdForProperty(property_index)); VisitForStackValue(value); switch (property->kind()) { @@ -2463,7 +2463,7 @@ void FullCodeGenerator::EmitClassDefineProperties(ClassLiteral* lit) { } else { __ push(Operand(esp, 0)); // prototype } - EmitPropertyKey(property); + EmitPropertyKey(property, lit->GetIdForProperty(i)); VisitForStackValue(value); EmitSetHomeObjectIfNeeded(value, 2); diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc index 25ec2b5..2aebbba 100644 --- a/src/mips/full-codegen-mips.cc +++ b/src/mips/full-codegen-mips.cc @@ -1795,7 +1795,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { __ Drop(2); } } else { - EmitPropertyKey(property); + EmitPropertyKey(property, expr->GetIdForProperty(property_index)); VisitForStackValue(value); switch (property->kind()) { @@ -2528,7 +2528,7 @@ void FullCodeGenerator::EmitClassDefineProperties(ClassLiteral* lit) { __ lw(scratch, MemOperand(sp, 0)); // prototype } __ push(scratch); - EmitPropertyKey(property); + EmitPropertyKey(property, lit->GetIdForProperty(i)); VisitForStackValue(value); EmitSetHomeObjectIfNeeded(value, 2); diff --git a/src/mips64/full-codegen-mips64.cc b/src/mips64/full-codegen-mips64.cc index d639c10..733ff81 100644 --- a/src/mips64/full-codegen-mips64.cc +++ b/src/mips64/full-codegen-mips64.cc @@ -1792,7 +1792,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { __ Drop(2); } } else { - EmitPropertyKey(property); + EmitPropertyKey(property, expr->GetIdForProperty(property_index)); VisitForStackValue(value); switch (property->kind()) { @@ -2525,7 +2525,7 @@ void FullCodeGenerator::EmitClassDefineProperties(ClassLiteral* lit) { __ ld(scratch, MemOperand(sp, 0)); // prototype } __ push(scratch); - EmitPropertyKey(property); + EmitPropertyKey(property, lit->GetIdForProperty(i)); VisitForStackValue(value); EmitSetHomeObjectIfNeeded(value, 2); diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index b766860..2fce910 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -1766,7 +1766,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { __ Drop(2); } } else { - EmitPropertyKey(property); + EmitPropertyKey(property, expr->GetIdForProperty(property_index)); VisitForStackValue(value); switch (property->kind()) { @@ -2462,7 +2462,7 @@ void FullCodeGenerator::EmitClassDefineProperties(ClassLiteral* lit) { } else { __ Push(Operand(rsp, 0)); // prototype } - EmitPropertyKey(property); + EmitPropertyKey(property, lit->GetIdForProperty(i)); VisitForStackValue(value); EmitSetHomeObjectIfNeeded(value, 2); diff --git a/src/x87/full-codegen-x87.cc b/src/x87/full-codegen-x87.cc index 13a6792..ed91743 100644 --- a/src/x87/full-codegen-x87.cc +++ b/src/x87/full-codegen-x87.cc @@ -1721,7 +1721,7 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { __ Drop(2); } } else { - EmitPropertyKey(property); + EmitPropertyKey(property, expr->GetIdForProperty(property_index)); VisitForStackValue(value); switch (property->kind()) { @@ -2450,7 +2450,7 @@ void FullCodeGenerator::EmitClassDefineProperties(ClassLiteral* lit) { } else { __ push(Operand(esp, 0)); // prototype } - EmitPropertyKey(property); + EmitPropertyKey(property, lit->GetIdForProperty(i)); VisitForStackValue(value); EmitSetHomeObjectIfNeeded(value, 2); diff --git a/test/mjsunit/regress/regress-crbug-451770.js b/test/mjsunit/regress/regress-crbug-451770.js new file mode 100644 index 0000000..942814a --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-451770.js @@ -0,0 +1,15 @@ +// 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. + +// Flags: --harmony-computed-property-names --harmony-classes --harmony-sloppy + +assertThrows(function f() { + var t = { toString: function() { throw new Error(); } }; + var o = { [t]: 23 }; +}, Error); + +assertThrows(function f() { + var t = { toString: function() { throw new Error(); } }; + class C { [t]() { return 23; } }; +}, Error); diff --git a/test/unittests/compiler/js-operator-unittest.cc b/test/unittests/compiler/js-operator-unittest.cc index 7d48906..fd70f55 100644 --- a/test/unittests/compiler/js-operator-unittest.cc +++ b/test/unittests/compiler/js-operator-unittest.cc @@ -68,7 +68,7 @@ const SharedOperator kSharedOperators[] = { SHARED(ToBoolean, Operator::kPure, 1, 0, 0, 0, 1, 0), SHARED(ToNumber, Operator::kNoProperties, 1, 1, 1, 1, 1, 1), SHARED(ToString, Operator::kNoProperties, 1, 0, 1, 1, 1, 1), - SHARED(ToName, Operator::kNoProperties, 1, 0, 1, 1, 1, 1), + SHARED(ToName, Operator::kNoProperties, 1, 1, 1, 1, 1, 1), SHARED(ToObject, Operator::kNoProperties, 1, 1, 1, 1, 1, 1), SHARED(Yield, Operator::kNoProperties, 1, 0, 1, 1, 1, 1), SHARED(Create, Operator::kEliminatable, 0, 0, 1, 1, 1, 1), -- 2.7.4