Closed Bug 968522 Opened 10 years ago Closed 10 years ago

JS Math bug on FF27

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 941381

People

(Reporter: azakai, Unassigned)

References

()

Details

Attachments

(1 file)

http://alteredqualia.com/xg/examples/test_firefox27.html

shows a problem in FF27 (current release). However, it works in later versions, so this seems to have been fixed. Filing anyhow if perhaps it's worth verifying what the underlying issue was.
Cc'ing folks who may be able to id or even dup this. It's worrisome we shipped it -- this bug can help cover that testsuite gap.

/be
Float32Array being mentioned in the demo page is suggestive.
Flags: needinfo?(benj)
Is this fixed in beta 28?
I can't seem to check - when I download beta, I get 27, and when I download aurora, I get 29. Must be exactly as the versions are being switched?
Ah, right, we don't spin a beta for a bit.

There are 28 builds in ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-aurora/ right now
Ok, confirmed it works on 28. This is just a bug up to 27.
Perhaps the same underlying issue as bug 968566.
Just ran a regression range over it. This bug is caused by:
e1226725f674	Benjamin Bouvier — Bug 918163 - Specialize some Maths function calls for Float32 in Ion. r=sstangl

Looking at the bug, there is one depending bug:
Bug 941381 - IonMonkey: Incorrect fround(asin())

Target Milestone shows the fix landed in Firefox 28. But I see no request to uplift this patch!
Tracking flags also show status-firefox27: affected!

This is really a shame. The fix is a stupid typo. And there were tests added in FF28. Off course they weren't present in FF27.
> Tracking flags also show status-firefox27: affected!

Yes, but no one ever requested tracking.  Please please just request tracking on regressions on bugs that affect branches, so this won't happen!
Attached file Shell test case
I have run a manual bisection on the reduced shell case. Hannes is totally right for all points: the bug has been introduced in bug 918163 and solved in bug 941381. I have asked for an approval-release for the fix.
Flags: needinfo?(benj)
(In reply to Hannes Verschore [:h4writer] from comment #8)
> This is really a shame. The fix is a stupid typo. And there were tests added
> in FF28. Off course they weren't present in FF27.

I would have thought you'd always be running the current version of your correctness tests on all branches.
The current version of the tests would obviously fail on any old branch, since we've fixed some things since then....
(In reply to Boris Zbarsky [:bz] from comment #12)
> The current version of the tests would obviously fail on any old branch,
> since we've fixed some things since then....

So? Just keep a list of which tests you *know* should fail because you fixed them, then compare that to the list that actually do fail so you can find ones you've missed. (note that I'm only suggesting this for correctness tests, not changes to APIs)
Patches accepted to make that work...
Depends on: 941381
Dave: ideally we would do that. Realistically it will take some work. Cc'ing Taras and Clint in case they can add this to a todo list, or suggest someone else who can.

/be
For the record, the patch from bug 941381 has been uplifted in mozilla-release, so this is in 27.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: