From ebe965a7447f7e8d460e6c5f00e40db05b1c28db Mon Sep 17 00:00:00 2001 From: Patryk Kaminski Date: Fri, 21 Mar 2025 15:47:52 +0100 Subject: [PATCH 1/9] Build and install umfd debug library on Windows when UMF_USE_DEBUG_POSTFIX CMake option is set. --- .github/workflows/reusable_basic.yml | 3 +++ CMakeLists.txt | 31 ++++++++++++++++++++++++++++ test/test_installation.py | 16 +++++++++++++- 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/.github/workflows/reusable_basic.yml b/.github/workflows/reusable_basic.yml index 41ce4b385..816c45a7a 100644 --- a/.github/workflows/reusable_basic.yml +++ b/.github/workflows/reusable_basic.yml @@ -241,6 +241,7 @@ jobs: shared_library: 'ON' level_zero_provider: 'ON' cuda_provider: 'ON' + umfd_lib: 'ON' - os: 'windows-2022' build_type: Release compiler: {c: cl, cxx: cl} @@ -289,6 +290,7 @@ jobs: -DUMF_BUILD_LEVEL_ZERO_PROVIDER=${{matrix.level_zero_provider}} -DUMF_BUILD_CUDA_PROVIDER=${{matrix.cuda_provider}} -DUMF_TESTS_FAIL_ON_SKIP=ON + -DUMF_USE_DEBUG_POSTFIX=${{matrix.umfd_lib}} - name: Build UMF run: cmake --build ${{env.BUILD_DIR}} --config ${{matrix.build_type}} -j $Env:NUMBER_OF_PROCESSORS @@ -307,6 +309,7 @@ jobs: ${{matrix.shared_library == 'ON' && '--proxy' || '' }} --umf-version ${{env.UMF_VERSION}} ${{ matrix.shared_library == 'ON' && '--shared-library' || ''}} + ${{ matrix.umfd_lib == 'ON' && '--umfd-lib' || ''}} - name: check /DEPENDENTLOADFLAG in umf.dll if: ${{matrix.shared_library == 'ON' && matrix.compiler.cxx == 'cl'}} diff --git a/CMakeLists.txt b/CMakeLists.txt index 146869a82..440ade8b6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -89,6 +89,8 @@ set(UMF_INSTALL_RPATH "Set the runtime search path to the directory with dependencies (e.g. hwloc)" ) +umf_option(UMF_USE_DEBUG_POSTFIX "Add a 'd' postfix to Windows debug libraries" + OFF) umf_option(UMF_DEVELOPER_MODE "Enable additional developer checks" OFF) umf_option( UMF_FORMAT_CODE_STYLE @@ -434,6 +436,27 @@ elseif(UMF_BUILD_CUDA_PROVIDER) message(STATUS "CUDA_INCLUDE_DIRS = ${CUDA_INCLUDE_DIRS}") endif() +if(WINDOWS AND UMF_USE_DEBUG_POSTFIX) + # Build debug umf library with the d suffix that is compiled with /MDd so + # users can link against it in debug builds. + set(CMAKE_DEBUG_POSTFIX d) + + add_custom_target( + umfd ALL + COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target umf + --config Debug + COMMENT "Building debug umf library with the d suffix") + + # Copy built UMF libraries to the Release build subdirectory + add_custom_command( + TARGET umfd + COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_BINARY_DIR}/bin/Debug/umfd.dll + ${CMAKE_BINARY_DIR}/bin/Release/umfd.dll + COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_BINARY_DIR}/lib/Debug/umfd.lib + ${CMAKE_BINARY_DIR}/lib/Release/umfd.lib + COMMENT "Copying debug libraries to the Release build directory") +endif() + # This build type check is not possible on Windows when CMAKE_BUILD_TYPE is not # set, because in this case the build type is determined after a CMake # configuration is done (at the build time) @@ -826,6 +849,14 @@ endif() # --------------------------------------------------------------------------- # # Configure make install/uninstall and packages # --------------------------------------------------------------------------- # +# Install umfd target +if(WINDOWS AND UMF_USE_DEBUG_POSTFIX) + install(FILES ${CMAKE_BINARY_DIR}/bin/Debug/umfd.dll + DESTINATION ${CMAKE_INSTALL_BINDIR}) + install(FILES ${CMAKE_BINARY_DIR}/lib/Debug/umfd.lib + DESTINATION ${CMAKE_INSTALL_LIBDIR}) +endif() + install(FILES ${PROJECT_SOURCE_DIR}/LICENSE.TXT DESTINATION "${CMAKE_INSTALL_DATAROOTDIR}/doc/${PROJECT_NAME}/") install( diff --git a/test/test_installation.py b/test/test_installation.py index ef30ac759..ff494101f 100644 --- a/test/test_installation.py +++ b/test/test_installation.py @@ -41,6 +41,7 @@ def __init__( proxy: bool, pools: List[str], umf_version: Version, + umfd_lib: bool, ): self.workspace_dir = workspace_dir self.build_dir = build_dir @@ -50,6 +51,7 @@ def __init__( self.proxy = proxy self.pools = pools self.umf_version = umf_version + self.umfd_lib = umfd_lib self.match_list = self._create_match_list() def _create_match_list(self) -> List[str]: @@ -74,10 +76,14 @@ def _create_match_list(self) -> List[str]: lib_prefix = "lib" bin = [] - if platform.system() == "Windows" and (self.shared_library or self.proxy): + if platform.system() == "Windows" and ( + self.shared_library or self.proxy or self.umfd_lib + ): bin.append("bin") if self.shared_library: bin.append("bin/umf.dll") + if self.umfd_lib: + bin.append("bin/umfd.dll") if self.proxy: bin.append("bin/umf_proxy.dll") @@ -101,6 +107,8 @@ def _create_match_list(self) -> List[str]: lib.append(f"lib/{lib_prefix}{pool}.{lib_ext_static}") if self.shared_library: lib.append(f"lib/{lib_prefix}umf.{lib_ext_shared}") + if platform.system() == "Windows" and self.umfd_lib: + lib.append(f"lib/{lib_prefix}umfd.{lib_ext_shared}") if platform.system() == "Linux": lib.append( @@ -283,6 +291,11 @@ def parse_arguments(self) -> argparse.Namespace: action="store", help="Current version of the UMF, e.g. 1.0.0", ) + self.parser.add_argument( + "--umfd-lib", + action="store_true", + help="Add this argument if the UMF was built with the umfd library", + ) return self.parser.parse_args() def run(self) -> None: @@ -306,6 +319,7 @@ def run(self) -> None: self.args.proxy, pools, umf_version, + self.args.umfd_lib, ) print("Installation test - BEGIN", flush=True) From 169864a005d8962eb0b978bc080d95fcc7030ef9 Mon Sep 17 00:00:00 2001 From: Patryk Kaminski Date: Fri, 4 Apr 2025 03:54:23 +0200 Subject: [PATCH 2/9] Fix building umfd.dll on single-config generators - add installation tests step to Windows generators nightly tests --- .github/workflows/nightly.yml | 29 +++++++++-- CMakeLists.txt | 98 ++++++++++++++++++++++++++++------- test/test_installation.py | 21 ++++++-- 3 files changed, 120 insertions(+), 28 deletions(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 1317482fd..3890e33b2 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -12,6 +12,7 @@ permissions: env: BUILD_DIR : "${{github.workspace}}/build" + INSTALL_DIR: "${{github.workspace}}/build/install" jobs: fuzz-test: @@ -96,11 +97,12 @@ jobs: strategy: matrix: os: ['windows-2019', 'windows-2022'] - build_type: [Release] + build_type: [Debug, Release] compiler: [{c: cl, cxx: cl}] shared_library: ['ON', 'OFF'] static_hwloc: ['ON', 'OFF'] generator: ['Ninja', 'NMake Makefiles'] + umfd_lib: ['ON', 'OFF'] runs-on: ${{matrix.os}} @@ -112,11 +114,11 @@ jobs: - name: Set VCPKG_PATH with hwloc if: matrix.static_hwloc == 'OFF' - run: echo "VCPKG_PATH='${{github.workspace}}/build/vcpkg/packages/hwloc_x64-windows;${{github.workspace}}/build/vcpkg/packages/tbb_x64-windows;${{github.workspace}}/build/vcpkg/packages/jemalloc_x64-windows'" >> $env:GITHUB_ENV + run: echo "VCPKG_PATH=${{github.workspace}}/build/vcpkg/packages/hwloc_x64-windows;${{github.workspace}}/build/vcpkg/packages/tbb_x64-windows;${{github.workspace}}/build/vcpkg/packages/jemalloc_x64-windows" >> $env:GITHUB_ENV - name: Set VCPKG_PATH without hwloc if: matrix.static_hwloc == 'ON' - run: echo "VCPKG_PATH='${{github.workspace}}/build/vcpkg/packages/tbb_x64-windows;${{github.workspace}}/build/vcpkg/packages/jemalloc_x64-windows'" >> $env:GITHUB_ENV + run: echo "VCPKG_PATH=${{github.workspace}}/build/vcpkg/packages/tbb_x64-windows;${{github.workspace}}/build/vcpkg/packages/jemalloc_x64-windows" >> $env:GITHUB_ENV - name: Initialize vcpkg uses: lukka/run-vcpkg@5e0cab206a5ea620130caf672fce3e4a6b5666a1 # v11.5 @@ -141,6 +143,7 @@ jobs: run: > cmake -B ${{env.BUILD_DIR}} + -DCMAKE_BUILD_TYPE=${{matrix.build_type}} -DCMAKE_PREFIX_PATH="${{env.VCPKG_PATH}}" -DCMAKE_C_COMPILER=${{matrix.compiler.c}} -DCMAKE_CXX_COMPILER=${{matrix.compiler.cxx}} @@ -153,6 +156,7 @@ jobs: -DUMF_BUILD_LEVEL_ZERO_PROVIDER=ON -DUMF_BUILD_CUDA_PROVIDER=ON -DUMF_TESTS_FAIL_ON_SKIP=ON + ${{ matrix.umfd_lib == 'ON' && '-DUMF_USE_DEBUG_POSTFIX=ON' || '' }} - name: Build UMF shell: cmd @@ -163,6 +167,25 @@ jobs: working-directory: ${{env.BUILD_DIR}} run: ctest -C ${{matrix.build_type}} --output-on-failure --test-dir test + - name: Get UMF version + run: | + $version = (git describe --tags --abbrev=0 | Select-String -Pattern '\d+\.\d+\.\d+').Matches.Value + echo "UMF_VERSION=$version" >> $env:GITHUB_ENV + shell: pwsh + + - name: Test UMF installation and uninstallation + # The '--shared-library' parameter is added to the installation test when the UMF is built as a shared library + # The '--umfd-lib' parameter is added when the UMF is built with the umfd library + run: > + python3 ${{github.workspace}}/test/test_installation.py + --build-dir ${{env.BUILD_DIR}} + --install-dir ${{env.INSTALL_DIR}} + --build-type ${{matrix.build_type}} + --umf-version ${{env.UMF_VERSION}} + ${{ matrix.shared_library == 'ON' && '--proxy --shared-library' || '' }} + ${{ matrix.umfd_lib == 'ON' && '--umfd-lib' || ''}} + ${{ matrix.static_hwloc == 'ON' && '--hwloc' || '' }} + icx: name: ICX env: diff --git a/CMakeLists.txt b/CMakeLists.txt index 440ade8b6..f7e640abc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -58,6 +58,8 @@ macro(umf_option) option(${ARGV}) endmacro() +# All CMake options have to be explicitly set in the build_umfd target's +# configuration command umf_option(UMF_BUILD_SHARED_LIBRARY "Build UMF as shared library" OFF) umf_option(UMF_BUILD_LEVEL_ZERO_PROVIDER "Build Level Zero memory provider" ON) umf_option(UMF_BUILD_CUDA_PROVIDER "Build CUDA memory provider" ON) @@ -148,6 +150,8 @@ if(UMF_DEVELOPER_MODE) UMF_DEVELOPER_MODE=1) endif() +message(STATUS "CMAKE_PREFIX_PATH=${CMAKE_PREFIX_PATH}") + if(NOT UMF_BUILD_LIBUMF_POOL_JEMALLOC) set(UMF_POOL_JEMALLOC_ENABLED FALSE) set(JEMALLOC_FOUND FALSE) @@ -293,6 +297,7 @@ else() set(HWLOC_ENABLE_TESTING OFF) set(HWLOC_SKIP_LSTOPO ON) set(HWLOC_SKIP_TOOLS ON) + set(HWLOC_SKIP_INCLUDES ON) FetchContent_Declare( hwloc_targ @@ -436,25 +441,72 @@ elseif(UMF_BUILD_CUDA_PROVIDER) message(STATUS "CUDA_INCLUDE_DIRS = ${CUDA_INCLUDE_DIRS}") endif() +# Build the umfd target in a separate directory with Debug configuration if(WINDOWS AND UMF_USE_DEBUG_POSTFIX) - # Build debug umf library with the d suffix that is compiled with /MDd so - # users can link against it in debug builds. - set(CMAKE_DEBUG_POSTFIX d) - + # The build_umfd target's configuration command requires to have + # CMAKE_PREFIX_PATH with semicolons escaped + string(JOIN "\;" UMFD_CMAKE_PREFIX_PATH ${CMAKE_PREFIX_PATH}) add_custom_target( - umfd ALL - COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target umf - --config Debug - COMMENT "Building debug umf library with the d suffix") + build_umfd ALL + COMMAND + ${CMAKE_COMMAND} -G "${CMAKE_GENERATOR}" -S ${UMF_CMAKE_SOURCE_DIR} + -B ${CMAKE_BINARY_DIR}/umfd_build -DCMAKE_BUILD_TYPE=Debug + -DCMAKE_DEBUG_POSTFIX=d + -DCMAKE_PREFIX_PATH="${UMFD_CMAKE_PREFIX_PATH}" + -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER} + -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER} + -DUMF_USE_DEBUG_POSTFIX=OFF + -DUMF_BUILD_SHARED_LIBRARY=${UMF_BUILD_SHARED_LIBRARY} + -DUMF_BUILD_LEVEL_ZERO_PROVIDER=${UMF_BUILD_LEVEL_ZERO_PROVIDER} + -DUMF_BUILD_CUDA_PROVIDER=${UMF_BUILD_CUDA_PROVIDER} + -DUMF_BUILD_LIBUMF_POOL_JEMALLOC=${UMF_BUILD_LIBUMF_POOL_JEMALLOC} + -DUMF_BUILD_TESTS=OFF -DUMF_BUILD_GPU_TESTS=OFF + -DUMF_BUILD_BENCHMARKS=OFF -DUMF_BUILD_BENCHMARKS_MT=OFF + -DUMF_BUILD_EXAMPLES=OFF -DUMF_BUILD_GPU_EXAMPLES=OFF + -DUMF_BUILD_FUZZTESTS=OFF -DUMF_DISABLE_HWLOC=${UMF_DISABLE_HWLOC} + -DUMF_LINK_HWLOC_STATICALLY=${UMF_LINK_HWLOC_STATICALLY} + -DUMF_HWLOC_NAME=${UMF_HWLOC_NAME} + -DUMF_INSTALL_RPATH=${UMF_INSTALL_RPATH} -DUMF_DEVELOPER_MODE=OFF + -DUMF_FORMAT_CODE_STYLE=OFF -DUMF_TESTS_FAIL_ON_SKIP=OFF + -DUMF_USE_ASAN=OFF -DUMF_USE_UBSAN=OFF -DUMF_USE_TSAN=OFF + -DUMF_USE_MSAN=OFF -DUMF_USE_VALGRIND=OFF -DUMF_USE_COVERAGE=OFF + -DUMF_PROXY_LIB_BASED_ON_POOL=${UMF_PROXY_LIB_BASED_ON_POOL} + COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR}/umfd_build --target + umf --config Debug + COMMENT + "Configuring and building umfd.dll in a separate directory with Debug configuration" + ) - # Copy built UMF libraries to the Release build subdirectory + # Copy built UMF libraries to the main binary directory and remove + # umfd_build + if(CMAKE_CONFIGURATION_TYPES) + # Multi-config generator (e.g., Visual Studio) + set(UMFD_DLL_SRC "${CMAKE_BINARY_DIR}/umfd_build/bin/Debug/umfd.dll") + set(UMFD_LIB_SRC "${CMAKE_BINARY_DIR}/umfd_build/lib/Debug/umfd.lib") + set(UMFD_DLL "${CMAKE_BINARY_DIR}/bin/$/umfd.dll") + set(UMFD_LIB "${CMAKE_BINARY_DIR}/lib/$/umfd.lib") + else() + # Single-config generator (e.g., Ninja) + set(UMFD_DLL_SRC "${CMAKE_BINARY_DIR}/umfd_build/bin/umfd.dll") + set(UMFD_LIB_SRC "${CMAKE_BINARY_DIR}/umfd_build/lib/umfd.lib") + set(UMFD_DLL "${CMAKE_BINARY_DIR}/bin/umfd.dll") + set(UMFD_LIB "${CMAKE_BINARY_DIR}/lib/umfd.lib") + endif() + + if(UMF_BUILD_SHARED_LIBRARY) + add_custom_command( + TARGET build_umfd + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${UMFD_DLL_SRC} + ${UMFD_DLL} + COMMENT "Copying umfd.dll to the main binary directory") + endif() add_custom_command( - TARGET umfd - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_BINARY_DIR}/bin/Debug/umfd.dll - ${CMAKE_BINARY_DIR}/bin/Release/umfd.dll - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_BINARY_DIR}/lib/Debug/umfd.lib - ${CMAKE_BINARY_DIR}/lib/Release/umfd.lib - COMMENT "Copying debug libraries to the Release build directory") + TARGET build_umfd + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${UMFD_LIB_SRC} + ${UMFD_LIB} + COMMAND ${CMAKE_COMMAND} -E remove_directory + ${CMAKE_BINARY_DIR}/umfd_build DEPENDS ${UMFD_DLL} + COMMENT "Copying umfd.lib to the main library directory") endif() # This build type check is not possible on Windows when CMAKE_BUILD_TYPE is not @@ -849,12 +901,18 @@ endif() # --------------------------------------------------------------------------- # # Configure make install/uninstall and packages # --------------------------------------------------------------------------- # -# Install umfd target +# Install the umfd library files as part of the umfd component if(WINDOWS AND UMF_USE_DEBUG_POSTFIX) - install(FILES ${CMAKE_BINARY_DIR}/bin/Debug/umfd.dll - DESTINATION ${CMAKE_INSTALL_BINDIR}) - install(FILES ${CMAKE_BINARY_DIR}/lib/Debug/umfd.lib - DESTINATION ${CMAKE_INSTALL_LIBDIR}) + if(UMF_BUILD_SHARED_LIBRARY) + install( + FILES ${UMFD_DLL} + DESTINATION ${CMAKE_INSTALL_BINDIR} + COMPONENT umfd) + endif() + install( + FILES ${UMFD_LIB} + DESTINATION ${CMAKE_INSTALL_LIBDIR} + COMPONENT umfd) endif() install(FILES ${PROJECT_SOURCE_DIR}/LICENSE.TXT diff --git a/test/test_installation.py b/test/test_installation.py index ff494101f..4cf789000 100644 --- a/test/test_installation.py +++ b/test/test_installation.py @@ -28,7 +28,8 @@ class UmfInstaller: proxy (bool): Determines whether the proxy library should be built together with the UMF library pools (List[str]): A list of enabled pools during the UMF compilation umf_version (Version): UMF version currently being built and installed - match_list (List[str]): A list of relative paths of files that should be installed + umfd_lib (bool): Determines if the UMF was built with the umfd library + hwloc (bool): Determines if hwloc is installed and should be checked """ def __init__( @@ -42,6 +43,7 @@ def __init__( pools: List[str], umf_version: Version, umfd_lib: bool, + hwloc: bool, ): self.workspace_dir = workspace_dir self.build_dir = build_dir @@ -52,6 +54,7 @@ def __init__( self.pools = pools self.umf_version = umf_version self.umfd_lib = umfd_lib + self.hwloc = hwloc self.match_list = self._create_match_list() def _create_match_list(self) -> List[str]: @@ -76,9 +79,7 @@ def _create_match_list(self) -> List[str]: lib_prefix = "lib" bin = [] - if platform.system() == "Windows" and ( - self.shared_library or self.proxy or self.umfd_lib - ): + if platform.system() == "Windows" and (self.shared_library or self.proxy): bin.append("bin") if self.shared_library: bin.append("bin/umf.dll") @@ -103,8 +104,11 @@ def _create_match_list(self) -> List[str]: f"lib/cmake/umf/umf-targets-{self.build_type}.cmake", "lib/cmake/umf/umf-targets.cmake", ] + for pool in self.pools: lib.append(f"lib/{lib_prefix}{pool}.{lib_ext_static}") + if platform.system() == "Windows" and self.hwloc: + lib.append(f"lib/{lib_prefix}hwloc.{lib_ext_static}") if self.shared_library: lib.append(f"lib/{lib_prefix}umf.{lib_ext_shared}") if platform.system() == "Windows" and self.umfd_lib: @@ -122,6 +126,8 @@ def _create_match_list(self) -> List[str]: lib.append(f"lib/{lib_prefix}umf.{self.umf_version}.{lib_ext_shared}") else: lib.append(f"lib/{lib_prefix}umf.{lib_ext_static}") + if self.umfd_lib and platform.system() == "Windows": + lib.append(f"lib/{lib_prefix}umfd.{lib_ext_static}") if self.proxy: lib.append(f"lib/{lib_prefix}umf_proxy.{lib_ext_shared}") @@ -135,7 +141,6 @@ def _create_match_list(self) -> List[str]: f"lib/{lib_prefix}umf_proxy.{self.umf_version.major}.{lib_ext_shared}" ) - share = [] share = [ "share", "share/doc", @@ -296,6 +301,11 @@ def parse_arguments(self) -> argparse.Namespace: action="store_true", help="Add this argument if the UMF was built with the umfd library", ) + self.parser.add_argument( + "--hwloc", + action="store_true", + help="Add this argument if hwloc is installed and should be checked", + ) return self.parser.parse_args() def run(self) -> None: @@ -320,6 +330,7 @@ def run(self) -> None: pools, umf_version, self.args.umfd_lib, + self.args.hwloc, ) print("Installation test - BEGIN", flush=True) From e90d42fa0db5004fb0e3fd9627db2ef94894433c Mon Sep 17 00:00:00 2001 From: Patryk Kaminski Date: Tue, 8 Apr 2025 15:13:36 +0200 Subject: [PATCH 3/9] Remove redundant Windows generators jobs --- .github/workflows/nightly.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 3890e33b2..4c40d4fd4 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -96,7 +96,6 @@ jobs: name: Windows ${{matrix.generator}} generator strategy: matrix: - os: ['windows-2019', 'windows-2022'] build_type: [Debug, Release] compiler: [{c: cl, cxx: cl}] shared_library: ['ON', 'OFF'] @@ -104,7 +103,7 @@ jobs: generator: ['Ninja', 'NMake Makefiles'] umfd_lib: ['ON', 'OFF'] - runs-on: ${{matrix.os}} + runs-on: windows-latest steps: - name: Checkout From 8aece345ea452cdaec6e765e67fba74eb1d78d68 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Mon, 31 Mar 2025 15:43:44 +0200 Subject: [PATCH 4/9] Use atomics to fix ASAN data races in critnib Signed-off-by: Lukasz Dorau --- src/critnib/critnib.c | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/critnib/critnib.c b/src/critnib/critnib.c index 5625781d3..7db9dd3dc 100644 --- a/src/critnib/critnib.c +++ b/src/critnib/critnib.c @@ -246,8 +246,8 @@ static void free_node(struct critnib *__restrict c, } ASSERT(!is_leaf(n)); - n->child[0] = c->deleted_node; - c->deleted_node = n; + utils_atomic_store_release_ptr((void **)&n->child[0], c->deleted_node); + utils_atomic_store_release_ptr((void **)&c->deleted_node, n); } /* @@ -277,8 +277,8 @@ static void free_leaf(struct critnib *__restrict c, return; } - k->value = c->deleted_leaf; - c->deleted_leaf = k; + utils_atomic_store_release_ptr((void **)&k->value, c->deleted_leaf); + utils_atomic_store_release_ptr((void **)&c->deleted_leaf, k); } /* @@ -319,8 +319,8 @@ int critnib_insert(struct critnib *c, word key, void *value, int update) { utils_annotate_memory_no_check(k, sizeof(struct critnib_leaf)); - k->key = key; - k->value = value; + utils_atomic_store_release_ptr((void **)&k->key, (void *)key); + utils_atomic_store_release_ptr((void **)&k->value, value); struct critnib_node *kn = (void *)((word)k | 1); @@ -358,7 +358,7 @@ int critnib_insert(struct critnib *c, word key, void *value, int update) { free_leaf(c, to_leaf(kn)); if (update) { - to_leaf(n)->value = value; + utils_atomic_store_release_ptr(&to_leaf(n)->value, value); utils_mutex_unlock(&c->mutex); return 0; } else { @@ -381,13 +381,14 @@ int critnib_insert(struct critnib *c, word key, void *value, int update) { utils_annotate_memory_no_check(m, sizeof(struct critnib_node)); for (int i = 0; i < SLNODES; i++) { - m->child[i] = NULL; + utils_atomic_store_release_ptr((void *)&m->child[i], NULL); } - m->child[slice_index(key, sh)] = kn; - m->child[slice_index(path, sh)] = n; + utils_atomic_store_release_ptr((void *)&m->child[slice_index(key, sh)], kn); + utils_atomic_store_release_ptr((void *)&m->child[slice_index(path, sh)], n); m->shift = sh; - m->path = key & path_mask(sh); + utils_atomic_store_release_u64((void *)&m->path, key & path_mask(sh)); + utils_atomic_store_release_ptr((void **)parent, m); utils_mutex_unlock(&c->mutex); @@ -569,12 +570,15 @@ static struct critnib_leaf *find_le(struct critnib_node *__restrict n, * that shift points at the nib's lower rather than upper edge, so it * needs to be masked away as well. */ - if ((key ^ n->path) >> (n->shift) & ~NIB) { + word path; + sh_t shift = n->shift; + utils_atomic_load_acquire_u64((uint64_t *)&n->path, (uint64_t *)&path); + if ((key ^ path) >> (shift) & ~NIB) { /* * subtree is too far to the left? * -> its rightmost value is good */ - if (n->path < key) { + if (path < key) { return find_predecessor(n); } @@ -759,8 +763,9 @@ int critnib_find(struct critnib *c, uintptr_t key, enum find_dir_t dir, k = (n && kk->key == key) ? kk : NULL; } if (k) { - _rkey = k->key; - _rvalue = k->value; + utils_atomic_load_acquire_u64((uint64_t *)&k->key, + (uint64_t *)&_rkey); + utils_atomic_load_acquire_ptr(&k->value, (void **)&_rvalue); } utils_atomic_load_acquire_u64(&c->remove_count, &wrs2); } while (wrs1 + DELETED_LIFE <= wrs2); From ec98c6d4ce40846d2f8829ff19d696020386721b Mon Sep 17 00:00:00 2001 From: Patryk Kaminski Date: Tue, 1 Apr 2025 09:10:40 +0200 Subject: [PATCH 5/9] Fix vcpkg tbb installation --- .github/workflows/nightly.yml | 8 ++++---- .github/workflows/reusable_basic.yml | 4 ++-- .github/workflows/reusable_codeql.yml | 4 ++-- .github/workflows/reusable_compatibility.yml | 4 ++-- .github/workflows/reusable_fast.yml | 4 ++-- .github/workflows/reusable_gpu.yml | 4 ++-- .github/workflows/reusable_sanitizers.yml | 4 ++-- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 4c40d4fd4..f209cf2c5 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -124,12 +124,12 @@ jobs: env: VCPKG_PATH: ${{env.VCPKG_PATH}} with: - vcpkgGitCommitId: 3dd44b931481d7a8e9ba412621fa810232b66289 + vcpkgGitCommitId: ea2a964f9303270322cf3f2d51c265ba146c422d # 1.04.2025 vcpkgDirectory: ${{env.BUILD_DIR}}/vcpkg vcpkgJsonGlob: '**/vcpkg.json' - name: Install dependencies - run: vcpkg install + run: vcpkg install --triplet x64-windows - name: Install Ninja if: matrix.generator == 'Ninja' @@ -212,12 +212,12 @@ jobs: - name: Initialize vcpkg uses: lukka/run-vcpkg@5e0cab206a5ea620130caf672fce3e4a6b5666a1 # v11.5 with: - vcpkgGitCommitId: 3dd44b931481d7a8e9ba412621fa810232b66289 + vcpkgGitCommitId: ea2a964f9303270322cf3f2d51c265ba146c422d # 1.04.2025 vcpkgDirectory: ${{env.BUILD_DIR}}/vcpkg vcpkgJsonGlob: '**/vcpkg.json' - name: Install dependencies - run: vcpkg install + run: vcpkg install --triplet x64-windows - name: Install Ninja uses: seanmiddleditch/gha-setup-ninja@96bed6edff20d1dd61ecff9b75cc519d516e6401 # v5 diff --git a/.github/workflows/reusable_basic.yml b/.github/workflows/reusable_basic.yml index 816c45a7a..b30bfed4c 100644 --- a/.github/workflows/reusable_basic.yml +++ b/.github/workflows/reusable_basic.yml @@ -260,12 +260,12 @@ jobs: - name: Initialize vcpkg uses: lukka/run-vcpkg@5e0cab206a5ea620130caf672fce3e4a6b5666a1 # v11.5 with: - vcpkgGitCommitId: 3dd44b931481d7a8e9ba412621fa810232b66289 + vcpkgGitCommitId: ea2a964f9303270322cf3f2d51c265ba146c422d # 1.04.2025 vcpkgDirectory: ${{env.BUILD_DIR}}/vcpkg vcpkgJsonGlob: '**/vcpkg.json' - name: Install dependencies - run: vcpkg install + run: vcpkg install --triplet x64-windows shell: pwsh # Specifies PowerShell as the shell for running the script. - name: Get UMF version diff --git a/.github/workflows/reusable_codeql.yml b/.github/workflows/reusable_codeql.yml index 046c32081..252e70eee 100644 --- a/.github/workflows/reusable_codeql.yml +++ b/.github/workflows/reusable_codeql.yml @@ -48,14 +48,14 @@ jobs: if: matrix.os == 'windows-latest' uses: lukka/run-vcpkg@5e0cab206a5ea620130caf672fce3e4a6b5666a1 # v11.5 with: - vcpkgGitCommitId: 3dd44b931481d7a8e9ba412621fa810232b66289 + vcpkgGitCommitId: ea2a964f9303270322cf3f2d51c265ba146c422d # 1.04.2025 vcpkgDirectory: ${{env.BUILD_DIR}}/vcpkg vcpkgJsonGlob: '**/vcpkg.json' - name: "[Win] Install dependencies" if: matrix.os == 'windows-latest' run: | - vcpkg install + vcpkg install --triplet x64-windows python3 -m pip install -r third_party/requirements.txt - name: "[Lin] Install apt packages" diff --git a/.github/workflows/reusable_compatibility.yml b/.github/workflows/reusable_compatibility.yml index 5116a59f4..523b09e12 100644 --- a/.github/workflows/reusable_compatibility.yml +++ b/.github/workflows/reusable_compatibility.yml @@ -118,14 +118,14 @@ jobs: - name: Initialize vcpkg uses: lukka/run-vcpkg@5e0cab206a5ea620130caf672fce3e4a6b5666a1 # v11.5 with: - vcpkgGitCommitId: 3dd44b931481d7a8e9ba412621fa810232b66289 + vcpkgGitCommitId: ea2a964f9303270322cf3f2d51c265ba146c422d # 1.04.2025 vcpkgDirectory: ${{github.workspace}}/vcpkg vcpkgJsonGlob: '**/vcpkg.json' # NOTE we use vcpkg setup from "tag" version - name: Install dependencies working-directory: ${{github.workspace}}/tag_version - run: vcpkg install + run: vcpkg install --triplet x64-windows shell: pwsh # Specifies PowerShell as the shell for running the script. - name: Configure "tag" UMF build diff --git a/.github/workflows/reusable_fast.yml b/.github/workflows/reusable_fast.yml index 90a8f023f..7b1087ed0 100644 --- a/.github/workflows/reusable_fast.yml +++ b/.github/workflows/reusable_fast.yml @@ -60,13 +60,13 @@ jobs: if: matrix.os == 'windows-latest' uses: lukka/run-vcpkg@5e0cab206a5ea620130caf672fce3e4a6b5666a1 # v11.5 with: - vcpkgGitCommitId: 3dd44b931481d7a8e9ba412621fa810232b66289 + vcpkgGitCommitId: ea2a964f9303270322cf3f2d51c265ba146c422d # 1.04.2025 vcpkgDirectory: ${{env.BUILD_DIR}}/vcpkg vcpkgJsonGlob: '**/vcpkg.json' - name: Install dependencies (windows-latest) if: matrix.os == 'windows-latest' - run: vcpkg install + run: vcpkg install --triplet x64-windows shell: pwsh # Specifies PowerShell as the shell for running the script. - name: Install dependencies diff --git a/.github/workflows/reusable_gpu.yml b/.github/workflows/reusable_gpu.yml index 721d85206..6fcd40820 100644 --- a/.github/workflows/reusable_gpu.yml +++ b/.github/workflows/reusable_gpu.yml @@ -68,13 +68,13 @@ jobs: if: matrix.os == 'Windows' uses: lukka/run-vcpkg@5e0cab206a5ea620130caf672fce3e4a6b5666a1 # v11.5 with: - vcpkgGitCommitId: 3dd44b931481d7a8e9ba412621fa810232b66289 + vcpkgGitCommitId: ea2a964f9303270322cf3f2d51c265ba146c422d # 1.04.2025 vcpkgDirectory: ${{env.BUILD_DIR}}/vcpkg vcpkgJsonGlob: '**/vcpkg.json' - name: "[Win] Install dependencies" if: matrix.os == 'Windows' - run: vcpkg install + run: vcpkg install --triplet x64-windows # note: disable all providers except the one being tested # '-DCMAKE_SUPPRESS_REGENERATION=ON' is the WA for the error: "CUSTOMBUILD : CMake error : Cannot restore timestamp" diff --git a/.github/workflows/reusable_sanitizers.yml b/.github/workflows/reusable_sanitizers.yml index 1a044308e..c74448e1d 100644 --- a/.github/workflows/reusable_sanitizers.yml +++ b/.github/workflows/reusable_sanitizers.yml @@ -106,12 +106,12 @@ jobs: - name: Initialize vcpkg uses: lukka/run-vcpkg@5e0cab206a5ea620130caf672fce3e4a6b5666a1 # v11.5 with: - vcpkgGitCommitId: 3dd44b931481d7a8e9ba412621fa810232b66289 + vcpkgGitCommitId: ea2a964f9303270322cf3f2d51c265ba146c422d # 1.04.2025 vcpkgDirectory: ${{env.BUILD_DIR}}/vcpkg vcpkgJsonGlob: '**/vcpkg.json' - name: Install dependencies - run: vcpkg install + run: vcpkg install --triplet x64-windows shell: pwsh # Specifies PowerShell as the shell for running the script. # TODO enable level zero provider From f74658d2e9c7a0c09454eac8d09078e4bb8cd93b Mon Sep 17 00:00:00 2001 From: Igor Chorazewicz Date: Tue, 8 Apr 2025 23:18:46 +0000 Subject: [PATCH 6/9] L0 provider: fix initialization In case of error in the init function, cleanup was being called on 'provider' which was being initialized at the very end of init function, instead of 'ze_provider'. --- src/provider/provider_level_zero.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/provider/provider_level_zero.c b/src/provider/provider_level_zero.c index a4c68b391..af81e84bc 100644 --- a/src/provider/provider_level_zero.c +++ b/src/provider/provider_level_zero.c @@ -629,7 +629,7 @@ static umf_result_t ze_memory_provider_initialize(void *params, umf_result_t result = query_min_page_size(ze_provider, &ze_provider->min_page_size); if (result != UMF_RESULT_SUCCESS) { - ze_memory_provider_finalize(provider); + ze_memory_provider_finalize(ze_provider); return result; } From 1f23a445ad9caa9f36c0ae891ca596090a3150e0 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Wed, 9 Apr 2025 14:09:07 +0200 Subject: [PATCH 7/9] Verify result of umfPoolFree() in poolFixtures.hpp Signed-off-by: Lukasz Dorau --- test/poolFixtures.hpp | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/test/poolFixtures.hpp b/test/poolFixtures.hpp index de5a54685..6244a7da8 100644 --- a/test/poolFixtures.hpp +++ b/test/poolFixtures.hpp @@ -136,7 +136,8 @@ TEST_P(umfPoolTest, allocFree) { auto *ptr = umfPoolMalloc(pool.get(), allocSize); ASSERT_NE(ptr, nullptr); std::memset(ptr, 0, allocSize); - umfPoolFree(pool.get(), ptr); + umf_result_t umf_result = umfPoolFree(pool.get(), ptr); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } TEST_P(umfPoolTest, allocFreeNonAlignedSizes) { @@ -144,7 +145,8 @@ TEST_P(umfPoolTest, allocFreeNonAlignedSizes) { auto *ptr = umfPoolMalloc(pool.get(), allocSize); ASSERT_NE(ptr, nullptr); std::memset(ptr, 0, allocSize); - umfPoolFree(pool.get(), ptr); + umf_result_t umf_result = umfPoolFree(pool.get(), ptr); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } } @@ -160,7 +162,8 @@ TEST_P(umfPoolTest, reallocFree) { auto *new_ptr = umfPoolRealloc(pool.get(), ptr, allocSize * multiplier); ASSERT_NE(new_ptr, nullptr); std::memset(new_ptr, 0, allocSize * multiplier); - umfPoolFree(pool.get(), new_ptr); + umf_result_t umf_result = umfPoolFree(pool.get(), new_ptr); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } TEST_P(umfPoolTest, callocFree) { @@ -174,7 +177,8 @@ TEST_P(umfPoolTest, callocFree) { for (size_t i = 0; i < num; ++i) { ASSERT_EQ(((int *)ptr)[i], 0); } - umfPoolFree(pool.get(), ptr); + umf_result_t umf_result = umfPoolFree(pool.get(), ptr); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } void pow2AlignedAllocHelper(umf_memory_pool_handle_t pool) { @@ -195,7 +199,8 @@ void pow2AlignedAllocHelper(umf_memory_pool_handle_t pool) { } for (auto &ptr : allocs) { - umfPoolFree(pool, ptr); + umf_result_t umf_result = umfPoolFree(pool, ptr); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } } } @@ -227,7 +232,8 @@ TEST_P(umfPoolTest, multiThreadedMallocFree) { } for (auto allocation : allocations) { - umfPoolFree(inPool, allocation); + umf_result_t umf_result = umfPoolFree(inPool, allocation); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } }; @@ -280,7 +286,8 @@ TEST_P(umfPoolTest, multiThreadedReallocFree) { for (auto allocation : allocations) { auto *ptr = umfPoolRealloc(inPool, allocation, allocSize * multiplier); - umfPoolFree(inPool, ptr); + umf_result_t umf_result = umfPoolFree(inPool, ptr); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } }; @@ -310,7 +317,8 @@ TEST_P(umfPoolTest, multiThreadedCallocFree) { } for (auto allocation : allocations) { - umfPoolFree(inPool, allocation); + umf_result_t umf_result = umfPoolFree(inPool, allocation); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } }; @@ -335,7 +343,8 @@ TEST_P(umfPoolTest, multiThreadedMallocFreeRandomSizes) { } for (auto allocation : allocations) { - umfPoolFree(inPool, allocation); + umf_result_t umf_result = umfPoolFree(inPool, allocation); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } }; @@ -375,7 +384,8 @@ TEST_P(umfMemTest, outOfMem) { ASSERT_NE(allocations.back(), nullptr); for (int i = 0; i < expectedRecycledPoolAllocs; i++) { - umfPoolFree(hPool, allocations.back()); + umf_result_t umf_result = umfPoolFree(hPool, allocations.back()); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); allocations.pop_back(); } @@ -385,7 +395,8 @@ TEST_P(umfMemTest, outOfMem) { } for (auto allocation : allocations) { - umfPoolFree(hPool, allocation); + umf_result_t umf_result = umfPoolFree(hPool, allocation); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } } @@ -490,7 +501,8 @@ TEST_P(umfPoolTest, mallocUsableSize) { // Make sure we can write to this memory memset(ptr, 123, result); - umfPoolFree(pool.get(), ptr); + umf_result_t umf_result = umfPoolFree(pool.get(), ptr); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } } } From 2880017a8815d98c6b3e3e1845612618b8e3cb8e Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Wed, 9 Apr 2025 14:46:56 +0200 Subject: [PATCH 8/9] Fix file_alloc_aligned() `new_offset_mmap` can be greater than `file_provider->size_mmap`, so `file_provider->size_mmap - new_offset_mmap` would be an underflow in this case. Signed-off-by: Lukasz Dorau --- src/provider/provider_file_memory.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/provider/provider_file_memory.c b/src/provider/provider_file_memory.c index 5cc377f32..12a923322 100644 --- a/src/provider/provider_file_memory.c +++ b/src/provider/provider_file_memory.c @@ -429,6 +429,8 @@ static umf_result_t file_alloc_aligned(file_memory_provider_t *file_provider, return UMF_RESULT_ERROR_UNKNOWN; } + assert(file_provider->offset_mmap <= file_provider->size_mmap); + if (file_provider->size_mmap - file_provider->offset_mmap < size) { umf_result = file_mmap_aligned(file_provider, size, alignment); if (umf_result != UMF_RESULT_SUCCESS) { @@ -454,7 +456,8 @@ static umf_result_t file_alloc_aligned(file_memory_provider_t *file_provider, size_t new_offset_fd = file_provider->offset_fd + new_offset_mmap - file_provider->offset_mmap; - if (file_provider->size_mmap - new_offset_mmap < size) { + // new_offset_mmap can be greater than file_provider->size_mmap + if (file_provider->size_mmap < size + new_offset_mmap) { umf_result = file_mmap_aligned(file_provider, size, alignment); if (umf_result != UMF_RESULT_SUCCESS) { utils_mutex_unlock(&file_provider->lock); From 9ce2e8d5c6ecf1b3db22a0fdbb1068b19c0722b4 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Thu, 10 Apr 2025 14:53:08 +0200 Subject: [PATCH 9/9] Add allocFreeAligned test to poolFixtures.hpp Signed-off-by: Lukasz Dorau --- test/poolFixtures.hpp | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/test/poolFixtures.hpp b/test/poolFixtures.hpp index 6244a7da8..5f39e021d 100644 --- a/test/poolFixtures.hpp +++ b/test/poolFixtures.hpp @@ -150,6 +150,28 @@ TEST_P(umfPoolTest, allocFreeNonAlignedSizes) { } } +TEST_P(umfPoolTest, allocFreeAligned) { +// ::aligned_alloc(alignment=4096, size=1) does not work under sanitizers for unknown reason +#if defined(_WIN32) || defined(__SANITIZE_ADDRESS__) || \ + defined(__SANITIZE_THREAD__) + // TODO: implement support for windows + GTEST_SKIP(); +#else + if (!umf_test::isAlignedAllocSupported(pool.get())) { + GTEST_SKIP(); + } + + size_t alignment = 4 * 1024; // 4kB + void *ptr = umfPoolAlignedMalloc(pool.get(), 1, alignment); + ASSERT_NE(ptr, nullptr); + ASSERT_TRUE(reinterpret_cast(ptr) % alignment == 0); + *(reinterpret_cast(ptr)) = (unsigned char)0xFF; + + umf_result_t umf_result = umfPoolFree(pool.get(), ptr); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); +#endif +} + TEST_P(umfPoolTest, reallocFree) { if (!umf_test::isReallocSupported(pool.get())) { GTEST_SKIP(); @@ -203,6 +225,27 @@ void pow2AlignedAllocHelper(umf_memory_pool_handle_t pool) { ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } } + +// ::aligned_alloc(alignment=4096, size=1) does not work under sanitizers for unknown reason +#if !defined(__SANITIZE_ADDRESS__) && !defined(__SANITIZE_THREAD__) + // the same for size = 1 + for (size_t alignment = 1; alignment <= maxAlignment; alignment <<= 1) { + std::vector allocs; + + for (size_t alloc = 0; alloc < numAllocs; alloc++) { + auto *ptr = umfPoolAlignedMalloc(pool, 1, alignment); + ASSERT_NE(ptr, nullptr); + ASSERT_TRUE(reinterpret_cast(ptr) % alignment == 0); + *(reinterpret_cast(ptr)) = (unsigned char)0xFF; + allocs.push_back(ptr); + } + + for (auto &ptr : allocs) { + umf_result_t umf_result = umfPoolFree(pool, ptr); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + } + } +#endif } TEST_P(umfPoolTest, pow2AlignedAlloc) {