Re: Wrong unconditional dependency on nanl

On Wed, 10 Oct 2018 at 16:52, Corinna Vinschen <[email protected]> wrote:
>
> On Oct 10 15:48, Christophe Lyon wrote:
> > On Wed, 10 Oct 2018 at 11:41, Corinna Vinschen <[email protected]> wrote:
> > >
> > > On Oct  8 15:06, Christophe Lyon wrote:
> > > > 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 <[email protected]>
> > > > 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?
> > >
> >
> > 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)

>
> 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.
>
> > In addition, there's still a warning:
> > 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
>
> --
> Corinna Vinschen
> Cygwin Maintainer
> Red Hat