Ivan Maidanski [Fri, 15 Dec 2017 07:15:05 +0000 (10:15 +0300)]
Workaround TSan hang in free_inner when called from at-fork child handler
* pthread_support.c [CAN_HANDLE_FORK] (GC_remove_all_threads_but_me):
Do not call GC_INTERNAL_FREE(p) if THREAD_SANITIZER and CAN_CALL_ATFORK;
add comment.
Ivan Maidanski [Thu, 14 Dec 2017 07:24:34 +0000 (10:24 +0300)]
Workaround TSan false positive in remove_all_threads_but_me
* pthread_support.c [CAN_HANDLE_FORK] (store_to_threads_table): New
static function (defined with GC_ATTR_NO_SANITIZE_THREAD attribute
if CAN_CALL_ATFORK); add comment.
* pthread_support.c [CAN_CALL_ATFORK] (GC_remove_all_threads_but_me):
Call store_to_threads_table instead of GC_threads[hv]=me.
Ivan Maidanski [Tue, 12 Dec 2017 16:24:02 +0000 (19:24 +0300)]
Fix gctest failure if compiled with TSan and parallel marker
As of clang-4.0, Thread Sanitizer does not support creation of threads
in the forked process (before exec). So, GC_start_mark_threads()
is a no-op now if TSan is enabled.
* misc.c [THREADS && PARALLEL_MARK && CAN_HANDLE_FORK]
(GC_start_mark_threads): Do not call GC_start_mark_threads_inner()
if THREAD_SANITIZER; add comment.
Fix marking of disclaim-reachable objects in the incremental mode
Issue #192 (bdwgc).
Unconditional marking is done analogously to marking from uncollectible
blocks, since they are meant to secure access to data for disclaim
notifiers after the data is no longer marked.
* mark.c [ENABLE_DISCLAIM] (GC_push_next_marked_dirty): Honor
MARK_UNCONDITIONALLY when rescanning dirty blocks during incremental
marking.
Ivan Maidanski [Fri, 1 Dec 2017 16:38:02 +0000 (19:38 +0300)]
Fix data race in do_local_mark when comparing active_count to helper_count
* mark.c [PARALLEL_MARK] (has_inactive_helpers): New static function
(which compares GC_active_count to GC_helper_count with the mark lock
held).
* mark.c [PARALLEL_MARK && GC_ASSERTIONS] (GC_do_local_mark): Remove
GC_acquire_mark_lock() with immediate GC_acquire_mark_unlock(); add
the appropriate comment to the function description instead.
* mark.c [PARALLEL_MARK] (GC_do_local_mark): Call has_inactive_helpers()
instead of GC_active_count<GC_helper_count; do not call
has_inactive_helpers() unless local_top>local_mark_stack+1.
Ivan Maidanski [Fri, 1 Dec 2017 08:48:08 +0000 (11:48 +0300)]
Workaround TSan warning about data race in generic_malloc_many
It is acceptable to update GC_bytes_found counter without
synchronization (so, it is OK to have inaccurate value in
GC_bytes_found in favor of performance).
* mallocx.c [PARALLEL_MARK && THREAD_SANITIZER]
(GC_generic_malloc_many): Acquire the allocation lock to update
GC_bytes_found (after successful GC_reclaim_generic).
* mallocx.c [PARALLEL_MARK && !THREAD_SANITIZER]
(GC_generic_malloc_many): Move GC_bytes_found update into the block
executed with the mark lock held.
Ivan Maidanski [Wed, 29 Nov 2017 22:18:19 +0000 (01:18 +0300)]
Remove done_init static variable from fnlz_mlc.c
(code refactoring)
GC_init_finalized_malloc!=0 is used instead of boolean done_init.
* fnlz_mlc.c [ENABLE_DISCLAIM] (done_init): Remove static variable.
* fnlz_mlc.c [ENABLE_DISCLAIM] (GC_init_finalized_malloc,
GC_finalized_malloc): Use GC_finalized_kind instead of done_init.
* fnlz_mlc.c [ENABLE_DISCLAIM] (GC_init_finalized_malloc): Add
assertion that the result of GC_new_kind_inner() is non-zero.
Ivan Maidanski [Wed, 29 Nov 2017 18:26:28 +0000 (21:26 +0300)]
Add a sanity check that load_acquire and store_release are available
* pthread_stop_world.c [(!AO_HAVE_load_acquire
|| !AO_HAVE_store_release) && !CPPCHECK]: Issue error that
libatomic_ops does not define AO_load_acquire and/or AO_store_release
so the client should manually define AO_REQUIRE_CAS macro.
Ivan Maidanski [Wed, 29 Nov 2017 18:09:52 +0000 (21:09 +0300)]
Remove explicit case of TRUE/FALSE to AO_t in suspend/resume_thread
(fix commit ce09fd5)
* pthread_stop_world.c [GC_ENABLE_SUSPEND_THREAD
&& !GC_OPENBSD_UTHREADS && !NACL] (GC_suspend_thread,
GC_resume_thread): Do not case TRUE/FALSE to AO_t explicitly.
Ivan Maidanski [Wed, 29 Nov 2017 17:15:49 +0000 (20:15 +0300)]
Fix data race in make_descriptor when setting explicit_typing_initialized
(fix commit 5f350a0)
* typd_mlc.c [AO_HAVE_load_acquire] (GC_explicit_typing_initialized):
Add volatile qualifier.
* typd_mlc.c [AO_HAVE_load_acquire] (GC_make_descriptor): Remove
cast of &GC_explicit_typing_initialized; do not check THREADS macro;
fallback to locked checking of GC_explicit_typing_initialized if
AO_HAVE_store_release is not defined; use AO_store_release to set
GC_explicit_typing_initialized (to true).
* typd_mlc.c (GC_make_descriptor): Reformat code dealing with
GC_explicit_typing_initialized (check whether AO_HAVE_load_acquire is
defined only once).
Ivan Maidanski [Wed, 29 Nov 2017 08:46:05 +0000 (11:46 +0300)]
Eliminate TSan false positive for stop_info.stack_ptr (v2)
Without this patch, Thread Sanitizer reports a data race between
GC_has_other_debug_info() and the code which sets stop_info.stack_ptr.
* include/private/pthread_support.h [THREAD_SANITIZER]: Include
dbg_mlc.h file.
* include/private/pthread_support.h [THREAD_SANITIZER] (GC_Thread_Rep):
Add dummy field (as the first field of the structure).
* pthread_support.c [THREAD_SANITIZER && CPPCHECK] (GC_new_thread):
Call GC_noop1(first_thread.dummy[0]) (to suppress cppcheck warning
about unused field).
Ivan Maidanski [Wed, 29 Nov 2017 08:42:47 +0000 (11:42 +0300)]
Eliminate 'this statement may fall through' GCC warnings
* cord/cordprnt.c (extract_conv_spec): Eliminate fall through in
a switch statement.
* cord/tests/de.c (do_command): Replace "fall through:" comment with
"FALLTHRU" formal comment (on a new line).
* win32_threads.c [!GC_PTHREADS && !GC_NO_THREADS_DISCOVERY]
(GC_DllMain): Likewise.
Ivan Maidanski [Thu, 23 Nov 2017 23:21:46 +0000 (02:21 +0300)]
Fix lack of barriers to synchronize memory for suspend_handler
pthread_kill is not on the list of functions which synchronize memory.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_world_is_stopped): Reformat comment.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_suspend_handler_inner): Use AO_load_acquire instead of AO_load
to fetch the value of GC_stop_count; add comment.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_start_world): Use AO_store_release instead of AO_store to reset
GC_world_is_stopped; add comment.
Ivan Maidanski [Thu, 23 Nov 2017 22:16:50 +0000 (01:16 +0300)]
Eliminate TSan false positive related to stop_info.stack_ptr access
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_store_stack_ptr): New inline function definition (move the code
from GC_suspend_handler_inner but stop_info.stack_ptr is now stored
atomically); add comment.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_suspend_handler_inner): Call GC_store_stack_ptr().
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_push_all_stacks): Use AO_load to fetch p->stop_info.stack_ptr.
Ivan Maidanski [Thu, 23 Nov 2017 17:15:54 +0000 (20:15 +0300)]
Fix data race in last_stop_count access (suspend_handler_inner)
* include/private/pthread_stop_world.h [!GC_OPENBSD_UTHREADS]
(thread_stop_info.last_stop_count): Do not define if NACL; change the
type from word to AO_t; add volatile qualifier; fix a typo in comment
("GC_stop_count").
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(update_last_stop_count): Remove.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_suspend_handler_inner): Replace update_last_stop_count() call with
AO_store_release(&me->stop_info.last_stop_count,my_stop_count).
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_suspend_all): Replace p->stop_info.last_stop_count with
AO_load(&p->stop_info.last_stop_count).
Ivan Maidanski [Wed, 22 Nov 2017 08:35:08 +0000 (11:35 +0300)]
Workaround TSan false positive in lookup_thread called by suspend_handler
* include/private/pthread_support.h (THREAD_TABLE_INDEX): Move the
macro from pthread_support.c file.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL
&& THREAD_SANITIZER] (GC_lookup_thread_async): New static function
(same implementation as of GC_lookup_thread but with
GC_ATTR_NO_SANITIZE_THREAD attribute); add comment.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL
&& !THREAD_SANITIZER] (GC_lookup_thread_async): Define as macro.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_suspend_handler_inner): Call GC_lookup_thread_async() instead of
GC_lookup_thread(); move the corresponding comment to
GC_lookup_thread_async.
Ivan Maidanski [Fri, 17 Nov 2017 17:36:47 +0000 (20:36 +0300)]
Allow to turn on spin locking even if thread-local allocations are used
* include/private/gc_locks.h [GC_PTHREADS && !GC_WIN32_THREADS
&& THREAD_LOCAL_ALLOC && !USE_PTHREAD_LOCKS] (UNCOND_LOCK,
UNCOND_UNLOCK): Use spin locks if USE_SPIN_LOCK is defined by client.
Ivan Maidanski [Fri, 17 Nov 2017 16:41:30 +0000 (19:41 +0300)]
Eliminate TSan false positive for stop_info.stack_ptr
Without this patch, Thread Sanitizer reports a data race between
GC_has_other_debug_info() and the code which sets stop_info.stack_ptr.
* include/private/pthread_support.h (GC_Thread_Rep.status): Move the
field to be before stop_info one (so, the latter now has the offset
of at least 3 in words, while oh.oh_sz has the offset of 2 in words
(when KEEP_BACK_PTRS, MAKE_BACK_GRAPH and NEED_CALLINFO are not
defined)).
Ivan Maidanski [Fri, 17 Nov 2017 08:07:03 +0000 (11:07 +0300)]
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.
Ivan Maidanski [Wed, 15 Nov 2017 22:15:14 +0000 (01:15 +0300)]
Workaround TSan false positives in extend_size_map
Thread Sanitizer reports data races between fill_size_map (called from
GC_extend_size_map) and GC_gcj_malloc[_ignore_off_page],
GC_malloc_kind_global, GC_generic_malloc_uncollectable,
GC_malloc_explicitly_typed_ignore_off_page which read a value from
GC_size_map before acquiring the allocation lock. These races could
be ignored as the value is verified after acquiring the lock.
* alloc.c: Refine comment about GC_allocobj (and GC_size_map) usage
for the case of a multi-threaded environment.
* malloc.c (fill_size_map): New static function (with
GC_ATTR_NO_SANITIZE_THREAD attribute).
* malloc.c (GC_extend_size_map): Use fill_size_map() to fill in
a region of GC_size_map.
Ivan Maidanski [Fri, 10 Nov 2017 17:07:16 +0000 (20:07 +0300)]
Fix data race when getting object size in explicitly-typed allocators
* typd_mlc.c (COMPLEX): Reformat comment.
* typd_mlc.c (GC_malloc_explicitly_typed,
GC_malloc_explicitly_typed_ignore_off_page, GC_calloc_explicitly_typed):
Always use BYTES_TO_GRANULES(GC_size(op)) instead of GC_size_map[lb] to
determine size of the allocated object in granules (because the value
of GC_size_map[lb] might be updated by another thread since the value
use in GC_malloc_kind); add comment.
Ivan Maidanski [Fri, 10 Nov 2017 15:14:55 +0000 (18:14 +0300)]
Workaround TSan false positive in write fault handler
The data races reported between GC_write_fault_handler,
catch_exception_raise and GC_install_header, GC_remove_header are OK
as the added or removed header should be already unprotected.
* os_dep.c [MPROTECT_VDB && THREADS] (is_header_found_async): New
static function (with GC_ATTR_NO_SANITIZE_THREAD attribute).
* os_dep.c [MPROTECT_VDB && !THREADS] (is_header_found_async): New
macro.
* os_dep.c [MPROTECT_VDB && !DARWIN] (GC_write_fault_handler): Use
is_header_found_async() instead of HDR(addr)!=0.
* os_dep.c [MPROTECT_VDB && DARWIN] (catch_exception_raise): Likewise.
Paul Bone [Fri, 10 Nov 2017 07:12:57 +0000 (10:12 +0300)]
Add basic calculation of the total full-collection time
Issue #139 (bdwgc).
New API functions: GC_start_performance_measurement,
GC_get_full_gc_total_time.
This patch is based on code originally written by Zoltan Somogyi
on 2008-03-18.
* alloc.c [!NO_CLOCK] (full_gc_total_time, measure_performance): New
static variable definition; add comment.
* alloc.c [!NO_CLOCK] (GC_start_performance_measurement,
GC_get_full_gc_total_time): New API function definition.
* alloc.c [!NO_CLOCK] (GC_try_to_collect_inner): Declare
start_time_valid local variable; set start_time_valid to true if
GET_TIME(start_time) is called; call GET_TIME(start_time) also if
measure_performance; declare time_diff local variable (used to store
the result of MS_TIME_DIFF()); GET_TIME(current_time) is called only
if start_time_valid; update full_gc_total_time if measure_performance.
* include/gc.h (GC_start_performance_measurement,
GC_get_full_gc_total_time): New API function declaration.
* tests/test.c (INIT_PERF_MEASUREMENT): New macro.
* tests/test.c (GC_COND_INIT): Call INIT_PERF_MEASUREMENT.
* tests/test.c [!NO_CLOCK] (check_heap_stats): Call
GC_get_full_gc_total_time() and print the total time of full
collections.
Ivan Maidanski [Fri, 3 Nov 2017 18:30:23 +0000 (21:30 +0300)]
Fix double lock in a_set(f(a_get())) expression in gctest
(fix commit 8f5746e)
* tests/test.c [!AO_HAVE_store_release] (AO_store_release): Replace
macro with a static function definition (so, new_val argument is
evaluated before acquiring the lock).
Ivan Maidanski [Fri, 3 Nov 2017 08:03:04 +0000 (11:03 +0300)]
Avoid data race in finalized_count (gctest)
* tests/test.c (FINALIZER_LOCK, FINALIZER_UNLOCK): New macro.
* tests/test.c [GC_PTHREADS] (incr_lock): Move static variable out
of finalizer() and mktree() (use a single lock variable to access
finalized_count in finalizer, mktree and tree_test).
* tests/test.c (dropped_something): Change type back to int; remove
volatile.
* tests/test.c (finalizer, mktree): Use FINALIZER_[UN]LOCK() instead
of PCR_ThCrSec_Enter/ExitSys(), pthread_mutex_[un]lock(),
Enter/LeaveCriticalSection().
* tests/test.c (tree_test): Use FINALIZER_[UN]LOCK() to avoid data
race when accessing finalized_count and dropped_something; do not use
AO_load() and AO_store() to access dropped_something.
* tests/test.c
Because it does not avoid potential data race between GC_malloc
(invoked from cons) and check_ints; the proper solution seems to
just use AO_load_acquire and AO_store_release to access A.aa.
Ivan Maidanski [Mon, 30 Oct 2017 22:09:56 +0000 (01:09 +0300)]
Workaround TSan false positive in invoke_finalizers
* finalize.c [THREADS && !THREAD_SANITIZER] (GC_invoke_finalizers): Do
not compare bytes_freed_before to GC_bytes_freed without the lock; add
comment.
* include/private/gc_priv.h (_GC_arrays._finalizer_bytes_freed): Replace
"mem." to "memory" in a comment.
Ivan Maidanski [Wed, 25 Oct 2017 08:45:38 +0000 (11:45 +0300)]
Do not use system clock consistently if NO_CLOCK
Issue #139 (bdwgc).
* alloc.c (GC_try_to_collect_inner, world_stopped_total_time,
world_stopped_total_divisor, MAX_TOTAL_TIME_DIVISOR, GC_stopped_mark):
Replace "ifndef SMALL_CONFIG" with "ifndef NO_CLOCK".
* reclaim.c (GC_reclaim_all): Likewise.
* alloc.c (GC_finish_collection): Replace "ifndef SMALL_CONFIG" with
"ifndef NO_CLOCK" except for GC_print_finalization_stats invocation.
* doc/README.macros (NO_CLOCK): Document.
* include/private/gc_priv.h (CLOCK_TYPE, GET_TIME, MS_TIME_DIFF): Do
not define if NO_CLOCK.
* include/private/gc_priv.h (GC_print_stats): Define as a macro (to 0)
only if both NO_CLOCK and SMALL_CONFIG are defined.
* misc.c (GC_print_stats): Do not define if both NO_CLOCK and
SMALL_CONFIG are defined.
* misc.c (GC_init): Do not set GC_print_stats only if both NO_CLOCK
and SMALL_CONFIG are defined.
* tests/test.c (run_one_test): Do not define start_time, reverse_time,
time_diff local variables (and do not use GET_TIME, MS_TIME_DIFF) if
NO_CLOCK is defined.
Ivan Maidanski [Tue, 24 Oct 2017 21:06:22 +0000 (00:06 +0300)]
Make extend_size_map() static
(code refactoring)
* include/private/gc_priv.h (GC_extend_size_map): Remove prototype.
* malloc.c (GC_extend_size_map): Move the function definition from
misc.c; change GC_INNER to STATIC; reformat code; remove j and
much_smaller_than_i local variables; add assertions that I_HOLD_LOCK
and GC_size_map[i]==0; remove comment that GC_size_map[i] is expected
to be zero.
* malloc.c (GC_generic_malloc_inner): Capitalize the first letter of
the description comment.
* misc.c (GC_extend_size_map): Remove the definition.