Skip to content

Commit

Permalink
Errno fix (#463)
Browse files Browse the repository at this point in the history
Correctly set errno on failure and improve the related test.

Previously the malloc test would emit an error message but not
abort if the errno was not as expected on failure. This
was because the return in the null == true case prevented the
check for failed == true at the end of check_result from
being reached. To resolve this just abort immediately as in the 
null case.

Also add tests of allocations that are expected to fail for
calloc and malloc.

To make the tests pass we need to set errno in several places,
making sure to keep this off the fast path.

We must also take care not to attempt to zero nullptr in case
of calloc failure.

See #461 and #463.
  • Loading branch information
rmn30 authored Feb 24, 2022
1 parent af8ab2d commit 86aa286
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 11 deletions.
1 change: 1 addition & 0 deletions src/backend/address_space_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ namespace snmalloc
if (align_bits == (bits::BITS - 1))
{
// Out of memory
errno = ENOMEM;
return nullptr;
}

Expand Down
1 change: 1 addition & 0 deletions src/mem/chunkallocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ namespace snmalloc
if (slab_sizeclass >= NUM_SLAB_SIZES)
{
// Your address space is not big enough for this allocation!
errno = ENOMEM;
return {nullptr, nullptr};
}

Expand Down
7 changes: 6 additions & 1 deletion src/mem/external_alloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,20 @@ namespace snmalloc::external_alloc
if constexpr (bits::BITS == 64)
{
if (size >= 0x10000000000)
{
errno = ENOMEM;
return nullptr;
}
}

if (alignment < sizeof(void*))
alignment = sizeof(void*);

void* result;
if (posix_memalign(&result, alignment, size) != 0)
int err = posix_memalign(&result, alignment, size);
if (err != 0)
{
errno = err;
result = nullptr;
}
return result;
Expand Down
4 changes: 2 additions & 2 deletions src/mem/localalloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ namespace snmalloc
if (meta != nullptr)
meta->initialise_large();

if (zero_mem == YesZero)
if (zero_mem == YesZero && chunk.unsafe_ptr() != nullptr)
{
SharedStateHandle::Pal::template zero<false>(
chunk.unsafe_ptr(), size);
Expand Down Expand Up @@ -427,7 +427,7 @@ namespace snmalloc
// would guarantee.
void* result = external_alloc::aligned_alloc(
natural_alignment(size), round_size(size));
if constexpr (zero_mem == YesZero)
if (zero_mem == YesZero && result != nullptr)
memset(result, 0, size);
return result;
#else
Expand Down
4 changes: 4 additions & 0 deletions src/override/malloc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ extern "C"
{
a.dealloc(ptr);
}
else
{
errno = ENOMEM;
}
return p;
}

Expand Down
7 changes: 6 additions & 1 deletion src/pal/pal_windows.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,18 @@ namespace snmalloc

void* ret = VirtualAlloc2FromApp(
nullptr, nullptr, size, flags, PAGE_READWRITE, &param, 1);
if (ret == nullptr)
errno = ENOMEM;
return ret;
}
# endif

static void* reserve(size_t size) noexcept
{
return VirtualAlloc(nullptr, size, MEM_RESERVE, PAGE_READWRITE);
void* ret = VirtualAlloc(nullptr, size, MEM_RESERVE, PAGE_READWRITE);
if (ret == nullptr)
errno = ENOMEM;
return ret;
}

/**
Expand Down
23 changes: 16 additions & 7 deletions src/test/func/malloc/malloc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ void check_result(size_t size, size_t align, void* p, int err, bool null)
bool failed = false;
if (errno != err && err != SUCCESS)
{
// Note: successful calls are allowed to spuriously set errno
printf("Expected error: %d but got %d\n", err, errno);
failed = true;
abort();
}

if (null)
Expand Down Expand Up @@ -238,6 +239,11 @@ int main(int argc, char** argv)

our_free(nullptr);

/* A very large allocation size that we expect to fail. */
const size_t too_big_size = ((size_t)-1) / 2;
check_result(too_big_size, 1, our_malloc(too_big_size), ENOMEM, true);
errno = SUCCESS;

for (smallsizeclass_t sc = 0; sc < (MAX_SMALL_SIZECLASS_BITS + 4); sc++)
{
const size_t size = bits::one_at_bit(sc);
Expand All @@ -252,6 +258,9 @@ int main(int argc, char** argv)

our_free(nullptr);

test_calloc(1, too_big_size, ENOMEM, true);
errno = SUCCESS;

for (smallsizeclass_t sc = 0; sc < NUM_SMALL_SIZECLASSES; sc++)
{
const size_t size = sizeclass_to_size(sc);
Expand All @@ -275,7 +284,7 @@ int main(int argc, char** argv)
const size_t size = sizeclass_to_size(sc);
test_realloc(our_malloc(size), size, SUCCESS, false);
test_realloc(nullptr, size, SUCCESS, false);
test_realloc(our_malloc(size), ((size_t)-1) / 2, ENOMEM, true);
test_realloc(our_malloc(size), too_big_size, ENOMEM, true);
for (smallsizeclass_t sc2 = 0; sc2 < NUM_SMALL_SIZECLASSES; sc2++)
{
const size_t size2 = sizeclass_to_size(sc2);
Expand All @@ -289,7 +298,7 @@ int main(int argc, char** argv)
const size_t size = bits::one_at_bit(sc);
test_realloc(our_malloc(size), size, SUCCESS, false);
test_realloc(nullptr, size, SUCCESS, false);
test_realloc(our_malloc(size), ((size_t)-1) / 2, ENOMEM, true);
test_realloc(our_malloc(size), too_big_size, ENOMEM, true);
for (smallsizeclass_t sc2 = 0; sc2 < (MAX_SMALL_SIZECLASS_BITS + 4); sc2++)
{
const size_t size2 = bits::one_at_bit(sc2);
Expand All @@ -302,7 +311,7 @@ int main(int argc, char** argv)
test_realloc(our_malloc(64), 4194304, SUCCESS, false);

test_posix_memalign(0, 0, EINVAL, true);
test_posix_memalign(((size_t)-1) / 2, 0, EINVAL, true);
test_posix_memalign(too_big_size, 0, EINVAL, true);
test_posix_memalign(OS_PAGE_SIZE, sizeof(uintptr_t) / 2, EINVAL, true);

for (size_t align = sizeof(uintptr_t); align < MAX_SMALL_SIZECLASS_SIZE * 8;
Expand All @@ -316,7 +325,7 @@ int main(int argc, char** argv)
test_memalign(size, align, SUCCESS, false);
}
test_posix_memalign(0, align, SUCCESS, false);
test_posix_memalign(((size_t)-1) / 2, align, ENOMEM, true);
test_posix_memalign(too_big_size, align, ENOMEM, true);
test_posix_memalign(0, align + 1, EINVAL, true);
}

Expand All @@ -327,7 +336,7 @@ int main(int argc, char** argv)
test_reallocarray(our_malloc(size), 1, size, SUCCESS, false);
test_reallocarray(our_malloc(size), 1, 0, SUCCESS, false);
test_reallocarray(nullptr, 1, size, SUCCESS, false);
test_reallocarray(our_malloc(size), 1, ((size_t)-1) / 2, ENOMEM, true);
test_reallocarray(our_malloc(size), 1, too_big_size, ENOMEM, true);
for (smallsizeclass_t sc2 = 0; sc2 < (MAX_SMALL_SIZECLASS_BITS + 4); sc2++)
{
const size_t size2 = bits::one_at_bit(sc2);
Expand All @@ -351,7 +360,7 @@ int main(int argc, char** argv)
printf("realloc alloc failed with %zu\n", size);
abort();
}
int r = our_reallocarr(&p, 1, ((size_t)-1) / 2);
int r = our_reallocarr(&p, 1, too_big_size);
if (r != ENOMEM)
{
printf("expected failure on allocation\n");
Expand Down

0 comments on commit 86aa286

Please sign in to comment.