binutils: zlib not correctly embedded

[geimer]

The way in which zlib is embedded in libbfd is half-way broken. While the zlib symbols correctly show up in libbfd.so

$ nm $EBROOTBINUTILS/lib/libbfd.so | grep inflate
00000000000f2d80 T inflate
00000000000f50a0 T inflateCodesUsed
00000000000f4e00 T inflateCopy
00000000000f49b0 T inflateEnd
[...]

they do not in libbfd.a:

$ nm $EBROOTBINUTILS/lib/libbfd.a | grep inflate
nm: libz.a: File format not recognized
                 U inflate
                 U inflateEnd
                 U inflateInit_
                 U inflateReset

As can be seen from the first line of the output, the reason is that the whole libz.a is included in libbfd.a rather than the individual object files of libz.a. This leads to problems when linking a program statically.

Example:

$ cat foo.c
#define PACKAGE_NAME
#define PACKAGE_VERSION

#include <stdio.h>

#include <bfd.h>

int main(int argc, char** argv)
{
   bfd_init();

   bfd * bfdFile = bfd_openr( "/bin/ls", "elf64-x86-64" );
   if ( bfdFile == NULL )
   {
       printf( "Error [%x]: %s\n", bfd_get_error(), bfd_errmsg(bfd_get_error()) );
       return 1;
   }
   if ( !bfd_check_format( bfdFile, bfd_object ))
   {
       printf( "Error [%x]: %s\n", bfd_get_error(), bfd_errmsg(bfd_get_error()) );
       return 1;
   }

   bfd_close( bfdFile );
   return 0;
}

$ gcc -o foo foo.c -static -lbfd -liberty -ldl
/.../binutils/2.29.1/lib/libbfd.a(plugin.o):plugin.c:function try_load_plugin: warning: Using 'dlopen' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/.../binutils/2.29.1/lib/libbfd.a(compress.o):compress.c:function decompress_contents: error: undefined reference to 'inflateInit_'
/.../binutils/2.29.1/lib/libbfd.a(compress.o):compress.c:function decompress_contents: error: undefined reference to 'inflateReset'
/.../binutils/2.29.1/lib/libbfd.a(compress.o):compress.c:function decompress_contents: error: undefined reference to 'inflate'
/.../binutils/2.29.1/lib/libbfd.a(compress.o):compress.c:function decompress_contents: error: undefined reference to 'inflateEnd'
/.../binutils/2.29.1/lib/libbfd.a(compress.o):compress.c:function bfd_compress_section_contents: error: undefined reference to 'compressBound'
/.../binutils/2.29.1/lib/libbfd.a(compress.o):compress.c:function bfd_compress_section_contents: error: undefined reference to 'compress'

I'm not entirely sure what the right way to fix this is. Maybe set ZLIB to -L$EBROOTZLIB/lib -lz rather than $EBROOTZLIB/lib/libz.a in all the Makefile.ins?

[geimer]

The suggestion above does not have the desired effect. I tried it and the sanity check fails saying that zlib was not statically linked.

The longer I look into this and think about it, the less I am convinced that embedding zlib in libbfd is a good idea. Meanwhile I believe that the original approach of zlib being a regular dependency is the right way to go. Yes, it adds a dependency. But it is much cleaner, avoids inconsistencies (libbfd.so including zlib symbols; libbfd.a not including them), avoids forcing a behavior the package has not been designed for, and it simply feels wrong to provide the same symbols by two different libraries.

@boegel, @wpoely86: What do you think? Any comments?

[wpoely86]

I'm actually more interested why we cannot static link zlib to libbfd.a. This should work and not just include the library archive as an object.

[geimer]

@wpoely86 Here is the corresponding excerpt from the build log:

/bin/bash ./libtool --tag=CC   --mode=link gcc -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Wstack-usage=262144 -I/opt/eb/software/Compiler/GCCcore/6.4.0/zlib/1.2.11/include -O2 -march=native -fPIC  -release `cat libtool-soversion`  -static-libstdc++ -static-libgcc -L/opt/eb/software/Compilers/GCCcore/6.4.0/lib64 -L/opt/eb/software/Compilers/GCCcore/6.4.0/lib -L/opt/eb/software/Compiler/GCCcore/6.4.0/flex/2.6.4/lib -L/opt/eb/software/Compiler/GCCcore/6.4.0/Bison/3.0.4/lib -L/opt/eb/software/Compiler/GCCcore/6.4.0/zlib/1.2.11/lib -L/opt/eb/software/Bootstrap/binutils/2.29.1/lib -o libbfd.la -rpath /opt/eb/software/Compiler/GCCcore/6.4.0/binutils/2.29.1/lib archive.lo archures.lo bfd.lo bfdio.lo bfdwin.lo cache.lo coff-bfd.lo compress.lo corefile.lo format.lo hash.lo init.lo libbfd.lo linker.lo merge.lo opncls.lo reloc.lo section.lo simple.lo stab-syms.lo stabs.lo syms.lo targets.lo binary.lo ihex.lo srec.lo tekhex.lo verilog.lo `cat ofiles` -L/tmp/eb-admin/binutils/2.29.1/GCCcore-6.4.0/binutils-2.29.1/bfd/../libiberty/pic -liberty -Wl,-lc,--as-needed,-lm,--no-as-needed -ldl /opt/eb/software/Compiler/GCCcore/6.4.0/zlib/1.2.11/lib/libz.a -ldl 

*** Warning: Linking the shared library libbfd.la against the
*** static library /opt/eb/software/Compiler/GCCcore/6.4.0/zlib/1.2.11/lib/libz.a is not portable!
libtool: link: gcc -shared  -fPIC -DPIC  .libs/archive.o .libs/archures.o .libs/bfd.o .libs/bfdio.o .libs/bfdwin.o .libs/cache.o .libs/coff-bfd.o .libs/compress.o .libs/corefile.o .libs/format.o .libs/hash.o .libs/init.o .libs/libbfd.o .libs/linker.o .libs/merge.o .libs/opncls.o .libs/reloc.o .libs/section.o .libs/simple.o .libs/stab-syms.o .libs/stabs.o .libs/syms.o .libs/targets.o .libs/binary.o .libs/ihex.o .libs/srec.o .libs/tekhex.o .libs/verilog.o .libs/elf64-x86-64.o .libs/elf-ifunc.o .libs/elf-nacl.o .libs/elf64.o .libs/elf.o .libs/elflink.o .libs/elf-attrs.o .libs/elf-strtab.o .libs/elf-properties.o .libs/elf-eh-frame.o .libs/dwarf1.o .libs/dwarf2.o .libs/elf32-i386.o .libs/elf-vxworks.o .libs/elf32.o .libs/i386linux.o .libs/aout32.o .libs/pei-i386.o .libs/peigen.o .libs/cofflink.o .libs/coffgen.o .libs/pei-x86_64.o .libs/pex64igen.o .libs/elf64-gen.o .libs/elf32-gen.o .libs/plugin.o .libs/cpu-i386.o .libs/cpu-iamcu.o .libs/cpu-l1om.o .libs/cpu-k1om.o .libs/cpu-plugin.o .libs/archive64.o   -L/opt/eb/software/Compilers/GCCcore/6.4.0/lib64 -L/opt/eb/software/Compilers/GCCcore/6.4.0/lib -L/opt/eb/software/Compiler/GCCcore/6.4.0/flex/2.6.4/lib -L/opt/eb/software/Compiler/GCCcore/6.4.0/Bison/3.0.4/lib -L/opt/eb/software/Compiler/GCCcore/6.4.0/zlib/1.2.11/lib -L/opt/eb/software/Bootstrap/binutils/2.29.1/lib -L/tmp/eb-admin/binutils/2.29.1/GCCcore-6.4.0/binutils-2.29.1/bfd/../libiberty/pic -liberty /opt/eb/software/Compiler/GCCcore/6.4.0/zlib/1.2.11/lib/libz.a -ldl  -march=native -Wl,-lc -Wl,--as-needed -Wl,-lm -Wl,--no-as-needed   -Wl,-soname -Wl,libbfd-2.29.1.so -o .libs/libbfd-2.29.1.so
libtool: link: (cd ".libs" && rm -f "libbfd.so" && ln -s "libbfd-2.29.1.so" "libbfd.so")
libtool: link: ar rc .libs/libbfd.a /opt/eb/software/Compiler/GCCcore/6.4.0/zlib/1.2.11/lib/libz.a  archive.o archures.o bfd.o bfdio.o bfdwin.o cache.o coff-bfd.o compress.o corefile.o format.o hash.o init.o libbfd.o linker.o merge.o opncls.o reloc.o section.o simple.o stab-syms.o stabs.o syms.o targets.o binary.o ihex.o srec.o tekhex.o verilog.o elf64-x86-64.o elf-ifunc.o elf-nacl.o elf64.o elf.o elflink.o elf-attrs.o elf-strtab.o elf-properties.o elf-eh-frame.o dwarf1.o dwarf2.o elf32-i386.o elf-vxworks.o elf32.o i386linux.o aout32.o pei-i386.o peigen.o cofflink.o coffgen.o pei-x86_64.o pex64igen.o elf64-gen.o elf32-gen.o plugin.o cpu-i386.o cpu-iamcu.o cpu-l1om.o cpu-k1om.o cpu-plugin.o archive64.o
libtool: link: ranlib .libs/libbfd.a
libtool: link: ( cd ".libs" && rm -f "libbfd.la" && ln -s "../libbfd.la" "libbfd.la" )

As you can see, libz.a is passed as an argument to the archiver (third line from the bottom). And the archiver does what it is told to do: include libz.a as is into the library archive. Maybe libtool is doing something bogus here, but what happens is certainly not intended.

[geimer]

Just to make it clear: There is no such thing as "statically linking a static library to a static library". You can either manually extract the object files from the library you want to embed and add them to the other static library, or you have to pass both libraries on the link line to the compiler in the right order (that's what the dependency_libs entry in the .la file is good for).

Honestly, I don't believe that there is a sensible way to make this work without significant changes to the Makefile -- which bears the risk to break even more things...

[geimer]

BTW: Shouldn't the RPATH stuff for dummy go into LDFLAGS rather than LIBS? In the end, it probably doesn't matter that much, but I'm usually in favor of following conventions...

[wpoely86]

It was put in LIBS for a reason but cannot remember what.

Maybe just use -Wl,-Bstatic -lz ? Static link de so but leave the a like it is?

[geimer]

Maybe it was put in LIBS to avoid linking against the default libs -lpthread -lm (which is a bad idea anyhow if you ask me, but that's a different story)?

I couldn't get the -Bstatic stuff to work, but as far as I understand this switch, it would only affect the shared library. So it doesn't solve anything, as the inconsistency between the static and the shared lib is still there.

[wpoely86]

Do we want to solve the inconsistency? With a static lib you must provide everything, for a dynamic the linker does the work for you?

[geimer]

The inconsistency is what is causing trouble, yes. We do use libbfd in Score-P and in our test suite we do both shared and static linking. While the shared linking succeeds since the zlib stuff is included in the .so, the static link fails due to -lz missing in the dependency_libs line of the .la file and the zlib module not being loaded as it is only a build dependency of binutils.

Bottom line: zlib can be included in the shared library, yes. But for the static library to work, one has to load the zlib module and explicitly link against -lz. So, if we need to have zlib as a dependency anyway, why not simply follow the "intended usage" and link against it in both cases?

[wpoely86]

If you want to link zlib statically, you have to load it as a build dep? Doesn't that make sense?

Anyway, I'm not against add zlib as a full dependency. But maybe we should tackle it in one go with the buildeps bundle (see mailinglist)?

[geimer]

I'm not sure the buildeps bundle will ever materialize. And to be honest, I'm not really a fan of it anyway, as it will most likely lead to endless discussions of which packages should be included and which ones not.

So, to make some progress (and to get rid of the custom easyblock I'm currently using), what about doing the following: We do the patching stuff only if zlib is listed as a build dependency, and configure with --with-system-zlib if it is either a dependency or a build dependency. This way we wouldn't break backwards compatibility, and people like me who prefer to have zlib be a real dependency only need to tweak the easyconfig.

[Frenzy-Alex]

I also had this error. From unknown me reasons during build binutils search libz.so in /usr/lib, and not in /lib. Creating symbolic link in /usr/lib ( ln -s /lib/libz.so /usr/lib/libz.so) resolve issue.

[Char-Aznable]

I got similar error building foss-2019a:

build failed (first 300 chars): zlib is not statically linked in /home/aznb/easybuild/software/binutils/2.31.1-GCCcore-8.2.0/bin/addr2line

[akesandgren]

Note that we should not touch this!!

The system versions of libbfd.a/so have the exact same setup, i.e. no definition of inflate* in libbfd.a
both for RH and Debian based systems.

I.e., libz.a should not be a part of libbfd.a at all. and it should not be used in the ar command for libbfd.a

[geimer]

Not embedding zlib is straightforward in the binutils easyblock, and I already have a working version of it. However, it also relies on adding zlib as a dependency in all binutils easyconfigs. Trivial, just work. I'm open to address this and create PRs for both, but first I would like to know whether we are in consensus that not embedding zlib is the way forward.

[boegel]

@geimer Is including zlib as a (runtime) dependency to binutils enough though to be perfectly equivalent?
Doesn't it also mean that wherever dynamic linking is done, we may have to also include -lz (whereas before it was "just working" because libz.a was embedded)?

If you're up for fixing this, please open the relevant PRs (and mention them here), so we can better assess the required changes.
As for adding zlib as a dependency in the binutils easyconfigs, I would only do this for recent versions (say 2.30 & newer?).

I'm also wondering if this implies that people will need to rebuild their existing binutils installations, or if this is a backward-compatible change w.r.t. current practice of embedded zlib into binutils (in terms of what people have to change in easyblocks/easyconfigs).

[akesandgren]

Ubuntu doesn't embed libz in libbfd.so so there shouldn't be a problem for us either.
binutils need a runtime dep on libz and it should work. As long as our libbfd.so is created with a dep for libz.so as it should be.

I.e., ignore my previous comment on this.

[akesandgren]

Anything that itself need libz of course have to link with -lz, but just using libbfd.so will work as usual.

[geimer]

In the current situation, one gets

• no reference to zlib in the so (i.e., reported by ldd libbfd.so)
• no reference to zlib in the la file (i.e., as part of dependency_libs)

Thus, ld.so is able to resolve zlib symbols as they are part of libbfd.so, but libtool can not when using static linking. With "plain" static linking (w/o libtool), one has to explicitly specify -lz.

With not trying to embed any zlib stuff, one gets

• a reference to zlib in the so
• a reference to zlib in the la file

Thus, both ld.so and libtool are able to resolve zlib symbols automatically. With plain static linking one also has to explicitly specify -lz.

[geimer]

I also should have said that the latter is what should happen and the current situation actually breaks things.

[boegel]

With #1732 and easybuilders/easybuild-easyconfigs#8340 merged, I think we can close this.
The problem reported here is fixed for binutils installation done with a non-dummy toolchain, which are the ones we care about most. The binutils installations done with a dummy toolchain are only used while building (build dependencies of) GCCcore and the binutils built with GCCcore, but not anymore afterwards, so having zlib embedded there shouldn't be a big issue; see also #1732 (comment) .

@geimer: agreed?

[geimer]

@boegel Yes, agreed. This issue should be fully addressed for "recent" binutils (>=2.28).