]> sourceware.org Git - lvm2.git/commitdiff
[MM] Make valgrind aware of the pool allocators
authorJoe Thornber <thornber@redhat.com>
Mon, 9 Aug 2010 10:56:01 +0000 (10:56 +0000)
committerJoe Thornber <thornber@redhat.com>
Mon, 9 Aug 2010 10:56:01 +0000 (10:56 +0000)
./configure with --enable-valgrind-pool to enable this.

Makefile.in
configure.in
libdm/mm/dbg_malloc.c
libdm/mm/pool-fast.c
unit-tests/mm/Makefile.in [new file with mode: 0644]
unit-tests/mm/TESTS [new file with mode: 0644]
unit-tests/mm/check_results [new file with mode: 0755]
unit-tests/mm/pool_valgrind_t.c [new file with mode: 0644]

index a6b2b626b0afd0c3bc8422bff0999fe7700a8f79..5173d57302fad2183a5019ea79bac3db1c95acf0 100644 (file)
@@ -137,9 +137,11 @@ RUBY=ruby1.9 -Ireport-generators/lib -Ireport-generators/test
 
 .PHONEY: unit-test ruby-test test-programs
 
+# FIXME: put dependencies on libdm and liblvm
 test-programs:
        cd unit-tests/regex && $(MAKE)
        cd unit-tests/datastruct && $(MAKE)
+       cd unit-tests/mm && $(MAKE)
 
 unit-test: test-programs
        $(RUBY) report-generators/unit_test.rb $(shell find . -name TESTS)
index d3df6973fff78908abaee35c45417b97f1e6c40a..70c6fa1f47c0e4354bc3367840846a23cfe92801 100644 (file)
@@ -716,6 +716,19 @@ if test "$TESTING" = yes; then
    fi
 fi
 
+################################################################################
+dnl -- Enable valgrind awareness of memory pools
+AC_MSG_CHECKING(whether to enable valgrind awareness of pools)
+AC_ARG_ENABLE(valgrind_pool,
+              AC_HELP_STRING(--enable-valgrind-pool, [enable valgrind awareness of pools]),
+             VALGRIND_POOL=$enableval, VALGRIND_POOL=no)
+AC_MSG_RESULT($VALGRIND_POOL)
+
+if test "$VALGRIND_POOL" = yes; then
+    AC_CHECK_HEADERS([valgrind/memcheck.h], , [AC_MSG_ERROR(bailing out)])
+    AC_DEFINE([VALGRIND_POOL], 1, [Enable a valgrind aware build of pool])
+fi
+
 ################################################################################
 dnl -- Disable devmapper
 AC_MSG_CHECKING(whether to use device-mapper)
@@ -1343,6 +1356,7 @@ tools/Makefile
 udev/Makefile
 unit-tests/datastruct/Makefile
 unit-tests/regex/Makefile
+unit-tests/mm/Makefile
 ])
 AC_OUTPUT
 
index d86326db13eca7924c086035930e993afb855d62..cace811058b3d6daf529c1635364b700f8843c64 100644 (file)
@@ -193,6 +193,13 @@ int dm_dump_memory_debug(void)
                log_very_verbose("You have a memory leak:");
 
        for (mb = _head; mb; mb = mb->next) {
+#ifdef VALGRIND_POOL
+               /*
+                * We can't look at the memory in case it has had
+                * VALGRIND_MAKE_MEM_NOACCESS called on it.
+                */
+               str[0] = '\0';
+#else
                for (c = 0; c < sizeof(str) - 1; c++) {
                        if (c >= mb->length)
                                str[c] = ' ';
@@ -204,6 +211,7 @@ int dm_dump_memory_debug(void)
                                str[c] = ((char *)mb->magic)[c];
                }
                str[sizeof(str) - 1] = '\0';
+#endif
 
                LOG_MESG(_LOG_INFO, mb->file, mb->line, 0,
                         "block %d at %p, size %" PRIsize_t "\t [%s]",
index c8f3429eef5a92ecc3595951cebdf94cec2f9780..daad79aaa5b704bbfb29ae36f66474e00ed0b041 100644 (file)
@@ -1,6 +1,6 @@
 /*
- * Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.   
- * Copyright (C) 2004-2006 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
+ * Copyright (C) 2004-2010 Red Hat, Inc. All rights reserved.
  *
  * This file is part of the device-mapper userspace tools.
  *
  * Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
+#ifdef VALGRIND_POOL
+#include "valgrind/memcheck.h"
+#endif
+
 #include "dmlib.h"
 
 struct chunk {
@@ -31,6 +35,7 @@ struct dm_pool {
 
 void _align_chunk(struct chunk *c, unsigned alignment);
 struct chunk *_new_chunk(struct dm_pool *p, size_t s);
+static void _free_chunk(struct chunk *c);
 
 /* by default things come out aligned for doubles */
 #define DEFAULT_ALIGNMENT __alignof__ (double)
@@ -59,11 +64,11 @@ struct dm_pool *dm_pool_create(const char *name, size_t chunk_hint)
 void dm_pool_destroy(struct dm_pool *p)
 {
        struct chunk *c, *pr;
-       dm_free(p->spare_chunk);
+       _free_chunk(p->spare_chunk);
        c = p->chunk;
        while (c) {
                pr = c->prev;
-               dm_free(c);
+               _free_chunk(c);
                c = pr;
        }
 
@@ -100,6 +105,11 @@ void *dm_pool_alloc_aligned(struct dm_pool *p, size_t s, unsigned alignment)
 
        r = c->begin;
        c->begin += s;
+
+#ifdef VALGRIND_POOL
+       VALGRIND_MAKE_MEM_UNDEFINED(r, s);
+#endif
+
        return r;
 }
 
@@ -122,11 +132,20 @@ void dm_pool_free(struct dm_pool *p, void *ptr)
                if (((char *) c < (char *) ptr) &&
                    ((char *) c->end > (char *) ptr)) {
                        c->begin = ptr;
+#ifdef VALGRIND_POOL
+                       VALGRIND_MAKE_MEM_NOACCESS(c->begin, c->end - c->begin);
+#endif
                        break;
                }
 
                if (p->spare_chunk)
-                       dm_free(p->spare_chunk);
+                       _free_chunk(p->spare_chunk);
+
+               c->begin = (char *) (c + 1);
+#ifdef VALGRIND_POOL
+                VALGRIND_MAKE_MEM_NOACCESS(c->begin, c->end - c->begin);
+#endif
+
                p->spare_chunk = c;
                c = c->prev;
        }
@@ -183,10 +202,24 @@ int dm_pool_grow_object(struct dm_pool *p, const void *extra, size_t delta)
                        return 0;
 
                _align_chunk(p->chunk, p->object_alignment);
+
+#ifdef VALGRIND_POOL
+               VALGRIND_MAKE_MEM_UNDEFINED(p->chunk->begin, p->object_len);
+#endif
+
                memcpy(p->chunk->begin, c->begin, p->object_len);
+
+#ifdef VALGRIND_POOL
+               VALGRIND_MAKE_MEM_NOACCESS(c->begin, p->object_len);
+#endif
+
                c = p->chunk;
        }
 
+#ifdef VALGRIND_POOL
+       VALGRIND_MAKE_MEM_UNDEFINED(p->chunk->begin + p->object_len, delta);
+#endif
+
        memcpy(c->begin + p->object_len, extra, delta);
        p->object_len += delta;
        return 1;
@@ -218,7 +251,7 @@ struct chunk *_new_chunk(struct dm_pool *p, size_t s)
        struct chunk *c;
 
        if (p->spare_chunk &&
-           ((p->spare_chunk->end - (char *) p->spare_chunk) >= s)) {
+           ((p->spare_chunk->end - p->spare_chunk->begin) >= s)) {
                /* reuse old chunk */
                c = p->spare_chunk;
                p->spare_chunk = 0;
@@ -229,12 +262,26 @@ struct chunk *_new_chunk(struct dm_pool *p, size_t s)
                        return NULL;
                }
 
+               c->begin = (char *) (c + 1);
                c->end = (char *) c + s;
+
+#ifdef VALGRIND_POOL
+               VALGRIND_MAKE_MEM_NOACCESS(c->begin, c->end - c->begin);
+#endif
        }
 
        c->prev = p->chunk;
-       c->begin = (char *) (c + 1);
        p->chunk = c;
-
        return c;
 }
+
+static void _free_chunk(struct chunk *c)
+{
+       if (c) {
+#ifdef VALGRIND_POOL
+               VALGRIND_MAKE_MEM_UNDEFINED(c, c->end - (char *) c);
+#endif
+
+               dm_free(c);
+       }
+}
diff --git a/unit-tests/mm/Makefile.in b/unit-tests/mm/Makefile.in
new file mode 100644 (file)
index 0000000..1b3499e
--- /dev/null
@@ -0,0 +1,31 @@
+#
+# Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
+# Copyright (C) 2004 Red Hat, Inc. All rights reserved.
+#
+# This file is part of LVM2.
+#
+# This copyrighted material is made available to anyone wishing to use,
+# modify, copy, or redistribute it subject to the terms and conditions
+# of the GNU General Public License v.2.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+srcdir = @srcdir@
+top_srcdir = @top_srcdir@
+top_builddir = @top_builddir@
+VPATH = @srcdir@
+
+SOURCES=\
+       pool_valgrind_t.c
+
+TARGETS=\
+       pool_valgrind_t
+
+include $(top_builddir)/make.tmpl
+DM_LIBS = -ldevmapper $(LIBS)
+
+pool_valgrind_t: pool_valgrind_t.o
+       $(CC) $(CFLAGS) -o $@ pool_valgrind_t.o $(LDFLAGS) $(DM_LIBS)
+
diff --git a/unit-tests/mm/TESTS b/unit-tests/mm/TESTS
new file mode 100644 (file)
index 0000000..3bf3154
--- /dev/null
@@ -0,0 +1 @@
+valgrind pool awareness:valgrind ./pool_valgrind_t 2>&1 | ./check_results
diff --git a/unit-tests/mm/check_results b/unit-tests/mm/check_results
new file mode 100755 (executable)
index 0000000..a7b0975
--- /dev/null
@@ -0,0 +1,31 @@
+#!/usr/bin/env ruby1.9
+
+require 'pp'
+
+patterns = [
+            /Invalid read of size 1/,
+            /Invalid write of size 1/,
+            /Invalid read of size 1/,
+            /still reachable: [0-9,]+ bytes in 3 blocks/
+           ]
+
+lines = STDIN.readlines
+pp lines
+
+result = catch(:done) do
+  patterns.each do |pat|
+    loop do
+      throw(:done, false) if lines.size == 0
+
+      line = lines.shift
+      if line =~ pat
+        STDERR.puts "matched #{pat}"
+        break;
+      end
+    end
+  end
+
+  throw(:done, true)
+end
+
+exit(result ? 0 : 1)
diff --git a/unit-tests/mm/pool_valgrind_t.c b/unit-tests/mm/pool_valgrind_t.c
new file mode 100644 (file)
index 0000000..b430a9c
--- /dev/null
@@ -0,0 +1,183 @@
+#include "libdevmapper.h"
+
+#include <assert.h>
+
+/*
+ * Checks that valgrind is picking up unallocated pool memory as
+ * uninitialised, even if the chunk has been recycled.
+ *
+ *     $ valgrind --track-origins=yes ./pool_valgrind_t
+ *
+ *     ==7023== Memcheck, a memory error detector
+ *     ==7023== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
+ *     ==7023== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for copyright info
+ *     ==7023== Command: ./pool_valgrind_t
+ *     ==7023==
+ *     first branch worked (as expected)
+ *     ==7023== Conditional jump or move depends on uninitialised value(s)
+ *     ==7023==    at 0x4009AC: main (in /home/ejt/work/lvm2/unit-tests/mm/pool_valgrind_t)
+ *     ==7023==  Uninitialised value was created by a client request
+ *     ==7023==    at 0x4E40CB8: dm_pool_free (in /home/ejt/work/lvm2/libdm/ioctl/libdevmapper.so.1.02)
+ *     ==7023==    by 0x4009A8: main (in /home/ejt/work/lvm2/unit-tests/mm/pool_valgrind_t)
+ *     ==7023==
+ *     second branch worked (valgrind should have flagged this as an error)
+ *     ==7023==
+ *     ==7023== HEAP SUMMARY:
+ *     ==7023==     in use at exit: 0 bytes in 0 blocks
+ *     ==7023==   total heap usage: 2 allocs, 2 frees, 2,104 bytes allocated
+ *     ==7023==
+ *     ==7023== All heap blocks were freed -- no leaks are possible
+ *     ==7023==
+ *     ==7023== For counts of detected and suppressed errors, rerun with: -v
+ *     ==7023== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 4 from 4)
+ */
+
+#define COUNT 10
+
+static void check_free()
+{
+        int i;
+        char *blocks[COUNT];
+        struct dm_pool *p = dm_pool_create("blah", 1024);
+
+        for (i = 0; i < COUNT; i++)
+                blocks[i] = dm_pool_alloc(p, 37);
+
+        /* check we can access the last block */
+        blocks[COUNT - 1][0] = 'E';
+        if (blocks[COUNT - 1][0] == 'E')
+                printf("first branch worked (as expected)\n");
+
+        dm_pool_free(p, blocks[5]);
+
+        if (blocks[COUNT - 1][0] == 'E')
+                printf("second branch worked (valgrind should have flagged this as an error)\n");
+
+        dm_pool_destroy(p);
+}
+
+/* Checks that freed chunks are marked NOACCESS */
+static void check_free2()
+{
+       struct dm_pool *p = dm_pool_create("", 900); /* 900 will get
+                                                     * rounded up to 1024,
+                                                     * 1024 would have got
+                                                     * rounded up to
+                                                     * 2048 */
+       char *data1, *data2;
+
+       assert(p);
+       data1 = dm_pool_alloc(p, 123);
+       assert(data1);
+
+       data1 = dm_pool_alloc(p, 1024);
+       assert(data1);
+
+       data2 = dm_pool_alloc(p, 123);
+       assert(data2);
+
+       data2[0] = 'A';         /* should work fine */
+
+       dm_pool_free(p, data1);
+
+       /*
+        * so now the first chunk is active, the second chunk has become
+        * the free one.
+        */
+       data2[0] = 'B';         /* should prompt an invalid write error */
+
+       dm_pool_destroy(p);
+}
+
+static void check_alignment()
+{
+       /*
+        * Pool always tries to allocate blocks with particular alignment.
+        * So there are potentially small gaps between allocations.  This
+        * test checks that valgrind is spotting illegal accesses to these
+        * gaps.
+        */
+
+       int i, sum;
+       struct dm_pool *p = dm_pool_create("blah", 1024);
+       char *data1, *data2;
+       char buffer[16];
+
+
+       data1 = dm_pool_alloc_aligned(p, 1, 4);
+       assert(data1);
+       data2 = dm_pool_alloc_aligned(p, 1, 4);
+       assert(data1);
+
+       snprintf(buffer, sizeof(buffer), "%c", *(data1 + 1)); /* invalid read size 1 */
+       dm_pool_destroy(p);
+}
+
+/*
+ * Looking at the code I'm not sure allocations that are near the chunk
+ * size are working.  So this test is trying to exhibit a specific problem.
+ */
+static void check_allocation_near_chunk_size()
+{
+       int i;
+       char *data;
+       struct dm_pool *p = dm_pool_create("", 900);
+
+       /*
+        * allocate a lot and then free everything so we know there
+        * is a spare chunk.
+        */
+       for (i = 0; i < 1000; i++) {
+               data = dm_pool_alloc(p, 37);
+               memset(data, 0, 37);
+               assert(data);
+       }
+
+       dm_pool_empty(p);
+
+       /* now we allocate something close to the chunk size ... */
+       data = dm_pool_alloc(p, 1020);
+       assert(data);
+       memset(data, 0, 1020);
+
+       dm_pool_destroy(p);
+}
+
+/* FIXME: test the dbg_malloc at exit (this test should be in dbg_malloc) */
+static void check_leak_detection()
+{
+       int i;
+       struct dm_pool *p = dm_pool_create("", 1024);
+
+       for (i = 0; i < 10; i++)
+               dm_pool_alloc(p, (i + 1) * 37);
+}
+
+/* we shouldn't get any errors from this one */
+static void check_object_growth()
+{
+       int i;
+       struct dm_pool *p = dm_pool_create("", 32);
+       char data[100];
+       void *obj;
+
+       memset(data, 0, sizeof(data));
+
+       dm_pool_begin_object(p, 43);
+       for (i = 1; i < 100; i++)
+               dm_pool_grow_object(p, data, i);
+       obj = dm_pool_end_object(p);
+
+       dm_pool_destroy(p);
+}
+
+int main(int argc, char **argv)
+{
+       check_free();
+       check_free2();
+       check_alignment();
+       check_allocation_near_chunk_size();
+       check_leak_detection();
+       check_object_growth();
+        return 0;
+}
This page took 0.047758 seconds and 5 git commands to generate.