Merge remote-tracking branch 'origin/topic/timw/159-coerce-counts'

* origin/topic/timw/159-coerce-counts:
  GHI-155: set the type of a vector based on the variable's type, not the value's type
  GH-159: Allow coercion of numeric values into other types
  Allow passing a location to BroObj::Warning and BroObj::Error.
  Add CLion directories to gitignore
  Move #define outside of max_type for clarity

.gitignore

@@ -1,3 +1,7 @@
 build
 tmp
 *.gcov
+
+# Configuration and build directories for CLion
+.idea
+cmake-build-debug

CHANGES

@@ -1,4 +1,20 @@
 
+2.6-369 | 2019-06-04 17:53:10 -0700
+
+  * GH-155: Improve coercion of expression lists to vector types (Tim Wojtulewicz, Corelight)
+
+  * GH-159: Allow coercion of numeric record field values to other types (Tim Wojtulewicz, Corelight)
+
+  * Allow passing a location to BroObj::Warning and BroObj::Error. (Tim Wojtulewicz, Corelight)
+
+    This allows callers (such as check_and_promote) to pass an expression
+    location to be logged if the location doesn't exist in the value being
+    promoted.
+
+  * Add CLion directories to gitignore (Tim Wojtulewicz, Corelight)
+
+  * Move #define outside of max_type for clarity (Tim Wojtulewicz, Corelight)
+
 2.6-361 | 2019-06-04 10:30:21 -0700
 
   * GH-293: Protect copy() against reference cycles. (Robin Sommer, Corelight)

VERSION

@@ -1 +1 @@
-2.6-361
+2.6-369

src/Expr.cc

@@ -2591,7 +2591,7 @@ bool AssignExpr::TypeCheck(attr_list* attrs)
 
 		if ( op2->Tag() == EXPR_LIST )
 			{
-			op2 = new VectorConstructorExpr(op2->AsListExpr());
+			op2 = new VectorConstructorExpr(op2->AsListExpr(), op1->Type());
 			return true;
 			}
 		}
@@ -4136,15 +4136,41 @@ RecordCoerceExpr::RecordCoerceExpr(Expr* op, RecordType* r)
 
 			if ( ! same_type(sup_t_i, sub_t_i) )
 				{
-				if ( sup_t_i->Tag() != TYPE_RECORD ||
+				auto is_arithmetic_promotable = [](BroType* sup, BroType* sub) -> bool
-				     sub_t_i->Tag() != TYPE_RECORD ||
-				     ! record_promotion_compatible(sup_t_i->AsRecordType(),
-				                                   sub_t_i->AsRecordType()) )
 					{
-					char buf[512];
+					auto sup_tag = sup->Tag();
-					safe_snprintf(buf, sizeof(buf),
+					auto sub_tag = sub->Tag();
+
+					if ( ! BothArithmetic(sup_tag, sub_tag) )
+						return false;
+
+					if ( sub_tag == TYPE_DOUBLE && IsIntegral(sup_tag) )
+						return false;
+
+					if ( sub_tag == TYPE_INT && sup_tag == TYPE_COUNT )
+						return false;
+
+					return true;
+					};
+
+				auto is_record_promotable = [](BroType* sup, BroType* sub) -> bool
+					{
+					if ( sup->Tag() != TYPE_RECORD )
+						return false;
+
+					if ( sub->Tag() != TYPE_RECORD )
+						return false;
+
+					return record_promotion_compatible(sup->AsRecordType(),
+					                                   sub->AsRecordType());
+					};
+
+				if ( ! is_arithmetic_promotable(sup_t_i, sub_t_i) &&
+				     ! is_record_promotable(sup_t_i, sub_t_i) )
+					{
+					string error_msg = fmt(
 						"type clash for field \"%s\"", sub_r->FieldName(i));
-					Error(buf, sub_t_i);
+					Error(error_msg.c_str(), sub_t_i);
 					SetError();
 					break;
 					}
@@ -4162,11 +4188,9 @@ RecordCoerceExpr::RecordCoerceExpr(Expr* op, RecordType* r)
 				{
 				if ( ! t_r->FieldDecl(i)->FindAttr(ATTR_OPTIONAL) )
 					{
-					char buf[512];
+					string error_msg = fmt(
-					safe_snprintf(buf, sizeof(buf),
+						"non-optional field \"%s\" missing", t_r->FieldName(i));
-					              "non-optional field \"%s\" missing",
+					Error(error_msg.c_str());
-					              t_r->FieldName(i));
-					Error(buf);
 					SetError();
 					break;
 					}
@@ -4254,6 +4278,20 @@ Val* RecordCoerceExpr::Fold(Val* v) const
 					rhs = new_val;
 					}
 				}
+			else if ( BothArithmetic(rhs_type->Tag(), field_type->Tag()) &&
+			          ! same_type(rhs_type, field_type) )
+				{
+				if ( Val* new_val = check_and_promote(rhs, field_type, false, op->GetLocationInfo()) )
+					{
+					// Don't call unref here on rhs because check_and_promote already called it.
+					rhs = new_val;
+					}
+				else
+					{
+					Unref(val);
+					RuntimeError("Failed type conversion");
+					}
+				}
 
 			val->Assign(i, rhs);
 			}

src/Obj.cc

@@ -100,21 +100,21 @@ BroObj::~BroObj()
 	delete location;
 	}
 
-void BroObj::Warn(const char* msg, const BroObj* obj2, int pinpoint_only) const
+void BroObj::Warn(const char* msg, const BroObj* obj2, int pinpoint_only, const Location* expr_location) const
 	{
 	ODesc d;
-	DoMsg(&d, msg, obj2, pinpoint_only);
+	DoMsg(&d, msg, obj2, pinpoint_only, expr_location);
 	reporter->Warning("%s", d.Description());
 	reporter->PopLocation();
 	}
 
-void BroObj::Error(const char* msg, const BroObj* obj2, int pinpoint_only) const
+void BroObj::Error(const char* msg, const BroObj* obj2, int pinpoint_only, const Location* expr_location) const
 	{
 	if ( suppress_errors )
 		return;
 
 	ODesc d;
-	DoMsg(&d, msg, obj2, pinpoint_only);
+	DoMsg(&d, msg, obj2, pinpoint_only, expr_location);
 	reporter->Error("%s", d.Description());
 	reporter->PopLocation();
 	}
@@ -200,7 +200,7 @@ void BroObj::UpdateLocationEndInfo(const Location& end)
 	}
 
 void BroObj::DoMsg(ODesc* d, const char s1[], const BroObj* obj2,
-			int pinpoint_only) const
+			int pinpoint_only, const Location* expr_location) const
 	{
 	d->SetShort();
 
@@ -211,6 +211,8 @@ void BroObj::DoMsg(ODesc* d, const char s1[], const BroObj* obj2,
 	if ( obj2 && obj2->GetLocationInfo() != &no_location &&
 		 *obj2->GetLocationInfo() != *GetLocationInfo() )
 		loc2 = obj2->GetLocationInfo();
+	else if ( expr_location )
+		loc2 = expr_location;
 
 	reporter->PushLocation(GetLocationInfo(), loc2);
 	}

src/Obj.h

@@ -118,9 +118,9 @@ public:
 	// included in the message, though if pinpoint_only is non-zero,
 	// then obj2 is only used to pinpoint the location.
 	void Warn(const char* msg, const BroObj* obj2 = 0,
-			int pinpoint_only = 0) const;
+			int pinpoint_only = 0, const Location* expr_location = 0) const;
 	void Error(const char* msg, const BroObj* obj2 = 0,
-			int pinpoint_only = 0) const;
+			int pinpoint_only = 0, const Location* expr_location = 0) const;
 
 	// Report internal errors.
 	void BadTag(const char* msg, const char* t1 = 0,
@@ -178,7 +178,7 @@ private:
 	friend class SuppressErrors;
 
 	void DoMsg(ODesc* d, const char s1[], const BroObj* obj2 = 0,
-			int pinpoint_only = 0) const;
+			int pinpoint_only = 0, const Location* expr_location = 0) const;
 	void PinPoint(ODesc* d, const BroObj* obj2 = 0,
 			int pinpoint_only = 0) const;
 

src/Type.cc

@@ -2123,6 +2123,10 @@ int is_assignable(BroType* t)
 	return 0;
 	}
 
+#define CHECK_TYPE(t) \
+	if ( t1 == t || t2 == t ) \
+		return t;
+
 TypeTag max_type(TypeTag t1, TypeTag t2)
 	{
 	if ( t1 == TYPE_INTERVAL || t1 == TYPE_TIME )
@@ -2132,10 +2136,6 @@ TypeTag max_type(TypeTag t1, TypeTag t2)
 
 	if ( BothArithmetic(t1, t2) )
 		{
-#define CHECK_TYPE(t) \
-	if ( t1 == t || t2 == t ) \
-		return t;
-
 		CHECK_TYPE(TYPE_DOUBLE);
 		CHECK_TYPE(TYPE_INT);
 		CHECK_TYPE(TYPE_COUNT);

src/Val.cc

@@ -564,6 +564,35 @@ void Val::ValDescribeReST(ODesc* d) const
 	}
 	}
 
+
+bool Val::WouldOverflow(const BroType* from_type, const BroType* to_type, const Val* val)
+	{
+	if ( !to_type || !from_type )
+		return true;
+	else if ( same_type(to_type, from_type) )
+		return false;
+
+	if ( to_type->InternalType() == TYPE_INTERNAL_DOUBLE )
+		return false;
+	else if ( to_type->InternalType() == TYPE_INTERNAL_UNSIGNED )
+		{
+		if ( from_type->InternalType() == TYPE_INTERNAL_DOUBLE )
+			return (val->InternalDouble() < 0.0 || val->InternalDouble() > static_cast<double>(UINT64_MAX));
+		else if ( from_type->InternalType() == TYPE_INTERNAL_INT )
+			return (val->InternalInt() < 0);
+		}
+	else if ( to_type->InternalType() == TYPE_INTERNAL_INT )
+		{
+		if ( from_type->InternalType() == TYPE_INTERNAL_DOUBLE )
+			return (val->InternalDouble() < static_cast<double>(INT64_MIN) ||
+			        val->InternalDouble() > static_cast<double>(INT64_MAX));
+		else if ( from_type->InternalType() == TYPE_INTERNAL_UNSIGNED )
+			return (val->InternalUnsigned() > INT64_MAX);
+		}
+
+	return false;
+	}
+
 MutableVal::~MutableVal()
 	{
 	for ( list<ID*>::iterator i = aliases.begin(); i != aliases.end(); ++i )
@@ -3565,7 +3594,7 @@ bool OpaqueVal::DoUnserialize(UnserialInfo* info)
 	return true;
 	}
 
-Val* check_and_promote(Val* v, const BroType* t, int is_init)
+Val* check_and_promote(Val* v, const BroType* t, int is_init, const Location* expr_location)
 	{
 	if ( ! v )
 		return 0;
@@ -3589,7 +3618,7 @@ Val* check_and_promote(Val* v, const BroType* t, int is_init)
 		if ( same_type(t, vt, is_init) )
 			return v;
 
-		t->Error("type clash", v);
+		t->Error("type clash", v, 0, expr_location);
 		Unref(v);
 		return 0;
 		}
@@ -3598,9 +3627,9 @@ Val* check_and_promote(Val* v, const BroType* t, int is_init)
 	     (! IsArithmetic(v_tag) || t_tag != TYPE_TIME || ! v->IsZero()) )
 		{
 		if ( t_tag == TYPE_LIST || v_tag == TYPE_LIST )
-			t->Error("list mixed with scalar", v);
+			t->Error("list mixed with scalar", v, 0, expr_location);
 		else
-			t->Error("arithmetic mixed with non-arithmetic", v);
+			t->Error("arithmetic mixed with non-arithmetic", v, 0, expr_location);
 		Unref(v);
 		return 0;
 		}
@@ -3608,12 +3637,12 @@ Val* check_and_promote(Val* v, const BroType* t, int is_init)
 	if ( v_tag == t_tag )
 		return v;
 
-	if ( t_tag != TYPE_TIME )
+	if ( t_tag != TYPE_TIME && ! BothArithmetic(t_tag, v_tag) )
 		{
 		TypeTag mt = max_type(t_tag, v_tag);
 		if ( mt != t_tag )
 			{
-			t->Error("over-promotion of arithmetic value", v);
+			t->Error("over-promotion of arithmetic value", v, 0, expr_location);
 			Unref(v);
 			return 0;
 			}
@@ -3630,7 +3659,13 @@ Val* check_and_promote(Val* v, const BroType* t, int is_init)
 	Val* promoted_v;
 	switch ( it ) {
 	case TYPE_INTERNAL_INT:
-		if ( t_tag == TYPE_INT )
+		if ( ( vit == TYPE_INTERNAL_UNSIGNED || vit == TYPE_INTERNAL_DOUBLE ) && Val::WouldOverflow(vt, t, v) )
+			{
+			t->Error("overflow promoting from unsigned/double to signed arithmetic value", v, 0, expr_location);
+			Unref(v);
+			return 0;
+			}
+		else if ( t_tag == TYPE_INT )
 			promoted_v = val_mgr->GetInt(v->CoerceToInt());
 		else if ( t_tag == TYPE_BOOL )
 			promoted_v = val_mgr->GetBool(v->CoerceToInt());
@@ -3644,7 +3679,13 @@ Val* check_and_promote(Val* v, const BroType* t, int is_init)
 		break;
 
 	case TYPE_INTERNAL_UNSIGNED:
-		if ( t_tag == TYPE_COUNT || t_tag == TYPE_COUNTER )
+		if ( ( vit == TYPE_INTERNAL_DOUBLE || vit == TYPE_INTERNAL_INT) && Val::WouldOverflow(vt, t, v) )
+			{
+			t->Error("overflow promoting from signed/double to unsigned arithmetic value", v, 0, expr_location);
+			Unref(v);
+			return 0;
+			}
+		else if ( t_tag == TYPE_COUNT || t_tag == TYPE_COUNTER )
 			promoted_v = val_mgr->GetCount(v->CoerceToUnsigned());
 		else // port
 			{

src/Val.h

@@ -367,6 +367,8 @@ public:
 		}
 #endif
 
+	static bool WouldOverflow(const BroType* from_type, const BroType* to_type, const Val* val);
+
 protected:
 
 	friend class EnumType;
@@ -1228,7 +1230,7 @@ protected:
 // Unref()'ing the original.  If not a match, generates an error message
 // and returns nil, also Unref()'ing v.  If is_init is true, then
 // the checking is done in the context of an initialization.
-extern Val* check_and_promote(Val* v, const BroType* t, int is_init);
+extern Val* check_and_promote(Val* v, const BroType* t, int is_init, const Location* expr_location = nullptr);
 
 // Given a pointer to where a Val's core (i.e., its BRO value) resides,
 // returns a corresponding newly-created or Ref()'d Val.  ptr must already

testing/btest/Baseline/language.type-coerce-numerics/double_convert_failure1.out

@@ -0,0 +1 @@
+error in ./double_convert_failure1.zeek, line 7 and double: type clash for field "cc" ((coerce [$cc=5.0] to myrecord) and double)

testing/btest/Baseline/language.type-coerce-numerics/double_convert_failure2.out

@@ -0,0 +1 @@
+error in ./double_convert_failure2.zeek, line 7 and double: type clash for field "cc" ((coerce [$cc=-5.0] to myrecord) and double)

testing/btest/Baseline/language.type-coerce-numerics/first_set.out

@@ -0,0 +1,14 @@
+error in int and ./first_set.zeek, line 46: overflow promoting from unsigned/double to signed arithmetic value (int and 9223372036854775808)
+expression error in ./first_set.zeek, line 46: Failed type conversion ((coerce [$ii=9223372036854775808] to record { ii:int; cc:count; dd:double; }))
+3
+int
+4
+int
+5
+int
+6
+int
+7.0
+double
+-5.0
+double

testing/btest/Baseline/language.type-coerce-numerics/int_convert_failure.out

@@ -0,0 +1 @@
+error in ./int_convert_failure.zeek, line 7 and int: type clash for field "cc" ((coerce [$cc=-5] to myrecord) and int)

testing/btest/Baseline/language.type-coerce-numerics/vectors.out

@@ -0,0 +1,18 @@
+vector of count
+vector of count
+vector of count
+[1, 2]
+[3, 4]
+[4, 6]
+vector of int
+vector of int
+vector of int
+[1, 2]
+[3, 4]
+[4, 6]
+vector of double
+vector of double
+vector of double
+[1.0, 2.0]
+[3.0, 4.0]
+[4.0, 6.0]

testing/btest/language/type-coerce-numerics.zeek

@@ -0,0 +1,132 @@
+# @TEST-EXEC: zeek -b first_set.zeek >first_set.out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff first_set.out
+
+# @TEST-EXEC-FAIL: zeek -b double_convert_failure1.zeek >double_convert_failure1.out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff double_convert_failure1.out
+
+# @TEST-EXEC-FAIL: zeek -b double_convert_failure2.zeek >double_convert_failure2.out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff double_convert_failure1.out
+
+# @TEST-EXEC-FAIL: zeek -b int_convert_failure.zeek >int_convert_failure.out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff int_convert_failure.out
+
+# @TEST-EXEC: zeek -b vectors.zeek >vectors.out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff vectors.out
+
+@TEST-START-FILE first_set.zeek
+type myrecord : record {
+	ii: int &optional;
+	cc: count &optional;
+	dd: double &optional;
+};
+
+# Allow coercion from count values to int
+global globalint: myrecord &redef;
+redef globalint = [$ii = 2];
+
+# All of these cases should succeed
+event zeek_init()
+	{
+	# Allow coercion from count values to int
+	local intconvert1 = myrecord($ii = 3);
+	print(intconvert1$ii);
+	print(type_name(intconvert1$ii));
+
+	local intconvert2: myrecord = record($ii = 4);
+	print(intconvert2$ii);
+	print(type_name(intconvert2$ii));
+
+	local intconvert3: myrecord = [$ii = 5];
+	print(intconvert3$ii);
+	print(type_name(intconvert3$ii));
+
+	local intconvert4: myrecord;
+	intconvert4$ii = 6;
+	print(intconvert4$ii);
+	print(type_name(intconvert4$ii));
+
+	# Convert from count/integer values into doubles
+	local doubleconvert1 = myrecord($dd = 7);
+	print(doubleconvert1$dd);
+	print(type_name(doubleconvert1$dd));
+
+	local doubleconvert2 = myrecord($dd = -5);
+	print(doubleconvert2$dd);
+	print(type_name(doubleconvert2$dd));
+	}
+
+event zeek_init()
+	{
+	# This value is INT64_MAX+1, which overflows a signed integer and
+	# throws an error
+	local overflow = myrecord($ii = 9223372036854775808);
+	}
+@TEST-END-FILE
+
+@TEST-START-FILE double_convert_failure1.zeek
+type myrecord : record {
+	cc: count &optional;
+};
+
+event zeek_init()
+	{
+	local convert = myrecord($cc = 5.0);
+	}
+@TEST-END-FILE
+
+@TEST-START-FILE double_convert_failure2.zeek
+type myrecord : record {
+	cc: count &optional;
+};
+
+event zeek_init()
+	{
+	local convert = myrecord($cc = -5.0);
+	}
+@TEST-END-FILE
+
+@TEST-START-FILE int_convert_failure.zeek
+type myrecord : record {
+	cc: count &optional;
+};
+
+event zeek_init()
+	{
+	local convert = myrecord($cc = -5);
+	}
+@TEST-END-FILE
+
+@TEST-START-FILE vectors.zeek
+event zeek_init()
+	{
+	local c1 : vector of count = { 1 , 2 };
+	local c2 : vector of count = { 3 , 4 };
+	local c3 = c1 + c2;
+	print type_name(c1);
+	print type_name(c2);
+	print type_name(c3);
+	print c1;
+	print c2;
+	print c3;
+
+	local i1 : vector of int = { 1, 2 };
+	local i2 : vector of int = { 3, 4 };
+	local i3 = i1 + i2;
+	print type_name(i1);
+	print type_name(i2);
+	print type_name(i3);
+	print i1;
+	print i2;
+	print i3;
+
+	local d1 : vector of double = { 1, 2 };
+	local d2 : vector of double = { 3, 4 };
+	local d3 = d1 + d2;
+	print type_name(d1);
+	print type_name(d2);
+	print type_name(d3);
+	print d1;
+	print d2;
+	print d3;
+	}
+@TEST-END-FILE