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 b95e528357794a5f076aea3ff4311889fdfe682f..30555984d55f8494f6145ee04256ff64c65f7897 100644 (file)
--- a/include/private/gc_priv.h
+++ b/include/private/gc_priv.h
@@ -2484,7 +2484,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 60088852d98ed4d21f2bddd86e98d5f0400249d8..7ab3db4274f01f43919cb1031871c7735158ed25 100644 (file)
--- a/mark.c
+++ b/mark.c
@@ -935,7 +935,7 @@ GC_INNER word GC_mark_no = 0;
 /* 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;
@@ -943,11 +943,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 86d345d12f48b71b96fb1c3f4565536b346acb37..90a330d8d875d6738abc778c775647c8f21698be 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 2361406ebe15b495f3dbd1c8697d0866883b80bd..78bd9c0a141e22b41f179f4d1a084df01b625519 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 df5bdc118e2c86d654dbddd120a473ae05b2de65..86035036a487ffdaafbcefa141f260f89632c507 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();