From: Bart Van Assche <bvanassche@acm.org>
Date: Wed, 27 Feb 2008 16:13:05 +0000 (+0000)
Subject: An error message is now printed before attempting to lock a non-recursive mutex recur... 
X-Git-Tag: svn/VALGRIND_3_4_0~1019
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7d068d3fa812536a1f6504f8d28a5adb1a85e904;p=thirdparty%2Fvalgrind.git

An error message is now printed before attempting to lock a non-recursive mutex recursively.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@7490
---

diff --git a/exp-drd/drd_clientreq.c b/exp-drd/drd_clientreq.c
index 8d35dd1710..822de43fd8 100644
--- a/exp-drd/drd_clientreq.c
+++ b/exp-drd/drd_clientreq.c
@@ -64,7 +64,7 @@ static void drd_post_cond_wait(const Addr cond, const Addr mutex,
                                const SizeT size, const MutexT mutex_type)
 {
    cond_post_wait(cond);
-   mutex_lock(mutex, size, mutex_type);
+   mutex_post_lock(mutex, size, mutex_type);
 }
 
 static void drd_pre_cond_signal(const Addr cond)
diff --git a/exp-drd/drd_main.c b/exp-drd/drd_main.c
index c59b99ce19..df25129d80 100644
--- a/exp-drd/drd_main.c
+++ b/exp-drd/drd_main.c
@@ -431,10 +431,7 @@ void drd_pre_mutex_lock(const DrdThreadId drd_tid,
                         const SizeT size,
                         const MutexT mutex_type)
 {
-   if (mutex_get(mutex) == 0)
-   {
-      mutex_init(mutex, size, mutex_type);
-   }
+   mutex_pre_lock(mutex, size, mutex_type);
 }
 
 void drd_post_mutex_lock(const DrdThreadId drd_tid,
@@ -442,7 +439,7 @@ void drd_post_mutex_lock(const DrdThreadId drd_tid,
                          const SizeT size,
                          const MutexT mutex_type)
 {
-   mutex_lock(mutex, size, mutex_type);
+   mutex_post_lock(mutex, size, mutex_type);
 }
 
 void drd_pre_mutex_unlock(const DrdThreadId drd_tid,
diff --git a/exp-drd/drd_mutex.c b/exp-drd/drd_mutex.c
index d7a1072213..491d047cc7 100644
--- a/exp-drd/drd_mutex.c
+++ b/exp-drd/drd_mutex.c
@@ -244,12 +244,40 @@ struct mutex_info* mutex_get(const Addr mutex)
   return 0;
 }
 
+/** Called before pthread_mutex_lock() is invoked. If a data structure for
+ *  the client-side object was not yet created, do this now. Also check whether
+ *  an attempt is made to lock recursively a synchronization object that must
+ *  not be locked recursively.
+ */
+void mutex_pre_lock(const Addr mutex, const SizeT size, MutexT mutex_type)
+{
+  struct mutex_info* p = mutex_get(mutex);
+  if (p == 0)
+  {
+    mutex_init(mutex, size, mutex_type);
+    p = mutex_get(mutex);
+  }
+  tl_assert(p);
+
+  if (p->owner == thread_get_running_tid()
+      && p->recursion_count >= 1
+      && mutex_type != mutex_type_recursive_mutex)
+  {
+    MutexErrInfo MEI = { p->mutex, p->recursion_count, p->owner };
+    VG_(maybe_record_error)(VG_(get_running_tid)(),
+                            MutexErr,
+                            VG_(get_IP)(VG_(get_running_tid)()),
+                            "Recursive locking not allowed",
+                            &MEI);
+  }
+}
+
 /**
  * Update mutex_info state when locking the pthread_mutex_t mutex.
  * Note: this function must be called after pthread_mutex_lock() has been
  * called, or a race condition is triggered !
  */
-int mutex_lock(const Addr mutex, const SizeT size, MutexT mutex_type)
+int mutex_post_lock(const Addr mutex, const SizeT size, MutexT mutex_type)
 {
   const DrdThreadId drd_tid = VgThreadIdToDrdThreadId(VG_(get_running_tid)());
   struct mutex_info* const p = mutex_get_or_allocate(mutex, size, mutex_type);
@@ -286,13 +314,6 @@ int mutex_lock(const Addr mutex, const SizeT size, MutexT mutex_type)
   tl_assert(p->mutex_type == mutex_type);
   tl_assert(p->size == size);
 
-  if (p->recursion_count >= 1 && mutex_type == mutex_type_spinlock)
-  {
-    // TO DO: tell the user in a more friendly way that it is not allowed to
-    // lock spinlocks recursively.
-    tl_assert(0);
-  }
-
   if (p->recursion_count == 0)
   {
     p->owner = drd_tid;
diff --git a/exp-drd/drd_mutex.h b/exp-drd/drd_mutex.h
index 7eb7821c16..8be15bdcd2 100644
--- a/exp-drd/drd_mutex.h
+++ b/exp-drd/drd_mutex.h
@@ -45,7 +45,8 @@ struct mutex_info* mutex_init(const Addr mutex, const SizeT size,
 void mutex_pre_destroy(struct mutex_info* const p);
 void mutex_post_destroy(const Addr mutex);
 struct mutex_info* mutex_get(const Addr mutex);
-int mutex_lock(const Addr mutex, const SizeT size, const MutexT mutex_type);
+void mutex_pre_lock(const Addr mutex, const SizeT size, const MutexT mutex_type);
+int mutex_post_lock(const Addr mutex, const SizeT size, const MutexT mutex_type);
 int mutex_unlock(const Addr mutex, const MutexT mutex_type);
 const char* mutex_get_typename(struct mutex_info* const p);
 const char* mutex_type_name(const MutexT mt);
diff --git a/exp-drd/tests/recursive_mutex.c b/exp-drd/tests/recursive_mutex.c
index 62d90e3cf8..069c862d85 100644
--- a/exp-drd/tests/recursive_mutex.c
+++ b/exp-drd/tests/recursive_mutex.c
@@ -1,5 +1,5 @@
-/** Initialize a recursive mutex and lock it twice.
- *  No error messages may be printed.
+/** Initialize several kinds of mutexes and lock each mutex twice.
+ *  Note: locking a regular mutex twice causes a deadlock.
  */
 
 #define _GNU_SOURCE
@@ -17,6 +17,9 @@ static void lock_twice(pthread_mutex_t* const p)
 
 int main(int argc, char** argv)
 {
+  /* Let the program abort after 3 seconds instead of leaving it deadlocked. */
+  alarm(3);
+
   {
     pthread_mutex_t m = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
 
@@ -51,7 +54,9 @@ int main(int argc, char** argv)
     pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER;
 
     printf("Non-recursive mutex.\n");
+    fflush(stdout);
     lock_twice(&m);
   } 
+  printf("Done.\n");
   return 0;
 }
diff --git a/exp-drd/tests/recursive_mutex.stderr.exp b/exp-drd/tests/recursive_mutex.stderr.exp
new file mode 100644
index 0000000000..4384a2bea0
--- /dev/null
+++ b/exp-drd/tests/recursive_mutex.stderr.exp
@@ -0,0 +1,17 @@
+
+Recursive locking not allowed: address 0x........, recursion count 1, owner 1.
+   at 0x........: pthread_mutex_lock (drd_intercepts.c:?)
+   by 0x........: lock_twice (recursive_mutex.c:?)
+   by 0x........: main (recursive_mutex.c:?)
+
+Attempt to unlock a mutex that is not locked: address 0x........, recursion count -1, owner 1.
+   at 0x........: pthread_mutex_unlock (drd_intercepts.c:?)
+   by 0x........: lock_twice (recursive_mutex.c:?)
+   by 0x........: main (recursive_mutex.c:?)
+
+Recursive locking not allowed: address 0x........, recursion count 1, owner 1.
+   at 0x........: pthread_mutex_lock (drd_intercepts.c:?)
+   by 0x........: lock_twice (recursive_mutex.c:?)
+   by 0x........: main (recursive_mutex.c:?)
+
+ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)
diff --git a/exp-drd/tests/recursive_mutex.stdout.exp b/exp-drd/tests/recursive_mutex.stdout.exp
new file mode 100644
index 0000000000..41dca6e038
--- /dev/null
+++ b/exp-drd/tests/recursive_mutex.stdout.exp
@@ -0,0 +1,4 @@
+Recursive mutex (statically initialized).
+Recursive mutex (initialized via mutex attributes).
+Error checking mutex.
+Non-recursive mutex.
diff --git a/exp-drd/tests/recursive_mutex.vgtest b/exp-drd/tests/recursive_mutex.vgtest
new file mode 100644
index 0000000000..ff88b467b0
--- /dev/null
+++ b/exp-drd/tests/recursive_mutex.vgtest
@@ -0,0 +1 @@
+prog: recursive_mutex