Page MenuHomePhabricator

[clang-shlib] Fix clang-shlib for PRIVATE dependencies
ClosedPublic

Authored by smeenai on Jul 11 2019, 12:02 PM.

Details

Summary

Any static library with a PRIVATE dependency ends up with a
$<LINK_ONLY:...> generator expression in its INTERFACE_LINK_LIBRARIES,
which won't be evaluated by the $<TARGET_PROPERTY:...>, so we end up
with an unevaluated generator expression in the generated build file and
Ninja chokes on the dollar sign. Just use the static library directly
for its dependencies instead of trying to propagate dependencies
manually.

Diff Detail

Repository
rL LLVM

Event Timeline

smeenai created this revision.Jul 11 2019, 12:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2019, 12:02 PM

@beanz, what do you think of this? I think it's kinda awful, but I can't think of a better alternative short of upgrading to CMake 3.12 (as detailed in my comment).

phosek accepted this revision.Jul 11 2019, 12:09 PM

While upgrading to newer CMake would be nice, I think it's unlikely that we could move all the way to 3.12 since that version was only released a year ago and still isn't available in most distributions, so this LGTM. Maybe leave a TODO that this should be revisited after we bump minimum CMake requirement to 3.12 or newer?

This revision is now accepted and ready to land.Jul 11 2019, 12:09 PM

While upgrading to newer CMake would be nice, I think it's unlikely that we could move all the way to 3.12 since that version was only released a year ago and still isn't available in most distributions, so this LGTM. Maybe leave a TODO that this should be revisited after we bump minimum CMake requirement to 3.12 or newer?

FWIW, CMake 3.4.3 was released Jan 25 2016 (https://cmake.org/pipermail/cmake/2016-January/062616.html) and we started requiring it on May 31 2016 (https://reviews.llvm.org/D20822), so we do have precedence for requiring a pretty recent CMake version. I guess we should figure out what other compelling reasons we'd have for moving to that version though.

While upgrading to newer CMake would be nice, I think it's unlikely that we could move all the way to 3.12 since that version was only released a year ago and still isn't available in most distributions, so this LGTM. Maybe leave a TODO that this should be revisited after we bump minimum CMake requirement to 3.12 or newer?

FWIW, CMake 3.4.3 was released Jan 25 2016 (https://cmake.org/pipermail/cmake/2016-January/062616.html) and we started requiring it on May 31 2016 (https://reviews.llvm.org/D20822), so we do have precedence for requiring a pretty recent CMake version. I guess we should figure out what other compelling reasons we'd have for moving to that version though.

I'll wait a bit in case @beanz has any thoughts and then commit this with the added TODO.

beanz accepted this revision.Jul 11 2019, 2:15 PM

Yea, this makes sense even if it is a bit sad. We do use the --whole-archive approach for libLLVM, and it works. I was hoping to avoid it here in part to free up the ability to link libclang_shared before or in parallel to archiving the libclang*.a archives.

Yea, this makes sense even if it is a bit sad. We do use the --whole-archive approach for libLLVM, and it works. I was hoping to avoid it here in part to free up the ability to link libclang_shared before or in parallel to archiving the libclang*.a archives.

Yeah, makes sense. I've had issues with static libraries getting reordered out of the --whole-archive with other projects in the past, which makes me wary of it.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2019, 2:23 PM

Hello @smeenai , this commit causes LLVM to fail to build when set set -D BUILD_SHARED_LIBS=ON. Here is the failing CMake and Ninja invocation:

/tools/build/cmake-3.14.5/bin/cmake \
    -G Ninja  \
    -D CMAKE_MAKE_PROGRAM=/tools/build/ninja-1.9.0/ninja \
    -D CMAKE_C_COMPILER=/usr/lib/llvm-7/bin/clang \
    -D CMAKE_CXX_COMPILER=/usr/lib/llvm-7/bin/clang++ \
    -D CMAKE_BUILD_TYPE=Release \
    -D CMAKE_INSTALL_PREFIX=/work/upstream/install \
    -D LLVM_TOOL_CLANG_BUILD=ON \
    -D LIBCXX_INCLUDE_BENCHMARKS=ON \
    -D BUILD_SHARED_LIBS=ON \
    -D LLVM_ENABLE_ASSERTIONS=ON \
    -D LLVM_TARGETS_TO_BUILD=X86 \
    /work/upstream/llvm-project/llvm

/tools/build/ninja-1.9.0/ninja -v install

I am able to successfully build LLVM when I change to -D BUILD_SHARED_LIBS=OFF.

The outputted errors happen during link time of lib/libclang_shared.so.9svn:

FAILED: lib/libclang_shared.so.9svn

...

tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/CodeGenModule.cpp.o: In function `llvm::cl::opt<bool, false, llvm::cl::parser<bool> >::~opt()':
CodeGenModule.cpp:(.text._ZN4llvm2cl3optIbLb0ENS0_6parserIbEEED2Ev[_ZN4llvm2cl3optIbLb0ENS0_6parserIbEEED2Ev]+0x7): undefined reference to `vtable for llvm::cl::Option'
tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/CodeGenModule.cpp.o: In function `clang::CodeGen::CodeGenModule::CodeGenModule(clang::ASTContext&, clang::HeaderSearchOptions const&, clang::PreprocessorOptions const&, clang::CodeGenOptions const&, llvm::Module&, clang::DiagnosticsEngine&, clang::CoverageSourceInfo*)':
CodeGenModule.cpp:(.text._ZN5clang7CodeGen13CodeGenModuleC2ERNS_10ASTContextERKNS_19HeaderSearchOptionsERKNS_19PreprocessorOptionsERKNS_14CodeGenOptionsERN4llvm6ModuleERNS_17DiagnosticsEngineEPNS_18CoverageSourceInfoE+0x57a): undefined reference to `llvm::FoldingSetBase::FoldingSetBase(unsigned int)'
CodeGenModule.cpp:(.text._ZN5clang7CodeGen13CodeGenModuleC2ERNS_10ASTContextERKNS_19HeaderSearchOptionsERKNS_19PreprocessorOptionsERKNS_14CodeGenOptionsERN4llvm6ModuleERNS_17DiagnosticsEngineEPNS_18CoverageSourceInfoE+0x5cc): undefined reference to `llvm::Type::getVoidTy(llvm::LLVMContext&)'
CodeGenModule.cpp:(.text._ZN5clang7CodeGen13CodeGenModuleC2ERNS_10ASTContextERKNS_19HeaderSearchOptionsERKNS_19PreprocessorOptionsERKNS_14CodeGenOptionsERN4llvm6ModuleERNS_17DiagnosticsEngineEPNS_18CoverageSourceInfoE+0x5d7): undefined reference to `llvm::Type::getInt8Ty(llvm::LLVMContext&)'
CodeGenModule.cpp:(.text._ZN5clang7CodeGen13CodeGenModuleC2ERNS_10ASTContextERKNS_19HeaderSearchOptionsERKNS_19PreprocessorOptionsERKNS_14CodeGenOptionsERN4llvm6ModuleERNS_17DiagnosticsEngineEPNS_18CoverageSourceInfoE+0x5e3): undefined reference to `llvm::Type::getInt16Ty(llvm::LLVMContext&)'

... (clipped thousands of similar errors)

WhitespaceManager.cpp:(.text._ZN5clang6formatL18AlignTokenSequenceIRZNS0_17WhitespaceManager28alignConsecutiveDeclarationsEvE3$_2EEvjjjOT_RN4llvm11SmallVectorINS2_6ChangeELj16EEE+0x129): undefined reference to `llvm::SmallVectorBase::grow_pod(void*, unsigned long, unsigned long)'
tools/clang/tools/clang-fuzzer/handle-cxx/CMakeFiles/obj.clangHandleCXX.dir/handle_cxx.cpp.o: In function `clang_fuzzer::HandleCXX(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<char const*, std::allocator<char const*> > const&)':
handle_cxx.cpp:(.text._ZN12clang_fuzzer9HandleCXXERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKSt6vectorIPKcSaISA_EE+0x67): undefined reference to `llvm::SmallVectorBase::grow_pod(void*, unsigned long, unsigned long)'
handle_cxx.cpp:(.text._ZN12clang_fuzzer9HandleCXXERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKSt6vectorIPKcSaISA_EE+0x32b): undefined reference to `llvm::MemoryBuffer::getMemBuffer(llvm::StringRef, llvm::StringRef, bool)'
handle_cxx.cpp:(.text._ZN12clang_fuzzer9HandleCXXERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKSt6vectorIPKcSaISA_EE+0x669): undefined reference to `llvm::SmallVectorBase::grow_pod(void*, unsigned long, unsigned long)'
clang: error: linker command failed with exit code 1 (use -v to see invocation)