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.

diff --git a/lib/isc/mem.c b/lib/isc/mem.c
index c368959fa7e2056c64c8de7625f8d02f38ea1c05..67235bf41fe7d723c7b8c15410089a24078bf254 100644 (file)
--- a/lib/isc/mem.c
+++ b/lib/isc/mem.c
@@ -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);
 }

diff --git a/lib/isc/tests/mem_test.c b/lib/isc/tests/mem_test.c
index d5b46f2a1b03cb540e6f39146ab81ab71da12e81..9e35af3fc7808d23cffece1c5e93a7670272b06b 100644 (file)
--- a/lib/isc/tests/mem_test.c
+++ b/lib/isc/tests/mem_test.c
@@ -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),