| Message ID | 1445184515-6049-1-git-send-email-bernd.kuhls@t-online.de |
|---|---|
| State | Changes Requested |
| Headers | show |
Bernd, On Sun, 18 Oct 2015 18:08:35 +0200, Bernd Kuhls wrote: > Adding -fPIC fixes > http://autobuild.buildroot.net/results/a19/a19023e094cbed491444665d6839a9e65a8eee6c/ > http://autobuild.buildroot.net/results/98d/98dde028d1d6199f05c904b498bc39bbaa112aa6/ > http://autobuild.buildroot.net/results/4ff/4ffcae6cae419df35e8ca29d429ee178bcf31882/ > http://autobuild.buildroot.net/results/8ce/8ced16874255ace4923f6b8888c3fca07f28b804/ None of these configurations have BR2_STATIC_LIBS=y, so I fail to see why adding -fPIC when BR2_STATIC_LIBS=y would fix those issues. Moreover, using -fPIC for statically linked configurations generally doesn't make sense: the point of PIC code is to be Position Independent, which is needed for shared libraries. But not for statically linked programs. > The upstream Makefile also allows building a shared library if > CONFIG_SHARED=1 is added to _BUILD_CMDS. This probably fixes the problem however. So, can you explain the addition of -fPIC for BR2_STATIC_LIBS=y configurations? Thanks! Thomas
Hi Thomas, Bernd, On Sun, Oct 18, 2015 at 9:05 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Bernd, > > On Sun, 18 Oct 2015 18:08:35 +0200, Bernd Kuhls wrote: >> Adding -fPIC fixes >> http://autobuild.buildroot.net/results/a19/a19023e094cbed491444665d6839a9e65a8eee6c/ >> http://autobuild.buildroot.net/results/98d/98dde028d1d6199f05c904b498bc39bbaa112aa6/ >> http://autobuild.buildroot.net/results/4ff/4ffcae6cae419df35e8ca29d429ee178bcf31882/ >> http://autobuild.buildroot.net/results/8ce/8ced16874255ace4923f6b8888c3fca07f28b804/ The coomit message is not as detailed and clear as it should be for this tricky fix... :-/ > > None of these configurations have BR2_STATIC_LIBS=y, so I fail to see > why adding -fPIC when BR2_STATIC_LIBS=y would fix those issues. > > Moreover, using -fPIC for statically linked configurations generally > doesn't make sense: the point of PIC code is to be Position > Independent, which is needed for shared libraries. But not for > statically linked programs. AFAI understand these build failures, the problem comes from the static library libdcadec.a that is unconditionnally built. When the config has BR2_SHARED_LIBS=y, this static library got built without -fPIC, then is used by ffmpeg to be linked against for some shared objects libraries (e.g. libavcodec.so). That's right here that error occurs. > >> The upstream Makefile also allows building a shared library if >> CONFIG_SHARED=1 is added to _BUILD_CMDS. > > This probably fixes the problem however. To fix this the options are: 1) build libdcadec as a shared library, if it is supported; 2) or, add -fPIC when BR2_SHARED_LIBS=y; 3) or, unconditionally add -fPIC. My preference is option 1). > > So, can you explain the addition of -fPIC for BR2_STATIC_LIBS=y > configurations? IIRC, -fPIC on static libs does not have any harmful consequences, only a slight overhead at some point due to some indirection. > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot Regards,
Hello, On Sun, 18 Oct 2015 21:52:38 +0200, Samuel Martin wrote: > > None of these configurations have BR2_STATIC_LIBS=y, so I fail to see > > why adding -fPIC when BR2_STATIC_LIBS=y would fix those issues. > > > > Moreover, using -fPIC for statically linked configurations generally > > doesn't make sense: the point of PIC code is to be Position > > Independent, which is needed for shared libraries. But not for > > statically linked programs. > > AFAI understand these build failures, the problem comes from the > static library libdcadec.a that is unconditionnally built. > When the config has BR2_SHARED_LIBS=y, this static library got built > without -fPIC, then is used by ffmpeg to be linked against for some > shared objects libraries (e.g. libavcodec.so). That's right here that > error occurs. This, I'm OK with. > >> The upstream Makefile also allows building a shared library if > >> CONFIG_SHARED=1 is added to _BUILD_CMDS. > > > > This probably fixes the problem however. > > To fix this the options are: > 1) build libdcadec as a shared library, if it is supported; > 2) or, add -fPIC when BR2_SHARED_LIBS=y; > 3) or, unconditionally add -fPIC. > > My preference is option 1). Obviously, (1) is the right solution here. > > So, can you explain the addition of -fPIC for BR2_STATIC_LIBS=y > > configurations? > > IIRC, -fPIC on static libs does not have any harmful consequences, > only a slight overhead at some point due to some indirection. Correct. But it doesn't make sense to explicitly add -fPIC when BR2_STATIC_LIBS=y. Thomas
diff --git a/package/libdcadec/libdcadec.mk b/package/libdcadec/libdcadec.mk index 76862a6..0c9325e 100644 --- a/package/libdcadec/libdcadec.mk +++ b/package/libdcadec/libdcadec.mk @@ -10,19 +10,28 @@ LIBDCADEC_LICENSE = LGPLv2.1+ LIBDCADEC_LICENSE_FILES = COPYING.LGPLv2.1 LIBDCADEC_INSTALL_STAGING = YES +LIBDCADEC_CFLAGS = $(TARGET_CFLAGS) -std=gnu99 + +ifeq ($(BR2_STATIC_LIBS),y) +LIBDCADEC_CFLAGS += -fPIC +else +LIBDCADEC_MAKE_OPTS = CONFIG_SHARED=1 +endif +LIBDCADEC_MAKE_OPTS += CFLAGS="$(LIBDCADEC_CFLAGS)" + define LIBDCADEC_BUILD_CMDS $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ - CFLAGS="$(TARGET_CFLAGS) -std=gnu99" -C $(@D) + $(LIBDCADEC_MAKE_OPTS) -C $(@D) endef define LIBDCADEC_INSTALL_STAGING_CMDS $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) \ - DESTDIR=$(STAGING_DIR) PREFIX=/usr install + $(LIBDCADEC_MAKE_OPTS) DESTDIR=$(STAGING_DIR) PREFIX=/usr install endef define LIBDCADEC_INSTALL_TARGET_CMDS $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) \ - DESTDIR=$(TARGET_DIR) PREFIX=/usr install + $(LIBDCADEC_MAKE_OPTS) DESTDIR=$(TARGET_DIR) PREFIX=/usr install endef $(eval $(generic-package))
Adding -fPIC fixes http://autobuild.buildroot.net/results/a19/a19023e094cbed491444665d6839a9e65a8eee6c/ http://autobuild.buildroot.net/results/98d/98dde028d1d6199f05c904b498bc39bbaa112aa6/ http://autobuild.buildroot.net/results/4ff/4ffcae6cae419df35e8ca29d429ee178bcf31882/ http://autobuild.buildroot.net/results/8ce/8ced16874255ace4923f6b8888c3fca07f28b804/ The upstream Makefile also allows building a shared library if CONFIG_SHARED=1 is added to _BUILD_CMDS. Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de> --- package/libdcadec/libdcadec.mk | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)