From d08d0d81acbe0dabba12448f589330e82d407b54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 24 Nov 2025 09:32:51 +0100 Subject: [PATCH 1/2] Remove internal memory filling in favor of jemalloc opt.junk Instead of having our own implementation of memory junk filling, rely on the jemalloc opt.junk feature (set with MALLOC_CONF="junk:true"). --- OPTIONS.md | 1 - bin/named/main.c | 8 ++---- lib/isc/include/isc/mem.h | 20 --------------- lib/isc/mem.c | 52 +++++++++------------------------------ meson.build | 1 - 5 files changed, 13 insertions(+), 69 deletions(-) diff --git a/OPTIONS.md b/OPTIONS.md index 71f556f4f1..b7eeee93c2 100644 --- a/OPTIONS.md +++ b/OPTIONS.md @@ -22,7 +22,6 @@ Some of these settings are: | `-DCHECK_SIBLING=0` | Don't check sibling glue in `named-checkzone` | | `-DISC_FACILITY=LOG_LOCAL0` | Change the default syslog facility for `named` | | `-DISC_HEAP_CHECK` | Test heap consistency after every heap operation; used when debugging | -| `-DISC_MEM_DEFAULTFILL=1` | Overwrite memory with tag values when allocating or freeing it; this impairs performance but makes debugging of memory problems easier | | `-DISC_MEM_TRACKLINES=0` | Don't track memory allocations by file and line number; this improves performance but makes debugging more difficult | | `-DNAMED_RUN_PID_DIR=0` | Create default PID files in `${localstatedir}/run` rather than `${localstatedir}/run/named/` | | `-DNS_CLIENT_DROPPORT=0` | Disable dropping queries from particular well-known ports | diff --git a/bin/named/main.c b/bin/named/main.c index d616910ca9..92d3b2b9b2 100644 --- a/bin/named/main.c +++ b/bin/named/main.c @@ -374,10 +374,7 @@ static struct flag_def { { "trace", ISC_MEM_DEBUGTRACE, false }, { "record", ISC_MEM_DEBUGRECORD, false }, { "usage", ISC_MEM_DEBUGUSAGE, false }, - { NULL, 0, false } }, - mem_context_flags[] = { { "fill", ISC_MEMFLAG_FILL, false }, - { "nofill", ISC_MEMFLAG_FILL, true }, - { NULL, 0, false } }; + { NULL, 0, false } }; static void set_flags(const char *arg, struct flag_def *defs, unsigned int *ret) { @@ -849,8 +846,7 @@ parse_command_line(int argc, char *argv[]) { named_g_logfile = isc_commandline_argument; break; case 'M': - set_flags(isc_commandline_argument, mem_context_flags, - &isc_mem_defaultflags); + named_main_earlywarning("option '-M' has been removed"); break; case 'm': set_flags(isc_commandline_argument, mem_debug_flags, diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 4792ed3d6b..5aaec6e705 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -35,8 +35,6 @@ #define ISC_MEM_TRACKLINES 0 #endif /* ifndef ISC_MEM_TRACKLINES */ -extern unsigned int isc_mem_defaultflags; - /*@{*/ #define ISC_MEM_DEBUGTRACE 0x00000001U #define ISC_MEM_DEBUGRECORD 0x00000002U @@ -76,24 +74,6 @@ extern unsigned int isc_mem_defaultflags; #define _ISC_MEM_FLARG #endif /* if ISC_MEM_TRACKLINES */ -/* - * Flags for isc_mem_create() calls. - */ -#define ISC_MEMFLAG_RESERVED1 0x00000001 /* reserved, obsoleted, don't use */ -#define ISC_MEMFLAG_RESERVED2 0x00000002 /* reserved, obsoleted, don't use */ -#define ISC_MEMFLAG_FILL \ - 0x00000004 /* fill with pattern after alloc and frees */ - -/*% - * Define ISC_MEM_DEFAULTFILL=1 to turn filling the memory with pattern - * after alloc and free. - */ -#if ISC_MEM_DEFAULTFILL -#define ISC_MEMFLAG_DEFAULT ISC_MEMFLAG_FILL -#else /* if !ISC_MEM_USE_INTERNAL_MALLOC */ -#define ISC_MEMFLAG_DEFAULT 0 -#endif /* if !ISC_MEM_USE_INTERNAL_MALLOC */ - /*% * A global 'default' memory context that can be used when we don't need more * specific memory context. It is always available. diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 7b391042d9..36498b3e8f 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -67,7 +67,6 @@ #endif /* ifndef ISC_MEM_DEBUGGING */ static unsigned int mem_debugging = ISC_MEM_DEBUGGING; -unsigned int isc_mem_defaultflags = ISC_MEMFLAG_DEFAULT; volatile void *isc__mem_malloc = mallocx; @@ -127,7 +126,6 @@ typedef union { struct isc_mem { unsigned int magic; - unsigned int flags; unsigned int jemalloc_flags; unsigned int debugging; isc_mutex_t lock; @@ -319,12 +317,6 @@ mem_get(isc_mem_t *ctx, size_t size, int flags) { ret = mallocx(size, flags | ctx->jemalloc_flags); INSIST(ret != NULL); - if ((flags & ISC__MEM_ZERO) == 0 && - (ctx->flags & ISC_MEMFLAG_FILL) != 0) - { - memset(ret, 0xbe, size); /* Mnemonic for "beef". */ - } - return ret; } @@ -336,15 +328,11 @@ static void mem_put(isc_mem_t *ctx, void *mem, size_t size, int flags) { ADJUST_ZERO_ALLOCATION_SIZE(size); - if ((ctx->flags & ISC_MEMFLAG_FILL) != 0) { - memset(mem, 0xde, size); /* Mnemonic for "dead". */ - } sdallocx(mem, size, flags | ctx->jemalloc_flags); } static void * -mem_realloc(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size, - int flags) { +mem_realloc(isc_mem_t *ctx, void *old_ptr, size_t new_size, int flags) { void *new_ptr = NULL; ADJUST_ZERO_ALLOCATION_SIZE(new_size); @@ -352,17 +340,6 @@ mem_realloc(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size, new_ptr = rallocx(old_ptr, new_size, flags | ctx->jemalloc_flags); INSIST(new_ptr != NULL); - if ((flags & ISC__MEM_ZERO) == 0 && - (ctx->flags & ISC_MEMFLAG_FILL) != 0) - { - if (new_size > old_size) { - size_t diff_size = new_size - old_size; - void *diff_ptr = (uint8_t *)new_ptr + old_size; - /* Mnemonic for "beef". */ - memset(diff_ptr, 0xbe, diff_size); - } - } - return new_ptr; } @@ -485,7 +462,7 @@ isc_mem_debugoff(unsigned int debugging) { static void mem_create(const char *name, isc_mem_t **ctxp, unsigned int debugging, - unsigned int flags, unsigned int jemalloc_flags) { + unsigned int jemalloc_flags) { isc_mem_t *ctx = NULL; REQUIRE(ctxp != NULL && *ctxp == NULL); @@ -497,7 +474,6 @@ mem_create(const char *name, isc_mem_t **ctxp, unsigned int debugging, *ctx = (isc_mem_t){ .magic = MEM_MAGIC, .debugging = debugging, - .flags = flags, .jemalloc_flags = jemalloc_flags, .checkfree = true, .name = strdup(name), @@ -748,7 +724,7 @@ isc__mem_reget(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size, DELETE_TRACE(ctx, old_ptr, old_size, func, file, line); mem_putstats(ctx, old_size); - new_ptr = mem_realloc(ctx, old_ptr, old_size, new_size, flags); + new_ptr = mem_realloc(ctx, old_ptr, new_size, flags); mem_getstats(ctx, new_size); ADD_TRACE(ctx, new_ptr, new_size, func, file, line); @@ -775,24 +751,18 @@ isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size, } else if (new_size == 0) { isc__mem_free(ctx, old_ptr, flags FLARG_PASS); } else { - size_t old_size = sallocx(old_ptr, flags | ctx->jemalloc_flags); + size_t size = sallocx(old_ptr, flags | ctx->jemalloc_flags); - DELETE_TRACE(ctx, old_ptr, old_size, func, file, line); - mem_putstats(ctx, old_size); + DELETE_TRACE(ctx, old_ptr, size, func, file, line); + mem_putstats(ctx, size); - new_ptr = mem_realloc(ctx, old_ptr, old_size, new_size, flags); + new_ptr = mem_realloc(ctx, old_ptr, new_size, flags); /* Recalculate the real allocated size */ - new_size = sallocx(new_ptr, flags | ctx->jemalloc_flags); + size = sallocx(new_ptr, flags | ctx->jemalloc_flags); - mem_getstats(ctx, new_size); - ADD_TRACE(ctx, new_ptr, new_size, func, 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. - */ + mem_getstats(ctx, size); + ADD_TRACE(ctx, new_ptr, size, func, file, line); } return new_ptr; @@ -1449,7 +1419,7 @@ error: void isc__mem_create(const char *name, isc_mem_t **mctxp FLARG) { - mem_create(name, mctxp, mem_debugging, isc_mem_defaultflags, 0); + mem_create(name, mctxp, mem_debugging, 0); #if ISC_MEM_TRACKLINES if ((mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { fprintf(stderr, "create mctx %p func %s file %s line %u\n", diff --git a/meson.build b/meson.build index f107018545..a4af8a153b 100644 --- a/meson.build +++ b/meson.build @@ -307,7 +307,6 @@ endif if developer_mode config.set('ISC_LIST_CHECKINIT', 1) - config.set('ISC_MEM_DEFAULTFILL', 1) config.set('ISC_MEM_TRACKLINES', 1) config.set('ISC_MUTEX_ERROR_CHECK', 1) config.set('ISC_SOCKET_DETAILS', 1) From 52e2cb4d56a1c7786adbfc5297a3ef7616acfe6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 26 Nov 2025 11:45:03 +0100 Subject: [PATCH 2/2] Enable junk filling via jemalloc option in the CI Since the filling memory with junk patterns have been removed from ISC memory context in favor of jemalloc opt.junk option, enable the jemalloc behaviour by default in the GitLab CI. --- .gitlab-ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ddb03c1f32..29b3f627f8 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -4,6 +4,9 @@ variables: # Locale settings do not affect the build, but might affect tests. LC_ALL: C + # enable junk filling via jemalloc option + MALLOC_CONF: "junk:true" + # automated commits will inherit identification from pipeline trigger GIT_AUTHOR_NAME: "$GITLAB_USER_NAME (GitLab job $CI_JOB_ID)" GIT_AUTHOR_EMAIL: "$GITLAB_USER_EMAIL"