Closed
Bug 1101821
Opened 10 years ago
Closed 9 years ago
Math.min re-bustage in Firefox Nightly
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: grantgalitz, Assigned: h4writer)
References
()
Details
Attachments
(2 files)
4.89 KB,
application/javascript
|
Details | |
3.36 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:36.0) Gecko/20100101 Firefox/36.0 Build ID: 20141118144012 Steps to reproduce: Exactly the same as https://bugzilla.mozilla.org/show_bug.cgi?id=1054169 Actual results: The bug reappearing again after a patch got relanded in firefox. Expected results: No fuzzy logic in math.min
Reporter | ||
Comment 1•10 years ago
|
||
Re-bustage time frame lines up with relanding of https://bugzilla.mozilla.org/show_bug.cgi?id=1042823
Reporter | ||
Comment 2•10 years ago
|
||
I'm wondering how much the surrounding code affects the triggering of the bug via making tricky optimization and bailout patterns. The bug only happens in ionmonkey optimized code. The unoptimized form is not affected. The troublesome line of code is the same as in the previous report. It's a Math.min with more than a specific number of integer arguments.
Reporter | ||
Comment 3•10 years ago
|
||
Re-crafting the Math.min statement into two or more nested Math.min's will avoid triggering this bug.
Comment 4•10 years ago
|
||
(In reply to Grant Galitz from comment #0) > Steps to reproduce: Exactly the same as > https://bugzilla.mozilla.org/show_bug.cgi?id=1054169 FWIW, these are: (In reply to Grant Galitz from comment #0) > User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:34.0) > Gecko/20100101 Firefox/34.0 (Beta/Release) > Build ID: 20140814005302 > > Steps to reproduce: > > Ran either gameboy-online or Iodine GBA with audio enabled. > > > Actual results: > > The white noise channel 4 audio was glitched and was usually silent when it > should be playing. A counter in the code went negative when that should have > been impossible. > > > Expected results: > > Correct counter logic so the channel would play correctly. It would still be best if we had a reduced testcase of sorts.
Comment 5•10 years ago
|
||
[Tracking Requested - why for this release]: Broken websites.
Reporter | ||
Comment 6•10 years ago
|
||
1) Open th
Reporter | ||
Comment 7•10 years ago
|
||
Woops, comment got cut off. 1) Open the url. 2) once the game boots, and you get past the intro of mario kart, get to the main menu. Main menu being the "select a game screen" 3) There should be a white noise audio effect audible. There is in Firefox Stable... It is lacking in nightly.
Reporter | ||
Comment 8•10 years ago
|
||
Culprit as mentioned many times, is firefox nightly incorrectly handling a specific Math.min usage case.
Assignee | ||
Comment 9•10 years ago
|
||
Thank you for the "steps to reproduce". I can reproduce.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 11•10 years ago
|
||
I'm seeing something similar with five-argument Math.max [sic] on a mix of positive and negative numbers (should all be int32 values): it will sometimes (rarely, unpredictably) pick the first positive number, not the largest one. This is on current mozilla-inbound.
Assignee | ||
Comment 13•10 years ago
|
||
Reducer gave me this. Expected result: test5.js:70:12 ReferenceError: speed is not defined Actual result: test5.js:126:6 ReferenceError: document is not defined
Assignee | ||
Comment 15•10 years ago
|
||
So apparently the fuzzers didn't find this issue because there is no min/max with more than 2 operations. I sure this is not the case anymore ;). Though stupid bug. I even think I saw this before landing and updated it. Seems not.
Attachment #8526816 -
Flags: review?(jdemooij)
Comment 16•10 years ago
|
||
Comment on attachment 8526816 [details] [diff] [review] Fix Review of attachment 8526816 [details] [diff] [review]: ----------------------------------------------------------------- Nice test :)
Attachment #8526816 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 17•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c35947d1691
Comment 18•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5c35947d1691
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Updated•9 years ago
|
Keywords: testcase-wanted
You need to log in
before you can comment on or make changes to this bug.
Description
•