Title: [210935] trunk/Source/_javascript_Core
- Revision
- 210935
- Author
- fpi...@apple.com
- Date
- 2017-01-19 12:53:42 -0800 (Thu, 19 Jan 2017)
Log Message
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):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (210934 => 210935)
--- trunk/Source/_javascript_Core/ChangeLog 2017-01-19 20:21:14 UTC (rev 210934)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-01-19 20:53:42 UTC (rev 210935)
@@ -1,3 +1,28 @@
+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-19 Myles C. Maxfield <mmaxfi...@apple.com>
[Cocoa] Variation fonts are erroneously disabled on iOS
Modified: trunk/Source/_javascript_Core/runtime/JSArray.cpp (210934 => 210935)
--- trunk/Source/_javascript_Core/runtime/JSArray.cpp 2017-01-19 20:21:14 UTC (rev 210934)
+++ trunk/Source/_javascript_Core/runtime/JSArray.cpp 2017-01-19 20:53:42 UTC (rev 210935)
@@ -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
