1779867 - Differential Testing: Different output message involving branch pruning and --fast-warmup

Status: RESOLVED FIXED

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

Description

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

try {
  (function () {
    "use strict";
    Object.defineProperty(this, "w", {});
  })();
} catch (e) {}
let x;
let y;
let z;
let i = 0;
while (i < 9) {
  z = [x, y].map(Array.prototype.concat, "");
  i++;
}
print(z);

Output (rev 315e476ecf38, m-c latest default tip):

$ ./js --fuzzing-safe --ion-offthread-compile=off --differential-testing --ion-eager testcase.js
,,0,,,,,1,,

$ ./js --fuzzing-safe --ion-offthread-compile=off --differential-testing --fast-warmup testcase.js
,,-1,,,,,-1,,

Output (rev 084ceb68d16d, parent of potential regressor):

$ ./js --fuzzing-safe --ion-offthread-compile=off --differential-testing --ion-pgo=on --ion-eager testcase.js
,,0,,,,,1,,

$ ./js --fuzzing-safe --ion-offthread-compile=off --differential-testing --ion-pgo=on --fast-warmup testcase.js
,,0,,,,,1,,

Output (rev a36bee113e48, potential regressor):

$ ./js --fuzzing-safe --ion-offthread-compile=off --differential-testing --ion-pgo=on --ion-eager testcase.js
,,0,,,,,1,,

$ ./js --fuzzing-safe --ion-offthread-compile=off --differential-testing --ion-pgo=on --fast-warmup testcase.js
,,-1,,,,,-1,,

At first branch pruning was pointed to:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/a14b2864e74d
user:        Iain Ireland
date:        Mon Apr 05 16:55:05 2021 +0000
summary:     Bug 1702306: Enable branch pruning by default r=jandem

However, I went back further, with --ion-pgo=on near this potential regressor:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/a36bee113e48
user:        Iain Ireland
date:        Thu Mar 25 16:19:32 2021 +0000
summary:     Bug 1697696: Prune branches based on unconditional bailouts r=nbp

--ion-pgo=on was changed to --ion-pruning=on in this changeset ee10e8481d49 and then branch pruning was turned on by default in changeset a14b2864e74d.

Compile with AR=ar sh ./configure --enable-simulator=arm64 --enable-debug --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests, tested on m-c rev 315e476ecf38. This also seems to happen on native ARM64 platforms, e.g. macOS.

Setting s-s to be safe as a start as this involves a JIT flag (--fast-warmup) that is present in fuzz-flags.txt.

Comment 1

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

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

Comment 2

[Nicolas B. Pierron [:nbp]]

I feel like Iain will soon submit a patch.
I am going to assign this bug to him.

Comment 3

[Iain Ireland [:iain]]

Attachment: Bug 1779867: Don't reuse input register for GetInlinedArgument on ARM64 r=jandem

We fixed this problem in bug 1697816 for x64 and MIPS. At the time I thought it didn't affect ARM64, but it turns out that in debug builds, this code will clobber the source value if src == dest.

I'm not adding the testcase because it's very fragile: eg removing an unrelated call to Object.defineProperty affects how WarpOracle handles GetIntrinsic ICs, which in turn affects register allocation enough to prevent the bug.

Comment 4

[Iain Ireland [:iain]]

This problem only occurs in debug builds, so it's not a security concern.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/00bf58475b34
Don't reuse input register for GetInlinedArgument on ARM64 r=jandem

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/00bf58475b34