Merge remote-tracking branch 'origin/topic/timw/393-vector-slicing'

* origin/topic/timw/393-vector-slicing:
  Fix memory leak in vector slice assignment
  Misc. tweaks to vector slicing implementation
  Add ability to grow/shrink a vector using slicing, also adds Insert/Remove methods for VectorVal
  Allow assignment for vectors using slices
  Check for integral slice indexes, add extra test for [:]
  Return an empty vector if the indices for slicing don't make sense
  GH-393: Add slice notation for vectors

CHANGES

@@ -1,4 +1,14 @@
 
+2.6-454 | 2019-06-19 09:39:06 -0700
+
+  * GH-393: Add slice notation for vectors (Tim Wojtulewicz, Corelight & Jon Siwek, Corelight)
+
+    Example Syntax:
+
+    local v = vector(1, 2, 3, 4, 5);
+    v[2:4] = vector(6, 7, 8);        # v is now [1, 2, 6, 7, 8, 5]
+    print v[:4];                     # prints [1, 2, 6, 7]
+
 2.6-446 | 2019-06-17 20:26:49 -0700
 
   * Rename bro to zeek in error messages (Daniel Thayer)
@@ -50,6 +60,7 @@
 
   * Parse TLS 1.3 pre-shared-key extension. (Johanna Amann)
 
+
     Adds new events:
 
     - ssl_extension_pre_shared_key_client_hello

NEWS

@@ -105,6 +105,12 @@ New Functionality
   - An ntp.log is produced by default, containing data extracted from
     NTP messages with modes 1-5.
 
+- Add support for vector slicing operations.  For example::
+
+    local v = vector(1, 2, 3, 4, 5);
+    v[2:4] = vector(6, 7, 8);        # v is now [1, 2, 6, 7, 8, 5]
+    print v[:4];                     # prints [1, 2, 6, 7]
+
 Changed Functionality
 ---------------------
 

VERSION

@@ -1 +1 @@
-2.6-446
+2.6-454

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,16 +2905,54 @@ bool AssignExpr::DoUnserialize(UnserialInfo* info)
 	return UNSERIALIZE(&is_init);
 	}
 
-IndexExpr::IndexExpr(Expr* arg_op1, ListExpr* arg_op2, bool is_slice)
+IndexSliceAssignExpr::IndexSliceAssignExpr(Expr* op1, Expr* op2, int is_init)
-: BinaryExpr(EXPR_INDEX, arg_op1, arg_op2)
+	: 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;
 
 	if ( is_slice )
 		{
-		if ( ! IsString(op1->Type()->Tag()) )
+		if ( ! IsString(op1->Type()->Tag()) && ! IsVector(op1->Type()->Tag()) )
-			ExprError("slice notation indexing only supported for strings currently");
+			ExprError("slice notation indexing only supported for strings and vectors currently");
 		}
 
 	else if ( IsString(op1->Type()->Tag()) )
@@ -2937,8 +2975,7 @@ IndexExpr::IndexExpr(Expr* arg_op1, ListExpr* arg_op2, bool is_slice)
 
 	else if ( ! op1->Type()->YieldType() )
 		{
-		if ( IsString(op1->Type()->Tag()) &&
+		if ( IsString(op1->Type()->Tag()) && match_type == MATCHES_INDEX_SCALAR )
-		     match_type == MATCHES_INDEX_SCALAR )
 			SetType(base_type(TYPE_STRING));
 		else
 		// It's a set - so indexing it yields void.  We don't
@@ -3104,7 +3141,32 @@ Val* IndexExpr::Fold(Val* v1, Val* v2) const
 
 	switch ( v1->Type()->Tag() ) {
 	case TYPE_VECTOR:
-		v = v1->AsVectorVal()->Lookup(v2);
+		{
+		VectorVal* vect = v1->AsVectorVal();
+		const ListVal* lv = v2->AsListVal();
+
+		if ( lv->Length() == 1 )
+			v = vect->Lookup(v2);
+		else
+			{
+			int len = vect->Size();
+			VectorVal* result = new VectorVal(vect->Type()->AsVectorType());
+
+			bro_int_t first = get_slice_index(lv->Index(0)->CoerceToInt(), len);
+			bro_int_t last = get_slice_index(lv->Index(1)->CoerceToInt(), len);
+			int sub_length = last - first;
+
+			if ( sub_length >= 0 )
+				{
+				result->Resize(sub_length);
+
+				for ( int idx = first; idx < last; idx++ )
+					result->Assign(idx - first, vect->Lookup(idx)->Ref());
+				}
+
+			return result;
+			}
+		}
 		break;
 
 	case TYPE_TABLE:
@@ -3175,7 +3237,27 @@ void IndexExpr::Assign(Frame* f, Val* v, Opcode op)
 
 	switch ( v1->Type()->Tag() ) {
 	case TYPE_VECTOR:
-		if ( ! v1->AsVectorVal()->Assign(v2, v, op) )
+		{
+		const ListVal* lv = v2->AsListVal();
+		VectorVal* v1_vect = v1->AsVectorVal();
+
+		if ( lv->Length() > 1 )
+			{
+			auto len = v1_vect->Size();
+			bro_int_t first = get_slice_index(lv->Index(0)->CoerceToInt(), len);
+			bro_int_t last = get_slice_index(lv->Index(1)->CoerceToInt(), len);
+
+			// Remove the elements from the vector within the slice
+			for ( auto idx = first; idx < last; idx++ )
+				v1_vect->Remove(first);
+
+			// Insert the new elements starting at the first position
+			VectorVal* v_vect = v->AsVectorVal();
+
+			for ( auto idx = 0u; idx < v_vect->Size(); idx++, first++ )
+				v1_vect->Insert(first, v_vect->Lookup(idx)->Ref());
+			}
+		else if ( ! v1_vect->Assign(v2, v, op) )
 			{
 			if ( v )
 				{
@@ -3192,6 +3274,7 @@ void IndexExpr::Assign(Frame* f, Val* v, Opcode op)
 				RuntimeErrorWithCallStack("assignment failed with null value");
 			}
 		break;
+		}
 
 	case TYPE_TABLE:
 		if ( ! v1->AsTableVal()->Assign(v2, v, op) )
@@ -3257,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)
@@ -5702,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/Type.cc

@@ -1773,10 +1773,12 @@ int VectorType::MatchesIndex(ListExpr*& index) const
 	{
 	expr_list& el = index->Exprs();
 
-	if ( el.length() != 1 )
+	if ( el.length() != 1 && el.length() != 2)
 		return DOES_NOT_MATCH_INDEX;
 
-	if ( el[0]->Type()->Tag() == TYPE_VECTOR )
+	if ( el.length() == 2 )
+		return MATCHES_INDEX_VECTOR;
+	else if ( el[0]->Type()->Tag() == TYPE_VECTOR )
 		return (IsIntegral(el[0]->Type()->YieldType()->Tag()) ||
 			 IsBool(el[0]->Type()->YieldType()->Tag())) ?
 				MATCHES_INDEX_VECTOR : DOES_NOT_MATCH_INDEX;

src/Val.cc

@@ -3407,6 +3407,45 @@ bool VectorVal::AssignRepeat(unsigned int index, unsigned int how_many,
 	return true;
 	}
 
+bool VectorVal::Insert(unsigned int index, Val* element)
+	{
+	if ( element &&
+	     ! same_type(element->Type(), vector_type->YieldType(), 0) )
+		{
+		Unref(element);
+		return false;
+		}
+
+	vector<Val*>::iterator it;
+
+	if ( index < val.vector_val->size() )
+		it = std::next(val.vector_val->begin(), index);
+	else
+		it = val.vector_val->end();
+
+	// Note: we do *not* Ref() the element, if any, at this point.
+	// AssignExpr::Eval() already does this; other callers must remember
+	// to do it similarly.
+	val.vector_val->insert(it, element);
+
+	Modified();
+	return true;
+	}
+
+bool VectorVal::Remove(unsigned int index)
+	{
+	if ( index >= val.vector_val->size() )
+		return false;
+
+	Val* val_at_index = (*val.vector_val)[index];
+	auto it = std::next(val.vector_val->begin(), index);
+	val.vector_val->erase(it);
+	Unref(val_at_index);
+
+	Modified();
+	return true;
+	}
+
 int VectorVal::AddTo(Val* val, int /* is_first_init */) const
 	{
 	if ( val->Type()->Tag() != TYPE_VECTOR )

src/Val.h

@@ -1194,6 +1194,12 @@ public:
 	// Won't shrink size.
 	unsigned int ResizeAtLeast(unsigned int new_num_elements);
 
+	// Insert an element at a specific position into the underlying vector.
+	bool Insert(unsigned int index, Val* element);
+
+	// Removes an element at a specific position.
+	bool Remove(unsigned int index);
+
 protected:
 	friend class Val;
 	VectorVal()	{ }

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,15 +485,7 @@ expr:
 			$$ = new IndexExpr($1, $3);
 			}
 
-	|	expr '[' opt_expr ':' opt_expr ']'
+	|	index_slice
-			{
-			set_location(@1, @6);
-			Expr* low = $3 ? $3 : new ConstExpr(val_mgr->GetCount(0));
-			Expr* high = $5 ? $5 : new SizeExpr($1);
-			ListExpr* le = new ListExpr(low);
-			le->Append(high);
-			$$ = new IndexExpr($1, le, true);
-			}
 
 	|	expr '$' TOK_ID
 			{
@@ -1267,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
 	|
@@ -1470,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/Baseline/language.vector/out

@@ -58,3 +58,12 @@ access element (PASS)
 && operator (PASS)
 || operator (PASS)
 += operator (PASS)
+slicing (PASS)
+slicing (PASS)
+slicing (PASS)
+slicing (PASS)
+slicing (PASS)
+slicing assignment (PASS)
+slicing assignment (PASS)
+slicing assignment grow (PASS)
+slicing assignment shrink (PASS)

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);
+	}

testing/btest/language/vector.zeek

@@ -168,5 +168,19 @@ event zeek_init()
 	v16 += 40;
 	test_case( "+= operator", all_set(v16 == vector( 10, 20, 30, 40 )) );
 
+	# Slicing tests.
+	local v17 = vector( 1, 2, 3, 4, 5 );
+	test_case( "slicing", all_set(v17[0:2] == vector( 1, 2 )) );
+	test_case( "slicing", all_set(v17[-3:-1] == vector( 3, 4 )) );
+	test_case( "slicing", all_set(v17[:2] == vector( 1, 2 )) );
+	test_case( "slicing", all_set(v17[2:] == vector( 3, 4, 5 )) );
+	test_case( "slicing", all_set(v17[:] == v17) );
+	v17[0:1] = vector(6);
+	test_case( "slicing assignment", all_set(v17 == vector(6, 2, 3, 4, 5)) );
+	v17[2:4] = vector(7, 8);
+	test_case( "slicing assignment", all_set(v17 == vector(6, 2, 7, 8, 5)) );
+	v17[2:4] = vector(9, 10, 11);
+	test_case( "slicing assignment grow", all_set(v17 == vector(6, 2, 9, 10, 11, 5)) );
+	v17[2:5] = vector(9);
+	test_case( "slicing assignment shrink", all_set(v17 == vector(6, 2, 9, 5)) );
 }
-