1246552 - Differential Testing: Different output message involving implicit calls to toString

Status: RESOLVED FIXED

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

Description

[Gary Kwong [:gkw] [:nth10sd]]

x = [];
y = x[13] = [];
Array.prototype.sort.apply(x, [function() {}]);
y.toString = (function() {
    print("foo");
});
x.forEach(String.prototype.sup, y);


$ ./js-dbg-64-dm-clang-darwin-76733110704b --fuzzing-safe --no-threads --baseline-eager testcase.js
foo
foo
foo
foo
foo
foo
foo
foo
foo
foo
foo
foo
foo
foo

$ ./js-dbg-64-dm-clang-darwin-76733110704b --fuzzing-safe --no-threads --ion-eager testcase.js
foo
foo


Tested this on m-c rev 76733110704b.

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-darwin14.5.0 --disable-jemalloc --enable-debug --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic" -r 76733110704b


autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/1c4b0a89fd5b
user:        Morgan Phillips
date:        Sun Jan 24 19:32:22 2016 -0600
summary:     Bug 715181 - Self-host Array.sort; r=till

Morgan, is bug 715181 a likely regressor?

Comment 1

[Morgan Phillips [:mrrrgn]]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #0)
> x = [];
> y = x[13] = [];
> Array.prototype.sort.apply(x, [function() {}]);
> y.toString = (function() {
>     print("foo");
> });
> x.forEach(String.prototype.sup, y);
> 
> 
> $ ./js-dbg-64-dm-clang-darwin-76733110704b --fuzzing-safe --no-threads
> --baseline-eager testcase.js
> foo
> foo
> foo
> foo
> foo
> foo
> foo
> foo
> foo
> foo
> foo
> foo
> foo
> foo
> 
> $ ./js-dbg-64-dm-clang-darwin-76733110704b --fuzzing-safe --no-threads
> --ion-eager testcase.js
> foo
> foo
> 
> 
> Tested this on m-c rev 76733110704b.
> 
> 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-darwin14.5.0 --disable-jemalloc --enable-debug
> --enable-more-deterministic --with-ccache --enable-gczeal
> --enable-debug-symbols --disable-tests
> 
> python -u ~/funfuzz/js/compileShell.py -b "--enable-debug
> --enable-more-deterministic" -r 76733110704b
> 
> 
> autoBisect shows this is probably related to the following changeset:
> 
> The first bad revision is:
> changeset:   https://hg.mozilla.org/mozilla-central/rev/1c4b0a89fd5b
> user:        Morgan Phillips
> date:        Sun Jan 24 19:32:22 2016 -0600
> summary:     Bug 715181 - Self-host Array.sort; r=till
> 
> Morgan, is bug 715181 a likely regressor?

It is, taking a look.

Comment 2

[Morgan Phillips [:mrrrgn]]

I harassed jandem about this thinking I'd caused ion to misbehave, but he ruled out Array.sort with another test: https://pastebin.mozilla.org/8858956 I think the root of this lies somewhere else.

Comment 3

[Jan de Mooij [:jandem]]

Below is an even simpler test.

I think the problem is that the self-hosted "sup" function calls the ToString intrinsic, Ion compiles that to MToString, and then DCE's the MToString. That's wrong because we have to bail out and call the toString function we installed.

MToString should probably be marked as guard if the input might be an object. Hannes can you take this? :)

var y = [];
y.toString = (function() { print("foo"); });
function test() {
    for (var i = 0; i < 14; i++) {
        String.prototype.sup.call(y);
    }
}
test();

Comment 5

[Hannes Verschore [:h4writer]]

Attachment: Patch

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 8736342 [details] [diff] [review]
Patch

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

Thanks! Can you add the test in comment 3? It should be easy to convert it to a jit-test by incrementing a global and then checking it at the end.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7f180a0e43ef

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7f180a0e43ef

Comment 9

[Eric Faust [:efaust]]

Hannes assures me this is not worth uplifting. WONTFIX 47.