From d3dd8a155d6cf010fc911d9e28158d96e5b5ec2c Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Tue, 17 Sep 2024 10:55:45 -0400 Subject: [PATCH 1/2] Fix port/enum values `SizeOf` not being a count 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. --- src/Val.cc | 12 ++++++++++-- testing/btest/Baseline/language.sizeof/.stderr | 2 +- testing/btest/Baseline/language.sizeof/output | 2 ++ testing/btest/language/sizeof.zeek | 16 ++++++++++++++++ 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/Val.cc b/src/Val.cc index e3574116cc..1844593a5f 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,7 +3133,15 @@ unsigned int RecordVal::ComputeFootprint(std::unordered_set* 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); 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..cbb6540cbb 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 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=, k=]: 3 Set: 3 String 'Hello': 5 diff --git a/testing/btest/language/sizeof.zeek b/testing/btest/language/sizeof.zeek index 5c0a043749..75f0a2acec 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 %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|); From 08348cd1775c90f5b355314cbecfa2bdc57caca8 Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Tue, 17 Sep 2024 12:04:19 -0400 Subject: [PATCH 2/2] Add enum value negative check 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. --- src/Type.cc | 6 ++++++ testing/btest/Baseline/language.enum-negative/output | 3 +++ testing/btest/language/enum-negative.zeek | 7 +++++++ 3 files changed, 16 insertions(+) create mode 100644 testing/btest/Baseline/language.enum-negative/output create mode 100644 testing/btest/language/enum-negative.zeek 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/testing/btest/Baseline/language.enum-negative/output b/testing/btest/Baseline/language.enum-negative/output new file mode 100644 index 0000000000..d1c65bc12b --- /dev/null +++ b/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 diff --git a/testing/btest/language/enum-negative.zeek b/testing/btest/language/enum-negative.zeek new file mode 100644 index 0000000000..03d1dc02dc --- /dev/null +++ b/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, +};