Skip to content

Add sanitizers #121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 40 additions & 3 deletions .github/workflows/basic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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: |
Expand All @@ -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: |
Expand All @@ -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
Expand All @@ -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:
Expand All @@ -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
Expand Down
31 changes: 25 additions & 6 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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})")

Expand All @@ -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)
Expand All @@ -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")
Expand All @@ -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
Expand Down
22 changes: 21 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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 |
51 changes: 50 additions & 1 deletion cmake/helpers.cmake
Original file line number Diff line number Diff line change
@@ -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
#
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls bump dates (in all files)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not done in the top-level CMakeLists

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: #150


# 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
Expand Down Expand Up @@ -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()
6 changes: 3 additions & 3 deletions src/critnib/critnib.c
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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) {
Expand Down
13 changes: 10 additions & 3 deletions test/common/provider.hpp
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -17,6 +17,7 @@

#include "base.hpp"
#include "cpp_helpers.hpp"
#include "test_helpers.h"

namespace umf_test {

Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions test/common/test_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down