From 066f10289f9711a0f6d0669aea97e134f1be2826 Mon Sep 17 00:00:00 2001 From: Simon Marlow Date: Tue, 11 Sep 2007 13:02:28 +0000 Subject: [PATCH] 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. --- compiler/ghci/ByteCodeGen.lhs | 14 ++++++++------ compiler/main/Constants.lhs | 7 +++++++ includes/Constants.h | 7 +++++++ rts/Apply.cmm | 4 +++- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/compiler/ghci/ByteCodeGen.lhs b/compiler/ghci/ByteCodeGen.lhs index f46eb94..846737e 100644 --- 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 --- 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 --- 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 --- 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); -- 1.7.10.4