ClearNonLiveTransitions has to hold on to non-map values.

This ensures that we don't accidentally throw away getters and/or setters that are still needed. To make sure the bug gets triggered, we have to construct a situation where the map is on the live side of a live->non-live transition. This ensures that the map is passed to ClearNonLiveTransitions.

BUG=v8:2163
TEST=test/mjsunit/regress/regress-2163.js

Review URL: https://chromiumcodereview.appspot.com/10535004

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@11713 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/objects.cc

@@ -7397,17 +7397,12 @@ void String::PrintOn(FILE* file) {
 
 // Clear a possible back pointer in case the transition leads to a dead map.
 // Return true in case a back pointer has been cleared and false otherwise.
-// Set *keep_entry to true when a live map transition has been found.
-static bool ClearBackPointer(Heap* heap, Object* target, bool* keep_entry) {
-  if (!target->IsMap()) return false;
+static bool ClearBackPointer(Heap* heap, Object* target) {
+  ASSERT(target->IsMap());
   Map* map = Map::cast(target);
-  if (Marking::MarkBitFrom(map).Get()) {
-    *keep_entry = true;
-    return false;
-  } else {
-    map->SetBackPointer(heap->undefined_value(), SKIP_WRITE_BARRIER);
-    return true;
-  }
+  if (Marking::MarkBitFrom(map).Get()) return false;
+  map->SetBackPointer(heap->undefined_value(), SKIP_WRITE_BARRIER);
+  return true;
 }
 
 
@@ -7427,17 +7422,22 @@ void Map::ClearNonLiveTransitions(Heap* heap) {
     switch (details.type()) {
       case MAP_TRANSITION:
       case CONSTANT_TRANSITION:
-        ClearBackPointer(heap, d->GetValue(i), &keep_entry);
+        keep_entry = !ClearBackPointer(heap, d->GetValue(i));
         break;
       case ELEMENTS_TRANSITION: {
         Object* object = d->GetValue(i);
         if (object->IsMap()) {
-          ClearBackPointer(heap, object, &keep_entry);
+          keep_entry = !ClearBackPointer(heap, object);
         } else {
           FixedArray* array = FixedArray::cast(object);
           for (int j = 0; j < array->length(); ++j) {
-            if (ClearBackPointer(heap, array->get(j), &keep_entry)) {
-              array->set_undefined(j);
+            Object* target = array->get(j);
+            if (target->IsMap()) {
+              if (ClearBackPointer(heap, target)) {
+                array->set_undefined(j);
+              } else {
+                keep_entry = true;
+              }
             }
           }
         }
@@ -7447,11 +7447,25 @@ void Map::ClearNonLiveTransitions(Heap* heap) {
         Object* object = d->GetValue(i);
         if (object->IsAccessorPair()) {
           AccessorPair* accessors = AccessorPair::cast(object);
-          if (ClearBackPointer(heap, accessors->getter(), &keep_entry)) {
-            accessors->set_getter(heap->the_hole_value());
+          Object* getter = accessors->getter();
+          if (getter->IsMap()) {
+            if (ClearBackPointer(heap, getter)) {
+              accessors->set_getter(heap->the_hole_value());
+            } else {
+              keep_entry = true;
+            }
+          } else if (!getter->IsTheHole()) {
+            keep_entry = true;
           }
-          if (ClearBackPointer(heap, accessors->setter(), &keep_entry)) {
-            accessors->set_setter(heap->the_hole_value());
+          Object* setter = accessors->setter();
+          if (setter->IsMap()) {
+            if (ClearBackPointer(heap, setter)) {
+              accessors->set_setter(heap->the_hole_value());
+            } else {
+              keep_entry = true;
+            }
+          } else if (!getter->IsTheHole()) {
+            keep_entry = true;
           }
         } else {
           keep_entry = true;

test/mjsunit/accessor-map-sharing.js

@@ -25,7 +25,7 @@
 // (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 --fast-accessor-properties
+// Flags: --allow-natives-syntax
 
 // Handy abbreviations.
 var dp = Object.defineProperty;

test/mjsunit/regress/regress-2163.js

+// 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: --expose-gc
+
+// Handy abbreviation.
+var dp = Object.defineProperty;
+
+function getter() { return 111; }
+function setter(x) { print(222); }
+function anotherGetter() { return 333; }
+function anotherSetter(x) { print(444); }
+var obj1, obj2;
+
+// obj1 and obj2 share the getter accessor.
+obj1 = {};
+dp(obj1, "alpha", { get: getter, set: setter });
+obj2 = {}
+dp(obj2, "alpha", { get: getter });
+obj1 = {};
+assertEquals(111, obj2.alpha);
+gc();
+assertEquals(111, obj2.alpha);
+
+// obj1, obj2, and obj3 share the getter accessor.
+obj1 = {};
+dp(obj1, "alpha", { get: getter, set: setter });
+obj2 = {}
+dp(obj2, "alpha", { get: getter });
+obj1 = {};
+gc();
+obj3 = {}
+dp(obj3, "alpha", { get: getter });
+
+
+// obj1 and obj2 share the getter and setter accessor.
+obj1 = {};
+dp(obj1, "alpha", { get: getter, set: setter });
+obj1.beta = 10;
+obj2 = {}
+dp(obj2, "alpha", { get: getter, set: setter });
+obj1 = {};
+assertEquals(111, obj2.alpha);
+gc();
+obj2.alpha = 100
+assertEquals(111, obj2.alpha);