Re: stack overflow

Re: stack overflow

[Neil Jerram]

ludo@gnu.org (Ludovic Courtès) writes:

> Hi,
>
> Rainer Tammer <tammer@tammer.net> writes:
>
>> I added -qtune=auto but this did not solve the problem...
>> I will try other optimization settings...
>
> You did "make clean all" after reconfiguring with your `CFLAGS', right?
> Also, do the actual compilation command lines show that your `CFLAGS'
> settings were taken into account?
>
> Besides, you can try adding "(debug-set! stack 40000)" to your
> `~/.guile' and see if that makes a difference, and increase it until it
> works, just to get an idea.

[moving over to guile-devel...]

Not to disagree with anything that's already been said in this
thread...  But I wonder if there is a way to make Guile's stack
overflow checking a bit less fragile - i.e. less subject to the
behaviour of particular compilers / OSs / optimization options?

I think all we're really trying to do, with the stack overflow
feature, is guard against a suspected infinite recursion, without
resorting to crashing the whole program.  Perhaps there is a cunning
way to do that without having to set an arbitrary stack depth limit?
Any ideas would be most welcome.

Regards,
        Neil

Re: stack overflow

[Ludovic Courtès]

Hi,

Neil Jerram <neil@ossau.uklinux.net> writes:

> Not to disagree with anything that's already been said in this
> thread...  But I wonder if there is a way to make Guile's stack
> overflow checking a bit less fragile - i.e. less subject to the
> behaviour of particular compilers / OSs / optimization options?
>
> I think all we're really trying to do, with the stack overflow
> feature, is guard against a suspected infinite recursion, without
> resorting to crashing the whole program.  Perhaps there is a cunning
> way to do that without having to set an arbitrary stack depth limit?
> Any ideas would be most welcome.

A platform-independent way to achieve this would be to somehow count
Scheme stack frames (since we are concerned with stack overflows in
Scheme code).  Of course, we don't want to traverse the whole Scheme
stack to determine the stack depth.  So the evaluator would need to
maintain the current stack depth in an integer.

In `eval.i.c', instead of:

  #ifdef EVAL_STACK_CHECKING
    if (scm_stack_checking_enabled_p && SCM_STACK_OVERFLOW_P (&proc))
      {

We could have something along the lines of:

  #ifdef EVAL_STACK_CHECKING
    if (scm_stack_checking_enabled_p)
      {
        scm_i_eval_stack_depth++;
        if (scm_i_eval_stack_depth > SCM_STACK_LIMIT)
          {

We must also change `RETURN' to do "scm_i_eval_stack_depth--".

Hopefully it's not too unreasonable performance-wise.

What do you think?

Thanks,
Ludovic.

Re: stack overflow

[Mikael Djurfeldt]

2008/2/14, Ludovic Courtès <ludo@gnu.org>:
>  We must also change `RETURN' to do "scm_i_eval_stack_depth--".
>
>  Hopefully it's not too unreasonable performance-wise.
>
>  What do you think?

I think you should measure the effect on performance.  Even if the hit
isn't dramatic, remember that many a little makes a mickle...

Speaking as a user, I would prefer a solution where the evaluator
measures stack size the same way as currently (i.e. without the need
to do extra work at every return).  It is possible to estimate the
average sizes of evaluator stack frames during startup and use this as
a conversion factor in the debug-options interface (scm_debug_opts) so
that the user setting is approximately consistent between platforms.

Re: stack overflow

[Ludovic Courtès]

Hi,

"Mikael Djurfeldt" <mikael@djurfeldt.com> writes:

> I think you should measure the effect on performance.  Even if the hit
> isn't dramatic, remember that many a little makes a mickle...

Indeed...

> Speaking as a user, I would prefer a solution where the evaluator
> measures stack size the same way as currently (i.e. without the need
> to do extra work at every return).  It is possible to estimate the
> average sizes of evaluator stack frames during startup and use this as
> a conversion factor in the debug-options interface (scm_debug_opts) so
> that the user setting is approximately consistent between platforms.

Hmm, I don't see how we could reliably estimate this, and I'm afraid it
would add non-determinism (e.g., estimate that varies with the phase of
moon, dubious estimates, loads of users suddenly reporting stack
overflows because Fedora Core now ships with a bleeding-edge compiler
noone else uses, etc.).

That said, I agree that an overhead-free solution similar to the current
one is preferable.

Thanks,
Ludovic.

Re: stack overflow

[Mikael Djurfeldt]

2008/2/14, Ludovic Courtès <ludo@gnu.org>:
>  > Speaking as a user, I would prefer a solution where the evaluator
>  > measures stack size the same way as currently (i.e. without the need
>  > to do extra work at every return).  It is possible to estimate the
>  > average sizes of evaluator stack frames during startup and use this as
>  > a conversion factor in the debug-options interface (scm_debug_opts) so
>  > that the user setting is approximately consistent between platforms.
>
>
> Hmm, I don't see how we could reliably estimate this, and I'm afraid it
>  would add non-determinism (e.g., estimate that varies with the phase of
>  moon, dubious estimates, loads of users suddenly reporting stack
>  overflows because Fedora Core now ships with a bleeding-edge compiler
>  noone else uses, etc.).

I was thinking about inserting code which actually *measures* the size
of frames during startup.  This could be done, for example, by
introducing a primitive which uses the internal stack measuring
functions.  One could use this primitive to measure how much stack
space some code sample uses.  By our knowledge of how many evaluator
stack frames this code sample uses, we can compute a reliable estimate
for the running instance of Guile.

Re: stack overflow

[Neil Jerram]

[-- Attachment #1: Type: text/plain, Size: 924 bytes --]

"Mikael Djurfeldt" <mikael@djurfeldt.com> writes:

> I was thinking about inserting code which actually *measures* the size
> of frames during startup.  This could be done, for example, by
> introducing a primitive which uses the internal stack measuring
> functions.  One could use this primitive to measure how much stack
> space some code sample uses.  By our knowledge of how many evaluator
> stack frames this code sample uses, we can compute a reliable estimate
> for the running instance of Guile.

Below is a proposed patch to do this.  When and if this gets deployed,
the third arg to %calibrate-stack-depth would be removed, so that it
doesn't generate any output.  But for now it's interesting to see what
results people on various OSs get.

Could people who've being getting "Stack overflow" errors try this
out, and also report (for interest) the ";; Stack calibration" line
that they get?

Thanks,
     Neil



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: new-stack-overflow.patch --]
[-- Type: text/x-diff, Size: 5003 bytes --]

Index: ice-9/boot-9.scm
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/ice-9/boot-9.scm,v
retrieving revision 1.356.2.10
diff -u -r1.356.2.10 boot-9.scm
--- ice-9/boot-9.scm	1 Sep 2007 17:11:00 -0000	1.356.2.10
+++ ice-9/boot-9.scm	25 Feb 2008 21:45:44 -0000
@@ -2289,6 +2289,14 @@
    (print-options print-enable print-disable)
    (print-set!)))
 
+;;; Stack depth calibration, for the 'stack debug option.
+
+(let ((x (%get-stack-depth)))
+  (let loop ((count 10))
+    (if (zero? count)
+	(%calibrate-stack-depth x (%get-stack-depth) 'report)
+	(cons count (loop (- count 1))))))
+
 \f
 
 ;;; {Running Repls}
Index: libguile/debug.h
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/debug.h,v
retrieving revision 1.58
diff -u -r1.58 debug.h
--- libguile/debug.h	4 Nov 2005 21:20:24 -0000	1.58
+++ libguile/debug.h	25 Feb 2008 21:45:44 -0000
@@ -75,6 +75,7 @@
     && scm_is_true (SCM_EXIT_FRAME_HDLR);\
   scm_debug_mode_p = SCM_DEVAL_P\
     || scm_check_entry_p || scm_check_apply_p || scm_check_exit_p;\
+  scm_calculate_stack_limit ();\
 } while (0)
 
 /* {Evaluator}
Index: libguile/stackchk.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/stackchk.c,v
retrieving revision 1.28.2.1
diff -u -r1.28.2.1 stackchk.c
--- libguile/stackchk.c	12 Feb 2006 13:42:51 -0000	1.28.2.1
+++ libguile/stackchk.c	25 Feb 2008 21:45:44 -0000
@@ -30,6 +30,13 @@
 
 #ifdef STACK_CHECKING
 int scm_stack_checking_enabled_p;
+int scm_stack_limit;
+
+/* As in y = mx + c.  These numbers define a linear transformation
+   from the stack depth specified as the 'stack debug option, to the
+   actual max stack depth that we allow. */
+static double calibrated_m = 1;
+static double calibrated_c = 0;
 
 SCM_SYMBOL (scm_stack_overflow_key, "stack-overflow");
 
@@ -44,6 +51,58 @@
 	     SCM_BOOL_F);
 }
 
+/* Stack depth calibration. */
+
+SCM_DEFINE (scm_sys_get_stack_depth, "%get-stack-depth", 0, 0, 0,
+	    (),
+	    "Return current stack depth.")
+#define FUNC_NAME s_scm_sys_get_stack_depth
+{
+  SCM_STACKITEM stack;
+  return scm_from_int (SCM_STACK_DEPTH (&stack));
+}
+#undef FUNC_NAME
+
+SCM_DEFINE (scm_sys_calibrate_stack_depth, "%calibrate-stack-depth", 2, 1, 0,
+	    (SCM d1, SCM d2, SCM debugp),
+	    "Calibrate linear transformation for stack depth limit checking.")
+#define FUNC_NAME s_scm_sys_calibrate_stack_depth
+{
+  /* x1 and x2 are the stack depth values that we get on a Debian
+     GNU/Linux ia32 system - which we take as our canonical system.
+     y1 and y2 are the values measured on the system where Guile is
+     currently running. */
+  int x1 = 170, x2 = 690, y1, y2;
+
+  SCM_VALIDATE_INT_COPY (1, d1, y1);
+  SCM_VALIDATE_INT_COPY (2, d2, y2);
+
+  calibrated_m = ((double) (y2 - y1)) / (x2 - x1);
+  calibrated_c = ((double) y2) - calibrated_m * x2;
+
+  if (scm_is_true (debugp) && !SCM_UNBNDP (debugp))
+    {
+      scm_puts (";; Stack calibration: (x1 x2 y1 y2 m c) = ",
+		scm_current_output_port ());
+      scm_write (scm_list_n (scm_from_int (x1), scm_from_int (x2),
+			     d1, d2,
+			     scm_from_double (calibrated_m),
+			     scm_from_double (calibrated_c),
+			     SCM_UNDEFINED),
+		 SCM_UNDEFINED);
+      scm_newline (SCM_UNDEFINED);
+    }
+
+  return SCM_UNSPECIFIED;
+}
+#undef FUNC_NAME
+
+void
+scm_calculate_stack_limit ()
+{
+  scm_stack_limit = (int) (calibrated_m * SCM_STACK_LIMIT + calibrated_c);
+}
+
 #endif
 
 long
Index: libguile/stackchk.h
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/stackchk.h,v
retrieving revision 1.20.2.1
diff -u -r1.20.2.1 stackchk.h
--- libguile/stackchk.h	12 Feb 2006 13:42:51 -0000	1.20.2.1
+++ libguile/stackchk.h	25 Feb 2008 21:45:44 -0000
@@ -35,14 +35,11 @@
 
 #ifdef STACK_CHECKING
 # if SCM_STACK_GROWS_UP
-#  define SCM_STACK_OVERFLOW_P(s)\
-   (SCM_STACK_PTR (s) \
-    > (SCM_I_CURRENT_THREAD->base + SCM_STACK_LIMIT))
+#  define SCM_STACK_DEPTH(s) (SCM_STACK_PTR (s) - SCM_I_CURRENT_THREAD->base)
 # else
-#  define SCM_STACK_OVERFLOW_P(s)\
-   (SCM_STACK_PTR (s) \
-    < (SCM_I_CURRENT_THREAD->base - SCM_STACK_LIMIT))
+#  define SCM_STACK_DEPTH(s) (SCM_I_CURRENT_THREAD->base - SCM_STACK_PTR (s))
 # endif
+# define SCM_STACK_OVERFLOW_P(s) (SCM_STACK_DEPTH (s) > scm_stack_limit)
 # define SCM_CHECK_STACK\
     {\
        SCM_STACKITEM stack;\
@@ -54,10 +51,14 @@
 #endif /* STACK_CHECKING */
 
 SCM_API int scm_stack_checking_enabled_p;
+SCM_API int scm_stack_limit;
 
 \f
 
 SCM_API void scm_report_stack_overflow (void);
+SCM_API SCM scm_sys_get_stack_depth (void);
+SCM_API SCM scm_sys_calibrate_stack_depth (SCM d1, SCM d2, SCM debugp);
+SCM_API void scm_calculate_stack_limit (void);
 SCM_API long scm_stack_size (SCM_STACKITEM *start);
 SCM_API void scm_stack_report (void);
 SCM_API void scm_init_stackchk (void);

Re: stack overflow

[Ludovic Courtès]

Hi,

Neil Jerram <neil@ossau.uklinux.net> writes:

> Below is a proposed patch to do this.  When and if this gets deployed,
> the third arg to %calibrate-stack-depth would be removed, so that it
> doesn't generate any output.  But for now it's interesting to see what
> results people on various OSs get.
>
> Could people who've being getting "Stack overflow" errors try this
> out, and also report (for interest) the ";; Stack calibration" line
> that they get?

I think time has come to integrate this patch as it's proved to fix
things for various people.  I tried it on several platforms, always
compiling with the default flags, i.e., `-O2'.  Here's what I got[*]:

  * i686-pc-linux-gnu, GCC 4.2.4
    ;; Stack calibration: (x1 x2 y1 y2 m c) = (170 690 170 690 1.0 0.0)

  * x86_64-unknown-linux-gnu, GCC 4.1.2
    ;; Stack calibration: (x1 x2 y1 y2 m c) = (170 690 41 181 0.269230769230769 -4.76923076923077)

  * sparc64-unknown-linux-gnu, GCC 4.1.3
    ;; Stack calibration: (x1 x2 y1 y2 m c) = (170 690 178 498 0.615384615384615 73.3846153846154)

  * hppa2.0w-hp-hpux11.11,
    HP92453-01 B.11.X.36086-36089-36092.GP HP C Compiler (cc)
    ;; Stack calibration: (x1 x2 y1 y2 m c) = (170 690 352 1472 2.15384615384615 -14.1538461538462)

  * ia64-unknown-linux-gnu (itanium2), GCC 4.1.2
    ;; Stack calibration: (x1 x2 y1 y2 m c) = (170 690 10 50 0.0769230769230769 -3.07692307692308)

  * i386-unknown-freebsd6.2, GCC 3.4.6
    ;; Stack calibration: (x1 x2 y1 y2 m c) = (170 690 114 394 0.538461538461538 22.4615384615385)

`pre-inst-guile' reaches the REPL in all cases, except on IA64 where it
stack-overflows (further investigation needed).

I'll comment the patch itself later on.

Thanks,
Ludovic.

[*] I really need to find a way to automate this.  If anyone knows of
    existing tools that would facilitate it (connecting to each machine,
    running `configure', `make', etc.), please let me know.  Otherwise,
    I guess it wouldn't be too hard to write a script to do that.

Stack calibration

[Ludovic Courtès]

Hi!

Neil Jerram <neil@ossau.uklinux.net> writes:

> "Mikael Djurfeldt" <mikael@djurfeldt.com> writes:
>
>> I was thinking about inserting code which actually *measures* the size
>> of frames during startup.  This could be done, for example, by
>> introducing a primitive which uses the internal stack measuring
>> functions.  One could use this primitive to measure how much stack
>> space some code sample uses.  By our knowledge of how many evaluator
>> stack frames this code sample uses, we can compute a reliable estimate
>> for the running instance of Guile.
>
> Below is a proposed patch to do this.  When and if this gets deployed,
> the third arg to %calibrate-stack-depth would be removed, so that it
> doesn't generate any output.  But for now it's interesting to see what
> results people on various OSs get.

That's the second important thing that should go in 1.8.6 IMO.

A few notes.

> --- ice-9/boot-9.scm	1 Sep 2007 17:11:00 -0000	1.356.2.10
> +++ ice-9/boot-9.scm	25 Feb 2008 21:45:44 -0000
> @@ -2289,6 +2289,14 @@
>     (print-options print-enable print-disable)
>     (print-set!)))
>  
> +;;; Stack depth calibration, for the 'stack debug option.
> +
> +(let ((x (%get-stack-depth)))
> +  (let loop ((count 10))
> +    (if (zero? count)
> +	(%calibrate-stack-depth x (%get-stack-depth) 'report)
> +	(cons count (loop (- count 1))))))
> +

[...]

> +SCM_DEFINE (scm_sys_calibrate_stack_depth, "%calibrate-stack-depth", 2, 1, 0,
> +	    (SCM d1, SCM d2, SCM debugp),
> +	    "Calibrate linear transformation for stack depth limit checking.")
> +#define FUNC_NAME s_scm_sys_calibrate_stack_depth
> +{
> +  /* x1 and x2 are the stack depth values that we get on a Debian
> +     GNU/Linux ia32 system - which we take as our canonical system.
> +     y1 and y2 are the values measured on the system where Guile is
> +     currently running. */
> +  int x1 = 170, x2 = 690, y1, y2;

These results are dependent on what the loop in `boot-9.scm' does.
Thus, it'd be nicer if they weren't that far away from it.

It might be worth mentioning the GCC version and optimization level that
led to this result.

Also, `x1' and `x2' can be made "static const" or some such.

> +  SCM_VALIDATE_INT_COPY (1, d1, y1);
> +  SCM_VALIDATE_INT_COPY (2, d2, y2);
> +
> +  calibrated_m = ((double) (y2 - y1)) / (x2 - x1);
> +  calibrated_c = ((double) y2) - calibrated_m * x2;

Shouldn't it be:

  calibrated_c = y1 - x1;

Also, the computation of `calibrated_m' needs more casts to `double' I
think.

> +  if (scm_is_true (debugp) && !SCM_UNBNDP (debugp))
> +    {
> +      scm_puts (";; Stack calibration: (x1 x2 y1 y2 m c) = ",
> +		scm_current_output_port ());
> +      scm_write (scm_list_n (scm_from_int (x1), scm_from_int (x2),
> +			     d1, d2,
> +			     scm_from_double (calibrated_m),
> +			     scm_from_double (calibrated_c),
> +			     SCM_UNDEFINED),
> +		 SCM_UNDEFINED);
> +      scm_newline (SCM_UNDEFINED);
> +    }

Could it be moved to a `%print-stack-calibration' function that we'd use
for troubleshooting?

> +void
> +scm_calculate_stack_limit ()
> +{
> +  scm_stack_limit = (int) (calibrated_m * SCM_STACK_LIMIT + calibrated_c);
> +}

How about entirely removing the startup overhead by computing the
calibration factors only once, at installation time?

This would mean:

  1. Compile all of Guile with the default calibration factors (m = 1
     and c = 0).

  2. Run a Scheme script that computes `m' and `c' and produces, say,
     `calibration.h', which is included by `stackchk.c'.  Both the
     computation and the reference stack depths (`x1' and `x2' above)
     would live in this script, which is clearer IMO.

  3. Invoke `make' recursively, which should rebuild libguile with the
     appropriate calibration factor (typically, only `stackchk.lo' would
     need to be recompiled). 

Would you like to do something like this?

Also, the on-line and Texi documentation of `eval-options' must be
updated to specify that the stack depth unit is "abstract" and
(hopefully) portable across platforms.

Thanks,
Ludo'.

Re: Stack calibration

[Neil Jerram]

[-- Attachment #1: Type: text/plain, Size: 3039 bytes --]

Hi there,

2008/9/12 Ludovic Courtès <ludo@gnu.org>:
>
> That's the second important thing that should go in 1.8.6 IMO.

Cool...

>> +  /* x1 and x2 are the stack depth values that we get on a Debian
>> +     GNU/Linux ia32 system - which we take as our canonical system.
>> +     y1 and y2 are the values measured on the system where Guile is
>> +     currently running. */
>> +  int x1 = 170, x2 = 690, y1, y2;
>
> These results are dependent on what the loop in `boot-9.scm' does.
> Thus, it'd be nicer if they weren't that far away from it.

Agreed, and I think I've addressed this in my latest version (below).

> It might be worth mentioning the GCC version and optimization level that
> led to this result.

Good idea, will do.  (This isn't yet in the attached patch.)

> Also, `x1' and `x2' can be made "static const" or some such.

In the new version, they are #defines.

>> +  SCM_VALIDATE_INT_COPY (1, d1, y1);
>> +  SCM_VALIDATE_INT_COPY (2, d2, y2);
>> +
>> +  calibrated_m = ((double) (y2 - y1)) / (x2 - x1);
>> +  calibrated_c = ((double) y2) - calibrated_m * x2;
>
> Shouldn't it be:
>
>  calibrated_c = y1 - x1;

No, don't think so!  My model equation is y = mx + c, so c = y - mx.

> Also, the computation of `calibrated_m' needs more casts to `double' I
> think.

I don't think so, and this hasn't changed in the new version.  Where
and why do you think more casts are needed?

> Could it be moved to a `%print-stack-calibration' function that we'd use
> for troubleshooting?

Yes.  In the attached patch, I've left this out completely, but I
think we should add it back in as %get-stack-calibration.

> How about entirely removing the startup overhead by computing the
> calibration factors only once, at installation time?
>
> This would mean:
>
>  1. Compile all of Guile with the default calibration factors (m = 1
>     and c = 0).

Agreed.

>  2. Run a Scheme script that computes `m' and `c' and produces, say,
>     `calibration.h', which is included by `stackchk.c'.  Both the
>     computation and the reference stack depths (`x1' and `x2' above)
>     would live in this script, which is clearer IMO.

Agreed, see new libguile/calibrate.scm file.

>  3. Invoke `make' recursively, which should rebuild libguile with the
>     appropriate calibration factor (typically, only `stackchk.lo' would
>     need to be recompiled).

I've done this part a bit differently - see the libguile/Makefile.am
changes - because I couldn't see exactly how the recursive make
approach would work.  If you think recursive make would be
significantly better, can you describe or propose the detailed changes
that would be needed?

> Also, the on-line and Texi documentation of `eval-options' must be
> updated to specify that the stack depth unit is "abstract" and
> (hopefully) portable across platforms.

I will do this (and also NEWS), but let's agree all the code first.

Many thanks for your detailed review, BTW!

Regards,
      Neil

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Avoid-stack-overflow-errors.patch --]
[-- Type: text/x-patch; name=0001-Avoid-stack-overflow-errors.patch, Size: 15738 bytes --]

From 6f84b52bd76e2777dbfddcda571f52dbc82895f3 Mon Sep 17 00:00:00 2001
From: Neil Jerram <neil@ossau.uklinux.net>
Date: Sat, 27 Sep 2008 19:08:07 +0100
Subject: [PATCH] Avoid stack overflow errors

... when building and testing Guile, by adjusting Guile's stack
overflow algorithm for the actual average depth of stack per eval call
that the build platform uses.

To avoid a penalty at runtime when using an installed Guile, we build
Guile in two stages.  An "uncalibrated" Guile (uguile) is built with
an arbitrary assumption about the platform's stack usage.  Then we use
uguile to run a Scheme script that measures actual stack usage, and
generates a modified C source file (stackchk-calibrated.c) that
contains these measurements.  Then we build a properly calibrated
Guile, from sources that include stackchk-calibrated.c.

* am/pre-inst-guile (preinstuguile, preinstuguiletool): New
  definitions.

* configure.in (UGUILE_FOR_BUILD): Set up in the same way as
  GUILE_FOR_BUILD.
  (pre-inst-uguile): Generate, from pre-inst-uguile.in.

* libguile/Makefile.am (noinst_PROGRAMS): Add uguile.
  (noinst_LTLIBRARIES): New, containing libuguile.la.
  (uguile_LDADD, uguile_CFLAGS, uguile_LDADD, uguile_LDFLAGS): New.
  (LIBGUILE_SOURCES): New, containing what used to be in libguile_la_SOURCES, minus stackchk.c.
  (libguile_la_SOURCES): Changed to LIBGUILE_SOURCES + stackchk-calibrated.c.
  (libuguile_la_CFLAGS): New.
  (libuguile_la_SOURCES): New, LIBGUILE_SOURCES + stackchk.c
  (EXTRA_libuguile_la_SOURCES): New, same as EXTRA_libguile_la_SOURCES.
  (libuguile_la_DEPENDENCIES, libuguile_la_LIBADD,
  libuguile_la_LDFLAGS): New, same as corresponding libguile*
  definitions.
  (stackchk-calibrated.c): New, built by uncalibrated guile.

* libguile/calibrate.scm: New, to generate stack calibration
  measurements.

* libguile/debug.h (SCM_RESET_DEBUG_MODE): Add
  scm_calculate_stack_limit () call.

* libguile/stackchk.c (scm_stack_limit, calibrated_m, calibrated_c):
  New variables.
  (scm_sys_get_stack_depth, scm_calculate_stack_limit): New functions.
  (scm_init_stackchk): If possible, calculate non-default values for
  calibrated_c and calibrated_m.  Also call scm_calculate_stack_limit ().

* libguile/stackchk.h (SCM_STACK_OVERFLOW_P): Rewrite to use
  scm_stack_limit instead of SCM_STACK_LIMIT.
  (scm_stack_limit, scm_sys_get_stack_depth,
  scm_calculate_stack_limit): New declarations.

* pre-inst-uguile.in: New file, just like pre-inst-guile.in, but to
  run the uncalibrated Guile instead of the calibrated one.
---
 am/pre-inst-guile      |    3 +
 configure.in           |   16 ++++++++
 libguile/Makefile.am   |   28 ++++++++++++-
 libguile/calibrate.scm |   32 +++++++++++++++
 libguile/debug.h       |    1 +
 libguile/stackchk.c    |   34 ++++++++++++++++
 libguile/stackchk.h    |   12 +++---
 pre-inst-uguile.in     |   99 ++++++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 216 insertions(+), 9 deletions(-)
 create mode 100644 libguile/calibrate.scm
 create mode 100644 pre-inst-uguile.in

diff --git a/am/pre-inst-guile b/am/pre-inst-guile
index c1a7407..35ba6c3 100644
--- a/am/pre-inst-guile
+++ b/am/pre-inst-guile
@@ -31,4 +31,7 @@
 preinstguile     = $(top_builddir_absolute)/pre-inst-guile
 preinstguiletool = GUILE="$(preinstguile)" $(top_srcdir)/scripts
 
+preinstuguile     = $(top_builddir_absolute)/pre-inst-uguile
+preinstuguiletool = GUILE="$(preinstuguile)" $(top_srcdir)/scripts
+
 ## am/pre-inst-guile ends here
diff --git a/configure.in b/configure.in
index 713e634..35118cf 100644
--- a/configure.in
+++ b/configure.in
@@ -1413,6 +1413,21 @@ if test "$cross_compiling" = "yes"; then
 fi
 AC_ARG_VAR(GUILE_FOR_BUILD,[guile for build system])
 AC_SUBST(GUILE_FOR_BUILD)
+
+if test "$cross_compiling" = "yes"; then
+  AC_MSG_CHECKING(uncalibrated guile for build)
+  UGUILE_FOR_BUILD="${UGUILE_FOR_BUILD-uguile}"
+else
+  UGUILE_FOR_BUILD='$(preinstuguile)'
+fi   
+
+## AC_MSG_CHECKING("if we are cross compiling")
+## AC_MSG_RESULT($cross_compiling)
+if test "$cross_compiling" = "yes"; then
+   AC_MSG_RESULT($UGUILE_FOR_BUILD)
+fi
+AC_ARG_VAR(UGUILE_FOR_BUILD,[uncalibrated guile for build system])
+AC_SUBST(UGUILE_FOR_BUILD)
   			
 ## If we're using GCC, ask for aggressive warnings.
 case "$GCC" in
@@ -1556,6 +1571,7 @@ AC_CONFIG_FILES([benchmark-guile], [chmod +x benchmark-guile])
 AC_CONFIG_FILES([guile-tools], [chmod +x guile-tools])
 AC_CONFIG_FILES([pre-inst-guile], [chmod +x pre-inst-guile])
 AC_CONFIG_FILES([pre-inst-guile-env], [chmod +x pre-inst-guile-env])
+AC_CONFIG_FILES([pre-inst-uguile], [chmod +x pre-inst-uguile])
 AC_CONFIG_FILES([libguile/guile-snarf],
                 [chmod +x libguile/guile-snarf])
 AC_CONFIG_FILES([libguile/guile-doc-snarf],
diff --git a/libguile/Makefile.am b/libguile/Makefile.am
index eb76237..4398e14 100644
--- a/libguile/Makefile.am
+++ b/libguile/Makefile.am
@@ -40,7 +40,8 @@ ETAGS_ARGS = --regex='/SCM_\(GLOBAL_\)?\(G?PROC\|G?PROC1\|SYMBOL\|VCELL\|CONST_L
 lib_LTLIBRARIES = libguile.la
 bin_PROGRAMS = guile
 
-noinst_PROGRAMS = guile_filter_doc_snarfage gen-scmconfig
+noinst_LTLIBRARIES = libuguile.la
+noinst_PROGRAMS = guile_filter_doc_snarfage gen-scmconfig uguile
 
 gen_scmconfig_SOURCES = gen-scmconfig.c
 
@@ -96,9 +97,14 @@ guile_CFLAGS = $(GUILE_CFLAGS)
 guile_LDADD = libguile.la
 guile_LDFLAGS = @DLPREOPEN@ $(GUILE_CFLAGS)
 
+uguile_SOURCES = guile.c
+uguile_CFLAGS = $(GUILE_CFLAGS)
+uguile_LDADD = libuguile.la
+uguile_LDFLAGS = @DLPREOPEN@ $(GUILE_CFLAGS)
+
 libguile_la_CFLAGS = $(GUILE_CFLAGS)
 
-libguile_la_SOURCES = alist.c arbiters.c async.c backtrace.c boolean.c	\
+LIBGUILE_SOURCES = alist.c arbiters.c async.c backtrace.c boolean.c	\
     chars.c continuations.c convert.c debug.c deprecation.c		\
     deprecated.c discouraged.c dynwind.c environments.c eq.c error.c	\
     eval.c evalext.c extensions.c feature.c fluids.c fports.c		\
@@ -110,11 +116,17 @@ libguile_la_SOURCES = alist.c arbiters.c async.c backtrace.c boolean.c	\
     modules.c numbers.c objects.c objprop.c options.c pairs.c ports.c	\
     print.c procprop.c procs.c properties.c random.c rdelim.c read.c	\
     root.c rw.c scmsigs.c script.c simpos.c smob.c sort.c srcprop.c	\
-    stackchk.c stacks.c stime.c strings.c srfi-4.c srfi-13.c srfi-14.c	\
+    stacks.c stime.c strings.c srfi-4.c srfi-13.c srfi-14.c	        \
     strorder.c strports.c struct.c symbols.c threads.c null-threads.c	\
     throw.c values.c variable.c vectors.c version.c vports.c weaks.c	\
     ramap.c unif.c
 
+libguile_la_SOURCES = stackchk-calibrated.c $(LIBGUILE_SOURCES)
+
+libuguile_la_CFLAGS = $(GUILE_CFLAGS)
+
+libuguile_la_SOURCES = stackchk.c $(LIBGUILE_SOURCES)
+
 DOT_X_FILES = alist.x arbiters.x async.x backtrace.x boolean.x chars.x	\
     continuations.x debug.x deprecation.x deprecated.x discouraged.x	\
     dynl.x dynwind.x environments.x eq.x error.x eval.x evalext.x	\
@@ -162,6 +174,8 @@ EXTRA_libguile_la_SOURCES = _scm.h		\
     debug-malloc.c mkstemp.c	\
     win32-uname.c win32-dirent.c win32-socket.c
 
+EXTRA_libuguile_la_SOURCES = $(EXTRA_libguile_la_SOURCES)
+
 ## delete guile-snarf.awk from the installation bindir, in case it's
 ## lingering there due to an earlier guile version not having been
 ## wiped out.
@@ -183,6 +197,10 @@ libguile_la_DEPENDENCIES = @LIBLOBJS@
 libguile_la_LIBADD = @LIBLOBJS@
 libguile_la_LDFLAGS = @LTLIBINTL@ -version-info @LIBGUILE_INTERFACE_CURRENT@:@LIBGUILE_INTERFACE_REVISION@:@LIBGUILE_INTERFACE_AGE@ -export-dynamic -no-undefined
 
+libuguile_la_DEPENDENCIES = $(libguile_la_DEPENDENCIES)
+libuguile_la_LIBADD = $(libguile_la_LIBADD)
+libuguile_la_LDFLAGS = $(libguile_la_LDFLAGS)
+
 # These are headers visible as <guile/mumble.h>
 pkginclude_HEADERS = gh.h
 
@@ -294,6 +312,10 @@ alldotdocfiles    = $(DOT_DOC_FILES) $(EXTRA_DOT_DOC_FILES)
 snarf2checkedtexi = GUILE="$(GUILE_FOR_BUILD)" $(top_srcdir)/scripts/snarf-check-and-output-texi
 dotdoc2texi       = cat $(alldotdocfiles) | $(snarf2checkedtexi)
 
+stackchk-calibrated.c: uguile$(EXEEXT)
+	$(UGUILE_FOR_BUILD) -s calibrate.scm > $@
+	cat stackchk.c >> $@
+
 guile.texi: $(alldotdocfiles) guile$(EXEEXT)
 	$(dotdoc2texi) --manual > $@ || { rm $@; false; }
 
diff --git a/libguile/calibrate.scm b/libguile/calibrate.scm
new file mode 100644
index 0000000..39abc7b
--- /dev/null
+++ b/libguile/calibrate.scm
@@ -0,0 +1,32 @@
+
+;;; Stack depth calibration, for the 'stack debug option.
+
+;; Make sure we don't overflow while performing this calibration!
+(debug-set! stack 0)
+
+;; Select the debugging evaluator.
+(debug-enable 'debug)
+
+;; Note that this loop must be non-tail-recursive!  170 and 690 are
+;; the values that we get for measured-depth1 and measured-depth2 when
+;; we run this code on a Debian GNU/Linux ia32 system - which we take
+;; as our canonical system.
+(let ((reference-depth1 170)
+      (reference-depth2 690)
+      (measured-depth1 (%get-stack-depth))
+      (measured-depth2 0))
+  (let loop ((count 10))
+    (if (zero? count)
+	(set! measured-depth2 (%get-stack-depth))
+	(cons count (loop (- count 1)))))
+  (format #t
+	  "
+#define GUILE_CALIBRATION_REFERENCE_DEPTH_1 ~a
+#define GUILE_CALIBRATION_REFERENCE_DEPTH_2 ~a
+#define GUILE_CALIBRATION_MEASURED_DEPTH_1 ~a
+#define GUILE_CALIBRATION_MEASURED_DEPTH_2 ~a
+"
+	  reference-depth1
+	  reference-depth2
+	  measured-depth1
+	  measured-depth2))
diff --git a/libguile/debug.h b/libguile/debug.h
index c292004..f6b1608 100644
--- a/libguile/debug.h
+++ b/libguile/debug.h
@@ -75,6 +75,7 @@ do {\
     && scm_is_true (SCM_EXIT_FRAME_HDLR);\
   scm_debug_mode_p = SCM_DEVAL_P\
     || scm_check_entry_p || scm_check_apply_p || scm_check_exit_p;\
+  scm_calculate_stack_limit ();\
 } while (0)
 
 /* {Evaluator}
diff --git a/libguile/stackchk.c b/libguile/stackchk.c
index 391ce21..f770822 100644
--- a/libguile/stackchk.c
+++ b/libguile/stackchk.c
@@ -33,6 +33,13 @@
 
 #ifdef STACK_CHECKING
 int scm_stack_checking_enabled_p;
+int scm_stack_limit;
+
+/* As in y = mx + c.  These numbers define a linear transformation
+   from the stack depth specified as the 'stack debug option, to the
+   actual max stack depth that we allow. */
+static double calibrated_m = 1;
+static double calibrated_c = 0;
 
 SCM_SYMBOL (scm_stack_overflow_key, "stack-overflow");
 
@@ -47,6 +54,24 @@ scm_report_stack_overflow ()
 	     SCM_BOOL_F);
 }
 
+/* Stack depth calibration. */
+
+SCM_DEFINE (scm_sys_get_stack_depth, "%get-stack-depth", 0, 0, 0,
+	    (),
+	    "Return current stack depth.")
+#define FUNC_NAME s_scm_sys_get_stack_depth
+{
+  SCM_STACKITEM stack;
+  return scm_from_int (SCM_STACK_DEPTH (&stack));
+}
+#undef FUNC_NAME
+
+void
+scm_calculate_stack_limit ()
+{
+  scm_stack_limit = (int) (calibrated_m * SCM_STACK_LIMIT + calibrated_c);
+}
+
 #endif
 
 long
@@ -81,6 +106,15 @@ scm_stack_report ()
 void
 scm_init_stackchk ()
 {
+#ifdef GUILE_CALIBRATION_MEASURED_DEPTH_1
+  /* Calculate calibrated stack depth limit. */
+  calibrated_m = ((double) (GUILE_CALIBRATION_MEASURED_DEPTH_2 - GUILE_CALIBRATION_MEASURED_DEPTH_1))
+    / (GUILE_CALIBRATION_REFERENCE_DEPTH_2 - GUILE_CALIBRATION_REFERENCE_DEPTH_1);
+  calibrated_c = ((double) GUILE_CALIBRATION_MEASURED_DEPTH_2)
+    - calibrated_m * GUILE_CALIBRATION_REFERENCE_DEPTH_2;
+#endif
+  scm_calculate_stack_limit ();
+
 #include "libguile/stackchk.x"
 }
 
diff --git a/libguile/stackchk.h b/libguile/stackchk.h
index 9a5c59f..d14e959 100644
--- a/libguile/stackchk.h
+++ b/libguile/stackchk.h
@@ -35,14 +35,11 @@
 
 #ifdef STACK_CHECKING
 # if SCM_STACK_GROWS_UP
-#  define SCM_STACK_OVERFLOW_P(s)\
-   (SCM_STACK_PTR (s) \
-    > (SCM_I_CURRENT_THREAD->base + SCM_STACK_LIMIT))
+#  define SCM_STACK_DEPTH(s) (SCM_STACK_PTR (s) - SCM_I_CURRENT_THREAD->base)
 # else
-#  define SCM_STACK_OVERFLOW_P(s)\
-   (SCM_STACK_PTR (s) \
-    < (SCM_I_CURRENT_THREAD->base - SCM_STACK_LIMIT))
+#  define SCM_STACK_DEPTH(s) (SCM_I_CURRENT_THREAD->base - SCM_STACK_PTR (s))
 # endif
+# define SCM_STACK_OVERFLOW_P(s) (SCM_STACK_DEPTH (s) > scm_stack_limit)
 # define SCM_CHECK_STACK\
     {\
        SCM_STACKITEM stack;\
@@ -54,10 +51,13 @@
 #endif /* STACK_CHECKING */
 
 SCM_API int scm_stack_checking_enabled_p;
+SCM_API int scm_stack_limit;
 
 \f
 
 SCM_API void scm_report_stack_overflow (void);
+SCM_API SCM scm_sys_get_stack_depth (void);
+SCM_API void scm_calculate_stack_limit (void);
 SCM_API long scm_stack_size (SCM_STACKITEM *start);
 SCM_API void scm_stack_report (void);
 SCM_API void scm_init_stackchk (void);
diff --git a/pre-inst-uguile.in b/pre-inst-uguile.in
new file mode 100644
index 0000000..fc8ffc3
--- /dev/null
+++ b/pre-inst-uguile.in
@@ -0,0 +1,99 @@
+#!/bin/sh
+
+#	Copyright (C) 2002, 2006, 2008 Free Software Foundation
+#
+#   This file is part of GUILE.
+#
+#   GUILE is free software; you can redistribute it and/or modify
+#   it under the terms of the GNU General Public License as
+#   published by the Free Software Foundation; either version 2, or
+#   (at your option) any later version.
+#
+#   GUILE 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 General Public License for more details.
+#
+#   You should have received a copy of the GNU General Public
+#   License along with GUILE; see the file COPYING.  If not, write
+#   to the Free Software Foundation, Inc., 51 Franklin Street, Fifth
+#   Floor, Boston, MA 02110-1301 USA
+
+# NOTE: at some point we might consider invoking this under
+# pre-inst-guile-env.  If this will work, then most of the code below
+# can be removed.
+
+# NOTE: If you update this file, please update pre-inst-guile-env.in
+# as well, if appropriate.
+
+# Commentary:
+
+# Usage: pre-inst-uguile [ARGS]
+#
+# This script arranges for the environment to support, and eventaully execs,
+# the uninstalled binary uncalibrated guile executable located under libguile/,
+# passing ARGS to it.  In the process, env var GUILE is clobbered, and the
+# following env vars are modified (but not clobbered):
+#   GUILE_LOAD_PATH
+#   LTDL_LIBRARY_PATH
+#
+# This script can be used as a drop-in replacement for $bindir/guile;
+# if there is a discrepency in behavior, that's a bug.
+
+# Code:
+
+# config
+subdirs_with_ltlibs="srfi guile-readline"       # maintain me
+
+# env (set by configure)
+top_srcdir="@top_srcdir_absolute@"
+top_builddir="@top_builddir_absolute@"
+
+[ x"$top_srcdir"   = x -o ! -d "$top_srcdir" -o \
+  x"$top_builddir" = x -o ! -d "$top_builddir" ] && {
+    echo $0: bad environment
+    echo top_srcdir=$top_srcdir
+    echo top_builddir=$top_builddir
+    exit 1
+}
+
+# handle GUILE_LOAD_PATH (no clobber)
+if [ x"$GUILE_LOAD_PATH" = x ]
+then
+    GUILE_LOAD_PATH="${top_srcdir}/guile-readline:${top_srcdir}"
+else
+  for d in "${top_srcdir}" "${top_srcdir}/guile-readline"
+  do
+    # This hair prevents double inclusion.
+    # The ":" prevents prefix aliasing.
+    case x"$GUILE_LOAD_PATH" in
+      x*${d}:*) ;;
+      *) GUILE_LOAD_PATH="${d}:$GUILE_LOAD_PATH" ;;
+    esac
+  done
+fi
+export GUILE_LOAD_PATH
+
+# handle LTDL_LIBRARY_PATH (no clobber)
+ltdl_prefix=""
+dyld_prefix=""
+for dir in $subdirs_with_ltlibs ; do
+    ltdl_prefix="${top_builddir}/${dir}:${ltdl_prefix}"
+    dyld_prefix="${top_builddir}/${dir}/.libs:${dyld_prefix}"
+done
+LTDL_LIBRARY_PATH="${ltdl_prefix}$LTDL_LIBRARY_PATH"
+export LTDL_LIBRARY_PATH
+DYLD_LIBRARY_PATH="${dyld_prefix}${top_builddir}/libguile/.libs:$DYLD_LIBRARY_PATH"
+export DYLD_LIBRARY_PATH
+
+# set GUILE (clobber)
+GUILE=${top_builddir}/libguile/uguile
+export GUILE
+
+# do it
+exec $GUILE "$@"
+
+# never reached
+exit 1
+
+# pre-inst-uguile ends here
-- 
1.5.6.5

Re: Stack calibration

[Ludovic Courtès]

Hello,

"Neil Jerram" <neiljerram@googlemail.com> writes:

>>> +  SCM_VALIDATE_INT_COPY (1, d1, y1);
>>> +  SCM_VALIDATE_INT_COPY (2, d2, y2);
>>> +
>>> +  calibrated_m = ((double) (y2 - y1)) / (x2 - x1);
>>> +  calibrated_c = ((double) y2) - calibrated_m * x2;
>>
>> Shouldn't it be:
>>
>>  calibrated_c = y1 - x1;
>
> No, don't think so!  My model equation is y = mx + c, so c = y - mx.

Hmm, OK.

>> Could it be moved to a `%print-stack-calibration' function that we'd use
>> for troubleshooting?
>
> Yes.  In the attached patch, I've left this out completely, but I
> think we should add it back in as %get-stack-calibration.

Yes.

> I've done this part a bit differently - see the libguile/Makefile.am
> changes - because I couldn't see exactly how the recursive make
> approach would work.  If you think recursive make would be
> significantly better, can you describe or propose the detailed changes
> that would be needed?

Your proposition looks very good actually.  I suppose the generated
makefile doesn't require recompilation of all `.lo' files to go from
`libuguile' to `libguile', right?

I'm not sure about cross-compilation (Dale Smith had also raised this
issue on IRC some time ago).  IIUC, the user is expected to provide a
`UGUILE_FOR_BUILD' at configure-time, which is then used to run
`calibrate.scm'; however, `UGUILE_FOR_BUILD' runs on the host, not the
target system, so the generated file will be erroneous, right?

Thus, when cross-compiling, shouldn't we avoid stack calibration
altogether and simply emit a warning a configure-time, for instance?

At any rate, it's not a problem when cross-compiling with tools like
Scratchbox, which actually "hide" the fact that we're cross-compiling
and can run executables for the target system through an emulator.

Thanks for your work!

Ludo'.

Re: Stack calibration

[Neil Jerram]

2008/9/28 Ludovic Courtès <ludo@gnu.org>:
>
> "Neil Jerram" <neiljerram@googlemail.com> writes:
>
>> I've done this part a bit differently - see the libguile/Makefile.am
>> changes - because I couldn't see exactly how the recursive make
>> approach would work.  If you think recursive make would be
>> significantly better, can you describe or propose the detailed changes
>> that would be needed?
>
> Your proposition looks very good actually.  I suppose the generated
> makefile doesn't require recompilation of all `.lo' files to go from
> `libuguile' to `libguile', right?

If you mean does it actually compile them all again?: yes, I'm afraid
it does.  I think this is because the generated makefile thinks that
libuguile_la-eval.lo and libguile_la-eval.lo are separate objects.

If you mean does it need to?: no, it doesn't, because none of the
files apart from stackchk.c/stackchk-calibrated.c have actually
changed at all.

I currently don't know of a good solution for this.

It might work to define:

(i) a convenience library consisting of everything in libguile except
for stackchk.c/stackchk-calibrated.c

(ii) libuguile.la, consisting of the convenience library + stackchk.c

(iii) libguile.la, consisting of the convenience library +
stackchk-calibrated.c.

But I would be surprised if that didn't cause a regression on some
less mainstream platforms.

Do you have any suggestions?

> I'm not sure about cross-compilation (Dale Smith had also raised this
> issue on IRC some time ago).  IIUC, the user is expected to provide a
> `UGUILE_FOR_BUILD' at configure-time, which is then used to run
> `calibrate.scm'; however, `UGUILE_FOR_BUILD' runs on the host, not the
> target system, so the generated file will be erroneous, right?

Probably, yes.

> Thus, when cross-compiling, shouldn't we avoid stack calibration
> altogether and simply emit a warning a configure-time, for instance?

Well, ideally we should have a solution that works automatically in
all circumstances...

> At any rate, it's not a problem when cross-compiling with tools like
> Scratchbox, which actually "hide" the fact that we're cross-compiling
> and can run executables for the target system through an emulator.

Agreed.

Regards,
        Neil

Re: Stack calibration

[Andy Wingo]

Heya,

Neil, I'd love to try your patches, can you push to a branch?

Andy

Re: Stack calibration

[Neil Jerram]

2008/10/2 Andy Wingo <wingo@pobox.com>:
> Heya,
>
> Neil, I'd love to try your patches, can you push to a branch?
>
> Andy
>

Sure, but I'm not yet familiar with how to do that.  I already have a
local "stack-calibration" branch; if you already know the incantation
for just pushing that to savannah, please let me know.

      Neil

Re: Stack calibration

[Neil Jerram]

2008/9/30 Neil Jerram <neiljerram@googlemail.com>:
>
> Well, ideally we should have a solution that works automatically in
> all circumstances...

FWIW, I'm actually thinking now that this stack calibration stuff is
becoming way too tricky, in at least two ways.

1) The concept of the 'stack debug option being expressed in terms of
some other "canonical" combination of OS, compiler and compiler
optimization level.  (I struggle to describe this clearly, here, and
in comments in the code, and I'm sure I would struggle in the manual
too - so that's a bad sign!)

2) The complexity that my latest patch adds to the Guile build
process.  And even with this complexity we still don't cover all the
cases (notably cross compiling).

Reviewing the history, we find that this work was prompted by people
reporting Stack overflow errors when building Guile or when running
make check [1].  We also find that such errors were _still_ reported
by people who had some version of the calibration patch in place [2].
And there was a case, not yet understood, where the error was
apparently caused by configuring without threads [3].

There was also a general discussion of how stack checking is
implemented [4], which I think is sufficient to say that we should
keep it basically in its current form (rather than implement an eval
frame counter, say).

[1] http://www.mail-archive.com/bug-guile@gnu.org/msg04401.html
[2] http://article.gmane.org/gmane.lisp.guile.bugs/3881
[3] http://thread.gmane.org/gmane.lisp.guile.user/6628/focus=6629
[4] http://www.nabble.com/Re:-stack-overflow-td15467458.html

Taking everything together, my thinking now is...

- The problem we actually need to solve is getting a stack overflow
while running make and/or make check, and there may be other ways of
doing that than trying to pick the right number, and to interpret that
number such that it has the same effect on all platforms.

(For example, I think it would be fine if stack checking was disabled
during make and make check, and (post build/install) when loading
boot-9.scm.  We don't need to guard against C stack overflow when
running our own test suite, or when loading boot-9.scm, because
overflow in those contexts isn't going to hurt anyone.  The point of
stack checking is to protect against runaway user/developer code (or
Guile code being called from user/developer code).)

- Based on [2], it sounds like there is part of this issue that we
don't yet understand - i.e. not just the stack growing bigger than the
default 20000 words.

I plan to try and investigate [2] and [3] more, then hopefully propose
something (simpler than my latest patch!).

Regards,
        Neil

Re: Stack calibration

[Ludovic Courtès]

Hello,

Sorry for the latency...

"Neil Jerram" <neiljerram@googlemail.com> writes:

> FWIW, I'm actually thinking now that this stack calibration stuff is
> becoming way too tricky, in at least two ways.
>
> 1) The concept of the 'stack debug option being expressed in terms of
> some other "canonical" combination of OS, compiler and compiler
> optimization level.  (I struggle to describe this clearly, here, and
> in comments in the code, and I'm sure I would struggle in the manual
> too - so that's a bad sign!)

That doesn't strike me as a bad idea.  "Portable Scheme stack
measurement unit" would be a good description, wouldn't it?

> 2) The complexity that my latest patch adds to the Guile build
> process.  And even with this complexity we still don't cover all the
> cases (notably cross compiling).

Yeah, that's the main issue.  Building all of libguile twice isn't
acceptable IMO.

> Taking everything together, my thinking now is...
>
> - The problem we actually need to solve is getting a stack overflow
> while running make and/or make check, and there may be other ways of
> doing that than trying to pick the right number, and to interpret that
> number such that it has the same effect on all platforms.

You mean "is *not* getting a stack overflow", right?

From that point of view, adding the right `eval-set!' incantation
somewhere in the test suite would suffice to fix the problem.

> - Based on [2], it sounds like there is part of this issue that we
> don't yet understand - i.e. not just the stack growing bigger than the
> default 20000 words.

What makes you say so?  The message just shows that the test suite
sometimes triggers a stack overflow.

Thanks,
Ludo'.

Re: Stack calibration

[Neil Jerram]

2008/10/6 Ludovic Courtès <ludo@gnu.org>:
> Hello,
>
> Sorry for the latency...

Hi, no problem!

> "Neil Jerram" <neiljerram@googlemail.com> writes:
>
>> - The problem we actually need to solve is getting a stack overflow
>> while running make and/or make check, and there may be other ways of
>> doing that than trying to pick the right number, and to interpret that
>> number such that it has the same effect on all platforms.
>
> You mean "is *not* getting a stack overflow", right?

Well, the problem is getting a stack overflow; the objective is *not*
to get one. :-)

> >From that point of view, adding the right `eval-set!' incantation
> somewhere in the test suite would suffice to fix the problem.

Yes - except that it's (debug-set! stack ...).

>> - Based on [2], it sounds like there is part of this issue that we
>> don't yet understand - i.e. not just the stack growing bigger than the
>> default 20000 words.
>
> What makes you say so?  The message just shows that the test suite
> sometimes triggers a stack overflow.

Yes, but the point is that in that case, the calibration patch makes
things worse!  (And I've now followed up in more detail in that
thread.)

> Thanks,
> Ludo'.

Regards,
       Neil

Re: Stack calibration

[Neil Jerram]

[-- Attachment #1: Type: text/plain, Size: 840 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Stack-calibration-mark-2.patch --]
[-- Type: text/x-patch; name=0001-Stack-calibration-mark-2.patch, Size: 13038 bytes --]

From ed168b4000df30e8ae205562838e2358c33f7c8d Mon Sep 17 00:00:00 2001
From: Neil Jerram <neil@ossau.uklinux.net>
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"
 \f
@@ -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

Re: Stack calibration

[Greg Troxel]

[-- Attachment #1: Type: text/plain, Size: 244 bytes --]


I am not really following, but does this make it harder to cross-compile
guile than it is now?

(I am working on a project cross-compiling code for arm11 linux; we
aren't using guile at the moment but I am now more sensitive to these
issues.)

[-- Attachment #2: Type: application/pgp-signature, Size: 193 bytes --]

Re: Stack calibration

[Neil Jerram]

On 10/10/2008, Greg Troxel <gdt@ir.bbn.com> wrote:
> I am not really following, but does this make it harder to cross-compile
>  guile than it is now?

I don't think so.  In the Guile build, 'make' already executes the
built guile in order to generate the online help
(guile-procedures.txt).  With this patch, 'make' will run guile again
to generate stack-limit-calibration.scm, which is then used by 'make
check'.

If running the built guile is a problem in your project, you
presumably cannot do 'make check'.  In that case, you could suppress
the generation of stack-limit-calibration.scm by removing it from
BUILT_SOURCES in libguile/Makefile.am.  Perhaps there is an
incantation for doing that automatically when cross-compiling? - patch
welcome!

Does that sound OK?  Let me know what you think.

   Neil

Re: Stack calibration

[Greg Troxel]

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]


"Neil Jerram" <neiljerram@googlemail.com> writes:

> On 10/10/2008, Greg Troxel <gdt@ir.bbn.com> wrote:
>> I am not really following, but does this make it harder to cross-compile
>>  guile than it is now?
>
> I don't think so.  In the Guile build, 'make' already executes the
> built guile in order to generate the online help
> (guile-procedures.txt).

That seems fixable by expecting native guile to be present when cross
building.

> With this patch, 'make' will run guile again to generate
> stack-limit-calibration.scm, which is then used by 'make check'.
>
> If running the built guile is a problem in your project, you
> presumably cannot do 'make check'.

Cross builds can't run what is built, but I think in general one has to
for make check, so it's just that cross builds can never do make check.
So therefore things are no worse, and that was all I was asking.

> In that case, you could suppress the generation of
> stack-limit-calibration.scm by removing it from BUILT_SOURCES in
> libguile/Makefile.am.  Perhaps there is an incantation for doing that
> automatically when cross-compiling? - patch welcome!

It would be nice to make that get generated only by make check, and not
the regular build, but I am not using guile on the project and probably
won't have enough Copious Spare Time to do this.

[-- Attachment #2: Type: application/pgp-signature, Size: 193 bytes --]

Re: Stack calibration

[Neil Jerram]

On 10/10/2008, Greg Troxel <gdt@ir.bbn.com> wrote:

> It would be nice to make that get generated only by make check, and not
>  the regular build,

Good idea, I'll look at doing that.

Re: Stack calibration

[Ludovic Courtès]

Hi Neil,

"Neil Jerram" <neiljerram@googlemail.com> writes:

> - 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!

Great!

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

The approach looks good to me.  It's just annoying that
`SCM_CHECK_STACK' (adding a slight overhead) and "threads.h" have to be
modified.

Instead of storing the high water mark in threads, could we have
`%get-stack-depth' and call it from somewhere deep in the code (similar
to what was in your previous patch)?

In that case, instead of using a void port for the REPL's output, we
could for instance use a soft port and measure the stack depth from
there:

  (let ((before (%get-stack-depth))
        (after  #f)
        (port   (make-soft-port (vector (lambda (chr)
                                          (set! after (%get-stack-depth)))
                                        (lambda (str)
                                          (set! after (%get-stack-depth)))
                                        (lambda ()
                                          (set! after (%get-stack-depth)))
                                        #f #f #f))))
    (with-output-to-port port
      (lambda ()
        (with-input-from-string "\n" top-repl)))
    (abs (- after before)))

>  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

As Greg suggested, this could be in `check_DATA' or something like that.

> +;; 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)

I'd tend to use the actual GNU triplet, like `i386-pc-linux-gnu'.

Thanks for working on it!

Ludo'.

Re: Stack calibration

[Neil Jerram]

Hi Ludovic,

2008/10/11 Ludovic Courtès <ludo@gnu.org>:
>
> The approach looks good to me.  It's just annoying that
> `SCM_CHECK_STACK' (adding a slight overhead) and "threads.h" have to be
> modified.
>
> Instead of storing the high water mark in threads, could we have
> `%get-stack-depth' and call it from somewhere deep in the code (similar
> to what was in your previous patch)?

Here's how my thought process went, in response to this.  I only
record because I think it's ultimately quite amusing (at my own
expense!)...

<imagine cinematic thinking effect>
Goodness, that Ludovic, can't he ever just be happy with what I've proposed...

...he does have a point though, there could be a performance impact of
the high water mark tracking... (and also, FWIW, I'm a bit worried
about the number of times we call pthread_getspecific() in mainline
code - is that efficient?)

...perhaps the high water marking tracking could be global, instead of
per thread; that would avoid adding a field to the thread structure,
and would still solve the main problem...

...oh, but that would require a mutex [which is vastly more inefficient]...

...well perhaps we could have an optional callout to Scheme, and do
the tracking in Scheme; then in the mainline case we'd still only be
checking one variable for being non-null, and the detail of whether
it's per thread, or if we need a mutex, can be delegated to the Scheme
code...

...oh but hang on, we already have arbitrary callouts to Scheme: the
evaluator traps!
</imagine cinematic thinking effect>

So yes, I think we can actually do this without any change to the C
code except adding %get-stack-depth, by using an evaluator trap.

> In that case, instead of using a void port for the REPL's output, we
> could for instance use a soft port and measure the stack depth from
> there:
>
>  (let ((before (%get-stack-depth))
>        (after  #f)
>        (port   (make-soft-port (vector (lambda (chr)
>                                          (set! after (%get-stack-depth)))
>                                        (lambda (str)
>                                          (set! after (%get-stack-depth)))
>                                        (lambda ()
>                                          (set! after (%get-stack-depth)))
>                                        #f #f #f))))
>    (with-output-to-port port
>      (lambda ()
>        (with-input-from-string "\n" top-repl)))
>    (abs (- after before)))

Cunning idea, but I'm pretty sure it wouldn't be effective, because
the reports from people who have seen Stack overflow during 'make
check' show that it occurs in a chain of module using: (top-repl) -
(use-modules (ice-9 debug)) - ... - before the REPL reaches the point
of printing anything.

Evaluator traps, on the other hand, will catch everything.  (At the
cost of running very slowly, but I think that's OK for just this one
calibration step.)

>>  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
>
> As Greg suggested, this could be in `check_DATA' or something like that.

Yes, but thanks for the more specific hint!

>> +;; 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)
>
> I'd tend to use the actual GNU triplet, like `i386-pc-linux-gnu'.

OK, will do.

> Thanks for working on it!

Thanks for not being happy!

    Neil

Re: Stack calibration

[Neil Jerram]

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

2008/10/12 Neil Jerram <neiljerram@googlemail.com>:
>
> So yes, I think we can actually do this without any change to the C
> code except adding %get-stack-depth, by using an evaluator trap.
>>
>> As Greg suggested, this could be in `check_DATA' or something like that.
>
> Yes, but thanks for the more specific hint!
>
>> I'd tend to use the actual GNU triplet, like `i386-pc-linux-gnu'.
>
> OK, will do.

Here's the new patch.  Please (as ever!) let me know what you think.

        Neil

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Stack-calibration-mark-3.patch --]
[-- Type: text/x-patch; name=0001-Stack-calibration-mark-3.patch, Size: 15380 bytes --]

From ff700bf19cf787a34a7a4f5c16b0403c4221e547 Mon Sep 17 00:00:00 2001
From: Neil Jerram <neil@ossau.uklinux.net>
Date: Sun, 12 Oct 2008 22:11:01 +0100
Subject: [PATCH] Stack calibration mark 3

---
 .gitignore                             |    1 +
 check-guile.in                         |    1 +
 configure.in                           |    2 +
 libguile/Makefile.am                   |   23 ++++++
 libguile/measure-hwm.scm               |  136 ++++++++++++++++++++++++++++++++
 libguile/stackchk.c                    |   12 +++
 libguile/stackchk.h                    |    1 +
 test-suite/standalone/test-use-srfi    |   67 ----------------
 test-suite/standalone/test-use-srfi.in |   67 ++++++++++++++++
 9 files changed, 243 insertions(+), 67 deletions(-)
 create mode 100644 libguile/measure-hwm.scm
 delete mode 100755 test-suite/standalone/test-use-srfi
 create mode 100755 test-suite/standalone/test-use-srfi.in

diff --git a/.gitignore b/.gitignore
index a122176..39c4b49 100644
--- a/.gitignore
+++ b/.gitignore
@@ -70,3 +70,4 @@ guile-readline/guile-readline-config.h
 guile-readline/guile-readline-config.h.in
 TAGS
 guile-1.8.pc
+stack-limit-calibration.scm
diff --git a/check-guile.in b/check-guile.in
index f66bf13..9ee2ea3 100644
--- a/check-guile.in
+++ b/check-guile.in
@@ -41,6 +41,7 @@ if [ ! -f guile-procedures.txt ] ; then
 fi
 
 exec $guile \
+    -l ${top_builddir}/libguile/stack-limit-calibration.scm \
     -e main -s "$TEST_SUITE_DIR/guile-test" \
     --test-suite "$TEST_SUITE_DIR/tests" \
     --log-file check-guile.log "$@"
diff --git a/configure.in b/configure.in
index 713e634..e77f460 100644
--- a/configure.in
+++ b/configure.in
@@ -1564,6 +1564,8 @@ AC_CONFIG_FILES([libguile/guile-func-name-check],
                 [chmod +x libguile/guile-func-name-check])
 AC_CONFIG_FILES([libguile/guile-snarf-docs],
                 [chmod +x libguile/guile-snarf-docs])
+AC_CONFIG_FILES([test-suite/standalone/test-use-srfi],
+                [chmod +x test-suite/standalone/test-use-srfi])
 
 AC_OUTPUT
 
diff --git a/libguile/Makefile.am b/libguile/Makefile.am
index eb76237..d44c5ca 100644
--- a/libguile/Makefile.am
+++ b/libguile/Makefile.am
@@ -313,6 +313,29 @@ guile-procedures.txt: guile-procedures.texi
 
 endif
 
+# Stack limit calibration for `make check'.  (For why we do this, see
+# the comments in measure-hwm.scm.)  We're relying here on a couple of
+# bits of Automake magic.
+#
+# 1. The fact that "libguile" comes before "test-suite" in SUBDIRS in
+# our toplevel Makefile.am.  This ensures that the
+# stack-limit-calibration.scm "test" will be run before any of the
+# tests under test-suite.
+#
+# 2. The fact that each test is invoked as $TESTS_ENVIRONMENT $test.
+# This allows us to ensure that the test will be considered to have
+# passed, by using `true' as TESTS_ENVIRONMENT.
+#
+# Why don't we care about the test "actually passing"?  Because the
+# important thing about stack-limit-calibration.scm is just that it is
+# generated in the first place, so that other tests under test-suite
+# can use it.
+TESTS = stack-limit-calibration.scm
+TESTS_ENVIRONMENT = true
+
+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/measure-hwm.scm b/libguile/measure-hwm.scm
new file mode 100644
index 0000000..364740b
--- /dev/null
+++ b/libguile/measure-hwm.scm
@@ -0,0 +1,136 @@
+;;;; 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)
+
+;; Define a variable to hold the measured stack high water mark (HWM).
+(define top-repl-hwm-measured 0)
+
+;; Install an evaluator trap to measure the stack size at every
+;; evaluation step, and increase top-repl-hwm-measured if it is less
+;; than the measured stack size.
+(use-modules (ice-9 debugging traps)
+	     (oop goops))
+(install-trap (make <entry-trap>
+		#:behaviour (lambda (trap-context)
+			      (let ((stack-size (%get-stack-size)))
+				(if (< top-repl-hwm-measured stack-size)
+				    (set! top-repl-hwm-measured stack-size))))))
+
+;; 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)))
+
+;; top-repl-hwm-measured now contains the stack HWM that resulted from
+;; running that code.
+
+;; 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-i686-pc-linux-gnu 9685)
+
+;; 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 i686 PC GNU/Linux system, after configuring with
+       ;; `./configure --enable-maintainer-mode --with-threads'.
+       ;; (Hereafter referred to as the `canonical' build platform.)
+       (top-repl-hwm-i686-pc-linux-gnu ~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-i686-pc-linux-gnu
+       ;; 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-i686-pc-linux-gnu
+	default-stack-limit
+	;; Use quotient here to get an integer result, rather than a
+	;; rational.
+	(quotient (* default-stack-limit top-repl-hwm-measured)
+		  top-repl-hwm-i686-pc-linux-gnu))
diff --git a/libguile/stackchk.c b/libguile/stackchk.c
index 391ce21..10a5e3f 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"
 \f
@@ -78,6 +79,17 @@ scm_stack_report ()
   scm_puts ("\n", port);
 }
 
+
+SCM_DEFINE (scm_sys_get_stack_size, "%get-stack-size", 0, 0, 0,
+	    (),
+	    "Return the current thread's C stack size (in units of sizeof (SCM_STACKITEM *)).")
+#define FUNC_NAME s_scm_sys_get_stack_size
+{
+  return scm_from_long (scm_stack_size (SCM_I_CURRENT_THREAD->base));
+}
+#undef FUNC_NAME
+
+
 void
 scm_init_stackchk ()
 {
diff --git a/libguile/stackchk.h b/libguile/stackchk.h
index 9a5c59f..c094e03 100644
--- a/libguile/stackchk.h
+++ b/libguile/stackchk.h
@@ -60,6 +60,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_stack_size (void);
 SCM_API void scm_init_stackchk (void);
 
 #endif  /* SCM_STACKCHK_H */
diff --git a/test-suite/standalone/test-use-srfi b/test-suite/standalone/test-use-srfi
deleted file mode 100755
index 7186b5a..0000000
--- a/test-suite/standalone/test-use-srfi
+++ /dev/null
@@ -1,67 +0,0 @@
-#!/bin/sh
-
-# Copyright (C) 2006 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
-
-
-# Test that two srfi numbers on the command line work.
-#
-guile -q --use-srfi=1,10 >/dev/null <<EOF
-(if (and (defined? 'partition)
-         (defined? 'define-reader-ctor))
-    (exit 0)   ;; good
-    (exit 1))  ;; bad
-EOF
-if test $? = 0; then :; else
-  echo "guile --use-srfi=1,10 fails to run"
-  exit 1
-fi
-
-
-# Test that running "guile --use-srfi=1" leaves the interactive REPL with
-# the srfi-1 version of iota.
-#
-# In guile 1.8.1 and earlier, and 1.6.8 and earlier, these failed because in
-# `top-repl' the core bindings got ahead of anything --use-srfi gave.
-#
-
-guile -q --use-srfi=1 >/dev/null <<EOF
-(catch #t
-  (lambda ()
-    (iota 2 3 4))
-  (lambda args
-    (exit 1))) ;; bad
-(exit 0)       ;; good
-EOF
-if test $? = 0; then :; else
-  echo "guile --use-srfi=1 doesn't give SRFI-1 iota"
-  exit 1
-fi
-
-
-# Similar test on srfi-17 car, which differs in being a #:replacement.  This
-# exercises duplicates handling in `top-repl' versus `use-srfis' (in
-# boot-9.scm).
-#
-guile -q --use-srfi=17 >/dev/null <<EOF
-(if (procedure-with-setter? car)
-    (exit 0)   ;; good
-    (exit 1))  ;; bad
-EOF
-if test $? = 0; then :; else
-  echo "guile --use-srfi=17 doesn't give SRFI-17 car"
-  exit 1
-fi
diff --git a/test-suite/standalone/test-use-srfi.in b/test-suite/standalone/test-use-srfi.in
new file mode 100755
index 0000000..57f84af
--- /dev/null
+++ b/test-suite/standalone/test-use-srfi.in
@@ -0,0 +1,67 @@
+#!/bin/sh
+
+# Copyright (C) 2006 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
+
+
+# Test that two srfi numbers on the command line work.
+#
+guile -q -l @top_builddir_absolute@/libguile/stack-limit-calibration.scm --use-srfi=1,10 >/dev/null <<EOF
+(if (and (defined? 'partition)
+         (defined? 'define-reader-ctor))
+    (exit 0)   ;; good
+    (exit 1))  ;; bad
+EOF
+if test $? = 0; then :; else
+  echo "guile --use-srfi=1,10 fails to run"
+  exit 1
+fi
+
+
+# Test that running "guile --use-srfi=1" leaves the interactive REPL with
+# the srfi-1 version of iota.
+#
+# In guile 1.8.1 and earlier, and 1.6.8 and earlier, these failed because in
+# `top-repl' the core bindings got ahead of anything --use-srfi gave.
+#
+
+guile -q -l @top_builddir_absolute@/libguile/stack-limit-calibration.scm --use-srfi=1 >/dev/null <<EOF
+(catch #t
+  (lambda ()
+    (iota 2 3 4))
+  (lambda args
+    (exit 1))) ;; bad
+(exit 0)       ;; good
+EOF
+if test $? = 0; then :; else
+  echo "guile --use-srfi=1 doesn't give SRFI-1 iota"
+  exit 1
+fi
+
+
+# Similar test on srfi-17 car, which differs in being a #:replacement.  This
+# exercises duplicates handling in `top-repl' versus `use-srfis' (in
+# boot-9.scm).
+#
+guile -q -l @top_builddir_absolute@/libguile/stack-limit-calibration.scm --use-srfi=17 >/dev/null <<EOF
+(if (procedure-with-setter? car)
+    (exit 0)   ;; good
+    (exit 1))  ;; bad
+EOF
+if test $? = 0; then :; else
+  echo "guile --use-srfi=17 doesn't give SRFI-17 car"
+  exit 1
+fi
-- 
1.5.6.5

Re: Stack calibration

[Neil Jerram]

2008/10/12 Neil Jerram <neiljerram@googlemail.com>:
>
> Here's the new patch.  Please (as ever!) let me know what you think.

One update to this below.  It isn't actually necessary or helpful, for
this case, to pull in the GOOPS interface to evaluator traps.

    Neil

diff --git a/libguile/measure-hwm.scm b/libguile/measure-hwm.scm
index 364740b..53a30d5 100644
--- a/libguile/measure-hwm.scm
+++ b/libguile/measure-hwm.scm
@@ -35,16 +35,16 @@
 ;; Define a variable to hold the measured stack high water mark (HWM).
 (define top-repl-hwm-measured 0)

-;; Install an evaluator trap to measure the stack size at every
+;; Use an evaluator trap to measure the stack size at every
 ;; evaluation step, and increase top-repl-hwm-measured if it is less
 ;; than the measured stack size.
-(use-modules (ice-9 debugging traps)
-            (oop goops))
-(install-trap (make <entry-trap>
-               #:behaviour (lambda (trap-context)
-                             (let ((stack-size (%get-stack-size)))
-                               (if (< top-repl-hwm-measured stack-size)
-                                   (set! top-repl-hwm-measured stack-size))))))
+(trap-set! enter-frame-handler
+          (lambda _
+            (let ((stack-size (%get-stack-size)))
+              (if (< top-repl-hwm-measured stack-size)
+                  (set! top-repl-hwm-measured stack-size)))))
+(trap-enable 'enter-frame)
+(trap-enable 'traps)

 ;; Call (turn-on-debugging) and (top-repl) in order to simulate as
 ;; closely as possible what happens - and in particular, how much
@@ -74,7 +74,7 @@

 ;; 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-i686-pc-linux-gnu 9685)
+(define top-repl-hwm-i686-pc-linux-gnu 9461)

 ;; Using the above results, output code that tests can run in order to
 ;; configure the stack limit correctly for the current build platform.

Re: Stack calibration

[Ludovic Courtès]

Hello,

"Neil Jerram" <neiljerram@googlemail.com> writes:

> diff --git a/.gitignore b/.gitignore
> index a122176..39c4b49 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -70,3 +70,4 @@ guile-readline/guile-readline-config.h
>  guile-readline/guile-readline-config.h.in
>  TAGS
>  guile-1.8.pc
> +stack-limit-calibration.scm

I'd put a leading `/', which means that it only pertains to this
directory.

> +SCM_DEFINE (scm_sys_get_stack_size, "%get-stack-size", 0, 0, 0,
> +	    (),
> +	    "Return the current thread's C stack size (in units of sizeof (SCM_STACKITEM *)).")

For the "end-user", it's really "in units of Scheme objects", no?

"Neil Jerram" <neiljerram@googlemail.com> writes:

> One update to this below.  It isn't actually necessary or helpful, for
> this case, to pull in the GOOPS interface to evaluator traps.

Even better!  :-)

I'm happy with this new version, please go ahead!

Thank you,
Ludo'.

Re: Stack calibration

[Neil Jerram]

2008/10/14 Ludovic Courtès <ludo@gnu.org>:
> Hello,
>
> "Neil Jerram" <neiljerram@googlemail.com> writes:
>
>> diff --git a/.gitignore b/.gitignore
>> index a122176..39c4b49 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -70,3 +70,4 @@ guile-readline/guile-readline-config.h
>>  guile-readline/guile-readline-config.h.in
>>  TAGS
>>  guile-1.8.pc
>> +stack-limit-calibration.scm
>
> I'd put a leading `/', which means that it only pertains to this
> directory.

OK - but it should be libguile/stack-calibration.scm - done.

>> +SCM_DEFINE (scm_sys_get_stack_size, "%get-stack-size", 0, 0, 0,
>> +         (),
>> +         "Return the current thread's C stack size (in units of sizeof (SCM_STACKITEM *)).")
>
> For the "end-user", it's really "in units of Scheme objects", no?

Yes, updated accordingly.

> "Neil Jerram" <neiljerram@googlemail.com> writes:
>
>> One update to this below.  It isn't actually necessary or helpful, for
>> this case, to pull in the GOOPS interface to evaluator traps.
>
> Even better!  :-)
>
> I'm happy with this new version, please go ahead!

Thanks.  Will be committed very shortly, after some test builds.

     Neil

Re: Stack calibration

[Ludovic Courtès]

Hi Neil,

"Neil Jerram" <neiljerram@googlemail.com> writes:

> <imagine cinematic thinking effect>
> Goodness, that Ludovic, can't he ever just be happy with what I've proposed...

Eh eh, I'm starting to have a reputation!  ;-)

I did think you may be annoyed by that review after all the work you had
put it, but well.  :-)

> ...oh but hang on, we already have arbitrary callouts to Scheme: the
> evaluator traps!
> </imagine cinematic thinking effect>
>
> So yes, I think we can actually do this without any change to the C
> code except adding %get-stack-depth, by using an evaluator trap.

Great!

> Evaluator traps, on the other hand, will catch everything.  (At the
> cost of running very slowly, but I think that's OK for just this one
> calibration step.)

Yep, sounds like a good idea.

Thanks!

Ludo' the nitpicker.  ;-)

Re: Stack calibration

[Ludovic Courtès]

One last thing...

"Neil Jerram" <neiljerram@googlemail.com> writes:

> @@ -81,6 +106,15 @@ scm_stack_report ()
>  void
>  scm_init_stackchk ()
>  {
> +#ifdef GUILE_CALIBRATION_MEASURED_DEPTH_1
> +  /* Calculate calibrated stack depth limit. */
> +  calibrated_m = ((double) (GUILE_CALIBRATION_MEASURED_DEPTH_2 - GUILE_CALIBRATION_MEASURED_DEPTH_1))
> +    / (GUILE_CALIBRATION_REFERENCE_DEPTH_2 - GUILE_CALIBRATION_REFERENCE_DEPTH_1);
> +  calibrated_c = ((double) GUILE_CALIBRATION_MEASURED_DEPTH_2)
> +    - calibrated_m * GUILE_CALIBRATION_REFERENCE_DEPTH_2;
> +#endif
> +  scm_calculate_stack_limit ();
> +
>  #include "libguile/stackchk.x"
>  }

I'd like it better it these were statically initialized.

Ludo'.

Re: Stack calibration

[Neil Jerram]

2008/9/28 Ludovic Courtès <ludo@gnu.org>:
> One last thing...
>
> "Neil Jerram" <neiljerram@googlemail.com> writes:
>
>> @@ -81,6 +106,15 @@ scm_stack_report ()
>>  void
>>  scm_init_stackchk ()
>>  {
>> +#ifdef GUILE_CALIBRATION_MEASURED_DEPTH_1
>> +  /* Calculate calibrated stack depth limit. */
>> +  calibrated_m = ((double) (GUILE_CALIBRATION_MEASURED_DEPTH_2 - GUILE_CALIBRATION_MEASURED_DEPTH_1))
>> +    / (GUILE_CALIBRATION_REFERENCE_DEPTH_2 - GUILE_CALIBRATION_REFERENCE_DEPTH_1);
>> +  calibrated_c = ((double) GUILE_CALIBRATION_MEASURED_DEPTH_2)
>> +    - calibrated_m * GUILE_CALIBRATION_REFERENCE_DEPTH_2;
>> +#endif
>> +  scm_calculate_stack_limit ();
>> +
>>  #include "libguile/stackchk.x"
>>  }
>
> I'd like it better it these were statically initialized.

Good idea, I'll do that.

   Neil

Re: stack overflow

[Han-Wen Nienhuys]

Neil Jerram escreveu:

> [moving over to guile-devel...]
> 
> Not to disagree with anything that's already been said in this
> thread...  But I wonder if there is a way to make Guile's stack
> overflow checking a bit less fragile - i.e. less subject to the
> behaviour of particular compilers / OSs / optimization options?

Isn't it be possible to catch SIGSEGV, and check whether it was
caused by overflow?

-- 
 Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

Re: stack overflow

[Mikael Djurfeldt]

2008/2/17, Han-Wen Nienhuys <hanwen@xs4all.nl>:
> Isn't it be possible to catch SIGSEGV, and check whether it was
> caused by overflow?

Couldn't that leave the interpreter in a strange state so that one
would need to quit and restart? The current scheme allows the
interpreter to continue to run after error.

stack calibration

[Andy Wingo]

Hey Guilers,

The recent commit to compile with the stack calibration file,
7ca96180f00800414a9cf855e5ca4dceb9baca07, breaks compilation because the
compile scripts have hash-bang lines like this:

#!/bin/sh
# -*- scheme -*-
exec ${GUILE-guile} -e '(@ (scripts compile) compile)' -s $0 "$@"
!#

Also, it is a bit irritating to have to load a file just so Guile won't
be broken (exaggerated wording, but I think that's what it is.)

So I have a proposal. We should set the stack limit to 60k words.

  Pros: 1) This way Guile will just work.
        2) It's simple.
        3) We avoid the evaluator this way.

  Cons: 1) A stack-consuming process will take longer to catch.

I cannot think of any other cons. GC just marks between the stack base
and the stack pointer anyway. This would be 500 kB on x86-64, but a
normal program would never use that, and last time I looked processes
had 2 MB of stack by default anyway. It would be 250 kB on Linux.

If we were really concerned about processes actually consuming all of
their stack, we should use a value from getrlimit(2) instead.

I'm going to create a dev/wingo branch now, so I can push things for
review without mucking up master. I'll push patches implementing this to
there.

Cheers,

Andy
-- 
http://wingolog.org/

Re: stack calibration

[Mike Gran]

> From: Andy Wingo <wingo@pobox.com>
> 
> So I have a proposal. We should set the stack limit to 60k words.

I always set the stack limit higher, so that I can run a version
that has been compiled without optimization, for the sake of 
GDB.

-Mike Gran

Re: stack calibration

[Julian Graham]

I do this as well, FWIW.


> I always set the stack limit higher, so that I can run a version
> that has been compiled without optimization, for the sake of
> GDB.

Re: stack calibration

[Neil Jerram]

Andy Wingo <wingo@pobox.com> writes:

> Hey Guilers,

Hi Andy,

In summary, I'm not sure I'm following the logic here...

> The recent commit to compile with the stack calibration file,
> 7ca96180f00800414a9cf855e5ca4dceb9baca07, breaks compilation because the
> compile scripts have hash-bang lines like this:
>
> #!/bin/sh
> # -*- scheme -*-
> exec ${GUILE-guile} -e '(@ (scripts compile) compile)' -s $0 "$@"
> !#

FWIW, I think this kind of incantation is really horrible.  Ditto for
usage of "guile-tools ...".  What kind of a scripting language is it
that needs to be bootstrapped by a different language?

Anyway, I see the breakage...  (Probably almost any change would break
something so fragile.)

> Also, it is a bit irritating to have to load a file just so Guile won't
> be broken (exaggerated wording, but I think that's what it is.)

I think you may be misunderstanding.  stack-limit-calibration.scm
should make precisely 0 difference on the "canonical build platform" -
which in practice means ia32 GNU/Linux.

As Guile stands (and notwithstanding your idea that we could use
getrlimit instead), we do hardcode _some_ stack depth limit.  Once
that was 20k, then 40k, and you're now suggesting 60k.  Those are
supposed to be values that will work on the canonical platform.

stack-limit-calibration.scm is all about scaling down/up the hardcoded
value for a non-canonical platform that might use more or less stack
on average than the canonical platform - e.g. because it has fewer
registers, because its pointers are twice the size (ia64) etc.

The primary purpose of stack-limit-calibration.scm is to allow "make
check" to succeed on those platforms, and it now makes sense to
generalize that to any other guile-using operations that we run during
the build - such as compiling.

It would probably also make sense to install
stack-limit-calibration.scm, so that 3rd party programs could also use
it; but we don't currently do that.

> So I have a proposal. We should set the stack limit to 60k words.

If we need 60k on the canonical build platform, we should have 60k.
No issue there.

>   Pros: 1) This way Guile will just work.

(For you.  Other platforms may still need stack-limit-calibration.scm.)

>         2) It's simple.
>         3) We avoid the evaluator this way.
>
>   Cons: 1) A stack-consuming process will take longer to catch.

Not a problem.  Any individual program can set the stack limit
differently, if they so choose.

> I cannot think of any other cons. GC just marks between the stack base
> and the stack pointer anyway. This would be 500 kB on x86-64, but a
> normal program would never use that, and last time I looked processes
> had 2 MB of stack by default anyway. It would be 250 kB on Linux.
>
> If we were really concerned about processes actually consuming all of
> their stack, we should use a value from getrlimit(2) instead.

I don't think that is the concern.  I think the idea is to catch
incorrectly written (i.e. usually non-tail-recursive) programs before
they crash.

Regards,
        Neil

Re: stack calibration

[Andy Wingo]

Hi Neil,

On Mon 30 Mar 2009 13:43, Neil Jerram <neil@ossau.uklinux.net> writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>> Hey Guilers,
>
> Hi Andy,
>
> In summary, I'm not sure I'm following the logic here...
>
>> The recent commit to compile with the stack calibration file,
>> 7ca96180f00800414a9cf855e5ca4dceb9baca07, breaks compilation because the
>> compile scripts have hash-bang lines like this:
>>
>> #!/bin/sh
>> # -*- scheme -*-
>> exec ${GUILE-guile} -e '(@ (scripts compile) compile)' -s $0 "$@"
>> !#
>
> FWIW, I think this kind of incantation is really horrible. Ditto for
> usage of "guile-tools ...". What kind of a scripting language is it
> that needs to be bootstrapped by a different language?

Dunno. While guile-tools should probably be written in Scheme, it
doesn't bother me. The strange invocation stuff is just to get around
posix's #! limitations -- it should be:

  #!/usr/bin/env guile -e ....

but we all know the problem with that.

As far as needing the -e clause, it's so we can (use-modules (scripts
compile)) in addition to being able to run it as a script. Not that I
use that feature, but it is interesting.

>> Also, it is a bit irritating to have to load a file just so Guile won't
>> be broken (exaggerated wording, but I think that's what it is.)
>
> I think you may be misunderstanding.  stack-limit-calibration.scm
> should make precisely 0 difference on the "canonical build platform" -
> which in practice means ia32 GNU/Linux.

ia32 GNU/Linux is sometimes broken -- for example, build it with -O0,
and things often don't work at all.

> stack-limit-calibration.scm is all about scaling down/up the hardcoded
> value for a non-canonical platform that might use more or less stack
> on average than the canonical platform - e.g. because it has fewer
> registers, because its pointers are twice the size (ia64) etc.

> The primary purpose of stack-limit-calibration.scm is to allow "make
> check" to succeed on those platforms, and it now makes sense to
> generalize that to any other guile-using operations that we run during
> the build - such as compiling.

You want to actually use Guile after it's installed too of course, and
in those cases stack-calibration.scm doesn't help you.

I think I explained my perspective as well as I can in the other mail --
perhaps we can follow that part of the discussion there?

Andy
-- 
http://wingolog.org/

Re: stack calibration

[Neil Jerram]

Andy Wingo <wingo@pobox.com> writes:

> Hi Neil,

Hi Andy,

> On Mon 30 Mar 2009 13:43, Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> FWIW, I think this kind of incantation is really horrible. Ditto for
>> usage of "guile-tools ...". What kind of a scripting language is it
>> that needs to be bootstrapped by a different language?
>
> Dunno. While guile-tools should probably be written in Scheme, it
> doesn't bother me. The strange invocation stuff is just to get around
> posix's #! limitations -- it should be:
>
>   #!/usr/bin/env guile -e ....
>
> but we all know the problem with that.

Only one argument being portably supported?  (I _think_ that's the
problem, but I'm not so sure that I don't want to check that that's
what you mean!)

> As far as needing the -e clause, it's so we can (use-modules (scripts
> compile)) in addition to being able to run it as a script. Not that I
> use that feature, but it is interesting.

I don't use it either, and I don't think it's interesting enough to
justify the oddness of the incantation.  I can't think of a scenario
where it really makes sense to have a module and main program combined
in the same file.  If the module part isn't generally useful it
doesn't need to be written as a module.  If the module is generally
useful, it should be given a place in the proper module tree (i.e. not
scripts/...), and the script file should (use-modules ...) it.

On the other hand, I took a look again at guile-tools and remembered
why that _is_ useful.  (Basically the same argument as "git ...",
i.e. not dropping tens of executables into the system $PATH.)  So I
take back some of my "FWIW..." rant. :-)

>> I think you may be misunderstanding.  stack-limit-calibration.scm
>> should make precisely 0 difference on the "canonical build platform" -
>> which in practice means ia32 GNU/Linux.
>
> ia32 GNU/Linux is sometimes broken -- for example, build it with -O0,
> and things often don't work at all.

Good point.

> I think I explained my perspective as well as I can in the other mail --
> perhaps we can follow that part of the discussion there?

D'oh! :-)  OK, let's do that.

      Neil

Re: stack calibration

[Andy Wingo]

Hi Neil,

On Tue 31 Mar 2009 15:47, Neil Jerram <neil@ossau.uklinux.net> writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>>   #!/usr/bin/env guile -e ....
>>
>> but we all know the problem with that.
>
> Only one argument being portably supported?  (I _think_ that's the
> problem, but I'm not so sure that I don't want to check that that's
> what you mean!)

Heh, yes. Sorry, I should have been more clear.

>> As far as needing the -e clause, it's so we can (use-modules (scripts
>> compile)) in addition to being able to run it as a script. Not that I
>> use that feature, but it is interesting.
>
> I don't use it either, and I don't think it's interesting enough to
> justify the oddness of the incantation.  I can't think of a scenario
> where it really makes sense to have a module and main program combined
> in the same file.  If the module part isn't generally useful it
> doesn't need to be written as a module.  If the module is generally
> useful, it should be given a place in the proper module tree (i.e. not
> scripts/...), and the script file should (use-modules ...) it.

I kindof agree. There's no need to have those scripts be executable
files -- we can have them be modules instead. Then guile-tools can just
use-module them, as you say.

But they can probably stay in the scripts directory, so as to mark them
as "runnable" -- so you can query "what scripts do I have installed?"
But at least they can go into module/scripts/. I'll do that at some
point if you have no objections.

Cheers,

Andy
-- 
http://wingolog.org/

Re: stack calibration

[Greg Troxel]

[-- Attachment #1: Type: text/plain, Size: 700 bytes --]


Neil Jerram <neil@ossau.uklinux.net> writes:

> The primary purpose of stack-limit-calibration.scm is to allow "make
> check" to succeed on those platforms,

That's fine.

> and it now makes sense to
> generalize that to any other guile-using operations that we run during
> the build - such as compiling.

Not quite; the build is potentially cross, and the guile we just built
may not run on the host (autoconf "build") machine.

It may be there should be a file of stack limits in various places,
found by autoconf tuple.  That's somewhat ugly, but avoids breaking
cross builds.

An alternative is to detect the cross situation and use a fixed value
then.  That doesn't impair self-hosted builds.

[-- Attachment #2: Type: application/pgp-signature, Size: 193 bytes --]

Re: stack calibration

[Andy Wingo]

Hi Neil & all,

On Mon 30 Mar 2009 22:43, Neil Jerram <neil@ossau.uklinux.net> writes:

> Andy Wingo <wingo@pobox.com> writes:
>> #!/bin/sh
>> # -*- scheme -*-
>> exec ${GUILE-guile} -e '(@ (scripts compile) compile)' -s $0 "$@"
>> !#
>
> FWIW, I think this kind of incantation is really horrible.  Ditto for
> usage of "guile-tools ...".  What kind of a scripting language is it
> that needs to be bootstrapped by a different language?

I've changed guile-tools in master to be a scheme script, and the
scripts themselves to be normal modules. Things moved around a bit, but
use-wise you shouldn't notice a difference.

Man, do I have a job ahead of me wrt NEWS updates...

Andy
-- 
http://wingolog.org/

Re: stack calibration

[Ludovic Courtès]

Hello,

Andy Wingo <wingo@pobox.com> writes:

> The recent commit to compile with the stack calibration file,
> 7ca96180f00800414a9cf855e5ca4dceb9baca07, breaks compilation because the
> compile scripts have hash-bang lines like this:
>
> #!/bin/sh
> # -*- scheme -*-
> exec ${GUILE-guile} -e '(@ (scripts compile) compile)' -s $0 "$@"
> !#

But it doesn't matter since makefiles use `am/guilec', which doesn't do
this, does it?  (I did a "make clean && make check" before committing.)

> So I have a proposal. We should set the stack limit to 60k words.
>
>   Pros: 1) This way Guile will just work.

I agree with Neil: it will work for you(tm), but we can't tell whether
it'll work on, say, hppa*-hpux* with HP's compiler.  So we need this
sort of hack.  Another hack would be to "(debug-set! stack 0)" when
building the compiler (assuming the compiler code is "sane" in terms of
stack usage), and maybe even when running tests.

>         2) It's simple.
>         3) We avoid the evaluator this way.

Before the compiler is compiled, we can't avoid the evaluator, can we?

Thanks,
Ludo'.