Fixed brk(); it was almost completely broken.

Added a regression test for it.

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

diff --git a/coregrind/vg_syscalls.c b/coregrind/vg_syscalls.c
index 3fd6090d1ec66bf1f275a06bf99f440e4368a880..01b8f08ab194075c5667278a239b85c0e44a7d5b 100644 (file)
--- a/coregrind/vg_syscalls.c
+++ b/coregrind/vg_syscalls.c
@@ -1327,33 +1327,42 @@ void VG_(perform_assumed_nonblocking_syscall) ( ThreadId tid )
          break;
 
       case __NR_brk: /* syscall 45 */
-         /* Haven't a clue if this is really right. */
-         /* int brk(void *end_data_segment); */
+         /* libc   says: int   brk(void *end_data_segment);
+            kernel says: void* brk(void* end_data_segment);  (more or less)
+
+            libc returns 0 on success, and -1 (and sets errno) on failure.
+            Nb: if you ask to shrink the dataseg end below what it
+            currently is, that always succeeds, even if the dataseg end
+            doesn't actually change (eg. brk(0)).  Unless it seg faults.
+
+            Kernel returns the new dataseg end.  If the brk() failed, this
+            will be unchanged from the old one.  That's why calling (kernel)
+            brk(0) gives the current dataseg end (libc brk() just returns
+            zero in that case).
+
+            Both will seg fault if you shrink it back into a text segment.
+         */
          MAYBE_PRINTF("brk ( %p ) --> ",arg1);
          KERNEL_DO_SYSCALL(tid,res);
          MAYBE_PRINTF("0x%x\n", res);
 
-         if (!VG_(is_kerror)(res)) {
-            if (arg1 == 0) {
-               /* Just asking where the current end is. (???) */
-               curr_dataseg_end = res;
-            } else
-            if (arg1 < curr_dataseg_end) {
-               /* shrinking the data segment. */
-               VG_TRACK( die_mem_brk, (Addr)arg1, 
+         if (res == arg1) {
+            /* brk() succeeded */
+            if (res < curr_dataseg_end) {
+               /* successfully shrunk the data segment. */
+               VG_TRACK( die_mem_brk, (Addr)arg1,
                                       curr_dataseg_end-arg1 );
-               curr_dataseg_end = arg1;
             } else
-            if (arg1 > curr_dataseg_end && res != 0) {
-               /* asked for more memory, and got it */
-               /* 
-               VG_(printf)("BRK: new area %x .. %x\n", 
-                           VG_(curr_dataseg_end, arg1-1 );
-               */
-               VG_TRACK( new_mem_brk, (Addr)curr_dataseg_end, 
-                                         arg1-curr_dataseg_end );
-               curr_dataseg_end = arg1;         
+            if (res > curr_dataseg_end && res != 0) {
+               /* successfully grew the data segment */
+               VG_TRACK( new_mem_brk, curr_dataseg_end,
+                                      arg1-curr_dataseg_end );
             }
+            curr_dataseg_end = res;
+
+         } else {
+            /* brk() failed */
+            vg_assert(curr_dataseg_end == res);
          }
          break;
 

diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am
index 4f9dc16aa1ba0d8f43d7d9df7b577b12b4cb278e..8b562eca21dbaa4e34367154bb8bebebac8bccf4 100644 (file)
--- a/memcheck/tests/Makefile.am
+++ b/memcheck/tests/Makefile.am
@@ -14,6 +14,7 @@ EXTRA_DIST = $(noinst_SCRIPTS) \
        badfree.stderr.exp badfree.vgtest \
        badjump.stderr.exp badjump.vgtest \
        badloop.stderr.exp badloop.vgtest \
+       brk.stderr.exp brk.vgtest \
        buflen_check.stderr.exp buflen_check.vgtest \
        clientperm.stderr.exp \
        clientperm.stdout.exp clientperm.vgtest \
@@ -59,8 +60,8 @@ EXTRA_DIST = $(noinst_SCRIPTS) \
        threadederrno.stderr.exp threadederrno.stdout.exp threadederrno.vgtest
 
 check_PROGRAMS = \
-       badaddrvalue badfree badjump badloop buflen_check clientperm \
-       custom_alloc \
+       badaddrvalue badfree badjump badloop brk buflen_check 
+       clientperm custom_alloc \
        doublefree error_counts errs1 exitprog fprw fwrite inits inline \
        malloc1 malloc2 malloc3 manuel1 manuel2 manuel3 \
        memalign_test memcmptest mmaptest nanoleak null_socket \
@@ -78,6 +79,7 @@ badaddrvalue_SOURCES  = badaddrvalue.c
 badfree_SOURCES        = badfree.c
 badjump_SOURCES        = badjump.c
 badloop_SOURCES        = badloop.c
+brk_SOURCES            = brk.c
 buflen_check_SOURCES   = buflen_check.c
 clientperm_SOURCES     = clientperm.c
 custom_alloc_SOURCES   = custom_alloc.c

diff --git a/memcheck/tests/brk.c b/memcheck/tests/brk.c
new file mode 100644 (file)
index 0000000..58c4260
--- /dev/null
+++ b/memcheck/tests/brk.c
@@ -0,0 +1,40 @@
+#include <assert.h>
+#include <stdio.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+// kernel brk() and libc brk() act quite differently...
+
+int main(void)
+{
+   int i;
+   void* orig_ds = sbrk(0);
+   void* ds = orig_ds;
+   void* vals[10];
+   void* res;
+
+   vals[0] = (void*)0;
+   vals[1] = (void*)1;
+   vals[2] = ds - 0x1;          // small shrink
+   vals[3] = ds;
+   vals[4] = ds + 0x1000;       // small growth
+   vals[5] = ds + 0x40000000;   // too-big growth
+   vals[6] = ds + 0x500;        // shrink a little, but still above start size
+   vals[7] = ds - 0x1;          // shrink below start size
+//   vals[8] = ds - 0x1000;       // shrink a lot below start size (into text)
+//   vals[9] = (void*)0xffffffff;
+   vals[8] = (void*)0xffffffff;
+
+   for (i = 0; (void*)0xffffffff != vals[i]; i++) {
+      res = (void*)syscall(__NR_brk, vals[i]);
+   }
+
+   assert( 0 == brk(orig_ds) );  // libc brk()
+
+   for (i = 0; (void*)0xffffffff != vals[i]; i++) {
+      res = (void*)brk(vals[i]);
+   }
+
+   return 0;
+}

diff --git a/memcheck/tests/brk.stderr.exp b/memcheck/tests/brk.stderr.exp
new file mode 100644 (file)
index 0000000..c4aa6f0
--- /dev/null
+++ b/memcheck/tests/brk.stderr.exp
@@ -0,0 +1,7 @@
+
+
+ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
+malloc/free: in use at exit: 0 bytes in 0 blocks.
+malloc/free: 0 allocs, 0 frees, 0 bytes allocated.
+For a detailed leak analysis,  rerun with: --leak-check=yes
+For counts of detected errors, rerun with: -v

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