[dpdk-dev,v2] mk: link combined shared lib using CC

Message ID 1414511329-3948-1-git-send-email-sergio.gonzalez.monroy@intel.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Sergio Gonzalez Monroy Oct. 28, 2014, 3:48 p.m. UTC
  If we set EXTRA_CFLAGS=-O0, build fails with following error:

/usr/bin/ld: test: hidden symbol `mknod' in /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO

Fix: link combined shared lib using CC if LINK_USING_CC is enabled.

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
 mk/rte.lib.mk      |  1 -
 mk/rte.sharelib.mk | 12 +++++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)
  

Comments

De Lara Guarch, Pablo Oct. 28, 2014, 4:09 p.m. UTC | #1
> -----Original Message-----
> From: Gonzalez Monroy, Sergio
> Sent: Tuesday, October 28, 2014 3:49 PM
> To: dev@dpdk.org
> Cc: thomas.monjalon@6wind.com; De Lara Guarch, Pablo
> Subject: [PATCH v2] mk: link combined shared lib using CC
> 
> If we set EXTRA_CFLAGS=-O0, build fails with following error:
> 
> /usr/bin/ld: test: hidden symbol `mknod' in
> /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
> 
> Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> 
> Signed-off-by: Sergio Gonzalez Monroy
> <sergio.gonzalez.monroy@intel.com>

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
  
Thomas Monjalon Dec. 16, 2014, 6:48 p.m. UTC | #2
2014-10-28 15:48, Sergio Gonzalez Monroy:
> If we set EXTRA_CFLAGS=-O0, build fails with following error:
> 
> /usr/bin/ld: test: hidden symbol `mknod' in /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
> 
> Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> 
> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> ---
>  mk/rte.lib.mk      |  1 -
>  mk/rte.sharelib.mk | 12 +++++++++++-
[...]
> --- a/mk/rte.lib.mk
> +++ b/mk/rte.lib.mk
> @@ -63,7 +63,6 @@ ifeq ($(LINK_USING_CC),1)
>  # Override the definition of LD here, since we're linking with CC
>  LD := $(CC)
>  LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
> -CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
>  endif

Why are you removing this line?

> --- a/mk/rte.sharelib.mk
> +++ b/mk/rte.sharelib.mk
[...]
  
Thomas Monjalon Dec. 16, 2014, 11:42 p.m. UTC | #3
2014-12-16 19:48, Thomas Monjalon:
> 2014-10-28 15:48, Sergio Gonzalez Monroy:
> > If we set EXTRA_CFLAGS=-O0, build fails with following error:
> > 
> > /usr/bin/ld: test: hidden symbol `mknod' in /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
> > 
> > Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> > 
> > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > ---
> >  mk/rte.lib.mk      |  1 -
> >  mk/rte.sharelib.mk | 12 +++++++++++-
> [...]
> > --- a/mk/rte.lib.mk
> > +++ b/mk/rte.lib.mk
> > @@ -63,7 +63,6 @@ ifeq ($(LINK_USING_CC),1)
> >  # Override the definition of LD here, since we're linking with CC
> >  LD := $(CC)
> >  LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
> > -CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
> >  endif
> 
> Why are you removing this line?

If it's really needed, it could be another patch.
 
> > --- a/mk/rte.sharelib.mk
> > +++ b/mk/rte.sharelib.mk
> [...]

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Applied without the change to rte.lib.mk.

Thanks
  
Sergio Gonzalez Monroy Dec. 17, 2014, 10:41 a.m. UTC | #4
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, December 16, 2014 11:43 PM
> 
> 2014-12-16 19:48, Thomas Monjalon:
> > 2014-10-28 15:48, Sergio Gonzalez Monroy:
> > > If we set EXTRA_CFLAGS=-O0, build fails with following error:
> > >
> > > /usr/bin/ld: test: hidden symbol `mknod' in
> > > /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
> > >
> > > Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> > >
> > > Signed-off-by: Sergio Gonzalez Monroy
> > > <sergio.gonzalez.monroy@intel.com>
> > > ---
> > >  mk/rte.lib.mk      |  1 -
> > >  mk/rte.sharelib.mk | 12 +++++++++++-
> > [...]
> > > --- a/mk/rte.lib.mk
> > > +++ b/mk/rte.lib.mk
> > > @@ -63,7 +63,6 @@ ifeq ($(LINK_USING_CC),1)  # Override the
> > > definition of LD here, since we're linking with CC  LD := $(CC)
> > > LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs) -CPU_LDFLAGS :=
> > > $(call linkerprefix,$(CPU_LDFLAGS))  endif
> >
> > Why are you removing this line?
> 
Hi Thomas,

Basically the problem is when CPU_LDFLAGS has a value, which in the currently only happens if we set CPU_LDFLAGS in the command line or build i686 (setting CPU_LDFLAGS=-melf_i386).
In those cases, given the way the makefiles are being included, CPU_LDFLAGS is being 'linkerprefixed' twice, resulting in build error.
Removing that line avoided the second linker prefix of the CPU_LDFLAGS (note that we reset the original var instead of using a different var name).

There probably is a better way but I could not see an straightforward fix for it.
I planned to improve the current build system (already sent an RFC) and was waiting until 1.8 release to resume the work on it.

Thanks,
Sergio

> If it's really needed, it could be another patch.
> 
> > > --- a/mk/rte.sharelib.mk
> > > +++ b/mk/rte.sharelib.mk
> > [...]
> 
> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> Applied without the change to rte.lib.mk.
> 
> Thanks
> --
> Thomas
  
Thomas Monjalon Dec. 17, 2014, 2:01 p.m. UTC | #5
2014-12-17 10:41, Gonzalez Monroy, Sergio:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-12-16 19:48, Thomas Monjalon:
> > > 2014-10-28 15:48, Sergio Gonzalez Monroy:
> > > > Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> > > >
> > > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > > > ---
> > > >  mk/rte.lib.mk      |  1 -
> > > >  mk/rte.sharelib.mk | 12 +++++++++++-
> > > [...]
> > > > --- a/mk/rte.lib.mk
> > > > +++ b/mk/rte.lib.mk
> > > > @@ -63,7 +63,6 @@ ifeq ($(LINK_USING_CC),1)
> > > >  # Override the definition of LD here, since we're linking with CC
> > > >  LD := $(CC)
> > > >  LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
> > > > -CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
> > > >  endif
> > >
> > > Why are you removing this line?
> > 
> Hi Thomas,
> 
> Basically the problem is when CPU_LDFLAGS has a value, which in the
> currently only happens if we set CPU_LDFLAGS in the command line or
> build i686 (setting CPU_LDFLAGS=-melf_i386).
> In those cases, given the way the makefiles are being included,

These 2 files don't include each other.
rte.sharelib.mk is included only in lib/Makefile
and rte.lib.mk is included in each lib Makefile.

I think the problem is not about how the files are included
but how this variable is exported.
CPU_LDFLAGS is modified in lib/Makefile via rte.sharelib.mk,
then exported (see mk/arch/*/rte.vars.mk) then modified
again in libs via rte.lib.mk.

> CPU_LDFLAGS is being 'linkerprefixed' twice, resulting in build error.
> Removing that line avoided the second linker prefix of the CPU_LDFLAGS
> (note that we reset the original var instead of using a different var name).
> 
> There probably is a better way but I could not see an straightforward fix for it.

I suggest 2 possible fixes:
- do not export this variable (why is it exported?)
- or do not override the variable, i.e. use a different variable for prefixing

> I planned to improve the current build system (already sent an RFC)
> and was waiting until 1.8 release to resume the work on it.

I agree we should work on improvements after 1.8.

> > If it's really needed, it could be another patch.
[...]
> > 
> > Applied without the change to rte.lib.mk.

It definitely deserves another patch with a proper fix.
  
Thomas Monjalon Dec. 17, 2014, 6:49 p.m. UTC | #6
2014-12-17 15:01, Thomas Monjalon:
> 2014-12-17 10:41, Gonzalez Monroy, Sergio:
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-12-16 19:48, Thomas Monjalon:
> > > > 2014-10-28 15:48, Sergio Gonzalez Monroy:
> > > > > Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> > > > >
> > > > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > > > > ---
> > > > >  mk/rte.lib.mk      |  1 -
> > > > >  mk/rte.sharelib.mk | 12 +++++++++++-
> > > > [...]
> > > > > --- a/mk/rte.lib.mk
> > > > > +++ b/mk/rte.lib.mk
> > > > > @@ -63,7 +63,6 @@ ifeq ($(LINK_USING_CC),1)
> > > > >  # Override the definition of LD here, since we're linking with CC
> > > > >  LD := $(CC)
> > > > >  LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
> > > > > -CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
> > > > >  endif
> > > >
> > > > Why are you removing this line?
> > > 
> > Hi Thomas,
> > 
> > Basically the problem is when CPU_LDFLAGS has a value, which in the
> > currently only happens if we set CPU_LDFLAGS in the command line or
> > build i686 (setting CPU_LDFLAGS=-melf_i386).
> > In those cases, given the way the makefiles are being included,
> 
> These 2 files don't include each other.
> rte.sharelib.mk is included only in lib/Makefile
> and rte.lib.mk is included in each lib Makefile.
> 
> I think the problem is not about how the files are included
> but how this variable is exported.
> CPU_LDFLAGS is modified in lib/Makefile via rte.sharelib.mk,
> then exported (see mk/arch/*/rte.vars.mk) then modified
> again in libs via rte.lib.mk.
> 
> > CPU_LDFLAGS is being 'linkerprefixed' twice, resulting in build error.
> > Removing that line avoided the second linker prefix of the CPU_LDFLAGS
> > (note that we reset the original var instead of using a different var name).
> > 
> > There probably is a better way but I could not see an straightforward fix for it.
> 
> I suggest 2 possible fixes:
> - do not export this variable (why is it exported?)
> - or do not override the variable, i.e. use a different variable for prefixing

I'm preparing a patch with the second approach.
It raised other problems like error when linking examples with combined library.
Patchset to come.

> > I planned to improve the current build system (already sent an RFC)
> > and was waiting until 1.8 release to resume the work on it.
> 
> I agree we should work on improvements after 1.8.
> 
> > > If it's really needed, it could be another patch.
> [...]
> > > 
> > > Applied without the change to rte.lib.mk.
> 
> It definitely deserves another patch with a proper fix.
  
Thomas Monjalon Dec. 17, 2014, 9:59 p.m. UTC | #7
When trying to fix the use of combined library, some bugs have been
raised. This patchset fix and clean some link options.

Thomas Monjalon (3):
  mk: fix link examples to combined library
  mk: forbid multiple definitions
  mk: fix link with CC

 mk/rte.app.mk      | 5 ++---
 mk/rte.lib.mk      | 9 +++++----
 mk/rte.shared.mk   | 6 ++----
 mk/rte.sharelib.mk | 9 +++++----
 mk/rte.vars.mk     | 9 +++++----
 5 files changed, 19 insertions(+), 19 deletions(-)
  
Thomas Monjalon Dec. 17, 2014, 11:35 p.m. UTC | #8
2014-12-17 22:59, Thomas Monjalon:
> When trying to fix the use of combined library, some bugs have been
> raised. This patchset fix and clean some link options.
> 
> Thomas Monjalon (3):
>   mk: fix link examples to combined library
>   mk: forbid multiple definitions
>   mk: fix link with CC

Applied quickly to validate building before the release.
  

Patch

diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index d83e808..a6abd6d 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -63,7 +63,6 @@  ifeq ($(LINK_USING_CC),1)
 # Override the definition of LD here, since we're linking with CC
 LD := $(CC)
 LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
-CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
 endif
 
 O_TO_A = $(AR) crus $(LIB) $(OBJS-y)
diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk
index c0a811a..1fac0ad 100644
--- a/mk/rte.sharelib.mk
+++ b/mk/rte.sharelib.mk
@@ -29,6 +29,8 @@ 
 #   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 #   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+include $(RTE_SDK)/mk/internal/rte.build-pre.mk
+
 # VPATH contains at least SRCDIR
 VPATH += $(SRCDIR)
 
@@ -45,7 +47,15 @@  sharelib: $(LIB_ONE) FORCE
 
 OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o)
 
-O_TO_S = $(LD) $(CPU_LDFLAGS) -shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
+ifeq ($(LINK_USING_CC),1)
+# Override the definition of LD here, since we're linking with CC
+LD := $(CC) $(CPU_CFLAGS)
+LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
+CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
+endif
+
+O_TO_S = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS)\
+	-o $(RTE_OUTPUT)/lib/$(LIB_ONE)
 O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
 O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")
 O_TO_S_CMD = "cmd_$@ = $(O_TO_S_STR)"