CMake fixes

- Updated README with CMake variable default values - CMake: support for tcmalloc / tcmalloc_minimal is now working - CMake: Lua lib: prefer mkstemp over tmpnam (tmpname is considered insecured) - CMake: use `valkey_pkg_config` to wrap `pkg_check_modules` - Reduced the CMake minimum version to 3.10 (this allows using CMake build on older systems) - CMake: sanitizer flags should be set for valkey-server only (no need for valkey-cli & valkey-benchmark) Signed-off-by: Eran Ifrah <[email protected]>
valkey-io · Nov 14, 2024 · a3a9eea · a3a9eea
1 parent 863d312
commit a3a9eea
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 29 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.20)
+cmake_minimum_required(VERSION 3.10)
 
 # Must be done first
 if (APPLE)

diff --git a/README.md b/README.md
@@ -297,19 +297,19 @@ Other options supported by Valkey's `CMake` build system:
 
 ## Special build flags
 
-- `-DBUILD_TLS=<on|off|module>` enable TLS build for Valkey
-- `-DBUILD_RDMA=<off|module>` enable RDMA module build (only module mode supported)
+- `-DBUILD_TLS=<yes|no>` enable TLS build for Valkey. Default: `no`
+- `-DBUILD_RDMA=<no|module>` enable RDMA module build (only module mode supported). Default: `no`
 - `-DBUILD_MALLOC=<libc|jemalloc|tcmalloc|tcmalloc_minimal>` choose the allocator to use. Default on Linux: `jemalloc`, for other OS: `libc`
-- `-DBUILD_SANITIZER=<address|thread|undefined>` build with address sanitizer enabled
-- `-DBUILD_UNIT_TESTS=[1|0]`  when set, the build will produce the executable `valkey-unit-tests`
-- `-DBUILD_TEST_MODULES=[1|0]`  when set, the build will include the modules located under the `tests/modules` folder
-- `-DBUILD_EXAMPLE_MODULES=[1|0]`  when set, the build will include the example modules located under the `src/modules` folder
+- `-DBUILD_SANITIZER=<address|thread|undefined>` build with address sanitizer enabled. Default: disabled (no sanitizer)
+- `-DBUILD_UNIT_TESTS=[yes|no]`  when set, the build will produce the executable `valkey-unit-tests`. Default: `no`
+- `-DBUILD_TEST_MODULES=[yes|no]`  when set, the build will include the modules located under the `tests/modules` folder. Default: `no`
+- `-DBUILD_EXAMPLE_MODULES=[yes|no]`  when set, the build will include the example modules located under the `src/modules` folder. Default: `no`
 
 ## Common flags
 
 - `-DCMAKE_BUILD_TYPE=<Debug|Release...>` define the build type, see CMake manual for more details
 - `-DCMAKE_INSTALL_PREFIX=/installation/path` override this value to define a custom install prefix. Default: `/usr/local`
-- `-G<Generator Name>` generate build files for "Generator Name". By default, CMake will generate `Makefile`s.
+- `-G"<Generator Name>"` generate build files for "Generator Name". By default, CMake will generate `Makefile`s.
 
 ## Verbose build
 

diff --git a/cmake/Modules/Utils.cmake b/cmake/Modules/Utils.cmake
@@ -100,3 +100,16 @@ function (valkey_parse_build_option OPTION_VALUE OUT_ARG_ENUM)
             PARENT_SCOPE)
     endif ()
 endfunction ()
+
+function (valkey_pkg_config PKGNAME OUT_VARIABLE)
+    if (NOT FOUND_PKGCONFIG)
+        # Locate pkg-config once
+        find_package(PkgConfig REQUIRED)
+        set(FOUND_PKGCONFIG 1)
+    endif ()
+    pkg_check_modules(__PREFIX REQUIRED ${PKGNAME})
+    message(STATUS "Found library for '${PKGNAME}': ${__PREFIX_LIBRARIES}")
+    set(${OUT_VARIABLE}
+        "${__PREFIX_LIBRARIES}"
+        PARENT_SCOPE)
+endfunction ()
diff --git a/cmake/Modules/ValkeySetup.cmake b/cmake/Modules/ValkeySetup.cmake
@@ -74,9 +74,11 @@ endmacro ()
 macro (valkey_build_and_install_bin target sources ld_flags libs link_name)
     add_executable(${target} ${sources})
 
-    if (USE_JEMALLOC)
-        # Using jemalloc
-        target_link_libraries(${target} jemalloc)
+    if (USE_JEMALLOC
+        OR USE_TCMALLOC
+        OR USE_TCMALLOC_MINIMAL)
+        # Using custom allocator
+        target_link_libraries(${target} ${ALLOCATOR_LIB})
     endif ()
 
     # Place this line last to ensure that ${ld_flags} is placed last on the linker line
@@ -151,16 +153,23 @@ endif ()
 if (BUILD_MALLOC)
     if ("${BUILD_MALLOC}" STREQUAL "jemalloc")
         set(MALLOC_LIB "jemalloc")
+        set(ALLOCATOR_LIB "jemalloc")
         add_valkey_server_compiler_options("-DUSE_JEMALLOC")
         set(USE_JEMALLOC 1)
     elseif ("${BUILD_MALLOC}" STREQUAL "libc")
         set(MALLOC_LIB "libc")
     elseif ("${BUILD_MALLOC}" STREQUAL "tcmalloc")
         set(MALLOC_LIB "tcmalloc")
+        valkey_pkg_config(libtcmalloc ALLOCATOR_LIB)
+
         add_valkey_server_compiler_options("-DUSE_TCMALLOC")
+        set(USE_TCMALLOC 1)
     elseif ("${BUILD_MALLOC}" STREQUAL "tcmalloc_minimal")
         set(MALLOC_LIB "tcmalloc_minimal")
+        valkey_pkg_config(libtcmalloc_minimal ALLOCATOR_LIB)
+
         add_valkey_server_compiler_options("-DUSE_TCMALLOC")
+        set(USE_TCMALLOC_MINIMAL 1)
     else ()
         message(FATAL_ERROR "BUILD_MALLOC can be one of: jemalloc, libc, tcmalloc or tcmalloc_minimal")
     endif ()
@@ -202,16 +211,12 @@ if (BUILD_RDMA)
         if (USE_RDMA EQUAL 2) # Module
             message(STATUS "Building RDMA as module")
             add_valkey_server_compiler_options("-DUSE_RDMA=2")
-            find_package(PkgConfig REQUIRED)
 
             # Locate librdmacm & libibverbs, fail if we can't find them
-            pkg_check_modules(RDMACM REQUIRED librdmacm)
-            pkg_check_modules(IBVERBS REQUIRED libibverbs)
+            valkey_pkg_config(librdmacm RDMACM_LIBS)
+            valkey_pkg_config(libibverbs IBVERBS_LIBS)
 
-            message(STATUS "${RDMACM_LINK_LIBRARIES};${IBVERBS_LINK_LIBRARIES}")
-            list(APPEND RDMA_LIBS "${RDMACM_LIBRARIES};${IBVERBS_LIBRARIES}")
-            unset(RDMACM_LINK_LIBRARIES CACHE)
-            unset(IBVERBS_LINK_LIBRARIES CACHE)
+            list(APPEND RDMA_LIBS "${RDMACM_LIBS};${IBVERBS_LIBS}")
             set(BUILD_RDMA_MODULE 1)
         elseif (USE_RDMA EQUAL 1)
             # RDMA can only be built as a module. So disable it
@@ -266,17 +271,18 @@ endif ()
 
 # Sanitizer
 if (BUILD_SANITIZER)
-    # For best results, force libc
-    set(MALLOC_LIB, "libc")
+    # Common CFLAGS
+    list(APPEND VALKEY_SANITAIZER_CFLAGS "-fno-sanitize-recover=all")
+    list(APPEND VALKEY_SANITAIZER_CFLAGS "-fno-omit-frame-pointer")
     if ("${BUILD_SANITIZER}" STREQUAL "address")
-        add_valkey_server_compiler_options("-fsanitize=address -fno-sanitize-recover=all -fno-omit-frame-pointer")
-        add_valkey_server_linker_option("-fsanitize=address")
+        list(APPEND VALKEY_SANITAIZER_CFLAGS "-fsanitize=address")
+        list(APPEND VALKEY_SANITAIZER_LDFLAGS "-fsanitize=address")
     elseif ("${BUILD_SANITIZER}" STREQUAL "thread")
-        add_valkey_server_compiler_options("-fsanitize=thread -fno-sanitize-recover=all -fno-omit-frame-pointer")
-        add_valkey_server_linker_option("-fsanitize=thread")
+        list(APPEND VALKEY_SANITAIZER_CFLAGS "-fsanitize=thread")
+        list(APPEND VALKEY_SANITAIZER_LDFLAGS "-fsanitize=thread")
     elseif ("${BUILD_SANITIZER}" STREQUAL "undefined")
-        add_valkey_server_compiler_options("-fsanitize=undefined -fno-sanitize-recover=all -fno-omit-frame-pointer")
-        add_valkey_server_linker_option("-fsanitize=undefined")
+        list(APPEND VALKEY_SANITAIZER_CFLAGS "-fsanitize=undefined")
+        list(APPEND VALKEY_SANITAIZER_LDFLAGS "-fsanitize=undefined")
     else ()
         message(FATAL_ERROR "Unknown sanitizer: ${BUILD_SANITIZER}")
     endif ()
@@ -366,7 +372,6 @@ include(SourceFiles)
 
 # Clear the below variables from the cache
 unset(CMAKE_C_FLAGS CACHE)
-unset(BUILD_SANITIZER CACHE)
 unset(VALKEY_SERVER_LDFLAGS CACHE)
 unset(VALKEY_SERVER_CFLAGS CACHE)
 unset(PYTHON_EXE CACHE)

diff --git a/deps/jemalloc/CMakeLists.txt b/deps/jemalloc/CMakeLists.txt
@@ -12,9 +12,18 @@ if (NOT EXISTS ${JEMALLOC_INSTALL_DIR}/lib/libjemalloc.a)
         COMMAND sh -c "${JEMALLOC_SRC_DIR}/configure --disable-cxx \
             --with-version=5.3.0-0-g0 --with-lg-quantum=3 --disable-cache-oblivious --with-jemalloc-prefix=je_ \
             --enable-static --disable-shared --prefix=${JEMALLOC_INSTALL_DIR}"
-        WORKING_DIRECTORY ${JEMALLOC_SRC_DIR} COMMAND_ERROR_IS_FATAL ANY)
+        WORKING_DIRECTORY ${JEMALLOC_SRC_DIR} RESULTS_VARIABLE CONFIGURE_RESULT)
+
+    if (NOT ${CONFIGURE_RESULT} EQUAL 0)
+        message(FATAL_ERROR "Jemalloc configure failed")
+    endif ()
+
     execute_process(COMMAND make -j${VALKEY_PROCESSOR_COUNT} lib/libjemalloc.a install
-                    WORKING_DIRECTORY "${JEMALLOC_SRC_DIR}")
+                    WORKING_DIRECTORY "${JEMALLOC_SRC_DIR}" RESULTS_VARIABLE MAKE_RESULT)
+
+    if (NOT ${MAKE_RESULT} EQUAL 0)
+        message(FATAL_ERROR "Jemalloc build failed")
+    endif ()
 endif ()
 
 # Import the compiled library as a CMake target

diff --git a/deps/lua/CMakeLists.txt b/deps/lua/CMakeLists.txt
@@ -1,5 +1,7 @@
 project(lualib)
 
+include(CheckFunctionExists)
+
 set(LUA_SRC_DIR "${CMAKE_CURRENT_LIST_DIR}/src")
 set(LUA_SRCS
     ${LUA_SRC_DIR}/fpconv.c
@@ -42,3 +44,10 @@ set(LUA_SRCS
 add_library(lualib STATIC "${LUA_SRCS}")
 target_include_directories(lualib PUBLIC "${LUA_SRC_DIR}")
 target_compile_definitions(lualib PRIVATE ENABLE_CJSON_GLOBAL)
+
+# Use mkstemp if available
+check_function_exists(mkstemp HAVE_MKSTEMP)
+if (HAVE_MKSTEMP)
+    target_compile_definitions(lualib PRIVATE LUA_USE_MKSTEMP)
+endif ()
+unset(HAVE_MKSTEMP CACHE)
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -22,6 +22,16 @@ if (VALKEY_RELEASE_BUILD)
     set_property(TARGET valkey-server PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE)
 endif ()
 
+if (BUILD_SANITIZER)
+    # 'BUILD_SANITIZER' is defined in ValkeySetup module (based on user input)
+    # If defined, the variables 'VALKEY_SANITAIZER_CFLAGS' and 'VALKEY_SANITAIZER_LDFLAGS'
+    # are set with the link & compile flags required
+    message(STATUS "Adding sanitizer flags for target valkey-server")
+    target_compile_options(valkey-server PRIVATE ${VALKEY_SANITAIZER_CFLAGS})
+    target_link_options(valkey-server PRIVATE ${VALKEY_SANITAIZER_LDFLAGS})
+endif ()
+unset(BUILD_SANITIZER CACHE)
+
 # Target: valkey-cli
 list(APPEND CLI_LIBS "linenoise")
 valkey_build_and_install_bin(valkey-cli "${VALKEY_CLI_SRCS}" "${VALKEY_SERVER_LDFLAGS}" "${CLI_LIBS}" "redis-cli")