[project @ 2001-03-02 16:15:53 by simonmar]

ASSERT in updateWithIndirection() that we haven't already updated this
object with an indirection, and fix two places in the RTS where this
could happen.

The problem only occurs when we're in a black-hole-style loop, and
there are multiple update frames on the stack pointing to the same
object (this is possible because of lazy black-holing).  Both stack
squeezing and asynchronous exception raising walk down the stack and
remove update frames, updating their contents with indirections.  If
we don't protect against multiple updates, the mutable list in the old
generation may get into a bogus state.

diff --git a/ghc/rts/GC.c b/ghc/rts/GC.c
index 63526f1..5388590 100644 (file)
--- a/ghc/rts/GC.c
+++ b/ghc/rts/GC.c
@@ -1,5 +1,5 @@
 /* -----------------------------------------------------------------------------
- * $Id: GC.c,v 1.97 2001/03/02 14:28:44 simonmar Exp $
+ * $Id: GC.c,v 1.98 2001/03/02 16:15:53 simonmar Exp $
  *
  * (c) The GHC Team 1998-1999
  *
@@ -3341,8 +3341,15 @@ threadSqueezeStack(StgTSO *tso)
       /* wasn't there something about update squeezing and ticky to be
        * sorted out?  oh yes: we aren't counting each enter properly
        * in this case.  See the log somewhere.  KSW 1999-04-21
+       *
+       * Check two things: that the two update frames don't point to
+       * the same object, and that the updatee_bypass isn't already an
+       * indirection.  Both of these cases only happen when we're in a
+       * block hole-style loop (and there are multiple update frames
+       * on the stack pointing to the same closure), but they can both
+       * screw us up if we don't check.
        */
-      if (updatee_bypass != updatee_keep) {
+      if (updatee_bypass != updatee_keep && !closure_IND(updatee_bypass)) {
          /* this wakes the threads up */
          UPD_IND_NOLOCK(updatee_bypass, updatee_keep);
       }

diff --git a/ghc/rts/Schedule.c b/ghc/rts/Schedule.c
index a835c7e..14be29b 100644 (file)
--- a/ghc/rts/Schedule.c
+++ b/ghc/rts/Schedule.c
@@ -1,5 +1,5 @@
 /* ---------------------------------------------------------------------------
- * $Id: Schedule.c,v 1.92 2001/03/02 14:25:04 simonmar Exp $
+ * $Id: Schedule.c,v 1.93 2001/03/02 16:15:53 simonmar Exp $
  *
  * (c) The GHC Team, 1998-2000
  *
@@ -2841,14 +2841,24 @@ raiseAsync(StgTSO *tso, StgClosure *exception)
        /* Replace the updatee with an indirection - happily
         * this will also wake up any threads currently
         * waiting on the result.
+        *
+        * Warning: if we're in a loop, more than one update frame on
+        * the stack may point to the same object.  Be careful not to
+        * overwrite an IND_OLDGEN in this case, because we'll screw
+        * up the mutable lists.  To be on the safe side, don't
+        * overwrite any kind of indirection at all.  See also
+        * threadSqueezeStack in GC.c, where we have to make a similar
+        * check.
         */
-       UPD_IND_NOLOCK(su->updatee,ap);  /* revert the black hole */
+       if (!closure_IND(su->updatee)) {
+           UPD_IND_NOLOCK(su->updatee,ap);  /* revert the black hole */
+       }
        su = su->link;
        sp += sizeofW(StgUpdateFrame) -1;
        sp[0] = (W_)ap; /* push onto stack */
        break;
       }
-      
+
     case CATCH_FRAME:
       {
        StgCatchFrame *cf = (StgCatchFrame *)su;

diff --git a/ghc/rts/Storage.h b/ghc/rts/Storage.h
index 2ae12f6..f446c0a 100644 (file)
--- a/ghc/rts/Storage.h
+++ b/ghc/rts/Storage.h
@@ -1,5 +1,5 @@
 /* -----------------------------------------------------------------------------
- * $Id: Storage.h,v 1.30 2001/03/02 14:36:16 simonmar Exp $
+ * $Id: Storage.h,v 1.31 2001/03/02 16:15:53 simonmar Exp $
  *
  * (c) The GHC Team, 1998-1999
  *
@@ -164,12 +164,17 @@ recordOldToNewPtrs(StgMutClosure *p)
 
 /* In the DEBUG case, we also zero out the slop of the old closure,
  * so that the sanity checker can tell where the next closure is.
+ *
+ * Two important invariants: we should never try to update a closure
+ * to point to itself, and the closure being updated should not
+ * already have been updated (the mutable list will get messed up
+ * otherwise).
  */
 #define updateWithIndirection(info, p1, p2)                            \
   {                                                                    \
     bdescr *bd;                                                                \
                                                                        \
-    ASSERT( p1 != p2 );                                                        \
+    ASSERT( p1 != p2 && !closure_IND(p1) );                            \
     bd = Bdescr((P_)p1);                                               \
     if (bd->gen->no == 0) {                                            \
       ((StgInd *)p1)->indirectee = p2;                                 \
@@ -203,7 +208,7 @@ recordOldToNewPtrs(StgMutClosure *p)
  */
 #define updateWithStaticIndirection(info, p1, p2)                      \
   {                                                                    \
-    ASSERT( p1 != p2 );                                                        \
+    ASSERT( p1 != p2 && !closure_IND(p1) );                            \
     ASSERT( ((StgMutClosure*)p1)->mut_link == NULL );                  \
                                                                        \
     ACQUIRE_LOCK(&sm_mutex);                                           \
@@ -222,7 +227,7 @@ updateWithPermIndirection(const StgInfoTable *info, StgClosure *p1, StgClosure *
 {
   bdescr *bd;
 
-  ASSERT( p1 != p2 );                                                  \
+  ASSERT( p1 != p2 && !closure_IND(p1) );
   bd = Bdescr((P_)p1);
   if (bd->gen->no == 0) {
     ((StgInd *)p1)->indirectee = p2;