A memory pool update from Graydon Hoare.

Here's an update to the mempool move / change client requests and sanity
checking. The following changes are present:

   - Added one more (hopefully last) client request, a predicate to
     test whether a mempool anchor address is currently tracked.
     It turns out mozilla's arena-using code is sufficiently inconsistent
     in its assumptions that it's very difficult to phrase the valgrind
     client-request annotations without this request. Namely: sometime
     arena-init and arena-free operations are assumed to be idempotent.

   - Fixed a very rapid tool-memory leak in the mempool sanity check
     routine. The previous version of the patch I posted would use all
     memory even on my Very Beefy Test Machine within ~15 minutes of
     browsing with firefox.

   - Added a little logging code to print the counts of pools and chunks
     active every ~10000 sanity checks, when running with -v.

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

diff --git a/include/valgrind.h b/include/valgrind.h
index 6f767c6b94701c77e860deceac85e396eb642c54..7820572216dff30ae2af67a8220f95450ab9d840 100644 (file)
--- a/include/valgrind.h
+++ b/include/valgrind.h
@@ -2289,6 +2289,9 @@ typedef
           VG_USERREQ__MEMPOOL_ALLOC    = 0x1305,
           VG_USERREQ__MEMPOOL_FREE     = 0x1306,
           VG_USERREQ__MEMPOOL_TRIM     = 0x1307,
+          VG_USERREQ__MOVE_MEMPOOL     = 0x1308,
+          VG_USERREQ__MEMPOOL_CHANGE   = 0x1309,
+          VG_USERREQ__MEMPOOL_EXISTS   = 0x130a,
 
           /* Allow printfs to valgrind log. */
           VG_USERREQ__PRINTF           = 0x1401,
@@ -2513,6 +2516,31 @@ VALGRIND_PRINTF_BACKTRACE(const char *format, ...)
                                pool, addr, size, 0, 0);           \
    }
 
+/* Resize and/or move a piece associated with a memory pool. */
+#define VALGRIND_MOVE_MEMPOOL(poolA, poolB)                       \
+   {unsigned int _qzz_res;                                        \
+    VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0,                       \
+                               VG_USERREQ__MOVE_MEMPOOL,          \
+                               poolA, poolB, 0, 0, 0);            \
+   }
+
+/* Resize and/or move a piece associated with a memory pool. */
+#define VALGRIND_MEMPOOL_CHANGE(pool, addrA, addrB, size)         \
+   {unsigned int _qzz_res;                                        \
+    VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0,                       \
+                               VG_USERREQ__MEMPOOL_CHANGE,        \
+                               pool, addrA, addrB, size, 0);      \
+   }
+
+/* Return 1 if a mempool exists, else 0. */
+#define VALGRIND_MEMPOOL_EXISTS(pool)                             \
+   ({unsigned int _qzz_res;                                       \
+    VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0,                       \
+                               VG_USERREQ__MEMPOOL_EXISTS,        \
+                               pool, 0, 0, 0, 0);                 \
+    _qzz_res;                                                     \
+   })
+
 /* Mark a piece of memory as being a stack. Returns a stack id. */
 #define VALGRIND_STACK_REGISTER(start, end)                       \
    ({unsigned int _qzz_res;                                       \

diff --git a/memcheck/mc_include.h b/memcheck/mc_include.h
index 4fa50d2433d9be95cb6368f7f325685b18ffd61b..23a6b2c6752e1c98bc746da0cc35e59094255586 100644 (file)
--- a/memcheck/mc_include.h
+++ b/memcheck/mc_include.h
@@ -87,6 +87,9 @@ extern void MC_(mempool_alloc)   ( ThreadId tid, Addr pool,
                                    Addr addr, SizeT size );
 extern void MC_(mempool_free)    ( Addr pool, Addr addr );
 extern void MC_(mempool_trim)    ( Addr pool, Addr addr, SizeT size );
+extern void MC_(move_mempool)    ( Addr poolA, Addr poolB );
+extern void MC_(mempool_change)  ( Addr pool, Addr addrA, Addr addrB, SizeT size );
+extern Bool MC_(mempool_exists)  ( Addr pool );
 
 extern MC_Chunk* MC_(get_freed_list_head)( void );
 

diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c
index 98052484dc57f8d6332e4a91097f59e04be092de..9495bc1c2caf15dbf096c6dc33ac32ec1833b4e4 100644 (file)
--- a/memcheck/mc_main.c
+++ b/memcheck/mc_main.c
@@ -4064,7 +4064,10 @@ static Bool mc_handle_client_request ( ThreadId tid, UWord* arg, UWord* ret )
     && VG_USERREQ__DESTROY_MEMPOOL  != arg[0]
     && VG_USERREQ__MEMPOOL_ALLOC    != arg[0]
     && VG_USERREQ__MEMPOOL_FREE     != arg[0]
-    && VG_USERREQ__MEMPOOL_TRIM     != arg[0])
+    && VG_USERREQ__MEMPOOL_TRIM     != arg[0]
+    && VG_USERREQ__MOVE_MEMPOOL     != arg[0]
+    && VG_USERREQ__MEMPOOL_CHANGE   != arg[0]
+    && VG_USERREQ__MEMPOOL_EXISTS   != arg[0])
       return False;
 
    switch (arg[0]) {
@@ -4239,6 +4242,32 @@ static Bool mc_handle_client_request ( ThreadId tid, UWord* arg, UWord* ret )
          return True;
       }
 
+      case VG_USERREQ__MOVE_MEMPOOL: {
+         Addr poolA     = (Addr)arg[1];
+         Addr poolB     = (Addr)arg[2];
+
+         MC_(move_mempool) ( poolA, poolB );
+         return True;
+      }
+
+      case VG_USERREQ__MEMPOOL_CHANGE: {
+         Addr pool      = (Addr)arg[1];
+         Addr addrA     = (Addr)arg[2];
+         Addr addrB     = (Addr)arg[3];
+         UInt size      =       arg[4];
+
+         MC_(mempool_change) ( pool, addrA, addrB, size );
+         return True;
+      }
+
+      case VG_USERREQ__MEMPOOL_EXISTS: {
+         Addr pool      = (Addr)arg[1];
+
+         *ret = (UWord) MC_(mempool_exists) ( pool );
+        return True;
+      }
+
+
       default:
          VG_(message)(Vg_UserMsg, 
                       "Warning: unknown memcheck client request code %llx",

diff --git a/memcheck/mc_malloc_wrappers.c b/memcheck/mc_malloc_wrappers.c
index 02df6e207c75e3174b7159326726068a5e314cc0..8e24d11d3e5476dcbd7482e4cf03ed6949d948e5 100644 (file)
--- a/memcheck/mc_malloc_wrappers.c
+++ b/memcheck/mc_malloc_wrappers.c
@@ -41,6 +41,7 @@
 #include "pub_tool_replacemalloc.h"
 #include "pub_tool_threadstate.h"
 #include "pub_tool_tooliface.h"     // Needed for mc_include.h
+#include "pub_tool_stacktrace.h"    // For VG_(get_and_pp_StackTrace)
 
 #include "mc_include.h"
 
@@ -53,6 +54,10 @@ static SizeT cmalloc_n_mallocs  = 0;
 static SizeT cmalloc_n_frees    = 0;
 static SizeT cmalloc_bs_mallocd = 0;
 
+/* For debug printing to do with mempools: what stack trace
+   depth to show. */
+#define MEMPOOL_DEBUG_STACKTRACE_DEPTH 16
+
 
 /*------------------------------------------------------------*/
 /*--- Tracking malloc'd and free'd blocks                  ---*/
@@ -392,7 +397,21 @@ void* MC_(realloc) ( ThreadId tid, void* p_old, SizeT new_size )
 
 void MC_(create_mempool)(Addr pool, UInt rzB, Bool is_zeroed)
 {
-   MC_Mempool* mp = VG_(malloc)(sizeof(MC_Mempool));
+   MC_Mempool* mp;
+
+   if (VG_(clo_verbosity) > 2) {
+      VG_(message)(Vg_UserMsg, "create_mempool(%p, %d, %d)", 
+                               pool, rzB, is_zeroed);
+      VG_(get_and_pp_StackTrace)
+         (VG_(get_running_tid)(), MEMPOOL_DEBUG_STACKTRACE_DEPTH);
+   }
+
+   mp = VG_(HT_lookup)(MC_(mempool_list), (UWord)pool);
+   if (mp != NULL) {
+     VG_(tool_panic)("MC_(create_mempool): duplicate pool creation");
+   }
+   
+   mp = VG_(malloc)(sizeof(MC_Mempool));
    mp->pool       = pool;
    mp->rzB        = rzB;
    mp->is_zeroed  = is_zeroed;
@@ -415,6 +434,12 @@ void MC_(destroy_mempool)(Addr pool)
    MC_Chunk*   mc;
    MC_Mempool* mp;
 
+   if (VG_(clo_verbosity) > 2) {
+      VG_(message)(Vg_UserMsg, "destroy_mempool(%p)", pool);
+      VG_(get_and_pp_StackTrace)
+         (VG_(get_running_tid)(), MEMPOOL_DEBUG_STACKTRACE_DEPTH);
+   }
+
    mp = VG_(HT_remove) ( MC_(mempool_list), (UWord)pool );
 
    if (mp == NULL) {
@@ -436,15 +461,106 @@ void MC_(destroy_mempool)(Addr pool)
    VG_(free)(mp);
 }
 
+static Int 
+mp_compar(void* n1, void* n2)
+{
+   MC_Chunk* mc1 = *(MC_Chunk**)n1;
+   MC_Chunk* mc2 = *(MC_Chunk**)n2;
+   return (mc1->data < mc2->data ? -1 : 1);
+}
+
+static void 
+check_mempool_sane(MC_Mempool* mp)
+{
+   UInt n_chunks, i, bad = 0;   
+   static UInt tick = 0;
+
+   MC_Chunk **chunks = (MC_Chunk**) VG_(HT_to_array)( mp->chunks, &n_chunks );
+   if (!chunks)
+      return;
+
+   if (VG_(clo_verbosity) > 1) {
+     if (tick++ >= 10000)
+       {
+        UInt total_pools = 0, total_chunks = 0;
+        MC_Mempool* mp2;
+        
+        VG_(HT_ResetIter)(MC_(mempool_list));
+        while ( (mp2 = VG_(HT_Next)(MC_(mempool_list))) ) {
+          total_pools++;
+          VG_(HT_ResetIter)(mp2->chunks);
+          while (VG_(HT_Next)(mp2->chunks)) {
+            total_chunks++;
+          }
+        }
+        
+        VG_(message)(Vg_UserMsg, 
+                      "Total mempools active: %d pools, %d chunks\n", 
+                     total_pools, total_chunks);
+        tick = 0;
+       }
+   }
+
+
+   VG_(ssort)((void*)chunks, n_chunks, sizeof(VgHashNode*), mp_compar);
+         
+   /* Sanity check; assert that the blocks are now in order */
+   for (i = 0; i < n_chunks-1; i++) {
+      if (chunks[i]->data > chunks[i+1]->data) {
+         VG_(message)(Vg_UserMsg, 
+                      "Mempool chunk %d / %d is out of order "
+                      "wrt. its successor", 
+                      i+1, n_chunks);
+         bad = 1;
+      }
+   }
+   
+   /* Sanity check -- make sure they don't overlap */
+   for (i = 0; i < n_chunks-1; i++) {
+      if (chunks[i]->data + chunks[i]->size > chunks[i+1]->data ) {
+         VG_(message)(Vg_UserMsg, 
+                      "Mempool chunk %d / %d overlaps with its successor", 
+                      i+1, n_chunks);
+         bad = 1;
+      }
+   }
+
+   if (bad) {
+         VG_(message)(Vg_UserMsg, 
+                "Bad mempool (%d chunks), dumping chunks for inspection:",
+                      n_chunks);
+         for (i = 0; i < n_chunks; ++i) {
+            VG_(message)(Vg_UserMsg, 
+                         "Mempool chunk %d / %d: %d bytes [%x,%x), allocated:",
+                         i+1, 
+                         n_chunks, 
+                         chunks[i]->size, 
+                         chunks[i]->data, 
+                         chunks[i]->data + chunks[i]->size);
+
+            VG_(pp_ExeContext)(chunks[i]->where);
+         }
+   }
+   VG_(free)(chunks);
+}
+
 void MC_(mempool_alloc)(ThreadId tid, Addr pool, Addr addr, SizeT size)
 {
-   MC_Mempool* mp = VG_(HT_lookup) ( MC_(mempool_list), (UWord)pool );
+   MC_Mempool* mp;
 
+   if (VG_(clo_verbosity) > 2) {     
+      VG_(message)(Vg_UserMsg, "mempool_alloc(%p, %p, %d)", pool, addr, size);
+      VG_(get_and_pp_StackTrace) (tid, MEMPOOL_DEBUG_STACKTRACE_DEPTH);
+   }
+
+   mp = VG_(HT_lookup) ( MC_(mempool_list), (UWord)pool );
    if (mp == NULL) {
       MC_(record_illegal_mempool_error) ( tid, pool );
    } else {
+      check_mempool_sane(mp);
       MC_(new_block)(tid, addr, size, /*ignored*/0, mp->rzB, mp->is_zeroed,
                      MC_AllocCustom, mp->chunks);
+      check_mempool_sane(mp);
    }
 }
 
@@ -460,13 +576,26 @@ void MC_(mempool_free)(Addr pool, Addr addr)
       return;
    }
 
+   if (VG_(clo_verbosity) > 2) {
+      VG_(message)(Vg_UserMsg, "mempool_free(%p, %p)", pool, addr);
+      VG_(get_and_pp_StackTrace) (tid, MEMPOOL_DEBUG_STACKTRACE_DEPTH);
+   }
+
+   check_mempool_sane(mp);
    mc = VG_(HT_remove)(mp->chunks, (UWord)addr);
    if (mc == NULL) {
       MC_(record_free_error)(tid, (Addr)addr);
       return;
    }
 
+   if (VG_(clo_verbosity) > 2) {
+      VG_(message)(Vg_UserMsg, 
+                  "mempool_free(%p, %p) freed chunk of %d bytes", 
+                  pool, addr, mc->size);
+   }
+
    die_and_free_mem ( tid, mc, mp->rzB );
+   check_mempool_sane(mp);
 }
 
 
@@ -478,12 +607,18 @@ void MC_(mempool_trim)(Addr pool, Addr addr, SizeT size)
    UInt         n_shadows, i;
    VgHashNode** chunks;
 
+   if (VG_(clo_verbosity) > 2) {
+      VG_(message)(Vg_UserMsg, "mempool_trim(%p, %p, %d)", pool, addr, size);
+      VG_(get_and_pp_StackTrace) (tid, MEMPOOL_DEBUG_STACKTRACE_DEPTH);
+   }
+
    mp = VG_(HT_lookup)(MC_(mempool_list), (UWord)pool);
    if (mp == NULL) {
       MC_(record_illegal_mempool_error)(tid, pool);
       return;
    }
 
+   check_mempool_sane(mp);
    chunks = VG_(HT_to_array) ( mp->chunks, &n_shadows );
    if (n_shadows == 0) {
      tl_assert(chunks == NULL);
@@ -493,7 +628,7 @@ void MC_(mempool_trim)(Addr pool, Addr addr, SizeT size)
    tl_assert(chunks != NULL);
    for (i = 0; i < n_shadows; ++i) {
 
-      Addr lo, hi;
+      Addr lo, hi, min, max;
 
       mc = (MC_Chunk*) chunks[i];
 
@@ -518,6 +653,7 @@ void MC_(mempool_trim)(Addr pool, Addr addr, SizeT size)
          if (VG_(HT_remove)(mp->chunks, (UWord)mc->data) == NULL) {
             MC_(record_free_error)(tid, (Addr)mc->data);
             VG_(free)(chunks);
+            check_mempool_sane(mp);
             return;
          }
          die_and_free_mem ( tid, mc, mp->rzB );  
@@ -532,13 +668,38 @@ void MC_(mempool_trim)(Addr pool, Addr addr, SizeT size)
          if (VG_(HT_remove)(mp->chunks, (UWord)mc->data) == NULL) {
             MC_(record_free_error)(tid, (Addr)mc->data);
             VG_(free)(chunks);
+            check_mempool_sane(mp);
             return;
          }
 
-         lo = mc->data > addr ? mc->data : addr;
-         hi = mc->data + mc->size < addr + size ? mc->data + mc->size : addr + size;
+         if (mc->data < addr) {
+           min = mc->data;
+           lo = addr;
+         } else {
+           min = addr;
+           lo = mc->data;
+         }
 
+         if (mc->data + size > addr + size) {
+           max = mc->data + size;
+           hi = addr + size;
+         } else {
+           max = addr + size;
+           hi = mc->data + size;
+         }
+
+         tl_assert(min <= lo);
          tl_assert(lo < hi);
+         tl_assert(hi <= max);
+
+         if (min < lo && !EXTENT_CONTAINS(min)) {
+           MC_(make_mem_noaccess)( min, lo - min);
+         }
+
+         if (hi < max && !EXTENT_CONTAINS(max)) {
+           MC_(make_mem_noaccess)( hi, max - hi );
+         }
+
          mc->data = lo;
          mc->size = (UInt) (hi - lo);
          VG_(HT_add_node)( mp->chunks, mc );        
@@ -547,9 +708,77 @@ void MC_(mempool_trim)(Addr pool, Addr addr, SizeT size)
 #undef EXTENT_CONTAINS
       
    }
+   check_mempool_sane(mp);
    VG_(free)(chunks);
 }
 
+void MC_(move_mempool)(Addr poolA, Addr poolB)
+{
+   MC_Mempool* mp;
+
+   if (VG_(clo_verbosity) > 2) {
+      VG_(message)(Vg_UserMsg, "move_mempool(%p, %p)", poolA, poolB);
+      VG_(get_and_pp_StackTrace)
+         (VG_(get_running_tid)(), MEMPOOL_DEBUG_STACKTRACE_DEPTH);
+   }
+
+   mp = VG_(HT_remove) ( MC_(mempool_list), (UWord)poolA );
+
+   if (mp == NULL) {
+      ThreadId tid = VG_(get_running_tid)();
+      MC_(record_illegal_mempool_error) ( tid, poolA );
+      return;
+   }
+
+   mp->pool = poolB;
+   VG_(HT_add_node)( MC_(mempool_list), mp );
+}
+
+void MC_(mempool_change)(Addr pool, Addr addrA, Addr addrB, SizeT size)
+{
+   MC_Mempool*  mp;
+   MC_Chunk*    mc;
+   ThreadId     tid = VG_(get_running_tid)();
+
+   if (VG_(clo_verbosity) > 2) {
+      VG_(message)(Vg_UserMsg, "mempool_change(%p, %p, %p, %d)", 
+                   pool, addrA, addrB, size);
+      VG_(get_and_pp_StackTrace) (tid, MEMPOOL_DEBUG_STACKTRACE_DEPTH);
+   }
+
+   mp = VG_(HT_lookup)(MC_(mempool_list), (UWord)pool);
+   if (mp == NULL) {
+      MC_(record_illegal_mempool_error)(tid, pool);
+      return;
+   }
+
+   check_mempool_sane(mp);
+
+   mc = VG_(HT_remove)(mp->chunks, (UWord)addrA);
+   if (mc == NULL) {
+      MC_(record_free_error)(tid, (Addr)addrA);
+      return;
+   }
+
+   mc->data = addrB;
+   mc->size = size;
+   VG_(HT_add_node)( mp->chunks, mc );
+
+   check_mempool_sane(mp);
+}
+
+Bool MC_(mempool_exists)(Addr pool)
+{
+   MC_Mempool*  mp;
+
+   mp = VG_(HT_lookup)(MC_(mempool_list), (UWord)pool);
+   if (mp == NULL) {
+       return False;
+   }
+   return True;
+}
+
+
 /*------------------------------------------------------------*/
 /*--- Statistics printing                                  ---*/
 /*------------------------------------------------------------*/