jansson icon indicating copy to clipboard operation
jansson copied to clipboard

Fix compilation with uclibc.

Open JoakimSoderberg opened this issue 12 years ago • 14 comments

uclibc does not have full math.h support built in so we need to explicitly link to libm in that case. Specifically isnan and isinf would be missing and not compile in this case.

JoakimSoderberg avatar Aug 13 '13 14:08 JoakimSoderberg

Doesn't the workaround in value.c work on uclibc? Does it define them as macros that call an external function?

akheron avatar Aug 14 '13 05:08 akheron

Well if I recall, that fix just checked if isnan was defined or not. If you look here at the math.h included with uclibc: https://github.com/waweber/uclibc-clang/blob/master/include/math.h#L286-L297

you can see that isnan is defined, but it in turn uses __isnan, and that's what I got a linking error on. And the only way to get that was to link against libm.

I guess it would be possible to do some more elaborate system introspection both in cmake and automake to check for it in a more advanced way than to check if it's defined, if that's more desirable?

JoakimSoderberg avatar Aug 14 '13 07:08 JoakimSoderberg

For reference, the isnan commit in value.c: 4118315afa994ff71c2970c9f8933ab48178dbcb

JoakimSoderberg avatar Aug 14 '13 07:08 JoakimSoderberg

Yeah. The #ifdef check is based on a "light-weight" solution from: http://www.gnu.org/software/autoconf/manual/autoconf-2.67/html_node/Function-Portability.html

Correctly checking for this would involve linking test programs that use isnan and isinf. I think it's quite easy to do that with autoconf, but how about CMake?

akheron avatar Aug 14 '13 07:08 akheron

The CMake project now checks for isnan/isinf. As stated in the commit log, we cannot simply redefine "isnan", since the discrepancy is on the linking, "isnan" will still be defined as a macro in math.h.

That is, writing a program that uses "isnan" with uclibc compiles fine, but won't link without libm.

JoakimSoderberg avatar Aug 14 '13 11:08 JoakimSoderberg

There are three possible cases:

  1. isnan() and isinf() are defined, no need for -lm
  2. isnan() and isinf() are defined, -lm is needed
  3. isnan() and isinf() have not been defined at all

To me, the best thing would be to use the built-in ones, and check whether -lm is needed or not. Only if the standard library doesn't define them at all, use the static workarounds in value.c. Checking for uClibc is a bit hacky, because there may still be other platforms that have the same problem.

I've implemented this with Autoconf in 5639db6bea63bcccb9c6847b20486dc1178e5cbb. It assumes that either both of the functions of neither of them is found. Could you test whether it works on your uClibc setup?

What comes to CMake, I don't know how much work it would be to implement the same tests (i.e. linking first without -lm, then linking with -lm, and finally falling back to defining JANSSON_HAVE_ISNAN_ISINF to 0). If it's not possible, then checking for uClibc is good enough.

akheron avatar Aug 15 '13 06:08 akheron

Yea that's a better solution.

However, your test for autoconf will not detect the need for -lm properly. I'm not entirely sure why this is, but redoing your tests with CMake the test passes without -lm for both uclibc and normal libc. However, using CMakes own check_function_exists macro will fail to find isnan for uclibc.

That test is implemented by trying to compile the following file:

char isnan();
int main(int argc, char **argv)
{
  isnan();
  return 0;
}

This gives the following outputs:

# Normal gcc
$ gcc tut.c
tut.c:1:6: warning: conflicting types for built-in function âisnanâ [enabled by default]

# uclibc without linking
$ /opt/toolchains/uclibc-crosstools-gcc-4.4.2-1/usr/bin/mips-linux-uclibc-gcc tut.c  tut.c:1: warning: conflicting types for built-in function 'isnan'
/tmp/cciyWOhq.o: In function `main':
tut.c:(.text+0x24): undefined reference to `isnan'
collect2: ld returned 1 exit status

# uclibc gcc linking with -lm
$ /opt/toolchains/uclibc-crosstools-gcc-4.4.2-1/usr/bin/mips-linux-uclibc-gcc -lm tut.c
tut.c:1: warning: conflicting types for built-in function 'isnan'

Also including math.h in that code would bring in the macro defined version for isnan, and you get the following:

$ gcc tut.c
tut.c:3:6: error: expected identifier or â(â before âsizeofâ
tut.c: In function âmainâ:
tut.c:7:2: error: expected expression before â)â token

So doing it like CMake does it seems to be the best strategy. I rewrote your autoconf test code for CMake, but doing the detection in this other way instead, and it successfully detects the different cases.

I will do a new pull request for that, since I forked from your new branch.

JoakimSoderberg avatar Aug 15 '13 09:08 JoakimSoderberg

If you #include <math.h>, you must not declare isnan anymore. So your test program should look like this:

#include <math.h>
int main(int argc, char **argv)
{
  isnan(0.1);
  return 0;
}

If you declare it again like this:

#include <math.h>
char isnan();

the declaration gets converted by the preprocessor to something involving sizeof (remember how the macro was defined in the uclibc-clang sources).

The only correct way to check whether a program using isnan() compiles and links is to write such a program and compile and link it :)

If you don't #include <math.h>, isnan() won't get converted to __isnan() by C preprocessor macro expansion, and you get an undefined reference because the function's real name in uClibc is __isnan, not isnan. This is exactly why I had to do it so complicatedly in Autoconf. There are helper macros in Autoconf to search for functions from libraries like your test program does above, but they don't work in this case.

So:

  1. Try to link the above program without -lm.
  2. Try to link it with -lm
  3. Use the static workaround functions in value.c

akheron avatar Aug 15 '13 10:08 akheron

I did the 3 tests with your test code, it works on both uclibc and normal libc. I did the same 3 tests with my version, it does not link on uclibc, but works fine with normal gcc/libc.

I'm aware of the stuff you said on how it works, you misunderstood what I meant. I know I cannot include math.h that way, that was my point :)

This test will compile fine with uclibc:

#include <math.h>
int main(int argc, char **argv)
{
  isnan(0.1);
  return 0;
}

There is no link error. It does not detect the problem.

If I however do it like CMakes built-in script does it:

char isnan();
int main(int argc, char **argv)
{
  isnan();
  return 0;
}

it will cause a link error on uclibc, but not on normal libc. The lib itself compiles and links fine, but when trying to link against the lib from another program you will get the link error:

Scanning dependencies of target jansson
[  4%] Building C object CMakeFiles/jansson.dir/src/memory.c.o
[  8%] Building C object CMakeFiles/jansson.dir/src/strbuffer.c.o
[ 12%] Building C object CMakeFiles/jansson.dir/src/value.c.o
[ 16%] Building C object CMakeFiles/jansson.dir/src/hashtable.c.o
[ 20%] Building C object CMakeFiles/jansson.dir/src/dump.c.o
[ 25%] Building C object CMakeFiles/jansson.dir/src/utf.c.o
[ 29%] Building C object CMakeFiles/jansson.dir/src/error.c.o
[ 33%] Building C object CMakeFiles/jansson.dir/src/strconv.c.o
[ 37%] Building C object CMakeFiles/jansson.dir/src/load.c.o
[ 41%] Building C object CMakeFiles/jansson.dir/src/pack_unpack.c.o
Linking C static library lib/libjansson.a
[ 41%] Built target jansson
Scanning dependencies of target json_process
[ 45%] Building C object CMakeFiles/json_process.dir/test/bin/json_process.c.o
Linking C executable bin/json_process
lib/libjansson.a(value.c.o): In function `json_real':
value.c:(.text+0x2df0): undefined reference to `__isnan'
value.c:(.text+0x2e14): undefined reference to `__isinf'
lib/libjansson.a(value.c.o): In function `json_real_set':
value.c:(.text+0x2f78): undefined reference to `__isnan'
value.c:(.text+0x2f9c): undefined reference to `__isinf'
collect2: ld returned 1 exit status
make[2]: *** [bin/json_process] Error 1
make[1]: *** [CMakeFiles/json_process.dir/all] Error 2
make: *** [all] Error 2

As you see above, it's when json_process tries to link against the lib that it balks. Using your method I cannot detect this and link against libm. My new pull request fixes this.

JoakimSoderberg avatar Aug 15 '13 11:08 JoakimSoderberg

For some reason your test does not result in a method call at all:

main:
        .frame  $fp,8,$31               # vars= 0, regs= 1/0, args= 0, gp= 0
        .mask   0x40000000,-4
        .fmask  0x00000000,0
        .set    noreorder
        .set    nomacro

        addiu   $sp,$sp,-8
        sw      $fp,4($sp)
        move    $fp,$sp
        move    $2,$0
        move    $sp,$fp
        lw      $fp,4($sp)
        addiu   $sp,$sp,8
        j       $31
        nop

        .set    macro
        .set    reorder
        .end    main
        .size   main, .-main
        .ident  "GCC: (Buildroot 2010.02-git) 4.4.2"

That's the difference with the CMake way of doing it, it forces a method call:

main:
        .frame  $fp,32,$31              # vars= 0, regs= 2/0, args= 16, gp= 8
        .mask   0xc0000000,-4
        .fmask  0x00000000,0
        .set    noreorder
        .set    nomacro

        addiu   $sp,$sp,-32
        sw      $31,28($sp)
        sw      $fp,24($sp)
        move    $fp,$sp
        lui     $28,%hi(__gnu_local_gp)
        addiu   $28,$28,%lo(__gnu_local_gp)
        .cprestore      16
        lw      $2,%call16(isnan)($28) <--------------------------------------
        move    $25,$2
        jalr    $25
        nop

        lw      $28,16($fp)
        move    $2,$0
        move    $sp,$fp
        lw      $31,28($sp)
        lw      $fp,24($sp)
        addiu   $sp,$sp,32
        j       $31
        nop

        .set    macro
        .set    reorder
        .end    main
        .size   main, .-main
        .ident  "GCC: (Buildroot 2010.02-git) 4.4.2"

JoakimSoderberg avatar Aug 15 '13 11:08 JoakimSoderberg

It could be because of some optimizations. gcc "knows" that 0.1 is not NaN, and can optimize the call away. I wonder what should be the correct level of "obfuscation" to make it not optimize anything...

akheron avatar Aug 15 '13 11:08 akheron

This was with -O0 so it shouldn't optimize should it?

JoakimSoderberg avatar Aug 15 '13 11:08 JoakimSoderberg

Hmm, now I think I understand what you mean. You check whether isnan and isinf symbols are found without linking to any extra library, and if not, then whether they are found from libm. ISTM this is a wrong approac, as according to the C99 standard, isnan and isinf should be defined as macros and need not have a symbol in any library.

The GNU libc might define both the macros and the symbols, I'm not sure, and that's why your test program links correctly with GNU libc.

What I tried to achieve with the #include <math.h> test program was to detect whether the macros in turn use some other symbols (like it seems to be the case in your uClibc version), or whether the macros don't exist at all (in which case we should define our own workarounds).

akheron avatar Aug 15 '13 11:08 akheron

With -O0 it doesn't optimize, and if it still doesn't call any functions then they are "pure" macros.

akheron avatar Aug 15 '13 12:08 akheron