From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH 1/2] mk: prevent overlinking in applications Date: Thu, 9 Jun 2016 14:05:13 +0100 Message-ID: <57596989.50601@intel.com> References: <574872B3.6040702@intel.com> <1464367686-3475-1-git-send-email-ferruh.yigit@intel.com> <7354386.zBuGXxEf9j@xps13> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Cc: dev@dpdk.org, Panu Matilainen , Christian Ehrhardt To: Thomas Monjalon Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 35DCC2142 for ; Thu, 9 Jun 2016 15:06:05 +0200 (CEST) In-Reply-To: <7354386.zBuGXxEf9j@xps13> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 6/9/2016 11:10 AM, Thomas Monjalon wrote: > Hi Ferruh, >=20 > 2016-05-27 17:48, Ferruh Yigit: >> Replace --no-as-needed linker flag with --as-needed flag, which will >> only link libraries directly called by application. This requires inte= r >> library dependencies resolved correctly. >> >> Not linking all libraries cause a compile error for lpcap and possible >> to have other similar compiler errors, so increasing the scope of >> --start-group argument. >=20 > What is the error? >=20 This is with -as-needed flag, and static library compilation: .../dpdk/build/lib/librte_pmd_pcap.a(rte_eth_pcap.o): In function `eth_dev_stop': rte_eth_pcap.c:(.text+0xcc1): undefined reference to `pcap_dump_close' rte_eth_pcap.c:(.text+0xcd6): undefined reference to `pcap_close' rte_eth_pcap.c:(.text+0xd19): undefined reference to `pcap_close' rte_eth_pcap.c:(.text+0xd58): undefined reference to `pcap_close' /root/development/dpdk/build/lib/librte_pmd_pcap.a(rte_eth_pcap.o): In function `open_tx_pcap': rte_eth_pcap.c:(.text+0x190b): undefined reference to `pcap_open_dead' rte_eth_pcap.c:(.text+0x191b): undefined reference to `pcap_dump_open' ... pcap pmd has libpcap calls, but while linking final application (testpmd), linker is not able to find objects that has these. The command line for compilation is: gcc ... -o test ... -Wl,--whole-archive -Wl,-lpcap -Wl,--start-group -Wl,-lrte_pmd_pcap -Wl,--end-group -Wl,--no-whole-archive -lpcap is provided, but since before -lrte_pmd_pcap, references not resolved. Previously, with "-no-as-needed" flag, all shared libraries linked always, which was preventing compile error. >> cmdline_test application causes compile error because of cyclic >> dependency between librte_eal <-> librte_mempool. A workaround added t= o >> cmdline_test for compile error. >> >> Signed-off-by: Ferruh Yigit >=20 >> --- a/app/cmdline_test/Makefile >> +++ b/app/cmdline_test/Makefile >> @@ -46,6 +46,7 @@ SRCS-y +=3D commands.c >> =20 >> CFLAGS +=3D -O3 >> CFLAGS +=3D $(WERROR_FLAGS) >=20 > A comment is required here to explain the workaround. >=20 Sure, I will add a comment and send a new version of the patch. >> +LDFLAGS +=3D -no-as-needed >=20 > The option should be --no-as-needed. Right >=20 >> --- a/mk/exec-env/linuxapp/rte.vars.mk >> +++ b/mk/exec-env/linuxapp/rte.vars.mk >> @@ -46,7 +46,7 @@ EXECENV_CFLAGS =3D -pthread >> endif >> =20 >> # Workaround lack of DT_NEEDED entry >=20 > This comment is obsolete now. Will remove. >=20 >> -EXECENV_LDFLAGS =3D --no-as-needed >> +EXECENV_LDFLAGS =3D --as-needed >=20 > Why put this option for Linux only? When this option not defined at all, different compilers behave different, some has --as-needed as default and cause compilation issues, I guess that is why for Linux --no-as-needed is forces. I assume BSD compilers by default use --no-as-needed, so we don't have any compilation problem. And it looks like there is no specific reason to not force BSD to --as-needed, I will test it. > Shouldn't we restrict this option to app.mk only? Having --no-needed flag only for app should be OK. Let me test first, and as a result should we move this to app.mk? >=20 >> --- a/mk/rte.app.mk >> +++ b/mk/rte.app.mk >> @@ -58,6 +58,7 @@ _LDLIBS-y +=3D -L$(RTE_SDK_BIN)/lib >> # >> =20 >> _LDLIBS-y +=3D --whole-archive >> +_LDLIBS-y +=3D --start-group >=20 > --start-group must be used only to solve circular dependencies. > Ideally we should not use it. I think it's needed only because > of EAL logs using mempool (must be removed). No, this is not the reason, please check above lpcap compile error, that is the reason of this update. > Why extending it? > I'm afraid we are masking some issues. But if we have a convention to add external libraries after dpdk libraries, that also should solve this issue, but that is more update than just updating --start-group. >=20 >> _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) +=3D -lrte_distributor >> _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) +=3D -lrte_reorder >> @@ -111,8 +112,6 @@ _LDLIBS-y +=3D -= lcrypto >> endif >> endif # !CONFIG_RTE_BUILD_SHARED_LIBS >> =20 >> -_LDLIBS-y +=3D --start-group >> - >> _LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS) +=3D -lrte_kvargs >> _LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF) +=3D -lrte_mbuf >> _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG) +=3D -lrte_ip_frag >=20