From: Nicholas Nethercote Date: Mon, 8 Aug 2011 01:58:50 +0000 (+0000) Subject: Fix a Massif bug: when realloc'ing a block, any values in the part of the X-Git-Tag: svn/VALGRIND_3_7_0~281 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a71f5978fc7845d579bc687dabd0572eb30b5900;p=thirdparty%2Fvalgrind.git Fix a Massif bug: when realloc'ing a block, any values in the part of the block beyond the original request weren't copied. They are now. This is important because a program could use malloc_usable_size to gain legitimate access to those extra bytes. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11956 --- diff --git a/Makefile.am b/Makefile.am index 8526048a5a..f7f26257a5 100644 --- a/Makefile.am +++ b/Makefile.am @@ -14,7 +14,8 @@ TOOLS = memcheck \ EXP_TOOLS = exp-sgcheck \ exp-bbv \ - exp-dhat + exp-dhat \ + exp-dmd # DDD: once all tools work on Darwin, TEST_TOOLS and TEST_EXP_TOOLS can be # replaced with TOOLS and EXP_TOOLS. diff --git a/configure.in b/configure.in index eaaf21beac..df305b5bdf 100644 --- a/configure.in +++ b/configure.in @@ -1994,6 +1994,8 @@ AC_CONFIG_FILES([ exp-bbv/tests/arm-linux/Makefile exp-dhat/Makefile exp-dhat/tests/Makefile + exp-dmd/Makefile + exp-dmd/tests/Makefile ]) AC_CONFIG_FILES([coregrind/link_tool_exe_linux], [chmod +x coregrind/link_tool_exe_linux]) diff --git a/exp-dmd/Makefile.am b/exp-dmd/Makefile.am new file mode 100644 index 0000000000..4e956d4fcf --- /dev/null +++ b/exp-dmd/Makefile.am @@ -0,0 +1,93 @@ +include $(top_srcdir)/Makefile.tool.am + +EXTRA_DIST = docs/dmd-manual.xml + +#---------------------------------------------------------------------------- +# dmd- +#---------------------------------------------------------------------------- + +noinst_PROGRAMS = exp-dmd-@VGCONF_ARCH_PRI@-@VGCONF_OS@ +if VGCONF_HAVE_PLATFORM_SEC +noinst_PROGRAMS += exp-dmd-@VGCONF_ARCH_SEC@-@VGCONF_OS@ +endif + +NONE_SOURCES_COMMON = dmd_main.c + +exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_SOURCES = \ + $(NONE_SOURCES_COMMON) +exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_CPPFLAGS = \ + $(AM_CPPFLAGS_@VGCONF_PLATFORM_PRI_CAPS@) +exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_CFLAGS = \ + $(AM_CFLAGS_@VGCONF_PLATFORM_PRI_CAPS@) +exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_DEPENDENCIES = \ + $(TOOL_DEPENDENCIES_@VGCONF_PLATFORM_PRI_CAPS@) +exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_LDADD = \ + $(TOOL_LDADD_@VGCONF_PLATFORM_PRI_CAPS@) +exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_LDFLAGS = \ + $(TOOL_LDFLAGS_@VGCONF_PLATFORM_PRI_CAPS@) +exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_LINK = \ + $(top_builddir)/coregrind/link_tool_exe_@VGCONF_OS@ \ + @VALT_LOAD_ADDRESS_PRI@ \ + $(LINK) \ + $(exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_CFLAGS) \ + $(exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_LDFLAGS) + +if VGCONF_HAVE_PLATFORM_SEC +exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_SOURCES = \ + $(NONE_SOURCES_COMMON) +exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_CPPFLAGS = \ + $(AM_CPPFLAGS_@VGCONF_PLATFORM_SEC_CAPS@) +exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_CFLAGS = \ + $(AM_CFLAGS_@VGCONF_PLATFORM_SEC_CAPS@) +exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_DEPENDENCIES = \ + $(TOOL_DEPENDENCIES_@VGCONF_PLATFORM_SEC_CAPS@) +exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_LDADD = \ + $(TOOL_LDADD_@VGCONF_PLATFORM_SEC_CAPS@) +exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_LDFLAGS = \ + $(TOOL_LDFLAGS_@VGCONF_PLATFORM_SEC_CAPS@) +exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_LINK = \ + $(top_builddir)/coregrind/link_tool_exe_@VGCONF_OS@ \ + @VALT_LOAD_ADDRESS_SEC@ \ + $(LINK) \ + $(exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_CFLAGS) \ + $(exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_LDFLAGS) +endif + +#---------------------------------------------------------------------------- +# vgpreload_exp_dmd-.so +#---------------------------------------------------------------------------- + +noinst_PROGRAMS += vgpreload_exp-dmd-@VGCONF_ARCH_PRI@-@VGCONF_OS@.so +if VGCONF_HAVE_PLATFORM_SEC +noinst_PROGRAMS += vgpreload_exp-dmd-@VGCONF_ARCH_SEC@-@VGCONF_OS@.so +endif + +if VGCONF_OS_IS_DARWIN +noinst_DSYMS = $(noinst_PROGRAMS) +endif + +vgpreload_exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_so_SOURCES = +vgpreload_exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_so_CPPFLAGS = \ + $(AM_CPPFLAGS_@VGCONF_PLATFORM_PRI_CAPS@) +vgpreload_exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_so_CFLAGS = \ + $(AM_CFLAGS_@VGCONF_PLATFORM_PRI_CAPS@) $(AM_CFLAGS_PIC) +vgpreload_exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_so_DEPENDENCIES = \ + $(LIBREPLACEMALLOC_@VGCONF_PLATFORM_PRI_CAPS@) +vgpreload_exp_dmd_@VGCONF_ARCH_PRI@_@VGCONF_OS@_so_LDFLAGS = \ + $(PRELOAD_LDFLAGS_@VGCONF_PLATFORM_PRI_CAPS@) \ + $(LIBREPLACEMALLOC_LDFLAGS_@VGCONF_PLATFORM_PRI_CAPS@) + +if VGCONF_HAVE_PLATFORM_SEC +vgpreload_exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_so_SOURCES = +vgpreload_exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_so_CPPFLAGS = \ + $(AM_CPPFLAGS_@VGCONF_PLATFORM_SEC_CAPS@) +vgpreload_exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_so_CFLAGS = \ + $(AM_CFLAGS_@VGCONF_PLATFORM_SEC_CAPS@) $(AM_CFLAGS_PIC) +vgpreload_exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_so_DEPENDENCIES = \ + $(LIBREPLACEMALLOC_@VGCONF_PLATFORM_SEC_CAPS@) +vgpreload_exp_dmd_@VGCONF_ARCH_SEC@_@VGCONF_OS@_so_LDFLAGS = \ + $(PRELOAD_LDFLAGS_@VGCONF_PLATFORM_SEC_CAPS@) \ + $(LIBREPLACEMALLOC_LDFLAGS_@VGCONF_PLATFORM_SEC_CAPS@) +endif + + diff --git a/exp-dmd/dmd_main.c b/exp-dmd/dmd_main.c new file mode 100644 index 0000000000..d3f4df17db --- /dev/null +++ b/exp-dmd/dmd_main.c @@ -0,0 +1,275 @@ + +/*--------------------------------------------------------------------*/ +/*--- DMD: A dark matter detector. dmd_main.c ---*/ +/*--------------------------------------------------------------------*/ + +/* + This file is part of Nulgrind, the minimal Valgrind tool, + which does no instrumentation or analysis. + + Copyright (C) 2002-2010 Nicholas Nethercote + njn@valgrind.org + + This program is free software; you can redistribute it and/or + modify it under the terms of the GNU General Public License as + published by the Free Software Foundation; either version 2 of the + License, or (at your option) any later version. + + This program is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + 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. + + The GNU General Public License is contained in the file COPYING. +*/ + +#include "pub_tool_basics.h" +#include "pub_tool_hashtable.h" +#include "pub_tool_libcbase.h" +#include "pub_tool_libcassert.h" +#include "pub_tool_libcprint.h" +#include "pub_tool_mallocfree.h" +#include "pub_tool_replacemalloc.h" +#include "pub_tool_tooliface.h" + +//------------------------------------------------------------// +//--- Heap management ---// +//------------------------------------------------------------// + +// Nb: first two fields must match core's VgHashNode. +typedef + struct _HeapBlock { + struct _HeapBlock* next; + Addr data; // Ptr to actual block + SizeT reqSzB; // Size requested + SizeT slopSzB; // Extra bytes given above those requested + ExeContext* where; // Where allocated; bottom-XPt + } + HeapBlock; + +static VgHashTable heapBlocks = NULL; // HeapBlocks + +static __inline__ +void* allocAndRecordBlock(ThreadId tid, SizeT reqSzB, SizeT reqAlignB, + Bool isZeroed) +{ + SizeT actualSzB, slopSzB; + void* p; + + if ((SSizeT)reqSzB < 0) + return NULL; + + // Allocate and zero if necessary. + p = VG_(cli_malloc)(reqAlignB, reqSzB); + if (!p) { + return NULL; + } + if (isZeroed) { + VG_(memset)(p, 0, reqSzB); + } + actualSzB = VG_(malloc_usable_size)(p); + tl_assert(actualSzB >= reqSzB); + slopSzB = actualSzB - reqSzB; + + VG_(printf)("allocAndRecordBlock: %ld + %ld\n", reqSzB, slopSzB); + + // Record block. + HeapBlock* hb = VG_(malloc)("dmd.main.rb.1", sizeof(HeapBlock)); + hb->reqSzB = reqSzB; + hb->slopSzB = slopSzB; + hb->data = (Addr)p; + hb->where = VG_(record_ExeContext)(tid, 0/*first_ip_delta*/); + VG_(HT_add_node)(heapBlocks, hb); + + return p; +} + +static __inline__ +void unrecordAndFreeBlock(void* p) +{ + HeapBlock* hb = VG_(HT_remove)(heapBlocks, (UWord)p); + if (NULL == hb) { + return; // must have been a bogus free() + } + VG_(free)(hb); hb = NULL; + VG_(cli_free)(p); + + VG_(printf)("unrecordAndFreeBlock\n"); +} + +static __inline__ +void* reallocAndRecordBlock(ThreadId tid, void* pOld, SizeT newReqSzB) +{ + HeapBlock* hb; + void* pNew; + SizeT oldReqSzB, oldSlopSzB, newSlopSzB, newActualSzB; + + // Remove the old block + hb = VG_(HT_remove)(heapBlocks, (UWord)pOld); + if (hb == NULL) { + return NULL; // must have been a bogus realloc() + } + + oldReqSzB = hb->reqSzB; + oldSlopSzB = hb->slopSzB; + + VG_(printf)("reallocAndRecordBlock: %ld\n", newReqSzB); + + if (newReqSzB <= oldReqSzB + oldSlopSzB) { + // New size is smaller or same; block not moved. + pNew = pOld; + newSlopSzB = oldSlopSzB + (oldReqSzB - newReqSzB); + + } else { + // New size is bigger; make new block, copy shared contents, free old. + pNew = VG_(cli_malloc)(VG_(clo_alignment), newReqSzB); + if (!pNew) { + // Nb: if realloc fails, NULL is returned but the old block is not + // touched. What an awful function. + return NULL; + } + // XXX: need the oldSlopSzB here, so does Massif! test + VG_(memcpy)(pNew, pOld, oldReqSzB /*+ oldSlopSzB*/); + VG_(cli_free)(pOld); + newActualSzB = VG_(malloc_usable_size)(pNew); + tl_assert(newActualSzB >= newReqSzB); + newSlopSzB = newActualSzB - newReqSzB; + } + + if (pNew) { + // Update HeapBlock. + hb->data = (Addr)pNew; + hb->reqSzB = newReqSzB; + hb->slopSzB = newSlopSzB; + hb->where = VG_(record_ExeContext)(tid, 0/*first_ip_delta*/); + } + + // Now insert the new hb (with a possibly new 'data' field) into + // heapBlocks. If this realloc() did not increase the memory size, we + // will have removed and then re-added hb unnecessarily. But that's ok + // because shrinking a block with realloc() is (presumably) much rarer + // than growing it, and this way simplifies the growing case. + VG_(HT_add_node)(heapBlocks, hb); + + return pNew; +} + + +//------------------------------------------------------------// +//--- malloc() et al replacement wrappers ---// +//------------------------------------------------------------// + +static void* dmd_malloc(ThreadId tid, SizeT szB) +{ + return allocAndRecordBlock(tid, szB, VG_(clo_alignment), /*isZeroed*/False); +} + +static void* dmd___builtin_new(ThreadId tid, SizeT szB) +{ + return allocAndRecordBlock(tid, szB, VG_(clo_alignment), /*isZeroed*/False); +} + +static void* dmd___builtin_vec_new(ThreadId tid, SizeT szB) +{ + return allocAndRecordBlock(tid, szB, VG_(clo_alignment), /*isZeroed*/False); +} + +static void* dmd_calloc(ThreadId tid, SizeT m, SizeT szB) +{ + return allocAndRecordBlock(tid, m*szB, VG_(clo_alignment), /*isZeroed*/True); +} + +static void *dmd_memalign(ThreadId tid, SizeT alignB, SizeT szB) +{ + return allocAndRecordBlock(tid, szB, alignB, /*isZeroed*/False); +} + +static void dmd_free(ThreadId tid __attribute__((unused)), void* p) +{ + unrecordAndFreeBlock(p); +} + +static void dmd___builtin_delete(ThreadId tid, void* p) +{ + unrecordAndFreeBlock(p); +} + +static void dmd___builtin_vec_delete(ThreadId tid, void* p) +{ + unrecordAndFreeBlock(p); +} + +static void* dmd_realloc(ThreadId tid, void* pOld, SizeT new_szB) +{ + return reallocAndRecordBlock(tid, pOld, new_szB); +} + +static SizeT dmd_malloc_usable_size(ThreadId tid, void* p) +{ + HeapBlock* hb = VG_(HT_lookup)(heapBlocks, (UWord)p); + return hb ? hb->reqSzB + hb->slopSzB : 0; +} + +//------------------------------------------------------------// +//--- Basic functions ---// +//------------------------------------------------------------// + +static void dmd_post_clo_init(void) +{ +} + +static +IRSB* dmd_instrument(VgCallbackClosure* closure, + IRSB* bb, + VexGuestLayout* layout, + VexGuestExtents* vge, + IRType gWordTy, IRType hWordTy) +{ + return bb; +} + +static void dmd_fini(Int exitcode) +{ +} + +static void dmd_pre_clo_init(void) +{ + VG_(details_name) ("DMD"); + VG_(details_version) (NULL); + VG_(details_description) ("a dark matter detector"); + VG_(details_copyright_author)( + "Copyright (C) 2011-2011, and GNU GPL'd, by Nicholas Nethercote."); + VG_(details_bug_reports_to) (VG_BUGS_TO); + + VG_(details_avg_translation_sizeB)(275); + + VG_(basic_tool_funcs) (dmd_post_clo_init, + dmd_instrument, + dmd_fini); + + // Needs. + VG_(needs_malloc_replacement)(dmd_malloc, + dmd___builtin_new, + dmd___builtin_vec_new, + dmd_memalign, + dmd_calloc, + dmd_free, + dmd___builtin_delete, + dmd___builtin_vec_delete, + dmd_realloc, + dmd_malloc_usable_size, + 0); + + heapBlocks = VG_(HT_construct)("DMD's heapBlocks"); +} + +VG_DETERMINE_INTERFACE_VERSION(dmd_pre_clo_init) + +/*--------------------------------------------------------------------*/ +/*--- end ---*/ +/*--------------------------------------------------------------------*/ diff --git a/exp-dmd/tests/Makefile.am b/exp-dmd/tests/Makefile.am new file mode 100644 index 0000000000..e69de29bb2 diff --git a/massif/ms_main.c b/massif/ms_main.c index 4609eadd8b..48661072f0 100644 --- a/massif/ms_main.c +++ b/massif/ms_main.c @@ -1708,7 +1708,7 @@ void* realloc_block ( ThreadId tid, void* p_old, SizeT new_req_szB ) // touched. What an awful function. return NULL; } - VG_(memcpy)(p_new, p_old, old_req_szB); + VG_(memcpy)(p_new, p_old, old_req_szB + old_slop_szB); VG_(cli_free)(p_old); new_actual_szB = VG_(malloc_usable_size)(p_new); tl_assert(new_actual_szB >= new_req_szB);