From d3870360c2bc117bd9363642b8bfa5455e42009b Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Wed, 18 Apr 2018 00:20:46 +0300 Subject: [PATCH] Fix the collector hang when it is configured with --enable-gc-debug (back-port of commit 7d6106bc from 'release-7_4') 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; change p type from ptr_t to void*. * dbg_mlc.c [GC_ASSERTIONS] (GC_store_debug_info_inner): Cast p to ptr_t when passed to CROSSES_HBLK(). * 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_inner): Define as GC_INNER (instead of STATIC); call GC_register_displacement_inner() instead of GC_register_displacement(). * 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; use OPT_RA. * 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. * gcj_mlc.c (GC_debug_gcj_malloc): Call GC_start_debugging_inner (holding the lock) instead of GC_start_debugging. * 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*. * include/private/gc_priv.h (GC_start_debugging): Rename to GC_start_debugging_inner; improve usage comment. * os_dep.c [SAVE_CALL_CHAIN] (GC_save_callers): Add assertion that the allocation lock is held; add comment. --- dbg_mlc.c | 130 ++++++++++---------------------------- gcj_mlc.c | 7 +- include/private/dbg_mlc.h | 3 +- include/private/gc_priv.h | 9 +-- os_dep.c | 6 ++ 5 files changed, 50 insertions(+), 105 deletions(-) diff --git a/dbg_mlc.c b/dbg_mlc.c index 997f53be..21d6c59a 100644 --- a/dbg_mlc.c +++ b/dbg_mlc.c @@ -248,15 +248,13 @@ # 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 ptr_t GC_store_debug_info_inner(ptr_t p, word sz, const char *string, - int linenum) +GC_INNER void *GC_store_debug_info_inner(void *p, word sz, + const char *string, int linenum) { word * result = (word *)((oh *)p + 1); GC_ASSERT(GC_size(p) >= sizeof(oh) + sz); - GC_ASSERT(!(SMALL_OBJ(sz) && CROSSES_HBLK(p, sz))); + GC_ASSERT(!(SMALL_OBJ(sz) && CROSSES_HBLK((ptr_t)p, sz))); # ifdef KEEP_BACK_PTRS ((oh *)p) -> oh_back_ptr = HIDE_BACK_PTR(NOT_MARKED); # endif @@ -274,14 +272,24 @@ STATIC ptr_t GC_store_debug_info_inner(ptr_t p, word sz, const char *string, return((ptr_t)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. */ +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 = 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; } @@ -433,7 +441,7 @@ STATIC void GC_debug_print_heap_obj_proc(ptr_t p) STATIC void GC_do_nothing(void) {} #endif -GC_INNER void GC_start_debugging(void) +GC_INNER void GC_start_debugging_inner(void) { # ifndef SHORT_DBG_HDRS GC_check_heap = GC_check_heap_proc; @@ -444,7 +452,7 @@ GC_INNER void GC_start_debugging(void) # endif GC_print_heap_obj = GC_debug_print_heap_obj_proc; GC_debugging_started = TRUE; - GC_register_displacement((word)sizeof(oh)); + GC_register_displacement_inner((word)sizeof(oh)); } size_t GC_debug_header_size = sizeof(oh); @@ -489,18 +497,7 @@ GC_API void * GC_CALL GC_debug_malloc(size_t lb, GC_EXTRA_PARAMS) GC_caller_func_offset(ra, &s, &i); } # endif - if (result == 0) { - GC_err_printf("GC_debug_malloc(%lu) returning NULL (", - (unsigned long) lb); - GC_err_puts(s); - GC_err_printf(":%ld)\n", (unsigned long)i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return (GC_store_debug_info(result, (word)lb, s, i)); + return store_debug_info(result, lb, "GC_debug_malloc", OPT_RA s, i); } GC_API void * GC_CALL GC_debug_malloc_ignore_off_page(size_t lb, @@ -508,18 +505,8 @@ GC_API void * GC_CALL GC_debug_malloc_ignore_off_page(size_t lb, { 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 (", - (unsigned long) lb); - GC_err_puts(s); - GC_err_printf(":%lu)\n", (unsigned long)i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return (GC_store_debug_info(result, (word)lb, s, i)); + return store_debug_info(result, lb, "GC_debug_malloc_ignore_off_page", + OPT_RA s, i); } GC_API void * GC_CALL GC_debug_malloc_atomic_ignore_off_page(size_t lb, @@ -528,18 +515,9 @@ GC_API void * GC_CALL GC_debug_malloc_atomic_ignore_off_page(size_t lb, 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 (", (unsigned long)lb); - GC_err_puts(s); - GC_err_printf(":%lu)\n", (unsigned long)i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return (GC_store_debug_info(result, (word)lb, s, i)); + return store_debug_info(result, lb, + "GC_debug_malloc_atomic_ignore_off_page", + OPT_RA s, i); } #ifdef DBG_HDRS_ALL @@ -583,18 +561,8 @@ GC_API void * GC_CALL GC_debug_malloc_atomic_ignore_off_page(size_t lb, { void * result = GC_malloc_stubborn(SIZET_SAT_ADD(lb, DEBUG_BYTES)); - if (result == 0) { - GC_err_printf("GC_debug_malloc(%lu) returning NULL (", - (unsigned long) lb); - GC_err_puts(s); - GC_err_printf(":%lu)\n", (unsigned long)i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return (GC_store_debug_info(result, (word)lb, s, i)); + return store_debug_info(result, lb, "GC_debug_malloc_stubborn", + OPT_RA s, i); } GC_API void GC_CALL GC_debug_change_stubborn(void *p) @@ -649,18 +617,8 @@ GC_API void * GC_CALL GC_debug_malloc_atomic(size_t lb, GC_EXTRA_PARAMS) { void * result = GC_malloc_atomic(SIZET_SAT_ADD(lb, DEBUG_BYTES)); - if (result == 0) { - GC_err_printf("GC_debug_malloc_atomic(%lu) returning NULL (", - (unsigned long) lb); - GC_err_puts(s); - GC_err_printf(":%lu)\n", (unsigned long)i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return (GC_store_debug_info(result, (word)lb, s, i)); + return store_debug_info(result, lb, "GC_debug_malloc_atomic", + OPT_RA s, i); } GC_API char * GC_CALL GC_debug_strdup(const char *str, GC_EXTRA_PARAMS) @@ -733,18 +691,8 @@ GC_API 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 (", - (unsigned long) lb); - GC_err_puts(s); - GC_err_printf(":%lu)\n", (unsigned long)i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return (GC_store_debug_info(result, (word)lb, s, i)); + return store_debug_info(result, lb, "GC_debug_malloc_uncollectable", + OPT_RA s, i); } #ifdef ATOMIC_UNCOLLECTABLE @@ -754,19 +702,9 @@ GC_API 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 (", - (unsigned long) lb); - GC_err_puts(s); - GC_err_printf(":%lu)\n", (unsigned long)i); - return(0); - } - if (!GC_debugging_started) { - GC_start_debugging(); - } - ADD_CALL_CHAIN(result, ra); - return (GC_store_debug_info(result, (word)lb, s, i)); + return store_debug_info(result, lb, + "GC_debug_malloc_atomic_uncollectable", + OPT_RA s, i); } #endif /* ATOMIC_UNCOLLECTABLE */ diff --git a/gcj_mlc.c b/gcj_mlc.c index e41c8345..7142c6fe 100644 --- a/gcj_mlc.c +++ b/gcj_mlc.c @@ -228,12 +228,13 @@ GC_API void * GC_CALL GC_debug_gcj_malloc(size_t lb, return((*oom_fn)(lb)); } *((void **)((ptr_t)result + sizeof(oh))) = ptr_to_struct_containing_descr; - UNLOCK(); if (!GC_debugging_started) { - GC_start_debugging(); + GC_start_debugging_inner(); } ADD_CALL_CHAIN(result, ra); - return (GC_store_debug_info(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 2ffbc5e7..8e08a1a9 100644 --- a/include/private/dbg_mlc.h +++ b/include/private/dbg_mlc.h @@ -121,8 +121,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 e0e144aa..103999eb 100644 --- a/include/private/gc_priv.h +++ b/include/private/gc_priv.h @@ -2118,12 +2118,13 @@ GC_INNER void GC_initialize_offsets(void); /* defined in obj_map.c */ GC_INNER void GC_bl_init(void); GC_INNER void GC_bl_init_no_interiors(void); /* defined in blacklst.c */ -GC_INNER void GC_start_debugging(void); /* defined in dbg_mlc.c */ +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 c2cbf802..47f3e7b2 100644 --- a/os_dep.c +++ b/os_dep.c @@ -4535,6 +4535,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); BCOPY(tmp_info+IGNORE_FRAMES, info, (npcs - IGNORE_FRAMES) * sizeof(void *)); -- 2.40.0