From aaa81cae5d752565365dec8707710b9a78196ba1 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 1 Sep 2023 18:18:31 +0200 Subject: [PATCH] CompositeHash: Skip record initialization when recovering vals Initializing fields of recovered records caused running &default expression of fields just so that they are re-assigned in the next step with the recovered fields. The second test case still shows that the loop var is initialized as well even though that's not needed. Add tests for iterating over records with &default attributes for both, tables and vectors. Fixes #3267 --- src/CompHash.cc | 5 +- src/Val.h | 1 + .../output | 14 +++ .../output | 19 ++++ .../output | 12 +++ .../output | 14 +++ .../output | 12 +++ .../table-iterate-record-key-default.zeek | 95 +++++++++++++++++++ .../vector-iterate-record-default.zeek | 59 ++++++++++++ 9 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 testing/btest/Baseline/language.table-iterate-record-key-default-2/output create mode 100644 testing/btest/Baseline/language.table-iterate-record-key-default-3/output create mode 100644 testing/btest/Baseline/language.table-iterate-record-key-default/output create mode 100644 testing/btest/Baseline/language.vector-iterate-record-default-2/output create mode 100644 testing/btest/Baseline/language.vector-iterate-record-default/output create mode 100644 testing/btest/language/table-iterate-record-key-default.zeek create mode 100644 testing/btest/language/vector-iterate-record-default.zeek diff --git a/src/CompHash.cc b/src/CompHash.cc index 54cf87bf70..aa0a9846f2 100644 --- a/src/CompHash.cc +++ b/src/CompHash.cc @@ -359,10 +359,11 @@ bool CompositeHash::RecoverOneVal(const HashKey& hk, Type* t, ValPtr* pval, bool ASSERT(int(values.size()) == num_fields); - auto rv = make_intrusive(IntrusivePtr{NewRef{}, rt}); + auto rv = make_intrusive(IntrusivePtr{NewRef{}, rt}, + false /* init_fields */); for ( int i = 0; i < num_fields; ++i ) - rv->Assign(i, std::move(values[i])); + rv->AppendField(std::move(values[i]), rt->GetFieldType(i)); *pval = std::move(rv); } diff --git a/src/Val.h b/src/Val.h index af688a956e..054d8b7d2b 100644 --- a/src/Val.h +++ b/src/Val.h @@ -1444,6 +1444,7 @@ protected: friend class zeek::detail::ValTrace; friend class zeek::detail::ZBody; friend class zeek::detail::CPPRuntime; + friend class zeek::detail::CompositeHash; RecordValPtr DoCoerceTo(RecordTypePtr other, bool allow_orphaning) const; diff --git a/testing/btest/Baseline/language.table-iterate-record-key-default-2/output b/testing/btest/Baseline/language.table-iterate-record-key-default-2/output new file mode 100644 index 0000000000..f6bb33bb06 --- /dev/null +++ b/testing/btest/Baseline/language.table-iterate-record-key-default-2/output @@ -0,0 +1,14 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +0, done parsing +0, my_seq() invoked +1, populating table, expecting 4 my_seq() invocations +1, my_seq() invoked +2, my_seq() invoked +3, my_seq() invoked +4, my_seq() invoked +5, iterating table, expecting no my_seq() invocations +5, it, [id=4], 3 +5, it, [id=5], 4 +5, it, [id=3], 2 +5, it, [id=2], 1 +5, done diff --git a/testing/btest/Baseline/language.table-iterate-record-key-default-3/output b/testing/btest/Baseline/language.table-iterate-record-key-default-3/output new file mode 100644 index 0000000000..360ed39356 --- /dev/null +++ b/testing/btest/Baseline/language.table-iterate-record-key-default-3/output @@ -0,0 +1,19 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +0, done parsing +0, my_seq() invoked +1, my_seq() invoked +2, populating table, expecting 8 my_seq() invocations +2, my_seq() invoked +3, my_seq() invoked +4, my_seq() invoked +5, my_seq() invoked +6, my_seq() invoked +7, my_seq() invoked +8, my_seq() invoked +9, my_seq() invoked +10, iterating table, expecting no my_seq() invocations +10, it, [id=8], [id=7] +10, it, [id=6], [id=5] +10, it, [id=4], [id=3] +10, it, [id=10], [id=9] +10, done diff --git a/testing/btest/Baseline/language.table-iterate-record-key-default/output b/testing/btest/Baseline/language.table-iterate-record-key-default/output new file mode 100644 index 0000000000..fd2a122b69 --- /dev/null +++ b/testing/btest/Baseline/language.table-iterate-record-key-default/output @@ -0,0 +1,12 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +0, populating table, expecting 4 my_seq() invocations +0, my_seq() invoked +1, my_seq() invoked +2, my_seq() invoked +3, my_seq() invoked +4, iterating table, expecting no my_seq() invocations +4, it, [id=1], 1 +4, it, [id=4], 4 +4, it, [id=3], 3 +4, it, [id=2], 2 +4, done diff --git a/testing/btest/Baseline/language.vector-iterate-record-default-2/output b/testing/btest/Baseline/language.vector-iterate-record-default-2/output new file mode 100644 index 0000000000..d82b99eb8c --- /dev/null +++ b/testing/btest/Baseline/language.vector-iterate-record-default-2/output @@ -0,0 +1,14 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +0, done parsing +0, my_seq() invoked +1, populating vector, expecting 4 my_seq() invocations +1, my_seq() invoked +2, my_seq() invoked +3, my_seq() invoked +4, my_seq() invoked +5, iterating vector, expecting no my_seq() invocations +5, it, 0, [id=2] +5, it, 1, [id=3] +5, it, 2, [id=4] +5, it, 3, [id=5] +5, done diff --git a/testing/btest/Baseline/language.vector-iterate-record-default/output b/testing/btest/Baseline/language.vector-iterate-record-default/output new file mode 100644 index 0000000000..16e0cd6b01 --- /dev/null +++ b/testing/btest/Baseline/language.vector-iterate-record-default/output @@ -0,0 +1,12 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +0, populating vector, expecting 4 my_seq() invocations +0, my_seq() invoked +1, my_seq() invoked +2, my_seq() invoked +3, my_seq() invoked +4, iterating vector, expecting no my_seq() invocations +4, it, 0, [id=1] +4, it, 1, [id=2] +4, it, 2, [id=3] +4, it, 3, [id=4] +4, done diff --git a/testing/btest/language/table-iterate-record-key-default.zeek b/testing/btest/language/table-iterate-record-key-default.zeek new file mode 100644 index 0000000000..2dcd848a39 --- /dev/null +++ b/testing/btest/language/table-iterate-record-key-default.zeek @@ -0,0 +1,95 @@ +# @TEST-DOC: Iterating over tables with record keys would previously evaluate &default during each iteration. Ensure this isn't happening. Regression test for #3267. +# @TEST-EXEC: zeek -b %INPUT >output +# @TEST-EXEC: btest-diff output + +global seq = 0; +function my_seq(): count { + print seq, "my_seq() invoked"; + return ++seq; +} + +type R: record { + id: count &default=my_seq(); +}; + +global tbl: table[R] of count; + +print seq, "populating table, expecting 4 my_seq() invocations"; + +tbl[R()] = 1; +tbl[R()] = 2; +tbl[R()] = 3; +tbl[R()] = 4; + +print seq, "iterating table, expecting no my_seq() invocations"; +for ( [r], c in tbl ) + print seq, "it", r, c; + +print seq, "done"; + +# @TEST-START-NEXT +# +# This acts subtly different - my_seq() is called maybe for the [r] local +# in the for loop?! +global seq = 0; +function my_seq(): count { + print seq, "my_seq() invoked"; + return ++seq; +} + +type R: record { + id: count &default=my_seq(); +}; + +global tbl: table[R] of count; + + event zeek_init() + { + print seq, "populating table, expecting 4 my_seq() invocations"; + + tbl[R()] = 1; + tbl[R()] = 2; + tbl[R()] = 3; + tbl[R()] = 4; + + print seq, "iterating table, expecting no my_seq() invocations"; + for ( [r], c in tbl ) + print seq, "it", r, c; + + print seq, "done"; + } + +print seq, "done parsing"; + +# @TEST-START-NEXT +# +# table[R] of R +global seq = 0; +function my_seq(): count { + print seq, "my_seq() invoked"; + return ++seq; +} + +type R: record { + id: count &default=my_seq(); +}; + +global tbl: table[R] of R; + + event zeek_init() + { + print seq, "populating table, expecting 8 my_seq() invocations"; + + tbl[R()] = R(); + tbl[R()] = R(); + tbl[R()] = R(); + tbl[R()] = R(); + + print seq, "iterating table, expecting no my_seq() invocations"; + for ( [r1], r2 in tbl ) + print seq, "it", r1, r2; + + print seq, "done"; + } + +print seq, "done parsing"; diff --git a/testing/btest/language/vector-iterate-record-default.zeek b/testing/btest/language/vector-iterate-record-default.zeek new file mode 100644 index 0000000000..2bcc4bf309 --- /dev/null +++ b/testing/btest/language/vector-iterate-record-default.zeek @@ -0,0 +1,59 @@ +# @TEST-DOC: Iterating over vectors holding record. This mirrors table-iterate-record-key-default, but for vectors. They didn't have the same issue. Regression test for #3267. +# @TEST-EXEC: zeek -b %INPUT >output +# @TEST-EXEC: btest-diff output + +global seq = 0; +function my_seq(): count { + print seq, "my_seq() invoked"; + return ++seq; +} + +type R: record { + id: count &default=my_seq(); +}; + +global vec: vector of R; + +print seq, "populating vector, expecting 4 my_seq() invocations"; +vec += R(); +vec += R(); +vec += R(); +vec += R(); + +print seq, "iterating vector, expecting no my_seq() invocations"; +for ( i, r in vec ) + print seq, "it", i, r; + +print seq, "done"; + +# @TEST-START-NEXT +# +# Same as above, but populate / iterate record in zeek_init. +global seq = 0; +function my_seq(): count { + print seq, "my_seq() invoked"; + return ++seq; +} + +type R: record { + id: count &default=my_seq(); +}; + +global vec: vector of R; + +event zeek_init() + { + print seq, "populating vector, expecting 4 my_seq() invocations"; + vec += R(); + vec += R(); + vec += R(); + vec += R(); + + print seq, "iterating vector, expecting no my_seq() invocations"; + for ( i, r in vec ) + print seq, "it", i, r; + + print seq, "done"; + } + +print seq, "done parsing";