Differential Testing: Different output message involving this

RESOLVED FIXED in mozilla35

Status

()

Core
JavaScript Engine: JIT
--
major
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gkw, Assigned: jandem)

Tracking

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

Trunk
mozilla35
x86_64
Mac OS X
regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
function f() {
    this.print("FOO")
}
x = new Array
this.f()
Object.defineProperty(x, 3, {
    get: function() {
        for (var j = 0; j < 1; ++j) {
            f()
        }
    }
})
try {
    x()
} catch (e) {}

$ ./js-dbg-opt-64-dm-nsprBuild-darwin-835ef55e175e --fuzzing-safe --no-threads --ion-eager testcase.js
FOO

$ ./js-dbg-opt-64-dm-nsprBuild-darwin-835ef55e175e --fuzzing-safe --no-threads --baseline-eager testcase.js
FOO
FOO

Tested this on m-c rev 835ef55e175e.

My configure flags are:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar 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 shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/1369bf46b89f
user:        Guptha Rajagopal
date:        Fri Aug 15 15:55:00 2014 -0400
summary:     Bug 924672 - Implement ES6 method definitions. r=jorendorff

Guptha, is bug 924672 likely related?
Flags: needinfo?(gupta.rajagopal)

Comment 1

3 years ago
Hmmm, this particular bug does not introduce changes to the jits themselves. So, I'm not sure how this would lead to differences in output between baseline and ion. It is possible this bug changes something that exposes an existing issue.

I'd investigate further, but I'm slammed with work right now. Let me leave the needinfo on - I will look into this when possible.

Thanks!
(Reporter)

Comment 2

3 years ago
(In reply to guptha from comment #1)
> Hmmm, this particular bug does not introduce changes to the jits themselves.
> So, I'm not sure how this would lead to differences in output between
> baseline and ion. It is possible this bug changes something that exposes an
> existing issue.
> 
> I'd investigate further, but I'm slammed with work right now.
/snip

Or maybe one of our JIT gurus can help you out. :)
Flags: needinfo?(jdemooij)
(Assignee)

Comment 3

3 years ago
Gary, can you try to run autoBisect with the testcase below? Same arguments as in comment 0?

I think I know what's happening but I'd like some confirmation that it's an older bug.

function f() {
    print(this);
}
this.f();
function gg() {
    for (var j = 0; j < 3; ++j) {
        f();
    }
};
gg();
(Assignee)

Updated

3 years ago
Flags: needinfo?(gary)
(Reporter)

Comment 4

3 years ago
Okay, will get to this bisection soon.
Summary: Differential Testing: Different output message involving this and defineProperty → Differential Testing: Different output message involving this
(Reporter)

Comment 5

3 years ago
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/39bfcadd6492
user:        Hannes Verschore
date:        Tue Nov 26 23:21:18 2013 +0100
summary:     Bug 942105 - IonMonkey: Remove the inlineUseCountRatio option, r=jandem

Hannes, is bug 942105 a likely regressor?
Blocks: 942105
No longer blocks: 924672
Flags: needinfo?(gary) → needinfo?(hv1989)
Back to jandem per IRC. The bug didn't introduce this issue. But I wasn't able yet to transform the testcase to fail before this revision, due to the interaction between different parts.
Flags: needinfo?(hv1989)
Flags: needinfo?(gupta.rajagopal)
(Assignee)

Comment 7

3 years ago
Created attachment 8501687 [details] [diff] [review]
Patch
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8501687 - Flags: review?(hv1989)
Flags: needinfo?(jdemooij)
Comment on attachment 8501687 [details] [diff] [review]
Patch

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

Oh this is a better fix than what I had in my mind!
Attachment #8501687 - Flags: review?(hv1989) → review+
https://hg.mozilla.org/mozilla-central/rev/d1780ae568a0
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.