1499277 - StructuredClone endianness fixes are incomplete

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[A. Wilcox [:awilfox]]

Bug 1481740 ( https://hg.mozilla.org/mozilla-central/rev/c7d62fc647ec ) removed endianness swapping for pointers.  Bug 1485460 ( https://hg.mozilla.org/mozilla-central/rev/b23f0be9191b ) put them back, but did not reapply the endianness swap in readPtr.  This bustage was giving me crashes in LZ4 when I tried to |mach run| the browser:

#3  0x000000011542a7bc in mozilla::Compression::LZ4::compress(char const*, unsigned long, char*) (aSource=0x4004855601000000 <error: Cannot access memory at address 0x4004855601000000>, aInputSize=<optimized out>, aDest=0x156a8f9ec "")
    at /var/clean/mozppc/mozilla-unified/mfbt/Compression.cpp:26

Note how, if you byteswap aSource, it becomes 0x156850440:

(gdb) p (char const *)0x0156850440
$4 = 0x156850440 "{\"version\":1,\"buildID\":\"20181013160528\",\"appVersion\":\"64.0a1\",\"locale\":\"en-US\",\"visibleDefaultEngines\":[\"amazondotcom\",\"bing\",\"ebay\",\"google-2018\",\"twitter\",\"wikipedia\",\"ddg\"],\"metaData\":{\"searchDefau"...

This also showed up in jsapi-tests, which made it thankfully much easier to poke at:

testMappedArrayBuffer_bug945152

Thread 1 "jsapi-tests" hit Breakpoint 1, 0x000000010086ad48 in JS_GetArrayBufferData (obj=0x3ffff4aa9080, isSharedMemory=0x3fffffffecf7) at /var/clean/mozppc/mozilla-unified/js/src/vm/ArrayBufferObject.cpp:1835
1835    {
(gdb) bt
#0  0x000000010086ad48 in JS_GetArrayBufferData (obj=0x3ffff4aa9080, isSharedMemory=0x3fffffffecf7) at /var/clean/mozppc/mozilla-unified/js/src/vm/ArrayBufferObject.cpp:1835
#1  0x00000001002f3768 in cls_testMappedArrayBuffer_bug945152::VerifyObject (this=0x1016a49d8 <cls_testMappedArrayBuffer_bug945152_instance>, obj=..., offset=<optimized out>, length=<optimized out>, mapped=<optimized out>)
    at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/testMappedArrayBuffer.cpp:96
#2  0x00000001002f3a48 in cls_testMappedArrayBuffer_bug945152::TestCreateObject (this=0x1016a49d8 <cls_testMappedArrayBuffer_bug945152_instance>, offset=<optimized out>, length=<optimized out>)
    at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/testMappedArrayBuffer.cpp:106
#3  0x00000001002fe198 in cls_testMappedArrayBuffer_bug945152::run (this=0x1016a49d8 <cls_testMappedArrayBuffer_bug945152_instance>, global=...) at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/testMappedArrayBuffer.cpp:35
#4  0x0000000100256de0 in main (argc=<optimized out>, argv=<optimized out>) at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/tests.cpp:134
(gdb) finish
cls_testMappedArrayBuffer_bug945152::VerifyObject (this=0x1016a49d8 <cls_testMappedArrayBuffer_bug945152_instance>, obj=..., offset=<optimized out>, length=<optimized out>, mapped=<optimized out>)
    at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/testMappedArrayBuffer.cpp:97
97          CHECK(data);
(gdb) p data
$1 = 0x3ffff7ff9008 "90ABCDEFGHIJ"
(gdb) c
Continuing.

Thread 1 "jsapi-tests" hit Breakpoint 1, 0x000000010086ad48 in JS_GetArrayBufferData (obj=0x3ffff4aa40a0, isSharedMemory=0x3fffffffebd7) at /var/clean/mozppc/mozilla-unified/js/src/vm/ArrayBufferObject.cpp:1835
1835    {
(gdb) bt
#0  0x000000010086ad48 in JS_GetArrayBufferData (obj=0x3ffff4aa40a0, isSharedMemory=0x3fffffffebd7) at /var/clean/mozppc/mozilla-unified/js/src/vm/ArrayBufferObject.cpp:1835
#1  0x00000001002f3768 in cls_testMappedArrayBuffer_bug945152::VerifyObject (this=0x1016a49d8 <cls_testMappedArrayBuffer_bug945152_instance>, obj=..., offset=<optimized out>, length=<optimized out>, mapped=<optimized out>)
    at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/testMappedArrayBuffer.cpp:96
#2  0x00000001002f8c40 in cls_testMappedArrayBuffer_bug945152::TestCloneObject (this=0x1016a49d8 <cls_testMappedArrayBuffer_bug945152_instance>) at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/testMappedArrayBuffer.cpp:144
#3  0x00000001002fe448 in cls_testMappedArrayBuffer_bug945152::run (this=0x1016a49d8 <cls_testMappedArrayBuffer_bug945152_instance>, global=...) at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/testMappedArrayBuffer.cpp:50
#4  0x0000000100256de0 in main (argc=<optimized out>, argv=<optimized out>) at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/tests.cpp:134
(gdb) finish
cls_testMappedArrayBuffer_bug945152::VerifyObject (this=0x1016a49d8 <cls_testMappedArrayBuffer_bug945152_instance>, obj=..., offset=<optimized out>, length=<optimized out>, mapped=<optimized out>)
    at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/testMappedArrayBuffer.cpp:97
97          CHECK(data);
(gdb) p data
$2 = 0x3ffff4aa40e0 "90ABCDEFGHIJMMMM\377\376MMMMMM\377\376MMMMMM"
(gdb) c
Continuing.

Thread 1 "jsapi-tests" hit Breakpoint 1, 0x000000010086ad48 in JS_GetArrayBufferData (obj=0x3ffff4aa91c0, isSharedMemory=0x3fffffffeaf7) at /var/clean/mozppc/mozilla-unified/js/src/vm/ArrayBufferObject.cpp:1835
1835    {
(gdb) bt
#0  0x000000010086ad48 in JS_GetArrayBufferData (obj=0x3ffff4aa91c0, isSharedMemory=0x3fffffffeaf7) at /var/clean/mozppc/mozilla-unified/js/src/vm/ArrayBufferObject.cpp:1835
#1  0x00000001002f3768 in cls_testMappedArrayBuffer_bug945152::VerifyObject (this=0x1016a49d8 <cls_testMappedArrayBuffer_bug945152_instance>, obj=..., offset=<optimized out>, length=<optimized out>, mapped=<optimized out>)
    at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/testMappedArrayBuffer.cpp:96
#2  0x00000001002fdf4c in cls_testMappedArrayBuffer_bug945152::TestTransferObject (this=0x1016a49d8 <cls_testMappedArrayBuffer_bug945152_instance>) at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/testMappedArrayBuffer.cpp:182
#3  0x00000001002fe470 in cls_testMappedArrayBuffer_bug945152::run (this=0x1016a49d8 <cls_testMappedArrayBuffer_bug945152_instance>, global=...) at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/testMappedArrayBuffer.cpp:56
#4  0x0000000100256de0 in main (argc=<optimized out>, argv=<optimized out>) at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/tests.cpp:134
(gdb) finish
cls_testMappedArrayBuffer_bug945152::VerifyObject (this=0x1016a49d8 <cls_testMappedArrayBuffer_bug945152_instance>, obj=..., offset=<optimized out>, length=<optimized out>, mapped=<optimized out>)
    at /var/clean/mozppc/mozilla-unified/js/src/jsapi-tests/testMappedArrayBuffer.cpp:97
97          CHECK(data);
(gdb) p data
$3 = 0x860fff7ff3f0000 <error: Cannot access memory at address 0x860fff7ff3f0000>
(gdb)

Comment 1

[A. Wilcox [:awilfox]]

Attachment: Fix SCInput::readPtr on big endian systems.

Attached patch adds the check back to readPtr, which allows jsapi-tests to pass successfully - and the engine now runs flawlessly on 64-bit big endian PowerPC!

Comment 2

[Steve Fink [:sfink] [:s:]]

Comment on attachment 9017404 [details] [diff] [review]
Fix SCInput::readPtr on big endian systems.

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

Wow. I can't believe I missed this over in bug 1485460. Thanks!

Comment 3

[Jason Orendorff [:jorendorff]]

I may be confused, but it seems like we should remove SCInput::readNativeEndian entirely and use SCInput::read() here.

Comment 4

[Steve Fink [:sfink] [:s:]]

(In reply to Jason Orendorff [:jorendorff] from comment #3)
> I may be confused, but it seems like we should remove
> SCInput::readNativeEndian entirely and use SCInput::read() here.

I do not think you are confused. I think you are right.

readNativeEndian is rather pointless now that we're no longer storing anything in native endian format. We have as well have SCInput::readEBCDIC.

Comment 5

[A. Wilcox [:awilfox]]

Attachment: Remove unnecessary SCInput::readNativeEndian; fix SCInput::readPtr on big endian systems.

New revision, tweaking readPtr and removing the superfluous readNativeEndian.

Comment 6

[Jason Orendorff [:jorendorff]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3918d51413895c55261613e9373b2440b1eefa0b

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a58f94c3f5a
Remove unnecessary SCInput::readNativeEndian; fix SCInput::readPtr on big endian systems. r=sfink

Comment 8

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/3a58f94c3f5a