Page MenuHomePhabricator

[lld][cmake] Fix LLVM_LINK_LLVM_DYLIB build
ClosedPublic

Authored by labath on Jan 6 2017, 5:01 AM.

Details

Summary

Lld's build had a couple of issues which prevented a successfull
LLVM_LINK_LLVM_DYLIB compilation.

  • add_llvm_library vs llvm_add_library: One adds a library to libLLVM.so, other one doesn't. Lld was using the wrong one, causing symbols to be mupltiply defined in things linking to libLLVM.
  • confusion when to use LINK_LIBS vs LINK_COMPONENTS in llvm_add_library
  • not using LLVM_LINK_COMPONENTS for add_lld_tool

With these fixes lld compiles and it's test suite passes both in
LLVM_LINK_LLVM_DYLIB mode and without it.

Diff Detail

Repository
rL LLVM

Event Timeline

labath updated this revision to Diff 83359.Jan 6 2017, 5:01 AM
labath retitled this revision from to [lld][cmake] Fix LLVM_LINK_LLVM_DYLIB build.
labath updated this object.
labath added reviewers: ruiu, beanz.
labath added a subscriber: llvm-commits.

I confirmed that this fixed the issue I was experiencing with Debian:
https://llvm.org/bugs/show_bug.cgi?id=27685

ruiu accepted this revision.Jan 6 2017, 10:23 PM
ruiu edited edge metadata.

LGTM. Thank you for doing this! This is what we do in other LLVM subprojects, so I think this is correct.

This revision is now accepted and ready to land.Jan 6 2017, 10:23 PM
This revision was automatically updated to reflect the committed changes.
labath added a comment.Jan 9 2017, 2:10 AM

The lldb problem is unrelated to this. I am going to try to fix that one separately.

TL;DR This broke lld.

I was trying to build an RPM for LLVM/Clang 4.0.0 RC1 toolchain and noticed issues, which at first looks a caused by this change.

With this change you are no more installing required shared libraries:
error: Failed dependencies:

liblldCOFF.so.4()(64bit) is needed by external+llvm+4.0.0-cms-1-1.x86_64
liblldDriver.so.4()(64bit) is needed by external+llvm+4.0.0-cms-1-1.x86_64
liblldELF.so.4()(64bit) is needed by external+llvm+4.0.0-cms-1-1.x86_64

I checked CMake install logs and they were not listed as installed.

Looking at packages (thus installed) lld binary it depends on them.

EricWF added a subscriber: EricWF.Jan 21 2017, 3:52 AM

TL;DR This broke lld.

I was trying to build an RPM for LLVM/Clang 4.0.0 RC1 toolchain and noticed issues, which at first looks a caused by this change.

With this change you are no more installing required shared libraries:
error: Failed dependencies:

liblldCOFF.so.4()(64bit) is needed by external+llvm+4.0.0-cms-1-1.x86_64
liblldDriver.so.4()(64bit) is needed by external+llvm+4.0.0-cms-1-1.x86_64
liblldELF.so.4()(64bit) is needed by external+llvm+4.0.0-cms-1-1.x86_64

I checked CMake install logs and they were not listed as installed.

Looking at packages (thus installed) lld binary it depends on them.

+1

I'm running into the same problems trying to use lld.

+1, I've been using trunk with this revision reverted since I need liblldELF as well. Would it fix all use cases if we added bits to install the libraries to add_lld_library?

Sorry about the breakage.

+1, I've been using trunk with this revision reverted since I need liblldELF as well. Would it fix all use cases if we added bits to install the libraries to add_lld_library?

Yes, I believe the install bits will fix your issue. I'll create a patch for that today.. I'd appreciate it if one of you could check that this fixes your problems