Title: [282663] trunk
- Revision
- 282663
- Author
- [email protected]
- Date
- 2021-09-17 08:56:53 -0700 (Fri, 17 Sep 2021)
Log Message
PutByVal and PutPrivateName ICs should emit a write barrier if a butterfly might be allocated
https://bugs.webkit.org/show_bug.cgi?id=230378
Reviewed by Yusuke Suzuki.
Right now, PutByVal and PutPrivateName check the value type to determine
if a write barrier is needed. For example, putting a primitive is considered
to not require a write barrier. This makes sense, except for the case when we
might allocate or re-allocate a butterfly in the IC. This does not emit a write
barrier, and so the GC might miss the new butterfly. That is somewhat undesirable.
This is a temporary conservative fix. If we don't write to the butterfly pointer,
then we still don't need a write barrier; this work is captured by
https://bugs.webkit.org/show_bug.cgi?id=230377
* dfg/DFGStoreBarrierInsertionPhase.cpp:
Modified Paths
Added Paths
Diff
Added: trunk/JSTests/stress/put-by-val-ic-write-barrier.js (0 => 282663)
--- trunk/JSTests/stress/put-by-val-ic-write-barrier.js (rev 0)
+++ trunk/JSTests/stress/put-by-val-ic-write-barrier.js 2021-09-17 15:56:53 UTC (rev 282663)
@@ -0,0 +1,22 @@
+//@ runDefault("--collectContinuously=1", "--useAccessInlining=0", "--verifyGC=1")
+
+function PutByValICPrimitive() {
+ let leak = []
+
+ function doByVal(o, s) {
+ o[s] = 1337
+ }
+ noInline(doByVal)
+
+ for (let i = 0; i < 1000000; ++i) {
+ let o1 = {a: 1, b: 2}
+ let o2 = {a: 1, b: 2}
+ let o3 = {a: 1, b: 2}
+ doByVal(o1, "x")
+ doByVal(o2, "y")
+ doByVal(o3, "z")
+ leak.push(o1, o2, o3)
+ }
+}
+noInline(PutByValICPrimitive)
+PutByValICPrimitive()
\ No newline at end of file
Added: trunk/JSTests/stress/put-private-name-ic-write-barrier.js (0 => 282663)
--- trunk/JSTests/stress/put-private-name-ic-write-barrier.js (rev 0)
+++ trunk/JSTests/stress/put-private-name-ic-write-barrier.js 2021-09-17 15:56:53 UTC (rev 282663)
@@ -0,0 +1,29 @@
+//@ runDefault("--collectContinuously=1", "--usePolyvariantDevirtualization=0", "--forceDebuggerBytecodeGeneration=1", "--verifyGC=1")
+// UsePolyvariantDevirtualization gives us a PutPrivateName (not byID) while still letting us generate an IC with only one AccessCase
+// DebuggerBytecodeGeneration seems to give the GC more time to interrupt the put because it forces reads from the stack
+
+function PutPrivateNameIC() {
+ let leak = []
+
+ class A {
+ constructor() {
+ this.a = 0
+ }
+ }
+ noInline(A)
+
+ class B extends A {
+ #b
+ #c
+ }
+ noInline(B)
+
+ for (let i = 0; i < 1000000; ++i) {
+ let b1 = new B
+ let b2 = new B
+ let b3 = new B
+ leak.push(b1, b2, b3)
+ }
+}
+noInline(PutPrivateNameIC)
+PutPrivateNameIC()
\ No newline at end of file
Modified: trunk/Source/_javascript_Core/ChangeLog (282662 => 282663)
--- trunk/Source/_javascript_Core/ChangeLog 2021-09-17 15:03:12 UTC (rev 282662)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-09-17 15:56:53 UTC (rev 282663)
@@ -1,3 +1,21 @@
+2021-09-17 Justin Michaud <[email protected]>
+
+ PutByVal and PutPrivateName ICs should emit a write barrier if a butterfly might be allocated
+ https://bugs.webkit.org/show_bug.cgi?id=230378
+
+ Reviewed by Yusuke Suzuki.
+
+ Right now, PutByVal and PutPrivateName check the value type to determine
+ if a write barrier is needed. For example, putting a primitive is considered
+ to not require a write barrier. This makes sense, except for the case when we
+ might allocate or re-allocate a butterfly in the IC. This does not emit a write
+ barrier, and so the GC might miss the new butterfly. That is somewhat undesirable.
+ This is a temporary conservative fix. If we don't write to the butterfly pointer,
+ then we still don't need a write barrier; this work is captured by
+ https://bugs.webkit.org/show_bug.cgi?id=230377
+
+ * dfg/DFGStoreBarrierInsertionPhase.cpp:
+
2021-09-16 Saam Barati <[email protected]>
Don't throw an exception in the middle of linking a CodeBlock
Modified: trunk/Source/_javascript_Core/dfg/DFGStoreBarrierInsertionPhase.cpp (282662 => 282663)
--- trunk/Source/_javascript_Core/dfg/DFGStoreBarrierInsertionPhase.cpp 2021-09-17 15:03:12 UTC (rev 282662)
+++ trunk/Source/_javascript_Core/dfg/DFGStoreBarrierInsertionPhase.cpp 2021-09-17 15:56:53 UTC (rev 282663)
@@ -237,9 +237,9 @@
case Array::BigInt64Array:
case Array::BigUint64Array: {
Edge child1 = m_graph.varArgChild(m_node, 0);
- Edge child3 = m_graph.varArgChild(m_node, 2);
if (!m_graph.m_slowPutByVal.contains(m_node) && (child1.useKind() == CellUse || child1.useKind() == KnownCellUse))
- considerBarrier(child1, child3);
+ // FIXME: there are some cases where we can avoid a store barrier by considering the value https://bugs.webkit.org/show_bug.cgi?id=230377
+ considerBarrier(child1);
break;
}
case Array::Contiguous:
@@ -277,7 +277,8 @@
case PutPrivateName: {
if (!m_graph.m_slowPutByVal.contains(m_node) && (m_node->child1().useKind() == CellUse || m_node->child1().useKind() == KnownCellUse))
- considerBarrier(m_node->child1(), m_node->child3());
+ // FIXME: there are some cases where we can avoid a store barrier by considering the value https://bugs.webkit.org/show_bug.cgi?id=230377
+ considerBarrier(m_node->child1());
break;
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
