Rework the condition variables support for VxWorks

This change reworks the condition variables support for VxWorks
to address the very legit points raised by Rasmus in

 https://gcc.gnu.org/pipermail/gcc/2020-May/232524.html

While some of the issues were taken care of by the use of semFlush,
a few others were indeed calling for adjustments.

We first considered resorting to the condvarLib library available
in VxWorks7. Unfortunately, it is vx7 only and we wanted something working
for at least vx 6.9 as well. It also turned out requiring the use of
recursive mutexes for condVarWait, which seemed unnecessarily constraining.

Instead, this change corrects the sequencing logic in a few places and
leverages the semExchange API to ensure the key atomicity requirement on
cond_wait operations.

2020-10-14  Alexandre Oliva  <oliva@adacore.com>

libgcc/
	* config/gthr-vxworks-thread.c: Include stdlib.h.
	(tls_delete_hook): Prototype it.
	(__gthread_cond_signal): Return early if no waiters.  Consume
	signal in case the semaphore got full.  Use semInfoGet instead
	of kernel-mode-only semInfo.
	(__gthread_cond_timedwait): Use semExchange.  Always take the
	mutex again before returning.
	* config/gthr-vxworks-cond.c (__ghtread_cond_wait): Likewise.

libgcc/config/gthr-vxworks-cond.c

@@ -66,13 +66,11 @@ __gthread_cond_wait (__gthread_cond_t *cond,
   if (!mutex)
     return ERROR;
 
-  __RETURN_ERRNO_IF_NOT_OK (semGive (*mutex));
-
-  __RETURN_ERRNO_IF_NOT_OK (semTake (*cond, WAIT_FOREVER));
+  int ret = __CHECK_RESULT (semExchange (*mutex, *cond, WAIT_FOREVER));
 
   __RETURN_ERRNO_IF_NOT_OK (semTake (*mutex, WAIT_FOREVER));
 
-  return OK;
+  return ret;
 }
 
 int

libgcc/config/gthr-vxworks-thread.c

@@ -29,6 +29,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 
 #include "gthr.h"
 #include <taskLib.h>
+#include <stdlib.h>
 
 #define __TIMESPEC_TO_NSEC(timespec) \
   ((long long)timespec.tv_sec * 1000000000 + (long long)timespec.tv_nsec)
@@ -38,7 +39,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
     / 1000000000)
 
 #ifdef __RTP__
-  void tls_delete_hook ();
+  void tls_delete_hook (void);
   #define __CALL_DELETE_HOOK(tcb) tls_delete_hook()
 #else
   /* In kernel mode, we need to pass the TCB to task_delete_hook. The TCB is
@@ -47,17 +48,55 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
   #define __CALL_DELETE_HOOK(tcb) tls_delete_hook((WIND_TCB *) ((tcb)->task_id))
 #endif
 
-/* -------------------- Timed Condition Variables --------------------- */
-
 int
 __gthread_cond_signal (__gthread_cond_t *cond)
 {
   if (!cond)
     return ERROR;
 
-  return __CHECK_RESULT (semGive (*cond));
+  /* If nobody is waiting, skip the semGive altogether: no one can get
+     in line while we hold the mutex associated with *COND.  We could
+     skip this test altogether, but it's presumed cheaper than going
+     through the give and take below, and that a signal without a
+     waiter occurs often enough for the test to be worth it.  */
+  SEM_INFO info;
+  memset (&info, 0, sizeof (info));
+  __RETURN_ERRNO_IF_NOT_OK (semInfoGet (*cond, &info));
+  if (info.numTasks == 0)
+    return OK;
+
+  int ret = __CHECK_RESULT (semGive (*cond));
+
+  /* It might be the case, however, that when we called semInfo, there
+     was a waiter just about to timeout, and by the time we called
+     semGive, it had already timed out, so our semGive would leave the
+     *cond semaphore full, so the next caller of wait would pass
+     through.  We don't want that.  So, make sure we leave the
+     semaphore empty.  Despite the window in which the semaphore will
+     be full, this works because:
+
+     - we're holding the mutex, so nobody else can semGive, and any
+       pending semTakes are actually within semExchange.  there might
+       be others blocked to acquire the mutex, but those are not
+       relevant for the analysis.
+
+     - if there was another non-timed out waiter, semGive will wake it
+       up immediately instead of leaving the semaphore full, so the
+       semTake below will time out, and the semantics are as expected
+
+     - otherwise, if all waiters timed out before the semGive (or if
+       there weren't any to begin with), our semGive completed leaving
+       the semaphore full, and our semTake below will consume it
+       before any other waiter has a change to reach the semExchange,
+       because we're holding the mutex.  */
+  if (ret == OK)
+    semTake (*cond, NO_WAIT);
+
+  return ret;
 }
 
+/* -------------------- Timed Condition Variables --------------------- */
+
 int
 __gthread_cond_timedwait (__gthread_cond_t *cond,
 			  __gthread_mutex_t *mutex,
@@ -93,13 +132,11 @@ __gthread_cond_timedwait (__gthread_cond_t *cond,
   if (waiting_ticks > INT_MAX)
     waiting_ticks = INT_MAX;
 
-  __RETURN_ERRNO_IF_NOT_OK (semGive (*mutex));
-
-  __RETURN_ERRNO_IF_NOT_OK (semTake (*cond, waiting_ticks));
+  int ret = __CHECK_RESULT (semExchange (*mutex, *cond, waiting_ticks));
 
   __RETURN_ERRNO_IF_NOT_OK (semTake (*mutex, WAIT_FOREVER));
 
-  return OK;
+  return ret;
 }
 
 /* --------------------------- Timed Mutexes ------------------------------ */