From 18597ea49c3fd99cba6b5a62b937ff0ed512bcc5 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 3 Apr 2025 16:39:28 +0200 Subject: [PATCH 1/4] type/id/zeekygen: Add EnumVal to enum identifiers Provide a direct way to go from a zeek:id::ID value to EnumVal without needing to go through the type. --- src/ID.cc | 4 ++++ src/Type.cc | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/ID.cc b/src/ID.cc index 0031d833bc..10719cdaca 100644 --- a/src/ID.cc +++ b/src/ID.cc @@ -481,6 +481,10 @@ void ID::DescribeReST(ODesc* d, bool roles_only) const { } if ( val && type && type->Tag() != TYPE_FUNC && type->InternalType() != TYPE_INTERNAL_VOID && + // Do not include a default value for enum const identifiers, + // as their value can't be changed. + ! IsEnumConst() && + // Values within Version module are likely to include a // constantly-changing version number and be a frequent // source of error/desynchronization, so don't include them. diff --git a/src/Type.cc b/src/Type.cc index a5d57d187c..451bb9934e 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -1567,7 +1567,7 @@ void EnumType::CheckAndAddName(const string& module_name, const char* name, zeek if ( deprecation ) id->MakeDeprecated({NewRef{}, deprecation}); - detail::zeekygen_mgr->Identifier(std::move(id), from_redef); + detail::zeekygen_mgr->Identifier(id, from_redef); } else { // We allow double-definitions if matching exactly. This is so that @@ -1591,6 +1591,12 @@ void EnumType::CheckAndAddName(const string& module_name, const char* name, zeek if ( vals.find(val) == vals.end() ) vals[val] = make_intrusive(IntrusivePtr{NewRef{}, this}, val); + if ( ! id->HasVal() ) + id->SetVal(vals[val]); + else if ( id->GetVal()->AsEnum() != val ) + reporter->InternalError("inconsistent enum integer value for '%s' (old %" PRId64 " new %" PRId64 ")", + fullname.c_str(), id->GetVal()->AsEnum(), val); + const auto& types = Type::Aliases(GetName()); for ( const auto& t : types ) From ab87ba97865a2892a6fa71adfaef5270ee3c7576 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 3 Apr 2025 19:28:01 +0200 Subject: [PATCH 2/4] Var/add_type: Do not clone EnumType when declared for the first time EnumType receives the name into its constructor. Even for the first declaration the name is not empty and instead the same as the identifier's name. Due to that, add_type() previously took the else path and created a shallow clone of the initial type instead of using it. This lead to buggy behavior where enum value identifiers declared within an enum's first body have a different TypePtr associated than the one that is found via `zeek::id::find_type()`. It also means that enum identifiers added via redef later would have a different TypePtr than those in the initial pointer. --- src/Var.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Var.cc b/src/Var.cc index e17e2dc15a..44ced49665 100644 --- a/src/Var.cc +++ b/src/Var.cc @@ -369,9 +369,18 @@ void add_type(ID* id, TypePtr t, std::unique_ptr> attr) { TypePtr tnew; - if ( (t->Tag() == TYPE_RECORD || t->Tag() == TYPE_ENUM) && old_type_name.empty() ) + if ( (t->Tag() == TYPE_RECORD || t->Tag() == TYPE_ENUM) && + (old_type_name.empty() || old_type_name == new_type_name) ) { // An extensible type (record/enum) being declared for first time. + // + // Enum types are initialized with the same name as their identifier + // when declared for the first time, double check that here. + if ( t->Tag() == TYPE_ENUM && new_type_name != old_type_name ) + reporter->InternalError("enum type has unexpected names: '%s' and '%s'", old_type_name.c_str(), + new_type_name.c_str()); + tnew = std::move(t); + } else { // If the old type is an error or the old type doesn't exist, then return // an error instead of trying to clone it. From cb610bdea270ac5ae009cc839c3c45e59f5268ab Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 4 Apr 2025 09:52:46 +0200 Subject: [PATCH 3/4] btest/plugin: Add plugin testing enum identifiers This plugin has a generic name, but for now just tests the API around enum types and enum vals at InitPostScript() time. --- .../btest/Baseline/plugins.api-plugin/output | 2 + testing/btest/plugins/api-plugin.zeek | 26 ++++++ .../btest/plugins/api-plugin/.btest-ignore | 0 .../btest/plugins/api-plugin/src/Plugin.cc | 88 +++++++++++++++++++ testing/btest/plugins/api-plugin/src/Plugin.h | 18 ++++ 5 files changed, 134 insertions(+) create mode 100644 testing/btest/Baseline/plugins.api-plugin/output create mode 100644 testing/btest/plugins/api-plugin.zeek create mode 100644 testing/btest/plugins/api-plugin/.btest-ignore create mode 100644 testing/btest/plugins/api-plugin/src/Plugin.cc create mode 100644 testing/btest/plugins/api-plugin/src/Plugin.h diff --git a/testing/btest/Baseline/plugins.api-plugin/output b/testing/btest/Baseline/plugins.api-plugin/output new file mode 100644 index 0000000000..f21c6a7f90 --- /dev/null +++ b/testing/btest/Baseline/plugins.api-plugin/output @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +TestEnumIdentifiers successful diff --git a/testing/btest/plugins/api-plugin.zeek b/testing/btest/plugins/api-plugin.zeek new file mode 100644 index 0000000000..f318cab1d7 --- /dev/null +++ b/testing/btest/plugins/api-plugin.zeek @@ -0,0 +1,26 @@ +# @TEST-DOC: A plugin testing some parts Zeek's C++ API +# @TEST-EXEC: ${DIST}/auxil/zeek-aux/plugin-support/init-plugin -u . Demo API +# @TEST-EXEC: cp -r %DIR/api-plugin/* . +# +# @TEST-EXEC: ./configure --zeek-dist=${DIST} && make +# +# @TEST-EXEC: ZEEK_PLUGIN_PATH=`pwd` zeek %INPUT >output +# +# @TEST-EXEC: TEST_DIFF_CANONIFIER= btest-diff output + +module DemoAPI; + +export { + type Severity: enum { + CRITICAL = 1, + ERROR = 2, + WARNING = 3, + INFO = 4, + }; +} + +module User; + +redef enum DemoAPI::Severity += { + USER_DEBUG = 50, +}; diff --git a/testing/btest/plugins/api-plugin/.btest-ignore b/testing/btest/plugins/api-plugin/.btest-ignore new file mode 100644 index 0000000000..e69de29bb2 diff --git a/testing/btest/plugins/api-plugin/src/Plugin.cc b/testing/btest/plugins/api-plugin/src/Plugin.cc new file mode 100644 index 0000000000..40ed229c66 --- /dev/null +++ b/testing/btest/plugins/api-plugin/src/Plugin.cc @@ -0,0 +1,88 @@ + +#include "Plugin.h" + +#include +#include +#include +#include +#include + +namespace btest::plugin::Demo_API { +Plugin plugin; +} + +using namespace btest::plugin::Demo_API; + +zeek::plugin::Configuration Plugin::Configure() { + zeek::plugin::Configuration config; + config.name = "Demo::API"; + config.description = "Use some of Zeek's API for testing"; + config.version.major = 1; + config.version.minor = 0; + config.version.patch = 0; + return config; +} + +namespace { + +// Test zeek::id::find for enums an their types. +void TestEnumIdentifiers() { + auto severity_type = zeek::id::find_type("DemoAPI::Severity"); + if ( ! severity_type ) + zeek::reporter->FatalError("DemoAPI::Severity not found"); + + // Expect 5 entries! + if ( severity_type->Names().size() != 5 ) + zeek::reporter->FatalError("Wrong number of severities %" PRId64, severity_type->Names().size()); + + // Ensure CRITICAL and USER_DEBUG identifiers have the same enum type as severity_type. + auto critical_id = zeek::id::find("DemoAPI::CRITICAL"); + auto critical_type = critical_id->GetType(); + auto user_debug_id = zeek::id::find("User::USER_DEBUG"); + auto user_debug_type = user_debug_id->GetType(); + + if ( critical_type != user_debug_type ) + zeek::reporter->FatalError("CRITICAL and USER_DEBUG have different types (%p and %p)", critical_type.get(), + user_debug_type.get()); + + // Ensure the critical_id and user_debug_type IDs have an EnumVal attached + // and that the value is the same as in script land. + auto critical_val = critical_id->GetVal(); + auto user_debug_val = user_debug_id->GetVal(); + + if ( ! critical_val || ! user_debug_val ) + zeek::reporter->FatalError("Missing values on enum value identifiers %p %p", critical_val.get(), + user_debug_val.get()); + + if ( critical_val->AsEnum() != 1 ) + zeek::reporter->FatalError("Wrong value for CRITICAL: %" PRId64, critical_val->AsEnum()); + + if ( user_debug_val->AsEnum() != 50 ) + zeek::reporter->FatalError("Wrong value for USER_DEBUG: %" PRId64, user_debug_val->AsEnum()); + + // Ensure all the types (identifiers and values) agree with severity_type. + if ( critical_type != severity_type ) + zeek::reporter->FatalError("CRITICAL identifier has the wrong enum type %p vs %p", critical_type.get(), + severity_type.get()); + + if ( user_debug_type != severity_type ) + zeek::reporter->FatalError("USER_DEBUG identifier has the wrong enum type %p vs %p", user_debug_type.get(), + severity_type.get()); + + if ( critical_val->GetType() != severity_type ) + zeek::reporter->FatalError("CRITICAL value has the wrong enum type %p vs %p", critical_val->GetType().get(), + severity_type.get()); + + if ( user_debug_val->GetType() != severity_type ) + zeek::reporter->FatalError("USER_DEBUG value has the wrong enum type %p vs %p", user_debug_val->GetType().get(), + severity_type.get()); + + std::cout << "TestEnumIdentifiers successful" << std::endl; +} + +} // namespace + +void Plugin::InitPostScript() { + // Other API tests if wanted. + TestEnumIdentifiers(); +} diff --git a/testing/btest/plugins/api-plugin/src/Plugin.h b/testing/btest/plugins/api-plugin/src/Plugin.h new file mode 100644 index 0000000000..2c2740e97a --- /dev/null +++ b/testing/btest/plugins/api-plugin/src/Plugin.h @@ -0,0 +1,18 @@ + +#pragma once + +#include + +namespace btest::plugin::Demo_API { + +class Plugin : public zeek::plugin::Plugin { +protected: + // Overridden from zeek::plugin::Plugin. + zeek::plugin::Configuration Configure() override; + + void InitPostScript() override; +}; + +extern Plugin plugin; + +} // namespace btest::plugin::Demo_API From 6bc36e8cf87593200c121caab8eccdaf94e2f841 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 3 Apr 2025 16:28:50 +0200 Subject: [PATCH 4/4] broker/main: Adapt enum values to agree with comm.bif Logic to detect this error already existed, but due to enum identifiers not having a value set, it never triggered before. Should probably backport this one. --- scripts/base/frameworks/broker/main.zeek | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/scripts/base/frameworks/broker/main.zeek b/scripts/base/frameworks/broker/main.zeek index 805f1f1e6c..1213555dc5 100644 --- a/scripts/base/frameworks/broker/main.zeek +++ b/scripts/base/frameworks/broker/main.zeek @@ -207,24 +207,26 @@ export { PEER_INVALID = 3, ## Remote peer not listening. PEER_UNAVAILABLE = 4, + ## Remote peer disconnected during the handshake. + PEER_DISCONNECT_DURING_HANDSHAKE = 5, ## A peering request timed out. - PEER_TIMEOUT = 5, + PEER_TIMEOUT = 6, ## Master with given name already exists. - MASTER_EXISTS = 6, + MASTER_EXISTS = 7, ## Master with given name does not exist. - NO_SUCH_MASTER = 7, + NO_SUCH_MASTER = 8, ## The given data store key does not exist. - NO_SUCH_KEY = 8, + NO_SUCH_KEY = 9, ## The store operation timed out. - REQUEST_TIMEOUT = 9, + REQUEST_TIMEOUT = 10, ## The operation expected a different type than provided. - TYPE_CLASH = 10, + TYPE_CLASH = 11, ## The data value cannot be used to carry out the desired operation. - INVALID_DATA = 11, + INVALID_DATA = 12, ## The storage backend failed to execute the operation. - BACKEND_FAILURE = 12, + BACKEND_FAILURE = 13, ## The storage backend failed to execute the operation. - STALE_DATA = 13, + STALE_DATA = 14, ## Catch-all for a CAF-level problem. CAF_ERROR = 100 };