From: Ivan Maidanski <ivmai@mail.ru>
Date: Mon, 21 Aug 2017 22:09:59 +0000 (+0300)
Subject: Eliminate unsigned fl_builder_count underflow in mark_thread
X-Git-Tag: v8.0.0~601
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=207ba534acd38fd1acd722e02a07cc33e09720c8;p=gc

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 804d0948..3a6eca63 100644
--- 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 f438d52e..ecbf1cb5 100644
--- 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 cb31824a..92565fbe 100644
--- 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 2f491218..5bd69979 100644
--- 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 54fa5d2b..126d332d 100644
--- 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();