Merge remote-tracking branch 'origin/topic/jsiwek/gh-1296-fix-list-type-checks'

* origin/topic/jsiwek/gh-1296-fix-list-type-checks:
  GH-1296: fix type-checks related to list-type equality

CHANGES

@@ -1,4 +1,32 @@
 
+4.1.0-dev.132 | 2021-01-21 13:59:35 -0800
+
+  * GH-1296: fix type-checks related to list-type equality (Jon Siwek, Corelight)
+
+    List-types as used in composite table/set indices, for example,
+    previously had incorrect same_type() comparisons due to flattening
+    of the list-type into a single type without checking whether the
+    number and kind of types all match.
+
+    This patch simply removes the flatten_type() call from same_type() since
+    it was already contradicting/preventing a subsequent full-comparison
+    between elements of two TYPE_LISTs.
+
+    There was also a superfluous special-case of the `in` operator's
+    type-checking for testing whether a record is in a table/set.  It's
+    superfluous because the general case will already do the type-checking
+    from MatchesIndex() after first wrapping the record operand in a
+    ListExpr.  The previous logic was incorrectly relying on the
+    flatten_type() for testing equality of a record-type against a
+    list-type, whereas the general case correctly normalizes to testing
+    equality of two list-types.
+
+    The special-cased type-checking logic for assigning a record value to a
+    table index during its initialization similarly needed minor
+    re-organization in order to maintain the same error messages as before.
+
+  * Guard against nullptr deref after AST-reduction consistency checks (Jon Siwek, Corelight)
+
 4.1.0-dev.128 | 2021-01-21 13:37:10 -0800
 
   * Install BTest with Zeek (Christian Kreibich, Corelight)

VERSION

@@ -1 +1 @@
-4.1.0-dev.128
+4.1.0-dev.132

src/Expr.cc

@@ -2651,19 +2651,28 @@ ValPtr AssignExpr::InitVal(const zeek::Type* t, ValPtr aggr) const
 
 		auto index = op1->InitVal(tt->GetIndices().get(), nullptr);
 
-		if ( ! same_type(*yt, *op2->GetType(), true) )
+		if ( yt->Tag() == TYPE_RECORD )
 			{
-			if ( yt->Tag() == TYPE_RECORD && op2->GetType()->Tag() == TYPE_RECORD )
+			if ( op2->GetType()->Tag() != TYPE_RECORD )
 				{
-				if ( ! record_promotion_compatible(yt->AsRecordType(),
-				                                   op2->GetType()->AsRecordType()) )
-					{
-					Error("type mismatch in table value initialization: "
-					      "incompatible record types");
-					return nullptr;
-					}
+				Error(util::fmt("type mismatch in table value initialization: "
+				                "assigning '%s' to table with values of type '%s'",
+				                type_name(op2->GetType()->Tag()), type_name(yt->Tag())));
+				return nullptr;
 				}
-			else
+
+			if ( ! same_type(*yt, *op2->GetType()) &&
+			     ! record_promotion_compatible(yt->AsRecordType(),
+			                                   op2->GetType()->AsRecordType()) )
+				{
+				Error("type mismatch in table value initialization: "
+				      "incompatible record types");
+				return nullptr;
+				}
+			}
+		else
+			{
+			if ( ! same_type(*yt, *op2->GetType(), true) )
 				{
 				Error(util::fmt("type mismatch in table value initialization: "
 				                "assigning '%s' to table with values of type '%s'",
@@ -4143,29 +4152,6 @@ InExpr::InExpr(ExprPtr arg_op1, ExprPtr arg_op2)
 			SetType(base_type(TYPE_BOOL));
 		}
 
-	else if ( op1->GetType()->Tag() == TYPE_RECORD )
-		{
-		if ( op2->GetType()->Tag() != TYPE_TABLE )
-			{
-			op2->GetType()->Error("table/set required");
-			SetError();
-			}
-
-		else
-			{
-			const auto& t1 = op1->GetType();
-			const auto& it = op2->GetType()->AsTableType()->GetIndices();
-
-			if ( ! same_type(t1, it) )
-				{
-				t1->Error("indexing mismatch", op2->GetType().get());
-				SetError();
-				}
-			else
-				SetType(base_type(TYPE_BOOL));
-			}
-		}
-
 	else if ( op1->GetType()->Tag() == TYPE_STRING &&
 		  op2->GetType()->Tag() == TYPE_STRING )
 		SetType(base_type(TYPE_BOOL));

src/Type.cc

@@ -1590,11 +1590,8 @@ bool same_type(const Type& arg_t1, const Type& arg_t2,
 	     arg_t2.Tag() == TYPE_ANY )
 		return true;
 
-	auto t1 = flatten_type(&arg_t1);
-	auto t2 = flatten_type(&arg_t2);
-
-	if ( t1 == t2 )
-		return true;
+	auto t1 = &arg_t1;
+	auto t2 = &arg_t2;
 
 	if ( t1->Tag() != t2->Tag() )
 		{

testing/btest/Baseline/language.type-check-func-call/out

@@ -0,0 +1,4 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in <...>/type-check-func-call.zeek, line 8 and <...>/type-check-func-call.zeek, line 21: type clash (set[string] and s)
+error in <...>/type-check-func-call.zeek, line 21 and <...>/type-check-func-call.zeek, line 8: type mismatch (s and set[string])
+error in <...>/type-check-func-call.zeek, line 21: argument type mismatch in function call (sort_set(s))

testing/btest/Baseline/language.type-check-operator-in/out

@@ -0,0 +1,8 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in int and <...>/type-check-operator-in.zeek, line 30: arithmetic mixed with non-arithmetic (int and myrec)
+error in <...>/type-check-operator-in.zeek, line 30 and int: type mismatch (myrec and int)
+error in <...>/type-check-operator-in.zeek, line 30: not an index type (myrec in asdf)
+error in <...>/type-check-operator-in.zeek, line 14 and <...>/type-check-operator-in.zeek, line 31: indexing mismatch (list of string,MyRec and myrec)
+error in <...>/type-check-operator-in.zeek, line 31: not an index type (myrec in string_records)
+error in <...>/type-check-operator-in.zeek, line 15 and <...>/type-check-operator-in.zeek, line 32: indexing mismatch (list of MyRec,string and myrec)
+error in <...>/type-check-operator-in.zeek, line 32: not an index type (myrec in record_strings)

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

@@ -0,0 +1,22 @@
+# @TEST-EXEC-FAIL: zeek -b %INPUT >out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out
+
+# @TEST-DOC: Test for type-checking of function call arguments.
+
+global s: set[string, string];
+
+function sort_set(s: set[string]): vector of string
+	{
+	local v: vector of string = vector();
+
+	for ( e in s )
+		v += e;
+
+	sort(v, strcmp);
+	return v;
+	}
+
+add s["hi", "there"];
+# This sort_set call should warn about mismatched table/set types.
+sort_set(s);
+

testing/btest/language/type-check-operator-in.zeek

@@ -0,0 +1,32 @@
+# @TEST-EXEC-FAIL: zeek -b %INPUT >out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out
+
+# @TEST-DOC: Test for type-checking of `in` operator.
+
+type MyRec: record {
+	a: count &default=1;
+};
+
+local myrec = MyRec();
+local strings: set[string] = set();
+local records: set[MyRec] = set();
+local string_counts: set[string, count] = set();
+local string_records: set[string, MyRec] = set();
+local record_strings: set[MyRec, string] = set();
+
+# These are all valid.
+["asdf"] in strings;
+["hi", 0] in string_counts;
+myrec in records;
+[myrec] in records;
+MyRec() in records;
+[$a = 2] in records;
+[MyRec()] in records;
+[[$a = 2]] in records;
+["hi", myrec] in string_records;
+
+# All below should fail type-checking.
+
+myrec in "asdf";
+myrec in string_records;
+myrec in record_strings;