From: Bart Van Assche <bvanassche@acm.org>
Date: Thu, 12 Mar 2009 18:39:31 +0000 (+0000)
Subject: Added comment block that explains why the creator thread waits until the creator... 
X-Git-Tag: svn/VALGRIND_3_5_0~862
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=cc744ab1c66451447aee59dd668c803d59e2bcac;p=thirdparty%2Fvalgrind.git

Added comment block that explains why the creator thread waits until the creator thread has copied the vector clock of the creator thread.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@9387
---

diff --git a/drd/drd_pthread_intercepts.c b/drd/drd_pthread_intercepts.c
index 2c29c44d07..0898cfc47a 100644
--- a/drd/drd_pthread_intercepts.c
+++ b/drd/drd_pthread_intercepts.c
@@ -61,6 +61,22 @@
 
 /* Defines. */
 
+/*
+ * Do not undefine the two macro's below, or the following two subtle race
+ * conditions will be introduced in the data race detection algorithm:
+ * - sg_init() runs on the context of the created thread and copies the
+ *   vector clock of the creator thread. This only works reliably if
+ *   the creator thread waits until this copy has been performed.
+ * - Since DRD_(thread_compute_minimum_vc)() does not take the vector
+ *   clocks into account that are involved in thread creation but
+ *   for which the corresponding thread has not yet been created, by
+ *   undefining the macro below it becomes possible that segments get
+ *   discarded that should not yet be discarded. Or: some data races
+ *   are not detected.
+ */
+#define WAIT_UNTIL_CREATED_THREAD_STARTED
+#define ALLOCATE_THREAD_ARGS_ON_THE_STACK
+
 #define PTH_FUNC(ret_ty, f, args...)                            \
   ret_ty VG_WRAP_FUNCTION_ZZ(libpthreadZdsoZd0,f)(args);        \
   ret_ty VG_WRAP_FUNCTION_ZZ(libpthreadZdsoZd0,f)(args)
@@ -73,7 +89,9 @@ typedef struct
   void* (*start)(void*);
   void* arg;
   int   detachstate;
+#if defined(WAIT_UNTIL_CREATED_THREAD_STARTED)
   int   wrapper_started;
+#endif
 } DrdPosixThreadArgs;
 
 
@@ -182,7 +200,15 @@ static void* DRD_(thread_wrapper)(void* arg)
 
   arg_ptr = (DrdPosixThreadArgs*)arg;
   arg_copy = *arg_ptr;
+#if defined(WAIT_UNTIL_CREATED_THREAD_STARTED)
   arg_ptr->wrapper_started = 1;
+#else
+#if defined(ALLOCATE_THREAD_ARGS_ON_THE_STACK)
+#error Defining ALLOCATE_THREAD_ARGS_ON_THE_STACK but not WAIT_UNTIL_CREATED_THREAD_STARTED is not supported.
+#else
+  free(arg_ptr);
+#endif
+#endif
 
   VALGRIND_DO_CLIENT_REQUEST(res, -1, VG_USERREQ__SET_PTHREADID,
                              pthread_self(), 0, 0, 0, 0);
@@ -274,39 +300,52 @@ PTH_FUNC(int, pthreadZucreateZa, // pthread_create*
   int    res;
   int    ret;
   OrigFn fn;
+#if defined(ALLOCATE_THREAD_ARGS_ON_THE_STACK)
   DrdPosixThreadArgs thread_args;
+#endif
+  DrdPosixThreadArgs* thread_args_p;
 
   VALGRIND_GET_ORIG_FN(fn);
 
-  DRD_IGNORE_VAR(thread_args.wrapper_started);
-  thread_args.start           = start;
-  thread_args.arg             = arg;
-  thread_args.wrapper_started = 0;
+#if defined(ALLOCATE_THREAD_ARGS_ON_THE_STACK)
+  thread_args_p = &thread_args;
+#else
+  thread_args_p = malloc(sizeof(*thread_args_p));
+#endif
+  assert(thread_args_p);
+
+  thread_args_p->start           = start;
+  thread_args_p->arg             = arg;
+#if defined(WAIT_UNTIL_CREATED_THREAD_STARTED)
+  DRD_IGNORE_VAR(thread_args_p->wrapper_started);
+  thread_args_p->wrapper_started = 0;
+#endif
   /*
    * Find out whether the thread will be started as a joinable thread
    * or as a detached thread. If no thread attributes have been specified,
    * this means that the new thread will be started as a joinable thread.
    */
-  thread_args.detachstate = PTHREAD_CREATE_JOINABLE;
+  thread_args_p->detachstate = PTHREAD_CREATE_JOINABLE;
   if (attr)
   {
-    if (pthread_attr_getdetachstate(attr, &thread_args.detachstate) != 0)
+    if (pthread_attr_getdetachstate(attr, &thread_args_p->detachstate) != 0)
     {
       assert(0);
     }
   }
-  assert(thread_args.detachstate == PTHREAD_CREATE_JOINABLE
-         || thread_args.detachstate == PTHREAD_CREATE_DETACHED);
+  assert(thread_args_p->detachstate == PTHREAD_CREATE_JOINABLE
+         || thread_args_p->detachstate == PTHREAD_CREATE_DETACHED);
 
   /* Suppress NPTL-specific conflicts between creator and created thread. */
   VALGRIND_DO_CLIENT_REQUEST(res, -1, VG_USERREQ__DRD_STOP_RECORDING,
                              0, 0, 0, 0, 0);
 
-  CALL_FN_W_WWWW(ret, fn, thread, attr, DRD_(thread_wrapper), &thread_args);
+  CALL_FN_W_WWWW(ret, fn, thread, attr, DRD_(thread_wrapper), thread_args_p);
 
   VALGRIND_DO_CLIENT_REQUEST(res, -1, VG_USERREQ__DRD_START_RECORDING,
                              0, 0, 0, 0, 0);
 
+#if defined(WAIT_UNTIL_CREATED_THREAD_STARTED)
   if (ret == 0)
   {
     /*
@@ -315,12 +354,21 @@ PTH_FUNC(int, pthreadZucreateZa, // pthread_create*
      *   passed via dynamically allocated memory and if the loop below is
      *   removed.
      */
-    while (! thread_args.wrapper_started)
+    while (! thread_args_p->wrapper_started)
     {
       sched_yield();
     }
   }
 
+#if defined(ALLOCATE_THREAD_ARGS_DYNAMICALLY)
+  free(thread_args_p);
+#endif
+
+#endif
+
+  VALGRIND_DO_CLIENT_REQUEST(res, -1, VG_USERREQ__DRD_START_NEW_SEGMENT,
+                             pthread_self(), 0, 0, 0, 0);
+
   return ret;
 }