Make CompHash computation/recovery for functions deterministic

Functions are now assigned a unique integer on construction which
CompositeHash can base hashes on.  Recovery then just involves
looking up the function pointer associated with that unique number.

src/CompHash.cc

@@ -5,6 +5,7 @@
 #include "CompHash.h"
 #include "Val.h"
 #include "Reporter.h"
+#include "Func.h"
 
 CompositeHash::CompositeHash(TypeList* composite_type)
 	{
@@ -156,10 +157,8 @@ char* CompositeHash::SingleValHash(int type_check, char* kp0,
 		{
 		if ( v->Type()->Tag() == TYPE_FUNC )
 			{
-			Val** kp = AlignAndPadType<Val*>(kp0);
+			uint32* kp = AlignAndPadType<uint32>(kp0);
-			v->Ref();
+			*kp = v->AsFunc()->GetUniqueFuncID();
-			// Ref((BroObj*) v->AsFunc());
-			*kp = v;
 			kp1 = reinterpret_cast<char*>(kp+1);
 			}
 
@@ -311,7 +310,7 @@ HashKey* CompositeHash::ComputeSingletonHash(const Val* v, int type_check) const
 	case TYPE_INTERNAL_VOID:
 	case TYPE_INTERNAL_OTHER:
 		if ( v->Type()->Tag() == TYPE_FUNC )
-			return new HashKey(v);
+			return new HashKey(v->AsFunc()->GetUniqueFuncID());
 
 		reporter->InternalError("bad index type in CompositeHash::ComputeSingletonHash");
 		return 0;
@@ -377,7 +376,7 @@ int CompositeHash::SingleTypeKeySize(BroType* bt, const Val* v,
 	case TYPE_INTERNAL_OTHER:
 		{
 		if ( bt->Tag() == TYPE_FUNC )
-			sz = SizeAlign(sz, sizeof(Val*));
+			sz = SizeAlign(sz, sizeof(uint32));
 
 		else if ( bt->Tag() == TYPE_RECORD )
 			{
@@ -639,30 +638,35 @@ const char* CompositeHash::RecoverOneVal(const HashKey* k, const char* kp0,
 		{
 		if ( t->Tag() == TYPE_FUNC )
 			{
-			Val* const * const kp = AlignType<Val*>(kp0);
+			const uint32* const kp = AlignType<uint32>(kp0);
 			kp1 = reinterpret_cast<const char*>(kp+1);
 
-			Val* v = *kp;
+			Func* f = Func::GetFuncPtr(*kp);
 
-			if ( ! v || ! v->Type() )
+			if ( f )
-				reporter->InternalError("bad aggregate Val in CompositeHash::RecoverOneVal()");
-
-			if ( t->Tag() != TYPE_FUNC &&
-			     // ### Maybe fix later, but may be fundamentally
-			     // un-checkable --US
-			     ! same_type(v->Type(), t) )
 				{
-				reporter->InternalError("inconsistent aggregate Val in CompositeHash::RecoverOneVal()");
+				pval = new Val(f);
+
+				if ( ! pval->Type() )
+					reporter->InternalError("bad aggregate Val in CompositeHash::RecoverOneVal()");
+
+				else if ( t->Tag() != TYPE_FUNC &&
+				          ! same_type(pval->Type(), t) )
+					// ### Maybe fix later, but may be fundamentally
+					// un-checkable --US
+					reporter->InternalError("inconsistent aggregate Val in CompositeHash::RecoverOneVal()");
+
+				// ### A crude approximation for now.
+				else if ( t->Tag() == TYPE_FUNC &&
+				          pval->Type()->Tag() != TYPE_FUNC )
+					reporter->InternalError("inconsistent aggregate Val in CompositeHash::RecoverOneVal()");
 				}
 
-			// ### A crude approximation for now.
+			else
-			if ( t->Tag() == TYPE_FUNC &&
-			     v->Type()->Tag() != TYPE_FUNC )
 				{
-				reporter->InternalError("inconsistent aggregate Val in CompositeHash::RecoverOneVal()");
+				reporter->InternalError("failed to look up unique function id %"PRIu32" in CompositeHash::RecoverOneVal()");
+				pval = 0;
 				}
-
-			pval = v->Ref();
 			}
 
 		else if ( t->Tag() == TYPE_RECORD )

src/Func.cc

@@ -53,6 +53,19 @@ extern	RETSIGTYPE sig_handler(int signo);
 const Expr* calling_expr = 0;
 bool did_builtin_init = false;
 
+vector<Func*> Func::unique_ids;
+
+Func::Func() : scope(0), id(0), return_value(0)
+	{
+	unique_id = unique_ids.size();
+	unique_ids.push_back(this);
+	}
+
+Func::Func(Kind arg_kind) : scope(0), kind(arg_kind), id(0), return_value(0)
+	{
+	unique_id = unique_ids.size();
+	unique_ids.push_back(this);
+	}
 
 Func::~Func()
 	{

src/Func.h

@@ -20,8 +20,7 @@ public:
 
 	enum Kind { BRO_FUNC, BUILTIN_FUNC };
 
-	Func(Kind arg_kind)
+	Func(Kind arg_kind);
-		{ scope = 0; kind = arg_kind; id = 0; return_value = 0; }
 
 	virtual ~Func();
 
@@ -68,8 +67,12 @@ public:
 	ID* GetReturnValueID() const;
 	virtual TraversalCode Traverse(TraversalCallback* cb) const;
 
+	uint32 GetUniqueFuncID() const { return unique_id; }
+	static Func* GetFuncPtr(size_t id)
+		{ return id >= unique_ids.size() ? 0 : unique_ids[id]; }
+
 protected:
-	Func()	{ scope = 0; id = 0; return_value = 0; }
+	Func();
 
 	DECLARE_ABSTRACT_SERIAL(Func);
 
@@ -78,6 +81,8 @@ protected:
 	Kind kind;
 	ID* id;
 	ID* return_value;
+	uint32 unique_id;
+	static vector<Func*> unique_ids;
 };
 
 

testing/btest/Baseline/scripts.base.frameworks.notice.default-policy-order/notice_policy.log

@@ -0,0 +1,11 @@
+#separator \x09
+#path	notice_policy
+#fields	position	priority	result	pred	halt	suppress_for
+#types	count	count	enum	func	bool	interval
+0	10	Notice::ACTION_NONE	anonymous-function\x0a{ \x0areturn ((Notice::n$note in Notice::lookup_location_types));\x0a}	F	-
+1	9	Notice::ACTION_NO_SUPPRESS	anonymous-function\x0a{ \x0areturn ((Notice::n$note in Notice::not_suppressed_types));\x0a}	F	-
+2	9	Notice::ACTION_NONE	anonymous-function\x0a{ \x0areturn ((Notice::n$note in Notice::ignored_types));\x0a}	T	-
+3	8	Notice::ACTION_EMAIL	anonymous-function\x0a{ \x0areturn ((Notice::n$note in Notice::emailed_types));\x0a}	F	-
+4	8	Notice::ACTION_ALARM	anonymous-function\x0a{ \x0areturn ((Notice::n$note in Notice::alarmed_types));\x0a}	F	-
+5	8	Notice::ACTION_NONE	anonymous-function\x0a{ \x0aif (Notice::n$note in Notice::type_suppression_intervals) \x0a\x09{ \x0a\x09Notice::n$suppress_for = Notice::type_suppression_intervals[Notice::n$note];\x0a\x09return (T);\x0a\x09}\x0a\x0areturn (F);\x0a}	F	-
+6	0	Notice::ACTION_LOG	-	F	-

testing/btest/Baseline/scripts.base.frameworks.notice.suppression-disable/notice.log

@@ -2,5 +2,5 @@
 #path	notice
 #fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	note	msg	sub	src	dst	p	n	peer_descr	actions	policy_items	suppress_for	dropped	remote_location.country_code	remote_location.region	remote_location.city	remote_location.latitude	remote_location.longitude
 #types	time	string	addr	port	addr	port	enum	string	string	addr	addr	port	count	string	table	table	interval	bool	string	string	string	double	double
-1316950520.397561	-	-	-	-	-	Test_Notice	test	-	-	-	-	-	bro	Notice::ACTION_NO_SUPPRESS,Notice::ACTION_LOG	6,1	3600.000000	-	-	-	-	-	-
+1317927277.508920	-	-	-	-	-	Test_Notice	test	-	-	-	-	-	bro	Notice::ACTION_NO_SUPPRESS,Notice::ACTION_LOG	6,1	3600.000000	-	-	-	-	-	-
-1316950520.397561	-	-	-	-	-	Test_Notice	another test	-	-	-	-	-	bro	Notice::ACTION_NO_SUPPRESS,Notice::ACTION_LOG	6,1	3600.000000	-	-	-	-	-	-
+1317927277.508920	-	-	-	-	-	Test_Notice	another test	-	-	-	-	-	bro	Notice::ACTION_NO_SUPPRESS,Notice::ACTION_LOG	6,1	3600.000000	-	-	-	-	-	-

testing/btest/scripts/base/frameworks/notice/default-policy-order.test

@@ -0,0 +1,4 @@
+# This test checks that the default notice policy ordering is the same
+# as a known baseline.
+# @TEST-EXEC: bro -e ''
+# @TEST-EXEC: btest-diff notice_policy.log