diff --git a/CHANGES b/CHANGES index c33d74a9c5..2e03a4d19a 100644 --- a/CHANGES +++ b/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) diff --git a/VERSION b/VERSION index 1f1c273a8a..6964157ec2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -4.1.0-dev.128 +4.1.0-dev.132 diff --git a/src/Expr.cc b/src/Expr.cc index 7bbd4ab590..3c73a40ff0 100644 --- a/src/Expr.cc +++ b/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)); diff --git a/src/Type.cc b/src/Type.cc index 7387368254..b88ebc58b5 100644 --- a/src/Type.cc +++ b/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() ) { diff --git a/testing/btest/Baseline/language.type-check-func-call/out b/testing/btest/Baseline/language.type-check-func-call/out new file mode 100644 index 0000000000..748eae71e2 --- /dev/null +++ b/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)) diff --git a/testing/btest/Baseline/language.type-check-operator-in/out b/testing/btest/Baseline/language.type-check-operator-in/out new file mode 100644 index 0000000000..68114462b3 --- /dev/null +++ b/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) diff --git a/testing/btest/language/type-check-func-call.zeek b/testing/btest/language/type-check-func-call.zeek new file mode 100644 index 0000000000..f1951b4e1a --- /dev/null +++ b/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); + diff --git a/testing/btest/language/type-check-operator-in.zeek b/testing/btest/language/type-check-operator-in.zeek new file mode 100644 index 0000000000..d5f340320f --- /dev/null +++ b/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;