diff mbox

[fix,for,2.1] makefile: Fix tools compile

Message ID 1404179460-29303-1-git-send-email-aik@ozlabs.ru
State New
Headers show

Commit Message

Alexey Kardashevskiy July 1, 2014, 1:51 a.m. UTC
The existing test whether "-lm" needs to be included or not is
insufficient as it reports false negative on Fedora20/ppc64.
As the result, qemu-nbd/qemu-io/qemu-img tools cannot compile.

This replaces sin() with log() in the test.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

---

The bug was triggered by efc6de0d0eb21bbd8fbc90da1faf7dd8ed9f5321
"block/iscsi: handle BUSY condition" which does not anything wrong :)

It compiled well on Fedora19 though what is weird.

Is log() good enough or we need to test for both?

Thanks.


ps. this is my test on the system upgraded from fc19 yesterday:

[aik@vpl2 ~]$ cat b.c
#include <math.h>
int main(void) { return isnan(log(0.0)); }

[aik@vpl2 ~]$ gcc b.c -o b
/tmp/ccqp1EI4.o: In function `main':
b.c:(.text+0x20): undefined reference to `log'
collect2: error: ld returned 1 exit status
[aik@vpl2 ~]$ gcc b.c -o b -lm
[aik@vpl2 ~]$ cat /etc/issue
Fedora release 20 (Heisenbug)
Kernel \r on an \m (\l)


---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Alexey Kardashevskiy July 1, 2014, 5:42 a.m. UTC | #1
On 07/01/2014 11:51 AM, Alexey Kardashevskiy wrote:
> The existing test whether "-lm" needs to be included or not is
> insufficient as it reports false negative on Fedora20/ppc64.
> As the result, qemu-nbd/qemu-io/qemu-img tools cannot compile.
> 
> This replaces sin() with log() in the test.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> ---
> 
> The bug was triggered by efc6de0d0eb21bbd8fbc90da1faf7dd8ed9f5321
> "block/iscsi: handle BUSY condition" which does not anything wrong :)
> 
> It compiled well on Fedora19 though what is weird.
> 
> Is log() good enough or we need to test for both?
> 
> Thanks.
> 
> 
> ps. this is my test on the system upgraded from fc19 yesterday:
> 
> [aik@vpl2 ~]$ cat b.c
> #include <math.h>
> int main(void) { return isnan(log(0.0)); }
> 
> [aik@vpl2 ~]$ gcc b.c -o b
> /tmp/ccqp1EI4.o: In function `main':
> b.c:(.text+0x20): undefined reference to `log'
> collect2: error: ld returned 1 exit status
> [aik@vpl2 ~]$ gcc b.c -o b -lm
> [aik@vpl2 ~]$ cat /etc/issue
> Fedora release 20 (Heisenbug)
> Kernel \r on an \m (\l)
> 
> 
> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 23ecb37..c7a2922 100755
> --- a/configure
> +++ b/configure
> @@ -3453,7 +3453,7 @@ fi
>  # Do we need libm
>  cat > $TMPC << EOF
>  #include <math.h>
> -int main(void) { return isnan(sin(0.0)); }
> +int main(void) { return isnan(log(0.0)); }


This is wrong actually. The problem here that compiler knows how to
optimize constants. sin(0.0) is the one while log(0.0) is not (it is
supposed to throw error or something as it the result is infinity).

So the correct test here could be:
int main(void) { volatile double x = 1; return isnan(sin(x)); }

But I am afraid pretty soon compilers will learn how to optimize this as
well :)

So - what would be the right fix there? Always add "-lm" (on ppc64?)?



>  EOF
>  if compile_prog "" "" ; then
>    :
>
Paolo Bonzini July 1, 2014, 6:52 a.m. UTC | #2
Il 01/07/2014 07:42, Alexey Kardashevskiy ha scritto:
> This is wrong actually. The problem here that compiler knows how to
> optimize constants. sin(0.0) is the one while log(0.0) is not (it is
> supposed to throw error or something as it the result is infinity).
>
> So the correct test here could be:
> int main(void) { volatile double x = 1; return isnan(sin(x)); }
>
> But I am afraid pretty soon compilers will learn how to optimize this as
> well :)

I think something like "double x; int f(void) {return isnan(sin(x));}" 
should be bullet proof.

Paolo
Peter Maydell July 1, 2014, 8:19 a.m. UTC | #3
On 1 July 2014 07:52, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 01/07/2014 07:42, Alexey Kardashevskiy ha scritto:
>
>> This is wrong actually. The problem here that compiler knows how to
>> optimize constants. sin(0.0) is the one while log(0.0) is not (it is
>> supposed to throw error or something as it the result is infinity).
>>
>> So the correct test here could be:
>> int main(void) { volatile double x = 1; return isnan(sin(x)); }
>>
>> But I am afraid pretty soon compilers will learn how to optimize this as
>> well :)
>
>
> I think something like "double x; int f(void) {return isnan(sin(x));}"
> should be bullet proof.

This is a compile_prog test, though -- the compiler could spot
that x and f are both unused, since it has the entire program
in hand. My suggestion would be:

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

thanks
-- PMM
Paolo Bonzini July 1, 2014, 8:26 a.m. UTC | #4
Il 01/07/2014 10:19, Peter Maydell ha scritto:
>> > I think something like "double x; int f(void) {return isnan(sin(x));}"
>> > should be bullet proof.
> This is a compile_prog test, though -- the compiler could spot
> that x and f are both unused, since it has the entire program
> in hand. My suggestion would be:
>
> int main(int argc, char **argv) { return isnan(sin((double)argc)); }

Ok, v2 of the pull request is on the way.

Paolo
diff mbox

Patch

diff --git a/configure b/configure
index 23ecb37..c7a2922 100755
--- a/configure
+++ b/configure
@@ -3453,7 +3453,7 @@  fi
 # Do we need libm
 cat > $TMPC << EOF
 #include <math.h>
-int main(void) { return isnan(sin(0.0)); }
+int main(void) { return isnan(log(0.0)); }
 EOF
 if compile_prog "" "" ; then
   :