In the DEBUG rts, track when CAFs are GC'd
authorSimon Marlow <marlowsd@gmail.com>
Thu, 21 Nov 2013 11:28:13 +0000 (11:28 +0000)
committerSimon Marlow <marlowsd@gmail.com>
Thu, 21 Nov 2013 13:27:34 +0000 (13:27 +0000)
This resurrects some old code and makes it work again.  The idea is
that we want to get an error message if we ever enter a CAF that has
been GC'd, rather than following its indirection which will likely
cause a segfault.  Without this patch, these bugs are hard to track
down in gdb, because the IND_STATIC code overwrites R1 (the pointer to
the CAF) with its indirectee before jumping into bad memory, so we've
lost the address of the CAF that got GC'd.

Some associated refactoring while I was here.

includes/rts/storage/GC.h
includes/stg/MiscClosures.h
rts/StgMiscClosures.cmm
rts/sm/GC.c
rts/sm/GCAux.c
rts/sm/Storage.c
rts/sm/Storage.h

index 1a92031..8133496 100644 (file)
@@ -180,8 +180,8 @@ void performMajorGC(void);
    The CAF table - used to let us revert CAFs in GHCi
    -------------------------------------------------------------------------- */
 
-StgWord newCAF    (StgRegTable *reg, StgClosure *caf, StgClosure *bh);
-StgWord newDynCAF (StgRegTable *reg, StgClosure *caf, StgClosure *bh);
+StgWord newCAF    (StgRegTable *reg, StgIndStatic *caf, StgClosure *bh);
+StgWord newDynCAF (StgRegTable *reg, StgIndStatic *caf, StgClosure *bh);
 void revertCAFs (void);
 
 // Request that all CAFs are retained indefinitely.
index d36c70d..fdd50eb 100644 (file)
@@ -115,6 +115,7 @@ RTS_ENTRY(stg_MUT_ARR_PTRS_FROZEN0);
 RTS_ENTRY(stg_MUT_VAR_CLEAN);
 RTS_ENTRY(stg_MUT_VAR_DIRTY);
 RTS_ENTRY(stg_END_TSO_QUEUE);
+RTS_ENTRY(stg_GCD_CAF);
 RTS_ENTRY(stg_STM_AWOKEN);
 RTS_ENTRY(stg_MSG_TRY_WAKEUP);
 RTS_ENTRY(stg_MSG_THROWTO);
index 9484031..b8122b4 100644 (file)
@@ -556,6 +556,13 @@ INFO_TABLE_CONSTR(stg_END_TSO_QUEUE,0,0,0,CONSTR_NOCAF_STATIC,"END_TSO_QUEUE","E
 CLOSURE(stg_END_TSO_QUEUE_closure,stg_END_TSO_QUEUE);
 
 /* ----------------------------------------------------------------------------
+   GCD_CAF
+   ------------------------------------------------------------------------- */
+
+INFO_TABLE_CONSTR(stg_GCD_CAF,0,0,0,CONSTR_NOCAF_STATIC,"GCD_CAF","GCD_CAF")
+{ foreign "C" barf("Evaluated a CAF that was GC'd!") never returns; }
+
+/* ----------------------------------------------------------------------------
    STM_AWOKEN
 
    This is a static nullary constructor (like []) that we use to mark a
index b5d6f03..80e34fb 100644 (file)
@@ -156,7 +156,7 @@ static void shutdown_gc_threads     (nat me);
 static void collect_gct_blocks      (void);
 static void collect_pinned_object_blocks (void);
 
-#if 0 && defined(DEBUG)
+#if defined(DEBUG)
 static void gcCAFs                  (void);
 #endif
 
@@ -667,7 +667,7 @@ GarbageCollect (nat collect_gen,
   }
 
  // mark the garbage collected CAFs as dead
-#if 0 && defined(DEBUG) // doesn't work at the moment
+#if defined(DEBUG)
   if (major_gc) { gcCAFs(); }
 #endif
 
@@ -1732,41 +1732,39 @@ resize_nursery (void)
    time.
    -------------------------------------------------------------------------- */
 
-#if 0 && defined(DEBUG)
+#if defined(DEBUG)
 
-static void
-gcCAFs(void)
+static void gcCAFs(void)
 {
-  StgClosure*  p;
-  StgClosure** pp;
-  const StgInfoTable *info;
-  nat i;
+    StgIndStatic *p, *prev;
 
-  i = 0;
-  p = caf_list;
-  pp = &caf_list;
+    const StgInfoTable *info;
+    nat i;
 
-  while (p != NULL) {
+    i = 0;
+    p = debug_caf_list;
+    prev = NULL;
 
-    info = get_itbl(p);
+    for (p = debug_caf_list; p != (StgIndStatic*)END_OF_STATIC_LIST;
+         p = (StgIndStatic*)p->saved_info) {
 
-    ASSERT(info->type == IND_STATIC);
+        info = get_itbl((StgClosure*)p);
+        ASSERT(info->type == IND_STATIC);
 
-    if (STATIC_LINK(info,p) == NULL) {
-       debugTrace(DEBUG_gccafs, "CAF gc'd at 0x%04lx", (long)p);
-       // black hole it
-       SET_INFO(p,&stg_BLACKHOLE_info);
-       p = STATIC_LINK2(info,p);
-       *pp = p;
-    }
-    else {
-      pp = &STATIC_LINK2(info,p);
-      p = *pp;
-      i++;
+        if (p->static_link == NULL) {
+            debugTrace(DEBUG_gccafs, "CAF gc'd at 0x%04lx", (long)p);
+            SET_INFO((StgClosure*)p,&stg_GCD_CAF_info); // stub it
+            if (prev == NULL) {
+                debug_caf_list = (StgIndStatic*)p->saved_info;
+            } else {
+                prev->saved_info = p->saved_info;
+            }
+        } else {
+            prev = p;
+            i++;
+        }
     }
 
-  }
-
-  debugTrace(DEBUG_gccafs, "%d CAFs live", i);
+    debugTrace(DEBUG_gccafs, "%d CAFs live", i);
 }
 #endif
index 29c1e9d..10df9dd 100644 (file)
@@ -117,15 +117,15 @@ revertCAFs( void )
 {
     StgIndStatic *c;
 
-    for (c = (StgIndStatic *)revertible_caf_list; 
+    for (c = revertible_caf_list;
          c != (StgIndStatic *)END_OF_STATIC_LIST; 
         c = (StgIndStatic *)c->static_link) 
     {
-       SET_INFO((StgClosure *)c, c->saved_info);
+        SET_INFO((StgClosure *)c, c->saved_info);
        c->saved_info = NULL;
        // could, but not necessary: c->static_link = NULL; 
     }
-    revertible_caf_list = END_OF_STATIC_LIST;
+    revertible_caf_list = (StgIndStatic*)END_OF_STATIC_LIST;
 }
 
 void
@@ -133,13 +133,13 @@ markCAFs (evac_fn evac, void *user)
 {
     StgIndStatic *c;
 
-    for (c = (StgIndStatic *)caf_list;
+    for (c = dyn_caf_list;
          c != (StgIndStatic*)END_OF_STATIC_LIST; 
         c = (StgIndStatic *)c->static_link) 
     {
        evac(user, &c->indirectee);
     }
-    for (c = (StgIndStatic *)revertible_caf_list; 
+    for (c = revertible_caf_list;
          c != (StgIndStatic*)END_OF_STATIC_LIST; 
         c = (StgIndStatic *)c->static_link) 
     {
index eaf7ddd..112ad83 100644 (file)
@@ -40,8 +40,9 @@
 /* 
  * All these globals require sm_mutex to access in THREADED_RTS mode.
  */
-StgClosure    *caf_list         = NULL;
-StgClosure    *revertible_caf_list = NULL;
+StgIndStatic  *dyn_caf_list        = NULL;
+StgIndStatic  *debug_caf_list      = NULL;
+StgIndStatic  *revertible_caf_list = NULL;
 rtsBool       keepCAFs;
 
 W_ large_alloc_lim;    /* GC if n_large_blocks in any nursery
@@ -171,8 +172,9 @@ initStorage (void)
 
   generations[0].max_blocks = 0;
 
-  caf_list = END_OF_STATIC_LIST;
-  revertible_caf_list = END_OF_STATIC_LIST;
+  dyn_caf_list = (StgIndStatic*)END_OF_STATIC_LIST;
+  debug_caf_list = (StgIndStatic*)END_OF_STATIC_LIST;
+  revertible_caf_list = (StgIndStatic*)END_OF_STATIC_LIST;
    
   /* initialise the allocate() interface */
   large_alloc_lim = RtsFlags.GcFlags.minAllocAreaSize * BLOCK_SIZE_W;
@@ -331,7 +333,7 @@ freeStorage (rtsBool free_heap)
 
    -------------------------------------------------------------------------- */
 
-STATIC_INLINE StgWord lockCAF (StgClosure *caf, StgClosure *bh)
+STATIC_INLINE StgWord lockCAF (StgIndStatic *caf, StgClosure *bh)
 {
     const StgInfoTable *orig_info;
 
@@ -360,23 +362,23 @@ STATIC_INLINE StgWord lockCAF (StgClosure *caf, StgClosure *bh)
 #endif
 
     // For the benefit of revertCAFs(), save the original info pointer
-    ((StgIndStatic *)caf)->saved_info  = orig_info;
+    caf->saved_info = orig_info;
 
-    ((StgIndStatic*)caf)->indirectee = bh;
+    caf->indirectee = bh;
     write_barrier();
-    SET_INFO(caf,&stg_IND_STATIC_info);
+    SET_INFO((StgClosure*)caf,&stg_IND_STATIC_info);
 
     return 1;
 }
 
 StgWord
-newCAF(StgRegTable *reg, StgClosure *caf, StgClosure *bh)
+newCAF(StgRegTable *reg, StgIndStatic *caf, StgClosure *bh)
 {
     if (lockCAF(caf,bh) == 0) return 0;
 
     if(keepCAFs)
     {
-        // HACK:
+        // Note [dyn_caf_list]
         // If we are in GHCi _and_ we are using dynamic libraries,
         // then we can't redirect newCAF calls to newDynCAF (see below),
         // so we make newCAF behave almost like newDynCAF.
@@ -387,19 +389,35 @@ newCAF(StgRegTable *reg, StgClosure *caf, StgClosure *bh)
         // do another hack here and do an address range test on caf to figure
         // out whether it is from a dynamic library.
 
-        ACQUIRE_SM_LOCK; // caf_list is global, locked by sm_mutex
-        ((StgIndStatic *)caf)->static_link = caf_list;
-        caf_list = caf;
+        ACQUIRE_SM_LOCK; // dyn_caf_list is global, locked by sm_mutex
+        caf->static_link = (StgClosure*)dyn_caf_list;
+        dyn_caf_list = caf;
         RELEASE_SM_LOCK;
     }
     else
     {
         // Put this CAF on the mutable list for the old generation.
-        ((StgIndStatic *)caf)->saved_info = NULL;
         if (oldest_gen->no != 0) {
-            recordMutableCap(caf, regTableToCapability(reg), oldest_gen->no);
+            recordMutableCap((StgClosure*)caf,
+                             regTableToCapability(reg), oldest_gen->no);
         }
+
+#ifdef DEBUG
+        // In the DEBUG rts, we keep track of live CAFs by chaining them
+        // onto a list debug_caf_list.  This is so that we can tell if we
+        // ever enter a GC'd CAF, and emit a suitable barf().
+        //
+        // The saved_info field of the CAF is used as the link field for
+        // debug_caf_list, because this field is only used by newDynCAF
+        // for revertible CAFs, and we don't put those on the
+        // debug_caf_list.
+        ACQUIRE_SM_LOCK; // debug_caf_list is global, locked by sm_mutex
+        ((StgIndStatic *)caf)->saved_info = (const StgInfoTable*)debug_caf_list;
+        debug_caf_list = (StgIndStatic*)caf;
+        RELEASE_SM_LOCK;
+#endif
     }
+
     return 1;
 }
 
@@ -420,13 +438,13 @@ setKeepCAFs (void)
 // The linker hackily arranges that references to newCaf from dynamic
 // code end up pointing to newDynCAF.
 StgWord
-newDynCAF (StgRegTable *reg STG_UNUSED, StgClosure *caf, StgClosure *bh)
+newDynCAF (StgRegTable *reg STG_UNUSED, StgIndStatic *caf, StgClosure *bh)
 {
     if (lockCAF(caf,bh) == 0) return 0;
 
     ACQUIRE_SM_LOCK;
 
-    ((StgIndStatic *)caf)->static_link = revertible_caf_list;
+    caf->static_link = (StgClosure*)revertible_caf_list;
     revertible_caf_list = caf;
 
     RELEASE_SM_LOCK;
index c4f8709..e433c2b 100644 (file)
@@ -115,8 +115,28 @@ extern bdescr *exec_block;
 
 void move_STACK  (StgStack *src, StgStack *dest);
 
-extern StgClosure * caf_list;
-extern StgClosure * revertible_caf_list;
+/* -----------------------------------------------------------------------------
+   CAF lists
+   
+   dyn_caf_list  (CAFs chained through static_link)
+      This is a chain of all CAFs in the program, used for
+      dynamically-linked GHCi.
+      See Note [dyn_caf_list].
+
+   debug_caf_list  (CAFs chained through saved_info)
+      A chain of all *live* CAFs in the program, that does not keep
+      the CAFs alive.  Used for detecting when we enter a GC'd CAF,
+      and to give diagnostics with +RTS -DG.
+
+   revertible_caf_list  (CAFs chained through static_link)
+      A chain of CAFs in object code loaded with the RTS linker.
+      These CAFs can be reverted to their unevaluated state using
+      revertCAFs.
+ --------------------------------------------------------------------------- */
+
+extern StgIndStatic * dyn_caf_list;
+extern StgIndStatic * debug_caf_list;
+extern StgIndStatic * revertible_caf_list;
 
 #include "EndPrivate.h"