[build] Better isolate compiler flagsupstream/tmpsantos-sanitizer

Previously we had UndefinedBehavior mixed with other sanitizers, in
special implicit conversions and overflows.

Also, run the sanitizers in Release configuration, so they will run
faster. -O1 is just needed for perfectly nice stack traces.

Also, moving the flags to an interface will prevent them to leak
to our external dependencies.

7 files changed, 54 insertions, 32 deletions

diff --git a/circle.yml b/circle.yml
index f14b4e2488..f35ac46a1f 100644
--- a/circle.yml
+++ b/circle.yml
@@ -53,7 +53,7 @@ workflows:
           target_is_linux: true
           requires:
             - next-linux-gcc8-release
-          config_params: '-G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER=gcc-8 -DCMAKE_CXX_COMPILER=g++-8 -DCMAKE_BUILD_TYPE=DebugCoverage'
+          config_params: '-G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER=gcc-8 -DCMAKE_CXX_COMPILER=g++-8 -DCMAKE_BUILD_TYPE=Debug -DMBGL_WITH_COVERAGE'
       - next-build-template:
           name: next-linux-clang8-release
           executor_name: ubuntu-disco
@@ -74,7 +74,7 @@ workflows:
           target_is_linux: true
           requires:
             - next-sanity-checks
-          config_params: '-G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER=clang-8 -DCMAKE_CXX_COMPILER=clang++-8 -DCMAKE_BUILD_TYPE=Sanitize -DMBGL_WITH_SANITIZER=address'
+          config_params: '-G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER=clang-8 -DCMAKE_CXX_COMPILER=clang++-8 -DMBGL_WITH_SANITIZER=address'
           test_params: '|| true'
       - next-build-template:
           name: next-linux-tsan
@@ -82,7 +82,7 @@ workflows:
           target_is_linux: true
           requires:
             - next-sanity-checks
-          config_params: '-G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER=clang-8 -DCMAKE_CXX_COMPILER=clang++-8 -DCMAKE_BUILD_TYPE=Sanitize -DMBGL_WITH_SANITIZER=thread'
+          config_params: '-G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER=clang-8 -DCMAKE_CXX_COMPILER=clang++-8 -DMBGL_WITH_SANITIZER=thread'
       - next-build-template:
           name: next-FIXME-linux-valgrind
           executor_name: ubuntu-disco
@@ -95,7 +95,7 @@ workflows:
           target_is_linux: true
           requires:
             - next-sanity-checks
-          config_params: '-G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER=clang-8 -DCMAKE_CXX_COMPILER=clang++-8 -DCMAKE_BUILD_TYPE=Sanitize -DMBGL_WITH_SANITIZER=undefined'
+          config_params: '-G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER=clang-8 -DCMAKE_CXX_COMPILER=clang++-8 -DMBGL_WITH_SANITIZER=undefined'
           test_params: '|| true'
       - next-build-template:
           name: next-qt5-linux-gcc5-release
diff --git a/next/CMakeLists.txt b/next/CMakeLists.txt
index 797a7e28f7..2c13e7ece8 100644
--- a/next/CMakeLists.txt
+++ b/next/CMakeLists.txt
@@ -17,11 +17,9 @@ include(CTest)
 set(
     CMAKE_CONFIGURATION_TYPES
     Debug
-    DebugCoverage
     MinSizeRel
     RelWithDebInfo
     Release
-    Sanitize
 )
 
 if(NOT CMAKE_BUILD_TYPE)
@@ -29,33 +27,46 @@ if(NOT CMAKE_BUILD_TYPE)
 endif()
 
 option(MBGL_WITH_CORE_ONLY "Build only the core bits, no platform code" OFF)
+option(MBGL_WITH_COVERAGE "Enable code coverage collection" OFF)
 option(MBGL_WITH_QT "Build Mapbox GL Qt bindings" OFF)
-option(MBGL_WITH_SANITIZER "Use [address|thread|memory|undefined] here" OFF)
+option(MBGL_WITH_SANITIZER "Use [address|thread|undefined] here" OFF)
 
-set(CMAKE_CXX_FLAGS_DEBUGCOVERAGE "${CMAKE_CXX_FLAGS_DEBUG} --coverage")
-set(CMAKE_C_FLAGS_DEBUGCOVERAGE "${CMAKE_C_FLAGS_DEBUG} --coverage")
-
-set(
-    CMAKE_CXX_FLAGS_SANITIZE
-    "-DSANITIZE -g -O1 -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=implicit-conversion -fsanitize=unsigned-integer-overflow -fsanitize=${MBGL_WITH_SANITIZER}"
+add_library(
+    mbgl-compiler-options INTERFACE
 )
-set(
-    CMAKE_C_FLAGS_SANITIZE
-    "-DSANITIZE -g -O1 -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=implicit-conversion -fsanitize=unsigned-integer-overflow -fsanitize=${MBGL_WITH_SANITIZER}"
+
+target_compile_options(
+    mbgl-compiler-options
+    INTERFACE
+        $<$<BOOL:${MBGL_WITH_COVERAGE}>:--coverage>
+        $<$<BOOL:${MBGL_WITH_SANITIZER}>:-DSANITIZE>
+        $<$<BOOL:${MBGL_WITH_SANITIZER}>:-fno-omit-frame-pointer>
+        $<$<BOOL:${MBGL_WITH_SANITIZER}>:-fno-optimize-sibling-calls>
+        $<$<STREQUAL:${MBGL_WITH_SANITIZER},address>:-fsanitize=address>
+        $<$<STREQUAL:${MBGL_WITH_SANITIZER},thread>:-fsanitize=thread>
+        $<$<STREQUAL:${MBGL_WITH_SANITIZER},undefined>:-fsanitize=implicit-conversion>
+        $<$<STREQUAL:${MBGL_WITH_SANITIZER},undefined>:-fsanitize=undefined>
+        $<$<STREQUAL:${MBGL_WITH_SANITIZER},undefined>:-fsanitize=unsigned-integer-overflow>
+        $<$<COMPILE_LANGUAGE:CXX>:-fno-rtti>
+        $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<NOT:$<PLATFORM_ID:Windows>>>:-Wall>
+        $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<NOT:$<PLATFORM_ID:Windows>>>:-Wextra>
+        $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<NOT:$<PLATFORM_ID:Windows>>>:-Werror>
+        $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<PLATFORM_ID:Android>>:-Wno-error=tautological-constant-compare>
+        $<$<CXX_COMPILER_ID:GNU>:-Wno-error=maybe-uninitialized>
+        $<$<CXX_COMPILER_ID:GNU>:-Wno-error=return-type>
+        $<$<CXX_COMPILER_ID:GNU>:-Wno-error=unknown-pragmas>
+        $<$<CXX_COMPILER_ID:AppleClang>:-Wno-error=deprecated-declarations>
+        $<$<CXX_COMPILER_ID:AppleClang>:-Wno-error=unused-parameter>
+        $<$<CXX_COMPILER_ID:AppleClang>:-Wno-error=unused-property-ivar>
 )
 
-add_compile_options(
-    $<$<COMPILE_LANGUAGE:CXX>:-fno-rtti>
-    $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<NOT:$<PLATFORM_ID:Windows>>>:-Wall>
-    $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<NOT:$<PLATFORM_ID:Windows>>>:-Wextra>
-    $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<NOT:$<PLATFORM_ID:Windows>>>:-Werror>
-    $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<PLATFORM_ID:Android>>:-Wno-error=tautological-constant-compare>
-    $<$<CXX_COMPILER_ID:GNU>:-Wno-error=maybe-uninitialized>
-    $<$<CXX_COMPILER_ID:GNU>:-Wno-error=return-type>
-    $<$<CXX_COMPILER_ID:GNU>:-Wno-error=unknown-pragmas>
-    $<$<CXX_COMPILER_ID:AppleClang>:-Wno-error=deprecated-declarations>
-    $<$<CXX_COMPILER_ID:AppleClang>:-Wno-error=unused-parameter>
-    $<$<CXX_COMPILER_ID:AppleClang>:-Wno-error=unused-property-ivar>
+target_link_libraries(
+    mbgl-compiler-options
+    INTERFACE
+        $<$<BOOL:${MBGL_WITH_COVERAGE}>:--coverage>
+        $<$<STREQUAL:${MBGL_WITH_SANITIZER},address>:-fsanitize=address>
+        $<$<STREQUAL:${MBGL_WITH_SANITIZER},thread>:-fsanitize=thread>
+        $<$<STREQUAL:${MBGL_WITH_SANITIZER},undefined>:-fsanitize=undefined>
 )
 
 cmake_policy(SET CMP0063 NEW)
@@ -931,6 +942,7 @@ target_link_libraries(
     PRIVATE
         Mapbox::Base::Extras::kdbush.hpp
         Mapbox::Base::supercluster.hpp
+        mbgl-compiler-options
         mbgl-vendor-boost
         mbgl-vendor-earcut.hpp
         mbgl-vendor-eternal
diff --git a/next/benchmark/CMakeLists.txt b/next/benchmark/CMakeLists.txt
index 117d556041..338c7185ba 100644
--- a/next/benchmark/CMakeLists.txt
+++ b/next/benchmark/CMakeLists.txt
@@ -34,6 +34,7 @@ target_link_libraries(
     PRIVATE
         ${MBGL_CORE_PRIVATE_LIBRARIES}
         mbgl-vendor-benchmark
+        mbgl-compiler-options
         mbgl-core
         $<$<PLATFORM_ID:Android>:mbgl-core-android>
 )
diff --git a/next/platform/glfw/CMakeLists.txt b/next/platform/glfw/CMakeLists.txt
index b0263362a7..a599082584 100644
--- a/next/platform/glfw/CMakeLists.txt
+++ b/next/platform/glfw/CMakeLists.txt
@@ -35,6 +35,7 @@ target_link_libraries(
         Mapbox::Base::Extras::rapidjson
         Mapbox::Map
         OpenGL::GL
+        mbgl-compiler-options
         mbgl-vendor-cheap-ruler-cpp
 )
 
diff --git a/next/platform/qt/qt.cmake b/next/platform/qt/qt.cmake
index 4fb56176d0..e2181c70c6 100644
--- a/next/platform/qt/qt.cmake
+++ b/next/platform/qt/qt.cmake
@@ -132,9 +132,11 @@ target_compile_definitions(
 
 target_link_libraries(
     qmapboxgl
-    Qt5::Core
-    Qt5::Gui
-    mbgl-core
+    PRIVATE
+        Qt5::Core
+        Qt5::Gui
+        mbgl-compiler-options
+        mbgl-core
 )
 
 add_executable(
@@ -150,7 +152,11 @@ set_property(TARGET mbgl-qt PROPERTY CXX_STANDARD 98)
 
 target_link_libraries(
     mbgl-qt
-    PRIVATE Qt5::Widgets Qt5::Gui qmapboxgl
+    PRIVATE
+        Qt5::Widgets
+        Qt5::Gui
+        mbgl-compiler-options
+        qmapboxgl
 )
 
 add_executable(
diff --git a/next/render-test/CMakeLists.txt b/next/render-test/CMakeLists.txt
index fa4bb0d615..3cec9dc4bd 100644
--- a/next/render-test/CMakeLists.txt
+++ b/next/render-test/CMakeLists.txt
@@ -42,6 +42,7 @@ target_link_libraries(
         Mapbox::Base::Extras::args
         Mapbox::Base::Extras::filesystem
         Mapbox::Base::pixelmatch-cpp
+        mbgl-compiler-options
         mbgl-vendor-boost
     PUBLIC mbgl-core
 )
diff --git a/next/test/CMakeLists.txt b/next/test/CMakeLists.txt
index 8a2e34fe9a..5063111c6c 100644
--- a/next/test/CMakeLists.txt
+++ b/next/test/CMakeLists.txt
@@ -156,6 +156,7 @@ target_link_libraries(
         ${MBGL_CORE_PRIVATE_LIBRARIES}
         Mapbox::Base::Extras::args
         Mapbox::Base::pixelmatch-cpp
+        mbgl-compiler-options
         mbgl-core
         $<$<PLATFORM_ID:Android>:mbgl-core-android>
 )