When reporting an error about an attempt to re-lock an already-locked

lock, also report the stack where the lock was previously locked.
This makes it easier to diagnose deadlocks.

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

diff --git a/helgrind/hg_errors.c b/helgrind/hg_errors.c
index a1ece966df8ab6cca1054b9ed460d233225b3f76..497fe58f0124173f9dc10c32b529758c4f3ffc25 100644 (file)
--- a/helgrind/hg_errors.c
+++ b/helgrind/hg_errors.c
@@ -222,8 +222,10 @@ typedef
             ExeContext* after_ec;
          } LockOrder;
          struct {
-            Thread* thr;
-            HChar*  errstr; /* persistent, in tool-arena */
+            Thread*     thr;
+            HChar*      errstr; /* persistent, in tool-arena */
+            HChar*      auxstr; /* optional, persistent, in tool-arena */
+            ExeContext* auxctx; /* optional */
          } Misc;
       } XE;
    }
@@ -507,7 +509,8 @@ void HG_(record_error_PthAPIerror) ( Thread* thr, HChar* fnname,
                             XE_PthAPIerror, 0, NULL, &xe );
 }
 
-void HG_(record_error_Misc) ( Thread* thr, HChar* errstr )
+void HG_(record_error_Misc_w_aux) ( Thread* thr, HChar* errstr,
+                                    HChar* auxstr, ExeContext* auxctx )
 {
    XError xe;
    tl_assert( HG_(is_sane_Thread)(thr) );
@@ -516,6 +519,8 @@ void HG_(record_error_Misc) ( Thread* thr, HChar* errstr )
    xe.tag = XE_Misc;
    xe.XE.Misc.thr    = thr;
    xe.XE.Misc.errstr = string_table_strdup(errstr);
+   xe.XE.Misc.auxstr = auxstr ? string_table_strdup(auxstr) : NULL;
+   xe.XE.Misc.auxctx = auxctx;
    // FIXME: tid vs thr
    tl_assert( HG_(is_sane_ThreadId)(thr->coretid) );
    tl_assert( thr->coretid != VG_INVALID_THREADID );
@@ -523,6 +528,11 @@ void HG_(record_error_Misc) ( Thread* thr, HChar* errstr )
                             XE_Misc, 0, NULL, &xe );
 }
 
+void HG_(record_error_Misc) ( Thread* thr, HChar* errstr )
+{
+   HG_(record_error_Misc_w_aux)(thr, errstr, NULL, NULL);
+}
+
 Bool HG_(eq_Error) ( VgRes not_used, Error* e1, Error* e2 )
 {
    XError *xe1, *xe2;
@@ -716,6 +726,11 @@ void HG_(pp_Error) ( Error* err )
                (Int)xe->XE.Misc.thr->errmsg_index );
          emit( "  </xwhat>\n" );
          VG_(pp_ExeContext)( VG_(get_error_where)(err) );
+         if (xe->XE.Misc.auxstr) {
+            emit("  <auxwhat>%s</auxwhat>\n", xe->XE.Misc.auxstr);
+            if (xe->XE.Misc.auxctx)
+               VG_(pp_ExeContext)( xe->XE.Misc.auxctx );
+         }
 
       } else {
 
@@ -723,6 +738,11 @@ void HG_(pp_Error) ( Error* err )
                (Int)xe->XE.Misc.thr->errmsg_index,
                xe->XE.Misc.errstr );
          VG_(pp_ExeContext)( VG_(get_error_where)(err) );
+         if (xe->XE.Misc.auxstr) {
+            emit(" %s\n", xe->XE.Misc.auxstr);
+            if (xe->XE.Misc.auxctx)
+               VG_(pp_ExeContext)( xe->XE.Misc.auxctx );
+         }
 
       }
       break;

diff --git a/helgrind/hg_errors.h b/helgrind/hg_errors.h
index 3714346497cd4b52fa18f477ad8f568fea963f82..2a19fe36bb3499bf83944bbc195948d74ac91fd0 100644 (file)
--- a/helgrind/hg_errors.h
+++ b/helgrind/hg_errors.h
@@ -59,7 +59,10 @@ void HG_(record_error_UnlockBogus)    ( Thread*, Addr );
 void HG_(record_error_PthAPIerror)    ( Thread*, HChar*, Word, HChar* );
 void HG_(record_error_LockOrder)      ( Thread*, Addr, Addr,
                                         ExeContext*, ExeContext* );
-void HG_(record_error_Misc)           ( Thread*, HChar* );
+void HG_(record_error_Misc_w_aux)     ( Thread*, HChar* errstr,
+                                        HChar* auxstr, ExeContext* auxctx );
+void HG_(record_error_Misc)           ( Thread* thr, HChar* errstr );
+
 
 /* Statistics pertaining to error management. */
 extern ULong HG_(stats__LockN_to_P_queries);

diff --git a/helgrind/hg_main.c b/helgrind/hg_main.c
index ab9da5d0deae6c715a69a3f6c81ee76a58ebc0ba..1be4f1382b37e854eb9cf592965e31c4d81f2022 100644 (file)
--- a/helgrind/hg_main.c
+++ b/helgrind/hg_main.c
@@ -1980,8 +1980,14 @@ static void evh__HG_PTHREAD_MUTEX_LOCK_PRE ( ThreadId tid,
          this is a real lock operation (not a speculative "tryLock"
          kind of thing).  Duh.  Deadlock coming up; but at least
          produce an error message. */
-      HG_(record_error_Misc)( thr, "Attempt to re-lock a "
-                                   "non-recursive lock I already hold" );
+      HChar* errstr = "Attempt to re-lock a "
+                      "non-recursive lock I already hold";
+      HChar* auxstr = "Lock was previously acquired";
+      if (lk->acquired_at) {
+         HG_(record_error_Misc_w_aux)( thr, errstr, auxstr, lk->acquired_at );
+      } else {
+         HG_(record_error_Misc)( thr, errstr );
+      }
    }
 }