Wrong unconditional dependency on nanl

[Christophe Lyon]

Hello,

On aarch64 I've notice that a simple hello.c fails to link against
current newlib master:
.../aarch64-elf/libc/usr/lib/libc.a(lib_a-strtorx.o): In function `ULtox':
.../newlib/libc/stdlib/strtorx.c:92: undefined reference to `nanl'

It seems to me that an unconditional dependency on nanl() was
introduced by commit
6c212a8b7873703c4f98c6b68579b234918be83a
Author: Masamichi Hosoda <***@trueroad.jp>
Date: Thu Aug 16 09:18:50 2018 +0900
Fix strtod ("nan") and strtold ("nan") returns wrong negative NaN

but I'm not sure how to fix this as I did not follow the discussion
around this patch.

It seems to me that nanl() is only defined if _LDBL_EQ_DBL
(newlib/libm/common/nanl.c), but the patch above introduces a call to
nanl() in a code path where !defined (_LDBL_EQ_DBL)

This I suspect the problem is present on other targets, too ?

Christophe

[Corinna Vinschen]

Post by Christophe Lyon
Hello,
On aarch64 I've notice that a simple hello.c fails to link against
.../newlib/libc/stdlib/strtorx.c:92: undefined reference to `nanl'
It seems to me that an unconditional dependency on nanl() was
introduced by commit
6c212a8b7873703c4f98c6b68579b234918be83a
Date: Thu Aug 16 09:18:50 2018 +0900
Fix strtod ("nan") and strtold ("nan") returns wrong negative NaN
but I'm not sure how to fix this as I did not follow the discussion
around this patch.
It seems to me that nanl() is only defined if _LDBL_EQ_DBL
(newlib/libm/common/nanl.c), but the patch above introduces a call to
nanl() in a code path where !defined (_LDBL_EQ_DBL)
This I suspect the problem is present on other targets, too ?

Cygwin has it's own nanl implemented in winsup/cygwin/math/nanl.c.

However, dropping it there and using a patch like this:

diff --git a/newlib/libm/common/nanl.c b/newlib/libm/common/nanl.c
index 40f898109edd..b8ae63ea7539 100644
--- a/newlib/libm/common/nanl.c
+++ b/newlib/libm/common/nanl.c
@@ -38,5 +38,11 @@ nanl (const char *tagp)
{
return nan(tagp);
}
+#elif __GNUC_PREREQ (3, 3)
+long double
+nanl (const char *tagp)
+{
+ return __builtin_nanl("");
+}
#endif

should help along all targets, right? Is there any target still using
a pre-3.3 GCC?


Corinna

[Christophe Lyon]

Post by Corinna Vinschen

Post by Christophe Lyon
Hello,
On aarch64 I've notice that a simple hello.c fails to link against
.../newlib/libc/stdlib/strtorx.c:92: undefined reference to `nanl'
It seems to me that an unconditional dependency on nanl() was
introduced by commit
6c212a8b7873703c4f98c6b68579b234918be83a
Date: Thu Aug 16 09:18:50 2018 +0900
Fix strtod ("nan") and strtold ("nan") returns wrong negative NaN
but I'm not sure how to fix this as I did not follow the discussion
around this patch.
It seems to me that nanl() is only defined if _LDBL_EQ_DBL
(newlib/libm/common/nanl.c), but the patch above introduces a call to
nanl() in a code path where !defined (_LDBL_EQ_DBL)
This I suspect the problem is present on other targets, too ?

Cygwin has it's own nanl implemented in winsup/cygwin/math/nanl.c.
diff --git a/newlib/libm/common/nanl.c b/newlib/libm/common/nanl.c
index 40f898109edd..b8ae63ea7539 100644
--- a/newlib/libm/common/nanl.c
+++ b/newlib/libm/common/nanl.c
@@ -38,5 +38,11 @@ nanl (const char *tagp)
{
return nan(tagp);
}
+#elif __GNUC_PREREQ (3, 3)
+long double
+nanl (const char *tagp)
+{
+ return __builtin_nanl("");
+}
#endif
should help along all targets, right? Is there any target still using
a pre-3.3 GCC?

It does help on aarch64-elf, but a change to rdimon.specs is required
because libc now depends on libm: this can be seen when compiling a
C++ hello.cpp.

In addition, there's still a warning:
newlib/libc/stdlib/strtorx.c:92:24: warning: implicit declaration of
function ‘nanl’

Christophe

Post by Corinna Vinschen
Corinna
--
Corinna Vinschen
Cygwin Maintainer
Red Hat

[Corinna Vinschen]

Post by Christophe Lyon

Post by Corinna Vinschen

Post by Christophe Lyon
Hello,
On aarch64 I've notice that a simple hello.c fails to link against
.../newlib/libc/stdlib/strtorx.c:92: undefined reference to `nanl'
It seems to me that an unconditional dependency on nanl() was
introduced by commit
6c212a8b7873703c4f98c6b68579b234918be83a
Date: Thu Aug 16 09:18:50 2018 +0900
Fix strtod ("nan") and strtold ("nan") returns wrong negative NaN
but I'm not sure how to fix this as I did not follow the discussion
around this patch.
It seems to me that nanl() is only defined if _LDBL_EQ_DBL
(newlib/libm/common/nanl.c), but the patch above introduces a call to
nanl() in a code path where !defined (_LDBL_EQ_DBL)
This I suspect the problem is present on other targets, too ?

Cygwin has it's own nanl implemented in winsup/cygwin/math/nanl.c.
diff --git a/newlib/libm/common/nanl.c b/newlib/libm/common/nanl.c
index 40f898109edd..b8ae63ea7539 100644
--- a/newlib/libm/common/nanl.c
+++ b/newlib/libm/common/nanl.c
@@ -38,5 +38,11 @@ nanl (const char *tagp)
{
return nan(tagp);
}
+#elif __GNUC_PREREQ (3, 3)
+long double
+nanl (const char *tagp)
+{
+ return __builtin_nanl("");
+}
#endif
should help along all targets, right? Is there any target still using
a pre-3.3 GCC?

It does help on aarch64-elf, but a change to rdimon.specs is required
because libc now depends on libm: this can be seen when compiling a
C++ hello.cpp.

Hmm, that's an interesting point. What I don't understand is why this
dependency wasn't already visible before. strtod.c and wcstod.c call
nan() and nanf(), vfprintf.c and vfwprintf.c call nanf(). Why is a
call to nanl() different?

Please note that this is not visible on Cygwin where libc and libm are
just link libs while all of their code is in cygwin1.dll anyway.

Post by Christophe Lyon
newlib/libc/stdlib/strtorx.c:92:24: warning: implicit declaration of
function ‘nanl’

That's an easy one after making sure why the above occurs.

As for a solution; provided we can rely on GCC 3.3 or later, we
could define internal macros __nan, __nanf and __nanl like this:

#ifdef _COMPILING_NEWLIB
#define __nan(_x) __builtin_nan(_x)
#define __nanf(_x) __builtin_nanf(_x)
#define __nanl(_x) __builtin_nanl(_x)
#endif

and use them within libc throughout.


Corinna

[Christophe Lyon]

Post by Corinna Vinschen

Post by Christophe Lyon

Post by Corinna Vinschen

Post by Christophe Lyon
Hello,
On aarch64 I've notice that a simple hello.c fails to link against
.../newlib/libc/stdlib/strtorx.c:92: undefined reference to `nanl'
It seems to me that an unconditional dependency on nanl() was
introduced by commit
6c212a8b7873703c4f98c6b68579b234918be83a
Date: Thu Aug 16 09:18:50 2018 +0900
Fix strtod ("nan") and strtold ("nan") returns wrong negative NaN
but I'm not sure how to fix this as I did not follow the discussion
around this patch.
It seems to me that nanl() is only defined if _LDBL_EQ_DBL
(newlib/libm/common/nanl.c), but the patch above introduces a call to
nanl() in a code path where !defined (_LDBL_EQ_DBL)
This I suspect the problem is present on other targets, too ?

Cygwin has it's own nanl implemented in winsup/cygwin/math/nanl.c.
diff --git a/newlib/libm/common/nanl.c b/newlib/libm/common/nanl.c
index 40f898109edd..b8ae63ea7539 100644
--- a/newlib/libm/common/nanl.c
+++ b/newlib/libm/common/nanl.c
@@ -38,5 +38,11 @@ nanl (const char *tagp)
{
return nan(tagp);
}
+#elif __GNUC_PREREQ (3, 3)
+long double
+nanl (const char *tagp)
+{
+ return __builtin_nanl("");
+}
#endif
should help along all targets, right? Is there any target still using
a pre-3.3 GCC?

It does help on aarch64-elf, but a change to rdimon.specs is required
because libc now depends on libm: this can be seen when compiling a
C++ hello.cpp.

Hmm, that's an interesting point. What I don't understand is why this
dependency wasn't already visible before. strtod.c and wcstod.c call
nan() and nanf(), vfprintf.c and vfwprintf.c call nanf(). Why is a
call to nanl() different?

The linker says nan() comes from libc.a(lib_a-s_nan.o) and
nanf() comes from libc.a(lib_a-sf_nan.o)

Post by Corinna Vinschen
Please note that this is not visible on Cygwin where libc and libm are
just link libs while all of their code is in cygwin1.dll anyway.

Post by Christophe Lyon
newlib/libc/stdlib/strtorx.c:92:24: warning: implicit declaration of
function ‘nanl’

That's an easy one after making sure why the above occurs.
As for a solution; provided we can rely on GCC 3.3 or later, we
#ifdef _COMPILING_NEWLIB
#define __nan(_x) __builtin_nan(_x)
#define __nanf(_x) __builtin_nanf(_x)
#define __nanl(_x) __builtin_nanl(_x)
#endif
and use them within libc throughout.
Corinna
--
Corinna Vinschen
Cygwin Maintainer
Red Hat

[Craig Howland]

Post by Corinna Vinschen

Post by Christophe Lyon
It does help on aarch64-elf, but a change to rdimon.specs is required
because libc now depends on libm: this can be seen when compiling a
C++ hello.cpp.

Hmm, that's an interesting point. What I don't understand is why this
dependency wasn't already visible before. strtod.c and wcstod.c call
nan() and nanf(), vfprintf.c and vfwprintf.c call nanf(). Why is a
call to nanl() different?

The nanl() call was only added perhaps a few weeks ago.  At that time we missed
the thought that it comes from libm.  We'd need to add it to newlib/Makefile to
be copied like s_nan and sf_nan presently are.

[Corinna Vinschen]

Post by Corinna Vinschen

Post by Christophe Lyon
It does help on aarch64-elf, but a change to rdimon.specs is required
because libc now depends on libm: this can be seen when compiling a
C++ hello.cpp.

Hmm, that's an interesting point. What I don't understand is why this
dependency wasn't already visible before. strtod.c and wcstod.c call
nan() and nanf(), vfprintf.c and vfwprintf.c call nanf(). Why is a
call to nanl() different?

The nanl() call was only added perhaps a few weeks ago.  At that time we
missed the thought that it comes from libm.  We'd need to add it to
newlib/Makefile to be copied like s_nan and sf_nan presently are.

Yeah, just found this myself. nan/nanf are pulled in via newlib's
top-level Makefile.am:

MATHOBJS_IN_LIBC = ...

Pulling in nanl is a problem because it's build depends on
HAVE_LONG_DOUBLE.

Maybe we should really just use __builtin_nanl in strtorx.c...?


Corinna

[Craig Howland]

Post by Corinna Vinschen

Post by Craig Howland

Post by Corinna Vinschen

Post by Christophe Lyon
It does help on aarch64-elf, but a change to rdimon.specs is required
because libc now depends on libm: this can be seen when compiling a
C++ hello.cpp.

Hmm, that's an interesting point. What I don't understand is why this
dependency wasn't already visible before. strtod.c and wcstod.c call
nan() and nanf(), vfprintf.c and vfwprintf.c call nanf(). Why is a
call to nanl() different?

The nanl() call was only added perhaps a few weeks ago.  At that time we
missed the thought that it comes from libm.  We'd need to add it to
newlib/Makefile to be copied like s_nan and sf_nan presently are.

Yeah, just found this myself. nan/nanf are pulled in via newlib's
MATHOBJS_IN_LIBC = ...
Pulling in nanl is a problem because it's build depends on
HAVE_LONG_DOUBLE.
Maybe we should really just use __builtin_nanl in strtorx.c...?
Corinna

I had actually sent this as a suggestion a couple of days ago, but somehow my
email didn't make it to the list.  Here's the whole thing I sent, even though
the two numbered points have since been realized and discussed:

I think the problem is probably two-fold.
1)  nanl() is in the math library, so -lm would be needed when linking.
2)  As you think, nanl() is only defined when LDBL_EQ_DBL.
I suggest that a workaround is to replace 'nanl("")' with '__builtin_nanl("")'. 
It is somewhat ugly, but it would be better than what it is now (it has the
major advantage of getting around the -lm problem), and arguably no worse. 
While strictly it would need something like "#if __GNUC_PREREQ (3, 3)" to go
with it, not doing so could just fail at link time like it is presently.

As Corrina had noted, it seems pretty unlikely that anyone is older than GCC 3.3
at this point.
Craig

[Corinna Vinschen]

Post by Corinna Vinschen

Post by Corinna Vinschen

Post by Christophe Lyon
It does help on aarch64-elf, but a change to rdimon.specs is required
because libc now depends on libm: this can be seen when compiling a
C++ hello.cpp.

Hmm, that's an interesting point. What I don't understand is why this
dependency wasn't already visible before. strtod.c and wcstod.c call
nan() and nanf(), vfprintf.c and vfwprintf.c call nanf(). Why is a
call to nanl() different?

The nanl() call was only added perhaps a few weeks ago.  At that time we
missed the thought that it comes from libm.  We'd need to add it to
newlib/Makefile to be copied like s_nan and sf_nan presently are.

Yeah, just found this myself. nan/nanf are pulled in via newlib's
MATHOBJS_IN_LIBC = ...
Pulling in nanl is a problem because it's build depends on
HAVE_LONG_DOUBLE.
Maybe we should really just use __builtin_nanl in strtorx.c...?

...and build strtorx.c only if HAVE_LONG_DOUBLE, just like strtold.c and
wcstold.c.


Corinna

[Corinna Vinschen]

Post by Corinna Vinschen

Post by Corinna Vinschen

Post by Corinna Vinschen

Post by Christophe Lyon
It does help on aarch64-elf, but a change to rdimon.specs is required
because libc now depends on libm: this can be seen when compiling a
C++ hello.cpp.

Hmm, that's an interesting point. What I don't understand is why this
dependency wasn't already visible before. strtod.c and wcstod.c call
nan() and nanf(), vfprintf.c and vfwprintf.c call nanf(). Why is a
call to nanl() different?

The nanl() call was only added perhaps a few weeks ago.  At that time we
missed the thought that it comes from libm.  We'd need to add it to
newlib/Makefile to be copied like s_nan and sf_nan presently are.

Yeah, just found this myself. nan/nanf are pulled in via newlib's
MATHOBJS_IN_LIBC = ...
Pulling in nanl is a problem because it's build depends on
HAVE_LONG_DOUBLE.
Maybe we should really just use __builtin_nanl in strtorx.c...?

...and build strtorx.c only if HAVE_LONG_DOUBLE, just like strtold.c and
wcstold.c.

I pushed three patches to handle all of the above. Please give them
a try.


Thanks,
Corinna

[Christophe Lyon]

Post by Corinna Vinschen

Post by Corinna Vinschen

Post by Corinna Vinschen

Post by Corinna Vinschen

Post by Christophe Lyon
It does help on aarch64-elf, but a change to rdimon.specs is required
because libc now depends on libm: this can be seen when compiling a
C++ hello.cpp.

Hmm, that's an interesting point. What I don't understand is why this
dependency wasn't already visible before. strtod.c and wcstod.c call
nan() and nanf(), vfprintf.c and vfwprintf.c call nanf(). Why is a
call to nanl() different?

The nanl() call was only added perhaps a few weeks ago. At that time we
missed the thought that it comes from libm. We'd need to add it to
newlib/Makefile to be copied like s_nan and sf_nan presently are.

Yeah, just found this myself. nan/nanf are pulled in via newlib's
MATHOBJS_IN_LIBC = ...
Pulling in nanl is a problem because it's build depends on
HAVE_LONG_DOUBLE.
Maybe we should really just use __builtin_nanl in strtorx.c...?

...and build strtorx.c only if HAVE_LONG_DOUBLE, just like strtold.c and
wcstold.c.

I pushed three patches to handle all of the above. Please give them
a try.

I confirm it works for me on aarch64.
Thanks!

Post by Corinna Vinschen
Thanks,
Corinna
--
Corinna Vinschen
Cygwin Maintainer
Red Hat

[Corinna Vinschen]

Post by Christophe Lyon

Post by Corinna Vinschen

Post by Corinna Vinschen

Post by Corinna Vinschen

Post by Corinna Vinschen

Post by Christophe Lyon
It does help on aarch64-elf, but a change to rdimon.specs is required
because libc now depends on libm: this can be seen when compiling a
C++ hello.cpp.

Hmm, that's an interesting point. What I don't understand is why this
dependency wasn't already visible before. strtod.c and wcstod.c call
nan() and nanf(), vfprintf.c and vfwprintf.c call nanf(). Why is a
call to nanl() different?

The nanl() call was only added perhaps a few weeks ago. At that time we
missed the thought that it comes from libm. We'd need to add it to
newlib/Makefile to be copied like s_nan and sf_nan presently are.

Yeah, just found this myself. nan/nanf are pulled in via newlib's
MATHOBJS_IN_LIBC = ...
Pulling in nanl is a problem because it's build depends on
HAVE_LONG_DOUBLE.
Maybe we should really just use __builtin_nanl in strtorx.c...?

...and build strtorx.c only if HAVE_LONG_DOUBLE, just like strtold.c and
wcstold.c.

I pushed three patches to handle all of the above. Please give them
a try.

I confirm it works for me on aarch64.
Thanks!

Thanks for testing.


Corinna