1361125 - Handle slot indices meaning slightly different thing on DOM proxies vs DOM non-proxies

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement)

Description

[Boris Zbarsky [:bzbarsky]]

We have code in CodeGenerator::visitGetDOMProperty/visitGetDOMMemberV/visitGetDOMMemberT that assumes that reserved slot indices for DOM objects correspond to fixed slot indices as long as the index is NativeObject::MAX_FIXED_SLOTS.

But that's not true for DOM proxies, because of the extra slot for the private.

We need to adjust this code to either not be reached for DOM proxies at all or take take into account the different slot indexing in proxies.

Comment 1

[Jan de Mooij [:jandem]]

If we don't care about inlining this for now, we could just add TemporaryTypeSet::maybeProxy, like the maybeCallable method, and then check that in IonBuilder. maybeProxy can check clasp->isProxy().

Comment 2

[Boris Zbarsky [:bzbarsky]]

That seems pretty reasonable for the moment.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Add a way to ask a TemporaryTypeSet whether it might contain proxies

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Disable Ion fast paths for DOM getters on proxies when the jitinfo indicates the value can live in a slot

We do this for now because the Ion fast paths assume things about whether slots
are fixed or not, and how reserved slot indices map to fixed slot indices, that
are not true for proxies, because they have an extra reserved slot.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 2, diff -w

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 8868394 [details] [diff] [review]
part 2.  Disable Ion fast paths for DOM getters on proxies when the jitinfo indicates the value can live in a slot

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

Thanks, LGTM.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc19302e6e0c
part 1.  Add a way to ask a TemporaryTypeSet whether it might contain proxies.  r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/556cd0250570
part 2.  Disable Ion fast paths for DOM getters on proxies when the jitinfo indicates the value can live in a slot.  r=jandem

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/cc19302e6e0c
https://hg.mozilla.org/mozilla-central/rev/556cd0250570