968522 - JS Math bug on FF27

Status: RESOLVED DUPLICATE of bug 941381

(Core :: JavaScript Engine, defect)

Description

[Alon Zakai (:azakai)]

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.

Comment 1

[Brendan Eich [:brendan]]

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

Comment 2

[Jeff Walden [:Waldo]]

Float32Array being mentioned in the demo page is suggestive.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Is this fixed in beta 28?

Comment 4

[Alon Zakai (:azakai)]

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?

Comment 5

[Boris Zbarsky [:bzbarsky]]

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

Comment 6

[Alon Zakai (:azakai)]

Ok, confirmed it works on 28. This is just a bug up to 27.

Comment 7

[Luke Wagner [:luke]]

Perhaps the same underlying issue as bug 968566.

Comment 8

[Hannes Verschore [:h4writer]]

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.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> 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!

Comment 10

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: 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.

Comment 11

[Dave Garrett]

(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.

Comment 12

[Boris Zbarsky [:bzbarsky]]

The current version of the tests would obviously fail on any old branch, since we've fixed some things since then....

Comment 13

[Dave Garrett]

(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)

Comment 14

[Boris Zbarsky [:bzbarsky]]

Patches accepted to make that work...

Comment 15

[Brendan Eich [:brendan]]

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

Comment 16

[Benjamin Bouvier [:bbouvier] (inactive)]

For the record, the patch from bug 941381 has been uplifted in mozilla-release, so this is in 27.