FIX #1466 (partly), which was causing concprog001(ghci) to fail

An AP_STACK now ensures that there is at least AP_STACK_SPLIM words of
stack headroom available after unpacking the payload.  Continuations
that require more than AP_STACK_SPLIM words of stack must do their own
stack checks instead of aggregating their stack usage into the parent
frame.  I have made this change for the interpreter, but not for
compiled code yet - we should do this in the glorious rewrite of the
code generator.

diff --git a/compiler/ghci/ByteCodeGen.lhs b/compiler/ghci/ByteCodeGen.lhs
index f46eb94..846737e 100644 (file)
--- a/compiler/ghci/ByteCodeGen.lhs
+++ b/compiler/ghci/ByteCodeGen.lhs
@@ -178,17 +178,19 @@ mkProtoBCO nm instrs_ordlist origin arity bitmap_size bitmap is_ret mallocd_bloc
         -- (hopefully rare) cases when the (overestimated) stack use
         -- exceeds iNTERP_STACK_CHECK_THRESH.
         maybe_with_stack_check
-          | is_ret = peep_d
-               -- don't do stack checks at return points;
+          | is_ret && stack_usage < aP_STACK_SPLIM = peep_d
+               -- don't do stack checks at return points,
                -- everything is aggregated up to the top BCO
-               -- (which must be a function)
-           | stack_overest >= iNTERP_STACK_CHECK_THRESH
-           = STKCHECK stack_overest : peep_d
+               -- (which must be a function).
+                -- That is, unless the stack usage is >= AP_STACK_SPLIM,
+                -- see bug #1466.
+           | stack_usage >= iNTERP_STACK_CHECK_THRESH
+           = STKCHECK stack_usage : peep_d
            | otherwise
            = peep_d    -- the supposedly common case
              
         -- We assume that this sum doesn't wrap
-        stack_overest = sum (map bciStackUse peep_d)
+        stack_usage = sum (map bciStackUse peep_d)
 
         -- Merge local pushes
         peep_d = peep (fromOL instrs_ordlist)

diff --git a/compiler/main/Constants.lhs b/compiler/main/Constants.lhs
index 0437217..8dc94d1 100644 (file)
--- a/compiler/main/Constants.lhs
+++ b/compiler/main/Constants.lhs
@@ -110,6 +110,13 @@ returning to the scheduler.
 rESERVED_STACK_WORDS = (RESERVED_STACK_WORDS :: Int)
 \end{code}
 
+Continuations that need more than this amount of stack should do their
+own stack check (see bug #1466).
+
+\begin{code}
+aP_STACK_SPLIM = (AP_STACK_SPLIM :: Int)
+\end{code}
+
 Size of a word, in bytes
 
 \begin{code}

diff --git a/includes/Constants.h b/includes/Constants.h
index 012acd1..e0949cb 100644 (file)
--- a/includes/Constants.h
+++ b/includes/Constants.h
@@ -118,6 +118,13 @@
 #define RESERVED_STACK_WORDS 21
 
 /* -----------------------------------------------------------------------------
+   The limit on the size of the stack check performed when we enter an
+   AP_STACK, in words.  See raiseAsync() and bug #1466.
+   -------------------------------------------------------------------------- */
+
+#define AP_STACK_SPLIM 1024
+
+/* -----------------------------------------------------------------------------
    Storage manager constants
    -------------------------------------------------------------------------- */
 

diff --git a/rts/Apply.cmm b/rts/Apply.cmm
index c9c7daa..0498f00 100644 (file)
--- a/rts/Apply.cmm
+++ b/rts/Apply.cmm
@@ -249,7 +249,9 @@ INFO_TABLE(stg_AP_STACK,/*special layout*/0,0,AP_STACK,"AP_STACK","AP_STACK")
    * closure, in which case we must enter the blackhole on return rather
    * than continuing to evaluate the now-defunct closure.
    */
-  STK_CHK_NP(WDS(Words) + SIZEOF_StgUpdateFrame);
+  STK_CHK_NP(WDS(Words) + SIZEOF_StgUpdateFrame + WDS(AP_STACK_SPLIM));
+  /* ensure there is at least AP_STACK_SPLIM words of headroom available
+   * after unpacking the AP_STACK. See bug #1466 */
 
   PUSH_UPD_FRAME(Sp - SIZEOF_StgUpdateFrame, R1);
   Sp = Sp - SIZEOF_StgUpdateFrame - WDS(Words);