GH-1503: Improve &expire_func compatibility type-checking

Previously, incompatible &expire_funcs could mistakenly be used, such as
when using that attribute on the unspecified table()/set()
initializations/assignments, resulting in invalid function calls that
eventually crash Zeek.

src/Attr.cc

@@ -498,51 +498,7 @@ void Attributes::CheckAttr(Attr* a)
 			break;
 			}
 
-		const auto& expire_func = a->GetExpr();
+		type->AsTableType()->CheckExpireFuncCompatibility({NewRef{}, a});
-
-		if ( expire_func->GetType()->Tag() != TYPE_FUNC )
-			{
-			Error("&expire_func attribute is not a function");
-			break;
-			}
-
-		const FuncType* e_ft = expire_func->GetType()->AsFuncType();
-
-		if ( e_ft->Flavor() != FUNC_FLAVOR_FUNCTION )
-			{
-			Error("&expire_func attribute is not a function");
-			break;
-			}
-
-		if ( e_ft->Yield()->Tag() != TYPE_INTERVAL )
-			{
-			Error("&expire_func must yield a value of type interval");
-			break;
-			}
-
-		const TableType* the_table = type->AsTableType();
-
-		if (the_table->IsUnspecifiedTable())
-			break;
-
-		const auto& func_index_types = e_ft->ParamList()->GetTypes();
-		// Keep backwards compatibility with idx: any idiom.
-		if ( func_index_types.size() == 2 )
-			{
-			if (func_index_types[1]->Tag() == TYPE_ANY)
-				break;
-			}
-
-		const auto& table_index_types = the_table->GetIndexTypes();
-
-		TypePList expected_args(1 + static_cast<int>(table_index_types.size()));
-		expected_args.push_back(type->AsTableType());
-
-		for ( const auto& t : table_index_types )
-			expected_args.push_back(t.get());
-
-		if ( ! e_ft->CheckArgs(&expected_args) )
-			Error("&expire_func argument type clash");
 
 		if ( Find(ATTR_BROKER_STORE ) )
 			Error("&broker_store and &expire_func cannot be used simultaneously");

src/Type.cc

@@ -526,6 +526,61 @@ TableType::TableType(TypeListPtr ind, TypePtr yield)
 		}
 	}
 
+bool TableType::CheckExpireFuncCompatibility(const detail::AttrPtr& attr)
+	{
+	assert(attr->Tag() == detail::ATTR_EXPIRE_FUNC);
+
+	const auto& expire_func = attr->GetExpr();
+
+	if ( expire_func->GetType()->Tag() != TYPE_FUNC )
+		{
+		attr->Error("&expire_func attribute is not a function");
+		return false;
+		}
+
+	const FuncType* e_ft = expire_func->GetType()->AsFuncType();
+
+	if ( e_ft->Flavor() != FUNC_FLAVOR_FUNCTION )
+		{
+		attr->Error("&expire_func attribute is not a function");
+		return false;
+		}
+
+	if ( e_ft->Yield()->Tag() != TYPE_INTERVAL )
+		{
+		attr->Error("&expire_func must yield a value of type interval");
+		return false;
+		}
+
+	if ( IsUnspecifiedTable() )
+		return true;
+
+	const auto& func_index_types = e_ft->ParamList()->GetTypes();
+	// Keep backwards compatibility with idx: any idiom.
+	if ( func_index_types.size() == 2 )
+		{
+		if ( func_index_types[1]->Tag() == TYPE_ANY )
+			return true;
+		}
+
+	const auto& table_index_types = GetIndexTypes();
+
+	std::vector<TypePtr> expected_args;
+	expected_args.reserve(1 + table_index_types.size());
+	expected_args.emplace_back(NewRef{}, this);
+
+	for ( const auto& t : table_index_types )
+		expected_args.emplace_back(t);
+
+	if ( ! e_ft->CheckArgs(expected_args) )
+		{
+		attr->Error("&expire_func argument type clash");
+		return false;
+		}
+
+	return true;
+	}
+
 TypePtr TableType::ShallowClone()
 	{
 	return make_intrusive<TableType>(indices, yield_type);

src/Type.h

@@ -391,6 +391,15 @@ class TableType : public IndexType {
 public:
 	TableType(TypeListPtr ind, TypePtr yield);
 
+	/**
+	 * Assesses whether an &expire_func attribute's function type is compatible
+	 * with this table type.
+	 * @param attr  the &expire_func attribute to check (this method must not be
+	 * called with other type of attributes).
+	 * @return  true if compatible, false if not
+	 */
+	bool CheckExpireFuncCompatibility(const detail::AttrPtr& attr);
+
 	TypePtr ShallowClone() override;
 
 	// Returns true if this table type is "unspecified", which is

src/Val.cc

@@ -1502,7 +1502,12 @@ void TableVal::SetAttrs(detail::AttributesPtr a)
 	const auto& ef = attrs->Find(detail::ATTR_EXPIRE_FUNC);
 
 	if ( ef )
-		expire_func = ef->GetExpr();
+		{
+		if ( GetType()->AsTableType()->CheckExpireFuncCompatibility(ef) )
+			expire_func = ef->GetExpr();
+		else
+			expire_func = nullptr;
+		}
 
 	const auto& cf = attrs->Find(detail::ATTR_ON_CHANGE);
 

testing/btest/Baseline/language.expire-func-type-check/output

@@ -1,4 +1,14 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
 error in <...>/expire-func-type-check.zeek, line 12: &expire_func attribute is not a function (&expire_func=myev)
+error in <...>/expire-func-type-check.zeek, line 12: &expire_func attribute is not a function (&expire_func=myev)
+error in <...>/expire-func-type-check.zeek, line 12: &expire_func attribute is not a function (&expire_func=myev)
+error in <...>/expire-func-type-check.zeek, line 13: &expire_func attribute is not a function (&expire_func=myhk)
+error in <...>/expire-func-type-check.zeek, line 13: &expire_func attribute is not a function (&expire_func=myhk)
 error in <...>/expire-func-type-check.zeek, line 13: &expire_func attribute is not a function (&expire_func=myhk)
 error in <...>/expire-func-type-check.zeek, line 14: &expire_func attribute is not a function (&expire_func=foo)
+error in <...>/expire-func-type-check.zeek, line 14: &expire_func attribute is not a function (&expire_func=foo)
+error in <...>/expire-func-type-check.zeek, line 14: &expire_func attribute is not a function (&expire_func=foo)
+warning in <...>/expire-func-type-check.zeek, line 17: Wrong number of arguments for function. Expected 3, got 2. (function(t:table[addr,port] of set[addr]; s:set[addr,port];) : interval)
+error in <...>/expire-func-type-check.zeek, line 18: &expire_func argument type clash (&expire_func=expire_ce)
+warning in <...>/expire-func-type-check.zeek, line 17: Wrong number of arguments for function. Expected 3, got 2. (function(t:table[addr,port] of set[addr]; s:set[addr,port];) : interval)
+error in <...>/expire-func-type-check.zeek, line 18: &expire_func argument type clash (&expire_func=expire_ce)

testing/btest/language/expire-func-type-check.zeek

@@ -12,3 +12,7 @@ global foo = 3;
 global t: table[string] of count &expire_func=myev;
 global tt: table[string] of count &expire_func=myhk;
 global ttt: table[string] of count &expire_func=foo;
+
+type ap: set[addr, port];
+global expire_ce: function(t: table[addr,port] of set[addr], s: ap): interval;
+global ce: table[addr, port] of set[addr]=table() &create_expire=1 secs &expire_func=expire_ce;