fixes for specialized ZAM operations needing to check whether record fields exist

src/Val.h

@@ -1269,7 +1269,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/constructors.op

@@ -76,7 +76,7 @@ macro	AssignFromRec(rhs)
 	auto is_managed = Z_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] )
 			{
@@ -156,7 +156,7 @@ class VV
 op-types R R
 eval	SetUpRecFieldOps(map)
 	for ( size_t i = 0U; i < n; ++i )
-		$1->RawOptField(lhs_map[i]) = $2->RawField(rhs_map[i]);
+		$1->RawOptField(lhs_map[i]) = FieldValWithCheck($2, rhs_map[i]);
 
 macro DoManagedRecAssign(lhs, rhs)
 	auto is_managed = Z_AUX->is_managed;
@@ -164,14 +164,14 @@ macro DoManagedRecAssign(lhs, rhs)
 		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
@@ -188,7 +188,7 @@ eval	SetUpRecFieldOps(map)
 	for ( size_t i = 0U; i < n; ++i )
 		{
 		auto& lhs_i = $1->RawOptField(lhs_map[i]);
-		auto rhs_i = $2->RawField(rhs_map[i]);
+		auto rhs_i = FieldValWithCheck($2, rhs_map[i]);
 		zeek::Ref(rhs_i.ManagedVal());
 		if ( lhs_i )
 			ZVal::DeleteManagedType(*lhs_i);
@@ -201,7 +201,9 @@ class VV
 op-types R R
 eval	SetUpRecFieldOps(map)
 	for ( size_t i = 0U; i < n; ++i )
-		$1->RawField(lhs_map[i]).AsIntRef() += $2->RawField(rhs_map[i]).AsInt();
+		$1->RawField(lhs_map[i]).AsIntRef() =
+			FieldValWithCheck($1, lhs_map[i]).AsInt() +
+			FieldValWithCheck($2, rhs_map[i]).AsInt();
 
 op Rec-Add-Double-Fields
 op1-read
@@ -209,7 +211,9 @@ class VV
 op-types R R
 eval	SetUpRecFieldOps(map)
 	for ( size_t i = 0U; i < n; ++i )
-		$1->RawField(lhs_map[i]).AsDoubleRef() += $2->RawField(rhs_map[i]).AsDouble();
+		$1->RawField(lhs_map[i]).AsDoubleRef() =
+			FieldValWithCheck($1, lhs_map[i]).AsDouble() +
+			FieldValWithCheck($2, rhs_map[i]).AsDouble();
 
 op Rec-Add-Fields
 op1-read
@@ -219,8 +223,10 @@ eval	SetUpRecFieldOps(map)
 	auto& types = Z_AUX->types;
 	for ( size_t i = 0U; i < n; ++i )
 		{
+		// We make this call to ensure that the field exists.
+		(void) FieldValWithCheck($1, lhs_map[i]);
 		auto& lhs_i = $1->RawField(lhs_map[i]);
-		auto rhs_i = $2->RawField(rhs_map[i]);
+		auto rhs_i = FieldValWithCheck($2, rhs_map[i]);
 		auto tag = types[i]->Tag();
 		if ( tag == TYPE_INT )
 			lhs_i.AsIntRef() += rhs_i.AsInt();

src/script_opt/ZAM/OPs/macros.op

@@ -71,4 +71,6 @@ macro TableIter(slot) (*tiv_ptr)[slot]
 macro DirectField(r, f) r->RawField(f)
 macro DirectOptField(r, f) r->RawOptField(f)
 
+macro FieldValWithCheck(r, f) ZBody::CheckAndLookupField(r, f, Z_LOC)
+
 macro LogEnum(v) v.ToVal(ZAM::log_ID_enum_type)

src/script_opt/ZAM/OPs/script-idioms.op

@@ -34,10 +34,16 @@ internal-op Func-Id-String
 class VV
 op-types S R
 eval	auto id_rec = $1;
-	auto orig_h = DirectField(id_rec, 0).AsAddr()->AsAddr().AsString();
+	auto orig_h =
-	auto resp_h = DirectField(id_rec, 2).AsAddr()->AsAddr().AsString();
+		FieldValWithCheck(id_rec, 0).AsAddr()->AsAddr().AsString();
-	auto orig_p = static_cast<uint32_t>(DirectField(id_rec, 1).AsCount()) & ~PORT_SPACE_MASK;
+	auto resp_h =
-	auto resp_p = static_cast<uint32_t>(DirectField(id_rec, 3).AsCount()) & ~PORT_SPACE_MASK;
+		FieldValWithCheck(id_rec, 2).AsAddr()->AsAddr().AsString();
+	auto orig_p =
+		static_cast<uint32_t>(FieldValWithCheck(id_rec, 1).AsCount()) &
+			~PORT_SPACE_MASK;
+	auto resp_p =
+		static_cast<uint32_t>(FieldValWithCheck(id_rec, 3).AsCount()) &
+			~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

@@ -65,6 +65,20 @@ public:
 private:
     friend class CPPCompile;
 
+    // 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;
+    }
+
     auto Instructions() const { return insts; }
     auto NumInsts() const { return end_pc; }
 

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