Fix sanity check crash in Helgrind. Partial fix for #255353.

(Philippe Waroquiers, philippe.waroquiers@skynet.be)

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

diff --git a/helgrind/hg_errors.c b/helgrind/hg_errors.c
index 753c739b915f9a44b0539e848541dc8e880d108b..6bc9482cc8c01924b5a0852356e289173f714f1a 100644 (file)
--- a/helgrind/hg_errors.c
+++ b/helgrind/hg_errors.c
@@ -123,7 +123,8 @@ static Lock* mk_LockP_from_LockN ( Lock* lkn )
    if (!VG_(lookupFM)( map_LockN_to_P, NULL, (Word*)&lkp, (Word)lkn)) {
       lkp = HG_(zalloc)( "hg.mLPfLN.2", sizeof(Lock) );
       *lkp = *lkn;
-      lkp->admin = NULL;
+      lkp->admin_next = NULL;
+      lkp->admin_prev = NULL;
       lkp->magic = LockP_MAGIC;
       /* Forget about the bag of lock holders - don't copy that.
          Also, acquired_at should be NULL whenever heldBy is, and vice

diff --git a/helgrind/hg_lock_n_thread.h b/helgrind/hg_lock_n_thread.h
index e4f782a503e501f9fe631f03defb37bb6ce89f2d..78948fe11206989bfddabbe0808d1e5d01ae1498 100644 (file)
--- a/helgrind/hg_lock_n_thread.h
+++ b/helgrind/hg_lock_n_thread.h
@@ -119,7 +119,8 @@ typedef
 typedef
    struct _Lock {
       /* ADMIN */
-      struct _Lock* admin;
+      struct _Lock* admin_next; /* fields for a double linked */
+      struct _Lock* admin_prev; /* list of these locks */
       ULong         unique; /* used for persistence-hashing */
       UInt          magic;  /* LockN_MAGIC or LockP_MAGIC */
       /* EXPOSITION */

diff --git a/helgrind/hg_main.c b/helgrind/hg_main.c
index f22f39ac57218a643a967d6c296f91420d62f263..21a473d210ce19b91256052b72a61102f105b185 100644 (file)
--- a/helgrind/hg_main.c
+++ b/helgrind/hg_main.c
@@ -132,7 +132,9 @@ static void all__sanity_check ( Char* who ); /* fwds */
 /* Admin linked list of Threads */
 static Thread* admin_threads = NULL;
 
-/* Admin linked list of Locks */
+/* Admin double linked list of Locks */
+/* We need a double linked list to properly and efficiently
+   handle del_LockN. */
 static Lock* admin_locks = NULL;
 
 /* Mapping table for core ThreadIds to Thread* */
@@ -182,10 +184,15 @@ static Thread* mk_Thread ( Thr* hbthr ) {
 }
 
 // Make a new lock which is unlocked (hence ownerless)
+// and insert the new lock in admin_locks double linked list.
 static Lock* mk_LockN ( LockKind kind, Addr guestaddr ) {
    static ULong unique = 0;
    Lock* lock             = HG_(zalloc)( "hg.mk_Lock.1", sizeof(Lock) );
-   lock->admin            = admin_locks;
+   lock->admin_next       = admin_locks;
+   lock->admin_prev       = NULL;
+   if (admin_locks)
+      admin_locks->admin_prev = lock;
+   admin_locks            = lock;
    lock->unique           = unique++;
    lock->magic            = LockN_MAGIC;
    lock->appeared_at      = NULL;
@@ -196,12 +203,11 @@ static Lock* mk_LockN ( LockKind kind, Addr guestaddr ) {
    lock->heldW            = False;
    lock->heldBy           = NULL;
    tl_assert(HG_(is_sane_LockN)(lock));
-   admin_locks            = lock;
    return lock;
 }
 
 /* Release storage for a Lock.  Also release storage in .heldBy, if
-   any. */
+   any. Removes from admin_locks double linked list. */
 static void del_LockN ( Lock* lk ) 
 {
    tl_assert(HG_(is_sane_LockN)(lk));
@@ -209,6 +215,16 @@ static void del_LockN ( Lock* lk )
    libhb_so_dealloc(lk->hbso);
    if (lk->heldBy)
       VG_(deleteBag)( lk->heldBy );
+   if (admin_locks == lk) {
+      admin_locks = lk->admin_next;
+      if (admin_locks)
+         admin_locks->admin_prev = NULL;
+   }
+   else {
+      lk->admin_prev->admin_next = lk->admin_next;
+      lk->admin_next->admin_prev = lk->admin_prev;
+   }
+      
    VG_(memset)(lk, 0xAA, sizeof(*lk));
    HG_(free)(lk);
 }
@@ -446,8 +462,9 @@ static void pp_Lock ( Int d, Lock* lk )
 {
    space(d+0); VG_(printf)("Lock %p (ga %#lx) {\n", lk, lk->guestaddr);
    if (sHOW_ADMIN) {
-      space(d+3); VG_(printf)("admin  %p\n",   lk->admin);
-      space(d+3); VG_(printf)("magic  0x%x\n", (UInt)lk->magic);
+      space(d+3); VG_(printf)("admin_n  %p\n",   lk->admin_next);
+      space(d+3); VG_(printf)("admin_p  %p\n",   lk->admin_prev);
+      space(d+3); VG_(printf)("magic    0x%x\n", (UInt)lk->magic);
    }
    space(d+3); VG_(printf)("unique %llu\n", lk->unique);
    space(d+3); VG_(printf)("kind   %s\n", show_LockKind(lk->kind));
@@ -471,11 +488,11 @@ static void pp_admin_locks ( Int d )
 {
    Int   i, n;
    Lock* lk;
-   for (n = 0, lk = admin_locks;  lk;  n++, lk = lk->admin) {
+   for (n = 0, lk = admin_locks;  lk;  n++, lk = lk->admin_next) {
       /* nothing */
    }
    space(d); VG_(printf)("admin_locks (%d records) {\n", n);
-   for (i = 0, lk = admin_locks;  lk;  i++, lk = lk->admin) {
+   for (i = 0, lk = admin_locks;  lk;  i++, lk = lk->admin_next) {
       if (0) {
          space(n); 
          VG_(printf)("admin_locks record %d of %d:\n", i, n);
@@ -843,7 +860,7 @@ static void locks__sanity_check ( Char* who )
    Lock*     lk;
    Int       i;
    // # entries in admin_locks == # entries in map_locks
-   for (i = 0, lk = admin_locks;  lk;  i++, lk = lk->admin)
+   for (i = 0, lk = admin_locks;  lk;  i++, lk = lk->admin_next)
       ;
    if (i != VG_(sizeFM)(map_locks)) BAD("1");
    // for each entry (gla, lk) in map_locks
@@ -855,7 +872,7 @@ static void locks__sanity_check ( Char* who )
    }
    VG_(doneIterFM)( map_locks );
    // scan through admin_locks ...
-   for (lk = admin_locks; lk; lk = lk->admin) {
+   for (lk = admin_locks; lk; lk = lk->admin_next) {
       // lock is sane.  Quite comprehensive, also checks that
       // referenced (holder) threads are sane.
       if (!HG_(is_sane_LockN)(lk)) BAD("3");