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

Tue Feb 26 15:12:54 PST 2019

Hi Jonas,

On Tue, Feb 26, 2019 at 8:59 AM Jonas Hahnfeld <hahnjo at hahnjo.de> wrote:
>
> Hi Joel,
>
> On 2019-02-21 03:27, Joel E. Denny wrote:
> > 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,

I forgot that your Clang's lib directory might indeed have a libomp.so
because linking often (or always so far for me) manages to succeed
when the wrong libomp.so is found.

> > so this whole discussion of where Clang places
> > libomp.so seems irrelevant to your use case.  Does that make sense?
>
> I guess you are absolutely right and I'm indeed linking against
> libomp.so from Clang's lib directory (according to -Wl,-verbose). What's
> saving me is that during runtime I get the "right" library because I'm
> setting LD_LIBRARY_PATH, so I'm seeing new behavior that I implement in
> my custom installation.
> So yes, please ignore this use case because it's not affected as you
> say.

Thanks for confirming.

It sounds like we so far have no use case that answers no to "Q3: Is
it really OK to elevate Clang's libraries?".

Joel

>
> Regards,
> Jonas