fix 298943 massif asserts with --pages-as-heap=yes when brk is changing by value...

* add a massif test to (somewhat) validate --pages-as-heap=yes
  with calls to brk not being a multiple of a page size

* fix the assert:
   only record new pages or unrecord old pages if at least one new
   full page (or one full old page) is added/removed.

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

diff --git a/NEWS b/NEWS
index 3542a7a9e60d58a8c940589f05749649c56bb71a..fdf661e5f4dae2d48b9af924e16c7771c07e1fc7 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -84,6 +84,7 @@ n-i-bz  s390x: Shadow registers can now be examined using vgdb
 297992  Support systems missing WIFCONTINUED (e.g. pre-2.6.10 Linux) 
 297993  Fix compilation of valgrind with gcc -g3.
 298394  s390x: Don't bail out on an unknown machine model. Assume it's a new model.
+298943  massif asserts with --pages-as-heap=yes when brk is chaning by value different of page size
 
 Release 3.7.0 (5 November 2011)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

diff --git a/massif/ms_main.c b/massif/ms_main.c
index d134ab12ac85ead6164bd6666222b860b0e99d9a..74b350aee77b613de9c6b01aa7c3446e42ce9907 100644 (file)
--- a/massif/ms_main.c
+++ b/massif/ms_main.c
@@ -1884,8 +1884,15 @@ void ms_new_mem_startup( Addr a, SizeT len,
 static
 void ms_new_mem_brk ( Addr a, SizeT len, ThreadId tid )
 {
-   tl_assert(VG_IS_PAGE_ALIGNED(len));
-   ms_record_page_mem(a, len);
+   // brk limit is not necessarily aligned on a page boundary.
+   // If new memory being brk-ed implies to allocate a new page,
+   // then call ms_record_page_mem with page aligned parameters
+   // otherwise just ignore.
+   Addr old_bottom_page = VG_PGROUNDDN(a - 1);
+   Addr new_top_page = VG_PGROUNDDN(a + len - 1);
+   if (old_bottom_page != new_top_page)
+      ms_record_page_mem(VG_PGROUNDDN(a),
+                         (new_top_page - old_bottom_page));
 }
 
 static
@@ -1906,8 +1913,14 @@ void ms_die_mem_munmap( Addr a, SizeT len )
 static
 void ms_die_mem_brk( Addr a, SizeT len )
 {
-   tl_assert(VG_IS_PAGE_ALIGNED(len));
-   ms_unrecord_page_mem(a, len);
+   // Call ms_unrecord_page_mem only if one or more pages are de-allocated.
+   // See ms_new_mem_brk for more details.
+   Addr new_bottom_page = VG_PGROUNDDN(a - 1);
+   Addr old_top_page = VG_PGROUNDDN(a + len - 1);
+   if (old_top_page != new_bottom_page)
+      ms_unrecord_page_mem(VG_PGROUNDDN(a),
+                           (old_top_page - new_bottom_page));
+
 }
 
 //------------------------------------------------------------//

diff --git a/massif/tests/Makefile.am b/massif/tests/Makefile.am
index bfdb842a076c8da293f4d4c7e421dcaa71d2d2df..c6caf309d2862d86548c853f0d5edef05c2fa6e8 100644 (file)
--- a/massif/tests/Makefile.am
+++ b/massif/tests/Makefile.am
@@ -28,6 +28,7 @@ EXTRA_DIST = \
        null.post.exp null.stderr.exp null.vgtest \
        one.post.exp one.post.exp2 one.stderr.exp one.vgtest \
        overloaded-new.post.exp overloaded-new.stderr.exp overloaded-new.vgtest \
+       pages_as_heap.stderr.exp pages_as_heap.vgtest \
        peak.post.exp peak.stderr.exp peak.vgtest \
        peak2.post.exp peak2.stderr.exp peak2.vgtest \
        realloc.post.exp realloc.stderr.exp realloc.vgtest \
@@ -57,6 +58,7 @@ check_PROGRAMS = \
        null \
        one \
        overloaded-new \
+       pages_as_heap \
        peak \
        realloc \
        thresholds \

diff --git a/massif/tests/pages_as_heap.c b/massif/tests/pages_as_heap.c
new file mode 100644 (file)
index 0000000..5ae994b
--- /dev/null
+++ b/massif/tests/pages_as_heap.c
@@ -0,0 +1,33 @@
+#include <stdio.h>
+#include <unistd.h>
+
+#define MAX 20000
+
+int main () {
+  char* ptr;
+  int i;
+  int inc_dec;
+  int delta;
+  int brk_stat;
+
+  // loop to first increase, then decrease
+  for (inc_dec = 1; inc_dec >= -1; inc_dec-=2) {
+     // loop to increase(decrease) with small then big delta
+     for (delta = 1; delta <= 400; delta+=399) {
+        if (0) printf("initial brk value for inc_dec %d delta %d: %p\n",
+               inc_dec, delta, sbrk(0));
+        for (i=0; i<MAX; i++) {
+           brk_stat = brk(sbrk(0) + inc_dec * delta);
+           if (brk_stat == -1) {
+              printf("brk value at failure: %p\n", sbrk(0));
+              perror ("brk() failed!\n");
+              return 0;
+           }
+        }
+        if (0) printf("resulting brk value for inc_dec %d delta %d: %p\n",
+               inc_dec, delta, sbrk(0));
+     }
+  }
+
+  return 0;
+} 

diff --git a/massif/tests/pages_as_heap.stderr.exp b/massif/tests/pages_as_heap.stderr.exp
new file mode 100644 (file)
index 0000000..139597f
--- /dev/null
+++ b/massif/tests/pages_as_heap.stderr.exp
@@ -0,0 +1,2 @@
+
+

diff --git a/massif/tests/pages_as_heap.vgtest b/massif/tests/pages_as_heap.vgtest
new file mode 100644 (file)
index 0000000..7834460
--- /dev/null
+++ b/massif/tests/pages_as_heap.vgtest
@@ -0,0 +1,6 @@
+prog: pages_as_heap
+vgopts: --stacks=no --time-unit=B --heap-admin=0 --pages-as-heap=yes --massif-out-file=massif.out --detailed-freq=3
+vgopts: --ignore-fn=mmap
+# would be nice to test that pages as heap works properly using
+# post: perl ../../massif/ms_print massif.out | ../../tests/filter_addresses
+cleanup: rm massif.out