Hi Ludo,

OK, here's my next attempt at a solution for this problem. :-)

Compared to the previous stack calibration patch/approach, the main
points of this one are that

- it uses a much larger amount of executed code to calibrate stack
usage: specifically, all the code involved in starting up a standard
debug-mode REPL

- it focusses on the problem of getting `make check' to pass (when it
should do so)

- it does not modify the value or meaning of the default, C-coded stack limit

- it doesn't require building the whole of Guile twice!

I'm only looking at this stage for general thoughts; if you think this
approach looks good, I will still need to

- incorporate a "-l ../libguile/stack-limit-calibration.scm" into the
running of test-suite/tests/*

- do ChangeLogs, docs and NEWS.

Please let me know what you think.

      Neil
From ed168b4000df30e8ae205562838e2358c33f7c8d Mon Sep 17 00:00:00 2001
From: Neil Jerram <[EMAIL PROTECTED]>
Date: Thu, 9 Oct 2008 23:36:46 +0100
Subject: [PATCH] Stack calibration mark 2

---
 libguile/Makefile.am                |    5 +-
 libguile/debug.h                    |    3 +-
 libguile/eval.c                     |    4 +-
 libguile/measure-hwm.scm            |  122 +++++++++++++++++++++++++++++++++++
 libguile/stackchk.c                 |   19 ++++++
 libguile/stackchk.h                 |    9 +++
 libguile/threads.c                  |    2 +-
 libguile/threads.h                  |    5 ++
 test-suite/standalone/test-use-srfi |    6 +-
 9 files changed, 168 insertions(+), 7 deletions(-)
 create mode 100644 libguile/measure-hwm.scm

diff --git a/libguile/Makefile.am b/libguile/Makefile.am
index eb76237..073deff 100644
--- a/libguile/Makefile.am
+++ b/libguile/Makefile.am
@@ -153,7 +153,7 @@ EXTRA_DOT_DOC_FILES = @EXTRA_DOT_DOC_FILES@
 
 BUILT_SOURCES = cpp_err_symbols.c cpp_sig_symbols.c libpath.h \
     version.h scmconfig.h \
-    $(DOT_X_FILES) $(EXTRA_DOT_X_FILES)
+    $(DOT_X_FILES) $(EXTRA_DOT_X_FILES) stack-limit-calibration.scm
 
 EXTRA_libguile_la_SOURCES = _scm.h		\
     inet_aton.c memmove.c putenv.c strerror.c	\
@@ -313,6 +313,9 @@ guile-procedures.txt: guile-procedures.texi
 
 endif
 
+stack-limit-calibration.scm: measure-hwm.scm guile$(EXEEXT)
+	$(preinstguile) -s measure-hwm.scm > $@
+
 c-tokenize.c: c-tokenize.lex
 	flex -t $(srcdir)/c-tokenize.lex > $@ || { rm $@; false; }
 
diff --git a/libguile/debug.h b/libguile/debug.h
index c292004..5a82cc6 100644
--- a/libguile/debug.h
+++ b/libguile/debug.h
@@ -58,7 +58,8 @@ SCM_API scm_t_option scm_debug_opts[];
 #define SCM_STACK_LIMIT		scm_debug_opts[12].val
 #define SCM_SHOW_FILE_NAME	scm_debug_opts[13].val
 #define SCM_WARN_DEPRECATED	scm_debug_opts[14].val
-#define SCM_N_DEBUG_OPTIONS 15
+#define SCM_STACK_HWM   	scm_debug_opts[15].val
+#define SCM_N_DEBUG_OPTIONS 16
 
 SCM_API int scm_debug_mode_p;
 SCM_API int scm_check_entry_p;
diff --git a/libguile/eval.c b/libguile/eval.c
index 897f164..b6bc52d 100644
--- a/libguile/eval.c
+++ b/libguile/eval.c
@@ -3101,7 +3101,8 @@ scm_t_option scm_debug_opts[] = {
   { SCM_OPTION_BOOLEAN, "debug", 0, "Use the debugging evaluator." },
   { SCM_OPTION_INTEGER, "stack", 20000, "Stack size limit (measured in words; 0 = no check)." },
   { SCM_OPTION_SCM, "show-file-name", (unsigned long)SCM_BOOL_T, "Show file names and line numbers in backtraces when not `#f'.  A value of `base' displays only base names, while `#t' displays full names."},
-  { SCM_OPTION_BOOLEAN, "warn-deprecated", 0, "Warn when deprecated features are used." }
+  { SCM_OPTION_BOOLEAN, "warn-deprecated", 0, "Warn when deprecated features are used." },
+  { SCM_OPTION_BOOLEAN, "stack-hwm", 0, "Track maximum stack size used (also known as a `high water mark', hence the option name)." }
 };
 
 scm_t_option scm_evaluator_trap_table[] = {
@@ -3266,6 +3267,7 @@ CEVAL (SCM x, SCM env)
   scm_i_set_last_debug_frame (&debug);
 #endif
 #ifdef EVAL_STACK_CHECKING
+  SCM_CHECK_STACK_HWM (&proc);
   if (scm_stack_checking_enabled_p && SCM_STACK_OVERFLOW_P (&proc))
     {
 #ifdef DEVAL
diff --git a/libguile/measure-hwm.scm b/libguile/measure-hwm.scm
new file mode 100644
index 0000000..85f8d2b
--- /dev/null
+++ b/libguile/measure-hwm.scm
@@ -0,0 +1,122 @@
+;;;; Copyright (C) 2008 Free Software Foundation, Inc.
+;;;;
+;;;; This library is free software; you can redistribute it and/or
+;;;; modify it under the terms of the GNU Lesser General Public
+;;;; License as published by the Free Software Foundation; either
+;;;; version 2.1 of the License, or (at your option) any later version.
+;;;; 
+;;;; This library is distributed in the hope that it will be useful,
+;;;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;;;; Lesser General Public License for more details.
+;;;; 
+;;;; You should have received a copy of the GNU Lesser General Public
+;;;; License along with this library; if not, write to the Free Software
+;;;; Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+;;;;
+
+;;; Commentary:
+
+;;; This code is run during the Guile build, in order to set the stack
+;;; limit to a value that will allow the `make check' tests to pass,
+;;; taking into account the average stack usage on the build platform.
+;;; For more detail, see the text below that gets written out to the
+;;; stack limit calibration file.
+
+;;; Code:
+
+;; Store off Guile's default stack limit.
+(define default-stack-limit (cadr (memq 'stack (debug-options))))
+
+;; Now disable the stack limit, so that we don't get a stack overflow
+;; while running this code!
+(debug-set! stack 0)
+
+;; Enable stack high water mark tracking.
+(debug-enable 'stack-hwm)
+
+;; Call (turn-on-debugging) and (top-repl) in order to simulate as
+;; closely as possible what happens - and in particular, how much
+;; stack is used - when a standard Guile REPL is started up.
+;;
+;; `make check' stack overflow errors have been reported in the past
+;; for:
+;;
+;; - test-suite/standalone/test-use-srfi, which runs `guile -q
+;;   --use-srfi=...' a few times, with standard input for the REPL
+;;   coming from a shell script
+;;
+;; - test-suite/tests/elisp.test, which does not involve the REPL, but
+;;   has a lot of `use-modules' calls.
+;;
+;; Stack high water mark (HWM) measurements show that the HWM is
+;; higher in the test-use-srfi case - specifically because of the
+;; complexity of (top-repl) - so that is what we simulate for our
+;; calibration model here.
+(turn-on-debugging)
+(with-output-to-port (%make-void-port "w")
+  (lambda ()
+    (with-input-from-string "\n" top-repl)))
+
+;; Get the stack HWM that resulted from running that code.
+(define top-repl-hwm-measured (%get-reset-stack-hwm))
+
+;; This is the value of top-repl-hwm-measured that we get on a
+;; `canonical' build platform.  (See text below for what that means.)
+(define top-repl-hwm-i386-gnu-linux 9184)
+
+;; Using the above results, output code that tests can run in order to
+;; configure the stack limit correctly for the current build platform.
+(format #t "\
+;; Stack limit calibration file.
+;;
+;; This file is automatically generated by Guile when it builds, in
+;; order to set the stack limit to a value that reflects the stack
+;; usage of the build platform (OS + compiler + compilation options),
+;; specifically so that none of Guile's own tests (which are run by
+;; `make check') fail because of a benign stack overflow condition.
+;;
+;; By a `benign' stack overflow condition, we mean one where the test
+;; code is behaving correctly, but exceeds the configured stack limit
+;; because the limit is set too low.  A non-benign stack overflow
+;; condition would be if a piece of test code behaved significantly
+;; differently on some platform to how it does normally, and as a
+;; result consumed a lot more stack.  Although they seem pretty
+;; unlikely, we would want to catch non-benign conditions like this,
+;; and that is why we don't just do `(debug-set! stack 0)' when
+;; running `make check'.
+;;
+;; Although the primary purpose of this file is to prevent `make
+;; check' from failing without good reason, Guile developers and users
+;; may also find the following information useful, when determining
+;; what stack limit to configure for their own programs.
+
+ (let (;; The stack high water mark measured when starting up the
+       ;; standard Guile REPL on the current build platform.
+       (top-repl-hwm-measured ~a)
+
+       ;; The value of top-repl-hwm-measured that we get when building
+       ;; Guile on an i386 GNU/Linux system, after configuring with
+       ;; `./configure --enable-maintainer-mode --with-threads'.
+       ;; (Hereafter referred to as the `canonical' build platform.)
+       (top-repl-hwm-i386-gnu-linux ~a)
+
+       ;; Guile's default stack limit (i.e. the initial, C-coded value
+       ;; of the 'stack debug option).  In the context of this file,
+       ;; the important thing about this number is that we know that
+       ;; it allows all of the `make check' tests to pass on the
+       ;; canonical build platform.
+       (default-stack-limit ~a)
+
+       ;; Calibrated stack limit.  This is the default stack limit,
+       ;; scaled by the factor between top-repl-hwm-i386-gnu-linux and
+       ;; top-repl-hwm-measured.
+       (calibrated-stack-limit ~a))
+
+   ;; Configure the calibrated stack limit.
+   (debug-set! stack calibrated-stack-limit))
+"
+	top-repl-hwm-measured
+	top-repl-hwm-i386-gnu-linux
+	default-stack-limit
+	(/ (* default-stack-limit top-repl-hwm-measured) top-repl-hwm-i386-gnu-linux))
diff --git a/libguile/stackchk.c b/libguile/stackchk.c
index 391ce21..d53c118 100644
--- a/libguile/stackchk.c
+++ b/libguile/stackchk.c
@@ -24,6 +24,7 @@
 #include "libguile/_scm.h"
 #include "libguile/ports.h"
 #include "libguile/root.h"
+#include "libguile/threads.h"
 
 #include "libguile/stackchk.h"
 
@@ -78,6 +79,24 @@ scm_stack_report ()
   scm_puts ("\n", port);
 }
 
+
+SCM_DEFINE (scm_sys_get_reset_stack_hwm, "%get-reset-stack-hwm", 0, 0, 0,
+	    (),
+	    "Get and reset the stack high water mark for the current thread.")
+#define FUNC_NAME s_scm_sys_get_reset_stack_hwm
+{
+  scm_i_thread *t = SCM_I_CURRENT_THREAD;
+#if SCM_STACK_GROWS_UP
+  int hwm = t->hwm - t->base;
+#else
+  int hwm = t->base - t->hwm;
+#endif
+  t->hwm = t->base;
+  return scm_from_int (hwm);
+}
+#undef FUNC_NAME
+
+
 void
 scm_init_stackchk ()
 {
diff --git a/libguile/stackchk.h b/libguile/stackchk.h
index 9a5c59f..2b1725b 100644
--- a/libguile/stackchk.h
+++ b/libguile/stackchk.h
@@ -38,14 +38,22 @@
 #  define SCM_STACK_OVERFLOW_P(s)\
    (SCM_STACK_PTR (s) \
     > (SCM_I_CURRENT_THREAD->base + SCM_STACK_LIMIT))
+#  define SCM_STACK_PASSED_HWM(s)\
+   (SCM_STACK_PTR (s) > SCM_I_CURRENT_THREAD->hwm)
 # else
 #  define SCM_STACK_OVERFLOW_P(s)\
    (SCM_STACK_PTR (s) \
     < (SCM_I_CURRENT_THREAD->base - SCM_STACK_LIMIT))
+#  define SCM_STACK_PASSED_HWM(s)\
+   (SCM_STACK_PTR (s) < SCM_I_CURRENT_THREAD->hwm)
 # endif
+# define SCM_CHECK_STACK_HWM(s)\
+   if (SCM_STACK_HWM && SCM_STACK_PASSED_HWM (s))\
+     SCM_I_CURRENT_THREAD->hwm = SCM_STACK_PTR (s)
 # define SCM_CHECK_STACK\
     {\
        SCM_STACKITEM stack;\
+       SCM_CHECK_STACK_HWM (&stack);\
        if (SCM_STACK_OVERFLOW_P (&stack) && scm_stack_checking_enabled_p)\
 	 scm_report_stack_overflow ();\
     }
@@ -60,6 +68,7 @@ SCM_API int scm_stack_checking_enabled_p;
 SCM_API void scm_report_stack_overflow (void);
 SCM_API long scm_stack_size (SCM_STACKITEM *start);
 SCM_API void scm_stack_report (void);
+SCM_API SCM scm_sys_get_reset_stack_hwm (void);
 SCM_API void scm_init_stackchk (void);
 
 #endif  /* SCM_STACKCHK_H */
diff --git a/libguile/threads.c b/libguile/threads.c
index 4377727..afd7628 100644
--- a/libguile/threads.c
+++ b/libguile/threads.c
@@ -423,7 +423,7 @@ guilify_self_1 (SCM_STACKITEM *base)
   t->block_asyncs = 1;
   t->pending_asyncs = 1;
   t->last_debug_frame = NULL;
-  t->base = base;
+  t->hwm = t->base = base;
 #ifdef __ia64__
   /* Calculate and store off the base of this thread's register
      backing store (RBS).  Unfortunately our implementation(s) of
diff --git a/libguile/threads.h b/libguile/threads.h
index d58a0fb..9b5f038 100644
--- a/libguile/threads.h
+++ b/libguile/threads.h
@@ -113,6 +113,11 @@ typedef struct scm_i_thread {
   scm_t_contregs *pending_rbs_continuation;
 #endif
 
+  /* Stack high water mark.  In other words, the highest stack address
+     reached, for a stack that grows upwards, and the lowest stack
+     address reached, for a stack that grows downwards. */
+  SCM_STACKITEM *hwm;
+
 } scm_i_thread;
 
 #define SCM_I_IS_THREAD(x)    SCM_SMOB_PREDICATE (scm_tc16_thread, x)
diff --git a/test-suite/standalone/test-use-srfi b/test-suite/standalone/test-use-srfi
index 7186b5a..6964af7 100755
--- a/test-suite/standalone/test-use-srfi
+++ b/test-suite/standalone/test-use-srfi
@@ -19,7 +19,7 @@
 
 # Test that two srfi numbers on the command line work.
 #
-guile -q --use-srfi=1,10 >/dev/null <<EOF
+guile -q -l ../../libguile/stack-limit-calibration.scm --use-srfi=1,10 >/dev/null <<EOF
 (if (and (defined? 'partition)
          (defined? 'define-reader-ctor))
     (exit 0)   ;; good
@@ -38,7 +38,7 @@ fi
 # `top-repl' the core bindings got ahead of anything --use-srfi gave.
 #
 
-guile -q --use-srfi=1 >/dev/null <<EOF
+guile -q -l ../../libguile/stack-limit-calibration.scm --use-srfi=1 >/dev/null <<EOF
 (catch #t
   (lambda ()
     (iota 2 3 4))
@@ -56,7 +56,7 @@ fi
 # exercises duplicates handling in `top-repl' versus `use-srfis' (in
 # boot-9.scm).
 #
-guile -q --use-srfi=17 >/dev/null <<EOF
+guile -q -l ../../libguile/stack-limit-calibration.scm --use-srfi=17 >/dev/null <<EOF
 (if (procedure-with-setter? car)
     (exit 0)   ;; good
     (exit 1))  ;; bad
-- 
1.5.6.5

Reply via email to