1237501 - Baseline IC for unshadowed DOM proxy gets never actually gets created

Status: RESOLVED FIXED

Description

[Boris Zbarsky [:bzbarsky, bz on IRC]]

This is because TryAttachNativeGetAccessorPropStub does:

    bool cacheableCall = IsCacheableGetPropCall(cx, obj, holder, shape, &isScripted,
                                                isTemporarilyUnoptimizable)

which passes isDOMProxy == false implicitly.  Then IsCacheableGetPropCall does IsCacheableProtoChain which returns false because isDOMProxy is false but we're looking at a DOM proxy.

It looks like we do still manage to set up the _ion_ IC in this situation, so this mostly affects cases that don't get ion-compiled.

I looked at the blame for this code, but it's been broken for a good long while.  Possibly on initial checkin.  :(  I really wish we had some regression tests for this stuff somehow.

Comment 1

[Boris Zbarsky [:bzbarsky, bz on IRC]]

I've verified that passing isDOMProxy correctly here fixes the testcase linked from bug 1237451 in two ways: the baseline IC spew actually shows us setting up the ICs now and a profile and breakpoints in a debugger both show us making direct calls to GenericBindingGetter from jitcode instead of via proxy machinery.

Comment 2

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Created attachment 8704961 [details] [diff] [review]
Actually create a baseline IC for a JSNative accessor property get that's unshadowed on a DOM proxy.  We apparently failed to ever do this

Comment 3

[Peter Van der Beken [:peterv]]

(In reply to Boris Zbarsky [:bz] (Vacation until Jan 4) from comment #0)
> I looked at the blame for this code, but it's been broken for a good long
> while.  Possibly on initial checkin.  :(

http://hg.mozilla.org/mozilla-central/diff/97ccac9529a6/js/src/jit/BaselineIC.cpp#l1.66

:-(

Comment 4

[Peter Van der Beken [:peterv]]

I guess the real issue is that http://hg.mozilla.org/mozilla-central/rev/9e86bfdf6bd8#l1.350 removed the isNative() check but didn't add back the isDOMProxy.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8704961 [details] [diff] [review]
Actually create a baseline IC for a JSNative accessor property get that's unshadowed on a DOM proxy.  We apparently failed to ever do this

Review of attachment 8704961 [details] [diff] [review]:
-----------------------------------------------------------------

Ugh. Great catch, thanks.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/52cf63c41944

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/52cf63c41944