-Werror is redundant because CMake not only checks the
exit status, but also parses the output of try_compile.
-Wformat-security is a special case, because specifying it
without -Wformat causes gcc to emit a warning in a format
not recognized by CMake, which makes the check pass.
-Werror is redundant because CMake not only checks the
I'm not sure this is the right thing to do. The regression was introduced in https://github.com/llvm/llvm-project/commit/f051c1ded40970169cda84b0966331ae7ad424ed
The fix seems correct to me.
Does this have to be fixed too?
I don't understand the motivation behind rGf051c1ded40970169cda84b0966331ae7ad424ed, why is it needed? check_cxx_compiler_flag checks if the flag is accepted, whether the flag itself causes Clang to emit error or warning shouldn't have any impact on the result of the check.
This shouldn't need -Werror at all.
I added -Werror because the changes I made in https://reviews.llvm.org/D131714 broke someone's internal build (see @wlei's comment). Apparently, gcc emits a warning if you try to use -Wformat-security without passing -Wformat too.
-Werror is probably only needed for -Wformat-security, but I think passing it in other places doesn't do any harm.
Hey, looks like this breaks check-sanitizer on all the bots. Easily reproducible on a vanilla build, just do check-sanitizer on something that builds compiler-rt.
[4/90] Generating Sanitizer-x86_64-Test-Nolibc FAILED: projects/compiler-rt/lib/sanitizer_common/tests/Sanitizer-x86_64-Test-Nolibc /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/projects/compiler-rt/lib/sanitizer_common/tests/Sanitizer-x86_64-Test-Nolibc cd /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/projects/compiler-rt/lib/sanitizer_common/tests && /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/./bin/clang++ sanitizer_nolibc_test_main.x86_64.o -Wl,-whole-archive libRTSanitizerCommon.test.nolibc.x86_64.a -Wl,-no-whole-archive -o /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build0/projects/compiler-rt/lib/sanitizer_common/tests/./Sanitizer-x86_64-Test-Nolibc -static -nostdlib -m64 -m64 ld: error: undefined symbol: memset >>> referenced by sanitizer_allocator_primary32.h:356 (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary32.h:356) >>> sanitizer_allocator.cpp.o:(__sanitizer::SizeClassAllocator32<__sanitizer::AP32>::PopulateFreeList(__sanitizer::AllocatorStats*, __sanitizer::SizeClassAllocator32LocalCache<__sanitizer::SizeClassAllocator32<__sanitizer::AP32> >*, __sanitizer::SizeClassAllocator32<__sanitizer::AP32>::SizeClassInfo*, unsigned long)) in archive libRTSanitizerCommon.test.nolibc.x86_64.a >>> referenced by sanitizer_deadlock_detector.h:244 (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_deadlock_detector.h:244) >>> sanitizer_deadlock_detector1.cpp.o:(__sanitizer::DeadlockDetector<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector<unsigned long> > >::addEdges(__sanitizer::DeadlockDetectorTLS<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector<unsigned long> > >*, unsigned long, unsigned int, int)) in archive libRTSanitizerCommon.test.nolibc.x86_64.a >>> referenced by sanitizer_printf.cpp:308 (/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_printf.cpp:308) >>> sanitizer_printf.cpp.o:(__sanitizer::SharedPrintfCode(bool, char const*, __va_list_tag*)) in archive libRTSanitizerCommon.test.nolibc.x86_64.a clang++: error: linker command failed with exit code 1 (use -v to see invocation) [5/90] Linking CXX static library lib/libllvm_gtest.a ninja: build stopped: subcommand failed.
Looks like @smeenai was working on a fix-forward in https://reviews.llvm.org/D147164. Those should be either merged and submitted together or landed together. Thanks :)
I've noticed the failures, but I couldn't relate it to this change. Thank you for clarifying this.
Please revert the patch. It will take some time before I learn how to properly do it.
Feel free to include the changes from D147164 as part of this patch, which should fix the errors.
Already reverted in https://reviews.llvm.org/rG270067d69532448c4f6d7956e68d7a392857f9ce :)
Unfortunately, I won't have time to do this this week.
I think it makes sense to merge this change into D147164, at least for the -ftree-auto-var-init part.
This now breaks M68k build https://lab.llvm.org/buildbot#builders/192/builds/1022
cc1: error: -Wformat-security ignored without -Wformat [-Werror=format-security]
It appears there is a typo in the use of COMPILER_RT_HAS_BUILTIN_FORMAT_SECURITY_FLAG:
append_list_if(COMPILER_RT_HAS_BUILTIN_FORMAL_SECURITY_FLAG -Werror=format-security flags)
FORMAL vs FORMAT.
OK, I'm going to drop these -Werrors.
CMake is smart enough to detect if adding a flag causes a warning and conclude that the flag is unsupported.
It didn't work because of a typo in the variable name.
Can you remove -Werror from the other checks in compiler-rt/cmake/config-ix.cmake as well?
I removed remaining occurrences of -Werror before -W flags and special-cased -Wformat-security.
I had to keep the flag before -m flags because clang may emit "argument unused during compilation" for those,
and this is something that CMake does not treat as a symptom of an unsupported option.
Fixing -ftrivial-auto-var-init=pattern case causes build bot failures; D147164 should address this.
Does it look fine now?
Thanks to @nikic for figuring out the cause of the problem.
The reason the check passes is that it includes -Wall, which implies -Wformat. The actual compilation command doesn't include -Wall, so it fails.