Improve type checking of records, addresses BIT-1159.

scripts/base/frameworks/files/main.bro

@@ -41,15 +41,15 @@ export {
 		## If this file was transferred over a network
 		## connection this should show the host or hosts that
 		## the data sourced from.
-		tx_hosts: set[addr] &log;
+		tx_hosts: set[addr] &default=addr_set() &log;
 
 		## If this file was transferred over a network
 		## connection this should show the host or hosts that
 		## the data traveled to.
-		rx_hosts: set[addr] &log;
+		rx_hosts: set[addr] &default=addr_set() &log;
 
 		## Connection UIDs over which the file was transferred.
-		conn_uids: set[string] &log;
+		conn_uids: set[string] &default=string_set() &log;
 
 		## An identification of the source of the file data.  E.g. it
 		## may be a network protocol over which it was transferred, or a
@@ -63,7 +63,7 @@ export {
 		depth: count &default=0 &log;
 
 		## A set of analysis types done during the file analysis.
-		analyzers: set[string] &log;
+		analyzers: set[string] &default=string_set() &log;
 
 		## A mime type provided by libmagic against the *bof_buffer*
 		## field of :bro:see:`fa_file`, or in the cases where no

src/Expr.cc

@@ -3392,22 +3392,12 @@ bool HasFieldExpr::DoUnserialize(UnserialInfo* info)
 	return UNSERIALIZE(&not_used) && UNSERIALIZE_STR(&field_name, 0) && UNSERIALIZE(&field);
 	}
 
-RecordConstructorExpr::RecordConstructorExpr(ListExpr* constructor_list,
+RecordConstructorExpr::RecordConstructorExpr(ListExpr* constructor_list)
-					     BroType* arg_type)
 : UnaryExpr(EXPR_RECORD_CONSTRUCTOR, constructor_list)
 	{
-	ctor_type = 0;
-
 	if ( IsError() )
 		return;
 
-	if ( arg_type && arg_type->Tag() != TYPE_RECORD )
-		{
-		Error("bad record constructor type", arg_type);
-		SetError();
-		return;
-		}
-
 	// Spin through the list, which should be comprised of
 	// either record's or record-field-assign, and build up a
 	// record type to associate with this constructor.
@@ -3447,17 +3437,11 @@ RecordConstructorExpr::RecordConstructorExpr(ListExpr* constructor_list,
 			}
 		}
 
-	ctor_type = new RecordType(record_types);
+	SetType(new RecordType(record_types));
-
-	if ( arg_type )
-		SetType(arg_type->Ref());
-	else
-		SetType(ctor_type->Ref());
 	}
 
 RecordConstructorExpr::~RecordConstructorExpr()
 	{
-	Unref(ctor_type);
 	}
 
 Val* RecordConstructorExpr::InitVal(const BroType* t, Val* aggr) const
@@ -3483,7 +3467,7 @@ Val* RecordConstructorExpr::InitVal(const BroType* t, Val* aggr) const
 Val* RecordConstructorExpr::Fold(Val* v) const
 	{
 	ListVal* lv = v->AsListVal();
-	RecordType* rt = ctor_type->AsRecordType();
+	RecordType* rt = type->AsRecordType();
 
 	if ( lv->Length() != rt->NumFields() )
 		Internal("inconsistency evaluating record constructor");
@@ -3493,19 +3477,6 @@ Val* RecordConstructorExpr::Fold(Val* v) const
 	for ( int i = 0; i < lv->Length(); ++i )
 		rv->Assign(i, lv->Index(i)->Ref());
 
-	if ( ! same_type(rt, type) )
-		{
-		RecordVal* new_val = rv->CoerceTo(type->AsRecordType());
-
-		if ( new_val )
-			{
-			Unref(rv);
-			rv = new_val;
-			}
-		else
-			Internal("record constructor coercion failed");
-		}
-
 	return rv;
 	}
 
@@ -3521,16 +3492,12 @@ IMPLEMENT_SERIAL(RecordConstructorExpr, SER_RECORD_CONSTRUCTOR_EXPR);
 bool RecordConstructorExpr::DoSerialize(SerialInfo* info) const
 	{
 	DO_SERIALIZE(SER_RECORD_CONSTRUCTOR_EXPR, UnaryExpr);
-	SERIALIZE_OPTIONAL(ctor_type);
 	return true;
 	}
 
 bool RecordConstructorExpr::DoUnserialize(UnserialInfo* info)
 	{
 	DO_UNSERIALIZE(UnaryExpr);
-	BroType* t = 0;
-	UNSERIALIZE_OPTIONAL(t, RecordType::Unserialize(info));
-	ctor_type = t->AsRecordType();
 	return true;
 	}
 
@@ -4214,6 +4181,26 @@ RecordCoerceExpr::~RecordCoerceExpr()
 	delete [] map;
 	}
 
+Val* RecordCoerceExpr::InitVal(const BroType* t, Val* aggr) const
+	{
+	Val* v = Eval(0);
+
+	if ( v )
+		{
+		RecordVal* rv = v->AsRecordVal();
+		RecordVal* ar = rv->CoerceTo(t->AsRecordType(), aggr);
+
+		if ( ar )
+			{
+			Unref(rv);
+			return ar;
+			}
+		}
+
+	Error("bad record initializer");
+	return 0;
+	}
+
 Val* RecordCoerceExpr::Fold(Val* v) const
 	{
 	RecordVal* val = new RecordVal(Type()->AsRecordType());
@@ -4238,6 +4225,13 @@ Val* RecordCoerceExpr::Fold(Val* v) const
 
 			assert(rhs || Type()->AsRecordType()->FieldDecl(i)->FindAttr(ATTR_OPTIONAL));
 
+			if ( ! rhs )
+				{
+				// Optional field is missing.
+				val->Assign(i, 0);
+				continue;
+				}
+
 			BroType* rhs_type = rhs->Type();
 			RecordType* val_type = val->Type()->AsRecordType();
 			BroType* field_type = val_type->FieldType(i);

src/Expr.h

@@ -753,7 +753,7 @@ protected:
 
 class RecordConstructorExpr : public UnaryExpr {
 public:
-	RecordConstructorExpr(ListExpr* constructor_list, BroType* arg_type = 0);
+	RecordConstructorExpr(ListExpr* constructor_list);
 	~RecordConstructorExpr();
 
 protected:
@@ -766,8 +766,6 @@ protected:
 	void ExprDescribe(ODesc* d) const;
 
 	DECLARE_SERIAL(RecordConstructorExpr);
-
-	RecordType* ctor_type; // type inferred from the ctor expression list args
 };
 
 class TableConstructorExpr : public UnaryExpr {
@@ -878,6 +876,7 @@ protected:
 	friend class Expr;
 	RecordCoerceExpr()	{ map = 0; }
 
+	Val* InitVal(const BroType* t, Val* aggr) const;
 	Val* Fold(Val* v) const;
 
 	DECLARE_SERIAL(RecordCoerceExpr);

src/Var.cc

@@ -169,8 +169,14 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init,
 			{
 			Val* aggr;
 			if ( t->Tag() == TYPE_RECORD )
+				{
 				aggr = new RecordVal(t->AsRecordType());
 
+				if ( init && t )
+					// Have an initialization and type is not deduced.
+					init = new RecordCoerceExpr(init, t->AsRecordType());
+				}
+
 			else if ( t->Tag() == TYPE_TABLE )
 				aggr = new TableVal(t->AsTableType(), id->Attrs());
 

src/parse.y

@@ -559,7 +559,8 @@ expr:
 				{
 				switch ( ctor_type->Tag() ) {
 				case TYPE_RECORD:
-					$$ = new RecordConstructorExpr($4, ctor_type);
+					$$ = new RecordCoerceExpr(new RecordConstructorExpr($4),
+					                          ctor_type->AsRecordType());
 					break;
 
 				case TYPE_TABLE:

testing/btest/Baseline/language.record-bad-ctor/out

@@ -1,3 +1,3 @@
 error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-bad-ctor/record-bad-ctor.bro, line 6: no type given (asdfasdf)
 error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-bad-ctor/record-bad-ctor.bro, line 7: uninitialized list value ($ports=asdfasdf)
-error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-bad-ctor/record-bad-ctor.bro, line 7: bad record initializer ([$ports=asdfasdf])
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-bad-ctor/record-bad-ctor.bro, line 7: bad record initializer ((coerce [$ports=asdfasdf] to record { ports:error; }))

testing/btest/Baseline/language.record-type-checking/out

@@ -0,0 +1,11 @@
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-type-checking/record-type-checking.bro, line 9 and count: type clash for field "a" ((coerce [$a=0] to MyRec) and count)
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-type-checking/record-type-checking.bro, line 9: bad record initializer ((coerce [$a=0] to error))
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-type-checking/record-type-checking.bro, line 12 and count: type clash for field "a" ((coerce [$a=1] to MyRec) and count)
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-type-checking/record-type-checking.bro, line 12: bad record initializer ((coerce (coerce [$a=1] to error) to error))
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-type-checking/record-type-checking.bro, line 18 and count: type clash for field "a" ((coerce [$a=2] to MyRec) and count)
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-type-checking/record-type-checking.bro, line 22 and count: type clash for field "a" ((coerce [$a=3] to MyRec) and count)
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-type-checking/record-type-checking.bro, line 22: bad record initializer ((coerce [$a=3] to error))
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-type-checking/record-type-checking.bro, line 27 and count: type clash for field "a" ((coerce [$a=1000] to MyRec) and count)
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-type-checking/record-type-checking.bro, line 33 and count: type clash for field "a" ((coerce [$a=1001] to MyRec) and count)
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-type-checking/record-type-checking.bro, line 40 and count: type clash for field "a" ((coerce [$a=1002] to MyRec) and count)
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.record-type-checking/record-type-checking.bro, line 46 and count: type clash for field "a" ((coerce [$a=1003] to MyRec) and count)

testing/btest/language/named-record-ctors.bro

@@ -1,6 +1,8 @@
-# @TEST-EXEC: bro -b frameworks/software/vulnerable %INPUT >out
+# @TEST-EXEC: bro -b %INPUT >out
 # @TEST-EXEC: btest-diff out
 
+@load frameworks/software/vulnerable
+
 type MyRec: record {
 	min: count &optional;
 	max: count;

testing/btest/language/record-extension.bro

@@ -9,7 +9,7 @@ type Foo: record {
 
 redef record Foo += {
         c: count &default=42;
-        d: count &optional;
+        d: string &optional;
         anotherset: set[count] &default=set();
 };
 

testing/btest/language/record-type-checking.bro

@@ -0,0 +1,47 @@
+# @TEST-EXEC-FAIL: bro -b %INPUT >out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out
+
+type MyRec: record {
+	a: port &default = 1/tcp;
+};
+
+# global, type deduction, named ctor
+global grdn = MyRec($a = 0); # type clash in init
+
+# global, type explicit, named ctor
+global gren: MyRec = MyRec($a = 1); # type clash in init
+
+# global, type deduction, anon ctor
+global grda = [$a = 2]; # fine
+event bro_init()
+	{
+	grda = MyRec($a = 2);  # type clash in assignment
+	}
+
+# global, type explicit, anon ctor
+global grea: MyRec = [$a = 3]; # type clash
+
+# local, type deduction, named ctor
+event bro_init()
+    {
+    local lrdn = MyRec($a = 1000); # type clash
+    }
+
+# local, type explicit, named ctor
+event bro_init()
+    {
+    local lren: MyRec = MyRec($a = 1001); # type clash
+    }
+
+# local, type deduction, anon ctor
+event bro_init()
+    {
+    local lrda = [$a = 1002];   # fine
+    lrda = MyRec($a = 1002);    # type clash
+    }
+
+# local, type explicit, anon ctor
+event bro_init()
+    {
+    local lrea: MyRec = [$a = 1003]; # type clash
+    }

testing/btest/language/sizeof.bro

@@ -28,7 +28,7 @@ global f:  file = open_log_file("sizeof_demo");
 global i:  int = -10;
 global iv: interval = -5sec;
 global p:  port = 80/tcp;
-global r:  example_record [ $i = 10 ];
+global r:  example_record = [ $i = +10 ];
 global si: set[int];
 global s:  string = "Hello";
 global sn: subnet = 192.168.0.0/24;

testing/btest/scripts/base/utils/conn-ids.test

@@ -4,8 +4,8 @@
 # This is loaded by default.
 #@load base/utils/conn-ids
 
-global c: conn_id = [ $orig_h = 10.0.0.100, $orig_p = 10000,
+global c: conn_id = [ $orig_h = 10.0.0.100, $orig_p = 10000/tcp,
-                      $resp_h = 10.0.0.200, $resp_p = 20000 ];
+                      $resp_h = 10.0.0.200, $resp_p = 20000/tcp ];
 
 print id_string(c);
 print reverse_id_string(c);

testing/btest/scripts/base/utils/directions-and-hosts.test

@@ -11,20 +11,20 @@ global local_ip = 10.0.0.100;
 global remote_ip = 192.168.1.100;
 
 global local2local: conn_id = [
-    $orig_h = 10.0.0.100, $orig_p = 10000,
+    $orig_h = 10.0.0.100, $orig_p = 10000/tcp,
-    $resp_h = 10.0.0.200, $resp_p = 20000 ];
+    $resp_h = 10.0.0.200, $resp_p = 20000/tcp ];
 
 global local2remote: conn_id = [
-    $orig_h = 10.0.0.100,    $orig_p = 10000,
+    $orig_h = 10.0.0.100,    $orig_p = 10000/tcp,
-    $resp_h = 192.168.1.100, $resp_p = 20000 ];
+    $resp_h = 192.168.1.100, $resp_p = 20000/tcp ];
 
 global remote2local: conn_id = [
-    $orig_h = 192.168.1.100,    $orig_p = 10000,
+    $orig_h = 192.168.1.100,    $orig_p = 10000/tcp,
-    $resp_h = 10.0.0.100,       $resp_p = 20000 ];
+    $resp_h = 10.0.0.100,       $resp_p = 20000/tcp ];
 
 global remote2remote: conn_id = [
-    $orig_h = 192.168.1.100, $orig_p = 10000,
+    $orig_h = 192.168.1.100, $orig_p = 10000/tcp,
-    $resp_h = 192.168.1.200, $resp_p = 20000 ];
+    $resp_h = 192.168.1.200, $resp_p = 20000/tcp ];
 
 function test_host(ip: addr, h: Host, expect: bool)
 	{