Bug 186284 - Set the activeLength of all ScratchBuffers to zero when exiting the VM
Summary: Set the activeLength of all ScratchBuffers to zero when exiting the VM
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-06-04 14:17 PDT by Simon Fraser (smfr)
Modified: 2018-06-04 22:15 PDT (History)
10 users (show)

See Also:

Attachments
patch (3.12 KB, patch)
2018-06-04 15:05 PDT, Saam Barati 		keith_miller: review+
Details | Formatted Diff | Diff
patch for landing (3.09 KB, patch)
2018-06-04 17:03 PDT, Saam Barati 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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