llvm: libcxx in LLVM formula sometimes causes problems

[zhipeng-jia]

When LLVM formula is installed with libcxx, libc++.dylib and libc++abi.dylib will be generated inside /usr/local/opt/llvm/lib. If a program wants to link against LLVM, the compile command line may look like:

clang++ -o main -std=c++11 -L/usr/local/opt/llvm/lib -I ... main.cpp

In this case, clang will use libc++ in /usr/local/opt/llvm/lib instead of OS X's libc++, and give compile errors:

Undefined symbols for architecture x86_64:
  "std::terminate()", referenced from:
      ___clang_call_terminate in main-1455b4.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

This is because libc++.dylib in /usr/local/opt/llvm/lib does not correctly export symbols of libc++abi.
Adding -lc++abi can make the program succeed to compile. But when executing it, following error is given:

dyld: Library not loaded: @rpath/libc++abi.1.dylib
  Referenced from: /usr/local/opt/llvm/lib/libc++.1.dylib
  Reason: image not found
fish: './main' terminated by signal SIGTRAP (Trace or breakpoint trap)

This error is caused by rpath is not set when compiling the program, i.e. rpath should be set to /usr/local/opt/llvm/lib.
Therefore, libc++.dylib and libc++abi.dylib in /usr/local/opt/llvm/lib can be really annoyed if the program wants to link LLVM. In most case, we do not intend to link LLVM's libc++ instead of system's. Also, even if we do not care which libc++ to link, linking LLVM's libc++ needs more compiler flags.

My suggestion is to remove libc++ from LLVM formula. Maybe we can create an individual formula for it.

[UniqMartin]

Thanks for the detailed report! It will certainly help to try to reproduce the issues you are describing.

However, right now I don't fully agree with your assessment and what should be done. Since libc++ and libc++abi aren't installed by default, there's no immediate harm for most Homebrew users. I do agree though, that offering those options means they should result in something useful/usable. I'll look into the issues you described and what can be done about them (it will take me some time, though).

@aw1621107 I'm going to look into this myself, given I accepted your recent changes to the llvm formula. Since you've contributed the support for the mentioned libraries in the current formula, I'd appreciate if you could have a look at the issue described here. (I had the impression you had tested and used llvm with all those options enabled. And I mean beyond making sure the build succeeds.)

[MikeMcQuaid]

However, right now I don't fully agree with your assessment and what should be done. Since libc++ and libc++abi aren't installed by default, there's no immediate harm for most Homebrew users. I do agree though, that offering those options means they should result in something useful/usable. I'll look into the issues you described and what can be done about them (it will take me some time, though).

It may be worth removing the options if there's outstanding issues we can't address.

[UniqMartin]

It may be worth removing the options if there's outstanding issues we can't address.

Yes, I agree. I'd like to take a shot at addressing them in the next few days. Or would you prefer them to be removed now and then re-added later once the outstanding issues are resolved?

[ts826848]

@UniqMartin I've only used LLVM for LLDB and its sanitizers so far, because Apple's clang isn't built with sanitizers. I don't have anything at the moment that links against libcxx/abi, unfortunately. I'll take another look...

[ts826848]

Looks like something similar-ish happened before in #32566 and #34169?

[ts826848]

Just making sure, did you include the LLVM lib/include directories in LDFLAGS and CPPFLAGS?

[zhipeng-jia]

@aw1621107 Yes. The problem does not come from libclang. It is caused by libcxx, which many Homebrew users do not install with LLVM.

[manphiz]

I think the problem is that in 7a72f5f libcxxabi was added which was not done right. As I mentioned in #44250 (comment), libcxx should always link system libc++abi whenever it is available. The only case it should provide its own libc++abi is for older system like 10.6 which doesn't have it. Check homebrew-versions/llvm* which should handle this correctly.

[ts826848]

@zhipeng-jia Yeah; just saw the same @rpath thing, and thought that the problem looked similar. First attempts at setting `CMAKE_INSTALL_RPATH didn't pan out, but there are still a few things I'd like to try.

There is a way of adding to the rpath manually using install_name_tool -add_rpath <path> but that would be a rather hacky way of solving the issue. Perhaps give that a try?

[ts826848]

@manphiz Odd thing is that libc++abi is supposed to be ABI compatible with the existing low-level stuff on OS X (or so the libc++abi page says). The libc++ docs also say that libc++ should work on OS X under i386/x64 with libc++abi, so evidently I'm missing something big...

The libc++ instructions also seem to suggest checking out libc++abi, and it doesn't look like anything special is done?

[manphiz]

@aw1621107 I think the problem here is not that they are not ABI compatible, but rather it's linking a different libc++abi. Programs should link to the same libc++abi from system if provided. I still suggest only enable custom libc++abi in older system.

EDIT: typo.

[ts826848]

@manphiz Shouldn't ABI compatibility mean that linking either library should work? And it looks like it isn't linking at all because it can't find a file to link against...

@zhipeng-jia It looks like LLVM actually hardcodes the rpath. From the top-level CMakeLists.txt, line ~560:

set(CMAKE_BUILD_WITH_INSTALL_RPATH ON)
if (APPLE)
  set(CMAKE_INSTALL_NAME_DIR "@rpath")
  set(CMAKE_INSTALL_RPATH "@executable_path/../lib")

So that would explain why passing -DCMAKE_INSTALL_RPATH or related didn't do anything.

Changing this to:

set(CMAKE_BUILD_WITH_INSTALL_RPATH ON)
if (APPLE)
  set(CMAKE_INSTALL_NAME_DIR "${CMAKE_INSTALL_PREFIX}/lib")
  set(CMAKE_INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/lib")

Seems to work? otool -l libc++.1.dylib gives:

Load command 10
          cmd LC_LOAD_DYLIB
      cmdsize 56
         name /usr/lib/libc++abi.dylib (offset 24)
   time stamp 2 Wed Dec 31 19:00:02 1969
      current version 125.0.0
compatibility version 1.0.0
Load command 11
          cmd LC_LOAD_DYLIB
      cmdsize 80
         name /usr/local/Cellar/llvm/HEAD/lib/libc++abi.1.dylib (offset 24)
   time stamp 2 Wed Dec 31 19:00:02 1969
      current version 1.0.0
compatibility version 1.0.0
Load command 12
          cmd LC_LOAD_DYLIB
      cmdsize 56
         name /usr/lib/libSystem.B.dylib (offset 24)
   time stamp 2 Wed Dec 31 19:00:02 1969
      current version 1226.10.1
compatibility version 1.0.0
Load command 13
          cmd LC_RPATH
      cmdsize 48
         path /usr/local/Cellar/llvm/HEAD/lib (offset 12)

And something similar for libc++abi.1.dylib

Does that look right to you? You know the details better than I do...

I did this in a fairly hacky way. Added raise "before cmake" to the top of the install do block in the LLVM formula, then ran brew install llvm --with-libcxx --HEAD --debug. When the program hit the raise I dropped to a shell, made the changes, then exited the shell and let the build continue.

[manphiz]

@aw1621107 What I'm saying is, if libc++ is linking its own libc++abi and the latter is not in DYLD_LIBRARY_PATH it will be problematic as of now. I suggest we just ditch libc++abi. I'll make a PR for this.

[manphiz]

@aw1621107 Also another problem with shipping libc++abi is, if the program you are building with this llvm links to another library that was built against system libc++abi, it will also cause trouble in runtime, as the end program will reference two copies of libc++abi. I still suggest we just make sure only one copy of libc++abi is supported, which should be the system one.

[zhipeng-jia]

@aw1621107 I also agree with @manphiz that having two libc++abi linking to one program is harmful.
Another problem is if libc++ is in /usr/local/opt/llvm/lib and /usr/local/opt/llvm/lib is in library path when compiling, clang will link LLVM's libc++ instead of system's. Ans it is also hard to prevent clang doing so. In most cases, we just want to link LLVM and libclang, and linking LLVM's libc++ is not an expected behavior.

[ts826848]

@manphiz It is possible to change the rpath to the Homebrew location so the trunk libc++ is hardcoded to use the trunk libc++abi, but I'm under the impression that that is a last-resort kind of thing because it involves the formula patching LLVM's sources. That would at least make setting DYLD_LIBRARY_PATH unnecessary.

I'm probably going to hop on the LLVM mailing list sometime soon-ish and ask why those CMake variables are hardcoded...

I don't think multiple copies of libc++abi would be an issue. LLVM libc++abi is (supposedly) ABI compatible with the OS X one, so the linker shouldn't complain if one is swapped for the other. And it shouldn't break a program in unusual ways -- if the trunk libc++abi introduces bugs or fixes bugs, you'll get something different, but that also applies if/when you update XCode. The libraries are being dynamically linked, so this kind of change shouldn't be unexpected...

Also, Darwin uses a dylib's full path for linking, so the two libc++abis should resolve to different files at link time. I guess duplicate symbols might be an issue, but I think that'd only be if a program somehow linked both libraries; if it's linking only one or another, the symbol to use shouldn't be ambiguous.

But in any case, how does splitting libc++abi into a separate option sound? It's there if anyone wants to/needs to use it, and if you just want libc++ you don't get broken things.

@zhipeng-jia I'm assuming specifying full paths to the libraries to be linked instead of their directory is not possible? And you can always symlink to the trunk versions in a different directory and add that to the search path, but that's an incredibly hacky way of doing that. Otherwise, it may be possible to change where libc++ is placed after building, but that'd likely have to be through a patch of some kind.

[zhipeng-jia]

@aw1621107 I agree we should move libc++.dylib to somewhere other than /usr/local/opt/llvm/lib. Another possible solution might be create a different formula for libc++.
The other problem is whether to retain libc++abi with libc++. If we are able to fix libc++.dylib's dependency problem, and linking two versions of libc++abi really has no bad results, maybe it's OK to retain it.

[manphiz]

@aw1621107 @zhipeng-jia I'm pretty sure if a program references two copies of the same library and if they have calls that cross DLL boundaries (calling functions that reference the other copy of the library and the return type is from this referenced library) it will have obscure failure in runtime, which is hardly avoidable considering this is the standard library.

Though dropping libc++abi altogether will be the safe option, I do see the point of retaining it as an option for people who are willing to experiment, and with a big caveat of the consequence.

[manphiz]

Just to add I dropped libc++abi in the PR as per Mike's suggestion.

[zhipeng-jia]

@manphiz Your PR makes the built libc++.dylib only has dependency on system's libc++abi:

> otool -L libc++.1.dylib
libc++.1.dylib:
    /usr/local/opt/llvm/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1.0.0)
    /usr/lib/libc++abi.dylib (compatibility version 1.0.0, current version 125.0.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)

Thanks for your contribution!

But I still wonder how we can solve the problem that clang links against LLVM's libc++ instead of system's as long as /usr/local/opt/llvm/lib is in library path.

[ts826848]

@zhipeng-jia Based on how the libraries are linked, it looks like it's possible that things in Homebrew LLVM depend on libraries being in their original positions, so I'm not totally sure if moving wouldn't break anything. Then again, if a LIBCXX_INSTALL_PATH is available, then moving shouldn't be an issue. I'll have to check sometime later.

As for the separate formula, I'm not that much into it. Tip-of-trunk LLVM stuff should generally be built with other tip-of-trunk stuff, so a separate formula seems more awkward and error-prone to me. And I'm probably going to split --with-libcxx into --with-libcxx and --with-libcxxabi anyways, so you can pick which one(s) you want.

Have you tried the CMakeLists.txt change in an earlier comment? It at least looks like it takes care of the original @rpath issue.

As for linking against LLVM's libc++, I don't think there is anything you can do if you pass /usr/local/opt/llvm/lib as a library path, but making a separate folder with symlinks to just libclang and libLLVM and prepending that to the search path could work.

[ts826848]

@manphiz Return types can't be an issue unless one or both versions are buggy. ABI compatibility implies that attempting to link a different version isn't going to cause link errors. The libraries are (or should be) API compatible too, so the code should/would not see a difference in whatever it gets back. Really the only thing being affected is resource utilization. Requiring a certain version of a shared library sort of defeats the purpose of shared libraries, too...

libc++abi is intended to be a reimplementation, not a rewrite/redesign. It shouldn't have any externally visible effects; if it does, it's probably a bug.

I think it should stay, albeit split from libc++. At least from what I can tell, the problems were CMake problems, not libc++abi problems.

[zhipeng-jia]

@aw1621107 Yes. Changing CMakeLists.txt as you suggested can solve rpath problem.

[ts826848]

@zhipeng-jia Alright, I'll try to get a patch submitted in a few hours

[manphiz]

@aw1621107 I'm still not quite convinced though I do think a test case may prove it right or wrong, though I don't really have time for that right now. Maybe after the holiday.

[manphiz]

@aw1621107 Also I'd suggest avoid making ad-hoc patches against upstream code just yet, which I believe is not recommended by Homebrew. Maybe open an upstream ticket first and see how LLVM devs respond.

[zhipeng-jia]

@manphiz @aw1621107 I suggest we should listen what other guys' thoughts. @UniqMartin @MikeMcQuaid

[ts826848]

@manphiz Yeah, I think I said that further up... But I was more intending it as a temporary patch while we get things sorted out with upstream. I don't know how long that would take; if it's short enough, then I'll wait.