From e82fa82933e6984c5b9507a0d857b0c579bd0f46 Mon Sep 17 00:00:00 2001 From: Simon Marlow Date: Thu, 21 Nov 2013 11:28:13 +0000 Subject: In the DEBUG rts, track when CAFs are GC'd 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 | 4 ++-- includes/stg/MiscClosures.h | 1 + rts/StgMiscClosures.cmm | 7 ++++++ rts/sm/GC.c | 56 ++++++++++++++++++++++----------------------- rts/sm/GCAux.c | 10 ++++---- rts/sm/Storage.c | 52 +++++++++++++++++++++++++++-------------- rts/sm/Storage.h | 24 +++++++++++++++++-- 7 files changed, 99 insertions(+), 55 deletions(-) diff --git a/includes/rts/storage/GC.h b/includes/rts/storage/GC.h index 1a9203148c..813349610c 100644 --- a/includes/rts/storage/GC.h +++ b/includes/rts/storage/GC.h @@ -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. diff --git a/includes/stg/MiscClosures.h b/includes/stg/MiscClosures.h index d36c70d114..fdd50eba8f 100644 --- a/includes/stg/MiscClosures.h +++ b/includes/stg/MiscClosures.h @@ -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); diff --git a/rts/StgMiscClosures.cmm b/rts/StgMiscClosures.cmm index 9484031832..b8122b479e 100644 --- a/rts/StgMiscClosures.cmm +++ b/rts/StgMiscClosures.cmm @@ -555,6 +555,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 diff --git a/rts/sm/GC.c b/rts/sm/GC.c index b5d6f030df..80e34fb96e 100644 --- a/rts/sm/GC.c +++ b/rts/sm/GC.c @@ -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 diff --git a/rts/sm/GCAux.c b/rts/sm/GCAux.c index 29c1e9d604..10df9dd84b 100644 --- a/rts/sm/GCAux.c +++ b/rts/sm/GCAux.c @@ -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) { diff --git a/rts/sm/Storage.c b/rts/sm/Storage.c index eaf7dddece..112ad8322a 100644 --- a/rts/sm/Storage.c +++ b/rts/sm/Storage.c @@ -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; diff --git a/rts/sm/Storage.h b/rts/sm/Storage.h index c4f8709847..e433c2b8fe 100644 --- a/rts/sm/Storage.h +++ b/rts/sm/Storage.h @@ -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" -- cgit v1.2.1