Title: [210960] branches/safari-603-branch/Source/_javascript_Core
Revision
210960
Author
matthew_han...@apple.com
Date
2017-01-20 08:25:41 -0800 (Fri, 20 Jan 2017)

Log Message

Merge r210935. rdar://problem/30101860

Modified Paths

Diff

Modified: branches/safari-603-branch/Source/_javascript_Core/ChangeLog (210959 => 210960)


--- branches/safari-603-branch/Source/_javascript_Core/ChangeLog	2017-01-20 12:17:50 UTC (rev 210959)
+++ branches/safari-603-branch/Source/_javascript_Core/ChangeLog	2017-01-20 16:25:41 UTC (rev 210960)
@@ -1,3 +1,32 @@
+2017-01-20  Matthew Hanson  <matthew_han...@apple.com>
+
+        Merge r210935. rdar://problem/30101860
+
+    2017-01-19  Filip Pizlo  <fpi...@apple.com>
+
+            The mutator needs to fire a barrier after memmoving stuff around in an object that the GC scans
+            https://bugs.webkit.org/show_bug.cgi?id=167208
+
+            Reviewed by Saam Barati.
+
+            It used to be that if you moved a value from one place to another in the same object
+            then there is no need for a barrier because the generational GC would have no need to
+            know that some old object still continues to refer to the same other old object.
+
+            But the concurrent GC might scan that object as the mutator moves pointers around in
+            it. If the ordering is right, this could mean that the collector never sees some of
+            those pointers. This can be fixed by adding a barrier.
+
+            This fixes the most obvious cases I found. There may be more and I'll continue to
+            audit. Most of the other memmove users seem to already use some kind of synchronization
+            to prevent this. For example, this can also be fixed by just holding the cell lock
+            around the memmove since we're dealing with indexing storage and the GC reads that
+            under the cell lock.
+
+            * runtime/JSArray.cpp:
+            (JSC::JSArray::shiftCountWithAnyIndexingType):
+            (JSC::JSArray::unshiftCountWithAnyIndexingType):
+
 2017-01-18  Matthew Hanson  <matthew_han...@apple.com>
 
         Merge r210858. rdar://problem/30069096

Modified: branches/safari-603-branch/Source/_javascript_Core/runtime/JSArray.cpp (210959 => 210960)


--- branches/safari-603-branch/Source/_javascript_Core/runtime/JSArray.cpp	2017-01-20 12:17:50 UTC (rev 210959)
+++ branches/safari-603-branch/Source/_javascript_Core/runtime/JSArray.cpp	2017-01-20 16:25:41 UTC (rev 210960)
@@ -1021,6 +1021,11 @@
             butterfly->contiguous()[i].clear();
         
         butterfly->setPublicLength(oldLength - count);
+
+        // Our memmoving of values around in the array could have concealed some of them from
+        // the collector. Let's make sure that the collector scans this object again.
+        vm.heap.writeBarrier(this);
+        
         return true;
     }
         
@@ -1169,6 +1174,10 @@
             butterfly->contiguous()[i + count].setWithoutWriteBarrier(v);
         }
         
+        // Our memmoving of values around in the array could have concealed some of them from
+        // the collector. Let's make sure that the collector scans this object again.
+        vm.heap.writeBarrier(this);
+        
         // NOTE: we're leaving being garbage in the part of the array that we shifted out
         // of. This is fine because the caller is required to store over that area, and
         // in contiguous mode storing into a hole is guaranteed to behave exactly the same
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to