Skip to content

Commit 7a19d5a

Browse files
bratpiorkabratpiorka
committed
add more secure compilation flags
1 parent fcac20b commit 7a19d5a

39 files changed

+220
-109
lines changed

‎CMakeLists.txt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ option(UMF_BUILD_BENCHMARKS "Build UMF benchmarks" OFF)
3737
option(UMF_BUILD_BENCHMARKS_MT "Build UMF multithreaded benchmarks" OFF)
3838
option(UMF_BUILD_EXAMPLES "Build UMF examples" ON)
3939
option(UMF_BUILD_GPU_EXAMPLES "Build UMF GPU examples" OFF)
40-
option(UMF_DEVELOPER_MODE "Enable developer checks, treats warnings as errors"
41-
OFF)
40+
option(UMF_DEVELOPER_MODE "Enable additional developer checks" OFF)
4241
option(UMF_FORMAT_CODE_STYLE
4342
"Add clang, cmake, and black -format-check and -format-apply targets"
4443
OFF)

‎benchmark/ubench.c

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,19 @@
2828
#include <unistd.h>
2929
#endif
3030

31+
// disable warning 6308:'realloc' might return null pointer: assigning null
32+
// pointer to 'failed_benchmarks', which is passed as an argument to 'realloc',
33+
// will cause the original memory block to be leaked.
34+
// disable warning 6001: Using uninitialized memory
35+
// '*ubench_state.benchmarks.name'.
36+
#if defined(_MSC_VER)
37+
#pragma warning(push)
38+
#pragma warning(disable : 6308)
39+
#pragma warning(disable : 6001)
40+
#endif // _MSC_VER
41+
3142
#include "ubench.h"
43+
3244
#include "utils_common.h"
3345

3446
#if (defined UMF_BUILD_GPU_TESTS)
@@ -63,7 +75,7 @@ static void do_benchmark(alloc_t *array, size_t iters, malloc_t malloc_f,
6375
int i = 0;
6476
do {
6577
array[i].ptr = malloc_f(provider, Alloc_size, 0);
66-
} while (array[i++].ptr != NULL && i < iters);
78+
} while (array[i++].ptr != NULL && i < (int)iters);
6779

6880
while (--i >= 0) {
6981
free_f(provider, array[i].ptr, Alloc_size);
@@ -110,14 +122,18 @@ UBENCH_EX(simple, glibc_malloc) {
110122

111123
static umf_os_memory_provider_params_t UMF_OS_MEMORY_PROVIDER_PARAMS = {
112124
/* .protection = */ UMF_PROTECTION_READ | UMF_PROTECTION_WRITE,
113-
/* .visibility */ UMF_MEM_MAP_PRIVATE,
125+
/* .visibility = */ UMF_MEM_MAP_PRIVATE,
126+
/* .shm_name = */ NULL,
114127

115128
// NUMA config
116-
/* .nodemask = */ NULL,
117-
/* .maxnode = */ 0,
129+
/* .numa_list = */ NULL,
130+
/* .numa_list_len = */ 0,
131+
118132
/* .numa_mode = */ UMF_NUMA_MODE_DEFAULT,
133+
/* .part_size = */ 0,
119134

120-
// others
135+
/* .partitions = */ NULL,
136+
/* .partitions_len = */ 0,
121137
};
122138

123139
static void *w_umfMemoryProviderAlloc(void *provider, size_t size,
@@ -487,3 +503,7 @@ UBENCH_EX(ipc, disjoint_pool_with_level_zero_provider) {
487503
#endif /* (defined UMF_BUILD_LIBUMF_POOL_DISJOINT && defined UMF_BUILD_LEVEL_ZERO_PROVIDER && defined UMF_BUILD_GPU_TESTS) */
488504

489505
UBENCH_MAIN()
506+
507+
#if defined(_MSC_VER)
508+
#pragma warning(pop)
509+
#endif // _MSC_VER

‎cmake/helpers.cmake

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -92,17 +92,20 @@ function(add_umf_target_compile_options name)
9292
${name}
9393
PRIVATE -fPIC
9494
-Wall
95+
-Wextra
96+
-Werror
9597
-Wpedantic
9698
-Wempty-body
9799
-Wunused-parameter
100+
-Wformat
101+
-Wformat-security
98102
$<$<CXX_COMPILER_ID:GNU>:-fdiagnostics-color=auto>)
99103
if(CMAKE_BUILD_TYPE STREQUAL "Release")
100104
target_compile_definitions(${name} PRIVATE -D_FORTIFY_SOURCE=2)
101105
endif()
102106
if(UMF_DEVELOPER_MODE)
103-
target_compile_options(
104-
${name} PRIVATE -Werror -fno-omit-frame-pointer
105-
-fstack-protector-strong)
107+
target_compile_options(${name} PRIVATE -fno-omit-frame-pointer
108+
-fstack-protector-strong)
106109
endif()
107110
if(USE_GCOV)
108111
if(NOT CMAKE_BUILD_TYPE STREQUAL "Debug")
@@ -113,12 +116,23 @@ function(add_umf_target_compile_options name)
113116
elseif(MSVC)
114117
target_compile_options(
115118
${name}
116-
PRIVATE $<$<CXX_COMPILER_ID:MSVC>:/MP> # clang-cl.exe does not
117-
# support /MP
118-
/W3 /MD$<$<CONFIG:Debug>:d> /GS)
119-
120-
if(UMF_DEVELOPER_MODE)
121-
target_compile_options(${name} PRIVATE /WX /GS)
119+
PRIVATE # clang-cl.exe does not support /MP
120+
$<$<CXX_COMPILER_ID:MSVC>:/MP>
121+
/W4
122+
/WX
123+
/MD$<$<CONFIG:Debug>:d>
124+
/Gy
125+
/GS
126+
/analyze
127+
# disable warning 6326: Potential comparison of a constant
128+
# with another constant
129+
/wd6326
130+
# disable 4200 warning: nonstandard extension used:
131+
# zero-sized array in struct/union
132+
/wd4200
133+
/DYNAMICBASE)
134+
if(CMAKE_BUILD_TYPE STREQUAL "Release")
135+
target_compile_options(${name} PRIVATE /sdl /LTCG /NXCOMPAT)
122136
endif()
123137
endif()
124138
endfunction()

‎examples/basic/ipc_ipcapi_consumer.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,13 +135,14 @@ int main(int argc, char *argv[]) {
135135
memset(recv_buffer, 0, RECV_BUFF_SIZE);
136136

137137
// receive a size of the IPC handle from the producer's
138-
ssize_t len = recv(producer_socket, recv_buffer, RECV_BUFF_SIZE, 0);
139-
if (len < 0) {
138+
ssize_t recv_len = recv(producer_socket, recv_buffer, RECV_BUFF_SIZE, 0);
139+
if (recv_len < 0) {
140140
fprintf(
141141
stderr,
142142
"[consumer] ERROR: receiving a size of the IPC handle failed\n");
143143
goto err_close_producer_socket;
144144
}
145+
size_t len = (size_t)recv_len;
145146

146147
size_t size_IPC_handle = *(size_t *)recv_buffer;
147148

@@ -151,11 +152,13 @@ int main(int argc, char *argv[]) {
151152
len, size_IPC_handle);
152153

153154
// send received size to the producer as a confirmation
154-
len = send(producer_socket, &size_IPC_handle, sizeof(size_IPC_handle), 0);
155-
if (len < 0) {
155+
recv_len =
156+
send(producer_socket, &size_IPC_handle, sizeof(size_IPC_handle), 0);
157+
if (recv_len < 0) {
156158
fprintf(stderr, "[consumer] ERROR: sending confirmation failed\n");
157159
goto err_close_producer_socket;
158160
}
161+
len = (size_t)recv_len;
159162

160163
fprintf(stderr,
161164
"[consumer] Sent a confirmation to the producer (%zu bytes)\n",
@@ -169,11 +172,13 @@ int main(int argc, char *argv[]) {
169172
}
170173

171174
// receive the IPC handle from the producer's
172-
len = recv(producer_socket, IPC_handle, size_IPC_handle, 0);
173-
if (len < 0) {
175+
recv_len = recv(producer_socket, IPC_handle, size_IPC_handle, 0);
176+
if (recv_len < 0) {
174177
fprintf(stderr, "[consumer] ERROR: receiving the IPC handle failed\n");
175178
goto err_free_IPC_handle;
176179
}
180+
len = (size_t)recv_len;
181+
177182
if (len < size_IPC_handle) {
178183
fprintf(stderr,
179184
"[consumer] ERROR: receiving the IPC handle failed - received "

‎examples/basic/ipc_level_zero.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,11 @@ int create_level_zero_pool(ze_context_handle_t context,
2020
ze_device_handle_t device,
2121
umf_memory_pool_handle_t *pool) {
2222
// setup params
23-
level_zero_memory_provider_params_t params = {0};
23+
level_zero_memory_provider_params_t params;
2424
params.level_zero_context_handle = context;
2525
params.level_zero_device_handle = device;
2626
params.memory_type = UMF_MEMORY_TYPE_DEVICE;
27+
2728
// create Level Zero provider
2829
umf_memory_provider_handle_t provider = 0;
2930
umf_result_t umf_result = umfMemoryProviderCreate(

‎include/umf/proxy_lib_new_delete.h

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,13 @@ SOFTWARE.
4848
#include <stdlib.h>
4949
#endif // _WIN32
5050

51+
// disable warning C28251: Inconsistent annotation for 'new': this instance
52+
// has no annotations.
53+
#if defined(_MSC_VER)
54+
#pragma warning(push)
55+
#pragma warning(disable : 28251)
56+
#endif // _MSC_VER
57+
5158
static inline void *internal_aligned_alloc(size_t alignment, size_t size) {
5259
#ifdef _WIN32
5360
return _aligned_malloc(size, alignment);
@@ -75,10 +82,18 @@ void operator delete(void *p, const std::nothrow_t &) noexcept { free(p); }
7582
void operator delete[](void *p, const std::nothrow_t &) noexcept { free(p); }
7683

7784
decl_new(n) void *operator new(std::size_t n) noexcept(false) {
78-
return malloc(n);
85+
void *ptr = malloc(n);
86+
if (ptr == nullptr) {
87+
throw std::bad_alloc();
88+
}
89+
return ptr;
7990
}
8091
decl_new(n) void *operator new[](std::size_t n) noexcept(false) {
81-
return malloc(n);
92+
void *ptr = malloc(n);
93+
if (ptr == nullptr) {
94+
throw std::bad_alloc();
95+
}
96+
return ptr;
8297
}
8398

8499
decl_new_nothrow(n) void *operator new(std::size_t n,
@@ -134,10 +149,18 @@ void operator delete[](void *p, std::align_val_t al,
134149
}
135150

136151
void *operator new(std::size_t n, std::align_val_t al) noexcept(false) {
137-
return internal_aligned_alloc(static_cast<size_t>(al), n);
152+
void *ptr = internal_aligned_alloc(static_cast<size_t>(al), n);
153+
if (ptr == nullptr) {
154+
throw std::bad_alloc();
155+
}
156+
return ptr;
138157
}
139158
void *operator new[](std::size_t n, std::align_val_t al) noexcept(false) {
140-
return internal_aligned_alloc(static_cast<size_t>(al), n);
159+
void *ptr = internal_aligned_alloc(static_cast<size_t>(al), n);
160+
if (ptr == nullptr) {
161+
throw std::bad_alloc();
162+
}
163+
return ptr;
141164
}
142165
void *operator new(std::size_t n, std::align_val_t al,
143166
const std::nothrow_t &) noexcept {
@@ -147,6 +170,11 @@ void *operator new[](std::size_t n, std::align_val_t al,
147170
const std::nothrow_t &) noexcept {
148171
return internal_aligned_alloc(static_cast<size_t>(al), n);
149172
}
173+
174+
#if defined(_MSC_VER)
175+
#pragma warning(pop)
176+
#endif // _MSC_VER
177+
150178
#endif // (__cplusplus > 201402L || defined(__cpp_aligned_new))
151179
#endif // defined(__cplusplus)
152180

‎src/base_alloc/base_alloc.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,12 @@ void *umf_ba_alloc(umf_ba_pool_t *pool) {
219219
// we'll mark the memory as undefined
220220
utils_annotate_memory_defined(chunk, sizeof(*chunk));
221221

222+
// check if the free list is not empty
223+
if (pool->metadata.free_list == NULL) {
224+
LOG_ERR("pool->metadata.free_list == NULL");
225+
return NULL;
226+
}
227+
222228
pool->metadata.free_list = pool->metadata.free_list->next;
223229
pool->metadata.n_allocs++;
224230
#ifndef NDEBUG

‎src/base_alloc/base_alloc_linear.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,9 @@ int umf_ba_linear_free(umf_ba_linear_pool_t *pool, void *ptr) {
197197
if ((pool->metadata.pool_n_allocs == 0) && pool->next_pool &&
198198
(pool->metadata.pool_size > page_size)) {
199199
// we can free the first (main) pool except of the first page containing the metadata
200-
void *ptr = (char *)pool + page_size;
200+
void *pool_ptr = (char *)pool + page_size;
201201
size_t size = pool->metadata.pool_size - page_size;
202-
ba_os_free(ptr, size);
202+
ba_os_free(pool_ptr, size);
203203
// update pool_size
204204
pool->metadata.pool_size = page_size;
205205
}
@@ -222,9 +222,9 @@ int umf_ba_linear_free(umf_ba_linear_pool_t *pool, void *ptr) {
222222
assert(prev_pool->next_pool == next_pool);
223223
prev_pool->next_pool = next_pool->next_pool;
224224
_DEBUG_EXECUTE(pool->metadata.n_pools--);
225-
void *ptr = next_pool;
225+
void *next_pool_ptr = next_pool;
226226
size_t size = next_pool->pool_size;
227-
ba_os_free(ptr, size);
227+
ba_os_free(next_pool_ptr, size);
228228
}
229229
_DEBUG_EXECUTE(ba_debug_checks(pool));
230230
util_mutex_unlock(&pool->metadata.lock);

‎src/base_alloc/base_alloc_windows.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ void *ba_os_alloc(size_t size) {
1616
return VirtualAlloc(NULL, size, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE);
1717
}
1818

19-
void ba_os_free(void *ptr, size_t size) { VirtualFree(ptr, 0, MEM_RELEASE); }
19+
void ba_os_free(void *ptr, size_t size) {
20+
(void)size;
21+
VirtualFree(ptr, 0, MEM_RELEASE);
22+
}
2023

2124
static void _ba_os_init_page_size(void) {
2225
SYSTEM_INFO SystemInfo;

‎src/cpp_helpers.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ template <typename T, typename ParamType> umf_memory_pool_ops_t poolMakeCOps() {
117117
}
118118

119119
if constexpr (std::is_same_v<ParamType, void>) {
120+
(void)params; // unused
120121
return detail::initialize<T>(reinterpret_cast<T *>(*obj),
121122
std::make_tuple(provider));
122123
} else {
@@ -145,6 +146,7 @@ umf_memory_provider_ops_t providerMakeCOps() {
145146
}
146147

147148
if constexpr (std::is_same_v<ParamType, void>) {
149+
(void)params; // unused
148150
return detail::initialize<T>(reinterpret_cast<T *>(*obj),
149151
std::make_tuple());
150152
} else {

‎src/libumf_windows.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
#if defined(UMF_SHARED_LIBRARY) /* SHARED LIBRARY */
1515

1616
BOOL APIENTRY DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved) {
17+
(void)hinstDLL; // unused
18+
(void)lpvReserved; // unused
19+
1720
if (fdwReason == DLL_PROCESS_ATTACH) {
1821
(void)umfInit();
1922
} else if (fdwReason == DLL_PROCESS_DETACH) {
@@ -32,6 +35,10 @@ INIT_ONCE init_once_flag = INIT_ONCE_STATIC_INIT;
3235

3336
BOOL CALLBACK initOnceCb(PINIT_ONCE InitOnce, PVOID Parameter,
3437
PVOID *lpContext) {
38+
(void)InitOnce;
39+
(void)Parameter;
40+
(void)lpContext;
41+
3542
int ret = umfInit();
3643
atexit(umfTearDown);
3744
return (ret == 0) ? TRUE : FALSE;

‎src/memory_targets/memory_target_numa.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
*/
99

1010
#include <assert.h>
11-
#include <hwloc.h>
1211
#include <stdlib.h>
1312

1413
#include <umf/pools/pool_disjoint.h>

‎src/memspace.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ umf_result_t umfMemspaceFilter(umf_memspace_handle_t hMemspace,
265265
}
266266

267267
size_t cloneIdx = 0;
268-
for (size_t cloneIdx = 0; cloneIdx < newMemspace->size; cloneIdx++) {
268+
for (cloneIdx = 0; cloneIdx < newMemspace->size; cloneIdx++) {
269269
ret = umfMemoryTargetClone(uniqueBestNodes[cloneIdx],
270270
&newMemspace->nodes[cloneIdx]);
271271
if (ret != UMF_RESULT_SUCCESS) {

‎src/memspaces/memspace_highest_bandwidth.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
#include <assert.h>
1111
#include <ctype.h>
12-
#include <hwloc.h>
1312
#include <stdlib.h>
1413

1514
#include "base_alloc_global.h"

‎src/memspaces/memspace_highest_capacity.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
*/
99

1010
#include <assert.h>
11-
#include <hwloc.h>
1211
#include <stdlib.h>
1312

1413
#include "base_alloc_global.h"

‎src/memspaces/memspace_host_all.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
*/
99

1010
#include <assert.h>
11-
#include <hwloc.h>
1211
#include <stdlib.h>
1312

1413
#include "base_alloc_global.h"

‎src/memspaces/memspace_lowest_latency.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
#include <assert.h>
1111
#include <ctype.h>
12-
#include <hwloc.h>
1312
#include <stdlib.h>
1413

1514
#include "base_alloc_global.h"

0 commit comments

Comments
 (0)