Bug 203231

Summary: Clients of JSArray::tryCreateUninitializedRestricted() should invoke the mutatorFence().
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, keith_miller, msaboff, rmorisset, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
work in progress for EWS testing.
none
proposed patch. saam: review+

Description Mark Lam 2019-10-21 19:38:16 PDT
Clients of JSArray::tryCreateUninitializedRestricted() creates a partially initialized JSArray, with the contract that it will take care of filling in all the missing indexed properties before unleashing the newly created array on the world.  We intentionally do not unconditionally write barrier newly created arrays and rely on an owner object (or GC root) that it gets put into to scan it.  That said, there's no guarantee that we won't reach a GC safe point while the newly created array is still on the stack before it gets put into an owner object (or GC root).  

We should ensure that all stores into the array are properly completed before that GC safe point.  Hence, we should invoke the mutatorFence() after the client of JSArray::tryCreateUninitializedRestricted() finishes initializing the array.
Comment 1 Radar WebKit Bug Importer 2019-10-21 19:38:56 PDT
<rdar://problem/56486552>
Comment 2 Mark Lam 2019-10-21 19:40:40 PDT
Created attachment 381501 [details]
work in progress for EWS testing.
Comment 3 Mark Lam 2019-10-21 23:37:51 PDT
Created attachment 381513 [details]
proposed patch.
Comment 4 Saam Barati 2019-10-22 12:03:48 PDT
Comment on attachment 381513 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=381513&action=review

> Source/JavaScriptCore/ChangeLog:18
> +        That said, there's no guarantee that we won't reach a GC safe point with the
> +        newly created array is on the stack before it gets put into an owner object
> +        (or GC root).

how does a safe point not do the required fencing?

I think this is necessary because when we store the array into another object. But I don't think it's necessary for this reason.
Comment 5 Mark Lam 2019-10-22 14:19:57 PDT
(In reply to Saam Barati from comment #4)
> I think this is necessary because when we store the array into another
> object. But I don't think it's necessary for this reason.

Thanks for the review.  I've fixed the comment.

Landed in r251456: <http://trac.webkit.org/r251456>.