Fix accounting for MC_(realloc). It was inconsistent as compared to

other wrappers in that it took place before the silly-args check.
Testcase and patch by Yann Droneaud (yann@droneaud.fr).
Fixes #281482

Also included is a related fix to MC_(new_block). Incrementing the
alloc counter and updating the allocated memory amount should
occur under the same condition (allocation succeeded).

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@12324

diff --git a/NEWS b/NEWS
index 3b3042acb6970abd174a9c64f0f24ea10fbe043e..a5d0ae6b25e88849589334483661a86f71ca0869 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -30,6 +30,7 @@ where XXXXXX is the bug number as listed below.
 286374  Running cachegrind with --branch-sim=yes on 64-bit PowerPC program fails
 287858  VG_(strerror): unknown error 
 289699  vgdb connection in relay mode erroneously closed due to buffer overrun 
+281482  valgrind's memcheck incorrect byte allocation count in realloc() for silly argument
 
 Release 3.7.0 (5 November 2011)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

diff --git a/memcheck/mc_malloc_wrappers.c b/memcheck/mc_malloc_wrappers.c
index e1fde6c65db707359d055f388f68b1993f2607e7..163ddfaf1d8338f896fe52016b22ea74f6c84399 100644 (file)
--- a/memcheck/mc_malloc_wrappers.c
+++ b/memcheck/mc_malloc_wrappers.c
@@ -238,8 +238,6 @@ void* MC_(new_block) ( ThreadId tid,
 {
    ExeContext* ec;
 
-   cmalloc_n_mallocs ++;
-
    // Allocate and zero if necessary
    if (p) {
       tl_assert(MC_AllocCustom == kind);
@@ -258,7 +256,8 @@ void* MC_(new_block) ( ThreadId tid,
       }
    }
 
-   // Only update this stat if allocation succeeded.
+   // Only update stats if allocation succeeded.
+   cmalloc_n_mallocs ++;
    cmalloc_bs_mallocd += (ULong)szB;
 
    ec = VG_(record_ExeContext)(tid, 0/*first_ip_delta*/);
@@ -392,13 +391,13 @@ void* MC_(realloc) ( ThreadId tid, void* p_old, SizeT new_szB )
    void*     p_new;
    SizeT     old_szB;
 
+   if (complain_about_silly_args(new_szB, "realloc")) 
+      return NULL;
+
    cmalloc_n_frees ++;
    cmalloc_n_mallocs ++;
    cmalloc_bs_mallocd += (ULong)new_szB;
 
-   if (complain_about_silly_args(new_szB, "realloc")) 
-      return NULL;
-
    /* Remove the old block */
    mc = VG_(HT_remove) ( MC_(malloc_list), (UWord)p_old );
    if (mc == NULL) {

diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am
index 327368c69486c1eccd2b4d1ea7c18107b8e567ed..25e8f1a63097a3cfde9c1e9c110135edf05f07aa 100644 (file)
--- a/memcheck/tests/Makefile.am
+++ b/memcheck/tests/Makefile.am
@@ -47,6 +47,7 @@ dist_noinst_SCRIPTS = \
 noinst_HEADERS = leak.h
 
 EXTRA_DIST = \
+       accounting.stderr.exp accounting.vgtest \
        addressable.stderr.exp addressable.stdout.exp addressable.vgtest \
        atomic_incs.stderr.exp atomic_incs.vgtest \
        atomic_incs.stdout.exp-32bit atomic_incs.stdout.exp-64bit \
@@ -211,6 +212,7 @@ EXTRA_DIST = \
        xml1.stderr.exp xml1.stdout.exp xml1.vgtest xml1.stderr.exp-s390x-mvc
 
 check_PROGRAMS = \
+       accounting \
        addressable \
        atomic_incs \
        badaddrvalue badfree badjump badjump2 \

diff --git a/memcheck/tests/accounting.c b/memcheck/tests/accounting.c
new file mode 100644 (file)
index 0000000..1654bb0
--- /dev/null
+++ b/memcheck/tests/accounting.c
@@ -0,0 +1,25 @@
+/*
+ * test case for valgrind realloc() bug
+ */
+
+#include <stdlib.h>
+#include <assert.h>
+
+int
+main(void)
+{
+       void *p;
+       void *r;
+
+       p = malloc(1);
+       assert(p != NULL);
+
+       r = realloc(p, -1);
+       assert(r == NULL);
+
+       free(p);
+
+       return 0;
+}
+
+

diff --git a/memcheck/tests/accounting.stderr.exp b/memcheck/tests/accounting.stderr.exp
new file mode 100644 (file)
index 0000000..fb31e6d
--- /dev/null
+++ b/memcheck/tests/accounting.stderr.exp
@@ -0,0 +1,11 @@
+
+Warning: silly arg (-1) to realloc()
+
+HEAP SUMMARY:
+    in use at exit: 0 bytes in 0 blocks
+  total heap usage: 1 allocs, 1 frees, 1 bytes allocated
+
+For a detailed leak analysis, rerun with: --leak-check=full
+
+For counts of detected and suppressed errors, rerun with: -v
+ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

diff --git a/memcheck/tests/accounting.vgtest b/memcheck/tests/accounting.vgtest
new file mode 100644 (file)
index 0000000..7918b5c
--- /dev/null
+++ b/memcheck/tests/accounting.vgtest
@@ -0,0 +1 @@
+prog: accounting