diff --git a/CHANGES b/CHANGES index 0848e420b7..6ae617acdc 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,9 @@ +7.1.0-dev.505 | 2024-11-12 10:26:15 +0100 + + * ZAM fixes for assignments involving "any" record fields (Vern Paxson, Corelight) + + * fixes for (mostly ZAM) vector operation issues found by ASAN (Vern Paxson, Corelight) + 7.1.0-dev.501 | 2024-11-11 21:05:41 +0100 * GH-4006: Fix nullptr deref in Spicy accept/decline input (Evan Typanski, Corelight) diff --git a/VERSION b/VERSION index 4f4e18af22..50f3af2132 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -7.1.0-dev.501 +7.1.0-dev.505 diff --git a/src/Val.cc b/src/Val.cc index 1844593a5f..bdbe3ceb35 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -3251,10 +3251,11 @@ bool VectorVal::Assign(unsigned int index, ValPtr element) { if ( yield_types ) { const auto& t = element->GetType(); - (*yield_types)[index] = t; + auto& yt_i = (*yield_types)[index]; auto& elem = vector_val[index]; if ( elem ) - ZVal::DeleteIfManaged(*elem, t); + ZVal::DeleteIfManaged(*elem, yt_i); + yt_i = t; elem = ZVal(std::move(element), t); } else { diff --git a/src/script_opt/Expr.cc b/src/script_opt/Expr.cc index 5a04a3bef1..861468b91d 100644 --- a/src/script_opt/Expr.cc +++ b/src/script_opt/Expr.cc @@ -1652,6 +1652,9 @@ ExprPtr AssignExpr::Reduce(Reducer* c, StmtPtr& red_stmt) { StmtPtr lhs_stmt; StmtPtr rhs_stmt; + if ( GetType()->Tag() == TYPE_ANY && op2->GetType()->Tag() != TYPE_ANY ) + op2 = with_location_of(make_intrusive(op2), op2); + auto lhs_e = field_e->Op()->Reduce(c, lhs_stmt); auto rhs_e = op2->ReduceToFieldAssignment(c, rhs_stmt); diff --git a/src/script_opt/ZAM/OPs/aggr-assignments.op b/src/script_opt/ZAM/OPs/aggr-assignments.op index c64b0077a6..b4c39c1fd9 100644 --- a/src/script_opt/ZAM/OPs/aggr-assignments.op +++ b/src/script_opt/ZAM/OPs/aggr-assignments.op @@ -20,7 +20,19 @@ op Any-Vector-Elem-Assign op1-read set-type $1 classes VVV VVC -eval EvalVectorElemAssign($1, $2,, vv->Assign(ind, $3.ToVal(Z_TYPE))) +eval auto ind = $2.AsCount(); + auto vv = $1.AsVector(); + auto yt = vv->RawYieldTypes(); + if ( ind < vv->Size() && yt && (*yt)[ind] && ZVal::IsManagedType((*yt)[ind]) ) + { + auto orig_elem = vv->RawVec()[ind]; + if ( ! vv->Assign(ind, $3.ToVal(Z_TYPE)) ) + ERROR("value used but not set"); + if ( orig_elem ) + ZVal::DeleteManagedType(*orig_elem); + } + else if ( ! vv->Assign(ind, $3.ToVal(Z_TYPE)) ) + ERROR("value used but not set"); op Vector-Elem-Assign-Any op1-read diff --git a/src/script_opt/ZAM/OPs/constructors.op b/src/script_opt/ZAM/OPs/constructors.op index 6220784a42..700daa3312 100644 --- a/src/script_opt/ZAM/OPs/constructors.op +++ b/src/script_opt/ZAM/OPs/constructors.op @@ -77,9 +77,14 @@ macro AssignFromRec(rhs) for ( size_t i = 0U; i < n; ++i ) { auto rhs_i = rhs->RawField(rhs_map[i]); + auto& init_i = init_vals[lhs_map[i]]; if ( is_managed[i] ) + { zeek::Ref(rhs_i.ManagedVal()); - init_vals[lhs_map[i]] = rhs_i; + if ( init_i ) + ZVal::DeleteManagedType(*init_i); + } + init_i = rhs_i; } op Construct-Known-Record-From diff --git a/src/script_opt/ZAM/OPs/non-uniform.op b/src/script_opt/ZAM/OPs/non-uniform.op index f53cc6f57a..eadeb7d162 100644 --- a/src/script_opt/ZAM/OPs/non-uniform.op +++ b/src/script_opt/ZAM/OPs/non-uniform.op @@ -241,13 +241,13 @@ eval auto& vsel = $1->RawVec(); auto& v1 = $2->RawVec(); auto& v2 = $3->RawVec(); auto n = v1.size(); - auto res = new vector>(n); + vector> res(n); for ( auto i = 0U; i < n; ++i ) if ( vsel[i] ) - (*res)[i] = vsel[i]->AsInt() ? v1[i] : v2[i]; + res[i] = vsel[i]->AsInt() ? v1[i] : v2[i]; auto& full_res = $$; Unref(full_res); - full_res = new VectorVal(cast_intrusive(Z_TYPE), res); + full_res = new VectorVal(cast_intrusive(Z_TYPE), &res); # Our instruction format doesn't accommodate two constants, so for # the singular case of a V ? C1 : C2 conditional, we split it into diff --git a/src/script_opt/ZAM/ZBody.cc b/src/script_opt/ZAM/ZBody.cc index d1528c9d12..4e1ff4f63d 100644 --- a/src/script_opt/ZAM/ZBody.cc +++ b/src/script_opt/ZAM/ZBody.cc @@ -205,8 +205,9 @@ static void vec_exec(ZOp op, TypePtr t, VectorVal*& v1, const VectorVal* v2, con std::string err = "overflow promoting from "; \ err += ov_err; \ err += " arithmetic value"; \ + /* The run-time error will throw an exception, so recover intermediary memory. */ \ + delete res_zv; \ ZAM_run_time_error(z_loc, err.c_str()); \ - res[i] = std::nullopt; \ } \ else \ res[i] = ZVal(cast(vi)); \ @@ -594,8 +595,7 @@ static void vec_exec(ZOp op, TypePtr t, VectorVal*& v1, const VectorVal* v2, con auto& vec2 = v2->RawVec(); auto n = vec2.size(); - auto vec1_ptr = new vector>(n); - auto& vec1 = *vec1_ptr; + vector> vec1(n); for ( auto i = 0U; i < n; ++i ) { if ( vec2[i] ) @@ -610,7 +610,7 @@ static void vec_exec(ZOp op, TypePtr t, VectorVal*& v1, const VectorVal* v2, con auto vt = cast_intrusive(std::move(t)); auto old_v1 = v1; - v1 = new VectorVal(std::move(vt), vec1_ptr); + v1 = new VectorVal(std::move(vt), &vec1); Unref(old_v1); } @@ -621,8 +621,13 @@ static void vec_exec(ZOp op, TypePtr t, VectorVal*& v1, const VectorVal* v2, con auto& vec2 = v2->RawVec(); auto& vec3 = v3->RawVec(); auto n = vec2.size(); - auto vec1_ptr = new vector>(n); - auto& vec1 = *vec1_ptr; + + if ( vec3.size() != n ) { + ZAM_run_time_error(util::fmt("vector operands are of different sizes (%d vs. %d)", int(n), int(vec3.size()))); + return; + } + + vector> vec1(n); for ( auto i = 0U; i < vec2.size(); ++i ) { if ( vec2[i] && vec3[i] ) @@ -637,7 +642,7 @@ static void vec_exec(ZOp op, TypePtr t, VectorVal*& v1, const VectorVal* v2, con auto vt = cast_intrusive(std::move(t)); auto old_v1 = v1; - v1 = new VectorVal(std::move(vt), vec1_ptr); + v1 = new VectorVal(std::move(vt), &vec1); Unref(old_v1); } diff --git a/testing/btest/Baseline/opt.regress-any-leak/output b/testing/btest/Baseline/opt.regress-any-leak/output new file mode 100644 index 0000000000..d5c6e60615 --- /dev/null +++ b/testing/btest/Baseline/opt.regress-any-leak/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. +[[a=abc-1]] +[1] diff --git a/testing/btest/Baseline/opt.regress-any/output b/testing/btest/Baseline/opt.regress-any/output new file mode 100644 index 0000000000..9627885028 --- /dev/null +++ b/testing/btest/Baseline/opt.regress-any/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. +[a=123, b=[abc]] +[a=123, b=1] diff --git a/testing/btest/Baseline/opt.regress-vector-mismatch/output b/testing/btest/Baseline/opt.regress-vector-mismatch/output new file mode 100644 index 0000000000..49d861c74c --- /dev/null +++ b/testing/btest/Baseline/opt.regress-vector-mismatch/output @@ -0,0 +1 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. diff --git a/testing/btest/core/mmdb/explicit-open.zeek b/testing/btest/core/mmdb/explicit-open.zeek index 8300ef1604..6f5713ed12 100644 --- a/testing/btest/core/mmdb/explicit-open.zeek +++ b/testing/btest/core/mmdb/explicit-open.zeek @@ -45,6 +45,9 @@ event new_packet(c: connection, p: pkt_hdr) event zeek_init() { - assert mmdb_open_asn_db(asn_fn); - assert mmdb_open_location_db(city_fn); + if ( ! mmdb_open_asn_db(asn_fn) ) + Reporter::fatal("failed to open asn_db " + asn_fn); + + if ( ! mmdb_open_location_db(city_fn) ) + Reporter::fatal("failed to open location db " + city_fn); } diff --git a/testing/btest/opt/regress-any-leak.zeek b/testing/btest/opt/regress-any-leak.zeek new file mode 100644 index 0000000000..118356aa07 --- /dev/null +++ b/testing/btest/opt/regress-any-leak.zeek @@ -0,0 +1,16 @@ +# @TEST-DOC: Regression test for leak when mixing "any" types (affected both ZAM and non-ZAM) +# @TEST-EXEC: zeek -b -O ZAM %INPUT >output +# @TEST-EXEC: btest-diff output + +type X: record { + a: string; +}; + +event zeek_init() + { + local vec: vector of any; + vec += X($a="abc-1"); + print vec; + vec[0] = 1; + print vec; + } diff --git a/testing/btest/opt/regress-any.zeek b/testing/btest/opt/regress-any.zeek new file mode 100644 index 0000000000..98c4116392 --- /dev/null +++ b/testing/btest/opt/regress-any.zeek @@ -0,0 +1,16 @@ +# @TEST-DOC: Regression test for reassigning an "any" field +# @TEST-EXEC: zeek -b -O ZAM %INPUT >output +# @TEST-EXEC: btest-diff output + +type X: record { + a: string; + b: any; +}; + +event zeek_init() + { + local x = X($a="123", $b=vector("abc")); + print x; + x$b = 1; + print x; + } diff --git a/testing/btest/opt/regress-vector-mismatch.zeek b/testing/btest/opt/regress-vector-mismatch.zeek new file mode 100644 index 0000000000..e07411750d --- /dev/null +++ b/testing/btest/opt/regress-vector-mismatch.zeek @@ -0,0 +1,17 @@ +# @TEST-DOC: Regression test for coercing vectors-of-any +# @TEST-EXEC: zeek -b -O ZAM %INPUT >output +# @TEST-EXEC: btest-diff output + +module X; + +export { + option o: vector of string = vector(); +} + +event zeek_init() + { + local x: any = vector(); + Config::set_value("X::o", vector("a") + (x as vector of string)); + print X::o; + print x; + }