Bug 114318

Summary: Add liveness tests to JSC API entry points
Product: WebKit Reporter: Oliver Hunt <oliver>
Component: New BugsAssignee: Oliver Hunt <oliver>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch ggaren: review+

Description Oliver Hunt 2013-04-09 16:21:09 PDT
Add liveness tests to JSC API entry points
Comment 1 Oliver Hunt 2013-04-09 16:22:30 PDT
Created attachment 197185 [details]
Patch
Comment 2 Oliver Hunt 2013-04-09 16:27:38 PDT
Committed r148062: <http://trac.webkit.org/changeset/148062>
Comment 3 Oliver Hunt 2013-04-09 17:46:10 PDT
Reopening to attach new patch.
Comment 4 Oliver Hunt 2013-04-09 17:46:11 PDT
Created attachment 197191 [details]
Patch
Comment 5 Geoffrey Garen 2013-04-09 18:13:24 PDT
Comment on attachment 197191 [details]
Patch

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

> Source/JavaScriptCore/API/JSObjectRef.cpp:343
>  void* JSObjectGetPrivate(JSObjectRef object)
>  {
> -    JSObject* jsObject = toJS(object);
> +    JSObject* jsObject = unsafeToJS(object);

Why does GetPrivate need to use the unsafe function?
Comment 6 Oliver Hunt 2013-04-09 18:16:44 PDT
(In reply to comment #5)
> (From update of attachment 197191 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=197191&action=review
> 
> > Source/JavaScriptCore/API/JSObjectRef.cpp:343
> >  void* JSObjectGetPrivate(JSObjectRef object)
> >  {
> > -    JSObject* jsObject = toJS(object);
> > +    JSObject* jsObject = unsafeToJS(object);
> 
> Why does GetPrivate need to use the unsafe function?

It's called by finalizers when (by definition) the structure chain for an object may no longer be perfect :-/

Somewhat annoying, but this still covers the majority of cases.
Comment 7 Geoffrey Garen 2013-04-09 18:19:11 PDT
> > Why does GetPrivate need to use the unsafe function?
> 
> It's called by finalizers when (by definition) the structure chain for an object may no longer be perfect :-/

OK. You should add a comment that explains that detail.

Also, let's rename the function to "uncheckedToJS". It's always safe to call the function, it just does less checking.
Comment 8 Geoffrey Garen 2013-04-09 18:19:25 PDT
Comment on attachment 197191 [details]
Patch

r=me with those changes
Comment 9 Oliver Hunt 2013-04-09 18:22:20 PDT
Committed r148073: <http://trac.webkit.org/changeset/148073>
Comment 10 Alexey Proskuryakov 2013-04-10 10:34:37 PDT
This has caused (or maybe uncovered) bug 114341.