[compiler-rt] Drop -Werror in compiler flag checks

[barannikov88]

Summary

-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.

[barannikov88]

Update commit message

[barannikov88]

I'm not sure this is the right thing to do. The regression was introduced in https://github.com/llvm/llvm-project/commit/f051c1ded40970169cda84b0966331ae7ad424ed

[ahatanak]

The fix seems correct to me.

compiler-rt/cmake/config-ix.cmake

96

Does this have to be fixed too?

[barannikov88]

Fix another occurance

[ahatanak]

LGTM

[phosek]

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.

compiler-rt/cmake/config-ix.cmake

96

This shouldn't need -Werror at all.

[ahatanak]

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.

[hctim]

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.

https://lab.llvm.org/buildbot/#/builders/85/builds/15278

[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 :)

[barannikov88]

In D146920#4231154, @hctim wrote:

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.

[smeenai]

In D146920#4231178, @barannikov88 wrote:

In D146920#4231154, @hctim wrote:

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.

[hctim]

Already reverted in https://reviews.llvm.org/rG270067d69532448c4f6d7956e68d7a392857f9ce :)

[barannikov88]

In D146920#4231272, @smeenai wrote:

Feel free to include the changes from D147164 as part of this patch, which should fix the errors.

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.

[barannikov88]

Drop changes to -ftrivial-auto-var-init causing build failures

[barannikov88]

Mention "Reland" in the commit title

[barannikov88]

This now breaks M68k build https://lab.llvm.org/buildbot#builders/192/builds/1022

cc1: error: -Wformat-security ignored without -Wformat [-Werror=format-security]

[barannikov88]

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.

[barannikov88]

In D146920#4230541, @ahatanak wrote:

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.

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.

[barannikov88]

Remove unnecessary -Werror.

[barannikov88]

Nah, it is a typo in both places...
I don't understand what's going on.

[barannikov88]

Fix the other typo

[phosek]

Can you remove -Werror from the other checks in compiler-rt/cmake/config-ix.cmake as well?

[barannikov88]

In D146920#4238845, @phosek wrote:

Can you remove -Werror from the other checks in compiler-rt/cmake/config-ix.cmake as well?

I can try. Hopefully it will not break more bots.

[barannikov88]

Remove remaining -Werrors

[barannikov88]

In D146920#4238845, @phosek wrote:

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?

[phosek]

LGTM

[barannikov88]

Rebase

[barannikov88]

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.

compiler-rt/cmake/Modules/CompilerRTUtils.cmake

function(add_security_warnings out_flags macosx_sdk_version)
   append_list_if(COMPILER_RT_HAS_SIZEOF_POINTER_MEMACCESS_FLAG -Werror=sizeof-pointer-memaccess flags)
   append_list_if(COMPILER_RT_HAS_SIZEOF_ARRAY_ARGUMENT_FLAG -Werror=sizeof-array-argument flags)
   append_list_if(COMPILER_RT_HAS_SUSPICIOUS_MEMACCESS_FLAG -Werror=suspicious-memaccess flags)
   append_list_if(COMPILER_RT_HAS_BUILTIN_MEMCPY_CHK_SIZE_FLAG -Werror=builtin-memcpy-chk-size flags)
   append_list_if(COMPILER_RT_HAS_ARRAY_BOUNDS_POINTER_ARITHMETIC_FLAG -Werror=array-bounds-pointer-arithmetic flags)
   append_list_if(COMPILER_RT_HAS_RETURN_STACK_ADDRESS_FLAG -Werror=return-stack-address flags)
   append_list_if(COMPILER_RT_HAS_SIZEOF_ARRAY_DECAY_FLAG -Werror=sizeof-array-decay flags)
   append_list_if(COMPILER_RT_HAS_FORMAT_INSUFFICIENT_ARGS_FLAG -Werror=format-insufficient-args flags)
-  append_list_if(COMPILER_RT_HAS_BUILTIN_FORMAL_SECURITY_FLAG -Werror=format-security flags)
+  append_list_if(COMPILER_RT_HAS_BUILTIN_FORMAT_SECURITY_FLAG -Werror=format-security flags)
   append_list_if(COMPILER_RT_HAS_SIZEOF_ARRAY_DIV_FLAG -Werror=sizeof-array-div)
   append_list_if(COMPILER_RT_HAS_SIZEOF_POINTER_DIV_FLAG -Werror=sizeof-pointer-div)
 
   # Add -Wformat-nonliteral only if we can avoid adding the definition of
   # eprintf. On Apple platforms, eprintf is needed only on macosx and only if
   # its version is older than 10.7.
   if ("${macosx_sdk_version}" VERSION_GREATER_EQUAL 10.7)
     list(APPEND flags -Werror=format-nonliteral -DDONT_DEFINE_EPRINTF)
   endif()
 
   set(${out_flags} "${flags}" PARENT_SCOPE)
 endfunction()

compiler-rt/cmake/config-ix.cmake

 check_cxx_compiler_flag(-fno-profile-instr-use COMPILER_RT_HAS_FNO_PROFILE_INSTR_USE_FLAG)
 check_cxx_compiler_flag(-fno-coverage-mapping COMPILER_RT_HAS_FNO_COVERAGE_MAPPING_FLAG)
 check_cxx_compiler_flag("-Werror -mcrc32"    COMPILER_RT_HAS_MCRC32_FLAG)
 check_cxx_compiler_flag("-Werror -msse3" COMPILER_RT_HAS_MSSE3_FLAG)
 check_cxx_compiler_flag("-Werror -msse4.2"   COMPILER_RT_HAS_MSSE4_2_FLAG)
 check_cxx_compiler_flag(--sysroot=.          COMPILER_RT_HAS_SYSROOT_FLAG)
 check_cxx_compiler_flag("-Werror -mcrc"      COMPILER_RT_HAS_MCRC_FLAG)
 check_cxx_compiler_flag(-fno-partial-inlining COMPILER_RT_HAS_FNO_PARTIAL_INLINING_FLAG)
 check_cxx_compiler_flag(-Werror -ftrivial-auto-var-init=pattern COMPILER_RT_HAS_TRIVIAL_AUTO_INIT)
 
 if(NOT WIN32 AND NOT CYGWIN)
   # MinGW warns if -fvisibility-inlines-hidden is used.
   check_cxx_compiler_flag("-fvisibility-inlines-hidden" COMPILER_RT_HAS_FVISIBILITY_INLINES_HIDDEN_FLAG)
 endif()
 
 check_cxx_compiler_flag(/GR COMPILER_RT_HAS_GR_FLAG)
 check_cxx_compiler_flag(/GS COMPILER_RT_HAS_GS_FLAG)

 check_cxx_compiler_flag(/wd4146 COMPILER_RT_HAS_WD4146_FLAG)
 check_cxx_compiler_flag(/wd4206 COMPILER_RT_HAS_WD4206_FLAG)
 check_cxx_compiler_flag(/wd4291 COMPILER_RT_HAS_WD4291_FLAG)
 check_cxx_compiler_flag(/wd4221 COMPILER_RT_HAS_WD4221_FLAG)
 check_cxx_compiler_flag(/wd4391 COMPILER_RT_HAS_WD4391_FLAG)
 check_cxx_compiler_flag(/wd4722 COMPILER_RT_HAS_WD4722_FLAG)
 check_cxx_compiler_flag(/wd4800 COMPILER_RT_HAS_WD4800_FLAG)
 
-check_cxx_compiler_flag("-Werror -Warray-bounds" COMPILER_RT_HAS_ARRAY_BOUNDS_FLAG)
-check_cxx_compiler_flag("-Werror -Wuninitialized" COMPILER_RT_HAS_UNINITIALIZED_FLAG)
-check_cxx_compiler_flag("-Werror -Wshadow" COMPILER_RT_HAS_SHADOW_FLAG)
-check_cxx_compiler_flag("-Werror -Wempty-body" COMPILER_RT_HAS_EMPTY_BODY_FLAG)
-check_cxx_compiler_flag("-Werror -Wsizeof-pointer-memaccess" COMPILER_RT_HAS_SIZEOF_POINTER_MEMACCESS_FLAG)
-check_cxx_compiler_flag("-Werror -Wsizeof-array-argument" COMPILER_RT_HAS_SIZEOF_ARRAY_ARGUMENT_FLAG)
-check_cxx_compiler_flag("-Werror -Wsuspicious-memaccess" COMPILER_RT_HAS_SUSPICIOUS_MEMACCESS_FLAG)
-check_cxx_compiler_flag("-Werror -Wbuiltin-memcpy-chk-size" COMPILER_RT_HAS_BUILTIN_MEMCPY_CHK_SIZE_FLAG)
-check_cxx_compiler_flag("-Werror -Warray-bounds-pointer-arithmetic" COMPILER_RT_HAS_ARRAY_BOUNDS_POINTER_ARITHMETIC_FLAG)
-check_cxx_compiler_flag("-Werror -Wreturn-stack-address" COMPILER_RT_HAS_RETURN_STACK_ADDRESS_FLAG)
-check_cxx_compiler_flag("-Werror -Wsizeof-array-decay" COMPILER_RT_HAS_SIZEOF_ARRAY_DECAY_FLAG)
-check_cxx_compiler_flag("-Werror -Wformat-insufficient-args" COMPILER_RT_HAS_FORMAT_INSUFFICIENT_ARGS_FLAG)
-check_cxx_compiler_flag("-Werror -Wformat-security" COMPILER_RT_HAS_BUILTIN_FORMAL_SECURITY_FLAG)
-check_cxx_compiler_flag("-Werror -Wsizeof-array-div" COMPILER_RT_HAS_SIZEOF_ARRAY_DIV_FLAG)
-check_cxx_compiler_flag("-Werror -Wsizeof-pointer-div" COMPILER_RT_HAS_SIZEOF_POINTER_DIV_FLAG)
+check_cxx_compiler_flag(-Warray-bounds COMPILER_RT_HAS_ARRAY_BOUNDS_FLAG)
+check_cxx_compiler_flag(-Wuninitialized COMPILER_RT_HAS_UNINITIALIZED_FLAG)
+check_cxx_compiler_flag(-Wshadow COMPILER_RT_HAS_SHADOW_FLAG)
+check_cxx_compiler_flag(-Wempty-body COMPILER_RT_HAS_EMPTY_BODY_FLAG)
+check_cxx_compiler_flag(-Wsizeof-pointer-memaccess COMPILER_RT_HAS_SIZEOF_POINTER_MEMACCESS_FLAG)
+check_cxx_compiler_flag(-Wsizeof-array-argument COMPILER_RT_HAS_SIZEOF_ARRAY_ARGUMENT_FLAG)
+check_cxx_compiler_flag(-Wsuspicious-memaccess COMPILER_RT_HAS_SUSPICIOUS_MEMACCESS_FLAG)
+check_cxx_compiler_flag(-Wbuiltin-memcpy-chk-size COMPILER_RT_HAS_BUILTIN_MEMCPY_CHK_SIZE_FLAG)
+check_cxx_compiler_flag(-Warray-bounds-pointer-arithmetic COMPILER_RT_HAS_ARRAY_BOUNDS_POINTER_ARITHMETIC_FLAG)
+check_cxx_compiler_flag(-Wreturn-stack-address COMPILER_RT_HAS_RETURN_STACK_ADDRESS_FLAG)
+check_cxx_compiler_flag(-Wsizeof-array-decay COMPILER_RT_HAS_SIZEOF_ARRAY_DECAY_FLAG)
+check_cxx_compiler_flag(-Wformat-insufficient-args COMPILER_RT_HAS_FORMAT_INSUFFICIENT_ARGS_FLAG)
+check_cxx_compiler_flag(-Wformat-security COMPILER_RT_HAS_BUILTIN_FORMAL_SECURITY_FLAG)
+check_cxx_compiler_flag(-Wsizeof-array-div COMPILER_RT_HAS_SIZEOF_ARRAY_DIV_FLAG)
+check_cxx_compiler_flag(-Wsizeof-pointer-div COMPILER_RT_HAS_SIZEOF_POINTER_DIV_FLAG)
 
 # Symbols.
 check_symbol_exists(__func__ "" COMPILER_RT_HAS_FUNC_SYMBOL)
 
 # Includes.
 check_cxx_compiler_flag(-nostdinc++ COMPILER_RT_HAS_NOSTDINCXX_FLAG)
 check_cxx_compiler_flag(-nostdlib++ COMPILER_RT_HAS_NOSTDLIBXX_FLAG)
 check_include_files("sys/auxv.h"    COMPILER_RT_HAS_AUXV)