Skip to content
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
6 changes: 6 additions & 0 deletions .github/workflows/reusable_sanitizers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ jobs:

- name: Run tests
working-directory: ${{env.BUILD_DIR}}
env:
ASAN_OPTIONS: allocator_may_return_null=1
TSAN_OPTIONS: allocator_may_return_null=1
run: |
${{ matrix.compiler.cxx == 'icpx' && '. /opt/intel/oneapi/setvars.sh' || true }}
ctest --output-on-failure
Expand Down Expand Up @@ -141,4 +144,7 @@ jobs:

- name: Run tests
working-directory: ${{env.BUILD_DIR}}
env:
ASAN_OPTIONS: allocator_may_return_null=1
TSAN_OPTIONS: allocator_may_return_null=1
run: ctest -C Debug --output-on-failure
10 changes: 8 additions & 2 deletions src/base_alloc/base_alloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,10 @@ static void *ba_os_alloc_annotated(size_t pool_size) {
}

umf_ba_pool_t *umf_ba_create(size_t size) {
size_t chunk_size = ALIGN_UP(size, MEMORY_ALIGNMENT);
size_t chunk_size = ALIGN_UP_SAFE(size, MEMORY_ALIGNMENT);
if (chunk_size == 0) {
return NULL;
}
size_t mutex_size = ALIGN_UP(utils_mutex_get_size(), MEMORY_ALIGNMENT);

size_t metadata_size = sizeof(struct umf_ba_main_pool_meta_t);
Expand All @@ -144,7 +147,10 @@ umf_ba_pool_t *umf_ba_create(size_t size) {
pool_size = MINIMUM_POOL_SIZE;
}

pool_size = ALIGN_UP(pool_size, ba_os_get_page_size());
pool_size = ALIGN_UP_SAFE(pool_size, ba_os_get_page_size());
if (pool_size == 0) {
return NULL;
}

umf_ba_pool_t *pool = (umf_ba_pool_t *)ba_os_alloc_annotated(pool_size);
if (!pool) {
Expand Down
17 changes: 15 additions & 2 deletions src/base_alloc/base_alloc_global.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,12 @@ static void *add_metadata_and_align(void *ptr, size_t size, size_t alignment) {
if (alignment <= ALLOC_METADATA_SIZE) {
user_ptr = (void *)((uintptr_t)ptr + ALLOC_METADATA_SIZE);
} else {
user_ptr =
(void *)ALIGN_UP((uintptr_t)ptr + ALLOC_METADATA_SIZE, alignment);
user_ptr = (void *)ALIGN_UP_SAFE((uintptr_t)ptr + ALLOC_METADATA_SIZE,
alignment);
if (!user_ptr) {
LOG_ERR("base_alloc: pointer alignment overflow");
return NULL;
}
}

size_t ptr_offset_from_original = (uintptr_t)user_ptr - (uintptr_t)ptr;
Expand Down Expand Up @@ -155,10 +159,19 @@ void *umf_ba_global_aligned_alloc(size_t size, size_t alignment) {
return NULL;
}

if (size > SIZE_MAX - ALLOC_METADATA_SIZE) {
LOG_ERR("base_alloc: allocation size (%zu) too large.", size);
return NULL;
}

// for metadata
size += ALLOC_METADATA_SIZE;

if (alignment > ALLOC_METADATA_SIZE) {
if (size > SIZE_MAX - alignment) {
LOG_ERR("base_alloc: allocation size (%zu) too large.", size);
return NULL;
}
size += alignment;
}

Expand Down
19 changes: 16 additions & 3 deletions src/base_alloc/base_alloc_linear.c
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ umf_ba_linear_pool_t *umf_ba_linear_create(size_t pool_size) {
pool_size = MINIMUM_LINEAR_POOL_SIZE;
}

pool_size = ALIGN_UP(pool_size, ba_os_get_page_size());
pool_size = ALIGN_UP_SAFE(pool_size, ba_os_get_page_size());
if (pool_size == 0) {
LOG_ERR("pool_size page alignment overflow");
return NULL;
}

umf_ba_linear_pool_t *pool = (umf_ba_linear_pool_t *)ba_os_alloc(pool_size);
if (!pool) {
Expand Down Expand Up @@ -122,15 +126,24 @@ void *umf_ba_linear_alloc(umf_ba_linear_pool_t *pool, size_t size) {
if (size == 0) {
return NULL;
}
size_t aligned_size = ALIGN_UP(size, MEMORY_ALIGNMENT);
size_t aligned_size = ALIGN_UP_SAFE(size, MEMORY_ALIGNMENT);
if (aligned_size == 0) {
LOG_ERR("size alignment overflow");
return NULL;
}
utils_mutex_lock(&pool->metadata.lock);
if (pool->metadata.size_left < aligned_size) {
size_t pool_size = MINIMUM_LINEAR_POOL_SIZE;
size_t usable_size =
pool_size - offsetof(umf_ba_next_linear_pool_t, data);
if (usable_size < aligned_size) {
pool_size += aligned_size - usable_size;
pool_size = ALIGN_UP(pool_size, ba_os_get_page_size());
pool_size = ALIGN_UP_SAFE(pool_size, ba_os_get_page_size());
if (pool_size == 0) {
utils_mutex_unlock(&pool->metadata.lock);
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the Helgrind log, I believe the problem is here. Are you sure it should be done here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I have to release the mutex before returning from this function.

LOG_ERR("pool_size page alignment overflow");
return NULL;
}
}

assert(pool_size - offsetof(umf_ba_next_linear_pool_t, data) >=
Expand Down
41 changes: 35 additions & 6 deletions src/provider/provider_os_memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,22 @@ validatePartitions(umf_os_memory_provider_params_t *params) {
return UMF_RESULT_SUCCESS;
}

static umf_result_t os_get_min_page_size(void *provider, void *ptr,
size_t *page_size);

static umf_result_t validatePartSize(os_memory_provider_t *provider,
umf_os_memory_provider_params_t *params) {
size_t page_size;
os_get_min_page_size(provider, NULL, &page_size);
if (ALIGN_UP(params->part_size, page_size) < params->part_size) {
LOG_ERR("partition size (%zu) is too big, cannot align with a page "
"size (%zu)",
params->part_size, page_size);
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}
return UMF_RESULT_SUCCESS;
}

static void free_bitmaps(os_memory_provider_t *provider) {
for (unsigned i = 0; i < provider->nodeset_len; i++) {
hwloc_bitmap_free(provider->nodeset[i]);
Expand Down Expand Up @@ -427,6 +443,14 @@ static umf_result_t translate_params(umf_os_memory_provider_params_t *in_params,
return result;
}

if (in_params->numa_mode == UMF_NUMA_MODE_INTERLEAVE) {
result = validatePartSize(provider, in_params);
if (result != UMF_RESULT_SUCCESS) {
LOG_ERR("incorrect partition size: %zu", in_params->part_size);
return result;
}
}

int is_dedicated_node_bind = dedicated_node_bind(in_params);
provider->numa_policy =
translate_numa_mode(in_params->numa_mode, is_dedicated_node_bind);
Expand Down Expand Up @@ -574,9 +598,6 @@ static void os_finalize(void *provider) {
umf_ba_global_free(os_provider);
}

static umf_result_t os_get_min_page_size(void *provider, void *ptr,
size_t *page_size);

// TODO: this function should be re-enabled when CTL is implemented
#if 0
static void print_numa_nodes(os_memory_provider_t *os_provider, void *addr,
Expand Down Expand Up @@ -813,12 +834,12 @@ static membind_t membindFirst(os_memory_provider_t *provider, void *addr,
membind_t membind;
memset(&membind, 0, sizeof(membind));

membind.alloc_size = ALIGN_UP(size, page_size);
membind.alloc_size = size;
membind.page_size = page_size;
membind.addr = addr;
membind.pages = membind.alloc_size / membind.page_size;
if (provider->nodeset_len == 1) {
membind.bind_size = ALIGN_UP(size, membind.page_size);
membind.bind_size = size;
membind.bitmap = provider->nodeset[0];
return membind;
}
Expand Down Expand Up @@ -924,7 +945,15 @@ static umf_result_t os_alloc(void *provider, size_t size, size_t alignment,

// Bind memory to NUMA nodes if numa_policy is other than DEFAULT
if (os_provider->numa_policy != HWLOC_MEMBIND_DEFAULT) {
membind_t membind = membindFirst(os_provider, addr, size, page_size);
size_t first_size = ALIGN_UP_SAFE(size, page_size);
if (first_size == 0) {
LOG_ERR("size is too big, page align failed");
(void)utils_munmap(addr, size);
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}

membind_t membind =
membindFirst(os_provider, addr, first_size, page_size);
if (membind.bitmap == NULL) {
goto err_unmap;
}
Expand Down
2 changes: 1 addition & 1 deletion src/proxy_lib/proxy_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ static inline void *ba_leak_realloc(void *ptr, size_t size, size_t max_size) {
static inline void *ba_leak_aligned_alloc(size_t alignment, size_t size) {
ba_leak_init_once();
void *ptr = umf_ba_linear_alloc(Base_alloc_leak, size + alignment);
return (void *)ALIGN_UP((uintptr_t)ptr, alignment);
return (void *)ALIGN_UP_SAFE((uintptr_t)ptr, alignment);
}

static inline int ba_leak_free(void *ptr) {
Expand Down
4 changes: 4 additions & 0 deletions src/utils/utils_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ typedef enum umf_purge_advise_t {
#define IS_NOT_ALIGNED(value, align) \
((align != 0 && (((value) & ((align)-1)) != 0)))
#define ALIGN_UP(value, align) (((value) + (align)-1) & ~((align)-1))
#define ALIGN_UP_SAFE(value, align) \
(((align) == 0) \
? (value) \
: (((value) + (align)-1) < (value) ? 0 : ALIGN_UP((value), (align))))
#define ALIGN_DOWN(value, align) ((value) & ~((align)-1))
#define ASSERT_IS_ALIGNED(value, align) \
DO_WHILE_EXPRS(assert(IS_ALIGNED(value, align)))
Expand Down
5 changes: 4 additions & 1 deletion test/common/provider.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ struct provider_ba_global : public provider_base_t {
// 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);
size_t aligned_size = ALIGN_UP_SAFE(size, align);
if (aligned_size == 0) {
return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
}

*ptr = umf_ba_global_aligned_alloc(aligned_size, align);

Expand Down
5 changes: 1 addition & 4 deletions test/common/test_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <umf/memory_provider_ops.h>

#include "provider_trace.h"
#include "utils_common.h"

#ifdef __cplusplus
extern "C" {
Expand All @@ -22,10 +23,6 @@ extern "C" {
// Needed for CI
#define TEST_SKIP_ERROR_CODE 125

#ifndef ALIGN_UP
#define ALIGN_UP(value, align) (((value) + (align)-1) & ~((align)-1))
#endif

int bufferIsFilledWithChar(void *ptr, size_t size, char c);

int buffersHaveSameContent(void *first, void *second, size_t size);
Expand Down
5 changes: 5 additions & 0 deletions test/poolFixtures.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -451,4 +451,9 @@ TEST_P(umfPoolTest, realloc_compliance) {

TEST_P(umfPoolTest, free_compliance) { free_compliance_test(pool.get()); }

TEST_P(umfPoolTest, allocMaxSize) {
auto *ptr = umfPoolMalloc(pool.get(), SIZE_MAX);
ASSERT_EQ(ptr, nullptr);
}

#endif /* UMF_TEST_POOL_FIXTURES_HPP */
6 changes: 6 additions & 0 deletions test/provider_os_memory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,12 @@ TEST_P(umfProviderTest, alloc_WRONG_SIZE) {
UMF_OS_RESULT_ERROR_ALLOC_FAILED);
}

TEST_P(umfProviderTest, alloc_MAX_SIZE) {
test_alloc_failure(provider.get(), SIZE_MAX, 0,
UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC,
UMF_OS_RESULT_ERROR_ALLOC_FAILED);
}

// other positive tests

TEST_P(umfProviderTest, get_min_page_size) {
Expand Down
18 changes: 18 additions & 0 deletions test/provider_os_memory_multiple_numa_nodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -774,3 +774,21 @@ TEST_F(testNuma, checkModeInterleaveIllegalArgSet) {
ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT);
ASSERT_EQ(os_memory_provider, nullptr);
}

// Interleave mode set with SIZE_MAX part size
TEST_F(testNuma, maxPartSize) {
std::vector<unsigned> numa_nodes = get_available_numa_nodes();

umf_os_memory_provider_params_t os_memory_provider_params =
UMF_OS_MEMORY_PROVIDER_PARAMS_TEST;
os_memory_provider_params.numa_mode = UMF_NUMA_MODE_INTERLEAVE;
os_memory_provider_params.part_size = SIZE_MAX;
os_memory_provider_params.numa_list = numa_nodes.data();
os_memory_provider_params.numa_list_len = numa_nodes.size();

auto res = umfMemoryProviderCreate(umfOsMemoryProviderOps(),
&os_memory_provider_params,
&os_memory_provider);
ASSERT_EQ(res, UMF_RESULT_ERROR_INVALID_ARGUMENT);
ASSERT_EQ(os_memory_provider, nullptr);
}
24 changes: 24 additions & 0 deletions test/supp/helgrind-umf_test-disjointPool.supp
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,27 @@
fun:*gthread_mutex_unlock*pthread_mutex_t
...
}

{
Incompatibility with helgrind's implementation ("pthread_rwlock_{rd,rw}lock with a pthread_mutex_t* argument")
Helgrind:Misc
obj:*vgpreload_helgrind-amd64-linux.so
fun:*glibcxx_rwlock_wrlock*pthread_rwlock_t
...
}

{
Incompatibility with helgrind's implementation ("pthread_rwlock_unlock with a pthread_mutex_t* argument")
Helgrind:Misc
obj:*vgpreload_helgrind-amd64-linux.so
fun:*glibcxx_rwlock_unlock*pthread_rwlock_t
...
}

{
Incompatibility with helgrind's implementation ("pthread_rwlock_{rd,rw}lock with a pthread_mutex_t* argument")
Helgrind:Misc
obj:*vgpreload_helgrind-amd64-linux.so
fun:*glibcxx_rwlock_rdlock*pthread_rwlock_t*
...
}
3 changes: 3 additions & 0 deletions test/test_valgrind.sh
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ for test in $(ls -1 umf_test-*); do
umf_test-memspace_lowest_latency)
FILTER='--gtest_filter="-*allocLocalMt*"'
;;
umf_test-memoryPool)
FILTER='--gtest_filter="-*allocMaxSize*"'
;;
esac

[ "$FILTER" != "" ] && echo -n "($FILTER) "
Expand Down
Loading