From 7063e112d2eef90b20baf11c7f85d86b8b390a72 Mon Sep 17 00:00:00 2001 From: "Austin S. Hemmelgarn" Date: Fri, 12 Jan 2024 11:25:07 -0500 Subject: 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 * 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 --- .github/dockerfiles/Dockerfile.clang | 3 +- CMakeLists.txt | 126 +++++++++++++++++++---------------- 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 -- cgit v1.2.3