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

Cmake links all abseil components and results in a link time error #225

Closed
bstaletic opened this issue Nov 24, 2018 · 30 comments
Closed

Cmake links all abseil components and results in a link time error #225

bstaletic opened this issue Nov 24, 2018 · 30 comments
Assignees
Labels
cmake related to CMake support

Comments

@bstaletic
Copy link
Contributor

No matter what I try, cmake builds the whole abseil and then links everything into my shared library. I could reproduce this with a minimal cmake example:

cmake_minimum_required(VERSION 2.8.12)
project(my_project)
set(CMAKE_CXX_FLAGS "-std=c++11 -fvisibility=hidden ${CMAKE_CXX_FLAGS}")
add_subdirectory(abseil-cpp)
add_library(my_lib SHARED main.cpp)
include_directories(SYSTEM abseil-cpp)
target_link_libraries(my_lib absl::hash absl::container)

main.cpp contains only the following:

#include <absl/container/flat_hash_map.h>
int foo(void) {
	absl::flat_hash_map<int,int> m;
	return 0;
}

For an actual project where I'm trying to use abseil, I get a linker error in debug mode that says the following:

/usr/sbin/ld: ../absail/absl/container/libabsl_container.a(raw_hash_set.cc.o): relocation R_X86_64_TPOFF32 against hidden symbol `_ZZN4absl18container_internal10RandomSeedEvE7counter' can not be used when making a shared object

The project in question is here.
Top level CmakeLists.txt
Shared object defining CMakeLists.txt
Abseil, at commit a06c4a1, is a submodule of my project. The build instructions are:

  • Clone bstaletic/ycmd and checkout aseil branch
  • git submodule update --init --recursive
  • mkdir build && cd build
  • cmake ../cpp -DCMAKE_BUILD_TYPE=Debug

Other build time dependencies are python headers. For python3 -DUSE_PYTHON2=OFF is needed.

@JonathanDCohen JonathanDCohen self-assigned this Nov 26, 2018
@bstaletic
Copy link
Contributor Author

Also, my Appveyor and Travis build fail with nonsense errors.

Appveyor: https://ci.appveyor.com/project/bstaletic/ycmd-noh9b/builds/20561052/job/y64dyb0cytehw06t
Travis: https://travis-ci.org/bstaletic/ycmd/builds/459085385

They are both mentioning (conflicting) redeclarations/redefinitions. This doesn't happen when I build on my own computer.

@JonathanDCohen
Copy link
Contributor

I'll look at the CI errors. Could you try your example but with absl::core_headers instead of container and hash? those two probably pull in a huge chunk of abseil between the two of them, whereas core_headers is the lowest-level target we have.

@bstaletic
Copy link
Contributor Author

So, actual lists of pulled in dependencies:

  • absl::hash and absl::container
    • libabsl_dynamic_annotations.a
    • libabsl_internal_spinlock_wait.a
    • libabsl_int128.a
    • libabsl_numeric.a
    • libabsl_container.a
    • libabsl_base.a
    • libabsl_utility.a
    • libabsl_span.a
    • libabsl_internal_throw_delegate.a
    • libabsl_strings.a
    • libstr_format_extension_internal.a
    • libstr_format_internal.a
    • libabsl_str_format.a
    • libabsl_hash.a
  • just absl::core_headers
    • Building target my_lib explicitly didn't build any abseil component.

@JonathanDCohen
Copy link
Contributor

Getting back around to this. We've been doing a lot of overhaul of our CMakeLists.txt files. Is this still an issue?

@bstaletic
Copy link
Contributor Author

bstaletic commented Dec 6, 2018

The CI errors are different, and it definitely looks like an improvement.

  • Python 3.4 also has a dynamic_annotations.h header in which it defines AnnotateBenignRaceSized, though signatures don't match and it's causing Travis to fail.
  • On python2, however, travis reports the same linker error that I see.
  • MSVC 2015 and 2017 report the following error:
    • In int128.h lines 134 and 137 conflict, because, apparently wchar_t is the same as unsigned short. It's the declaration of absl::uint128::operator wchar_t and absl::uint128::operator unsigned short.
    • Same thing happens later on lines 478 and 471 for definitions.

EDIT: In case you glossed over it, I am compiling everything with -fvisibility=hidden.

@JonathanDCohen
Copy link
Contributor

Thanks for the update.

I'm a little confused, how is python being used here?

@bstaletic
Copy link
Contributor Author

Our project links against libpython.so to expose some C++ classes/functions to python. That means there's #include <Python.h> in some place, that pulls in python's dynamic_annotations.h.

For the MSVC failures, if you think that is an okay solution, I can submit a PR that wraps the overloads in #ifdef _MSVC (or whatever the macro is).

@JonathanDCohen
Copy link
Contributor

Our project links against libpython.so to expose some C++ classes/functions to python. That means there's #include <Python.h> in some place, that pulls in python's dynamic_annotations.h.

cries. We probably should have prepended AbslInternal to our names, and ABSL_ to the macros. This will require some substantial migrations to fix as far as I can see. Or Python could take an Abseil dep :P

For the MSVC that might work, but we would need to also release a migration tool for that as well, since that would be an api-breaking change.

@JonathanDCohen
Copy link
Contributor

Adding Alex who has more insight both on int128 and also the migration tools required for these bugfixes

@bstaletic
Copy link
Contributor Author

cries. We probably should have prepended AbslInternal to our names, and ABSL_ to the macros. This will require some substantial migrations to fix as far as I can see. Or Python could take an Abseil dep :P

I took a quick look at dynamic_annotations.h and had a similar reaction when I saw that those things aren't namespaced.

For the MSVC that might work, but we would need to also release a migration tool for that as well, since that would be an api-breaking change.

According to compiler explorer wchar_t and unsigned short aren't the same and all trivial cases that I've tried on there actually allowed overloads on wchar_t/unsigned short. Still, Appveyor failed with the above mentioned error: https://ci.appveyor.com/project/bstaletic/ycmd-noh9b/builds/20825796/job/17hppomji75uemha#L412

@JonathanDCohen
Copy link
Contributor

For the MSVC bug, https://docs.microsoft.com/en-us/previous-versions/dh8che7s(v=vs.140) looks promising

@bstaletic
Copy link
Contributor Author

Wow, thanks for that! Turns out, on MSVC, our project was explicitly using /Zc:wchar_t-. Though git blame wasn't useful to find out why as that flag was there since before the git repository was created, some... at least 5 years ago.
Anyway, Appveyor is healed.

That leaves us with the linker error and the conflict with the CPython function.

For the linker error, every search I've been able to make was about -fPIC or -shared missing somewhere. When I manually inline the absl::container_internal::RandomSeed() in absl::container_internal::ShouldInsertBackwards(), I get a slightly different linker error:

/usr/sbin/ld: ../absail/absl/container/libabsl_container.a(raw_hash_set.cc.o): relocation R_X86_64_TPOFF32 against `_ZZN4absl18container_internal21ShouldInsertBackwardsEmPaE7counter' can not be used when making a shared object; recompile with -fPIC

Indeed, all abseil components that are compiled are compiled into static libraries. Then I tried adding -fPIC to the global flags and it got linked correctly. But adding -fPIC manually and globally doesn't seem like the right thing to do. Did I miss anything?

Once again, thanks for the help.

@bstaletic
Copy link
Contributor Author

For the linker error, this is what I'm currently doing:

list( APPEND LLVM_FLAGS "-fPIC" )
list( APPEND GCC_FLAGS "-fPIC" )

And this solved the linker error. I am fine with this, since -fPIC isn't global.

@JonathanDCohen
Copy link
Contributor

Let me check in to that. I'm pretty sure we build with PIC internally, I'm surprised we don't externally. I'm not sure if I missed it somewhere but can you detail the platform and toolchain you're using?

As for the MSVC bug, that's awesome! I also wrote a change today to wrap the wchar_t typedefs so we're always picking up __wchar_t on MSVC. So if for whatever reason you actually need that /Zwchar_t- option you should be good.

@bstaletic
Copy link
Contributor Author

I'm able to reproduce the linker error on three completely different systems, all of which end up compiling without -fPIC.

  • My local - GCC 8.2.1, linux x86_64 or Clang 7.0.0
  • Travis - Ubuntu 14.04 GCC 4.9/Clang 3.4
  • CircleCI - macOS AppleClang 9.0.0.9000038

With -fPIC and no /Zc:wchar_t-, "all" that's left is the conflicting dynamic_annotations.h, which is actually the same as the abseil one, only a couple years older (it says /* Copyright (c) 2008-2009, Google Inc.).

@Mizux
Copy link
Contributor

Mizux commented Dec 20, 2018

for -fPIC we should add

set_property(TARGET ${_NAME} PROPERTY POSITION_INDEPENDENT_CODE ON)

just here:

# INTERFACE libraries can't have the CXX_STANDARD property set
set_property(TARGET ${_NAME} PROPERTY CXX_STANDARD ${ABSL_CXX_STANDARD})
set_property(TARGET ${_NAME} PROPERTY CXX_STANDARD_REQUIRED ON)

src: https://cmake.org/cmake/help/latest/prop_tgt/POSITION_INDEPENDENT_CODE.html

@JonathanDCohen
Copy link
Contributor

Do we want to do that ourselves? Wouldn't that cause issues if our users didn't compile with PIC on? Seems like we need to inherit that somehow from our users

@bstaletic
Copy link
Contributor Author

I've tried it out on our project. Usually, compiling our benchmark looks like this:

  • Compile Boost.Filesystem and Boost.Regex into libBoostParts.a (static library with -fPIC)
  • Compile ycm_core.so, which is the C++ part of the project (dynamic library with -fPIC)
  • Compile libbenchmark.a - the google benchmark library.
  • Link everything into a ycm_core_benchmarks executable.

I changed our CMakeLists.txt for ycm_core.so to compile a static library without -fPIC and left everything else the same. I confirmed that BoostParts was compiled with PIC and ycm_core without and the benchmarks worked.

Then I tried to compile our test suit and it worked as well. So current builds wouldn't be broken if PROPERTY POSITION_INDEPENDENT_CODE ON was set unconditionally.

However, I also measured the size and compile time of BoostParts with and without -fPIC. With -fPIC the size increased by 4kB (0.04%) and compile time increased by 0.03s (0.07%). In my opinion the added time and library size is negligible.

@JonathanDCohen
Copy link
Contributor

Hey there! I'm just now getting back from vacation, sorry I didn't say anything on this thread. Is there any update on your end? I can talk with the team about turning on PIC. It doesn't seem super controversial, although I think we should try to figure out why we aren't linking on some systems without PIC, as there are other systems where we build and link just fine.

@JonathanDCohen JonathanDCohen added the cmake related to CMake support label Jan 28, 2019
@abseil abseil deleted a comment Jan 28, 2019
@abseil abseil deleted a comment Jan 28, 2019
@abseil abseil deleted a comment Jan 28, 2019
@bstaletic
Copy link
Contributor Author

No real news. I've been following Abseil's master and everything works as long as I add-fPIC and avoid Python 3.4.

@JonathanDCohen
Copy link
Contributor

Quick question. I just reread teh comments and looked more closely into -fvisibility=hidden. It seems to only really affect DSOs. Are you compiling Abseil into a DSO? This may cause some issues, but I'm not up to date on all of them.

I have a guess. From https://gcc.gnu.org/wiki/Visibility

It lets the optimiser produce better code. PLT indirections (when a function call or variable access must be looked up via the Global Offset Table such as in PIC code) can be completely avoided, thus substantially avoiding pipeline stalls on modern processors and thus much faster code. Furthermore when most of the symbols are bound locally, they can be safely elided (removed) completely through the entire DSO. This gives greater latitude especially to the inliner which no longer needs to keep an entry point around "just in case".

I wonder if there aren't linker collisions in symbols which are elided when PIC is turned on? I'm going to investigate this, just keeping you up to speed with what I'm looking at. I also asked another Abseil engineer who is more well-versed in DSO design to look into this as well.

@bstaletic
Copy link
Contributor Author

Yes, -fvisibility=hidden affects DSOs only. I am compiling Abseil into a DSO, otherwise relocation R_X86_64_TPOFF32 wouldn't be forbidden. -fvisibility=hidden made the DSO significantly smaller and even boosted performance a bit.

I've tried nm --dynamic ycm_core.so | grep absl which resulted in 0 symbols. nm ycm_core.so | grep absl however has shown 5252 symbols. So whatever links against my ycm_core.so won't be able to see any traces of abseil. This is the effect of -fvisibility=hidden.

@bstaletic
Copy link
Contributor Author

I managed to create a minimal example that shows the issue with missing -fPIC!

CMakeLists.txt:

cmake_minimum_required(VERSION 2.8.12)
project(my_project)
add_subdirectory(abseil-cpp)
set(THREADS_PREFER_PTHREAD_FLAG TRUE)
find_package (Threads REQUIRED)
add_library(my_lib SHARED foo.cpp)
target_link_libraries(my_lib absl::flat_hash_map Threads::Threads)

foo.cpp:

#include "abseil-cpp/absl/container/flat_hash_map.h"

int main(void) {
	absl::flat_hash_map<int, int> x;
	printf("%d\n", x.size());
}

Generate Makefile with cmake -DCMAKE_BUILD_TYPE=Debug and compile with make my_lib.

The linker error:

/usr/sbin/ld: abseil-cpp/absl/container/libabsl_internal_hashtablez_sampler.a(hashtablez_sampler.cc.o): relocation R_X86_64_TPOFF32 against `_ZZN4absl18container_internal12_GLOBAL__N_120GetGeometricVariableElE3rng' can not be used when making a shared object; recompile with -fPIC
/usr/sbin/ld: final link failed: nonrepresentable section on output
collect2: error: ld returned 1 exit status

@JonathanDCohen
Copy link
Contributor

I reproduced your error, and it looks like it's the rng in GetGeometricVariable. However changing the rng to a static instead of thread_local reveals similar issues in the standard library.

usr/bin/ld: abseil-cpp/absl/container/libabsl_internal_hashtablez_sampler.a(hashtablez_sampler.cc.o): relocation R_X86_64_PC32 against symbol `_ZNSt14numeric_limitsIlE3maxEv' can not be used when making a shared object; recompile with -fPIC

Also, I tried building my_lib as a static library and everything worked perfectly. I think the moral of the story here is you just have to use -fPIC for shared libraries. Are there any objections to closing this at this point?

@bstaletic
Copy link
Contributor Author

I don't mind closing the issue, but I've got a question. Currently I'm using list( APPEND LLVM_FLAGS "-fPIC" ) to insert -fPIC into abseil's flags. It feels like a hack to poke around, what I assumed are, abseil internals. Should I keep using LLVM_FLAGS or keep watching abseil development for a different, supported, way to insert necessary flags?

@JonathanDCohen
Copy link
Contributor

Looks like you want to use CMAKE_POSITION_INDEPENDENT_CODE as suggested by @Mizux earlier.

I added set(CMAKE_POSITION_INDEPENDENT_CODE TRUE) to your CMakeLists.txt file from above and confirmed everything built and linked.

I'm closing this, please feel free to reopen

@bstaletic
Copy link
Contributor Author

Great, that worked! Thanks for helping out.

@linrongbin16
Copy link

linrongbin16 commented Oct 18, 2021

Hi, I'm facing an almost the same issue, using abseil with pybind11.

environment: ubuntu 20.04 LTS, python 3.8, gcc-9, cmake 3.16.3 installed with apt-get.

Here's a minimum reproduce folder structure:

+ workspace
   - abseil-cpp (use https://github.com/abseil/abseil-cpp 20210324.2)
   - pybind11 (use https://github.com/pybind/pybind11 v2.8.0)
   - CMakeLists.txt
   - foo.cpp

Here's the CMakeLists.txt:

cmake_minimum_required(VERSION 3.8)
project(myfoo)
add_compile_options("-std=c++11")
add_compile_options("-Wall")

set(THREADS_PREFER_PTHREAD_FLAG TRUE)
find_packages(Threads REQUIRED)

find_package(Python COMPONENTS Interpreter Development)
message("Python_FOUND: ${Python_FOUND}")
message("Python_VERSION: ${Python_VERSION}")

add_subdirectory(pybind11)
add_subdirectory(abseil-cpp)

set(CMAKE_POSITION_INDEPENDENT_CODE TRUE)   # Notice: whether this line exists, the final build is failed
add_library(myfoo MODULE foo.cpp)
target_link_libraries(myfoo PRIVATE pybind11::module pybind11::lto pybind11::windows_extras absl::flat_hash_map absl::strings Threads::Threads)
pybind11_extension(myfoo)
pybind11_strip(myfoo)
set_target_properties(myfoo PROPERTIES
    CXX_VISIBILITY_PRESET "hidden"
    CUDA_VISIBILITY_PRESET "hidden")

Here's the foo.cpp:

#include "abseil-cpp/absl/container/flat_hash_map.h"
#include "abseil-cpp/absl/strings/string_view.h"
#include "pybind11/pybind11.h"

namespace py = pybind11;

struct Counter {
  absl::flat_hash_map<int, int> map_;
  int count(int key) const {
    auto result = map_.find(key);
    return result == map_.end() ? 0 : result->second;
  }
};

PYBIND11_MODULE(myfoo, m) {
  m.doc() = "my foo shared library for python3 binding";
  py::class_<Counter>(m, "Counter").def(py::init<>()).def("count", &Counter::count, py::arg("key"));
}

build commands and informations

$ ~/workspace: cmake -DCMAKE_BUILD_TYPE=Release -S . -B _build
Python_FOUND:TRUE
Python_VERSION:3.8.10
-- pybind11 v2.8.0 
-- Configuring done
-- Generating done
-- Build files have been written to: /home/rolin/workspace/_build

$ ~/workspace: cd _build
$ ~/workspace: make
[  1%] Built target absl_log_severity
...
[ 54%] Built target absl_cord
[ 55%] Linking CXX shared module myfoo.cpython-38-x86_64-linux-gnu.so
/usr/bin/ld: abseil-cpp/absl/container/libabsl_raw_hash_set.a(raw_hash_set.cc.o): relocation R_X86_64_TPOFF32 against symbol `_ZZN4absl18container_internal10RandomSeedEvE7counter' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: abseil-cpp/absl/hash/libabsl_hash.a(hash.cc.o): relocation R_X86_64_PC32 against symbol `_ZN4absl13hash_internal15MixingHashState5kSeedE' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: bad value
collect2: error: ld returned 1 exit status
make[2]: *** [CMakeFiles/myfoo.dir/build.make:116: myfoo.cpython-38-x86_64-linux-gnu.so] Error 1
make[1]: *** [CMakeFiles/Makefile2:667: CMakeFiles/myfoo.dir/all] Error 2
make: *** [Makefile:84: all] Error 2

@Mizux
Copy link
Contributor

Mizux commented Oct 18, 2021

@linrongbin16 in your CMakeLists.txt before add_subdirectory() calls you should add

set(CMAKE_POSITION_INDEPENDENT_CODE ON)

see: https://github.com/google/or-tools/blob/86d4c543f717fa8716a9d66c25143660668bf825/cmake/dependencies/CMakeLists.txt#L40

Also you should set the C++ dialect instead of using the ugly add_compile_options("-std=c++11") since abseil-cpp is sensitive to it in its headers....
EDIT: For "ugly" I mean in Modern CMake you should prefer to use target_compile_option() and/or CMAKE_CXX_... also on Windows you must use -std:c++11 so you can break portability when using link option instead of trying to use CMake built-in options.
note: I apology, I may have been a little bit rude...
i.e. prefer to use:

# Pick the C++ standard to compile with.
# Abseil currently supports C++11, C++14, and C++17.
set(CMAKE_CXX_STANDARD 11)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

before add_subdirectory()
note: this is also written in the README.md ;) -> https://github.com/abseil/abseil-cpp/blame/ee0ebdae4a9e789b92f5abbe8573ddeeaead4864/CMake/README.md#L43-L48

@linrongbin16
Copy link

Thanks, got it.

vt-alt pushed a commit to altlinux/specs that referenced this issue Aug 30, 2022
- first build for Sisyphus

libabseil-cpp-20211102.0-alt3

- rebuilt with -DCMAKE_POSITION_INDEPENDENT_CODE=ON
  (see abseil/abseil-cpp#225)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake related to CMake support
Projects
None yet
Development

No branches or pull requests

5 participants