Merge pull request #121 from kswiecicki/add-sanitizers

Add sanitizers

Author: ldorau

.github/workflows/basic.yml

@@ -17,6 +17,7 @@ jobs:
         pool_tracking: ['ON', 'OFF']
         shared_library: ['OFF']
         os_provider: ['ON']
+        sanitizers: [{asan: 'OFF', ubsan: 'OFF', tsan: 'OFF'}]
         include:
           - os: 'ubuntu-20.04'
             build_type: Release
@@ -42,17 +43,34 @@ jobs:
             compiler: {c: gcc, cxx: g++}
             shared_library: 'OFF'
             os_provider: 'OFF'
+          # TODO: Enable sanitizer checks with pool tracking enabled. There's
+          # a leak reported by sanitizer in jemalloc pool implementation that
+          # points to a tracker_value_t not being free'd.
+          # TODO: Move jobs with sanitizer checks to a separate workflow file.
+          - os: 'ubuntu-22.04'
+            build_type: Debug
+            compiler: {c: clang, cxx: clang++}
+            pool_tracking: 'OFF'
+            shared_library: 'OFF'
+          # TSAN is mutually exclusive with other sanitizers
+            sanitizers: [{asan: 'ON', ubsan: 'ON', tsan: 'OFF'}, {asan: 'OFF', ubsan: 'OFF', tsan: 'ON'}]
+          - os: 'ubuntu-22.04'
+            build_type: Debug
+            compiler: {c: gcc, cxx: g++}
+            pool_tracking: 'OFF'
+            shared_library: 'OFF'
+            sanitizers: [{asan: 'ON', ubsan: 'ON', tsan: 'OFF'}, {asan: 'OFF', ubsan: 'OFF', tsan: 'ON'}]
     runs-on: ${{matrix.os}}
 
     steps:
       - name: Checkout
         uses: actions/checkout@v4
-        
+
       - name: Install apt packages
         run: |
           sudo apt-get update
           sudo apt-get install -y clang cmake libnuma-dev libjemalloc-dev libtbb-dev
-      
+
       - name: Install g++-7
         if: matrix.compiler.cxx == 'g++-7'
         run: |
@@ -74,11 +92,14 @@ jobs:
           -DUMF_BUILD_LIBUMF_POOL_JEMALLOC=ON
           -DUMF_BUILD_LIBUMF_POOL_DISJOINT=ON
           -DUMF_BUILD_LIBUMF_POOL_SCALABLE=ON
+          -DUSE_ASAN=${{matrix.sanitizers.asan}}
+          -DUSE_UBSAN=${{matrix.sanitizers.ubsan}}
+          -DUSE_TSAN=${{matrix.sanitizers.tsan}}
 
       - name: Build UMF
         run: |
           cmake --build ${{env.BUILD_DIR}} -j $(nproc)
-          
+
       - name: Run tests
         working-directory: ${{env.BUILD_DIR}}
         run: |
@@ -102,6 +123,8 @@ jobs:
         compiler: [{c: cl, cxx: cl}]
         pool_tracking: ['ON', 'OFF']
         shared_library: ['OFF']
+        # ASAN is the only available sanitizer on Windows
+        sanitizers: [{asan: 'OFF'}]
         include:
           - os: 'windows-2022'
             build_type: Release
@@ -113,6 +136,19 @@ jobs:
             compiler: {c: cl, cxx: cl}
             pool_tracking: 'ON'
             shared_library: 'ON'
+          - os: 'windows-2022'
+            build_type: Debug
+            compiler: {c: clang-cl, cxx: clang-cl}
+            pool_tracking: 'OFF'
+            shared_library: 'OFF'
+            sanitizers: [{asan: 'ON'}]
+          - os: 'windows-2022'
+            build_type: Debug
+            compiler: {c: cl, cxx: cl}
+            pool_tracking: 'OFF'
+            shared_library: 'OFF'
+            sanitizers: [{asan: 'ON'}]
+
     runs-on: ${{matrix.os}}
 
     steps:
@@ -130,6 +166,7 @@ jobs:
           -DUMF_FORMAT_CODE_STYLE=OFF
           -DUMF_DEVELOPER_MODE=ON
           -DUMF_BUILD_LIBUMF_POOL_DISJOINT=ON
+          -DUSE_ASAN=${{matrix.sanitizers.asan}}
 
       - name: Build UMF
         run: cmake --build ${{env.BUILD_DIR}} --config ${{matrix.build_type}} -j $Env:NUMBER_OF_PROCESSORS

CMakeLists.txt

@@ -26,6 +26,10 @@ option(UMF_BUILD_BENCHMARKS "Build UMF benchmarks" OFF)
 option(UMF_ENABLE_POOL_TRACKING "Build UMF with pool tracking" ON)
 option(UMF_DEVELOPER_MODE "Enable developer checks, treats warnings as errors" OFF)
 option(UMF_FORMAT_CODE_STYLE "Format UMF code with clang-format" OFF)
+option(USE_ASAN "Enable AddressSanitizer checks" OFF)
+option(USE_UBSAN "Enable UndefinedBehaviorSanitizer checks" OFF)
+option(USE_TSAN "Enable ThreadSanitizer checks" OFF)
+option(USE_MSAN "Enable MemorySanitizer checks" OFF)
 
 if(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
     set(LINUX TRUE)
@@ -57,6 +61,21 @@ if(MSVC)
     set(CUSTOM_COMMAND_BINARY_DIR ${CUSTOM_COMMAND_BINARY_DIR}/$<CONFIG>)
 endif()
 
+# Sanitizer flags
+if(USE_ASAN)
+    add_sanitizer_flag(address)
+endif()
+if(USE_UBSAN)
+    add_sanitizer_flag(undefined)
+endif()
+if(USE_TSAN)
+    add_sanitizer_flag(thread)
+endif()
+if(USE_MSAN)
+    message(WARNING "MemorySanitizer requires instrumented libraries to prevent reporting false-positives")
+    add_sanitizer_flag(memory)
+endif()
+
 # A header only library to specify include directories in transitive
 # dependencies.
 add_library(umf_headers INTERFACE)
@@ -94,7 +113,7 @@ endif()
 if(UMF_FORMAT_CODE_STYLE)
     find_program(CLANG_FORMAT NAMES clang-format-15 clang-format-15.0 clang-format)
 
-    if(CLANG_FORMAT)        
+    if(CLANG_FORMAT)
         get_program_version_major_minor(${CLANG_FORMAT} CLANG_FORMAT_VERSION)
         message(STATUS "Found clang-format: ${CLANG_FORMAT} (version: ${CLANG_FORMAT_VERSION})")
 
@@ -105,7 +124,7 @@ if(UMF_FORMAT_CODE_STYLE)
     else()
         message(FATAL_ERROR "UMF_FORMAT_CODE_STYLE=ON, but clang-format not found (required version: ${CLANG_FORMAT_REQUIRED})")
     endif()
-    
+
     # Obtain files for clang-format check
     set(format_glob)
     foreach(DIR IN ITEMS include src test benchmark)
@@ -123,11 +142,11 @@ if(UMF_FORMAT_CODE_STYLE)
     file(GLOB_RECURSE format_list ${format_glob})
 
     message(STATUS "Adding clang-format-check and clang-format-apply make targets")
-    
+
     add_custom_target(clang-format-check
         COMMAND ${CLANG_FORMAT}
-            --style=file            
-            --dry-run 
+            --style=file
+            --dry-run
             -Werror
             ${format_list}
         COMMENT "Check files formatting using clang-format")
@@ -137,7 +156,7 @@ if(UMF_FORMAT_CODE_STYLE)
             --style=file
             --i
             ${format_list}
-        COMMENT "Format files using clang-format")        
+        COMMENT "Format files using clang-format")
 endif()
 
 # Add license to the installation path

README.md

@@ -13,7 +13,7 @@ The Unified Memory Framework (UMF) is a library for constructing allocators and
 
 A UMF memory pool is a combination of a pool allocator and a memory provider. A memory provider is responsible for coarse-grained memory allocations and management of memory pages, while the pool allocator controls memory pooling and handles fine-grained memory allocations.
 
-Pool allocator can leverage existing allocators (e.g. jemalloc or tbbmalloc) or be written from scratch. 
+Pool allocator can leverage existing allocators (e.g. jemalloc or tbbmalloc) or be written from scratch.
 
 UMF comes with predefined pool allocators (see include/pool) and providers (see include/provider). UMF can also work with user-defined pools and providers that implement a specific interface (see include/umf/memory_pool_ops.h and include/umf/memory_provider_ops.h)
 
@@ -98,6 +98,22 @@ $ cd build
 $ cmake {path_to_source_dir}
 $ make
 ```
+
+### Sanitizers
+
+List of sanitizers available on Linux:
+- AddressSanitizer
+- UndefinedBehaviorSanitizer
+- ThreadSanitizer
+   - Is mutually exclusive with other sanitizers.
+- MemorySanitizer
+   - Requires linking against MSan-instrumented libraries to prevent false positive reports. More information [here](https://github.com/google/sanitizers/wiki/MemorySanitizerLibcxxHowTo).
+
+List of sanitizers available on Windows:
+- AddressSanitizer
+
+Listed sanitizers can be enabled with appropriate [CMake options](#cmake-standard-options).
+
 ## Contributions
 
 All code has to be formatted using clang-format. To check the formatting do:
@@ -131,3 +147,7 @@ List of options provided by CMake:
 | UMF_ENABLE_POOL_TRACKING | Build UMF with pool tracking | ON/OFF | ON |
 | UMF_DEVELOPER_MODE | Treat warnings as errors and enables additional checks | ON/OFF | OFF |
 | UMF_FORMAT_CODE_STYLE | Add clang-format-check and clang-format-apply targets to make | ON/OFF | OFF |
+| USE_ASAN | Enable AddressSanitizer checks | ON/OFF | OFF |
+| USE_UBSAN | Enable UndefinedBehaviorSanitizer checks | ON/OFF | OFF |
+| USE_TSAN | Enable ThreadSanitizer checks | ON/OFF | OFF |
+| USE_MSAN | Enable MemorySanitizer checks | ON/OFF | OFF |

cmake/helpers.cmake

@@ -1,11 +1,15 @@
-# Copyright (C) 2023 Intel Corporation
+# Copyright (C) 2023-2024 Intel Corporation
 # Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 #
 # helpers.cmake -- helper functions for top-level CMakeLists.txt
 #
 
+# CMake modules that check whether the C/C++ compiler supports a given flag
+include(CheckCCompilerFlag)
+include(CheckCXXCompilerFlag)
+
 # Sets ${ret} to version of program specified by ${name} in major.minor format
 function(get_program_version_major_minor name ret)
     execute_process(COMMAND ${name} --version
@@ -104,3 +108,48 @@ function(add_umf_library)
     add_umf_target_compile_options(${ARG_NAME})
     add_umf_target_link_options(${ARG_NAME})
 endfunction()
+
+# Add sanitizer ${flag}, if it is supported, for both C and C++ compiler
+macro(add_sanitizer_flag flag)
+    # Save current 'CMAKE_REQUIRED_LIBRARIES' state and temporarily extend it with
+    # '-fsanitize=${flag}'. It is required by CMake to check the compiler for
+    # availability of provided sanitizer ${flag}.
+    if(WINDOWS)
+        set(SANITIZER_FLAG "/fsanitize=${flag}")
+        set(SANITIZER_ARGS "")
+    else()
+        set(SANITIZER_FLAG "-fsanitize=${flag}")
+        set(SANITIZER_ARGS "-fno-sanitize-recover=all")
+    endif()
+
+    set(SAVED_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
+    set(CMAKE_REQUIRED_LIBRARIES "${CMAKE_REQUIRED_LIBRARIES} ${SANITIZER_FLAG}")
+
+    if(${flag} STREQUAL "address")
+        set(check_name "HAS_ASAN")
+    elseif(${flag} STREQUAL "undefined")
+        set(check_name "HAS_UBSAN")
+    elseif(${flag} STREQUAL "thread")
+        set(check_name "HAS_TSAN")
+    elseif(${flag} STREQUAL "memory")
+        set(check_name "HAS_MSAN")
+    endif()
+
+    # Check C and CXX compilers for given sanitizer flag.
+    check_c_compiler_flag("${SANITIZER_FLAG}" "C_${check_name}")
+    check_cxx_compiler_flag("${SANITIZER_FLAG}" "CXX_${check_name}")
+    if (${C_${check_name}} OR ${CXX_${check_name}})
+        # Set appropriate linker flags for building executables.
+        set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${SANITIZER_FLAG} ${SANITIZER_ARGS}")
+        if (${C_${check_name}})
+            set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${SANITIZER_FLAG} ${SANITIZER_ARGS}")
+        endif()
+        if (${CXX_${check_name}})
+            set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${SANITIZER_FLAG} ${SANITIZER_ARGS}")
+        endif()
+    else()
+        message(FATAL_ERROR "${flag} sanitizer is not supported (neither by C nor CXX compiler)")
+    endif()
+
+    set(CMAKE_REQUIRED_LIBRARIES ${SAVED_CMAKE_REQUIRED_LIBRARIES})
+endmacro()

src/critnib/critnib.c

@@ -1,6 +1,6 @@
 /*
  *
- * Copyright (C) 2023 Intel Corporation
+ * Copyright (C) 2023-2024 Intel Corporation
  *
  * Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
  * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
@@ -715,8 +715,8 @@ int critnib_find(struct critnib *c, uintptr_t key, enum find_dir_t dir,
                  uintptr_t *rkey, void **rvalue) {
     uint64_t wrs1, wrs2;
     struct critnib_leaf *k;
-    uintptr_t _rkey;
-    void **_rvalue;
+    uintptr_t _rkey = (uintptr_t)0x0;
+    void **_rvalue = NULL;
 
     /* <42 ≡ ≤41 */
     if (dir < -1) {

test/common/provider.hpp

@@ -1,6 +1,6 @@
 /*
  *
- * Copyright (C) 2023 Intel Corporation
+ * Copyright (C) 2023-2024 Intel Corporation
  *
  * Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
  * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
@@ -17,6 +17,7 @@
 
 #include "base.hpp"
 #include "cpp_helpers.hpp"
+#include "test_helpers.h"
 
 namespace umf_test {
 
@@ -68,10 +69,16 @@ struct provider_malloc : public provider_base_t {
             align = 8;
         }
 
+        // aligned_malloc returns a valid pointer despite not meeting the
+        // requirement of 'size' being multiple of 'align' even though the
+        // documentation says that it has to. AddressSanitizer returns an
+        // error because of this issue.
+        size_t aligned_size = ALIGN_UP(size, align);
+
 #ifdef _WIN32
-        *ptr = _aligned_malloc(size, align);
+        *ptr = _aligned_malloc(aligned_size, align);
 #else
-        *ptr = ::aligned_alloc(align, size);
+        *ptr = ::aligned_alloc(align, aligned_size);
 #endif
 
         return (*ptr) ? UMF_RESULT_SUCCESS

test/common/test_helpers.h

@@ -69,6 +69,8 @@ static inline void UT_OUT(const char *format, ...) {
                       (unsigned long long)(rhs)),                              \
              0)))
 
+#define ALIGN_UP(size, align) (((size) + (align)-1) & ~((align)-1))
+
 int bufferIsFilledWithChar(void *ptr, size_t size, char c);
 
 int buffersHaveSameContent(void *first, void *second, size_t size);