[Bug math/14273] New: cosh() returns incorrect results

[Bug math/14273] New: cosh() returns incorrect results

[pdemarco at maplesoft dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14273

             Bug #: 14273
           Summary: cosh() returns incorrect results
           Product: glibc
           Version: 2.15
            Status: NEW
          Severity: critical
          Priority: P2
         Component: math
        AssignedTo: unassigned@sourceware.org
        ReportedBy: pdemarco@maplesoft.com
    Classification: Unclassified


cosh() returns incorrect results in the most recent version of libm.  
Here is an example:

ubuntu64ma337>> cat test.c
#include <stdio.h>
#include <math.h>

int main(int argv, char *argc[])
        {
        double np = 709.889355812725967;
        printf("%g\n", cosh(709.889355812725967) );
        printf("%g\n", cosh(np) );
        double nm = -709.889355812725967;
        printf("%g\n", cosh(-709.889355812725967) );
        printf("%g\n", cosh(nm) );
        return 0;
        }
ubuntu64ma337>> gcc test.c -lm
ubuntu64ma337>> a.out
1e+308
1e+308
1e+308
inf


The last result is wrong.

I believe this is because of the following change made to glibc 2.15:

* Integrate libm with gcc's -ffinite-math-only option.
   Implemented by Ulrich Drepper.

Note that http://gcc.gnu.org/onlinedocs/gcc-3.3.6/g77/Optimize-Options.html has
this to say about the finite-math-only option:

-ffinite-math-only
Allow optimizations for floating-point arithmetic that assume that arguments
and results are not NaNs or +-Infs. 
This option should never be turned on by any -O option since it can result in
incorrect output for programs which depend on an exact implementation of IEEE
or ISO rules/specifications. 
The default is -fno-finite-math-only.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/14273] cosh() returns incorrect results

[jsm28 at gcc dot gnu.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=14273

Joseph Myers <jsm28 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |carlos_odonell at mentor
                   |                            |dot com

--- Comment #1 from Joseph Myers <jsm28 at gcc dot gnu.org> 2012-06-21 14:26:55 UTC ---
This appears to be specific to the wordsize-64 version of cosh; it's about the
64-bit optimizations, not -ffinite-math-only.  A comparison previously using
32-bit high parts and low parts, the former having been ANDed to make it the
high part of the absolute value, now uses the full 64-bit value - a simpler
comparison, but the ANDing is missing.

Fixing is simple enough.  Carlos, what do you want to do about this sort of
regression fix for release branches (2.15 and 2.16), given that testcases for
it are quite likely to involve ULPs updates for all architectures?

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/14273] cosh() returns incorrect results

[carlos_odonell at mentor dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14273

--- Comment #2 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-06-21 14:37:35 UTC ---
(In reply to comment #1)
> This appears to be specific to the wordsize-64 version of cosh; it's about the
> 64-bit optimizations, not -ffinite-math-only.  A comparison previously using
> 32-bit high parts and low parts, the former having been ANDed to make it the
> high part of the absolute value, now uses the full 64-bit value - a simpler
> comparison, but the ANDing is missing.
> 
> Fixing is simple enough.  Carlos, what do you want to do about this sort of
> regression fix for release branches (2.15 and 2.16), given that testcases for
> it are quite likely to involve ULPs updates for all architectures?

My initial reaction is that changes that require global architecture updates
are not appropriate for stable branches. I say this only because it is a huge
burden to place upon the patch creator to update all of the machine ULPs files.

Is that too conservative?

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/14273] cosh() returns incorrect results

[bugdal at aerifal dot cx]

http://sourceware.org/bugzilla/show_bug.cgi?id=14273

Rich Felker <bugdal at aerifal dot cx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugdal at aerifal dot cx

--- Comment #3 from Rich Felker <bugdal at aerifal dot cx> 2012-06-21 15:06:31 UTC ---
If the concern is about possibly introducing a new bug, I can see holding off
on this. But if it's just that fixing the bug imposes an excessive maintainer's
burden due to policy, then what you're effectively saying is that policy tells
you to make a buggy release even though you know the change would be safe... To
me, that seems like buggy policy.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/14273] cosh() returns incorrect results

[carlos_odonell at mentor dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14273

--- Comment #4 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-06-21 16:31:10 UTC ---
(In reply to comment #3)
> If the concern is about possibly introducing a new bug, I can see holding off
> on this. But if it's just that fixing the bug imposes an excessive maintainer's
> burden due to policy, then what you're effectively saying is that policy tells
> you to make a buggy release even though you know the change would be safe... To
> me, that seems like buggy policy.

The policy in this case is to keep the test results from regressing, which is
an important policy to have for a stable branch. Do we agree on that?

I would accept the patch, but such a patch would have to show it passes on all
machines that use this code (as opposed to the trunk criteria which would
probably be less strict), and update ULPs on all of those machines (more strict
than trunk again). It's a lot of work, but I guess if someone is willing to do
the work and show the test results, then I see no reason why not to backport
the patch.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/14273] cosh() returns incorrect results

[carlos_odonell at mentor dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14273

--- Comment #5 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-06-21 16:37:28 UTC ---
(In reply to comment #4)
> (In reply to comment #3)
> > If the concern is about possibly introducing a new bug, I can see holding off
> > on this. But if it's just that fixing the bug imposes an excessive maintainer's
> > burden due to policy, then what you're effectively saying is that policy tells
> > you to make a buggy release even though you know the change would be safe... To
> > me, that seems like buggy policy.
> 
> The policy in this case is to keep the test results from regressing, which is
> an important policy to have for a stable branch. Do we agree on that?
> 
> I would accept the patch, but such a patch would have to show it passes on all
> machines that use this code (as opposed to the trunk criteria which would
> probably be less strict), and update ULPs on all of those machines (more strict
> than trunk again). It's a lot of work, but I guess if someone is willing to do
> the work and show the test results, then I see no reason why not to backport
> the patch.

... or just fix the bug and leave out the testcase changes :-)

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/14273] cosh() returns incorrect results

[jsm28 at gcc dot gnu.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=14273

Joseph Myers <jsm28 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |glibc_2.15

--- Comment #6 from Joseph Myers <jsm28 at gcc dot gnu.org> 2012-06-21 19:29:56 UTC ---
Fixed (minus testcase) for 2.16 by:

commit b7abb4bf78443f4f8d05a9dfa768fdee65b99d42
Author: Joseph Myers <joseph@codesourcery.com>
Date:   Thu Jun 21 19:26:09 2012 +0000

    Fix wordsize-64 cosh regression (bug 14273).

I think the fix without testcase will also be appropriate for 2.15 branch
(subject to testing of a backport).

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/14273] cosh() returns incorrect results

[bugdal at aerifal dot cx]

http://sourceware.org/bugzilla/show_bug.cgi?id=14273

--- Comment #7 from Rich Felker <bugdal at aerifal dot cx> 2012-06-22 03:27:29 UTC ---
I agree that avoiding regressions in the test results is a useful policy, but
if that policy leads to the conclusion that known bugs (especially known
regression bugs that could be fixed just by reverting the regression) must be
kept in a stable release, the policy needs to be relaxed or bent.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/14273] cosh() returns incorrect results

[carlos_odonell at mentor dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14273

--- Comment #8 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-06-22 13:48:42 UTC ---
(In reply to comment #7)
> I agree that avoiding regressions in the test results is a useful policy, but
> if that policy leads to the conclusion that known bugs (especially known
> regression bugs that could be fixed just by reverting the regression) must be
> kept in a stable release, the policy needs to be relaxed or bent.

Right, and the "relaxation" is to checkin the fix but not the testcase. Thus
the risk is that 2.15 might regress and break cosh again, but hopefully it
doesn't because it's a stable branch and we aren't doing active development.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/14273] cosh() returns incorrect results

[jsm28 at gcc dot gnu.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=14273

--- Comment #9 from Joseph Myers <jsm28 at gcc dot gnu.org> 2012-06-26 18:55:39 UTC ---
Created attachment 6486
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6486
Patch backport to 2.15

Carlos, is this patch backport OK for 2.15?  Tested x86_64.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/14273] cosh() returns incorrect results

[carlos_odonell at mentor dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14273

--- Comment #10 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-06-28 02:25:05 UTC ---
(In reply to comment #9)
> Created attachment 6486 [details]
> Patch backport to 2.15
> 
> Carlos, is this patch backport OK for 2.15?  Tested x86_64.

Yes, this patch looks good. Please check this into 2.15.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/14273] cosh() returns incorrect results

[jsm28 at gcc dot gnu.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=14273

--- Comment #11 from Joseph Myers <jsm28 at gcc dot gnu.org> 2012-06-28 12:15:12 UTC ---
Fixed for 2.15 by:

commit 725b8ee08aff8f6a6edf3de90c5e7ef0ffcae2f8
Author: Joseph Myers <joseph@codesourcery.com>
Date:   Thu Jun 21 19:26:09 2012 +0000

    Fix wordsize-64 cosh regression (bug 14273).
    (cherry picked from commit b7abb4bf78443f4f8d05a9dfa768fdee65b99d42)

Will close this bug once 2.17 is open and I've checked in the testcase.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/14273] cosh() returns incorrect results

[jsm28 at gcc dot gnu.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=14273

Joseph Myers <jsm28 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #12 from Joseph Myers <jsm28 at gcc dot gnu.org> 2012-07-03 17:08:23 UTC ---
Testcases added for 2.17 by:

commit e2283f38febb8f1721f896680bed251ceef58a6b
Author: Joseph Myers <joseph@codesourcery.com>
Date:   Tue Jul 3 17:06:55 2012 +0000

    Add cosh tests for bug 14273.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/14273] cosh() returns incorrect results

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=14273

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.