I can confirm the slot is being corrupted. With the attached patch, running `./mach jit-test -G auto-regress/bug1538542-1.js` we see a failure. Investigating the failure shows the corruption happens as part of `JSObject::swap`. The expando slot is uninitialized memory after `ProxyObject::initExternalValueArrayAfterSwap` runs. I tried writing a simple fix, where I'd stash the expando object and try to restore it. What confuses me though is how to handle the case where you're not swapping two proxies. _i.e._ ``` diff --git a/js/src/vm/JSObject.cpp b/js/src/vm/JSObject.cpp --- a/js/src/vm/JSObject.cpp +++ b/js/src/vm/JSObject.cpp @@ -1739,6 +1739,10 @@ void JSObject::swap(JSContext* cx, Handl } } + RootedObject proxyExpandoA( + cx, proxyA ? proxyA->expando().toObjectOrNull() : nullptr); + RootedObject proxyExpandoB( + cx, proxyB ? proxyB->expando().toObjectOrNull() : nullptr); // Swap the main fields of the objects, whether they are native objects or // proxies. char tmp[sizeof(JSObject_Slots0)]; @@ -1771,6 +1775,14 @@ void JSObject::swap(JSContext* cx, Handl oomUnsafe.crash("initExternalValueArray"); } } + + // Swap proxy expando objects + if (a->is<ProxyObject>()) { + b->as<ProxyObject>().setExpando(proxyExpandoA); + } + if (b->is<ProxyObject>()) { + a->as<ProxyObject>().setExpando(proxyExpandoB); + } } // Swapping the contents of two objects invalidates type sets which contain ``` Doesn't work, because it assumes that `a` and `b` are either both proxies or not; which doesn't seem to hold for this test case,
Bug 1658070 Comment 5 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
I can confirm the slot is being corrupted. With the attached patch, running `./mach jit-test -G auto-regress/bug1538542-1.js` we see a failure. Investigating the failure shows the corruption happens as part of `JSObject::swap`. The expando slot is uninitialized memory after `ProxyObject::initExternalValueArrayAfterSwap` runs. I tried writing a simple fix, where I'd stash the expando object and try to restore it. What confuses me though is how to handle the case where you're not swapping two proxies. _i.e._ ```diff diff --git a/js/src/vm/JSObject.cpp b/js/src/vm/JSObject.cpp --- a/js/src/vm/JSObject.cpp +++ b/js/src/vm/JSObject.cpp @@ -1739,6 +1739,10 @@ void JSObject::swap(JSContext* cx, Handl } } + RootedObject proxyExpandoA( + cx, proxyA ? proxyA->expando().toObjectOrNull() : nullptr); + RootedObject proxyExpandoB( + cx, proxyB ? proxyB->expando().toObjectOrNull() : nullptr); // Swap the main fields of the objects, whether they are native objects or // proxies. char tmp[sizeof(JSObject_Slots0)]; @@ -1771,6 +1775,14 @@ void JSObject::swap(JSContext* cx, Handl oomUnsafe.crash("initExternalValueArray"); } } + + // Swap proxy expando objects + if (a->is<ProxyObject>()) { + b->as<ProxyObject>().setExpando(proxyExpandoA); + } + if (b->is<ProxyObject>()) { + a->as<ProxyObject>().setExpando(proxyExpandoB); + } } // Swapping the contents of two objects invalidates type sets which contain ``` Doesn't work, because it assumes that `a` and `b` are either both proxies or not; which doesn't seem to hold for this test case,