Merge remote-tracking branch 'origin/topic/timw/157-global-attributes'

* origin/topic/timw/157-global-attributes:
  GH-157: Mark some attributes as not allowed for global variables

Fixes GH-157

CHANGES

@@ -1,4 +1,12 @@
 
+2.6-595 | 2019-07-12 13:34:08 -0700
+
+  * GH-157: Mark some attributes as not allowed for global variables (Tim Wojtulewicz, Corelight)
+
+    This disallows &default for global values that are not tables, and &optional for all globals.
+
+  * Fix uncaught exceptions from Val cloning failures (Jon Siwek, Corelight)
+
 2.6-591 | 2019-07-11 13:29:28 -0700
 
   * Fix potential thread safety issue with zeekenv util function

VERSION

@@ -1 +1 @@
-2.6-591
+2.6-595

src/Attr.cc

@@ -130,11 +130,12 @@ void Attr::AddTag(ODesc* d) const
 		d->Add(attr_name(Tag()));
 	}
 
-Attributes::Attributes(attr_list* a, BroType* t, bool arg_in_record)
+Attributes::Attributes(attr_list* a, BroType* t, bool arg_in_record, bool is_global)
 	{
 	attrs = new attr_list(a->length());
 	type = t->Ref();
 	in_record = arg_in_record;
+	global_var = is_global;
 
 	SetLocationInfo(&start_location, &end_location);
 
@@ -250,10 +251,14 @@ void Attributes::CheckAttr(Attr* a)
 	{
 	switch ( a->Tag() ) {
 	case ATTR_DEPRECATED:
-	case ATTR_OPTIONAL:
 	case ATTR_REDEF:
 		break;
 
+	case ATTR_OPTIONAL:
+		if ( global_var )
+			Error("&optional is not valid for global variables");
+		break;
+
 	case ATTR_ADD_FUNC:
 	case ATTR_DEL_FUNC:
 		{
@@ -283,6 +288,14 @@ void Attributes::CheckAttr(Attr* a)
 
 	case ATTR_DEFAULT:
 		{
+		// &default is allowed for global tables, since it's used in initialization
+		// of table fields. it's not allowed otherwise.
+		if ( global_var && ! type->IsSet() && type->Tag() != TYPE_TABLE )
+			{
+			Error("&default is not valid for global variables");
+			break;
+			}
+
 		BroType* atype = a->AttrExpr()->Type();
 
 		if ( type->Tag() != TYPE_TABLE || (type->IsSet() && ! in_record) )
@@ -410,9 +423,10 @@ void Attributes::CheckAttr(Attr* a)
 
 #if 0
 		//### not easy to test this w/o knowing the ID.
-		if ( ! IsGlobal() )
+		if ( ! global_var )
 			Error("expiration not supported for local variables");
 #endif
+
 		break;
 
 	case ATTR_EXPIRE_FUNC:

src/Attr.h

@@ -77,7 +77,7 @@ class Attr : public BroObj {
 // Manages a collection of attributes.
 class Attributes : public BroObj {
 public:
-	Attributes(attr_list* a, BroType* t, bool in_record);
+	Attributes(attr_list* a, BroType* t, bool in_record, bool is_global);
 	~Attributes() override;
 
 	void AddAttr(Attr* a);
@@ -101,6 +101,7 @@ class Attributes : public BroObj {
 	BroType* type;
 	attr_list* attrs;
 	bool in_record;
+	bool global_var;
 };
 
 #endif

src/Expr.cc

@@ -3133,7 +3133,7 @@ TableConstructorExpr::TableConstructorExpr(ListExpr* constructor_list,
 			}
 		}
 
-	attrs = arg_attrs ? new Attributes(arg_attrs, type, false) : 0;
+	attrs = arg_attrs ? new Attributes(arg_attrs, type, false, false) : 0;
 
 	type_list* indices = type->AsTableType()->Indices()->Types();
 	const expr_list& cle = constructor_list->Exprs();
@@ -3240,7 +3240,7 @@ SetConstructorExpr::SetConstructorExpr(ListExpr* constructor_list,
 	else if ( type->Tag() != TYPE_TABLE || ! type->AsTableType()->IsSet() )
 		SetError("values in set(...) constructor do not specify a set");
 
-	attrs = arg_attrs ? new Attributes(arg_attrs, type, false) : 0;
+	attrs = arg_attrs ? new Attributes(arg_attrs, type, false, false) : 0;
 
 	type_list* indices = type->AsTableType()->Indices()->Types();
 	expr_list& cle = constructor_list->Exprs();

src/ID.cc

@@ -181,7 +181,7 @@ void ID::UpdateValAttrs()
 				TypeDecl* fd = rt->FieldDecl(i);
 
 				if ( ! fd->attrs )
-					fd->attrs = new Attributes(new attr_list, rt->FieldType(i), true);
+					fd->attrs = new Attributes(new attr_list, rt->FieldType(i), true, IsGlobal());
 
 				fd->attrs->AddAttr(new Attr(ATTR_LOG));
 				}
@@ -195,7 +195,7 @@ void ID::MakeDeprecated(Expr* deprecation)
 		return;
 
 	attr_list* attr = new attr_list{new Attr(ATTR_DEPRECATED, deprecation)};
-	AddAttrs(new Attributes(attr, Type(), false));
+	AddAttrs(new Attributes(attr, Type(), false, IsGlobal()));
 	}
 
 string ID::GetDeprecationWarning() const
@@ -245,7 +245,7 @@ void ID::SetOption()
 	if ( ! IsRedefinable() )
 		{
 		attr_list* attr = new attr_list{new Attr(ATTR_REDEF)};
-		AddAttrs(new Attributes(attr, Type(), false));
+		AddAttrs(new Attributes(attr, Type(), false, IsGlobal()));
 		}
 	}
 

src/Type.cc

@@ -650,7 +650,7 @@ void FuncType::DescribeReST(ODesc* d, bool roles_only) const
 TypeDecl::TypeDecl(BroType* t, const char* i, attr_list* arg_attrs, bool in_record)
 	{
 	type = t;
-	attrs = arg_attrs ? new Attributes(arg_attrs, t, in_record) : 0;
+	attrs = arg_attrs ? new Attributes(arg_attrs, t, in_record, false) : 0;
 	id = i;
 	}
 
@@ -841,7 +841,7 @@ const char* RecordType::AddFields(type_decl_list* others, attr_list* attr)
 		if ( log )
 			{
 			if ( ! td->attrs )
-				td->attrs = new Attributes(new attr_list, td->type, true);
+				td->attrs = new Attributes(new attr_list, td->type, true, false);
 
 			td->attrs->AddAttr(new Attr(ATTR_LOG));
 			}

src/Var.cc

@@ -108,7 +108,7 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init,
 	id->SetType(t);
 
 	if ( attr )
-		id->AddAttrs(new Attributes(attr, t, false));
+		id->AddAttrs(new Attributes(attr, t, false, id->IsGlobal()));
 
 	if ( init )
 		{
@@ -286,7 +286,7 @@ void add_type(ID* id, BroType* t, attr_list* attr)
 	id->MakeType();
 
 	if ( attr )
-		id->SetAttrs(new Attributes(attr, tnew, false));
+		id->SetAttrs(new Attributes(attr, tnew, false, false));
 	}
 
 static void transfer_arg_defaults(RecordType* args, RecordType* recv)
@@ -304,7 +304,7 @@ static void transfer_arg_defaults(RecordType* args, RecordType* recv)
 		if ( ! recv_i->attrs )
 			{
 			attr_list* a = new attr_list{def};
-			recv_i->attrs = new Attributes(a, recv_i->type, true);
+			recv_i->attrs = new Attributes(a, recv_i->type, true, false);
 			}
 
 		else if ( ! recv_i->attrs->FindAttr(ATTR_DEFAULT) )

testing/btest/Baseline/language.attr-default-global-set-error/out

@@ -1,2 +1,5 @@
-error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 4: arithmetic mixed with non-arithmetic (set[string] and 0)
-error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 4: &default value has inconsistent type (0 and set[string])
+error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 4: arithmetic mixed with non-arithmetic (set[string] and 0)
+error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 4: &default value has inconsistent type (0 and set[string])
+error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 9: &default is not valid for global variables (&default=10)
+error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 9: &default is not valid for global variables (&optional, &default=9)
+error in /Users/tim/Desktop/projects/zeek/testing/btest/.tmp/language.attr-default-global-set-error/attr-default-global-set-error.zeek, line 9: &optional is not valid for global variables (&optional, &default=9, &optional)

testing/btest/language/attr-default-global-set-error.zeek

@@ -2,3 +2,8 @@
 # @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out
 
 global ss: set[string] &default=0;
+global d: count &default = 10
+		&default = 9
+		&optional
+		&log
+		&add_func = function(): count { return 3; };