[PATCH][BZ #14477] Fix exception table of pthread_cond_wait on i386

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

Hi,

I had earlier thought that this bug may be a slightly different
manifestation of the problem in bz #14417, but it is not.

pthread_cond_wait on i386 uses a slightly different code path for
priority inherited mutexes, by using the FUTEX_WAIT_REQUEUE_PI futex
operation instead of the usual FUTEX_WAIT.  When moving into the
syscall, it increments %ebx by 4 to have it point to the futex for the
syscall and then decrements it after returning from the syscall.

The exception table for the function compensates for this by marking
this part of the code and setting up the cleanup handler jump to
__condvar_w_cleanup2 instead of __condvar_w_cleanup, where the former
first decrements %ebx and then proceeds to cleanup.  A similar entry
was missing for the PI case, which resulted in the deadlock described
in BZ #14477.  Attached patch adds this entry for pthread_cond_wait and
pthread_cond_timedwait for i386.  The patch also has a test case to
verify this fix.

I have tested this patch on a 32-bit build and it does not cause any
regressions in the testsuite.  I have also verified that this test case
passes and that the test case fails without this patch.

OK to commit?

Regards,
Siddhesh

nptl/ChangeLog:

	[BZ #14477]
	* Makefile (tests): Add tst-cond-except.
	* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
	(__pthread_cond_timedwait): Mark instructions where %ebx is
	incremented in PI case.
	(.gcc_except_table): Add entry to jump to __condvar_tw_cleanup2
	for the marked PI case instructions.
	* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
	(__pthread_cond_wait): Mark instructions where %ebx is
	incremented in PI case.
	(.gcc_except_table): Add entry to jump to __condvar_w_cleanup2
	for the marked PI case instructions.
	* tst-cond-except.c: New test case.

diff --git a/nptl/Makefile b/nptl/Makefile
index b081b07..b9c73b3 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -206,7 +206,7 @@ tests = tst-typesizes \
 	tst-cond1 tst-cond2 tst-cond3 tst-cond4 tst-cond5 tst-cond6 tst-cond7 \
 	tst-cond8 tst-cond9 tst-cond10 tst-cond11 tst-cond12 tst-cond13 \
 	tst-cond14 tst-cond15 tst-cond16 tst-cond17 tst-cond18 tst-cond19 \
-	tst-cond20 tst-cond21 tst-cond22 tst-cond23 \
+	tst-cond20 tst-cond21 tst-cond22 tst-cond23 tst-cond-except \
 	tst-robust1 tst-robust2 tst-robust3 tst-robust4 tst-robust5 \
 	tst-robust6 tst-robust7 tst-robust8 tst-robust9 \
 	tst-robustpi1 tst-robustpi2 tst-robustpi3 tst-robustpi4 tst-robustpi5 \
diff --git a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
index 5f1fd5d..d14d7de 100644
--- a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
+++ b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
@@ -200,9 +200,11 @@ __pthread_cond_timedwait:
 42:	leal	(%ebp), %esi
 	movl	28(%esp), %edx
 	addl	$cond_futex, %ebx
+.Ladd_cond_futex_pi:
 	movl	$SYS_futex, %eax
 	ENTER_KERNEL
 	subl	$cond_futex, %ebx
+.Lsub_cond_futex_pi:
 	movl	%eax, %esi
 	/* Set the pi-requeued flag only if the kernel has returned 0. The
 	   kernel does not hold the mutex on ETIMEDOUT or any other error.  */
@@ -638,7 +640,15 @@ __condvar_tw_cleanup:
 	.uleb128 .Lcstend-.Lcstbegin
 .Lcstbegin:
 	.long	.LcleanupSTART-.LSTARTCODE
-	.long	.Ladd_cond_futex-.LcleanupSTART
+	.long	.Ladd_cond_futex_pi-.LcleanupSTART
+	.long	__condvar_tw_cleanup-.LSTARTCODE
+	.uleb128  0
+	.long	.Ladd_cond_futex_pi-.LSTARTCODE
+	.long	.Lsub_cond_futex_pi-.Ladd_cond_futex_pi
+	.long	__condvar_tw_cleanup2-.LSTARTCODE
+	.uleb128  0
+	.long	.Lsub_cond_futex_pi-.LSTARTCODE
+	.long	.Ladd_cond_futex-.Lsub_cond_futex_pi
 	.long	__condvar_tw_cleanup-.LSTARTCODE
 	.uleb128  0
 	.long	.Ladd_cond_futex-.LSTARTCODE
diff --git a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
index 2ae7af2..366de69 100644
--- a/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
+++ b/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
@@ -141,9 +141,11 @@ __pthread_cond_wait:
 	movl	%ebp, %edx
 	xorl	%esi, %esi
 	addl	$cond_futex, %ebx
+.Ladd_cond_futex_pi:
 	movl	$SYS_futex, %eax
 	ENTER_KERNEL
 	subl	$cond_futex, %ebx
+.Lsub_cond_futex_pi:
 	/* Set the pi-requeued flag only if the kernel has returned 0. The
 	   kernel does not hold the mutex on error.  */
 	cmpl	$0, %eax
@@ -630,7 +632,15 @@ __condvar_w_cleanup:
 	.uleb128 .Lcstend-.Lcstbegin
 .Lcstbegin:
 	.long	.LcleanupSTART-.LSTARTCODE
-	.long	.Ladd_cond_futex-.LcleanupSTART
+	.long	.Ladd_cond_futex_pi-.LcleanupSTART
+	.long	__condvar_w_cleanup-.LSTARTCODE
+	.uleb128  0
+	.long	.Ladd_cond_futex_pi-.LSTARTCODE
+	.long	.Lsub_cond_futex_pi-.Ladd_cond_futex_pi
+	.long	__condvar_w_cleanup2-.LSTARTCODE
+	.uleb128  0
+	.long	.Lsub_cond_futex_pi-.LSTARTCODE
+	.long	.Ladd_cond_futex-.Lsub_cond_futex_pi
 	.long	__condvar_w_cleanup-.LSTARTCODE
 	.uleb128  0
 	.long	.Ladd_cond_futex-.LSTARTCODE
diff --git a/nptl/tst-cond-except.c b/nptl/tst-cond-except.c
new file mode 100644
index 0000000..b9871ba
--- /dev/null
+++ b/nptl/tst-cond-except.c
@@ -0,0 +1,108 @@
+/* Verify that exception table for pthread_cond_wait is correct.
+   Copyright (C) 2012 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <pthread.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+pthread_mutex_t mutex;
+pthread_cond_t cond;
+
+#define CHECK_RETURN_VAL_OR_FAIL(ret,str) \
+  ({ if ((ret) != 0) \
+       { \
+         printf ("%s failed: %s\n", (str), strerror (ret)); \
+         ret = 1; \
+         goto out; \
+       } \
+  })
+
+
+void
+clean (void *arg)
+{
+  puts ("clean: Unlocking mutex...");
+  pthread_mutex_unlock ((pthread_mutex_t *) arg);
+  puts ("clean: Mutex unlocked...");
+}
+
+void *
+thr (void *arg)
+{
+  int ret = 0;
+  pthread_mutexattr_t mutexAttr;
+  ret = pthread_mutexattr_init (&mutexAttr);
+  CHECK_RETURN_VAL_OR_FAIL (ret, "pthread_mutexattr_init");
+
+  ret = pthread_mutexattr_setprotocol (&mutexAttr, PTHREAD_PRIO_INHERIT);
+  CHECK_RETURN_VAL_OR_FAIL (ret, "pthread_mutexattr_setprotocol");
+
+  ret = pthread_mutex_init (&mutex, &mutexAttr);
+  CHECK_RETURN_VAL_OR_FAIL (ret, "pthread_mutex_init");
+
+  ret = pthread_cond_init (&cond, 0);
+  CHECK_RETURN_VAL_OR_FAIL (ret, "pthread_cond_init");
+
+  puts ("th: Init done, entering wait...");
+
+  pthread_cleanup_push (clean, (void *) &mutex);
+  ret = pthread_mutex_lock (&mutex);
+  CHECK_RETURN_VAL_OR_FAIL (ret, "pthread_mutex_lock");
+  while (1)
+    {
+      ret = pthread_cond_wait (&cond, &mutex);
+      CHECK_RETURN_VAL_OR_FAIL (ret, "pthread_cond_wait");
+    }
+  pthread_cleanup_pop (1);
+
+out:
+  return (void *)ret;
+}
+
+int
+do_test ()
+{
+  pthread_t thread;
+  int ret = 0;
+  void *thr_ret = 0;
+  ret = pthread_create (&thread, 0, thr, &thr_ret);
+  CHECK_RETURN_VAL_OR_FAIL (ret, "pthread_create");
+
+  puts ("main: Thread created, waiting a bit...");
+  sleep (2);
+
+  puts ("main: Cancelling thread...");
+  ret = pthread_cancel (thread);
+  CHECK_RETURN_VAL_OR_FAIL (ret, "pthread_cancel");
+
+  puts ("main: Joining th...");
+  ret = pthread_join (thread, NULL);
+  CHECK_RETURN_VAL_OR_FAIL (ret, "pthread_join");
+
+  if (thr_ret != NULL)
+    return 1;
+
+  puts ("main: Joined thread, done!");
+
+out:
+  return ret;
+}
+
+#define TIMEOUT 5
+#include "../test-skeleton.c"