From 34dd47358ad1071028c925984a88fb448573a9c2 Mon Sep 17 00:00:00 2001 From: Peter Bex Date: Thu, 1 Oct 2015 15:57:56 +0200 Subject: [PATCH 1/2] Avoid allocating argvectors on the temporary stack. The temporary stack should be strictly reserved for setting aside live data just before performing a GC & longjump (which resets the stack). There are several problems with (ab)using the temporary stack in non-GC situations, which surfaced only after the re-use of argvectors was made more prevalent: 1) Anything allocated on the temp stack will be kept around during a GC, which may cause objects to unnecessarily stick around for another round of GC. 2) When the longjmp is performed, if the argvector is allocated in the temporary stack and then the temporary_stack pointer is reset, we may scribble over the argvector whenever something is put on the temp stack. With argvector re-use, this will more commonly trigger errors. Now the restart trampoline will move the saved data from the temporary stack to the C stack. This should probably slightly more cause garbage collection events, but it will also make it easier to eventually make a dynamically allocated temporary stack (see also #1098). --- runtime.c | 82 +++++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 46 insertions(+), 36 deletions(-) diff --git a/runtime.c b/runtime.c index a2da0db..6e4cd1a 100644 --- a/runtime.c +++ b/runtime.c @@ -1499,8 +1499,10 @@ C_word CHICKEN_run(void *toplevel) serious_signal_occurred = 0; if(!return_to_host) { - C_word *p = C_temporary_stack; + int argcount = C_temporary_stack_bottom - C_temporary_stack; + C_word *p = C_alloc(argcount); + C_memcpy(p, C_temporary_stack, argcount * sizeof(C_word)); C_temporary_stack = C_temporary_stack_bottom; ((C_proc)C_restart_trampoline)(C_restart_c, p); } @@ -3105,11 +3107,13 @@ C_regparm C_word C_fcall C_mutate_scratch_slot(C_word *slot, C_word val) void C_save_and_reclaim(void *trampoline, int n, C_word *av) { - if(C_temporary_stack != av) { /* used in apply */ - C_temporary_stack = C_temporary_stack_bottom - n; - C_memmove(C_temporary_stack, av, n * sizeof(C_word)); - } + assert(av > C_temporary_stack_bottom || av < C_temporary_stack_limit); + assert(C_temporary_stack == C_temporary_stack_bottom); + C_temporary_stack = C_temporary_stack_bottom - n; + assert(C_temporary_stack >= C_temporary_stack_limit); + + C_memmove(C_temporary_stack, av, n * sizeof(C_word)); C_reclaim(trampoline, n); } @@ -3238,6 +3242,7 @@ C_regparm void C_fcall C_reclaim(void *trampoline, C_word c) /* Clear the mutated slot stack: */ mutation_stack_top = mutation_stack_bottom; + assert(C_temporary_stack >= C_temporary_stack_limit); /* Mark live values: */ for(p = C_temporary_stack; p < C_temporary_stack_bottom; ++p) mark(p); @@ -7136,41 +7141,40 @@ void C_ccall C_apply(C_word c, C_word *av) /* closure = av[ 0 ] */ k = av[ 1 ], fn = av[ 2 ]; - int i, n = c - 3; - int m = n - 1; - C_word x, skip, *ptr; + int new_av_size, i, n = c - 3; + int non_list_args = n - 1; + C_word rest, *ptr, *new_av; if(c < 4) C_bad_min_argc(c, 4); if(C_immediatep(fn) || C_header_bits(fn) != C_CLOSURE_TYPE) barf(C_NOT_A_CLOSURE_ERROR, "apply", fn); - ptr = C_temporary_stack_limit; - *(ptr++) = fn; - *(ptr++) = k; + rest = av[ c - 1 ]; + if(rest != C_SCHEME_END_OF_LIST && (C_immediatep(rest) || C_block_header(rest) != C_PAIR_TAG)) + barf(C_BAD_ARGUMENT_TYPE_ERROR, "apply", rest); - if(n > 1) { - C_memmove(ptr, av + 3, m * sizeof(C_word)); - ptr += m; - } + new_av_size = 2 + non_list_args + C_unfix(C_u_i_length(rest)); - x = av[ c - 1 ]; + if (!C_demand(new_av_size)) + C_save_and_reclaim((void *)C_apply, c, av); - if(x != C_SCHEME_END_OF_LIST && (C_immediatep(x) || C_block_header(x) != C_PAIR_TAG)) - barf(C_BAD_ARGUMENT_TYPE_ERROR, "apply", x); + new_av = ptr = C_alloc(new_av_size); + *(ptr++) = fn; + *(ptr++) = k; - for(skip = x; !C_immediatep(skip) && C_block_header(skip) == C_PAIR_TAG; skip = C_u_i_cdr(skip)) { - x = C_u_i_car(skip); - - if(ptr >= C_temporary_stack_bottom) - barf(C_TOO_MANY_PARAMETERS_ERROR, "apply"); + if(non_list_args > 0) { + C_memcpy(ptr, av + 3, non_list_args * sizeof(C_word)); + ptr += non_list_args; + } - *(ptr++) = x; - ++m; + while(!C_immediatep(rest) && C_block_header(rest) == C_PAIR_TAG) { + *(ptr++) = C_u_i_car(rest); + rest = C_u_i_cdr(rest); } + assert((ptr - new_av) == new_av_size); - C_temporary_stack = C_temporary_stack_bottom; - ((C_proc)(void *)C_block_item(fn, 0))(m + 2, C_temporary_stack_limit); + ((C_proc)(void *)C_block_item(fn, 0))(new_av_size, new_av); } @@ -7287,22 +7291,27 @@ void C_ccall C_apply_values(C_word c, C_word *av) lst = av[ 2 ]; + if(lst != C_SCHEME_END_OF_LIST && (C_immediatep(lst) || C_block_header(lst) != C_PAIR_TAG)) + barf(C_BAD_ARGUMENT_TYPE_ERROR, "apply", lst); + /* Check continuation wether it receives multiple values: */ if(C_block_item(k, 0) == (C_word)values_continuation) { - C_word - *av2, - *ptr = C_temporary_stack_limit; + C_word *av2, *ptr; + + n = C_unfix(C_u_i_length(lst)) + 1; + + if (!C_demand(n)) + C_save_and_reclaim((void *)C_apply_values, c, av); - for(n = 0; !C_immediatep(lst) && C_block_header(lst) == C_PAIR_TAG; ++n) { + av2 = C_alloc(n + 1); + av2[ 0 ] = k; + ptr = av2 + 1; + while(!C_immediatep(lst) && C_block_header(lst) == C_PAIR_TAG) { *(ptr++) = C_u_i_car(lst); lst = C_u_i_cdr(lst); } - /* copy into new array */ - av2 = C_alloc(n + 1); - av2[ 0 ] = k; - C_memcpy(av2 + 1, C_temporary_stack_limit, n * sizeof(C_word)); - C_do_apply(n + 1, av2); + C_do_apply(n, av2); } if(C_immediatep(lst)) { @@ -11352,6 +11361,7 @@ static void bignum_to_str_2(C_word c, C_word *av) void C_ccall C_make_structure(C_word c, C_word *av) { if(!C_demand(c - 1)) { + assert(C_temporary_stack == C_temporary_stack_bottom); C_temporary_stack = C_temporary_stack_bottom - (c - 1); C_memmove(C_temporary_stack, av + 1, (c - 1) * sizeof(C_word)); C_reclaim((void *)make_structure_2, c - 1); -- 2.1.4