1123011 - Differential Testing: Different output message involving "use strict"

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

count = 0
try {
    function f() {
        eval("\"use strict\";this.b=(function(){})")
    }
    for (var j = 0; j < 9; ++j) {
        count += 1
        print("count = " + count + ": " + f())
    }
} catch (e) {}

$ ./js-dbg-opt-64-dm-nsprBuild-darwin-369a8f14ccf8 --fuzzing-safe --no-threads --ion-eager testcase.js
count = 1: undefined
count = 2: undefined
count = 3: undefined
count = 4: undefined
count = 5: undefined
count = 6: undefined
count = 7: undefined
count = 8: undefined

$ ./js-dbg-opt-64-dm-nsprBuild-darwin-369a8f14ccf8 --fuzzing-safe --no-threads --baseline-eager testcase.js
count = 1: undefined
count = 2: undefined
count = 3: undefined
count = 4: undefined
count = 5: undefined
count = 6: undefined
count = 7: undefined
count = 8: undefined
count = 9: undefined

Tested this on m-c rev 369a8f14ccf8.

My configure flags are:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

autoBisect is running.

Comment 1

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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/95e326d4fd7d
parent:      214107:e066f112521a
user:        Hannes Verschore
date:        Wed Nov 05 16:46:21 2014 +0100
summary:     Bug 914255 - Reduce the number of objects tracked in a TypeSet, r=bhackett

Hannes, is bug 914255 a likely regressor?

Comment 2

[Hannes Verschore [:h4writer]]

I'm pretty sure that if you try the following it will fail before the revision you are proposing.
(I'm not on my own computer, so I haven't checked if the '40' is high enough. But I think we track up to 32 objects before this revision. So 40 should do it.)


count = 0
try {
    function f() {
        eval("\"use strict\";this.b=(function(){})")
    }
    for (var j = 0; j < 40; ++j) {
        count += 1
        print("count = " + count + ": " + f())
    }
} catch (e) {}

Comment 3

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

You're right. It seems to go back before end-2013 [1]. Anyone able to look at this in any case?

[1] https://hg.mozilla.org/mozilla-central/rev/df3c2a1e86d3

Comment 4

[Jan de Mooij [:jandem]]

Simpler testcase:

var global = this;
function f() {
    return eval("'use strict'; this;");
}
for (var j = 0; j < 5; ++j) {
    assertEq(f(), global);
}

Comment 5

[Jan de Mooij [:jandem]]

This probably goes back to bug 743394. The problem is that we need to pass the correct, boxed |this| to eval scripts, because in strict mode the JSOP_THIS there will just return what's in the this-slot, without boxing it.

Brian, should we check if the eval string contains |use strict|, in addition to "arguments"/"eval" we check now? Can you think of a better fix?

Comment 6

[Brian Hackett [Laid off!]]

Attachment: patch

I don't think it's necessary to avoid executing the eval at all if it contains 'use strict', as in this case the strictness just affects the 'this' value we compute, and it's simple to do that in DirectEvalStringFromIon.  This patch does that, and also avoids optimizing 'eval' in arrow functions, as it looks like we weren't handling 'this' computation there correctly.

Comment 7

[Jan de Mooij [:jandem]]

Comment on attachment 8552722 [details] [diff] [review]
patch

Review of attachment 8552722 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.

Comment 8

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4d669523c7dd

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/4d669523c7dd