From 68e19f5cba128f3c8c554cdcbd8af06da742b268 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Staniewski?= Date: Wed, 20 Nov 2024 08:39:25 +0000 Subject: [PATCH] Enable `-Werror` in developer build --- cmake/helpers.cmake | 8 ++++++-- src/ipc_cache.c | 5 +++++ src/memtargets/memtarget_numa.c | 6 +++--- src/provider/provider_cuda.c | 2 +- src/provider/provider_level_zero.c | 4 ++-- src/provider/provider_os_memory.c | 3 ++- test/CMakeLists.txt | 5 ++++- test/memspaces/memspace_fixtures.hpp | 2 ++ test/poolFixtures.hpp | 5 +++++ test/providers/provider_level_zero.cpp | 4 ++-- 10 files changed, 32 insertions(+), 12 deletions(-) diff --git a/cmake/helpers.cmake b/cmake/helpers.cmake index 1372531a02..2a16de742a 100644 --- a/cmake/helpers.cmake +++ b/cmake/helpers.cmake @@ -244,8 +244,9 @@ function(add_umf_target_compile_options name) target_compile_definitions(${name} PRIVATE -D_FORTIFY_SOURCE=2) endif() if(UMF_DEVELOPER_MODE) - target_compile_options(${name} PRIVATE -fno-omit-frame-pointer - -fstack-protector-strong) + target_compile_options( + ${name} PRIVATE -fno-omit-frame-pointer + -fstack-protector-strong -Werror) endif() if(UMF_USE_COVERAGE) if(NOT CMAKE_BUILD_TYPE STREQUAL "Debug") @@ -279,6 +280,9 @@ function(add_umf_target_compile_options name) # disable 4200 warning: nonstandard extension used: # zero-sized array in struct/union /wd4200) + if(UMF_DEVELOPER_MODE) + target_compile_options(${name} PRIVATE /WX) + endif() if(${CMAKE_C_COMPILER_ID} MATCHES "MSVC") target_compile_options( ${name} diff --git a/src/ipc_cache.c b/src/ipc_cache.c index 40ecc3978b..60072d4dfa 100644 --- a/src/ipc_cache.c +++ b/src/ipc_cache.c @@ -17,6 +17,11 @@ #include "utils_log.h" #include "utlist.h" +// HASH_ADD macro produces `warning C4702: unreachable code` on MSVC +#ifdef _MSC_VER +#pragma warning(disable : 4702) +#endif + struct ipc_handle_cache_entry_t; typedef struct ipc_handle_cache_entry_t *hash_map_t; diff --git a/src/memtargets/memtarget_numa.c b/src/memtargets/memtarget_numa.c index cb0e4ce003..34ba7fc100 100644 --- a/src/memtargets/memtarget_numa.c +++ b/src/memtargets/memtarget_numa.c @@ -40,7 +40,7 @@ static umf_result_t numa_initialize(void *params, void **memTarget) { return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY; } - numaTarget->physical_id = config->physical_id; + numaTarget->physical_id = (unsigned)config->physical_id; *memTarget = numaTarget; return UMF_RESULT_SUCCESS; } @@ -100,7 +100,7 @@ static umf_result_t numa_memory_provider_create_from_memspace( params.partitions = (umf_numa_split_partition_t *)policy->ops.split.part; - params.partitions_len = policy->ops.split.part_len; + params.partitions_len = (unsigned)policy->ops.split.part_len; break; default: return UMF_RESULT_ERROR_INVALID_ARGUMENT; @@ -131,7 +131,7 @@ static umf_result_t numa_memory_provider_create_from_memspace( for (size_t i = 0; i < numNodesProvider; i++) { params.numa_list[i] = numaTargets[i]->physical_id; } - params.numa_list_len = numNodesProvider; + params.numa_list_len = (unsigned)numNodesProvider; } umf_memory_provider_handle_t numaProvider = NULL; diff --git a/src/provider/provider_cuda.c b/src/provider/provider_cuda.c index 68fe0da230..5a686d8579 100644 --- a/src/provider/provider_cuda.c +++ b/src/provider/provider_cuda.c @@ -217,7 +217,7 @@ static void cu_memory_provider_finalize(void *provider) { umf_ba_global_free(provider); } -/* +/* * This function is used by the CUDA provider to make sure that * the required context is set. If the current context is * not the required one, it will be saved in restore_ctx. diff --git a/src/provider/provider_level_zero.c b/src/provider/provider_level_zero.c index 3339042ce3..6b4468da67 100644 --- a/src/provider/provider_level_zero.c +++ b/src/provider/provider_level_zero.c @@ -148,12 +148,12 @@ static umf_result_t ze_memory_provider_initialize(void *params, } if ((ze_params->memory_type == UMF_MEMORY_TYPE_HOST) == - (bool)ze_params->level_zero_device_handle) { + (ze_params->level_zero_device_handle != NULL)) { return UMF_RESULT_ERROR_INVALID_ARGUMENT; } if ((bool)ze_params->resident_device_count != - (bool)ze_params->resident_device_handles) { + (ze_params->resident_device_handles != NULL)) { return UMF_RESULT_ERROR_INVALID_ARGUMENT; } diff --git a/src/provider/provider_os_memory.c b/src/provider/provider_os_memory.c index 3ea5cf2200..dae947651c 100644 --- a/src/provider/provider_os_memory.c +++ b/src/provider/provider_os_memory.c @@ -1207,8 +1207,9 @@ static umf_result_t os_get_ipc_handle(void *provider, const void *ptr, os_ipc_data->visibility = os_provider->visibility; os_ipc_data->shm_name_len = strlen(os_provider->shm_name); if (os_ipc_data->shm_name_len > 0) { + // NOTE: +1 for '\0' at the end of the string strncpy(os_ipc_data->shm_name, os_provider->shm_name, - os_ipc_data->shm_name_len); + os_ipc_data->shm_name_len + 1); } else { os_ipc_data->fd = os_provider->fd; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 9899991ce6..1f2ef5959d 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -75,8 +75,11 @@ function(build_umf_test) # tests retrieve arguments using 'GetParam()', which applies a 'const' # qualifier often discarded in the test scenarios. target_compile_options(${TEST_TARGET_NAME} PRIVATE -Wno-cast-qual) - endif() + if(UMF_DEVELOPER_MODE) + target_compile_options(${TEST_TARGET_NAME} PRIVATE -Werror) + endif() + endif() target_link_directories(${TEST_TARGET_NAME} PRIVATE ${LIB_DIRS}) target_include_directories( diff --git a/test/memspaces/memspace_fixtures.hpp b/test/memspaces/memspace_fixtures.hpp index 9bcfc5fbe6..da174c4f1a 100644 --- a/test/memspaces/memspace_fixtures.hpp +++ b/test/memspaces/memspace_fixtures.hpp @@ -87,6 +87,8 @@ struct memspaceProviderTest : ::memspaceGetTest { } auto [isQuerySupported, memspaceGet] = ::memspaceGetTest::GetParam(); + (void)memspaceGet; + isQuerySupported(nodeIds.front()); // The test has been marked as skipped in isQuerySupported, diff --git a/test/poolFixtures.hpp b/test/poolFixtures.hpp index e1c1cc7225..d761a3cfd4 100644 --- a/test/poolFixtures.hpp +++ b/test/poolFixtures.hpp @@ -70,6 +70,11 @@ struct umfPoolTest : umf_test::test, auto [pool_ops, pool_params, provider_ops, provider_params, coarse_params] = this->GetParam(); + (void)pool_ops; + (void)pool_params; + (void)provider_params; + (void)coarse_params; + if (provider_ops == umfDevDaxMemoryProviderOps()) { char *path = getenv("UMF_TESTS_DEVDAX_PATH"); if (path == nullptr || path[0] == 0) { diff --git a/test/providers/provider_level_zero.cpp b/test/providers/provider_level_zero.cpp index 4041362f09..347fa9888a 100644 --- a/test/providers/provider_level_zero.cpp +++ b/test/providers/provider_level_zero.cpp @@ -254,9 +254,9 @@ TEST_P(umfLevelZeroProviderTest, allocInvalidSize) { ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); ASSERT_NE(provider, nullptr); - // try to alloc (int)-1 void *ptr = nullptr; - umf_result = umfMemoryProviderAlloc(provider, -1, 0, &ptr); + umf_result = umfMemoryProviderAlloc( + provider, std::numeric_limits::max(), 0, &ptr); ASSERT_EQ(umf_result, UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC); const char *message; int32_t error;