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

Description Simon Fraser (smfr) 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.
Comment 1 Radar WebKit Bug Importer 2018-06-04 14:17:47 PDT 
<rdar://problem/40780738>
Comment 2 Saam Barati 2018-06-04 14:34:44 PDT 
Patch forthcoming
Comment 3 Saam Barati 2018-06-04 15:05:14 PDT 
Created attachment 341922 [details]
patch
Comment 4 Keith Miller 2018-06-04 16:50:01 PDT 
Comment on attachment 341922 [details]
patch

r=me.
Comment 5 Saam Barati 2018-06-04 17:00:23 PDT 
Keith mentioned doing this in a follow-up:
https://bugs.webkit.org/show_bug.cgi?id=186292
Comment 6 Saam Barati 2018-06-04 17:03:31 PDT 
Created attachment 341940 [details]
patch for landing
Comment 7 WebKit Commit Bot 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>
Comment 8 WebKit Commit Bot 2018-06-04 18:13:06 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 9 Geoffrey Garen 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...
Comment 10 Saam Barati 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