[libvirt] [PATCH] build: fix linking libqemutestdriver with LTO enabled
openSUSE Factory is in the process of enabling Link Time Optimization [0] and stumbled upon missing symbols when linking libqemutestdriver libtool: link: gcc -shared -fPIC -DPIC ../src/libvirt_qemu_probes.o \ -Wl,--whole-archive ../src/.libs/libvirt_driver_qemu_impl.a \ ../src/.libs/libvirt_driver_network_impl.a \ ../src/.libs/libvirt_driver_storage_impl.a -Wl,--no-whole-archive \ -lcap-ng -lgnutls -lnl-route-3 -lnl-3 -ldbus-1 -lselinux -lapparmor \ /usr/lib/libxml2.so -ldl -lz -llzma -lm -lblkid -ltirpc \ -fstack-protector-strong -O2 -fstack-protector-strong -flto=16 -g -O2 \ -fstack-protector-strong -flto=16 -g \ -Wl,-soname -Wl,libqemutestdriver.so -o .libs/libqemutestdriver.so ... /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `mdir_name' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `rpl_ioctl' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `rpl_pipe2' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `recvfd' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `c_strcasecmp' The missing symbols are provided by GNUlib. Add it when linking libqemutestdriver. [0] https://en.opensuse.org/openSUSE:LTO Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- tests/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 46d94d2236..1d80647c9f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -557,7 +557,7 @@ endif WITH_DTRACE_PROBES libqemutestdriver_la_SOURCES = libqemutestdriver_la_LDFLAGS = $(DRIVERLIB_LDFLAGS) -libqemutestdriver_la_LIBADD = $(qemu_LDADDS) +libqemutestdriver_la_LIBADD = $(qemu_LDADDS) $(GNULIB_LIBS) qemucpumock_la_SOURCES = \ qemucpumock.c testutilshostcpus.h @@ -729,7 +729,7 @@ lxcconf2xmltest_SOURCES = \ lxcconf2xmltest_LDADD = $(lxc_LDADDS) else ! WITH_LXC EXTRA_DIST += lxcxml2xmltest.c testutilslxc.c testutilslxc.h -endif ! WITH_LXC +-endif ! WITH_LXC if WITH_OPENVZ openvzutilstest_SOURCES = \ -- 2.21.0
On 5/29/19 7:44 PM, Jim Fehlig wrote:
openSUSE Factory is in the process of enabling Link Time Optimization [0] and stumbled upon missing symbols when linking libqemutestdriver
libtool: link: gcc -shared -fPIC -DPIC ../src/libvirt_qemu_probes.o \ -Wl,--whole-archive ../src/.libs/libvirt_driver_qemu_impl.a \ ../src/.libs/libvirt_driver_network_impl.a \ ../src/.libs/libvirt_driver_storage_impl.a -Wl,--no-whole-archive \ -lcap-ng -lgnutls -lnl-route-3 -lnl-3 -ldbus-1 -lselinux -lapparmor \ /usr/lib/libxml2.so -ldl -lz -llzma -lm -lblkid -ltirpc \ -fstack-protector-strong -O2 -fstack-protector-strong -flto=16 -g -O2 \ -fstack-protector-strong -flto=16 -g \ -Wl,-soname -Wl,libqemutestdriver.so -o .libs/libqemutestdriver.so ... /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `mdir_name' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `rpl_ioctl' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `rpl_pipe2' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `recvfd' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `c_strcasecmp'
The missing symbols are provided by GNUlib. Add it when linking libqemutestdriver.
[0] https://en.opensuse.org/openSUSE:LTO
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- tests/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am index 46d94d2236..1d80647c9f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -557,7 +557,7 @@ endif WITH_DTRACE_PROBES
libqemutestdriver_la_SOURCES = libqemutestdriver_la_LDFLAGS = $(DRIVERLIB_LDFLAGS) -libqemutestdriver_la_LIBADD = $(qemu_LDADDS) +libqemutestdriver_la_LIBADD = $(qemu_LDADDS) $(GNULIB_LIBS)
qemucpumock_la_SOURCES = \ qemucpumock.c testutilshostcpus.h
ACK and safe for freeze to this hunk. Alternatively, we might go with $(LDADDS) which includes $(GNULIB_LIBS).
@@ -729,7 +729,7 @@ lxcconf2xmltest_SOURCES = \ lxcconf2xmltest_LDADD = $(lxc_LDADDS) else ! WITH_LXC EXTRA_DIST += lxcxml2xmltest.c testutilslxc.c testutilslxc.h -endif ! WITH_LXC +-endif ! WITH_LXC
if WITH_OPENVZ openvzutilstest_SOURCES = \
Not so for this one ;-) Michal
On 5/30/19 3:08 AM, Michal Privoznik wrote:
On 5/29/19 7:44 PM, Jim Fehlig wrote:
openSUSE Factory is in the process of enabling Link Time Optimization [0] and stumbled upon missing symbols when linking libqemutestdriver
libtool: link: gcc -shared -fPIC -DPIC ../src/libvirt_qemu_probes.o \ -Wl,--whole-archive ../src/.libs/libvirt_driver_qemu_impl.a \ ../src/.libs/libvirt_driver_network_impl.a \ ../src/.libs/libvirt_driver_storage_impl.a -Wl,--no-whole-archive \ -lcap-ng -lgnutls -lnl-route-3 -lnl-3 -ldbus-1 -lselinux -lapparmor \ /usr/lib/libxml2.so -ldl -lz -llzma -lm -lblkid -ltirpc \ -fstack-protector-strong -O2 -fstack-protector-strong -flto=16 -g -O2 \ -fstack-protector-strong -flto=16 -g \ -Wl,-soname -Wl,libqemutestdriver.so -o .libs/libqemutestdriver.so ... /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `mdir_name' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `rpl_ioctl' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `rpl_pipe2' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `recvfd' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `c_strcasecmp'
The missing symbols are provided by GNUlib. Add it when linking libqemutestdriver.
[0] https://en.opensuse.org/openSUSE:LTO
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- tests/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am index 46d94d2236..1d80647c9f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -557,7 +557,7 @@ endif WITH_DTRACE_PROBES libqemutestdriver_la_SOURCES = libqemutestdriver_la_LDFLAGS = $(DRIVERLIB_LDFLAGS) -libqemutestdriver_la_LIBADD = $(qemu_LDADDS) +libqemutestdriver_la_LIBADD = $(qemu_LDADDS) $(GNULIB_LIBS) qemucpumock_la_SOURCES = \ qemucpumock.c testutilshostcpus.h
ACK and safe for freeze to this hunk. Alternatively, we might go with $(LDADDS) which includes $(GNULIB_LIBS).
Do you have a preference? LDADDS includes some other things which AFAIK are not needed.
@@ -729,7 +729,7 @@ lxcconf2xmltest_SOURCES = \ lxcconf2xmltest_LDADD = $(lxc_LDADDS) else ! WITH_LXC EXTRA_DIST += lxcxml2xmltest.c testutilslxc.c testutilslxc.h -endif ! WITH_LXC +-endif ! WITH_LXC if WITH_OPENVZ openvzutilstest_SOURCES = \
Not so for this one ;-)
Not sure how that got in there :-). Regards, Jim
On 5/30/19 4:44 PM, Jim Fehlig wrote:
On 5/30/19 3:08 AM, Michal Privoznik wrote:
On 5/29/19 7:44 PM, Jim Fehlig wrote:
openSUSE Factory is in the process of enabling Link Time Optimization [0] and stumbled upon missing symbols when linking libqemutestdriver
libtool: link: gcc -shared -fPIC -DPIC ../src/libvirt_qemu_probes.o \ -Wl,--whole-archive ../src/.libs/libvirt_driver_qemu_impl.a \ ../src/.libs/libvirt_driver_network_impl.a \ ../src/.libs/libvirt_driver_storage_impl.a -Wl,--no-whole-archive \ -lcap-ng -lgnutls -lnl-route-3 -lnl-3 -ldbus-1 -lselinux -lapparmor \ /usr/lib/libxml2.so -ldl -lz -llzma -lm -lblkid -ltirpc \ -fstack-protector-strong -O2 -fstack-protector-strong -flto=16 -g -O2 \ -fstack-protector-strong -flto=16 -g \ -Wl,-soname -Wl,libqemutestdriver.so -o .libs/libqemutestdriver.so ... /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `mdir_name' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `rpl_ioctl' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `rpl_pipe2' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `recvfd' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `c_strcasecmp'
The missing symbols are provided by GNUlib. Add it when linking libqemutestdriver.
[0] https://en.opensuse.org/openSUSE:LTO
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- tests/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am index 46d94d2236..1d80647c9f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -557,7 +557,7 @@ endif WITH_DTRACE_PROBES libqemutestdriver_la_SOURCES = libqemutestdriver_la_LDFLAGS = $(DRIVERLIB_LDFLAGS) -libqemutestdriver_la_LIBADD = $(qemu_LDADDS) +libqemutestdriver_la_LIBADD = $(qemu_LDADDS) $(GNULIB_LIBS) qemucpumock_la_SOURCES = \ qemucpumock.c testutilshostcpus.h
ACK and safe for freeze to this hunk. Alternatively, we might go with $(LDADDS) which includes $(GNULIB_LIBS).
Do you have a preference? LDADDS includes some other things which AFAIK are not needed.
Well, other test libs use LDADDS and I'd say that LTO doesn't link anything that's not needed. But maybe I'm mistaken. So, no, I don't have any preference. Michal
On 5/30/19 8:56 AM, Michal Privoznik wrote:
On 5/30/19 4:44 PM, Jim Fehlig wrote:
On 5/30/19 3:08 AM, Michal Privoznik wrote:
On 5/29/19 7:44 PM, Jim Fehlig wrote:
openSUSE Factory is in the process of enabling Link Time Optimization [0] and stumbled upon missing symbols when linking libqemutestdriver
libtool: link: gcc -shared -fPIC -DPIC ../src/libvirt_qemu_probes.o \ -Wl,--whole-archive ../src/.libs/libvirt_driver_qemu_impl.a \ ../src/.libs/libvirt_driver_network_impl.a \ ../src/.libs/libvirt_driver_storage_impl.a -Wl,--no-whole-archive \ -lcap-ng -lgnutls -lnl-route-3 -lnl-3 -ldbus-1 -lselinux -lapparmor \ /usr/lib/libxml2.so -ldl -lz -llzma -lm -lblkid -ltirpc \ -fstack-protector-strong -O2 -fstack-protector-strong -flto=16 -g -O2 \ -fstack-protector-strong -flto=16 -g \ -Wl,-soname -Wl,libqemutestdriver.so -o .libs/libqemutestdriver.so ... /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `mdir_name' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `rpl_ioctl' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `rpl_pipe2' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `recvfd' /usr/lib/gcc/i586-suse-linux/9/../../../../i586-suse-linux/bin/ld: ./.libs/libqemutestdriver.so: undefined reference to `c_strcasecmp'
The missing symbols are provided by GNUlib. Add it when linking libqemutestdriver.
[0] https://en.opensuse.org/openSUSE:LTO
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- tests/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am index 46d94d2236..1d80647c9f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -557,7 +557,7 @@ endif WITH_DTRACE_PROBES libqemutestdriver_la_SOURCES = libqemutestdriver_la_LDFLAGS = $(DRIVERLIB_LDFLAGS) -libqemutestdriver_la_LIBADD = $(qemu_LDADDS) +libqemutestdriver_la_LIBADD = $(qemu_LDADDS) $(GNULIB_LIBS) qemucpumock_la_SOURCES = \ qemucpumock.c testutilshostcpus.h
ACK and safe for freeze to this hunk. Alternatively, we might go with $(LDADDS) which includes $(GNULIB_LIBS).
Do you have a preference? LDADDS includes some other things which AFAIK are not needed.
Well, other test libs use LDADDS and I'd say that LTO doesn't link anything that's not needed. But maybe I'm mistaken. So, no, I don't have any preference.
Most of the *qemu*test use LDADDS so I changed the patch to use it and pushed. Regards, Jim
On Thu, 2019-05-30 at 16:56 +0200, Michal Privoznik wrote:
On 5/30/19 4:44 PM, Jim Fehlig wrote:
On 5/30/19 3:08 AM, Michal Privoznik wrote:
On 5/29/19 7:44 PM, Jim Fehlig wrote:
-libqemutestdriver_la_LIBADD = $(qemu_LDADDS) +libqemutestdriver_la_LIBADD = $(qemu_LDADDS) $(GNULIB_LIBS) qemucpumock_la_SOURCES = \ qemucpumock.c testutilshostcpus.h
ACK and safe for freeze to this hunk. Alternatively, we might go with $(LDADDS) which includes $(GNULIB_LIBS).
Do you have a preference? LDADDS includes some other things which AFAIK are not needed.
Well, other test libs use LDADDS and I'd say that LTO doesn't link anything that's not needed. But maybe I'm mistaken. So, no, I don't have any preference.
I think we don't really care about potentially overlinking when it comes to test programs, so there's no need to have the kind of granularity your patch implements. Actually I'd go one further and adopt what Xen tests are doing: there's an explicit libxl_LDADDS += $(LDADDS) and then most tests include at least $(libxl_LDADDS) in their _(LD|LIB)ADDs, whereas most QEMU tests need to use _(LD|LIB)ADD = $(qemu_LDADDS) $(LDADDS) My suggestion would be to copy that approach, have qemu_LDADDS += $(LDADDS) and then drop the extra $(LDADDS) from QEMU tests, which will not only fix your linkin problem but also clean up Makefile.am pretty nicely :) -- Andrea Bolognani / Red Hat / Virtualization
On 5/30/19 10:00 AM, Andrea Bolognani wrote:
On Thu, 2019-05-30 at 16:56 +0200, Michal Privoznik wrote:
On 5/30/19 4:44 PM, Jim Fehlig wrote:
On 5/30/19 3:08 AM, Michal Privoznik wrote:
On 5/29/19 7:44 PM, Jim Fehlig wrote:
-libqemutestdriver_la_LIBADD = $(qemu_LDADDS) +libqemutestdriver_la_LIBADD = $(qemu_LDADDS) $(GNULIB_LIBS) qemucpumock_la_SOURCES = \ qemucpumock.c testutilshostcpus.h
ACK and safe for freeze to this hunk. Alternatively, we might go with $(LDADDS) which includes $(GNULIB_LIBS).
Do you have a preference? LDADDS includes some other things which AFAIK are not needed.
Well, other test libs use LDADDS and I'd say that LTO doesn't link anything that's not needed. But maybe I'm mistaken. So, no, I don't have any preference.
I think we don't really care about potentially overlinking when it comes to test programs, so there's no need to have the kind of granularity your patch implements.
Actually I'd go one further and adopt what Xen tests are doing: there's an explicit
libxl_LDADDS += $(LDADDS)
and then most tests include at least $(libxl_LDADDS) in their _(LD|LIB)ADDs, whereas most QEMU tests need to use
_(LD|LIB)ADD = $(qemu_LDADDS) $(LDADDS)
My suggestion would be to copy that approach, have
qemu_LDADDS += $(LDADDS)
and then drop the extra $(LDADDS) from QEMU tests, which will not only fix your linkin problem but also clean up Makefile.am pretty nicely :)
Sorry for not noticing your reply before pushing. As it turns out your suggestion is not as simple as I expected. The attached patch fails with link errors such as /usr/lib64/gcc/x86_64-suse-linux/8/../../../../x86_64-suse-linux/bin/ld: ../src/libvirt_probes.o:/home/jfehlig/virt/upstream/libvirt/src/libvirt_probes.o.dtrace-temp.c:15: multiple definition of `libvirt_event_poll_add_handle_semaphore'; ../src/libvirt_probes.o:/home/jfehlig/virt/upstream/libvirt/src/libvirt_probes.o.dtrace-temp.c:15: first defined here /usr/lib64/gcc/x86_64-suse-linux/8/../../../../x86_64-suse-linux/bin/ld: ../src/libvirt_probes.o:/home/jfehlig/virt/upstream/libvirt/src/libvirt_probes.o.dtrace-temp.c:24: multiple definition of `libvirt_event_poll_update_handle_semaphore'; ../src/libvirt_probes.o:/home/jfehlig/virt/upstream/libvirt/src/libvirt_probes.o.dtrace-temp.c:24: first defined here This is due to LDADDS including ../src/libvirt_probes.lo (PROBES_O) and qemu_LDADDS including ../src/libvirt_qemu_probes.lo when WITH_DTRACE_PROBES is defined. I tried a couple of approaches to fixing this but found nothing satisfying. (More) Suggestions welcomed :-). Regards, Jim
On Mon, 2019-06-03 at 15:41 -0600, Jim Fehlig wrote:
On 5/30/19 10:00 AM, Andrea Bolognani wrote:
Actually I'd go one further and adopt what Xen tests are doing: there's an explicit
libxl_LDADDS += $(LDADDS)
and then most tests include at least $(libxl_LDADDS) in their _(LD|LIB)ADDs, whereas most QEMU tests need to use
_(LD|LIB)ADD = $(qemu_LDADDS) $(LDADDS)
My suggestion would be to copy that approach, have
qemu_LDADDS += $(LDADDS)
and then drop the extra $(LDADDS) from QEMU tests, which will not only fix your linkin problem but also clean up Makefile.am pretty nicely :)
Sorry for not noticing your reply before pushing. As it turns out your suggestion is not as simple as I expected. The attached patch fails with link errors such as
/usr/lib64/gcc/x86_64-suse-linux/8/../../../../x86_64-suse-linux/bin/ld: ../src/libvirt_probes.o:/home/jfehlig/virt/upstream/libvirt/src/libvirt_probes.o.dtrace-temp.c:15: multiple definition of `libvirt_event_poll_add_handle_semaphore'; ../src/libvirt_probes.o:/home/jfehlig/virt/upstream/libvirt/src/libvirt_probes.o.dtrace-temp.c:15: first defined here /usr/lib64/gcc/x86_64-suse-linux/8/../../../../x86_64-suse-linux/bin/ld: ../src/libvirt_probes.o:/home/jfehlig/virt/upstream/libvirt/src/libvirt_probes.o.dtrace-temp.c:24: multiple definition of `libvirt_event_poll_update_handle_semaphore'; ../src/libvirt_probes.o:/home/jfehlig/virt/upstream/libvirt/src/libvirt_probes.o.dtrace-temp.c:24: first defined here
This is due to LDADDS including ../src/libvirt_probes.lo (PROBES_O) and qemu_LDADDS including ../src/libvirt_qemu_probes.lo when WITH_DTRACE_PROBES is defined. I tried a couple of approaches to fixing this but found nothing satisfying. (More) Suggestions welcomed :-).
Yeah, it's not as straightforward as I assumed it would be: I tried it too after sending the email, and bumped into the very same issue. I have something almost reasonable in a local branch, I'll polish it up in the next few days and then post it. -- Andrea Bolognani / Red Hat / Virtualization
On Tue, 2019-06-04 at 09:33 +0200, Andrea Bolognani wrote:
I have something almost reasonable in a local branch, I'll polish it up in the next few days and then post it.
Done :) https://www.redhat.com/archives/libvir-list/2019-June/msg00339.html -- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani -
Jim Fehlig -
Michal Privoznik