1527148 - Differential Testing: Different output message involving Array.prototype

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

function g(f) {
    var x = [];
    for (var k = 0; k < 2; ++k) {
        x.push(f());
    }
    print(x);
}
for (var i = 0; i < 1; i++) {
    f2 = function() {};
}
function f() {}
g(f);
Array.prototype.push = f2;
g(f);
f2.__proto__ = [];
g(f);

$ ./js-dbg-64-dm-linux-x86_64-b9187fa10f13 --fuzzing-safe --no-threads --ion-eager testcase.js
,

,
$ ./js-dbg-64-dm-linux-x86_64-b9187fa10f13 --fuzzing-safe --no-threads --baseline-eager --no-ion testcase.js
,

$

Tested this on m-c rev b9187fa10f13.

My configure flags are:

AR=ar sh ./configure --enable-debug --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests --disable-cranelift

python3 -u -m funfuzz.js.compile_shell -b "--enable-debug --enable-more-deterministic" -r b9187fa10f13

Running bisection soon...

Comment 1

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

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/954eeda43262
user: Jan de Mooij
date: Fri Apr 21 10:05:12 2017 +0200
summary: Bug 1357680 part 1 - Track Ion-inlined scripts explicitly so we can inline functions with unknown properties. r=bhackett

Jan, is bug 1357680 a likely regressor?

Setting s-s too because Ion-inlining bugs sound scary, pending Jan's analysis.

Comment 2

[Jan de Mooij [:jandem]]

I looked into this and will post a patch soon.

It's probably either sec-low/moderate or sec-nothing, but it's one of these "it's hard to be 100% confident about it so better safe than sorry" situations.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1527148 part 1 - Fix an Ion polymorphic inlining issue. r?bhackett!

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1527148 part 2 - Add test. r?bhackett!

Depends on D21540

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 9047380 [details]
Bug 1527148 part 1 - Fix an Ion polymorphic inlining issue. r?bhackett!

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Unclear. It might not even be a security bug but I'm treating it as sec-high just to be safe.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: All
• If not all supported branches, which bug introduced the flaw?: Bug 1357680
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: Should apply or be easy to apply.
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely.

Comment 6

[Al Billings [:abillings - ex-MoCo]]

sec-approval+ for trunk.
We'll want Beta and ESR60 patches nominated as well.

Comment 7

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/autoland/rev/5e31168aa777

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/5e31168aa777

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Please request Beta/ESR60 approval on this when you get a chance.

Comment 10

[Jan de Mooij [:jandem]]

Comment on attachment 9047380 [details]
Bug 1527148 part 1 - Fix an Ion polymorphic inlining issue. r?bhackett!

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1357680
• User impact if declined: Maybe security 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): Small patch. Disables an optimization in some cases.
• String changes made/needed: None

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9047380 [details]
Bug 1527148 part 1 - Fix an Ion polymorphic inlining issue. r?bhackett!

Disables an optimization in some cases to avoid a possible sec issue. Approved for 66.0b14 and 60.6esr.

Comment 12

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

https://hg.mozilla.org/releases/mozilla-beta/rev/5d767f7d3d48

Comment 13

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

https://hg.mozilla.org/releases/mozilla-esr60/rev/6dc8565d8cf0

Comment 14

[Jan de Mooij [:jandem]]

Test: https://hg.mozilla.org/integration/autoland/rev/4e26a433731f33d73050ac1cc12513eb398828cb

Comment 15

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

Test: https://hg.mozilla.org/mozilla-central/rev/4e26a433731f

Comment 16

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

Bugbug thinks this bug is a regression, but please revert this change in case of error.