From: Ivan Maidanski Date: Thu, 15 Mar 2018 07:10:39 +0000 (+0300) Subject: Fix the collector hang when it is configured with --enable-gc-debug X-Git-Tag: v8.0.0~287 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d6ccabe90f203f4789e1bc5ee98b8bd7c1b82361;p=gc Fix the collector hang when it is configured with --enable-gc-debug Issue #205 (bdwgc). * dbg_mlc.c (GC_store_debug_info_inner): Remove comment (as it exists in the header); change from STATIC to GC_INNER. * dbg_mlc.c (STORE_DEBUG_INFO): New macro. * dbg_mlc.c (GC_store_debug_info): Change from GC_INNER to static; remove GC_ prefix; replace ptr_t to void*; add fn argument; replace "const char *string, int linenum" with GC_EXTRA_PARAMS; change "word sz" argument to "size_t lb"; allow p to be null (print error message in this case); call GC_start_debugging_inner unless GC_debugging_started; call ADD_CALL_CHAIN. * dbg_mlc.c (GC_start_debugging): Remove. * dbg_mlc.c (GC_debug_malloc, GC_debug_malloc_ignore_off_page, GC_debug_malloc_atomic_ignore_off_page, GC_debug_generic_malloc, GC_debug_malloc_atomic, GC_debug_malloc_uncollectable): Call STORE_DEBUG_INFO() instead of checking result for null and calling GC_start_debugging, ADD_CALL_CHAIN, GC_store_debug_info. * dbg_mlc.c [STUBBORN_ALLOC] (GC_debug_malloc_stubborn): Likewise. * dbg_mlc.c [GC_ATOMIC_UNCOLLECTABLE] (GC_debug_malloc_atomic_uncollectable): Likewise. * gcj_mlc.c [GC_GCJ_SUPPORT] (GC_debug_gcj_malloc): Call ADD_CALL_CHAIN while holding the lock; call GC_store_debug_info_inner (holding the lock) instead of GC_store_debug_info. * include/private/dbg_mlc.h (ADD_CALL_CHAIN): Update comment. * include/private/gc_priv.h (GC_store_debug_info): Replace with GC_store_debug_info_inner; update comment; change ptr_t to void*. * os_dep.c [SAVE_CALL_CHAIN] (GC_save_callers): Add assertion that the allocation lock is held; add comment. --- diff --git a/dbg_mlc.c b/dbg_mlc.c index 610728da..77497ebc 100644 --- a/dbg_mlc.c +++ b/dbg_mlc.c @@ -272,10 +272,8 @@ # define CROSSES_HBLK(p, sz) \ (((word)((p) + sizeof(oh) + (sz) - 1) ^ (word)(p)) >= HBLKSIZE) -/* Store debugging info into p. Return displaced pointer. */ -/* This version assumes we do hold the allocation lock. */ -STATIC void *GC_store_debug_info_inner(void *p, word sz GC_ATTR_UNUSED, - const char *string, int linenum) +GC_INNER void *GC_store_debug_info_inner(void *p, word sz GC_ATTR_UNUSED, + const char *string, int linenum) { word * result = (word *)((oh *)p + 1); @@ -298,14 +296,29 @@ STATIC void *GC_store_debug_info_inner(void *p, word sz GC_ATTR_UNUSED, return result; } -GC_INNER ptr_t GC_store_debug_info(ptr_t p, word sz, const char *string, - int linenum) +/* Check the allocation is successful, store debugging info into p, */ +/* start the debugging mode (if not yet), and return displaced pointer. */ +#ifdef GC_ADD_CALLER +# define STORE_DEBUG_INFO(p, lb, fn) store_debug_info(p, lb, fn, ra, s, i) +#else +# define STORE_DEBUG_INFO(p, lb, fn) store_debug_info(p, lb, fn, s, i) +#endif +static void *store_debug_info(void *p, size_t lb, + const char *fn, GC_EXTRA_PARAMS) { - ptr_t result; + void *result; DCL_LOCK_STATE; + if (NULL == p) { + GC_err_printf("%s(%lu) returning NULL (%s:%d)\n", + fn, (unsigned long)lb, s, i); + return NULL; + } LOCK(); - result = (ptr_t)GC_store_debug_info_inner(p, sz, string, linenum); + if (!GC_debugging_started) + GC_start_debugging_inner(); + ADD_CALL_CHAIN(p, ra); + result = GC_store_debug_info_inner(p, (word)lb, s, i); UNLOCK(); return result; } @@ -485,20 +498,6 @@ GC_INNER void GC_start_debugging_inner(void) GC_register_displacement_inner((word)sizeof(oh)); } -#ifdef THREADS - STATIC void GC_start_debugging(void) - { - DCL_LOCK_STATE; - - LOCK(); - if (!GC_debugging_started) - GC_start_debugging_inner(); - UNLOCK(); - } -#else -# define GC_start_debugging GC_start_debugging_inner -#endif /* !THREADS */ - size_t GC_debug_header_size = sizeof(oh); GC_API void GC_CALL GC_debug_register_displacement(size_t offset) @@ -546,16 +545,7 @@ GC_API GC_ATTR_MALLOC void * GC_CALL GC_debug_malloc(size_t lb, GC_caller_func_offset(ra, &s, &i); } # endif - if (result == 0) { - GC_err_printf("GC_debug_malloc(%lu) returning NULL (%s:%d)\n", - (unsigned long)lb, s, i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return GC_store_debug_info((ptr_t)result, (word)lb, s, i); + return STORE_DEBUG_INFO(result, lb, "GC_debug_malloc"); } GC_API GC_ATTR_MALLOC void * GC_CALL @@ -563,16 +553,7 @@ GC_API GC_ATTR_MALLOC void * GC_CALL { void * result = GC_malloc_ignore_off_page(SIZET_SAT_ADD(lb, DEBUG_BYTES)); - if (result == 0) { - GC_err_printf("GC_debug_malloc_ignore_off_page(%lu)" - " returning NULL (%s:%d)\n", (unsigned long)lb, s, i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return GC_store_debug_info((ptr_t)result, (word)lb, s, i); + return STORE_DEBUG_INFO(result, lb, "GC_debug_malloc_ignore_off_page"); } GC_API GC_ATTR_MALLOC void * GC_CALL @@ -581,33 +562,15 @@ GC_API GC_ATTR_MALLOC void * GC_CALL void * result = GC_malloc_atomic_ignore_off_page( SIZET_SAT_ADD(lb, DEBUG_BYTES)); - if (result == 0) { - GC_err_printf("GC_debug_malloc_atomic_ignore_off_page(%lu)" - " returning NULL (%s:%d)\n", (unsigned long)lb, s, i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return GC_store_debug_info((ptr_t)result, (word)lb, s, i); + return STORE_DEBUG_INFO(result, lb, + "GC_debug_malloc_atomic_ignore_off_page"); } STATIC void * GC_debug_generic_malloc(size_t lb, int knd, GC_EXTRA_PARAMS) { void * result = GC_generic_malloc(SIZET_SAT_ADD(lb, DEBUG_BYTES), knd); - if (NULL == result) { - GC_err_printf( - "GC_debug_generic_malloc(%lu, %d) returning NULL (%s:%d)\n", - (unsigned long)lb, knd, s, i); - return NULL; - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return GC_store_debug_info((ptr_t)result, (word)lb, s, i); + return STORE_DEBUG_INFO(result, lb, "GC_debug_generic_malloc"); } #ifdef DBG_HDRS_ALL @@ -658,16 +621,7 @@ STATIC void * GC_debug_generic_malloc(size_t lb, int knd, GC_EXTRA_PARAMS) { void * result = GC_malloc_stubborn(SIZET_SAT_ADD(lb, DEBUG_BYTES)); - if (result == 0) { - GC_err_printf("GC_debug_malloc_stubborn(%lu)" - " returning NULL (%s:%d)\n", (unsigned long)lb, s, i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return GC_store_debug_info((ptr_t)result, (word)lb, s, i); + return STORE_DEBUG_INFO(result, lb, "GC_debug_malloc_stubborn"); } GC_API void GC_CALL GC_debug_change_stubborn(const void *p) @@ -721,16 +675,7 @@ GC_API GC_ATTR_MALLOC void * GC_CALL GC_debug_malloc_atomic(size_t lb, { void * result = GC_malloc_atomic(SIZET_SAT_ADD(lb, DEBUG_BYTES)); - if (result == 0) { - GC_err_printf("GC_debug_malloc_atomic(%lu) returning NULL (%s:%d)\n", - (unsigned long)lb, s, i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return GC_store_debug_info((ptr_t)result, (word)lb, s, i); + return STORE_DEBUG_INFO(result, lb, "GC_debug_malloc_atomic"); } GC_API GC_ATTR_MALLOC char * GC_CALL GC_debug_strdup(const char *str, @@ -801,16 +746,7 @@ GC_API GC_ATTR_MALLOC void * GC_CALL GC_debug_malloc_uncollectable(size_t lb, void * result = GC_malloc_uncollectable( SIZET_SAT_ADD(lb, UNCOLLECTABLE_DEBUG_BYTES)); - if (result == 0) { - GC_err_printf("GC_debug_malloc_uncollectable(%lu)" - " returning NULL (%s:%d)\n", (unsigned long)lb, s, i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return GC_store_debug_info((ptr_t)result, (word)lb, s, i); + return STORE_DEBUG_INFO(result, lb, "GC_debug_malloc_uncollectable"); } #ifdef GC_ATOMIC_UNCOLLECTABLE @@ -820,16 +756,8 @@ GC_API GC_ATTR_MALLOC void * GC_CALL GC_debug_malloc_uncollectable(size_t lb, void * result = GC_malloc_atomic_uncollectable( SIZET_SAT_ADD(lb, UNCOLLECTABLE_DEBUG_BYTES)); - if (result == 0) { - GC_err_printf("GC_debug_malloc_atomic_uncollectable(%lu)" - " returning NULL (%s:%d)\n", (unsigned long)lb, s, i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return GC_store_debug_info((ptr_t)result, (word)lb, s, i); + return STORE_DEBUG_INFO(result, lb, + "GC_debug_malloc_atomic_uncollectable"); } #endif /* GC_ATOMIC_UNCOLLECTABLE */ diff --git a/gcj_mlc.c b/gcj_mlc.c index 55af7a03..35b3d3f1 100644 --- a/gcj_mlc.c +++ b/gcj_mlc.c @@ -223,9 +223,10 @@ GC_API GC_ATTR_MALLOC void * GC_CALL GC_debug_gcj_malloc(size_t lb, if (!GC_debugging_started) { GC_start_debugging_inner(); } - UNLOCK(); ADD_CALL_CHAIN(result, ra); - return GC_store_debug_info((ptr_t)result, (word)lb, s, i); + result = GC_store_debug_info_inner(result, (word)lb, s, i); + UNLOCK(); + return result; } /* There is no THREAD_LOCAL_ALLOC for GC_gcj_malloc_ignore_off_page(). */ diff --git a/include/private/dbg_mlc.h b/include/private/dbg_mlc.h index 02c95113..69cf95c3 100644 --- a/include/private/dbg_mlc.h +++ b/include/private/dbg_mlc.h @@ -123,8 +123,7 @@ typedef struct { #define SIMPLE_ROUNDED_UP_WORDS(n) BYTES_TO_WORDS((n) + WORDS_TO_BYTES(1) - 1) /* ADD_CALL_CHAIN stores a (partial) call chain into an object */ -/* header. It may be called with or without the allocation */ -/* lock. */ +/* header; it should be called with the allocation lock held. */ /* PRINT_CALL_CHAIN prints the call chain stored in an object */ /* to stderr. It requires that we do not hold the lock. */ #if defined(SAVE_CALL_CHAIN) diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index 002945c1..1d682a94 100644 --- a/include/private/gc_priv.h +++ b/include/private/gc_priv.h @@ -2392,9 +2392,9 @@ GC_INNER void GC_start_debugging_inner(void); /* defined in dbg_mlc.c. */ /* Should not be called if GC_debugging_started. */ /* Store debugging info into p. Return displaced pointer. */ -/* Assumes we don't hold allocation lock. */ -GC_INNER ptr_t GC_store_debug_info(ptr_t p, word sz, const char *str, - int linenum); +/* Assumes we hold the allocation lock. */ +GC_INNER void *GC_store_debug_info_inner(void *p, word sz, const char *str, + int linenum); #ifdef REDIRECT_MALLOC # ifdef GC_LINUX_THREADS diff --git a/os_dep.c b/os_dep.c index 75dc95b8..b10e9e99 100644 --- a/os_dep.c +++ b/os_dep.c @@ -4664,6 +4664,12 @@ GC_INNER void GC_save_callers(struct callinfo info[NFRAMES]) } GC_in_save_callers = TRUE; # endif + + GC_ASSERT(I_HOLD_LOCK()); + /* backtrace may call dl_iterate_phdr which is also */ + /* used by GC_register_dynamic_libraries, and */ + /* dl_iterate_phdr is not guaranteed to be reentrant. */ + GC_STATIC_ASSERT(sizeof(struct callinfo) == sizeof(void *)); npcs = backtrace((void **)tmp_info, NFRAMES + IGNORE_FRAMES); if (npcs > IGNORE_FRAMES)