From 4c0c507b1fabbdab557d66eb9d90a2d3c8009880 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Tue, 1 Mar 2022 02:04:35 +0100 Subject: [PATCH 01/14] Filter: Clean up function call instruction Pass instructions of function call arguments as vararg arguments to FI_CALL instruction constructor and move necessary magic from parser code to interpreter / instruction code. --- filter/config.Y | 23 +++++++++-------------- filter/f-inst.c | 24 +++++++++++++++++------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/filter/config.Y b/filter/config.Y index 8916ea97..0ab32a57 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -700,27 +700,22 @@ var_list: /* EMPTY */ { $$ = NULL; } | var_list ',' term { $$ = $3; $$->next = $1; } function_call: - CF_SYM_KNOWN '(' var_list ')' { + CF_SYM_KNOWN '(' var_list ')' + { if ($1->class != SYM_FUNCTION) cf_error("You can't call something which is not a function. Really."); - struct f_inst *fc = f_new_inst(FI_CALL, $1); - uint args = 0; + /* Revert the var_list */ + struct f_inst *args = NULL; while ($3) { - args++; - struct f_inst *tmp = $3->next; - $3->next = fc; + struct f_inst *tmp = $3; + $3 = $3->next; - fc = $3; - $3 = tmp; + tmp->next = args; + args = tmp; } - if (args != $1->function->args) - cf_error("Function call '%s' got %u arguments, need %u arguments.", - $1->name, args, $1->function->args); - - $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_VOID }); - $$->next = fc; + $$ = f_new_inst(FI_CALL, args, $1); } ; diff --git a/filter/f-inst.c b/filter/f-inst.c index 901d2939..0d935859 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -1079,8 +1079,22 @@ INST(FI_CALL, 0, 1) { NEVER_CONSTANT; + VARARG; SYMBOL; + FID_NEW_BODY() + ASSERT(sym->class == SYM_FUNCTION); + + if (whati->varcount != sym->function->args) + cf_error("Function '%s' expects %u arguments, got %u arguments", + sym->name, sym->function->args, whati->varcount); + + /* Add implicit void slot for the return value */ + struct f_inst *tmp = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_VOID }); + tmp->next = whati->fvar; + whati->fvar = tmp; + what->size += tmp->size; + FID_SAME_BODY() if (!(f1->sym->flags & SYM_FLAG_SAME)) return 0; @@ -1092,15 +1106,11 @@ /* Push the body on stack */ LINEX(sym->function); + curline.vbase = curline.ventry; curline.emask |= FE_RETURN; - /* Before this instruction was called, there was the T_VOID - * automatic return value pushed on value stack and also - * sym->function->args function arguments. Setting the - * vbase to point to first argument. */ - ASSERT(curline.ventry >= sym->function->args); - curline.ventry -= sym->function->args; - curline.vbase = curline.ventry; + /* Arguments on stack */ + fstk->vcnt += sym->function->args; /* Storage for local variables */ memset(&(fstk->vstk[fstk->vcnt]), 0, sizeof(struct f_val) * sym->function->vars); From 93d6096c8714f3c7c9450df89e812021a212b978 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Thu, 3 Mar 2022 03:38:12 +0100 Subject: [PATCH 02/14] Filter: Implement type checks for function calls Keep list of function parameters in f_line and use it to verify types of arguments for function calls. Only static type checks are implemented. --- conf/confbase.Y | 1 + filter/config.Y | 36 ++++++++++++++++++++++++++---------- filter/f-inst.c | 13 +++++++++++++ filter/f-inst.h | 6 ++++++ 4 files changed, 46 insertions(+), 10 deletions(-) diff --git a/conf/confbase.Y b/conf/confbase.Y index 5f45c507..18ca8489 100644 --- a/conf/confbase.Y +++ b/conf/confbase.Y @@ -75,6 +75,7 @@ CF_DECLS struct f_static_attr fsa; struct f_lval flv; struct f_line *fl; + struct f_arg *fa; const struct filter *f; struct f_tree *e; struct f_trie *trie; diff --git a/filter/config.Y b/filter/config.Y index 0ab32a57..a4b4d3d3 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -302,9 +302,10 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN, %type filter where_filter %type filter_body function_body %type lvalue -%type type function_args function_vars +%type type function_vars +%type function_argsn function_args %type ec_kind -%type break_command +%type break_command %type cnum %type pair_item ec_item lc_item set_item switch_item set_items switch_items switch_body %type fprefix_set @@ -403,18 +404,21 @@ type: ; function_argsn: - /* EMPTY */ + /* EMPTY */ { $$ = NULL; } | function_argsn type symbol ';' { if ($3->scope->slots >= 0xfe) cf_error("Too many declarations, at most 255 allowed"); - cf_define_symbol($3, SYM_VARIABLE | $2, offset, $3->scope->slots++); + $$ = cfg_alloc(sizeof(struct f_arg)); + $$->arg = cf_define_symbol($3, SYM_VARIABLE | $2, offset, $3->scope->slots++); + $$->next = $1; } ; function_args: - '(' ')' { $$ = 0; } + '(' ')' { $$ = NULL; } | '(' function_argsn type symbol ')' { - cf_define_symbol($4, SYM_VARIABLE | $3, offset, $4->scope->slots++); - $$ = $4->scope->slots; + $$ = cfg_alloc(sizeof(struct f_arg)); + $$->arg = cf_define_symbol($4, SYM_VARIABLE | $3, offset, $4->scope->slots++); + $$->next = $2; } ; @@ -459,9 +463,21 @@ function_def: FUNCTION symbol { DBG( "Beginning of function %s\n", $2->name ); $2 = cf_define_symbol($2, SYM_FUNCTION, function, NULL); cf_push_scope($2); - } function_args function_body { - DBG("Definition of function %s with %u args and %u local vars.\n", $2->name, $4, $5->vars); - $5->args = $4; + } function_args function_body + { + $5->arg_list = NULL; + $5->args = 0; + + /* Revert the args */ + while ($4) { + struct f_arg *tmp = $4; + $4 = $4->next; + + tmp->next = $5->arg_list; + $5->arg_list = tmp; + $5->args++; + } + $2->function = $5; cf_pop_scope(); } diff --git a/filter/f-inst.c b/filter/f-inst.c index 0d935859..e9bae684 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -1089,6 +1089,19 @@ cf_error("Function '%s' expects %u arguments, got %u arguments", sym->name, sym->function->args, whati->varcount); + /* Typecheck individual arguments */ + struct f_inst *a = fvar; + struct f_arg *b = sym->function->arg_list; + for (uint i = 1; a && b; a = a->next, b = b->next, i++) + { + enum f_type b_type = b->arg->class & 0xff; + + if (a->type && (a->type != b_type) && !f_const_promotion(a, b_type)) + cf_error("Argument %u of '%s' must be %s, got %s", + i, sym->name, f_type_name(b_type), f_type_name(a->type)); + } + ASSERT(!a && !b); + /* Add implicit void slot for the return value */ struct f_inst *tmp = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_VOID }); tmp->next = whati->fvar; diff --git a/filter/f-inst.h b/filter/f-inst.h index df45f88e..12df40f6 100644 --- a/filter/f-inst.h +++ b/filter/f-inst.h @@ -35,12 +35,18 @@ const char *f_instruction_name_(enum f_instruction_code fi); static inline const char *f_instruction_name(enum f_instruction_code fi) { return f_instruction_name_(fi) + 3; } +struct f_arg { + struct symbol *arg; + struct f_arg *next; +}; + /* Filter structures for execution */ /* Line of instructions to be unconditionally executed one after another */ struct f_line { uint len; /* Line length */ u8 args; /* Function: Args required */ u8 vars; + struct f_arg *arg_list; struct f_line_item items[0]; /* The items themselves */ }; From cde8094c1f81a74d8a434c4cbf41b4d9183e2879 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Thu, 3 Mar 2022 15:11:05 +0100 Subject: [PATCH 03/14] Filter: Improve description of type system --- filter/f-inst.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/filter/f-inst.c b/filter/f-inst.c index e9bae684..2120440d 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -62,6 +62,7 @@ * m4_dnl INST(FI_NOP, in, out) { enum value, input args, output args * m4_dnl ARG(num, type); argument, its id (in data fields) and type accessible by v1, v2, v3 * m4_dnl ARG_ANY(num); argument with no type check accessible by v1, v2, v3 + * m4_dnl ARG_TYPE(num, type); just declare the type of argument * m4_dnl VARARG; variable-length argument list; accessible by vv(i) and whati->varcount * m4_dnl LINE(num, unused); this argument has to be converted to its own f_line * m4_dnl SYMBOL; symbol handed from config @@ -80,7 +81,9 @@ * m4_dnl ) * * m4_dnl RESULT(type, union-field, value); putting this on value stack + * m4_dnl RESULT_(type, union-field, value); like RESULT(), but do not declare the type * m4_dnl RESULT_VAL(value-struct); pass the struct f_val directly + * m4_dnl RESULT_TYPE(type); just declare the type of result value * m4_dnl RESULT_VOID; return undef * m4_dnl } * @@ -91,6 +94,24 @@ * * Other code is just copied into the interpreter part. * + * The filter language uses a simple type system, where values have types + * (constants T_*) and also terms (instructions) are statically typed. Our + * static typing is partial (some terms do not declare types of arguments + * or results), therefore it can detect most but not all type errors and + * therefore we still have runtime type checks. + * + * m4_dnl Types of arguments are declared by macros ARG() and ARG_TYPE(), + * m4_dnl types of results are declared by RESULT() and RESULT_TYPE(). + * m4_dnl Macros ARG_ANY(), RESULT_() and RESULT_VAL() do not declare types + * m4_dnl themselves, but can be combined with ARG_TYPE() / RESULT_TYPE(). + * + * m4_dnl Note that types should be declared only once. If there are + * m4_dnl multiple RESULT() macros in an instruction definition, they must + * m4_dnl use the exact same expression for type, or they should be replaced + * m4_dnl by multiple RESULT_() macros and a common RESULT_TYPE() macro. + * m4_dnl See e.g. FI_EA_GET or FI_MIN instructions. + * + * * If you are satisfied with this, you don't need to read the following * detailed description of what is really done with the instruction definitions. * @@ -216,6 +237,37 @@ * * m4_dnl If you are stymied, see FI_CALL or FI_CONSTANT or just search for * m4_dnl the mentioned macros in this file to see what is happening there in wild. + * + * + * A note about soundness of the type system: + * + * A type system is sound when types of expressions are consistent with + * types of values resulting from evaluation of such expressions. Untyped + * expressions are ok, but badly typed expressions are not sound. So is + * the type system of BIRD filtering code sound? There are some points: + * + * All cases of (one) m4_dnl RESULT() macro are obviously ok, as the macro + * both declares a type and returns a value. One have to check instructions + * that use m4_dnl RESULT_TYPE() macro. There are two issues: + * + * FI_AND, FI_OR - second argument is statically checked to be T_BOOL and + * passed as result without dynamic typecheck, declared to be T_BOOL. If + * an untyped non-bool expression is used as a second argument, then + * the mismatched type is returned. + * + * FI_VAR_GET - soundness depends on consistency of declared symbol types + * and stored values. This is maintained when values are stored by + * FI_VAR_SET, but when they are stored by FI_CALL, only static checking is + * used, so when an untyped expression returning mismatched value is used + * as a function argument, then inconsistent value is stored and subsequent + * FI_VAR_GET would be unsound. + * + * Both of these issues are inconsequential, as mismatched values from + * unsound expressions will be caught by dynamic typechecks like mismatched + * values from untyped expressions. + * + * Also note that FI_CALL is the only expression without properly declared + * result type. */ /* Binary operators */ From 9b302c133f02f96edc5caa769dfd21dc9ef038b5 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Thu, 3 Mar 2022 20:34:44 +0100 Subject: [PATCH 04/14] Filter: Ensure that all expressions declared return type All instructions with a return value (i.e. expressions, ones with non-zero outval, third argument in INST()) should declare their return type. Check that automatically by M4 macros. Set outval of FI_RETURN to 0. The instruction adds one value to stack, but syntactically it is a statement, not an expression. Add fake return type declaration to FI_CALL, otherwise the automatic check would fail builds. --- filter/decl.m4 | 8 +++++++- filter/f-inst.c | 7 +++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/filter/decl.m4 b/filter/decl.m4 index 5242c04c..4f7ca3ad 100644 --- a/filter/decl.m4 +++ b/filter/decl.m4 @@ -238,9 +238,13 @@ m4_define(ERROR, # This macro specifies result type and makes there are no conflicting definitions m4_define(RESULT_TYPE, `m4_ifdef([[INST_RESULT_TYPE]], - [[m4_ifelse(INST_RESULT_TYPE,$1,,[[ERROR([[Multiple type definitons]])]])]], + [[m4_ifelse(INST_RESULT_TYPE,$1,,[[ERROR([[Multiple type definitions in]] INST_NAME)]])]], [[m4_define(INST_RESULT_TYPE,$1) RESULT_TYPE_($1)]])') +m4_define(RESULT_TYPE_CHECK, + `m4_ifelse(INST_OUTVAL,0,, + [[m4_ifdef([[INST_RESULT_TYPE]],,[[ERROR([[Missing type definition in]] INST_NAME)]])]])') + m4_define(RESULT_TYPE_, ` FID_NEW_BODY()m4_dnl what->type = $1; @@ -294,6 +298,7 @@ m4_define(FID_ITERATE, `FID_ZONE(10, Iteration)') # This macro does all the code wrapping. See inline comments. m4_define(INST_FLUSH, `m4_ifdef([[INST_NAME]], [[ +RESULT_TYPE_CHECK()m4_dnl Check for defined RESULT_TYPE() FID_ENUM()m4_dnl Contents of enum fi_code { ... } INST_NAME(), FID_ENUM_STR()m4_dnl Contents of const char * indexed by enum fi_code @@ -392,6 +397,7 @@ m4_define(INST, `m4_dnl This macro is called on beginning of each instruction INST_FLUSH()m4_dnl First, old data is flushed m4_define([[INST_NAME]], [[$1]])m4_dnl Then we store instruction name, m4_define([[INST_INVAL]], [[$2]])m4_dnl instruction input value count, +m4_define([[INST_OUTVAL]], [[$3]])m4_dnl instruction output value count, m4_undefine([[INST_NEVER_CONSTANT]])m4_dnl reset NEVER_CONSTANT trigger, m4_undefine([[INST_RESULT_TYPE]])m4_dnl and reset RESULT_TYPE value. FID_INTERPRET_BODY()m4_dnl By default, every code is interpreter code. diff --git a/filter/f-inst.c b/filter/f-inst.c index 2120440d..2a02c8e5 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -1103,7 +1103,7 @@ } } - INST(FI_RETURN, 1, 1) { + INST(FI_RETURN, 1, 0) { NEVER_CONSTANT; /* Acquire the return value */ ARG_ANY(1); @@ -1134,6 +1134,9 @@ VARARG; SYMBOL; + /* Fake result type declaration */ + RESULT_TYPE(T_VOID); + FID_NEW_BODY() ASSERT(sym->class == SYM_FUNCTION); @@ -1443,7 +1446,7 @@ } - INST(FI_FORMAT, 1, 0) { /* Format */ + INST(FI_FORMAT, 1, 1) { /* Format */ ARG_ANY(1); RESULT(T_STRING, s, val_format_str(fpool, &v1)); } From 8f3c6151b4ff11d98a20f6f9919723f0fb719161 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Fri, 4 Mar 2022 02:01:34 +0100 Subject: [PATCH 05/14] Nest: Cleanups in as_path_filter() Use struct f_val as a common argument for as_path_filter(), as suggested by Alexander Zubkov. That allows to use NULL sets as valid arguments. --- filter/data.h | 2 ++ filter/f-inst.c | 21 ++++++--------------- nest/a-path.c | 12 +++++++----- nest/a-path_test.c | 6 ++++-- nest/attrs.h | 3 ++- 5 files changed, 21 insertions(+), 23 deletions(-) diff --git a/filter/data.h b/filter/data.h index 4cb6b7a8..8cba8c0e 100644 --- a/filter/data.h +++ b/filter/data.h @@ -282,6 +282,8 @@ static inline int eclist_set_type(const struct f_tree *set) { return set->from.type == T_EC; } static inline int lclist_set_type(const struct f_tree *set) { return set->from.type == T_LC; } +static inline int path_set_type(const struct f_tree *set) +{ return set->from.type == T_INT; } const struct adata *clist_filter(struct linpool *pool, const struct adata *list, const struct f_val *set, int pos); const struct adata *eclist_filter(struct linpool *pool, const struct adata *list, const struct f_val *set, int pos); diff --git a/filter/f-inst.c b/filter/f-inst.c index 2a02c8e5..5d42c40a 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -1035,7 +1035,7 @@ RESULT(T_INT, i, v1.val.lc.ldp2); } - INST(FI_MIN, 1, 1) { /* Get minimum element from set */ + INST(FI_MIN, 1, 1) { /* Get minimum element from list */ ARG_ANY(1); RESULT_TYPE(f_type_element_type(v1.type)); switch(v1.type) @@ -1069,7 +1069,7 @@ } } - INST(FI_MAX, 1, 1) { /* Get maximum element from set */ + INST(FI_MAX, 1, 1) { /* Get maximum element from list */ ARG_ANY(1); RESULT_TYPE(f_type_element_type(v1.type)); switch(v1.type) @@ -1291,17 +1291,10 @@ if (v1.type == T_PATH) { - const struct f_tree *set = NULL; - u32 key = 0; - - if (v2.type == T_INT) - key = v2.val.i; - else if ((v2.type == T_SET) && (v2.val.t->from.type == T_INT)) - set = v2.val.t; + if ((v2.type == T_SET) && path_set_type(v2.val.t) || (v2.type == T_INT)) + RESULT_(T_PATH, ad, [[ as_path_filter(fpool, v1.val.ad, &v2, 0) ]]); else runtime("Can't delete non-integer (set)"); - - RESULT_(T_PATH, ad, [[ as_path_filter(fpool, v1.val.ad, set, key, 0) ]]); } else if (v1.type == T_CLIST) @@ -1353,10 +1346,8 @@ if (v1.type == T_PATH) { - u32 key = 0; - - if ((v2.type == T_SET) && (v2.val.t->from.type == T_INT)) - RESULT_(T_PATH, ad, [[ as_path_filter(fpool, v1.val.ad, v2.val.t, key, 1) ]]); + if ((v2.type == T_SET) && path_set_type(v2.val.t)) + RESULT_(T_PATH, ad, [[ as_path_filter(fpool, v1.val.ad, &v2, 1) ]]); else runtime("Can't filter integer"); } diff --git a/nest/a-path.c b/nest/a-path.c index 2e34a3d1..d5b01635 100644 --- a/nest/a-path.c +++ b/nest/a-path.c @@ -602,8 +602,10 @@ as_path_match_set(const struct adata *path, const struct f_tree *set) } const struct adata * -as_path_filter(struct linpool *pool, const struct adata *path, const struct f_tree *set, u32 key, int pos) +as_path_filter(struct linpool *pool, const struct adata *path, const struct f_val *set, int pos) { + ASSERT((set->type == T_SET) || (set->type == T_INT)); + if (!path) return NULL; @@ -629,13 +631,13 @@ as_path_filter(struct linpool *pool, const struct adata *path, const struct f_tr u32 as = get_as(p); int match; - if (set) + if (set->type == T_SET) { struct f_val v = {T_INT, .val.i = as}; - match = !!find_tree(set, &v); + match = !!find_tree(set->val.t, &v); } - else - match = (as == key); + else /* T_INT */ + match = (as == set->val.i); if (match == pos) { diff --git a/nest/a-path_test.c b/nest/a-path_test.c index 9ed0a786..97924c00 100644 --- a/nest/a-path_test.c +++ b/nest/a-path_test.c @@ -12,6 +12,7 @@ #include "nest/route.h" #include "nest/attrs.h" #include "lib/resource.h" +#include "filter/data.h" #define TESTS_NUM 30 #define AS_PATH_LENGTH 1000 @@ -136,8 +137,9 @@ t_path_include(void) int counts_of_contains = count_asn_in_array(as_nums, as_nums[i]); bt_assert_msg(as_path_contains(as_path, as_nums[i], counts_of_contains), "AS Path should contains %d-times number %d", counts_of_contains, as_nums[i]); - bt_assert(as_path_filter(lp, as_path, NULL, as_nums[i], 0) != NULL); - bt_assert(as_path_filter(lp, as_path, NULL, as_nums[i], 1) != NULL); + struct f_val v = { .type = T_INT, .val.i = as_nums[i] }; + bt_assert(as_path_filter(lp, as_path, &v, 0) != NULL); + bt_assert(as_path_filter(lp, as_path, &v, 1) != NULL); } for (i = 0; i < 10000; i++) diff --git a/nest/attrs.h b/nest/attrs.h index ef2b95e6..22e2ff4a 100644 --- a/nest/attrs.h +++ b/nest/attrs.h @@ -28,6 +28,7 @@ * to 16bit slot (like in 16bit AS_PATH). See RFC 4893 for details */ +struct f_val; struct f_tree; int as_path_valid(byte *data, uint len, int bs, int sets, int confed, char *err, uint elen); @@ -49,7 +50,7 @@ int as_path_get_last(const struct adata *path, u32 *last_as); u32 as_path_get_last_nonaggregated(const struct adata *path); int as_path_contains(const struct adata *path, u32 as, int min); int as_path_match_set(const struct adata *path, const struct f_tree *set); -const struct adata *as_path_filter(struct linpool *pool, const struct adata *path, const struct f_tree *set, u32 key, int pos); +const struct adata *as_path_filter(struct linpool *pool, const struct adata *path, const struct f_val *set, int pos); static inline struct adata *as_path_prepend(struct linpool *pool, const struct adata *path, u32 as) { return as_path_prepend2(pool, path, AS_PATH_SEQUENCE, as); } From b2d6d2948af268812a8f55f260d340194eb3f7ac Mon Sep 17 00:00:00 2001 From: Alexander Zubkov Date: Fri, 4 Mar 2022 14:07:58 +0100 Subject: [PATCH 06/14] Filter: Add literal for empty set Add literal for empty set [], which works both for tree-based sets and prefix sets by using existing constant promotion mechanism. Minor changes by committer. --- filter/config.Y | 2 ++ filter/data.c | 14 +++++++++++ filter/data.h | 8 +++--- filter/decl.m4 | 5 ++++ filter/f-inst.c | 2 +- filter/test.conf | 64 ++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 90 insertions(+), 5 deletions(-) diff --git a/filter/config.Y b/filter/config.Y index a4b4d3d3..a4e82c7d 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -674,6 +674,7 @@ bgp_path: bgp_path_tail: NUM bgp_path_tail { $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_PATH_MASK_ITEM, .val.pmi = { .asn = $1, .kind = PM_ASN, }, }); $$->next = $2; } | NUM DDOT NUM bgp_path_tail { $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_PATH_MASK_ITEM, .val.pmi = { .from = $1, .to = $3, .kind = PM_ASN_RANGE }, }); $$->next = $4; } + | '[' ']' bgp_path_tail { $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_PATH_MASK_ITEM, .val.pmi = { .set = NULL, .kind = PM_ASN_SET }, }); $$->next = $3; } | '[' set_items ']' bgp_path_tail { if ($2->from.type != T_INT) cf_error("Only integer sets allowed in path mask"); $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_PATH_MASK_ITEM, .val.pmi = { .set = build_tree($2), .kind = PM_ASN_SET }, }); $$->next = $4; @@ -693,6 +694,7 @@ constant: | fipa { $$ = f_new_inst(FI_CONSTANT, $1); } | VPN_RD { $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_RD, .val.ec = $1, }); } | net_ { $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_NET, .val.net = $1, }); } + | '[' ']' { $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_SET, .val.t = NULL, }); } | '[' set_items ']' { DBG( "We've got a set here..." ); $$ = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_SET, .val.t = build_tree($2), }); diff --git a/filter/data.c b/filter/data.c index 56c1fb17..276738a9 100644 --- a/filter/data.c +++ b/filter/data.c @@ -79,6 +79,8 @@ f_type_element_type(enum f_type t) }; } +const struct f_trie f_const_empty_trie = { .ipv4 = -1, }; + const struct f_val f_const_empty_path = { .type = T_PATH, .val.ad = &null_adata, @@ -91,6 +93,9 @@ const struct f_val f_const_empty_path = { }, f_const_empty_lclist = { .type = T_LCLIST, .val.ad = &null_adata, +}, f_const_empty_prefix_set = { + .type = T_PREFIX_SET, + .val.ti = &f_const_empty_trie, }; static struct adata * @@ -301,6 +306,12 @@ val_same(const struct f_val *v1, const struct f_val *v2) int clist_set_type(const struct f_tree *set, struct f_val *v) { + if (!set) + { + v->type = T_VOID; + return 1; + } + switch (set->from.type) { case T_PAIR: @@ -537,6 +548,9 @@ val_in_range(const struct f_val *v1, const struct f_val *v2) if (v2->type != T_SET) return F_CMP_ERROR; + if (!v2->val.t) + return 0; + /* With integrated Quad<->IP implicit conversion */ if ((v1->type == v2->val.t->from.type) || ((v1->type == T_QUAD) && val_is_ip4(&(v2->val.t->from)) && val_is_ip4(&(v2->val.t->to)))) diff --git a/filter/data.h b/filter/data.h index 8cba8c0e..221e4229 100644 --- a/filter/data.h +++ b/filter/data.h @@ -279,11 +279,11 @@ int val_in_range(const struct f_val *v1, const struct f_val *v2); int clist_set_type(const struct f_tree *set, struct f_val *v); static inline int eclist_set_type(const struct f_tree *set) -{ return set->from.type == T_EC; } +{ return !set || set->from.type == T_EC; } static inline int lclist_set_type(const struct f_tree *set) -{ return set->from.type == T_LC; } +{ return !set || set->from.type == T_LC; } static inline int path_set_type(const struct f_tree *set) -{ return set->from.type == T_INT; } +{ return !set || set->from.type == T_INT; } const struct adata *clist_filter(struct linpool *pool, const struct adata *list, const struct f_val *set, int pos); const struct adata *eclist_filter(struct linpool *pool, const struct adata *list, const struct f_val *set, int pos); @@ -299,7 +299,7 @@ undef_value(struct f_val v) (v.val.ad == &null_adata); } -extern const struct f_val f_const_empty_path, f_const_empty_clist, f_const_empty_eclist, f_const_empty_lclist; +extern const struct f_val f_const_empty_path, f_const_empty_clist, f_const_empty_eclist, f_const_empty_lclist, f_const_empty_prefix_set; enum filter_return f_eval(const struct f_line *expr, struct linpool *tmp_pool, struct f_val *pres); diff --git a/filter/decl.m4 b/filter/decl.m4 index 4f7ca3ad..fc26e193 100644 --- a/filter/decl.m4 +++ b/filter/decl.m4 @@ -501,6 +501,11 @@ f_const_promotion(struct f_inst *arg, enum f_type want) return 1; } + else if ((c->type == T_SET) && (!c->val.t) && (want == T_PREFIX_SET)) { + *c = f_const_empty_prefix_set; + return 1; + } + return 0; } diff --git a/filter/f-inst.c b/filter/f-inst.c index 5d42c40a..d321f06d 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -401,7 +401,7 @@ break; case T_SET: - if (vv(i).val.t->from.type != T_INT) + if (!path_set_type(vv(i).val.t)) runtime("Only integer sets allowed in path mask"); pm->item[i] = (struct f_path_mask_item) { diff --git a/filter/test.conf b/filter/test.conf index 484628e5..50c58b3a 100644 --- a/filter/test.conf +++ b/filter/test.conf @@ -130,12 +130,18 @@ define is3 = [5, 17, 2, 11, 8, 15, 3, 19]; function t_int_set() int set is; { + is = []; + bt_assert(is = []); + bt_assert(0 !~ is); + bt_assert(1 ~ [1,2,3]); bt_assert(5 ~ [1..20]); bt_assert(2 ~ [ 1, 2, 3 ]); bt_assert(5 ~ [ 4 .. 7 ]); bt_assert(1 !~ [ 2, 3, 4 ]); bt_assert(999 !~ [ 666, 333 ]); + bt_assert(1 !~ []); + bt_assert(1 !~ is); is = [ 2, 3, 4, 7..11 ]; bt_assert(10 ~ is); @@ -170,6 +176,7 @@ int set is; bt_assert([1,4..10,20] = [1,4..10,20]); bt_assert(format([ 1, 2, 1, 1, 1, 3, 4, 1, 1, 1, 5 ]) = "[1, 1, 1, 1, 1, 1, 1, 2, 3, 4, 5]"); + bt_assert(format([]) = "[]"); } bt_test_suite(t_int_set, "Testing sets of integers"); @@ -237,6 +244,9 @@ pair pp; pair set ps; { pp = (1, 2); + ps = []; + bt_assert(pp !~ ps); + ps = [(1,(one+one)), (3,4)..(4,8), (5,*), (6,3..6)]; bt_assert(format(ps) = "[(1,2), (3,4)..(4,8), (5,0)..(5,65535), (6,3)..(6,6)]"); bt_assert(pp ~ ps); @@ -253,6 +263,7 @@ pair set ps; bt_assert((6,6+one) !~ ps); bt_assert(((one+6),2) !~ ps); bt_assert((1,1) !~ ps); + bt_assert(pp !~ []); ps = [(20..150, 200..300), (50100..50200, 1000..50000), (*, 5+5)]; bt_assert((100,200) ~ ps); @@ -304,6 +315,7 @@ quad qq; qq = 1.2.3.4; bt_assert(qq ~ [1.2.3.4, 5.6.7.8]); bt_assert(qq !~ [1.2.1.1, 1.2.3.5]); + bt_assert(qq !~ []); } bt_test_suite(t_quad_set, "Testing sets of quads"); @@ -384,6 +396,7 @@ ip set ips; bt_assert(1.2.3.4 !~ [ 1.2.3.3, 1.2.3.5 ]); bt_assert(1.2.3.4 ~ [ 1.2.3.3..1.2.3.5 ]); + bt_assert(1.2.3.4 !~ []); } bt_test_suite(t_ip_set, "Testing sets of ip address"); @@ -473,13 +486,34 @@ function test_pxset(prefix set pxs) bt_assert(1.0.0.0/8 ~ [ 1.0.0.0/8+ ]); bt_assert(1.0.0.0/9 !~ [ 1.0.0.0/8- ]); bt_assert(1.2.0.0/17 !~ [ 1.0.0.0/8{ 15 , 16 } ]); + bt_assert(net10 !~ []); bt_assert([ 10.0.0.0/8{ 15 , 17 } ] = [ 10.0.0.0/8{ 15 , 17 } ]); } +function test_empty_pxset(prefix set pxs) +int set s0; +prefix set s1; +{ + s0 = []; + s1 = []; + bt_assert(pxs != s0); + bt_assert(pxs = s1); + #bt_assert(pxs = []); +} + function t_prefix_set() prefix set pxs; { + pxs = []; + bt_assert(format(pxs) = "[]"); + #bt_assert(pxs = []); + bt_assert(1.2.0.0/16 !~ []); + bt_assert(1.2.0.0/16 !~ pxs); + + test_empty_pxset([]); + test_empty_pxset(pxs); + pxs = [ 1.2.0.0/16, 1.4.0.0/16+, 44.66.88.64/30{24,28}, 12.34.56.0/24{8,16} ]; bt_assert(format(pxs) = "[1.2.0.0/16{0.1.0.0}, 1.4.0.0/16{0.1.255.255}, 12.34.0.0/16{1.255.0.0}, 44.66.88.64/28{0.0.1.240}]"); @@ -564,6 +598,12 @@ bt_test_suite(t_prefix6, "Testing prefix IPv6"); function t_prefix6_set() prefix set pxs; { + pxs = []; + bt_assert(format(pxs) = "[]"); + #bt_assert(pxs = []); + bt_assert(12::34/128 !~ []); + bt_assert(12::34/128 !~ pxs); + bt_assert(1180::/16 ~ [ 1100::/8{15, 17} ]); bt_assert(12::34 = 12::34); bt_assert(12::34 ~ [ 12::33..12::35 ]); @@ -681,6 +721,7 @@ int set set12; bt_assert(3 ~ p2); bt_assert(p2 ~ [2, 10..20]); bt_assert(p2 ~ [4, 10..20]); + bt_assert(p2 !~ []); p2 = prepend(p2, 5); bt_assert(p2 !~ pm1); @@ -691,6 +732,8 @@ int set set12; bt_assert(p2 ~ [= 5 [2, 4, 6] 3 [1..2] 1 =]); bt_assert(p2 ~ [= 5 set35 3 set12 set12 =]); bt_assert(p2 ~ mkpath(5, 4)); + bt_assert(p2 ~ [= * [3] * =]); + bt_assert(p2 !~ [= * [] * =]); bt_assert(p2.len = 5); bt_assert(p2.first = 5); @@ -699,6 +742,10 @@ int set set12; bt_assert(p2.len = 5); bt_assert(delete(p2, 3) = prepend(prepend(prepend(prepend(+empty+, 1), 2), 4), 5)); bt_assert(filter(p2, [1..3]) = prepend(prepend(prepend(+empty+, 1), 2), 3)); + bt_assert(delete(p2, []) = p2); + bt_assert(filter(p2, []) = +empty+); + bt_assert(delete(prepend(prepend(+empty+, 0), 1), []) = prepend(prepend(+empty+, 0), 1)); + bt_assert(filter(prepend(prepend(+empty+, 0), 1), []) = +empty+); p2 = prepend( + empty +, 5 ); p2 = prepend( p2, 4 ); @@ -759,6 +806,7 @@ clist r; bt_assert(l ~ [(2,2..3)]); bt_assert(l ~ [(1,1..2)]); bt_assert(l ~ [(1,1)..(1,2)]); + bt_assert(l !~ []); l = add(l, (2,5)); l = add(l, (5,one)); @@ -796,6 +844,9 @@ clist r; bt_assert(l !~ [(*,(one+6))]); bt_assert(l !~ [(*, (one+one+one))]); + bt_assert(delete(l, []) = l); + bt_assert(filter(l, []) = -empty-); + l = delete(l, [(*,(one+onef(3)))]); l = delete(l, [(*,(4+one))]); bt_assert(l = add(-empty-, (3,1))); @@ -913,11 +964,15 @@ eclist r; bt_assert((ro, 10.20.30.40, 100) !~ el); bt_assert(el !~ [(rt, 10, 35..40)]); bt_assert(el !~ [(ro, 10, *)]); + bt_assert(el !~ []); el = add(el, (rt, 10, 40)); el2 = filter(el, [(rt, 10, 20..40)] ); el2 = add(el2, (rt, 10, 50)); + bt_assert(delete(el, []) = el); + bt_assert(filter(el, []) = --empty--); + # eclist A (1,30,40) bt_assert(el = add(add(add(--empty--, (rt, 10, 1)), (rt, 10, 30)), (rt, 10, 40))); bt_assert(format(el) = "(eclist (rt, 10, 1) (rt, 10, 30) (rt, 10, 40))"); @@ -1035,6 +1090,9 @@ lclist r; ll2 = add(ll2, (30, 30, 30)); ll2 = add(ll2, (40, 40, 40)); + bt_assert(delete(ll, []) = ll); + bt_assert(filter(ll, []) = ---empty---); + # lclist A (10, 20, 30) bt_assert(format(ll) = "(lclist (10, 10, 10) (20, 20, 20) (30, 30, 30))"); @@ -1094,6 +1152,7 @@ lc set lls; bt_assert(ll !~ [(5,10,15), (10,21,30)]); bt_assert(ll !~ [(10,21..25,*)]); bt_assert(ll !~ [(11, *, *)]); + bt_assert(ll !~ []); lls = [(10, 10, 10), (20, 20, 15..25), (30, 30, *), (40, 35..45, *), (50, *, *), (55..65, *, *)]; bt_assert(format(lls) = "[(10, 10, 10), (20, 20, 15)..(20, 20, 25), (30, 30, 0)..(30, 30, 4294967295), (40, 35, 0)..(40, 45, 4294967295), (50, 0, 0)..(50, 4294967295, 4294967295), (55, 0, 0)..(65, 4294967295, 4294967295)]"); @@ -1150,6 +1209,10 @@ bt_test_suite(t_rd, "Testing route distinguishers"); function t_rd_set() rd set rds; { + rds = []; + bt_assert(rds = []); + bt_assert(10:20 !~ rds); + rds = [10:20, 100000:100..100000:200]; bt_assert(format(rds) = "[10:20, 100000:100..100000:200]"); @@ -1160,6 +1223,7 @@ rd set rds; bt_assert(100000:128 ~ rds); bt_assert(100000:200 ~ rds); bt_assert(100010:150 !~ rds); + bt_assert(100010:150 !~ []); } bt_test_suite(t_rd_set, "Testing sets of route distinguishers"); From fb1d8f65136aa6190b527b691f24abe16a461471 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Fri, 4 Mar 2022 17:51:50 +0100 Subject: [PATCH 07/14] Filter: Apply constant promotion for FI_EQ / FI_NEQ Equality comparison is defined on all values, even of different types, but we still want to do constant promotion if possible. --- filter/data.c | 2 +- filter/decl.m4 | 6 ++++++ filter/f-inst.c | 2 ++ filter/test.conf | 8 ++++---- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/filter/data.c b/filter/data.c index 276738a9..2bb5920c 100644 --- a/filter/data.c +++ b/filter/data.c @@ -192,7 +192,7 @@ val_compare(const struct f_val *v1, const struct f_val *v2) if (val_is_ip4(v1) && (v2->type == T_QUAD)) return uint_cmp(ipa_to_u32(v1->val.ip), v2->val.i); - debug( "Types do not match in val_compare\n" ); + DBG( "Types do not match in val_compare\n" ); return F_CMP_ERROR; } diff --git a/filter/decl.m4 b/filter/decl.m4 index fc26e193..a2a8c88a 100644 --- a/filter/decl.m4 +++ b/filter/decl.m4 @@ -186,6 +186,12 @@ if (f$1->type && f$2->type && (f$1->type != f$2->type) && cf_error("Arguments $1 and $2 of %s must be of the same type", f_instruction_name(what->fi_code)); FID_INTERPRET_BODY()') +m4_define(ARG_PREFER_SAME_TYPE, ` +FID_NEW_BODY()m4_dnl +if (f$1->type && f$2->type && (f$1->type != f$2->type)) + (void) (f_const_promotion(f$2, f$1->type) || f_const_promotion(f$1, f$2->type)); +FID_INTERPRET_BODY()') + # Executing another filter line. This replaces the recursion # that was needed in the former implementation. m4_define(LINEX, `FID_INTERPRET_EXEC()LINEX_($1)FID_INTERPRET_NEW()return $1 FID_INTERPRET_BODY()') diff --git a/filter/f-inst.c b/filter/f-inst.c index d321f06d..500732c6 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -423,12 +423,14 @@ INST(FI_NEQ, 2, 1) { ARG_ANY(1); ARG_ANY(2); + ARG_PREFER_SAME_TYPE(1, 2); RESULT(T_BOOL, i, !val_same(&v1, &v2)); } INST(FI_EQ, 2, 1) { ARG_ANY(1); ARG_ANY(2); + ARG_PREFER_SAME_TYPE(1, 2); RESULT(T_BOOL, i, val_same(&v1, &v2)); } diff --git a/filter/test.conf b/filter/test.conf index 50c58b3a..401a999b 100644 --- a/filter/test.conf +++ b/filter/test.conf @@ -498,8 +498,8 @@ prefix set s1; s0 = []; s1 = []; bt_assert(pxs != s0); - bt_assert(pxs = s1); - #bt_assert(pxs = []); + bt_assert(pxs = s1); + bt_assert(pxs = []); } function t_prefix_set() @@ -507,7 +507,7 @@ prefix set pxs; { pxs = []; bt_assert(format(pxs) = "[]"); - #bt_assert(pxs = []); + bt_assert(pxs = []); bt_assert(1.2.0.0/16 !~ []); bt_assert(1.2.0.0/16 !~ pxs); @@ -600,7 +600,7 @@ prefix set pxs; { pxs = []; bt_assert(format(pxs) = "[]"); - #bt_assert(pxs = []); + bt_assert(pxs = []); bt_assert(12::34/128 !~ []); bt_assert(12::34/128 !~ pxs); From 26bc4f9904b014c9949489e8ae28366da473e85d Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Sun, 6 Mar 2022 02:18:01 +0100 Subject: [PATCH 08/14] Filter: Implement direct recursion Direct recursion almost worked, just crashed on function signature check. Split function parsing such that function signature is saved before function body is processed. Recursive calls are marked so they can be avoided during f_same() and similar code walking. Also, include tower of hanoi solver as a test case. --- filter/config.Y | 23 ++++++++++-------- filter/decl.m4 | 2 ++ filter/f-inst.c | 9 +++++-- filter/f-inst.h | 2 +- filter/test.conf | 63 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 13 deletions(-) diff --git a/filter/config.Y b/filter/config.Y index a4e82c7d..82a072ab 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -460,25 +460,28 @@ function_body: conf: function_def ; function_def: - FUNCTION symbol { DBG( "Beginning of function %s\n", $2->name ); + FUNCTION symbol { + DBG( "Beginning of function %s\n", $2->name ); $2 = cf_define_symbol($2, SYM_FUNCTION, function, NULL); cf_push_scope($2); - } function_args function_body - { - $5->arg_list = NULL; - $5->args = 0; + } function_args { + /* Make dummy f_line for storing function prototype */ + struct f_line *dummy = cfg_allocz(sizeof(struct f_line)); + $2->function = dummy; /* Revert the args */ while ($4) { struct f_arg *tmp = $4; $4 = $4->next; - tmp->next = $5->arg_list; - $5->arg_list = tmp; - $5->args++; + tmp->next = dummy->arg_list; + dummy->arg_list = tmp; + dummy->args++; } - - $2->function = $5; + } function_body { + $6->args = $2->function->args; + $6->arg_list = $2->function->arg_list; + $2->function = $6; cf_pop_scope(); } ; diff --git a/filter/decl.m4 b/filter/decl.m4 index a2a8c88a..3d118e34 100644 --- a/filter/decl.m4 +++ b/filter/decl.m4 @@ -376,6 +376,7 @@ case INST_NAME(): { #undef whati #undef item dest->items[pos].fi_code = what->fi_code; + dest->items[pos].flags = what->flags; dest->items[pos].lineno = what->lineno; break; } @@ -647,6 +648,7 @@ FID_WR_PUT(4)m4_dnl struct f_inst { struct f_inst *next; /* Next instruction */ enum f_instruction_code fi_code; /* Instruction code */ + enum f_instruction_flags flags; /* Flags, instruction-specific */ enum f_type type; /* Type of returned value, if known */ int size; /* How many instructions are underneath */ int lineno; /* Line number */ diff --git a/filter/f-inst.c b/filter/f-inst.c index 500732c6..e75b5e01 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -1165,11 +1165,16 @@ whati->fvar = tmp; what->size += tmp->size; + /* Mark recursive calls, they have dummy f_line */ + if (!sym->function->len) + what->flags |= FIF_RECURSIVE; + FID_SAME_BODY() - if (!(f1->sym->flags & SYM_FLAG_SAME)) - return 0; + if (!(f1->sym->flags & SYM_FLAG_SAME) && !(f1_->flags & FIF_RECURSIVE)) + return 0; FID_ITERATE_BODY() + if (!(what->flags & FIF_RECURSIVE)) BUFFER_PUSH(fit->lines) = whati->sym->function; FID_INTERPRET_BODY() diff --git a/filter/f-inst.h b/filter/f-inst.h index 12df40f6..58563c79 100644 --- a/filter/f-inst.h +++ b/filter/f-inst.h @@ -22,7 +22,7 @@ /* Flags for instructions */ enum f_instruction_flags { - FIF_PRINTED = 1, /* FI_PRINT_AND_DIE: message put in buffer */ + FIF_RECURSIVE = 1, /* FI_CALL: function is directly recursive */ } PACKED; /* Include generated filter instruction declarations */ diff --git a/filter/test.conf b/filter/test.conf index 401a999b..e8b6a663 100644 --- a/filter/test.conf +++ b/filter/test.conf @@ -1290,7 +1290,60 @@ function fifteen() return 15; } +function factorial(int x) +{ + if x = 0 then return 0; + if x = 1 then return 1; + else return x * factorial(x - 1); +} + +function fibonacci(int x) +{ + if x = 0 then return 0; + if x = 1 then return 1; + else return fibonacci(x - 1) + fibonacci(x - 2); +} + +function hanoi_init(int a; int b) +{ + if b = 0 + then return +empty+; + else return prepend(hanoi_init(a + 1, b - 1), a); +} + +function hanoi_solve(int n; bgppath h_src; bgppath h_dst; bgppath h_aux; bool x; bool y) +bgppath tmp1; +bgppath tmp2; +int v; +{ + # x -> return src or dst + # y -> print state + + if n = 0 then { if x then return h_src; else return h_dst; } + + tmp1 = hanoi_solve(n - 1, h_src, h_aux, h_dst, true, y); + tmp2 = hanoi_solve(n - 1, h_src, h_aux, h_dst, false, false); + h_src = tmp1; + h_aux = tmp2; + + v = h_src.first; + # bt_assert(h_dst = +empty+ || v < h_dst.first); + h_src = delete(h_src, v); + h_dst = prepend(h_dst, v); + + if y then + print "move: ", v, " src: ", h_src, " dst:", h_dst, " aux:", h_aux; + + tmp1 = hanoi_solve(n - 1, h_aux, h_dst, h_src, true, y); + tmp2 = hanoi_solve(n - 1, h_aux, h_dst, h_src, false, false); + h_aux = tmp1; + h_dst = tmp2; + + if x then return h_src; else return h_dst; +} + function t_call_function() +bgppath h_src; { bt_assert(fifteen() = 15); @@ -1302,6 +1355,16 @@ function t_call_function() bt_assert(callme(4, 4) = 16); bt_assert(callme(7, 2) = 14); bt_assert(callmeagain(1, 2, 3) = 6); + + bt_assert(factorial(5) = 120); + bt_assert(factorial(10) = 3628800); + + bt_assert(fibonacci(10) = 55); + bt_assert(fibonacci(20) = 6765); + + h_src = hanoi_init(1, 6); + bt_assert(format(h_src) = "(path 1 2 3 4 5 6)"); + bt_assert(hanoi_solve(6, h_src, +empty+, +empty+, false, false) = h_src); } bt_test_suite(t_call_function, "Testing calling functions"); From 946cedfcfe3bdfd850613c4d891f8abfd6b2cafd Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Sun, 6 Mar 2022 16:37:30 +0100 Subject: [PATCH 09/14] Filter: Implement soft scopes Soft scopes are anonymous scopes that most likely do not contain any symbol, so allocating regular scope is postponed when it is really needed. --- conf/cf-lex.l | 58 +++++++++++++++++++++++++++++++++++++++++++--- conf/conf.h | 9 ++++++- sysdep/unix/main.c | 2 +- 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/conf/cf-lex.l b/conf/cf-lex.l index c9d2f5a5..d6f46223 100644 --- a/conf/cf-lex.l +++ b/conf/cf-lex.l @@ -574,6 +574,8 @@ check_eof(void) return 0; } +static inline void cf_swap_soft_scope(void); + static struct symbol * cf_new_symbol(const byte *c) { @@ -583,6 +585,8 @@ cf_new_symbol(const byte *c) if (l > SYM_MAX_LEN) cf_error("Symbol too long"); + cf_swap_soft_scope(); + s = cfg_allocz(sizeof(struct symbol) + l + 1); *s = (struct symbol) { .scope = conf_this_scope, .class = SYM_VOID, }; strcpy(s->name, c); @@ -654,10 +658,10 @@ cf_localize_symbol(struct symbol *sym) /* If the symbol type is void, it has been recently allocated just in this scope. */ if (!sym->class) return sym; - + /* If the scope is the current, it is already defined in this scope. */ - if (sym->scope == conf_this_scope) - cf_error("Symbol already defined"); + if (cf_symbol_is_local(sym)) + cf_error("Symbol '%s' already defined", sym->name); /* Not allocated here yet, doing it now. */ return cf_new_symbol(sym->name); @@ -787,12 +791,60 @@ cf_push_scope(struct symbol *sym) void cf_pop_scope(void) { + ASSERT(!conf_this_scope->soft_scopes); + conf_this_scope->active = 0; conf_this_scope = conf_this_scope->next; ASSERT(conf_this_scope); } +/** + * cf_push_soft_scope - enter new soft scope + * + * If we want to enter a new anonymous scope that most likely will not contain + * any symbols, we can use cf_push_soft_scope() insteas of cf_push_scope(). + * Such scope will be converted to a regular scope on first use. + */ +void +cf_push_soft_scope(void) +{ + if (conf_this_scope->soft_scopes < 0xfe) + conf_this_scope->soft_scopes++; + else + cf_push_scope(NULL); +} + +/** + * cf_pop_soft_scope - leave a soft scope + * + * Leave a soft scope entered by cf_push_soft_scope(). + */ +void +cf_pop_soft_scope(void) +{ + if (conf_this_scope->soft_scopes) + conf_this_scope->soft_scopes--; + else + cf_pop_scope(); +} + +/** + * cf_swap_soft_scope - convert soft scope to regular scope + * + * Soft scopes cannot hold symbols, so they must be converted to regular scopes + * on first use. It is done automatically by cf_new_symbol(). + */ +static inline void +cf_swap_soft_scope(void) +{ + if (conf_this_scope->soft_scopes) + { + conf_this_scope->soft_scopes--; + cf_push_scope(NULL); + } +} + /** * cf_symbol_class_name - get name of a symbol class * @sym: symbol diff --git a/conf/conf.h b/conf/conf.h index 3bc37959..be46f172 100644 --- a/conf/conf.h +++ b/conf/conf.h @@ -133,7 +133,8 @@ struct sym_scope { struct sym_scope *next; /* Next on scope stack */ struct symbol *name; /* Name of this scope */ uint slots; /* Variable slots */ - int active; /* Currently entered */ + byte active; /* Currently entered */ + byte soft_scopes; /* Number of soft scopes above */ }; struct bytestring { @@ -190,6 +191,9 @@ struct symbol *cf_get_symbol(const byte *c); struct symbol *cf_default_name(char *template, int *counter); struct symbol *cf_localize_symbol(struct symbol *sym); +static inline int cf_symbol_is_local(struct symbol *sym) +{ return (sym->scope == conf_this_scope) && !conf_this_scope->soft_scopes; } + /** * cf_define_symbol - define meaning of a symbol * @sym: symbol to be defined @@ -213,6 +217,9 @@ struct symbol *cf_localize_symbol(struct symbol *sym); void cf_push_scope(struct symbol *); void cf_pop_scope(void); +void cf_push_soft_scope(void); +void cf_pop_soft_scope(void); + char *cf_symbol_class_name(struct symbol *sym); /* Parser */ diff --git a/sysdep/unix/main.c b/sysdep/unix/main.c index 392aff9d..f87d0c43 100644 --- a/sysdep/unix/main.c +++ b/sysdep/unix/main.c @@ -116,7 +116,7 @@ add_num_const(char *name, int val, const char *file, const uint line) struct f_val *v = cfg_alloc(sizeof(struct f_val)); *v = (struct f_val) { .type = T_INT, .val.i = val }; struct symbol *sym = cf_get_symbol(name); - if (sym->class && (sym->scope == conf_this_scope)) + if (sym->class && cf_symbol_is_local(sym)) cf_error("Error reading value for %s from %s:%d: already defined", name, file, line); cf_define_symbol(sym, SYM_CONSTANT | T_INT, val, v); From 1e6acf34bbf2d9c31246c6fc450ac3b0232531e2 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Sun, 6 Mar 2022 22:57:33 +0100 Subject: [PATCH 10/14] Filter: Fix bug in variable shadowing When a new variable used the same name as an existing symbol in an outer scope, then offset number was defined based on a scope of the existing symbol ($3) instead of a scope of the new symbol (sym_). That can lead to two variables sharing the same memory slot. --- filter/config.Y | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/filter/config.Y b/filter/config.Y index 82a072ab..51e7822f 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -408,7 +408,7 @@ function_argsn: | function_argsn type symbol ';' { if ($3->scope->slots >= 0xfe) cf_error("Too many declarations, at most 255 allowed"); $$ = cfg_alloc(sizeof(struct f_arg)); - $$->arg = cf_define_symbol($3, SYM_VARIABLE | $2, offset, $3->scope->slots++); + $$->arg = cf_define_symbol($3, SYM_VARIABLE | $2, offset, sym_->scope->slots++); $$->next = $1; } ; @@ -417,7 +417,7 @@ function_args: '(' ')' { $$ = NULL; } | '(' function_argsn type symbol ')' { $$ = cfg_alloc(sizeof(struct f_arg)); - $$->arg = cf_define_symbol($4, SYM_VARIABLE | $3, offset, $4->scope->slots++); + $$->arg = cf_define_symbol($4, SYM_VARIABLE | $3, offset, sym_->scope->slots++); $$->next = $2; } ; @@ -425,7 +425,7 @@ function_args: function_vars: /* EMPTY */ { $$ = 0; } | function_vars type symbol ';' { - cf_define_symbol($3, SYM_VARIABLE | $2, offset, $3->scope->slots++); + cf_define_symbol($3, SYM_VARIABLE | $2, offset, sym_->scope->slots++); $$ = $1 + 1; } ; From f31f4e6eef1762a4b9bf54a274f1ba9e9e161802 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Wed, 9 Mar 2022 00:31:39 +0100 Subject: [PATCH 11/14] Filter: Simplify handling of command sequences Command sequences in curly braces used a separate nonterminal in grammar. Handle them as a regular command. --- filter/config.Y | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/filter/config.Y b/filter/config.Y index 51e7822f..a3acf245 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -296,7 +296,7 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN, %nonassoc ELSE %type cmds_int cmd_prep -%type term block cmd cmds constant constructor print_list var_list function_call symbol_value bgp_path_expr bgp_path bgp_path_tail +%type term cmd cmds constant constructor print_list var_list function_call symbol_value bgp_path_expr bgp_path bgp_path_tail %type dynamic_attr %type static_attr %type filter where_filter @@ -514,15 +514,6 @@ cmds_int: cmd_prep } ; -block: - cmd { - $$=$1; - } - | '{' cmds '}' { - $$=$2; - } - ; - /* * Complex types, their bison value is struct f_val */ @@ -864,10 +855,13 @@ print_list: /* EMPTY */ { $$ = NULL; } ; cmd: - IF term THEN block { + '{' cmds '}' { + $$ = $2; + } + | IF term THEN cmd { $$ = f_new_inst(FI_CONDITION, $2, $4, NULL); } - | IF term THEN block ELSE block { + | IF term THEN cmd ELSE cmd { $$ = f_new_inst(FI_CONDITION, $2, $4, $6); } | CF_SYM_KNOWN '=' term ';' { From a2527ee53d9d8fe7a1c29b56f8450b9ef1f9c7bc Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Wed, 9 Mar 2022 02:32:29 +0100 Subject: [PATCH 12/14] Filter: Improve handling of stack frames in filter bytecode When f_line is done, we have to pop the stack frame. The old code just removed nominal number of args/vars. Change it to use stored ventry value modified by number of returned values. This allows to allocate variables on a stack frame during execution of f_lines instead of just at start. But we need to know the number of returned values for a f_line. It is 1 for term, 0 for cmd. Store that to f_line during linearization. --- conf/confbase.Y | 4 ++-- filter/config.Y | 14 +++++++------- filter/decl.m4 | 6 ++++-- filter/f-inst.c | 8 ++++---- filter/f-inst.h | 7 ++++--- filter/f-util.c | 2 +- filter/filter.c | 3 +-- nest/config.Y | 4 ++-- proto/static/config.Y | 2 +- 9 files changed, 26 insertions(+), 24 deletions(-) diff --git a/conf/confbase.Y b/conf/confbase.Y index 18ca8489..1d5738ff 100644 --- a/conf/confbase.Y +++ b/conf/confbase.Y @@ -153,14 +153,14 @@ conf: definition ; definition: DEFINE symbol '=' term ';' { struct f_val *val = cfg_allocz(sizeof(struct f_val)); - if (f_eval(f_linearize($4), cfg_mem, val) > F_RETURN) cf_error("Runtime error"); + if (f_eval(f_linearize($4, 1), cfg_mem, val) > F_RETURN) cf_error("Runtime error"); cf_define_symbol($2, SYM_CONSTANT | val->type, val, val); } ; expr: NUM - | '(' term ')' { $$ = f_eval_int(f_linearize($2)); } + | '(' term ')' { $$ = f_eval_int(f_linearize($2, 1)); } | CF_SYM_KNOWN { if ($1->class != (SYM_CONSTANT | T_INT)) cf_error("Number constant expected"); $$ = SYM_VAL($1).i; } diff --git a/filter/config.Y b/filter/config.Y index a3acf245..b67ca925 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -329,7 +329,7 @@ filter_def: conf: filter_eval ; filter_eval: - EVAL term { f_eval_int(f_linearize($2)); } + EVAL term { f_eval_int(f_linearize($2, 1)); } ; conf: custom_attr ; @@ -453,7 +453,7 @@ where_filter: function_body: function_vars '{' cmds '}' { - $$ = f_linearize($3); + $$ = f_linearize($3, 0); $$->vars = $1; } ; @@ -537,7 +537,7 @@ set_atom: | VPN_RD { $$.type = T_RD; $$.val.ec = $1; } | ENUM { $$.type = pair_a($1); $$.val.i = pair_b($1); } | '(' term ')' { - if (f_eval(f_linearize($2), cfg_mem, &($$)) > F_RETURN) cf_error("Runtime error"); + if (f_eval(f_linearize($2, 1), cfg_mem, &($$)) > F_RETURN) cf_error("Runtime error"); if (!f_valid_set_type($$.type)) cf_error("Set-incompatible type"); } | CF_SYM_KNOWN { @@ -549,13 +549,13 @@ set_atom: switch_atom: NUM { $$.type = T_INT; $$.val.i = $1; } - | '(' term ')' { $$.type = T_INT; $$.val.i = f_eval_int(f_linearize($2)); } + | '(' term ')' { $$.type = T_INT; $$.val.i = f_eval_int(f_linearize($2, 1)); } | fipa { $$ = $1; } | ENUM { $$.type = pair_a($1); $$.val.i = pair_b($1); } ; cnum: - term { $$ = f_eval_int(f_linearize($1)); } + term { $$ = f_eval_int(f_linearize($1, 1)); } pair_item: '(' cnum ',' cnum ')' { $$ = f_new_pair_item($2, $2, $4, $4); } @@ -642,7 +642,7 @@ switch_body: /* EMPTY */ { $$ = NULL; } | switch_body switch_items ':' cmds { /* Fill data fields */ struct f_tree *t; - struct f_line *line = f_linearize($4); + struct f_line *line = f_linearize($4, 0); for (t = $2; t; t = t->left) t->data = line; $$ = f_merge_items($1, $2); @@ -651,7 +651,7 @@ switch_body: /* EMPTY */ { $$ = NULL; } struct f_tree *t = f_new_tree(); t->from.type = t->to.type = T_VOID; t->right = t; - t->data = f_linearize($3); + t->data = f_linearize($3, 0); $$ = f_merge_items($1, t); } ; diff --git a/filter/decl.m4 b/filter/decl.m4 index 3d118e34..4c56cd9c 100644 --- a/filter/decl.m4 +++ b/filter/decl.m4 @@ -216,7 +216,7 @@ whati->f$1 = f$1; FID_DUMP_BODY()m4_dnl f_dump_line(item->fl$1, indent + 1); FID_LINEARIZE_BODY()m4_dnl -item->fl$1 = f_linearize(whati->f$1); +item->fl$1 = f_linearize(whati->f$1, $2); FID_SAME_BODY()m4_dnl if (!f_same(f1->fl$1, f2->fl$1)) return 0; FID_ITERATE_BODY()m4_dnl @@ -568,7 +568,7 @@ FID_WR_PUT(8) } struct f_line * -f_linearize_concat(const struct f_inst * const inst[], uint count) +f_linearize_concat(const struct f_inst * const inst[], uint count, uint results) { uint len = 0; for (uint i=0; ilen = linearize(out, inst[i], out->len); + out->results = results; + #ifdef LOCAL_DEBUG f_dump_line(out, 0); #endif diff --git a/filter/f-inst.c b/filter/f-inst.c index e75b5e01..5b8310c3 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -64,7 +64,7 @@ * m4_dnl ARG_ANY(num); argument with no type check accessible by v1, v2, v3 * m4_dnl ARG_TYPE(num, type); just declare the type of argument * m4_dnl VARARG; variable-length argument list; accessible by vv(i) and whati->varcount - * m4_dnl LINE(num, unused); this argument has to be converted to its own f_line + * m4_dnl LINE(num, out); this argument has to be converted to its own f_line * m4_dnl SYMBOL; symbol handed from config * m4_dnl STATIC_ATTR; static attribute definition * m4_dnl DYNAMIC_ATTR; dynamic attribute definition @@ -298,7 +298,7 @@ RESULT_TYPE(T_BOOL); if (v1.val.i) - LINE(2,0); + LINE(2,1); else RESULT_VAL(v1); } @@ -308,7 +308,7 @@ RESULT_TYPE(T_BOOL); if (!v1.val.i) - LINE(2,0); + LINE(2,1); else RESULT_VAL(v1); } @@ -536,7 +536,7 @@ if (v1.val.i) LINE(2,0); else - LINE(3,1); + LINE(3,0); } INST(FI_PRINT, 0, 0) { diff --git a/filter/f-inst.h b/filter/f-inst.h index 58563c79..e35f71c6 100644 --- a/filter/f-inst.h +++ b/filter/f-inst.h @@ -46,14 +46,15 @@ struct f_line { uint len; /* Line length */ u8 args; /* Function: Args required */ u8 vars; + u8 results; /* Results left on stack: cmd -> 0, term -> 1 */ struct f_arg *arg_list; struct f_line_item items[0]; /* The items themselves */ }; /* Convert the f_inst infix tree to the f_line structures */ -struct f_line *f_linearize_concat(const struct f_inst * const inst[], uint count); -static inline struct f_line *f_linearize(const struct f_inst *root) -{ return f_linearize_concat(&root, 1); } +struct f_line *f_linearize_concat(const struct f_inst * const inst[], uint count, uint results); +static inline struct f_line *f_linearize(const struct f_inst *root, uint results) +{ return f_linearize_concat(&root, 1, results); } void f_dump_line(const struct f_line *, uint indent); diff --git a/filter/f-util.c b/filter/f-util.c index 410999a6..fdb314b5 100644 --- a/filter/f-util.c +++ b/filter/f-util.c @@ -37,7 +37,7 @@ struct filter *f_new_where(struct f_inst *where) f_new_inst(FI_DIE, F_REJECT)); struct filter *f = cfg_allocz(sizeof(struct filter)); - f->root = f_linearize(cond); + f->root = f_linearize(cond, 0); return f; } diff --git a/filter/filter.c b/filter/filter.c index e505d570..20a380dc 100644 --- a/filter/filter.c +++ b/filter/filter.c @@ -215,8 +215,7 @@ interpret(struct filter_state *fs, const struct f_line *line, struct f_val *val) } /* End of current line. Drop local variables before exiting. */ - fstk->vcnt -= curline.line->vars; - fstk->vcnt -= curline.line->args; + fstk->vcnt = curline.ventry + curline.line->results; fstk->ecnt--; } diff --git a/nest/config.Y b/nest/config.Y index 17999422..ac599c09 100644 --- a/nest/config.Y +++ b/nest/config.Y @@ -165,7 +165,7 @@ rtrid: idval: NUM { $$ = $1; } - | '(' term ')' { $$ = f_eval_int(f_linearize($2)); } + | '(' term ')' { $$ = f_eval_int(f_linearize($2, 1)); } | IP4 { $$ = ip4_to_u32($1); } | CF_SYM_KNOWN { if ($1->class == (SYM_CONSTANT | T_INT) || $1->class == (SYM_CONSTANT | T_QUAD)) @@ -860,7 +860,7 @@ CF_CLI(DUMP FILTER ALL,,, [[Dump all filters in linearized form]]) { filters_dump_all(); cli_msg(0, ""); } ; CF_CLI(EVAL, term, , [[Evaluate an expression]]) -{ cmd_eval(f_linearize($2)); } ; +{ cmd_eval(f_linearize($2, 1)); } ; CF_CLI_HELP(ECHO, ..., [[Control echoing of log messages]]) CF_CLI(ECHO, echo_mask echo_size, (all | off | { debug|trace|info|remote|warning|error|auth [, ...] }) [], [[Control echoing of log messages]]) { diff --git a/proto/static/config.Y b/proto/static/config.Y index 41e10dbf..9d26ee82 100644 --- a/proto/static/config.Y +++ b/proto/static/config.Y @@ -40,7 +40,7 @@ static_route_finish(void) if (net_type_match(this_srt->net, NB_DEST) == !this_srt->dest) cf_error("Unexpected or missing nexthop/type"); - this_srt->cmds = f_linearize(this_srt_cmds); + this_srt->cmds = f_linearize(this_srt_cmds, 0); } CF_DECLS From 1ac8e11bba15551ad6473a57a585649757fefa6b Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Thu, 10 Mar 2022 01:02:45 +0100 Subject: [PATCH 13/14] Filter: Implement mixed declarations of local variables Allow variable declarations mixed with code, also in nested blocks with proper scoping, and with variable initializers. E.g: function fn(int a) { int b; int c = 10; if a > 20 then { b = 30; int d = c * 2; print a, b, c, d; } string s = "Hello"; } --- doc/bird.sgml | 23 +++++++++++-------- filter/config.Y | 59 ++++++++++++++++++++++++++++++++++++++++++------ filter/f-inst.c | 12 ++++++++++ filter/test.conf | 59 ++++++++++++++++++++++++++++++++---------------- 4 files changed, 116 insertions(+), 37 deletions(-) diff --git a/doc/bird.sgml b/doc/bird.sgml index 326fc7a8..f933128c 100644 --- a/doc/bird.sgml +++ b/doc/bird.sgml @@ -1260,8 +1260,8 @@ this: filter not_too_far -int var; { + int var; if defined( rip_metric ) then var = rip_metric; else { @@ -1290,9 +1290,9 @@ local variables. Recursion is not allowed. Function definitions look like this: function name () -int local_variable; { - local_variable = 5; + int local_variable; + int another_variable = 5; } function with_parameters (int parameter) @@ -1301,16 +1301,19 @@ function with_parameters (int parameter) } -

Unlike in C, variables are declared after the name(); with_parameters(5);. Function may return -values using the return command. Returning a value exits -from current function (this is similar to C). +

Like in C programming language, variables are declared inside function body, +either at the beginning, or mixed with other statements. Declarations may +contain initialization. You can also declare variables in nested blocks, such +variables have scope restricted to such block. There is a deprecated syntax to +declare variables after the name(); with_parameters(5);. Function +may return values using the return command. Returning a +value exits from current function (this is similar to C). -

Filters are defined in a way similar to functions except they can't have +

Filters are defined in a way similar to functions except they cannot have explicit parameters. They get a route table entry as an implicit parameter, it is also passed automatically to any functions called. The filter must terminate -with either A nice trick to debug filters is to use show route filter diff --git a/filter/config.Y b/filter/config.Y index b67ca925..f8f47862 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -22,6 +22,36 @@ static inline u32 pair_b(u32 p) { return p & 0xFFFF; } #define f_generate_complex(fi_code, da, arg) \ f_new_inst(FI_EA_SET, f_new_inst(fi_code, f_new_inst(FI_EA_GET, da), arg), da) +static int +f_new_var(struct sym_scope *s) +{ + /* + * - A variable is an offset on vstack from vbase. + * - Vbase is set on filter start / function call. + * - Scopes contain anonymous scopes (blocks) inside filter/function scope + * - Each scope knows number of vars in that scope + * - Offset is therefore a sum of 'slots' up to named scope + * - New variables are added on top of vstk, so intermediate values cannot + * be there during FI_VAR_INIT. I.e. no 'var' inside 'term'. + * - Also, each f_line must always have its scope, otherwise a variable may + * be defined but not initialized if relevant f_line is not executed. + */ + + int offset = s->slots++; + + while (!s->name) + { + s = s->next; + ASSERT(s); + offset += s->slots; + } + + if (offset >= 0xff) + cf_error("Too many variables, at most 255 allowed"); + + return offset; +} + /* * Sets and their items are during parsing handled as lists, linked * through left ptr. The first item in a list also contains a pointer @@ -296,7 +326,7 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN, %nonassoc ELSE %type cmds_int cmd_prep -%type term cmd cmds constant constructor print_list var_list function_call symbol_value bgp_path_expr bgp_path bgp_path_tail +%type term cmd cmd_var cmds cmds_scoped constant constructor print_list var var_init var_list function_call symbol_value bgp_path_expr bgp_path bgp_path_tail %type dynamic_attr %type static_attr %type filter where_filter @@ -425,7 +455,7 @@ function_args: function_vars: /* EMPTY */ { $$ = 0; } | function_vars type symbol ';' { - cf_define_symbol($3, SYM_VARIABLE | $2, offset, sym_->scope->slots++); + cf_define_symbol($3, SYM_VARIABLE | $2, offset, f_new_var(sym_->scope)); $$ = $1 + 1; } ; @@ -492,7 +522,11 @@ cmds: /* EMPTY */ { $$ = NULL; } | cmds_int { $$ = $1.begin; } ; -cmd_prep: cmd { +cmds_scoped: { cf_push_soft_scope(); } cmds { cf_pop_soft_scope(); $$ = $2; } ; + +cmd_var: var | cmd ; + +cmd_prep: cmd_var { $$.begin = $$.end = $1; if ($1) while ($$.end->next) @@ -639,7 +673,7 @@ fprefix_set: ; switch_body: /* EMPTY */ { $$ = NULL; } - | switch_body switch_items ':' cmds { + | switch_body switch_items ':' cmds_scoped { /* Fill data fields */ struct f_tree *t; struct f_line *line = f_linearize($4, 0); @@ -647,7 +681,7 @@ switch_body: /* EMPTY */ { $$ = NULL; } t->data = line; $$ = f_merge_items($1, $2); } - | switch_body ELSECOL cmds { + | switch_body ELSECOL cmds_scoped { struct f_tree *t = f_new_tree(); t->from.type = t->to.type = T_VOID; t->right = t; @@ -854,8 +888,19 @@ print_list: /* EMPTY */ { $$ = NULL; } } ; +var_init: + /* empty */ { $$ = f_new_inst(FI_CONSTANT, (struct f_val) { }); } + | '=' term { $$ = $2; } + ; + +var: + type symbol var_init ';' { + struct symbol *sym = cf_define_symbol($2, SYM_VARIABLE | $1, offset, f_new_var(sym_->scope)); + $$ = f_new_inst(FI_VAR_INIT, $3, sym); + } + cmd: - '{' cmds '}' { + '{' cmds_scoped '}' { $$ = $2; } | IF term THEN cmd { @@ -912,7 +957,7 @@ cmd: | PRINTN print_list ';' { $$ = f_new_inst(FI_PRINT, $2); } - | function_call ';' { $$ = f_new_inst(FI_DROP_RESULT, $1); } + | function_call ';' { $$ = f_new_inst(FI_DROP_RESULT, $1); } | CASE term '{' switch_body '}' { $$ = f_new_inst(FI_SWITCH, $2, build_tree($4)); } diff --git a/filter/f-inst.c b/filter/f-inst.c index 5b8310c3..dc243b4e 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -501,6 +501,18 @@ RESULT(T_BOOL, i, ipa_is_ip4(v1.val.ip)); } + INST(FI_VAR_INIT, 1, 0) { + NEVER_CONSTANT; + ARG_ANY(1); + SYMBOL; + ARG_TYPE(1, sym->class & 0xff); + + /* New variable is always the last on stack */ + uint pos = curline.vbase + sym->offset; + fstk->vstk[pos] = v1; + fstk->vcnt = pos + 1; + } + /* Set to indirect value prepared in v1 */ INST(FI_VAR_SET, 1, 0) { NEVER_CONSTANT; diff --git a/filter/test.conf b/filter/test.conf index e8b6a663..436031a3 100644 --- a/filter/test.conf +++ b/filter/test.conf @@ -44,9 +44,8 @@ bt_test_same(onef, twof, 0); */ function t_bool() -bool b; { - b = true; + bool b = true; bt_assert(b); bt_assert(!!b); @@ -82,12 +81,11 @@ define xyzzy = (120+10); define '1a-a1' = (xyzzy-100); function t_int() -int i; { bt_assert(xyzzy = 130); bt_assert('1a-a1' = 30); - i = four; + int i = four; i = 12*100 + 60/2 + i; i = (i + 0); bt_assert(i = 1234); @@ -128,9 +126,8 @@ define is2 = [(17+2), 17, 15, 11, 8, 5, 3, 2]; define is3 = [5, 17, 2, 11, 8, 15, 3, 19]; function t_int_set() -int set is; { - is = []; + int set is = []; bt_assert(is = []); bt_assert(0 !~ is); @@ -190,9 +187,8 @@ bt_test_suite(t_int_set, "Testing sets of integers"); */ function t_string() -string st; { - st = "Hello"; + string st = "Hello"; bt_assert(format(st) = "Hello"); bt_assert(st ~ "Hell*"); bt_assert(st ~ "?ello"); @@ -217,9 +213,8 @@ function 'mkpair-a'(int a) } function t_pair() -pair pp; { - pp = (1, 2); + pair pp = (1, 2); bt_assert(format(pp) = "(1,2)"); bt_assert((1,2) = pp); bt_assert((1,1+1) = pp); @@ -240,11 +235,9 @@ bt_test_suite(t_pair, "Testing pairs"); */ function t_pair_set() -pair pp; -pair set ps; { - pp = (1, 2); - ps = []; + pair pp = (1, 2); + pair set ps = []; bt_assert(pp !~ ps); ps = [(1,(one+one)), (3,4)..(4,8), (5,*), (6,3..6)]; @@ -1290,6 +1283,34 @@ function fifteen() return 15; } +function local_vars(int j) +{ + int k = 10; + bt_assert(j = 5 && k = 10); + { + int j = 15; + k = 20; + bt_assert(j = 15 && k = 20); + } + bt_assert(j = 5 && k = 20); + + if j < 10 then + { + int j = 25; + string k = "hello"; + bt_assert(j = 25 && k = "hello"); + } + bt_assert(j = 5 && k = 20); + + int m = 100; + { + j = 35; + int k = 40; + bt_assert(j = 35 && k = 40 && m = 100); + } + bt_assert(j = 35 && k = 20 && m = 100); +} + function factorial(int x) { if x = 0 then return 0; @@ -1312,21 +1333,18 @@ function hanoi_init(int a; int b) } function hanoi_solve(int n; bgppath h_src; bgppath h_dst; bgppath h_aux; bool x; bool y) -bgppath tmp1; -bgppath tmp2; -int v; { # x -> return src or dst # y -> print state if n = 0 then { if x then return h_src; else return h_dst; } - tmp1 = hanoi_solve(n - 1, h_src, h_aux, h_dst, true, y); - tmp2 = hanoi_solve(n - 1, h_src, h_aux, h_dst, false, false); + bgppath tmp1 = hanoi_solve(n - 1, h_src, h_aux, h_dst, true, y); + bgppath tmp2 = hanoi_solve(n - 1, h_src, h_aux, h_dst, false, false); h_src = tmp1; h_aux = tmp2; - v = h_src.first; + int v = h_src.first; # bt_assert(h_dst = +empty+ || v < h_dst.first); h_src = delete(h_src, v); h_dst = prepend(h_dst, v); @@ -1355,6 +1373,7 @@ bgppath h_src; bt_assert(callme(4, 4) = 16); bt_assert(callme(7, 2) = 14); bt_assert(callmeagain(1, 2, 3) = 6); + local_vars(5); bt_assert(factorial(5) = 120); bt_assert(factorial(10) = 3628800); From cb339a30677901f2c248de08ff535cf0a9efab3d Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Mon, 14 Mar 2022 20:36:20 +0100 Subject: [PATCH 14/14] Filter: Implement for loops For loops allow to iterate over elements in compound data like BGP paths or community lists. The syntax is: for [ ] in do --- doc/bird.sgml | 26 ++++++++++---- filter/config.Y | 21 ++++++++++- filter/data.c | 1 + filter/f-inst.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ filter/test.conf | 40 ++++++++++++++++++++- nest/a-path.c | 29 +++++++++++++++ nest/a-set.c | 48 +++++++++++++++++++++++++ nest/attrs.h | 4 +++ 8 files changed, 254 insertions(+), 8 deletions(-) diff --git a/doc/bird.sgml b/doc/bird.sgml index f933128c..89b1541c 100644 --- a/doc/bird.sgml +++ b/doc/bird.sgml @@ -1683,7 +1683,8 @@ prefix and an ASN as arguments. Control structures

Filters support two control structures: conditions and case switches. +

Filters support several control structures: conditions, for loops and case +switches.

Syntax of a condition is: if boolean expression then and you can use { and you can use { boolean expression is true, For loops allow to iterate over elements in compound data like BGP paths or +community lists. The syntax is: for [ and you can also use compound command like in conditions. +The expression is evaluated to a compound data, then for each element from such +data the command is executed with the item assigned to the variable. A variable +may be an existing one (when just name is used) or a locally defined (when type +and name is used). In both cases, it must have the same type as elements. +

The case is similar to case from Pascal. Syntax is case . The expression after case can be of any type which can be @@ -1703,16 +1712,21 @@ neither of the Here is example that uses +if 1234 = i then printn "."; else { + print "not 1234"; + print "You need {} around multiple commands"; +} + +for int asn in bgp_path do { + printn "ASN: ", asn; + if asn < 65536 then print " (2B)"; else print " (4B)"; +} + case arg1 { 2: print "two"; print "I can do more commands without {}"; 3 .. 5: print "three to five"; else: print "something else"; } - -if 1234 = i then printn "."; else { - print "not 1234"; - print "You need {} around multiple commands"; -} diff --git a/filter/config.Y b/filter/config.Y index f8f47862..c7c82068 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -296,7 +296,7 @@ assert_assign(struct f_lval *lval, struct f_inst *expr, const char *start, const checker = f_new_inst(FI_EQ, expr, getter); setter->next = checker; - + return assert_done(setter, start, end); } @@ -307,6 +307,7 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN, INT, BOOL, IP, TYPE, PREFIX, RD, PAIR, QUAD, EC, LC, SET, STRING, BGPMASK, BGPPATH, CLIST, ECLIST, LCLIST, IF, THEN, ELSE, CASE, + FOR, IN, DO, TRUE, FALSE, RT, RO, UNKNOWN, GENERIC, FROM, GW, NET, MASK, PROTO, SOURCE, SCOPE, DEST, IFNAME, IFINDEX, WEIGHT, GW_MPLS, PREFERENCE, @@ -342,6 +343,7 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN, %type set_atom switch_atom fipa %type fprefix %type get_cf_position +%type for_var CF_GRAMMAR @@ -899,6 +901,11 @@ var: $$ = f_new_inst(FI_VAR_INIT, $3, sym); } +for_var: + type symbol { $$ = cf_define_symbol($2, SYM_VARIABLE | $1, offset, f_new_var(sym_->scope)); } + | CF_SYM_KNOWN { $$ = $1; cf_assert_symbol($1, SYM_VARIABLE); } + ; + cmd: '{' cmds_scoped '}' { $$ = $2; @@ -909,6 +916,18 @@ cmd: | IF term THEN cmd ELSE cmd { $$ = f_new_inst(FI_CONDITION, $2, $4, $6); } + | FOR { + /* Reserve space for walk data on stack */ + cf_push_scope(NULL); + conf_this_scope->slots += 2; + } for_var IN + /* Parse term in the parent scope */ + { conf_this_scope->active = 0; } term { conf_this_scope->active = 1; } + DO cmd { + cf_pop_scope(); + $$ = f_new_inst(FI_FOR_INIT, $6, $3); + $$->next = f_new_inst(FI_FOR_NEXT, $3, $9); + } | CF_SYM_KNOWN '=' term ';' { switch ($1->class) { case SYM_VARIABLE_RANGE: diff --git a/filter/data.c b/filter/data.c index 2bb5920c..87c438fc 100644 --- a/filter/data.c +++ b/filter/data.c @@ -72,6 +72,7 @@ enum f_type f_type_element_type(enum f_type t) { switch(t) { + case T_PATH: return T_INT; case T_CLIST: return T_PAIR; case T_ECLIST: return T_EC; case T_LCLIST: return T_LC; diff --git a/filter/f-inst.c b/filter/f-inst.c index dc243b4e..30db96f2 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -87,6 +87,11 @@ * m4_dnl RESULT_VOID; return undef * m4_dnl } * + * Note that runtime arguments m4_dnl (ARG*, VARARG) must be defined before + * parse-time arguments m4_dnl (LINE, SYMBOL, ...). During linearization, + * first ones move position in f_line by linearizing arguments first, while + * second ones store data to the current position. + * * Also note that the { ... } blocks are not respected by M4 at all. * If you get weird unmatched-brace-pair errors, check what it generated and why. * What is really considered as one instruction is not the { ... } block @@ -543,6 +548,94 @@ RESULT_VAL(val); } + INST(FI_FOR_INIT, 1, 0) { + NEVER_CONSTANT; + ARG_ANY(1); + SYMBOL; + + FID_NEW_BODY() + ASSERT((sym->class & ~0xff) == SYM_VARIABLE); + + /* Static type check */ + if (f1->type) + { + enum f_type t_var = (sym->class & 0xff); + enum f_type t_arg = f_type_element_type(f1->type); + if (!t_arg) + cf_error("Value of expression in FOR must be iterable, got %s", + f_type_name(f1->type)); + if (t_var != t_arg) + cf_error("Loop variable '%s' in FOR must be %s, is %s", + sym->name, f_type_name(t_arg), f_type_name(t_var)); + } + + FID_INTERPRET_BODY() + + /* Dynamic type check */ + if ((sym->class & 0xff) != f_type_element_type(v1.type)) + runtime("Mismatched argument and variable type"); + + /* Setup the index */ + v2 = (struct f_val) { .type = T_INT, .val.i = 0 }; + + /* Keep v1 and v2 on the stack */ + fstk->vcnt += 2; + } + + INST(FI_FOR_NEXT, 2, 0) { + NEVER_CONSTANT; + SYMBOL; + + /* Type checks are done in FI_FOR_INIT */ + + /* Loop variable */ + struct f_val *var = &fstk->vstk[curline.vbase + sym->offset]; + int step = 0; + + switch(v1.type) + { + case T_PATH: + var->type = T_INT; + step = as_path_walk(v1.val.ad, &v2.val.i, &var->val.i); + break; + + case T_CLIST: + var->type = T_PAIR; + step = int_set_walk(v1.val.ad, &v2.val.i, &var->val.i); + break; + + case T_ECLIST: + var->type = T_EC; + step = ec_set_walk(v1.val.ad, &v2.val.i, &var->val.ec); + break; + + case T_LCLIST: + var->type = T_LC; + step = lc_set_walk(v1.val.ad, &v2.val.i, &var->val.lc); + break; + + default: + runtime( "Clist or lclist expected" ); + } + + if (step) + { + /* Keep v1 and v2 on the stack */ + fstk->vcnt += 2; + + /* Repeat this instruction */ + curline.pos--; + + /* Execute the loop body */ + LINE(1, 0); + + /* Space for loop variable, may be unused */ + fstk->vcnt += 1; + } + else + var->type = T_VOID; + } + INST(FI_CONDITION, 1, 0) { ARG(1, T_BOOL); if (v1.val.i) diff --git a/filter/test.conf b/filter/test.conf index 436031a3..bc5f898e 100644 --- a/filter/test.conf +++ b/filter/test.conf @@ -758,6 +758,15 @@ int set set12; bt_assert(delete(p2, [4..5]) = prepend(prepend(prepend(prepend(+empty+, 3), 3), 2), 1)); bt_assert(format([= 1 2+ 3 =]) = "[= 1 2 + 3 =]"); + + # iteration over path + int x = 0; + int y = 0; + for int i in p2 do { + x = x + i; + y = y + x; + } + bt_assert(x = 18 && y = 50); } bt_test_suite(t_path, "Testing paths"); @@ -884,6 +893,12 @@ clist r; bt_assert(format(r) = "(clist (2,1) (1,3) (2,2) (3,1) (2,3))"); bt_assert(r.min = (1,3)); bt_assert(r.max = (3,1)); + + # iteration over clist + int x = 0; + for pair c in r do + x = x + c.asn * c.asn * c.data; + bt_assert(x = 36); } bt_test_suite(t_clist, "Testing lists of communities"); @@ -999,6 +1014,13 @@ eclist r; bt_assert(format(r) = "(eclist (rt, 2, 1) (rt, 1, 3) (rt, 2, 2) (rt, 3, 1) (rt, 2, 3))"); bt_assert(r.min = (rt, 1, 3)); bt_assert(r.max = (rt, 3, 1)); + + # iteration over eclist + int x = 0; + for ec c in r do + if c > (rt, 2, 0) && c < (rt, 3, 0) then + x = x + 1; + bt_assert(x = 3); } bt_test_suite(t_eclist, "Testing lists of extended communities"); @@ -1117,6 +1139,19 @@ lclist r; bt_assert(format(r) = "(lclist (2, 3, 3) (1, 2, 3) (2, 3, 1) (3, 1, 2) (2, 1, 3))"); bt_assert(r.min = (1, 2, 3)); bt_assert(r.max = (3, 1, 2)); + + # iteration over lclist + int x = 0; + int y = 0; + lc mx = (0, 0, 0); + for lc c in r do { + int asn2 = c.asn * c.asn; + x = x + asn2 * c.data1; + y = y + asn2 * c.data2; + if c > mx then mx = c; + } + bt_assert(x = 39 && y = 49); + bt_assert(mx = r.max); } bt_test_suite(t_lclist, "Testing lists of large communities"); @@ -1580,13 +1615,16 @@ filter vpn_filter bt_assert(net.type != NET_IP6); bt_assert(net.rd = 0:1:2); + bool b = false; case (net.type) { NET_IP4: print "IPV4"; NET_IP6: print "IPV6"; + else: b = true; } + bt_assert(b); bt_check_assign(from, 10.20.30.40); - bt_check_assign(gw, 55.55.55.44); + # bt_check_assign(gw, 55.55.55.44); bgp_community.add((3,5)); bgp_ext_community.add((ro, 135, 999)); diff --git a/nest/a-path.c b/nest/a-path.c index d5b01635..6bb18285 100644 --- a/nest/a-path.c +++ b/nest/a-path.c @@ -669,6 +669,35 @@ as_path_filter(struct linpool *pool, const struct adata *path, const struct f_va return res; } +int +as_path_walk(const struct adata *path, uint *pos, uint *val) +{ + if (!path) + return 0; + + const u8 *p = path->data; + const u8 *q = p + path->length; + uint n, x = *pos; + + while (p < q) + { + n = p[1]; + p += 2; + + if (x < n) + { + *val = get_as(p + x * BS); + *pos += 1; + return 1; + } + + p += n * BS; + x -= n; + } + + return 0; +} + struct pm_pos { diff --git a/nest/a-set.c b/nest/a-set.c index 71fbac94..40ed573c 100644 --- a/nest/a-set.c +++ b/nest/a-set.c @@ -693,3 +693,51 @@ lc_set_max(const struct adata *list, lcomm *val) *val = (lcomm) { res[0], res[1], res[2] }; return 1; } + +int +int_set_walk(const struct adata *list, uint *pos, uint *val) +{ + if (!list) + return 0; + + if (*pos >= (uint) int_set_get_size(list)) + return 0; + + u32 *res = int_set_get_data(list) + *pos; + *val = *res; + *pos += 1; + + return 1; +} + +int +ec_set_walk(const struct adata *list, uint *pos, u64 *val) +{ + if (!list) + return 0; + + if (*pos >= (uint) int_set_get_size(list)) + return 0; + + u32 *res = int_set_get_data(list) + *pos; + *val = ec_generic(res[0], res[1]); + *pos += 2; + + return 1; +} + +int +lc_set_walk(const struct adata *list, uint *pos, lcomm *val) +{ + if (!list) + return 0; + + if (*pos >= (uint) int_set_get_size(list)) + return 0; + + u32 *res = int_set_get_data(list) + *pos; + *val = (lcomm) { res[0], res[1], res[2] }; + *pos += 3; + + return 1; +} diff --git a/nest/attrs.h b/nest/attrs.h index 22e2ff4a..9412439b 100644 --- a/nest/attrs.h +++ b/nest/attrs.h @@ -51,6 +51,7 @@ u32 as_path_get_last_nonaggregated(const struct adata *path); int as_path_contains(const struct adata *path, u32 as, int min); int as_path_match_set(const struct adata *path, const struct f_tree *set); const struct adata *as_path_filter(struct linpool *pool, const struct adata *path, const struct f_val *set, int pos); +int as_path_walk(const struct adata *path, uint *pos, uint *val); static inline struct adata *as_path_prepend(struct linpool *pool, const struct adata *path, u32 as) { return as_path_prepend2(pool, path, AS_PATH_SEQUENCE, as); } @@ -225,6 +226,9 @@ int lc_set_min(const struct adata *list, lcomm *val); int int_set_max(const struct adata *list, u32 *val); int ec_set_max(const struct adata *list, u64 *val); int lc_set_max(const struct adata *list, lcomm *val); +int int_set_walk(const struct adata *list, uint *pos, u32 *val); +int ec_set_walk(const struct adata *list, uint *pos, u64 *val); +int lc_set_walk(const struct adata *list, uint *pos, lcomm *val); void ec_set_sort_x(struct adata *set); /* Sort in place */