diff --git a/NEWS b/NEWS index 914152bbf8..d676e18698 100644 --- a/NEWS +++ b/NEWS @@ -298,6 +298,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 diff --git a/src/Type.cc b/src/Type.cc index eb38c19b0b..93fd84857c 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -1021,14 +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 (%s)\n", obj_desc_short(rt).c_str(), - // rt->FieldName(ci.first), - // ci.second->InitExpr() ? obj_desc_short(ci.second->InitExpr()).c_str() : "", - // typeid(ci).name()); - 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); } } @@ -1120,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/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;