Closed Bug 1367815 Opened 3 years ago Closed 3 years ago

Investigate whether proxies other than CCWs store a cross compartment pointer in their private slot

Categories

(Core :: JavaScript: GC, enhancement)

55 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jonco, Assigned: jonco)

References

Details

Attachments

(1 file)

I didn't think this was supposed to happen but the assertion failures in bug 1365564 after the initial patch landed suggest that it does.
This seems scary. The odds of these sorts of things handling cross-compartment pointers correctly is extremely low.
I couldn't figure out what was causing this.  This patch just adds a couple of assertions to catch this if it ever happens.
Assignee: nobody → jcoppeard
Attachment #8873415 - Flags: review?(sphink)
Comment on attachment 8873415 [details] [diff] [review]
bug1367815-cc-proxies

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

nice
Attachment #8873415 - Flags: review?(sphink) → review+
Comment on attachment 8873415 [details] [diff] [review]
bug1367815-cc-proxies

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

::: js/src/vm/ProxyObject.cpp
@@ +92,5 @@
>      values->init(proxy->numReservedSlots());
>  
>      proxy->data.handler = handler;
> +    if (IsCrossCompartmentWrapper(proxy))
> +        proxy->setCrossCompartmentPrivate(priv);

Would it be possible to assert this in setCrossCompartmentPrivate? IsCrossCompartmentWrapper will add a bunch of overhead in opt builds (it's not inlined).
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e5ac6fa7858
Add assertions to prevent proxies other than cross compartment wrappers from having cross compartment targets r=sfink
https://hg.mozilla.org/mozilla-central/rev/9e5ac6fa7858
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.