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

* origin/topic/vern/zam-record-fields-fixes:
  fixes for specialized ZAM operations needing to check whether record fields exist

(cherry picked from commit d7fbd49d9e)

CHANGES

@@ -1,3 +1,9 @@
+7.0.9-3 | 2025-08-22 13:31:54 -0700
+
+  * fixes for specialized ZAM operations needing to check whether record fields exist (Vern Paxson, Corelight)
+
+    (cherry picked from commit d7fbd49d9e0699685fc5be035b966b19867785f8)
+
 7.0.9-2 | 2025-08-22 13:02:51 -0700
 
   * modified merge_types() to skip work if given identical types, which (Vern Paxson, Corelight)

VERSION

@@ -1 +1 @@
-7.0.9-2
+7.0.9-3

src/Val.h

@@ -1249,7 +1249,7 @@ public:
 
     /**
      * Returns the value of a given field index if it's previously been
-     * assigned, * or else returns the value created from evaluating the
+     * assigned, or else returns the value created from evaluating the
      * record field's &default expression.
      * @param field  The field index to retrieve.
      * @return  The value at the given field index or the default value if

src/script_opt/ZAM/OPs/ZAM.op

@@ -1247,13 +1247,16 @@ type V
 eval	auto init_vals = z.aux->ToZValVecWithMap(frame);
 	ConstructRecordPost()
 
+macro Z_LOC z.loc
+macro FieldValWithCheck(r, f) ZBody::CheckAndLookupField(r, f, Z_LOC)
+
 macro	AssignFromRec()
 	/* The following is defined below, for use by Rec-Assign-Fields */
 	SetUpRecFieldOps(lhs_map)
 	auto is_managed = aux->is_managed;
 	for ( size_t i = 0U; i < n; ++i )
 		{
-		auto rhs_i = rhs->RawField(rhs_map[i]);
+        auto rhs_i = FieldValWithCheck(rhs, rhs_map[i]);
 		auto& init_i = init_vals[lhs_map[i]];
 		if ( is_managed[i] )
 			{
@@ -1328,7 +1331,7 @@ op1-read
 type VV
 eval	SetUpRecFieldOps(map)
 	for ( size_t i = 0U; i < n; ++i )
-		lhs->RawOptField(lhs_map[i]) = rhs->RawField(rhs_map[i]);
+		lhs->RawOptField(lhs_map[i]) = FieldValWithCheck(rhs, rhs_map[i]);
 
 macro DoManagedRecAssign()
 	auto is_managed = aux->is_managed;
@@ -1336,14 +1339,14 @@ macro DoManagedRecAssign()
 		if ( is_managed[i] )
 			{
 			auto& lhs_i = lhs->RawOptField(lhs_map[i]);
-			auto rhs_i = rhs->RawField(rhs_map[i]);
+			auto rhs_i = FieldValWithCheck(rhs, rhs_map[i]);
 			zeek::Ref(rhs_i.ManagedVal());
 			if ( lhs_i )
 				ZVal::DeleteManagedType(*lhs_i);
 			lhs_i = rhs_i;
 			}
 		else
-			lhs->RawOptField(lhs_map[i]) = rhs->RawField(rhs_map[i]);
+			lhs->RawOptField(lhs_map[i]) = FieldValWithCheck(rhs, rhs_map[i]);
 op Rec-Assign-Fields-Managed
 op1-read
 type VV
@@ -1357,7 +1360,7 @@ eval	SetUpRecFieldOps(map)
 	for ( size_t i = 0U; i < n; ++i )
 		{
 		auto& lhs_i = lhs->RawOptField(lhs_map[i]);
-		auto rhs_i = rhs->RawField(rhs_map[i]);
+		auto rhs_i = FieldValWithCheck(rhs, rhs_map[i]);
 		zeek::Ref(rhs_i.ManagedVal());
 		if ( lhs_i )
 			ZVal::DeleteManagedType(*lhs_i);
@@ -1369,14 +1372,18 @@ op1-read
 type VV
 eval	SetUpRecFieldOps(map)
 	for ( size_t i = 0U; i < n; ++i )
-		lhs->RawField(lhs_map[i]).int_val += rhs->RawField(rhs_map[i]).int_val;
+		lhs->RawField(lhs_map[i]).int_val =
+            FieldValWithCheck(lhs, lhs_map[i]).AsInt() +
+            FieldValWithCheck(rhs, rhs_map[i]).AsInt();
 
 op Rec-Add-Double-Fields
 op1-read
 type VV
 eval	SetUpRecFieldOps(map)
 	for ( size_t i = 0U; i < n; ++i )
-		lhs->RawField(lhs_map[i]).double_val += rhs->RawField(rhs_map[i]).double_val;
+		lhs->RawField(lhs_map[i]).double_val =
+            FieldValWithCheck(lhs, lhs_map[i]).AsDouble() +
+            FieldValWithCheck(rhs, rhs_map[i]).AsDouble();
 
 op Rec-Add-Fields
 op1-read
@@ -1385,8 +1392,9 @@ eval	SetUpRecFieldOps(map)
 	auto& types = aux->types;
 	for ( size_t i = 0U; i < n; ++i )
 		{
+        (void) FieldValWithCheck(lhs, lhs_map[i]);
 		auto& lhs_i = lhs->RawField(lhs_map[i]);
-		auto rhs_i = rhs->RawField(rhs_map[i]);
+		auto rhs_i = FieldValWithCheck(rhs, rhs_map[i]);
 		auto tag = types[i]->Tag();
 		if ( tag == TYPE_INT )
 			lhs_i.int_val += rhs_i.int_val;
@@ -3318,10 +3326,10 @@ eval	SetBytesThresh(frame[z.v2].record_val, z.c.uint_val, zeek_uint_t(z.v3))
 internal-op Func-Id-String
 type VV
 eval	auto id_rec = frame[z.v2].record_val;
-	auto orig_h = id_rec->RawField(0).addr_val->AsAddr().AsString();
+    auto orig_h = FieldValWithCheck(id_rec, 0).addr_val->AsAddr().AsString();
-	auto resp_h = id_rec->RawField(2).addr_val->AsAddr().AsString();
+    auto resp_h = FieldValWithCheck(id_rec, 2).addr_val->AsAddr().AsString();
-	auto orig_p = static_cast<uint32_t>(id_rec->RawField(1).uint_val) & ~PORT_SPACE_MASK;
+    auto orig_p = static_cast<uint32_t>(FieldValWithCheck(id_rec, 1).uint_val & ~PORT_SPACE_MASK);
-	auto resp_p = static_cast<uint32_t>(id_rec->RawField(3).uint_val) & ~PORT_SPACE_MASK;
+    auto resp_p = static_cast<uint32_t>(FieldValWithCheck(id_rec, 3).uint_val & ~PORT_SPACE_MASK);
 	/* Maximum address size is for IPv6 with no compression. Each
 	 * 8 16-bit hex elements plus 7 colons between them plus the two []'s
 	 * = 8*4 + 7 + 2 = 41 characters.

src/script_opt/ZAM/ZBody.h

@@ -80,6 +80,20 @@ private:
     void StmtDescribe(ODesc* d) const override;
     TraversalCode Traverse(TraversalCallback* cb) const override;
 
+    // Helper run-time function for looking up a field in a record, checking
+    // that it exists and complaining if it does not. A member here rather than
+    // a standalone run-time function because ZBody is a "friend" of RecordVal
+    // and can use its low-level record field accessors.
+    ZVal CheckAndLookupField(RecordVal* r, int f, const std::shared_ptr<ZAMLocInfo>& loc) {
+        auto opt_zv = r->RawOptField(f);
+        if ( ! opt_zv ) {
+            auto fn = r->GetType<RecordType>()->FieldName(f);
+            ZAM_run_time_error(loc, util::fmt("field value missing ($%s)", fn));
+        }
+
+        return *opt_zv;
+    }
+
     std::string func_name;
 
     const ZInst* insts = nullptr;

testing/btest/Baseline/opt.regress-missing-fields/error-messages

@@ -0,0 +1,7 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+runtime error in <...>/regress-missing-fields.zeek, line 11: field value missing ($orig_h)
+runtime error in <...>/regress-missing-fields.zeek, line 29: field value missing ($v1)
+runtime error in <...>/regress-missing-fields.zeek, line 39: field value missing ($v3)
+runtime error in <...>/regress-missing-fields.zeek, line 49: field value missing ($v4)
+runtime error in <...>/regress-missing-fields.zeek, line 64: field value missing ($vv1)
+runtime error in <...>/regress-missing-fields.zeek, line 73: field value missing ($vv2)

testing/btest/opt/regress-missing-fields.zeek

@@ -0,0 +1,75 @@
+# @TEST-DOC: Regression test for specialized operations checking for missing record fields
+# @TEST-REQUIRES: test "${ZEEK_USE_CPP}" != "1"
+# @TEST-EXEC: zeek -O ZAM -b %INPUT >error-messages 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff error-messages
+
+@load base/utils/conn-ids.zeek
+
+event zeek_init()
+	{
+	local id: conn_id; # not initialized!
+	print id_string(id);
+	}
+
+# Testing for ZAM specialized operation for adding multiple record fields.
+type R1: record {
+	v1: count &optional;
+	v2: count &optional;
+	v3: count &optional;
+	v4: count &optional;
+};
+
+event zeek_init()
+	{
+	local l1: R1;
+	local l2: R1;
+
+	# Both LHS and RHS are uninitialized, so use the same fields
+	# because we don't presuppose which one generates the error first.
+	l2$v1 += l1$v1;
+	l2$v2 += l1$v2; # We never get here
+	}
+
+event zeek_init()
+	{
+	local l1 = R1($v1 = 1);
+	local l2: R1;
+
+	# Should report v3, since v1 is good-to-go.
+	l2$v3 += l1$v1;
+	l2$v2 += l1$v2; # We never get here
+	}
+
+event zeek_init()
+	{
+	local l1: R1;
+	local l2 = R1($v1 = 1);
+
+	# Should report v4, since v1 is good-to-go.
+	l2$v1 += l1$v4;
+	l2$v2 += l1$v2; # We never get here
+	}
+
+# Testing for ZAM specialized operation for assigning multiple record fields.
+type R2: record {
+	vv1: vector of count &optional;
+	vv2: vector of count &optional;
+};
+
+event zeek_init()
+	{
+	local l1: R2;
+	local l2: R2;
+
+	l2$vv1 = l1$vv1;
+	l2$vv2 = l1$vv2; # We don't get here
+	}
+
+event zeek_init()
+	{
+	local l1 = R2($vv1 = vector());
+	local l2: R2;
+
+	l2$vv1 = l1$vv1;
+	l2$vv2 = l1$vv2; # We should get here, but then fail
+	}