[Valgrind-users] Mempool annotation for glib/gslice

[David W. <dw...@in...>]

The glib library has a memory pool / slab allocator of its own. Mostly
when running glib programs under Valgrind, we bypass it completely by
setting GSLICE=always_malloc.

However, that's not optimal -- partly because it papers over the cases
where a program allocates using g_slice and then invalidly frees with
the normal (g_)free. So I wanted to add proper annotations to make it
work nicely with Valgrind. My attempt is below. The file being patched
is http://git.gnome.org/browse/glib/tree/glib/gslice.c


However, it doesn't quite work as I want -- the 'superblocks' used by
the slab allocator are allocated with memalign, so they are tracked by
valgrind already. And thus although I seem to mostly get the right
warnings at the right times, the allocation stack trace is the wrong
one. For example, this test case:

	foo = g_slice_alloc(20);
	foo[20] = 0;
	g_slice_free1(20, foo);
	foo = g_slice_alloc(20);
	g_free(foo);

...gives this output:

==18242== Invalid write of size 1
==18242==    at 0x4005CD: main (slice.c:9)
==18242==  Address 0x4f38014 is 20 bytes inside a block of size 496 alloc'd
==18242==    at 0x4A04360: memalign (vg_replace_malloc.c:532)
==18242==    by 0x4A043B9: posix_memalign (vg_replace_malloc.c:660)
==18242==    by 0x4C6831B: slab_allocator_alloc_chunk (gslice.c:1174)
==18242==    by 0x4C69A74: g_slice_alloc (gslice.c:682)
==18242==    by 0x4005B5: main (slice.c:7)
==18242== 
==18242== Invalid free() / delete / delete[]
==18242==    at 0x4A04D72: free (vg_replace_malloc.c:325)
==18242==    by 0x4005FF: main (slice.c:12)
==18242==  Address 0x4f38020 is 32 bytes inside a block of size 496 alloc'd
==18242==    at 0x4A04360: memalign (vg_replace_malloc.c:532)
==18242==    by 0x4A043B9: posix_memalign (vg_replace_malloc.c:660)
==18242==    by 0x4C6831B: slab_allocator_alloc_chunk (gslice.c:1174)
==18242==    by 0x4C69A74: g_slice_alloc (gslice.c:682)
==18242==    by 0x4005B5: main (slice.c:7)

How can I make Valgrind *forget* that the slab allocator's superblock
was originally allocated with memalign?

diff --git a/glib/gslice.c b/glib/gslice.c
index 05de6b3..19bdddc 100644
--- a/glib/gslice.c
+++ b/glib/gslice.c
@@ -52,6 +52,7 @@
 #include "gthread.h"
 #include "gthreadprivate.h"
 #include "glib_trace.h"
+#include "memcheck.h"
 
 /* the GSlice allocator is split up into 4 layers, roughly modelled after the slab
  * allocator and magazine extensions as outlined in:
@@ -304,6 +305,8 @@ g_slice_init_nomessage (void)
   mem_assert (sys_page_size == 0);
   mem_assert (MIN_MAGAZINE_SIZE >= 4);
 
+  VALGRIND_CREATE_MEMPOOL(allocator, 0, 0);
+
 #ifdef G_OS_WIN32
   {
     SYSTEM_INFO system_info;
@@ -831,11 +834,15 @@ g_slice_alloc (gsize mem_size)
             thread_memory_magazine1_reload (tmem, ix);
         }
       mem = thread_memory_magazine1_alloc (tmem, ix);
+      VALGRIND_MAKE_MEM_NOACCESS(mem, chunk_size);
+      VALGRIND_MEMPOOL_ALLOC(allocator, mem, mem_size);
     }
   else if (acat == 2)           /* allocate through slab allocator */
     {
       g_mutex_lock (allocator->slab_mutex);
       mem = slab_allocator_alloc_chunk (chunk_size);
+      VALGRIND_MAKE_MEM_NOACCESS(mem, chunk_size);
+      VALGRIND_MEMPOOL_ALLOC(allocator, mem, mem_size);
       g_mutex_unlock (allocator->slab_mutex);
     }
   else                          /* delegate to system malloc */
@@ -890,6 +897,8 @@ g_slice_free1 (gsize    mem_size,
         }
       if (G_UNLIKELY (g_mem_gc_friendly))
         memset (mem_block, 0, chunk_size);
+      VALGRIND_MEMPOOL_FREE(allocator, mem_block);
+      VALGRIND_MAKE_MEM_UNDEFINED(mem_block, sizeof(ChunkLink));
       thread_memory_magazine2_free (tmem, ix, mem_block);
     }
   else if (acat == 2)                   /* allocate through slab allocator */
@@ -897,6 +906,8 @@ g_slice_free1 (gsize    mem_size,
       if (G_UNLIKELY (g_mem_gc_friendly))
         memset (mem_block, 0, chunk_size);
       g_mutex_lock (allocator->slab_mutex);
+      VALGRIND_MEMPOOL_FREE(allocator, mem_block);
+      VALGRIND_MAKE_MEM_UNDEFINED(mem_block, sizeof(ChunkLink));
       slab_allocator_free_chunk (chunk_size, mem_block);
       g_mutex_unlock (allocator->slab_mutex);
     }
@@ -951,6 +962,8 @@ g_slice_free_chain_with_offset (gsize    mem_size,
             }
           if (G_UNLIKELY (g_mem_gc_friendly))
             memset (current, 0, chunk_size);
+          VALGRIND_MEMPOOL_FREE(allocator, current);
+          VALGRIND_MAKE_MEM_UNDEFINED(current, sizeof(ChunkLink));
           thread_memory_magazine2_free (tmem, ix, current);
         }
     }
@@ -966,6 +979,8 @@ g_slice_free_chain_with_offset (gsize    mem_size,
             abort();
           if (G_UNLIKELY (g_mem_gc_friendly))
             memset (current, 0, chunk_size);
+          VALGRIND_MEMPOOL_FREE(allocator, current);
+          VALGRIND_MAKE_MEM_UNDEFINED(current, sizeof(ChunkLink));
           slab_allocator_free_chunk (chunk_size, current);
         }
       g_mutex_unlock (allocator->slab_mutex);


-- 
dwmw2