Bug 203231 - Clients of JSArray::tryCreateUninitializedRestricted() should invoke the mutatorFence().
Summary: Clients of JSArray::tryCreateUninitializedRestricted() should invoke the muta...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-21 19:38 PDT by Mark Lam
Modified: 2019-10-22 14:19 PDT (History)
8 users (show)

See Also:


Attachments
work in progress for EWS testing. (4.06 KB, patch)
2019-10-21 19:40 PDT, Mark Lam
no flags Details | Formatted Diff | Diff
proposed patch. (7.00 KB, patch)
2019-10-21 23:37 PDT, Mark Lam
saam: review+
Details | Formatted Diff | Diff

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