diff --git a/libc/bionic/brk.cpp b/libc/bionic/brk.cpp index 4d08a6148..4e461931d 100644 --- a/libc/bionic/brk.cpp +++ b/libc/bionic/brk.cpp @@ -56,19 +56,19 @@ void* sbrk(ptrdiff_t increment) { } // 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))) { + uintptr_t old_brk = reinterpret_cast(__bionic_brk); + if ((increment > 0 && static_cast(increment) > (UINTPTR_MAX - old_brk)) || + (increment < 0 && static_cast(-increment) > old_brk)) { 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/tests/unistd_test.cpp b/tests/unistd_test.cpp index 3f3186128..6aa259afb 100644 --- a/tests/unistd_test.cpp +++ b/tests/unistd_test.cpp @@ -56,21 +56,82 @@ TEST(unistd, brk_ENOMEM) { ASSERT_EQ(ENOMEM, errno); } +#if defined(__GLIBC__) +#define SBRK_MIN INTPTR_MIN +#define SBRK_MAX INTPTR_MAX +#else +#define SBRK_MIN PTRDIFF_MIN +#define SBRK_MAX PTRDIFF_MAX +#endif + TEST(unistd, sbrk_ENOMEM) { - intptr_t current_brk = reinterpret_cast(get_brk()); +#if defined(__BIONIC__) && !defined(__LP64__) + // There is no way to guarantee that all overflow conditions can be tested + // without manipulating the underlying values of the current break. + extern void* __bionic_brk; + + class ScopedBrk { + public: + ScopedBrk() : saved_brk_(__bionic_brk) {} + virtual ~ScopedBrk() { __bionic_brk = saved_brk_; } + + private: + void* saved_brk_; + }; + + ScopedBrk scope_brk; + + // Set the current break to a point that will cause an overflow. + __bionic_brk = reinterpret_cast(static_cast(PTRDIFF_MAX) + 2); // 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); + // Set the current break to a point that will cause an overflow. + __bionic_brk = reinterpret_cast(static_cast(PTRDIFF_MAX)); -#if defined(__BIONIC__) - // 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); + + __bionic_brk = reinterpret_cast(static_cast(PTRDIFF_MAX) - 1); + + ASSERT_EQ(reinterpret_cast(-1), sbrk(PTRDIFF_MIN + 1)); + ASSERT_EQ(ENOMEM, errno); +#else + class ScopedBrk { + public: + ScopedBrk() : saved_brk_(get_brk()) {} + virtual ~ScopedBrk() { brk(saved_brk_); } + + private: + void* saved_brk_; + }; + + ScopedBrk scope_brk; + + uintptr_t cur_brk = reinterpret_cast(get_brk()); + if (cur_brk < static_cast(-(SBRK_MIN+1))) { + // Do the overflow test for a max negative increment. + ASSERT_EQ(reinterpret_cast(-1), sbrk(SBRK_MIN)); +#if defined(__BIONIC__) + // GLIBC does not set errno in overflow case. + ASSERT_EQ(ENOMEM, errno); +#endif + } + + uintptr_t overflow_brk = static_cast(SBRK_MAX) + 2; + if (cur_brk < overflow_brk) { + // Try and move the value to PTRDIFF_MAX + 2. + cur_brk = reinterpret_cast(sbrk(overflow_brk)); + } + if (cur_brk >= overflow_brk) { + ASSERT_EQ(reinterpret_cast(-1), sbrk(SBRK_MAX)); +#if defined(__BIONIC__) + // GLIBC does not set errno in overflow case. + ASSERT_EQ(ENOMEM, errno); +#endif + } #endif }