Re: Clang buildworld failure due to multiple definitions of __isnanf

On 2011-06-27 04:32, Eric McCorkle wrote:
> I've both seen reports and experienced make buildworld with clang
> failing in usr.bin/xlint/lint1 (really, make kernel-toolchain is what
> fails), because lint1 is statically linked, and there is a definition of
> __isnanf in both libc and libm.  GCC, on the other hand, builds just fine.
...

I have never seen this failure, and neither does the clang buildbot, so
maybe there is something in your build environment causing this problem?
Can you please post:

- Your build architecture (e.g. i386 or amd64)
- Your /etc/make.conf and /etc/src.conf, if applicable
- Any build-related environment variables (WITH_FOO, WITHOUT_FOO,
   CFLAGS, etc)


> The file tree.c in usr.bin/xlint/lint1 calls both isnan and finite from
> math.h.  After some investigation, I figured out what's going on.
> math.h includes a macro version of isnan, which expands out to an
> expression that calls isnan, __isnanl, and __isnanf.  GCC seems to treat
> all of these as builtin functions, and implements them with its code
> generator, rather than generating calls.  Clang, on the other hand, does
> not, which leaves calls to __isnanf in the resulting object file, which
> will result in multiple definitions at link time.

I don't see this at all here.  Clang simply expands the macro:

#define isnan(x)                                        \
     ((sizeof (x) == sizeof (float)) ? __isnanf(x)       \
     : (sizeof (x) == sizeof (double)) ? isnan(x)        \
     : __isnanl(x))

then sees x is a double, so eliminates the unneeded parts of the ?:
operators, and simply calls the isnan() function.  There is no trace of
__isnanf() anywhere in the resulting .s or .o file.

Maybe you compiled with non-default optimization flags?  If so, please
supply them.


> There are several possible solutions.  The workaround I used is to add
> -Wl,--allow-multiple-definition to LDADD in the makefile for xlint.

Actually, I think the problem is that there are multiple definitions of
__isnanf(), one in libc and one in libm.  In lib/libc/gen/isnan.c,
there is this comment:

/*
  * XXX These routines belong in libm, but they must remain in libc for
  *     binary compat until we can bump libm's major version number.
  */

after which both __isnan() and __isnanf() are defined, and weakly bound
to isnan() and isnanf().  Then, in lib/msun/src/s_isnan.c, the isnan()
definition is commented out, but not the __isnanf() definition.

I think this never caused a problem with gcc, since, as you said, it
usually expands it using builtins.

Weirdly enough, a small test program that directly calls __isnanf(), and
is linked with -lm doesn't cause any problems here, neither with gcc,
nor with clang.


> A better solution, I think, is to modify math.h with something like this:
>
> #ifdef __clang__
> #define isnan(n) __builtin_isnan(n)
> ...
> #endif
>
> (It might be a good idea to add these kinds of definitions for all of
> clang's builtins, actually.)

I'm not sure why gcc inserts builtins for isnan() and friends, since if
you wanted those, you should probably #define isnan as __builtin_isnan.
Otherwise, the implementations in libc and libm would never be used.

I hope those are 'better' than the ones builtin to gcc (or clang), but
this is not really for me to say, better ask Bruce Evans, David Schultz
or Steve Kargl. :)