Closed Bug 1237501 Opened 5 years ago Closed 5 years ago
Baseline IC for unshadowed DOM proxy gets never actually gets created
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.
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.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
(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 :-(
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 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.
Attachment #8704961 - Flags: review?(jdemooij) → review+
You need to log in before you can comment on or make changes to this bug.