bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

seq regression on x86 in coreutils 7.0


From: Paul Eggert
Subject: seq regression on x86 in coreutils 7.0
Date: Fri, 24 Oct 2008 22:52:22 -0700
User-agent: Gnus/5.11 (Gnus v5.11) Emacs/22.3 (gnu/linux)

Here's the bug:

$ seq 9223372036854775807 9223372036854775808
9223372036854775807
9223372036854775808
9223372036854775809

This is Debian stable x86, compiled with GCC 4.3.2.
This worked correctly in older 'seq' versions.

The problem is that the 2008-04-14 seq patch introduced an abs_rel_diff
function that isn't numerically accurate.  I looked into why that
function was introduced; it was to work around a bug in Solaris 8 and
some other older systems, where strold ("0.90000000000000000000") <
strold ("0.9").  We shouldn't worry about working around numeric bugs
like these in C libraries in older systems, unless the bugs are likely
to come up in real applications, which is not the case here.  Better to
modify the test case to not tickle the bug in the buggy C library.

We can't easily add the above test case, since it's not portable to
hosts where long double == double.

Here's a patch:

Undo workaround for Solaris 8 floating-point bug that introduced other bugs.

* src/seq.c: Don't include <math.h>, <float.h>.
(abs_rel_diff): Remove.
(print_numbers): Test for equality, not for an epsilonish value.
This reverts the change of 2008-04-14, which broke 'seq' on the x86;
for example, it causes "seq 9223372036854775807 9223372036854775808"
to incorrectly output 3 numbers instead of 2.  It's better to punish obsolescent
hosts that have incorrectly-working floating-point than to punish
correctly-working hosts.
* tests/misc/seq: Use 0.9000000000000, rather than 0.90000000000000000000,
to avoid tickling a bug in Solaris 8 strtold, which converts "0.9" and
"0.9000000000000" correctly, but incorrectly converts "0.90000000000000000000"
to a smaller value.

diff --git a/src/seq.c b/src/seq.c
index b41aab6..3ae158b 100644
--- a/src/seq.c
+++ b/src/seq.c
@@ -20,8 +20,6 @@
 #include <getopt.h>
 #include <stdio.h>
 #include <sys/types.h>
-#include <math.h>
-#include <float.h>
 
 #include "system.h"
 #include "c-strtod.h"
@@ -258,14 +256,6 @@ long_double_format (char const *fmt, struct layout *layout)
   return NULL;
 }
 
-/* Return the absolute relative difference from x to y.  */
-static double
-abs_rel_diff (double x, double y)
-{
-  double s = (y == 0.0 ? 1 : y);
-  return fabs ((y - x) / s);
-}
-
 /* Actually print the sequence of numbers in the specified range, with the
    given or default stepping and format.  */
 
@@ -310,7 +300,7 @@ print_numbers (char const *fmt, struct layout layout,
              x_str[x_strlen - layout.suffix_len] = '\0';
 
              if (xstrtold (x_str + layout.prefix_len, NULL, &x_val, c_strtold)
-                 && abs_rel_diff (x_val, last) < DBL_EPSILON)
+                 && x_val == last)
                {
                  char *x0_str = NULL;
                  if (asprintf (&x0_str, fmt, x0) < 0)
diff --git a/tests/misc/seq b/tests/misc/seq
index 8e76f42..2271322 100755
--- a/tests/misc/seq
+++ b/tests/misc/seq
@@ -47,7 +47,10 @@ my @Tests =
     {OUT_SUBST => 's,^-0\.0$,0.0,'},
    ],
    ['float-5', qw(0.8 1e-1 0.9),       {OUT => [qw(0.8 0.9)]}],
-   ['float-6', qw(0.8 0.1 0.90000000000000000000),     {OUT => [qw(0.8 0.9)]}],
+   # Don't append lots of zeros to that 0.9000...; for example, changing the
+   # number to 0.90000000000000000000 tickles a bug in Solaris 8 strtold
+   # that would cause the test to fail.
+   ['float-6', qw(0.8 0.1 0.9000000000000),    {OUT => [qw(0.8 0.9)]}],
 
    ['wid-1',   qw(.8 1e-2 .81),  {OUT => [qw(0.80 0.81)]}],
    ['wid-2',   qw(.89999 1e-7 .8999901),  {OUT => [qw(0.8999900 0.8999901)]}],




reply via email to

[Prev in Thread] Current Thread [Next in Thread]