Differential Testing: Different output message involving implicit calls to toString

RESOLVED FIXED in Firefox 48

Status

()

--
major
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gkw, Assigned: h4writer)

Tracking

(Blocks: 2 bugs, {regression, testcase})

Trunk
mozilla48
x86_64
All
regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 wontfix, firefox48 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
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?
Flags: needinfo?(winter2718)
(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.
Flags: needinfo?(winter2718)
Assignee: nobody → winter2718
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.
Assignee: winter2718 → nobody
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();
Flags: needinfo?(hv1989)
(Reporter)

Updated

3 years ago
Summary: Differential Testing: Different output message involving .sort → Differential Testing: Different output message involving .sup

Updated

3 years ago
Duplicate of this bug: 1247877

Updated

3 years ago
Summary: Differential Testing: Different output message involving .sup → Differential Testing: Different output message involving implicit calls to toString
(Assignee)

Comment 5

3 years ago
Created attachment 8736342 [details] [diff] [review]
Patch
Assignee: nobody → hv1989
Flags: needinfo?(hv1989)
Attachment #8736342 - Flags: review?(jdemooij)
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.
Attachment #8736342 - Flags: review?(jdemooij) → review+

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7f180a0e43ef
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48

Comment 9

3 years ago
Hannes assures me this is not worth uplifting. WONTFIX 47.
status-firefox47: affected → wontfix
You need to log in before you can comment on or make changes to this bug.