[libc++] Disable coverage with sanitize-coverage=0

[bungeman]

Summary

When building libcxx, libcxxabi, and libunwind the build environment may
specify any number of sanitizers. For some build feature tests these
sanitizers must be disabled to prevent spurious linking errors. With
-fsanitize= this is straight forward with -fno-sanitize=all. With
-fsanitize-coverage= there is no -fno-sanitize-coverage=all, but there
is the equivalent undocumented but tested -fsanitize-coverage=0.

The current build rules fail to disable 'trace-pc-guard'. By disabling
all sanitize-coverage flags, including 'trace-pc-guard', possible
spurious linker errors are prevented. In particular, this allows libcxx,
libcxxabi, and libunwind to be built with HonggFuzz.

CMAKE_REQUIRED_FLAGS is extra compile flags when running CMake build
configuration steps (like check_cxx_compiler_flag). It does not affect
the compile flags for the actual build of the project (unless of course
these flags change whether or not a given source compiles and links or
not). So libcxx, libcxxabi, and libunwind will still be built with any
specified sanitize-coverage as before. The build configuration steps
(which are mostly checking to see if certain compiler flags are
available) will not try to compile and link "int main() { return 0;}"
(or other specified source) with sanitize-coverage (which can fail to
link at this stage in building, since the final compile flags required
are yet to be determined).

The change to LIBFUZZER_CFLAGS was done to keep it consistent with the
obvious intention of disabling all sanitize-coverage. This appears to
be intentional, preventing the fuzzer driver itself from showing up in
any coverage calculations.

[ldionne]

I'm not super familiar with -fsanitize-coverage, but I don't understand why this change is correct. Why is it correct to go from excluding edge,trace-cmp,indirect-calls,8bit-counters from -fsanitize-coverage to excluding everything? Aren't we basically losing the ability to generate coverage information?

[MaskRay]

-fsanitize-coverage was introduced in D31639 (CC @phosek).

At a glance, the intention might to catch layering issue (libc++abi is very low, a runtime library using libc++abi used by libc++abi may be problematic). For many -fsanitize-coverage=, AIUI the __sanitizer_cov_* hooks are provided by the user. If the user hooks use C++, the layering may be weird.

[ldionne]

Thanks for the background @MaskRay. I will let @phosek take a look at this, if he's fine with it, I am too.

[bungeman]

Perhaps the commit comment could have been a bit more detailed.

CMAKE_REQUIRED_FLAGS is extra compile flags when running CMake build configuration steps (like check_cxx_compiler_flag). It does not affect the compile flags for the actual build of the project (unless of course these flags change whether or not a given source compiles and links or not). So libcxx, libcxxabi, and libunwind would still be built with any specified sanitize-coverage as before. The build configuration steps (which are mostly checking to see if certain compiler flags are available) will not try to compile and link int main() { return 0;} (or other specified source) with sanitize-coverage (which can fail to link at this stage in building, since the final compile flags required are yet to be determined).

The change to LIBFUZZER_CFLAGS was done to keep it consistent with the obvious intention of disabling all sanitize-coverage. This appears to be intentional, preventing the fuzzer driver itself from showing up in any coverage calculations. This is somewhat less important for my use case, but it seemed a good idea to update all of these.

[ldionne]

LGTM but please update the commit message with the info in your last comment. Thanks!

[phosek]

LGTM

[bungeman]

Add additional explanation to commit message.

[bungeman]

After reading through the contribution doc again, it looks like I need to mention here that I don't have commit access and so will need someone to commit this on my behalf.

[vitalybuka]

In D116050#3222382, @bungeman wrote:

Add additional explanation to commit message.

I guess we can't see update in patch description if it's in local git commit.
Can you make sure that Review summary above includes the update?

[vitalybuka]

In D116050#3222397, @bungeman wrote:

After reading through the contribution doc again, it looks like I need to mention here that I don't have commit access and so will need someone to commit this on my behalf.

Unless any other volunteers, I will land the patch later today.

[bungeman]

In D116050#3228219, @vitalybuka wrote:

In D116050#3222382, @bungeman wrote:

Add additional explanation to commit message.

I guess we can't see update in patch description if it's in local git commit.
Can you make sure that Review summary above includes the update?

I've made sure that the Review summary on this page and the commit message of the latest patch are the same and include the additional explanation (the last two paragraphs).

compiler-rt/lib/fuzzer/CMakeLists.txt

 elseif(TARGET cxx-headers OR HAVE_LIBCXX)
   # libFuzzer uses C++ standard library headers.
   set(LIBFUZZER_DEPS cxx-headers)
 endif()
 
 append_list_if(COMPILER_RT_HAS_OMIT_FRAME_POINTER_FLAG -fno-omit-frame-pointer LIBFUZZER_CFLAGS)
 
 if (CMAKE_CXX_FLAGS MATCHES "fsanitize-coverage")
-  list(APPEND LIBFUZZER_CFLAGS -fno-sanitize-coverage=trace-pc-guard,edge,trace-cmp,indirect-calls,8bit-counters)
+  list(APPEND LIBFUZZER_CFLAGS -fsanitize-coverage=0)
 endif()
 
 if(MSVC)
   # Silence warnings by turning off exceptions in MSVC headers and avoid an
   # error by unnecessarily defining thread_local when it isn't even used on
   # Windows.
   list(APPEND LIBFUZZER_CFLAGS -D_HAS_EXCEPTIONS=0)
 else()

libcxx/cmake/config-ix.cmake

set(MINGW_LIBRARIES mingw32 ${MINGW_RUNTIME} moldname mingwex msvcrt advapi32
                         shell32 user32 kernel32 mingw32 ${MINGW_RUNTIME}
                         moldname mingwex msvcrt)
     list(APPEND CMAKE_REQUIRED_LIBRARIES ${MINGW_LIBRARIES})
   endif()
   if (CMAKE_C_FLAGS MATCHES -fsanitize OR CMAKE_CXX_FLAGS MATCHES -fsanitize)
     set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize=all")
   endif ()
   if (CMAKE_C_FLAGS MATCHES -fsanitize-coverage OR CMAKE_CXX_FLAGS MATCHES -fsanitize-coverage)
-    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize-coverage=edge,trace-cmp,indirect-calls,8bit-counters")
+    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fsanitize-coverage=0")
   endif ()
 endif ()
 
 # Check compiler pragmas
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   cmake_push_check_state()
   set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Werror=unknown-pragmas")
   check_c_source_compiles("

libcxxabi/cmake/config-ix.cmake

set(MINGW_LIBRARIES mingw32 ${MINGW_RUNTIME} moldname mingwex msvcrt advapi32
                         shell32 user32 kernel32 mingw32 ${MINGW_RUNTIME}
                         moldname mingwex msvcrt)
     list(APPEND CMAKE_REQUIRED_LIBRARIES ${MINGW_LIBRARIES})
   endif()
   if (CMAKE_C_FLAGS MATCHES -fsanitize OR CMAKE_CXX_FLAGS MATCHES -fsanitize)
     set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize=all")
   endif ()
   if (CMAKE_C_FLAGS MATCHES -fsanitize-coverage OR CMAKE_CXX_FLAGS MATCHES -fsanitize-coverage)
-    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize-coverage=edge,trace-cmp,indirect-calls,8bit-counters")
+    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fsanitize-coverage=0")
   endif ()
 endif ()
 
 # Check compiler pragmas
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   cmake_push_check_state()
   set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Werror=unknown-pragmas")
   check_c_source_compiles("

libunwind/cmake/config-ix.cmake

set(MINGW_LIBRARIES mingw32 ${MINGW_RUNTIME} moldname mingwex msvcrt advapi32
                         shell32 user32 kernel32 mingw32 ${MINGW_RUNTIME}
                         moldname mingwex msvcrt)
     list(APPEND CMAKE_REQUIRED_LIBRARIES ${MINGW_LIBRARIES})
   endif()
   if (CMAKE_C_FLAGS MATCHES -fsanitize OR CMAKE_CXX_FLAGS MATCHES -fsanitize)
     set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize=all")
   endif ()
   if (CMAKE_C_FLAGS MATCHES -fsanitize-coverage OR CMAKE_CXX_FLAGS MATCHES -fsanitize-coverage)
-    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize-coverage=edge,trace-cmp,indirect-calls,8bit-counters")
+    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fsanitize-coverage=0")
   endif ()
 endif ()
 
 # Check compiler pragmas
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   cmake_push_check_state()
   set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Werror=unknown-pragmas")
   check_c_source_compiles("