Merge patch from JeremyF:
22-mutex-destroy-unlock:
It seems that glibc assumes in its internal use of pthreads that
destroying a lock implicity unlocks it. This patch handles this case
so that lock ownership tracking is accurate.
Also handles the case of the dyanmic linker wanting to do locking
before Valgrind has started up. vg_libpthread now implements toy
lock/unlock functions to keep it happy and leave the locks in a
sensible state. Implements some untested code to handle the case where
a lock is taken before Valgrind is running but released afterwards.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@1297 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/coregrind/arch/x86-linux/vg_libpthread.c b/coregrind/arch/x86-linux/vg_libpthread.c
index e4a680e..7d5a789 100644
--- a/coregrind/arch/x86-linux/vg_libpthread.c
+++ b/coregrind/arch/x86-linux/vg_libpthread.c
@@ -198,11 +198,6 @@
}
-static void not_inside ( char* msg )
-{
- VG_(startup)();
-}
-
__attribute__((noreturn))
void vgPlain_unimp ( char* what )
{
@@ -896,15 +891,19 @@
int __pthread_mutex_lock(pthread_mutex_t *mutex)
{
int res;
- static int moans = N_MOANS;
+
if (RUNNING_ON_VALGRIND) {
VALGRIND_MAGIC_SEQUENCE(res, 0 /* default */,
VG_USERREQ__PTHREAD_MUTEX_LOCK,
mutex, 0, 0, 0);
return res;
} else {
- if (moans-- > 0)
- not_inside("pthread_mutex_lock");
+ /* Play at locking */
+ if (0)
+ kludged("prehistoric lock");
+ mutex->__m_owner = (_pthread_descr)1;
+ mutex->__m_count = 1;
+ mutex->__m_kind |= VG_PTHREAD_PREHISTORY;
return 0; /* success */
}
}
@@ -913,16 +912,20 @@
int __pthread_mutex_trylock(pthread_mutex_t *mutex)
{
int res;
- static int moans = N_MOANS;
+
if (RUNNING_ON_VALGRIND) {
VALGRIND_MAGIC_SEQUENCE(res, 0 /* default */,
VG_USERREQ__PTHREAD_MUTEX_TRYLOCK,
mutex, 0, 0, 0);
return res;
} else {
- if (moans-- > 0)
- not_inside("pthread_mutex_trylock");
- return 0;
+ /* Play at locking */
+ if (0)
+ kludged("prehistoric trylock");
+ mutex->__m_owner = (_pthread_descr)1;
+ mutex->__m_count = 1;
+ mutex->__m_kind |= VG_PTHREAD_PREHISTORY;
+ return 0; /* success */
}
}
@@ -930,16 +933,20 @@
int __pthread_mutex_unlock(pthread_mutex_t *mutex)
{
int res;
- static int moans = N_MOANS;
+
if (RUNNING_ON_VALGRIND) {
VALGRIND_MAGIC_SEQUENCE(res, 0 /* default */,
VG_USERREQ__PTHREAD_MUTEX_UNLOCK,
mutex, 0, 0, 0);
return res;
} else {
- if (moans-- > 0)
- not_inside("pthread_mutex_unlock");
- return 0;
+ /* Play at locking */
+ if (0)
+ kludged("prehistoric unlock");
+ mutex->__m_owner = 0;
+ mutex->__m_count = 0;
+ mutex->__m_kind &= ~VG_PTHREAD_PREHISTORY;
+ return 0; /* success */
}
}
@@ -949,9 +956,13 @@
/* Valgrind doesn't hold any resources on behalf of the mutex, so no
need to involve it. */
if (mutex->__m_count > 0) {
- pthread_error("pthread_mutex_destroy: "
- "mutex is still in use");
- return EBUSY;
+ /* Oh, the horror. glibc's internal use of pthreads "knows"
+ that destroying a lock does an implicit unlock. Make it
+ explicit. */
+ __pthread_mutex_unlock(mutex);
+ pthread_error("pthread_mutex_destroy: "
+ "mutex is still in use");
+ return EBUSY;
}
mutex->__m_count = 0;
mutex->__m_owner = (_pthread_descr)VG_INVALID_THREADID;
@@ -2254,6 +2265,7 @@
if (pid == 0) {
/* I am the child */
run_fork_handlers(2 /* child */);
+ __pthread_mutex_unlock(&pthread_atfork_lock);
__pthread_mutex_init(&pthread_atfork_lock, NULL);
} else {
/* I am the parent */
diff --git a/coregrind/vg_include.h b/coregrind/vg_include.h
index a36b67d..0db3c65 100644
--- a/coregrind/vg_include.h
+++ b/coregrind/vg_include.h
@@ -805,6 +805,13 @@
} while (0)
+/* This is or'd into a pthread mutex's __m_kind field if it is used
+ before Valgrind is up and running (prehistory). This is used so
+ that if some early code (like the dynamic linker) takes a lock
+ before Valgrind starts and then releases it afterwards, we can work
+ out what's happening. */
+#define VG_PTHREAD_PREHISTORY 0x80000000
+
/* ---------------------------------------------------------------------
Exports of vg_signals.c
------------------------------------------------------------------ */
diff --git a/coregrind/vg_libpthread.c b/coregrind/vg_libpthread.c
index e4a680e..7d5a789 100644
--- a/coregrind/vg_libpthread.c
+++ b/coregrind/vg_libpthread.c
@@ -198,11 +198,6 @@
}
-static void not_inside ( char* msg )
-{
- VG_(startup)();
-}
-
__attribute__((noreturn))
void vgPlain_unimp ( char* what )
{
@@ -896,15 +891,19 @@
int __pthread_mutex_lock(pthread_mutex_t *mutex)
{
int res;
- static int moans = N_MOANS;
+
if (RUNNING_ON_VALGRIND) {
VALGRIND_MAGIC_SEQUENCE(res, 0 /* default */,
VG_USERREQ__PTHREAD_MUTEX_LOCK,
mutex, 0, 0, 0);
return res;
} else {
- if (moans-- > 0)
- not_inside("pthread_mutex_lock");
+ /* Play at locking */
+ if (0)
+ kludged("prehistoric lock");
+ mutex->__m_owner = (_pthread_descr)1;
+ mutex->__m_count = 1;
+ mutex->__m_kind |= VG_PTHREAD_PREHISTORY;
return 0; /* success */
}
}
@@ -913,16 +912,20 @@
int __pthread_mutex_trylock(pthread_mutex_t *mutex)
{
int res;
- static int moans = N_MOANS;
+
if (RUNNING_ON_VALGRIND) {
VALGRIND_MAGIC_SEQUENCE(res, 0 /* default */,
VG_USERREQ__PTHREAD_MUTEX_TRYLOCK,
mutex, 0, 0, 0);
return res;
} else {
- if (moans-- > 0)
- not_inside("pthread_mutex_trylock");
- return 0;
+ /* Play at locking */
+ if (0)
+ kludged("prehistoric trylock");
+ mutex->__m_owner = (_pthread_descr)1;
+ mutex->__m_count = 1;
+ mutex->__m_kind |= VG_PTHREAD_PREHISTORY;
+ return 0; /* success */
}
}
@@ -930,16 +933,20 @@
int __pthread_mutex_unlock(pthread_mutex_t *mutex)
{
int res;
- static int moans = N_MOANS;
+
if (RUNNING_ON_VALGRIND) {
VALGRIND_MAGIC_SEQUENCE(res, 0 /* default */,
VG_USERREQ__PTHREAD_MUTEX_UNLOCK,
mutex, 0, 0, 0);
return res;
} else {
- if (moans-- > 0)
- not_inside("pthread_mutex_unlock");
- return 0;
+ /* Play at locking */
+ if (0)
+ kludged("prehistoric unlock");
+ mutex->__m_owner = 0;
+ mutex->__m_count = 0;
+ mutex->__m_kind &= ~VG_PTHREAD_PREHISTORY;
+ return 0; /* success */
}
}
@@ -949,9 +956,13 @@
/* Valgrind doesn't hold any resources on behalf of the mutex, so no
need to involve it. */
if (mutex->__m_count > 0) {
- pthread_error("pthread_mutex_destroy: "
- "mutex is still in use");
- return EBUSY;
+ /* Oh, the horror. glibc's internal use of pthreads "knows"
+ that destroying a lock does an implicit unlock. Make it
+ explicit. */
+ __pthread_mutex_unlock(mutex);
+ pthread_error("pthread_mutex_destroy: "
+ "mutex is still in use");
+ return EBUSY;
}
mutex->__m_count = 0;
mutex->__m_owner = (_pthread_descr)VG_INVALID_THREADID;
@@ -2254,6 +2265,7 @@
if (pid == 0) {
/* I am the child */
run_fork_handlers(2 /* child */);
+ __pthread_mutex_unlock(&pthread_atfork_lock);
__pthread_mutex_init(&pthread_atfork_lock, NULL);
} else {
/* I am the parent */
diff --git a/coregrind/vg_scheduler.c b/coregrind/vg_scheduler.c
index 0226d46..a183e4d 100644
--- a/coregrind/vg_scheduler.c
+++ b/coregrind/vg_scheduler.c
@@ -2480,6 +2480,13 @@
return;
}
+ /* If this was locked before the dawn of time, pretend it was
+ locked now so that it balances with unlocks */
+ if (mutex->__m_kind & VG_PTHREAD_PREHISTORY) {
+ mutex->__m_kind &= ~VG_PTHREAD_PREHISTORY;
+ VG_TRACK( post_mutex_lock, (ThreadId)mutex->__m_owner, mutex );
+ }
+
/* More paranoia ... */
switch (mutex->__m_kind) {
# ifndef GLIBC_2_1
diff --git a/glibc-2.2.supp b/glibc-2.2.supp
index 3dbb2bb..859ed58 100644
--- a/glibc-2.2.supp
+++ b/glibc-2.2.supp
@@ -138,6 +138,7 @@
#}
#-------- Threading bugs?
+# glibc 'knows' that destroying a locked mutex will unlock it
{
pthread_error/__pthread_mutex_destroy/__closedir
core:PThread
@@ -161,13 +162,6 @@
fun:_IO_funlockfile
}
-{
- __pthread_mutex_unlock/__register_frame_info
- core:PThread
- fun:__pthread_mutex_unlock
- fun:__register_frame_info
-}
-
# even more glibc suppressions ?
{
libc-2.2.4.so/libc-2.2.4.so/libc-2.2.4.so(Cond)