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 --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,
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,

Back to Bug 1658070 Comment 5