From bbcc4b00fbfcae78db1b86a0d2d0caf58206f882 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Fri, 15 Dec 2023 14:35:53 -0700 Subject: [PATCH 1/9] Set -Werror / /WX via target_compile_options to force warnings as errors --- .cirrus.yml | 14 +++++++------- CMakeLists.txt | 12 ++++++++++++ cmake | 2 +- configure | 4 ++++ src/CMakeLists.txt | 11 +++++++++++ 5 files changed, 35 insertions(+), 8 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index c5a0280a78..a1c6e2203f 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -10,13 +10,13 @@ btest_jobs: &BTEST_JOBS 4 btest_retries: &BTEST_RETRIES 2 memory: &MEMORY 16GB -config: &CONFIG --build-type=release --disable-broker-tests --prefix=$CIRRUS_WORKING_DIR/install --ccache -no_spicy_config: &NO_SPICY_CONFIG --build-type=release --disable-broker-tests --disable-spicy --prefix=$CIRRUS_WORKING_DIR/install --ccache -static_config: &STATIC_CONFIG --build-type=release --disable-broker-tests --enable-static-broker --enable-static-binpac --prefix=$CIRRUS_WORKING_DIR/install --ccache -asan_sanitizer_config: &ASAN_SANITIZER_CONFIG --build-type=debug --disable-broker-tests --sanitizers=address --enable-fuzzers --enable-coverage --disable-spicy --ccache -ubsan_sanitizer_config: &UBSAN_SANITIZER_CONFIG --build-type=debug --disable-broker-tests --sanitizers=undefined --enable-fuzzers --disable-spicy --ccache -tsan_sanitizer_config: &TSAN_SANITIZER_CONFIG --build-type=debug --disable-broker-tests --sanitizers=thread --enable-fuzzers --disable-spicy --ccache -openssl30_config: &OPENSSL30_CONFIG --build-type=release --disable-broker-tests --with-openssl=/opt/openssl --prefix=$CIRRUS_WORKING_DIR/install --ccache +config: &CONFIG --build-type=release --disable-broker-tests --prefix=$CIRRUS_WORKING_DIR/install --ccache --enable-werror +no_spicy_config: &NO_SPICY_CONFIG --build-type=release --disable-broker-tests --disable-spicy --prefix=$CIRRUS_WORKING_DIR/install --ccache --enable-werror +static_config: &STATIC_CONFIG --build-type=release --disable-broker-tests --enable-static-broker --enable-static-binpac --prefix=$CIRRUS_WORKING_DIR/install --ccache --enable-werror +asan_sanitizer_config: &ASAN_SANITIZER_CONFIG --build-type=debug --disable-broker-tests --sanitizers=address --enable-fuzzers --enable-coverage --disable-spicy --ccache --enable-werror +ubsan_sanitizer_config: &UBSAN_SANITIZER_CONFIG --build-type=debug --disable-broker-tests --sanitizers=undefined --enable-fuzzers --disable-spicy --ccache --enable-werror +tsan_sanitizer_config: &TSAN_SANITIZER_CONFIG --build-type=debug --disable-broker-tests --sanitizers=thread --enable-fuzzers --disable-spicy --ccache --enable-werror +openssl30_config: &OPENSSL30_CONFIG --build-type=release --disable-broker-tests --with-openssl=/opt/openssl --prefix=$CIRRUS_WORKING_DIR/install --ccache --enable-werror resources_template: &RESOURCES_TEMPLATE cpu: *CPUS diff --git a/CMakeLists.txt b/CMakeLists.txt index 94119d4c96..eeffce462e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -165,8 +165,18 @@ if (MSVC) # Disable Spicy as it is not yet supported in Windows. set(DISABLE_SPICY true) + + if (BUILD_WITH_WERROR) + # TODO: This is disabled for now because there a bunch of known + # compiler warnings on Windows that we don't have good fixes for. + #set(WERROR_FLAG "/WX") + #set(WERROR_FLAG "/WX") + endif () else () include(GNUInstallDirs) + if (BUILD_WITH_WERROR) + set(WERROR_FLAG "-Werror") + endif () endif () include(cmake/CommonCMakeConfig.cmake) @@ -254,6 +264,7 @@ endif () foreach (name zeek_exe zeek_lib zeek_fuzzer_shared) if (TARGET ${name}) target_compile_definitions(${name} PRIVATE ZEEK_CONFIG_SKIP_VERSION_H) + target_compile_options(${name} PRIVATE ${WERROR_FLAG}) endif () endforeach () @@ -347,6 +358,7 @@ function (zeek_add_subdir_library name) add_dependencies(${target_name} zeek_autogen_files) target_link_libraries(${target_name} PRIVATE $) add_clang_tidy_files(${FN_ARGS_SOURCES}) + target_compile_options(${target_name} PRIVATE ${WERROR_FLAG}) # Take care of compiling BIFs. if (FN_ARGS_BIFS) diff --git a/cmake b/cmake index 507d120121..01fcb68300 160000 --- a/cmake +++ b/cmake @@ -1 +1 @@ -Subproject commit 507d1201213a7b308298e0c5d6ac0c9f870e2bb8 +Subproject commit 01fcb683005e3c3a71ae867aa983b772a77e32d1 diff --git a/configure b/configure index e74d38280e..18f019b29c 100755 --- a/configure +++ b/configure @@ -64,6 +64,7 @@ Usage: $0 [OPTION]... [VAR=VALUE]... --enable-perftools-debug use Google's perftools for debugging --enable-static-binpac build binpac statically (ignored if --with-binpac is specified) --enable-static-broker build Broker statically (ignored if --with-broker is specified) + --enable-werror build with -Werror --disable-af-packet don't include native AF_PACKET support (Linux only) --disable-archiver don't build or install zeek-archiver tool --disable-auxtools don't build or install auxiliary tools @@ -275,6 +276,9 @@ while [ $# -ne 0 ]; do --enable-static-broker) append_cache_entry BUILD_STATIC_BROKER BOOL true ;; + --enable-werror) + append_cache_entry BUILD_WITH_WERROR BOOL true + ;; --disable-af-packet) append_cache_entry DISABLE_AF_PACKET BOOL true ;; diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 27f0834715..4101e97be1 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -41,8 +41,16 @@ set(BISON_FLAGS "--debug") if (MSVC) set(SIGN_COMPARE_FLAG "/wd4018") + if (BUILD_WITH_WERROR) + # TODO: This is disabled for now because there a bunch of known + # compiler warnings on Windows that we don't have good fixes for. + #set(WERROR_FLAG "/WX") + endif () else () set(SIGN_COMPARE_FLAG "-Wno-sign-compare") + if (BUILD_WITH_WERROR) + set(WERROR_FLAG "-Werror") + endif () endif () # Rule parser/scanner @@ -517,6 +525,7 @@ collect_headers(zeek_HEADERS ${zeek_SRCS}) add_library(zeek_objs OBJECT ${zeek_SRCS}) target_compile_features(zeek_objs PRIVATE ${ZEEK_CXX_STD}) +target_compile_options(zeek_objs PRIVATE ${WERROR_FLAG}) set_target_properties(zeek_objs PROPERTIES CXX_EXTENSIONS OFF) target_link_libraries(zeek_objs PRIVATE $) target_compile_definitions(zeek_objs PRIVATE ZEEK_CONFIG_SKIP_VERSION_H) @@ -530,6 +539,7 @@ endif () if (TARGET zeek_exe) target_sources(zeek_exe PRIVATE main.cc ${zeek_HEADERS}) + target_compile_options(zeek_exe PRIVATE ${WERROR_FLAG}) # npcap/winpcap need to be loaded in delayed mode so that we can set the load # path correctly at runtime. See @@ -553,6 +563,7 @@ endif () if (TARGET zeek_lib) target_sources(zeek_lib PRIVATE ${zeek_HEADERS}) + target_compile_options(zeek_lib PRIVATE ${WERROR_FLAG}) target_link_libraries(zeek_lib PUBLIC ${zeekdeps} ${CMAKE_THREAD_LIBS_INIT} ${CMAKE_DL_LIBS}) endif () From b639f1426fc1ce55f4cf6c2b693d567437d68917 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Wed, 3 Jan 2024 14:35:41 -0700 Subject: [PATCH 2/9] Fix warning with attribute string lookup --- src/Attr.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Attr.cc b/src/Attr.cc index f3a5ab6b9e..e080bfcfd6 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -45,7 +45,10 @@ const char* attr_name(AttrTag t) { }; // clang-format on - return attr_names[int(t)]; + if ( int(t) >= 0 && int(t) < NUM_ATTRS ) + return attr_names[int(t)]; + else + return ""; } Attr::Attr(AttrTag t, ExprPtr e) : expr(std::move(e)) { From 3d5aaf9aec78a245049942585ae36c124af6921b Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 4 Jan 2024 12:44:46 -0700 Subject: [PATCH 3/9] Update src/3rdparty submodule to fix sprintf warning in modp --- src/3rdparty | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/3rdparty b/src/3rdparty index c2763e952e..06d7496bfa 160000 --- a/src/3rdparty +++ b/src/3rdparty @@ -1 +1 @@ -Subproject commit c2763e952ea899f86bec2b60f840d38861cefd03 +Subproject commit 06d7496bfadf333e121409604eb55460f09cfcae From d87e2ec70c76bbd3c55283644bb080fd807feb6a Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 4 Jan 2024 12:44:58 -0700 Subject: [PATCH 4/9] Avoid unused-result warning in Supervisor --- src/supervisor/Supervisor.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/supervisor/Supervisor.cc b/src/supervisor/Supervisor.cc index 9921c9b15d..a176f033e3 100644 --- a/src/supervisor/Supervisor.cc +++ b/src/supervisor/Supervisor.cc @@ -42,7 +42,7 @@ extern "C" { #ifdef DEBUG #define DBG_STEM(...) stem->LogDebug(__VA_ARGS__); #else -#define DBG_STEM +#define DBG_STEM(...) #endif using namespace zeek; From ac59b11f33539dae18aa79b68349e2c9f38b76bf Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 4 Jan 2024 13:14:51 -0700 Subject: [PATCH 5/9] Bump zeekjs to pick up dprintf warning fix --- auxil/zeekjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auxil/zeekjs b/auxil/zeekjs index 867e90b47f..70d88f4b10 160000 --- a/auxil/zeekjs +++ b/auxil/zeekjs @@ -1 +1 @@ -Subproject commit 867e90b47f657b92d03808af0475bfd34cc9bcef +Subproject commit 70d88f4b10e36f8715eb0f2a25eb0fc0eb4fce37 From ca29793eccc4ad3ace1b359bf482070820615cf9 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 4 Jan 2024 13:44:11 -0700 Subject: [PATCH 6/9] ZAM: Create ListValPtr directly instead of a stack object --- src/script_opt/ZAM/Ops.in | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/script_opt/ZAM/Ops.in b/src/script_opt/ZAM/Ops.in index bda9bd91dd..66bb15c8fb 100644 --- a/src/script_opt/ZAM/Ops.in +++ b/src/script_opt/ZAM/Ops.in @@ -770,10 +770,9 @@ eval auto op1 = frame[z.v1].ToVal(z.t); # the main instruction type, as always. macro EvalVal2InTableCore(op1, op2) - ListVal lv(TYPE_ANY); - lv.Append(op1); - lv.Append(op2); - ListValPtr lvp = {NewRef{}, &lv}; + auto lvp = zeek::make_intrusive(TYPE_ANY); + lvp->Append(op1); + lvp->Append(op2); macro EvalVal2InTableAssignCore(slot) frame[z.v1].int_val = frame[z.slot].table_val->Find(std::move(lvp)) != nullptr; From 016121b6f75f9aaa0028e411d3b397382cd3a73b Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 4 Jan 2024 13:59:56 -0700 Subject: [PATCH 7/9] Use instead of --- src/DNS_Mgr.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DNS_Mgr.cc b/src/DNS_Mgr.cc index 53727290ae..a707d8ead0 100644 --- a/src/DNS_Mgr.cc +++ b/src/DNS_Mgr.cc @@ -6,7 +6,7 @@ #include #include -#include +#include #include #include #include From df65b668b750d4f025b37b794e8efef4bd51dd92 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 4 Jan 2024 16:45:13 -0700 Subject: [PATCH 8/9] CPP-gen: Don't emit extra braces if only one element --- src/script_opt/CPP/InitsInfo.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/script_opt/CPP/InitsInfo.cc b/src/script_opt/CPP/InitsInfo.cc index 3dc3536edc..c8150dbe96 100644 --- a/src/script_opt/CPP/InitsInfo.cc +++ b/src/script_opt/CPP/InitsInfo.cc @@ -47,9 +47,13 @@ void CPP_InitsInfo::GenerateInitializers(CPPCompile* c) { if ( ++n > 1 ) c->Emit(""); - c->Emit("{"); - BuildCohort(c, cohort); - c->Emit("},"); + if ( cohort.size() == 1 ) + BuildCohort(c, cohort); + else { + c->Emit("{"); + BuildCohort(c, cohort); + c->Emit("},"); + } } c->Emit("}"); From 652ba502aab843574402ec53aa0a6561b21253f3 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Fri, 5 Jan 2024 10:12:54 -0700 Subject: [PATCH 9/9] CI: Remove unused openssl30_config --- .cirrus.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index a1c6e2203f..036c10e176 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -16,7 +16,6 @@ static_config: &STATIC_CONFIG --build-type=release --disable-broker-tests --enab asan_sanitizer_config: &ASAN_SANITIZER_CONFIG --build-type=debug --disable-broker-tests --sanitizers=address --enable-fuzzers --enable-coverage --disable-spicy --ccache --enable-werror ubsan_sanitizer_config: &UBSAN_SANITIZER_CONFIG --build-type=debug --disable-broker-tests --sanitizers=undefined --enable-fuzzers --disable-spicy --ccache --enable-werror tsan_sanitizer_config: &TSAN_SANITIZER_CONFIG --build-type=debug --disable-broker-tests --sanitizers=thread --enable-fuzzers --disable-spicy --ccache --enable-werror -openssl30_config: &OPENSSL30_CONFIG --build-type=release --disable-broker-tests --with-openssl=/opt/openssl --prefix=$CIRRUS_WORKING_DIR/install --ccache --enable-werror resources_template: &RESOURCES_TEMPLATE cpu: *CPUS