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.

Sign up for GitHub

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

Jump to bottom

Static build needs -fPIC CXXFLAG with CMake #854

Closed
xor-gate opened this issue Aug 23, 2016 · 3 comments
Closed

Static build needs -fPIC CXXFLAG with CMake #854

xor-gate opened this issue Aug 23, 2016 · 3 comments

Comments

@xor-gate
Copy link

xor-gate commented Aug 23, 2016 •
edited
Loading

With the introduction of the new release 1.8.0 compilation runned fine but the linking failed.

System info:

  • Googletest 1.8.0
  • Debian 8 AMD64
  • GCC 4.9.2 (debian)
  • CMake 3.5.1

Error:

[ 44%] Linking CXX executable time
/usr/bin/ld: gtest-prefix/src/gtest-build/libgtest.a(gtest-all.cc.o): relocation R_X86_64_32S against `_ZTVN7testing8internal17TestEventRepeaterE' can not be used when making a shared object
; recompile with -fPIC
gtest-prefix/src/gtest-build/libgtest.a: error adding symbols: Bad value
collect2: error: ld returned 1 exit status
tests/CMakeFiles/time.dir/build.make:97: recipe for target 'tests/time' failed
make[3]: *** [tests/time] Error 1
CMakeFiles/Makefile2:1672: recipe for target 'tests/CMakeFiles/time.dir/all' failed
make[2]: *** [tests/CMakeFiles/time.dir/all] Error 2

We use CMake ExternalProject_Add

# gtest.cmake

ExternalProject_Add(
        gtest
        DOWNLOAD_COMMAND ""
        SOURCE_DIR ${PROJECT_SOURCE_DIR}/common/gtest/googletest
        CMAKE_ARGS -DCMAKE_BUILD_TYPE=Release
        INSTALL_COMMAND ""
)
ExternalProject_Get_Property(gtest binary_dir)
ExternalProject_Get_Property(gtest source_dir)
set(gtest_main_library ${binary_dir}/libgtest_main.a)
set(gtest_library ${binary_dir}/libgtest.a)
set(gtest_include_dir "${source_dir}/include")
include_directories(SYSTEM "${gtest_include_dir}")

# application
include("gtest.txt")
add_executable(${test} ${test}.cpp)
add_dependencies(${test} di gtest)
target_link_libraries(${test} ${gtest_library})
target_link_libraries(${test} ${gtest_main_library})

I patch the googletest CMakeLists.txt localy with (from here):

#-----------------------------------------------------------------------------
# Add needed flag for gnu on linux like enviroments to build static common libs
# suitable for linking with shared object libs.
if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64")
  if(NOT "${CMAKE_CXX_FLAGS}" MATCHES "-fPIC")
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC")
  endif()
  if(NOT "${CMAKE_C_FLAGS}" MATCHES "-fPIC")
    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fPIC")
  endif()
endif()

This seems to work but beter would be a option which is enabled by default like LLVM does:

LLVM_ENABLE_PIC:BOOL
Add the -fPIC flag to the compiler command-line, if the compiler supports this flag. Some systems, like Windows, do not need this flag. Defaults to ON.

Which will look like this (taken from LLVM):

option(gtest_enable_pic "Build with -fPIC flag" ON)

function(add_flag_or_print_warning flag name)
  check_cxx_compiler_flag("-Werror ${flag}" "CXX_SUPPORTS_${name}")
  if (CXX_SUPPORTS_${name})
    message(STATUS "Building with ${flag}")
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${flag}" PARENT_SCOPE)
  else()
    message(WARNING "${flag} is not supported.")
  endif()
endfunction()

if( gtest_enable_pic )
  if( XCODE )
    # Xcode has -mdynamic-no-pic on by default, which overrides -fPIC. I don't
    # know how to disable this, so just force ENABLE_PIC off for now.
    message(WARNING "-fPIC not supported with Xcode.")
  elseif( WIN32 OR CYGWIN)
    # On Windows all code is PIC. MinGW warns if -fPIC is used.
  else()
    add_flag_or_print_warning("-fPIC" FPIC)
  endif()
endif()

What do you think?
@martingkelly
Copy link

I also would favor something like this, as I have hit the same bug in gtest.

@gennadiycivil
Copy link
Contributor

@xor-gate
Thank you very much for this report. The best way to approach this would be to create a proper PR and submit it for consideration.

@gennadiycivil
Copy link
Contributor

I have been cleaning up older and inactive GitHub googletest issues. You may see an issue "closed" if it appears to be inactive/abandoned
Thank you

SlavaMelanko added a commit to SlavaMelanko/rtb that referenced this issue Dec 20, 2018
@SlavaMelanko
Gtest: [-fPIC](google/googletest#854)
eb3011a
@zchrissirhcz zchrissirhcz mentioned this issue Nov 13, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@xor-gate @martingkelly @gennadiycivil