Return non-NULL pointer on zero-sized allocations and reallocations

Previously, the zero-sized allocations would return NULL pointer and the
caller had to make sure to not dereference such pointer.  The C standard
defines the zero-sized calls to malloc() as implementation specific and
jemalloc mallocx() with zero size would be undefined behaviour.  This
complicated the code as it had to handle such cases in a special manner
in all allocator and deallocator functions.

Now, for realloc(), the situation is even more complicated.  In C
standard up to C11, the behavior would be implementation defined, and
actually some implementation would free to orig ptr and some would not.
Since C17 (via DR400) would deprecate such usage and since C23, the
behaviour would be undefined.

This commits changes helper mem_get(), mem_put() and mem_realloc()
functions to grow the zero-allocation from 0 to sizeof(void *).

This way we get a predicable behaviour that all the allocations will
always return valid pointer.
This commit is contained in:
Ondřej Surý
2021-09-23 21:19:25 +02:00
parent aeb3d1cab3
commit 4cdb3abf27
2 changed files with 55 additions and 27 deletions

View File

@@ -70,11 +70,12 @@ unsigned int isc_mem_defaultflags = ISC_MEMFLAG_DEFAULT;
* Constants.
*/
#define ALIGNMENT 8U /*%< must be a power of 2 */
#define ALIGNMENT_SIZE sizeof(size_info)
#define DEBUG_TABLE_COUNT 512U
#define STATS_BUCKETS 512U
#define STATS_BUCKET_SIZE 32U
#define ZERO_ALLOCATION_SIZE sizeof(void *)
#define ALIGNMENT 8U /*%< must be a power of 2 */
#define ALIGNMENT_SIZE sizeof(size_info)
#define DEBUG_TABLE_COUNT 512U
#define STATS_BUCKETS 512U
#define STATS_BUCKET_SIZE 32U
/*
* Types.
@@ -323,12 +324,21 @@ unlock:
}
#endif /* ISC_MEM_TRACKLINES */
#define ADJUST_ZERO_ALLOCATION_SIZE(s) \
if (ISC_UNLIKELY(s == 0)) { \
s = ZERO_ALLOCATION_SIZE; \
}
/*!
* Perform a malloc, doing memory filling and overrun detection as necessary.
*/
static inline void *
mem_get(isc_mem_t *ctx, size_t size) {
char *ret = mallocx(size, 0);
char *ret = NULL;
ADJUST_ZERO_ALLOCATION_SIZE(size);
ret = mallocx(size, 0);
if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) {
memset(ret, 0xbe, size); /* Mnemonic for "beef". */
@@ -343,6 +353,8 @@ mem_get(isc_mem_t *ctx, size_t size) {
/* coverity[+free : arg-1] */
static inline void
mem_put(isc_mem_t *ctx, void *mem, size_t size) {
ADJUST_ZERO_ALLOCATION_SIZE(size);
if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) {
memset(mem, 0xde, size); /* Mnemonic for "dead". */
}
@@ -353,6 +365,8 @@ static inline void *
mem_realloc(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size) {
void *new_ptr = NULL;
ADJUST_ZERO_ALLOCATION_SIZE(new_size);
new_ptr = rallocx(old_ptr, new_size, 0);
if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) {
@@ -607,7 +621,7 @@ isc__mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG) {
REQUIRE(ctxp != NULL && VALID_CONTEXT(*ctxp));
REQUIRE(ptr != NULL);
REQUIRE(size > 0);
REQUIRE(size != 0);
ctx = *ctxp;
*ctxp = NULL;
@@ -735,9 +749,7 @@ isc__mem_get(isc_mem_t *ctx, size_t size FLARG) {
REQUIRE(VALID_CONTEXT(ctx));
if (ISC_LIKELY(size != 0)) {
ptr = mem_get(ctx, size);
}
ptr = mem_get(ctx, size);
mem_getstats(ctx, size);
ADD_TRACE(ctx, ptr, size, file, line);
@@ -750,15 +762,11 @@ isc__mem_get(isc_mem_t *ctx, size_t size FLARG) {
void
isc__mem_put(isc_mem_t *ctx, void *ptr, size_t size FLARG) {
REQUIRE(VALID_CONTEXT(ctx));
REQUIRE(ISC_LIKELY(ptr != NULL && size != 0) ||
ISC_UNLIKELY(ptr == NULL && size == 0));
DELETE_TRACE(ctx, ptr, size, file, line);
mem_putstats(ctx, ptr, size);
if (ISC_LIKELY(ptr != NULL)) {
mem_put(ctx, ptr, size);
}
mem_put(ctx, ptr, size);
CALL_LO_WATER(ctx);
}
@@ -875,12 +883,10 @@ isc__mem_allocate(isc_mem_t *ctx, size_t size FLARG) {
REQUIRE(VALID_CONTEXT(ctx));
if (ISC_LIKELY(size != 0)) {
ptr = mem_get(ctx, size);
ptr = mem_get(ctx, size);
/* Recalculate the real allocated size */
size = sallocx(ptr, 0);
}
/* Recalculate the real allocated size */
size = sallocx(ptr, 0);
mem_getstats(ctx, size);
ADD_TRACE(ctx, ptr, size, file, line);
@@ -963,16 +969,12 @@ isc__mem_free(isc_mem_t *ctx, void *ptr FLARG) {
REQUIRE(VALID_CONTEXT(ctx));
if (ISC_LIKELY(ptr != NULL)) {
size = sallocx(ptr, 0);
}
size = sallocx(ptr, 0);
DELETE_TRACE(ctx, ptr, size, file, line);
mem_putstats(ctx, ptr, size);
if (ISC_LIKELY(ptr != NULL)) {
mem_put(ctx, ptr, size);
}
mem_put(ctx, ptr, size);
CALL_LO_WATER(ctx);
}

View File

@@ -220,21 +220,44 @@ isc_mem_inuse_test(void **state) {
isc_mem_destroy(&mctx2);
}
static void
isc_mem_zeroget_test(void **state) {
uint8_t *data = NULL;
UNUSED(state);
data = isc_mem_get(test_mctx, 0);
assert_non_null(data);
isc_mem_put(test_mctx, data, 0);
}
#define REGET_INIT_SIZE 1024
#define REGET_GROW_SIZE 2048
#define REGET_SHRINK_SIZE 512
static void
isc_mem_reget_test(void **state) {
uint8_t *data = isc_mem_get(test_mctx, REGET_INIT_SIZE);
uint8_t *data = NULL;
UNUSED(state);
/* test that we can reget NULL */
data = isc_mem_reget(test_mctx, NULL, 0, REGET_INIT_SIZE);
assert_non_null(data);
isc_mem_put(test_mctx, data, REGET_INIT_SIZE);
/* test that we can re-get a zero-length allocation */
data = isc_mem_get(test_mctx, 0);
assert_non_null(data);
data = isc_mem_reget(test_mctx, data, 0, REGET_INIT_SIZE);
assert_non_null(data);
for (size_t i = 0; i < REGET_INIT_SIZE; i++) {
data[i] = i % UINT8_MAX;
}
data = isc_mem_reget(test_mctx, data, REGET_INIT_SIZE, REGET_GROW_SIZE);
assert_non_null(data);
for (size_t i = 0; i < REGET_INIT_SIZE; i++) {
assert_int_equal(data[i], i % UINT8_MAX);
@@ -246,6 +269,7 @@ isc_mem_reget_test(void **state) {
data = isc_mem_reget(test_mctx, data, REGET_GROW_SIZE,
REGET_SHRINK_SIZE);
assert_non_null(data);
for (size_t i = REGET_SHRINK_SIZE; i > 0; i--) {
assert_int_equal(data[i - 1], i % UINT8_MAX);
@@ -470,6 +494,8 @@ main(void) {
_teardown),
cmocka_unit_test_setup_teardown(isc_mem_inuse_test, _setup,
_teardown),
cmocka_unit_test_setup_teardown(isc_mem_zeroget_test, _setup,
_teardown),
cmocka_unit_test_setup_teardown(isc_mem_reget_test, _setup,
_teardown),