[heap] Remove adhoc weakness in TransitionArray.

Currently transition array targets have conditional weakness depending
on the type of the target. Map targets are weak and all other targets
are strong. This patch wraps maps in transitions arrays in weak cells,
which allows us to treat all elements of transition arrays strongly.

Conditional weakness is unsafe for concurrent marking because the
condition can change during marking.

Bug: chromium:694255
Change-Id: I64e5d0699698fc7c1758f3fbc52da43014c247af
Reviewed-on: https://chromium-review.googlesource.com/641271
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48034}

src/heap/concurrent-marking.cc

@@ -215,11 +215,12 @@ class ConcurrentMarkingVisitor final
   }
 
   int VisitTransitionArray(Map* map, TransitionArray* array) {
-    if (marking_state_.IsGrey(array)) {
-      // TODO(ulan): process transition arrays.
-      bailout_.Push(array);
-    }
-    return 0;
+    if (!ShouldVisit(array)) return 0;
+    VisitMapPointer(array, array->map_slot());
+    int size = TransitionArray::BodyDescriptor::SizeOf(map, array);
+    TransitionArray::BodyDescriptor::IterateBody(array, size, this);
+    weak_objects_->transition_arrays.Push(task_id_, array);
+    return size;
   }
 
   int VisitWeakCell(Map* map, WeakCell* object) {

src/heap/mark-compact.cc

@@ -2878,12 +2878,8 @@ bool MarkCompactCollector::CompactTransitionArray(
         RecordSlot(transitions, key_slot, key);
         Object* raw_target = transitions->GetRawTarget(i);
         transitions->SetTarget(transition_index, raw_target);
-        // Maps are not compacted, but for cached handlers the target slot
-        // must be recorded.
-        if (!raw_target->IsMap()) {
-          Object** target_slot = transitions->GetTargetSlot(transition_index);
-          RecordSlot(transitions, target_slot, raw_target);
-        }
+        Object** target_slot = transitions->GetTargetSlot(transition_index);
+        RecordSlot(transitions, target_slot, raw_target);
       }
       transition_index++;
     }

src/heap/objects-visiting-inl.h

@@ -194,23 +194,10 @@ template <typename ConcreteVisitor>
 int MarkingVisitor<ConcreteVisitor>::VisitTransitionArray(
     Map* map, TransitionArray* array) {
   ConcreteVisitor* visitor = static_cast<ConcreteVisitor*>(this);
-  // Visit strong references.
-  if (array->HasPrototypeTransitions()) {
-    visitor->VisitPointer(array, array->GetPrototypeTransitionsSlot());
-  }
-  int num_transitions = array->number_of_entries();
-  for (int i = 0; i < num_transitions; ++i) {
-    visitor->VisitPointer(array, array->GetKeySlot(i));
-    // A TransitionArray can hold maps or (transitioning StoreIC) handlers.
-    // Maps have custom weak handling; handlers (which in turn weakly point
-    // to maps) are marked strongly for now, and will be cleared during
-    // compaction when the maps they refer to are dead.
-    if (!array->GetRawTarget(i)->IsMap()) {
-      visitor->VisitPointer(array, array->GetTargetSlot(i));
-    }
-  }
+  int size = TransitionArray::BodyDescriptor::SizeOf(map, array);
+  TransitionArray::BodyDescriptor::IterateBody(array, size, visitor);
   collector_->AddTransitionArray(array);
-  return TransitionArray::BodyDescriptor::SizeOf(map, array);
+  return size;
 }
 
 template <typename ConcreteVisitor>

src/transitions-inl.h

@@ -119,8 +119,9 @@ PropertyDetails TransitionsAccessor::GetTargetDetails(Name* name, Map* target) {
 
 // static
 Map* TransitionsAccessor::GetTargetFromRaw(Object* raw) {
-  if (raw->IsMap()) return Map::cast(raw);
-  if (raw->IsTuple3()) {
+  if (raw->IsWeakCell()) {
+    return Map::cast(WeakCell::cast(raw)->value());
+  } else if (raw->IsTuple3()) {
     return Map::cast(StoreHandler::GetTuple3TransitionCell(raw)->value());
   } else {
     DCHECK(raw->IsFixedArray());
@@ -162,6 +163,7 @@ Map* TransitionsAccessor::GetTarget(int transition_number) {
 }
 
 void TransitionArray::SetTarget(int transition_number, Object* value) {
+  DCHECK(!value->IsMap());
   DCHECK(transition_number < number_of_transitions());
   set(ToTargetIndex(transition_number), value);
 }

src/transitions.cc

@@ -68,12 +68,14 @@ void TransitionsAccessor::Insert(Handle<Name> name, Handle<Map> target,
                                  SimpleTransitionFlag flag) {
   DCHECK(!map_handle_.is_null());
   Isolate* isolate = map_->GetIsolate();
+  Handle<WeakCell> weak_cell_with_target = Map::WeakCellForMap(target);
+  Reload();
   target->SetBackPointer(map_);
 
   // If the map doesn't have any transitions at all yet, install the new one.
   if (encoding() == kUninitialized) {
     if (flag == SIMPLE_PROPERTY_TRANSITION) {
-      ReplaceTransitions(*Map::WeakCellForMap(target));
+      ReplaceTransitions(*weak_cell_with_target);
       return;
     }
     // If the flag requires a full TransitionArray, allocate one.
@@ -94,7 +96,7 @@ void TransitionsAccessor::Insert(Handle<Name> name, Handle<Map> target,
     if (flag == SIMPLE_PROPERTY_TRANSITION && key->Equals(*name) &&
         old_details.kind() == new_details.kind() &&
         old_details.attributes() == new_details.attributes()) {
-      ReplaceTransitions(*Map::WeakCellForMap(target));
+      ReplaceTransitions(*weak_cell_with_target);
       return;
     }
     // Otherwise allocate a full TransitionArray with slack for a new entry.
@@ -103,10 +105,8 @@ void TransitionsAccessor::Insert(Handle<Name> name, Handle<Map> target,
     Reload();
     simple_transition = GetSimpleTransition();
     if (simple_transition != nullptr) {
-      Object* value = raw_transitions_->IsWeakCell()
-                          ? WeakCell::cast(raw_transitions_)->value()
-                          : raw_transitions_;
-      result->Set(0, GetSimpleTransitionKey(simple_transition), value);
+      result->Set(0, GetSimpleTransitionKey(simple_transition),
+                  raw_transitions_);
     } else {
       result->SetNumberOfTransitions(0);
     }
@@ -138,7 +138,7 @@ void TransitionsAccessor::Insert(Handle<Name> name, Handle<Map> target,
                             &insertion_index);
     // If an existing entry was found, overwrite it and return.
     if (index != kNotFound) {
-      array->SetTarget(index, *target);
+      array->SetTarget(index, *weak_cell_with_target);
       return;
     }
 
@@ -154,7 +154,7 @@ void TransitionsAccessor::Insert(Handle<Name> name, Handle<Map> target,
         array->SetTarget(index, array->GetRawTarget(index - 1));
       }
       array->SetKey(index, *name);
-      array->SetTarget(index, *target);
+      array->SetTarget(index, *weak_cell_with_target);
       SLOW_DCHECK(array->IsSortedNoDuplicates());
       return;
     }
@@ -202,7 +202,7 @@ void TransitionsAccessor::Insert(Handle<Name> name, Handle<Map> target,
   for (int i = 0; i < insertion_index; ++i) {
     result->Set(i, array->GetKey(i), array->GetRawTarget(i));
   }
-  result->Set(insertion_index, *name, *target);
+  result->Set(insertion_index, *name, *weak_cell_with_target);
   for (int i = insertion_index; i < number_of_transitions; ++i) {
     result->Set(i + 1, array->GetKey(i), array->GetRawTarget(i));
   }
@@ -456,6 +456,7 @@ void TransitionsAccessor::PutPrototypeTransition(Handle<Object> prototype,
   int entry = header + last;
 
   Handle<WeakCell> target_cell = Map::WeakCellForMap(target_map);
+  Reload();  // Reload after possible GC.
   cache->set(entry, *target_cell);
   TransitionArray::SetNumberOfPrototypeTransitions(*cache, last + 1);
 }
@@ -553,18 +554,19 @@ void TransitionsAccessor::EnsureHasFullTransitionArray() {
   int nof = encoding() == kUninitialized ? 0 : 1;
   Handle<TransitionArray> result =
       TransitionArray::Allocate(map_->GetIsolate(), nof);
-  DisallowHeapAllocation no_gc;
   Reload();  // Reload after possible GC.
   if (nof == 1) {
-    Map* target = GetSimpleTransition();
-    if (target == nullptr) {
+    if (encoding() == kUninitialized) {
       // If allocation caused GC and cleared the target, trim the new array.
       result->Shrink(TransitionArray::ToKeyIndex(0));
       result->SetNumberOfTransitions(0);
     } else {
       // Otherwise populate the new array.
-      Name* key = GetSimpleTransitionKey(target);
-      result->Set(0, key, target);
+      Handle<Map> target(GetSimpleTransition());
+      Handle<WeakCell> weak_cell_with_target = Map::WeakCellForMap(target);
+      Reload();  // Reload after possible GC.
+      Name* key = GetSimpleTransitionKey(*target);
+      result->Set(0, key, *weak_cell_with_target);
     }
   }
   ReplaceTransitions(*result);