From e660691d443e991ea9d40132d1d3f39fdf766e31 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 1 Sep 2021 14:28:26 +0100 Subject: [PATCH 01/14] CMake cleanup. Modernise and tidy the CMake a bit: - Use generator expressions for a lot of conditionals so that things are more reliable with multi-config generators (and less verbose). - Remove C as a needed language. None of the code was C but we were using C to test if headers worked. This was fragile because a build with `CMAKE_CXX_COMPILER` set might have checked things compiled with the system C compiler and then failed when the specified C++ compiler used different headers. - Rename the `BACKTRACE_HEADER` macro to `SNMALLOC_BACKTRACE_HEADER`. This is exposed into code that consumes snmalloc and so should be 'namespaced' (to the degree that's possible with C macros). - Use functions instead of macros for better variable scoping. - Factor out some duplicated bits into functions. - Update to the latest way of telling CMake to use C++17 or C++20. - Migrate everything that's setting global properties to setting only per-target properties. - Link with -nostdlib++ if it's available. If it isn't, fall back to enabling the C language and linking with the C compiler. - Make the per-test log messages verbose outputs. These kept scrolling important messages off the top of the screen for me. - Make building as a header-only library a public option. - Add install targets that install all of the headers and provide a config option. This works with the header-only configuration for integration with things like vcpkg. - Fix a missing `#endif` in the `malloc_useable_size` check. This was failing co compile on all platforms because of the missing `#endif`. - Bump the minimum version to 3.14 so that we have access to target_link_options. This is necessary to use generator expressions for linker flags. With these changes, it's now possible to install snmalloc and then, in another project, do something like this: ```cmake find_package(snmalloc CONFIG REQUIRED) target_link_libraries(t1 snmalloc::snmalloc) target_link_libraries(t2 snmalloc::snmallocshim-static) ``` In this example, `t1` gets all of the compile flags necessary to include snmalloc headers for its build configuration. `t2` is additionally linked to the snmalloc static shim library. --- CMakeLists.txt | 284 ++++++++++++++++++++++++-------------------- src/pal/pal_posix.h | 4 +- 2 files changed, 157 insertions(+), 131 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c8b0d0da9..bd465ebe4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,5 @@ -cmake_minimum_required(VERSION 3.8) -project(snmalloc C CXX) +cmake_minimum_required(VERSION 3.14) +project(snmalloc CXX) if (NOT CMAKE_BUILD_TYPE) message(STATUS "No build type selected, default to: Release") @@ -7,7 +7,8 @@ if (NOT CMAKE_BUILD_TYPE) endif() include(CheckCXXCompilerFlag) -include(CheckCSourceCompiles) +include(CheckCXXSourceCompiles) +include(CheckLinkerFlag) option(USE_SNMALLOC_STATS "Track allocation stats" OFF) option(SNMALLOC_CI_BUILD "Disable features not sensible for CI" OFF) @@ -18,14 +19,20 @@ option(SNMALLOC_STATIC_LIBRARY "Build static libraries" ON) option(SNMALLOC_QEMU_WORKAROUND "Disable using madvise(DONT_NEED) to zero memory on Linux" Off) option(SNMALLOC_OPTIMISE_FOR_CURRENT_MACHINE "Compile for current machine architecture" Off) set(SNMALLOC_STATIC_LIBRARY_PREFIX "sn_" CACHE STRING "Static library function prefix") +option(SNMALLOC_HEADER_ONLY_LIBRARY "Use snmalloc has a header-only library" OFF) option(SNMALLOC_USE_CXX17 "Build as C++17 for legacy support." OFF) option(SNMALLOC_USE_PTHREAD_DESTRUCTORS "Build using pthread destructors. Reduces the system dependencies, but may interact badly with C++ on some platforms." OFF) +# Define a generator expression for things that will be enabled in either CI +# builds or debug mode. +set(ci_or_debug "$,$>") + # malloc.h will error if you include it on FreeBSD, so this test must not # unconditionally include it. -CHECK_C_SOURCE_COMPILES(" +CHECK_CXX_SOURCE_COMPILES(" #if __has_include() #include +#endif #if __has_include() #include #else @@ -35,9 +42,9 @@ size_t malloc_usable_size(const void* ptr) { return 0; } int main() { return 0; } " CONST_QUALIFIED_MALLOC_USABLE_SIZE) -# older libcs might not have getentropy, e.g. it appeared in gliobc 2.25 +# Some libcs might not have getentropy, e.g. it appeared in glibc 2.25 # so we need to fallback if we cannot compile this -CHECK_C_SOURCE_COMPILES(" +CHECK_CXX_SOURCE_COMPILES(" #if __has_include() # include #endif @@ -51,8 +58,11 @@ int main() { } " SNMALLOC_PLATFORM_HAS_GETENTROPY) -# Provide as macro so other projects can reuse -macro(warnings_high) +# Provide as function so other projects can reuse +# FIXME: This modifies some variables that may or may not be the ones that +# provide flags and so is broken by design. It should be removed once Verona +# no longer uses it. +function(warnings_high) if(MSVC) # Force to always compile with W4 if(CMAKE_CXX_FLAGS MATCHES "/W[0-4]") @@ -68,9 +78,9 @@ macro(warnings_high) endif () add_compile_options(-Wall -Wextra -Werror -Wundef) endif() -endmacro() +endfunction() -macro(clangformat_targets) +function(clangformat_targets) # The clang-format tool is installed under a variety of different names. Try # to find a sensible one. Only look for versions 9 explicitly - we don't # know whether our clang-format file will work with newer versions of the @@ -95,118 +105,86 @@ macro(clangformat_targets) -i ${ALL_SOURCE_FILES}) endif() -endmacro() +endfunction() + +# The main target for snmalloc. This is the exported target for the +# header-only configuration and is used as a dependency for all of the builds +# that compile anything. +add_library(snmalloc INTERFACE) -# Have to set this globally, as can't be set on an interface target. if(SNMALLOC_USE_CXX17) - set(CMAKE_CXX_STANDARD 17) + target_compile_definitions(snmalloc INTERFACE -DSNMALLOC_USE_CXX17) + target_compile_features(snmalloc INTERFACE cxx_std_17) else() - set(CMAKE_CXX_STANDARD 20) + target_compile_features(snmalloc INTERFACE cxx_std_20) endif() -# The main target for snmalloc -add_library(snmalloc_lib INTERFACE) -target_include_directories(snmalloc_lib INTERFACE src/) - -if(SNMALLOC_USE_CXX17) - target_compile_definitions(snmalloc_lib INTERFACE -DSNMALLOC_USE_CXX17) -endif() +# Add header paths. +target_include_directories(snmalloc + INTERFACE + $ + $) if(NOT MSVC) - find_package(Threads REQUIRED COMPONENTS snmalloc_lib) - target_link_libraries(snmalloc_lib INTERFACE ${CMAKE_THREAD_LIBS_INIT}) - if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") - target_link_libraries(snmalloc_lib INTERFACE atomic) - endif() + find_package(Threads REQUIRED COMPONENTS snmalloc) + target_link_libraries(snmalloc INTERFACE + ${CMAKE_THREAD_LIBS_INIT} $<$:atomic>) endif() if (WIN32) set(WIN8COMPAT FALSE CACHE BOOL "Avoid Windows 10 APIs") - if (WIN8COMPAT) - target_compile_definitions(snmalloc_lib INTERFACE -DWINVER=0x0603) - message(STATUS "snmalloc: Avoiding Windows 10 APIs") - else() - message(STATUS "snmalloc: Using Windows 10 APIs") - # VirtualAlloc2 is exposed by mincore.lib, not Kernel32.lib (as the - # documentation says) - target_link_libraries(snmalloc_lib INTERFACE mincore) - endif() + target_compile_definitions(snmalloc INTERFACE $<$) + # VirtualAlloc2 is exposed by mincore.lib, not Kernel32.lib (as the + # documentation says) + target_link_libraries(snmalloc INTERFACE $<$>:mincore>) + message(STATUS "snmalloc: Avoiding Windows 10 APIs is ${WIN8COMPAT}") endif() -# detect support for cmpxchg16b; werror is needed to make sure mcx16 must be used by targets +# Detect support for cmpxchg16b; Werror is needed to make sure mcx16 must be used by targets check_cxx_compiler_flag("-Werror -Wextra -Wall -mcx16" SNMALLOC_COMPILER_SUPPORT_MCX16) if(SNMALLOC_COMPILER_SUPPORT_MCX16) - target_compile_options(snmalloc_lib INTERFACE $<$:-mcx16>) -endif() - -if(USE_SNMALLOC_STATS) - target_compile_definitions(snmalloc_lib INTERFACE -DUSE_SNMALLOC_STATS) -endif() - -if(SNMALLOC_QEMU_WORKAROUND) - target_compile_definitions(snmalloc_lib INTERFACE -DSNMALLOC_QEMU_WORKAROUND) -endif() - -if(SNMALLOC_CI_BUILD) - target_compile_definitions(snmalloc_lib INTERFACE -DSNMALLOC_CI_BUILD) -endif() - -if(SNMALLOC_PLATFORM_HAS_GETENTROPY) - target_compile_definitions(snmalloc_lib INTERFACE -DSNMALLOC_PLATFORM_HAS_GETENTROPY) + target_compile_options(snmalloc INTERFACE $<$:-mcx16>) endif() -if(SNMALLOC_USE_PTHREAD_DESTRUCTORS) - target_compile_definitions(snmalloc_lib INTERFACE -DSNMALLOC_USE_PTHREAD_DESTRUCTORS) +# Helper function that conditionally defines a macro for the build target if +# the CMake variable of the same name is set. +function(add_as_define FLAG) + target_compile_definitions(snmalloc INTERFACE $<$:${FLAG}>) +endfunction() + +add_as_define(USE_SNMALLOC_STATS) +add_as_define(SNMALLOC_QEMU_WORKAROUND) +add_as_define(SNMALLOC_CI_BUILD) +add_as_define(SNMALLOC_PLATFORM_HAS_GETENTROPY) +add_as_define(SNMALLOC_USE_PTHREAD_DESTRUCTORS) +target_compile_definitions(snmalloc INTERFACE $<$:MALLOC_USABLE_SIZE_QUALIFIER=const>) + +# In debug and CI builds, link the backtrace library so that we can get stack +# traces on errors. +find_package(Backtrace) +if(${Backtrace_FOUND}) + target_compile_definitions(snmalloc INTERFACE + $<${ci_or_debug}:SNMALLOC_BACKTRACE_HEADER="${Backtrace_HEADER}">) + target_link_libraries(snmalloc INTERFACE + $<${ci_or_debug}:${Backtrace_LIBRARIES}>) + target_include_directories(snmalloc INTERFACE + $<${ci_or_debug}:${Backtrace_INCLUDE_DIRS}>) endif() -if(CONST_QUALIFIED_MALLOC_USABLE_SIZE) - target_compile_definitions(snmalloc_lib INTERFACE -DMALLOC_USABLE_SIZE_QUALIFIER=const) +if(MSVC) + target_compile_definitions(snmalloc INTERFACE -D_HAS_EXCEPTIONS=0) +else() + if (NOT CMAKE_SYSTEM_NAME MATCHES "Haiku") + # Get better stack traces in CI and debug builds. + target_link_options(snmalloc INTERFACE $<${ci_or_debug}:-rdynamic>) + endif() endif() -# To build with just the header library target define SNMALLOC_ONLY_HEADER_LIBRARY -# in containing Cmake file. -if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) - - warnings_high() - - if(MSVC) - set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} /Zi") - set(CMAKE_EXE_LINKER_FLAGS_RELEASE "${CMAKE_EXE_LINKER_FLAGS_RELEASE} /DEBUG") - target_compile_definitions(snmalloc_lib INTERFACE -D_HAS_EXCEPTIONS=0) - add_compile_options(/std:c++latest) - else() - add_compile_options(-fno-exceptions -fno-rtti -fomit-frame-pointer -ffunction-sections) - # Static TLS model is unsupported on Haiku. - # All symbols are always dynamic on haiku and -rdynamic is redundant (and unsupported). - if (NOT CMAKE_SYSTEM_NAME MATCHES "Haiku") - add_compile_options(-ftls-model=initial-exec) - if(SNMALLOC_CI_BUILD OR (${CMAKE_BUILD_TYPE} MATCHES "Debug")) - # Get better stack traces in CI and Debug. - target_link_libraries(snmalloc_lib INTERFACE "-rdynamic") - add_compile_options(-g) - endif() - endif() - - if(SNMALLOC_OPTIMISE_FOR_CURRENT_MACHINE) - check_cxx_compiler_flag(-march=native SUPPORT_MARCH_NATIVE) - if (SUPPORT_MARCH_NATIVE) - add_compile_options(-march=native) - else() - message(WARNING "Compiler does not support `-march=native` required by SNMALLOC_OPTIMISE_FOR_CURRENT_MACHINE") - endif() - endif() +# To build with just the header library target define SNMALLOC_HEADER_ONLY_LIBRARY +if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) - find_package(Backtrace) - if(${Backtrace_FOUND}) - target_compile_definitions(snmalloc_lib INTERFACE -DBACKTRACE_HEADER="${Backtrace_HEADER}") - target_link_libraries(snmalloc_lib INTERFACE ${Backtrace_LIBRARIES}) - target_include_directories(snmalloc_lib INTERFACE ${Backtrace_INCLUDE_DIRS}) - endif() - - endif() - - macro(subdirlist result curdir) + function(subdirlist result curdir) file(GLOB children CONFIGURE_DEPENDS LIST_DIRECTORIES true RELATIVE ${curdir} ${curdir}/* ) set(dirlist "") foreach(child ${children}) @@ -214,43 +192,70 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) list(APPEND dirlist ${child}) endif() endforeach() - set(${result} ${dirlist}) - endmacro() + set(${result} ${dirlist} PARENT_SCOPE) + endfunction() - if(CMAKE_VERSION VERSION_LESS 3.14) - set(CMAKE_REQUIRED_LIBRARIES -fuse-ld=lld) - else() - set(CMAKE_REQUIRED_LINK_OPTIONS -fuse-ld=lld) - endif() + set(CMAKE_REQUIRED_LINK_OPTIONS -fuse-ld=lld) check_cxx_source_compiles("int main() { return 1; }" LLD_WORKS) + if (LLD_WORKS) + message(STATUS "Using LLD to link snmalloc shims") + endif() - macro(add_shim name type) + function(add_shim name type) add_library(${name} ${type} ${ARGN}) - target_link_libraries(${name} snmalloc_lib) - if(NOT MSVC) - target_compile_definitions(${name} PRIVATE "SNMALLOC_EXPORT=__attribute__((visibility(\"default\")))") - endif() + target_link_libraries(${name} snmalloc) set_target_properties(${name} PROPERTIES CXX_VISIBILITY_PRESET hidden) - # Ensure that we do not link against C++ stdlib when compiling shims. - if(NOT MSVC) - set_target_properties(${name} PROPERTIES LINKER_LANGUAGE C) - if (LLD_WORKS) - message("Using LLD.") + if(MSVC) + target_compile_options(${name} /Zi /W4 /WX /wd4127 /wd4324 /wd4201) + set(CMAKE_EXE_LINKER_FLAGS_RELEASE "${CMAKE_EXE_LINKER_FLAGS_RELEASE} /DEBUG") + else() + target_compile_definitions(${name} PRIVATE "SNMALLOC_EXPORT=__attribute__((visibility(\"default\")))") + target_compile_options(${name} PRIVATE + -fno-exceptions -fno-rtti -fomit-frame-pointer -ffunction-sections + -Wall -Wextra -Werror -Wundef + $<$:-Wsign-conversion -Wconversion>) + # Static TLS model is unsupported on Haiku. + # All symbols are always dynamic on haiku and -rdynamic is redundant (and unsupported). + if (NOT CMAKE_SYSTEM_NAME MATCHES "Haiku") + target_compile_options(${name} PRIVATE -ftls-model=initial-exec) + target_compile_options(${name} PRIVATE $<$:-g>) + endif() + + if(SNMALLOC_OPTIMISE_FOR_CURRENT_MACHINE) + check_cxx_compiler_flag(-march=native SUPPORT_MARCH_NATIVE) + if (SUPPORT_MARCH_NATIVE) + target_compile_options(${name} -march=native) + else() + message(WARNING "Compiler does not support `-march=native` required by SNMALLOC_OPTIMISE_FOR_CURRENT_MACHINE") + endif() + endif() + + # Ensure that we do not link against C++ stdlib when compiling shims. + # If the compiler supports excluding the C++ stdlib implementation, use + # it. Otherwise, fall back to linking the library as if it were C, which + # has roughly the same effect. + check_linker_flag(CXX "-nostdlib++" SNMALLOC_LINKER_SUPPORT_NOSTDLIBXX) + if (SNMALLOC_LINKER_SUPPORT_NOSTDLIBXX) + target_link_options(${name} PRIVATE -nostdlib++) + else() + enable_language(C) + set_target_properties(${name} PROPERTIES LINKER_LANGUAGE C) + endif() # Only include the dependencies that actually are touched. # Required so C++ library is not included with direct pthread access. - target_link_libraries(${name} -Wl,--as-needed) - + target_link_options(${name} PRIVATE $<$:-Wl,--as-needed>) # Remove all the duplicate new/malloc and free/delete definitions - target_link_libraries(${name} -Wl,--icf=all -fuse-ld=lld) - endif() + target_link_options(${name} PRIVATE $<$:-Wl,--icf=all -fuse-ld=lld>) endif() - endmacro() + install(TARGETS ${name} EXPORT snmallocConfig) + + endfunction() if (SNMALLOC_STATIC_LIBRARY) - add_shim(snmallocshim-static STATIC src/override/malloc.cc) + add_shim(snmallocshim-static STATIC src/override/new.cc) target_compile_definitions(snmallocshim-static PRIVATE SNMALLOC_STATIC_LIBRARY_PREFIX=${SNMALLOC_STATIC_LIBRARY_PREFIX}) endif () @@ -274,6 +279,7 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) subdirlist(TEST_CATEGORIES ${TESTDIR}) list(REVERSE TEST_CATEGORIES) foreach(TEST_CATEGORY ${TEST_CATEGORIES}) + message(STATUS "Adding ${TEST_CATEGORY} tests") subdirlist(TESTS ${TESTDIR}/${TEST_CATEGORY}) foreach(TEST ${TESTS}) if (WIN32 @@ -301,31 +307,31 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) if (${FLAVOUR} STREQUAL "check") target_compile_definitions(${TESTNAME} PRIVATE SNMALLOC_CHECK_CLIENT) endif() - target_link_libraries(${TESTNAME} snmalloc_lib) + target_link_libraries(${TESTNAME} snmalloc) if (${TEST} MATCHES "release-.*") - message(STATUS "Adding test: ${TESTNAME} only for release configs") + message(VERBOSE "Adding test: ${TESTNAME} only for release configs") add_test(NAME ${TESTNAME} COMMAND ${TESTNAME} CONFIGURATIONS "Release") else() - message(STATUS "Adding test: ${TESTNAME}") + message(VERBOSE "Adding test: ${TESTNAME}") add_test(${TESTNAME} ${TESTNAME}) endif() if (${TEST_CATEGORY} MATCHES "perf") - message(STATUS "Single threaded test: ${TESTNAME}") + message(VERBOSE "Single threaded test: ${TESTNAME}") set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) endif() if(WIN32) # On Windows these tests use a lot of memory as it doesn't support # lazy commit. if (${TEST} MATCHES "two_alloc_types") - message(STATUS "Single threaded test: ${TESTNAME}") + message(VERBOSE "Single threaded test: ${TESTNAME}") set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) endif() if (${TEST} MATCHES "fixed_region") - message(STATUS "Single threaded test: ${TESTNAME}") + message(VERBOSE "Single threaded test: ${TESTNAME}") set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) endif() if (${TEST} MATCHES "memory") - message(STATUS "Single threaded test: ${TESTNAME}") + message(VERBOSE "Single threaded test: ${TESTNAME}") set_tests_properties(${TESTNAME} PROPERTIES PROCESSORS 4) endif() endif() @@ -338,3 +344,23 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) clangformat_targets() endif() + +install(TARGETS snmalloc EXPORT snmallocConfig) + +install(TARGETS EXPORT snmallocConfig DESTINATION ${CMAKE_INSTALL_LIBDIR} + PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/snmalloc) + +install(DIRECTORY src/aal DESTINATION include/snmalloc) +install(DIRECTORY src/ds DESTINATION include/snmalloc) +install(DIRECTORY src/override DESTINATION include/snmalloc) +install(DIRECTORY src/backend DESTINATION include/snmalloc) +install(DIRECTORY src/mem DESTINATION include/snmalloc) +install(DIRECTORY src/pal DESTINATION include/snmalloc) +install(FILES src/snmalloc.h;src/snmalloc_core.h;src/snmalloc_front.h DESTINATION include/snmalloc) + +install(EXPORT snmallocConfig + FILE snmalloc-config.cmake + NAMESPACE snmalloc:: + DESTINATION "share/snmalloc" +) + diff --git a/src/pal/pal_posix.h b/src/pal/pal_posix.h index ce932fdca..b087c9298 100644 --- a/src/pal/pal_posix.h +++ b/src/pal/pal_posix.h @@ -4,8 +4,8 @@ # include #endif #include "../ds/address.h" -#if defined(BACKTRACE_HEADER) -# include BACKTRACE_HEADER +#if defined(SNMALLOC_BACKTRACE_HEADER) +# include SNMALLOC_BACKTRACE_HEADER #endif #include #include From 9a6a762637f79f919dfbaeea3d347e361e04126b Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 1 Sep 2021 15:03:31 +0100 Subject: [PATCH 02/14] Fix on cmake < 3.18 and with gcc. --- CMakeLists.txt | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bd465ebe4..c4ce36a84 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,6 @@ endif() include(CheckCXXCompilerFlag) include(CheckCXXSourceCompiles) -include(CheckLinkerFlag) option(USE_SNMALLOC_STATS "Track allocation stats" OFF) option(SNMALLOC_CI_BUILD "Disable features not sensible for CI" OFF) @@ -23,6 +22,23 @@ option(SNMALLOC_HEADER_ONLY_LIBRARY "Use snmalloc has a header-only library" OFF option(SNMALLOC_USE_CXX17 "Build as C++17 for legacy support." OFF) option(SNMALLOC_USE_PTHREAD_DESTRUCTORS "Build using pthread destructors. Reduces the system dependencies, but may interact badly with C++ on some platforms." OFF) +if (NOT MSVC) + # If CheckLinkerFlag doesn't exist then provide a dummy implementation that + # always fails. The fallback can be removed when we move to CMake 3.18 as the + # baseline. + include(CheckLinkerFlag OPTIONAL RESULT_VARIABLE CHECK_LINKER_FLAG) + if (NOT CHECK_LINKER_FLAG) + function(check_linker_flag) + endfunction() + endif () + # If the target compiler doesn't support -nostdlib++ then we must enable C at + # the global scope for the fallbacks to work. + check_linker_flag(CXX "-nostdlib++" SNMALLOC_LINKER_SUPPORT_NOSTDLIBXX) + if (NOT SNMALLOC_LINKER_SUPPORT_NOSTDLIBXX AND NOT SNMALLOC_HEADER_ONLY_LIBRARY) + enable_language(C) + endif() +endif() + # Define a generator expression for things that will be enabled in either CI # builds or debug mode. set(ci_or_debug "$,$>") @@ -239,7 +255,6 @@ if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) if (SNMALLOC_LINKER_SUPPORT_NOSTDLIBXX) target_link_options(${name} PRIVATE -nostdlib++) else() - enable_language(C) set_target_properties(${name} PROPERTIES LINKER_LANGUAGE C) endif() From 60efd1c1db1198cc4349a6d1a0a592a6534dd3a0 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 1 Sep 2021 15:04:00 +0100 Subject: [PATCH 03/14] Fix on Windows. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c4ce36a84..0d0f4c51b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -223,7 +223,7 @@ if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) set_target_properties(${name} PROPERTIES CXX_VISIBILITY_PRESET hidden) if(MSVC) - target_compile_options(${name} /Zi /W4 /WX /wd4127 /wd4324 /wd4201) + target_compile_options(${name} PRIVATE /Zi /W4 /WX /wd4127 /wd4324 /wd4201) set(CMAKE_EXE_LINKER_FLAGS_RELEASE "${CMAKE_EXE_LINKER_FLAGS_RELEASE} /DEBUG") else() target_compile_definitions(${name} PRIVATE "SNMALLOC_EXPORT=__attribute__((visibility(\"default\")))") From 096544e05bac72e869b062c958ad213b2b10ad75 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 1 Sep 2021 15:12:10 +0100 Subject: [PATCH 04/14] Use C++17 for CI builds on old Ubuntu. --- .github/workflows/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index c04e28d8b..1592ff9fa 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -18,12 +18,12 @@ jobs: # Build each combination of OS and release/debug variants os: [ "ubuntu-latest", "ubuntu-18.04", "macos-11", "macos-10.15", "freebsd-12.2", "freebsd-13.0" ] build-type: [ Release, Debug ] - extra-cmake-flags: [ " " ] # Modify the complete matrix include: # Provide the dependency installation for each platform - os: "ubuntu-18.04" dependencies: "sudo apt install ninja-build" + cmake-flags: "-DSNMALLOC_USE_CXX17=ON" - os: "ubuntu-latest" dependencies: "sudo apt install ninja-build" - os: "macos-11" @@ -68,7 +68,7 @@ jobs: - name: Install build dependencies run: ${{ matrix.dependencies }} - name: Configure CMake - run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{matrix.build-type}} -G Ninja ${{ matrix.extra-cmake-flags }} + run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{matrix.build-type}} -G Ninja ${{ matrix.cmake-flags }} ${{ matrix.extra-cmake-flags }} # Build with a nice ninja status line - name: Build working-directory: ${{github.workspace}}/build From 226ee281bc10e54d0ae080173e33449311d5ea6a Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 1 Sep 2021 15:14:10 +0100 Subject: [PATCH 05/14] Also use C++17 for the clang-format builds. These are on old-Ubuntu because we depend on an old clang-format. --- .github/workflows/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1592ff9fa..4b0a59035 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -201,7 +201,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: Configure CMake - run: cmake -B ${{github.workspace}}/build + run: cmake -B ${{github.workspace}}/build -DSNMALLOC_USE_CXX17=ON - name: Install clang-tidy run: sudo apt install clang-tidy-9 # Run the clang-format check and error if it generates a diff @@ -213,7 +213,7 @@ jobs: git diff --exit-code - name: Run clang-tidy run: | - clang-tidy-9 src/override/malloc.cc -header-filter="`pwd`/*" -warnings-as-errors='*' -export-fixes=tidy.fail -- -std=c++17 -mcx16 -DSNMALLOC_PLATFORM_HAS_GETENTROPY=0 + clang-tidy-9 src/override/malloc.cc -header-filter="`pwd`/*" -warnings-as-errors='*' -export-fixes=tidy.fail -- -std=c++17 -mcx16 -DSNMALLOC_PLATFORM_HAS_GETENTROPY=0 if [ -f tidy.fail ] ; then cat tidy.fail exit 1 From 8ac4448f86540d6ba192f61f7b7c84bf666b0187 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 1 Sep 2021 16:23:24 +0100 Subject: [PATCH 06/14] Fix the build on Windows. --- CMakeLists.txt | 2 +- src/override/new.cc | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0d0f4c51b..26072f69f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -149,7 +149,7 @@ endif() if (WIN32) set(WIN8COMPAT FALSE CACHE BOOL "Avoid Windows 10 APIs") - target_compile_definitions(snmalloc INTERFACE $<$) + target_compile_definitions(snmalloc INTERFACE $<$:WINVER=0x0603>) # VirtualAlloc2 is exposed by mincore.lib, not Kernel32.lib (as the # documentation says) target_link_libraries(snmalloc INTERFACE $<$>:mincore>) diff --git a/src/override/new.cc b/src/override/new.cc index 96b1c94d2..c0acc953a 100644 --- a/src/override/new.cc +++ b/src/override/new.cc @@ -1,7 +1,11 @@ #include "malloc.cc" #ifdef _WIN32 -# define EXCEPTSPEC +# ifdef __clang__ +# define EXCEPTSPEC noexcept +# else +# define EXCEPTSPEC +# endif #else # ifdef _GLIBCXX_USE_NOEXCEPT # define EXCEPTSPEC _GLIBCXX_USE_NOEXCEPT From 6b45033fd8eb27a228c9eee260c20ccb9556ffb4 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 1 Sep 2021 16:36:38 +0100 Subject: [PATCH 07/14] Really prevent linking libstdc++ and hopefully fix CI. --- .github/workflows/main.yml | 2 ++ CMakeLists.txt | 2 ++ 2 files changed, 4 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 4b0a59035..8e02008c1 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -18,6 +18,7 @@ jobs: # Build each combination of OS and release/debug variants os: [ "ubuntu-latest", "ubuntu-18.04", "macos-11", "macos-10.15", "freebsd-12.2", "freebsd-13.0" ] build-type: [ Release, Debug ] + extra-cmake-flags: [ "" ] # Modify the complete matrix include: # Provide the dependency installation for each platform @@ -41,6 +42,7 @@ jobs: - os: "ubuntu-latest" build-type: Debug self-host: true + extra-cmake-flags: "-DSNMALLOC_USE_PTHREAD_DESTRUCTORS=On" # Extra build to check using pthread library for destructing local state. - os: "ubuntu-latest" variant: "Ubuntu (with pthread destructors)." diff --git a/CMakeLists.txt b/CMakeLists.txt index 26072f69f..ca3fd068a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,6 +22,8 @@ option(SNMALLOC_HEADER_ONLY_LIBRARY "Use snmalloc has a header-only library" OFF option(SNMALLOC_USE_CXX17 "Build as C++17 for legacy support." OFF) option(SNMALLOC_USE_PTHREAD_DESTRUCTORS "Build using pthread destructors. Reduces the system dependencies, but may interact badly with C++ on some platforms." OFF) +set(CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "") + if (NOT MSVC) # If CheckLinkerFlag doesn't exist then provide a dummy implementation that # always fails. The fallback can be removed when we move to CMake 3.18 as the From 1977459dbd2a80e9990452c527e0fd7fae4468fd Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 1 Sep 2021 17:55:58 +0100 Subject: [PATCH 08/14] Improve how cleanup is exposed in cmake. --- CMakeLists.txt | 53 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ca3fd068a..0e726b540 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,18 @@ endif() include(CheckCXXCompilerFlag) include(CheckCXXSourceCompiles) +include(CMakeDependentOption) + +if (${CMAKE_SYSTEM_NAME} EQUAL FreeBSD) + set(SNMALLOC_CLEANUP_DEFAULT THREAD_CLEANUP) +elseif (UNIX) + set(SNMALLOC_CLEANUP_DEFAULT PTHREAD_DESTRUCTORS) +else () + set(SNMALLOC_CLEANUP_DEFAULT CXX11_DESTRUCTORS) +endif() + +option(SNMALLOC_HEADER_ONLY_LIBRARY "Use snmalloc has a header-only library" OFF) option(USE_SNMALLOC_STATS "Track allocation stats" OFF) option(SNMALLOC_CI_BUILD "Disable features not sensible for CI" OFF) option(EXPOSE_EXTERNAL_PAGEMAP "Expose the global pagemap" OFF) @@ -18,13 +29,15 @@ option(SNMALLOC_STATIC_LIBRARY "Build static libraries" ON) option(SNMALLOC_QEMU_WORKAROUND "Disable using madvise(DONT_NEED) to zero memory on Linux" Off) option(SNMALLOC_OPTIMISE_FOR_CURRENT_MACHINE "Compile for current machine architecture" Off) set(SNMALLOC_STATIC_LIBRARY_PREFIX "sn_" CACHE STRING "Static library function prefix") -option(SNMALLOC_HEADER_ONLY_LIBRARY "Use snmalloc has a header-only library" OFF) option(SNMALLOC_USE_CXX17 "Build as C++17 for legacy support." OFF) -option(SNMALLOC_USE_PTHREAD_DESTRUCTORS "Build using pthread destructors. Reduces the system dependencies, but may interact badly with C++ on some platforms." OFF) +set(SNMALLOC_CLEANUP ${SNMALLOC_CLEANUP_DEFAULT} CACHE STRING "The mechanism that snmalloc will use for thread destructors. Valid options are: CXX11_DESTRUCTORS (use C++11 destructors, may depend on the C++ runtime library), PTHREAD_DESTRUCTORS (use pthreads, may interact badly with C++ on some platforms, such as macOS) THREAD_CLEANUP (depend on an explicit call to _malloc_thread_cleanup on thread exit, supported by FreeBSD's threading implementation and possibly elsewhere)") +set_property(CACHE SNMALLOC_CLEANUP PROPERTY STRINGS THREAD_CLEANUP PTHREAD_DESTRUCTORS CXX11_DESTRUCTORS) -set(CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "") +if (NOT SNMALLOC_CLEANUP STREQUAL CXX11_DESTRUCTORS) + set(CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "") +endif() -if (NOT MSVC) +if (NOT MSVC AND NOT (SNMALLOC_CLEANUP STREQUAL CXX11_DESTRUCTORS)) # If CheckLinkerFlag doesn't exist then provide a dummy implementation that # always fails. The fallback can be removed when we move to CMake 3.18 as the # baseline. @@ -174,7 +187,6 @@ add_as_define(USE_SNMALLOC_STATS) add_as_define(SNMALLOC_QEMU_WORKAROUND) add_as_define(SNMALLOC_CI_BUILD) add_as_define(SNMALLOC_PLATFORM_HAS_GETENTROPY) -add_as_define(SNMALLOC_USE_PTHREAD_DESTRUCTORS) target_compile_definitions(snmalloc INTERFACE $<$:MALLOC_USABLE_SIZE_QUALIFIER=const>) # In debug and CI builds, link the backtrace library so that we can get stack @@ -192,7 +204,7 @@ endif() if(MSVC) target_compile_definitions(snmalloc INTERFACE -D_HAS_EXCEPTIONS=0) else() - if (NOT CMAKE_SYSTEM_NAME MATCHES "Haiku") + if (NOT CMAKE_SYSTEM_NAME STREQUAL "Haiku") # Get better stack traces in CI and debug builds. target_link_options(snmalloc INTERFACE $<${ci_or_debug}:-rdynamic>) endif() @@ -223,6 +235,7 @@ if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) add_library(${name} ${type} ${ARGN}) target_link_libraries(${name} snmalloc) set_target_properties(${name} PROPERTIES CXX_VISIBILITY_PRESET hidden) + target_compile_definitions(${name} PRIVATE "SNMALLOC_USE_${SNMALLOC_CLEANUP}") if(MSVC) target_compile_options(${name} PRIVATE /Zi /W4 /WX /wd4127 /wd4324 /wd4201) @@ -235,7 +248,7 @@ if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) $<$:-Wsign-conversion -Wconversion>) # Static TLS model is unsupported on Haiku. # All symbols are always dynamic on haiku and -rdynamic is redundant (and unsupported). - if (NOT CMAKE_SYSTEM_NAME MATCHES "Haiku") + if (NOT CMAKE_SYSTEM_NAME STREQUAL "Haiku") target_compile_options(${name} PRIVATE -ftls-model=initial-exec) target_compile_options(${name} PRIVATE $<$:-g>) endif() @@ -253,18 +266,16 @@ if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) # If the compiler supports excluding the C++ stdlib implementation, use # it. Otherwise, fall back to linking the library as if it were C, which # has roughly the same effect. - check_linker_flag(CXX "-nostdlib++" SNMALLOC_LINKER_SUPPORT_NOSTDLIBXX) - if (SNMALLOC_LINKER_SUPPORT_NOSTDLIBXX) - target_link_options(${name} PRIVATE -nostdlib++) - else() - set_target_properties(${name} PROPERTIES LINKER_LANGUAGE C) + if (NOT ${SNMALLOC_CLEANUP} STREQUAL CXX11_DESTRUCTORS) + check_linker_flag(CXX "-nostdlib++" SNMALLOC_LINKER_SUPPORT_NOSTDLIBXX) + if (SNMALLOC_LINKER_SUPPORT_NOSTDLIBXX) + target_link_options(${name} PRIVATE -nostdlib++) + else() + set_target_properties(${name} PROPERTIES LINKER_LANGUAGE C) + endif() endif() - - # Only include the dependencies that actually are touched. - # Required so C++ library is not included with direct pthread access. - target_link_options(${name} PRIVATE $<$:-Wl,--as-needed>) - # Remove all the duplicate new/malloc and free/delete definitions - target_link_options(${name} PRIVATE $<$:-Wl,--icf=all -fuse-ld=lld>) + # Remove all the duplicate new/malloc and free/delete definitions + target_link_options(${name} PRIVATE $<$:-Wl,--icf=all -fuse-ld=lld>) endif() install(TARGETS ${name} EXPORT snmallocConfig) @@ -295,6 +306,11 @@ if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) set(TESTDIR ${CMAKE_CURRENT_SOURCE_DIR}/src/test) subdirlist(TEST_CATEGORIES ${TESTDIR}) list(REVERSE TEST_CATEGORIES) + if (${SNMALLOC_CLEANUP} STREQUAL THREAD_CLEANUP) + set(TEST_CLEANUP PTHREAD_DESTRUCTORS) + else () + set(TEST_CLEANUP ${SNMALLOC_CLEANUP}) + endif() foreach(TEST_CATEGORY ${TEST_CATEGORIES}) message(STATUS "Adding ${TEST_CATEGORY} tests") subdirlist(TESTS ${TESTDIR}/${TEST_CATEGORY}) @@ -325,6 +341,7 @@ if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) target_compile_definitions(${TESTNAME} PRIVATE SNMALLOC_CHECK_CLIENT) endif() target_link_libraries(${TESTNAME} snmalloc) + target_compile_definitions(${TESTNAME} PRIVATE "SNMALLOC_USE_${TEST_CLEANUP}") if (${TEST} MATCHES "release-.*") message(VERBOSE "Adding test: ${TESTNAME} only for release configs") add_test(NAME ${TESTNAME} COMMAND ${TESTNAME} CONFIGURATIONS "Release") From 77bdf39d1ce0a024b9d9b70f8306debc17a147e1 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 1 Sep 2021 18:13:26 +0100 Subject: [PATCH 09/14] Clean up options a bit. Options that affect tests or shims should not appear when configuring as a header-only library. --- CMakeLists.txt | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0e726b540..a08a4fefe 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,28 +10,34 @@ include(CheckCXXCompilerFlag) include(CheckCXXSourceCompiles) include(CMakeDependentOption) -if (${CMAKE_SYSTEM_NAME} EQUAL FreeBSD) - set(SNMALLOC_CLEANUP_DEFAULT THREAD_CLEANUP) -elseif (UNIX) - set(SNMALLOC_CLEANUP_DEFAULT PTHREAD_DESTRUCTORS) -else () - set(SNMALLOC_CLEANUP_DEFAULT CXX11_DESTRUCTORS) -endif() - - option(SNMALLOC_HEADER_ONLY_LIBRARY "Use snmalloc has a header-only library" OFF) +# Options that apply globally option(USE_SNMALLOC_STATS "Track allocation stats" OFF) option(SNMALLOC_CI_BUILD "Disable features not sensible for CI" OFF) -option(EXPOSE_EXTERNAL_PAGEMAP "Expose the global pagemap" OFF) -option(EXPOSE_EXTERNAL_RESERVE "Expose an interface to reserve memory using the default memory provider" OFF) -option(SNMALLOC_RUST_SUPPORT "Build static library for rust" OFF) -option(SNMALLOC_STATIC_LIBRARY "Build static libraries" ON) option(SNMALLOC_QEMU_WORKAROUND "Disable using madvise(DONT_NEED) to zero memory on Linux" Off) -option(SNMALLOC_OPTIMISE_FOR_CURRENT_MACHINE "Compile for current machine architecture" Off) -set(SNMALLOC_STATIC_LIBRARY_PREFIX "sn_" CACHE STRING "Static library function prefix") option(SNMALLOC_USE_CXX17 "Build as C++17 for legacy support." OFF) -set(SNMALLOC_CLEANUP ${SNMALLOC_CLEANUP_DEFAULT} CACHE STRING "The mechanism that snmalloc will use for thread destructors. Valid options are: CXX11_DESTRUCTORS (use C++11 destructors, may depend on the C++ runtime library), PTHREAD_DESTRUCTORS (use pthreads, may interact badly with C++ on some platforms, such as macOS) THREAD_CLEANUP (depend on an explicit call to _malloc_thread_cleanup on thread exit, supported by FreeBSD's threading implementation and possibly elsewhere)") -set_property(CACHE SNMALLOC_CLEANUP PROPERTY STRINGS THREAD_CLEANUP PTHREAD_DESTRUCTORS CXX11_DESTRUCTORS) +# Options that apply only if we're not building the header-only library +cmake_dependent_option(SNMALLOC_RUST_SUPPORT "Build static library for rust" OFF "NOT SNMALLOC_HEADER_ONLY_LIBRARY" OFF) +cmake_dependent_option(SNMALLOC_STATIC_LIBRARY "Build static libraries" ON "NOT SNMALLOC_STATIC_LIBRARY" OFF) +cmake_dependent_option(SNMALLOC_OPTIMISE_FOR_CURRENT_MACHINE "Compile for current machine architecture" Off "NOT SNMALLOC_STATIC_LIBRARY" OFF) +if (NOT SNMALLOC_HEADER_ONLY_LIBRARY) + # Pick a sensible default for the thread cleanup mechanism + if (${CMAKE_SYSTEM_NAME} STREQUAL FreeBSD) + set(SNMALLOC_CLEANUP_DEFAULT THREAD_CLEANUP) + elseif (UNIX) + set(SNMALLOC_CLEANUP_DEFAULT PTHREAD_DESTRUCTORS) + else () + set(SNMALLOC_CLEANUP_DEFAULT CXX11_DESTRUCTORS) + endif() + # Specify the thread cleanup mechanism to use. + set(SNMALLOC_CLEANUP ${SNMALLOC_CLEANUP_DEFAULT} CACHE STRING "The mechanism that snmalloc will use for thread destructors. Valid options are: CXX11_DESTRUCTORS (use C++11 destructors, may depend on the C++ runtime library), PTHREAD_DESTRUCTORS (use pthreads, may interact badly with C++ on some platforms, such as macOS) THREAD_CLEANUP (depend on an explicit call to _malloc_thread_cleanup on thread exit, supported by FreeBSD's threading implementation and possibly elsewhere)") + set_property(CACHE SNMALLOC_CLEANUP PROPERTY STRINGS THREAD_CLEANUP PTHREAD_DESTRUCTORS CXX11_DESTRUCTORS) + + set(SNMALLOC_STATIC_LIBRARY_PREFIX "sn_" CACHE STRING "Static library function prefix") +else () + unset(SNMALLOC_STATIC_LIBRARY_PREFIX CACHE) + unset(SNMALLOC_CLEANUP CACHE) +endif () if (NOT SNMALLOC_CLEANUP STREQUAL CXX11_DESTRUCTORS) set(CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "") From 17762693f6e875e468408a1a6ef954832f48eebb Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Thu, 2 Sep 2021 09:23:27 +0100 Subject: [PATCH 10/14] Fix typo, address @mjp41's comments. Also add a linker warning that all symbols are defined and fix the error that it found. --- CMakeLists.txt | 29 ++++++++++++++++++----------- src/mem/threadalloc.h | 11 ++++++++++- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a08a4fefe..bf35219d2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -18,13 +18,13 @@ option(SNMALLOC_QEMU_WORKAROUND "Disable using madvise(DONT_NEED) to zero memory option(SNMALLOC_USE_CXX17 "Build as C++17 for legacy support." OFF) # Options that apply only if we're not building the header-only library cmake_dependent_option(SNMALLOC_RUST_SUPPORT "Build static library for rust" OFF "NOT SNMALLOC_HEADER_ONLY_LIBRARY" OFF) -cmake_dependent_option(SNMALLOC_STATIC_LIBRARY "Build static libraries" ON "NOT SNMALLOC_STATIC_LIBRARY" OFF) -cmake_dependent_option(SNMALLOC_OPTIMISE_FOR_CURRENT_MACHINE "Compile for current machine architecture" Off "NOT SNMALLOC_STATIC_LIBRARY" OFF) +cmake_dependent_option(SNMALLOC_STATIC_LIBRARY "Build static libraries" ON "NOT SNMALLOC_HEADER_ONLY_LIBRARY" OFF) +cmake_dependent_option(SNMALLOC_OPTIMISE_FOR_CURRENT_MACHINE "Compile for current machine architecture" Off "NOT SNMALLOC_HEADER_ONLY_LIBRARY" OFF) if (NOT SNMALLOC_HEADER_ONLY_LIBRARY) # Pick a sensible default for the thread cleanup mechanism if (${CMAKE_SYSTEM_NAME} STREQUAL FreeBSD) set(SNMALLOC_CLEANUP_DEFAULT THREAD_CLEANUP) - elseif (UNIX) + elseif (UNIX AND NOT APPLE) set(SNMALLOC_CLEANUP_DEFAULT PTHREAD_DESTRUCTORS) else () set(SNMALLOC_CLEANUP_DEFAULT CXX11_DESTRUCTORS) @@ -210,12 +210,23 @@ endif() if(MSVC) target_compile_definitions(snmalloc INTERFACE -D_HAS_EXCEPTIONS=0) else() + # All symbols are always dynamic on haiku and -rdynamic is redundant (and unsupported). if (NOT CMAKE_SYSTEM_NAME STREQUAL "Haiku") # Get better stack traces in CI and debug builds. target_link_options(snmalloc INTERFACE $<${ci_or_debug}:-rdynamic>) endif() endif() +check_linker_flag(CXX "-Wl,--no-undefined" SNMALLOC_LINKER_SUPPORT_NO_ALLOW_SHLIB_UNDEF) + +function(add_warning_flags name) + target_compile_options(${name} PRIVATE + $<$:/Zi /W4 /WX /wd4127 /wd4324 /wd4201 $<${ci_or_debug}:/DEBUG>> + $<$>:-fno-exceptions -fno-rtti -Wall -Wextra -Werror -Wundef> + $<$:-Wsign-conversion -Wconversion>) + target_link_options(${name} PRIVATE $<$:-Wl,--no-undefined>) +endfunction() + # To build with just the header library target define SNMALLOC_HEADER_ONLY_LIBRARY if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) @@ -243,17 +254,12 @@ if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) set_target_properties(${name} PROPERTIES CXX_VISIBILITY_PRESET hidden) target_compile_definitions(${name} PRIVATE "SNMALLOC_USE_${SNMALLOC_CLEANUP}") - if(MSVC) - target_compile_options(${name} PRIVATE /Zi /W4 /WX /wd4127 /wd4324 /wd4201) - set(CMAKE_EXE_LINKER_FLAGS_RELEASE "${CMAKE_EXE_LINKER_FLAGS_RELEASE} /DEBUG") - else() + add_warning_flags(${name}) + if(NOT MSVC) target_compile_definitions(${name} PRIVATE "SNMALLOC_EXPORT=__attribute__((visibility(\"default\")))") target_compile_options(${name} PRIVATE - -fno-exceptions -fno-rtti -fomit-frame-pointer -ffunction-sections - -Wall -Wextra -Werror -Wundef - $<$:-Wsign-conversion -Wconversion>) + -fomit-frame-pointer -ffunction-sections) # Static TLS model is unsupported on Haiku. - # All symbols are always dynamic on haiku and -rdynamic is redundant (and unsupported). if (NOT CMAKE_SYSTEM_NAME STREQUAL "Haiku") target_compile_options(${name} PRIVATE -ftls-model=initial-exec) target_compile_options(${name} PRIVATE $<$:-g>) @@ -339,6 +345,7 @@ if(NOT SNMALLOC_HEADER_ONLY_LIBRARY) set(TESTNAME "${TEST_CATEGORY}-${TEST}-${FLAVOUR}") add_executable(${TESTNAME} ${SRC}) + add_warning_flags(${TESTNAME}) if (${FLAVOUR} STREQUAL "malloc") target_compile_definitions(${TESTNAME} PRIVATE SNMALLOC_PASS_THROUGH) diff --git a/src/mem/threadalloc.h b/src/mem/threadalloc.h index 3c75c42ae..bf5f822a1 100644 --- a/src/mem/threadalloc.h +++ b/src/mem/threadalloc.h @@ -153,8 +153,17 @@ namespace snmalloc * Entry point that allows libc to call into the allocator for per-thread * cleanup. */ -void _malloc_thread_cleanup() +inline void _malloc_thread_cleanup() { snmalloc::ThreadAlloc::get().teardown(); } + +namespace snmalloc +{ + /** + * No-op version of register_clean_up. This is called unconditionally by + * globalconfig but is not necessary when using a libc hook. + */ + inline void register_clean_up() {} +} #endif From f4ba1e5ea9c402f428a333d3e8b8612fe92efa45 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Thu, 2 Sep 2021 09:47:51 +0100 Subject: [PATCH 11/14] Unconditionally include CheckLinkerFlags. --- CMakeLists.txt | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bf35219d2..35157bbac 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -43,15 +43,16 @@ if (NOT SNMALLOC_CLEANUP STREQUAL CXX11_DESTRUCTORS) set(CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "") endif() +# If CheckLinkerFlag doesn't exist then provide a dummy implementation that +# always fails. The fallback can be removed when we move to CMake 3.18 as the +# baseline. +include(CheckLinkerFlag OPTIONAL RESULT_VARIABLE CHECK_LINKER_FLAG) +if (NOT CHECK_LINKER_FLAG) + function(check_linker_flag) + endfunction() +endif () + if (NOT MSVC AND NOT (SNMALLOC_CLEANUP STREQUAL CXX11_DESTRUCTORS)) - # If CheckLinkerFlag doesn't exist then provide a dummy implementation that - # always fails. The fallback can be removed when we move to CMake 3.18 as the - # baseline. - include(CheckLinkerFlag OPTIONAL RESULT_VARIABLE CHECK_LINKER_FLAG) - if (NOT CHECK_LINKER_FLAG) - function(check_linker_flag) - endfunction() - endif () # If the target compiler doesn't support -nostdlib++ then we must enable C at # the global scope for the fallbacks to work. check_linker_flag(CXX "-nostdlib++" SNMALLOC_LINKER_SUPPORT_NOSTDLIBXX) From 3ae3702ad63a8e2c28e682494f4d2c1292b4f83d Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Thu, 2 Sep 2021 10:03:13 +0100 Subject: [PATCH 12/14] Don't pass -fno-rtti and -fno-exceptions to clang-cl --- CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 35157bbac..87196b974 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -223,7 +223,8 @@ check_linker_flag(CXX "-Wl,--no-undefined" SNMALLOC_LINKER_SUPPORT_NO_ALLOW_SHLI function(add_warning_flags name) target_compile_options(${name} PRIVATE $<$:/Zi /W4 /WX /wd4127 /wd4324 /wd4201 $<${ci_or_debug}:/DEBUG>> - $<$>:-fno-exceptions -fno-rtti -Wall -Wextra -Werror -Wundef> + $<$,$>>:-fno-exceptions -fno-rtti> + $<$>:-Wall -Wextra -Werror -Wundef> $<$:-Wsign-conversion -Wconversion>) target_link_options(${name} PRIVATE $<$:-Wl,--no-undefined>) endfunction() From f6cd4a68a19529066aa6a95e001c96a9157f62e9 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Thu, 2 Sep 2021 10:07:40 +0100 Subject: [PATCH 13/14] Fix CI? --- .github/workflows/main.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 8e02008c1..157ea9e10 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -42,7 +42,6 @@ jobs: - os: "ubuntu-latest" build-type: Debug self-host: true - extra-cmake-flags: "-DSNMALLOC_USE_PTHREAD_DESTRUCTORS=On" # Extra build to check using pthread library for destructing local state. - os: "ubuntu-latest" variant: "Ubuntu (with pthread destructors)." From 49bcb7278ad4782bf4a1dee0128e2ac459b1043c Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Thu, 2 Sep 2021 10:29:08 +0100 Subject: [PATCH 14/14] Don't enable clang-cl Wall and Wextra. These add a load of things on Windows that generate huge numbers of spurious warnings. --- CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 87196b974..79950cf66 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -223,8 +223,7 @@ check_linker_flag(CXX "-Wl,--no-undefined" SNMALLOC_LINKER_SUPPORT_NO_ALLOW_SHLI function(add_warning_flags name) target_compile_options(${name} PRIVATE $<$:/Zi /W4 /WX /wd4127 /wd4324 /wd4201 $<${ci_or_debug}:/DEBUG>> - $<$,$>>:-fno-exceptions -fno-rtti> - $<$>:-Wall -Wextra -Werror -Wundef> + $<$,$>>:-fno-exceptions -fno-rtti -Wall -Wextra -Werror -Wundef> $<$:-Wsign-conversion -Wconversion>) target_link_options(${name} PRIVATE $<$:-Wl,--no-undefined>) endfunction()