Fix memory leak in vector slice assignment

Two parts to this:

  * Only allow vector slice assignment in statement contexts, not in
    arbitrary assignment expressions.  E.g. it's not clear what the
    resulting value of `(v[1:2] = vector(1))` is for further expression
    chaining.  For reference, Python doesn't allow it either.

  * Add a subclass of AssignExpr to specialize the behavior for index
    slice assignments (because its behavior regarding expression
    chaining is different per the previous point) and Unref the RHS
    of things like `v[1:2] = vector(1)` after IndexExpr::Assign is
    finished inserting it (since no one else takes ownership of it).

Instead of using an Expr subclass, IndexSliceAssignExpr, we could
use a proper Stmt, since that's the only context we currently use it
for, but if we did ever to decide on allowing its use in arbitrary
expression contexts, then I expect we'll need it this way anyway
(just with a different IndexSliceAssignExpr::Eval implementation).

src/Expr.cc

@@ -32,7 +32,7 @@ const char* expr_name(BroExprTag t)
 		"$=", "in", "<<>>",
 		"()", "event", "schedule",
 		"coerce", "record_coerce", "table_coerce",
-		"sizeof", "flatten", "cast", "is"
+		"sizeof", "flatten", "cast", "is", "[:]="
 	};
 
 	if ( int(t) >= NUM_EXPRS )
@@ -2905,8 +2905,46 @@ bool AssignExpr::DoUnserialize(UnserialInfo* info)
 	return UNSERIALIZE(&is_init);
 	}
 
-IndexExpr::IndexExpr(Expr* arg_op1, ListExpr* arg_op2, bool is_slice)
-: BinaryExpr(EXPR_INDEX, arg_op1, arg_op2)
+IndexSliceAssignExpr::IndexSliceAssignExpr(Expr* op1, Expr* op2, int is_init)
+	: AssignExpr(op1, op2, is_init)
+	{
+	}
+
+Val* IndexSliceAssignExpr::Eval(Frame* f) const
+	{
+	if ( is_init )
+		{
+		RuntimeError("illegal assignment in initialization");
+		return 0;
+		}
+
+	Val* v = op2->Eval(f);
+
+	if ( v )
+		{
+		op1->Assign(f, v);
+		Unref(v);
+		}
+
+	return 0;
+	}
+
+IMPLEMENT_SERIAL(IndexSliceAssignExpr, SER_INDEX_SLICE_ASSIGN_EXPR);
+
+bool IndexSliceAssignExpr::DoSerialize(SerialInfo* info) const
+	{
+	DO_SERIALIZE(SER_INDEX_SLICE_ASSIGN_EXPR, AssignExpr);
+	return true;
+	}
+
+bool IndexSliceAssignExpr::DoUnserialize(UnserialInfo* info)
+	{
+	DO_UNSERIALIZE(AssignExpr);
+	return true;
+	}
+
+IndexExpr::IndexExpr(Expr* arg_op1, ListExpr* arg_op2, bool arg_is_slice)
+: BinaryExpr(EXPR_INDEX, arg_op1, arg_op2), is_slice(arg_is_slice)
 	{
 	if ( IsError() )
 		return;
@@ -3302,13 +3340,13 @@ IMPLEMENT_SERIAL(IndexExpr, SER_INDEX_EXPR);
 bool IndexExpr::DoSerialize(SerialInfo* info) const
 	{
 	DO_SERIALIZE(SER_INDEX_EXPR, BinaryExpr);
-	return true;
+	return SERIALIZE(is_slice);
 	}
 
 bool IndexExpr::DoUnserialize(UnserialInfo* info)
 	{
 	DO_UNSERIALIZE(BinaryExpr);
-	return true;
+	return UNSERIALIZE(&is_slice);
 	}
 
 FieldExpr::FieldExpr(Expr* arg_op, const char* arg_field_name)
@@ -5747,6 +5785,8 @@ Expr* get_assign_expr(Expr* op1, Expr* op2, int is_init)
 	if ( op1->Type()->Tag() == TYPE_RECORD &&
 	     op2->Type()->Tag() == TYPE_LIST )
 		return new RecordAssignExpr(op1, op2, is_init);
+	else if ( op1->Tag() == EXPR_INDEX && op1->AsIndexExpr()->IsSlice() )
+		return new IndexSliceAssignExpr(op1, op2, is_init);
 	else
 		return new AssignExpr(op1, op2, is_init);
 	}

src/Expr.h

@@ -49,7 +49,8 @@ typedef enum {
 	EXPR_FLATTEN,
 	EXPR_CAST,
 	EXPR_IS,
-#define NUM_EXPRS (int(EXPR_IS) + 1)
+	EXPR_INDEX_SLICE_ASSIGN,
+#define NUM_EXPRS (int(EXPR_INDEX_SLICE_ASSIGN) + 1)
 } BroExprTag;
 
 extern const char* expr_name(BroExprTag t);
@@ -58,6 +59,7 @@ class Stmt;
 class Frame;
 class ListExpr;
 class NameExpr;
+class IndexExpr;
 class AssignExpr;
 class CallExpr;
 class EventExpr;
@@ -187,6 +189,18 @@ public:
 		return (AssignExpr*) this;
 		}
 
+	const IndexExpr* AsIndexExpr() const
+		{
+		CHECK_TAG(tag, EXPR_INDEX, "ExprVal::AsIndexExpr", expr_name)
+		return (const IndexExpr*) this;
+		}
+
+	IndexExpr* AsIndexExpr()
+		{
+		CHECK_TAG(tag, EXPR_INDEX, "ExprVal::AsIndexExpr", expr_name)
+		return (IndexExpr*) this;
+		}
+
 	void Describe(ODesc* d) const override;
 
 	bool Serialize(SerialInfo* info) const;
@@ -663,6 +677,18 @@ protected:
 	Val* val;	// optional
 };
 
+class IndexSliceAssignExpr : public AssignExpr {
+public:
+	IndexSliceAssignExpr(Expr* op1, Expr* op2, int is_init);
+	Val* Eval(Frame* f) const override;
+
+protected:
+	friend class Expr;
+	IndexSliceAssignExpr() {}
+
+	DECLARE_SERIAL(IndexSliceAssignExpr);
+};
+
 class IndexExpr : public BinaryExpr {
 public:
 	IndexExpr(Expr* op1, ListExpr* op2, bool is_slice = false);
@@ -682,6 +708,8 @@ public:
 
 	TraversalCode Traverse(TraversalCallback* cb) const override;
 
+	bool IsSlice() const { return is_slice; }
+
 protected:
 	friend class Expr;
 	IndexExpr()	{ }
@@ -691,6 +719,8 @@ protected:
 	void ExprDescribe(ODesc* d) const override;
 
 	DECLARE_SERIAL(IndexExpr);
+
+	bool is_slice;
 };
 
 class FieldExpr : public UnaryExpr {

src/SerialTypes.h

@@ -166,6 +166,7 @@ SERIAL_EXPR(CAST_EXPR, 45)
 SERIAL_EXPR(IS_EXPR_, 46) // Name conflict with internal SER_IS_EXPR constant.
 SERIAL_EXPR(BIT_EXPR, 47)
 SERIAL_EXPR(COMPLEMENT_EXPR, 48)
+SERIAL_EXPR(INDEX_SLICE_ASSIGN_EXPR, 49)
 
 #define SERIAL_STMT(name, val) SERIAL_CONST(name, val, STMT)
 SERIAL_STMT(STMT, 1)

src/parse.y

@@ -5,7 +5,7 @@
 // Switching parser table type fixes ambiguity problems.
 %define lr.type ielr
 
-%expect 103
+%expect 104
 
 %token TOK_ADD TOK_ADD_TO TOK_ADDR TOK_ANY
 %token TOK_ATENDIF TOK_ATELSE TOK_ATIF TOK_ATIFDEF TOK_ATIFNDEF
@@ -57,7 +57,7 @@
 %type <ic> init_class
 %type <expr> opt_init
 %type <val> TOK_CONSTANT
-%type <expr> expr opt_expr init anonymous_function
+%type <expr> expr opt_expr init anonymous_function index_slice
 %type <event_expr> event
 %type <stmt> stmt stmt_list func_body for_head
 %type <type> type opt_type enum_body
@@ -464,6 +464,12 @@ expr:
 	|	expr '=' expr
 			{
 			set_location(@1, @3);
+
+			if ( $1->Tag() == EXPR_INDEX && $1->AsIndexExpr()->IsSlice() )
+				reporter->Error("index slice assignment may not be used"
+				                " in arbitrary expression contexts, only"
+				                " as a statement");
+
 			$$ = get_assign_expr($1, $3, in_init);
 			}
 
@@ -479,19 +485,7 @@ expr:
 			$$ = new IndexExpr($1, $3);
 			}
 
-	|	expr '[' opt_expr ':' opt_expr ']'
-			{
-			set_location(@1, @6);
-			Expr* low = $3 ? $3 : new ConstExpr(val_mgr->GetCount(0));
-			Expr* high = $5 ? $5 : new SizeExpr($1);
-
-			if ( ! IsIntegral(low->Type()->Tag()) || ! IsIntegral(high->Type()->Tag()) )
-				reporter->Error("slice notation must have integral values as indexes");
-
-			ListExpr* le = new ListExpr(low);
-			le->Append(high);
-			$$ = new IndexExpr($1, le, true);
-			}
+	|	index_slice
 
 	|	expr '$' TOK_ID
 			{
@@ -1271,6 +1265,21 @@ init:
 	|	expr
 	;
 
+index_slice:
+		expr '[' opt_expr ':' opt_expr ']'
+			{
+			set_location(@1, @6);
+			Expr* low = $3 ? $3 : new ConstExpr(val_mgr->GetCount(0));
+			Expr* high = $5 ? $5 : new SizeExpr($1);
+
+			if ( ! IsIntegral(low->Type()->Tag()) || ! IsIntegral(high->Type()->Tag()) )
+				reporter->Error("slice notation must have integral values as indexes");
+
+			ListExpr* le = new ListExpr(low);
+			le->Append(high);
+			$$ = new IndexExpr($1, le, true);
+			}
+
 opt_attr:
 		attr_list
 	|
@@ -1474,6 +1483,15 @@ stmt:
 			    brofiler.DecIgnoreDepth();
 			}
 
+	|	index_slice '=' expr ';' opt_no_test
+			{
+			set_location(@1, @4);
+			$$ = new ExprStmt(get_assign_expr($1, $3, in_init));
+
+			if ( ! $5 )
+				brofiler.AddStmt($$);
+			}
+
 	|	expr ';' opt_no_test
 			{
 			set_location(@1, @2);

testing/btest/core/leaks/vector-indexing.zeek

@@ -0,0 +1,30 @@
+# Needs perftools support.
+#
+# @TEST-GROUP: leaks
+#
+# @TEST-REQUIRES: zeek  --help 2>&1 | grep -q mem-leaks
+#
+# @TEST-EXEC: HEAP_CHECK_DUMP_DIRECTORY=. HEAPCHECK=local btest-bg-run zeek zeek -m -b -r $TRACES/wikipedia.trace %INPUT
+# @TEST-EXEC: btest-bg-wait 60
+
+global did_it = F;
+
+event new_connection(c: connection)
+	{
+	if ( did_it )
+		return;
+
+	did_it = T;
+
+	# Slicing tests.
+	local v17 = vector( 1, 2, 3, 4, 5 );
+	print v17[0:2];
+	print v17[-3:-1];
+	print v17[:2];
+	print v17[2:];
+	print v17[:];
+	v17[0:1] = vector(6);
+	v17[2:4] = vector(7, 8);
+	v17[2:4] = vector(9, 10, 11);
+	v17[2:5] = vector(9);
+	}