Closed Bug 1101821 Opened 10 years ago Closed 9 years ago

Math.min re-bustage in Firefox Nightly

Categories

(Core :: JavaScript Engine: JIT, defect)

36 Branch
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox36 + fixed

People

(Reporter: grantgalitz, Assigned: h4writer)

References

()

Details

Attachments

(2 files)

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
Re-bustage time frame lines up with relanding of https://bugzilla.mozilla.org/show_bug.cgi?id=1042823
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.
Re-crafting the Math.min statement into two or more nested Math.min's will avoid triggering this bug.
(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.
Component: Untriaged → JavaScript Engine: JIT
Keywords: testcase-wanted
Product: Firefox → Core
[Tracking Requested - why for this release]:
Broken websites.

1) Open th
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.
Culprit as mentioned many times, is firefox nightly incorrectly handling a specific Math.min usage case.
Thank you for the "steps to reproduce". I can reproduce.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
Working on it ;)
Assignee: nobody → hv1989
Attached file test5.js
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
Tracking: we don't want to break some websites.
Attached patch FixSplinter Review
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 on attachment 8526816 [details] [diff] [review]
Fix

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

Nice test :)
Attachment #8526816 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/5c35947d1691
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.