Bug 29302 - isfinite returns wrong result at -O1
Summary: isfinite returns wrong result at -O1
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.2.0
: P3 normal
Target Milestone: 4.2.0
Assignee: Eric Christopher
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2006-09-30 20:45 UTC by Jack Howarth
Modified: 2007-01-21 07:00 UTC (History)
1 user (show)

See Also:
Host: powerpc-apple-darwin8
Target: powerpc-apple-darwin8
Build: powerpc-apple-darwin8
Known to work: 4.2.0 4.3.0
Known to fail: 4.0.1
Last reconfirmed: 2006-11-08 00:06:59


Attachments
assembly file for nan_inf_fmt that seqfaults on Darwin PPC with Xcode 2.4 (2.43 KB, text/plain)
2006-09-30 20:46 UTC, Jack Howarth
Details
preprocessed file for libgfortran/io/write.c on Darwin PPC (25.43 KB, text/plain)
2006-11-01 05:10 UTC, Jack Howarth
Details
assembly file from libgfortran/io/write.c on Darwin PPC (21.57 KB, text/plain)
2006-11-01 06:06 UTC, Jack Howarth
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jack Howarth 2006-09-30 20:45:23 UTC
With the Xcode 2.4 release, the nan_inf_fmt.f90 testcase in  gfortran.fortran-torture/execute
has begun to segfault on Darwin PPC at both -m32 and -m64. The backtrace from a segfault
is...

Starting program: /Users/howarth/testdir/nan_inf_fmt/a.out 
Reading symbols for shared libraries .++ done

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x3fffe615
0x90101fc8 in strtol_l ()
(gdb) bt
#0  0x90101fc8 in strtol_l ()
#1  0x00255810 in output_float (dtp=0xbfffeb30, f=0x1808c50, value=inf) at ../../../gcc-4.2-20060928/libgfortran/io/write.c:496
#2  0x00256124 in write_float (dtp=0xbfffeb30, f=0x1808c50, source=0xa <Address 0xa out of bounds>, len=-1610603800) at ../../../gcc-4.2-20060928/libgfortran/io/write.c:912
#3  0x002507ec in formatted_transfer (dtp=0xbfffeb30, type=BT_REAL, p=0xa, kind=4, size=4, nelems=1) at ../../../gcc-4.2-20060928/libgfortran/io/transfer.c:954
#4  0x0024eac8 in *__gfortran_transfer_real (dtp=0x250314, p=0xbfffeaf8, kind=4) at ../../../gcc-4.2-20060928/libgfortran/io/transfer.c:1237
#5  0x00002a04 in MAIN__ () at nan_inf_fmt.f90:17
#6  0x00003af0 in main (argc=1073735189, argv=0x0) at ../../../gcc-4.2-20060928/libgfortran/fmain.c:18

It has been suggested on the list that this might be due to size mismatch and that
the new Xcode 2.4 is exposing a latent bug in libgfortran...

http://gcc.gnu.org/ml/fortran/2006-09/msg00103.html
Comment 1 Jack Howarth 2006-09-30 20:46:40 UTC
Created attachment 12362 [details]
assembly file for nan_inf_fmt that seqfaults on Darwin PPC with Xcode 2.4
Comment 2 Jerry DeLisle 2006-09-30 21:09:13 UTC
I will see if I can spot something here.  Jack, I will need your help testing.
Comment 3 Jerry DeLisle 2006-09-30 21:36:35 UTC
Valgrind shows tight as a drum on i686-linux.  Do you have something equivalent to valgrind for Darwin PPC?

I am not seeing anything obvious in the code. On i686 the call at line 496 is to atoi().  Your system is showing a strtol_l.  These are slightly different and maybe atoi() is mapped to strtol_l on this archtecture.  I don't know.
Comment 4 Jerry DeLisle 2006-09-30 21:53:27 UTC
Jack, I would forward this one to Apple.
Comment 5 Jack Howarth 2006-09-30 22:50:23 UTC
Jerry,
     I've run this with guardmalloc...

http://developer.apple.com/documentation/Darwin/Reference/ManPages/man3/libgmalloc.3.html

using all the permutations and nothing gets triggered.
               Jack
ps It seems the crashes are specific to those lines that have...

       write(l,fmt=fmt)pos_inf

or

       write(l,fmt=fmt)neg_inf

The lines with...

       write(l,fmt=fmt)nan

never cause a segfault so a minimal failing testcase is...

       implicit none
       character*40 l
       character*12 fmt
       real zero, pos_inf
       zero = 0.0

       pos_inf =  1.0/zero
       fmt = '(F0.0)'
       write(l,fmt=fmt)pos_inf
       end
Comment 6 kargls 2006-10-01 00:40:07 UTC
(In reply to comment #5)
> Jerry,
>      I've run this with guardmalloc...
>
> 

Jack, This is an Apple library problem.  Please report the
inappropriate use of strtol_l to Apple.


Comment 7 Geoff Keating 2006-10-04 23:57:50 UTC
Probably atoi() tail-calls strtol_l.
Comment 8 Jack Howarth 2006-10-05 02:03:53 UTC
Geoff,
     Can you expand on this? Assuming it is a atoi() tail-calls strtol_l
why is Darwin the only arch effected and what recourse do we have
in fixing this? I tried a brute force replacement of the atoi() with a strl()
in libgfortran/io/write.c and nan_inf_fmt still segfaults...

  /* Read the exponent back in.  */
  /* e = atoi (&buffer[ndigits + 3]) + 1; */
  e = strtol(&buffer[ndigits + 3], (char **)NULL, 10);
  e = e +1;
Comment 9 Jack Howarth 2006-10-06 01:32:00 UTC
Geoff,
     This bug is also radar://4759173. I've been asked to...

Please debug and produce a small self-contained (that is, not including gfortran) testcase showing what goes wrong.

Could you expand in that radar report on what you suspect is
causing this?
            Jack
Comment 10 Geoff Keating 2006-10-31 01:02:40 UTC
Hi Jack,

I have no idea what's causing this, and I don't know why you think I do.  I am the one asking you to provide a testcase in the Radar.  You're claiming that this is a bug in Xcode 2.4 (which is supported by Apple), and not a bug in gfortran (which is not supported by Apple).  I am asking for a testcase which removes the non-supported part and shows the bug.

My comment about strtol_l is because if atoi() tail-calls strtol_l, possibly after changing parameters, then you would see strtol_l in the backtrace; that would be normal and not a bug or a symptom of a bug.
Comment 11 Geoff Keating 2006-10-31 01:03:42 UTC
Please do not add 'geoffk@apple.com' to the CC list of any bugzilla bug.
Comment 12 Jerry DeLisle 2006-10-31 03:18:46 UTC
Geoff,

I am trying to understand what you are saying.

I could extract a segment of the C code that is being used implement here so that you can see why it is failing.

I think it is because Apple code is returning a value that is long-long when what is expected is int so we are overwriting a value.  Maybe we need a cast in there.

Its been a while since I looked at this, but I will do so and try to send an example, unless I figure out a fix in gfortran first.  :)
Comment 13 Jack Howarth 2006-10-31 14:14:37 UTC
As I mentioned to Jerry in a private email, his suggestion of...

Index: io/write.c
===================================================================
--- io/write.c  (revision 118229)
+++ io/write.c  (working copy)
@@ -468,6 +468,7 @@ output_float (st_parameter_dt *dtp, cons
   int i;
   sign_t sign;
   double abslog;
+  long tmp_e;
 
   ft = f->format;
   w = f->u.real.w;
@@ -554,7 +555,8 @@ output_float (st_parameter_dt *dtp, cons
       internal_error (&dtp->common, "printf is broken");
 
   /* Read the exponent back in.  */
-  e = atoi (&buffer[ndigits + 3]) + 1;
+  tmp_e = atoi (&buffer[ndigits + 3]) + 1;
+  e = (int) tmp-e;
 
   /* Make sure zero comes out as 0.0e0.  */
   if (value == 0.0)

eliminates the failure in the nan_inf_fmt.f90 testcase on Darwin PPC at all optimizations as well as -m64. The failure is also suppressed by the simple placement of...

st_printf("buffer=%s", buffer);

in front of the existing call to atoi().
Comment 14 Jerry DeLisle 2006-10-31 23:24:18 UTC
Jack,

I don't think this is the final fix.  However, after you test that other possibility I sent you I think I can finalize a patch.  If zero terminating buffer does not solve the problem, then I think it confirms the real problem and this is just a hack around.

Comment 15 Jack Howarth 2006-11-01 04:33:03 UTC
   I have pinned down the problem. On Darwin PPC with Xcode 2.4, at -O1 or higher we have problems with the isfinite() macro from libgfortran.h. For the nan_inf_fmt testcase it should always return 0, but
it doesn't at -O1 or higher. I think we are seeing the same glitch that was fixed in python for gcc 4.2.
The current macro looks like...

#if !defined(isfinite)
#if !defined(fpclassify)
#define isfinite(x) ((x) - (x) == 0)
#else
#define isfinite(x) (fpclassify(x) != FP_NAN && fpclassify(x) != FP_INFINITE)
#endif /* !defined(fpclassify) */
#endif /* !defined(isfinite)  */

I think it we need to cast both x variables in the macro to unsigned long.
Comment 16 Andrew Pinski 2006-11-01 04:48:25 UTC
(In reply to comment #15)
> I think it we need to cast both x variables in the macro to unsigned long.

No, that would be incorrect.
Jack, can you attach the preprocessed source for write.c?
Comment 17 Jerry DeLisle 2006-11-01 04:56:08 UTC
Jack is right about one thing and I should have noticed this.  For that test case we should never get into output_float where we are segfaulting.  The handling of nan and inf are handled in the calling function write_float.  So optimization is breaking something.
Comment 18 Jack Howarth 2006-11-01 05:05:24 UTC
Andrew,
     I'll post the preprocessed source for write.c shortly. I did a quick test though moving...

#define isfinite(x) (fpclassify(x) != FP_NAN && fpclassify(x) != FP_INFINITE)

outside of the preprocessor statements so it is declared in libgfortran.h. This eliminates the
incorrect evaluation of isfinite for the nan_inf_fmt testcase. If I move the other form...

#define isfinite(x) ((x) - (x) == 0)

outside of the preprocessor statement, it fails as before so that form seems to have a problem
with optimizations beyond -O0.
Comment 19 Jack Howarth 2006-11-01 05:10:17 UTC
Created attachment 12524 [details]
preprocessed file for libgfortran/io/write.c on Darwin PPC
Comment 20 Jack Howarth 2006-11-01 06:06:26 UTC
Created attachment 12525 [details]
assembly file from libgfortran/io/write.c on Darwin PPC
Comment 21 Jack Howarth 2006-11-01 14:00:22 UTC
This bug is rather confusing. It appears that the actual macros for isfinite on Darwin PPC look like...

  static __inline__ int __inline_isinff( float __x ) { return __builtin_fabsf(__x) == __builtin_inff(); }
  static __inline__ int __inline_isinfd( double __x ) { return __builtin_fabs(__x) == __builtin_inf(); }
  static __inline__ int __inline_isinf( long double __x ) { return __builtin_fabsl(__x) == __builtin_infl(); }
  static __inline__ int __inline_isfinitef( float __x ) { return __x == __x && __builtin_fabsf(__x) != __builtin_inff(); }
  static __inline__ int __inline_isfinited( double __x ) { return __x == __x && __builtin_fabs(__x) != __builtin_inf(); }
  static __inline__ int __inline_isfinite( long double __x ) { return __x == __x && __builtin_fabsl(__x) != __builtin_infl(); }

I assume that the __builtin_##### entries are actually library function calls and not macros themselves.
If that is the case, why would the isfinite macros break at higher optimization levels?
Comment 22 Jack Howarth 2006-11-02 02:51:44 UTC
A couple other observations. The latest Xcode 2.4.1 release doesn't fix this problem. Also, shorter version of the testcase...

       implicit none
       character*40 l
       character*12 fmt
       real zero, pos_inf
       zero = 0.0

       pos_inf = 1.0/zero
       fmt = '(F0.0)'
       write(l,fmt=fmt)pos_inf
       end

fails when zero and pos_inf is declared as real, real*4 or real*8 so all forms of the builtin isfinite macros are broken on Darwin PPC with Xcode 2.4.x. Also, if I compile write.c with -DHAVE_BROKEN_ISFINITE, the resulting libgfortran passes the nan_inf_fmt test.
Comment 23 Jack Howarth 2006-11-02 03:16:30 UTC
One more observation. If I change...

Index: write.c
===================================================================
--- write.c     (revision 118343)
+++ write.c     (working copy)
@@ -893,7 +893,7 @@ write_l (st_parameter_dt *dtp, const fno
 static void
 write_float (st_parameter_dt *dtp, const fnode *f, const char *source, int len)
 {
-  GFC_REAL_LARGEST n;
+  double n;
   int nb =0, res, save_scale_factor;
   char * p, fin;
   fnode *f2 = NULL;

the crash in nan_int_fmt. So this reinforces what I initially suspected. This likely another side effect of the broken long double support on Darwin PPC.
Comment 24 Jack Howarth 2006-11-02 03:19:03 UTC
The previous comment should indicate the crash in nan_inf_fmt disappears when n is a double.
Comment 25 Francois-Xavier Coudert 2006-11-05 08:19:29 UTC
This is completely a target bug, and I have made a simple C testcase. I filed this with Apple bug reporter under id# 4820385. Adding geoffk in CC list since he wanted a C-only testcase, and now we have one.

$ cat ppc_longdouble.c 
#include <stdlib.h>
#include <stdio.h>
#include <math.h>

void foo_ (long double *y)
{
  long double x;
  int n;

  x = *y;
  n = isfinite(x);
  if (n == 0)
    printf ("!finite: %d %Lg\n", n, x);
  else
    printf ("finite: %d %Lg\n", n, x);
}

int main (void)
{
  long double x;
  x = 0.0;
  x = 1 / x;
  foo_ (&x);
  return 0;
}
$ gcc-4.0 -g ppc_longdouble.c -O0 && ./a.out
finite: 1 inf
$ gcc-4.0 -g ppc_longdouble.c -O1 && ./a.out
finite: 1 inf
Comment 26 Eric Christopher 2006-11-08 00:06:59 UTC
I'll take this one since I've got the apple radar on it as well, removing Geoff.
Comment 27 Eric Christopher 2006-12-15 22:42:09 UTC
Submitted patch.
Comment 28 Roger Sayle 2006-12-19 04:17:22 UTC
Subject: Bug 29302

Author: sayle
Date: Tue Dec 19 04:17:11 2006
New Revision: 120040

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=120040
Log:
2006-12-18  Roger Sayle  <roger@eyesopen.com>
	    Eric Christopher  <echristo@apple.com>

	PR target/29302
	* real.c (real_maxval): Correctly handle IBM extended double format.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/real.c

Comment 29 Jack Howarth 2006-12-19 14:04:23 UTC
The nan_inf_fmt.f90 failures also occur in gcc 4.2 branch. Can we apply this fix there as well?
Comment 30 Eric Christopher 2006-12-19 20:26:04 UTC
Subject: Bug 29302

Author: echristo
Date: Tue Dec 19 20:25:49 2006
New Revision: 120058

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=120058
Log:
2006-12-19  Eric Christopher  <echristo@apple.com>

        PR target/29302
        * gcc.c-torture/execute/pr29302-1.c: New.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr29302-1.c
Modified:
    trunk/gcc/testsuite/ChangeLog

Comment 31 Eric Christopher 2007-01-17 23:30:45 UTC
Subject: Bug 29302

Author: echristo
Date: Wed Jan 17 23:30:30 2007
New Revision: 120884

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=120884
Log:
2007-01-17  Eric Christopher  <echristo@apple.com>

        Backport from mainline:
        2006-12-18  Roger Sayle  <roger@eyesopen.com>
                    Eric Christopher  <echristo@apple.com>

        PR target/29302
        * real.c (real_maxval): Correctly handle IBM extended double format.

2007-01-17  Eric Christopher  <echristo@apple.com>

        Backport from mainline:
        2006-12-19  Eric Christopher  <echristo@apple.com>

        PR target/29302
        * gcc.c-torture/execute/pr29302-1.c: New.


Added:
    branches/gcc-4_2-branch/gcc/testsuite/gcc.c-torture/execute/pr29302-1.c
Modified:
    branches/gcc-4_2-branch/gcc/ChangeLog
    branches/gcc-4_2-branch/gcc/real.c
    branches/gcc-4_2-branch/gcc/testsuite/ChangeLog

Comment 32 Andrew Pinski 2007-01-21 07:00:00 UTC
Fixed.