Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: Allow building Cling using the Clang shared library. #433

Closed

Conversation

Apteryks
Copy link
Contributor

@Apteryks Apteryks commented Sep 8, 2021

The officially supported way to build LLVM/Clang as a shared library
is via the LLVM_BUILD_LLVM_DYLIB and LLVM_LINK_LLVM_DYLIB CMake
options (see: https://llvm.org/docs/BuildingADistribution.html). When
built this way, the whole of Clang API is exposed as a shared
library (libclang-cpp.so).

  • CMakeLists.txt: Query if we're in shared mode via llvm-config, and
    register the result as LLVM_LIB_IS_SHARED.
    [LLVM_LIB_IS_SHARED] <target_link_libraries>: Use the PUBLIC interface of the
    LLVM shared library.
  • lib/Interpreter/CMakeLists.txt [LLVM_LIB_IS_SHARED]: When defined, replace the
    individual Clang components by clang-cpp.
  • lib/MetaProcessor/CMakeLists.txt: Likewise.
  • lib/Utils/CMakeLists.txt: Likewise.
  • tools/Jupyter/CMakeLists.txt: Likewise.
  • tools/driver/CMakeLists.txt: Likewise.
  • tools/libcling/CMakeLists.txt: Likewise.

The officially supported way to build LLVM/Clang as a shared library
is via the LLVM_BUILD_LLVM_DYLIB and LLVM_LINK_LLVM_DYLIB CMake
options (see: https://llvm.org/docs/BuildingADistribution.html).  When
built this way, the whole of Clang API is exposed as a shared
library (libclang-cpp.so).

* CMakeLists.txt: Query if we're in shared mode via llvm-config, and
register the result as LLVM_LIB_IS_SHARED.
[LLVM_LIB_IS_SHARED] <target_link_libraries>: Use the PUBLIC interface of the
LLVM shared library.
* lib/Interpreter/CMakeLists.txt [LLVM_LIB_IS_SHARED]: When defined, replace the
individual Clang components by clang-cpp.
* lib/MetaProcessor/CMakeLists.txt: Likewise.
* lib/Utils/CMakeLists.txt: Likewise.
* tools/Jupyter/CMakeLists.txt: Likewise.
* tools/driver/CMakeLists.txt: Likewise.
* tools/libcling/CMakeLists.txt: Likewise.
@Apteryks
Copy link
Contributor Author

Apteryks commented Sep 8, 2021

This fixes #430.

@SylvainCorlay
Copy link
Contributor

Hey @Apteryks
Thank you for this PR. I am also encountering the same issue as I am trying to update the cling conda package to the latest version.
Your patch appears to improve the situation, but I am still getting errors about LLVM command line options being registered more than once (namely aarch64-enable-ccmp).

@SylvainCorlay
Copy link
Contributor

Ah, it seems that this is due to LLVM options being set twice:

  • once from cling linking statically with LLVM libraries
  • once from clang linking with libLLVM.so

We should probably not link statically with LLVM libraries in this case.

@SylvainCorlay
Copy link
Contributor

Logging the value LLVM_LIB_IS_SHARED shows that the --shared-mode does not actually create a new output for llvm-config here.

@dimitry-ishenko
Copy link

dimitry-ishenko commented Oct 20, 2021

@Apteryks thanks for this patch. Saved me a lot of headaches.

I've build patched versions of LLVM and Clang with the Cling patches:
https://github.com/dimitry-ishenko-cpp/llvm-project

and made a small PPA with Debian packages:
https://github.com/ppa-verse/cling

When trying to build standalone Cling I was running into similar problems to yours - first, link errors, and then "option registered more than once."

With your patch I am now able to build Cling. Hoping to make a Debian package for it as well.

if( LLVM_ENABLE_PIC )
set(ENABLE_SHARED SHARED)
endif()

Copy link

@dimitry-ishenko dimitry-ishenko Oct 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am curious, why did you disable shared libcling? AFAIK it's used by xeus-cling

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC that mode is officially unsupported; it predates the global LLVM.so dynamic library feature that this patch relies on:

One very important note: Distributions should never be built using the BUILD_SHARED_LIBS CMake option. That option exists for optimizing developer workflow only. Due to design and implementation decisions, LLVM relies on global data which can end up being duplicated across shared libraries resulting in bugs. As such this is not a safe way to distribute LLVM or LLVM-based tools.

Source: https://llvm.org/docs/BuildingADistribution.html#general-distribution-guidance

@Apteryks
Copy link
Contributor Author

Apteryks commented Nov 6, 2021

Logging the value LLVM_LIB_IS_SHARED shows that the --shared-mode does not actually create a new output for llvm-config here.

I'm using a custom LLVM 9 built with the following flags:

(list "-DLLVM_PARALLEL_LINK_JOBS=1" ;cater to smaller build machines
                 ;; Only enable compiler support for the host architecture to
                 ;; save on build time.
                 "-DLLVM_TARGETS_TO_BUILD=host;NVPTX"
                 "-DLLVM_INSTALL_UTILS=ON"
                 "-DLLVM_ENABLE_RTTI=ON"
                 "-DLLVM_ENABLE_FFI=ON"
                 "-DLLVM_BUILD_LLVM_DYLIB=ON"
                 "-DLLVM_LINK_LLVM_DYLIB=ON")

Perhaps you are missing the DYLIB stuff? For the gnarly details, see how I packaged it in GNU Guix here: https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/cpp.scm?h=core-updates-frozen-batched-changes&id=24a20a2178b4ac945b079b21e938bee2c644f1da#n1366.

@dimitry-ishenko
Copy link

@Apteryks for some reason I am getting segfaults when trying to run cling on Ubuntu 22.04:

(gdb) bt
#0  0x00007ffff18252fd in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::operator=(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&) () from /lib/x86_64-linux-gnu/libstdc++.so.6
#1  0x0000555555960726 in clang::TextDiagnosticPrinter::setPrefix (this=0x555555b83a70, Value="") at /usr/lib/llvm-9/include/clang/Frontend/TextDiagnosticPrinter.h:47
#2  0x00005555559647eb in (anonymous namespace)::SetupDiagnostics (DiagOpts=..., ExeName="") at ./lib/Interpreter/CIFactory.cpp:864
#3  0x000055555596d22b in (anonymous namespace)::createCIImpl (Buffer=std::unique_ptr<llvm::MemoryBuffer> = {...}, COpts=..., LLVMDir=0x0, 
    customConsumer=std::unique_ptr<clang::ASTConsumer> = {...}, moduleExtensions=std::vector of length 0, capacity 0, OnlyLex=false, HasInput=false)
    at ./lib/Interpreter/CIFactory.cpp:1329
#4  0x000055555596ef21 in cling::CIFactory::createCI (Code=..., Opts=..., LLVMDir=0x0, consumer=std::unique_ptr<clang::ASTConsumer> = {...}, 
    moduleExtensions=std::vector of length 0, capacity 0) at ./lib/Interpreter/CIFactory.cpp:1686
#5  0x000055555588dce7 in cling::IncrementalParser::IncrementalParser (this=0x555555b6c320, interp=0x7fffffffdfd0, llvmdir=0x0, moduleExtensions=std::vector of length 0, capacity 0)
    at ./lib/Interpreter/IncrementalParser.cpp:243
#6  0x000055555565d916 in cling::Interpreter::Interpreter (this=0x7fffffffdfd0, argc=1, argv=0x7fffffffe4e8, llvmdir=0x0, moduleExtensions=std::vector of length 0, capacity 0, 
    noRuntime=false, parentInterp=0x0) at ./lib/Interpreter/Interpreter.cpp:216
#7  0x0000555555610c93 in cling::Interpreter::Interpreter (this=0x7fffffffdfd0, argc=1, argv=0x7fffffffe4e8, llvmdir=0x0, moduleExtensions=std::vector of length 0, capacity 0, 
    noRuntime=false) at ./include/cling/Interpreter/Interpreter.h:342
#8  0x00005555556110f6 in main (argc=1, argv=0x7fffffffe4e8) at ./tools/driver/cling.cpp:79

I've compiled LLVM-9 and Cling with your patches for Ubuntu 22.04 and this is what I get. Same exact recipe works on Ubuntu 20.04 (and possibly Ubuntu 21.10 - need to confirm that).

Any ideas what could cause this?

@Apteryks
Copy link
Contributor Author

Apteryks commented Aug 1, 2022

@dimitry-ishenko Hi! I don't have the time to investigate, but if you just want to use it rather than figure out the root cause, you could install cling via GNU Guix; guix install cling. These patches were made while packaging it for Guix.

@dimitry-ishenko
Copy link

dimitry-ishenko commented Aug 1, 2022

@Apteryks thank you. No rush obviously, but if/when you get a chance, I could use some pointers.

I have been able to build Cling statically (the official ROOT way) for Ubuntu 22.04 by disabling LTO. So, I do have a working version. But, I am also trying to do it "the right way" :) Plus, if I figure this out, I will be able to compile xeus-cling and run C++ in Jupyter.

I will keep trying myself in the meantime.

@dimitry-ishenko
Copy link

@Apteryks FYI I've figured this out. Had to roll my own .deb package for LLVM/Clang 9. I suspect the problem had to do with Debian using stage2 to build their libs and it was causing some kind of ABI differences.

Thanks again for your patch. 👍🏻

@Apteryks
Copy link
Contributor Author

Hi! Is there something preventing this branch to be merged?

@jhadida
Copy link

jhadida commented Dec 23, 2023

It seems that this might help with issue #491 as well.
What are the conflicts that must be resolved to merge? It's been over a year 🙁
@vgvassilev Would you be able to help unblock?

@ferdymercury
Copy link
Contributor

I believe this repo is just a mirror, consider moving your PR here instead: https://github.com/root-project/root/pulls (subdirectory interpreter/cling). Thanks!

@Apteryks
Copy link
Contributor Author

I believe this repo is just a mirror, consider moving your PR here instead: https://github.com/root-project/root/pulls (subdirectory interpreter/cling). Thanks!

This repo has 182 PR merged already, and I see no obvious mention that it is a mirror, so I'll leave it here.

@ferdymercury
Copy link
Contributor

I believe this repo is just a mirror, consider moving your PR here instead: https://github.com/root-project/root/pulls (subdirectory interpreter/cling). Thanks!

This repo has 182 PR merged already, and I see no obvious mention that it is a mirror, so I'll leave it here.

If you take a closer look, 182 PR were closed, not merged.

See for example #460 (comment)

@Apteryks
Copy link
Contributor Author

Ah! That is confusing. I'll try migrating the PR then, thanks.

@dimitry-ishenko
Copy link

@ferdymercury is it no longer possible to build cling without ROOT? I thought this was a standalone version of cling...

@ferdymercury
Copy link
Contributor

ferdymercury commented Apr 12, 2024

Yes, you can. But PR need to happen in the big ROOT repo and are automatically synced to this standalone repo. Sorry for the hassle.

@dimitry-ishenko
Copy link

Yes, you can. But PR need to happen in the big ROOT repo and are automatically synced to this standalone repo. Sorry for the hassle.

Oh I see. Thanks for clarifying.

@Apteryks
Copy link
Contributor Author

Yes, you can. But PR need to happen in the big ROOT repo and are automatically synced to this standalone repo. Sorry for the hassle.

I believe a Github repo can be configured to disallow pull requests; perhaps this should be done here, with some guidance in the README, to avoid future confusion?

@ferdymercury
Copy link
Contributor

Yep, I upvote.
@pcanal do you think the README could incorporate this info?

@Apteryks
Copy link
Contributor Author

@ferdymercury I've now migrated this PR against the ROOT monorepo, see root-project/root#15563.

@Apteryks Apteryks closed this May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants