diff mbox

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

Message ID 1453580875-29240-1-git-send-email-aurelien@aurel32.net
State New
Headers show

Commit Message

Aurelien Jarno Jan. 23, 2016, 8:27 p.m. UTC 
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(-)

Comments

Andreas Schwab Jan. 23, 2016, 9:38 p.m. UTC | #1 
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. UTC | #2 
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. UTC | #3 
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.
diff mbox

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;