From 953341040ff0016ab234098b49c532a9c8fa2ea4 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 29 Apr 2024 08:50:16 -0700 Subject: [PATCH 1/3] Require c++20 for plugin builds --- BuiltInSpicyAnalyzer.cmake | 2 +- ZeekPluginCommon.cmake | 2 +- ZeekPluginDynamic.cmake | 2 +- ZeekPluginStatic.cmake | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/BuiltInSpicyAnalyzer.cmake b/BuiltInSpicyAnalyzer.cmake index 604993b..0d0284e 100644 --- a/BuiltInSpicyAnalyzer.cmake +++ b/BuiltInSpicyAnalyzer.cmake @@ -92,7 +92,7 @@ function (spicy_add_analyzer) set(lib "spicy_${SPICY_ANALYZER_NAME}") add_library(${lib} OBJECT ${generated_sources} ${cxx_sources}) - target_compile_features(${lib} PRIVATE cxx_std_17) + target_compile_features(${lib} PRIVATE cxx_std_20) set_target_properties(${lib} PROPERTIES CXX_EXTENSIONS OFF) target_include_directories(${lib} PRIVATE ${SPICY_PLUGIN_PATH}/include diff --git a/ZeekPluginCommon.cmake b/ZeekPluginCommon.cmake index 505a4c1..fb85c6d 100644 --- a/ZeekPluginCommon.cmake +++ b/ZeekPluginCommon.cmake @@ -23,7 +23,7 @@ macro (zeek_plugin_begin ns name) set(_plugin_bif_files "") set(_plugin_pac_args "") - target_compile_features(${_plugin_lib} PRIVATE cxx_std_17) + target_compile_features(${_plugin_lib} PRIVATE cxx_std_20) set_target_properties(${_plugin_lib} PROPERTIES CXX_EXTENSIONS OFF) endmacro () diff --git a/ZeekPluginDynamic.cmake b/ZeekPluginDynamic.cmake index 98f93ad..4538809 100644 --- a/ZeekPluginDynamic.cmake +++ b/ZeekPluginDynamic.cmake @@ -29,7 +29,7 @@ function (zeek_add_dynamic_plugin ns name) if (NOT TARGET ${target_name}) add_library(${target_name} MODULE) - target_compile_features(${target_name} PRIVATE cxx_std_17) + target_compile_features(${target_name} PRIVATE cxx_std_20) set_target_properties(${target_name} PROPERTIES CXX_EXTENSIONS OFF) endif () diff --git a/ZeekPluginStatic.cmake b/ZeekPluginStatic.cmake index cb08943..ed2ca3f 100644 --- a/ZeekPluginStatic.cmake +++ b/ZeekPluginStatic.cmake @@ -17,7 +17,7 @@ function (zeek_add_static_plugin ns name) if (NOT TARGET ${target_name}) add_library(${target_name} OBJECT) - target_compile_features(${target_name} PRIVATE cxx_std_17) + target_compile_features(${target_name} PRIVATE cxx_std_20) set_target_properties(${target_name} PROPERTIES CXX_EXTENSIONS OFF) endif () add_dependencies(${target_name} zeek_autogen_files) From 5fa05353f7577809c71b5b0d5e0eab3b321e547a Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 2 May 2024 16:45:11 -0700 Subject: [PATCH 2/3] Rename/rewrite RequireCXXStd to check for C++20 support --- BuiltInSpicyAnalyzer.cmake | 3 +- RequireCXX17.cmake | 87 -------------------------------------- RequireCXXStd.cmake | 39 +++++++++++++++++ ZeekPluginCommon.cmake | 4 +- ZeekPluginDynamic.cmake | 3 +- ZeekPluginStatic.cmake | 3 +- 6 files changed, 48 insertions(+), 91 deletions(-) delete mode 100644 RequireCXX17.cmake create mode 100644 RequireCXXStd.cmake diff --git a/BuiltInSpicyAnalyzer.cmake b/BuiltInSpicyAnalyzer.cmake index 0d0284e..7d69d6e 100644 --- a/BuiltInSpicyAnalyzer.cmake +++ b/BuiltInSpicyAnalyzer.cmake @@ -13,6 +13,7 @@ set(ZEEK_LEGACY_ANALYZERS CACHE INTERNAL "") set(ZEEK_SKIPPED_ANALYZERS CACHE INTERNAL "") +include(RequireCXXStd) # Force Spicy include directories to the front of the include paths. # @@ -92,7 +93,7 @@ function (spicy_add_analyzer) set(lib "spicy_${SPICY_ANALYZER_NAME}") add_library(${lib} OBJECT ${generated_sources} ${cxx_sources}) - target_compile_features(${lib} PRIVATE cxx_std_20) + target_compile_features(${lib} PRIVATE ${ZEEK_CXX_STD}) set_target_properties(${lib} PROPERTIES CXX_EXTENSIONS OFF) target_include_directories(${lib} PRIVATE ${SPICY_PLUGIN_PATH}/include diff --git a/RequireCXX17.cmake b/RequireCXX17.cmake deleted file mode 100644 index 0e44ea9..0000000 --- a/RequireCXX17.cmake +++ /dev/null @@ -1,87 +0,0 @@ -# Detect if compiler version is sufficient for supporting C++17. -# If it is, CMAKE_CXX_FLAGS are modified appropriately and HAVE_CXX17 -# is set to a true value. Else, CMake exits with a fatal error message. -# This currently only works for GCC and Clang compilers. -# In Cmake 3.8+, CMAKE_CXX_STANDARD_REQUIRED should be able to replace -# all the logic below. - -if (DEFINED HAVE_CXX17) - return() -endif () - -set(required_gcc_version 8.0) -set(required_clang_version 9.0) -set(required_msvc_version 19.14) -set(required_apple_clang_version 11.4) - -if (MSVC) - if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS ${required_msvc_version}) - message( - FATAL_ERROR - "MSVC version must be at least " - "${required_gcc_version} for C++17 support, detected: " - "${CMAKE_CXX_COMPILER_VERSION}") - endif () - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /std:c++17") - set(HAVE_CXX17 true) - return() -endif () - -include(CheckCXXSourceCompiles) - -set(cxx17_testcode "#include - int main() { std::optional a; }") - -check_cxx_source_compiles("${cxx17_testcode}" cxx17_already_works) - -if (cxx17_already_works) - # Nothing to do. Flags already select a suitable C++ version. - set(HAVE_CXX17 true) - return() -endif () - -set(cxx17_flag "-std=c++17") - -if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") - if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS ${required_gcc_version}) - message( - FATAL_ERROR - "GCC version must be at least " - "${required_gcc_version} for C++17 support, detected: " - "${CMAKE_CXX_COMPILER_VERSION}") - endif () -elseif (CMAKE_CXX_COMPILER_ID STREQUAL "Clang") - if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS ${required_clang_version}) - message( - FATAL_ERROR - "Clang version must be at least " - "${required_clang_version} for C++17 support, detected: " - "${CMAKE_CXX_COMPILER_VERSION}") - endif () - if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 5) - set(cxx17_flag "-std=c++1z") - endif () -elseif (CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang") - if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS ${required_apple_clang_version}) - message( - FATAL_ERROR - "Apple Clang version must be at least " - "${required_apple_clang_version} for C++17 support, detected: " - "${CMAKE_CXX_COMPILER_VERSION}") - endif () -else () - # Unrecognized compiler: fine to be permissive of other compilers as long - # as they are able to support C++17 and can compile the test program, but - # we just won't be able to give specific advice on what compiler version a - # user needs in the case it actually doesn't support C++17. -endif () - -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${cxx17_flag}") - -check_cxx_source_compiles("${cxx17_testcode}" cxx17_works) - -if (NOT cxx17_works) - message(FATAL_ERROR "failed using C++17 for compilation") -endif () - -set(HAVE_CXX17 true) diff --git a/RequireCXXStd.cmake b/RequireCXXStd.cmake new file mode 100644 index 0000000..a2373bf --- /dev/null +++ b/RequireCXXStd.cmake @@ -0,0 +1,39 @@ +# Check for a specific C++ standard level, and require the compiler to +# support that via some CMake settings. + +if (DEFINED ZEEK_CXX_STD) + return() +endif () + +# Require a specific C++ standard and set the proper flag when creating targets. +set(CMAKE_CXX_STANDARD_REQUIRED ON) +set(CMAKE_CXX_STANDARD 20) + +# Disable using extensions provided by various compilers. Notably this keeps us +# setting it to c++20 instead of gnu++20 with GCC. +set(CMAKE_CXX_EXTENSIONS OFF) + +set(_old_cmake_required_flags "${CMAKE_REQUIRED_FLAGS}") +if (MSVC) + set(CMAKE_REQUIRED_FLAGS "/std:c++20") +else () + set(CMAKE_REQUIRED_FLAGS "-std=c++20") +endif () + +include(CheckCXXSourceCompiles) + +# The header is a good baseline version of C++20 support for us +# since we can use it to determine support for various features in other +# places. +set(cxx_std_testcode "#include + int main() { }") + +check_cxx_source_compiles("${cxx_std_testcode}" cxx_std_works) + +set(CMAKE_REQUIRED_FLAGS "${_old_cmake_required_flags}") + +if (cxx_std_works) + set(ZEEK_CXX_STD cxx_std_20) +else () + message(FATAL_ERROR "failed using C++20 for compilation") +endif () diff --git a/ZeekPluginCommon.cmake b/ZeekPluginCommon.cmake index fb85c6d..754e83f 100644 --- a/ZeekPluginCommon.cmake +++ b/ZeekPluginCommon.cmake @@ -3,6 +3,8 @@ ## This set is used by both static and dynamic plugins via ## ZeekPluginStatic and ZeekPluginDynamic, respectively. +include(RequireCXXStd) + # Begins a plugin definition, giving its namespace and name as the arguments. # The DISABLE_CPP_TESTS option disables unit test support. When not provided, # unit-testing is enabled when Zeek supports it, and disabled otherwise. @@ -23,7 +25,7 @@ macro (zeek_plugin_begin ns name) set(_plugin_bif_files "") set(_plugin_pac_args "") - target_compile_features(${_plugin_lib} PRIVATE cxx_std_20) + target_compile_features(${_plugin_lib} PRIVATE ${ZEEK_CXX_STD}) set_target_properties(${_plugin_lib} PROPERTIES CXX_EXTENSIONS OFF) endmacro () diff --git a/ZeekPluginDynamic.cmake b/ZeekPluginDynamic.cmake index 4538809..108fffe 100644 --- a/ZeekPluginDynamic.cmake +++ b/ZeekPluginDynamic.cmake @@ -1,4 +1,5 @@ include(GetArchitecture) +include(RequireCXXStd) # Sets `target` to contain the CMake target name for a dynamic plugin. macro (zeek_get_dynamic_plugin_target target ns name) @@ -29,7 +30,7 @@ function (zeek_add_dynamic_plugin ns name) if (NOT TARGET ${target_name}) add_library(${target_name} MODULE) - target_compile_features(${target_name} PRIVATE cxx_std_20) + target_compile_features(${target_name} PRIVATE ${ZEEK_CXX_STD}) set_target_properties(${target_name} PROPERTIES CXX_EXTENSIONS OFF) endif () diff --git a/ZeekPluginStatic.cmake b/ZeekPluginStatic.cmake index ed2ca3f..6ae07a8 100644 --- a/ZeekPluginStatic.cmake +++ b/ZeekPluginStatic.cmake @@ -1,5 +1,6 @@ include(BifCl) include(BinPAC) +include(RequireCXXStd) # Sets `target` to contain the CMake target name for a static plugin. macro (zeek_get_static_plugin_target target ns name) @@ -17,7 +18,7 @@ function (zeek_add_static_plugin ns name) if (NOT TARGET ${target_name}) add_library(${target_name} OBJECT) - target_compile_features(${target_name} PRIVATE cxx_std_20) + target_compile_features(${target_name} PRIVATE ${ZEEK_CXX_STD}) set_target_properties(${target_name} PROPERTIES CXX_EXTENSIONS OFF) endif () add_dependencies(${target_name} zeek_autogen_files) From 53a2d51b256399813c18e07b47eeb7a73bd03eb7 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 5 Jun 2025 15:08:30 -0700 Subject: [PATCH 3/3] Avoid reporting errors under -Werror for a couple of known GCC bugs. These both are more prevalent when building with -std=c++20, but have existed in GCC for a while. --- ZeekPluginStatic.cmake | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ZeekPluginStatic.cmake b/ZeekPluginStatic.cmake index 6ae07a8..8acf520 100644 --- a/ZeekPluginStatic.cmake +++ b/ZeekPluginStatic.cmake @@ -59,6 +59,19 @@ function (zeek_add_static_plugin ns name) #set(WERROR_FLAG "/WX") else () set(WERROR_FLAG "-Werror") + + # With versions >=13.0 GCC gained `-Warray-bounds` which reports false + # positives, see e.g., https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111273. + if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 13.0) + list(APPEND WERROR_FLAG "-Wno-error=array-bounds") + endif () + + # With versions >=11.0 GCC is retruning false positives for -Wrestrict. See + # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100366. It's more prevalent + # building with -std=c++20. + if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 11.0) + list(APPEND WERROR_FLAG "-Wno-error=restrict") + endif () endif () endif ()