Fixed brk(); it was almost completely broken.
Added a regression test for it.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@1810 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/coregrind/vg_syscalls.c b/coregrind/vg_syscalls.c
index 3fd6090..01b8f08 100644
--- a/coregrind/vg_syscalls.c
+++ b/coregrind/vg_syscalls.c
@@ -1327,33 +1327,42 @@
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 4f9dc16..8b562ec 100644
--- a/memcheck/tests/Makefile.am
+++ b/memcheck/tests/Makefile.am
@@ -14,6 +14,7 @@
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 @@
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 @@
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
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
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
index 0000000..b1766ff
--- /dev/null
+++ b/memcheck/tests/brk.vgtest
@@ -0,0 +1 @@
+prog: brk