From 6d1109486121c879179c5df3bbb3dbe8d2585174 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Tue, 12 Nov 2024 12:01:49 +0100 Subject: [PATCH 1/9] Fix format string Signed-off-by: Lukasz Dorau --- test/ipc_os_prov_proxy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ipc_os_prov_proxy.c b/test/ipc_os_prov_proxy.c index a175186586..0a4b644425 100644 --- a/test/ipc_os_prov_proxy.c +++ b/test/ipc_os_prov_proxy.c @@ -220,7 +220,7 @@ int main(int argc, char *argv[]) { fprintf( stderr, "[producer] ERROR: The consumer did NOT write the correct value " - "(the old one / 2 = %llu) to my shared memory: %llu\n", + "(the old one / 2 = %zu) to my shared memory: %llu\n", expected_val, new_val); } From db2d8604603af95e6a5d14b5a242742a3f8c1fe4 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Fri, 8 Nov 2024 10:57:00 +0100 Subject: [PATCH 2/9] Fix checks in umfMemoryProviderAllocationSplit/Merge() Signed-off-by: Lukasz Dorau --- src/memory_provider.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/memory_provider.c b/src/memory_provider.c index 3d823a4a8b..f2a361e17a 100644 --- a/src/memory_provider.c +++ b/src/memory_provider.c @@ -305,6 +305,8 @@ umf_result_t umfMemoryProviderAllocationSplit(umf_memory_provider_handle_t hProvider, void *ptr, size_t totalSize, size_t firstSize) { + UMF_CHECK((hProvider != NULL), UMF_RESULT_ERROR_INVALID_ARGUMENT); + if (!ptr) { return UMF_RESULT_ERROR_INVALID_ARGUMENT; } @@ -325,6 +327,8 @@ umf_result_t umfMemoryProviderAllocationMerge(umf_memory_provider_handle_t hProvider, void *lowPtr, void *highPtr, size_t totalSize) { + UMF_CHECK((hProvider != NULL), UMF_RESULT_ERROR_INVALID_ARGUMENT); + if (!lowPtr || !highPtr) { return UMF_RESULT_ERROR_INVALID_ARGUMENT; } @@ -334,7 +338,7 @@ umfMemoryProviderAllocationMerge(umf_memory_provider_handle_t hProvider, if ((uintptr_t)lowPtr >= (uintptr_t)highPtr) { return UMF_RESULT_ERROR_INVALID_ARGUMENT; } - if ((uintptr_t)highPtr - (uintptr_t)lowPtr > totalSize) { + if ((uintptr_t)highPtr - (uintptr_t)lowPtr >= totalSize) { return UMF_RESULT_ERROR_INVALID_ARGUMENT; } From ab5735e4d863778af2f2619e96a5a99f987a7c66 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Sun, 10 Nov 2024 18:26:07 +0100 Subject: [PATCH 3/9] Do not assert(ptr) in umfMemoryTrackerGetAllocInfo() Do not assert(ptr) in umfMemoryTrackerGetAllocInfo(), return UMF_RESULT_ERROR_INVALID_ARGUMENT instead. Replace LOG_WARN() with LOG_DEBUG(). --- src/provider/provider_tracking.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/provider/provider_tracking.c b/src/provider/provider_tracking.c index 769ed6a946..e726feefb0 100644 --- a/src/provider/provider_tracking.c +++ b/src/provider/provider_tracking.c @@ -106,16 +106,19 @@ umf_memory_pool_handle_t umfMemoryTrackerGetPool(const void *ptr) { umf_result_t umfMemoryTrackerGetAllocInfo(const void *ptr, umf_alloc_info_t *pAllocInfo) { - assert(ptr); assert(pAllocInfo); + if (ptr == NULL) { + return UMF_RESULT_ERROR_INVALID_ARGUMENT; + } + if (TRACKER == NULL) { - LOG_ERR("tracker is not created"); + LOG_ERR("tracker does not exist"); return UMF_RESULT_ERROR_NOT_SUPPORTED; } if (TRACKER->map == NULL) { - LOG_ERR("tracker's map is not created"); + LOG_ERR("tracker's map does not exist"); return UMF_RESULT_ERROR_NOT_SUPPORTED; } @@ -124,9 +127,8 @@ umf_result_t umfMemoryTrackerGetAllocInfo(const void *ptr, int found = critnib_find(TRACKER->map, (uintptr_t)ptr, FIND_LE, (void *)&rkey, (void **)&rvalue); if (!found || (uintptr_t)ptr >= rkey + rvalue->size) { - LOG_WARN("pointer %p not found in the " - "tracker, TRACKER=%p", - ptr, (void *)TRACKER); + LOG_DEBUG("pointer %p not found in the tracker, TRACKER=%p", ptr, + (void *)TRACKER); return UMF_RESULT_ERROR_INVALID_ARGUMENT; } From a68f8c416073e26f600b8b3f060ec113c6c059e5 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Mon, 11 Nov 2024 10:30:45 +0100 Subject: [PATCH 4/9] Add utils_env_var_get_str() to utils_common Add utils_env_var_get_str() to utils_common. Use utils_env_var_get_str() inside utils_env_var_has_str() and utils_is_running_in_proxy_lib(). Signed-off-by: Lukasz Dorau --- src/utils/utils_common.c | 13 ++----------- src/utils/utils_common.h | 15 +++++++++++---- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/utils/utils_common.c b/src/utils/utils_common.c index 25169f6cf8..bffc9f3559 100644 --- a/src/utils/utils_common.c +++ b/src/utils/utils_common.c @@ -53,18 +53,9 @@ void utils_align_ptr_down_size_up(void **ptr, size_t *size, size_t alignment) { *size = s; } -int utils_env_var_has_str(const char *envvar, const char *str) { +char *utils_env_var_get_str(const char *envvar, const char *str) { char *value = getenv(envvar); - if (value && strstr(value, str)) { - return 1; - } - - return 0; -} - -// check if we are running in the proxy library -int utils_is_running_in_proxy_lib(void) { - return utils_env_var_has_str("LD_PRELOAD", "libumf_proxy.so"); + return value ? strstr(value, str) : NULL; } const char *utils_parse_var(const char *var, const char *option, diff --git a/src/utils/utils_common.h b/src/utils/utils_common.h index c25fda2ab0..7751a2ac96 100644 --- a/src/utils/utils_common.h +++ b/src/utils/utils_common.h @@ -62,8 +62,18 @@ typedef enum umf_purge_advise_t { #endif /* _WIN32 */ +// get the address of the given string in the environment variable (or NULL) +char *utils_env_var_get_str(const char *envvar, const char *str); + // Check if the environment variable contains the given string. -int utils_env_var_has_str(const char *envvar, const char *str); +static inline int utils_env_var_has_str(const char *envvar, const char *str) { + return utils_env_var_get_str(envvar, str) ? 1 : 0; +} + +// check if we are running in the proxy library +static inline int utils_is_running_in_proxy_lib(void) { + return utils_env_var_get_str("LD_PRELOAD", "libumf_proxy.so") ? 1 : 0; +} // utils_parse_var - Parses var for a prefix, // optionally identifying a following argument. @@ -82,9 +92,6 @@ int utils_env_var_has_str(const char *envvar, const char *str); const char *utils_parse_var(const char *var, const char *option, const char **extraArg); -// check if we are running in the proxy library -int utils_is_running_in_proxy_lib(void); - size_t utils_get_page_size(void); // align a pointer up and a size down From e02670edaf6c4fa36208645eb5ae0dd3eff9d7f1 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Tue, 12 Nov 2024 09:22:10 +0100 Subject: [PATCH 5/9] Do not destroy tracker in umfTearDown() under the proxy library Signed-off-by: Lukasz Dorau --- src/base_alloc/base_alloc_global.c | 2 ++ src/libumf.c | 24 ++++++++++++++++++++++++ src/utils/utils_common.h | 8 ++++++++ 3 files changed, 34 insertions(+) diff --git a/src/base_alloc/base_alloc_global.c b/src/base_alloc/base_alloc_global.c index 11d88b731a..2aca5d29cf 100644 --- a/src/base_alloc/base_alloc_global.c +++ b/src/base_alloc/base_alloc_global.c @@ -67,6 +67,8 @@ static void umf_ba_create_global(void) { size_t smallestSize = BASE_ALLOC.ac_sizes[0]; BASE_ALLOC.smallest_ac_size_log2 = log2Utils(smallestSize); + + LOG_DEBUG("UMF base allocator created"); } // returns index of the allocation class for a given size diff --git a/src/libumf.c b/src/libumf.c index 3b69ce396d..ab9f3c0da8 100644 --- a/src/libumf.c +++ b/src/libumf.c @@ -13,6 +13,7 @@ #include "ipc_cache.h" #include "memspace_internal.h" #include "provider_tracking.h" +#include "utils_common.h" #include "utils_log.h" #if !defined(UMF_NO_HWLOC) #include "topology.h" @@ -30,11 +31,20 @@ int umfInit(void) { LOG_ERR("Failed to create memory tracker"); return -1; } + + LOG_DEBUG("UMF tracker created"); + umf_result_t umf_result = umfIpcCacheGlobalInit(); if (umf_result != UMF_RESULT_SUCCESS) { LOG_ERR("Failed to initialize IPC cache"); return -1; } + + LOG_DEBUG("UMF IPC cache initialized"); + } + + if (TRACKER) { + LOG_DEBUG("UMF library initialized"); } return 0; @@ -50,12 +60,26 @@ void umfTearDown(void) { umfDestroyTopology(); #endif umfIpcCacheGlobalTearDown(); + + if (utils_is_running_in_proxy_lib_with_size_threshold()) { + // We cannot destroy the TRACKER nor the base allocator + // when we are running in the proxy library with a size threshold, + // because it could lead to calling the system free() with an invalid pointer + // and a segfault as a result. + goto fini_umfTearDown; + } + // make sure TRACKER is not used after being destroyed umf_memory_tracker_handle_t t = TRACKER; TRACKER = NULL; umfMemoryTrackerDestroy(t); + LOG_DEBUG("UMF tracker destroyed"); umf_ba_destroy_global(); + LOG_DEBUG("UMF base allocator destroyed"); + + fini_umfTearDown: + LOG_DEBUG("UMF library finalized"); } } diff --git a/src/utils/utils_common.h b/src/utils/utils_common.h index 7751a2ac96..90b95e3ba5 100644 --- a/src/utils/utils_common.h +++ b/src/utils/utils_common.h @@ -75,6 +75,14 @@ static inline int utils_is_running_in_proxy_lib(void) { return utils_env_var_get_str("LD_PRELOAD", "libumf_proxy.so") ? 1 : 0; } +// check if we are running in the proxy library with a size threshold +static inline int utils_is_running_in_proxy_lib_with_size_threshold(void) { + return (utils_env_var_get_str("LD_PRELOAD", "libumf_proxy.so") && + utils_env_var_get_str("UMF_PROXY", "size.threshold=")) + ? 1 + : 0; +} + // utils_parse_var - Parses var for a prefix, // optionally identifying a following argument. // Parameters: From 36eca5a0ec455b2e55dc605778d8af6b7456b958 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Wed, 13 Nov 2024 16:28:08 +0100 Subject: [PATCH 6/9] Add size threshold to proxy lib to call system allocator (Linux only) Add a size threshold to proxy lib to call system allocator when the size is less than the given threshold (Linux only yet). Signed-off-by: Lukasz Dorau --- README.md | 3 + src/proxy_lib/proxy_lib.c | 169 ++++++++++++++++++++++++++++++++++---- 2 files changed, 156 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index f0c98c112b..2b50ade237 100644 --- a/README.md +++ b/README.md @@ -317,6 +317,9 @@ The memory used by the proxy memory allocator is mmap'ed: - `page.disposition=shared-shm` - IPC uses the named shared memory. An SHM name is generated using the `umf_proxy_lib_shm_pid_$PID` pattern, where `$PID` is the PID of the process. It creates the `/dev/shm/umf_proxy_lib_shm_pid_$PID` file. - `page.disposition=shared-fd` - IPC uses the file descriptor duplication. It requires using `pidfd_getfd(2)` to obtain a duplicate of another process's file descriptor. Permission to duplicate another process's file descriptor is governed by a ptrace access mode `PTRACE_MODE_ATTACH_REALCREDS` check (see `ptrace(2)`) that can be changed using the `/proc/sys/kernel/yama/ptrace_scope` interface. `pidfd_getfd(2)` is supported since Linux 5.6. +The **size threshold** feature (Linux only) can be enabled by adding the `size.threshold=` string to the `UMF_PROXY` environment variable (with `';'` as a separator), for example: `UMF_PROXY="page.disposition=shared-shm;size.threshold=64"`. +It causes that all allocations of size less than the given threshold value go to the default system allocator instead of the proxy library. + #### Windows In case of Windows it requires: diff --git a/src/proxy_lib/proxy_lib.c b/src/proxy_lib/proxy_lib.c index 5a5912b134..12088a005d 100644 --- a/src/proxy_lib/proxy_lib.c +++ b/src/proxy_lib/proxy_lib.c @@ -27,6 +27,12 @@ * - _aligned_offset_recalloc() */ +#ifndef _WIN32 +#define _GNU_SOURCE // for RTLD_NEXT +#include +#undef _GNU_SOURCE +#endif /* _WIN32 */ + #if (defined PROXY_LIB_USES_JEMALLOC_POOL) #include #define umfPoolManagerOps umfJemallocPoolOps @@ -48,6 +54,7 @@ #include "base_alloc_linear.h" #include "proxy_lib.h" #include "utils_common.h" +#include "utils_load_library.h" #include "utils_log.h" #ifdef _WIN32 /* Windows ***************************************/ @@ -94,6 +101,24 @@ void utils_init_once(UTIL_ONCE_FLAG *flag, void (*onceCb)(void)); * of a UMF pool to allocate memory needed by an application. It should be freed * by an application. */ +#ifndef _WIN32 +typedef void *(*system_aligned_alloc_t)(size_t alignment, size_t size); +typedef void *(*system_calloc_t)(size_t nmemb, size_t size); +typedef void (*system_free_t)(void *ptr); +typedef void *(*system_malloc_t)(size_t size); +typedef size_t (*system_malloc_usable_size_t)(void *ptr); +typedef void *(*system_realloc_t)(void *ptr, size_t size); + +// pointers to the default system allocator's API +static system_aligned_alloc_t System_aligned_alloc; +static system_calloc_t System_calloc; +static system_free_t System_free; +static system_malloc_t System_malloc; +static system_malloc_usable_size_t System_malloc_usable_size; +static system_realloc_t System_realloc; + +static size_t Size_threshold_value = 0; +#endif /* _WIN32 */ static UTIL_ONCE_FLAG Base_alloc_leak_initialized = UTIL_ONCE_FLAG_INIT; static umf_ba_linear_pool_t *Base_alloc_leak = NULL; @@ -107,6 +132,58 @@ static __TLS int was_called_from_umfPool = 0; /*** The constructor and destructor of the proxy library *********************/ /*****************************************************************************/ +#ifndef _WIN32 +static size_t get_size_threshold(void) { + char *str_threshold = utils_env_var_get_str("UMF_PROXY", "size.threshold="); + if (!str_threshold) { + return 0; + } + + // move to the beginning of the number + str_threshold += strlen("size.threshold="); + // find ';' at the end + char *end = strstr(str_threshold, ";"); + if (end) { + // replace ';' with '\0' to mark end of the string + *end = '\0'; + } + + size_t int_threshold = (size_t)atoi(str_threshold); + LOG_DEBUG("Size_threshold_value = (char *) %s, (int) %zu", str_threshold, + int_threshold); + + return int_threshold; +} + +static int get_system_allocator_symbols(void) { + *((void **)(&System_aligned_alloc)) = + utils_get_symbol_addr(RTLD_NEXT, "aligned_alloc", NULL); + *((void **)(&System_calloc)) = + utils_get_symbol_addr(RTLD_NEXT, "calloc", NULL); + *((void **)(&System_free)) = utils_get_symbol_addr(RTLD_NEXT, "free", NULL); + *((void **)(&System_malloc)) = + utils_get_symbol_addr(RTLD_NEXT, "malloc", NULL); + *((void **)(&System_malloc_usable_size)) = + utils_get_symbol_addr(RTLD_NEXT, "malloc_usable_size", NULL); + *((void **)(&System_realloc)) = + utils_get_symbol_addr(RTLD_NEXT, "realloc", NULL); + + if (System_aligned_alloc && System_calloc && System_free && System_malloc && + System_malloc_usable_size && System_realloc) { + return 0; + } + + *((void **)(&System_aligned_alloc)) = NULL; + *((void **)(&System_calloc)) = NULL; + *((void **)(&System_free)) = NULL; + *((void **)(&System_malloc)) = NULL; + *((void **)(&System_malloc_usable_size)) = NULL; + *((void **)(&System_realloc)) = NULL; + + return -1; +} +#endif /* _WIN32 */ + void proxy_lib_create_common(void) { utils_log_init(); umf_os_memory_provider_params_t os_params = @@ -114,11 +191,21 @@ void proxy_lib_create_common(void) { umf_result_t umf_result; #ifndef _WIN32 - char shm_name[NAME_MAX]; + size_t _threshold = get_size_threshold(); + if (_threshold > 0) { + if (get_system_allocator_symbols()) { + LOG_ERR("initialization of the system allocator failed!"); + exit(-1); + } + + Size_threshold_value = _threshold; + LOG_INFO("system allocator initialized, size threshold value = %zu", + Size_threshold_value); + } if (utils_env_var_has_str("UMF_PROXY", "page.disposition=shared-fd")) { - LOG_DEBUG("proxy_lib: using the MAP_SHARED visibility mode with the " - "file descriptor duplication"); + LOG_INFO("proxy_lib: using the MAP_SHARED visibility mode with the " + "file descriptor duplication"); os_params.visibility = UMF_MEM_MAP_SHARED; os_params.shm_name = NULL; @@ -126,15 +213,16 @@ void proxy_lib_create_common(void) { "page.disposition=shared-shm")) { os_params.visibility = UMF_MEM_MAP_SHARED; + char shm_name[NAME_MAX]; memset(shm_name, 0, NAME_MAX); sprintf(shm_name, "umf_proxy_lib_shm_pid_%i", utils_getpid()); os_params.shm_name = shm_name; - LOG_DEBUG("proxy_lib: using the MAP_SHARED visibility mode with the " - "named shared memory: %s", - os_params.shm_name); + LOG_INFO("proxy_lib: using the MAP_SHARED visibility mode with the " + "named shared memory: %s", + os_params.shm_name); } -#endif +#endif /* _WIN32 */ umf_result = umfMemoryProviderCreate(umfOsMemoryProviderOps(), &os_params, &OS_memory_provider); @@ -149,8 +237,10 @@ void proxy_lib_create_common(void) { LOG_ERR("creating UMF pool manager failed"); exit(-1); } + // The UMF pool has just been created (Proxy_pool != NULL). Stop using // the linear allocator and start using the UMF pool allocator from now on. + LOG_DEBUG("proxy library initialized"); } void proxy_lib_destroy_common(void) { @@ -158,7 +248,7 @@ void proxy_lib_destroy_common(void) { // We cannot destroy 'Base_alloc_leak' nor 'Proxy_pool' nor 'OS_memory_provider', // because it could lead to use-after-free in the program's unloader // (for example _dl_fini() on Linux). - return; + goto fini_proxy_lib_destroy_common; } umf_memory_pool_handle_t pool = Proxy_pool; @@ -168,6 +258,10 @@ void proxy_lib_destroy_common(void) { umf_memory_provider_handle_t provider = OS_memory_provider; OS_memory_provider = NULL; umfMemoryProviderDestroy(provider); + LOG_DEBUG("proxy library destroyed"); + +fini_proxy_lib_destroy_common: + LOG_DEBUG("proxy library finalized"); } /*****************************************************************************/ @@ -246,6 +340,12 @@ static inline size_t ba_leak_pool_contains_pointer(void *ptr) { /*****************************************************************************/ void *malloc(size_t size) { +#ifndef _WIN32 + if (size < Size_threshold_value) { + return System_malloc(size); + } +#endif /* _WIN32 */ + if (!was_called_from_umfPool && Proxy_pool) { was_called_from_umfPool = 1; void *ptr = umfPoolMalloc(Proxy_pool, size); @@ -257,6 +357,12 @@ void *malloc(size_t size) { } void *calloc(size_t nmemb, size_t size) { +#ifndef _WIN32 + if ((nmemb * size) < Size_threshold_value) { + return System_calloc(nmemb, size); + } +#endif /* _WIN32 */ + if (!was_called_from_umfPool && Proxy_pool) { was_called_from_umfPool = 1; void *ptr = umfPoolCalloc(Proxy_pool, nmemb, size); @@ -276,15 +382,22 @@ void free(void *ptr) { return; } - if (Proxy_pool) { + if (Proxy_pool && (umfPoolByPtr(ptr) == Proxy_pool)) { if (umfPoolFree(Proxy_pool, ptr) != UMF_RESULT_SUCCESS) { LOG_ERR("umfPoolFree() failed"); - assert(0); } return; } - assert(0); +#ifndef _WIN32 + if (Size_threshold_value) { + System_free(ptr); + return; + } +#endif /* _WIN32 */ + + LOG_ERR("free() failed: %p", ptr); + return; } @@ -303,18 +416,31 @@ void *realloc(void *ptr, size_t size) { return ba_leak_realloc(ptr, size, leak_pool_contains_pointer); } - if (Proxy_pool) { + if (Proxy_pool && (umfPoolByPtr(ptr) == Proxy_pool)) { was_called_from_umfPool = 1; void *new_ptr = umfPoolRealloc(Proxy_pool, ptr, size); was_called_from_umfPool = 0; return new_ptr; } - assert(0); +#ifndef _WIN32 + if (Size_threshold_value) { + return System_realloc(ptr, size); + } +#endif /* _WIN32 */ + + LOG_ERR("realloc() failed: %p", ptr); + return NULL; } void *aligned_alloc(size_t alignment, size_t size) { +#ifndef _WIN32 + if (size < Size_threshold_value) { + return System_aligned_alloc(alignment, size); + } +#endif /* _WIN32 */ + if (!was_called_from_umfPool && Proxy_pool) { was_called_from_umfPool = 1; void *ptr = umfPoolAlignedMalloc(Proxy_pool, size, alignment); @@ -330,19 +456,30 @@ size_t _msize(void *ptr) { #else size_t malloc_usable_size(void *ptr) { #endif - - // a check to verify we are running the proxy library + // a check to verify if we are running the proxy library if (ptr == (void *)0x01) { return 0xDEADBEEF; } - if (!was_called_from_umfPool && Proxy_pool) { + if (ba_leak_pool_contains_pointer(ptr)) { + return 0; // unsupported in case of the ba_leak allocator + } + + if (Proxy_pool && (umfPoolByPtr(ptr) == Proxy_pool)) { was_called_from_umfPool = 1; size_t size = umfPoolMallocUsableSize(Proxy_pool, ptr); was_called_from_umfPool = 0; return size; } +#ifndef _WIN32 + if (Size_threshold_value) { + return System_malloc_usable_size(ptr); + } +#endif /* _WIN32 */ + + LOG_ERR("malloc_usable_size() failed: %p", ptr); + return 0; // unsupported in this case } From 4ac6b3ede9e5175bcc8e874c9873ec3c124b28a1 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Thu, 14 Nov 2024 09:59:37 +0100 Subject: [PATCH 7/9] Add tests for the size threshold of the proxy library (Linux only) The proxyLib_size_threshold_* tests test the size threshold of the proxy library (Linux only yet). The size threshold is set to 64 bytes in this test, so all allocations of: 1) size < 64 go through the default system allocator and (umfPoolByPtr(ptr_size < 64) == nullptr) 2) size >= 64 go through the proxy lib allocator and (umfPoolByPtr(ptr_size >= 64) != nullptr). Ref: #894 Signed-off-by: Lukasz Dorau --- .github/workflows/reusable_proxy_lib.yml | 19 ++- test/CMakeLists.txt | 3 + test/test_proxy_lib.cpp | 173 +++++++++++++++++++---- 3 files changed, 166 insertions(+), 29 deletions(-) diff --git a/.github/workflows/reusable_proxy_lib.yml b/.github/workflows/reusable_proxy_lib.yml index 56211b97d2..3c1903ff91 100644 --- a/.github/workflows/reusable_proxy_lib.yml +++ b/.github/workflows/reusable_proxy_lib.yml @@ -61,9 +61,14 @@ jobs: # TODO enable the provider_file_memory_ipc test when the IPC tests with the proxy library are fixed # see the issue: https://github.com/oneapi-src/unified-memory-framework/issues/864 + # TODO enable the proxy_lib_basic test for the JEMALLOC proxy_lib_pool + # see the issue: https://github.com/oneapi-src/unified-memory-framework/issues/894 - name: Run "ctest --output-on-failure" with proxy library working-directory: ${{env.BUILD_DIR}} - run: LD_PRELOAD=./lib/libumf_proxy.so ctest --output-on-failure -E provider_file_memory_ipc + run: > + LD_PRELOAD=./lib/libumf_proxy.so + ctest --output-on-failure + ${{ matrix.proxy_lib_pool == 'JEMALLOC' && '-E "provider_file_memory_ipc|proxy_lib_basic"' || '-E provider_file_memory_ipc' }} - name: Run "./test/umf_test-memoryPool" with proxy library working-directory: ${{env.BUILD_DIR}} @@ -77,6 +82,18 @@ jobs: working-directory: ${{env.BUILD_DIR}} run: UMF_PROXY="page.disposition=shared-shm" LD_PRELOAD=./lib/libumf_proxy.so /usr/bin/date + # TODO enable the provider_file_memory_ipc test when the IPC tests with the proxy library are fixed + # see the issue: https://github.com/oneapi-src/unified-memory-framework/issues/864 + # TODO enable the proxy_lib_basic test for the JEMALLOC proxy_lib_pool + # see the issue: https://github.com/oneapi-src/unified-memory-framework/issues/894 + - name: Run "ctest --output-on-failure" with proxy library and size.threshold=128 + working-directory: ${{env.BUILD_DIR}} + run: > + UMF_PROXY="size.threshold=128" + LD_PRELOAD=./lib/libumf_proxy.so + ctest --output-on-failure + ${{ matrix.proxy_lib_pool == 'JEMALLOC' && '-E "provider_file_memory_ipc|proxy_lib_basic"' || '-E provider_file_memory_ipc' }} + - name: Check coverage if: ${{ matrix.build_type == 'Debug' }} working-directory: ${{env.BUILD_DIR}} diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index d24244ab05..54271d7307 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -414,6 +414,9 @@ if(UMF_PROXY_LIB_ENABLED AND UMF_BUILD_SHARED_LIBRARY) SRCS ${BA_SOURCES_FOR_TEST} test_proxy_lib.cpp LIBS ${UMF_UTILS_FOR_TEST} umf_proxy) + set_property(TEST umf-proxy_lib_basic + PROPERTY ENVIRONMENT "UMF_PROXY=size.threshold=64") + # the memoryPool test run with the proxy library add_umf_test( NAME proxy_lib_memoryPool diff --git a/test/test_proxy_lib.cpp b/test/test_proxy_lib.cpp index 85afc65be8..12f17ce5fc 100644 --- a/test/test_proxy_lib.cpp +++ b/test/test_proxy_lib.cpp @@ -19,13 +19,14 @@ using umf_test::test; -#define SIZE_64 64 +// size threshold defined by the env variable UMF_PROXY="size.threshold=64" +#define SIZE_THRESHOLD 64 +#define SIZE_EQ (SIZE_THRESHOLD) +#define SIZE_LT (SIZE_THRESHOLD - 1) + #define ALIGN_1024 1024 TEST_F(test, proxyLib_basic) { - - ::free(::malloc(SIZE_64)); - // a check to verify we are running the proxy library void *ptr = (void *)0x01; @@ -41,46 +42,162 @@ TEST_F(test, proxyLib_basic) { } TEST_F(test, proxyLib_realloc_size0) { - // realloc(ptr, 0) == free (ptr) + // realloc(ptr, 0) == free(ptr) // realloc(ptr, 0) returns NULL - ASSERT_EQ(::realloc(::malloc(SIZE_64), 0), nullptr); + ASSERT_EQ(::realloc(::malloc(SIZE_EQ), 0), nullptr); } -TEST_F(test, proxyLib_malloc_usable_size) { - - void *ptr = ::malloc(SIZE_64); - ASSERT_NE(ptr, nullptr); - if (ptr == nullptr) { - // Fix for the following CodeQL's warning on Windows: - // 'ptr' could be '0': this does not adhere to the specification for the function '_msize'. - return; - } +// The proxyLib_size_threshold_* tests test the size threshold of the proxy library. +// The size threshold is set to SIZE_THRESHOLD bytes in this test, so all allocations of: +// 1) size < SIZE_THRESHOLD go through the default system allocator +// (umfPoolByPtr(ptr_size < SIZE_THRESHOLD) == nullptr) +// 2) size >= SIZE_THRESHOLD go through the proxy library allocator +// (umfPoolByPtr(ptr_size >= SIZE_THRESHOLD) != nullptr) +TEST_F(test, proxyLib_size_threshold_aligned_alloc) { #ifdef _WIN32 - size_t size = _msize(ptr); -#elif __APPLE__ - size_t size = ::malloc_size(ptr); + void *ptr_LT = _aligned_malloc(SIZE_LT, ALIGN_1024); + void *ptr_EQ = _aligned_malloc(SIZE_EQ, ALIGN_1024); #else - size_t size = ::malloc_usable_size(ptr); + void *ptr_LT = ::aligned_alloc(ALIGN_1024, SIZE_LT); + void *ptr_EQ = ::aligned_alloc(ALIGN_1024, SIZE_EQ); #endif - ASSERT_EQ((int)(size == 0 || size >= SIZE_64), 1); + ASSERT_NE(ptr_LT, nullptr); + ASSERT_NE(ptr_EQ, nullptr); - ::free(ptr); -} + // verify alignment + ASSERT_EQ((int)(IS_ALIGNED((uintptr_t)ptr_LT, ALIGN_1024)), 1); + ASSERT_EQ((int)(IS_ALIGNED((uintptr_t)ptr_EQ, ALIGN_1024)), 1); + +#ifndef _WIN32 /* the size threshold works only on Linux for now */ + // umfPoolByPtr(ptr_size_LT) == nullptr + ASSERT_EQ(umfPoolByPtr(ptr_LT), nullptr); +#endif + // umfPoolByPtr(ptr_size_EQ) != nullptr + ASSERT_NE(umfPoolByPtr(ptr_EQ), nullptr); -TEST_F(test, proxyLib_aligned_alloc) { #ifdef _WIN32 - void *ptr = _aligned_malloc(SIZE_64, ALIGN_1024); + _aligned_free(ptr_LT); + _aligned_free(ptr_EQ); #else - void *ptr = ::aligned_alloc(ALIGN_1024, SIZE_64); + ::free(ptr_LT); + ::free(ptr_EQ); +#endif +} + +TEST_F(test, proxyLib_size_threshold_malloc) { + void *ptr_LT = malloc(SIZE_LT); + void *ptr_EQ = malloc(SIZE_EQ); + + ASSERT_NE(ptr_LT, nullptr); + ASSERT_NE(ptr_EQ, nullptr); + +#ifndef _WIN32 /* the size threshold works only on Linux for now */ + // umfPoolByPtr(ptr_size_LT) == nullptr + ASSERT_EQ(umfPoolByPtr(ptr_LT), nullptr); +#endif + // umfPoolByPtr(ptr_size_EQ) != nullptr + ASSERT_NE(umfPoolByPtr(ptr_EQ), nullptr); + + ::free(ptr_LT); + ::free(ptr_EQ); +} + +TEST_F(test, proxyLib_size_threshold_calloc) { + void *ptr_LT = calloc(SIZE_LT, 1); + void *ptr_EQ = calloc(SIZE_EQ, 1); + + ASSERT_NE(ptr_LT, nullptr); + ASSERT_NE(ptr_EQ, nullptr); + +#ifndef _WIN32 /* the size threshold works only on Linux for now */ + // umfPoolByPtr(ptr_size_LT) == nullptr + ASSERT_EQ(umfPoolByPtr(ptr_LT), nullptr); +#endif + // umfPoolByPtr(ptr_size_EQ) != nullptr + ASSERT_NE(umfPoolByPtr(ptr_EQ), nullptr); + + ::free(ptr_LT); + ::free(ptr_EQ); +} + +TEST_F(test, proxyLib_size_threshold_realloc_up) { + void *ptr_LT = malloc(SIZE_LT); + void *ptr_EQ = malloc(SIZE_EQ); + + ASSERT_NE(ptr_LT, nullptr); + ASSERT_NE(ptr_EQ, nullptr); + + void *ptr_LT_r = realloc(ptr_LT, 2 * SIZE_LT); + void *ptr_EQ_r = realloc(ptr_EQ, 2 * SIZE_EQ); + + ASSERT_NE(ptr_LT_r, nullptr); + ASSERT_NE(ptr_EQ_r, nullptr); + +#ifndef _WIN32 /* the size threshold works only on Linux for now */ + // umfPoolByPtr(ptr_size_LT) == nullptr + ASSERT_EQ(umfPoolByPtr(ptr_LT_r), nullptr); #endif + // umfPoolByPtr(ptr_size_EQ) != nullptr + ASSERT_NE(umfPoolByPtr(ptr_EQ_r), nullptr); - ASSERT_EQ((int)(IS_ALIGNED((uintptr_t)ptr, ALIGN_1024)), 1); + ::free(ptr_LT_r); + ::free(ptr_EQ_r); +} + +TEST_F(test, proxyLib_size_threshold_realloc_down) { + void *ptr_LT = malloc(SIZE_LT); + void *ptr_EQ = malloc(SIZE_EQ); + + ASSERT_NE(ptr_LT, nullptr); + ASSERT_NE(ptr_EQ, nullptr); + + void *ptr_LT_r = realloc(ptr_LT, SIZE_LT / 2); + void *ptr_EQ_r = realloc(ptr_EQ, SIZE_EQ / 2); + + ASSERT_NE(ptr_LT_r, nullptr); + ASSERT_NE(ptr_EQ_r, nullptr); + +#ifndef _WIN32 /* the size threshold works only on Linux for now */ + // umfPoolByPtr(ptr_size_LT) == nullptr + ASSERT_EQ(umfPoolByPtr(ptr_LT_r), nullptr); +#endif + // umfPoolByPtr(ptr_size_EQ) != nullptr + ASSERT_NE(umfPoolByPtr(ptr_EQ_r), nullptr); + + ::free(ptr_LT_r); + ::free(ptr_EQ_r); +} + +TEST_F(test, proxyLib_size_threshold_malloc_usable_size) { + + void *ptr_LT = ::malloc(SIZE_LT); + void *ptr_EQ = ::malloc(SIZE_EQ); + + ASSERT_NE(ptr_LT, nullptr); + ASSERT_NE(ptr_EQ, nullptr); + + if (ptr_LT == nullptr || ptr_EQ == nullptr) { + // Fix for the following CodeQL's warning on Windows: + // 'ptr' could be '0': this does not adhere to the specification for the function '_msize'. + return; + } #ifdef _WIN32 - _aligned_free(ptr); + size_t size_LT = _msize(ptr_LT); + size_t size_EQ = _msize(ptr_EQ); +#elif __APPLE__ + size_t size_LT = ::malloc_size(ptr_LT); + size_t size_EQ = ::malloc_size(ptr_EQ); #else - ::free(ptr); + size_t size_LT = ::malloc_usable_size(ptr_LT); + size_t size_EQ = ::malloc_usable_size(ptr_EQ); #endif + + ASSERT_EQ((int)(size_LT == 0 || size_LT >= SIZE_LT), 1); + ASSERT_EQ((int)(size_EQ == 0 || size_EQ >= SIZE_EQ), 1); + + ::free(ptr_LT); + ::free(ptr_EQ); } From 9d2aec57af07982a557cb765835f1a27e4f34940 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Thu, 14 Nov 2024 10:02:14 +0100 Subject: [PATCH 8/9] Enable size threshold in the proxy library on_Windows Signed-off-by: Lukasz Dorau --- src/proxy_lib/proxy_lib.c | 69 +++++++++++++++++++++++++-------------- test/test_proxy_lib.cpp | 13 -------- 2 files changed, 44 insertions(+), 38 deletions(-) diff --git a/src/proxy_lib/proxy_lib.c b/src/proxy_lib/proxy_lib.c index 12088a005d..2137d57d3d 100644 --- a/src/proxy_lib/proxy_lib.c +++ b/src/proxy_lib/proxy_lib.c @@ -101,7 +101,7 @@ void utils_init_once(UTIL_ONCE_FLAG *flag, void (*onceCb)(void)); * of a UMF pool to allocate memory needed by an application. It should be freed * by an application. */ -#ifndef _WIN32 + typedef void *(*system_aligned_alloc_t)(size_t alignment, size_t size); typedef void *(*system_calloc_t)(size_t nmemb, size_t size); typedef void (*system_free_t)(void *ptr); @@ -110,6 +110,7 @@ typedef size_t (*system_malloc_usable_size_t)(void *ptr); typedef void *(*system_realloc_t)(void *ptr, size_t size); // pointers to the default system allocator's API +static void *System_library_handle; static system_aligned_alloc_t System_aligned_alloc; static system_calloc_t System_calloc; static system_free_t System_free; @@ -118,7 +119,6 @@ static system_malloc_usable_size_t System_malloc_usable_size; static system_realloc_t System_realloc; static size_t Size_threshold_value = 0; -#endif /* _WIN32 */ static UTIL_ONCE_FLAG Base_alloc_leak_initialized = UTIL_ONCE_FLAG_INIT; static umf_ba_linear_pool_t *Base_alloc_leak = NULL; @@ -132,7 +132,24 @@ static __TLS int was_called_from_umfPool = 0; /*** The constructor and destructor of the proxy library *********************/ /*****************************************************************************/ -#ifndef _WIN32 +// atoi() is defined in stdlib.h, but we cannot include it on Windows +static size_t custom_atoi(const char *str) { + size_t result = 0; + + for (int i = 0; str[i]; i++) { + if (str[i] < '0' || str[i] > '9') { + LOG_ERR("proxy_lib_create_common: size threshold is not a valid " + "number: %s", + str); + return 0; + } + + result = 10 * result + (size_t)(str[i] - '0'); + } + + return result; +} + static size_t get_size_threshold(void) { char *str_threshold = utils_env_var_get_str("UMF_PROXY", "size.threshold="); if (!str_threshold) { @@ -148,7 +165,7 @@ static size_t get_size_threshold(void) { *end = '\0'; } - size_t int_threshold = (size_t)atoi(str_threshold); + size_t int_threshold = (size_t)custom_atoi(str_threshold); LOG_DEBUG("Size_threshold_value = (char *) %s, (int) %zu", str_threshold, int_threshold); @@ -156,17 +173,24 @@ static size_t get_size_threshold(void) { } static int get_system_allocator_symbols(void) { +#ifdef _WIN32 + System_library_handle = utils_open_library("msvcrt.dll", 0); +#else + System_library_handle = RTLD_NEXT; +#endif /* _WIN32 */ + *((void **)(&System_aligned_alloc)) = - utils_get_symbol_addr(RTLD_NEXT, "aligned_alloc", NULL); + utils_get_symbol_addr(System_library_handle, "aligned_alloc", NULL); *((void **)(&System_calloc)) = - utils_get_symbol_addr(RTLD_NEXT, "calloc", NULL); - *((void **)(&System_free)) = utils_get_symbol_addr(RTLD_NEXT, "free", NULL); + utils_get_symbol_addr(System_library_handle, "calloc", NULL); + *((void **)(&System_free)) = + utils_get_symbol_addr(System_library_handle, "free", NULL); *((void **)(&System_malloc)) = - utils_get_symbol_addr(RTLD_NEXT, "malloc", NULL); - *((void **)(&System_malloc_usable_size)) = - utils_get_symbol_addr(RTLD_NEXT, "malloc_usable_size", NULL); + utils_get_symbol_addr(System_library_handle, "malloc", NULL); + *((void **)(&System_malloc_usable_size)) = utils_get_symbol_addr( + System_library_handle, "malloc_usable_size", NULL); *((void **)(&System_realloc)) = - utils_get_symbol_addr(RTLD_NEXT, "realloc", NULL); + utils_get_symbol_addr(System_library_handle, "realloc", NULL); if (System_aligned_alloc && System_calloc && System_free && System_malloc && System_malloc_usable_size && System_realloc) { @@ -182,7 +206,6 @@ static int get_system_allocator_symbols(void) { return -1; } -#endif /* _WIN32 */ void proxy_lib_create_common(void) { utils_log_init(); @@ -190,7 +213,6 @@ void proxy_lib_create_common(void) { umfOsMemoryProviderParamsDefault(); umf_result_t umf_result; -#ifndef _WIN32 size_t _threshold = get_size_threshold(); if (_threshold > 0) { if (get_system_allocator_symbols()) { @@ -203,6 +225,7 @@ void proxy_lib_create_common(void) { Size_threshold_value); } +#ifndef _WIN32 if (utils_env_var_has_str("UMF_PROXY", "page.disposition=shared-fd")) { LOG_INFO("proxy_lib: using the MAP_SHARED visibility mode with the " "file descriptor duplication"); @@ -258,6 +281,14 @@ void proxy_lib_destroy_common(void) { umf_memory_provider_handle_t provider = OS_memory_provider; OS_memory_provider = NULL; umfMemoryProviderDestroy(provider); + +#ifdef _WIN32 + if (System_library_handle) { + utils_close_library(System_library_handle); + System_library_handle = NULL; + } +#endif /* _WIN32 */ + LOG_DEBUG("proxy library destroyed"); fini_proxy_lib_destroy_common: @@ -340,11 +371,9 @@ static inline size_t ba_leak_pool_contains_pointer(void *ptr) { /*****************************************************************************/ void *malloc(size_t size) { -#ifndef _WIN32 if (size < Size_threshold_value) { return System_malloc(size); } -#endif /* _WIN32 */ if (!was_called_from_umfPool && Proxy_pool) { was_called_from_umfPool = 1; @@ -357,11 +386,9 @@ void *malloc(size_t size) { } void *calloc(size_t nmemb, size_t size) { -#ifndef _WIN32 if ((nmemb * size) < Size_threshold_value) { return System_calloc(nmemb, size); } -#endif /* _WIN32 */ if (!was_called_from_umfPool && Proxy_pool) { was_called_from_umfPool = 1; @@ -389,12 +416,10 @@ void free(void *ptr) { return; } -#ifndef _WIN32 if (Size_threshold_value) { System_free(ptr); return; } -#endif /* _WIN32 */ LOG_ERR("free() failed: %p", ptr); @@ -423,11 +448,9 @@ void *realloc(void *ptr, size_t size) { return new_ptr; } -#ifndef _WIN32 if (Size_threshold_value) { return System_realloc(ptr, size); } -#endif /* _WIN32 */ LOG_ERR("realloc() failed: %p", ptr); @@ -435,11 +458,9 @@ void *realloc(void *ptr, size_t size) { } void *aligned_alloc(size_t alignment, size_t size) { -#ifndef _WIN32 if (size < Size_threshold_value) { return System_aligned_alloc(alignment, size); } -#endif /* _WIN32 */ if (!was_called_from_umfPool && Proxy_pool) { was_called_from_umfPool = 1; @@ -472,11 +493,9 @@ size_t malloc_usable_size(void *ptr) { return size; } -#ifndef _WIN32 if (Size_threshold_value) { return System_malloc_usable_size(ptr); } -#endif /* _WIN32 */ LOG_ERR("malloc_usable_size() failed: %p", ptr); diff --git a/test/test_proxy_lib.cpp b/test/test_proxy_lib.cpp index 12f17ce5fc..1d37bb88c3 100644 --- a/test/test_proxy_lib.cpp +++ b/test/test_proxy_lib.cpp @@ -70,10 +70,8 @@ TEST_F(test, proxyLib_size_threshold_aligned_alloc) { ASSERT_EQ((int)(IS_ALIGNED((uintptr_t)ptr_LT, ALIGN_1024)), 1); ASSERT_EQ((int)(IS_ALIGNED((uintptr_t)ptr_EQ, ALIGN_1024)), 1); -#ifndef _WIN32 /* the size threshold works only on Linux for now */ // umfPoolByPtr(ptr_size_LT) == nullptr ASSERT_EQ(umfPoolByPtr(ptr_LT), nullptr); -#endif // umfPoolByPtr(ptr_size_EQ) != nullptr ASSERT_NE(umfPoolByPtr(ptr_EQ), nullptr); @@ -93,10 +91,8 @@ TEST_F(test, proxyLib_size_threshold_malloc) { ASSERT_NE(ptr_LT, nullptr); ASSERT_NE(ptr_EQ, nullptr); -#ifndef _WIN32 /* the size threshold works only on Linux for now */ // umfPoolByPtr(ptr_size_LT) == nullptr ASSERT_EQ(umfPoolByPtr(ptr_LT), nullptr); -#endif // umfPoolByPtr(ptr_size_EQ) != nullptr ASSERT_NE(umfPoolByPtr(ptr_EQ), nullptr); @@ -111,10 +107,8 @@ TEST_F(test, proxyLib_size_threshold_calloc) { ASSERT_NE(ptr_LT, nullptr); ASSERT_NE(ptr_EQ, nullptr); -#ifndef _WIN32 /* the size threshold works only on Linux for now */ // umfPoolByPtr(ptr_size_LT) == nullptr ASSERT_EQ(umfPoolByPtr(ptr_LT), nullptr); -#endif // umfPoolByPtr(ptr_size_EQ) != nullptr ASSERT_NE(umfPoolByPtr(ptr_EQ), nullptr); @@ -135,10 +129,8 @@ TEST_F(test, proxyLib_size_threshold_realloc_up) { ASSERT_NE(ptr_LT_r, nullptr); ASSERT_NE(ptr_EQ_r, nullptr); -#ifndef _WIN32 /* the size threshold works only on Linux for now */ // umfPoolByPtr(ptr_size_LT) == nullptr ASSERT_EQ(umfPoolByPtr(ptr_LT_r), nullptr); -#endif // umfPoolByPtr(ptr_size_EQ) != nullptr ASSERT_NE(umfPoolByPtr(ptr_EQ_r), nullptr); @@ -159,15 +151,10 @@ TEST_F(test, proxyLib_size_threshold_realloc_down) { ASSERT_NE(ptr_LT_r, nullptr); ASSERT_NE(ptr_EQ_r, nullptr); -#ifndef _WIN32 /* the size threshold works only on Linux for now */ // umfPoolByPtr(ptr_size_LT) == nullptr ASSERT_EQ(umfPoolByPtr(ptr_LT_r), nullptr); -#endif // umfPoolByPtr(ptr_size_EQ) != nullptr ASSERT_NE(umfPoolByPtr(ptr_EQ_r), nullptr); - - ::free(ptr_LT_r); - ::free(ptr_EQ_r); } TEST_F(test, proxyLib_size_threshold_malloc_usable_size) { From 250ebb1bacbd75a32d14236341accccba660e7d4 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Thu, 14 Nov 2024 11:24:25 +0100 Subject: [PATCH 9/9] DEBUG run ProxyLib --- .github/workflows/pr_push.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr_push.yml b/.github/workflows/pr_push.yml index 9623b69f1b..5755f38dba 100644 --- a/.github/workflows/pr_push.yml +++ b/.github/workflows/pr_push.yml @@ -58,7 +58,7 @@ jobs: needs: [Build] uses: ./.github/workflows/reusable_benchmarks.yml ProxyLib: - needs: [Build] + needs: [CodeChecks, DocsBuild] uses: ./.github/workflows/reusable_proxy_lib.yml Valgrind: needs: [Build]