Skip to contentSkip to conversation

bfin: remove the dependency of flat shared mode to shared library

PATCH New Sonic Zhang 3 participants 11 messages Mar 18, 2015 – Dec 30, 2015 1 message not in the archive

lore
Share & tools
lore

Sonic Zhang

author
#0 lore raw

PATCH 1/2 bfin: remove the dependency of flat shared mode to shared library

From: Sonic Zhang <[email protected]>

Blackfin flat shared library isn't generated and linked in the same way as the ELF shared library. So, don't mark it depend on the static library only.

Signed-off-by Sonic Zhang <[email protected]>
---
 arch/Config.in |    1 -
 1 file changed, 1 deletion(-)
arch/Config.in +0 -1
index 16ad8be..6179960 100644
@@ -295,7 +295,6 @@ config BR2_BINFMT_FLAT_SEP_DATA
 
 config BR2_BINFMT_FLAT_SHARED
 	bool "Shared binary"
-	select BR2_BINFMT_SUPPORTS_SHARED
 	help
 	  Allow to load and link indiviual FLAT binaries at run time.
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH 2/2 bfin: put the libc link path of the flat shared and sep data formats before the sysroot link path

From: Sonic Zhang <[email protected]>

The libc of the shared flat and sep data formats are different from the standard flat libc in the sysroot lib path. In order to make application link with proper libc, put their path before the sysroot link path.

Signed-off-by Sonic Zhang <[email protected]>
---
 package/Makefile.in |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
package/Makefile.in +2 -2
index 803b162..e88d1ad 100644
@@ -153,12 +153,12 @@ TARGET_LDFLAGS += $(if $($(PKG)_FLAT_STACKSIZE),-elf2flt=-s$($(PKG)_FLAT_STACKSI
 endif
 
 ifeq ($(BR2_BINFMT_FLAT_SHARED),y)
-TARGET_LDFLAGS += -mid-shared-library -mshared-library-id=0
+TARGET_LDFLAGS += -mid-shared-library -mshared-library-id=0 -L$(STAGING_DIR)/usr/lib/mid-shared-library
 TARGET_CFLAGS += -mid-shared-library -mshared-library-id=0
 TARGET_CXXFLAGS += -mid-shared-library -mshared-library-id=0
 endif
 ifeq ($(BR2_BINFMT_FLAT_SEP_DATA),y)
-TARGET_LDFLAGS += -msep-data
+TARGET_LDFLAGS += -msep-data -L$(STAGING_DIR)/usr/lib/msep-data
 TARGET_CFLAGS += -msep-data
 TARGET_CXXFLAGS += -msep-data
 endif
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH 2/2 bfin: put the libc link path of the flat shared and sep data formats before the sysroot link path

Dear Sonic Zhang,

Quoted 20 lines · Sonic Zhang

From: Sonic Zhang <[email protected]>

The libc of the shared flat and sep data formats are different from the standard flat libc in the sysroot lib path. In order to make application link with proper libc, put their path before the sysroot link path.

Signed-off-by Sonic Zhang <[email protected]>
---
 package/Makefile.in |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
package/Makefile.in +0 -1
index 803b162..e88d1ad 100644
@@ -153,12 +153,12 @@ TARGET_LDFLAGS += $(if $($(PKG)_FLAT_STACKSIZE),-elf2flt=-s$($(PKG)_FLAT_STACKSI
 endif
 
 ifeq ($(BR2_BINFMT_FLAT_SHARED),y)
-TARGET_LDFLAGS += -mid-shared-library -mshared-library-id=0

package/Makefile.in TARGET_LDFLAGS += $(if $($(PKG)_FLAT_STACKSIZE),-elf2flt=-s$($(PKG)_FLAT_STACKSI · line 164

Quoted 10 lines
package/Makefile.in +2 -1
+TARGET_LDFLAGS += -mid-shared-library -mshared-library-id=0 -L$(STAGING_DIR)/usr/lib/mid-shared-library
 TARGET_CFLAGS += -mid-shared-library -mshared-library-id=0
 TARGET_CXXFLAGS += -mid-shared-library -mshared-library-id=0
 endif
 ifeq ($(BR2_BINFMT_FLAT_SEP_DATA),y)
-TARGET_LDFLAGS += -msep-data
+TARGET_LDFLAGS += -msep-data -L$(STAGING_DIR)/usr/lib/msep-data
 TARGET_CFLAGS += -msep-data
 TARGET_CXXFLAGS += -msep-data
 endif

Thanks for this patch. I did some quick testing with BR2_BINFMT_FLAT_SHARED with and without your patch, and I do not see a difference. For example, I've built strace with and without your patch applied, and the resulting binary has the same size. Looking at strace.gdb using readelf, I can see that the C library functions are not inside the strace binary itself, while with a BR2_BINFMT_FLAT_ONE build, the strace binary is definitely bigger and a readelf on strace.gdb shows that the C library functions have been copied into the binary.

So, could you give us an example of what this patch is fixing?

I'm sorry to ask such questions, but while the Blackfin architecture is probably well known to you, it isn't to most of us, so some additional details are always useful.

Thanks a lot!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#3 lore raw

PATCH 2/2 bfin: put the libc link path of the flat shared and sep data formats before the sysroot link path

Hi Thomas,

Quoted 44 lines · Thomas Petazzoni

Dear Sonic Zhang,

Quoted 20 lines · Sonic Zhang

From: Sonic Zhang <[email protected]>

The libc of the shared flat and sep data formats are different from the standard flat libc in the sysroot lib path. In order to make application link with proper libc, put their path before the sysroot link path.

Signed-off-by Sonic Zhang <[email protected]>
---
 package/Makefile.in |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
package/Makefile.in +0 -1
index 803b162..e88d1ad 100644
@@ -153,12 +153,12 @@ TARGET_LDFLAGS += $(if $($(PKG)_FLAT_STACKSIZE),-elf2flt=-s$($(PKG)_FLAT_STACKSI
 endif
 
 ifeq ($(BR2_BINFMT_FLAT_SHARED),y)
-TARGET_LDFLAGS += -mid-shared-library -mshared-library-id=0

package/Makefile.in TARGET_LDFLAGS += $(if $($(PKG)_FLAT_STACKSIZE),-elf2flt=-s$($(PKG)_FLAT_STACKSI · line 164

Quoted 10 lines
package/Makefile.in +2 -1
+TARGET_LDFLAGS += -mid-shared-library -mshared-library-id=0 -L$(STAGING_DIR)/usr/lib/mid-shared-library
 TARGET_CFLAGS += -mid-shared-library -mshared-library-id=0
 TARGET_CXXFLAGS += -mid-shared-library -mshared-library-id=0
 endif
 ifeq ($(BR2_BINFMT_FLAT_SEP_DATA),y)
-TARGET_LDFLAGS += -msep-data
+TARGET_LDFLAGS += -msep-data -L$(STAGING_DIR)/usr/lib/msep-data
 TARGET_CFLAGS += -msep-data
 TARGET_CXXFLAGS += -msep-data
 endif

Thanks for this patch. I did some quick testing with BR2_BINFMT_FLAT_SHARED with and without your patch, and I do not see a difference. For example, I've built strace with and without your patch applied, and the resulting binary has the same size. Looking at strace.gdb using readelf, I can see that the C library functions are not inside the strace binary itself, while with a BR2_BINFMT_FLAT_ONE build, the strace binary is definitely bigger and a readelf on strace.gdb shows that the C library functions have been copied into the binary.

Quoted 2 lines

So, could you give us an example of what this patch is fixing?

If you build libgcrypt-1.6.3 without this patch, you will get following error.

libtool: link: ( cd ".libs" && rm -f "libgcrypt.la" && ln -s
"../libgcrypt.la" "libgcrypt.la" )
/bin/bash ../libtool --tag=CC --mode=link
/home/sonic/projects/buildroot/output/host/usr/bin/bfin-uclinux-gcc -I/home/sonic/projects/buildroot/output/host/usr/bfin-buildroot-uclinux-uclibc/sysroot/usr/include -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os
-Wl,-elf2flt -mid-shared-library -mshared-library-id=0 -D__NOMMU__
-D__uClinux__ -static -fvisibility=hidden -Wall -elf2flt
-mid-shared-library -mshared-library-id=0 -static -o mpicalc
mpicalc-mpicalc.o libgcrypt.la -L/home/sonic/projects/buildroot/output/host/usr/bfin-buildroot-uclinux-uclibc/sysroot/usr/lib -lgpg-error
libtool: link: /home/sonic/projects/buildroot/output/host/usr/bin/bfin-uclinux-gcc -I/home/sonic/projects/buildroot/output/host/usr/bfin-buildroot-uclinux-uclibc/sysroot/usr/include -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os
-Wl,-elf2flt -mid-shared-library -mshared-library-id=0 -D__NOMMU__
-D__uClinux__ -static -fvisibility=hidden -Wall -elf2flt
-mid-shared-library -mshared-library-id=0 -static -o mpicalc
mpicalc-mpicalc.o ./.libs/libgcrypt.a -L/home/sonic/projects/buildroot/output/host/usr/bfin-buildroot-uclinux-uclibc/sysroot/usr/lib /home/sonic/projects/buildroot/output/host/usr/bfin-buildroot-uclinux-uclibc/sysroot/usr/lib/libgpg-error.a /home/sonic/projects/buildroot/output/host/usr/bfin-buildroot-uclinux-uclibc/sysroot/usr/lib/mid-shared-library/crt1.o:
In function `__start':
/home/test/workspace/src/toolchain/uClibc/libc/sysdeps/linux/bfin/crt1.S:92:
undefined reference to `___shared_flat_add_library'
collect2: ld returned 1 exit status
make[3]: *** [mpicalc] Error 1
make[3]: Leaving directory
`/home/sonic/projects/buildroot/output/build/libgcrypt-1.6.3/src'
make[2]: *** [all-recursive] Error 1
make[2]: Leaving directory
`/home/sonic/projects/buildroot/output/build/libgcrypt-1.6.3'
make[1]: *** [all] Error 2
make[1]: Leaving directory
`/home/sonic/projects/buildroot/output/build/libgcrypt-1.6.3'
make: *** [/home/sonic/projects/buildroot/output/build/libgcrypt-1.6.3/.stamp_built]
Error 2

Regards,

Sonic

#4 lore raw

PATCH 2/2 bfin: put the libc link path of the flat shared and sep data formats before the sysroot link path

In reply to Sonic Zhang

Sonic,

Quoted 20 lines · Sonic Zhang

From: Sonic Zhang <[email protected]>

The libc of the shared flat and sep data formats are different from the standard flat libc in the sysroot lib path. In order to make application link with proper libc, put their path before the sysroot link path.

Signed-off-by Sonic Zhang <[email protected]>
---
 package/Makefile.in |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
package/Makefile.in +0 -1
index 803b162..e88d1ad 100644
@@ -153,12 +153,12 @@ TARGET_LDFLAGS += $(if $($(PKG)_FLAT_STACKSIZE),-elf2flt=-s$($(PKG)_FLAT_STACKSI
 endif
 
 ifeq ($(BR2_BINFMT_FLAT_SHARED),y)
-TARGET_LDFLAGS += -mid-shared-library -mshared-library-id=0

package/Makefile.in TARGET_LDFLAGS += $(if $($(PKG)_FLAT_STACKSIZE),-elf2flt=-s$($(PKG)_FLAT_STACKSI · line 164

Quoted 10 lines
package/Makefile.in +2 -1
+TARGET_LDFLAGS += -mid-shared-library -mshared-library-id=0 -L$(STAGING_DIR)/usr/lib/mid-shared-library
 TARGET_CFLAGS += -mid-shared-library -mshared-library-id=0
 TARGET_CXXFLAGS += -mid-shared-library -mshared-library-id=0
 endif
 ifeq ($(BR2_BINFMT_FLAT_SEP_DATA),y)
-TARGET_LDFLAGS += -msep-data
+TARGET_LDFLAGS += -msep-data -L$(STAGING_DIR)/usr/lib/msep-data
 TARGET_CFLAGS += -msep-data
 TARGET_CXXFLAGS += -msep-data
 endif

We finally discussed your patch today during a review meeting. Unfortunately, your solution is not appropriate for two reasons:

1) It applies to all cases, while the problem is completely specific to the Analog Devices Blackfin toolchain.

2) The problem that needs to be solved is in fact that the external toolchain logic in Buildroot seems to not be copying the right sysroot. If you build with -mid-shared-library, then it's directly this libc.a variant that should be in $(STAGING_DIR)/usr/lib.

Could you instead investigate the external toolchain code to understand why the appropriate sysroot doesn't get copied?

In the mean time, I'll mark your patch as Changes Requested in our patch tracking system.

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw

PATCH 2/2 bfin: put the libc link path of the flat shared and sep data formats before the sysroot link path

Hello Sonic,

First of all, thanks for replying so quickly. Since we haven't seen much patches from you improving the Blackfin support in upstream Buildroot, I wasn't sure you would answer our e-mail.

Quoted 5 lines · Sonic

The right sysroot of the Blackfin toolchain is copied by the external toolchain logic in buildroot. The problem is that the libc files for FLAT, Shared FLAT and Shared FLAT with separate data formats are put into different subfolders of the same blackfin toolchain for FLAT binary.

Absolutely.

Quoted 3 lines

Do you suggest to copy the libc for current FLAT format type into $(STAGING_DIR)/usr/lib in the toolchain copy logic? If so, I can send a new patch.

Yes, but this should normally happen "magically". Let me take an example from another toolchain to illustrate how it works.

If you take the CodeSourcery ARM toolchain, it has three sysroots:

- One for ARMv5 (the default one)
- One for ARMv4
- One for ARMv7 Thumb2

See below:

$ ./bin/arm-none-linux-gnueabi-gcc -print-file-name=libc.a
/home/thomas/projets/buildroot/output/host/opt/ext-toolchain/bin/../arm-none-linux-gnueabi/libc/usr/lib/libc.a
$ ./bin/arm-none-linux-gnueabi-gcc -march=armv4t -print-file-name=libc.a
/home/thomas/projets/buildroot/output/host/opt/ext-toolchain/bin/../arm-none-linux-gnueabi/libc/armv4t/usr/lib/libc.a
$ ./bin/arm-none-linux-gnueabi-gcc -march=armv7-a -mthumb -print-file-name=libc.a
/home/thomas/projets/buildroot/output/host/opt/ext-toolchain/bin/../arm-none-linux-gnueabi/libc/thumb2/usr/lib/libc.a

If I build an ARMv5 system in Buildroot, then the sysroot from libc/ will be copied to $(STAGING_DIR) :

$ md5sum output/staging/usr/lib/libc.a 
0c6a145ebbc860800152c2d8c82e5af9  output/staging/usr/lib/libc.a
$ md5sum output/host/opt/ext-toolchain/bin/../arm-none-linux-gnueabi/libc/usr/lib/libc.a
0c6a145ebbc860800152c2d8c82e5af9  output/host/opt/ext-toolchain/bin/../arm-none-linux-gnueabi/libc/usr/lib/libc.a

If I now build an ARMv4 system in Buildroot, then the sysroot from libc/armv4t/ will be copied to $(STAGING_DIR) :

$ md5sum output/staging/usr/lib/libc.a 
21109c9b68203d10437446d998d782c7  output/staging/usr/lib/libc.a
$ md5sum output/host/opt/ext-toolchain/bin/../arm-none-linux-gnueabi/libc/armv4t/usr/lib/libc.a
21109c9b68203d10437446d998d782c7  output/host/opt/ext-toolchain/bin/../arm-none-linux-gnueabi/libc/armv4t/usr/lib/libc.a

And finally, if I build an ARMv7 Thumb2 system in Buildroot, then the sysroot from libc/thumb2/ will be copied to $(STAGING_DIR) :

$ md5sum output/staging/usr/lib/libc.a 
d94693412ceac0839883ed0ece38c43f  output/staging/usr/lib/libc.a
$ md5sum output/host/opt/ext-toolchain/bin/../arm-none-linux-gnueabi/libc/thumb2/usr/lib/libc.a
d94693412ceac0839883ed0ece38c43f  output/host/opt/ext-toolchain/bin/../arm-none-linux-gnueabi/libc/thumb2/usr/lib/libc.a

This all gets done magically by the existing external toolchain logic, with no special handling for this CodeSourcery toolchain. In all three cases, we end up with $(STAGING_DIR) containing a copy of the sysroot that corresponds to the selected architecture flags. If you want to try this out by ourself, try the following configurations:

- For ARMv5

BR2_arm=y
BR2_TOOLCHAIN_EXTERNAL=y

- For ARMv4

BR2_arm=y
BR2_arm920t=y
BR2_TOOLCHAIN_EXTERNAL=y

- For ARMv7/Thumb2

BR2_arm=y
BR2_cortex_a8=y
BR2_ARM_EABI=y
BR2_ARM_SOFT_FLOAT=y
BR2_ARM_INSTRUCTIONS_THUMB2=y
BR2_TOOLCHAIN_EXTERNAL=y

Now, back to the Blackfin toolchain, if we ask gcc for the location of libc.a (like I did before with the ARM toolchain), we get :

- With no special flags

$ ./bin/bfin-uclinux-gcc -print-file-name=libc.a
/home/thomas/projets/buildroot/output/host/opt/ext-toolchain/bfin-uclinux/bin/../bfin-uclinux/runtime/usr/lib/libc.a

- With -mid-shared-library

$ ./bin/bfin-uclinux-gcc -mid-shared-library -print-file-name=libc.a
/home/thomas/projets/buildroot/output/host/opt/ext-toolchain/bfin-uclinux/bin/../bfin-uclinux/runtime/usr/lib/mid-shared-library/libc.a

- With -msep-data

$ ./bin/bfin-uclinux-gcc -msep-data -print-file-name=libc.a
/home/thomas/projets/buildroot/output/host/opt/ext-toolchain/bfin-uclinux/bin/../bfin-uclinux/runtime/usr/lib/msep-data/libc.a

As you can see, it behaves like the CodeSourcery ARM toolchain:
depending on the gcc flags passed, it points to a different sysroot. So my expectation is that the external toolchain logic that works for the CodeSourcery ARM toolchain should also work for the ADI Blackfin toolchain. Note that Buildroot specifically uses the location of libc.a to determine where the sysroot is. See:

# Returns the location of the libc.a file for the given compiler + flags define toolchain_find_libc_a
$$(readlink -f $$(LANG=C $(1) -print-file-name=libc.a)) endef

Can you investigate why the external toolchain logic doesn't work for the Blackfin use case, and whether it can be fixed/extended to cover this case ?

I saw your patch, which could be a work-around specific to the Blackfin toolchain. It takes into account the mid-shared-library and msep-data cases, but it doesn't take into account the other sysroot available in the Blackfin toolchain:

$ ./output/host/opt/ext-toolchain/bfin-uclinux/bin/bfin-uclinux-gcc -print-multi-lib
.;
bf532-none;@mcpu=bf532-none
mid-shared-library;@mid-shared-library
msep-data;@msep-data
bf532-none/mid-shared-library;@mcpu=bf532-none at mid-shared-library
bf532-none/msep-data;@mcpu=bf532-none at msep-data

So we would have to add other cases for the bf532-none flag (but what is this flag ? the gcc documentation documents -mcpu=bf532, but certainly not -mcpu=bf532-none).

Could you look into this ?

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#7 lore raw

PATCH 1/2 bfin: remove the dependency of flat shared mode to shared library

In reply to Sonic Zhang

Dear Sonic Zhang,

Quoted 16 lines · Sonic Zhang

From: Sonic Zhang <[email protected]>

Blackfin flat shared library isn't generated and linked in the same way as the ELF shared library. So, don't mark it depend on the static library only.

Signed-off-by Sonic Zhang <[email protected]>
---
 arch/Config.in |    1 -
 1 file changed, 1 deletion(-)
arch/Config.in +0 -0
index 16ad8be..6179960 100644

arch/Config.in config BR2_BINFMT_FLAT_SEP_DATA · line 298

Quoted 4 lines
arch/Config.in +0 -1
@@ -295,7 +295,6 @@ config BR2_BINFMT_FLAT_SEP_DATA
 
 config BR2_BINFMT_FLAT_SHARED
 	bool "Shared binary"
-	select BR2_BINFMT_SUPPORTS_SHARED

If you don't select BR2_BINFMT_SUPPORTS_SHARED, then the only available option amongst: static libs, static libs + shared libs, shared libs is "static libs", which means that the "-static" flag is always passed. If that's OK, then I'm fine with the patch.

The second sentence of your commit log is confusing however. You say "don't mark it depend on the static library only". But it is exactly what your patch is doing: since it's no longer select BR2_BINFMT_SUPPORTS_SHARED, it means that after your patch is applied, when BR2_BINFMT_FLAT_SHARED is used, the *only* possibility is static library only.

Could you clarify?

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#8 lore raw

PATCH 1/2 bfin: remove the dependency of flat shared mode to shared library

Hi Thomas,

Quoted 20 lines · Thomas Petazzoni

Dear Sonic Zhang,

Quoted 17 lines · Sonic Zhang

From: Sonic Zhang <[email protected]>

Blackfin flat shared library isn't generated and linked in the same way as the ELF shared library. So, don't mark it depend on the static library only.

Signed-off-by Sonic Zhang <[email protected]>
---
 arch/Config.in |    1 -
 1 file changed, 1 deletion(-)
arch/Config.in +0 -0
index 16ad8be..6179960 100644
@@ -295,7 +295,6 @@ config BR2_BINFMT_FLAT_SEP_DATA
Quoted 9 lines

arch/Config.in config BR2_BINFMT_FLAT_SEP_DATA · line 298

Quoted 3 lines
arch/Config.in +0 -1
 config BR2_BINFMT_FLAT_SHARED
      bool "Shared binary"
-     select BR2_BINFMT_SUPPORTS_SHARED

If you don't select BR2_BINFMT_SUPPORTS_SHARED, then the only available option amongst: static libs, static libs + shared libs, shared libs is "static libs", which means that the "-static" flag is always passed. If that's OK, then I'm fine with the patch.

Yes, -static is OK in the link flag for FLAT shared mode.

Quoted 6 lines

The second sentence of your commit log is confusing however. You say "don't mark it depend on the static library only". But it is exactly what your patch is doing: since it's no longer select BR2_BINFMT_SUPPORTS_SHARED, it means that after your patch is applied, when BR2_BINFMT_FLAT_SHARED is used, the *only* possibility is static library only.

Sorry for the typo error. Should be "mark it depend on the static library only"".

Regards,

Sonic

#9 lore raw

PATCH 1/2 bfin: remove the dependency of flat shared mode to shared library

Quoted 33 lines · Sonic Zhang

Hi Thomas,

Quoted 20 lines · Thomas Petazzoni

Dear Sonic Zhang,

Quoted 17 lines · Sonic Zhang

From: Sonic Zhang <[email protected]>

Blackfin flat shared library isn't generated and linked in the same way as the ELF shared library. So, don't mark it depend on the static library only.

Signed-off-by Sonic Zhang <[email protected]>
---
 arch/Config.in |    1 -
 1 file changed, 1 deletion(-)
arch/Config.in +0 -0
index 16ad8be..6179960 100644
@@ -295,7 +295,6 @@ config BR2_BINFMT_FLAT_SEP_DATA
Quoted 9 lines

arch/Config.in config BR2_BINFMT_FLAT_SEP_DATA · line 298

Quoted 3 lines
arch/Config.in +0 -1
 config BR2_BINFMT_FLAT_SHARED
      bool "Shared binary"
-     select BR2_BINFMT_SUPPORTS_SHARED

If you don't select BR2_BINFMT_SUPPORTS_SHARED, then the only available option amongst: static libs, static libs + shared libs, shared libs is "static libs", which means that the "-static" flag is always passed. If that's OK, then I'm fine with the patch.

Quoted 1 line

Yes, -static is OK in the link flag for FLAT shared mode.

But then it won't generate dynamically linked executables, right?

Quoted 9 lines
Quoted 6 lines

The second sentence of your commit log is confusing however. You say "don't mark it depend on the static library only". But it is exactly what your patch is doing: since it's no longer select BR2_BINFMT_SUPPORTS_SHARED, it means that after your patch is applied, when BR2_BINFMT_FLAT_SHARED is used, the *only* possibility is static library only.

Sorry for the typo error. Should be "mark it depend on the static library only"".

The problem is the following: when we say 'depends on !STATIC', that typically
means that the package is using dlopen, which is not possible with FLAT. So it would be wrong to say that FLAT selects BR2_STATIC_LIBS.

The right solution would be to introduce something else for dlopen, and rename
all the BR2_STATIC_LIBS dependencies to that new symbol.

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
author
#10 lore raw

PATCH 1/2 bfin: remove the dependency of flat shared mode to shared library

Hi Arnout,

Quoted 34 lines · Arnout Vandecappelle
Quoted 33 lines · Sonic Zhang

Hi Thomas,

Quoted 29 lines · Thomas Petazzoni

Dear Sonic Zhang,

Quoted 16 lines · Sonic Zhang

From: Sonic Zhang <[email protected]>

Blackfin flat shared library isn't generated and linked in the same way as the ELF shared library. So, don't mark it depend on the static library only.

Signed-off-by Sonic Zhang <[email protected]>
---
 arch/Config.in |    1 -
 1 file changed, 1 deletion(-)
arch/Config.in +0 -0
index 16ad8be..6179960 100644

arch/Config.in config BR2_BINFMT_FLAT_SEP_DATA · line 298

Quoted 4 lines
arch/Config.in +0 -1
@@ -295,7 +295,6 @@ config BR2_BINFMT_FLAT_SEP_DATA
 
 config BR2_BINFMT_FLAT_SHARED
      bool "Shared binary"
-     select BR2_BINFMT_SUPPORTS_SHARED

If you don't select BR2_BINFMT_SUPPORTS_SHARED, then the only available option amongst: static libs, static libs + shared libs, shared libs is "static libs", which means that the "-static" flag is always passed. If that's OK, then I'm fine with the patch.

Quoted 3 lines
Quoted 1 line

Yes, -static is OK in the link flag for FLAT shared mode.

But then it won't generate dynamically linked executables, right?

Yes, Bfin FLAT shared format isn't the standard dynamic linked ELF. For example, following command is used for FLAT shared.

bfin-uclinux-gcc -Wl,-elf2flt -mid-shared-library
-mshared-library-id=0 -static -D__NOMMU__ -Wall -o mpicalc
mpicalc-mpicalc.o ./.libs/libgcrypt.a -L/home/sonic/projects/buildroot/output/host/usr/bfin-buildroot-uclinux-uclibc/sysroot/usr/lib /home/sonic/projects/buildroot/output/host/usr/bfin-buildroot-uclinux-uclibc/sysroot/usr/lib/libgpg-error.a

Quoted 9 lines
Quoted 9 lines
Quoted 8 lines

The second sentence of your commit log is confusing however. You say "don't mark it depend on the static library only". But it is exactly what your patch is doing: since it's no longer select BR2_BINFMT_SUPPORTS_SHARED, it means that after your patch is applied, when BR2_BINFMT_FLAT_SHARED is used, the *only* possibility is static library only.

Quoted 6 lines
Quoted 2 lines

Sorry for the typo error. Should be "mark it depend on the static library only"".

The problem is the following: when we say 'depends on !STATIC', that typically
means that the package is using dlopen, which is not possible with FLAT. So it would be wrong to say that FLAT selects BR2_STATIC_LIBS.

The FLAT share mode depends on STATIC other than !STATIC. So, I see no error here.

Regards,

Quoted 18 lines

The right solution would be to introduce something else for dlopen, and rename
all the BR2_STATIC_LIBS dependencies to that new symbol.

Regards,
Arnout

--

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#11 lore raw

PATCH 1/2 bfin: remove the dependency of flat shared mode to shared library

Quoted 41 lines · Sonic Zhang

Hi Arnout,

Quoted 34 lines · Arnout Vandecappelle
Quoted 33 lines · Sonic Zhang

Hi Thomas,

Quoted 29 lines · Thomas Petazzoni

Dear Sonic Zhang,

Quoted 16 lines · Sonic Zhang

From: Sonic Zhang <[email protected]>

Blackfin flat shared library isn't generated and linked in the same way as the ELF shared library. So, don't mark it depend on the static library only.

Signed-off-by Sonic Zhang <[email protected]>
---
 arch/Config.in |    1 -
 1 file changed, 1 deletion(-)
arch/Config.in +0 -0
index 16ad8be..6179960 100644

arch/Config.in config BR2_BINFMT_FLAT_SEP_DATA · line 298

Quoted 4 lines
arch/Config.in +0 -1
@@ -295,7 +295,6 @@ config BR2_BINFMT_FLAT_SEP_DATA
 
 config BR2_BINFMT_FLAT_SHARED
      bool "Shared binary"
-     select BR2_BINFMT_SUPPORTS_SHARED

If you don't select BR2_BINFMT_SUPPORTS_SHARED, then the only available option amongst: static libs, static libs + shared libs, shared libs is "static libs", which means that the "-static" flag is always passed. If that's OK, then I'm fine with the patch.

Quoted 3 lines
Quoted 1 line

Yes, -static is OK in the link flag for FLAT shared mode.

But then it won't generate dynamically linked executables, right?

Quoted 8 lines

Yes, Bfin FLAT shared format isn't the standard dynamic linked ELF. For example, following command is used for FLAT shared.

bfin-uclinux-gcc -Wl,-elf2flt -mid-shared-library
-mshared-library-id=0 -static -D__NOMMU__ -Wall -o mpicalc
mpicalc-mpicalc.o ./.libs/libgcrypt.a -L/home/sonic/projects/buildroot/output/host/usr/bfin-buildroot-uclinux-uclibc/sysroot/usr/lib /home/sonic/projects/buildroot/output/host/usr/bfin-buildroot-uclinux-uclibc/sysroot/usr/lib/libgpg-error.a

So I tried linking with a few different options, and it looks that with the
-mid-shared-library option, it's always a shared executable that is created even if -static is specified as well. flthdr tells me:

Flags: 0x2 ( Has-PIC-GOT )

When I link without -mid-shared-library and with -static, I get an executable with:

Flags: 0x1 ( Load-to-Ram )

So basically, with BR2_BINFMT_FLAT_SHARED, you _always_ get shared and the
static options should be removed. Except that it's not shared in the way that we use it in buildroot, because libdl is not available.

So basically, the fix by Sonic Zhang does the correct thing, but it looks
pretty strange. Perhaps after adding an explanatory comment it could be accepted.

I'll resend.

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F