From f82f4d1d77c58e0de238b46104d43275045cecb3 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. (cherry picked from commit 41a0ee10717a5afaa2bfdd0225245feecc09eea9) --- Makefile.top | 12 ++++++++++-- lib/isc/include/isc/mem.h | 26 ++++++++++++++++++++++++++ lib/isc/mem.c | 2 ++ tests/libtest/dns.c | 7 ++----- 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/Makefile.top b/Makefile.top index 62c5293c2c..e186d15a0c 100644 --- a/Makefile.top +++ b/Makefile.top @@ -20,12 +20,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 LIBDNS_CFLAGS = \ -I$(top_srcdir)/lib/dns/include \ diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index bd59f735b8..3201ba4c8b 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -184,7 +184,33 @@ 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; + +#ifndef CMM_ACCESS_ONCE +/* + * This macro has been borrowed from Userspace-RCU to ensure the access + * to isc__mem_malloc will not be optimized away by the compiler. + */ +#define CMM_ACCESS_ONCE(x) (*(__volatile__ __typeof__(x) *)&(x)) +#endif + +#define isc_mem_create(cp) \ + { \ + ISCMEMFUNC(create)((cp)_ISC_MEM_FILELINE); \ + isc__mem_malloc = mallocx; \ + ISC_INSIST(CMM_ACCESS_ONCE(isc__mem_malloc) != NULL); \ + } +#else #define isc_mem_create(cp) ISCMEMFUNC(create)((cp)_ISC_MEM_FILELINE) +#endif void ISCMEMFUNC(create)(isc_mem_t **_ISC_MEM_FLARG); /*!< diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 6560833752..4fbe7d0342 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -73,6 +73,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 ae3748fb41..c01b1bb770 100644 --- a/tests/libtest/dns.c +++ b/tests/libtest/dns.c @@ -24,9 +24,6 @@ #include #include -#define UNIT_TESTING -#include - #include #include #include @@ -259,7 +256,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); } @@ -426,7 +423,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, dns_rootname, 0, NULL); - assert_int_equal(result, ISC_R_SUCCESS); + INSIST(result == ISC_R_SUCCESS); isc_buffer_free(&b); } From afb0b3971c8bbf0db03b76c605f3d5152d83a572 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. (cherry picked from commit 197de93bdc8eb5cc4dd18e0812cbb87c2ed7fa61) --- lib/isc/include/isc/mem.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 3201ba4c8b..ab8dd2c2c0 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -190,7 +190,13 @@ 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; From 3d0bfa3f28848917a478bc35c6c71b44e7fd7c46 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 (cherry picked from commit 62152068015e36586ea8d2221e5ffd2d292750a4) --- 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 e0bcda8923a650cbb5c05f527df4816129a25bec 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] (cherry picked from commit ec12682933a0a731c5f18431198b016460026608) --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index a0ea728fc9..1eec056345 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] + 6324. [bug] Changes to "listen-on" statements were ignored on reconfiguration unless the port or interface address was changed, making it impossible to change a related