[build] Better isolate compiler flags

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.

12 files changed, 82 insertions, 45 deletions

diff --git a/circle.yml b/circle.yml
index 58ec727908..33fa2fd121 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=ON'
       - 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/bin/CMakeLists.txt b/next/bin/CMakeLists.txt
index b002783770..daef945a41 100644
--- a/next/bin/CMakeLists.txt
+++ b/next/bin/CMakeLists.txt
@@ -5,7 +5,7 @@ add_executable(
 
 target_link_libraries(
     mbgl-offline
-    PRIVATE Mapbox::Base::Extras::args mbgl-core
+    PRIVATE Mapbox::Base::Extras::args mbgl-compiler-options mbgl-core
 )
 
 add_executable(
@@ -15,7 +15,7 @@ add_executable(
 
 target_link_libraries(
     mbgl-render
-    PRIVATE Mapbox::Base::Extras::args mbgl-core
+    PRIVATE Mapbox::Base::Extras::args mbgl-compiler-options mbgl-core
 )
 
 install(TARGETS mbgl-offline mbgl-render RUNTIME DESTINATION bin)
diff --git a/next/platform/android/android.cmake b/next/platform/android/android.cmake
index 9d19edd560..559baf3cbc 100644
--- a/next/platform/android/android.cmake
+++ b/next/platform/android/android.cmake
@@ -254,7 +254,7 @@ target_include_directories(
 
 target_link_libraries(
     mbgl-core-android
-    PRIVATE Mapbox::Base::jni.hpp mbgl-core
+    PRIVATE Mapbox::Base::jni.hpp mbgl-compiler-options mbgl-core
 )
 
 add_library(
@@ -269,7 +269,11 @@ target_include_directories(
 
 target_link_libraries(
     mapbox-gl
-    PRIVATE Mapbox::Base::jni.hpp mbgl-core mbgl-core-android
+    PRIVATE
+        Mapbox::Base::jni.hpp
+        mbgl-compiler-options
+        mbgl-core
+        mbgl-core-android
 )
 
 add_library(
@@ -291,6 +295,7 @@ target_link_libraries(
         Mapbox::Base::value
         Mapbox::Base::weak
         log
+        mbgl-compiler-options
 )
 
 add_executable(
@@ -301,7 +306,11 @@ add_executable(
 
 target_link_libraries(
     mbgl-test-runner
-    PRIVATE Mapbox::Base::jni.hpp mapbox-gl mbgl-test
+    PRIVATE
+        Mapbox::Base::jni.hpp
+        mapbox-gl
+        mbgl-compiler-options
+        mbgl-test
 )
 
 add_executable(
@@ -312,7 +321,11 @@ add_executable(
 
 target_link_libraries(
     mbgl-benchmark-runner
-    PRIVATE Mapbox::Base::jni.hpp mapbox-gl mbgl-benchmark
+    PRIVATE
+        Mapbox::Base::jni.hpp
+        mapbox-gl
+        mbgl-compiler-options
+        mbgl-benchmark
 )
 
 add_library(
@@ -335,6 +348,7 @@ target_link_libraries(
         Mapbox::Base::jni.hpp
         android
         log
+        mbgl-compiler-options
         mbgl-render-test
 )
 
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/linux/linux.cmake b/next/platform/linux/linux.cmake
index 5374ede323..cb866d8334 100644
--- a/next/platform/linux/linux.cmake
+++ b/next/platform/linux/linux.cmake
@@ -96,7 +96,7 @@ target_compile_definitions(
 
 target_link_libraries(
     mbgl-test-runner
-    PRIVATE mbgl-test
+    PRIVATE mbgl-compiler-options mbgl-test
 )
 
 add_executable(
@@ -106,7 +106,7 @@ add_executable(
 
 target_link_libraries(
     mbgl-benchmark-runner
-    PRIVATE mbgl-benchmark
+    PRIVATE mbgl-compiler-options mbgl-benchmark
 )
 
 add_executable(
@@ -116,7 +116,7 @@ add_executable(
 
 target_link_libraries(
     mbgl-render-test-runner
-    PRIVATE mbgl-render-test
+    PRIVATE mbgl-compiler-options mbgl-render-test
 )
 
 add_test(NAME mbgl-benchmark-runner COMMAND mbgl-benchmark-runner WORKING_DIRECTORY ${MBGL_ROOT})
diff --git a/next/platform/macos/macos.cmake b/next/platform/macos/macos.cmake
index 721d19e625..915bd99c0e 100644
--- a/next/platform/macos/macos.cmake
+++ b/next/platform/macos/macos.cmake
@@ -169,7 +169,7 @@ target_compile_definitions(
 
 target_link_libraries(
     mbgl-test-runner
-    PRIVATE mbgl-test
+    PRIVATE mbgl-compiler-options mbgl-test
 )
 
 add_executable(
@@ -179,7 +179,7 @@ add_executable(
 
 target_link_libraries(
     mbgl-benchmark-runner
-    PRIVATE mbgl-benchmark
+    PRIVATE mbgl-compiler-options mbgl-benchmark
 )
 
 add_executable(
@@ -189,7 +189,7 @@ add_executable(
 
 target_link_libraries(
     mbgl-render-test-runner
-    PRIVATE mbgl-render-test
+    PRIVATE mbgl-compiler-options mbgl-render-test
 )
 
 set_property(TARGET mbgl-benchmark-runner PROPERTY FOLDER Executables)
diff --git a/next/platform/node/CMakeLists.txt b/next/platform/node/CMakeLists.txt
index 840fab2964..adabd620c3 100644
--- a/next/platform/node/CMakeLists.txt
+++ b/next/platform/node/CMakeLists.txt
@@ -46,7 +46,7 @@ target_link_libraries(
 
 # FIXME: Node bindings only run fully on Linux now because it requires libuv RunLoop (which is the default on Linux). Also, Sanitizer will
 # not work here because node will do a dlopen(), which is not currently supported.
-if(CMAKE_SYSTEM_NAME STREQUAL Linux AND NOT CMAKE_BUILD_TYPE STREQUAL Sanitize)
+if(CMAKE_SYSTEM_NAME STREQUAL Linux AND NOT MBGL_WITH_SANITIZER)
     add_test(NAME mbgl-node-memory COMMAND npm run test-memory WORKING_DIRECTORY ${MBGL_ROOT})
     add_test(NAME mbgl-node-query COMMAND npm run test-query WORKING_DIRECTORY ${MBGL_ROOT})
     add_test(NAME mbgl-node-test COMMAND npm run test WORKING_DIRECTORY ${MBGL_ROOT})
diff --git a/next/platform/qt/qt.cmake b/next/platform/qt/qt.cmake
index 4fb56176d0..4d81891947 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(
@@ -168,6 +174,7 @@ target_link_libraries(
     PRIVATE
         Qt5::Gui
         Qt5::OpenGL
+        mbgl-compiler-options
         mbgl-test
         pthread
 )
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>
 )