Skip to content

Commit

Permalink
Fix double lock in GC_malloc called from backtrace()
Browse files Browse the repository at this point in the history
Issue #613 (bdwgc).

As backtrace() might call a redirected malloc, we need to release the
allocator lock temporarily before backtrace() call, and we should not
call backtrace() at all for the cases where it is not possible to
release the allocator lock temporarily (i.e. for internal allocations).

* dbg_mlc.c [DBG_HDRS_ALL] (GC_debug_generic_malloc_inner): Call
ADD_CALL_CHAIN_INNER() instead of ADD_CALL_CHAIN().
* include/private/dbg_mlc.h [SAVE_CALL_CHAIN && REDIRECT_MALLOC
&& THREADS && DBG_HDRS_ALL && NARGS==0 && NFRAMES%2==0
&& GC_HAVE_BUILTIN_BACKTRACE] (GC_save_callers_no_unlock): Declare
function.
* include/private/dbg_mlc.h [DBG_HDRS_ALL] (ADD_CALL_CHAIN_INNER):
Define.
* os_dep.c [NEED_CALLINFO && SAVE_CALL_CHAIN && NARGS==0
&& NFRAMES%2==0 && GC_HAVE_BUILTIN_BACKTRACE && REDIRECT_MALLOC
&& THREADS && DBG_HDRS_ALL]: Include dbg_mlc.h.
* os_dep.c [NEED_CALLINFO && SAVE_CALL_CHAIN && NARGS==0
&& NFRAMES%2==0 && GC_HAVE_BUILTIN_BACKTRACE && REDIRECT_MALLOC
&& THREADS && DBG_HDRS_ALL] (GC_save_callers_no_unlock): Implement.
* os_dep.c [NEED_CALLINFO && SAVE_CALL_CHAIN && NARGS==0
&& NFRAMES%2==0 && GC_HAVE_BUILTIN_BACKTRACE && REDIRECT_MALLOC]
(GC_save_callers): Wrap backtrace() into UNLOCK/LOCK(); add comment.
  • Loading branch information
ivmai committed Mar 19, 2024
1 parent f4f943b commit b37ca67
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 4 deletions.
2 changes: 1 addition & 1 deletion dbg_mlc.c
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ STATIC void * GC_debug_generic_malloc(size_t lb, int k, GC_EXTRA_PARAMS)
if (!GC_debugging_started)
GC_start_debugging_inner();
result = GC_store_debug_info_inner(base, (word)lb, "INTERNAL", 0);
ADD_CALL_CHAIN(base, GC_RETURN_ADDR);
ADD_CALL_CHAIN_INNER(base);
return result;
}
#endif /* DBG_HDRS_ALL */
Expand Down
11 changes: 11 additions & 0 deletions include/private/dbg_mlc.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ typedef struct {
/* to stderr; it requires we do not hold the allocator lock. */
#if defined(SAVE_CALL_CHAIN)
# define ADD_CALL_CHAIN(base, ra) GC_save_callers(((oh *)(base)) -> oh_ci)
# if defined(REDIRECT_MALLOC) && defined(THREADS) && defined(DBG_HDRS_ALL) \
&& NARGS == 0 && NFRAMES % 2 == 0 && defined(GC_HAVE_BUILTIN_BACKTRACE)
GC_INNER void GC_save_callers_no_unlock(struct callinfo info[NFRAMES]);
# define ADD_CALL_CHAIN_INNER(base) \
GC_save_callers_no_unlock(((oh *)(base)) -> oh_ci)
# endif
# define PRINT_CALL_CHAIN(base) GC_print_callers(((oh *)(base)) -> oh_ci)
#elif defined(GC_ADD_CALLER)
# define ADD_CALL_CHAIN(base, ra) ((oh *)(base)) -> oh_ci[0].ci_pc = (ra)
Expand All @@ -135,6 +141,11 @@ typedef struct {
# define PRINT_CALL_CHAIN(base)
#endif

#if !defined(ADD_CALL_CHAIN_INNER) && defined(DBG_HDRS_ALL)
/* A variant of ADD_CALL_CHAIN() used for internal allocations. */
# define ADD_CALL_CHAIN_INNER(base) ADD_CALL_CHAIN(base, GC_RETURN_ADDR)
#endif

#ifdef GC_ADD_CALLER
# define OPT_RA ra,
#else
Expand Down
24 changes: 21 additions & 3 deletions os_dep.c
Original file line number Diff line number Diff line change
Expand Up @@ -5251,7 +5251,21 @@ GC_API int GC_CALL GC_get_pages_executable(void)
/* Deal with possible malloc calls in backtrace by omitting */
/* the infinitely recursing backtrace. */
STATIC GC_bool GC_in_save_callers = FALSE;
# endif

# if defined(THREADS) && defined(DBG_HDRS_ALL)
# include "private/dbg_mlc.h"

/* A dummy version of GC_save_callers() which does not call */
/* backtrace(). */
GC_INNER void GC_save_callers_no_unlock(
struct callinfo info[NFRAMES])
{
GC_ASSERT(I_HOLD_LOCK());
info[0].ci_pc = (word)(&GC_save_callers_no_unlock);
BZERO(&info[1], sizeof(void *) * (NFRAMES - 1));
}
# endif
# endif /* REDIRECT_MALLOC */

GC_INNER void GC_save_callers(struct callinfo info[NFRAMES])
{
Expand All @@ -5271,11 +5285,15 @@ GC_API int GC_CALL GC_get_pages_executable(void)
return;
}
GC_in_save_callers = TRUE;
/* backtrace() might call a redirected malloc. */
UNLOCK();
npcs = backtrace((void **)tmp_info, NFRAMES + 1);
LOCK();
# else
npcs = backtrace((void **)tmp_info, NFRAMES + 1);
# endif

/* We retrieve NFRAMES+1 pc values, but discard the first one, */
/* since it points to our own frame. */
npcs = backtrace((void **)tmp_info, NFRAMES + 1);
i = 0;
if (npcs > 1) {
i = npcs - 1;
Expand Down

0 comments on commit b37ca67

Please sign in to comment.