From 96d757c13fe4b2e21b265275af9ac7d1e2c3f075 Mon Sep 17 00:00:00 2001 From: Jan Maria Matejka Date: Tue, 21 May 2019 16:33:37 +0000 Subject: [PATCH] Filter: Store variables and function arguments on stack --- conf/conf.h | 3 +- conf/confbase.Y | 2 +- filter/config.Y | 140 ++++++++++++++++++------------------------- filter/data.h | 4 +- filter/decl.m4 | 3 +- filter/f-inst.c | 93 ++++++++++------------------ filter/f-inst.h | 3 +- filter/f-util.c | 22 ------- filter/filter.c | 44 ++++++++------ filter/filter_test.c | 3 +- filter/test.conf | 3 +- 11 files changed, 125 insertions(+), 195 deletions(-) diff --git a/conf/conf.h b/conf/conf.h index f14c0e21..d88d9a44 100644 --- a/conf/conf.h +++ b/conf/conf.h @@ -116,7 +116,8 @@ struct symbol { const struct filter *filter; /* For SYM_FILTER */ struct rtable_config *table; /* For SYM_TABLE */ struct f_dynamic_attr *attribute; /* For SYM_ATTRIBUTE */ - struct f_val *val; /* For SYM_CONSTANT or SYM_VARIABLE */ + struct f_val *val; /* For SYM_CONSTANT */ + uint offset; /* For SYM_VARIABLE */ }; char name[0]; diff --git a/conf/confbase.Y b/conf/confbase.Y index e104e54f..9978aec8 100644 --- a/conf/confbase.Y +++ b/conf/confbase.Y @@ -70,7 +70,7 @@ CF_DECLS struct f_dynamic_attr fda; struct f_static_attr fsa; struct f_lval flv; - const struct f_line *fl; + struct f_line *fl; const struct filter *f; struct f_tree *e; struct f_trie *trie; diff --git a/filter/config.Y b/filter/config.Y index 3eccc3ed..5f9b8356 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -15,6 +15,8 @@ CF_HDR CF_DEFINES +static uint decls_count; + static inline u32 pair(u32 a, u32 b) { return (a << 16) | b; } static inline u32 pair_a(u32 p) { return p >> 16; } static inline u32 pair_b(u32 p) { return p & 0xFFFF; } @@ -398,8 +400,8 @@ assert_assign(struct f_lval *lval, struct f_inst *expr, const char *start, const struct f_inst *setter, *getter, *checker; switch (lval->type) { case F_LVAL_VARIABLE: - setter = f_new_inst(FI_SET, expr, lval->sym); - getter = f_new_inst(FI_VARIABLE, lval->sym); + setter = f_new_inst(FI_VAR_SET, expr, lval->sym); + getter = f_new_inst(FI_VAR_GET, lval->sym); break; case F_LVAL_PREFERENCE: setter = f_new_inst(FI_PREF_SET, expr); @@ -446,14 +448,14 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN, %nonassoc THEN %nonassoc ELSE -%type cmds_int function_body declsn function_params -%type term block cmd cmds constant constructor print_one print_list var_list var_listn function_call symbol_value bgp_path_expr bgp_path bgp_path_tail one_decl decls +%type cmds_int +%type term block cmd cmds constant constructor print_one 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 -%type filter_body +%type filter_body function_body %type lvalue -%type type +%type type function_params %type ec_kind %type break_command %type cnum @@ -553,43 +555,24 @@ type: } ; -one_decl: - type CF_SYM_VOID { - struct f_val * val = cfg_alloc(sizeof(struct f_val)); - val->type = T_VOID; - $2 = cf_define_symbol($2, SYM_VARIABLE | $1, val, val); - DBG( "New variable %s type %x\n", $2->name, $1 ); - $$ = f_new_inst(FI_SET, NULL, $2); - } - ; - -/* Decls with ';' at the end. Beware; these are reversed. */ -decls: /* EMPTY */ { $$ = NULL; } - | one_decl ';' decls { - $$ = $1; - $$->next = $3; - } +/* Declarations with ';' at the end */ +decls: + /* EMPTY */ + | declsn ';' ; /* Declarations that have no ';' at the end. */ -declsn: one_decl { $$[0] = $$[1] = $1; } - | one_decl ';' declsn { - $3[1]->next = $1; - $$[1] = $3[1] = $1; - $$[0] = $3[0]; +declsn: + type CF_SYM_VOID { + cf_define_symbol($2, SYM_VARIABLE | $1, offset, decls_count++); + } + | declsn ';' type CF_SYM_VOID { + if (decls_count >= 0xff) cf_error("Too many declarations, at most 255 allowed"); + cf_define_symbol($4, SYM_VARIABLE | $3, offset, decls_count++); } ; -filter_body: - function_body { - if ($1[0]) { - const struct f_inst *inst[2] = { $1[0], $1[1] }; - $$ = f_postfixify_concat(inst, 2); - } - else - $$ = f_postfixify($1[1]); - } - ; +filter_body: { decls_count = 0; } function_body { $$ = $2; } ; filter: CF_SYM_KNOWN { @@ -611,14 +594,14 @@ where_filter: ; function_params: - '(' declsn ')' { $$[0] = $2[0]; $$[1] = $2[1]; } - | '(' ')' { $$[0] = $$[1] = NULL; } + '(' declsn ')' { $$ = decls_count; } + | '(' ')' { $$ = 0; } ; function_body: decls '{' cmds '}' { - $$[0] = $1 ? f_clear_local_vars($1) : NULL; - $$[1] = $3; + $$ = f_postfixify($3); + $$->vars = decls_count; } ; @@ -627,33 +610,11 @@ function_def: FUNCTION CF_SYM_VOID { DBG( "Beginning of function %s\n", $2->name ); $2 = cf_define_symbol($2, SYM_FUNCTION, function, NULL); cf_push_scope($2); + decls_count = 0; } function_params function_body { - const struct f_inst *catlist[4]; - uint count = 0; - - /* Argument setters */ - if ($4[0]) - catlist[count++] = $4[0]; - - /* Local var clearers */ - if ($5[0]) - catlist[count++] = $5[0]; - - /* Return void if no return is needed */ - catlist[count++] = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_VOID }); - - /* Function body itself */ - if ($5[1]) - catlist[count++] = $5[1]; - - struct f_line *fl = f_postfixify_concat(catlist, count); - - fl->args = 0; - for (const struct f_inst *arg = $4[0]; arg; arg = arg->next) - fl->args++; - - $2->function = fl; - + $5->vars -= $4; + $5->args = $4; + $2->function = $5; cf_pop_scope(); } ; @@ -862,9 +823,33 @@ constructor: ; +/* This generates the function_call variable list backwards. */ +var_list: /* EMPTY */ { $$ = NULL; } + | term { $$ = $1; } + | var_list ',' term { $$ = $3; $$->next = $1; } + function_call: CF_SYM_KNOWN '(' var_list ')' { - $$ = f_new_inst(FI_CALL, $1, $3); + 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; + while ($3) { + args++; + struct f_inst *tmp = $3->next; + $3->next = fc; + + fc = $3; + $3 = 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; } ; @@ -872,8 +857,10 @@ symbol_value: CF_SYM_KNOWN { switch ($1->class) { case SYM_CONSTANT_RANGE: + $$ = f_new_inst(FI_CONSTANT_DEFINED, $1); + break; case SYM_VARIABLE_RANGE: - $$ = f_new_inst(FI_VARIABLE, $1); + $$ = f_new_inst(FI_VAR_GET, $1); break; case SYM_ATTRIBUTE: $$ = f_new_inst(FI_EA_GET, *$1->attribute); @@ -988,19 +975,6 @@ print_list: /* EMPTY */ { $$ = NULL; } } ; -var_listn: term { - $$ = $1; - } - | term ',' var_listn { - $$ = $1; - $$->next = $3; - } - ; - -var_list: /* EMPTY */ { $$ = NULL; } - | var_listn { $$ = $1; } - ; - cmd: IF term THEN block { $$ = f_new_inst(FI_CONDITION, $2, $4, NULL); @@ -1011,7 +985,7 @@ cmd: | CF_SYM_KNOWN '=' term ';' { switch ($1->class) { case SYM_VARIABLE_RANGE: - $$ = f_new_inst(FI_SET, $3, $1); + $$ = f_new_inst(FI_VAR_SET, $3, $1); break; case SYM_ATTRIBUTE: $$ = f_new_inst(FI_EA_SET, $3, *$1->attribute); diff --git a/filter/data.h b/filter/data.h index 6ef2024f..6973008f 100644 --- a/filter/data.h +++ b/filter/data.h @@ -17,8 +17,8 @@ /* Internal types */ enum f_type { -/* Do not use type of zero, that way we'll see errors easier. */ - T_VOID = 1, +/* Nothing. Simply nothing. */ + T_VOID = 0, /* User visible types, which fit in int */ T_INT = 0x10, diff --git a/filter/decl.m4 b/filter/decl.m4 index bdd59f20..c9b5c8c5 100644 --- a/filter/decl.m4 +++ b/filter/decl.m4 @@ -209,6 +209,7 @@ do { estk.item[estk.cnt].pos = 0; estk.item[estk.cnt].line = $1; estk.item[estk.cnt].ventry = vstk.cnt; + estk.item[estk.cnt].vbase = estk.item[estk.cnt-1].vbase; estk.item[estk.cnt].emask = 0; estk.cnt++; } while (0)m4_dnl @@ -377,7 +378,7 @@ FID_WR_PUT(4) /* Filter instruction structure for config */ struct f_inst { - const struct f_inst *next; /* Next instruction */ + struct f_inst *next; /* Next instruction */ enum f_instruction_code fi_code; /* Instruction code */ int size; /* How many instructions are underneath */ int lineno; /* Line number */ diff --git a/filter/f-inst.c b/filter/f-inst.c index d6c292b6..4a4f6016 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -261,23 +261,28 @@ } /* Set to indirect value prepared in v1 */ - INST(FI_SET, 1, 0) { + INST(FI_VAR_SET, 1, 0) { ARG_ANY(2); SYMBOL(1); if ((sym->class != (SYM_VARIABLE | v1.type)) && (v1.type != T_VOID)) { /* IP->Quad implicit conversion */ if ((sym->class == (SYM_VARIABLE | T_QUAD)) && val_is_ip4(&v1)) - { - *(sym->val) = (struct f_val) { + v1 = (struct f_val) { .type = T_QUAD, .val.i = ipa_to_u32(v1.val.ip), - }; - break; - } - runtime( "Assigning to variable of incompatible type" ); + }; + else + runtime( "Assigning to variable of incompatible type" ); } - *(sym->val) = v1; + + vstk.val[curline.vbase + sym->offset] = v1; + } + + INST(FI_VAR_GET, 0, 1) { + SYMBOL(1); + res = vstk.val[curline.vbase + sym->offset]; + RESULT_OK; } /* some constants have value in a[1], some in *a[0].p, strange. */ @@ -301,7 +306,7 @@ res = whati->val; RESULT_OK; } - INST(FI_VARIABLE, 0, 1) { + INST(FI_CONSTANT_DEFINED, 0, 1) { FID_STRUCT_IN const struct symbol *sym; FID_LINE_IN @@ -314,18 +319,9 @@ FID_POSTFIXIFY_BODY item->valp = (item->sym = what->sym)->val; FID_SAME_BODY - if (strcmp(f1->sym->name, f2->sym->name) || (f1->sym->class != f2->sym->class)) return 0; + if (strcmp(f1->sym->name, f2->sym->name) || !val_same(f1->sym->val, f2->sym->val)) return 0; FID_DUMP_BODY - switch (item->sym->class) { - case SYM_CONSTANT_RANGE: - debug("%sconstant %s with value %s\n", INDENT, item->sym->name, val_dump(item->valp)); - break; - case SYM_VARIABLE_RANGE: - debug("%svariable %s with current value %s\n", INDENT, item->sym->name, val_dump(item->valp)); - break; - default: - bug("Symbol %s of type %d doesn't reference a value", item->sym->name, item->sym->class); - } + debug("%sconstant %s with value %s\n", INDENT, item->sym->name, val_dump(item->valp)); FID_ALL res = *whati->valp; @@ -768,56 +764,31 @@ else runtime("Can't return non-bool from non-function"); - /* Set the value stack position */ - vstk.cnt = estk.item[estk.cnt].ventry; + /* Set the value stack position, overwriting the former implicit void */ + vstk.cnt = estk.item[estk.cnt].ventry - 1; /* Copy the return value */ RESULT_VAL(vstk.val[retpos]); } INST(FI_CALL, 0, 1) { - FID_LINE_IN - const struct f_line *args; - const struct f_line *body; - struct symbol *sym; - FID_STRUCT_IN - struct symbol *sym; - const struct f_inst *args; - FID_NEW_ARGS - , struct symbol * sym - , const struct f_inst *args - FID_NEW_BODY - if (sym->class != SYM_FUNCTION) - cf_error("You can't call something which is not a function. Really."); + SYMBOL; - uint count = 0; - for (const struct f_inst *inst = args; inst; inst = inst->next) - count++; - - if (count != sym->function->args) - cf_error("Function %s takes %u arguments, got %u.", sym->name, sym->function->args, count); - - what->sym = sym; - what->args = args; - FID_DUMP_BODY - debug("%scalling %s with following args\n", INDENT, item->sym->name); - f_dump_line(item->args, indent + 1); - FID_POSTFIXIFY_BODY - item->args = f_postfixify(what->args); - item->body = (item->sym = what->sym)->function; - FID_SAME_BODY - /* To be done better */ - if (strcmp(f1->sym->name, f2->sym->name)) return 0; - if (!f_same(f1->args, f2->args)) return 0; - if (!f_same(f1->body, f2->body)) return 0; - FID_ALL - - /* First push the body on stack */ - LINEX(whati->body); + /* Push the body on stack */ + LINEX(sym->function); 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; - /* Then push the arguments */ - LINEX(whati->args); + /* Storage for local variables */ + memset(&(vstk.val[vstk.cnt]), 0, sizeof(struct f_val) * sym->function->vars); + vstk.cnt += sym->function->vars; } INST(FI_DROP_RESULT, 1, 0) { diff --git a/filter/f-inst.h b/filter/f-inst.h index 1e2d63a2..21cec454 100644 --- a/filter/f-inst.h +++ b/filter/f-inst.h @@ -63,13 +63,12 @@ enum f_instruction_flags { /* Convert the instruction back to the enum name */ const char *f_instruction_name(enum f_instruction_code fi); -struct f_inst *f_clear_local_vars(struct f_inst *decls); - /* 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_line_item items[0]; /* The items themselves */ }; diff --git a/filter/f-util.c b/filter/f-util.c index 35944b2c..79201fba 100644 --- a/filter/f-util.c +++ b/filter/f-util.c @@ -30,11 +30,6 @@ filter_name(const struct filter *filter) return filter->sym->name; } -void f_inst_next(struct f_inst *first, const struct f_inst *append) -{ - first->next = append; -} - struct filter *f_new_where(const struct f_inst *where) { struct f_inst acc = { @@ -67,23 +62,6 @@ struct filter *f_new_where(const struct f_inst *where) return f; } -struct f_inst *f_clear_local_vars(struct f_inst *decls) -{ - /* Prepend instructions to clear local variables */ - struct f_inst *head = NULL; - - for (const struct f_inst *si = decls; si; si = si->next) { - struct f_inst *cur = f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_VOID }); - if (head) - f_inst_next(cur, head); - else - f_inst_next(cur, si); - head = cur; /* The first FI_CONSTANT put there */ - } - - return head; -} - #define CA_KEY(n) n->name, n->fda.type #define CA_NEXT(n) n->next #define CA_EQ(na,ta,nb,tb) (!strcmp(na,nb) && (ta == tb)) diff --git a/filter/filter.c b/filter/filter.c index 50d9414b..dbc2376b 100644 --- a/filter/filter.c +++ b/filter/filter.c @@ -135,6 +135,8 @@ static struct tbf rl_runtime_err = TBF_DEFAULT_LOG_LIMITS; static enum filter_return interpret(struct filter_state *fs, const struct f_line *line, struct f_val *val) { + /* No arguments allowed */ + ASSERT(line->args == 0); #define F_VAL_STACK_MAX 4096 /* Value stack for execution */ @@ -145,8 +147,12 @@ interpret(struct filter_state *fs, const struct f_line *line, struct f_val *val) /* The stack itself is intentionally kept as-is for performance reasons. * Do NOT rewrite this to initialization by struct literal. It's slow. - */ - vstk.cnt = 0; + * + * Reserving space for local variables. */ + + vstk.cnt = line->vars; + memset(vstk.val, 0, sizeof(struct f_val) * line->vars); + #define F_EXEC_STACK_MAX 4096 /* Exception bits */ @@ -160,6 +166,7 @@ interpret(struct filter_state *fs, const struct f_line *line, struct f_val *val) const struct f_line *line; /* The line that is being executed */ uint pos; /* Instruction index in the line */ uint ventry; /* Value stack depth on entry */ + uint vbase; /* Where to index variable positions from */ enum f_exception emask; /* Exception mask */ } item[F_EXEC_STACK_MAX]; uint cnt; /* Current stack size; 0 for empty */ @@ -181,7 +188,6 @@ interpret(struct filter_state *fs, const struct f_line *line, struct f_val *val) while (curline.pos < curline.line->len) { const struct f_line_item *what = &(curline.line->items[curline.pos++]); - switch (what->fi_code) { #define res vstk.val[vstk.cnt] #define v1 vstk.val[vstk.cnt] @@ -207,26 +213,28 @@ interpret(struct filter_state *fs, const struct f_line *line, struct f_val *val) #undef ACCESS_EATTRS } } + + /* End of current line. Drop local variables before exiting. */ + vstk.cnt -= curline.line->vars; + vstk.cnt -= curline.line->args; estk.cnt--; } - switch (vstk.cnt) { - case 0: - if (val) { - log_rl(&rl_runtime_err, L_ERR "filters: No value left on stack"); - return F_ERROR; - } - return F_NOP; - case 1: - if (val) { - *val = vstk.val[0]; - return F_NOP; - } - /* fallthrough */ - default: - log_rl(&rl_runtime_err, L_ERR "Too many items left on stack: %u", vstk.cnt); + if (vstk.cnt == 0) { + if (val) { + log_rl(&rl_runtime_err, L_ERR "filters: No value left on stack"); return F_ERROR; + } + return F_NOP; } + + if (val && (vstk.cnt == 1)) { + *val = vstk.val[0]; + return F_NOP; + } + + log_rl(&rl_runtime_err, L_ERR "Too many items left on stack: %u", vstk.cnt); + return F_ERROR; } diff --git a/filter/filter_test.c b/filter/filter_test.c index d0dd281a..3abe095b 100644 --- a/filter/filter_test.c +++ b/filter/filter_test.c @@ -56,8 +56,7 @@ run_function(const void *arg) } linpool *tmp = lp_new_default(&root_pool); - struct f_val res; - enum filter_return fret = f_eval(t->fn, tmp, &res); + enum filter_return fret = f_eval(t->fn, tmp, NULL); rfree(tmp); return (fret < F_REJECT); diff --git a/filter/test.conf b/filter/test.conf index ba25a34b..9abd76f3 100644 --- a/filter/test.conf +++ b/filter/test.conf @@ -7,8 +7,7 @@ router id 62.168.0.1; /* We have to setup any protocol */ -protocol static { ipv4; } - +protocol device { }