|
From: <sv...@va...> - 2011-08-08 02:03:40
|
Author: njn
Date: 2011-08-08 02:58:50 +0100 (Mon, 08 Aug 2011)
New Revision: 11956
Log:
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.
Added:
trunk/exp-dmd/
trunk/exp-dmd/Makefile.am
trunk/exp-dmd/dmd_main.c
trunk/exp-dmd/docs/
trunk/exp-dmd/tests/
trunk/exp-dmd/tests/Makefile.am
Modified:
trunk/Makefile.am
trunk/configure.in
trunk/massif/ms_main.c
Modified: trunk/Makefile.am
===================================================================
--- trunk/Makefile.am 2011-08-01 22:35:21 UTC (rev 11955)
+++ trunk/Makefile.am 2011-08-08 01:58:50 UTC (rev 11956)
@@ -14,7 +14,8 @@
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.
Modified: trunk/configure.in
===================================================================
--- trunk/configure.in 2011-08-01 22:35:21 UTC (rev 11955)
+++ trunk/configure.in 2011-08-08 01:58:50 UTC (rev 11956)
@@ -1994,6 +1994,8 @@
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])
Added: trunk/exp-dmd/Makefile.am
===================================================================
--- trunk/exp-dmd/Makefile.am (rev 0)
+++ trunk/exp-dmd/Makefile.am 2011-08-08 01:58:50 UTC (rev 11956)
@@ -0,0 +1,93 @@
+include $(top_srcdir)/Makefile.tool.am
+
+EXTRA_DIST = docs/dmd-manual.xml
+
+#----------------------------------------------------------------------------
+# dmd-<platform>
+#----------------------------------------------------------------------------
+
+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-<platform>.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
+
+
Added: trunk/exp-dmd/dmd_main.c
===================================================================
--- trunk/exp-dmd/dmd_main.c (rev 0)
+++ trunk/exp-dmd/dmd_main.c 2011-08-08 01:58:50 UTC (rev 11956)
@@ -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
+ nj...@va...
+
+ 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 ---*/
+/*--------------------------------------------------------------------*/
Added: trunk/exp-dmd/tests/Makefile.am
===================================================================
Modified: trunk/massif/ms_main.c
===================================================================
--- trunk/massif/ms_main.c 2011-08-01 22:35:21 UTC (rev 11955)
+++ trunk/massif/ms_main.c 2011-08-08 01:58:50 UTC (rev 11956)
@@ -1708,7 +1708,7 @@
// 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);
|
|
From: Bart V. A. <bva...@ac...> - 2011-08-08 10:14:55
|
On Mon, Aug 8, 2011 at 3:58 AM, <sv...@va...> wrote: > Author: njn > Date: 2011-08-08 02:58:50 +0100 (Mon, 08 Aug 2011) > New Revision: 11956 > > > Added: trunk/exp-dmd/dmd_main.c > =================================================================== > --- trunk/exp-dmd/dmd_main.c (rev 0) > +++ trunk/exp-dmd/dmd_main.c 2011-08-08 01:58:50 UTC (rev 11956) > @@ -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. > I think the above comment should be updated. Bart. |
|
From: Tom H. <to...@co...> - 2011-08-08 10:32:44
|
On 08/08/11 11:14, Bart Van Assche wrote: > On Mon, Aug 8, 2011 at 3:58 AM, <sv...@va... > <mailto:sv...@va...>> wrote: > > Author: njn > Date: 2011-08-08 02:58:50 +0100 (Mon, 08 Aug 2011) > New Revision: 11956 > > + This file is part of Nulgrind, the minimal Valgrind tool, > + which does no instrumentation or analysis. > > > I think the above comment should be updated. If you look at Nick's next commit you'll see that he didn't mean to commit that file and has now removed it again. Tom -- Tom Hughes (to...@co...) http://compton.nu/ |