diff --git a/CHANGES b/CHANGES index 8c4da966ec..5a99cb5ba3 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,17 @@ +8.0.0-dev.809 | 2025-07-30 10:28:06 +0200 + + * RecordType: Ensure &default fields are always re-initialized (Arne Welzel, Corelight) + + This started working partly after the deferral logic introduced with + Zeek 6.0 so this finishes it :-) + + * Attr: Deprecate using &default and &optional together on record fields (Arne Welzel, Corelight) + + If &default implies re-initialization of the field, using them together + doesn't make much sense. + + * RecordType: Allow deferring &default=vector(), set(), table() fields (Arne Welzel, Corelight) + 8.0.0-dev.805 | 2025-07-30 10:07:50 +0200 * fixes for specialized ZAM operations needing to check whether record fields exist (Vern Paxson, Corelight) diff --git a/NEWS b/NEWS index 10657eb73b..51f6edf47f 100644 --- a/NEWS +++ b/NEWS @@ -306,6 +306,10 @@ Changed Functionality `Analyzer::DebugLogging`. Furthermore the default options changed to enable more detailed output by default. +- Record fields with a ``&default`` attribute are now consistently re-initialized + after deleting such fields. Previously, this would only work for constant + expressions, but has been extended to apply to arbitrary expressions. + - Publishing remote events with vector arguments that contain holes is now rejected. The receiver side never had a chance to figure out where these holes would have been. There's a chance this breaks scripts that accidentally @@ -339,6 +343,10 @@ Changed Functionality - The PPPoE parser now respects the size value given in the PPPoE header. Data beyon the size given in the header will be truncated. +- Record fields with ``&default`` attributes initializing empty ``vector``, ``table`` + or ``set`` instances are now deferred until they are accessed, potentially + improving memory usage when such fields are never accessed. + Removed Functionality --------------------- @@ -369,6 +377,10 @@ Deprecated Functionality The replacement script doesn't populate the ``email_body_sections`` anymore either. +- Using ``&default`` and ``&optional`` together on a record field has been deprecated + as it would only result in ``&default`` behavior. This will become an error starting + with Zeek 8.1. + - The ``zeek::Event()`` constructor was deprecated. Use ``event_mgr::Enqueue()`` or ``event_mgr::Dispatch()`` instead. diff --git a/VERSION b/VERSION index 83951273b0..92b1107829 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -8.0.0-dev.805 +8.0.0-dev.809 diff --git a/scripts/base/frameworks/notice/weird.zeek b/scripts/base/frameworks/notice/weird.zeek index 987bed11d2..43ff6dd356 100644 --- a/scripts/base/frameworks/notice/weird.zeek +++ b/scripts/base/frameworks/notice/weird.zeek @@ -52,7 +52,7 @@ export { ## The peer that originated this weird. This is helpful in ## cluster deployments if a particular cluster node is having ## trouble to help identify which node is having trouble. - peer: string &log &optional &default=peer_description; + peer: string &log &default=peer_description; ## The source of the weird. When reported by an analyzer, this ## should be the name of the analyzer. diff --git a/src/Attr.cc b/src/Attr.cc index 038d5ac1fb..4dc568818a 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -5,6 +5,7 @@ #include "zeek/Desc.h" #include "zeek/Expr.h" #include "zeek/IntrusivePtr.h" +#include "zeek/Reporter.h" #include "zeek/Val.h" #include "zeek/input/Manager.h" #include "zeek/threading/SerialTypes.h" @@ -298,6 +299,12 @@ bool Attributes::CheckAttr(Attr* a) { case ATTR_OPTIONAL: if ( global_var ) return AttrError("&optional is not valid for global variables"); + + // Remove in v8.1: Call AttrError() + if ( in_record && Find(ATTR_DEFAULT) ) + zeek::reporter->Deprecation( + "Remove in v8.1: Using &default and &optional together results in &default behavior"); + break; case ATTR_ADD_FUNC: @@ -335,6 +342,11 @@ bool Attributes::CheckAttr(Attr* a) { if ( Find(ATTR_DEFAULT_INSERT) ) return AttrError("&default and &default_insert cannot be used together"); + // Remove in v8.1: Call AttrError() + if ( in_record && Find(ATTR_OPTIONAL) ) + zeek::reporter->Deprecation( + "Remove in v8.1: Using &default and &optional together results in &default behavior"); + std::string err_msg; if ( ! check_default_attr(a, type, global_var, in_record, err_msg) && ! err_msg.empty() ) return AttrError(err_msg.c_str()); diff --git a/src/Type.cc b/src/Type.cc index 1db3d15433..68ff2d9de9 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -929,6 +929,20 @@ public: return rt->IsDeferrable() && constructor_list->Exprs().empty(); } + // Allow deferring fairly common &default=vector() field initialization... + if ( init_expr->Tag() == EXPR_VECTOR_CONSTRUCTOR ) { + auto vce = zeek::cast_intrusive(init_expr); + return vce->GetType()->IsUnspecifiedVector(); + } + + // ...and also &default=table() and &default=set(). + if ( init_expr->Tag() == EXPR_TABLE_COERCE || init_expr->Tag() == EXPR_TABLE_CONSTRUCTOR || + init_expr->Tag() == EXPR_SET_CONSTRUCTOR ) { + auto une = zeek::cast_intrusive(init_expr); + return une->Op()->GetType()->Tag() == TYPE_TABLE && + une->Op()->GetType()->IsUnspecifiedTable(); + } + return false; } @@ -1007,12 +1021,29 @@ private: void OptimizeCreationInits(RecordType* rt) { int i = 0; for ( auto& ci : rt->creation_inits ) { - if ( ! ci.second->IsDeferrable() ) + if ( ! ci.second->IsDeferrable() ) { rt->creation_inits[i++] = std::move(ci); + + // A non-deferrable field with a &default attribute is expected to also exist in deferred_inits + // such that re-initialization after deletion of the field works. + if ( rt->FieldDecl(ci.first)->GetAttr(detail::ATTR_DEFAULT) != detail::Attr::nil ) { + if ( ! rt->deferred_inits[ci.first] ) + zeek::reporter->InternalError("non-deferrable field %s$%s with &default not in deferred_inits", + rt->GetName().c_str(), rt->FieldName(i)); + + if ( rt->deferred_inits[ci.first] != rt->creation_inits[i - 1].second ) + zeek::reporter->InternalError("non-deferrable field %s$%s with &default has inconsistent inits", + rt->GetName().c_str(), rt->FieldName(i)); + } + } else { - // std::fprintf(stderr, "deferred %s$%s: %s\n", obj_desc_short(rt).c_str(), rt->FieldName(ci.first), - // ci.second->InitExpr() ? obj_desc_short(ci.second->InitExpr()).c_str() : ""); - assert(! rt->deferred_inits[ci.first]); + // If deferred_inits already has a value, it should be the same as the one + // stored in creation_inits. This happens for deferrable record fields + // that have a &default attribute. + if ( rt->deferred_inits[ci.first] && rt->deferred_inits[ci.first] != ci.second ) + zeek::reporter->InternalError("deferrable field %s$%s has inconsistent inits", + rt->GetName().c_str(), rt->FieldName(i)); + rt->deferred_inits[ci.first].swap(ci.second); } } @@ -1104,8 +1135,13 @@ void RecordType::AddField(unsigned int field, const TypeDecl* td) { } else { - auto efi = std::make_shared(def_expr, type); - creation_inits.emplace_back(field, std::move(efi)); + // Note that init is placed into creation_inits and deferred_inits + // such that accessing a record field after deleting it will run the + // &default expression to re-initialize it again. + // + // Also see RecordType::CreationInitsOptimizer. + init = std::make_shared(def_expr, type); + creation_inits.emplace_back(field, init); } } diff --git a/testing/btest/Baseline/language.delete-field-set-2/output b/testing/btest/Baseline/language.delete-field-set-2/output new file mode 100644 index 0000000000..b917b4c7ca --- /dev/null +++ b/testing/btest/Baseline/language.delete-field-set-2/output @@ -0,0 +1,4 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +default, [v=[1, 2, 3, 100], r=[c=101]] +after changing, [v=[1, 2, 3, 100, 4711], r=[c=42]] +after delete, [v=[1, 2, 3, 102], r=[c=103]] diff --git a/testing/btest/Baseline/language.delete-field-set/output b/testing/btest/Baseline/language.delete-field-set/output index 85fc672ea8..e99c5e4bf8 100644 --- a/testing/btest/Baseline/language.delete-field-set/output +++ b/testing/btest/Baseline/language.delete-field-set/output @@ -1,2 +1,6 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -[a=, b=, c=] +[a={ + +}, b={ + +}, c=[]] diff --git a/testing/btest/Baseline/language.optional-and-default-field/.stderr b/testing/btest/Baseline/language.optional-and-default-field/.stderr new file mode 100644 index 0000000000..e7dd6cd393 --- /dev/null +++ b/testing/btest/Baseline/language.optional-and-default-field/.stderr @@ -0,0 +1,6 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +warning in <...>/optional-and-default-field.zeek, line 10: Remove in v8.1: Using &default and &optional together results in &default behavior +warning in <...>/optional-and-default-field.zeek, line 11: Remove in v8.1: Using &default and &optional together results in &default behavior +warning in <...>/optional-and-default-field.zeek, line 12: Remove in v8.1: Using &default and &optional together results in &default behavior +warning in <...>/optional-and-default-field.zeek, line 13: Remove in v8.1: Using &default and &optional together results in &default behavior +warning in <...>/optional-and-default-field.zeek, line 14: Remove in v8.1: Using &default and &optional together results in &default behavior diff --git a/testing/btest/Baseline/language.optional-and-default-field/.stdout b/testing/btest/Baseline/language.optional-and-default-field/.stdout new file mode 100644 index 0000000000..67b39afcc6 --- /dev/null +++ b/testing/btest/Baseline/language.optional-and-default-field/.stdout @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +[c=5, i=-5, v=[], r0=[], r1=[]] diff --git a/testing/btest/language/delete-field-set.zeek b/testing/btest/language/delete-field-set.zeek index 8f1482c6c2..97f67a7af9 100644 --- a/testing/btest/language/delete-field-set.zeek +++ b/testing/btest/language/delete-field-set.zeek @@ -1,3 +1,5 @@ +# @TEST-DOC: Test behavior of &default when deleting a record field and subsequently accessing it again. +# # @TEST-EXEC: zeek -b %INPUT >output 2>&1 # @TEST-EXEC: btest-diff output @@ -14,3 +16,30 @@ delete test$b; delete test$c; print test; + +# @TEST-START-NEXT +global c = 99; + +# Helper function that's running as part of R's default construction. +function seq(): count { + ++c; + return c; +} + +type R: record { + c: count &default=seq(); +}; + +type FooBar: record { + v: vector of count &default=vector(1, 2, 3, seq()); + r: R &default=R(); +}; + +global test: FooBar; +print "default", test; +test$v += 4711; +test$r$c = 42; +print "after changing", test; +delete test$v; +delete test$r; +print "after delete", test; diff --git a/testing/btest/language/optional-and-default-field.zeek b/testing/btest/language/optional-and-default-field.zeek new file mode 100644 index 0000000000..5379b21ce2 --- /dev/null +++ b/testing/btest/language/optional-and-default-field.zeek @@ -0,0 +1,18 @@ +# @TEST-DOC: Warn on record fields that have both, &optional and &default +# +# @TEST-EXEC: zeek -b %INPUT +# @TEST-EXEC: btest-diff .stdout +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr + +type R: record { }; + +type X: record { + c: count &optional &default=5; + i: int &default=-5 &optional; + v: vector of string &optional &default=vector(); + r0: R &optional &default=R(); + r1: R &default=R() &optional; +}; + +global x = X(); +print x;