Closed Bug 1410545 Opened 2 years ago Closed Last year

Fix js/src/tests/ecma_6/TypedArray/sort_snans.js on big endian

Categories

(Core :: JavaScript Engine, defect, P5)

52 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox-esr60 - fixed
firefox58 --- wontfix
firefox59 --- wontfix
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 --- fixed

People

(Reporter: bunk, Assigned: bunk)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch tests-snans-be.patch (obsolete) — Splinter Review
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.9) Gecko/20100101 Goanna/3.3 Firefox/52.9 PaleMoon/27.5.1
Build ID: 20171018184505

Steps to reproduce:

In mozjs52:
make -C js/src check-jstests


Actual results:

## ecma_6/TypedArray/sort_snans.js: rc = 3, run time = 0.40651
ecma_6/shell.js:93:23 Error: Assertion failed: got -1.8938930325389462e+304, expected NaN at _[0]
Stack:
  assertSameValue@ecma_6/shell.js:93:23
  check@ecma_6/shell.js:198:21
  assertSameProps@ecma_6/shell.js:162:25
  check@ecma_6/shell.js:213:25
  assertDeepEq@ecma_6/shell.js:221:13
  testFloat64NaNRanges@ecma_6/TypedArray/sort_snans.js:60:5
  @ecma_6/TypedArray/sort_snans.js:68:1
TEST-UNEXPECTED-FAIL | ecma_6/TypedArray/sort_snans.js | (args: "")



Expected results:

TEST-PASS | ecma_6/TypedArray/sort_snans.js | (args: "")
Attachment #8920710 - Attachment is patch: true
Attachment #8920710 - Attachment mime type: application/x-patch → text/plain
Attachment #8920710 - Flags: review?(jwalden+bmo)
Priority: -- → P5
Comment on attachment 8920710 [details] [diff] [review]
tests-snans-be.patch

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

::: mozjs52-52.3.1.orig/js/src/tests/ecma_6/TypedArray/sort_snans.js
@@ +38,5 @@
> +    // Swap on big endian platforms
> +    if ((new Uint32Array((new Uint8Array([1,2,3,4])).buffer))[0] === 0x01020304) {
> +	let tmp = startHi;
> +	startHi = startLow;
> +	startLow = tmp;

Seems like a good place, for readability, to do

  [startHi, startLow] = [startLow, startHi];

and then

  [endHi, endLow] = [endLow, endHi];

just below.
Attachment #8920710 - Flags: review?(jwalden+bmo) → review+
Attachment #8920710 - Attachment is obsolete: true
(In reply to Jeff Walden [:Waldo] (I'm baaaaaaack...) from comment #1)
> Comment on attachment 8920710 [details] [diff] [review]
> tests-snans-be.patch
> 
> Review of attachment 8920710 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mozjs52-52.3.1.orig/js/src/tests/ecma_6/TypedArray/sort_snans.js
> @@ +38,5 @@
> > +    // Swap on big endian platforms
> > +    if ((new Uint32Array((new Uint8Array([1,2,3,4])).buffer))[0] === 0x01020304) {
> > +	let tmp = startHi;
> > +	startHi = startLow;
> > +	startLow = tmp;
> 
> Seems like a good place, for readability, to do
> 
>   [startHi, startLow] = [startLow, startHi];
> 
> and then
> 
>   [endHi, endLow] = [endLow, endHi];
> 
> just below.

Thanks for the suggestion, updated patch attached.
[Tracking Requested - why for this release]:
Fix is test-only, currently required to be carried by distributions that use mozjs, support big endian architectures and care about test results (e.g. Debian, Ubuntu).
Waldo, can you give this a final once-over and push it if it looks good? I'd be happy to uplift it to ESR60 as a test-only change, but it needs to be on m-c first.
Assignee: nobody → bunk
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(jwalden+bmo)
Comment on attachment 8928670 [details] [diff] [review]
tests-snans-bev2.patch

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

Seems fine from that context.
Attachment #8928670 - Flags: review+
Flags: needinfo?(jwalden+bmo)
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e8aec0dccbb8
Fix sort_snans.js test on big endian.  r=jwalden
https://hg.mozilla.org/mozilla-central/rev/e8aec0dccbb8
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.