GH-293: Protect copy() against reference cycles.

Reference cycles shouldn't occur but there's nothing really preventing
people from creating them, so may just as well be safe and deal with
them when cloning values. While the code is a bit more cumbersome this
way, it could actually be bit faster as well as it no longer caches
non-mutable values. (I measured it with the test suite: That's about
the same in execution time, maybe tiny little bit faster now;
definitly not slower).

src/OpaqueVal.cc

@@ -97,7 +97,7 @@ Val* MD5Val::DoClone(CloneState* state)
 		EVP_MD_CTX_copy_ex(out->ctx, ctx);
 		}
 
-	return out;
+	return state->NewClone(this, out);
 	}
 
 void MD5Val::digest(val_list& vlist, u_char result[MD5_DIGEST_LENGTH])
@@ -241,7 +241,7 @@ Val* SHA1Val::DoClone(CloneState* state)
 		EVP_MD_CTX_copy_ex(out->ctx, ctx);
 		}
 
-	return out;
+	return state->NewClone(this, out);
 	}
 
 void SHA1Val::digest(val_list& vlist, u_char result[SHA_DIGEST_LENGTH])
@@ -376,7 +376,7 @@ Val* SHA256Val::DoClone(CloneState* state)
 		EVP_MD_CTX_copy_ex(out->ctx, ctx);
 		}
 
-	return out;
+	return state->NewClone(this, out);
 	}
 
 void SHA256Val::digest(val_list& vlist, u_char result[SHA256_DIGEST_LENGTH])
@@ -517,7 +517,7 @@ Val* EntropyVal::DoClone(CloneState* state)
 	uinfo.cache = false;
 	Val* clone = Unserialize(&uinfo, type);
 	free(data);
-	return clone;
+	return state->NewClone(this, clone);
 	}
 
 bool EntropyVal::Feed(const void* data, size_t size)
@@ -639,10 +639,10 @@ Val* BloomFilterVal::DoClone(CloneState* state)
 		{
 		auto bf = new BloomFilterVal(bloom_filter->Clone());
 		bf->Typify(type);
-		return bf;
+		return state->NewClone(this, bf);
 		}
 
-	return new BloomFilterVal();
+	return state->NewClone(this, new BloomFilterVal());
 	}
 
 bool BloomFilterVal::Typify(BroType* arg_type)
@@ -801,7 +801,8 @@ CardinalityVal::~CardinalityVal()
 
 Val* CardinalityVal::DoClone(CloneState* state)
 	{
-	return new CardinalityVal(new probabilistic::CardinalityCounter(*c));
+	return state->NewClone(this,
+			       new CardinalityVal(new probabilistic::CardinalityCounter(*c)));
 	}
 
 IMPLEMENT_SERIAL(CardinalityVal, SER_CARDINALITY_VAL);

src/Val.cc

@@ -1155,8 +1155,8 @@ unsigned int StringVal::MemoryAllocation() const
 
 Val* StringVal::DoClone(CloneState* state)
 	{
-	return new StringVal(new BroString((u_char*) val.string_val->Bytes(),
-	                                   val.string_val->Len(), 1));
+	return state->NewClone(this, new StringVal(new BroString((u_char*) val.string_val->Bytes(),
+								 val.string_val->Len(), 1)));
 	}
 
 IMPLEMENT_SERIAL(StringVal, SER_STRING_VAL);
@@ -1226,7 +1226,7 @@ Val* PatternVal::DoClone(CloneState* state)
 	auto re = new RE_Matcher(val.re_val->PatternText(),
 	                         val.re_val->AnywherePatternText());
 	re->Compile();
-	return new PatternVal(re);
+	return state->NewClone(this, new PatternVal(re));
 	}
 
 IMPLEMENT_SERIAL(PatternVal, SER_PATTERN_VAL);
@@ -1331,6 +1331,7 @@ Val* ListVal::DoClone(CloneState* state)
 	{
 	auto lv = new ListVal(tag);
 	lv->vals.resize(vals.length());
+	state->NewClone(this, lv);
 
 	loop_over_list(vals, i)
 		lv->Append(vals[i]->Clone(state));
@@ -2541,6 +2542,7 @@ void TableVal::ReadOperation(Val* index, TableEntryVal* v)
 Val* TableVal::DoClone(CloneState* state)
 	{
 	auto tv = new TableVal(table_type);
+	state->NewClone(this, tv);
 
 	const PDict(TableEntryVal)* tbl = AsTable();
 	IterCookie* cookie = tbl->InitForIteration();
@@ -3158,6 +3160,7 @@ Val* RecordVal::DoClone(CloneState* state)
 	// we don't touch it.
 	auto rv = new RecordVal(Type()->AsRecordType(), false);
 	rv->origin = nullptr;
+	state->NewClone(this, rv);
 
 	loop_over_list(*val.val_list_val, i)
 		{
@@ -3454,6 +3457,7 @@ Val* VectorVal::DoClone(CloneState* state)
 	{
 	auto vv = new VectorVal(vector_type);
 	vv->val.vector_val->reserve(val.vector_val->size());
+	state->NewClone(this, vv);
 
 	for ( unsigned int i = 0; i < val.vector_val->size(); ++i )
 		{

src/Val.h

@@ -424,7 +424,16 @@ protected:
 
 	// For internal use by the Val::Clone() methods.
 	struct CloneState {
-	    std::unordered_map<const Val*, Val*> clones;
+		// Caches a cloned value for later reuse during the same
+		// cloning operation. For recursive types, call this *before*
+		// descending down.
+		Val* NewClone(Val *src, Val* dst)
+			{
+			clones.insert(std::make_pair(src, dst));
+			return dst;
+			}
+
+		std::unordered_map<Val*, Val*> clones;
 	};
 
 	Val* Clone(CloneState* state);

src/probabilistic/Topk.cc

@@ -187,7 +187,7 @@ Val* TopkVal::DoClone(CloneState* state)
 	{
 	auto clone = new TopkVal(size);
 	clone->Merge(this);
-	return clone;
+	return state->NewClone(this, clone);
 	}
 
 bool TopkVal::DoSerialize(SerialInfo* info) const

testing/btest/Baseline/language.copy-cycle/out

@@ -0,0 +1,3 @@
+F (expected: F)
+T (expected: T)
+T (expected: T)

testing/btest/language/copy-cycle.zeek

@@ -0,0 +1,23 @@
+# @TEST-EXEC: zeek -b %INPUT >out
+# @TEST-EXEC: btest-diff out
+
+type B: record {
+	x: any &optional;
+	};
+
+type A: record  {
+        x: any &optional;
+        y: B;
+	};
+
+event zeek_init()
+	{
+	local x: A;
+	x$x = x;
+	x$y$x = x;
+	local y = copy(x);
+
+	print fmt("%s (expected: F)", same_object(x, y));
+	print fmt("%s (expected: T)", same_object(y, y$x));
+	print fmt("%s (expected: T)", same_object(y, y$y$x));
+	}