From 2893eea0456dd47bb930dd5e25a726eb4c5c24c2 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 5 Jun 2020 17:57:42 -0700 Subject: [PATCH 1/2] Improve Func.h inclusion Now forward declares some Broker types since Broker/CAF headers generally slow things down and also Coverity Scan currently has a catastrophic error on some CAF headers. Also a few other changes to EventHandler/BifReturnVal to reduce number of places that depend on Func.h. --- src/BifReturnVal.cc | 11 ++++++++ src/BifReturnVal.h | 28 ++++++++++++++++++++ src/CMakeLists.txt | 1 + src/CompHash.cc | 1 + src/EventHandler.cc | 6 +++++ src/EventHandler.h | 9 +++---- src/EventRegistry.cc | 1 + src/Func.cc | 7 ----- src/Func.h | 35 ++++++++----------------- src/bro-bif.h | 1 + src/file_analysis/analyzer/x509/X509.cc | 2 ++ 11 files changed, 66 insertions(+), 36 deletions(-) create mode 100644 src/BifReturnVal.cc create mode 100644 src/BifReturnVal.h diff --git a/src/BifReturnVal.cc b/src/BifReturnVal.cc new file mode 100644 index 0000000000..8a8444161b --- /dev/null +++ b/src/BifReturnVal.cc @@ -0,0 +1,11 @@ +// See the file "COPYING" in the main distribution directory for copyright. + +#include "BifReturnVal.h" +#include "Val.h" + +BifReturnVal::BifReturnVal(std::nullptr_t) noexcept + {} + +BifReturnVal::BifReturnVal(Val* v) noexcept + : rval(AdoptRef{}, v) + {} diff --git a/src/BifReturnVal.h b/src/BifReturnVal.h new file mode 100644 index 0000000000..2f6293cdf8 --- /dev/null +++ b/src/BifReturnVal.h @@ -0,0 +1,28 @@ +// See the file "COPYING" in the main distribution directory for copyright. + +#pragma once + +#include "IntrusivePtr.h" + +class Val; + +/** + * A simple wrapper class to use for the return value of BIFs so that + * they may return either a Val* or IntrusivePtr (the former could + * potentially be deprecated). + */ +class BifReturnVal { +public: + + template + BifReturnVal(IntrusivePtr v) noexcept + : rval(AdoptRef{}, v.release()) + { } + + BifReturnVal(std::nullptr_t) noexcept; + + [[deprecated("Remove in v4.1. Return an IntrusivePtr instead.")]] + BifReturnVal(Val* v) noexcept; + + IntrusivePtr rval; +}; diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index e8ccafb544..d19badde10 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -218,6 +218,7 @@ set(MAIN_SRCS Anon.cc Attr.cc Base64.cc + BifReturnVal.cc Brofiler.cc BroString.cc CCL.cc diff --git a/src/CompHash.cc b/src/CompHash.cc index e8d2c5a681..5243e4241e 100644 --- a/src/CompHash.cc +++ b/src/CompHash.cc @@ -2,6 +2,7 @@ #include "zeek-config.h" +#include #include #include diff --git a/src/EventHandler.cc b/src/EventHandler.cc index 68673801d1..890bda3a4d 100644 --- a/src/EventHandler.cc +++ b/src/EventHandler.cc @@ -44,6 +44,12 @@ const IntrusivePtr& EventHandler::GetType(bool check_export) return type; } +void EventHandler::SetFunc(IntrusivePtr f) + { local = std::move(f); } + +void EventHandler::SetLocalHandler(Func* f) + { SetFunc({NewRef{}, f}); } + void EventHandler::Call(zeek::Args* vl, bool no_remote) { #ifdef PROFILE_BRO_FUNCTIONS diff --git a/src/EventHandler.h b/src/EventHandler.h index c2a053cac8..056165271b 100644 --- a/src/EventHandler.h +++ b/src/EventHandler.h @@ -5,11 +5,12 @@ #include "BroList.h" #include "ZeekArgs.h" #include "Type.h" -#include "Func.h" #include #include +class Func; + class EventHandler { public: explicit EventHandler(std::string name); @@ -28,12 +29,10 @@ public: FuncType* FType(bool check_export = true) { return GetType().get(); } - void SetFunc(IntrusivePtr f) - { local = std::move(f); } + void SetFunc(IntrusivePtr f); [[deprecated("Remove in v4.1. Use SetFunc().")]] - void SetLocalHandler(Func* f) - { SetFunc({NewRef{}, f}); } + void SetLocalHandler(Func* f); void AutoPublish(std::string topic) { diff --git a/src/EventRegistry.cc b/src/EventRegistry.cc index 7cf05a7042..29c06dc26f 100644 --- a/src/EventRegistry.cc +++ b/src/EventRegistry.cc @@ -1,5 +1,6 @@ #include "EventRegistry.h" #include "EventHandler.h" +#include "Func.h" #include "RE.h" #include "Reporter.h" diff --git a/src/Func.cc b/src/Func.cc index f0cb8e693a..38ec25b137 100644 --- a/src/Func.cc +++ b/src/Func.cc @@ -892,10 +892,3 @@ function_ingredients::function_ingredients(IntrusivePtr scope, IntrusiveP priority = (attrs ? get_func_priority(*attrs) : 0); this->body = std::move(body); } - -BifReturnVal::BifReturnVal(std::nullptr_t) noexcept - { } - -BifReturnVal::BifReturnVal(Val* v) noexcept - : rval(AdoptRef{}, v) - { } diff --git a/src/Func.h b/src/Func.h index 5b2ee5eebc..c8a284e3bd 100644 --- a/src/Func.h +++ b/src/Func.h @@ -9,15 +9,13 @@ #include #include -#include -#include - #include "BroList.h" #include "Obj.h" #include "IntrusivePtr.h" #include "Type.h" /* for function_flavor */ #include "TraverseTypes.h" #include "ZeekArgs.h" +#include "BifReturnVal.h" class Val; class ListExpr; @@ -28,6 +26,16 @@ class ID; class CallExpr; class Scope; +namespace caf { +template class expected; +} + +namespace broker { +class data; +using vector = std::vector; +using caf::expected; +} + class Func : public BroObj { public: static inline const IntrusivePtr nil; @@ -205,27 +213,6 @@ private: bool weak_closure_ref = false; }; -/** - * A simple wrapper class to use for the return value of BIFs so that - * they may return either a Val* or IntrusivePtr (the former could - * potentially be deprecated). - */ -class BifReturnVal { -public: - - template - BifReturnVal(IntrusivePtr v) noexcept - : rval(AdoptRef{}, v.release()) - { } - - BifReturnVal(std::nullptr_t) noexcept; - - [[deprecated("Remove in v4.1. Return an IntrusivePtr instead.")]] - BifReturnVal(Val* v) noexcept; - - IntrusivePtr rval; -}; - using built_in_func = BifReturnVal (*)(Frame* frame, const zeek::Args* args); class BuiltinFunc final : public Func { diff --git a/src/bro-bif.h b/src/bro-bif.h index b72d676312..7af9c1a6d3 100644 --- a/src/bro-bif.h +++ b/src/bro-bif.h @@ -7,3 +7,4 @@ #include "Reporter.h" #include "ID.h" #include "EventRegistry.h" +#include "BifReturnVal.h" diff --git a/src/file_analysis/analyzer/x509/X509.cc b/src/file_analysis/analyzer/x509/X509.cc index 9da3adc349..dc48b0a6d3 100644 --- a/src/file_analysis/analyzer/x509/X509.cc +++ b/src/file_analysis/analyzer/x509/X509.cc @@ -12,6 +12,8 @@ #include "file_analysis/Manager.h" #include +#include +#include #include #include From 8561c79363a12e5592b78f53822b35b3000e90ae Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 5 Jun 2020 18:20:05 -0700 Subject: [PATCH 2/2] Remove inline from some static KeyedHash members Coverity Scan builds currently encounter catastrophic error, claiming alignas requires use on both declaration and definition, so appears to actually not understand "static inline" in combo with alignas. --- src/Hash.cc | 4 ++++ src/Hash.h | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Hash.cc b/src/Hash.cc index a5a375750b..ebc997f709 100644 --- a/src/Hash.cc +++ b/src/Hash.cc @@ -13,6 +13,10 @@ #include "highwayhash/highwayhash_target.h" #include "highwayhash/instruction_sets.h" +alignas(32) uint64_t KeyedHash::shared_highwayhash_key[4]; +alignas(32) uint64_t KeyedHash::cluster_highwayhash_key[4]; +alignas(16) unsigned long long KeyedHash::shared_siphash_key[2]; + // we use the following lines to not pull in the highwayhash headers in Hash.h - but to check the types did not change underneath us. static_assert(std::is_same::value, "Highwayhash return values must match hash_x_t"); static_assert(std::is_same::value, "Highwayhash return values must match hash_x_t"); diff --git a/src/Hash.h b/src/Hash.h index 9e39909d58..0bc4daeec0 100644 --- a/src/Hash.h +++ b/src/Hash.h @@ -186,11 +186,11 @@ public: private: // actually HHKey. This key changes each start (unless a seed is specified) - alignas(32) inline static uint64_t shared_highwayhash_key[4]; + alignas(32) static uint64_t shared_highwayhash_key[4]; // actually HHKey. This key is installation specific and sourced from the digest_salt script-level const. - alignas(32) inline static uint64_t cluster_highwayhash_key[4]; + alignas(32) static uint64_t cluster_highwayhash_key[4]; // actually HH_U64, which has the same type. This key changes each start (unless a seed is specified) - alignas(16) inline static unsigned long long shared_siphash_key[2]; + alignas(16) static unsigned long long shared_siphash_key[2]; // This key changes each start (unless a seed is specified) inline static uint8_t shared_hmac_md5_key[16]; inline static bool seeds_initialized = false;