diff mbox

Don't do lock elision on an error checking mutex (bug 17514)

Message ID mvma8o8qz1c.fsf@hawking.suse.de
State New
Headers show

Commit Message

Andreas Schwab Jan. 14, 2016, 9:40 a.m. UTC 
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    | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 2 deletions(-)
 create mode 100644 nptl/tst-mutex-errorcheck.c

Comments

Florian Weimer Jan. 14, 2016, 11:04 a.m. UTC | #1 
On 01/14/2016 10:40 AM, Andreas Schwab wrote:
> +  pthread_mutexattr_init (&mutexattr);
> +  pthread_mutexattr_settype (&mutexattr, PTHREAD_MUTEX_ERRORCHECK_NP);
> +
> +  pthread_mutex_init (&mutex, &mutexattr);
> +  pthread_mutexattr_destroy (&mutexattr);
> +
> +  pthread_mutex_timedlock (&mutex, &tms);
> +  /* The preceding call to pthread_mutex_timedlock erroneously enabled
> +     lock elision on the mutex, which triggered an assertion failure
> +     during unlock.  */

This comment is confusing because it temporal relationship is unclear
(past execution vs. past versions of glibc).

> +  pthread_mutex_unlock (&mutex);

Please add error checking for all the return values.  I believe the
error-checking mutex will give you a consistent result here.

Florian
diff mbox

Patch

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..58d0ff9
--- /dev/null
+++ b/nptl/tst-mutex-errorcheck.c
@@ -0,0 +1,42 @@ 
+/* 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 <pthread.h>
+#include <time.h>
+
+int
+main (void)
+{
+  struct timespec tms = { 0 };
+  pthread_mutex_t mutex;
+  pthread_mutexattr_t mutexattr;
+
+  pthread_mutexattr_init (&mutexattr);
+  pthread_mutexattr_settype (&mutexattr, PTHREAD_MUTEX_ERRORCHECK_NP);
+
+  pthread_mutex_init (&mutex, &mutexattr);
+  pthread_mutexattr_destroy (&mutexattr);
+
+  pthread_mutex_timedlock (&mutex, &tms);
+  /* The preceding call to pthread_mutex_timedlock erroneously enabled
+     lock elision on the mutex, which triggered an assertion failure
+     during unlock.  */
+  pthread_mutex_unlock (&mutex);
+
+  return 0;
+}