Bug 165401 – GetByID IC is wrongly unwrapping the global proxy this value for getter/setters.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

Here's the test case:

function test(x) {
    x.__proto__
}

for (var i = 0; i < 10000; i++)
    test(this);


As soon as test is JIT compiled and the GetById IC gets optimized, we'll get an exception:
    TypeError: Object.prototype.__proto__ called on null or undefined

This should never be the case because x (in the test above) is never null or undefined.

[Mark Lam]

Created attachment 296250 [details]
proposed patch.

[Mark Lam]

<rdar://problem/29442665>

[Mark Lam]

<rdar://problem/29442665> is the wrong bug.

[Mark Lam]

Created attachment 296327 [details]
patch for EWS testing.  Not ready for review.

I'm temporarily using RELEASE_ASSERTs instead of ASSERTs because I'm testing with a release build locally.  Will replace with ASSERTs when I'm done.  I am expecting one of these asserts to fail though, thereby indicating another latent bug ... but will see if I'm right.  Will fix if needed.

[Mark Lam]

Created attachment 296353 [details]
proposed patch.

Ready for a review.

[Saam Barati]

Comment on attachment 296353 [details]
proposed patch.

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

r=me

> Source/JavaScriptCore/ChangeLog:8
> +        Getters/setters are expecting the unwrapped global proxy object.  This is

Don't you mean wrapped, not unwrapped?

[Saam Barati]

(In reply to comment #6)
> Comment on attachment 296353 [details]
> proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=296353&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/ChangeLog:8
> > +        Getters/setters are expecting the unwrapped global proxy object.  This is
> 
> Don't you mean wrapped, not unwrapped?

Maybe this terminology is weird. I'd just say, they're expecting a JSProxy, not a JSGlobalObject.

[Mark Lam]

Comment on attachment 296353 [details]
proposed patch.

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

>> Source/JavaScriptCore/ChangeLog:8
>> +        Getters/setters are expecting the unwrapped global proxy object.  This is
> 
> Don't you mean wrapped, not unwrapped?

Oops, that should be "wrapped".  Thanks for catching this.

[Mark Lam]

(In reply to comment #8)
> Comment on attachment 296353 [details]
> proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=296353&action=review
> 
> >> Source/JavaScriptCore/ChangeLog:8
> >> +        Getters/setters are expecting the unwrapped global proxy object.  This is
> > 
> > Don't you mean wrapped, not unwrapped?
> 
> Oops, that should be "wrapped".  Thanks for catching this.

Agreed.  I'll change that blob of text to say:

"When the this value for a property access is the JS global and that property access is via a GetterSetter, the underlying getter / setter functions would expect the this value they receive to be the JSProxy instance instead of the JSGlobalObject.  This is consistent with how the LLINT and runtime code behaves.  The IC code should behave the same way."

[Mark Lam]

Thanks for the review.  Landed in r209442: <http://trac.webkit.org/r209442>.