Closed Bug 1481740 Opened 3 years ago Closed 3 years ago
Big endian bug in Structured
When a StructuredClone buffer is written and read, the "content" pointer is swapped to little endian byte order. However, on destruction this does not happen. This is easy to overlook as readPtr() does the swapping, but getPtr (used in this case as static function) does not. This may cause a call to js_free() with a byte swapped pointer on big endian platforms, usually causing a crash. Fix is trivial, see attached patch.
So this is what I'm thinking probably ought to be done to fit in with the current scheme. But I'm no longer sure the insistence on not byte-swapping pointers makes sense. I think I originally felt that it was another way to ensure that we didn't accidentally write pointers to disk. But I think I did it when I was still writing things out in network byte order? Either way, it might be simpler now to byte swap everywhere, since now that things are written out little-endian, I'm essentially using our big-endian user base as testers. What do you think?
Attachment #8998593 - Flags: review?(lhansen)
Assignee: nobody → sphink
Status: NEW → ASSIGNED
Comment on attachment 8998593 [details] [diff] [review] Never do endianness swapping on pointers Review of attachment 8998593 [details] [diff] [review]: ----------------------------------------------------------------- The patch seems fine. I observe that SharedArrayBuffer pointers are written using writeBytes() and read using readBytes(), and not endian swapped. (The WHATWG cabal decided that these objects are not "transfered", so they don't show up in the SC transfer code.) It would be most consistent to byteswap pointers though I agree it doesn't matter at the moment, with an agent cluster being an intraprocess thing always. I would be inclined to do it anyway, to be consistent (a rule with no exceptions is easier than a rule with one (two?) exceptions.)
Attachment #8998593 - Flags: review?(lhansen) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c7d62fc647ec Never do endianness swapping on pointers, r=lth
Filed a new bug to byteswap everything (bug 1485460), landing this for now.
You need to log in before you can comment on or make changes to this bug.