Merge remote-tracking branch 'origin/topic/vern/zam-asan-fixes'

* origin/topic/vern/zam-asan-fixes:
  ZAM fixes for assignments involving "any" record fields
  fixes for (mostly ZAM) vector operation issues found by ASAN

Including a fix for mmdb/explicit-open.zeek to avoid using assert.

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)

VERSION

@@ -1 +1 @@
-7.1.0-dev.501
+7.1.0-dev.505

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 {

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<CoerceToAnyExpr>(op2), op2);
+
         auto lhs_e = field_e->Op()->Reduce(c, lhs_stmt);
         auto rhs_e = op2->ReduceToFieldAssignment(c, rhs_stmt);
 

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

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

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<std::optional<ZVal>>(n);
+	vector<std::optional<ZVal>> 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<VectorType>(Z_TYPE), res);
+	full_res = new VectorVal(cast_intrusive<VectorType>(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

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<std::optional<ZVal>>(n);
+    vector<std::optional<ZVal>> vec1(n);
-    auto& vec1 = *vec1_ptr;
 
     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<VectorType>(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<std::optional<ZVal>>(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<std::optional<ZVal>> 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<VectorType>(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);
 }
 

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]

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]

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.

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);
+	if ( ! mmdb_open_asn_db(asn_fn) )
-	assert mmdb_open_location_db(city_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);
 	}

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;
+	}

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;
+	}

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;
+	}