[commit: ghc] master: rts: Claim AP_STACK before adjusting Sp (bade356)

git at git.haskell.org git at git.haskell.org
Thu Jul 20 16:05:12 UTC 2017


Repository : ssh://git@git.haskell.org/ghc

On branch  : master
Link       : http://ghc.haskell.org/trac/ghc/changeset/bade356f79d44c9f6e8918a89d9ffac7f5608dbf/ghc

>---------------------------------------------------------------

commit bade356f79d44c9f6e8918a89d9ffac7f5608dbf
Author: Ben Gamari <bgamari.foss at gmail.com>
Date:   Thu Jul 20 11:33:51 2017 -0400

    rts: Claim AP_STACK before adjusting Sp
    
    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


>---------------------------------------------------------------

bade356f79d44c9f6e8918a89d9ffac7f5608dbf
 rts/Apply.cmm | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/rts/Apply.cmm b/rts/Apply.cmm
index 64f0a9b..ffcd035 100644
--- a/rts/Apply.cmm
+++ b/rts/Apply.cmm
@@ -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);



More information about the ghc-commits mailing list