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>
Austin S. Hemmelgarn committed
Jan 12, 2024 at 11:25 UTC
7063e112d2eef90b20baf11c7f85d86b8b390a72
2 files changed
+71
-58
.github/dockerfiles/Dockerfile.clang
+2
-1
@@ -9,7 +9,8 @@ RUN /tmp/install-required-packages.sh --dont-wait --non-interactive netdata-all
9
10
# Install Clang and set as default CC
11
RUN apt-get install -y clang && \
12
- update-alternatives --install /usr/bin/cc cc /usr/bin/clang 100
12
+ update-alternatives --install /usr/bin/cc cc /usr/bin/clang 100 && \
13
+ update-alternatives --install /usr/bin/c++ c++ /usr/bin/clang++ 100
14
15
WORKDIR /netdata
16
COPY . .
CMakeLists.txt
+69
-57
@@ -119,91 +119,103 @@ option(ENABLE_LOGS_MANAGEMENT_TESTS "enable logs management tests" True)
119
#
120
121
include(CheckCCompilerFlag)
122
+include(CheckCXXCompilerFlag)
123
124
# Disable hardening for debug builds by default.
125
if(CMAKE_BUILD_TYPE STREQUAL "Debug")
126
option(DISABLE_HARDENING "disable adding extra compiler flags for hardening" TRUE)
127
else()
127
- # FIXME: Until https://github.com/netdata/netdata/pull/16731 resolves the issue
128
- option(DISABLE_HARDENING "disable adding extra compiler flags for hardening" TRUE)
128
+ option(DISABLE_HARDENING "disable adding extra compiler flags for hardening" FALSE)
129
endif()
130
131
-set(EXTRA_HARDENING_FLAGS "")
131
+# Construct a pre-processor safe name
132
+function(make_cpp_safe_name value target)
133
+ string(REPLACE "-" "_" tmp "${value}")
134
+ string(REPLACE "=" "_" tmp "${tmp}")
135
+ set(${target} "${tmp}" PARENT_SCOPE)
136
+endfunction()
137
133
-if(NOT ${DISABLE_HARDENING})
134
- if(NOT ${CMAKE_C_FLAGS} MATCHES "stack-protector")
135
- check_c_compiler_flag("-fstack-protector-strong" HAVE_STACK_PROTECTOR_STRONG_FLAG)
136
- if(HAVE_STACK_PROTECTOR_STRONG_FLAG)
137
- set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -fstack-protector-strong")
138
- else()
139
- check_c_compiler_flag("-fstack-protector" HAVE_STACK_PROTECTOR)
140
- if(HAVE_STACK_PROTECTOR)
141
- set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -fstack-protector")
142
- endif()
143
- endif()
144
- endif()
138
+# Conditionally add an extra compiler flag to C and C++ flags.
139
+#
140
+# If the language flags already match the `match` argument, skip this flag.
141
+# Otherwise, check for support for `flag` and if support is found, add it to
142
+# the language-specific `target` flag group.
143
+function(add_simple_extra_compiler_flag match flag target)
144
+ set(CMAKE_REQUIRED_FLAGS "-Werror")
145
+
146
+ make_cpp_safe_name("${flag}" flag_name)
147
146
- if(NOT ${CMAKE_C_FLAGS} MATCHES "stack-clash-protection")
147
- check_c_compiler_flag("-fstack-clash-protection" HAVE_STACK_CLASH_FLAG)
148
- if(HAVE_STACK_CLASH_FLAG)
149
- set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -fstack-clash-protection")
148
+ if(NOT ${CMAKE_C_FLAGS} MATCHES ${match})
149
+ check_c_compiler_flag("${flag}" HAVE_C_${flag_name})
150
+ if(HAVE_C_${flag_name})
151
+ set(${target}_C_FLAGS "${${target}_C_FLAGS} ${flag}" PARENT_SCOPE)
152
endif()
153
endif()
154
153
- if(NOT ${CMAKE_C_FLAGS} MATCHES "-fcf-protection")
154
- check_c_compiler_flag("-fcf-protection=full" HAVE_CFI_FLAG)
155
- if(HAVE_CFI_FLAG)
156
- set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -fcf-protection=full")
155
+ if(NOT ${CMAKE_CXX_FLAGS} MATCHES ${match})
156
+ check_cxx_compiler_flag("${flag}" HAVE_CXX_${flag_name})
157
+ if(HAVE_CXX_${flag_name})
158
+ set(${target}_CXX_FLAGS "${${target}_CXX_FLAGS} ${flag}" PARENT_SCOPE)
159
endif()
160
endif()
161
+endfunction()
162
160
- if(NOT ${CMAKE_C_FLAGS} MATCHES "branch-protection")
161
- check_c_compiler_flag("-mbranch-protection=standard" HAVE_BRANCH_PROT_FLAG)
162
- if(HAVE_BRANCH_PROT_FLAG)
163
- set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -mbranch-protection=standard")
163
+# Same as add_simple_extra_compiler_flag, but check for a second flag if the
164
+# first one is unsupported.
165
+function(add_double_extra_compiler_flag match flag1 flag2 target)
166
+ set(CMAKE_REQUIRED_FLAGS "-Werror")
167
+
168
+ make_cpp_safe_name("${flag1}" flag1_name)
169
+ make_cpp_safe_name("${flag2}" flag2_name)
170
+
171
+ if(NOT ${CMAKE_C_FLAGS} MATCHES ${match})
172
+ check_c_compiler_flag("${flag1}" HAVE_C_${flag1_name})
173
+ if(HAVE_C_${flag1_name})
174
+ set(${target}_C_FLAGS "${${target}_C_FLAGS} ${flag1}" PARENT_SCOPE)
175
+ else()
176
+ check_c_compiler_flag("${flag2}" HAVE_C_${flag2_name})
177
+ if(HAVE_C_${flag2_name})
178
+ set(${target}_C_FLAGS "${${target}_C_FLAGS} ${flag2}" PARENT_SCOPE)
179
+ endif()
180
endif()
181
endif()
182
167
- if(NOT ${CMAKE_C_FLAGS} MATCHES "_FORTIFY_SOURCE")
168
- check_c_compiler_flag("-D_FORTIFY_SOURCE=3" HAVE_FORTIFY_SOURCE_3)
169
- if(HAVE_FORTIFY_SOURCE_3)
170
- set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -D_FRTIFY_SOURCE=3")
183
+ if(NOT ${CMAKE_CXX_FLAGS} MATCHES ${match})
184
+ check_cxx_compiler_flag("${flag1}" HAVE_CXX_${flag1_name})
185
+ if(HAVE_CXX_${flag1_name})
186
+ set(${target}_CXX_FLAGS "${${target}_CXX_FLAGS} ${flag1}" PARENT_SCOPE)
187
else()
172
- check_c_compiler_flag("-D_FORTIFY_SOURCE=2" HAVE_FORTIFY_SOURCE_2)
173
- if(HAVE_FORTIFY_SOURCE_2)
174
- set(EXTRA_HARDENING_FLAGS "${EXTRA_HARDENING_FLAGS} -D_FRTIFY_SOURCE=2")
188
+ check_cxx_compiler_flag("${flag2}" HAVE_CXX_${flag2_name})
189
+ if(HAVE_CXX_${flag2_name})
190
+ set(${target}_CXX_FLAGS "${${target}_CXX_FLAGS} ${flag2}" PARENT_SCOPE)
191
endif()
192
endif()
193
endif()
178
-endif()
194
+endfunction()
195
180
-set(EXTRA_OPT_FLAGS "")
196
+set(EXTRA_HARDENING_C_FLAGS "")
197
+set(EXTRA_HARDENING_CXX_FLAGS "")
198
182
-if(NOT ${CMAKE_C_FLAGS} MATCHES "function-sections")
183
- check_c_compiler_flag("-ffunction-sections" HAVE_FUNCTION_SECTIONS)
184
- if(HAVE_FUNCTION_SECTIONS)
185
- set(EXTRA_OPT_FLAGS "${EXTRA_OPT_FLAGS} -ffunction-sections")
186
- endif()
187
-endif()
199
+set(EXTRA_OPT_C_FLAGS "")
200
+set(EXTRA_OPT_CXX_FLAGS "")
201
189
-if(NOT ${CMAKE_C_FLAGS} MATCHES "data-sections")
190
- check_c_compiler_flag("-fdata-sections" HAVE_DATA_SECTIONS)
191
- if(HAVE_DATA_SECTIONS)
192
- set(EXTRA_OPT_FLAGS "${EXTRA_OPT_FLAGS} -fdata-sections")
193
- endif()
202
+if(NOT ${DISABLE_HARDENING})
203
+ add_double_extra_compiler_flag("stack-protector" "-fstack-protector-strong" "-fstack-protector" EXTRA_HARDENING)
204
+ add_double_extra_compiler_flag("_FORTIFY_SOURCE" "-D_FORTIFY_SOURCE=3" "-D_FORTIFY_SOURCE=2" EXTRA_HARDENING)
205
+ add_simple_extra_compiler_flag("stack-clash-protection" "-fstack-clash-protection" EXTRA_HARDENING)
206
+ add_simple_extra_compiler_flag("-fcf-protection" "-fcf-protection=full" EXTRA_HARDENING)
207
+ add_simple_extra_compiler_flag("branch-protection" "-mbranch-protection=standard" EXTRA_HARDENING)
208
endif()
209
196
-set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
197
-set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
198
-
199
-set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
200
-set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
201
-
202
-set(CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
203
-set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
210
+foreach(FLAG function-sections data-sections)
211
+ add_simple_extra_compiler_flag("${FLAG}" "-f${FLAG}" EXTRA_OPT)
212
+endforeach()
213
205
-set(CMAKE_C_FLAGS_MINSIZEREL "${CMAKE_C_FLAGS_MINSIZEREL} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
206
-set(CMAKE_CXX_FLAGS_MINSIZEREL "${CMAKE_CXX_FLAGS_MINSIZEREL} ${EXTRA_HARDENING_FLAGS} ${EXTRA_OPT_FLAGS}")
214
+foreach(RELTYP RELEASE DEBUG RELWITHDEBINFO MINSIZEREL)
215
+ foreach(L C CXX)
216
+ set(CMAKE_${L}_FLAGS_${RELTYP} "${CMAKE_${L}_FLAGS_${RELTYP}} ${EXTRA_HARDENING_C_FLAGS} ${EXTRA_OPT_C_FLAGS}")
217
+ endforeach()
218
+endforeach()
219
220
#
221
# detect OS