mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.5k stars 1.59k forks source link

Don't use absolute path to shared libraries that don't have a SONAME set #7766

Open plattfot opened 3 years ago

plattfot commented 3 years ago

Describe the bug

I have notice meson is passing in absolute paths to shared libraries instead of using -L -l. This have some implication for libraries that doesn't have the SONAME set, as the linkers in binutils will use the absolute path if the SONAME is not set. This results in hard coded paths to external libraries ends up in the NEEDED section. This is undesirable as it defeats the purpose of using shared libraries in the first place.

Using -L then -l does not result in this behavior.

I encountered this when linking against mkl-2020.1, but we have a bunch of in-house libraries that lacks the SONAME section.

To Reproduce Here is a simple example on what I mean

foo.hpp:

#pragma once

int foo();

foo.cpp:

#include "foo.hpp"

int foo()
{
  return 1;
}

main.cpp:

#include <iostream>
#include "foo.hpp"

int main()
{
  std::cout<<foo()<<std::endl;
  return 0;
}

I'm using gcc-9.3.0 and binutils-2.29.1 to compile the example:

  1. Compiling and linking the executable foo with libfoo.so using the absolute path to the library:
mkdir -p /var/tmp/scratch/test/private/build/foo-1.0.0
g++ -o /var/tmp/scratch/test/private/build/foo-1.0.0/libfoo.so -shared foo.cpp
g++ -o /var/tmp/scratch/test/private/build/foo-1.0.0/foo main.cpp -Wl,--start-group /var/tmp/scratch/test/private/build/foo-1.0.0/libfoo.so -Wl,--end-group
  1. Use the same library as in 1, but instead of setting the absolute to libfoo.so; use -L to add the search path and -l to specify the library name to search for.
g++ -o /var/tmp/scratch/test/private/build/foo-1.0.0/foo_Ll main.cpp -Wl,--start-group -L/var/tmp/scratch/test/private/build/foo-1.0.0 -lfoo -Wl,--end-group
  1. Compile the library with a soname, libfoo_soname. Then compile and link the executable foo_soname in the same manner as 1.
g++ -o /var/tmp/scratch/test/private/build/foo-1.0.0/libfoo_soname.so -shared foo.cpp -Wl,-soname=libfoo.so
g++ -o /var/tmp/scratch/test/private/build/foo-1.0.0/foo_soname main.cpp -Wl,--start-group /var/tmp/scratch/test/private/build/foo-1.0.0/libfoo_soname.so -Wl,--end-group

Running readelf -d private/build/foo-1.0.0/foo*. Note: I shorten the output to just show the NEEDED section.

File: private/build/foo-1.0.0/foo

Dynamic section at offset 0xdd0 contains 28 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [/var/tmp/scratch/test/private/build/foo-1.0.0/libfoo.so]
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]

File: private/build/foo-1.0.0/foo_Ll

Dynamic section at offset 0xdd0 contains 28 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libfoo.so]
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]

File: private/build/foo-1.0.0/foo_soname

Dynamic section at offset 0xdd0 contains 28 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libfoo.so]
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]

As you can see the full path to libfoo.so is stamped into the NEEDED section for the first executable. Where as the other two it does not.

I have tested this with both the bfd linker and the gold linker. I haven't tested this with any other linker so not sure if the behavior is the same across the board or if it is just the GNU toolchain.

Expected behavior

Expect the NEEDED section to be free of hard coded paths.

system parameters

plattfot commented 3 years ago

Ok, with help from the #mesonbuild IRC channel, I think I have located where in meson this is happening. Namely PkgConfigDependency._search_libs() in mesonbuild/dependencies/base.py.

And I've also found a workaround:

Using a custom mkl pkg-config file:

#libdir=/path/to/mkl/lib
includedir=/path/to/mkl/include

Name: mkl
Description: Core math functions hand-optimized for Intel processors
Version: 11.2.3

# Remove -L${libdir} to workaround a bug in meson
Libs:  -lmkl_rt
Cflags: -I${includedir}

Note I'm not adding -L${libdir} to Libs, this is the key to the workaround. As meson will fallback to using -l<lib> if it cannot locate the library.

After that is getting the linker to pick up the search path. The only way I found to do this was to use a linker script.

NOTE: I only tested this with ld.bfd as couldn't find how to dump the default linker script with ld.gold.

First thing is to dump the default linker script as there is no option for ld.bfd to append to the original script. And using an implicit linker script does not allow to use the SEARCH_DIR() command.

ld --verbose | awk 'BEGIN{flag=0};/^=+$/{flag=!flag; next} flag' > linker.script 

Using awk to extract the linker script from the verbose output.

Then I'm just appending the search path to mkl using the linker command SEARCH_DIR()

echo "SEARCH_DIR(/path/to/mkl/libs)" >> linker.script

To pass this to the linker via meson I'm using a native file:

[properties]
c_link_args=['-Wl,-dT/path/to/linker.script']
cpp_link_args=['-Wl,-dT/path/to/linker.script']

I was hoping I could use LIBRARY_PATH to point the linker to the correct location of the linker.script instead of using the absolute path. But meson seem to clean that environment variable before doing the actual build. It works when it does the sanity check in the beginning.

Then in the setup step I'm adding the native file meson build --native-file=linker.ini ... and that fixes my issue.

jpakkane commented 3 years ago

As an additional note: if you have shared libraries without SONAMEs, the best solution is to get them built with SONAMEs. Not having them can cause many other things besides Meson to fail in interesting ways. It is also more portable as that is what Windows does by default (and possibly mac too, but not 100% sure about that one).

plattfot commented 3 years ago

I agree that's the best way to go. But most of these libraries are A) from proprietary vendors (e.g. Autodesk Maya, SideFX Houdini and Intel MKL) and B) legacy libraries e.g. at work we still have packages using mkl-11.2.3. Therefore I don't have any control over how these get built. Most likely easier to run patchelf --set-soname on them but that is not guaranteed to work.

I'm curious, what other things have you seen caused by not having a SONAME set? This is the only issue I've come across.

nirbheek commented 3 years ago

We resolve -L -l pairs fetched from pkg-config to actual on-disk libraries on purpose, because relying on the linker to do it for us is very unreliable. If we pass -L -l pairs as-is to the linker, we're going to end up linking to libraries from the wrong prefix in many cases. For instance, if you have multiple prefixes with pkgconfig files in them. This is something that every build system other than Meson gets wrong.

Also, since we abstract away the contents of the linker commandline from users (the order of dependencies passed to a build target is not supposed to be significant) we actually can't pass -L -l pairs for all dependencies. Then there's the -L de-dup that we do in the CompilerArgs class, which yields significant size improvements in build.ninja which improves setup and build time.

Maybe we could inspect the .so library and if it doesn't have a soname set, we could reject it and fallback to -L and -l pairs for that specific library. I'd be happy to review a PR that does this.

rhd commented 3 years ago

I had the same problem w/ vendor supplied .so files. From what I remember, there are 2 fixes...

  1. https://github.com/NixOS/patchelf - fix the .so file yourself or
  2. use -lname syntax directly in the meson.build file (not find_library()) and hope the system is properly configured.

I'd love it if meson could detect this and do a fallback to using -L -l.

plattfot commented 3 years ago

Any ideas what the best way would be to inspect the .so? The ones I can come up with are:

  1. Shell out to program that can read the elf format to get the info in the dynamic section, then in python check if it contains the SONAME. E.g. using readelf from binutils or llvm-readelf from llvm as the external program and just call it with the -d flag to only get the dynamic section. I'm not familiar with the code base, but it seems it should be fairly straight forward to implement but requires an external dependency. It looks like it's already doing something similar with the clib_compiler.find_library().

  2. Implement an elf reader in pure python and see if the SONAME d_tag exist. Will need to be able to parse the elf header, jump to the dynamic section and scan for the SONAME. At least that what it seems from the little information I've gathered, but I could be way off. Looks to be way harder to implement than 1 and I have no idea how many edge cases their are, at least it needs to deal with endianness and 64 or 32 bit.

nirbheek commented 3 years ago
2\. Implement an elf reader in pure python and see if the SONAME d_tag exist

We already have a pure-python ELF reader: mesonbuild/scripts/depfixer.py, so we could reuse that code.

hwti commented 3 years ago

Would this be only for pkg-config dependencies (ie in _search_libs only), or for find_library too (ie find_library_real returning [ '-L' + trial.parent, '-l' + libname ] ) ?

nirbheek commented 3 years ago

This would be implemented in find_library_real(), which will make it work for PkgConfigDependency._search_libs() too.