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

Wed Feb 20 18:14:40 PST 2019

Hi Petr,

On Sun, Feb 17, 2019 at 1:39 PM Petr Hosek <phosek at chromium.org> wrote:
>
> On Thu, Feb 14, 2019 at 2:29 PM Joel E. Denny via cfe-dev
> <cfe-dev at lists.llvm.org> wrote:
> >
> > 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.
> >
> > 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.
> >
> > 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.
>
> That's not the motivation behind LLVM_ENABLE_PER_TARGET_RUNTIME_DIR,
> it's just a side-effect of that change. The motivation behind this
> change is to support distributing and using runtimes for different
> targets in a single Clang toolchain.
>
> > * `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].
>
> This layout has one major downside, which is the fact that
> architecture and operating system is insufficient. For example, say I
> want to distribute runtimes built for x86_64-linux-gnu and
> x86_64-linux-musl, where would these go? You cannot place them both in
> lib/clang/9.0.0/lib/linux/x86_64 because they're incompatible. You
> could consider adding another component to differentiate environment,
> but what about targets that don't use it? Target triples already solve
> all that which is why LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses those.
>
> I talked to pirama who originally introduced this layout about
> migrating to the LLVM_ENABLE_PER_TARGET_RUNTIME_DIR layout and he was
> fine with it, I just haven't had time to do it (it's a bit more
> involved than just making LLVM side changes because we would also need
> to make sure that nothing is using it on the Android side). However, I
> think it would be the right thing to do to reduce the number of
> layouts we support.

Thanks for explaining.  No arguments here.

>
> > * 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.
> >
> > 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?
> >
> > 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.
> >
> > * 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).
>
> What's your proposed alternative?

My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
bullets.  In other words, you wouldn't need to specify
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
always on (except the directories might be different than now if the
version locking issue is important, as noted above).  Is that what
you're asking?

> Having support for per-target
> runtime directories has been proposed several times in the past. First
> proposal I found was from chandlerc in 2011
> (http://lists.llvm.org/pipermail/llvm-dev/2011-November/045565.html),

I hadn't seen that one.  This has been discussed for quite a while.

Thanks.

Joel

> another one from mgorny
> (http://lists.llvm.org/pipermail/cfe-dev/2017-January/052512.html) and
> the last one from myself
> (http://lists.llvm.org/pipermail/cfe-dev/2017-December/056442.html). I
> implemented my proposal (which is basically the same as chandlerc's)
> which is LLVM_ENABLE_PER_TARGET_RUNTIME_DIR. This is currently
> supported for all the platforms except for Darwin which is
> more-complicated due to use of fat binaries, but beanz has been
> working on that.
>
> We currently distribute our Clang toolchain for multiple different
> host platforms (x86_64-linux-gnu, aarch64-linux-gnu,
> x86_64-apple-darwin) and support multiple different target platforms
> (i386-linux-gnu, armhf-linux-gnueabi, x86_64-linux-gnu,
> aarch64-linux-gnu, x86_64-fuchsia, aarch-fuchsia). This what the
> resource directory currently looks like:
>
> $prefix/lib/clang/9.0.0/include
> $prefix/lib/clang/9.0.0/i386-linux-gnu
> $prefix/lib/clang/9.0.0/aarch64-fuchsia
> $prefix/lib/clang/9.0.0/aarch64-linux-gnu
> $prefix/lib/clang/9.0.0/armhf-linux-gnueabi
> $prefix/lib/clang/9.0.0/x86_64-fuchsia
> $prefix/lib/clang/9.0.0/x86_64-linux-gnu
>
> I'm also trying to build runtimes for Windows. This toolchain is being
> used by several projects (Fuchsia, Flutter, Dart and probably some
> others I'm not aware of). The idea is that you shouldn't need one
> toolchain per target, a single toolchain should be able to target
> multiple different targets, but for that you need runtimes for each
> target platform (plus sysroot).
>
> Alternative that has been suggested in the past is to ship runtimes
> inside sysroot and not with Clang. That would work but IMHO it's
> inconvenient. It's true that runtimes (e.g. libc++) aren't in theory
> version locked to Clang, but there have been and will be cases when a
> new flag is implemented in Clang and immediately used in libc++. Very
> often these changes address various issues that users run into. We
> want to consume those improvements as soon as they're available.
> That's why we're building all the runtimes with the toolchain and with
> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we can do so with a single
> CMake+Ninja invocation. If we were to ship these runtimes with the
> sysroot, it would mean re-building and packaging those runtimes
> separately which would require significantly more effort.
>
> If you want to drop LLVM_ENABLE_PER_TARGET_RUNTIME_DIR, then you
> really need to provide some alternative that's going to support the
> same use cases and ideally would be as convenient to use.
>
> > 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
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev