This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix nptl semaphore cleanup invocation


As mentioned the other week, we need to pull operations into a seperate
function if we want to be certain that gcc will wrap an exception region
around it.  We fixed aio_suspend.c already since it was directly causing
a testsuite failure.

The pattern for the problematic case is:

	pthread_cleanup_push (..., ...);
	...
	int oldtype = __pthread_enable_asynccancel ();
	op();
	__pthread_disable_asynccancel (oldtype);

In the sem_wait and sem_timedwait implementations which are in C, we
don't handle this properly and therefore the cleanups might not be
executed if we trigger an unwind from the futex op.

Any objections?

2011-09-02  David S. Miller  <davem@davemloft.net>

	* sysdeps/unix/sysv/linux/sem_timedwait.c (do_futex_timed_wait):
	New function.
	(sem_timedwait): Call it to force an exception region around
	the async cancel enable and the futex operation.
	* sysdeps/unix/sysv/linux/sparc/sem_timedwait.c: Likewise.
	* sysdeps/unix/sysv/linux/sparc/sparc32/sem_timedwait.c: Likewise.
	* sysdeps/unix/sysv/linux/sem_wait.c (do_futex_wait): New function.
	(__new_sem_wait): Call it to force an exception region around
	the async cancel enable and the futex operation.
	* sysdeps/unix/sysv/linux/sparc/sem_wait.c: Likewise.
	* sysdeps/unix/sysv/linux/sparc/sparc32/sem_wait.c: Likewise.

diff --git a/nptl/sysdeps/unix/sysv/linux/sem_timedwait.c b/nptl/sysdeps/unix/sysv/linux/sem_timedwait.c
index fdf0d74..9b40835 100644
--- a/nptl/sysdeps/unix/sysv/linux/sem_timedwait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sem_timedwait.c
@@ -30,6 +30,18 @@
 
 extern void __sem_wait_cleanup (void *arg) attribute_hidden;
 
+static int
+__attribute__ ((noinline))
+do_futex_timed_wait(struct new_sem *isem, struct timespec *rt)
+{
+  int err, oldtype = __pthread_enable_asynccancel ();
+
+  err = lll_futex_timed_wait (&isem->value, 0, rt,
+			      isem->private ^ FUTEX_PRIVATE_FLAG);
+
+  __pthread_disable_asynccancel (oldtype);
+  return err;
+}
 
 int
 sem_timedwait (sem_t *sem, const struct timespec *abstime)
@@ -80,16 +92,7 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime)
       /* Do wait.  */
       rt.tv_sec = sec;
       rt.tv_nsec = nsec;
-
-      /* Enable asynchronous cancellation.  Required by the standard.  */
-      int oldtype = __pthread_enable_asynccancel ();
-
-      err = lll_futex_timed_wait (&isem->value, 0, &rt,
-				  isem->private ^ FUTEX_PRIVATE_FLAG);
-
-      /* Disable asynchronous cancellation.  */
-      __pthread_disable_asynccancel (oldtype);
-
+      err = do_futex_timed_wait(isem, &rt);
       if (err != 0 && err != -EWOULDBLOCK)
 	{
 	  __set_errno (-err);
diff --git a/nptl/sysdeps/unix/sysv/linux/sem_wait.c b/nptl/sysdeps/unix/sysv/linux/sem_wait.c
index 20e2b48..5e3c52b 100644
--- a/nptl/sysdeps/unix/sysv/linux/sem_wait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sem_wait.c
@@ -37,6 +37,17 @@ __sem_wait_cleanup (void *arg)
   atomic_decrement (&isem->nwaiters);
 }
 
+static int
+__attribute__ ((noinline))
+do_futex_wait(struct new_sem *isem)
+{
+  int err, oldtype = __pthread_enable_asynccancel ();
+
+  err = lll_futex_wait (&isem->value, 0, isem->private ^ FUTEX_PRIVATE_FLAG);
+
+  __pthread_disable_asynccancel (oldtype);
+  return err;
+}
 
 int
 __new_sem_wait (sem_t *sem)
@@ -53,15 +64,7 @@ __new_sem_wait (sem_t *sem)
 
   while (1)
     {
-      /* Enable asynchronous cancellation.  Required by the standard.  */
-      int oldtype = __pthread_enable_asynccancel ();
-
-      err = lll_futex_wait (&isem->value, 0,
-			    isem->private ^ FUTEX_PRIVATE_FLAG);
-
-      /* Disable asynchronous cancellation.  */
-      __pthread_disable_asynccancel (oldtype);
-
+      err = do_futex_wait(isem);
       if (err != 0 && err != -EWOULDBLOCK)
 	{
 	  __set_errno (-err);
diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/sem_timedwait.c b/nptl/sysdeps/unix/sysv/linux/sparc/sem_timedwait.c
index 01952f3..b847c53 100644
--- a/nptl/sysdeps/unix/sysv/linux/sparc/sem_timedwait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sparc/sem_timedwait.c
@@ -30,6 +30,18 @@
 
 extern void __sem_wait_cleanup (void *arg) attribute_hidden;
 
+static int
+__attribute__ ((noinline))
+do_futex_timed_wait(struct sparc_new_sem *isem, struct timespec *rt)
+{
+  int err, oldtype = __pthread_enable_asynccancel ();
+
+  err = lll_futex_timed_wait (&isem->value, 0, rt,
+			      isem->private ^ FUTEX_PRIVATE_FLAG);
+
+  __pthread_disable_asynccancel (oldtype);
+  return err;
+}
 
 int
 sem_timedwait (sem_t *sem, const struct timespec *abstime)
@@ -80,16 +92,7 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime)
       /* Do wait.  */
       rt.tv_sec = sec;
       rt.tv_nsec = nsec;
-
-      /* Enable asynchronous cancellation.  Required by the standard.  */
-      int oldtype = __pthread_enable_asynccancel ();
-
-      err = lll_futex_timed_wait (&isem->value, 0, &rt,
-				  isem->private ^ FUTEX_PRIVATE_FLAG);
-
-      /* Disable asynchronous cancellation.  */
-      __pthread_disable_asynccancel (oldtype);
-
+      err = do_futex_timed_wait(isem, &rt);
       if (err != 0 && err != -EWOULDBLOCK)
 	{
 	  __set_errno (-err);
diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/sem_wait.c b/nptl/sysdeps/unix/sysv/linux/sparc/sem_wait.c
index a846f20..5e61464 100644
--- a/nptl/sysdeps/unix/sysv/linux/sparc/sem_wait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sparc/sem_wait.c
@@ -37,6 +37,17 @@ __sem_wait_cleanup (void *arg)
   atomic_decrement (&isem->nwaiters);
 }
 
+static int
+__attribute__ ((noinline))
+do_futex_wait(struct sparc_new_sem *isem)
+{
+  int err, oldtype = __pthread_enable_asynccancel ();
+
+  err = lll_futex_wait (&isem->value, 0, isem->private ^ FUTEX_PRIVATE_FLAG);
+
+  __pthread_disable_asynccancel (oldtype);
+  return err;
+}
 
 int
 __new_sem_wait (sem_t *sem)
@@ -53,15 +64,7 @@ __new_sem_wait (sem_t *sem)
 
   while (1)
     {
-      /* Enable asynchronous cancellation.  Required by the standard.  */
-      int oldtype = __pthread_enable_asynccancel ();
-
-      err = lll_futex_wait (&isem->value, 0,
-			    isem->private ^ FUTEX_PRIVATE_FLAG);
-
-      /* Disable asynchronous cancellation.  */
-      __pthread_disable_asynccancel (oldtype);
-
+      err = do_futex_wait(isem);
       if (err != 0 && err != -EWOULDBLOCK)
 	{
 	  __set_errno (-err);
diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_timedwait.c b/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_timedwait.c
index 55f3e2e..fc512d6 100644
--- a/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_timedwait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_timedwait.c
@@ -30,6 +30,18 @@
 
 extern void __sem_wait_cleanup (void *arg) attribute_hidden;
 
+static int
+__attribute__ ((noinline))
+do_futex_timed_wait(struct sparc_new_sem *isem, struct timespec *rt)
+{
+  int err, oldtype = __pthread_enable_asynccancel ();
+
+  err = lll_futex_timed_wait (&isem->value, 0, rt,
+			      isem->private ^ FUTEX_PRIVATE_FLAG);
+
+  __pthread_disable_asynccancel (oldtype);
+  return err;
+}
 
 int
 sem_timedwait (sem_t *sem, const struct timespec *abstime)
@@ -99,16 +111,7 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime)
       /* Do wait.  */
       rt.tv_sec = sec;
       rt.tv_nsec = nsec;
-
-      /* Enable asynchronous cancellation.  Required by the standard.  */
-      int oldtype = __pthread_enable_asynccancel ();
-
-      err = lll_futex_timed_wait (&isem->value, 0, &rt,
-				  isem->private ^ FUTEX_PRIVATE_FLAG);
-
-      /* Disable asynchronous cancellation.  */
-      __pthread_disable_asynccancel (oldtype);
-
+      err = do_futex_timed_wait(isem, &rt);
       if (err != 0 && err != -EWOULDBLOCK)
 	{
 	  __set_errno (-err);
diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_wait.c b/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_wait.c
index b14f976..308b820 100644
--- a/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_wait.c
+++ b/nptl/sysdeps/unix/sysv/linux/sparc/sparc32/sem_wait.c
@@ -44,6 +44,17 @@ __sem_wait_cleanup (void *arg)
     }
 }
 
+static int
+__attribute__ ((noinline))
+do_futex_wait(struct sparc_new_sem *isem)
+{
+  int err, oldtype = __pthread_enable_asynccancel ();
+
+  err = lll_futex_wait (&isem->value, 0, isem->private ^ FUTEX_PRIVATE_FLAG);
+
+  __pthread_disable_asynccancel (oldtype);
+  return err;
+}
 
 int
 __new_sem_wait (sem_t *sem)
@@ -77,15 +88,7 @@ __new_sem_wait (sem_t *sem)
 
   while (1)
     {
-      /* Enable asynchronous cancellation.  Required by the standard.  */
-      int oldtype = __pthread_enable_asynccancel ();
-
-      err = lll_futex_wait (&isem->value, 0,
-			    isem->private ^ FUTEX_PRIVATE_FLAG);
-
-      /* Disable asynchronous cancellation.  */
-      __pthread_disable_asynccancel (oldtype);
-
+      err = do_futex_wait(isem);
       if (err != 0 && err != -EWOULDBLOCK)
 	{
 	  __set_errno (-err);


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]