From: Ondřej Surý Date: Wed, 5 Aug 2026 18:39:35 +0000 (+0200) Subject: Re-add the flags-based isc_mem_*x() API with ISC_MEM_ALIGN X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=250215df8145220ed3b32c3f47bd0e99dafa8f14;p=thirdparty%2Fbind9.git Re-add the flags-based isc_mem_*x() API with ISC_MEM_ALIGN This effectively reverts d9048b3db18 ("Remove ISC_MEM_ZERO and isc_mem_*x() API") and 8ac679a9808 ("Remove ISC_MEM_ALIGN() memory flag"); aligning the zone and database structures to cache lines gives the flags a user again. Unlike the removed version, ISC_MEM_ALIGN() no longer degrades to a no-op without the jemalloc API, and the memory context itself is once again allocated aligned to the cache line size. The non-jemalloc shim now has a single header-based implementation instead of the malloc_usable_size()/malloc_size() variants: every allocation carries a size_info header at a flags-derived offset before the returned pointer, so the matching flags passed on deallocation are enough to recover the base address and the exact allocation size, and to INSIST that the alignment flags really do match. Aligned allocations always move on reallocation, which keeps plain blocks malloc()/realloc() provenance and aligned blocks aligned_alloc() provenance. That split makes C23 free_sized()/free_aligned_sized() legal to use: they are called with exact allocation sizes when the libc provides them (new meson checks) and fall back to local free() wrappers otherwise. The cost is one size_info header per allocation in non-jemalloc builds - the fallback and sanitizer configurations - where memory accounting consequently returns to exact requested sizes rather than malloc_usable_size(). --- diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 2eece8021de..be22fad4588 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -15,8 +15,10 @@ /*! \file isc/mem.h */ +#include #include #include +#include #include #include @@ -117,30 +119,51 @@ extern isc_mem_t *isc_g_mctx; */ /*% + * Flags that can be passed to the isc_mem_*x() variants of the macros. + * * The definitions of the macros have been pulled directly from jemalloc.h * and checked for consistency in mem.c. * - *\li ISC__MEM_ZERO - fill the memory with zeroes before returning + *\li ISC_MEM_ALIGN(alignment) - use when you need aligned allocation, + * + * NOTE: Set the matching flag, when freeing aligned memory allocation. + * + *\li ISC_MEM_ZERO - fill the memory with zeroes before returning */ -#define ISC__MEM_ZERO ((int)0x40) +#if __SIZEOF_POINTER__ == 4 +#define ISC_MEM_ALIGN(a) ((int)(ffs((int)(a)) - 1)) +#else +#define ISC_MEM_ALIGN(a) \ + ((int)(((size_t)(a) < (size_t)INT_MAX) \ + ? ffs((int)(a)) - 1 \ + : ffs((int)(((size_t)(a)) >> 32)) + 31)) +#endif +#define ISC_MEM_ZERO ((int)0x40) #define isc_mem_get(c, s) isc__mem_get((c), (s), 0 _ISC_MEM_FILELINE) #define isc_mem_cget(c, n, s) \ isc__mem_get((c), ISC_CHECKED_MUL((n), (s)), \ - ISC__MEM_ZERO _ISC_MEM_FILELINE) + ISC_MEM_ZERO _ISC_MEM_FILELINE) +#define isc_mem_getx(c, s, f) isc__mem_get((c), (s), (f)_ISC_MEM_FILELINE) #define isc_mem_reget(c, p, o, n) \ isc__mem_reget((c), (p), (o), (n), 0 _ISC_MEM_FILELINE) #define isc_mem_creget(c, p, o, n, s) \ isc__mem_reget((c), (p), ISC_CHECKED_MUL((o), (s)), \ ISC_CHECKED_MUL((n), (s)), \ - ISC__MEM_ZERO _ISC_MEM_FILELINE) + ISC_MEM_ZERO _ISC_MEM_FILELINE) +#define isc_mem_regetx(c, p, o, n, f) \ + isc__mem_reget((c), (p), (o), (n), (f)_ISC_MEM_FILELINE) #define isc_mem_allocate(c, s) isc__mem_allocate((c), (s), 0 _ISC_MEM_FILELINE) +#define isc_mem_allocatex(c, s, f) \ + isc__mem_allocate((c), (s), (f)_ISC_MEM_FILELINE) #define isc_mem_callocate(c, n, s) \ isc__mem_allocate((c), ISC_CHECKED_MUL((n), (s)), \ - ISC__MEM_ZERO _ISC_MEM_FILELINE) + ISC_MEM_ZERO _ISC_MEM_FILELINE) #define isc_mem_reallocate(c, p, s) \ isc__mem_reallocate((c), (p), (s), 0 _ISC_MEM_FILELINE) +#define isc_mem_reallocatex(c, p, s, f) \ + isc__mem_reallocate((c), (p), (s), (f)_ISC_MEM_FILELINE) #define isc_mem_strdup(c, p) isc__mem_strdup((c), (p)_ISC_MEM_FILELINE) #define isc_mempool_get(c) isc__mempool_get((c)_ISC_MEM_FILELINE) @@ -152,19 +175,34 @@ extern isc_mem_t *isc_g_mctx; #define isc_mem_cput(c, p, n, s) \ do { \ isc__mem_put((c), (p), ISC_CHECKED_MUL((n), (s)), \ - ISC__MEM_ZERO _ISC_MEM_FILELINE); \ + ISC_MEM_ZERO _ISC_MEM_FILELINE); \ (p) = NULL; \ } while (0) +#define isc_mem_putx(c, p, s, f) \ + do { \ + isc__mem_put((c), (p), (s), (f)_ISC_MEM_FILELINE); \ + (p) = NULL; \ + } while (0) #define isc_mem_putanddetach(c, p, s) \ do { \ isc__mem_putanddetach((c), (p), (s), 0 _ISC_MEM_FILELINE); \ (p) = NULL; \ } while (0) +#define isc_mem_putanddetachx(c, p, s, f) \ + do { \ + isc__mem_putanddetach((c), (p), (s), (f)_ISC_MEM_FILELINE); \ + (p) = NULL; \ + } while (0) #define isc_mem_free(c, p) \ do { \ isc__mem_free((c), (p), 0 _ISC_MEM_FILELINE); \ (p) = NULL; \ } while (0) +#define isc_mem_freex(c, p, f) \ + do { \ + isc__mem_free((c), (p), (f)_ISC_MEM_FILELINE); \ + (p) = NULL; \ + } while (0) #define isc_mempool_put(c, p) \ do { \ isc__mempool_put((c), (p)_ISC_MEM_FILELINE); \ diff --git a/lib/isc/jemalloc_shim.h b/lib/isc/jemalloc_shim.h index 454494204f9..56553aee876 100644 --- a/lib/isc/jemalloc_shim.h +++ b/lib/isc/jemalloc_shim.h @@ -15,101 +15,116 @@ #if !defined(HAVE_JEMALLOC) +#include #include +#include #include #include +#include #include #include const char *malloc_conf = NULL; +static size_t +get_aligned_size(size_t alignment, size_t size) { + /* + * C23 dropped the requirement that size be an integral multiple of + * alignment, but some implementations still enforce it at runtime, + * so round the size up. + */ + return ISC_CHECKED_ADD(size, alignment - 1) & ~(alignment - 1); +} + +#ifndef HAVE_FREE_SIZED +static void +free_sized(void *ptr, size_t size ISC_ATTR_UNUSED) { + free(ptr); +} +#endif + +#ifndef HAVE_FREE_ALIGNED_SIZED +static void +free_aligned_sized(void *ptr, size_t alignment ISC_ATTR_UNUSED, + size_t size ISC_ATTR_UNUSED) { + free(ptr); +} +#endif + /* - * The MALLOCX_ZERO and MALLOCX_ZERO_GET macros were taken literal from - * jemalloc_macros.h and jemalloc_internal_types.h headers respectively. + * The MALLOCX_ZERO, MALLOCX_ALIGN and MALLOCX_*_GET macros were taken + * literal from jemalloc_macros.h and jemalloc_internal_types.h headers + * respectively. */ #define MALLOCX_ZERO ((int)0x40) #define MALLOCX_ZERO_GET(flags) ((bool)(flags & MALLOCX_ZERO)) -#if defined(HAVE_MALLOC_SIZE) || defined(HAVE_MALLOC_USABLE_SIZE) - -#ifdef HAVE_MALLOC_SIZE - -#include +#define MALLOCX_LG_ALIGN_MASK ((int)0x3f) +#if __SIZEOF_POINTER__ == 4 +#define MALLOCX_ALIGN(a) ((int)(ffs((int)(a)) - 1)) +#else +#define MALLOCX_ALIGN(a) \ + ((int)(((size_t)(a) < (size_t)INT_MAX) \ + ? ffs((int)(a)) - 1 \ + : ffs((int)(((size_t)(a)) >> 32)) + 31)) +#endif +#define MALLOCX_ALIGN_GET(flags) \ + (((size_t)1) << (flags & MALLOCX_LG_ALIGN_MASK)) -static inline size_t -sallocx(void *ptr, int flags ISC_ATTR_UNUSED) { - return malloc_size(ptr); -} +/* + * malloc() already guarantees alignment suitable for any fundamental + * type; only larger alignment requests need the aligned paths below. + */ +#define MALLOCX_NEEDS_ALIGN(flags) \ + (MALLOCX_ALIGN_GET(flags) > _Alignof(max_align_t)) -#elif HAVE_MALLOC_USABLE_SIZE +typedef union { + struct { + size_t size; + int flags; + }; + max_align_t __alignment; +} size_info; -#ifdef __DragonFly__ /* - * On DragonFly BSD 'man 3 malloc' advises us to include the following - * header to have access to malloc_usable_size(). + * The size_info header sits a multiple of the requested alignment before the + * returned pointer. The caller passes the matching MALLOCX_ALIGN() flag on + * deallocation too, so the base address is always recomputable from the flags + * and nothing needs to be stored. */ -#include -#else -#include -#endif static inline size_t -sallocx(void *ptr, int flags ISC_ATTR_UNUSED) { - return malloc_usable_size(ptr); +get_header_size(int flags) { + return get_aligned_size(MALLOCX_ALIGN_GET(flags), sizeof(size_info)); } -#endif /* HAVE_MALLOC_SIZE */ - -static inline void * -mallocx(size_t size, int flags) { - void *ptr = malloc(size); - INSIST(ptr != NULL); - - if ((flags & MALLOCX_ZERO) != 0) { - memset(ptr, 0, size); - } - - return ptr; -} - -static inline void -sdallocx(void *ptr, size_t size ISC_ATTR_UNUSED, int flags ISC_ATTR_UNUSED) { - free(ptr); +static inline size_info * +get_size_info(void *ptr, int flags) { + return (size_info *)((uint8_t *)ptr - get_header_size(flags)); } -static inline void * -rallocx(void *ptr, size_t size, int flags) { - REQUIRE(size != 0); - REQUIRE((flags & MALLOCX_ZERO) == 0); - - ptr = realloc(ptr, size); - INSIST(ptr != NULL); - - return ptr; -} - -#else - -typedef union { - size_t size; - max_align_t __alignment; -} size_info; - static inline void * mallocx(size_t size, int flags) { - void *ptr = NULL; - - size_t bytes = ISC_CHECKED_ADD(size, sizeof(size_info)); - size_info *si = malloc(bytes); + size_t alignment = MALLOCX_ALIGN_GET(flags); + size_t header_size = get_header_size(flags); + size_info *si; + + if (MALLOCX_NEEDS_ALIGN(flags)) { + size_t bytes = ISC_CHECKED_ADD( + get_aligned_size(alignment, size), header_size); + si = aligned_alloc(alignment, bytes); + } else { + si = malloc(ISC_CHECKED_ADD(size, header_size)); + } if (si == NULL) { return NULL; } - si->size = size; - ptr = &si[1]; + si->flags = flags; + void *ptr = (uint8_t *)si + header_size; if (MALLOCX_ZERO_GET(flags)) { memset(ptr, 0, size); } @@ -118,38 +133,74 @@ mallocx(size_t size, int flags) { } static inline void -sdallocx(void *ptr, size_t size ISC_ATTR_UNUSED, int flags ISC_ATTR_UNUSED) { - size_info *si = &(((size_info *)ptr)[-1]); - - free(si); +sdallocx(void *ptr, size_t size, int flags) { + size_info *si = get_size_info(ptr, flags); + size_t alignment = MALLOCX_ALIGN_GET(flags); + size_t header_size = get_header_size(flags); + + INSIST((flags & MALLOCX_LG_ALIGN_MASK) == + (si->flags & MALLOCX_LG_ALIGN_MASK)); + + if (MALLOCX_NEEDS_ALIGN(flags)) { + size_t bytes = ISC_CHECKED_ADD( + get_aligned_size(alignment, size), header_size); + free_aligned_sized(si, alignment, bytes); + } else { + free_sized(si, ISC_CHECKED_ADD(si->size, header_size)); + } } static inline size_t -sallocx(void *ptr, int flags ISC_ATTR_UNUSED) { - size_info *si = &(((size_info *)ptr)[-1]); +sallocx(void *ptr, int flags) { + size_info *si = get_size_info(ptr, flags); return si[0].size; } static inline void * rallocx(void *ptr, size_t size, int flags) { - size_t bytes = ISC_CHECKED_ADD(size, sizeof(size_info)); - size_info *si = realloc(&(((size_info *)ptr)[-1]), bytes); + size_t header_size = get_header_size(flags); + size_info *si = get_size_info(ptr, flags); + size_t old_size = si->size; + + INSIST((flags & MALLOCX_LG_ALIGN_MASK) == + (si->flags & MALLOCX_LG_ALIGN_MASK)); + + if (MALLOCX_NEEDS_ALIGN(flags)) { + /* + * realloc() cannot preserve the requested alignment; + * move the data to a fresh aligned allocation. + */ + void *new_ptr = mallocx(size, flags & ~MALLOCX_ZERO); + if (new_ptr == NULL) { + return NULL; + } + memmove(new_ptr, ptr, ISC_MIN(old_size, size)); + if (MALLOCX_ZERO_GET(flags) && size > old_size) { + memset((uint8_t *)new_ptr + old_size, 0, + size - old_size); + } + sdallocx(ptr, old_size, flags); + + si = get_size_info(new_ptr, flags); + si->flags = flags; + + return new_ptr; + } + + si = realloc(si, ISC_CHECKED_ADD(size, header_size)); if (si == NULL) { return NULL; } + si->size = size; + si->flags = flags; - if (MALLOCX_ZERO_GET(flags) && size > si->size) { - memset((uint8_t *)si + sizeof(*si) + si->size, 0, - size - si->size); + ptr = (uint8_t *)si + header_size; + if (MALLOCX_ZERO_GET(flags) && size > old_size) { + memset((uint8_t *)ptr + old_size, 0, size - old_size); } - si->size = size; - ptr = &si[1]; - return ptr; } -#endif /* defined(HAVE_MALLOC_SIZE) || defined(HAVE_MALLOC_USABLE_SIZE) */ - #endif /* !defined(HAVE_JEMALLOC) */ diff --git a/lib/isc/mem.c b/lib/isc/mem.c index cd3a8e5a89c..309cedbeb5b 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -58,6 +58,9 @@ #include #define JEMALLOC_API_SUPPORTED 1 #else +#if defined(__GLIBC__) +#include +#endif #include "jemalloc_shim.h" #endif @@ -503,12 +506,18 @@ debugging_enabled(const char *name) { void isc__mem_initialize(void) { -/* - * Check if the values copied from jemalloc still match - */ -#ifdef JEMALLOC_API_SUPPORTED - RUNTIME_CHECK(ISC__MEM_ZERO == MALLOCX_ZERO); + /* + * Check if the values copied from jemalloc still match; the + * shim defines the MALLOCX_* macros too, so this holds on + * every allocator path. + */ + RUNTIME_CHECK(ISC_MEM_ZERO == MALLOCX_ZERO); + RUNTIME_CHECK(ISC_MEM_ALIGN(sizeof(void *)) == + MALLOCX_ALIGN(sizeof(void *))); + RUNTIME_CHECK(ISC_MEM_ALIGN(ISC_OS_CACHELINE_SIZE) == + MALLOCX_ALIGN(ISC_OS_CACHELINE_SIZE)); +#ifdef JEMALLOC_API_SUPPORTED /* * ignore errors — volumetric-based purge in mem_put handles the rest * regardless @@ -605,7 +614,8 @@ mem_create(const char *name, isc_mem_t **ctxp, unsigned int debugging, REQUIRE(ctxp != NULL && *ctxp == NULL); REQUIRE(name != NULL); - ctx = mallocx(sizeof(*ctx), jemalloc_flags); + ctx = mallocx(sizeof(*ctx), + jemalloc_flags | ISC_MEM_ALIGN(isc_os_cacheline())); CHECK_OOM(ctx, sizeof(*ctx)); *ctx = (isc_mem_t){ @@ -677,7 +687,7 @@ mem_destroy(isc_mem_t *ctx) { INSIST(!ctx->checkfree || dl->ptr == NULL); ISC_LIST_UNLINK(ctx->debuglist[i], dl, link); - sdallocx(dl, sizeof(*dl), ctx->jemalloc_flags); + sdallocx(dl, dl->dlsize, ctx->jemalloc_flags); } } @@ -706,7 +716,8 @@ mem_destroy(isc_mem_t *ctx) { } #endif /* ISC_MEM_TRACKLINES */ - sdallocx(ctx, sizeof(*ctx), ctx->jemalloc_flags); + sdallocx(ctx, sizeof(*ctx), + ctx->jemalloc_flags | ISC_MEM_ALIGN(isc_os_cacheline())); } #if ISC_MEM_TRACE @@ -868,19 +879,7 @@ isc__mem_reget(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size, ADJUST_ZERO_ALLOCATION_SIZE(new_size); -#ifdef JEMALLOC_API_SUPPORTED new_ptr = mem_realloc(ctx, old_ptr, new_size, flags); -#else - new_ptr = mem_realloc(ctx, old_ptr, new_size, - flags & ~ISC__MEM_ZERO); - - if ((flags & ISC__MEM_ZERO) != 0) { - if (new_size > old_size) { - memset((uint8_t *)new_ptr + old_size, 0, - new_size - old_size); - } - } -#endif mem_getstats(ctx, new_size); ADD_TRACE(ctx, new_ptr, new_size, func, file, line); diff --git a/meson.build b/meson.build index dd536d86c9c..4fd6811e950 100644 --- a/meson.build +++ b/meson.build @@ -565,6 +565,10 @@ foreach fn, header : { # CSPRNG 'arc4random': '#include ', + # C23 sized frees + 'free_sized': '#include ', + 'free_aligned_sized': '#include ', + # Misc. 'chroot': '#include ', 'clock_gettime': '#include ', @@ -816,32 +820,6 @@ if jemalloc_opt.allowed() endif endif -foreach fn : [ - 'malloc_usable_size', -] - if cc.has_function( - fn, - prefix: '#include ', - args: sys_defines, - dependencies: thread_dep, - ) - config.set('HAVE_@0@'.format(fn.to_upper()), 1) - endif -endforeach - -foreach fn : [ - 'malloc_size', -] - if cc.has_function( - fn, - prefix: '#include ', - args: sys_defines, - dependencies: thread_dep, - ) - config.set('HAVE_@0@'.format(fn.to_upper()), 1) - endif -endforeach - ## dnstap dnstap_dep = null_dep # Will be filled later diff --git a/tests/isc/mem_test.c b/tests/isc/mem_test.c index 5462b628d48..a5a474acc8d 100644 --- a/tests/isc/mem_test.c +++ b/tests/isc/mem_test.c @@ -125,6 +125,70 @@ ISC_RUN_TEST_IMPL(isc_mem_get) { isc_mempool_destroy(&mp1); } +/* aligned memory system tests */ +ISC_RUN_TEST_IMPL(isc_mem_get_align) { + isc_mem_t *mctx2 = NULL; + void *ptr; + size_t alignment; + uintptr_t aligned; + + /* Check different alignment sizes up to the page size */ + for (alignment = sizeof(void *); alignment <= 4096; alignment *= 2) { + size_t size = alignment / 2 - 1; + ptr = isc_mem_getx(isc_g_mctx, size, ISC_MEM_ALIGN(alignment)); + + /* Check if the pointer is properly aligned */ + aligned = (((uintptr_t)ptr / alignment) * alignment); + assert_ptr_equal(aligned, (uintptr_t)ptr); + + /* Check if we can resize to range */ + ptr = isc_mem_regetx(isc_g_mctx, ptr, size, + size * 2 + alignment, + ISC_MEM_ALIGN(alignment)); + + /* Check if the pointer is still properly aligned */ + aligned = (((uintptr_t)ptr / alignment) * alignment); + assert_ptr_equal(aligned, (uintptr_t)ptr); + + isc_mem_putx(isc_g_mctx, ptr, size * 2 + alignment, + ISC_MEM_ALIGN(alignment)); + + /* Check whether isc_mem_putanddetachx() also works */ + isc_mem_create("mctx2", &mctx2); + ptr = isc_mem_getx(mctx2, size, ISC_MEM_ALIGN(alignment)); + isc_mem_putanddetachx(&mctx2, ptr, size, + ISC_MEM_ALIGN(alignment)); + } +} + +/* aligned memory system tests */ +ISC_RUN_TEST_IMPL(isc_mem_allocate_align) { + void *ptr; + size_t alignment; + uintptr_t aligned; + + /* Check different alignment sizes up to the page size */ + for (alignment = sizeof(void *); alignment <= 4096; alignment *= 2) { + size_t size = alignment / 2 - 1; + ptr = isc_mem_allocatex(isc_g_mctx, size, + ISC_MEM_ALIGN(alignment)); + + /* Check if the pointer is properly aligned */ + aligned = (((uintptr_t)ptr / alignment) * alignment); + assert_ptr_equal(aligned, (uintptr_t)ptr); + + /* Check if we can resize to range */ + ptr = isc_mem_reallocatex(isc_g_mctx, ptr, size * 2 + alignment, + ISC_MEM_ALIGN(alignment)); + + /* Check if the pointer is still properly aligned */ + aligned = (((uintptr_t)ptr / alignment) * alignment); + assert_ptr_equal(aligned, (uintptr_t)ptr); + + isc_mem_freex(isc_g_mctx, ptr, ISC_MEM_ALIGN(alignment)); + } +} + /* zeroed memory system tests */ ISC_RUN_TEST_IMPL(isc_mem_cget_zero) { uint8_t *ptr; @@ -557,6 +621,8 @@ ISC_RUN_TEST_IMPL(isc_mem_benchmark) { ISC_TEST_LIST_START ISC_TEST_ENTRY(isc_mem_get) +ISC_TEST_ENTRY(isc_mem_get_align) +ISC_TEST_ENTRY(isc_mem_allocate_align) ISC_TEST_ENTRY(isc_mem_cget_zero) ISC_TEST_ENTRY(isc_mem_callocate_zero) ISC_TEST_ENTRY(isc_mem_inuse)