diff --git a/libc/Android.mk b/libc/Android.mk index 4f6afe8e2..2ea7f1fdb 100644 --- a/libc/Android.mk +++ b/libc/Android.mk @@ -171,7 +171,6 @@ libc_bionic_src_files := \ bionic/recv.cpp \ bionic/rename.cpp \ bionic/rmdir.cpp \ - bionic/sbrk.cpp \ bionic/scandir.cpp \ bionic/sched_getaffinity.cpp \ bionic/sched_getcpu.cpp \ diff --git a/libc/bionic/brk.cpp b/libc/bionic/brk.cpp index 633b914c4..4d08a6148 100644 --- a/libc/bionic/brk.cpp +++ b/libc/bionic/brk.cpp @@ -26,16 +26,49 @@ * SUCH DAMAGE. */ +#include #include -/* Shared with sbrk.c. */ -extern "C" void* __bionic_brk; // TODO: should be __LIBC_HIDDEN__ but accidentally exported by NDK :-( +#if __LP64__ +static void* __bionic_brk; +#else +void* __bionic_brk; // Accidentally exported by the NDK. +#endif int brk(void* end_data) { - void* new_brk = __brk(end_data); - if (new_brk != end_data) { + __bionic_brk = __brk(end_data); + if (__bionic_brk < end_data) { + errno = ENOMEM; return -1; } - __bionic_brk = new_brk; return 0; } + +void* sbrk(ptrdiff_t increment) { + // Initialize __bionic_brk if necessary. + if (__bionic_brk == NULL) { + __bionic_brk = __brk(NULL); + } + + // Don't ask the kernel if we already know the answer. + if (increment == 0) { + return __bionic_brk; + } + + // Avoid overflow. + intptr_t old_brk = reinterpret_cast(__bionic_brk); + if ((increment > 0 && INTPTR_MAX - increment > old_brk) || + (increment < 0 && (increment == PTRDIFF_MIN || old_brk < -increment))) { + errno = ENOMEM; + return reinterpret_cast(-1); + } + + void* desired_brk = reinterpret_cast(old_brk + increment); + __bionic_brk = __brk(desired_brk); + + if (__bionic_brk < desired_brk) { + errno = ENOMEM; + return reinterpret_cast(-1); + } + return reinterpret_cast(old_brk); +} diff --git a/libc/bionic/sbrk.cpp b/libc/bionic/sbrk.cpp deleted file mode 100644 index 6c9b534c3..000000000 --- a/libc/bionic/sbrk.cpp +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright (C) 2008 The Android Open Source Project - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in - * the documentation and/or other materials provided with the - * distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS - * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE - * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, - * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS - * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED - * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, - * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT - * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ - -#include -#include - -/* Shared with brk.c. */ -extern "C" { - void* __bionic_brk; // TODO: should be __LIBC_HIDDEN__ but accidentally exported by NDK :-( -} - -void* sbrk(ptrdiff_t increment) { - if (__bionic_brk == NULL) { - __bionic_brk = __brk(NULL); - } - - void* original_brk = __bionic_brk; - void* desired_brk = (void*) ((uintptr_t) original_brk + increment); - - void* new_brk = __brk(desired_brk); - if (new_brk == (void*) -1) { - return new_brk; - } else if (new_brk < desired_brk) { - errno = ENOMEM; - return (void*) -1; - } - - __bionic_brk = new_brk; - return original_brk; -} diff --git a/tests/unistd_test.cpp b/tests/unistd_test.cpp index 70b23bd75..f56b767dc 100644 --- a/tests/unistd_test.cpp +++ b/tests/unistd_test.cpp @@ -18,6 +18,8 @@ #include "ScopedSignalHandler.h" #include "TemporaryFile.h" +#define __STDC_LIMIT_MACROS // For glibc. + #include #include #include @@ -29,14 +31,49 @@ TEST(unistd, sysconf_SC_MONOTONIC_CLOCK) { ASSERT_GT(sysconf(_SC_MONOTONIC_CLOCK), 0); } -TEST(unistd, sbrk) { - void* initial_break = sbrk(0); +static void* get_brk() { + return sbrk(0); +} - void* new_break = reinterpret_cast(reinterpret_cast(initial_break) + 2000); +static void* page_align(uintptr_t addr) { + uintptr_t mask = sysconf(_SC_PAGE_SIZE) - 1; + return reinterpret_cast((addr + mask) & ~mask); +} + +TEST(unistd, brk) { + void* initial_break = get_brk(); + + // The kernel aligns the break to a page. + void* new_break = reinterpret_cast(reinterpret_cast(initial_break) + 1); ASSERT_EQ(0, brk(new_break)); + ASSERT_GE(get_brk(), new_break); - void* final_break = sbrk(0); - ASSERT_EQ(final_break, new_break); + new_break = page_align(reinterpret_cast(initial_break) + sysconf(_SC_PAGE_SIZE)); + ASSERT_EQ(0, brk(new_break)); + ASSERT_EQ(get_brk(), new_break); +} + +TEST(unistd, brk_ENOMEM) { + ASSERT_EQ(-1, brk(reinterpret_cast(-1))); + ASSERT_EQ(ENOMEM, errno); +} + +TEST(unistd, sbrk_ENOMEM) { + intptr_t current_brk = reinterpret_cast(get_brk()); + + // Can't increase by so much that we'd overflow. + ASSERT_EQ(reinterpret_cast(-1), sbrk(PTRDIFF_MAX)); + ASSERT_EQ(ENOMEM, errno); + + // Can't reduce by more than the current break. + ASSERT_EQ(reinterpret_cast(-1), sbrk(-(current_brk + 1))); + ASSERT_EQ(ENOMEM, errno); + +#if !defined(__GLIBC__) + // The maximum negative value is an interesting special case that glibc gets wrong. + ASSERT_EQ(reinterpret_cast(-1), sbrk(PTRDIFF_MIN)); + ASSERT_EQ(ENOMEM, errno); +#endif } TEST(unistd, truncate) {