From: Nicholas Nethercote Date: Tue, 20 May 2003 16:38:24 +0000 (+0000) Subject: Added VALGRIND_MALLOCLIKE_BLOCK and VALGRIND_FREELIKE_BLOCK which allow you to X-Git-Tag: svn/VALGRIND_2_0_0~141 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=40571c6f66d092a61d0d7ee5f46541b083b67bf9;p=thirdparty%2Fvalgrind.git Added VALGRIND_MALLOCLIKE_BLOCK and VALGRIND_FREELIKE_BLOCK which allow you to use a custom-allocator and detect almost as many errors as you could detect if you used malloc/new/new[]. (eg. leaks detected, free errors, free mismatch, etc). Had to fiddle with mac_malloc_wrappers.c a bit to factor out the appropriate code to be called from the client request handling code. Also had to add a new element `MAC_AllocCustom' to the MAC_AllocKind type. Also added a little documentation, and a regression test. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@1643 --- diff --git a/coregrind/docs/coregrind_core.html b/coregrind/docs/coregrind_core.html index 2ce6162c8c..edf4d49a96 100644 --- a/coregrind/docs/coregrind_core.html +++ b/coregrind/docs/coregrind_core.html @@ -882,6 +882,18 @@ A brief description of the available macros: were leaked, dubious, reachable and suppressed. Again, useful for test harness code.

+

  • VALGRIND_MALLOCLIKE_BLOCK: If your program manages its own + memory instead of using the standard + malloc()/new/new[], Memcheck will + not detect nearly as many errors, and the error messages won't be as + informative. To improve this situation, use this macro just after your + custom allocator allocates some new memory. See the comments in + memcheck/memcheck.h for information on how to use it. +

    +

  • VALGRIND_FREELIKE_BLOCK: This should be used in conjunction + with VALGRIND_MALLOCLIKE_BLOCK. Again, see + memcheck/memcheck.h for information on how to use it. +

    diff --git a/memcheck/mac_malloc_wrappers.c b/memcheck/mac_malloc_wrappers.c index 5d22cceecd..239f2c10f3 100644 --- a/memcheck/mac_malloc_wrappers.c +++ b/memcheck/mac_malloc_wrappers.c @@ -115,13 +115,9 @@ MAC_Chunk* MAC_(first_matching_freed_MAC_Chunk) ( Bool (*p)(MAC_Chunk*) ) return NULL; } -/* Allocate a user-chunk of size bytes. Also allocate its shadow - block, make the shadow block point at the user block. Put the - shadow chunk on the appropriate list, and set all memory - protections correctly. */ - -static void add_MAC_Chunk ( ThreadState* tst, - Addr p, UInt size, MAC_AllocKind kind ) +/* Allocate its shadow chunk, put it on the appropriate list. */ +static +void add_MAC_Chunk ( ThreadState* tst, Addr p, UInt size, MAC_AllocKind kind ) { MAC_Chunk* mc; @@ -145,28 +141,22 @@ void (*MAC_(die_mem_heap)) ( Addr a, UInt len ); void (*MAC_(copy_mem_heap))( Addr from, Addr to, UInt len ); /* Allocate memory and note change in memory available */ -static __inline__ -void* alloc_and_new_mem ( ThreadState* tst, UInt size, UInt alignment, - Bool is_zeroed, MAC_AllocKind kind ) +__inline__ +void MAC_(new_block) ( ThreadState* tst, Addr p, UInt size, + UInt rzB, Bool is_zeroed, MAC_AllocKind kind ) { - Addr p; - VGP_PUSHCC(VgpCliMalloc); cmalloc_n_mallocs ++; cmalloc_bs_mallocd += size; - p = (Addr)VG_(cli_malloc)(alignment, size); + add_MAC_Chunk( tst, p, size, kind ); - add_MAC_Chunk ( tst, p, size, kind ); - - MAC_(ban_mem_heap)( p-VG_(vg_malloc_redzone_szB), - VG_(vg_malloc_redzone_szB) ); + MAC_(ban_mem_heap)( p-rzB, rzB ); MAC_(new_mem_heap)( p, size, is_zeroed ); - MAC_(ban_mem_heap)( p+size, VG_(vg_malloc_redzone_szB) ); + MAC_(ban_mem_heap)( p+size, rzB ); VGP_POPCC(VgpCliMalloc); - return (void*)p; } void* SK_(malloc) ( ThreadState* tst, Int n ) @@ -175,8 +165,10 @@ void* SK_(malloc) ( ThreadState* tst, Int n ) VG_(message)(Vg_UserMsg, "Warning: silly arg (%d) to malloc()", n ); return NULL; } else { - return alloc_and_new_mem ( tst, n, VG_(clo_alignment), - /*is_zeroed*/False, MAC_AllocMalloc ); + Addr p = (Addr)VG_(cli_malloc)( VG_(clo_alignment), n ); + MAC_(new_block) ( tst, p, n, VG_(vg_malloc_redzone_szB), + /*is_zeroed*/False, MAC_AllocMalloc ); + return (void*)p; } } @@ -186,8 +178,10 @@ void* SK_(__builtin_new) ( ThreadState* tst, Int n ) VG_(message)(Vg_UserMsg, "Warning: silly arg (%d) to __builtin_new()", n); return NULL; } else { - return alloc_and_new_mem ( tst, n, VG_(clo_alignment), - /*is_zeroed*/False, MAC_AllocNew ); + Addr p = (Addr)VG_(cli_malloc)( VG_(clo_alignment), n ); + MAC_(new_block) ( tst, p, n, VG_(vg_malloc_redzone_szB), + /*is_zeroed*/False, MAC_AllocNew ); + return (void*)p; } } @@ -198,8 +192,10 @@ void* SK_(__builtin_vec_new) ( ThreadState* tst, Int n ) "Warning: silly arg (%d) to __builtin_vec_new()", n ); return NULL; } else { - return alloc_and_new_mem ( tst, n, VG_(clo_alignment), - /*is_zeroed*/False, MAC_AllocNewVec ); + Addr p = (Addr)VG_(cli_malloc)( VG_(clo_alignment), n ); + MAC_(new_block) ( tst, p, n, VG_(vg_malloc_redzone_szB), + /*is_zeroed*/False, MAC_AllocNewVec ); + return (void*)p; } } @@ -209,41 +205,42 @@ void* SK_(memalign) ( ThreadState* tst, Int align, Int n ) VG_(message)(Vg_UserMsg, "Warning: silly arg (%d) to memalign()", n); return NULL; } else { - return alloc_and_new_mem ( tst, n, align, /*is_zeroed*/False, - MAC_AllocMalloc ); + Addr p = (Addr)VG_(cli_malloc)( align, n ); + MAC_(new_block) ( tst, p, n, VG_(vg_malloc_redzone_szB), + /*is_zeroed*/False, MAC_AllocMalloc ); + return (void*)p; } } void* SK_(calloc) ( ThreadState* tst, Int nmemb, Int size1 ) { - void* p; - Int size, i; + Int n, i; - size = nmemb * size1; + n = nmemb * size1; if (nmemb < 0 || size1 < 0) { VG_(message)(Vg_UserMsg, "Warning: silly args (%d,%d) to calloc()", nmemb, size1 ); return NULL; } else { - p = alloc_and_new_mem ( tst, size, VG_(clo_alignment), - /*is_zeroed*/True, MAC_AllocMalloc ); - for (i = 0; i < size; i++) + Addr p = (Addr)VG_(cli_malloc)( VG_(clo_alignment), n ); + MAC_(new_block) ( tst, p, n, VG_(vg_malloc_redzone_szB), + /*is_zeroed*/True, MAC_AllocMalloc ); + for (i = 0; i < n; i++) ((UChar*)p)[i] = 0; - return p; + return (void*)p; } } static void die_and_free_mem ( ThreadState* tst, MAC_Chunk* mc, - MAC_Chunk** prev_chunks_next_ptr ) + MAC_Chunk** prev_chunks_next_ptr, UInt rzB ) { /* Note: ban redzones again -- just in case user de-banned them with a client request... */ - MAC_(ban_mem_heap)( mc->data-VG_(vg_malloc_redzone_szB), - VG_(vg_malloc_redzone_szB) ); + MAC_(ban_mem_heap)( mc->data-rzB, rzB ); MAC_(die_mem_heap)( mc->data, mc->size ); - MAC_(ban_mem_heap)( mc->data+mc->size, VG_(vg_malloc_redzone_szB) ); + MAC_(ban_mem_heap)( mc->data+mc->size, rzB ); /* Remove mc from the malloclist using prev_chunks_next_ptr to avoid repeating the hash table lookup. Can't remove until at least @@ -254,13 +251,17 @@ void die_and_free_mem ( ThreadState* tst, MAC_Chunk* mc, /* Record where freed */ mc->where = VG_(get_ExeContext) ( tst ); - /* Put it out of harm's way for a while. */ - add_to_freed_queue ( mc ); + /* Put it out of harm's way for a while, if not from a client request */ + if (MAC_AllocCustom != mc->allockind) + add_to_freed_queue ( mc ); + else + VG_(free) ( mc ); } -static __inline__ -void handle_free ( ThreadState* tst, void* p, MAC_AllocKind kind ) +__inline__ +void MAC_(handle_free) ( ThreadState* tst, Addr p, UInt rzB, + MAC_AllocKind kind ) { MAC_Chunk* mc; MAC_Chunk** prev_chunks_next_ptr; @@ -271,35 +272,34 @@ void handle_free ( ThreadState* tst, void* p, MAC_AllocKind kind ) mc = (MAC_Chunk*)VG_(HT_get_node) ( MAC_(malloc_list), (UInt)p, (VgHashNode***)&prev_chunks_next_ptr ); - if (mc == NULL) { - MAC_(record_free_error) ( tst, (Addr)p ); + MAC_(record_free_error) ( tst, p ); VGP_POPCC(VgpCliMalloc); return; } /* check if its a matching free() / delete / delete [] */ if (kind != mc->allockind) { - MAC_(record_freemismatch_error) ( tst, (Addr)p ); + MAC_(record_freemismatch_error) ( tst, p ); } - die_and_free_mem ( tst, mc, prev_chunks_next_ptr ); + die_and_free_mem ( tst, mc, prev_chunks_next_ptr, rzB ); VGP_POPCC(VgpCliMalloc); } void SK_(free) ( ThreadState* tst, void* p ) { - handle_free(tst, p, MAC_AllocMalloc); + MAC_(handle_free)(tst, (Addr)p, VG_(vg_malloc_redzone_szB), MAC_AllocMalloc); } void SK_(__builtin_delete) ( ThreadState* tst, void* p ) { - handle_free(tst, p, MAC_AllocNew); + MAC_(handle_free)(tst, (Addr)p, VG_(vg_malloc_redzone_szB), MAC_AllocNew); } void SK_(__builtin_vec_delete) ( ThreadState* tst, void* p ) { - handle_free(tst, p, MAC_AllocNewVec); + MAC_(handle_free)(tst, (Addr)p, VG_(vg_malloc_redzone_szB), MAC_AllocNewVec); } void* SK_(realloc) ( ThreadState* tst, void* p, Int new_size ) @@ -370,7 +370,8 @@ void* SK_(realloc) ( ThreadState* tst, void* p, Int new_size ) ((UChar*)p_new)[i] = ((UChar*)p)[i]; /* Free old memory */ - die_and_free_mem ( tst, mc, prev_chunks_next_ptr ); + die_and_free_mem ( tst, mc, prev_chunks_next_ptr, + VG_(vg_malloc_redzone_szB) ); /* this has to be after die_and_free_mem, otherwise the former succeeds in shorting out the new block, not the diff --git a/memcheck/mac_needs.c b/memcheck/mac_needs.c index 874a6da792..b48765c56f 100644 --- a/memcheck/mac_needs.c +++ b/memcheck/mac_needs.c @@ -277,7 +277,7 @@ void MAC_(pp_shared_SkinError) ( Error* err ) switch (VG_(get_error_kind)(err)) { case FreeErr: - VG_(message)(Vg_UserMsg,"Invalid free() / delete / delete[]"); + VG_(message)(Vg_UserMsg, "Invalid free() / delete / delete[]"); /* fall through */ case FreeMismatchErr: if (VG_(get_error_kind)(err) == FreeMismatchErr) @@ -773,17 +773,34 @@ void MAC_(common_fini)(void (*leak_check)(void)) Bool MAC_(handle_common_client_requests)(ThreadState* tst, UInt* arg, UInt* ret ) { - UInt** argp = (UInt**)arg; + UInt* argv = (UInt*)arg; switch (arg[0]) { - case VG_USERREQ__COUNT_LEAKS: /* count leaked bytes */ + case VG_USERREQ__COUNT_LEAKS: { /* count leaked bytes */ + UInt** argp = (UInt**)arg; *argp[1] = MAC_(total_bytes_leaked); *argp[2] = MAC_(total_bytes_dubious); *argp[3] = MAC_(total_bytes_reachable); *argp[4] = MAC_(total_bytes_suppressed); *ret = 0; return True; + } + case VG_USERREQ__MALLOCLIKE_BLOCK: { + Addr p = (Addr)argv[1]; + UInt sizeB = argv[2]; + UInt rzB = argv[3]; + Bool is_zeroed = (Bool)argv[4]; + + MAC_(new_block) ( tst, p, sizeB, rzB, is_zeroed, MAC_AllocCustom ); + return True; + } + case VG_USERREQ__FREELIKE_BLOCK: { + Addr p = (Addr)argv[1]; + UInt rzB = argv[2]; + MAC_(handle_free) ( tst, p, rzB, MAC_AllocCustom ); + return True; + } default: return False; } diff --git a/memcheck/mac_shared.h b/memcheck/mac_shared.h index 302d9ffe69..ff1cea3f44 100644 --- a/memcheck/mac_shared.h +++ b/memcheck/mac_shared.h @@ -128,7 +128,8 @@ typedef enum { MAC_AllocMalloc = 0, MAC_AllocNew = 1, - MAC_AllocNewVec = 2 + MAC_AllocNewVec = 2, + MAC_AllocCustom = 3 } MAC_AllocKind; @@ -285,6 +286,12 @@ extern void MAC_(clear_MAC_Error) ( MAC_Error* err_extra ); extern Bool MAC_(shared_recognised_suppression) ( Char* name, Supp* su ); +extern void MAC_(new_block) ( ThreadState* tst, Addr p, UInt size, + UInt rzB, Bool is_zeroed, + MAC_AllocKind kind ); +extern void MAC_(handle_free) ( ThreadState* tst, Addr p, UInt rzB, + MAC_AllocKind kind ); + extern void MAC_(record_address_error) ( Addr a, Int size, Bool isWrite ); extern void MAC_(record_core_mem_error) ( ThreadState* tst, Bool isWrite, Char* s ); diff --git a/memcheck/memcheck.h b/memcheck/memcheck.h index f598beb1e5..8a38cce0c2 100644 --- a/memcheck/memcheck.h +++ b/memcheck/memcheck.h @@ -80,7 +80,9 @@ typedef VG_USERREQ__CHECK_WRITABLE, VG_USERREQ__CHECK_READABLE, VG_USERREQ__DO_LEAK_CHECK, - VG_USERREQ__COUNT_LEAKS + VG_USERREQ__COUNT_LEAKS, + VG_USERREQ__MALLOCLIKE_BLOCK, + VG_USERREQ__FREELIKE_BLOCK, } Vg_MemCheckClientRequest; @@ -170,6 +172,44 @@ typedef (volatile unsigned char *)&(__lvalue), \ (unsigned int)(sizeof (__lvalue))) +/* Mark a block of memory as having been allocated by a malloc()-like + function. `addr' is the start of the usable block (ie. after any + redzone) `rzB' is redzone size if the allocator can apply redzones; + use '0' if not. Adding redzones makes it more likely Valgrind will spot + block overruns. `is_zeroed' indicates if the memory is zeroed, as it is + for calloc(). Put it immediately after the point where a block is + allocated. + + If you're allocating memory via superblocks, and then handing out small + chunks of each superblock, if you don't have redzones on your small + blocks, it's worth marking the superblock with VALGRIND_MAKE_NOACCESS + when it's created, so that block overruns are detected. But if you can + put redzones on, it's probably better to not do this, so that messages + for small overruns are described in terms of the small block rather than + the superblock (but if you have a big overrun that skips over a redzone, + you could miss an error this way). See memcheck/tests/custom_alloc.c + for an example. + + Nb: block must be freed via a free()-like function specified + with VALGRIND_FREELIKE_BLOCK or mismatch errors will occur. */ +#define VALGRIND_MALLOCLIKE_BLOCK(addr, sizeB, rzB, is_zeroed) \ + {unsigned int _qzz_res; \ + VALGRIND_MAGIC_SEQUENCE(_qzz_res, 0, \ + VG_USERREQ__MALLOCLIKE_BLOCK, \ + addr, sizeB, rzB, is_zeroed); \ + } + +/* Mark a block of memory as having been freed by a free()-like function. + `rzB' is redzone size; it must match that given to + VALGRIND_MALLOCLIKE_BLOCK. Memory not freed will be detected by the leak + checker. Put it immediately after the point where the block is freed. */ +#define VALGRIND_FREELIKE_BLOCK(addr, rzB) \ + {unsigned int _qzz_res; \ + VALGRIND_MAGIC_SEQUENCE(_qzz_res, 0, \ + VG_USERREQ__FREELIKE_BLOCK, \ + addr, rzB, 0, 0); \ + } + /* Do a memory leak check mid-execution. */ #define VALGRIND_DO_LEAK_CHECK \ {unsigned int _qzz_res; \ diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am index dfd9f8af42..1704e040c7 100644 --- a/memcheck/tests/Makefile.am +++ b/memcheck/tests/Makefile.am @@ -24,6 +24,7 @@ EXTRA_DIST = \ buflen_check.stderr.exp buflen_check.vgtest \ clientperm.stderr.exp \ clientperm.stdout.exp clientperm.vgtest \ + custom_alloc.stderr.exp custom_alloc.vgtest \ doublefree.stderr.exp doublefree.vgtest \ error_counts.stderr.exp error_counts.stdout.exp error_counts.vgtest \ errs1.stderr.exp errs1.vgtest \ @@ -63,6 +64,7 @@ EXTRA_DIST = \ check_PROGRAMS = \ badaddrvalue badfree badjump badloop buflen_check clientperm \ + custom_alloc \ doublefree error_counts errs1 exitprog fprw fwrite inits inline \ malloc1 malloc2 malloc3 manuel1 manuel2 manuel3 \ memalign_test memcmptest mmaptest nanoleak null_socket \ @@ -82,6 +84,7 @@ badjump_SOURCES = badjump.c badloop_SOURCES = badloop.c buflen_check_SOURCES = buflen_check.c clientperm_SOURCES = clientperm.c +custom_alloc_SOURCES = custom_alloc.c doublefree_SOURCES = doublefree.c error_counts_SOURCES = error_counts.c errs1_SOURCES = errs1.c diff --git a/memcheck/tests/custom_alloc.c b/memcheck/tests/custom_alloc.c new file mode 100644 index 0000000000..0a6ec8b4ea --- /dev/null +++ b/memcheck/tests/custom_alloc.c @@ -0,0 +1,96 @@ +#include +#include +#include +#include + +#include "../memcheck.h" + +#define SUPERBLOCK_SIZE 100000 + +//------------------------------------------------------------------------- +// Allocator +//------------------------------------------------------------------------- + +void* get_superblock(void) +{ + void* p = mmap( 0, SUPERBLOCK_SIZE, PROT_READ|PROT_WRITE|PROT_EXEC, + MAP_PRIVATE|MAP_ANONYMOUS, -1, 0 ); + + assert(p != ((void*)(-1))); + + // Mark it no access; although it's addressible we don't want the + // program to be using it unless its handed out by custom_alloc() + + // with redzones, better not to have it + VALGRIND_MAKE_NOACCESS(p, SUPERBLOCK_SIZE); + + return p; +} + +// has a redzone +static void* custom_alloc(int size) +{ +#define RZ 8 + static void* hp = 0; // current heap pointer + static void* hp_lim = 0; // maximum usable byte in current block + int size2 = size + RZ*2; + void* p; + + if (hp + size2 > hp_lim) { + hp = get_superblock(); + hp_lim = hp + SUPERBLOCK_SIZE - 1; + } + + p = hp + RZ; + hp += size2; + + VALGRIND_MALLOCLIKE_BLOCK( p, size, RZ, /*is_zeroed*/1 ); + return (void*)p; +} + +static void custom_free(void* p) +{ + // don't actually free any memory... but mark it as freed + VALGRIND_FREELIKE_BLOCK( p, RZ ); +} +#undef RZ + + + +//------------------------------------------------------------------------- +// Rest +//------------------------------------------------------------------------- + +void make_leak(void) +{ + int* array2 = custom_alloc(sizeof(int) * 10); + array2 = 0; // leak + return; +} + +int main(void) +{ + int* array; + int* array3; + + array = custom_alloc(sizeof(int) * 10); + array[8] = 8; + array[9] = 8; + array[10] = 10; // invalid write (ok w/o MALLOCLIKE -- in superblock) + + custom_free(array); // ok + + custom_free(NULL); // invalid free (ok without MALLOCLIKE) + + array3 = malloc(sizeof(int) * 10); + custom_free(array3); // mismatched free (ok without MALLOCLIKE) + + make_leak(); + return array[0]; // use after free (ok without MALLOCLIKE/MAKE_NOACCESS) + // (nb: initialised because is_zeroed==1 above) + // unfortunately not identified as being in a free'd + // block because the freeing of the block and shadow + // chunk isn't postponed. + + // leak from make_leak() +} diff --git a/memcheck/tests/custom_alloc.stderr.exp b/memcheck/tests/custom_alloc.stderr.exp new file mode 100644 index 0000000000..9ce0280df1 --- /dev/null +++ b/memcheck/tests/custom_alloc.stderr.exp @@ -0,0 +1,44 @@ + +Invalid write of size 4 + at 0x........: main (custom_alloc.c:79) + by 0x........: __libc_start_main (...libc...) + by 0x........: ... + Address 0x........ is 48 bytes inside a block of size 100000 client-defined + at 0x........: get_superblock (custom_alloc.c:25) + by 0x........: custom_alloc (custom_alloc.c:40) + by 0x........: main (custom_alloc.c:76) + by 0x........: __libc_start_main (...libc...) + +Invalid free() / delete / delete[] + at 0x........: custom_free (custom_alloc.c:54) + by 0x........: main (custom_alloc.c:83) + by 0x........: __libc_start_main (...libc...) + by 0x........: ... + Address 0x........ is not stack'd, malloc'd or free'd + +Mismatched free() / delete / delete [] + at 0x........: custom_free (custom_alloc.c:54) + by 0x........: main (custom_alloc.c:86) + by 0x........: __libc_start_main (...libc...) + by 0x........: ... + Address 0x........ is 0 bytes inside a block of size 40 alloc'd + at 0x........: malloc (vg_replace_malloc.c:...) + by 0x........: main (custom_alloc.c:85) + by 0x........: __libc_start_main (...libc...) + by 0x........: ... + +Invalid read of size 4 + at 0x........: main (custom_alloc.c:89) + by 0x........: __libc_start_main (...libc...) + by 0x........: ... + Address 0x........ is 8 bytes inside a block of size 100000 client-defined + at 0x........: get_superblock (custom_alloc.c:25) + by 0x........: custom_alloc (custom_alloc.c:40) + by 0x........: main (custom_alloc.c:76) + by 0x........: __libc_start_main (...libc...) + +ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 0 from 0) +malloc/free: in use at exit: 40 bytes in 1 blocks. +malloc/free: 3 allocs, 3 frees, 120 bytes allocated. +For a detailed leak analysis, rerun with: --leak-check=yes +For counts of detected errors, rerun with: -v diff --git a/memcheck/tests/custom_alloc.vgtest b/memcheck/tests/custom_alloc.vgtest new file mode 100644 index 0000000000..2b8a141542 --- /dev/null +++ b/memcheck/tests/custom_alloc.vgtest @@ -0,0 +1 @@ +prog: custom_alloc