Merge pull request #27753 from azat/osx-jemalloc-fix

Fix jemalloc under osx (zone_register() had been optimized out again)

CMakeLists.txt

@@ -593,7 +593,23 @@ macro (add_executable target)
         # disabled for TSAN and gcc since libtsan.a provides overrides too
         if (TARGET clickhouse_new_delete)
             # operator::new/delete for executables (MemoryTracker stuff)
-            target_link_libraries (${target} PRIVATE clickhouse_new_delete ${MALLOC_LIBRARIES})
+            target_link_libraries (${target} PRIVATE clickhouse_new_delete)
+        endif()
+
+        # In case of static jemalloc, because zone_register() is located in zone.c and
+        # is never used outside (it is declared as constructor) it is omitted
+        # by the linker, and so jemalloc will not be registered as system
+        # allocator under osx [1], and clickhouse will SIGSEGV.
+        #
+        #   [1]: https://github.com/jemalloc/jemalloc/issues/708
+        #
+        # About symbol name:
+        # - _zone_register not zone_register due to Mach-O binary format,
+        # - _je_zone_register due to JEMALLOC_PRIVATE_NAMESPACE=je_ under OS X.
+        # - but jemalloc-cmake does not run private_namespace.sh
+        #   so symbol name should be _zone_register
+        if (ENABLE_JEMALLOC AND MAKE_STATIC_LIBRARIES AND OS_DARWIN)
+            set_property(TARGET ${target} APPEND PROPERTY LINK_OPTIONS -u_zone_register)
         endif()
     endif()
 endmacro()

contrib/jemalloc-cmake/CMakeLists.txt

@@ -1,9 +1,10 @@
-# Disabled under OSX until https://github.com/ClickHouse/ClickHouse/issues/27568 is fixed
 if (SANITIZE OR NOT (
-    ((OS_LINUX OR OS_FREEBSD) AND (ARCH_AMD64 OR ARCH_ARM OR ARCH_PPC64LE))))
+    ((OS_LINUX OR OS_FREEBSD) AND (ARCH_AMD64 OR ARCH_ARM OR ARCH_PPC64LE)) OR
+    (OS_DARWIN AND (CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo" OR CMAKE_BUILD_TYPE STREQUAL "Debug"))
+))
     if (ENABLE_JEMALLOC)
         message (${RECONFIGURE_MESSAGE_LEVEL}
-                 "jemalloc is disabled implicitly: it doesn't work with sanitizers and can only be used with x86_64, aarch64, or ppc64le Linux or FreeBSD builds")
+                 "jemalloc is disabled implicitly: it doesn't work with sanitizers and can only be used with x86_64, aarch64, or ppc64le Linux or FreeBSD builds and RelWithDebInfo macOS builds.")
     endif ()
     set (ENABLE_JEMALLOC OFF)
 else ()
@@ -138,9 +139,5 @@ target_compile_options(jemalloc PRIVATE -Wno-redundant-decls)
 target_compile_options(jemalloc PRIVATE -D_GNU_SOURCE)
 
 set_property(TARGET jemalloc APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS USE_JEMALLOC=1)
-if (MAKE_STATIC_LIBRARIES)
-    # To detect whether we need to register jemalloc for osx as default zone.
-    set_property(TARGET jemalloc APPEND PROPERTY INTERFACE_COMPILE_DEFINITIONS BUNDLED_STATIC_JEMALLOC=1)
-endif()
 
 message (STATUS "Using jemalloc")

src/Common/memory.cpp

@@ -1,25 +0,0 @@
-#if defined(OS_DARWIN) && defined(BUNDLED_STATIC_JEMALLOC)
-
-extern "C"
-{
-    extern void zone_register();
-}
-
-struct InitializeJemallocZoneAllocatorForOSX
-{
-    InitializeJemallocZoneAllocatorForOSX()
-    {
-        /// In case of OSX jemalloc register itself as a default zone allocator.
-        ///
-        /// But when you link statically then zone_register() will not be called,
-        /// and even will be optimized out:
-        ///
-        /// It is ok to call it twice (i.e. in case of shared libraries)
-        /// Since zone_register() is a no-op if the default zone is already replaced with something.
-        ///
-        /// https://github.com/jemalloc/jemalloc/issues/708
-        zone_register();
-    }
-} initializeJemallocZoneAllocatorForOSX;
-
-#endif