From 0af79a7a16d6f54fb2c35a5a82633c7655ad31dc Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Wed, 12 Jun 2019 13:48:56 -0700 Subject: [PATCH 1/7] GH-393: Add slice notation for vectors --- src/Expr.cc | 35 ++++++++++++++++++---- src/Type.cc | 6 ++-- testing/btest/Baseline/language.vector/out | 4 +++ testing/btest/language/vector.zeek | 7 ++++- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/Expr.cc b/src/Expr.cc index 1ec357e945..3486ea1d6b 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -2913,8 +2913,8 @@ IndexExpr::IndexExpr(Expr* arg_op1, ListExpr* arg_op2, bool is_slice) if ( is_slice ) { - if ( ! IsString(op1->Type()->Tag()) ) - ExprError("slice notation indexing only supported for strings currently"); + if ( ! IsString(op1->Type()->Tag()) && ! IsVector(op1->Type()->Tag()) ) + ExprError("slice notation indexing only supported for strings and vectors currently"); } else if ( IsString(op1->Type()->Tag()) ) @@ -2937,8 +2937,7 @@ IndexExpr::IndexExpr(Expr* arg_op1, ListExpr* arg_op2, bool is_slice) else if ( ! op1->Type()->YieldType() ) { - if ( IsString(op1->Type()->Tag()) && - match_type == MATCHES_INDEX_SCALAR ) + if ( IsString(op1->Type()->Tag()) && 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 +3103,33 @@ 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 = nullptr; + + 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 = new VectorVal(vect->Type()->AsVectorType()); + 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: diff --git a/src/Type.cc b/src/Type.cc index 3fda9aa7ce..094a974255 100644 --- a/src/Type.cc +++ b/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; diff --git a/testing/btest/Baseline/language.vector/out b/testing/btest/Baseline/language.vector/out index 0fdcc1fa24..d6ead054f1 100644 --- a/testing/btest/Baseline/language.vector/out +++ b/testing/btest/Baseline/language.vector/out @@ -58,3 +58,7 @@ access element (PASS) && operator (PASS) || operator (PASS) += operator (PASS) +slicing (PASS) +slicing (PASS) +slicing (PASS) +slicing (PASS) diff --git a/testing/btest/language/vector.zeek b/testing/btest/language/vector.zeek index 0564e52e4f..c4147b76ad 100644 --- a/testing/btest/language/vector.zeek +++ b/testing/btest/language/vector.zeek @@ -168,5 +168,10 @@ 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 )) ); } - From f1383d98c2dbfc76a412f57de806d2d17d86da31 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Wed, 12 Jun 2019 14:29:11 -0700 Subject: [PATCH 2/7] Return an empty vector if the indices for slicing don't make sense --- src/Expr.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Expr.cc b/src/Expr.cc index 3486ea1d6b..8d5bfcdc83 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -3112,7 +3112,7 @@ Val* IndexExpr::Fold(Val* v1, Val* v2) const else { int len = vect->Size(); - VectorVal* result = nullptr; + 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); @@ -3120,7 +3120,6 @@ Val* IndexExpr::Fold(Val* v1, Val* v2) const if ( sub_length >= 0 ) { - result = new VectorVal(vect->Type()->AsVectorType()); result->Resize(sub_length); for ( int idx = first; idx < last; idx++ ) From 964e2c91a3e5330431eb6d82e7e0b901a6fe1f00 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 13 Jun 2019 13:36:10 -0700 Subject: [PATCH 3/7] Check for integral slice indexes, add extra test for [:] --- src/parse.y | 2 ++ testing/btest/Baseline/language.vector/out | 1 + testing/btest/language/vector.zeek | 1 + 3 files changed, 4 insertions(+) diff --git a/src/parse.y b/src/parse.y index 2861b95dc8..07a544bb64 100644 --- a/src/parse.y +++ b/src/parse.y @@ -484,6 +484,8 @@ 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->FatalError("slice notation must have integral values as indexes"); ListExpr* le = new ListExpr(low); le->Append(high); $$ = new IndexExpr($1, le, true); diff --git a/testing/btest/Baseline/language.vector/out b/testing/btest/Baseline/language.vector/out index d6ead054f1..9e12403ef7 100644 --- a/testing/btest/Baseline/language.vector/out +++ b/testing/btest/Baseline/language.vector/out @@ -62,3 +62,4 @@ slicing (PASS) slicing (PASS) slicing (PASS) slicing (PASS) +slicing (PASS) diff --git a/testing/btest/language/vector.zeek b/testing/btest/language/vector.zeek index c4147b76ad..04218eb023 100644 --- a/testing/btest/language/vector.zeek +++ b/testing/btest/language/vector.zeek @@ -174,4 +174,5 @@ event zeek_init() 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) ); } From 23f9fb0ae92b182e645ed52e729ff33575513cf4 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 13 Jun 2019 15:37:31 -0700 Subject: [PATCH 4/7] Allow assignment for vectors using slices --- src/Expr.cc | 22 +++++++++++++++++++++- testing/btest/Baseline/language.vector/out | 2 ++ testing/btest/language/vector.zeek | 4 ++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/Expr.cc b/src/Expr.cc index 8d5bfcdc83..768d70ef0b 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -3199,7 +3199,26 @@ 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(); + + if ( lv->Length() > 1 ) + { + int len = v1->AsVectorVal()->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); + int slice_length = last - first; + + const VectorVal *v_vect = v->AsVectorVal(); + if ( slice_length != v_vect->Size()) + RuntimeError("vector being assigned to slice does not match size of slice"); + else if ( slice_length >= 0 ) + { + for ( int idx = first; idx < last; idx++ ) + v1->AsVectorVal()->Assign(idx, v_vect->Lookup(idx - first)->Ref(), op); + } + } + else if ( !v1->AsVectorVal()->Assign(v2, v, op)) { if ( v ) { @@ -3216,6 +3235,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) ) diff --git a/testing/btest/Baseline/language.vector/out b/testing/btest/Baseline/language.vector/out index 9e12403ef7..cfd9b75413 100644 --- a/testing/btest/Baseline/language.vector/out +++ b/testing/btest/Baseline/language.vector/out @@ -63,3 +63,5 @@ slicing (PASS) slicing (PASS) slicing (PASS) slicing (PASS) +slicing assignment (PASS) +slicing assignment (PASS) diff --git a/testing/btest/language/vector.zeek b/testing/btest/language/vector.zeek index 04218eb023..e8b3f68e76 100644 --- a/testing/btest/language/vector.zeek +++ b/testing/btest/language/vector.zeek @@ -175,4 +175,8 @@ event zeek_init() 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)) ); } From 502ad9abc38bb2b7421ac23dd68f47f52b3c610c Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Fri, 14 Jun 2019 11:23:28 -0700 Subject: [PATCH 5/7] Add ability to grow/shrink a vector using slicing, also adds Insert/Remove methods for VectorVal --- src/Expr.cc | 20 +++++++----- src/Val.cc | 38 ++++++++++++++++++++++ src/Val.h | 6 ++++ testing/btest/Baseline/language.vector/out | 2 ++ testing/btest/language/vector.zeek | 4 +++ 5 files changed, 61 insertions(+), 9 deletions(-) diff --git a/src/Expr.cc b/src/Expr.cc index 768d70ef0b..ea8253e347 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -3201,24 +3201,26 @@ void IndexExpr::Assign(Frame* f, Val* v, Opcode op) case TYPE_VECTOR: { const ListVal *lv = v2->AsListVal(); + VectorVal* v1_vect = v1->AsVectorVal(); if ( lv->Length() > 1 ) { - int len = v1->AsVectorVal()->Size(); + int 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); - int slice_length = last - first; - const VectorVal *v_vect = v->AsVectorVal(); - if ( slice_length != v_vect->Size()) - RuntimeError("vector being assigned to slice does not match size of slice"); - else if ( slice_length >= 0 ) + // Remove the elements from the vector within the slice + for ( int idx = first; idx < last; idx++ ) + v1_vect->Remove(first); + + // Insert the new elements starting at the first position + VectorVal* v_vect = v->AsVectorVal(); + for ( int idx = 0; idx < v_vect->Size(); idx++, first++ ) { - for ( int idx = first; idx < last; idx++ ) - v1->AsVectorVal()->Assign(idx, v_vect->Lookup(idx - first)->Ref(), op); + v1_vect->Insert(first, v_vect->Lookup(idx)->Ref()); } } - else if ( !v1->AsVectorVal()->Assign(v2, v, op)) + else if ( !v1_vect->Assign(v2, v, op) ) { if ( v ) { diff --git a/src/Val.cc b/src/Val.cc index bb9a3d1601..0a6c7dd985 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -3407,6 +3407,44 @@ 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::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 ) diff --git a/src/Val.h b/src/Val.h index 2890c4c5e8..20302e3a9d 100644 --- a/src/Val.h +++ b/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 or a range of elements from a specific position. + bool Remove(unsigned int start_index); + protected: friend class Val; VectorVal() { } diff --git a/testing/btest/Baseline/language.vector/out b/testing/btest/Baseline/language.vector/out index cfd9b75413..2955eda26c 100644 --- a/testing/btest/Baseline/language.vector/out +++ b/testing/btest/Baseline/language.vector/out @@ -65,3 +65,5 @@ slicing (PASS) slicing (PASS) slicing assignment (PASS) slicing assignment (PASS) +slicing assignment grow (PASS) +slicing assignment shrink (PASS) diff --git a/testing/btest/language/vector.zeek b/testing/btest/language/vector.zeek index e8b3f68e76..ea330f3842 100644 --- a/testing/btest/language/vector.zeek +++ b/testing/btest/language/vector.zeek @@ -179,4 +179,8 @@ event zeek_init() 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)) ); } From 91835752b74234ba455445855bb0ccd0dd6721b8 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 18 Jun 2019 17:25:32 -0700 Subject: [PATCH 6/7] Misc. tweaks to vector slicing implementation * Minor style/format changes * Fix a signed/unsigned comparison compiler warning * Use a non-fatal error for non-integral slice indices so we can report any further scripting errors instead of stopping the parse right there --- NEWS | 6 ++++++ src/Expr.cc | 13 ++++++------- src/Val.cc | 1 + src/Val.h | 4 ++-- src/parse.y | 4 +++- 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 6abb21c055..7ae2250cfe 100644 --- a/NEWS +++ b/NEWS @@ -94,6 +94,12 @@ New Functionality is available in the events: ``ssl_extension_pre_shared_key_client_hello`` and ``ssl_extension_pre_shared_key_server_hello``. +- 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 --------------------- diff --git a/src/Expr.cc b/src/Expr.cc index ea8253e347..58963579fd 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -3200,27 +3200,26 @@ void IndexExpr::Assign(Frame* f, Val* v, Opcode op) switch ( v1->Type()->Tag() ) { case TYPE_VECTOR: { - const ListVal *lv = v2->AsListVal(); + const ListVal* lv = v2->AsListVal(); VectorVal* v1_vect = v1->AsVectorVal(); if ( lv->Length() > 1 ) { - int len = v1_vect->Size(); + 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 ( int idx = first; idx < last; idx++ ) + 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 ( int idx = 0; idx < v_vect->Size(); idx++, first++ ) - { + + 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) ) + else if ( ! v1_vect->Assign(v2, v, op) ) { if ( v ) { diff --git a/src/Val.cc b/src/Val.cc index 0a6c7dd985..21a2d4a7db 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -3417,6 +3417,7 @@ bool VectorVal::Insert(unsigned int index, Val* element) } vector::iterator it; + if ( index < val.vector_val->size() ) it = std::next(val.vector_val->begin(), index); else diff --git a/src/Val.h b/src/Val.h index 20302e3a9d..66ad7e907e 100644 --- a/src/Val.h +++ b/src/Val.h @@ -1197,8 +1197,8 @@ public: // Insert an element at a specific position into the underlying vector. bool Insert(unsigned int index, Val* element); - // Removes an element or a range of elements from a specific position. - bool Remove(unsigned int start_index); + // Removes an element at a specific position. + bool Remove(unsigned int index); protected: friend class Val; diff --git a/src/parse.y b/src/parse.y index 07a544bb64..4c39ee191d 100644 --- a/src/parse.y +++ b/src/parse.y @@ -484,8 +484,10 @@ 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->FatalError("slice notation must have integral values as indexes"); + reporter->Error("slice notation must have integral values as indexes"); + ListExpr* le = new ListExpr(low); le->Append(high); $$ = new IndexExpr($1, le, true); From 385f500497589727e9b40a6b97b6bb68a119b13b Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 18 Jun 2019 17:42:06 -0700 Subject: [PATCH 7/7] 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 | 50 +++++++++++++++++-- src/Expr.h | 32 +++++++++++- src/SerialTypes.h | 1 + src/parse.y | 48 ++++++++++++------ testing/btest/core/leaks/vector-indexing.zeek | 30 +++++++++++ 5 files changed, 140 insertions(+), 21 deletions(-) create mode 100644 testing/btest/core/leaks/vector-indexing.zeek diff --git a/src/Expr.cc b/src/Expr.cc index 58963579fd..8f003de030 100644 --- a/src/Expr.cc +++ b/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); } diff --git a/src/Expr.h b/src/Expr.h index e268f07648..111cc20e60 100644 --- a/src/Expr.h +++ b/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 { diff --git a/src/SerialTypes.h b/src/SerialTypes.h index 029048a80f..a0b3f4231b 100644 --- a/src/SerialTypes.h +++ b/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) diff --git a/src/parse.y b/src/parse.y index 4c39ee191d..48c8143f35 100644 --- a/src/parse.y +++ b/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 init_class %type opt_init %type TOK_CONSTANT -%type expr opt_expr init anonymous_function +%type expr opt_expr init anonymous_function index_slice %type event %type stmt stmt_list func_body for_head %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); diff --git a/testing/btest/core/leaks/vector-indexing.zeek b/testing/btest/core/leaks/vector-indexing.zeek new file mode 100644 index 0000000000..540913bd08 --- /dev/null +++ b/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); + }