Opened 8 years ago

Last modified 2 months ago

#1466 new bug

Stack check for AP_STACK

Reported by: simonmar Owned by: simonmar
Priority: normal Milestone: 7.12.1
Component: Compiler Version: 6.6.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case: concprog001
Blocked By: #4258 Blocking:
Related Tickets: Differential Revisions:

Description

This is a bug I uncovered in the interpreter, that I think is also present in the compiler.

When compiling code for a function or thunk, we aggregate the stack usage from case continuations up to the top of the function/thunk and perform a single stack check. This normally works fine: we know the maximum amount of stack that will be required in the evaluation of this function/thunk, and we check for it up front.

However, this goes wrong if the evaluation is suspended by an asynchronous exception: the stack is broken into chunks and stored in AP_STACK objects, which may later be resumed. On resumption, we might not have enough stack any more: the code might now be running on another stack entirely, even.

Our proposed solution is as follows:

  • Continuations that require more than a certain amount of stack (say 4k) do their own stack checks.
  • an AP_STACK object records the amount of stack available at the time it was suspended, if the amount is <4k. On resumption of an AP_STACK, we ensure that at least this amount of stack is available. (there's a spare half-word field in AP_STACK that we can use for this).

The 4k limit is important: it puts a bound on the amount of stack growth due to evaluating an AP_STACK. Essentially the 4k limit is large enough that it almost never happens.

Change History (16)

comment:1 Changed 7 years ago by simonmar

Partly done:

Tue Sep 11 14:02:28 BST 2007  Simon Marlow <[email protected]>
  * 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.

comment:2 Changed 7 years ago by simonmar

  • Milestone changed from 6.8 branch to 6.10 branch

Do this as part of the new back end

comment:3 Changed 6 years ago by simonmar

  • Architecture changed from Unknown to Unknown/Multiple

comment:4 Changed 6 years ago by simonmar

  • Operating System changed from Unknown to Unknown/Multiple

comment:5 Changed 6 years ago by igloo

  • Milestone changed from 6.10 branch to 6.12.1

comment:6 Changed 6 years ago by simonmar

  • Component changed from Runtime System to Compiler
  • Milestone changed from 6.12.1 to 6.14.1

Bumping to 6.14.1 as this is work to do in the new codegen.

comment:7 Changed 5 years ago by simonmar

  • difficulty changed from Moderate (1 day) to Moderate (less than a day)

comment:8 Changed 4 years ago by igloo

  • Milestone changed from 7.0.1 to 7.0.2

comment:9 Changed 4 years ago by igloo

  • Milestone changed from 7.0.2 to 7.2.1

comment:10 Changed 3 years ago by igloo

  • Blocked By 4258 added

comment:11 Changed 3 years ago by igloo

  • Milestone changed from 7.2.1 to 7.6.1
  • Type of failure set to None/Unknown

comment:12 Changed 2 years ago by igloo

  • Milestone changed from 7.6.1 to 7.6.2

comment:13 follow-up: Changed 2 years ago by morabbin

Bump; does this work in the new codegen as indicated above?

comment:14 in reply to: ↑ 13 Changed 2 years ago by simonmar

Replying to morabbin:

Bump; does this work in the new codegen as indicated above?

No, not done yet.

comment:15 Changed 8 months ago by thoughtpolice

  • Milestone changed from 7.6.2 to 7.10.1

Moving to 7.10.1.

comment:16 Changed 2 months ago by thoughtpolice

  • Milestone changed from 7.10.1 to 7.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

Note: See TracTickets for help on using tickets.