PPC/s390 (be/native): testcase harmony/atomics.js failing

[jbajwa]

>out/s390x.release/d8 --test --random-seed=1476338298 --nohard-abort --nodead-code-elimination --nofold-constants --harmony-sharedarraybuffer test/mjsunit/mjsunit.js test/mjsunit/harmony/atomics.js

test/mjsunit/mjsunit.js:208: Object <Error()> is not an instance of <RangeError> but of <TypeError>
Stack: Error
    at new MjsUnitAssertionError (test/mjsunit/mjsunit.js:31:16)
    at failWithMessage (test/mjsunit/mjsunit.js:208:11)
    at assertInstanceof (test/mjsunit/mjsunit.js:402:7)
    at assertThrows (test/mjsunit/mjsunit.js:370:9)
    at Function.<anonymous> (test/mjsunit/harmony/atomics.js:70:5)
    at Array.forEach (native)
    at TestBadIndex (test/mjsunit/harmony/atomics.js:67:14)
    at test/mjsunit/harmony/atomics.js:120:3
    throw new MjsUnitAssertionError(message);
    ^
Error
    at new MjsUnitAssertionError (test/mjsunit/mjsunit.js:31:16)
    at failWithMessage (test/mjsunit/mjsunit.js:208:11)
    at assertInstanceof (test/mjsunit/mjsunit.js:402:7)
    at assertThrows (test/mjsunit/mjsunit.js:370:9)
    at Function.<anonymous> (test/mjsunit/harmony/atomics.js:70:5)
    at Array.forEach (native)
    at TestBadIndex (test/mjsunit/harmony/atomics.js:67:14)
    at test/mjsunit/harmony

[mcornac]

The same line fixes issue 10 and 11.

https://chromiumcodereview.appspot.com/2436893003/diff/40001/src/builtins/builtins-sharedarraybuffer.cc old: a->BitFieldDecodeJSArrayBuffer::IsShared(a->LoadObjectField(array_buffer, JSArrayBuffer::kBitFieldSlot)); current (buggy): a->IsSetWord32JSArrayBuffer::IsShared(a->LoadObjectField(array_buffer, JSArrayBuffer::kBitFieldSlot, MachineType::Uint32()));

[mcornac]

I think the fix is to use JSArrayBuffer::kBitFieldOffset instead of JSArrayBuffer::kBitFieldSlot since it includes accounting for BigEndian/64-bit if I am reading src/objects.h correctly.

Diff:

-  Node* is_buffer_shared =
-      a->IsSetWord32<JSArrayBuffer::IsShared>(a->LoadObjectField(
-          array_buffer, JSArrayBuffer::kBitFieldSlot, MachineType::Uint32()));
+  Node* is_buffer_shared = a->IsSetWord32<JSArrayBuffer::IsShared>(
+      a->LoadObjectField(array_buffer, JSArrayBuffer::kBitFieldOffset,
+                         MachineType::Uint32()));

Using the change above I see this test case and the ones from issues 10 and 11 fixed on 64-bit zLinux.

[jbajwa]

Thanks, the fix lgtm. I see in src/object.h

10947   static const int kBitFieldSlot = kBackingStoreOffset + kPointerSize;
10948 #if V8_TARGET_LITTLE_ENDIAN || !V8_HOST_ARCH_64_BIT
10949   static const int kBitFieldOffset = kBitFieldSlot;
10950 #else
10951   static const int kBitFieldOffset = kBitFieldSlot + kIntSize;
10952 #endif

so JSArrayBuffer::kBitFieldOffset should be the correct variable to use, since it takes into account endianness. Do you want to go ahead and upstream this change?

[mcornac]

code review here: https://codereview.chromium.org/2470023003/

[mcornac]

Commit: fb7841b314e73e665c3c13a24ca8d96eec553ac3