Bug 144067 - SparseArrayEntry's write barrier owner should be the SparseArrayValueMap.
Summary: SparseArrayEntry's write barrier owner should be the SparseArrayValueMap.
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
Keywords: InRadar
Depends on:
Reported: 2015-04-22 12:38 PDT by Mark Lam
Modified: 2015-04-22 13:47 PDT (History)
9 users (show)

See Also:

the patch. (5.54 KB, patch)
2015-04-22 13:16 PDT, Mark Lam
msaboff: 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 2015-04-22 12:38:04 PDT
Currently, there are a few places where the JSObject that owns the SparseArrayValueMap is designated as the owner of the SparseArrayEntry write barrier.  This is a bug and can result in the GC collecting the SparseArrayEntry even though it is being referenced by the SparseArrayValueMap.
Comment 1 Mark Lam 2015-04-22 12:39:52 PDT
Comment 2 Mark Lam 2015-04-22 13:16:23 PDT
Created attachment 251362 [details]
the patch.
Comment 3 Mark Lam 2015-04-22 13:23:30 PDT
For the record, I made SparseArrayEntry privately inherit WriteBarrier<Unknown> and created differently named setter functions which wraps the WriteBarrier ones, and then did a build to let Clang tell me of every place where SparseArrayEntry::set() is used.  That is how I know I've covered all explicit calls to SparseArrayEntry::set().

I also searched for "set(" in JSObject.h/cpp, JSArray.h/cpp, and SparseArrayValueMap.h/cpp, and audited them visually.
Comment 4 Michael Saboff 2015-04-22 13:41:04 PDT
Comment on attachment 251362 [details]
the patch.

Comment 5 Mark Lam 2015-04-22 13:47:28 PDT
Thanks for the review.  Per Michael's offline suggestion, I added a comment to the new test to indicate that it should not crash if the bug is fixed.

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