From d9df01a6829fac03bb35469895b17c1ab5adb828 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Fri, 17 Nov 2017 11:07:03 +0300 Subject: [PATCH] Eliminate TSan warning about data race when accessing GC_debugging_started Now GC_debugging_started variable is double-checked with the allocation lock held before calling GC_start_debugging_inner. * dbg_mlc.c (GC_start_debugging_inner): Define as GC_INNER (instead of STATIC). * dbg_mlc.c [THREADS] (GC_start_debugging): Define as STATIC; do not call GC_start_debugging_inner() if GC_debugging_started. * dbg_mlc.c [!THREADS] (GC_start_debugging): Define as macro (redirect to GC_start_debugging_inner). * gcj_mlc.c (GC_debug_gcj_malloc): Call GC_start_debugging_inner (holding the lock) instead of GC_start_debugging. * include/private/gc_priv.h (GC_start_debugging): Rename to GC_start_debugging_inner; improve usage comment. --- dbg_mlc.c | 21 +++++++++++++-------- gcj_mlc.c | 4 ++-- include/private/gc_priv.h | 3 ++- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/dbg_mlc.c b/dbg_mlc.c index c22435ae..085ba2e7 100644 --- a/dbg_mlc.c +++ b/dbg_mlc.c @@ -465,7 +465,7 @@ STATIC void GC_debug_print_heap_obj_proc(ptr_t p) STATIC void GC_do_nothing(void) {} #endif -STATIC void GC_start_debugging_inner(void) +GC_INNER void GC_start_debugging_inner(void) { GC_ASSERT(I_HOLD_LOCK()); # ifndef SHORT_DBG_HDRS @@ -480,14 +480,19 @@ STATIC void GC_start_debugging_inner(void) GC_register_displacement_inner((word)sizeof(oh)); } -GC_INNER void GC_start_debugging(void) -{ - DCL_LOCK_STATE; +#ifdef THREADS + STATIC void GC_start_debugging(void) + { + DCL_LOCK_STATE; - LOCK(); - GC_start_debugging_inner(); - UNLOCK(); -} + 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); diff --git a/gcj_mlc.c b/gcj_mlc.c index 7b4ea9da..bcd95b05 100644 --- a/gcj_mlc.c +++ b/gcj_mlc.c @@ -219,10 +219,10 @@ GC_API GC_ATTR_MALLOC 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(); } + UNLOCK(); ADD_CALL_CHAIN(result, ra); return (GC_store_debug_info(result, (word)lb, s, i)); } diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index dcb7e5d8..83298d67 100644 --- a/include/private/gc_priv.h +++ b/include/private/gc_priv.h @@ -2318,7 +2318,8 @@ 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. */ -- 2.40.0