From 84f4783c47a0fcb74b7f46d3ff96fe8a3ad1b278 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Sat, 6 Nov 2021 15:54:52 +1100 Subject: [PATCH] oceani: free variables as soon as they go out of scope. Each 'exec' now keeps track of the variables that go out-of-scope when the exec completes. CondScope variables need to be re-linked when they get merged. We now poison a variable when it is freed to ensure it doesn't get used again by mistake. The final cleanup now only needs to handle global variables Signed-off-by: NeilBrown --- csrc/oceani-tests.mdc | 2 +- csrc/oceani.mdc | 125 +++++++++++++++++++++++++++++++----------- 2 files changed, 93 insertions(+), 34 deletions(-) diff --git a/csrc/oceani-tests.mdc b/csrc/oceani-tests.mdc index 776c1a6..dccfb2a 100644 --- a/csrc/oceani-tests.mdc +++ b/csrc/oceani-tests.mdc @@ -80,7 +80,7 @@ arguments separated from the name by commas. For each test, there is a section @mv *.gcov coverage ; [ -f .gcov ] && mv .gcov coverage || true @ awk '/NOTEST/ { next } /^ *[1-9]/ {ran+=1} /^ *###/ {skip+=1} \ END {printf "coverage: %6.2f%%\n", ran * 100 / (ran + skip); \ - if (ran < (ran + skip) *0.958) exit(1) }' \ + if (ran < (ran + skip) *0.959) exit(1) }' \ coverage/oceani.mdc.gcov @rm -f .tmp* diff --git a/csrc/oceani.mdc b/csrc/oceani.mdc index 215a9a7..821775c 100644 --- a/csrc/oceani.mdc +++ b/csrc/oceani.mdc @@ -263,7 +263,7 @@ structures can be used. free(s); s = t; } - ## free context vars + ## free global vars ## free context types ## free context storage exit(context.parse_error ? 1 : 0); @@ -498,8 +498,10 @@ Named type are stored in a simple linked list. Objects of each type are static void free_value(struct type *type, struct value *v) { - if (type && v) + if (type && v) { type->free(type, v); + memset(v, 0x5a, type->size); + } } static void type_print(struct type *type, FILE *f) @@ -846,6 +848,42 @@ cannot nest, so a declaration while a name is in-scope is an error. ## variable fields }; +When a scope closes, the values of the variables might need to be freed. +This happens in the context of some `struct exec` and each `exec` will +need to know which variables need to be freed when it completes. + +####### exec fields + struct variable *to_free; + +####### variable fields + struct exec *cleanup_exec; + struct variable *next_free; + +####### interp exec cleanup + { + struct variable *v; + for (v = e->to_free; v; v = v->next_free) { + struct value *val = var_value(c, v); + free_value(v->type, val); + } + } + +###### ast functions + static void variable_unlink_exec(struct variable *v) + { + struct variable **vp; + if (!v->cleanup_exec) + return; + for (vp = &v->cleanup_exec->to_free; + *vp; vp = &(*vp)->next_free) { + if (*vp != v) + continue; + *vp = v->next_free; + v->cleanup_exec = NULL; + break; + } + } + While the naming seems strange, we include local constants in the definition of variables. A name declared `var := value` can subsequently be changed, but a name declared `var ::= value` cannot - @@ -1010,13 +1048,14 @@ need to be freed. For this we need to be able to find it, so assume that v->merged == secondary->merged) { v->scope = OutScope; v->merged = primary; + variable_unlink_exec(v); } } ###### forward decls static struct value *var_value(struct parse_context *c, struct variable *v); -###### free context vars +###### free global vars while (context.varlist) { struct binding *b = context.varlist; @@ -1027,10 +1066,11 @@ need to be freed. For this we need to be able to find it, so assume that struct variable *t = v; v = t->previous; - free_value(t->type, var_value(&context, t)); - if (t->depth == 0) - // This is a global constant - free_exec(t->where_decl); + if (t->global) { + free_value(t->type, var_value(&context, t)); + if (t->depth == 0) + free_exec(t->where_decl); + } free(t); } } @@ -1065,7 +1105,7 @@ switch. Other scopes are "sequential". When exiting a parallel scope we check if there are any variables that were previously pending and are still visible. If there are, then -there weren't redeclared in the most recent scope, so they cannot be +they weren't redeclared in the most recent scope, so they cannot be merged and must become out-of-scope. If it is not the first of parallel scopes (based on `child_count`), we check that there was a previous binding that is still pending-scope. If there isn't, the new @@ -1139,7 +1179,8 @@ all pending-scope variables become conditionally scoped. return v; } - static void var_block_close(struct parse_context *c, enum closetype ct) + static void var_block_close(struct parse_context *c, enum closetype ct, + struct exec *e) { /* Close off all variables that are in_scope. * Some variables in c->scope may already be not-in-scope, @@ -1163,6 +1204,14 @@ all pending-scope variables become conditionally scoped. * closed the scope. */ continue; + v->min_depth = c->scope_depth; + if (v->scope == InScope) { + /* This variable gets cleaned up when 'e' finishes */ + variable_unlink_exec(v); + v->cleanup_exec = e; + v->next_free = e->to_free; + e->to_free = v; + } switch (ct) { case CloseElse: case CloseParallel: /* handle PendingScope */ @@ -1195,7 +1244,7 @@ all pending-scope variables become conditionally scoped. * parallel scope to be nested immediately * in a parallel scope, and that never * happens. - */ + */ // NOTEST case OutScope: /* Not possible as we already tested for * OutScope @@ -1219,9 +1268,9 @@ all pending-scope variables become conditionally scoped. for (v2 = v; v2 && v2->scope == PendingScope; v2 = v2->previous) - if (v2->type == Tlabel) { + if (v2->type == Tlabel) v2->scope = CondScope; - } else + else v2->scope = OutScope; break; case CondScope: @@ -1261,7 +1310,7 @@ is started, so there is no need to allocate until the size is known. { if (!v->global) { if (!c->local || !v->type) - return NULL; + return NULL; // NOTEST if (v->frame_pos + v->type->size > c->local_size) { printf("INVALID frame_pos\n"); // NOTEST exit(2); // NOTEST @@ -1377,6 +1426,7 @@ from the `exec_types` enum. struct exec { enum exec_types type; int line, column; + ## exec fields }; struct binode { struct exec; @@ -1480,6 +1530,15 @@ also want to know what sort of bracketing to use. print_binode(cast(binode, e), indent, bracket); break; ## print exec cases } + if (e->to_free) { + struct variable *v; + do_indent(indent, "/* FREE"); + for (v = e->to_free; v; v = v->next_free) + printf(" %.*s(%c%d+%d)", v->name->name.len, v->name->name.txt, + v->global ? 'G':'L', + v->frame_pos, v->type ? v->type->size:0); + printf(" */\n"); + } } ###### forward decls @@ -1625,6 +1684,7 @@ in `rval`. ret.lval = lrv; ret.rval = rv; ret.type = rvtype; + ## interp exec cleanup return ret; } @@ -2296,7 +2356,7 @@ or as an indented list of one parameter per line $0->op = Func; $0->left = reorder_bilist($right = $scope_stack && !c->parse_error) abort(); }$ | func main IN OpenScope OptNL Args OUT OptNL do Block Newlines ${ @@ -2304,7 +2364,7 @@ or as an indented list of one parameter per line $0->op = Func; $0->left = reorder_bilist($right = $scope_stack && !c->parse_error) abort(); }$ | func main NEWLINE OpenScope OptNL do Block Newlines ${ @@ -2312,7 +2372,7 @@ or as an indented list of one parameter per line $0->op = Func; $0->left = NULL; $0->right = $scope_stack && !c->parse_error) abort(); }$ @@ -3665,7 +3725,6 @@ it is declared, and error will be raised as the name is created as struct value *val; v = v->merged; val = var_value(c, v); - free_value(v->type, val); if (v->type->prepare_type) v->type->prepare_type(c, v->type, 0); if (b->right) { @@ -3872,13 +3931,13 @@ casepart` to track a list of case parts. $0->forpart = $thenpart = $looppart = $forpart = $looppart = $condpart = $next = $0->casepart; $0->casepart = $condpart = $next = $0->casepart; $0->casepart = $condpart = $IP.condpart; $IP.condpart = NULL; $0->thenpart = $IP.thenpart; $IP.thenpart = NULL; // This is where we close an "if" statement - var_block_close(c, CloseSequential); + var_block_close(c, CloseSequential, $0); }$ CondSuffix -> IfSuffix ${ @@ -3927,12 +3986,12 @@ casepart` to track a list of case parts. ElsePart -> else OpenBlock Newlines ${ $0 = new(cond_statement); $0->elsepart = $elsepart); }$ | else OpenScope CondStatement ${ $0 = new(cond_statement); $0->elsepart = $elsepart); }$ $*casepart @@ -3940,7 +3999,7 @@ casepart` to track a list of case parts. $0 = calloc(1,sizeof(struct casepart)); $0->value = $action = $action); }$ $*exec @@ -3951,7 +4010,7 @@ casepart` to track a list of case parts. ThenPart -> then OpenBlock ${ $0 = $op = Loop; $0->left = $right = $right); + var_block_close(c, CloseSequential, $0); }$ | while OpenScope Expression OpenScope ColonBlock ${ $0 = new(binode); $0->op = Loop; $0->left = $right = $right); + var_block_close(c, CloseSequential, $0); }$ $cond_statement IfPart -> if UseBlock OptNL then OpenBlock ${ $0.condpart = $