Patchwork pthread_mutex_unlock: fix wrong assertion with lock elision (bug#19515)

login
register
mail settings
Submitter Aurelien Jarno
Date Jan. 23, 2016, 8:27 p.m.
Message ID <1453580875-29240-1-git-send-email-aurelien@aurel32.net>
Download mbox | patch
Permalink /patch/10532/
State New
Headers show

Comments

Aurelien Jarno - Jan. 23, 2016, 8:27 p.m.
Commit e8c659d7 slightly changed the mutex type to store the elision
FLAG via the ELISION. It means the mutex type should no be accessed
using PTHREAD_MUTEX_TYPE. One path has been forgotten in this patch,
causing the assert to wrongly trigger when lock elision is enabled
and available. Fix that.

2016-01-23  Aurelien Jarno  <aurelien@aurel32.net>

	[BZ #19515]
	* nptl/pthread_mutex_unlock.c (__pthread_mutex_unlock_usercnt):
	Use PTHREAD_MUTEX_TYPE to get the mutex type.
---
 ChangeLog                   | 6 ++++++
 nptl/pthread_mutex_unlock.c | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)
Andreas Schwab - Jan. 23, 2016, 9:38 p.m.
Aurelien Jarno <aurelien@aurel32.net> writes:

> 	[BZ #19515]
> 	* nptl/pthread_mutex_unlock.c (__pthread_mutex_unlock_usercnt):
> 	Use PTHREAD_MUTEX_TYPE to get the mutex type.

That doesn't fix the bug.  The correct patch is at
<http://permalink.gmane.org/gmane.comp.lib.glibc.alpha/58565>.

Andreas.
Aurelien Jarno - Jan. 23, 2016, 9:59 p.m.
On 2016-01-23 22:38, Andreas Schwab wrote:
> Aurelien Jarno <aurelien@aurel32.net> writes:
> 
> > 	[BZ #19515]
> > 	* nptl/pthread_mutex_unlock.c (__pthread_mutex_unlock_usercnt):
> > 	Use PTHREAD_MUTEX_TYPE to get the mutex type.
> 
> That doesn't fix the bug.  The correct patch is at
> <http://permalink.gmane.org/gmane.comp.lib.glibc.alpha/58565>.

Thanks for the link and sorry for the duplicate bug. While I agree your
patch is correct to fix the issue, I still believe the assert is wrongly
accessing the mutex type directly. It might work, but that is something
fragile that might break if the mutex flags are changed.
Andreas Schwab - Jan. 23, 2016, 10:31 p.m.
Aurelien Jarno <aurelien@aurel32.net> writes:

> Thanks for the link and sorry for the duplicate bug. While I agree your
> patch is correct to fix the issue, I still believe the assert is wrongly
> accessing the mutex type directly.

It has correctly exposed the bug.

Andreas.

Patch

diff --git a/ChangeLog b/ChangeLog
index 9418e26..5f86942 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@ 
+2016-01-23  Aurelien Jarno  <aurelien@aurel32.net>
+
+	[BZ #19515]
+	* nptl/pthread_mutex_unlock.c (__pthread_mutex_unlock_usercnt):
+	Use PTHREAD_MUTEX_TYPE to get the mutex type.
+
 2016-01-22  Paul E. Murphy  <murphyp@linux.vnet.ibm.com>
 
 	* nptl/tst-setuid3.c (is_invalid_barrier_ret): New function.
diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
index 334ce38..98571ae 100644
--- a/nptl/pthread_mutex_unlock.c
+++ b/nptl/pthread_mutex_unlock.c
@@ -82,7 +82,7 @@  __pthread_mutex_unlock_usercnt (pthread_mutex_t *mutex, int decr)
   else
     {
       /* Error checking mutex.  */
-      assert (type == PTHREAD_MUTEX_ERRORCHECK_NP);
+      assert (PTHREAD_MUTEX_TYPE (mutex) == PTHREAD_MUTEX_ERRORCHECK_NP);
       if (mutex->__data.__owner != THREAD_GETMEM (THREAD_SELF, tid)
 	  || ! lll_islocked (mutex->__data.__lock))
 	return EPERM;