Bugfix: A TransitionArray can disappear during copy.

During handlification of TransitionArray code, an error was introduced
in TransitionArray::CopyInsert because after creating a copy of a
TransitionArray, it may be that the array disappears during GC
because it is modified during the marking of the owning map.

R=verwaest@chromium.org

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

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

diff --git a/src/objects.cc b/src/objects.cc
index 34e0e02..f66275a 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -2375,7 +2375,7 @@ bool Map::InstancesNeedRewriting(Map* target,
 
 Handle<TransitionArray> Map::SetElementsTransitionMap(
     Handle<Map> map, Handle<Map> transitioned_map) {
-  Handle<TransitionArray> transitions = Map::AddTransition(
+  Handle<TransitionArray> transitions = TransitionArray::CopyInsert(
       map,
       map->GetIsolate()->factory()->elements_transition_symbol(),
       transitioned_map,
@@ -2505,18 +2505,6 @@ void JSObject::MigrateToMap(Handle<JSObject> object, Handle<Map> new_map) {
 }
 
 
-Handle<TransitionArray> Map::AddTransition(Handle<Map> map,
-                                           Handle<Name> key,
-                                           Handle<Map> target,
-                                           SimpleTransitionFlag flag) {
-  if (map->HasTransitionArray()) {
-    return TransitionArray::CopyInsert(map, key, target);
-  }
-  return TransitionArray::NewWith(
-      flag, key, target, handle(map->GetBackPointer(), map->GetIsolate()));
-}
-
-
 void JSObject::GeneralizeFieldRepresentation(Handle<JSObject> object,
                                              int modify_index,
                                              Representation new_representation,
@@ -6875,7 +6863,7 @@ Handle<Map> Map::ShareDescriptor(Handle<Map> map,
   Handle<Map> result = Map::CopyDropDescriptors(map);
   Handle<Name> name = descriptor->GetKey();
   Handle<TransitionArray> transitions =
-      Map::AddTransition(map, name, result, SIMPLE_TRANSITION);
+      TransitionArray::CopyInsert(map, name, result, SIMPLE_TRANSITION);
 
   // Ensure there's space for the new descriptor in the shared descriptor array.
   if (descriptors->NumberOfSlackDescriptors() == 0) {
@@ -6924,7 +6912,7 @@ Handle<Map> Map::CopyReplaceDescriptors(Handle<Map> map,
   result->InitializeDescriptors(*descriptors);
 
   if (flag == INSERT_TRANSITION && map->CanHaveMoreTransitions()) {
-    Handle<TransitionArray> transitions = Map::AddTransition(
+    Handle<TransitionArray> transitions = TransitionArray::CopyInsert(
         map, name, result, simple_flag);
     map->set_transitions(*transitions);
     result->SetBackPointer(*map);
@@ -6960,8 +6948,8 @@ Handle<Map> Map::CopyInstallDescriptors(Handle<Map> map,
   result->set_owns_descriptors(false);
 
   Handle<Name> name = handle(descriptors->GetKey(new_descriptor));
-  Handle<TransitionArray> transitions = Map::AddTransition(map, name, result,
-                                                           SIMPLE_TRANSITION);
+  Handle<TransitionArray> transitions = TransitionArray::CopyInsert(
+      map, name, result, SIMPLE_TRANSITION);
 
   map->set_transitions(*transitions);
   result->SetBackPointer(*map);
@@ -7032,9 +7020,8 @@ Handle<Map> Map::CopyForObserved(Handle<Map> map) {
     new_map = Map::Copy(map);
   }
 
-  Handle<TransitionArray> transitions =
-      Map::AddTransition(map, isolate->factory()->observed_symbol(), new_map,
-                         FULL_TRANSITION);
+  Handle<TransitionArray> transitions = TransitionArray::CopyInsert(
+      map, isolate->factory()->observed_symbol(), new_map, FULL_TRANSITION);
 
   map->set_transitions(*transitions);
 

diff --git a/src/objects.h b/src/objects.h
index 8627364..c8b2753 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -6191,10 +6191,6 @@ class Map: public HeapObject {
   inline int SearchTransition(Name* name);
   inline FixedArrayBase* GetInitialElements();
 
-  static Handle<TransitionArray> AddTransition(Handle<Map> map,
-                                               Handle<Name> key,
-                                               Handle<Map> target,
-                                               SimpleTransitionFlag flag);
   DECL_ACCESSORS(transitions, TransitionArray)
   inline void ClearTransitions(Heap* heap,
                                WriteBarrierMode mode = UPDATE_WRITE_BARRIER);

diff --git a/src/transitions.cc b/src/transitions.cc
index 76af6d5..43a0eb7 100644 (file)
--- a/src/transitions.cc
+++ b/src/transitions.cc
@@ -80,20 +80,20 @@ static bool InsertionPointFound(Name* key1, Name* key2) {
 }
 
 
-Handle<TransitionArray> TransitionArray::NewWith(SimpleTransitionFlag flag,
-                                                 Handle<Name> key,
+Handle<TransitionArray> TransitionArray::NewWith(Handle<Map> map,
+                                                 Handle<Name> name,
                                                  Handle<Map> target,
-                                                 Handle<Object> back_pointer) {
+                                                 SimpleTransitionFlag flag) {
   Handle<TransitionArray> result;
-  Factory* factory = key->GetIsolate()->factory();
+  Factory* factory = name->GetIsolate()->factory();
 
   if (flag == SIMPLE_TRANSITION) {
     result = factory->NewSimpleTransitionArray(target);
   } else {
     result = factory->NewTransitionArray(1);
-    result->NoIncrementalWriteBarrierSet(0, *key, *target);
+    result->NoIncrementalWriteBarrierSet(0, *name, *target);
   }
-  result->set_back_pointer_storage(*back_pointer);
+  result->set_back_pointer_storage(map->GetBackPointer());
   return result;
 }
 
@@ -116,9 +116,11 @@ MaybeObject* TransitionArray::ExtendToFullTransitionArray() {
 
 Handle<TransitionArray> TransitionArray::CopyInsert(Handle<Map> map,
                                                     Handle<Name> name,
-                                                    Handle<Map> target) {
-  ASSERT(map->HasTransitionArray());
-  Handle<TransitionArray> result;
+                                                    Handle<Map> target,
+                                                    SimpleTransitionFlag flag) {
+  if (!map->HasTransitionArray()) {
+    return TransitionArray::NewWith(map, name, target, flag);
+  }
 
   int number_of_transitions = map->transitions()->number_of_transitions();
   int new_size = number_of_transitions;
@@ -126,12 +128,29 @@ Handle<TransitionArray> TransitionArray::CopyInsert(Handle<Map> map,
   int insertion_index = map->transitions()->Search(*name);
   if (insertion_index == kNotFound) ++new_size;
 
-  result = map->GetIsolate()->factory()->NewTransitionArray(new_size);
+  Handle<TransitionArray> result =
+      map->GetIsolate()->factory()->NewTransitionArray(new_size);
 
-  // The map's transition array may have grown smaller during the allocation
-  // above as it was weakly traversed. Trim the result copy if needed, and
-  // recompute variables.
+  // The map's transition array may have disappeared or grown smaller during
+  // the allocation above as it was weakly traversed. Trim the result copy if
+  // needed, and recompute variables.
   DisallowHeapAllocation no_gc;
+  if (!map->HasTransitionArray()) {
+    if (flag == SIMPLE_TRANSITION) {
+      ASSERT(result->length() >= kSimpleTransitionSize);
+      result->Shrink(kSimpleTransitionSize);
+      result->set(kSimpleTransitionTarget, *target);
+    } else {
+      ASSERT(result->length() >= ToKeyIndex(1));
+      result->Shrink(ToKeyIndex(1));
+      result->set(kPrototypeTransitionsIndex, Smi::FromInt(0));
+      result->NoIncrementalWriteBarrierSet(0, *name, *target);
+    }
+    result->set_back_pointer_storage(map->GetBackPointer());
+
+    return result;
+  }
+
   TransitionArray* array = map->transitions();
   if (array->number_of_transitions() != number_of_transitions) {
     ASSERT(array->number_of_transitions() < number_of_transitions);
@@ -142,7 +161,7 @@ Handle<TransitionArray> TransitionArray::CopyInsert(Handle<Map> map,
     insertion_index = array->Search(*name);
     if (insertion_index == kNotFound) ++new_size;
 
-    result->Shrink(new_size);
+    result->Shrink(ToKeyIndex(new_size));
   }
 
   if (array->HasPrototypeTransitions()) {

diff --git a/src/transitions.h b/src/transitions.h
index 6ee2f86..f23523e 100644 (file)
--- a/src/transitions.h
+++ b/src/transitions.h
@@ -96,19 +96,21 @@ class TransitionArray: public FixedArray {
   inline int number_of_entries() { return number_of_transitions(); }
 
   // Allocate a new transition array with a single entry.
-  static Handle<TransitionArray> NewWith(SimpleTransitionFlag flag,
-                                         Handle<Name> key,
+  static Handle<TransitionArray> NewWith(Handle<Map> map,
+                                         Handle<Name> name,
                                          Handle<Map> target,
-                                         Handle<Object> back_pointer);
+                                         SimpleTransitionFlag flag);
 
   MUST_USE_RESULT MaybeObject* ExtendToFullTransitionArray();
 
-  // Copy the transition array, inserting a new transition.
+  // Create a transition array, copying from the owning map if it already has
+  // one, otherwise creating a new one according to flag.
   // TODO(verwaest): This should not cause an existing transition to be
   // overwritten.
   static Handle<TransitionArray> CopyInsert(Handle<Map> map,
                                             Handle<Name> name,
-                                            Handle<Map> target);
+                                            Handle<Map> target,
+                                            SimpleTransitionFlag flag);
 
   // Copy a single transition from the origin array.
   inline void NoIncrementalWriteBarrierCopyFrom(TransitionArray* origin,