| Message ID | 1453740558-16303-3-git-send-email-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
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
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
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 <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 <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 --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
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(+) -- 2.7.0