Revision: 11470
Author:   [email protected]
Date:     Mon Apr 30 07:59:13 2012
Log:      Fix LFastLiteral to check boilerplate elements kind.

Adds a missing check that the elements kind of the boilerplate object
still has the expected elements kind, unoptimized code can transition
the boilerplate. Corner cases might cause the optimized code to be
reentered again.

[email protected]
TEST=mjsunit/regress/regress-fast-literal-transition

Review URL: https://chromiumcodereview.appspot.com/10254006
http://code.google.com/p/v8/source/detail?r=11470

Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-fast-literal-transition.js
Modified:
 /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc
 /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc
 /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc

=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-fast-literal-transition.js Mon Apr 30 07:59:13 2012
@@ -0,0 +1,62 @@
+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax --always-opt --expose-gc
+
+// Test that the elements kind of the boilerplate object is sufficiently
+// checked in LFastLiteral, so that unoptimized code can transition the
+// boilerplate. The --always-opt flag makes sure that optimized code is
+// not thrown away at deoptimization.
+
+// The switch statement in f() makes sure that f() is not inlined. If we
+// start inlining switch statements, we will still catch the bug on the
+// final --stress-opt run.
+
+function f(x) {
+  switch(x) {
+    case 1: return 1.4;
+    case 2: return 1.5;
+    case 3: return {};
+    default: gc();
+  }
+}
+
+function g(x) {
+  return [1.1, 1.2, 1.3, f(x)];
+}
+
+// Step 1: Optimize g() to contain a FAST_DOUBLE_ELEMENTS boilerplate.
+assertEquals([1.1, 1.2, 1.3, 1.4], g(1));
+assertEquals([1.1, 1.2, 1.3, 1.5], g(2));
+%OptimizeFunctionOnNextCall(g);
+
+// Step 2: Deoptimize g() and transition to FAST_ELEMENTS boilerplate.
+assertEquals([1.1, 1.2, 1.3, {}], g(3));
+
+// Step 3: Cause a GC while broken clone of boilerplate is on the heap,
+// hence causing heap verification to catch it.
+assertEquals([1.1, 1.2, 1.3, undefined], g(4));
=======================================
--- /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Thu Apr 26 05:43:00 2012 +++ /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Mon Apr 30 07:59:13 2012
@@ -4824,6 +4824,23 @@

 void LCodeGen::DoFastLiteral(LFastLiteral* instr) {
   int size = instr->hydrogen()->total_size();
+  ElementsKind boilerplate_elements_kind =
+      instr->hydrogen()->boilerplate()->GetElementsKind();
+
+ // Deopt if the literal boilerplate ElementsKind is of a type different than + // the expected one. The check isn't necessary if the boilerplate has already
+  // been converted to FAST_ELEMENTS.
+  if (boilerplate_elements_kind != FAST_ELEMENTS) {
+    __ LoadHeapObject(r1, instr->hydrogen()->boilerplate());
+    // Load map into r2.
+    __ ldr(r2, FieldMemOperand(r1, HeapObject::kMapOffset));
+    // Load the map's "bit field 2".
+    __ ldrb(r2, FieldMemOperand(r2, Map::kBitField2Offset));
+    // Retrieve elements_kind from bit field 2.
+    __ ubfx(r2, r2, Map::kElementsKindShift, Map::kElementsKindBitCount);
+    __ cmp(r2, Operand(boilerplate_elements_kind));
+    DeoptimizeIf(ne, instr->environment());
+  }

   // Allocate all objects that are part of the literal in one big
   // allocation. This avoids multiple limit checks.
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Fri Apr 27 06:05:45 2012 +++ /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Mon Apr 30 07:59:13 2012
@@ -4462,6 +4462,13 @@
     __ LoadHeapObject(ecx, object);
     __ cmp(source, ecx);
     __ Assert(equal, "Unexpected object literal boilerplate");
+    __ mov(ecx, FieldOperand(source, HeapObject::kMapOffset));
+    __ cmp(ecx, Handle<Map>(object->map()));
+    __ Assert(equal, "Unexpected boilerplate map");
+    __ mov(ecx, FieldOperand(ecx, Map::kBitField2Offset));
+    __ and_(ecx, Map::kElementsKindMask);
+    __ cmp(ecx, object->GetElementsKind() << Map::kElementsKindShift);
+    __ Assert(equal, "Unexpected boilerplate elements kind");
   }

   // Only elements backing stores for non-COW arrays need to be copied.
@@ -4558,6 +4565,23 @@
 void LCodeGen::DoFastLiteral(LFastLiteral* instr) {
   ASSERT(ToRegister(instr->context()).is(esi));
   int size = instr->hydrogen()->total_size();
+  ElementsKind boilerplate_elements_kind =
+      instr->hydrogen()->boilerplate()->GetElementsKind();
+
+ // Deopt if the literal boilerplate ElementsKind is of a type different than + // the expected one. The check isn't necessary if the boilerplate has already
+  // been converted to FAST_ELEMENTS.
+  if (boilerplate_elements_kind != FAST_ELEMENTS) {
+    __ LoadHeapObject(ebx, instr->hydrogen()->boilerplate());
+    __ mov(ecx, FieldOperand(ebx, HeapObject::kMapOffset));
+    // Load the map's "bit field 2". We only need the first byte,
+    // but the following masking takes care of that anyway.
+    __ mov(ecx, FieldOperand(ecx, Map::kBitField2Offset));
+    // Retrieve elements_kind from bit field 2.
+    __ and_(ecx, Map::kElementsKindMask);
+    __ cmp(ecx, boilerplate_elements_kind << Map::kElementsKindShift);
+    DeoptimizeIf(not_equal, instr->environment());
+  }

   // Allocate all objects that are part of the literal in one big
   // allocation. This avoids multiple limit checks.
=======================================
--- /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Thu Apr 26 05:43:00 2012 +++ /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Mon Apr 30 07:59:13 2012
@@ -4322,6 +4322,23 @@

 void LCodeGen::DoFastLiteral(LFastLiteral* instr) {
   int size = instr->hydrogen()->total_size();
+  ElementsKind boilerplate_elements_kind =
+      instr->hydrogen()->boilerplate()->GetElementsKind();
+
+ // Deopt if the literal boilerplate ElementsKind is of a type different than + // the expected one. The check isn't necessary if the boilerplate has already
+  // been converted to FAST_ELEMENTS.
+  if (boilerplate_elements_kind != FAST_ELEMENTS) {
+    __ LoadHeapObject(rbx, instr->hydrogen()->boilerplate());
+    __ movq(rcx, FieldOperand(rbx, HeapObject::kMapOffset));
+    // Load the map's "bit field 2".
+    __ movb(rcx, FieldOperand(rcx, Map::kBitField2Offset));
+    // Retrieve elements_kind from bit field 2.
+    __ and_(rcx, Immediate(Map::kElementsKindMask));
+    __ cmpb(rcx, Immediate(boilerplate_elements_kind <<
+                           Map::kElementsKindShift));
+    DeoptimizeIf(not_equal, instr->environment());
+  }

   // Allocate all objects that are part of the literal in one big
   // allocation. This avoids multiple limit checks.

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to