Closed Bug 1268224 Opened 6 years ago Closed 5 years ago

Differential Testing: Different output message involving division

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
All
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox49 --- affected
firefox50 --- fixed

People

(Reporter: gkw, Assigned: jandem)

Details

(Keywords: testcase)

Attachments

(1 file)

y = new Float64Array(9);
for (var i = 0; i < 2; ++i) {
    x = x ^ x;
    x = x / x;
}
y[0] = x;
var x;
print(uneval(new Int8Array(y.buffer)));


$ ./js-dbg-64-dm-clang-darwin-ab0044bfa1df --fuzzing-safe --no-threads --ion-eager testcase.js
({0:0, 4:0, 8:0, 12:0, 16:0, 20:0, 24:0, 28:0, 32:0, 36:0, 40:0, 44:0, 48:0, 52:0, 56:0, 60:0, 64:0, 68:0, 1:0, 5:0, 9:0, 13:0, 17:0, 21:0, 25:0, 29:0, 33:0, 37:0, 41:0, 45:0, 49:0, 53:0, 57:0, 61:0, 65:0, 69:0, 2:0, 6:-8, 10:0, 14:0, 18:0, 22:0, 26:0, 30:0, 34:0, 38:0, 42:0, 46:0, 50:0, 54:0, 58:0, 62:0, 66:0, 70:0, 3:0, 7:-1, 11:0, 15:0, 19:0, 23:0, 27:0, 31:0, 35:0, 39:0, 43:0, 47:0, 51:0, 55:0, 59:0, 63:0, 67:0, 71:0})

$ ./js-dbg-64-dm-clang-darwin-ab0044bfa1df --fuzzing-safe --no-threads --no-baseline --no-ion testcase.js
({0:0, 4:0, 8:0, 12:0, 16:0, 20:0, 24:0, 28:0, 32:0, 36:0, 40:0, 44:0, 48:0, 52:0, 56:0, 60:0, 64:0, 68:0, 1:0, 5:0, 9:0, 13:0, 17:0, 21:0, 25:0, 29:0, 33:0, 37:0, 41:0, 45:0, 49:0, 53:0, 57:0, 61:0, 65:0, 69:0, 2:0, 6:-8, 10:0, 14:0, 18:0, 22:0, 26:0, 30:0, 34:0, 38:0, 42:0, 46:0, 50:0, 54:0, 58:0, 62:0, 66:0, 70:0, 3:0, 7:127, 11:0, 15:0, 19:0, 23:0, 27:0, 31:0, 35:0, 39:0, 43:0, 47:0, 51:0, 55:0, 59:0, 63:0, 67:0, 71:0})

Tested this on m-c rev ab0044bfa1df.

Note the value of the top set of flags (7:-1) vs the bottom set of flags (7:127).

My configure flags are:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin14.5.0 --disable-jemalloc --enable-debug --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic" -r ab0044bfa1df

This seems to occur before m-c rev dc4b163f7db7 (Oct 2014), but since this involves JIT and typed arrays, setting s-s as a start, and needinfo? from Jan/Hannes to move this forward.
Flags: needinfo?(jdemooij)
Flags: needinfo?(hv1989)
This looks like NaN canonicalization again somehow.  But it's float64, and I thought we did that in deterministic builds, so...
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #1)
> This looks like NaN canonicalization again somehow.  But it's float64, and I
> thought we did that in deterministic builds, so...

The Baseline IC stub for x / x does |divsd %xmm1, %xmm0| and because x == 0, that returns NaN. Unfortunately that's a NaN value with the sign bit set: 0xfff8000000000000.

Unfortunately, NumberDiv has a special path for this case (0 / 0) that returns GenericNaN(), a NaN value *without* the sign bit set...

https://dxr.mozilla.org/mozilla-central/rev/86730d0a82093d705e44f33a34973d28b269f1ea/js/src/jslibmath.h#51-57
Flags: needinfo?(jdemooij)
Flags: needinfo?(hv1989)
Group: javascript-core-security
Summary: Differential Testing: Different output message involving .buffer → Differential Testing: Different output message involving division
Jan, what's next here?
Flags: needinfo?(jdemooij)
Er nevermind my comment 2 - NaN canonicalization on TA stores in more-deterministic builds should take care of that. Let me take another look.
Attached patch PatchSplinter Review
At some point (SAB/atomics maybe?) the canonicalization in more-deterministic builds broke.

This also makes DataView setFloat32/setFloat64 canonicalize.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8751780 - Flags: review?(lhansen)
Comment on attachment 8751780 [details] [diff] [review]
Patch

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

I agree, the problem with setElement() is almost certainly my bug.  The other one I won't claim yet :)
Attachment #8751780 - Flags: review?(lhansen) → review+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7affa37fb9c
Fix canonicalization in more-deterministic builds. r=lth
https://hg.mozilla.org/mozilla-central/rev/a7affa37fb9c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Jan, can we also get this on mozilla-aurora? (It just missed the merge window...) Thanks!
Flags: needinfo?(jdemooij)
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #9)
> Jan, can we also get this on mozilla-aurora? (It just missed the merge
> window...) Thanks!

Sorry for the late response. I think this can ride the trains now.
Flags: needinfo?(jdemooij)
You need to log in before you can comment on or make changes to this bug.