From: Ivan Maidanski <ivmai@mail.ru>
Date: Thu, 23 Nov 2017 17:15:54 +0000 (+0300)
Subject: Fix data race in last_stop_count access (suspend_handler_inner)
X-Git-Tag: v8.0.0~494
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1179203276dc068719946dc26c12b9896ebd2054;p=gc

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).
---

diff --git a/include/private/pthread_stop_world.h b/include/private/pthread_stop_world.h
index 49b87bc6..ec5dfea6 100644
--- a/include/private/pthread_stop_world.h
+++ b/include/private/pthread_stop_world.h
@@ -19,10 +19,10 @@
 #define GC_PTHREAD_STOP_WORLD_H
 
 struct thread_stop_info {
-#   ifndef GC_OPENBSD_UTHREADS
-      word last_stop_count;     /* GC_last_stop_count value when thread */
-                                /* last successfully handled a suspend  */
-                                /* signal.                              */
+#   if !defined(GC_OPENBSD_UTHREADS) && !defined(NACL)
+      volatile AO_t last_stop_count;
+                        /* The value of GC_stop_count when the thread   */
+                        /* last successfully handled a suspend signal.  */
 #   endif
 
     ptr_t stack_ptr;            /* Valid only when stopped.             */
diff --git a/pthread_stop_world.c b/pthread_stop_world.c
index 0370cf31..dde9de24 100644
--- a/pthread_stop_world.c
+++ b/pthread_stop_world.c
@@ -261,12 +261,6 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy, void *context);
 # define GC_lookup_thread_async GC_lookup_thread
 #endif
 
-GC_ATTR_NO_SANITIZE_THREAD
-static void update_last_stop_count(GC_thread me, AO_t my_stop_count)
-{
-  me -> stop_info.last_stop_count = my_stop_count;
-}
-
 STATIC void GC_suspend_handler_inner(ptr_t dummy GC_ATTR_UNUSED,
                                      void * context GC_ATTR_UNUSED)
 {
@@ -341,7 +335,7 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy GC_ATTR_UNUSED,
   /* thread has been stopped.  Note that sem_post() is          */
   /* the only async-signal-safe primitive in LinuxThreads.      */
   sem_post(&GC_suspend_ack_sem);
-  update_last_stop_count(me, my_stop_count);
+  AO_store_release(&me->stop_info.last_stop_count, my_stop_count);
 
   /* Wait until that thread tells us to restart by sending      */
   /* this thread a GC_sig_thr_restart signal (should be masked  */
@@ -671,7 +665,8 @@ STATIC int GC_suspend_all(void)
 #             ifdef GC_ENABLE_SUSPEND_THREAD
                 if (p -> suspended_ext) continue;
 #             endif
-              if (p -> stop_info.last_stop_count == GC_stop_count) continue;
+              if (AO_load(&p->stop_info.last_stop_count) == GC_stop_count)
+                continue;
               n_live_threads++;
 #           endif
 #           ifdef DEBUG_THREADS