Issue involving negative zero and Math.pow
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr68 | --- | unaffected |
firefox-esr78 | --- | unaffected |
firefox77 | --- | unaffected |
firefox78 | --- | unaffected |
firefox79 | - | fixed |
firefox80 | - | fixed |
People
(Reporter: gkw, Assigned: anba)
References
(Regression)
Details
(Keywords: regression, testcase)
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
|
Details |
let x = [];
let y = [2147483648, 2147483649];
for (let i = 0; i < 2; ++i) {
x.push(Math.pow(-2147483648, -y[i]));
}
assertEq(uneval(x[1]), "-0");
AR=ar sh ./configure --enable-debug --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests
Assertion fails on latest m-c rev efa2336315eda0aaf78c2e94d6c9c98106ea136b with --fuzzing-safe --no-threads --ion-eager
, using clang 9.0.1, passes when run with --fuzzing-safe --no-threads --baseline-eager --no-ion
. Also passes when run on rev 5b9343de266b3a4b7fa585345027b6589ea4614f, the parent of the potential regressor on both sets of flags.
$ ./js-dbg-64-dm-linux-x86_64-efa2336315ed --fuzzing-safe --no-threads --ion-eager testcase-assertEq.js
testcase-assertEq.js:18:9 Error: Assertion failed: got "0", expected "-0"
Stack:
@testcase-assertEq.js:18:0
The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/bfc836af8677
user: André Bargull
date: Wed Jun 10 11:51:58 2020 +0000
summary: Bug 1564942 - Part 2: Avoid negative zero check when the base operand in MPow is an Int32. r=jandem
Unsure how bad this is, so marking s-s and setting needinfo? from :anba and :jandem. Please open up if this is not the case.
Comment 1•4 years ago
|
||
Thanks for reporting, Gary! I'll leave the analysis to anba.
Assignee | ||
Comment 2•4 years ago
|
||
I don't think this can be exploited in any way, because we're only losing the sign, but all other parts around MPow
and MToNumberInt32
are still working correctly. And in the past we also didn't hide +0/-0 bugs, so it doesn't look like we're aware of any issues which can make this exploitable: bug 1308802, bug 1312620, bug 1314438.
Assignee | ||
Updated•4 years ago
|
Comment 3•4 years ago
|
||
Not tracking based on comment 2.
Assignee | ||
Comment 4•4 years ago
|
||
Removes the incorrect optimisation added in D37584 for IonBuilder and keeps it
only in MPow::foldsTo()
when optimising the cases when power ∈ {2,3,4}.
Also changes MPow::canBeNegativeZero()
's access modifier to private, now that
it's only used within MPow::foldsTo()
.
Assignee | ||
Comment 5•4 years ago
|
||
I'll request uplift, because the fix is easy and safe to do.
Comment 6•4 years ago
|
||
(In reply to Julien Cristau [:jcristau] from comment #3)
Not tracking based on comment 2.
Note that it's still a correctness bug though. Not a very interesting one but I thought we still wanted to track those.
Comment 7•4 years ago
|
||
Comment 8•4 years ago
|
||
Assignee | ||
Comment 9•4 years ago
|
||
Comment on attachment 9161556 [details]
Bug 1650526: Preserve negative zero for MPow with int32 specialisation. r=jandem!
Beta/Release Uplift Approval Request
- User impact if declined: The
Math.pow
built-in can return positive zero (+0
) instead of negative zero (-0
) in certain cases. - 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): Not risky because it only removes an incorrect optimisation added in bug 1564942, so we're back to what we had before bug 1564942.
- String changes made/needed:
Comment 10•4 years ago
|
||
Comment on attachment 9161556 [details]
Bug 1650526: Preserve negative zero for MPow with int32 specialisation. r=jandem!
Approved for 79.0b6.
Comment 11•4 years ago
|
||
Updated•4 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Description
•