Merge pull request #889 from staniewzki/enable-werror

Enable -Werror flag in developer build

Author: ldorau

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}

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;

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;

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.

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;
     }
 

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;
     }

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(

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,

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) {

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<size_t>::max(), 0, &ptr);
     ASSERT_EQ(umf_result, UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC);
     const char *message;
     int32_t error;