Merge remote-tracking branch 'origin/topic/etyp/enum-port-sizeof'

* origin/topic/etyp/enum-port-sizeof:
  Add enum value negative check
  Fix port/enum values `SizeOf` not being a count

CHANGES

@@ -1,3 +1,22 @@
+7.1.0-dev.328 | 2024-09-18 19:10:43 +0200
+
+  * Add enum value negative check (Evan Typanski, Corelight)
+
+    There was one already at parse time, this adds a check later so that
+    cases like overflows or internal enums with negative values get caught.
+
+  * Fix port/enum values `SizeOf` not being a count (Evan Typanski, Corelight)
+
+    Really, they both should be count. But, they were getting provided as an
+    integer. Port is easy since it is backed by an unsigned value. Enums
+    *should* be unsigned, but aren't. This doesn't address that, it just
+    takes the other name for this operator (absolute value) and makes the
+    enum value positive if it's negative.
+
+    This fixes a case where using the size of operator on enum/port values
+    in certain contexts (like the default parameter of a struct) would cause
+    an internal error.
+
 7.1.0-dev.324 | 2024-09-17 18:51:34 +0200
 
   * btest: Update baselines for removal-hooks addition (Arne Welzel, Corelight)

VERSION

@@ -1 +1 @@
-7.1.0-dev.324
+7.1.0-dev.328

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);
 

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,7 +3133,15 @@ unsigned int RecordVal::ComputeFootprint(std::unordered_set<const Val*>* analyze
     return fp;
 }
 
-ValPtr EnumVal::SizeVal() const { return val_mgr->Int(AsInt()); }
+ValPtr EnumVal::SizeVal() const {
+    // Negative enums are rejected at parse time, but not internally. Handle the
+    // negative case just like a signed integer, as that is an enum's underlying
+    // type.
+    if ( AsInt() < 0 )
+        return val_mgr->Count(-AsInt());
+    else
+        return val_mgr->Count(AsInt());
+}
 
 void EnumVal::ValDescribe(ODesc* d) const {
     const char* ename = type->AsEnumType()->Lookup(int_val);

testing/btest/Baseline/language.enum-negative/output

@@ -0,0 +1,3 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in <...>/enum-negative.zeek, line 5: enumerator is not a count constant
+error in <...>/enum-negative.zeek, line 6: enumerator value cannot be negative

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)

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 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 65616
 Record [i=10, j=<uninitialized>, k=<uninitialized>]: 3
 Set: 3
 String 'Hello': 5

testing/btest/language/enum-negative.zeek

@@ -0,0 +1,7 @@
+# @TEST-EXEC-FAIL: zeek -b %INPUT >output 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER="$SCRIPTS/diff-remove-abspath" btest-diff output
+
+type my_enum: enum {
+	explicitly_negative = -1,
+	overflow = 9223372036854775808,
+};

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 %d", with_enum$e, |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 %d", with_port$p, |with_port$p|);
+
 # Size of record: returns number of fields (assigned + unassigned)
 print fmt("Record %s: %d", r, |r|);