rts: Claim AP_STACK before adjusting Sp
authorBen Gamari <bgamari.foss@gmail.com>
Thu, 20 Jul 2017 15:33:51 +0000 (11:33 -0400)
committerBen Gamari <ben@smart-cactus.org>
Thu, 20 Jul 2017 15:33:52 +0000 (11:33 -0400)
In the fix to #13615 we introduced some logic to atomically blackhole
AP_STACKs closures upon entry. However, this logic was placed *after* a
stack pointer adjustment. This meant that if someone else beat us to
blackholing the AP_STACK we would suspend the thread with uninitialized
content on the stack.  This would then later blow up when threadPaused
attempted to walk the stack, hence #13970.

Silly bug but still cost lots of head-scratching to find.

Thanks to albertov for the great repro.

Fixes #13970. Bug originally introduced by the fix to #13615.

Reviewers: austin, erikd, simonmar

Reviewed By: erikd, simonmar

Subscribers: rwbarton, thomie

GHC Trac Issues: #13970, #13615

Differential Revision: https://phabricator.haskell.org/D3760

rts/Apply.cmm

index 64f0a9b..ffcd035 100644 (file)
@@ -637,11 +637,6 @@ INFO_TABLE(stg_AP_STACK,/*special layout*/0,0,AP_STACK,"AP_STACK","AP_STACK")
    * than continuing to evaluate the now-defunct closure.
    */
   STK_CHK_ENTER(WDS(Words) + SIZEOF_StgUpdateFrame + WDS(AP_STACK_SPLIM), R1);
-  /* 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);
 
   /*
    * It is imperative that we blackhole lest we may duplicate computation which
@@ -657,6 +652,11 @@ INFO_TABLE(stg_AP_STACK,/*special layout*/0,0,AP_STACK,"AP_STACK","AP_STACK")
   prim_write_barrier;
   SET_INFO(ap, __stg_EAGER_BLACKHOLE_info);
 
+  /* 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);
+
   TICK_ENT_AP();
   LDV_ENTER(ap);
   ENTER_CCS_THUNK(ap);