[cfe-dev] RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

[Jonas Hahnfeld via cfe-dev]

Hi Joel,

Am Donnerstag, den 14.02.2019, 17:29 -0500 schrieb Joel E. Denny:
> Hi,
> 
> This RFC discusses placing Clang's libraries in Clang-dedicated
> directories (somewhere within `lib/clang` instead of `lib`) in both
> build trees and install trees.  While fixing broken linking with
> openmp libraries was the original motivation, subprojects potentially
> affected by that problem or this change also include libcxx,
> libunwind, and compiler-rt.
> 
> Motivation: Broken Default Linking
> ----------------------------------
> 
> Clang by default prefers system openmp libraries over the ones built
> alongside Clang. The effect is that Clang-compiled openmp applications
> sometimes misbehave if the user doesn't adjust the library search
> path.
> 
> For example, where `/home/jdenny/llvm` is built and installed from git
> master, but where my system openmp libraries under
> `/usr/lib/x86_64-linux-gnu` are from LLVM 6.0:
> 
> ```
> $ cd /home/jdenny/llvm
> $ cat test.c
> #include <stdio.h>
> int main() {
>   #pragma omp target teams
>   printf("hello\n");
>   return 0;
> }
> $ ./bin/clang -fopenmp -fopenmp-targets=nvptx64 test.c
> $ ./a.out
> ./a.out: error while loading shared libraries: libomptarget.so: cannot
> open shared object file: No such file or directory
> $ LD_LIBRARY_PATH=lib ./a.out
> Segmentation fault (core dumped)
> $ LD_LIBRARY_PATH=lib ldd a.out | grep libomp
>         libomp.so.5 => /usr/lib/x86_64-linux-gnu/libomp.so.5
> (0x00007fab59748000)
>         libomptarget.so => lib/libomptarget.so (0x00007fab59515000)
> ```
> 
> The problem here is a little subtle.  `-v` reveals that Clang
> specifies `-L/usr/lib/x86_64-linux-gnu` before
> `-L/home/jdenny/llvm/lib` when linking, and I have:
> 
> ```
> $ ls -l /usr/lib/x86_64-linux-gnu/libomp.so
> lrwxrwxrwx 1 root root 11 Jan  7  2018
> /usr/lib/x86_64-linux-gnu/libomp.so -> libomp.so.5
> ```
> 
> As a result, Clang links the executable as follows:
> 
> ```
> $ readelf -d a.out | grep libomp.so
>  0x0000000000000001 (NEEDED)             Shared library: [libomp.so.5]
> ```
> 
> Now `LD_LIBRARY_PATH` cannot force `a.out` to find Clang's `libomp.so`
> because `a.out` wants `libomp.so.5` instead.
> 
> As far as I know, there's nothing unusual about my system, which is a
> typical Ubuntu 18.04.2 installation.  However, I have learned from
> others that `libomp.so.5` is installed by Debian's packages not by
> LLVM's cmake files [1], so many people might not see this particular
> problem.
> 
> Even if `/usr/lib/x86_64-linux-gnu/libomp.so` weren't a symbolic link,
> it's still the wrong library, and incompatibilities could have caused
> a link failure instead of a run-time failure.
> 
> Either way, it seems surprising and confusing that Clang doesn't
> always prefer its own libraries.
> 
> Solution: Early `-L` for Clang-Dedicated Directory
> --------------------------------------------------
> 
> The easiest solution is probably to assume users will manually specify
> `-L` when necessary to link against the right `libomp.so`.  However,
> it seems like Clang should just do that automatically to help
> unsuspecting users avoid surprises like the one above.

+1 that Clang should get this right in most cases.

> I wrote a patch to make that happen [2].  The patch's basic approach
> is to place `libomp .so` in a directory for which, when linking, Clang
> automatically specifies a `-L` earlier than any `-L` for a system
> library directory.  It's important that the directory is a
> Clang-dedicated directory (somewhere under `lib/clang`) so that other
> libraries unrelated to Clang are not accidentally elevated in the
> library search path.  When possible, the patch makes this change for
> both the build tree and install tree so users can work out of either
> tree.

The drawback is that this directory is added before LIBRARY_PATH, see
below.

> This solution seems straight-forward, but it raises many questions.
> 
> Q1: What is the right Clang-dedicated directory?
> ------------------------------------------------
> 
> There are many possibilities.  On my system:
> 
> * `lib`: This directory is obviously not Clang-dedicated, but it's
> where libcxx, libunwind, and openmp currently place their libraries by
> default.
> 
> * `lib/clang/9.0.0/lib/linux`: This directory is where compiler-rt
> libraries are placed by default, and their names are like
> `libclang_rt.asan-x86_64.so`.
> 
> * `lib/clang/9.0.0/x86_64-unknown-linux-gnu/lib`: When the cmake
> option `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` is set, this
> directory is where compiler-rt, libcxx, and libunwind libraries are
> placed, and the compiler-rt libraries then drop the redundant
> `-x86_64` from their names.
> 
> * `lib/clang/9.0.0/lib/linux/x86_64`: This directory has the nice
> advantage that `-frtlib-add-rpath` tells Clang to add a `-rpath` for
> this directory so you don't have to set `LD_LIBRARY_PATH` when
> executing `a.out`.  However, it turns out that `-rpath` doesn't work
> correctly for me in the case of openmp offloading [3].  As far as I
> can tell, this directory is currently populated only by some Android
> tool chains [3].
> 
> * For libcxx and libunwind libraries, it's been suggested that a
> Clang-dedicated directory should not be specific to the Clang version
> (shouldn't contain `9.0.0`) because the libraries themselves are not
> locked to the Clang version, and a possible directory layout has been
> proposed [4].  My understanding is that this approach generally makes
> sense when the shared libraries have version numbers, like
> `libomp.so.5`.  In that case, it needs to be possible to find older
> shared libraries that older software has already become dependent upon
> while using older Clang installations.  It's easier to find them, with
> `LD_LIBRARY_PATH` for example, if every Clang installation places such
> libraries in the same Clang-version-independent directory, where the
> libraries are still distinct due to the version numbers in their
> names, instead of placing them in a bunch of different
> Clang-version-specific directories.  Please correct me if I've
> misunderstood this situation.

AFAIK libc++ and libunwind are using the same ABI version .1 since the
beginning. That's probably kind of the same guarantee that libomp has:
You can run an old binary with a newer version of the library.
However, that defeats the point of being "distinct" when installing the
libraries in a single directory that is independent of the Clang
version.

> Which choice is right for openmp libraries?  Debian is giving version
> numbers to `libomp.so`, but is that right?  If it is, the last choice
> might make sense.  If not, perhaps one of the previous choices is
> better.
> 
> Q2: Should `lib` also contain the libraries?
> --------------------------------------------
> 
> As mentioned above, `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` moves
> libcxx, libunwind, and compiler-rt libraries to their new
> Clang-dedicated directory.  However, the patch I've proposed for
> openmp instead duplicates libraries into their new Clang-dedicated
> directory.  I figured duplicating would be better for backward
> compatibility than moving.  Is it necessary?
> 
> (Actually, I should probably be creating symlinks instead of duplicates.)
> 
> Q3: Is it really OK to elevate Clang's libraries?
> -------------------------------------------------
> 
> For me, this is the biggest unknown about this change.  That is, does
> any user depend on being able to drop his own `libomp.so` in a system
> library directory so that Clang will prefer it over Clang's own
> `libomp.so`?  If so, is that an important enough use case that we
> should continue to risk subtly breaking the compiles of other users by
> default?

As I said in the reviews we are building and using custom versions of
libomp with a globally installed (release) version of Clang (like we
can do with the Intel Compiler). For my usage I'm currently relying on
a combination of LIBRARY_PATH and LD_LIBRARY_PATH which many people
think is fragile, but it has worked so far. I guess we could switch to
using `-L` (and LD_LIBRARY_PATH), but setting an environment variable
is easier than telling all kind of build systems to modify their
compiler invocations...

I'm not sure if that qualifies as "important enough", but I think
that's a valid use case that would break with the proposed changes.
Maybe we can have a directory that is added after LIBRARY_PATH but
before all "default" system directories?

> Q4: Should libcxx, libunwind, or compiler-rt be changed too?
> ------------------------------------------------------------
> 
> An early reaction to my patch was that we need to either handle the
> various Clang libraries consistently or show why they should be
> handled differently.  My patch so far only changes openmp, and it does
> not make it consistent with other subprojects' libraries.
> 
> Keep in mind that, while compiler-rt libraries are already placed in a
> Clang-dedicated directory by default, libcxx and libunwind libraries
> are not and so could potentially be affected by the motivating example
> I gave.  Must users specify
> `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` (which adds the problem of
> Clang-version locking) to avoid that problem?  Shouldn't the default
> configuration be the most user-friendly?
> 
> Conclusion
> ----------
> 
> So far, my estimate at the best path forward is as follows:
> 
> * openmp and compiler-rt libraries should always be placed in a
> Clang-dedicated, Clang-version-specific directory.

Based on the reasons given above I don't understand why openmp needs to
be version-locked with Clang. I think the guarantee is that you can run
old binaries with a newer version of the library, just as you can with
libc++ and friends. So why should openmp be treated differently?

Regards,
Jonas

> * libcxx and libunwind libraries should always be placed in a
> Clang-dedicated, Clang-version-independent directory.
> 
> * For each of those directories, Clang should specify `-L` earlier
> than any system library directory.
> 
> * openmp, libcxx, and libunwind libraries should also be symlinked
> directly in `lib`, for backward compatibility at least.
> 
> * `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` should be dropped and
> dependent projects updated.  Alternatively, for backward
> compatibility, this option could revert the Clang-dedicated directory
> layout to the layout it currently produces (in particular, it places
> libcxx and libunwind libraries in Clang-version-specific directories).
> 
> There are details like exact directory names to discuss, but does this
> high-level plan make sense?
> 
> Thanks.
> 
> Joel
> 
> [1]: 
https://reviews.llvm.org/D55725#1370823> 
> [2]: 
https://reviews.llvm.org/D55725> 
> [3]: 
https://reviews.llvm.org/D55725#1371807> 
> [4]: 
https://reviews.llvm.org/D30733#697781> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190217/164ea928/attachment.sig>