Eliminate unsigned fl_builder_count underflow in mark_thread

(refactor commit 0ca6d3f)

* include/private/gc_priv.h [PARALLEL_MARK] (GC_fl_builder_count):
Change type from word to signed_word.
* reclaim.c [PARALLEL_MARK] (GC_fl_builder_count): Likewise.
* mark.c [PARALLEL_MARK] (GC_wait_for_markers_init): Change type of
count local variable to signed_word; add assertion that count is
non-negative.
* pthread_support.c [PARALLEL_MARK] (GC_mark_thread): Add comment that
GC_fl_builder_count can be negative here.
* win32_threads.c [PARALLEL_MARK] (GC_mark_thread): Likewise.
* reclaim.c [PARALLEL_MARK] (GC_fl_builder_count): Refine comment.

diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h
index 804d094857de5fac231b03774e64518ba6ce3694..3a6eca632c1f6128d590009b5685eeb7e5453543 100644 (file)
--- a/include/private/gc_priv.h
+++ b/include/private/gc_priv.h
@@ -2481,7 +2481,7 @@ GC_INNER ptr_t GC_store_debug_info(ptr_t p, word sz, const char *str,
   GC_INNER void GC_notify_all_builder(void);
   GC_INNER void GC_wait_for_reclaim(void);
 
-  GC_EXTERN word GC_fl_builder_count;   /* Protected by mark lock.      */
+  GC_EXTERN signed_word GC_fl_builder_count; /* Protected by mark lock. */
 
   GC_INNER void GC_notify_all_marker(void);
   GC_INNER void GC_wait_marker(void);

diff --git a/mark.c b/mark.c
index f438d52e800d9e336df99a1f73e775e47225f696..ecbf1cb53674be2435f4e03e59d810ca082a3a1a 100644 (file)
--- a/mark.c
+++ b/mark.c
@@ -946,7 +946,7 @@ static mse *main_local_mark_stack;
 /* marker_[b]sp, marker_mach_threads, GC_marker_Id).            */
 GC_INNER void GC_wait_for_markers_init(void)
 {
-  word count;
+  signed_word count;
 
   if (GC_markers_m1 == 0)
     return;
@@ -969,11 +969,13 @@ GC_INNER void GC_wait_for_markers_init(void)
   /* Reuse marker lock and builders count to synchronize        */
   /* marker threads startup.                                    */
   GC_acquire_mark_lock();
-  GC_fl_builder_count += (word)GC_markers_m1;
+  GC_fl_builder_count += GC_markers_m1;
   count = GC_fl_builder_count;
   GC_release_mark_lock();
-  if (count != 0)
+  if (count != 0) {
+    GC_ASSERT(count > 0);
     GC_wait_for_reclaim();
+  }
 }
 
 /* Steal mark stack entries starting at mse low into mark stack local   */

diff --git a/pthread_support.c b/pthread_support.c
index cb31824a419efc41776a7c386e2dbba537e07cfb..92565fbee6078994ca6e197752382881095be814 100644 (file)
--- a/pthread_support.c
+++ b/pthread_support.c
@@ -357,7 +357,7 @@ STATIC void * GC_mark_thread(void * id)
 
   /* Inform GC_start_mark_threads about completion of marker data init. */
   GC_acquire_mark_lock();
-  if (0 == --GC_fl_builder_count)
+  if (0 == --GC_fl_builder_count) /* count may have a negative value */
     GC_notify_all_builder();
   GC_release_mark_lock();
 

diff --git a/reclaim.c b/reclaim.c
index 2f49121868e54b9a67670a46a5c3e96daaf4b1b6..5bd69979400f5b6f13d8bec401b8d85ed183d706 100644 (file)
--- a/reclaim.c
+++ b/reclaim.c
@@ -28,10 +28,11 @@ GC_INNER signed_word GC_bytes_found = 0;
                         /* on free lists which we had to drop.     */
 
 #if defined(PARALLEL_MARK)
-  GC_INNER word GC_fl_builder_count = 0;
+  GC_INNER signed_word GC_fl_builder_count = 0;
         /* Number of threads currently building free lists without      */
         /* holding GC lock.  It is not safe to collect if this is       */
-        /* nonzero.                                                     */
+        /* nonzero.  Also, together with the mark lock, it is used as   */
+        /* a semaphore during marker threads startup.                   */
 #endif /* PARALLEL_MARK */
 
 /* We defer printing of leaked objects until we're done with the GC     */

diff --git a/win32_threads.c b/win32_threads.c
index 54fa5d2b3bffa8e31ce567e4573dd4e71b1d720d..126d332d72cfc28c57eca8db950d9ecfa3f4fc5c 100644 (file)
--- a/win32_threads.c
+++ b/win32_threads.c
@@ -1741,7 +1741,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
 
     /* Inform GC_start_mark_threads about completion of marker data init. */
     GC_acquire_mark_lock();
-    if (0 == --GC_fl_builder_count)
+    if (0 == --GC_fl_builder_count) /* count may have a negative value */
       GC_notify_all_builder();
     GC_release_mark_lock();