Cling visibility and semantic interposition#8204
Cling visibility and semantic interposition#8204Axel-Naumann merged 7 commits intoroot-project:masterfrom
Conversation
|
Starting build on |
|
Yes! Could you also now try running clingtest as part of the regular ROOT testing procedure? I believe (with minor adjustments) it will work. |
interpreter/CMakeLists.txt
Outdated
| if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") | ||
| ROOT_ADD_CXX_FLAG(CMAKE_CXX_FLAGS "-fno-semantic-interposition") | ||
| endif() |
There was a problem hiding this comment.
We likely also want this for Clang, right? (or is CMAKE_CXX_COMPILER_ID set to GNU in that case as well?)
There was a problem hiding this comment.
I found this restriction in https://reviews.llvm.org/D102453:
Note: we cannot add -fno-semantic-interposition for Clang. Clang's implementation additionally optimizes global variables, which is incompatible with unfortunate ELF -fno-pic default: direct access relocations for external data. If the executable has a -fno-pic object file referencing a global variable declared in a public header, the direct access relocation will cause a copy relocation. The executable and libLLVM.so/libclang-cpp.so will disagree on the address.
OTOH why not try :-)
There was a problem hiding this comment.
Ugh, unfortunate. On the other hand, we don't want users to call into LLVM directly so we should be fine?
Axel-Naumann
force-pushed
the
cling-visibility-semantic-interpos
branch
from
e469190 to
56b0727
Compare
|
Starting build on |
Sure, but this PR makes this neither easier nor more difficult, nor should it have any other effect :-) We probably want a new build flavor, to make sure that things work both with and without cling-test. I.e. enable cling-test only on some builds. Which ones - one Fedora, one Mac? And I agree we should have this PR tested with cling-test. |
Axel-Naumann
force-pushed
the
cling-visibility-semantic-interpos
branch
from
56b0727 to
cdf7d78
Compare
|
Starting build on |
|
Build failed on windows10/cxx14. |
Axel-Naumann
force-pushed
the
cling-visibility-semantic-interpos
branch
from
cdf7d78 to
c2ddba2
Compare
|
Starting build on |
Axel-Naumann
force-pushed
the
cling-visibility-semantic-interpos
branch
from
c2ddba2 to
2b5f647
Compare
|
Starting build on |
Axel-Naumann
force-pushed
the
cling-visibility-semantic-interpos
branch
from
2b5f647 to
2bf3685
Compare
|
Starting build on |
Instead of relying on CLING_CXXFLAGS to collect as needed, set the visbility explicitly, and use target_properties instead of CMAKE_CXXFLAGS.
To prevent non-ROOT LLVM-libraries from replacing the symbols cling needs (with possible version mismatches), tell GCC to just always resolve calls to dynamic symbols to symbols from libCling itself.
Rather than string-replacing visibility, use the CMake interface. Do not set visibility in the ROOT case; ROOT can handle that just fine itself. ROOT actually does *not* need visibility=default, now that the few symbols that are required for using cling (apart from clingtest) are explicitly requested through attributes.
|
Starting build on |
|
@phsft-bot build |
|
Starting build on |
|
@phsft-bot build with flags -Dclingtest=On |
|
Starting build on |
|
Build failed on ROOT-fedora31/noimt. Failing tests: |
|
Build failed on ROOT-performance-centos8-multicore/default. Failing tests: |
|
Build failed on windows10/cxx14. Errors:
|
|
Build failed on ROOT-fedora30/cxx14. Failing tests: |
|
Build failed on ROOT-ubuntu16/nortcxxmod. Failing tests: |
|
Build failed on mac11.0/cxx17. Failing tests: |
|
Build failed on mac1014/python3. Failing tests: |
|
Build failed on ROOT-debian10-i386/cxx14. Failing tests: |
|
Macs were failing exactly like this before (in numba, with |
oshadura
left a comment
oshadura
left a comment
There was a problem hiding this comment.
CMake changes makes sense to me, thanks for improvements!
5 participants
Wit kudos to @hahnjo !