Bug 186284

Summary: Set the activeLength of all ScratchBuffers to zero when exiting the VM
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, ggaren, keith_miller, mark.lam, msaboff, saam, simon.fraser, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
keith_miller: review+
patch for landing none

Simon Fraser (smfr)
Reported 2018-06-04 14:17:01 PDT
Even after the fix for bug 186223, there are code paths that can leave ScratchBuffers with non-zero activeLength(), which can potentially cause things to be GC roots via the conservative scan. We should just set the activeLength of all scratch buffers to zero when leaving VM entry scope.
Attachments
patch (3.12 KB, patch)
2018-06-04 15:05 PDT, Saam Barati 		keith_miller: review+
DetailsFormatted DiffDiff
patch for landing (3.09 KB, patch)
2018-06-04 17:03 PDT, Saam Barati 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2018-06-04 14:17:47 PDT
<rdar://problem/40780738>
Saam Barati
Comment 2 2018-06-04 14:34:44 PDT
Patch forthcoming
Saam Barati
Comment 3 2018-06-04 15:05:14 PDT
Created attachment 341922 [details] patch
Keith Miller
Comment 4 2018-06-04 16:50:01 PDT
Comment on attachment 341922 [details] patch r=me.
Saam Barati
Comment 5 2018-06-04 17:00:23 PDT
Keith mentioned doing this in a follow-up: https://bugs.webkit.org/show_bug.cgi?id=186292
Saam Barati
Comment 6 2018-06-04 17:03:31 PDT
Created attachment 341940 [details] patch for landing
WebKit Commit Bot
Comment 7 2018-06-04 18:13:04 PDT
Comment on attachment 341940 [details] patch for landing Clearing flags on attachment: 341940 Committed r232490: <https://trac.webkit.org/changeset/232490>
WebKit Commit Bot
Comment 8 2018-06-04 18:13:06 PDT
All reviewed patches have been landed. Closing bug.
Geoffrey Garen
Comment 9 2018-06-04 19:28:08 PDT
Under what conditions do we enter the garbage collector with a live scratch buffer? OSR exit with object re-materialization, maybe? Just wondering why we need to mark scratch buffers at all...
Saam Barati
Comment 10 2018-06-04 22:15:02 PDT
(In reply to Geoffrey Garen from comment #9) > Under what conditions do we enter the garbage collector with a live scratch > buffer? OSR exit with object re-materialization, maybe? Just wondering why > we need to mark scratch buffers at all... There are probably more cases than that. Some quick grepping: - OSR entry in loops and catch. I guess it's non-obvious if we need contents marked here. - Array push with > 1 argument - NewArray - NewArrayWithSpread - OSR exit as you said
Note You need to log in before you can comment on or make changes to this bug.