From b916f53a04df5e9c3823487e06ab22ec57d05151 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 ac4a84a..105a7e7 100644 --- a/runtime.c +++ b/runtime.c @@ -1410,8 +1410,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); } @@ -2706,11 +2708,13 @@ C_mutate_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); } @@ -2839,6 +2843,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); @@ -5958,41 +5963,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); } @@ -6109,22 +6113,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)) { @@ -7804,6 +7813,7 @@ void C_ccall C_fixnum_to_string(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