Changeset 199496 in webkit

Timestamp:

Apr 13, 2016 9:10:13 AM (8 years ago)

Author:

mark.lam@apple.com

Message:

ShadowChicken::visitChildren() should not visit tailMarkers and throwMarkers.
​https://bugs.webkit.org/show_bug.cgi?id=156532

Reviewed by Saam Barati and Filip Pizlo.

ShadowChicken can store tailMarkers and throwMarkers in its log, specifically in
the callee field of a log packet. However, ShadowChicken::visitChildren()
unconditionally visits the callee field of each packet as if they are real
objects. If visitChildren() encounters one of these markers in the log, we get a
crash.

This crash was observed in the v8-v6/v8-regexp.js stress test running with shadow
chicken when r199393 landed. r199393 introduced tail calls to a RegExp split
fast path, and the v8-regexp.js test exercised this fast path a lot. Throw in
some timely GCs, and we get a crash party.

The fix is to have ShadowChicken::visitChildren() filter out the tailMarker and
throwMarker.

Alternatively, if perf is an issue, we can allocate 2 dedicated objects for
these markers so that ShadowChicken can continue to visit them. For now, I'm
going with the filter.

• interpreter/ShadowChicken.cpp:

(JSC::ShadowChicken::visitChildren):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• interpreter/ShadowChicken.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-04-13  Mark Lam  <mark.lam@apple.com>
+
+        ShadowChicken::visitChildren() should not visit tailMarkers and throwMarkers.
+        https://bugs.webkit.org/show_bug.cgi?id=156532
+
+        Reviewed by Saam Barati and Filip Pizlo.
+
+        ShadowChicken can store tailMarkers and throwMarkers in its log, specifically in
+        the callee field of a log packet.  However, ShadowChicken::visitChildren()
+        unconditionally visits the callee field of each packet as if they are real
+        objects.  If visitChildren() encounters one of these markers in the log, we get a
+        crash.
+
+        This crash was observed in the v8-v6/v8-regexp.js stress test running with shadow
+        chicken when r199393 landed.  r199393 introduced tail calls to a RegExp split
+        fast path, and the v8-regexp.js test exercised this fast path a lot.  Throw in
+        some timely GCs, and we get a crash party.
+
+        The fix is to have ShadowChicken::visitChildren() filter out the tailMarker and
+        throwMarker.
+
+        Alternatively, if perf is an issue, we can allocate 2 dedicated objects for
+        these markers so that ShadowChicken can continue to visit them.  For now, I'm
+        going with the filter.
+
+        * interpreter/ShadowChicken.cpp:
+        (JSC::ShadowChicken::visitChildren):
+
 2016-04-13  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/JavaScriptCore/interpreter/ShadowChicken.cpp

@@ -355 +355 @@
 void ShadowChicken::visitChildren(SlotVisitor& visitor)
 {
-    for (unsigned i = m_logCursor - m_log; i--;)
-        visitor.appendUnbarrieredReadOnlyPointer(m_log[i].callee);
+    for (unsigned i = m_logCursor - m_log; i--;) {
+        JSObject* callee = m_log[i].callee;
+        if (callee != Packet::tailMarker() && callee != Packet::throwMarker())
+            visitor.appendUnbarrieredReadOnlyPointer(callee);
+    }
     
     for (Frame& frame : m_stack)