Fix handling of hardening flags with Clang (#16731)

* Use `-Werror` when checking compiler flags.

This should ensure that Clang rejects unknown flags correctly instead of
blindly eating some of them.

* Explicitly check C and C++ flags separately.

This should better handle the unusual case of mismatched compilers.

* Properly use Clang for C++ in our CI checks that build using Clang.

* Apply suggestions from code review

Co-authored-by: Ilya Mashchenko <ilya@netdata.cloud>

* Use functions and loops when possible.

* Fix typos and broken loops.

* Fix more typos.

* Fix bogus commas.

* Fix caching of compiler flag checks.

* Fix flag variable names to make them behave correctly in checks.

CMake adds a preprocessor define with the name of the variable being
defined by a compiler flag check, so we need to ensure not only that
the variable name is unique, but also that it is a valid name for a C
preprocessor definition.

* Fix scoping.

* Fix botched merge during previous rebase.

---------

Co-authored-by: Ilya Mashchenko <ilya@netdata.cloud>

2 files changed, 71 insertions, 58 deletions

diff --git a/.github/dockerfiles/Dockerfile.clang b/.github/dockerfiles/Dockerfile.clang
index f04d1bd501..869254198c 100644
--- a/.github/dockerfiles/Dockerfile.clang
+++ b/.github/dockerfiles/Dockerfile.clang
@@ -9,7 +9,8 @@ RUN /tmp/install-required-packages.sh --dont-wait --non-interactive netdata-all
 
 # Install Clang and set as default CC
 RUN apt-get install -y clang && \
-    update-alternatives --install /usr/bin/cc cc /usr/bin/clang 100
+    update-alternatives --install /usr/bin/cc cc /usr/bin/clang 100 && \
+    update-alternatives --install /usr/bin/c++ c++ /usr/bin/clang++ 100
 
 WORKDIR /netdata
 COPY . .
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5c193b32bb..3bbdeeea0a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -119,91 +119,103 @@ option(ENABLE_LOGS_MANAGEMENT_TESTS "enable logs management tests" True)
 #
 
 include(CheckCCompilerFlag)
+include(CheckCXXCompilerFlag)
 
 # Disable hardening for debug builds by default.
 if(CMAKE_BUILD_TYPE STREQUAL "Debug")
         option(DISABLE_HARDENING "disable adding extra compiler flags for hardening" TRUE)
 else()
-        # FIXME: Until https://github.com/netdata/netdata/pull/16731 resolves the issue
-        option(DISABLE_HARDENING "disable adding extra compiler flags for hardening" TRUE)
+        option(DISABLE_HARDENING "disable adding extra compiler flags for hardening" FALSE)
 endif()
 
-set(EXTRA_HARDENING_FLAGS "")
+# Construct a pre-processor safe name
+function(make_cpp_safe_name value target)
+        string(REPLACE "-" "_" tmp "${value}")
+        string(REPLACE "=" "_" tmp "${tmp}")
+        set(${target} "${tmp}" PARENT_SCOPE)
+endfunction()
 
-if(NOT ${DISABLE_HARDENING})
-        if(NOT ${CMAKE_C_FLAGS} MATCHES "stack-protector")
-                check_c_compiler_flag("-fstack-protector-strong" HAVE_STACK_PROTECTOR_STRONG_FLAG)
-                if(HAVE_STACK_PROTECTOR_STRONG_FLAG)
-                        set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -fstack-protector-strong")
-                else()
-                        check_c_compiler_flag("-fstack-protector" HAVE_STACK_PROTECTOR)
-                        if(HAVE_STACK_PROTECTOR)
-                                set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -fstack-protector")
-                        endif()
-                endif()
-        endif()
+# Conditionally add an extra compiler flag to C and C++ flags.
+#
+# If the language flags already match the `match` argument, skip this flag.
+# Otherwise, check for support for `flag` and if support is found, add it to
+# the language-specific `target` flag group.
+function(add_simple_extra_compiler_flag match flag target)
+        set(CMAKE_REQUIRED_FLAGS "-Werror")
+
+        make_cpp_safe_name("${flag}" flag_name)
 
-        if(NOT ${CMAKE_C_FLAGS} MATCHES "stack-clash-protection")
-                check_c_compiler_flag("-fstack-clash-protection" HAVE_STACK_CLASH_FLAG)
-                if(HAVE_STACK_CLASH_FLAG)
-                        set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -fstack-clash-protection")
+        if(NOT ${CMAKE_C_FLAGS} MATCHES ${match})
+                check_c_compiler_flag("${flag}" HAVE_C_${flag_name})
+                if(HAVE_C_${flag_name})
+                        set(${target}_C_FLAGS "${${target}_C_FLAGS} ${flag}" PARENT_SCOPE)
                 endif()
         endif()
 
-        if(NOT ${CMAKE_C_FLAGS} MATCHES "-fcf-protection")
-                check_c_compiler_flag("-fcf-protection=full" HAVE_CFI_FLAG)
-                if(HAVE_CFI_FLAG)
-                        set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -fcf-protection=full")
+        if(NOT ${CMAKE_CXX_FLAGS} MATCHES ${match})
+                check_cxx_compiler_flag("${flag}" HAVE_CXX_${flag_name})
+                if(HAVE_CXX_${flag_name})
+                        set(${target}_CXX_FLAGS "${${target}_CXX_FLAGS} ${flag}" PARENT_SCOPE)
                 endif()
         endif()
+endfunction()
 
-        if(NOT ${CMAKE_C_FLAGS} MATCHES "branch-protection")
-                check_c_compiler_flag("-mbranch-protection=standard" HAVE_BRANCH_PROT_FLAG)
-                if(HAVE_BRANCH_PROT_FLAG)
-                        set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -mbranch-protection=standard")
+# Same as add_simple_extra_compiler_flag, but check for a second flag if the
+# first one is unsupported.
+function(add_double_extra_compiler_flag match flag1 flag2 target)
+        set(CMAKE_REQUIRED_FLAGS "-Werror")
+
+        make_cpp_safe_name("${flag1}" flag1_name)
+        make_cpp_safe_name("${flag2}" flag2_name)
+
+        if(NOT ${CMAKE_C_FLAGS} MATCHES ${match})
+                check_c_compiler_flag("${flag1}" HAVE_C_${flag1_name})
+                if(HAVE_C_${flag1_name})
+                        set(${target}_C_FLAGS "${${target}_C_FLAGS} ${flag1}" PARENT_SCOPE)
+                else()
+                        check_c_compiler_flag("${flag2}" HAVE_C_${flag2_name})
+                        if(HAVE_C_${flag2_name})
+                                set(${target}_C_FLAGS "${${target}_C_FLAGS} ${flag2}" PARENT_SCOPE)
+                        endif()
                 endif()
         endif()
 
-        if(NOT ${CMAKE_C_FLAGS} MATCHES "_FORTIFY_SOURCE")
-                check_c_compiler_flag("-D_FORTIFY_SOURCE=3" HAVE_FORTIFY_SOURCE_3)
-                if(HAVE_FORTIFY_SOURCE_3)
-                        set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -D_FRTIFY_SOURCE=3")
+        if(NOT ${CMAKE_CXX_FLAGS} MATCHES ${match})
+                check_cxx_compiler_flag("${flag1}" HAVE_CXX_${flag1_name})
+                if(HAVE_CXX_${flag1_name})
+                        set(${target}_CXX_FLAGS "${${target}_CXX_FLAGS} ${flag1}" PARENT_SCOPE)
                 else()
-                        check_c_compiler_flag("-D_FORTIFY_SOURCE=2" HAVE_FORTIFY_SOURCE_2)
-                        if(HAVE_FORTIFY_SOURCE_2)
-                                set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -D_FRTIFY_SOURCE=2")
+                        check_cxx_compiler_flag("${flag2}" HAVE_CXX_${flag2_name})
+                        if(HAVE_CXX_${flag2_name})
+                                set(${target}_CXX_FLAGS "${${target}_CXX_FLAGS} ${flag2}" PARENT_SCOPE)
                         endif()
                 endif()
         endif()
-endif()
+endfunction()
 
-set(EXTRA_OPT_FLAGS "")
+set(EXTRA_HARDENING_C_FLAGS "")
+set(EXTRA_HARDENING_CXX_FLAGS "")
 
-if(NOT ${CMAKE_C_FLAGS} MATCHES "function-sections")
-        check_c_compiler_flag("-ffunction-sections" HAVE_FUNCTION_SECTIONS)
-        if(HAVE_FUNCTION_SECTIONS)
-                set(EXTRA_OPT_FLAGS "${EXTRA_OPT_FLAGS} -ffunction-sections")
-        endif()
-endif()
+set(EXTRA_OPT_C_FLAGS "")
+set(EXTRA_OPT_CXX_FLAGS "")
 
-if(NOT ${CMAKE_C_FLAGS} MATCHES "data-sections")
-        check_c_compiler_flag("-fdata-sections" HAVE_DATA_SECTIONS)
-        if(HAVE_DATA_SECTIONS)
-                set(EXTRA_OPT_FLAGS "${EXTRA_OPT_FLAGS} -fdata-sections")
-        endif()
+if(NOT ${DISABLE_HARDENING})
+        add_double_extra_compiler_flag("stack-protector" "-fstack-protector-strong" "-fstack-protector" EXTRA_HARDENING)
+        add_double_extra_compiler_flag("_FORTIFY_SOURCE" "-D_FORTIFY_SOURCE=3" "-D_FORTIFY_SOURCE=2" EXTRA_HARDENING)
+        add_simple_extra_compiler_flag("stack-clash-protection" "-fstack-clash-protection" EXTRA_HARDENING)
+        add_simple_extra_compiler_flag("-fcf-protection" "-fcf-protection=full" EXTRA_HARDENING)
+        add_simple_extra_compiler_flag("branch-protection" "-mbranch-protection=standard" EXTRA_HARDENING)
 endif()
 
-set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
-set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
-
-set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
-set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
-
-set(CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
-set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
+foreach(FLAG function-sections data-sections)
+        add_simple_extra_compiler_flag("${FLAG}" "-f${FLAG}" EXTRA_OPT)
+endforeach()
 
-set(CMAKE_C_FLAGS_MINSIZEREL "${CMAKE_C_FLAGS_MINSIZEREL} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
-set(CMAKE_CXX_FLAGS_MINSIZEREL "${CMAKE_CXX_FLAGS_MINSIZEREL} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
+foreach(RELTYP RELEASE DEBUG RELWITHDEBINFO MINSIZEREL)
+        foreach(L C CXX)
+                set(CMAKE_${L}_FLAGS_${RELTYP} "${CMAKE_${L}_FLAGS_${RELTYP}} ${EXTRA_HARDENING_C_FLAGS} ${EXTRA_OPT_C_FLAGS}")
+        endforeach()
+endforeach()
 
 #
 # detect OS