From 7b2a49544e3789c8f4ea03271fb1d13d44de1770 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 4 Sep 2003 20:57:51 +0000 Subject: [PATCH] Fixed brk(); it was almost completely broken. Added a regression test for it. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@1810 --- coregrind/vg_syscalls.c | 49 +++++++++++++++++++++-------------- memcheck/tests/Makefile.am | 6 +++-- memcheck/tests/brk.c | 40 ++++++++++++++++++++++++++++ memcheck/tests/brk.stderr.exp | 7 +++++ memcheck/tests/brk.vgtest | 1 + 5 files changed, 81 insertions(+), 22 deletions(-) create mode 100644 memcheck/tests/brk.c create mode 100644 memcheck/tests/brk.stderr.exp create mode 100644 memcheck/tests/brk.vgtest diff --git a/coregrind/vg_syscalls.c b/coregrind/vg_syscalls.c index 3fd6090d1..01b8f08ab 100644 --- 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 4f9dc16aa..8b562eca2 100644 --- 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 index 000000000..58c42604a --- /dev/null +++ b/memcheck/tests/brk.c @@ -0,0 +1,40 @@ +#include +#include +#include +#include +#include + +// 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 000000000..c4aa6f04f --- /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 000000000..b1766ffc3 --- /dev/null +++ b/memcheck/tests/brk.vgtest @@ -0,0 +1 @@ +prog: brk