Hi all,
As we figured out in ticket #910, there are two problems when directly
invoking procedures with a large number of rest arguments (not via APPLY):
- there's a bug in GCC 4.5's code generation which causes random errors
like segfaults and illegal instructions to occur.
- even with a correct C compiler, when going beyond the temporary stack's
limit, this isn't checked and segfaults will occur.
Here's a patch to add detection for the latter situation, and an improved
version of the apply-test, which will detect the former situation as well.
This can help users to determine whether their CHICKEN is built correctly
and can produce working executables with procedure applications of large
argument counts, and tells them to upgrade GCC if it segfaults.
Unfortunately, the test takes rather long to build (it has to be
compiled now), but I think it's worth it regardless as it will help
prevent spurious bug reports by detecting both error situations.
Cheers,
Peter
--
http://www.more-magic.net
From 6d07a5699e8fd13405c0b750870a5b8e85daef21 Mon Sep 17 00:00:00 2001
From: Peter Bex peter@xs4all.nl
Date: Wed, 24 Jul 2013 21:05:10 +0200
Subject: [PATCH] Add checks for hitting the rest arg count limit on direct
procedure application. Fixes some of the confusion from #910
---
NEWS | 2 ++
chicken.h| 4 +++-
runtime.c| 7 +++
tests/apply-test.scm | 49 +++--
tests/runtests.bat | 4
tests/runtests.sh| 2 ++
6 files changed, 61 insertions(+), 7 deletions(-)
diff --git a/NEWS b/NEWS
index a13e423..0f77a4c 100644
--- a/NEWS
+++ b/NEWS
@@ -46,6 +46,8 @@
- Runtime system
- Special events in poll() are now handled, avoiding hangs in threaded apps.
+ - When invoking procedures with many rest arguments directly (not via APPLY),
+raise an error when argument count limit was reached instead of crashing.
- C API
- Deprecated C_get_argument[_2] and C_get_environment_variable[_2] functions.
diff --git a/chicken.h b/chicken.h
index 6d5d7f9..d09d3e3 100644
--- a/chicken.h
+++ b/chicken.h
@@ -1003,7 +1003,7 @@ extern double trunc(double);
#define C_save(x) (*(--C_temporary_stack) = (C_word)(x))
#define C_adjust_stack(n) (C_temporary_stack -= (n))
#define C_rescue(x, i) (C_temporary_stack[ i ] = (x))
-#define C_save_rest(s, c, n) for(va_start(v, s); c-- (n);
C_save(va_arg(v, C_word)))
+#define C_save_rest(s, c, n) do { if((C_temporary_stack - (c - (n)))
C_temporary_stack_limit) C_temp_stack_overflow(); for(va_start(v, s); c--
(n); C_save(va_arg(v, C_word))); }while(0)
#define C_rest_count(c)((C_temporary_stack_bottom -
C_temporary_stack) - (c))
#define C_restore (*(C_temporary_stack++))
#define C_heaptop ((C_word **)(C_fromspace_top))
@@ -1584,6 +1584,7 @@ C_varextern C_TLS time_t C_startup_time_seconds;
C_varextern C_TLS C_word
*C_temporary_stack,
*C_temporary_stack_bottom,
+ *C_temporary_stack_limit,
*C_stack_limit;
C_varextern C_TLS C_long
C_timer_interrupt_counter,
@@ -1689,6 +1690,7 @@ 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_temp_stack_overflow(void) 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 d8bea05..bbeb2f6 100644
--- a/runtime.c
+++ b/runtime.c
@@ -2383,6 +2383,13 @@ void C_stack_overflow_with_msg(C_char *msg)
barf(C_STACK_OVERFLOW_ERROR, NULL);
}
+void C_temp_stack_overflow(void)
+{
+ /* Just raise a too many parameters error; it isn't very useful to
+ show a different message here. */
+ barf(C_TOO_MANY_PARAMETERS_ERROR, NULL);
+}
+
void C_unbound_error(C_word sym)
{
diff --git a/tests/apply-test.scm b/tests/apply-test.scm
index d05356b..4704576 100644
--- a/tests/apply-test.scm
+++ b/tests/apply-test.scm
@@ -1,14 +1,51 @@
(require-extension srfi-1)
-(define manyargs (feature? 'manyargs))
+(define max-argcount ##sys#apply-argument-limit)
-(when manyargs (print many arguments supported.))
+(begin-for-syntax
+ (define max-direct-argcount
+ (cond-expand
+ ;; This depends the temp stack's size (as does max-argcount w/ manyargs).
+ ;; We can't use the foreign value for C_TEMPORARY_STACK_SIZE here because
+ ;; we're evaluating this in the compiler, not compiling it (confused yet?)
+ (compiling 2048)
+ ;; But in interpreted mode, everything boils down to apply, so if no
apply
+ ;; hack is available, we're more limited in csi than in csc.
+ (else ##sys#apply-argument-limit
+
+(when (feature? 'manyargs) (print many arguments
