diff mbox

[RFC,2/4] configure: introduce --extra-libs

Message ID 1453740558-16303-3-git-send-email-alex.bennee@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Bennée Jan. 25, 2016, 4:49 p.m. UTC
If for example you want to use the thread sanitizer you want to ensure all
binaries are linked with the library:

  ./configure ${TARGETS} --cc=gcc-5 --cxx=g++-5 \
    --extra-cflags="-fsanitize=thread" --extra-libs="-ltsan"

This is more explicit than just specifying --extra-ldflags which might
not get applied in the right place all the time.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 configure | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Peter Maydell Jan. 25, 2016, 5:08 p.m. UTC | #1
On 25 January 2016 at 16:49, Alex Bennée <alex.bennee@linaro.org> wrote:
> If for example you want to use the thread sanitizer you want to ensure all
> binaries are linked with the library:
>
>   ./configure ${TARGETS} --cc=gcc-5 --cxx=g++-5 \
>     --extra-cflags="-fsanitize=thread" --extra-libs="-ltsan"
>
> This is more explicit than just specifying --extra-ldflags which might
> not get applied in the right place all the time.

When would they differ? The commit message makes this sound like
it's working around a bug in the code that uses LDFLAGS...

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  configure | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/configure b/configure
> index 7d23c6c..194bae9 100755
> --- a/configure
> +++ b/configure
> @@ -365,6 +365,8 @@ for opt do
>    --extra-ldflags=*) LDFLAGS="$LDFLAGS $optarg"
>                       EXTRA_LDFLAGS="$optarg"
>    ;;
> +  --extra-libs=*)    EXTRA_LIBS="$optarg"
> +  ;;
>    --enable-debug-info) debug_info="yes"
>    ;;
>    --disable-debug-info) debug_info="no"

The reason for having this special case to set the variables for
CC related things early is to ensure that they apply for all
compilations including any that get done very early in configure...

> @@ -785,6 +787,8 @@ for opt do
>    ;;
>    --extra-ldflags=*)
>    ;;
> +  --extra-libs=*)
> +  ;;
>    --enable-debug-info)
>    ;;
>    --disable-debug-info)
> @@ -1281,6 +1285,7 @@ Advanced options (experts only):
>    --objcc=OBJCC            use Objective-C compiler OBJCC [$objcc]
>    --extra-cflags=CFLAGS    append extra C compiler flags QEMU_CFLAGS
>    --extra-ldflags=LDFLAGS  append extra linker flags LDFLAGS
> +  --extra-libs=LIBS        append extra libraries when linking
>    --make=MAKE              use specified make [$make]
>    --install=INSTALL        use specified install [$install]
>    --python=PYTHON          use specified python [$python]
> @@ -4718,6 +4723,11 @@ libs_softmmu="$pixman_libs $libs_softmmu"
>  CFLAGS="$CFLAGS $EXTRA_CFLAGS"
>  QEMU_CFLAGS="$QEMU_CFLAGS $EXTRA_CFLAGS"
>
> +# extra-libs
> +LIBS="$LIBS $EXTRA_LIBS"

...but if you don't set LIBS until way down here there's no
point in defining EXTRA_LIBS early.

We should handle all the compiler-option related flags the
same way, I think, for consistency.

> +libs_softmmu="$libs_softmmu $EXTRA_LIBS"
> +libs_qga="$libs_qga $EXTRA_LIBS"
> +
>  echo "Install prefix    $prefix"
>  echo "BIOS directory    `eval echo $qemu_datadir`"
>  echo "binary directory  `eval echo $bindir`"
> @@ -4888,6 +4898,7 @@ fi
>  echo "qemu_helperdir=$libexecdir" >> $config_host_mak
>  echo "extra_cflags=$EXTRA_CFLAGS" >> $config_host_mak
>  echo "extra_ldflags=$EXTRA_LDFLAGS" >> $config_host_mak
> +echo "extra_libs=$EXTRA_LIBS" >> $config_host_mak
>  echo "qemu_localedir=$qemu_localedir" >> $config_host_mak
>  echo "libs_softmmu=$libs_softmmu" >> $config_host_mak

thanks
-- PMM
Alex Bennée Jan. 25, 2016, 5:25 p.m. UTC | #2
Peter Maydell <peter.maydell@linaro.org> writes:

> On 25 January 2016 at 16:49, Alex Bennée <alex.bennee@linaro.org> wrote:
>> If for example you want to use the thread sanitizer you want to ensure all
>> binaries are linked with the library:
>>
>>   ./configure ${TARGETS} --cc=gcc-5 --cxx=g++-5 \
>>     --extra-cflags="-fsanitize=thread" --extra-libs="-ltsan"
>>
>> This is more explicit than just specifying --extra-ldflags which might
>> not get applied in the right place all the time.
>
> When would they differ? The commit message makes this sound like
> it's working around a bug in the code that uses LDFLAGS...

Well LDFLAGS doesn't get applied everywhere so with:

--cc=gcc-5 --cxx=g++-5 --extra-cflags="-fsanitize=thread"
  --extra-ldflags="-ltsan" --with-coroutine=gthread

You get compile failures in the ancillary binaries that are used during
testing.

  LINK  tests/qemu-iotests/socket_scm_helper
tests/qemu-iotests/socket_scm_helper.o: In function `main':
/home/alex/lsrc/qemu/qemu.git/tests/qemu-iotests/socket_scm_helper.c:95: undefined reference to `__tsan_func_entry'
/home/alex/lsrc/qemu/qemu.git/tests/qemu-iotests/socket_scm_helper.c:106: undefined reference to `__tsan_read8'
/home/alex/lsrc/qemu/qemu.git/tests/qemu-iotests/socket_scm_helper.c:106: undefined reference to `__tsan_read8'
...

I think this stems from my confusion from exactly which binaries are
meant to be affected by which flags. QEMU_CFLAGS seems to imply it's
QEMU only, but we don't have a QEMU_LDFLAGS so should LDFLAGS be shared
with all binaries we build?

--
Alex Bennée
Peter Maydell Jan. 25, 2016, 5:36 p.m. UTC | #3
On 25 January 2016 at 17:25, Alex Bennée <alex.bennee@linaro.org> wrote:
> Well LDFLAGS doesn't get applied everywhere so with:
>
> --cc=gcc-5 --cxx=g++-5 --extra-cflags="-fsanitize=thread"
>   --extra-ldflags="-ltsan" --with-coroutine=gthread
>
> You get compile failures in the ancillary binaries that are used during
> testing.
>
>   LINK  tests/qemu-iotests/socket_scm_helper
> tests/qemu-iotests/socket_scm_helper.o: In function `main':
> /home/alex/lsrc/qemu/qemu.git/tests/qemu-iotests/socket_scm_helper.c:95: undefined reference to `__tsan_func_entry'
> /home/alex/lsrc/qemu/qemu.git/tests/qemu-iotests/socket_scm_helper.c:106: undefined reference to `__tsan_read8'
> /home/alex/lsrc/qemu/qemu.git/tests/qemu-iotests/socket_scm_helper.c:106: undefined reference to `__tsan_read8'
> ...

That seems like a bug -- we should be applying LDFLAGS there
I think. (Consider that we put things like -g and -m32 there.)

> I think this stems from my confusion from exactly which binaries are
> meant to be affected by which flags. QEMU_CFLAGS seems to imply it's
> QEMU only, but we don't have a QEMU_LDFLAGS so should LDFLAGS be shared
> with all binaries we build?

Back in 2012 and commit caa50971f2e the distinction was apparently
that QEMU_CFLAGS was for flags without which QEMU can't compile,
whereas CFLAGS was for flags like "-g -O2" which the user can
safely override. This may even still be true :-)

Given that I suspect the reason we don't have a QEMU_LDFLAGS is
that users are less in the habit of trying to run make with a
hand-specified CFLAGS.

thanks
-- PMM
Paolo Bonzini Jan. 25, 2016, 5:58 p.m. UTC | #4
On 25/01/2016 17:49, Alex Bennée wrote:
> If for example you want to use the thread sanitizer you want to ensure all
> binaries are linked with the library:
> 
>   ./configure ${TARGETS} --cc=gcc-5 --cxx=g++-5 \
>     --extra-cflags="-fsanitize=thread" --extra-libs="-ltsan"

Shouldn't -fsanitize=thread work as a linker command line flag too?
Perhaps if that works there's a better place to put -fsanitizer flags.

Paolo

> This is more explicit than just specifying --extra-ldflags which might
> not get applied in the right place all the time.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  configure | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/configure b/configure
> index 7d23c6c..194bae9 100755
> --- a/configure
> +++ b/configure
> @@ -365,6 +365,8 @@ for opt do
>    --extra-ldflags=*) LDFLAGS="$LDFLAGS $optarg"
>                       EXTRA_LDFLAGS="$optarg"
>    ;;
> +  --extra-libs=*)    EXTRA_LIBS="$optarg"
> +  ;;
>    --enable-debug-info) debug_info="yes"
>    ;;
>    --disable-debug-info) debug_info="no"
> @@ -785,6 +787,8 @@ for opt do
>    ;;
>    --extra-ldflags=*)
>    ;;
> +  --extra-libs=*)
> +  ;;
>    --enable-debug-info)
>    ;;
>    --disable-debug-info)
> @@ -1281,6 +1285,7 @@ Advanced options (experts only):
>    --objcc=OBJCC            use Objective-C compiler OBJCC [$objcc]
>    --extra-cflags=CFLAGS    append extra C compiler flags QEMU_CFLAGS
>    --extra-ldflags=LDFLAGS  append extra linker flags LDFLAGS
> +  --extra-libs=LIBS        append extra libraries when linking
>    --make=MAKE              use specified make [$make]
>    --install=INSTALL        use specified install [$install]
>    --python=PYTHON          use specified python [$python]
> @@ -4718,6 +4723,11 @@ libs_softmmu="$pixman_libs $libs_softmmu"
>  CFLAGS="$CFLAGS $EXTRA_CFLAGS"
>  QEMU_CFLAGS="$QEMU_CFLAGS $EXTRA_CFLAGS"
>  
> +# extra-libs
> +LIBS="$LIBS $EXTRA_LIBS"
> +libs_softmmu="$libs_softmmu $EXTRA_LIBS"
> +libs_qga="$libs_qga $EXTRA_LIBS"
> +
>  echo "Install prefix    $prefix"
>  echo "BIOS directory    `eval echo $qemu_datadir`"
>  echo "binary directory  `eval echo $bindir`"
> @@ -4888,6 +4898,7 @@ fi
>  echo "qemu_helperdir=$libexecdir" >> $config_host_mak
>  echo "extra_cflags=$EXTRA_CFLAGS" >> $config_host_mak
>  echo "extra_ldflags=$EXTRA_LDFLAGS" >> $config_host_mak
> +echo "extra_libs=$EXTRA_LIBS" >> $config_host_mak
>  echo "qemu_localedir=$qemu_localedir" >> $config_host_mak
>  echo "libs_softmmu=$libs_softmmu" >> $config_host_mak
>  
>
Alex Bennée Jan. 25, 2016, 6:15 p.m. UTC | #5
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 25/01/2016 17:49, Alex Bennée wrote:
>> If for example you want to use the thread sanitizer you want to ensure all
>> binaries are linked with the library:
>>
>>   ./configure ${TARGETS} --cc=gcc-5 --cxx=g++-5 \
>>     --extra-cflags="-fsanitize=thread" --extra-libs="-ltsan"
>
> Shouldn't -fsanitize=thread work as a linker command line flag too?

No, the sanitizers are compile time options as they instrument the
generated code. It's just in the case of the ThreadSanitizer you also
need the support library.

> Perhaps if that works there's a better place to put -fsanitizer flags.
>
> Paolo
>
>> This is more explicit than just specifying --extra-ldflags which might
>> not get applied in the right place all the time.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  configure | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/configure b/configure
>> index 7d23c6c..194bae9 100755
>> --- a/configure
>> +++ b/configure
>> @@ -365,6 +365,8 @@ for opt do
>>    --extra-ldflags=*) LDFLAGS="$LDFLAGS $optarg"
>>                       EXTRA_LDFLAGS="$optarg"
>>    ;;
>> +  --extra-libs=*)    EXTRA_LIBS="$optarg"
>> +  ;;
>>    --enable-debug-info) debug_info="yes"
>>    ;;
>>    --disable-debug-info) debug_info="no"
>> @@ -785,6 +787,8 @@ for opt do
>>    ;;
>>    --extra-ldflags=*)
>>    ;;
>> +  --extra-libs=*)
>> +  ;;
>>    --enable-debug-info)
>>    ;;
>>    --disable-debug-info)
>> @@ -1281,6 +1285,7 @@ Advanced options (experts only):
>>    --objcc=OBJCC            use Objective-C compiler OBJCC [$objcc]
>>    --extra-cflags=CFLAGS    append extra C compiler flags QEMU_CFLAGS
>>    --extra-ldflags=LDFLAGS  append extra linker flags LDFLAGS
>> +  --extra-libs=LIBS        append extra libraries when linking
>>    --make=MAKE              use specified make [$make]
>>    --install=INSTALL        use specified install [$install]
>>    --python=PYTHON          use specified python [$python]
>> @@ -4718,6 +4723,11 @@ libs_softmmu="$pixman_libs $libs_softmmu"
>>  CFLAGS="$CFLAGS $EXTRA_CFLAGS"
>>  QEMU_CFLAGS="$QEMU_CFLAGS $EXTRA_CFLAGS"
>>
>> +# extra-libs
>> +LIBS="$LIBS $EXTRA_LIBS"
>> +libs_softmmu="$libs_softmmu $EXTRA_LIBS"
>> +libs_qga="$libs_qga $EXTRA_LIBS"
>> +
>>  echo "Install prefix    $prefix"
>>  echo "BIOS directory    `eval echo $qemu_datadir`"
>>  echo "binary directory  `eval echo $bindir`"
>> @@ -4888,6 +4898,7 @@ fi
>>  echo "qemu_helperdir=$libexecdir" >> $config_host_mak
>>  echo "extra_cflags=$EXTRA_CFLAGS" >> $config_host_mak
>>  echo "extra_ldflags=$EXTRA_LDFLAGS" >> $config_host_mak
>> +echo "extra_libs=$EXTRA_LIBS" >> $config_host_mak
>>  echo "qemu_localedir=$qemu_localedir" >> $config_host_mak
>>  echo "libs_softmmu=$libs_softmmu" >> $config_host_mak
>>
>>


--
Alex Bennée
Paolo Bonzini Jan. 25, 2016, 10:10 p.m. UTC | #6
On 25/01/2016 19:15, Alex Bennée wrote:
> 
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 25/01/2016 17:49, Alex Bennée wrote:
>>> If for example you want to use the thread sanitizer you want to ensure all
>>> binaries are linked with the library:
>>>
>>>   ./configure ${TARGETS} --cc=gcc-5 --cxx=g++-5 \
>>>     --extra-cflags="-fsanitize=thread" --extra-libs="-ltsan"
>>
>> Shouldn't -fsanitize=thread work as a linker command line flag too?
> 
> No, the sanitizers are compile time options as they instrument the
> generated code. It's just in the case of the ThreadSanitizer you also
> need the support library.

That's certainly not the case.  My system has at least a libubsan, 
libasan and liblsan (in addition to libtsan), and "gcc -dumpspecs"
suggests that the -fsanitize options are also valid at link time:

   %{%:sanitize(address):%{!shared:libasan_preinit%O%s} %{static-libasan:%{!shared:-Bstatic --whole-archive -lasan --no-whole-archive -Bdynamic}}%{!static-libasan:-lasan}}
   %{%:sanitize(thread):%{static-libtsan:%{!shared:-Bstatic --whole-archive -ltsan --no-whole-archive -Bdynamic}}%{!static-libtsan:-ltsan}}
   %{%:sanitize(leak):%{static-liblsan:%{!shared:-Bstatic --whole-archive -llsan --no-whole-archive -Bdynamic}}%{!static-liblsan:-llsan}}

(GCC specs are what they are, but you get the idea).

Paolo
Alex Bennée Jan. 26, 2016, 8:43 a.m. UTC | #7
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 25/01/2016 19:15, Alex Bennée wrote:
>>
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>
>>> On 25/01/2016 17:49, Alex Bennée wrote:
>>>> If for example you want to use the thread sanitizer you want to ensure all
>>>> binaries are linked with the library:
>>>>
>>>>   ./configure ${TARGETS} --cc=gcc-5 --cxx=g++-5 \
>>>>     --extra-cflags="-fsanitize=thread" --extra-libs="-ltsan"
>>>
>>> Shouldn't -fsanitize=thread work as a linker command line flag too?
>>
>> No, the sanitizers are compile time options as they instrument the
>> generated code. It's just in the case of the ThreadSanitizer you also
>> need the support library.
>
> That's certainly not the case.  My system has at least a libubsan,
> libasan and liblsan (in addition to libtsan), and "gcc -dumpspecs"
> suggests that the -fsanitize options are also valid at link time:
>
>    %{%:sanitize(address):%{!shared:libasan_preinit%O%s} %{static-libasan:%{!shared:-Bstatic --whole-archive -lasan --no-whole-archive -Bdynamic}}%{!static-libasan:-lasan}}
>    %{%:sanitize(thread):%{static-libtsan:%{!shared:-Bstatic --whole-archive -ltsan --no-whole-archive -Bdynamic}}%{!static-libtsan:-ltsan}}
>    %{%:sanitize(leak):%{static-liblsan:%{!shared:-Bstatic --whole-archive -llsan --no-whole-archive -Bdynamic}}%{!static-liblsan:-llsan}}
>
> (GCC specs are what they are, but you get the idea).

Hmm odd. I ran the undefined and address sanitizers without having to
mess with the ldflags. I'll have a deeper dive into the docs to see
whats going on.

>
> Paolo


--
Alex Bennée
diff mbox

Patch

diff --git a/configure b/configure
index 7d23c6c..194bae9 100755
--- a/configure
+++ b/configure
@@ -365,6 +365,8 @@  for opt do
   --extra-ldflags=*) LDFLAGS="$LDFLAGS $optarg"
                      EXTRA_LDFLAGS="$optarg"
   ;;
+  --extra-libs=*)    EXTRA_LIBS="$optarg"
+  ;;
   --enable-debug-info) debug_info="yes"
   ;;
   --disable-debug-info) debug_info="no"
@@ -785,6 +787,8 @@  for opt do
   ;;
   --extra-ldflags=*)
   ;;
+  --extra-libs=*)
+  ;;
   --enable-debug-info)
   ;;
   --disable-debug-info)
@@ -1281,6 +1285,7 @@  Advanced options (experts only):
   --objcc=OBJCC            use Objective-C compiler OBJCC [$objcc]
   --extra-cflags=CFLAGS    append extra C compiler flags QEMU_CFLAGS
   --extra-ldflags=LDFLAGS  append extra linker flags LDFLAGS
+  --extra-libs=LIBS        append extra libraries when linking
   --make=MAKE              use specified make [$make]
   --install=INSTALL        use specified install [$install]
   --python=PYTHON          use specified python [$python]
@@ -4718,6 +4723,11 @@  libs_softmmu="$pixman_libs $libs_softmmu"
 CFLAGS="$CFLAGS $EXTRA_CFLAGS"
 QEMU_CFLAGS="$QEMU_CFLAGS $EXTRA_CFLAGS"
 
+# extra-libs
+LIBS="$LIBS $EXTRA_LIBS"
+libs_softmmu="$libs_softmmu $EXTRA_LIBS"
+libs_qga="$libs_qga $EXTRA_LIBS"
+
 echo "Install prefix    $prefix"
 echo "BIOS directory    `eval echo $qemu_datadir`"
 echo "binary directory  `eval echo $bindir`"
@@ -4888,6 +4898,7 @@  fi
 echo "qemu_helperdir=$libexecdir" >> $config_host_mak
 echo "extra_cflags=$EXTRA_CFLAGS" >> $config_host_mak
 echo "extra_ldflags=$EXTRA_LDFLAGS" >> $config_host_mak
+echo "extra_libs=$EXTRA_LIBS" >> $config_host_mak
 echo "qemu_localedir=$qemu_localedir" >> $config_host_mak
 echo "libs_softmmu=$libs_softmmu" >> $config_host_mak