Page MenuHomePhabricator

[CMake] Use __libc_start_main rather than fopen when checking for C library
ClosedPublic

Authored by phosek on Jan 23 2019, 11:39 PM.

Details

Summary

The check_library_exists CMake uses a custom symbol definition. This
is a problem when checking for C library symbols because Clang
recognizes many of them as builtins, and returns the
-Wbuiltin-requires-header (or -Wincompatible-library-redeclaration)
error. When building with -Werror which is the default, this causes
the check_library_exists check fail making the build think that C
library isn't available.

To avoid this issue, we should use a symbol that isn't recognized by
Clang and wouldn't cause the same issue. __libc_start_main seems like
reasonable choice that fits the bill.

Diff Detail

Repository
rL LLVM

Event Timeline

phosek created this revision.Jan 23 2019, 11:39 PM

I ran into this while debugging http://lab.llvm.org:8014/builders/fuchsia-x86_64-linux/builds/146/steps/ninja-build/logs/stdio, this fails because CMake fails to find libatomic, but that's not the real culprit, the real culprit is that CMake fails to find libc, so it doesn't include it on the link line when testing for libatomic which makes that check fail for obvious reasons.

smeenai accepted this revision.Jan 25 2019, 2:23 PM

-Werror is the default in general, or for some particular configuration? Passing -Werror to configuration checks seems like a bad idea in general...

This seems reasonable, but it also seems fragile (e.g. what if clang starts recognizing atexit some day). Thoughts on a more robust fix? Perhaps check_library_exists should strip out any -Werror flags, but that would be a CMake-side change... maybe we could do something with CMAKE_REQUIRED_FLAGS?

This revision is now accepted and ready to land.Jan 25 2019, 2:23 PM

I don't know if it works with the way CMake checks stuff but I think autotools can be fooled by checking for main.

phosek updated this revision to Diff 183783.Jan 27 2019, 4:39 PM
phosek retitled this revision from [CMake] Use atexit rather than fopen when checking for C library to [CMake] Use __libc_start_main rather than fopen when checking for C library.
phosek edited the summary of this revision. (Show Details)

-Werror is the default in general, or for some particular configuration? Passing -Werror to configuration checks seems like a bad idea in general...

Agreed, but I think that's a broader discussion. There are also cases where that's desirable.

This seems reasonable, but it also seems fragile (e.g. what if clang starts recognizing atexit some day). Thoughts on a more robust fix? Perhaps check_library_exists should strip out any -Werror flags, but that would be a CMake-side change... maybe we could do something with CMAKE_REQUIRED_FLAGS?

I could wrap the check_library_exists in cmake_push_check_state and cmake_pop_check_state and add -Wno-error to CMAKE_REQUIRED_FLAGS.

I don't know if it works with the way CMake checks stuff but I think autotools can be fooled by checking for main.

I don't think main works because that's not a symbol provided by libc, but this gave me an idea to use __libc_start_main which is better than atexit because while the latter might be eventually recognized by Clang, the former won't be since that one isn't defined in any public libc header. This also matches what we do for other libraries like libgcc_s (__gcc_personality_v0) or libc++ (__cxa_throw) where we also rely on internal symbols.

This revision was automatically updated to reflect the committed changes.

This change breaks building on Mac OS X - at least for libc++.
The user visible failure is:

-- Performing Test LIBCXX_SUPPORTS_STD_EQ_CXX11_FLAG
-- Performing Test LIBCXX_SUPPORTS_STD_EQ_CXX11_FLAG - Failed
-- Performing Test LIBCXX_SUPPORTS_STD_COLON_CXX11_FLAG
-- Performing Test LIBCXX_SUPPORTS_STD_COLON_CXX11_FLAG - Failed
CMake Error at CMakeLists.txt:526 (message):
  C++11 or greater is required but the compiler does not support c++11

and in the log, I see:

Performing C++ SOURCE FILE Test LIBCXX_SUPPORTS_STD_EQ_CXX11_FLAG failed with the following output:
Change Dir: /Sources/LLVM/build/test-libcxx/CMakeFiles/CMakeTmp

Run Build Command:"/usr/local/bin/ninja" "cmTC_4ab59"
[1/2] Building CXX object CMakeFiles/cmTC_4ab59.dir/src.cxx.o
[2/2] Linking CXX executable cmTC_4ab59
FAILED: cmTC_4ab59 
: && /Sources/LLVM/bin/bin/clang++  -DLIBCXX_SUPPORTS_STD_EQ_CXX11_FLAG  -nodefaultlibs -Wl,-search_paths_first -Wl,-headerpad_max_install_names  CMakeFiles/cmTC_4ab59.dir/src.cxx.o  -o cmTC_4ab59   && :
ld: dynamic main executables must link with libSystem.dylib for architecture x86_64
clang-9: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

Source file was:
int main() { return 0; }