Bug 156532

Summary: ShadowChicken::visitChildren() should not visit tailMarkers and throwMarkers.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, benjamin, fpizlo, ggaren, keith_miller, msaboff, saam
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch. saam: review+

Mark Lam
Reported 2016-04-13 00:00:44 PDT
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.
Attachments
proposed patch. (2.49 KB, patch)
2016-04-13 00:06 PDT, Mark Lam
saam: review+
Mark Lam
Comment 1 2016-04-13 00:06:09 PDT
Created attachment 276308 [details] proposed patch.
Filip Pizlo
Comment 2 2016-04-13 07:22:02 PDT
R=me too
Mark Lam
Comment 3 2016-04-13 09:10:48 PDT
Thanks for the review. Landed in r199496: <http://trac.webkit.org/r199496>.
Mark Lam
Comment 4 2016-04-14 11:04:56 PDT
Note You need to log in before you can comment on or make changes to this bug.