Re: [PATCH] Don't do lock elision on an error checking mutex (bug 17514)

[Andreas Schwab <schwab at suse dot de>]

Error checking mutexes are not supposed to be subject to lock elision.
That would defeat the error checking nature of the mutex because lock
elision doesn't record ownership.

	[BZ #17514]
	* nptl/pthread_mutex_timedlock.c (pthread_mutex_timedlock)
	<case PTHREAD_MUTEX_ERRORCHECK_NP>: Don't do lock elision.
	* nptl/Makefile (tests): Add tst-mutex-errorcheck.
	* nptl/tst-mutex-errorcheck.c: New file.
---
 nptl/Makefile                  |  2 +-
 nptl/pthread_mutex_timedlock.c |  3 ++-
 nptl/tst-mutex-errorcheck.c    | 58 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 nptl/tst-mutex-errorcheck.c

diff --git a/nptl/Makefile b/nptl/Makefile
index 66364d7..9dda4df 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -288,7 +288,7 @@ tests = tst-typesizes \
 	tst-initializers1 $(addprefix tst-initializers1-,\
 			    c89 gnu89 c99 gnu99 c11 gnu11) \
 	tst-bad-schedattr \
-	tst-thread_local1
+	tst-thread_local1 tst-mutex-errorcheck
 xtests = tst-setuid1 tst-setuid1-static tst-setuid2 \
 	tst-mutexpp1 tst-mutexpp6 tst-mutexpp10
 test-srcs = tst-oddstacklimit
diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index 9055e11..07f0901 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -89,7 +89,8 @@ pthread_mutex_timedlock (pthread_mutex_t *mutex,
       if (__glibc_unlikely (mutex->__data.__owner == id))
 	return EDEADLK;
 
-      /* FALLTHROUGH */
+      /* Don't do lock elision on an error checking mutex.  */
+      goto simple;
 
     case PTHREAD_MUTEX_TIMED_NP:
       FORCE_ELISION (mutex, goto elision);
diff --git a/nptl/tst-mutex-errorcheck.c b/nptl/tst-mutex-errorcheck.c
new file mode 100644
index 0000000..9922169
--- /dev/null
+++ b/nptl/tst-mutex-errorcheck.c
@@ -0,0 +1,58 @@
+/* Check that error checking mutexes are not subject to lock elision.
+   Copyright (C) 2016 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 <stdio.h>
+#include <errno.h>
+#include <time.h>
+#include <pthread.h>
+
+int
+main (void)
+{
+  struct timespec tms = { 0 };
+  pthread_mutex_t mutex;
+  pthread_mutexattr_t mutexattr;
+  int ret = 0;
+
+  if (pthread_mutexattr_init (&mutexattr) != 0)
+    return 1;
+  if (pthread_mutexattr_settype (&mutexattr, PTHREAD_MUTEX_ERRORCHECK) != 0)
+    return 1;
+
+  if (pthread_mutex_init (&mutex, &mutexattr) != 0)
+    return 1;
+  if (pthread_mutexattr_destroy (&mutexattr) != 0)
+    return 1;
+
+  /* The call to pthread_mutex_timedlock erroneously enabled lock elision
+     on the mutex, which then triggered an assertion failure in
+     pthread_mutex_unlock.  It would also defeat the error checking nature
+     of the mutex.  */
+  if (pthread_mutex_timedlock (&mutex, &tms) != 0)
+    return 1;
+  if (pthread_mutex_timedlock (&mutex, &tms) != EDEADLK)
+    {
+      printf ("Failed error checking on locked mutex\n");
+      ret = 1;
+    }
+
+  if (pthread_mutex_unlock (&mutex) != 0)
+    ret = 1;
+
+  return ret;
+}
-- 
2.7.0

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."