llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28k stars 11.56k forks source link

Scudo tests do not link when building LLVM with ThinLTO #46182

Closed aeubanks closed 4 years ago

aeubanks commented 4 years ago
Bugzilla Link 46838
Resolution FIXED
Resolved on Jul 29, 2020 13:23
Version unspecified
OS Linux
CC @zmodem,@hctim,@pcc

Extended Description

$ cmake -GNinja -DCMAKE_BUILD_TYPE=Release '-DLLVM_ENABLE_PROJECTS=clang;compiler-rt;lld' '-DLLVM_TARGETS_TO_BUILD=X86' -DLLVM_ENABLE_LLD=ON -DLLVM_ENABLE_LTO=Thin -DCMAKE_C_COMPILER=$PWD/../build_bootstrap/bin/clang -DCMAKE_CXX_COMPILER=$PWD/../build_bootstrap/bin/clang++ ../llvm

$ ninja TScudoCUnitTest-i386-Test [2591/2591] Generating ScudoCUnitTest-i386-Test FAILED: projects/compiler-rt/lib/scudo/standalone/tests/ScudoCUnitTest-i386-Test cd /usr/local/google/home/aeubanks/repos/llvm-project/build_cmake/projects/compiler-rt/lib/scudo/standalone/tests && /usr/local/google/home/aeubanks/repos/llvm-project/build_cmake/./bin/clang ScudoUnitTestsObjects.wrappers_c_test.cpp.i386.o ScudoUnitTestsObjects.scudo_unit_test_main.cpp.i386.o ScudoUnitTestsObjects.gtest-all.cc.i386.o /usr/local/google/home/aeubanks/repos/llvm-project/build_cmake/lib/libRTScudoCUnitTest.i386.a -o /usr/local/google/home/aeubanks/repos/llvm-project/build_cmake/projects/compiler-rt/lib/scudo/standalone/tests/./ScudoCUnitTest-i386-Test -lstdc++ -pthread -latomic -m32 ScudoUnitTestsObjects.wrappers_c_test.cpp.i386.o: file not recognized: file format not recognized clang-12: error: linker command failed with exit code 1 (use -v to see invocation) ninja: build stopped: subcommand failed.

The command to build ScudoUnitTestsObjects.wrappers_c_test.cpp.i386.o in build.ninja is:

COMMAND = cd /usr/local/google/home/aeubanks/repos/llvm-project/build_cmake/projects/compiler-rt/lib/scudo/standalone/tests && /usr/local/google/home/aeubanks/repos/llvm-project/build_cmake/./bin/clang -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -flto=thin -Wall -std=c++14 -Wno-unused-parameter -Wno-unknown-warning-option -Wno-covered-switch-default -Wno-suggest-override -DGTEST_NO_LLVM_SUPPORT=1 -DGTEST_HAS_RTTI=0 -I/usr/local/google/home/aeubanks/repos/llvm-project/llvm/utils/unittest/googletest/include -I/usr/local/google/home/aeubanks/repos/llvm-project/llvm/utils/unittest/googletest -I/usr/local/google/home/aeubanks/repos/llvm-project/compiler-rt/include -I/usr/local/google/home/aeubanks/repos/llvm-project/compiler-rt/lib -I/usr/local/google/home/aeubanks/repos/llvm-project/compiler-rt/lib/scudo/standalone -I/usr/local/google/home/aeubanks/repos/llvm-project/compiler-rt/lib/scudo/standalone/include -DGTEST_HAS_RTTI=0 -DSCUDO_DEBUG=1 -Wno-mismatched-new-delete -DGWP_ASAN_HOOKS -m32 -c -o ScudoUnitTestsObjects.wrappers_c_test.cpp.i386.o /usr/local/google/home/aeubanks/repos/llvm-project/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp

The compile step contains -flto=thin, but the link step does not.

I think this has something to do with the add_library() in compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt which uses whatever flags CMake has set, but generate_compiler_rt_tests, which calls sanitizer_test_compile, which calls clang_compile, does not.

I have a review out for a related issue in https://reviews.llvm.org/D84466, which doesn't solve this problem.

aeubanks commented 4 years ago

https://reviews.llvm.org/D84805 should fix this.

aeubanks commented 4 years ago

I believe I have a fix, currently testing.

aeubanks commented 4 years ago

$ ar x ../lib/libRTScudoCUnitTest.i386.a $ for i in *; do file $i; done

backtrace_linux_libc.cpp.o: ELF 32-bit LSB relocatable, Intel 80386, version 1 (SYSV), with debug_info, not stripped checksum.cpp.o: LLVM IR bitcode common.cpp.o: ELF 32-bit LSB relocatable, Intel 80386, version 1 (SYSV), with debug_info, not stripped common_posix.cpp.o: ELF 32-bit LSB relocatable, Intel 80386, version 1 (SYSV), with debug_info, not stripped crash_handler.cpp.o: ELF 32-bit LSB relocatable, Intel 80386, version 1 (SYSV), with debug_info, not stripped crc32_hw.cpp.o: LLVM IR bitcode flags.cpp.o: LLVM IR bitcode flags_parser.cpp.o: LLVM IR bitcode fuchsia.cpp.o: LLVM IR bitcode guarded_pool_allocator.cpp.o: ELF 32-bit LSB relocatable, Intel 80386, version 1 (SYSV), with debug_info, not stripped guarded_pool_allocator_posix.cpp.o: ELF 32-bit LSB relocatable, Intel 80386, version 1 (SYSV), with debug_info, not stripped linux.cpp.o: LLVM IR bitcode mutex_posix.cpp.o: ELF 32-bit LSB relocatable, Intel 80386, version 1 (SYSV), with debug_info, not stripped random.cpp.o: ELF 32-bit LSB relocatable, Intel 80386, version 1 (SYSV), with debug_info, not stripped release.cpp.o: LLVM IR bitcode report.cpp.o: LLVM IR bitcode segv_handler_posix.cpp.o: ELF 32-bit LSB relocatable, Intel 80386, version 1 (SYSV), with debug_info, not stripped stack_trace_compressor.cpp.o: ELF 32-bit LSB relocatable, Intel 80386, version 1 (SYSV), with debug_info, not stripped string_utils.cpp.o: LLVM IR bitcode utilities_posix.cpp.o: ELF 32-bit LSB relocatable, Intel 80386, version 1 (SYSV), with debug_info, not stripped wrappers_c.cpp.o: LLVM IR bitcode

Looks like some files are either compiled with LTO or without LTO. Some compile commands have -fno-lto, which seems to be from:

function(add_compiler_rt_runtime name type) ...

Until we support this some other way, build compiler-rt runtime without LTO

to allow non-LTO projects to link with it.

if(COMPILER_RT_HAS_FNO_LTO_FLAG) set(NO_LTO_FLAGS "-fno-lto") else() set(NO_LTO_FLAGS "") endif()

Maybe -fno-lto is missing from some places.

aeubanks commented 4 years ago

I think https://reviews.llvm.org/D84466 helps with this issue, but still there's the underlying issue of using add_library(). If instead of creating a static library, the CMake infra directly forwarded the test sources to generate_compiler_rt_tests, that might work?

https://github.com/llvm/llvm-project/blob/809600d6642773f71245f76995dab355effc73af/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt#L60

cryptoad commented 4 years ago

If https://reviews.llvm.org/D84466 solves it for TSan, would the same idea apply for Scudo?

aeubanks commented 4 years ago

assigned to @aeubanks