From 31d4f926fc949255b45d4121c532ed59d0695723 Mon Sep 17 00:00:00 2001 From: Julian Seward Date: Thu, 29 Jul 2010 05:28:02 +0000 Subject: [PATCH] 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 --- helgrind/hg_errors.c | 26 +++++++++++++++++++++++--- helgrind/hg_errors.h | 5 ++++- helgrind/hg_main.c | 10 ++++++++-- 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/helgrind/hg_errors.c b/helgrind/hg_errors.c index a1ece966df..497fe58f01 100644 --- 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( " \n" ); 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 ); + } } 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 3714346497..2a19fe36bb 100644 --- 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 ab9da5d0de..1be4f1382b 100644 --- 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 ); + } } } -- 2.47.2