From 9df672f2367e6b33a8932ffa6f02e25fb68324df Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 25 Mar 2011 20:07:25 +0000 Subject: [PATCH] Add VALGRIND_RESIZEINPLACE_BLOCK() and hence close #267819. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11666 --- coregrind/m_scheduler/scheduler.c | 1 + docs/xml/manual-core-adv.xml | 9 ++++++ drd/drd_clientreq.c | 16 +++++++++++ include/valgrind.h | 33 +++++++++++++++++++--- massif/ms_main.c | 9 ++++++ memcheck/mc_errors.c | 5 ++-- memcheck/mc_include.h | 3 ++ memcheck/mc_main.c | 10 +++++++ memcheck/mc_malloc_wrappers.c | 31 +++++++++++++++++++++ memcheck/tests/badfree-2trace.stderr.exp | 4 +-- memcheck/tests/badfree.stderr.exp | 4 +-- memcheck/tests/badfree3.stderr.exp | 4 +-- memcheck/tests/custom_alloc.c | 21 +++++++++++++- memcheck/tests/custom_alloc.stderr.exp | 35 +++++++++++++++++++----- memcheck/tests/doublefree.stderr.exp | 2 +- memcheck/tests/fprw.stderr.exp | 2 +- memcheck/tests/malloc2.stderr.exp | 2 +- memcheck/tests/memalign_test.stderr.exp | 2 +- memcheck/tests/suppfree.stderr.exp | 2 +- memcheck/tests/xml1.stderr.exp | 4 +-- 20 files changed, 172 insertions(+), 27 deletions(-) diff --git a/coregrind/m_scheduler/scheduler.c b/coregrind/m_scheduler/scheduler.c index fb25c9be86..200adaef1b 100644 --- a/coregrind/m_scheduler/scheduler.c +++ b/coregrind/m_scheduler/scheduler.c @@ -1584,6 +1584,7 @@ void do_client_request ( ThreadId tid ) } case VG_USERREQ__MALLOCLIKE_BLOCK: + case VG_USERREQ__RESIZEINPLACE_BLOCK: case VG_USERREQ__FREELIKE_BLOCK: // Ignore them if the addr is NULL; otherwise pass onto the tool. if (!arg[1]) { diff --git a/docs/xml/manual-core-adv.xml b/docs/xml/manual-core-adv.xml index 9eea68c3db..4d00ebfcda 100644 --- a/docs/xml/manual-core-adv.xml +++ b/docs/xml/manual-core-adv.xml @@ -151,6 +151,15 @@ tool-specific macros). + + VALGRIND_RESIZEINPLACE_BLOCK: + + Informs a Valgrind tool that the size of an allocated block has been + modified but not its address. See valgrind.h for + more information on how to use it. + + + VALGRIND_CREATE_MEMPOOL, diff --git a/drd/drd_clientreq.c b/drd/drd_clientreq.c index 6b63dd904d..ba0a5a4495 100644 --- a/drd/drd_clientreq.c +++ b/drd/drd_clientreq.c @@ -97,6 +97,22 @@ static Bool handle_client_request(ThreadId vg_tid, UWord* arg, UWord* ret) DRD_(malloclike_block)(vg_tid, arg[1]/*addr*/, arg[2]/*size*/); break; + case VG_USERREQ__RESIZEINPLACE_BLOCK: + if (!DRD_(freelike_block)(vg_tid, arg[1]/*addr*/, False)) + { + GenericErrInfo GEI = { + .tid = DRD_(thread_get_running_tid)(), + .addr = 0, + }; + VG_(maybe_record_error)(vg_tid, + GenericErr, + VG_(get_IP)(vg_tid), + "Invalid VG_USERREQ__RESIZEINPLACE_BLOCK request", + &GEI); + } + DRD_(malloclike_block)(vg_tid, arg[1]/*addr*/, arg[3]/*newSize*/); + break; + case VG_USERREQ__FREELIKE_BLOCK: if (arg[1] && ! DRD_(freelike_block)(vg_tid, arg[1]/*addr*/, False)) { diff --git a/include/valgrind.h b/include/valgrind.h index 4843a753ed..75441bcac1 100644 --- a/include/valgrind.h +++ b/include/valgrind.h @@ -4846,6 +4846,7 @@ typedef /* These are useful and can be interpreted by any tool that tracks malloc() et al, by using vg_replace_malloc.c. */ VG_USERREQ__MALLOCLIKE_BLOCK = 0x1301, + VG_USERREQ__RESIZEINPLACE_BLOCK = 0x130b, VG_USERREQ__FREELIKE_BLOCK = 0x1302, /* Memory pool support. */ VG_USERREQ__CREATE_MEMPOOL = 0x1303, @@ -5178,7 +5179,24 @@ VALGRIND_PRINTF_BACKTRACE(const char *format, ...) VALGRIND_FREELIKE_BLOCK should be put immediately after the point where a heap block is deallocated. - In many cases, these two client requests will not be enough to get your + VALGRIND_RESIZEINPLACE_BLOCK informs a tool about reallocation. For + Memcheck, it does four things: + + - It records that the size of a block has been changed. This assumes that + the block was annotated as having been allocated via + VALGRIND_MALLOCLIKE_BLOCK. Otherwise, an error will be issued. + + - If the block shrunk, it marks the freed memory as being unaddressable. + + - If the block grew, it marks the new area as undefined and defines a red + zone past the end of the new block. + + - The V-bits of the overlap between the old and the new block are preserved. + + VALGRIND_RESIZEINPLACE_BLOCK should be put after allocation of the new block + and before deallocation of the old block. + + In many cases, these three client requests will not be enough to get your allocator working well with Memcheck. More specifically, if your allocator writes to freed blocks in any way then a VALGRIND_MAKE_MEM_UNDEFINED call will be necessary to mark the memory as addressable just before the zeroing @@ -5196,9 +5214,6 @@ VALGRIND_PRINTF_BACKTRACE(const char *format, ...) understand the distinction between the allocator and the rest of the program. - Note: there is currently no VALGRIND_REALLOCLIKE_BLOCK client request; it - has to be emulated with MALLOCLIKE/FREELIKE and memory copying. - Ignored if addr == 0. */ #define VALGRIND_MALLOCLIKE_BLOCK(addr, sizeB, rzB, is_zeroed) \ @@ -5208,6 +5223,16 @@ VALGRIND_PRINTF_BACKTRACE(const char *format, ...) addr, sizeB, rzB, is_zeroed, 0); \ } +/* See the comment for VALGRIND_MALLOCLIKE_BLOCK for details. + Ignored if addr == 0. +*/ +#define VALGRIND_RESIZEINPLACE_BLOCK(addr, oldSizeB, newSizeB, rzB)\ + {unsigned int _qzz_res; \ + VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0, \ + VG_USERREQ__RESIZEINPLACE_BLOCK, \ + addr, oldSizeB, newSizeB, rzB, 0); \ + } + /* See the comment for VALGRIND_MALLOCLIKE_BLOCK for details. Ignored if addr == 0. */ diff --git a/massif/ms_main.c b/massif/ms_main.c index ae39b7a527..67702cda8f 100644 --- a/massif/ms_main.c +++ b/massif/ms_main.c @@ -1988,6 +1988,15 @@ static Bool ms_handle_client_request ( ThreadId tid, UWord* argv, UWord* ret ) *ret = 0; return True; } + case VG_USERREQ__RESIZEINPLACE_BLOCK: { + void* p = (void*)argv[1]; + SizeT newSizeB = argv[3]; + + unrecord_block(p, /*maybe_snapshot*/True); + record_block(tid, p, newSizeB, /*slop_szB*/0, + /*exclude_first_entry*/False, /*maybe_snapshot*/True); + return True; + } case VG_USERREQ__FREELIKE_BLOCK: { void* p = (void*)argv[1]; unrecord_block(p, /*maybe_snapshot*/True); diff --git a/memcheck/mc_errors.c b/memcheck/mc_errors.c index 43da9880b4..e09d5147e5 100644 --- a/memcheck/mc_errors.c +++ b/memcheck/mc_errors.c @@ -582,12 +582,13 @@ void MC_(pp_Error) ( Error* err ) case Err_Free: if (xml) { emit( " InvalidFree\n" ); - emit( " Invalid free() / delete / delete[]\n" ); + emit( " Invalid free() / delete / delete[]" + " / realloc()\n" ); VG_(pp_ExeContext)( VG_(get_error_where)(err) ); mc_pp_AddrInfo( VG_(get_error_address)(err), &extra->Err.Free.ai, False ); } else { - emit( "Invalid free() / delete / delete[]\n" ); + emit( "Invalid free() / delete / delete[] / realloc()\n" ); VG_(pp_ExeContext)( VG_(get_error_where)(err) ); mc_pp_AddrInfo( VG_(get_error_address)(err), &extra->Err.Free.ai, False ); diff --git a/memcheck/mc_include.h b/memcheck/mc_include.h index c2ef03eb65..ece6eac314 100644 --- a/memcheck/mc_include.h +++ b/memcheck/mc_include.h @@ -127,6 +127,9 @@ void MC_(__builtin_vec_delete) ( ThreadId tid, void* p ); void* MC_(realloc) ( ThreadId tid, void* p, SizeT new_size ); SizeT MC_(malloc_usable_size) ( ThreadId tid, void* p ); +void MC_(handle_resizeInPlace)(ThreadId tid, Addr p, + SizeT oldSizeB, SizeT newSizeB, SizeT rzB); + /*------------------------------------------------------------*/ /*--- Origin tracking translate-time support ---*/ diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c index f62ad47d1c..37735cfe0a 100644 --- a/memcheck/mc_main.c +++ b/memcheck/mc_main.c @@ -4987,6 +4987,7 @@ static Bool mc_handle_client_request ( ThreadId tid, UWord* arg, UWord* ret ) if (!VG_IS_TOOL_USERREQ('M','C',arg[0]) && VG_USERREQ__MALLOCLIKE_BLOCK != arg[0] + && VG_USERREQ__RESIZEINPLACE_BLOCK != arg[0] && VG_USERREQ__FREELIKE_BLOCK != arg[0] && VG_USERREQ__CREATE_MEMPOOL != arg[0] && VG_USERREQ__DESTROY_MEMPOOL != arg[0] @@ -5121,6 +5122,15 @@ static Bool mc_handle_client_request ( ThreadId tid, UWord* arg, UWord* ret ) MC_AllocCustom, MC_(malloc_list) ); return True; } + case VG_USERREQ__RESIZEINPLACE_BLOCK: { + Addr p = (Addr)arg[1]; + SizeT oldSizeB = arg[2]; + SizeT newSizeB = arg[3]; + UInt rzB = arg[4]; + + MC_(handle_resizeInPlace) ( tid, p, oldSizeB, newSizeB, rzB ); + return True; + } case VG_USERREQ__FREELIKE_BLOCK: { Addr p = (Addr)arg[1]; UInt rzB = arg[2]; diff --git a/memcheck/mc_malloc_wrappers.c b/memcheck/mc_malloc_wrappers.c index 2e8b78103c..d8f40a869b 100644 --- a/memcheck/mc_malloc_wrappers.c +++ b/memcheck/mc_malloc_wrappers.c @@ -486,6 +486,37 @@ SizeT MC_(malloc_usable_size) ( ThreadId tid, void* p ) return ( mc ? mc->szB : 0 ); } +/* This handles the in place resize of a block, as performed by the + VALGRIND_RESIZEINPLACE_BLOCK client request. It is unrelated to, + and not used for, handling of the normal libc realloc() + function. */ +void MC_(handle_resizeInPlace)(ThreadId tid, Addr p, + SizeT oldSizeB, SizeT newSizeB, SizeT rzB) +{ + MC_Chunk* mc = VG_(HT_lookup) ( MC_(malloc_list), (UWord)p ); + if (!mc || mc->szB != oldSizeB || newSizeB == 0) { + /* Reject if: p is not found, or oldSizeB is wrong, + or new block would be empty. */ + MC_(record_free_error) ( tid, p ); + return; + } + + if (oldSizeB == newSizeB) + return; + + mc->szB = newSizeB; + if (newSizeB < oldSizeB) { + MC_(make_mem_noaccess)( p + newSizeB, oldSizeB - newSizeB + rzB ); + } else { + ExeContext* ec = VG_(record_ExeContext)(tid, 0/*first_ip_delta*/); + UInt ecu = VG_(get_ECU_from_ExeContext)(ec); + MC_(make_mem_undefined_w_otag)( p + oldSizeB, newSizeB - oldSizeB, + ecu | MC_OKIND_HEAP ); + if (rzB > 0) + MC_(make_mem_noaccess)( p + newSizeB, rzB ); + } +} + /*------------------------------------------------------------*/ /*--- Memory pool stuff. ---*/ diff --git a/memcheck/tests/badfree-2trace.stderr.exp b/memcheck/tests/badfree-2trace.stderr.exp index 5b937c19e9..a3ea7a413e 100644 --- a/memcheck/tests/badfree-2trace.stderr.exp +++ b/memcheck/tests/badfree-2trace.stderr.exp @@ -1,9 +1,9 @@ -Invalid free() / delete / delete[] +Invalid free() / delete / delete[] / realloc() at 0x........: free (vg_replace_malloc.c:...) by 0x........: main (badfree.c:12) Address 0x........ is not stack'd, malloc'd or (recently) free'd -Invalid free() / delete / delete[] +Invalid free() / delete / delete[] / realloc() at 0x........: free (vg_replace_malloc.c:...) by 0x........: main (badfree.c:15) Address 0x........ is on thread 1's stack diff --git a/memcheck/tests/badfree.stderr.exp b/memcheck/tests/badfree.stderr.exp index 5b937c19e9..a3ea7a413e 100644 --- a/memcheck/tests/badfree.stderr.exp +++ b/memcheck/tests/badfree.stderr.exp @@ -1,9 +1,9 @@ -Invalid free() / delete / delete[] +Invalid free() / delete / delete[] / realloc() at 0x........: free (vg_replace_malloc.c:...) by 0x........: main (badfree.c:12) Address 0x........ is not stack'd, malloc'd or (recently) free'd -Invalid free() / delete / delete[] +Invalid free() / delete / delete[] / realloc() at 0x........: free (vg_replace_malloc.c:...) by 0x........: main (badfree.c:15) Address 0x........ is on thread 1's stack diff --git a/memcheck/tests/badfree3.stderr.exp b/memcheck/tests/badfree3.stderr.exp index ca3ecf55e3..f276c3578a 100644 --- a/memcheck/tests/badfree3.stderr.exp +++ b/memcheck/tests/badfree3.stderr.exp @@ -1,9 +1,9 @@ -Invalid free() / delete / delete[] +Invalid free() / delete / delete[] / realloc() at 0x........: free (coregrind/vg_replace_malloc.c:...) by 0x........: main (memcheck/tests/badfree.c:12) Address 0x........ is not stack'd, malloc'd or (recently) free'd -Invalid free() / delete / delete[] +Invalid free() / delete / delete[] / realloc() at 0x........: free (coregrind/vg_replace_malloc.c:...) by 0x........: main (memcheck/tests/badfree.c:15) Address 0x........ is on thread 1's stack diff --git a/memcheck/tests/custom_alloc.c b/memcheck/tests/custom_alloc.c index 9f22851b01..13bf12a2aa 100644 --- a/memcheck/tests/custom_alloc.c +++ b/memcheck/tests/custom_alloc.c @@ -53,7 +53,7 @@ static void custom_free(void* p) // don't actually free any memory... but mark it as freed VALGRIND_FREELIKE_BLOCK( p, RZ ); } -#undef RZ + @@ -78,6 +78,23 @@ int main(void) array[9] = 8; array[10] = 10; // invalid write (ok w/o MALLOCLIKE -- in superblock) + VALGRIND_RESIZEINPLACE_BLOCK(array, sizeof(int) * 10, sizeof(int) * 5, RZ); + array[4] = 7; + array[5] = 9; // invalid write + + // Make the entire array defined again such that it can be verified whether + // the red zone is marked properly when resizing in place. + VALGRIND_MAKE_MEM_DEFINED(array, sizeof(int) * 10); + + VALGRIND_RESIZEINPLACE_BLOCK(array, sizeof(int) * 5, sizeof(int) * 7, RZ); + if (array[5]) array[4]++; // uninitialized read of array[5] + array[5] = 11; + array[6] = 7; + array[7] = 8; // invalid write + + // invalid realloc + VALGRIND_RESIZEINPLACE_BLOCK(array+1, sizeof(int) * 7, sizeof(int) * 8, RZ); + custom_free(array); // ok custom_free((void*)0x1); // invalid free @@ -101,3 +118,5 @@ int main(void) // leak from make_leak() } + +#undef RZ diff --git a/memcheck/tests/custom_alloc.stderr.exp b/memcheck/tests/custom_alloc.stderr.exp index b3c2b09a32..bb55f8c0cf 100644 --- a/memcheck/tests/custom_alloc.stderr.exp +++ b/memcheck/tests/custom_alloc.stderr.exp @@ -4,21 +4,42 @@ Invalid write of size 4 at 0x........: custom_alloc (custom_alloc.c:47) by 0x........: main (custom_alloc.c:76) -Invalid free() / delete / delete[] +Invalid write of size 4 + at 0x........: main (custom_alloc.c:83) + Address 0x........ is 0 bytes after a block of size 20 alloc'd + at 0x........: custom_alloc (custom_alloc.c:47) + by 0x........: main (custom_alloc.c:76) + +Conditional jump or move depends on uninitialised value(s) + at 0x........: main (custom_alloc.c:90) + +Invalid write of size 4 + at 0x........: main (custom_alloc.c:93) + Address 0x........ is 0 bytes after a block of size 28 alloc'd + at 0x........: custom_alloc (custom_alloc.c:47) + by 0x........: main (custom_alloc.c:76) + +Invalid free() / delete / delete[] / realloc() + at 0x........: main (custom_alloc.c:96) + Address 0x........ is 4 bytes inside a block of size 28 alloc'd + at 0x........: custom_alloc (custom_alloc.c:47) + by 0x........: main (custom_alloc.c:76) + +Invalid free() / delete / delete[] / realloc() at 0x........: custom_free (custom_alloc.c:54) - by 0x........: main (custom_alloc.c:83) + by 0x........: main (custom_alloc.c:100) Address 0x........ is not stack'd, malloc'd or (recently) free'd Mismatched free() / delete / delete [] at 0x........: custom_free (custom_alloc.c:54) - by 0x........: main (custom_alloc.c:86) + by 0x........: main (custom_alloc.c:103) 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........: main (custom_alloc.c:102) Invalid read of size 4 - at 0x........: main (custom_alloc.c:89) - Address 0x........ is 0 bytes inside a block of size 40 free'd + at 0x........: main (custom_alloc.c:106) + Address 0x........ is 0 bytes inside a block of size 28 free'd at 0x........: custom_free (custom_alloc.c:54) - by 0x........: main (custom_alloc.c:81) + by 0x........: main (custom_alloc.c:98) diff --git a/memcheck/tests/doublefree.stderr.exp b/memcheck/tests/doublefree.stderr.exp index 672eda01bd..9ed5375c72 100644 --- a/memcheck/tests/doublefree.stderr.exp +++ b/memcheck/tests/doublefree.stderr.exp @@ -1,4 +1,4 @@ -Invalid free() / delete / delete[] +Invalid free() / delete / delete[] / realloc() at 0x........: free (vg_replace_malloc.c:...) by 0x........: main (doublefree.c:10) Address 0x........ is 0 bytes inside a block of size 177 free'd diff --git a/memcheck/tests/fprw.stderr.exp b/memcheck/tests/fprw.stderr.exp index d083387ce6..4719f80b41 100644 --- a/memcheck/tests/fprw.stderr.exp +++ b/memcheck/tests/fprw.stderr.exp @@ -28,7 +28,7 @@ Invalid write of size 4 at 0x........: free (vg_replace_malloc.c:...) by 0x........: main (fprw.c:19) -Invalid free() / delete / delete[] +Invalid free() / delete / delete[] / realloc() at 0x........: free (vg_replace_malloc.c:...) by 0x........: main (fprw.c:22) Address 0x........ is not stack'd, malloc'd or (recently) free'd diff --git a/memcheck/tests/malloc2.stderr.exp b/memcheck/tests/malloc2.stderr.exp index 0c29fa4db2..7ba0426992 100644 --- a/memcheck/tests/malloc2.stderr.exp +++ b/memcheck/tests/malloc2.stderr.exp @@ -4,7 +4,7 @@ Invalid write of size 1 at 0x........: free (vg_replace_malloc.c:...) by 0x........: main (malloc2.c:38) -Invalid free() / delete / delete[] +Invalid free() / delete / delete[] / realloc() at 0x........: free (vg_replace_malloc.c:...) by 0x........: main (malloc2.c:43) Address 0x........ is 0 bytes inside a block of size 429 free'd diff --git a/memcheck/tests/memalign_test.stderr.exp b/memcheck/tests/memalign_test.stderr.exp index 5a01c4ad7f..aa8d588336 100644 --- a/memcheck/tests/memalign_test.stderr.exp +++ b/memcheck/tests/memalign_test.stderr.exp @@ -1,4 +1,4 @@ -Invalid free() / delete / delete[] +Invalid free() / delete / delete[] / realloc() at 0x........: free (vg_replace_malloc.c:...) by 0x........: main (memalign_test.c:25) Address 0x........ is 0 bytes inside a block of size 111,110 free'd diff --git a/memcheck/tests/suppfree.stderr.exp b/memcheck/tests/suppfree.stderr.exp index a75d356042..e29d17bfde 100644 --- a/memcheck/tests/suppfree.stderr.exp +++ b/memcheck/tests/suppfree.stderr.exp @@ -1,4 +1,4 @@ -Invalid free() / delete / delete[] +Invalid free() / delete / delete[] / realloc() at 0x........: free (vg_replace_malloc.c:...) by 0x........: ddd (suppfree.c:7) by 0x........: ccc (suppfree.c:12) diff --git a/memcheck/tests/xml1.stderr.exp b/memcheck/tests/xml1.stderr.exp index 8471e5fa91..b6cb890b26 100644 --- a/memcheck/tests/xml1.stderr.exp +++ b/memcheck/tests/xml1.stderr.exp @@ -198,7 +198,7 @@ 0x........ ... InvalidFree - Invalid free() / delete / delete[] + Invalid free() / delete / delete[] / realloc() 0x........ @@ -290,7 +290,7 @@ 0x........ ... InvalidFree - Invalid free() / delete / delete[] + Invalid free() / delete / delete[] / realloc() 0x........ -- 2.47.2