From a26d2dd56cbab822cb4eb8658e8566261764b6a9 Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Wed, 11 Sep 2024 17:46:42 +0200 Subject: [PATCH] Fix `|...|` operator for inner ports/enums The `|...|` (sizeof) operator applies to ports/enums. That has returned a count, but it causes an internal error when used as an inner value in a record. This fixes that internal error for ports by just making the sizeof operator return a count. Enums could technically be negative before this change, but that is rejected at parse time. It seems reasonable to modify the enum value to only be non-negative, which makes the sizeof operator easier since it can just return a count. Changing that to return an int would potentially break scripts that use the sizeof operator and assign it to a count. This could technically break code that internally sets the enum value to a negative value, but I don't think that's a very likely use. --- NEWS | 4 ++++ src/Type.cc | 6 ++++++ src/Val.cc | 6 +++--- src/Val.h | 7 +++---- src/script_opt/CPP/RuntimeInitSupport.cc | 2 +- src/script_opt/CPP/RuntimeInitSupport.h | 2 +- testing/btest/Baseline/language.sizeof/.stderr | 2 +- testing/btest/Baseline/language.sizeof/output | 2 ++ testing/btest/language/sizeof.zeek | 16 ++++++++++++++++ 9 files changed, 37 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index d9e7e2cd0d..17a341a044 100644 --- a/NEWS +++ b/NEWS @@ -25,6 +25,10 @@ Breaking Changes are not affected by this change, so we keep backwards compatibility with existing log writers. +* Enum values can no longer be negative even if created internally. This is + enforced at parse time for scripts, but not internally. With this change, the + ``|...|`` operator for enums will properly return a count in all instances. + New Functionality ----------------- diff --git a/src/Type.cc b/src/Type.cc index 80b4e7c18b..fe58ffbbeb 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -1542,6 +1542,12 @@ void EnumType::CheckAndAddName(const string& module_name, const char* name, zeek return; } + if ( val < 0 ) { + reporter->Error("enumerator value cannot be negative"); + SetError(); + return; + } + auto fullname = detail::make_full_var_name(module_name.c_str(), name); auto id = id::find(fullname); diff --git a/src/Val.cc b/src/Val.cc index e3574116cc..37a62ac8ea 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -594,7 +594,7 @@ void IntervalVal::ValDescribe(ODesc* d) const { } } -ValPtr PortVal::SizeVal() const { return val_mgr->Int(uint_val); } +ValPtr PortVal::SizeVal() const { return val_mgr->Count(uint_val); } uint32_t PortVal::Mask(uint32_t port_num, TransportProto port_type) { // Note, for ICMP one-way connections: @@ -3133,10 +3133,10 @@ unsigned int RecordVal::ComputeFootprint(std::unordered_set* analyze return fp; } -ValPtr EnumVal::SizeVal() const { return val_mgr->Int(AsInt()); } +ValPtr EnumVal::SizeVal() const { return val_mgr->Count(AsCount()); } void EnumVal::ValDescribe(ODesc* d) const { - const char* ename = type->AsEnumType()->Lookup(int_val); + const char* ename = type->AsEnumType()->Lookup(uint_val); if ( ! ename ) ename = ""; diff --git a/src/Val.h b/src/Val.h index 8d5d8f3d52..31998d2f69 100644 --- a/src/Val.h +++ b/src/Val.h @@ -4,7 +4,6 @@ #include // for u_char #include -#include #include #include #include @@ -1497,7 +1496,7 @@ private: const std::vector& is_managed; }; -class EnumVal final : public detail::IntValImplementation { +class EnumVal final : public detail::UnsignedValImplementation { public: ValPtr SizeVal() const override; @@ -1505,12 +1504,12 @@ protected: friend class Val; friend class EnumType; - friend EnumValPtr make_enum__CPP(TypePtr t, zeek_int_t i); + friend EnumValPtr make_enum__CPP(TypePtr t, zeek_uint_t i); template friend IntrusivePtr make_intrusive(Ts&&... args); - EnumVal(EnumTypePtr t, zeek_int_t i) : detail::IntValImplementation(std::move(t), i) {} + EnumVal(EnumTypePtr t, zeek_uint_t i) : detail::UnsignedValImplementation(std::move(t), i) {} void ValDescribe(ODesc* d) const override; ValPtr DoClone(CloneState* state) override; diff --git a/src/script_opt/CPP/RuntimeInitSupport.cc b/src/script_opt/CPP/RuntimeInitSupport.cc index 3674d7605d..ed1fa03934 100644 --- a/src/script_opt/CPP/RuntimeInitSupport.cc +++ b/src/script_opt/CPP/RuntimeInitSupport.cc @@ -223,7 +223,7 @@ EnumTypePtr get_enum_type__CPP(const string& enum_type_name) { return make_intrusive(enum_type_name); } -EnumValPtr make_enum__CPP(TypePtr t, zeek_int_t i) { +EnumValPtr make_enum__CPP(TypePtr t, zeek_uint_t i) { auto et = cast_intrusive(std::move(t)); return make_intrusive(et, i); } diff --git a/src/script_opt/CPP/RuntimeInitSupport.h b/src/script_opt/CPP/RuntimeInitSupport.h index 33625ea613..a0906545a1 100644 --- a/src/script_opt/CPP/RuntimeInitSupport.h +++ b/src/script_opt/CPP/RuntimeInitSupport.h @@ -95,7 +95,7 @@ extern EnumTypePtr get_enum_type__CPP(const std::string& enum_type_name); // Returns an enum value corresponding to the given low-level value 'i' // in the context of the given enum type 't'. -extern EnumValPtr make_enum__CPP(TypePtr t, zeek_int_t i); +extern EnumValPtr make_enum__CPP(TypePtr t, zeek_uint_t i); } // namespace detail } // namespace zeek diff --git a/testing/btest/Baseline/language.sizeof/.stderr b/testing/btest/Baseline/language.sizeof/.stderr index 837fbeb902..8f4cbe78e0 100644 --- a/testing/btest/Baseline/language.sizeof/.stderr +++ b/testing/btest/Baseline/language.sizeof/.stderr @@ -1,2 +1,2 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -expression warning in <...>/sizeof.zeek, line 73: count underflow (5 - 9) +expression warning in <...>/sizeof.zeek, line 83: count underflow (5 - 9) diff --git a/testing/btest/Baseline/language.sizeof/output b/testing/btest/Baseline/language.sizeof/output index 0982d6914a..9828a521c1 100644 --- a/testing/btest/Baseline/language.sizeof/output +++ b/testing/btest/Baseline/language.sizeof/output @@ -7,11 +7,13 @@ Expr: 18446744073709551612 Signed Expr: 4 Double -1.23: 1.230000 Enum ENUM3: 2 +Enum in record: 2 File 21.000000 Function add_interface: 2 Integer -10: 10 Interval -5.0 secs: 5.000000 Port 80/tcp: 65616 +Port in record: 65616 Record [i=10, j=, k=]: 3 Set: 3 String 'Hello': 5 diff --git a/testing/btest/language/sizeof.zeek b/testing/btest/language/sizeof.zeek index 5c0a043749..edafc62464 100644 --- a/testing/btest/language/sizeof.zeek +++ b/testing/btest/language/sizeof.zeek @@ -20,6 +20,14 @@ type example_record: record { k: int &optional; }; +type example_record_with_enum: record { + e: count &default = |ENUM3|; +} &redef; + +type example_record_with_port: record { + p: count &default = |80/tcp|; +} &redef; + global a: addr = 1.2.3.4; global a6: addr = [::1]; global b: bool = T; @@ -36,6 +44,8 @@ global sn: subnet = 192.168.0.0/24; global t: table[string] of string; global ti: time = current_time(); global v: vector of string; +global with_enum: example_record_with_enum; +global with_port: example_record_with_port; # Additional initialization # @@ -80,6 +90,9 @@ print fmt("Double %s: %f", d, |d|); # Size of enum: returns numeric value of enum constant. print fmt("Enum %s: %d", ENUM3, |ENUM3|); +# Within a record, enum sizeof should still be ok +print fmt("Enum in record: %d", |with_enum$e|); + # Size of file: returns current file size. # Note that this is a double so that file sizes >> 4GB # can be expressed. @@ -97,6 +110,9 @@ print fmt("Interval %s: %f", iv, |iv|); # Size of port: returns port number as a count. print fmt("Port %s: %d", p, |p|); +# Within a record, port sizeof should still be ok +print fmt("Port in record: %d", |with_port$p|); + # Size of record: returns number of fields (assigned + unassigned) print fmt("Record %s: %d", r, |r|);