Use atomic operation to read the length of a fixed array.

This fixes a race where
- mutator changes the fixed array length by trimming it,
- sweeper thread reads the length of the fixed array.

Also rename FROM_GC and FROM_MUTATOR to be more precise.

BUG=chromium:462908
LOG=NO

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

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

diff --git a/src/elements.cc b/src/elements.cc
index c523818..db971cc 100644 (file)
--- a/src/elements.cc
+++ b/src/elements.cc
@@ -884,7 +884,7 @@ class FastElementsAccessor
         if (length == 0) {
           array->initialize_elements();
         } else {
-          isolate->heap()->RightTrimFixedArray<Heap::FROM_MUTATOR>(
+          isolate->heap()->RightTrimFixedArray<Heap::CONCURRENT_TO_SWEEPER>(
               *backing_store, old_capacity - length);
         }
       } else {

diff --git a/src/factory.cc b/src/factory.cc
index 57e5da7..9f359bb 100644 (file)
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -1994,7 +1994,8 @@ void Factory::ReinitializeJSProxy(Handle<JSProxy> proxy, InstanceType type,
   if (size_difference > 0) {
     Address address = proxy->address();
     heap->CreateFillerObjectAt(address + map->instance_size(), size_difference);
-    heap->AdjustLiveBytes(address, -size_difference, Heap::FROM_MUTATOR);
+    heap->AdjustLiveBytes(address, -size_difference,
+                          Heap::CONCURRENT_TO_SWEEPER);
   }
 
   // Reset the map for the object.

diff --git a/src/heap/heap.cc b/src/heap/heap.cc
index 6ee6c09..2fb9114 100644 (file)
--- a/src/heap/heap.cc
+++ b/src/heap/heap.cc
@@ -3452,7 +3452,7 @@ bool Heap::CanMoveObjectStart(HeapObject* object) {
 void Heap::AdjustLiveBytes(Address address, int by, InvocationMode mode) {
   if (incremental_marking()->IsMarking() &&
       Marking::IsBlack(Marking::MarkBitFrom(address))) {
-    if (mode == FROM_GC) {
+    if (mode == SEQUENTIAL_TO_SWEEPER) {
       MemoryChunk::IncrementLiveBytesFromGC(address, by);
     } else {
       MemoryChunk::IncrementLiveBytesFromMutator(address, by);
@@ -3502,7 +3502,7 @@ FixedArrayBase* Heap::LeftTrimFixedArray(FixedArrayBase* object,
 
   // Maintain consistency of live bytes during incremental marking
   marking()->TransferMark(object->address(), new_start);
-  AdjustLiveBytes(new_start, -bytes_to_trim, Heap::FROM_MUTATOR);
+  AdjustLiveBytes(new_start, -bytes_to_trim, Heap::CONCURRENT_TO_SWEEPER);
 
   // Notify the heap profiler of change in object layout.
   OnMoveEvent(new_object, object, new_object->Size());
@@ -3511,10 +3511,10 @@ FixedArrayBase* Heap::LeftTrimFixedArray(FixedArrayBase* object,
 
 
 // Force instantiation of templatized method.
-template
-void Heap::RightTrimFixedArray<Heap::FROM_GC>(FixedArrayBase*, int);
-template
-void Heap::RightTrimFixedArray<Heap::FROM_MUTATOR>(FixedArrayBase*, int);
+template void Heap::RightTrimFixedArray<Heap::SEQUENTIAL_TO_SWEEPER>(
+    FixedArrayBase*, int);
+template void Heap::RightTrimFixedArray<Heap::CONCURRENT_TO_SWEEPER>(
+    FixedArrayBase*, int);
 
 
 template<Heap::InvocationMode mode>

diff --git a/src/heap/heap.h b/src/heap/heap.h
index 4f624a3..4a50523 100644 (file)
--- a/src/heap/heap.h
+++ b/src/heap/heap.h
@@ -729,9 +729,11 @@ class Heap {
 
   bool CanMoveObjectStart(HeapObject* object);
 
-  // Indicates whether live bytes adjustment is triggered from within the GC
-  // code or from mutator code.
-  enum InvocationMode { FROM_GC, FROM_MUTATOR };
+  // Indicates whether live bytes adjustment is triggered
+  // - from within the GC code before sweeping started (SEQUENTIAL_TO_SWEEPER),
+  // - or from within GC (CONCURRENT_TO_SWEEPER),
+  // - or mutator code (CONCURRENT_TO_SWEEPER).
+  enum InvocationMode { SEQUENTIAL_TO_SWEEPER, CONCURRENT_TO_SWEEPER };
 
   // Maintain consistency of live bytes during incremental marking.
   void AdjustLiveBytes(Address address, int by, InvocationMode mode);

diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
index 7a284ae..1370656 100644 (file)
--- a/src/heap/mark-compact.cc
+++ b/src/heap/mark-compact.cc
@@ -2562,7 +2562,7 @@ void MarkCompactCollector::ClearMapTransitions(Map* map, Map* dead_transition) {
     // Non-full-TransitionArray cases can never reach this point.
     DCHECK(TransitionArray::IsFullTransitionArray(transitions));
     TransitionArray* t = TransitionArray::cast(transitions);
-    heap_->RightTrimFixedArray<Heap::FROM_GC>(
+    heap_->RightTrimFixedArray<Heap::SEQUENTIAL_TO_SWEEPER>(
         t, trim * TransitionArray::kTransitionSize);
     t->SetNumberOfTransitions(transition_index);
     // The map still has a full transition array.
@@ -2578,7 +2578,7 @@ void MarkCompactCollector::TrimDescriptorArray(Map* map,
   int to_trim = number_of_descriptors - number_of_own_descriptors;
   if (to_trim == 0) return;
 
-  heap_->RightTrimFixedArray<Heap::FROM_GC>(
+  heap_->RightTrimFixedArray<Heap::SEQUENTIAL_TO_SWEEPER>(
       descriptors, to_trim * DescriptorArray::kDescriptorSize);
   descriptors->SetNumberOfDescriptors(number_of_own_descriptors);
 
@@ -2606,12 +2606,13 @@ void MarkCompactCollector::TrimEnumCache(Map* map,
 
   int to_trim = enum_cache->length() - live_enum;
   if (to_trim <= 0) return;
-  heap_->RightTrimFixedArray<Heap::FROM_GC>(descriptors->GetEnumCache(),
-                                            to_trim);
+  heap_->RightTrimFixedArray<Heap::SEQUENTIAL_TO_SWEEPER>(
+      descriptors->GetEnumCache(), to_trim);
 
   if (!descriptors->HasEnumIndicesCache()) return;
   FixedArray* enum_indices_cache = descriptors->GetEnumIndicesCache();
-  heap_->RightTrimFixedArray<Heap::FROM_GC>(enum_indices_cache, to_trim);
+  heap_->RightTrimFixedArray<Heap::SEQUENTIAL_TO_SWEEPER>(enum_indices_cache,
+                                                          to_trim);
 }
 
 

diff --git a/src/isolate.cc b/src/isolate.cc
index 978c52e..da298ef 100644 (file)
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -2626,8 +2626,8 @@ void Isolate::CheckDetachedContextsAfterGC() {
   if (new_length == 0) {
     heap()->set_detached_contexts(heap()->empty_fixed_array());
   } else if (new_length < length) {
-    heap()->RightTrimFixedArray<Heap::FROM_MUTATOR>(*detached_contexts,
-                                                    length - new_length);
+    heap()->RightTrimFixedArray<Heap::CONCURRENT_TO_SWEEPER>(
+        *detached_contexts, length - new_length);
   }
 }
 

diff --git a/src/layout-descriptor.cc b/src/layout-descriptor.cc
index 4bb48c0..66a1f0f 100644 (file)
--- a/src/layout-descriptor.cc
+++ b/src/layout-descriptor.cc
@@ -245,7 +245,7 @@ LayoutDescriptor* LayoutDescriptor::Trim(Heap* heap, Map* map,
   if (current_length != array_length) {
     DCHECK_LT(array_length, current_length);
     int delta = current_length - array_length;
-    heap->RightTrimFixedArray<Heap::FROM_GC>(this, delta);
+    heap->RightTrimFixedArray<Heap::SEQUENTIAL_TO_SWEEPER>(this, delta);
   }
   memset(DataPtr(), 0, DataSize());
   LayoutDescriptor* layout_descriptor =

diff --git a/src/objects.cc b/src/objects.cc
index 8f8b6c8..eac53cb 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -1030,7 +1030,8 @@ bool String::MakeExternal(v8::String::ExternalStringResource* resource) {
   self->set_resource(resource);
   if (is_internalized) self->Hash();  // Force regeneration of the hash value.
 
-  heap->AdjustLiveBytes(this->address(), new_size - size, Heap::FROM_MUTATOR);
+  heap->AdjustLiveBytes(this->address(), new_size - size,
+                        Heap::CONCURRENT_TO_SWEEPER);
   return true;
 }
 
@@ -1090,7 +1091,8 @@ bool String::MakeExternal(v8::String::ExternalOneByteStringResource* resource) {
   self->set_resource(resource);
   if (is_internalized) self->Hash();  // Force regeneration of the hash value.
 
-  heap->AdjustLiveBytes(this->address(), new_size - size, Heap::FROM_MUTATOR);
+  heap->AdjustLiveBytes(this->address(), new_size - size,
+                        Heap::CONCURRENT_TO_SWEEPER);
   return true;
 }
 
@@ -2118,7 +2120,7 @@ void JSObject::MigrateFastToFast(Handle<JSObject> object, Handle<Map> new_map) {
   // If there are properties in the new backing store, trim it to the correct
   // size and install the backing store into the object.
   if (external > 0) {
-    heap->RightTrimFixedArray<Heap::FROM_MUTATOR>(*array, inobject);
+    heap->RightTrimFixedArray<Heap::CONCURRENT_TO_SWEEPER>(*array, inobject);
     object->set_properties(*array);
   }
 
@@ -2131,7 +2133,8 @@ void JSObject::MigrateFastToFast(Handle<JSObject> object, Handle<Map> new_map) {
     Address address = object->address();
     heap->CreateFillerObjectAt(
         address + new_instance_size, instance_size_delta);
-    heap->AdjustLiveBytes(address, -instance_size_delta, Heap::FROM_MUTATOR);
+    heap->AdjustLiveBytes(address, -instance_size_delta,
+                          Heap::CONCURRENT_TO_SWEEPER);
   }
 
   // We are storing the new map using release store after creating a filler for
@@ -4643,7 +4646,7 @@ void JSObject::MigrateFastToSlow(Handle<JSObject> object,
     heap->CreateFillerObjectAt(object->address() + new_instance_size,
                                instance_size_delta);
     heap->AdjustLiveBytes(object->address(), -instance_size_delta,
-                          Heap::FROM_MUTATOR);
+                          Heap::CONCURRENT_TO_SWEEPER);
   }
 
   // We are storing the new map using release store after creating a filler for
@@ -8127,7 +8130,7 @@ Handle<PolymorphicCodeCacheHashTable> PolymorphicCodeCacheHashTable::Put(
 void FixedArray::Shrink(int new_length) {
   DCHECK(0 <= new_length && new_length <= length());
   if (new_length < length()) {
-    GetHeap()->RightTrimFixedArray<Heap::FROM_MUTATOR>(
+    GetHeap()->RightTrimFixedArray<Heap::CONCURRENT_TO_SWEEPER>(
         this, length() - new_length);
   }
 }
@@ -9490,7 +9493,7 @@ Handle<String> SeqString::Truncate(Handle<SeqString> string, int new_length) {
     // that are a multiple of pointer size.
     heap->CreateFillerObjectAt(start_of_string + new_size, delta);
   }
-  heap->AdjustLiveBytes(start_of_string, -delta, Heap::FROM_MUTATOR);
+  heap->AdjustLiveBytes(start_of_string, -delta, Heap::CONCURRENT_TO_SWEEPER);
 
   // We are storing the new length using release store after creating a filler
   // for the left-over space to avoid races with the sweeper thread.
@@ -9910,7 +9913,8 @@ void SharedFunctionInfo::EvictFromOptimizedCodeMap(Code* optimized_code,
   }
   if (dst != length) {
     // Always trim even when array is cleared because of heap verifier.
-    GetHeap()->RightTrimFixedArray<Heap::FROM_MUTATOR>(code_map, length - dst);
+    GetHeap()->RightTrimFixedArray<Heap::CONCURRENT_TO_SWEEPER>(code_map,
+                                                                length - dst);
     if (code_map->length() == kEntriesStart) ClearOptimizedCodeMap();
   }
 }
@@ -9921,7 +9925,8 @@ void SharedFunctionInfo::TrimOptimizedCodeMap(int shrink_by) {
   DCHECK(shrink_by % kEntryLength == 0);
   DCHECK(shrink_by <= code_map->length() - kEntriesStart);
   // Always trim even when array is cleared because of heap verifier.
-  GetHeap()->RightTrimFixedArray<Heap::FROM_GC>(code_map, shrink_by);
+  GetHeap()->RightTrimFixedArray<Heap::SEQUENTIAL_TO_SWEEPER>(code_map,
+                                                              shrink_by);
   if (code_map->length() == kEntriesStart) {
     ClearOptimizedCodeMap();
   }

diff --git a/src/objects.h b/src/objects.h
index 66c04d6..722a542 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -2531,7 +2531,8 @@ class FixedArray: public FixedArrayBase {
   class BodyDescriptor : public FlexibleBodyDescriptor<kHeaderSize> {
    public:
     static inline int SizeOf(Map* map, HeapObject* object) {
-      return SizeFor(reinterpret_cast<FixedArray*>(object)->length());
+      return SizeFor(
+          reinterpret_cast<FixedArray*>(object)->synchronized_length());
     }
   };
 

diff --git a/src/runtime/runtime-regexp.cc b/src/runtime/runtime-regexp.cc
index d3212f4..478573a 100644 (file)
--- a/src/runtime/runtime-regexp.cc
+++ b/src/runtime/runtime-regexp.cc
@@ -638,7 +638,7 @@ MUST_USE_RESULT static Object* StringReplaceGlobalRegExpWithEmptyString(
   // thread can not get confused with the filler creation. No synchronization
   // needed.
   heap->CreateFillerObjectAt(end_of_string, delta);
-  heap->AdjustLiveBytes(answer->address(), -delta, Heap::FROM_MUTATOR);
+  heap->AdjustLiveBytes(answer->address(), -delta, Heap::CONCURRENT_TO_SWEEPER);
   return *answer;
 }
 

diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index 447266c..9354056 100644 (file)
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -5328,7 +5328,8 @@ static void TestRightTrimFixedTypedArray(v8::ExternalArrayType type,
   Handle<FixedTypedArrayBase> array =
       factory->NewFixedTypedArray(initial_length, type);
   int old_size = array->size();
-  heap->RightTrimFixedArray<Heap::FROM_MUTATOR>(*array, elements_to_trim);
+  heap->RightTrimFixedArray<Heap::CONCURRENT_TO_SWEEPER>(*array,
+                                                         elements_to_trim);
 
   // Check that free space filler is at the right place and did not smash the
   // array header.