From 41a0ee10717a5afaa2bfdd0225245feecc09eea9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 21 Dec 2023 11:12:54 +0100 Subject: [PATCH 1/4] Add workaround for jemalloc linking order Because we don't use jemalloc functions directly, but only via the libisc library, the dynamic linker might pull the jemalloc library too late when memory has been already allocated via standard libc allocator. Add a workaround round isc_mem_create() that makes the dynamic linker to pull jemalloc earlier than libc. --- Makefile.top | 12 ++++++++++-- lib/isc/include/isc/mem.h | 19 +++++++++++++++++++ lib/isc/mem.c | 2 ++ tests/libtest/dns.c | 7 ++----- 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/Makefile.top b/Makefile.top index d01317dadb..da95a2d47a 100644 --- a/Makefile.top +++ b/Makefile.top @@ -23,12 +23,20 @@ AM_LDFLAGS += \ -Wl,-flat_namespace endif HOST_MACOS -LIBISC_CFLAGS = \ +if HAVE_JEMALLOC +LIBISC_CFLAGS = $(JEMALLOC_CFLAGS) +LIBISC_LIBS = $(JEMALLOC_LIBS) +else +LIBISC_CFLAGS = +LIBISC_LIBS = +endif + +LIBISC_CFLAGS += \ -I$(top_srcdir)/include \ -I$(top_srcdir)/lib/isc/include \ -I$(top_builddir)/lib/isc/include -LIBISC_LIBS = $(top_builddir)/lib/isc/libisc.la +LIBISC_LIBS += $(top_builddir)/lib/isc/libisc.la if HAVE_DTRACE LIBISC_DTRACE = $(top_builddir)/lib/isc/probes.lo endif diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 6c099d8742..b7143bdbdd 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -23,6 +23,7 @@ #include #include #include +#include ISC_LANG_BEGINDECLS @@ -183,7 +184,25 @@ extern unsigned int isc_mem_defaultflags; } while (0) /*@{*/ +/* + * This is a little hack to help with dynamic link order, + * see https://github.com/jemalloc/jemalloc/issues/2566 + * for more information. + */ +#if HAVE_JEMALLOC +#include + +extern volatile void *isc__mem_malloc; + +#define isc_mem_create(cp) \ + { \ + isc__mem_create((cp)_ISC_MEM_FILELINE); \ + isc__mem_malloc = mallocx; \ + ISC_INSIST(CMM_ACCESS_ONCE(isc__mem_malloc) != NULL); \ + } +#else #define isc_mem_create(cp) isc__mem_create((cp)_ISC_MEM_FILELINE) +#endif void isc__mem_create(isc_mem_t **_ISC_MEM_FLARG); diff --git a/lib/isc/mem.c b/lib/isc/mem.c index e43ae4fc03..c23a39075e 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -69,6 +69,8 @@ unsigned int isc_mem_defaultflags = ISC_MEMFLAG_DEFAULT; #define ISC_MEM_ILLEGAL_ARENA (UINT_MAX) +volatile void *isc__mem_malloc = mallocx; + /* * Constants. */ diff --git a/tests/libtest/dns.c b/tests/libtest/dns.c index 73fb630807..29ac69e5fa 100644 --- a/tests/libtest/dns.c +++ b/tests/libtest/dns.c @@ -24,9 +24,6 @@ #include #include -#define UNIT_TESTING -#include - #include #include #include @@ -264,7 +261,7 @@ dns_test_tohex(const unsigned char *data, size_t len, char *buf, memset(buf, 0, buflen); isc_buffer_init(&target, buf, buflen); result = isc_hex_totext((isc_region_t *)&source, 1, " ", &target); - assert_int_equal(result, ISC_R_SUCCESS); + INSIST(result == ISC_R_SUCCESS); return (buf); } @@ -428,7 +425,7 @@ dns_test_namefromstring(const char *namestr, dns_fixedname_t *fname) { isc_buffer_putmem(b, (const unsigned char *)namestr, length); result = dns_name_fromtext(name, b, NULL, 0, NULL); - assert_int_equal(result, ISC_R_SUCCESS); + INSIST(result == ISC_R_SUCCESS); isc_buffer_free(&b); } From 197de93bdc8eb5cc4dd18e0812cbb87c2ed7fa61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ayd=C4=B1n=20Mercan?= Date: Thu, 4 Jan 2024 10:40:54 +0300 Subject: [PATCH 2/4] Forward declare mallocx in isc/mem.h cmocka.h and jemalloc.h/malloc_np.h has conflicting macro definitions. While fixing them with push_macro for only malloc is done below, we only need the non-standard mallocx interface which is easy to just define by ourselves. --- lib/isc/include/isc/mem.h | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index b7143bdbdd..2ee8c9e200 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -190,14 +190,20 @@ extern unsigned int isc_mem_defaultflags; * for more information. */ #if HAVE_JEMALLOC -#include + +/* + * cmocka.h has confliction definitions with the jemalloc header but we only + * need the mallocx symbol from jemalloc. + */ +void * +mallocx(size_t size, int flags); extern volatile void *isc__mem_malloc; -#define isc_mem_create(cp) \ - { \ - isc__mem_create((cp)_ISC_MEM_FILELINE); \ - isc__mem_malloc = mallocx; \ +#define isc_mem_create(cp) \ + { \ + isc__mem_create((cp)_ISC_MEM_FILELINE); \ + isc__mem_malloc = mallocx; \ ISC_INSIST(CMM_ACCESS_ONCE(isc__mem_malloc) != NULL); \ } #else From 62152068015e36586ea8d2221e5ffd2d292750a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ayd=C4=B1n=20Mercan?= Date: Thu, 11 Jan 2024 13:59:11 +0300 Subject: [PATCH 3/4] Link jemalloc again for testing unit build order --- Makefile.tests | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Makefile.tests b/Makefile.tests index e1b7e0e046..a0ea914d40 100644 --- a/Makefile.tests +++ b/Makefile.tests @@ -21,3 +21,8 @@ AM_CPPFLAGS += \ LDADD += \ $(top_builddir)/tests/libtest/libtest.la \ $(CMOCKA_LIBS) + +if HAVE_JEMALLOC +AM_CFLAGS += $(JEMALLOC_CFLAGS) +LDADD += $(JEMALLOC_LIBS) +endif From ec12682933a0a731c5f18431198b016460026608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 21 Dec 2023 11:16:29 +0100 Subject: [PATCH 4/4] Add CHANGES note for [GL #4404] --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index dad4e32413..2a72463b1d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6326. [func] Add workaround to enforce dynamic linker to pull + jemalloc earlier than libc to ensure all memory + allocations are done via jemalloc. [GL #4404] + 6325. [func] Expose the TCP client count in statistics channel. [GL #4425]