1671812 - [warp] Differential behavior with BigInt RangeError

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Christian Holler (:decoder)]

The following testcase produces differential behavior on mozilla-central revision 20201016-c0f6b814ea1d (debug build, run with --fuzzing-safe --ion-offthread-compile=off --baseline-eager):

function main() {
  let v13 = Math.sqrt(9007199254740992);
  v19 = new Float32Array(1337);
  for (let v26 = 0; v26 < 100; v26++)
    v27 = v19[v26];
  v32 = BigInt(v13);
}
main();

The test should throw as follows:

test.js:6:9 RangeError: 94906265.62425156 can't be converted to BigInt because it isn't an integer
Stack:
  [e-mail]:6:9
  @test.js:8:1

However, if you add --fast-warmup to the options, the error disappears. Found with fuzzilli + my differential testing patches.

Marking s-s until triaged.

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Jan de Mooij [:jandem]]

Good find. This is a bug with the float32 analysis and OSR. Doubles can be incorrectly converted to float32. I don't think it's security-sensitive, just a loss of precision.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1671812 - Don't specialize OSR phis as float32. r?iain!

In GuessPhiType we skip the phi operand that has the incoming OSR value, but we
also need to ensure we don't specialize the phi as float32 or we risk loss of
precision.

The float32 optimization is pretty complicated. Hopefully this can be factored out
and simplified the coming months (bug 1655773).

Comment 4

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/autoland/rev/0c34dcab98807ca775304955c39230962eb8bc5c

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 9182309 [details]
Bug 1671812 - Don't specialize OSR phis as float32. r?iain!

Beta/Release Uplift Approval Request

• User impact if declined: Correctness issues.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• 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): It disables an optimization (float32 specialization) in a certain case.
• String changes made/needed: None

Comment 6

[Jan de Mooij [:jandem]]

[Tracking Requested - why for this release]:

Comment 7

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/0c34dcab9880

Comment 8

[Pascal Chevrel:pascalc]

Comment on attachment 9182309 [details]
Bug 1671812 - Don't specialize OSR phis as float32. r?iain!

Approved for 83 beta 3.

Comment 9

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/releases/mozilla-beta/rev/4c4fae92b0cf