[turbofan] Fix known issue about computed property names.
authormstarzinger <mstarzinger@chromium.org>
Tue, 26 May 2015 08:45:10 +0000 (01:45 -0700)
committerCommit bot <commit-bot@chromium.org>
Tue, 26 May 2015 08:45:16 +0000 (08:45 +0000)
This fixes a corner-case where deoptimization while evaluating the
value to a __proto__ property after computed property names appeared
in an object literal, lead to environments not being in sync with
unoptimized code.

R=arv@chromium.org
TEST=mjsunit/harmony/computed-property-names-deopt

Review URL: https://codereview.chromium.org/1158443004

Cr-Commit-Position: refs/heads/master@{#28613}

src/compiler/ast-graph-builder.cc
test/mjsunit/harmony/computed-property-names-deopt.js [new file with mode: 0644]

index d3506e7..bf2da02 100644 (file)
@@ -1898,14 +1898,23 @@ void AstGraphBuilder::VisitObjectLiteral(ObjectLiteral* expr) {
   for (; property_index < expr->properties()->length(); property_index++) {
     ObjectLiteral::Property* property = expr->properties()->at(property_index);
 
+    if (property->kind() == ObjectLiteral::Property::PROTOTYPE) {
+      environment()->Push(literal);  // Duplicate receiver.
+      VisitForValue(property->value());
+      Node* value = environment()->Pop();
+      Node* receiver = environment()->Pop();
+      const Operator* op =
+          javascript()->CallRuntime(Runtime::kInternalSetPrototype, 2);
+      Node* call = NewNode(op, receiver, value);
+      PrepareFrameState(call, BailoutId::None());
+      continue;
+    }
+
     environment()->Push(literal);  // Duplicate receiver.
     VisitForValue(property->key());
     Node* name = BuildToName(environment()->Pop(),
                              expr->GetIdForProperty(property_index));
     environment()->Push(name);
-    // 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. :)
     VisitForValue(property->value());
     Node* value = environment()->Pop();
     Node* key = environment()->Pop();
@@ -1923,13 +1932,9 @@ void AstGraphBuilder::VisitObjectLiteral(ObjectLiteral* expr) {
         PrepareFrameState(call, BailoutId::None());
         break;
       }
-      case ObjectLiteral::Property::PROTOTYPE: {
-        const Operator* op =
-            javascript()->CallRuntime(Runtime::kInternalSetPrototype, 2);
-        Node* call = NewNode(op, receiver, value);
-        PrepareFrameState(call, BailoutId::None());
+      case ObjectLiteral::Property::PROTOTYPE:
+        UNREACHABLE();  // Handled specially above.
         break;
-      }
       case ObjectLiteral::Property::GETTER: {
         Node* attr = jsgraph()->Constant(NONE);
         const Operator* op = javascript()->CallRuntime(
diff --git a/test/mjsunit/harmony/computed-property-names-deopt.js b/test/mjsunit/harmony/computed-property-names-deopt.js
new file mode 100644 (file)
index 0000000..1f0b058
--- /dev/null
@@ -0,0 +1,30 @@
+// Copyright 2014 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 --allow-natives-syntax
+
+
+(function TestProtoDeopt() {
+  var proto = {};
+
+  function deoptMe() {
+    %DeoptimizeFunction(f);
+    return proto;
+  }
+
+  function checkObject(name, value, o) {
+    assertSame(proto, Object.getPrototypeOf(o));
+    assertTrue(o.hasOwnProperty(name));
+    assertEquals(value, o[name]);
+  }
+
+  function f(name, value) {
+    return { [name]: value, __proto__: deoptMe() };
+  }
+
+  checkObject("a", 1, f("a", 1));
+  checkObject("b", 2, f("b", 2));
+  %OptimizeFunctionOnNextCall(f);
+  checkObject("c", 3, f("c", 3));
+})();