| Message ID | 1453976119-24372-3-git-send-email-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 28/01/2016 11:15, Alex Bennée wrote: > index d0de2d4..d30532f 100644 > --- a/Makefile > +++ b/Makefile > @@ -329,9 +329,9 @@ qemu-ga: qemu-ga$(EXESUF) $(QGA_VSS_PROVIDER) $(QEMU_GA_MSI) > endif > > ivshmem-client$(EXESUF): $(ivshmem-client-obj-y) > - $(call LINK, $^) > + $(call LINK, $^, $(ldflags)) > ivshmem-server$(EXESUF): $(ivshmem-server-obj-y) libqemuutil.a libqemustub.a > - $(call LINK, $^) > + $(call LINK, $^, $(ldflags)) > > clean: > # avoid old build problems by removing potentially incorrect old files This seems spurious? There's no $2 in the LINK macro. > diff --git a/configure b/configure > index bd29ba7..148b79a 100755 > --- a/configure > +++ b/configure > @@ -5871,7 +5871,7 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then > ldflags="$ldflags $textseg_ldflags" > fi > > -echo "LDFLAGS+=$ldflags" >> $config_target_mak > +echo "LDFLAGS+=$ldflags" >> $config_host_mak > echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak > > done # for target in $targets > -- 2.7.0 This is good. Paolo
On 28/01/2016 11:15, Alex Bennée wrote: > diff --git a/configure b/configure > index bd29ba7..148b79a 100755 > --- a/configure > +++ b/configure > @@ -5871,7 +5871,7 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then > ldflags="$ldflags $textseg_ldflags" > fi > > -echo "LDFLAGS+=$ldflags" >> $config_target_mak > +echo "LDFLAGS+=$ldflags" >> $config_host_mak > echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak > > done # for target in $targets Hmm wait, it's not okay. This adds the *target* LDFLAGS to config-host.mak, and adds them a zillion times. extra-ldflags is already added to LDFLAGS in config-host.mak: --extra-ldflags=*) LDFLAGS="$LDFLAGS $optarg" EXTRA_LDFLAGS="$optarg" ;; ... echo "LDFLAGS=$LDFLAGS" >> $config_host_mak So I'm totally confused as to what this patch is trying to achieve... Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > On 28/01/2016 11:15, Alex Bennée wrote: >> diff --git a/configure b/configure >> index bd29ba7..148b79a 100755 >> --- a/configure >> +++ b/configure >> @@ -5871,7 +5871,7 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then >> ldflags="$ldflags $textseg_ldflags" >> fi >> >> -echo "LDFLAGS+=$ldflags" >> $config_target_mak >> +echo "LDFLAGS+=$ldflags" >> $config_host_mak >> echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak >> >> done # for target in $targets > > Hmm wait, it's not okay. > > This adds the *target* LDFLAGS to config-host.mak, and adds them a > zillion times. extra-ldflags is already added to LDFLAGS in > config-host.mak: > > --extra-ldflags=*) LDFLAGS="$LDFLAGS $optarg" > EXTRA_LDFLAGS="$optarg" > ;; > > ... > > echo "LDFLAGS=$LDFLAGS" >> $config_host_mak > > So I'm totally confused as to what this patch is trying to achieve... It seems so was I. So I was having problems with ancillary binaries failing to link against tsan but as you point out this should work with "-fsantiize=thread" in the ldflags which are already available to config_host.mak On my Gentoo (GCC 4.9) system without this I can build with: ./configure ${TARGETS} --extra-cflags="-fsanitize=thread -fPIE" \ --extra-ldflags="-pie -fsanitize=thread" --with-coroutine=gthread Although I get make check failures: GTESTER tests/check-qdict FATAL: ThreadSanitizer can not mmap the shadow memory (something is mapped at 0x555555554000 < 0x7cf000000000) FATAL: Make sure to compile with -fPIE and to link with -pie. /home/alex/lsrc/qemu/qemu.git/tests/Makefile:629: recipe for target 'check-tests/check-qdict' failed make: *** [check-tests/check-qdict] Error 1 But I suspect this is possibly an ASLR issue. I think this patch can be dropped altogether. With the other patches can you build with tsan the proper way? What are you running? I'll add it to the VMs I have to double check. -- Alex Bennée
diff --git a/Makefile b/Makefile index d0de2d4..d30532f 100644 --- a/Makefile +++ b/Makefile @@ -329,9 +329,9 @@ qemu-ga: qemu-ga$(EXESUF) $(QGA_VSS_PROVIDER) $(QEMU_GA_MSI) endif ivshmem-client$(EXESUF): $(ivshmem-client-obj-y) - $(call LINK, $^) + $(call LINK, $^, $(ldflags)) ivshmem-server$(EXESUF): $(ivshmem-server-obj-y) libqemuutil.a libqemustub.a - $(call LINK, $^) + $(call LINK, $^, $(ldflags)) clean: # avoid old build problems by removing potentially incorrect old files diff --git a/configure b/configure index bd29ba7..148b79a 100755 --- a/configure +++ b/configure @@ -5871,7 +5871,7 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then ldflags="$ldflags $textseg_ldflags" fi -echo "LDFLAGS+=$ldflags" >> $config_target_mak +echo "LDFLAGS+=$ldflags" >> $config_host_mak echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak done # for target in $targets
If for example you want to use the thread sanitizer you want to ensure all binaries have the extra linker flags applied: ./configure ${TARGETS} --cc=gcc-5 --cxx=g++-5 \ --extra-cflags="-fsanitize=thread" --extra-ldflags="-fsanitize=thread" Or possibly: ./configure ${TARGETS} --extra-cflags="-fsanitize=thread -fPIC" \ --extra-ldflags="-fsanitize=thread -pie" Unlike CFLAGS we don't have a QEMU_LDFLAGS which would be appropriate if we only ever needed to affect ${TARGET} binaries. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- v1 - new for v1 - --extra-ldflags alternative to -ltsan - yet another example invocation (gcc 4.9/Gentoo) --- Makefile | 4 ++-- configure | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)