1650526 - Issue involving negative zero and Math.pow

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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

[Jan de Mooij [:jandem]]

Thanks for reporting, Gary! I'll leave the analysis to anba.

Comment 2

[André Bargull [:anba]]

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.

Comment 3

[Julien Cristau [:jcristau]]

Not tracking based on comment 2.

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1650526: Preserve negative zero for MPow with int32 specialisation. r=jandem!

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().

Comment 5

[André Bargull [:anba]]

I'll request uplift, because the fix is easy and safe to do.

Comment 6

[Jan de Mooij [:jandem]]

(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

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/537d4a5a3bfd8dfa45416d3edcc88ba337c84c24

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/537d4a5a3bfd

Comment 9

[André Bargull [:anba]]

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

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9161556 [details]
Bug 1650526: Preserve negative zero for MPow with int32 specialisation. r=jandem!

Approved for 79.0b6.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/20ba4e8523b6