Differential Testing: Different output message involving IonMonkey on ARM64 and Math.trunc
Categories
(Core :: JavaScript Engine: JIT, defect, P2)
Tracking
()
People
(Reporter: gkw, Assigned: sstangl)
References
(Blocks 1 open bug)
Details
(Keywords: regression, testcase, Whiteboard: [fuzzblocker])
Attachments
(1 file)
47 bytes,
text/x-phabricator-request
|
lizzard
:
approval-mozilla-beta+
|
Details | Review |
x = [8589934592, -0];
for (let i = 0; i < 2; ++i) {
print(uneval(Math.trunc(Math.tan(x[i]))));
}
Note that 8589934592
is 2^33
and 8589934591 (2^33 -1)
does not seem to show the issue.
$ ./js-dbg-64-dm-armsim64-linux-x86_64-2c49e736571b --fuzzing-safe --no-threads --baseline-eager --no-ion testcase.js
1
-0
$
$ ./js-dbg-64-dm-armsim64-linux-x86_64-2c49e736571b --fuzzing-safe --no-threads --ion-eager testcase.js
1
0
$
Tested this on m-c rev 2c49e736571b.
My configure flags are:
AR=ar sh ./configure --enable-simulator=arm64 --enable-debug --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests --disable-cranelift
python3 -u -m funfuzz.js.compile_shell -b "--enable-debug --enable-more-deterministic --enable-simulator=arm64" -r 2c49e736571b
Setting needinfo? from Sean and Nicolas since this is IonMonkey on ARM64 (and does not show on a regular debug deterministic 64-bit shell). Also setting [fuzzblocker] because this is hard to differentiate from the other fuzzblocking compare_jit issues.
Bisection is underway...
![]() |
Reporter | |
Comment 1•2 years ago
|
||
autobisectjs shows this is probably related to the following changeset:
The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/afb2e1e1665f
user: Sean Stangl
date: Thu Mar 07 03:57:23 2019 +0000
summary: Bug 1528869 - Enable IonMonkey in the ARM64 shell, but keep it disabled in the browser. r=nbp
![]() |
Reporter | |
Updated•2 years ago
|
Assignee | ||
Comment 2•2 years ago
|
||
P2, this reproduces. I'll take it.
Assignee | ||
Comment 3•2 years ago
|
||
The existing truncation code did not correctly handle the case of negative zero.
The fix is to avoid using FCMP floating-point comparisons, and check
the sign bit explicitly in a GPR.
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Pushed by sstangl@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f14763229806 Fix -0 handling in ARM64 visitTrunc(). r=nbp
Comment 5•2 years ago
|
||
bugherder |
Do you think this is safe to uplift to beta or should we let it ride to release in 68?
Assignee | ||
Comment 7•2 years ago
|
||
Comment on attachment 9056251 [details]
Bug 1538083 - Fix -0 handling in ARM64 visitTrunc(). r=nbp
Beta/Release Uplift Approval Request
- Feature/Bug causing the regression: None
- User impact if declined: Math calculations involving truncation of
-0
will be incorrect. - Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): The fix was validated. But given that this functionality is currently broken in 64, even if it were still broken, it would just keep being broken.
- String changes made/needed:
Comment on attachment 9056251 [details]
Bug 1538083 - Fix -0 handling in ARM64 visitTrunc(). r=nbp
OK for uplift for beta 12.
Comment 9•2 years ago
|
||
bugherderuplift |
Updated•2 years ago
|
Updated•2 years ago
|
Description
•