Hi Peter, On 2016-08-15 21:52, Peter Bex wrote: > With this patch, CHICKEN no longer borks via an assertion failure. > Instead, it will either loop forever if the stack itself is too small
I think looping forever in this situation is potentially quite nasty for users, since without an understanding of how `apply` works under the hood it'd be very difficult to debug. The attached patch makes CHICKEN raise a Scheme-level exception when this happens, instead. The approach is extremely simple, please see the commit message for an explanation. Unless I'm overlooking something this will actually work for most other procedures, too, but for right now I've only applied it (heh) to C_apply and C_apply_values since those are the functions most likely to have an unsatisfiable stack demand. If it looks good, though, it might be worth investigating as a general safety measure, assuming the performance impact is negligible. This patch applies atop the previous one in the thread, so if it looks OK I'm happy to push that one first. Cheers, Evan
From e05f8eed98a1f46a4f922e9a77f89970701af6cc Mon Sep 17 00:00:00 2001 From: Evan Hanson <[email protected]> Date: Thu, 8 Sep 2016 22:32:21 +1200 Subject: [PATCH] Detect and signal error on stack overflow in `apply` When a demand for nursery space is unsatisfied even after popping the stack and reinvoking the caller, going back to C_save_and_reclaim to try to reclaim space *again* just leads to an infinite loop. Since there's not much else we can do in this situation, it's better to signal the problem as a Scheme-level error. To do this, we can just remember that there was a stack demand when jumping into the GC from C_apply or C_apply_values and, if the same demand fails when we bounce back and try again, we barf. The one tricky thing to this is that we must forget the remembered demand if the jump to the GC is redirected to the interrupt handler. Otherwise, the next call to C_apply/C_apply_values would only get one chance to demand nursery space. --- chicken.h | 4 ++-- runtime.c | 30 ++++++++++++++++++++---------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/chicken.h b/chicken.h index f5a706f..0aaa956 100644 --- a/chicken.h +++ b/chicken.h @@ -1076,7 +1076,7 @@ typedef void (C_ccall *C_proc)(C_word, C_word *) C_noret; # define C_stress 1 #endif -#define C_stack_overflow_check C_stack_check1(C_stack_overflow()) +#define C_stack_overflow_check C_stack_check1(C_stack_overflow(NULL)) #if C_STACK_GROWS_DOWNWARD # define C_demand(n) (C_stress && ((C_word)(C_stack_pointer - C_stack_limit) > (n))) @@ -1716,7 +1716,7 @@ C_fctexport void C_bad_argc(int c, int n) C_noret; C_fctexport void C_bad_min_argc(int c, int n) C_noret; C_fctexport void C_bad_argc_2(int c, int n, C_word closure) C_noret; C_fctexport void C_bad_min_argc_2(int c, int n, C_word closure) C_noret; -C_fctexport void C_stack_overflow(void) C_noret; +C_fctexport void C_stack_overflow(C_char *loc) C_noret; C_fctexport void C_unbound_error(C_word sym) C_noret; C_fctexport void C_no_closure_error(C_word x) C_noret; C_fctexport void C_div_by_zero_error(char *loc) C_noret; diff --git a/runtime.c b/runtime.c index 80f0690..4416e93 100644 --- a/runtime.c +++ b/runtime.c @@ -454,6 +454,7 @@ static volatile C_TLS int static C_TLS unsigned int mutation_count, tracked_mutation_count, + stack_check_demand, stack_size; static C_TLS int chicken_is_initialized; #ifdef HAVE_SIGSETJMP @@ -2418,17 +2419,12 @@ void C_bad_min_argc_2(int c, int n, C_word closure) } -void C_stack_overflow(void) +void C_stack_overflow(C_char *loc) { - barf(C_STACK_OVERFLOW_ERROR, NULL); + barf(C_STACK_OVERFLOW_ERROR, loc); } -void C_stack_overflow_with_msg(C_char *msg) -{ - barf(C_STACK_OVERFLOW_ERROR, NULL); -} - void C_unbound_error(C_word sym) { barf(C_UNBOUND_VARIABLE_ERROR, NULL, sym); @@ -2878,8 +2874,10 @@ C_regparm void C_fcall C_reclaim(void *trampoline, C_word c) /* assert(C_timer_interrupt_counter >= 0); */ - if(pending_interrupts_count > 0 && C_interrupts_enabled) + if(pending_interrupts_count > 0 && C_interrupts_enabled) { + stack_check_demand = 0; /* forget demand: we're not going to gc yet */ handle_interrupt(trampoline); + } cell.enabled = 0; cell.event = C_DEBUG_GC; @@ -6273,8 +6271,14 @@ void C_ccall C_apply(C_word c, C_word *av) len = C_unfix(C_u_i_length(lst)); av2_size = 2 + non_list_args + len; - if(!C_demand(av2_size)) + if(C_demand(av2_size)) + stack_check_demand = 0; + else if(stack_check_demand) + C_stack_overflow("apply"); + else { + stack_check_demand = av2_size; C_save_and_reclaim((void *)C_apply, c, av); + } av2 = ptr = C_alloc(av2_size); *(ptr++) = fn; @@ -6418,8 +6422,14 @@ void C_ccall C_apply_values(C_word c, C_word *av) len = C_unfix(C_u_i_length(lst)); n = len + 1; - if(!C_demand(n)) + if(C_demand(n)) + stack_check_demand = 0; + else if(stack_check_demand) + C_stack_overflow("apply"); + else { + stack_check_demand = n; C_save_and_reclaim((void *)C_apply_values, c, av); + } av2 = C_alloc(n); av2[ 0 ] = k; -- 2.1.4
From c2fadc5f62b1175b7ece26519301cb85c72a83c0 Mon Sep 17 00:00:00 2001 From: Evan Hanson <[email protected]> Date: Thu, 8 Sep 2016 22:32:21 +1200 Subject: [PATCH] Detect and signal error on stack overflow in `apply` When a demand for nursery space is unsatisfied even after popping the stack and reinvoking the caller, going back to C_save_and_reclaim to try to reclaim space *again* just leads to an infinite loop. Since there's not much else we can do in this situation, it's better to signal the problem as a Scheme-level error. To do this, we can just remember that there was a stack demand when jumping into the GC from C_apply or C_apply_values and, if the same demand fails when we bounce back and try again, we barf. The one tricky thing to this is that we must forget the remembered demand if the jump to the GC is redirected to the interrupt handler. Otherwise, the next call to C_apply/C_apply_values would only get one chance to demand nursery space. --- chicken.h | 4 ++-- runtime.c | 30 ++++++++++++++++++++---------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/chicken.h b/chicken.h index b076661..9e73666 100644 --- a/chicken.h +++ b/chicken.h @@ -1132,7 +1132,7 @@ typedef void (C_ccall *C_proc)(C_word, C_word *) C_noret; # define C_stress 1 #endif -#define C_stack_overflow_check C_stack_check1(C_stack_overflow()) +#define C_stack_overflow_check C_stack_check1(C_stack_overflow(NULL)) /* TODO: The C_scratch_usage checks should probably be moved. Maybe * we should add a core#allocate_scratch_inline which will insert @@ -1847,7 +1847,7 @@ C_fctexport void C_bad_argc(int c, int n) C_noret; C_fctexport void C_bad_min_argc(int c, int n) C_noret; C_fctexport void C_bad_argc_2(int c, int n, C_word closure) C_noret; C_fctexport void C_bad_min_argc_2(int c, int n, C_word closure) C_noret; -C_fctexport void C_stack_overflow(void) C_noret; +C_fctexport void C_stack_overflow(C_char *loc) C_noret; C_fctexport void C_unbound_error(C_word sym) C_noret; C_fctexport void C_no_closure_error(C_word x) C_noret; C_fctexport void C_div_by_zero_error(char *loc) C_noret; diff --git a/runtime.c b/runtime.c index fea8714..403ac8f 100644 --- a/runtime.c +++ b/runtime.c @@ -483,6 +483,7 @@ static volatile C_TLS int static C_TLS unsigned int mutation_count, tracked_mutation_count, + stack_check_demand, stack_size; static C_TLS int chicken_is_initialized; #ifdef HAVE_SIGSETJMP @@ -2586,17 +2587,12 @@ void C_bad_min_argc_2(int c, int n, C_word closure) } -void C_stack_overflow(void) +void C_stack_overflow(C_char *loc) { - barf(C_STACK_OVERFLOW_ERROR, NULL); + barf(C_STACK_OVERFLOW_ERROR, loc); } -void C_stack_overflow_with_msg(C_char *msg) -{ - barf(C_STACK_OVERFLOW_ERROR, NULL); -} - void C_unbound_error(C_word sym) { barf(C_UNBOUND_VARIABLE_ERROR, NULL, sym); @@ -3285,8 +3281,10 @@ C_regparm void C_fcall C_reclaim(void *trampoline, C_word c) /* assert(C_timer_interrupt_counter >= 0); */ - if(pending_interrupts_count > 0 && C_interrupts_enabled) + if(pending_interrupts_count > 0 && C_interrupts_enabled) { + stack_check_demand = 0; /* forget demand: we're not going to gc yet */ handle_interrupt(trampoline); + } cell.enabled = 0; cell.event = C_DEBUG_GC; @@ -7460,8 +7458,14 @@ void C_ccall C_apply(C_word c, C_word *av) len = C_unfix(C_u_i_length(lst)); av2_size = 2 + non_list_args + len; - if(!C_demand(av2_size)) + if(C_demand(av2_size)) + stack_check_demand = 0; + else if(stack_check_demand) + C_stack_overflow("apply"); + else { + stack_check_demand = av2_size; C_save_and_reclaim((void *)C_apply, c, av); + } av2 = ptr = C_alloc(av2_size); *(ptr++) = fn; @@ -7605,8 +7609,14 @@ void C_ccall C_apply_values(C_word c, C_word *av) len = C_unfix(C_u_i_length(lst)); n = len + 1; - if (!C_demand(n)) + if(C_demand(n)) + stack_check_demand = 0; + else if(stack_check_demand) + C_stack_overflow("apply"); + else { + stack_check_demand = n; C_save_and_reclaim((void *)C_apply_values, c, av); + } av2 = C_alloc(n); av2[ 0 ] = k; -- 2.1.4
signature.asc
Description: Digital signature
_______________________________________________ Chicken-hackers mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/chicken-hackers
