diff --git a/CHANGES b/CHANGES index f527025ed9..a777a0500e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5721. [func] New isc_mem_reget() realloc-like function was + introduced into the libisc API, and zero-sized + allocations now return non-NULL pointers. [GL !5440] + 5720. [contrib] Remove old-style DLZ drivers that had to be enabled during compile time. [GL #2814] diff --git a/lib/isc/buffer.c b/lib/isc/buffer.c index 1db0eb026e..05bf4143b4 100644 --- a/lib/isc/buffer.c +++ b/lib/isc/buffer.c @@ -553,8 +553,7 @@ isc_buffer_allocate(isc_mem_t *mctx, isc_buffer_t **dynbuffer, isc_result_t isc_buffer_reserve(isc_buffer_t **dynbuffer, unsigned int size) { - unsigned char *bdata; - uint64_t len; + size_t len; REQUIRE(dynbuffer != NULL); REQUIRE(ISC_BUFFER_VALID(*dynbuffer)); @@ -581,18 +580,9 @@ isc_buffer_reserve(isc_buffer_t **dynbuffer, unsigned int size) { return (ISC_R_NOMEMORY); } - /* - * XXXMUKS: This is far more expensive than plain realloc() as - * it doesn't remap pages, but does ordinary copy. So is - * isc_mem_reallocate(), which has additional issues. - */ - bdata = isc_mem_get((*dynbuffer)->mctx, (unsigned int)len); - - memmove(bdata, (*dynbuffer)->base, (*dynbuffer)->length); - isc_mem_put((*dynbuffer)->mctx, (*dynbuffer)->base, - (*dynbuffer)->length); - - (*dynbuffer)->base = bdata; + (*dynbuffer)->base = isc_mem_reget((*dynbuffer)->mctx, + (*dynbuffer)->base, + (*dynbuffer)->length, len); (*dynbuffer)->length = (unsigned int)len; return (ISC_R_SUCCESS); diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 0e0d24a3b1..a513890cc7 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -134,7 +134,9 @@ extern unsigned int isc_mem_defaultflags; #define ISCMEMFUNC(sfx) isc__mem_##sfx #define ISCMEMPOOLFUNC(sfx) isc__mempool_##sfx -#define isc_mem_get(c, s) ISCMEMFUNC(get)((c), (s)_ISC_MEM_FILELINE) +#define isc_mem_get(c, s) ISCMEMFUNC(get)((c), (s)_ISC_MEM_FILELINE) +#define isc_mem_reget(c, p, o, n) \ + ISCMEMFUNC(reget)((c), (p), (o), (n)_ISC_MEM_FILELINE) #define isc_mem_allocate(c, s) ISCMEMFUNC(allocate)((c), (s)_ISC_MEM_FILELINE) #define isc_mem_reallocate(c, p, s) \ ISCMEMFUNC(reallocate)((c), (p), (s)_ISC_MEM_FILELINE) @@ -483,6 +485,9 @@ void ISCMEMFUNC(free)(isc_mem_t *, void *_ISC_MEM_FLARG); ISC_ATTR_MALLOC_DEALLOCATOR_IDX(ISCMEMFUNC(put), 2) void *ISCMEMFUNC(get)(isc_mem_t *, size_t _ISC_MEM_FLARG); +ISC_ATTR_DEALLOCATOR_IDX(ISCMEMFUNC(put), 2) +void *ISCMEMFUNC(reget)(isc_mem_t *, void *, size_t, size_t _ISC_MEM_FLARG); + ISC_ATTR_MALLOC_DEALLOCATOR_IDX(ISCMEMFUNC(free), 2) void *ISCMEMFUNC(allocate)(isc_mem_t *, size_t _ISC_MEM_FLARG); diff --git a/lib/isc/mem.c b/lib/isc/mem.c index e598afce8b..67235bf41f 100644 --- 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,12 +353,34 @@ 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". */ } sdallocx(mem, size, 0); } +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)) { + ssize_t diff_size = new_size - old_size; + void *diff_ptr = (uint8_t *)new_ptr + old_size; + if (diff_size > 0) { + /* Mnemonic for "beef". */ + memset(diff_ptr, 0xbe, diff_size); + } + } + + return (new_ptr); +} + #define stats_bucket(ctx, size) \ ((size / STATS_BUCKET_SIZE) >= STATS_BUCKETS \ ? &ctx->stats[STATS_BUCKETS] \ @@ -589,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; @@ -717,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); @@ -732,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); } @@ -857,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); @@ -872,6 +896,37 @@ isc__mem_allocate(isc_mem_t *ctx, size_t size FLARG) { return (ptr); } +void * +isc__mem_reget(isc_mem_t *ctx, void *old_ptr, size_t old_size, + size_t new_size FLARG) { + void *new_ptr = NULL; + + if (ISC_UNLIKELY(old_ptr == NULL)) { + REQUIRE(old_size == 0); + new_ptr = isc__mem_get(ctx, new_size FLARG_PASS); + } else if (ISC_UNLIKELY(new_size == 0)) { + isc__mem_put(ctx, old_ptr, old_size FLARG_PASS); + } else { + DELETE_TRACE(ctx, old_ptr, old_size, file, line); + mem_putstats(ctx, old_ptr, old_size); + + new_ptr = mem_realloc(ctx, old_ptr, old_size, new_size); + + mem_getstats(ctx, new_size); + ADD_TRACE(ctx, new_ptr, new_size, file, line); + + /* + * We want to postpone the call to water in edge case + * where the realloc will exactly hit on the boundary of + * the water and we would call water twice. + */ + CALL_LO_WATER(ctx); + CALL_HI_WATER(ctx); + } + + return (new_ptr); +} + void * isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size FLARG) { void *new_ptr = NULL; @@ -888,16 +943,7 @@ isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size FLARG) { DELETE_TRACE(ctx, old_ptr, old_size, file, line); mem_putstats(ctx, old_ptr, old_size); - new_ptr = rallocx(old_ptr, new_size, 0); - - if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) { - ssize_t diff_size = new_size - old_size; - void *diff_ptr = (uint8_t *)new_ptr + old_size; - if (diff_size >= 0) { - /* Mnemonic for "beef". */ - memset(diff_ptr, 0xbe, diff_size); - } - } + new_ptr = mem_realloc(ctx, old_ptr, old_size, new_size); /* Recalculate the real allocated size */ new_size = sallocx(new_ptr, 0); @@ -923,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/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index cd051ee517..5b1ff7cdb8 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -1258,8 +1258,10 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree FLARG) { isc_astack_destroy(sock->inactivereqs); sock->magic = 0; - isc_mem_free(sock->mgr->mctx, sock->ah_frees); - isc_mem_free(sock->mgr->mctx, sock->ah_handles); + isc_mem_put(sock->mgr->mctx, sock->ah_frees, + sock->ah_size * sizeof(sock->ah_frees[0])); + isc_mem_put(sock->mgr->mctx, sock->ah_handles, + sock->ah_size * sizeof(sock->ah_handles[0])); isc_mutex_destroy(&sock->lock); isc_condition_destroy(&sock->scond); #if HAVE_LIBNGHTTP2 @@ -1471,9 +1473,9 @@ isc___nmsocket_init(isc_nmsocket_t *sock, isc_nm_t *mgr, isc_nmsocket_type type, isc_nm_attach(mgr, &sock->mgr); sock->uv_handle.handle.data = sock; - sock->ah_frees = isc_mem_allocate( - mgr->mctx, sock->ah_size * sizeof(sock->ah_frees[0])); - sock->ah_handles = isc_mem_allocate( + sock->ah_frees = isc_mem_get(mgr->mctx, + sock->ah_size * sizeof(sock->ah_frees[0])); + sock->ah_handles = isc_mem_get( mgr->mctx, sock->ah_size * sizeof(sock->ah_handles[0])); ISC_LINK_INIT(&sock->quotacb, link); for (size_t i = 0; i < 32; i++) { @@ -1638,12 +1640,14 @@ isc___nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t *peer, LOCK(&sock->lock); /* We need to add this handle to the list of active handles */ if ((size_t)atomic_load(&sock->ah) == sock->ah_size) { - sock->ah_frees = - isc_mem_reallocate(sock->mgr->mctx, sock->ah_frees, - sock->ah_size * 2 * sizeof(size_t)); - sock->ah_handles = isc_mem_reallocate( + sock->ah_frees = isc_mem_reget( + sock->mgr->mctx, sock->ah_frees, + sock->ah_size * sizeof(sock->ah_frees[0]), + sock->ah_size * 2 * sizeof(sock->ah_frees[0])); + sock->ah_handles = isc_mem_reget( sock->mgr->mctx, sock->ah_handles, - sock->ah_size * 2 * sizeof(isc_nmhandle_t *)); + sock->ah_size * sizeof(sock->ah_handles[0]), + sock->ah_size * 2 * sizeof(sock->ah_handles[0])); for (size_t i = sock->ah_size; i < sock->ah_size * 2; i++) { sock->ah_frees[i] = i; diff --git a/lib/isc/tests/mem_test.c b/lib/isc/tests/mem_test.c index e848200e2a..9e35af3fc7 100644 --- a/lib/isc/tests/mem_test.c +++ b/lib/isc/tests/mem_test.c @@ -220,6 +220,64 @@ 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 = 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); + } + + for (size_t i = REGET_GROW_SIZE; i > 0; i--) { + data[i - 1] = i % UINT8_MAX; + } + + 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); + } + + isc_mem_put(test_mctx, data, REGET_SHRINK_SIZE); +} + #if ISC_MEM_TRACKLINES /* test mem with no flags */ @@ -436,6 +494,10 @@ 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), #if !defined(__SANITIZE_THREAD__) cmocka_unit_test_setup_teardown(isc_mem_benchmark, _setup,