Re: gawk number to string bug

[Paul Eggert]

"Andrew J. Schorr" <address@hidden> writes:

> why wouldn't something as simple as the attached patch do the trick
> (i.e. simply convert the floating point value to a long, then
> convert that value back to floating-point, and see whether it
> matches the original value)?

Neither your patch nor Arnold's suffice, unfortunately.

With your patch, suppose long is 64 bits and double is 64-bit IEEE
double.  The string "9223372036854775808" (which is 2**63) can be
converted to double exactly, without loss of information; but since
2**63 is too big for long, it is converted to LONG_MAX when you
convert it to long.  So far so good, but now when you compute

  if ((AWKNUM)(num) != s->number)

converting LONG_MAX to AWKNUM yields 9223372036854775808 again, since
2**63 is the double value that is closest to LONG_MAX.  So the
comparison is equal, and gawk then incorrectly thinks that no
conversion problem had occurred.

Aharon's patch is better, but still doesn't solve the problem in
general.  For example, on 64-bit Solaris 8 sparc (compiled with GCC
4.0.2, gcc -m64) with his patch I get this behavior:

$ awk 'BEGIN { print 9223372036854776832 }' </dev/null
9223372036854775807

The output should be "9.22337e+18", since the requested number exceeds
LONG_MAX.  But the code converts to long and then prints an integer
that is off by 1025.


Here is a proposed patch.

2005-12-19  Paul Eggert  <address@hidden>

        * node.c (format_val): Fix bug when handling numbers close to
        LONG_MIN and LONG_MAX.
        * awk.h (FLT_RADIX, FLT_MANT_DIG, DBL_MANT_DIG, AWKSMALL_MANT_DIG):
        (AWKNUM_MANT_DIG, ASKNUM_FRACTION_BITS): Moved here from builtin.c.
        * builtin.c: Move those macros to awk.h.
        * awk.h (DBL_FRACTION_BITS): New macro.

--- node.c-bak1 2005-11-30 13:33:44.000000000 -0800
+++ node.c      2005-12-19 13:00:51.000000000 -0800
@@ -163,9 +163,16 @@ format_val(const char *format, int index
                return tmp_string(trans, strlen(trans));
        }
 
-       /* not an integral value, or out of range */
+       /*
+        * Cconversion to long overflows, or out of range, or not integral.
+        * The test against DBL_FRACTION_BITS guards against numbers that
+        * compare "equal" to LONG_MIN or LONG_MAX due to rounding error,
+        * even though they are out of range.
+        */
        if ((val = double_to_int(s->numbr)) != s->numbr
-           || val < LONG_MIN || val > LONG_MAX) {
+           || (DBL_FRACTION_BITS < CHAR_BIT * sizeof(long)
+               ? val <= LONG_MIN || val >= LONG_MAX
+               : val < LONG_MIN || val > LONG_MAX)) {
                /*
                 * Once upon a time, if GFMT_WORKAROUND wasn't defined,
                 * we just blindly did this:
--- awk.h-bak1  2005-07-26 11:07:43.000000000 -0700
+++ awk.h       2005-12-19 12:52:42.000000000 -0800
@@ -293,6 +293,50 @@ extern double gawk_strtod();
 
 #define AWKNUM double
 
+/* Assume IEEE-754 arithmetic on pre-C89 hosts.  */
+#ifndef FLT_RADIX
+#define FLT_RADIX 2
+#endif
+#ifndef FLT_MANT_DIG
+#define FLT_MANT_DIG 24
+#endif
+#ifndef DBL_MANT_DIG
+#define DBL_MANT_DIG 53
+#endif
+
+/*
+ * The number of base-FLT_RADIX digits in an AWKNUM fraction, assuming
+ * that AWKNUM is not long double.
+ */
+#define AWKSMALL_MANT_DIG \
+  (sizeof (AWKNUM) == sizeof (double) ? DBL_MANT_DIG : FLT_MANT_DIG)
+
+/*
+ * The number of base-FLT_DIGIT digits in an AWKNUM fraction, even if
+ * AWKNUM is long double.  Don't mention 'long double' unless
+ * LDBL_MANT_DIG is defined, for the sake of ancient compilers that
+ * lack 'long double'.
+ */
+#ifdef LDBL_MANT_DIG
+#define AWKNUM_MANT_DIG \
+  (sizeof (AWKNUM) == sizeof (long double) ? LDBL_MANT_DIG : AWKSMALL_MANT_DIG)
+#else
+#define AWKNUM_MANT_DIG AWKSMALL_MANT_DIG
+#endif
+
+/*
+ * The number of bits in an AWKNUM fraction, assuming FLT_RADIX is
+ * either 2 or 16.  IEEE and VAX formats use radix 2, and IBM
+ * mainframe format uses radix 16; we know of no other radices in
+ * practical use.
+ */
+#if FLT_RADIX != 2 && FLT_RADIX != 16
+Please port the following code to your weird host;
+#endif
+#define AWKNUM_FRACTION_BITS (AWKNUM_MANT_DIG * (FLT_RADIX == 2 ? 1 : 4))
+#define DBL_FRACTION_BITS (DBL_MANT_DIG * (FLT_RADIX == 2 ? 1 : 4))
+
+
 #ifndef TRUE
 /* a bit hackneyed, but what the heck */
 #define TRUE   1
--- builtin.c-bak1      2005-07-26 11:07:43.000000000 -0700
+++ builtin.c   2005-12-19 12:53:06.000000000 -0800
@@ -73,17 +73,6 @@ extern int output_is_tty;
 
 static NODE *sub_common P((NODE *tree, long how_many, int backdigs));
 
-/* Assume IEEE-754 arithmetic on pre-C89 hosts.  */
-#ifndef FLT_RADIX
-#define FLT_RADIX 2
-#endif
-#ifndef FLT_MANT_DIG
-#define FLT_MANT_DIG 24
-#endif
-#ifndef DBL_MANT_DIG
-#define DBL_MANT_DIG 53
-#endif
-
 #ifdef _CRAY
 /* Work around a problem in conversion of doubles to exact integers. */
 #define Floor(n) floor((n) * (1.0 + DBL_EPSILON))
@@ -2645,37 +2634,6 @@ sgfmt(char *buf, /* return buffer; assum
 }
 #endif /* GFMT_WORKAROUND */
 
-/*
- * The number of base-FLT_RADIX digits in an AWKNUM fraction, assuming
- * that AWKNUM is not long double.
- */
-#define AWKSMALL_MANT_DIG \
-  (sizeof (AWKNUM) == sizeof (double) ? DBL_MANT_DIG : FLT_MANT_DIG)
-
-/*
- * The number of base-FLT_DIGIT digits in an AWKNUM fraction, even if
- * AWKNUM is long double.  Don't mention 'long double' unless
- * LDBL_MANT_DIG is defined, for the sake of ancient compilers that
- * lack 'long double'.
- */
-#ifdef LDBL_MANT_DIG
-#define AWKNUM_MANT_DIG \
-  (sizeof (AWKNUM) == sizeof (long double) ? LDBL_MANT_DIG : AWKSMALL_MANT_DIG)
-#else
-#define AWKNUM_MANT_DIG AWKSMALL_MANT_DIG
-#endif
-
-/*
- * The number of bits in an AWKNUM fraction, assuming FLT_RADIX is
- * either 2 or 16.  IEEE and VAX formats use radix 2, and IBM
- * mainframe format uses radix 16; we know of no other radices in
- * practical use.
- */
-#if FLT_RADIX != 2 && FLT_RADIX != 16
-Please port the following code to your weird host;
-#endif
-#define AWKNUM_FRACTION_BITS (AWKNUM_MANT_DIG * (FLT_RADIX == 2 ? 1 : 4))
- 
 /* tmp_integer - Convert an integer to a temporary number node.  */
 
 static NODE *