1791352 - Differential Testing: Different output message involving --ion-eager

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

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

function f(x, y, m) {
    let w = Math.log(y) && Math.min(Math.max(x / x, x), x);
    if (m) {
        print(w);
    }
}
let z = [1, 0, 2];
for (let i = 0; i < 9; i++) {
    f(1, z[i], false);
}
f(0, 1, false);
f(0, 0, false);
f(0, 2, true);

Output (rev 95d4708dc680, m-c latest default tip):

$ ./js --fuzzing-safe --differential-testing --no-threads --ion-eager testcase.js 
0

$ ./js --fuzzing-safe --differential-testing --no-threads --baseline-eager --no-ion testcase.js 
NaN

Output (rev 8b53f7a9dea2, parent of potential regressor):

$ ./js --fuzzing-safe --differential-testing --no-threads --ion-eager testcase.js
NaN

$ ./js --fuzzing-safe --differential-testing --no-threads --baseline-eager --no-ion testcase.js
NaN

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/bb791e77a175
user:        André Bargull
date:        Mon Aug 08 17:56:26 2022 +0000
summary:     Bug 1782771 - Part 5: Fold min(x, min(x, y)) and max(x, max(x, y)). r=jandem

Compile with AR=ar sh ./configure --enable-debug --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests, tested on m-c rev 95d4708dc680.

Setting s-s to be safe. Andre, is bug 1782771 a likely regressor?

Comment 1

[André Bargull [:anba]]

This is similar to bug 1688346, where an instruction is optimised out which can bail out. Adding setGuardRangeBailoutsUnchecked() fixes this bug.

Comment 2

[André Bargull [:anba]]

Attachment: Bug 1791352: Preserve bailouts when folding MMinMax. r=jandem!

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1782771

Comment 4

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

Preserve bailouts when folding MMinMax. r=jandem
https://hg.mozilla.org/integration/autoland/rev/5ad292b847e44f0f00e351a05a7a4c4935db703a
https://hg.mozilla.org/mozilla-central/rev/5ad292b847e4

Comment 5

[André Bargull [:anba]]

FWIW I don't think this is exploitable for the same reason bug 1688346 wasn't exploitable.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:anba, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox106 to wontfix.

For more information, please visit auto_nag documentation.

Comment 7

[André Bargull [:anba]]

Comment on attachment 9295235 [details]
Bug 1791352: Preserve bailouts when folding MMinMax. r=jandem!

Beta/Release Uplift Approval Request

• User impact if declined: Incorrect optimization leading to wrong results (0 vs NaN)
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• 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 change should be non-risky. It uses the same approach as in bug 1688346.
• String changes made/needed:
• Is Android affected?: Yes

Comment 8

[Pascal Chevrel:pascalc]

Comment on attachment 9295235 [details]
Bug 1791352: Preserve bailouts when folding MMinMax. r=jandem!

Approved for 106.0b3, thanks.

Comment 9

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/6893a4024f7d

Comment 10

[Camelia Badau [:cbadau], Desktop Test Engineering]

Hello, André!
Is manual QA testing needed here? Is something that QA should check manually here?
Thanks!

Comment 11

[André Bargull [:anba]]

(In reply to Camelia Badau [:cbadau], Release Desktop QA from comment #10)

Hello, André!
Is manual QA testing needed here? Is something that QA should check manually here?
Thanks!

Oh, I'm so sorry, manual QA testing isn't actually needed, the change has a test case, so it gets automatically checked. I didn't pay enough attention when filling out the form and apparently just clicked through the "Yes" buttons.