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

Wed Feb 20 18:27:21 PST 2019

On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <jdenny.ornl at gmail.com> wrote:
>
> Hi Jonas,
>
> On Sun, Feb 17, 2019 at 4:05 AM Jonas Hahnfeld <hahnjo at hahnjo.de> wrote:
> >
> > 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.
>
> I'm not close enough to these libraries to make a decision here.  It
> sounds like libomp, libc++, and libunwind are not handled
> consistently, and so far we don't have any solid justification for
> that inconsistency.
>
> >
> > > 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.
>
> I don't know if it's important enough either.  However, it suffers
> from the problem I mentioned in the example in the motivation of the
> RFC.  That is, system directories that cause the trouble in that
> example have precedence over Clang's lib directory, which has
> precedence over LIBRARY_PATH.  Thus, libomp.so in LIBRARY_PATH will
> not be seen if such a system directory has a libomp.so.
>
> > Maybe we can have a directory that is added after LIBRARY_PATH but
> > before all "default" system directories?
>
> That's not possible because the system directories are before LIBRARY_PATH.

Would this change even affect your use case?  Given that LIBRARY_PATH
comes after Clang's lib directory, if Clang's lib directory contains
libomp.so, then LIBRARY_PATH will not affect which libomp.so Clang
finds.  That leads me to assume your Clang lib directory must not have
a libomp.so at all, so this whole discussion of where Clang places
libomp.so seems irrelevant to your use case.  Does that make sense?

Thanks.

Joel

>
> Thanks.
>
>
> Joel
>
> >
> > > 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>