Closed Bug 1081850 Opened 6 years ago Closed 6 years ago

Differential Testing: Different output message involving prototype

Categories

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

x86_64
macOS
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox34 --- fixed
firefox35 --- fixed
firefox36 --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: gkw, Assigned: djvj)

References

Details

(Keywords: regression, testcase)

Attachments

(1 file)

try {
    function f() {
        print(eval.prototype)
    }
    f()
    function eval()(0)
    f()
} catch (e) {}

$ ./js-dbg-opt-64-dm-nsprBuild-darwin-f547cf19d104 --fuzzing-safe --no-threads --ion-eager testcase.js
undefined
undefined

$ ./js-dbg-opt-64-dm-nsprBuild-darwin-f547cf19d104 --fuzzing-safe --no-threads --no-baseline --no-ion testcase.js
undefined
[object Object]

Tested this on m-c rev f547cf19d104.

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

This seems to go back beyond e8558ecd9b16.

Jan, any idea what's up here?
Flags: needinfo?(jdemooij)
Looks like a Baseline regression from bug 928928. It works if I make TryAttachNativeDoesNotExistStub return early.
Flags: needinfo?(jdemooij) → needinfo?(kvijayan)
Blocks: 928928
Assignee: nobody → kvijayan
Flags: needinfo?(kvijayan)
Was not checking for class resolve hooks when looking for no-such-property operations to optimize.
Attachment #8520884 - Flags: review?(jdemooij)
Comment on attachment 8520884 [details] [diff] [review]
bug-1081850.patch

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

Looks good; please add a testcase like the one Gary posted in comment 0 that fails without the patch.
Attachment #8520884 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/e3c785a79ee9
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment on attachment 8520884 [details] [diff] [review]
bug-1081850.patch

Approval Request Comment
[Feature/regressing bug #]:
Bug 928928.

[User impact if declined]:
Wrong JS execution semantics in the wild.

[Describe test coverage new/current, TBPL]:
Added test to cover case (see commit e3c785a79ee9)

[Risks and why]:
Low risk.  Disables baseline-jit optimization of particular cases, adds no other new logic.

[String/UUID change made/needed]:
None.
Attachment #8520884 - Flags: approval-mozilla-aurora?
Comment on attachment 8520884 [details] [diff] [review]
bug-1081850.patch

The offending patch landed in June 11.  The bug has made it into beta.

[Feature/regressing bug #]:
Bug 928928.

[User impact if declined]:
Wrong JS execution semantics in the wild.

[Describe test coverage new/current, TBPL]:
Added test to cover case (see commit e3c785a79ee9)

[Risks and why]:
Low risk.  Disables baseline-jit optimization of particular cases, adds no other new logic.

[String/UUID change made/needed]:
None.
Attachment #8520884 - Flags: approval-mozilla-beta?
(In reply to Kannan Vijayan [:djvj] from comment #7)
> Comment on attachment 8520884 [details] [diff] [review]
> bug-1081850.patch
> 
> The offending patch landed in June 11.  The bug has made it into beta.

According to bug 928928, this bug shipped in Firefox 33.

> [User impact if declined]:
> Wrong JS execution semantics in the wild.

Is this visible to the user? If so, how does this bug impact the user and do you have a concrete error case?
Flags: needinfo?(kvijayan)
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #8)
> (In reply to Kannan Vijayan [:djvj] from comment #7)
> > Comment on attachment 8520884 [details] [diff] [review]
> > bug-1081850.patch
> > 
> > The offending patch landed in June 11.  The bug has made it into beta.
> 
> According to bug 928928, this bug shipped in Firefox 33.

Should I be asking for uplift to release?
 
> > [User impact if declined]:
> > Wrong JS execution semantics in the wild.
> 
> Is this visible to the user? If so, how does this bug impact the user and do
> you have a concrete error case?

It seems that bug 1092204 is a dup of this one - that's a bug reporting broken pages on a private site.  I'm waiting for confirmation from reporter that it has been fixed before marking as a dup, but I suspect it'll come back positive.

It seems serious enough to uplift to beta at least.
Flags: needinfo?(kvijayan)
Comment on attachment 8520884 [details] [diff] [review]
bug-1081850.patch

(In reply to Kannan Vijayan [:djvj] from comment #9)
> (In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #8)
> > (In reply to Kannan Vijayan [:djvj] from comment #7)
> > > Comment on attachment 8520884 [details] [diff] [review]
> > > bug-1081850.patch
> > > 
> > > The offending patch landed in June 11.  The bug has made it into beta.
> > 
> > According to bug 928928, this bug shipped in Firefox 33.
> 
> Should I be asking for uplift to release?

No. Given how close we are to 34 I don't think this bug warrants a point release.

> > > [User impact if declined]:
> > > Wrong JS execution semantics in the wild.
> > 
> > Is this visible to the user? If so, how does this bug impact the user and do
> > you have a concrete error case?
> 
> It seems that bug 1092204 is a dup of this one - that's a bug reporting
> broken pages on a private site.  I'm waiting for confirmation from reporter
> that it has been fixed before marking as a dup, but I suspect it'll come
> back positive.
> 
> It seems serious enough to uplift to beta at least.

Thank you for the bug reference. I agree. Beta+

Can you please land this on Beta yourself as I think we don't have sheriff coverage right now and really need the fix in before 2pm PT.
Flags: needinfo?(kvijayan)
Attachment #8520884 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8520884 [details] [diff] [review]
bug-1081850.patch

Aurora+
Attachment #8520884 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #10)
> Thank you for the bug reference. I agree. Beta+
> 
> Can you please land this on Beta yourself as I think we don't have sheriff
> coverage right now and really need the fix in before 2pm PT.

Landing now.  Haven't landed on beta before so I need to pull the repos first.
Duplicate of this bug: 1092204
You need to log in before you can comment on or make changes to this bug.