Re: bug#34817: coreutils 8.31 compile failure on rhel5

Re: bug#34817: coreutils 8.31 compile failure on rhel5

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 290 bytes --]

On 3/11/19 10:16 AM, Jesse wrote:
> The latest coreutils no longer compiles cleanly on rhel5 x32 and x64
> (I know rhel5 is EOL).

Thanks for the bug report. I installed the attached patch into Gnulib.
Would you please give it a try and report back? I can't easily test on
RHEL 5. Thanks.


[-- Attachment #2: 0001-strtod-fix-clash-with-strtold.patch --]
[-- Type: text/x-patch, Size: 2268 bytes --]

From 0562b040fa17f1722ba2b3096067b45d0582ca53 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 11 Mar 2019 16:40:29 -0700
Subject: [PATCH] strtod: fix clash with strtold

Problem reported for RHEL 5 by Jesse Caldwell (Bug#34817).
* lib/strtod.c (compute_minus_zero, minus_zero):
Simplify by remving the macro / external variable,
and having just a function.  User changed.  This avoids
the need for an external variable that might clash.
---
 ChangeLog    |  9 +++++++++
 lib/strtod.c | 11 +++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b633380b5..e5ba0caa4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2019-03-11  Paul Eggert  <eggert@cs.ucla.edu>
+
+	strtod: fix clash with strtold
+	Problem reported for RHEL 5 by Jesse Caldwell (Bug#34817).
+	* lib/strtod.c (compute_minus_zero, minus_zero):
+	Simplify by remving the macro / external variable,
+	and having just a function.  User changed.  This avoids
+	the need for an external variable that might clash.
+
 2019-03-10  Bruno Haible  <bruno@clisp.org>
 
 	alloca-opt: Fix conflict mingw's new <alloca.h> file.
diff --git a/lib/strtod.c b/lib/strtod.c
index b9eaa51b4..69b1564e1 100644
--- a/lib/strtod.c
+++ b/lib/strtod.c
@@ -294,16 +294,15 @@ parse_number (const char *nptr,
    ICC 10.0 has a bug when optimizing the expression -zero.
    The expression -MIN * MIN does not work when cross-compiling
    to PowerPC on Mac OS X 10.5.  */
-#if defined __hpux || defined __sgi || defined __ICC
 static DOUBLE
-compute_minus_zero (void)
+minus_zero (void)
 {
+#if defined __hpux || defined __sgi || defined __ICC
   return -MIN * MIN;
-}
-# define minus_zero compute_minus_zero ()
 #else
-DOUBLE minus_zero = -0.0;
+  return -0.0;
 #endif
+}
 
 /* Convert NPTR to a DOUBLE.  If ENDPTR is not NULL, a pointer to the
    character after the last one used in the number is put in *ENDPTR.  */
@@ -479,6 +478,6 @@ STRTOD (const char *nptr, char **endptr)
   /* Special case -0.0, since at least ICC miscompiles negation.  We
      can't use copysign(), as that drags in -lm on some platforms.  */
   if (!num && negative)
-    return minus_zero;
+    return minus_zero ();
   return negative ? -num : num;
 }
-- 
2.20.1

Re: bug#34817: coreutils 8.31 compile failure on rhel5

[Jesse]

> 
> On 3/11/19 10:16 AM, Jesse wrote:
> > The latest coreutils no longer compiles cleanly on rhel5 x32 and x64
> > (I know rhel5 is EOL).
> 
> Thanks for the bug report. I installed the attached patch into Gnulib.
> Would you please give it a try and report back? I can't easily test on
> RHEL 5. Thanks.

Hello,

coreutils 8.31 compiles cleanly (warnings, but no errors) on rhel5 x32
and x64 with the provided patch applied. That you for the prompt response!

Jesse

Re: bug#34817: coreutils 8.31 compile failure on rhel5

[Bruno Haible]

Paul Eggert wrote:
> Thanks for the bug report. I installed the attached patch into Gnulib.

A simpler fix would have been to mark the 'minus_zero' variable as 'static':


--- a/lib/strtod.c
+++ b/lib/strtod.c
@@ -302,7 +302,7 @@ compute_minus_zero (void)
 }
 # define minus_zero compute_minus_zero ()
 #else
-DOUBLE minus_zero = -0.0;
+static DOUBLE minus_zero = -0.0;
 #endif
 
 /* Convert NPTR to a DOUBLE.  If ENDPTR is not NULL, a pointer to the


When this variable was introduced, it ought to have been 'static' from the
beginning (2010-11-05); but this was harmless until 2019-01-30. I did not
notice the problem when doing the 2019-01-30 changes; sorry.

Bruno

Re: bug#34817: coreutils 8.31 compile failure on rhel5

[Paul Eggert]

On 3/13/19 11:35 AM, Bruno Haible wrote:
> A simpler fix would have been to mark the 'minus_zero' variable as 'static':

Yes, that would have been a smaller change. Although I originally did
that, there's a reason I went for the larger change.

Making the variable 'static' might cause some compilers to notice that
the variable never changes, so it's a constant, so it can be folded, and
then they (mistakenly) convert -0.0 to 0.0. Originally the variable was
extern and the extern hack prevents most compilers from doing this
incorrect optimization (unless they're doing whole-program optimization,
which typically they aren't). Since making the variable static removed
the hack, I decided it was clearer to use the larger change, which makes
it more obvious that we aren't trying to work around this potential
problem any more.

I hope the issue I mentioned is not a real problem. If it is, we can go
back to the old code except rename the variable so that it's called
"lminus_zero" for the long double case. I considered doing that too, but
that's even more confusing and I hope we don't have to be that confusing.

Re: bug#34817: coreutils 8.31 compile failure on rhel5

[Bruno Haible]

Hi Paul,

> Making the variable 'static' might cause some compilers to notice that
> the variable never changes, so it's a constant, so it can be folded, and
> then they (mistakenly) convert -0.0 to 0.0. Originally the variable was
> extern and the extern hack prevents most compilers from doing this
> incorrect optimization (unless they're doing whole-program optimization,
> which typically they aren't). Since making the variable static removed
> the hack, I decided it was clearer to use the larger change, which makes
> it more obvious that we aren't trying to work around this potential
> problem any more.

Yes, I perfectly agree. When I said "a simpler fix", I did not mean
"a better fix". Your fix is the more reliable one!

Bruno