Skip to content

Conversation

@wesm
Copy link
Contributor

@wesm wesm commented Feb 19, 2019

Linking with -lpthread instead of -pthread can cause failures of the sort like

/home/travis/build/apache/arrow/cpp-toolchain/bin/ld: /home/travis/build/apache/arrow/cpp-toolchain/lib/libbenchmark.a(benchmark.cc.o): in function `benchmark::RunSpecifiedBenchmarks(benchmark::BenchmarkReporter*, benchmark::BenchmarkReporter*)':
benchmark.cc:(.text+0x2235): undefined reference to `std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)())'
/home/travis/build/apache/arrow/cpp-toolchain/bin/ld: /home/travis/build/apache/arrow/cpp-toolchain/lib/libbenchmark.a(benchmark.cc.o): in function `std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(benchmark::internal::Benchmark::Instance const*, unsigned long, int, benchmark::internal::ThreadManager*), benchmark::internal::Benchmark::Instance const*, unsigned long, int, benchmark::internal::ThreadManager*> > >::~_State_impl()':
benchmark.cc:(.text._ZNSt6thread11_State_implINS_8_InvokerISt5tupleIJPFvPKN9benchmark8internal9Benchmark8InstanceEmiPNS4_13ThreadManagerEES8_miSA_EEEEED2Ev[_ZNSt6thread11_State_implINS_8_InvokerISt5tupleIJPFvPKN9benchmark8internal9Benchmark8InstanceEmiPNS4_13ThreadManagerEES8_miSA_EEEEED5Ev]+0x10): undefined reference to `std::thread::_State::~_State()'
/home/travis/build/apache/arrow/cpp-toolchain/bin/ld: /home/travis/build/apache/arrow/cpp-toolchain/lib/libbenchmark.a(benchmark.cc.o): in function `std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(benchmark::internal::Benchmark::Instance const*, unsigned long, int, benchmark::internal::ThreadManager*), benchmark::internal::Benchmark::Instance const*, unsigned long, int, benchmark::internal::ThreadManager*> > >::~_State_impl()':
benchmark.cc:(.text._ZNSt6thread11_State_implINS_8_InvokerISt5tupleIJPFvPKN9benchmark8internal9Benchmark8InstanceEmiPNS4_13ThreadManagerEES8_miSA_EEEEED0Ev[_ZNSt6thread11_State_implINS_8_InvokerISt5tupleIJPFvPKN9benchmark8internal9Benchmark8InstanceEmiPNS4_13ThreadManagerEES8_miSA_EEEEED5Ev]+0x14): undefined reference to `std::thread::_State::~_State()'
/home/travis/build/apache/arrow/cpp-toolchain/bin/ld: /home/travis/build/apache/arrow/cpp-toolchain/lib/libbenchmark.a(benchmark.cc.o):(.data.rel.ro._ZTINSt6thread11_State_implINS_8_InvokerISt5tupleIJPFvPKN9benchmark8internal9Benchmark8InstanceEmiPNS4_13ThreadManagerEES8_miSA_EEEEEE[_ZTINSt6thread11_State_implINS_8_InvokerISt5tupleIJPFvPKN9benchmark8internal9Benchmark8InstanceEmiPNS4_13ThreadManagerEES8_miSA_EEEEEE]+0x10): undefined reference to `typeinfo for std::thread::_State'
collect2: error: ld returned 1 exit status

when using libraries cross-compiled on one host for use on another. CMake version 3.1 and higher has the THREADS_PREFER_PTHREAD_FLAG flag which toggles the behavior of ${CMAKE_THREAD_LIBS_INIT}.

See https://cmake.org/cmake/help/v3.1/module/FindThreads.html

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

Googlers can find more info about SignCLA and this PR by following this link.

@wesm wesm force-pushed the use-pthreads-flag branch from 2b81027 to 059cee3 Compare February 19, 2019 02:53
@googlebot
Copy link

CLAs look good, thanks!

Googlers can find more info about SignCLA and this PR by following this link.

@dmah42 dmah42 merged commit 7c57133 into google:master Feb 19, 2019
@dmah42
Copy link
Member

dmah42 commented Feb 19, 2019

Thanks!

@LebedevRI
Copy link
Collaborator

Is this actually correct/sufficient though?
https://cmake.org/cmake/help/v3.1/module/FindThreads.html

If the use of the -pthread compiler and linker flag is prefered then the caller can set
  THREADS_PREFER_PTHREAD_FLAG
Please note that the compiler flag can only be used with the imported target. Use of both the imported target as well as this switch is highly recommended for new code.

And yet benchmark uses CMAKE_THREAD_LIBS_INIT, not the Threads::Threads import target,
as far as i can see:

target_link_libraries(benchmark ${BENCHMARK_CXX_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT})

@wesm
Copy link
Contributor Author

wesm commented Feb 19, 2019

@LebedevRI if you use Threads::Threads then this is what appears in benchmarkTargets.cmake instead of -pthread. So consumers of the installed library would need to also use FindThreads in order to use the CMake config

@wesm
Copy link
Contributor Author

wesm commented Feb 19, 2019

I haven't tested this extensively on other platforms so would be interested to get feedback from experts who understand the nuances more deeply

@jcfr
Copy link

jcfr commented Feb 19, 2019

(Background: @wesm I am commenting here following up you last tweet.)

So consumers of the installed library would need to also use FindThreads in order to use the CMake config

The key here is to update cmake/Config.cmake.in so that it has:

include(CMakeFindDependencyMacro)
set(THREADS_PREFER_PTHREAD_FLAG 1)
find_dependency(Threads)

Doing so would ensure that consumer do not have to worry about the private dependencies of the benchmark project and would allow to use Thread::Thread target.

Note that CMakeLists.txt would have to be updated to also set THREADS_PREFER_PTHREAD_FLAG

@wesm
Copy link
Contributor Author

wesm commented Feb 19, 2019

Seems reasonable. @jcfr are you able to submit a PR for this?

JBakamovic pushed a commit to JBakamovic/benchmark that referenced this pull request Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants