Skip to content

Commit 8dfd094

Browse files
haoxian2pcolberg
authored andcommitted
Fixed coverity in acl_hal_mmd.cpp: Resource leak (RESOURCE_LEAK)
mmd_libraries were not closed after execution, which causes memory leaks. But mmd_libraries cannot be closed trivially since closing the library invalidates the symbol return by dlsym(), which means calling the function further down is undefined behaviour. To solve this issues, I added a `dl_wrapper_t` vector as a global initialized vector that would be deallocated when closing the runtime library. This vector would contain all the opened libraries that would be closed by the destructor of `dl_wrapper_t` once the vector deallocates.
1 parent b36c589 commit 8dfd094

File tree

1 file changed

+37
-21
lines changed

1 file changed

+37
-21
lines changed

src/acl_hal_mmd.cpp

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
#include <cstring>
1717
#include <fstream>
1818
#include <iostream>
19+
#include <memory>
20+
#include <optional>
1921
#include <string>
2022
#include <vector>
2123
#ifdef __linux__
@@ -188,8 +190,6 @@ unsigned acl_convert_mmd_capabilities(unsigned mmd_capabilities);
188190
const static size_t MIN_SOF_SIZE = 1;
189191
const static size_t MIN_PLL_CONFIG_SIZE = 1;
190192

191-
std::vector<acl_mmd_dispatch_t> internal_mmd_dispatch;
192-
193193
// Dynamically load board mmd & symbols
194194
static size_t num_board_pkgs;
195195
static double min_MMD_version = DBL_MAX;
@@ -426,14 +426,26 @@ static void *my_dlsym(void *library, const char *function_name,
426426
}
427427

428428
static void my_dlclose(void *library) {
429-
acl_assert_locked();
430429
#ifdef _WIN32
431430
FreeLibrary((HMODULE)library);
432431
#else
433432
dlclose(library);
434433
#endif
435434
}
436435

436+
struct acl_mmd_library {
437+
struct deleter {
438+
void operator()(void *library) { my_dlclose(library); }
439+
};
440+
441+
using pointer = std::unique_ptr<void, deleter>;
442+
443+
pointer ptr;
444+
acl_mmd_dispatch_t dispatch;
445+
};
446+
447+
static std::vector<acl_mmd_library> internal_mmd_dispatch;
448+
437449
cl_bool l_load_board_functions(acl_mmd_dispatch_t *mmd_dispatch,
438450
const char *library_name, void *mmd_library,
439451
char *error_msg) {
@@ -564,7 +576,7 @@ cl_bool l_load_single_board_library(const char *library_name,
564576
return CL_FALSE;
565577
}
566578
#endif
567-
auto *mmd_library = my_dlopen(library_name, &error_msg);
579+
acl_mmd_library::pointer mmd_library{my_dlopen(library_name, &error_msg)};
568580
if (!mmd_library) {
569581
std::cout << "Error: Could not load board library " << library_name;
570582
if (error_msg && error_msg[0] != '\0') {
@@ -575,14 +587,15 @@ cl_bool l_load_single_board_library(const char *library_name,
575587
}
576588

577589
if (load_libraries) {
578-
auto result =
579-
l_load_board_functions(&(internal_mmd_dispatch[num_boards_found]),
580-
library_name, mmd_library, error_msg);
590+
auto result = l_load_board_functions(
591+
&internal_mmd_dispatch[num_boards_found].dispatch, library_name,
592+
mmd_library.get(), error_msg);
581593
if (result == CL_FALSE) {
582594
std::cout << "Error: Could not load board library " << library_name
583595
<< " due to failure to load symbols\n";
584596
return result;
585597
}
598+
std::swap(mmd_library, internal_mmd_dispatch[num_boards_found].ptr);
586599
}
587600
++num_boards_found;
588601

@@ -1104,7 +1117,7 @@ void l_close_device(unsigned int physical_device_id,
11041117
return;
11051118
}
11061119

1107-
static acl_mmd_dispatch_t *get_msim_mmd_layer() {
1120+
static std::optional<acl_mmd_library> get_msim_mmd_layer() {
11081121
#ifdef _WIN32
11091122

11101123
const char *acl_root_dir = acl_getenv("INTELFPGAOCLSDKROOT");
@@ -1129,7 +1142,7 @@ static acl_mmd_dispatch_t *get_msim_mmd_layer() {
11291142
ipa_simulator ? mmd_lib_name_ipa : mmd_lib_name_aoc;
11301143

11311144
char *error_msg = nullptr;
1132-
auto *mmd_lib = my_dlopen(mmd_lib_name, &error_msg);
1145+
acl_mmd_library::pointer mmd_lib{my_dlopen(mmd_lib_name, &error_msg)};
11331146
typedef acl_mmd_dispatch_t *(*fcn_type)();
11341147
if (!mmd_lib) {
11351148
std::cout << "Error: Could not load simulation MMD library "
@@ -1138,17 +1151,17 @@ static acl_mmd_dispatch_t *get_msim_mmd_layer() {
11381151
std::cout << " (error_msg: " << error_msg << ")";
11391152
}
11401153
std::cout << "\n";
1141-
return nullptr;
1154+
return {};
11421155
}
1143-
auto *sym = my_dlsym(mmd_lib, sym_name, &error_msg);
1156+
auto *sym = my_dlsym(mmd_lib.get(), sym_name, &error_msg);
11441157
if (!sym) {
11451158
std::cout << "Error: Symbol " << sym_name
11461159
<< " not found in simulation MMD library ";
11471160
if (error_msg && error_msg[0] != '\0') {
11481161
std::cout << "(message: " << error_msg << ")";
11491162
}
11501163
std::cout << "\n";
1151-
return nullptr;
1164+
return {};
11521165
}
11531166

11541167
// Now call the function. Ignore the Windows cast to fcn pointer
@@ -1157,10 +1170,13 @@ static acl_mmd_dispatch_t *get_msim_mmd_layer() {
11571170
#pragma warning(push)
11581171
#pragma warning(disable : 4055)
11591172
#endif
1160-
return ((fcn_type)sym)();
1173+
acl_mmd_dispatch_t *mmd_dispatch = ((fcn_type)sym)();
11611174
#ifdef _MSC_VER
11621175
#pragma warning(pop)
11631176
#endif
1177+
1178+
return std::make_optional<acl_mmd_library>(
1179+
{std::move(mmd_lib), *mmd_dispatch});
11641180
}
11651181

11661182
ACL_HAL_EXPORT const acl_hal_t *
@@ -1225,11 +1241,11 @@ acl_mmd_get_system_definition(acl_system_def_t *sys,
12251241
if (use_offline_only == ACL_CONTEXT_MPSIM) {
12261242

12271243
// Substitute the simulator MMD layer.
1228-
auto *result = get_msim_mmd_layer();
1229-
if (!result)
1244+
auto result = get_msim_mmd_layer();
1245+
if (!result) {
12301246
return nullptr;
1231-
else
1232-
internal_mmd_dispatch.push_back(*result);
1247+
}
1248+
internal_mmd_dispatch.push_back(std::move(*result));
12331249

12341250
ACL_HAL_DEBUG_MSG_VERBOSE(1, "Use simulation MMD\n");
12351251
num_board_pkgs = 1;
@@ -1239,7 +1255,7 @@ acl_mmd_get_system_definition(acl_system_def_t *sys,
12391255
internal_mmd_dispatch.resize(num_board_pkgs);
12401256
ACL_HAL_DEBUG_MSG_VERBOSE(1, "Board MMD is statically linked\n");
12411257

1242-
acl_mmd_dispatch_t &dispatch = internal_mmd_dispatch[0];
1258+
acl_mmd_dispatch_t &dispatch = internal_mmd_dispatch[0].dispatch;
12431259

12441260
dispatch.library_name = "runtime_static";
12451261
dispatch.mmd_library = nullptr;
@@ -1315,7 +1331,7 @@ acl_mmd_get_system_definition(acl_system_def_t *sys,
13151331
sys->num_devices = 0;
13161332
num_physical_devices = 0;
13171333
for (unsigned iboard = 0; iboard < num_board_pkgs; ++iboard) {
1318-
acl_mmd_dispatch_t &dispatch = internal_mmd_dispatch[iboard];
1334+
acl_mmd_dispatch_t &dispatch = internal_mmd_dispatch[iboard].dispatch;
13191335

13201336
dispatch.aocl_mmd_get_offline_info(AOCL_MMD_VERSION, sizeof(buf), buf,
13211337
NULL);
@@ -1387,7 +1403,7 @@ int acl_hal_mmd_try_devices(cl_uint num_devices, const cl_device_id *devices,
13871403
// names might get cached by various routines
13881404

13891405
for (unsigned iboard = 0; iboard < num_board_pkgs; ++iboard) {
1390-
acl_mmd_dispatch_t &dispatch = internal_mmd_dispatch[iboard];
1406+
acl_mmd_dispatch_t &dispatch = internal_mmd_dispatch[iboard].dispatch;
13911407

13921408
dispatch.aocl_mmd_get_offline_info(AOCL_MMD_BOARD_NAMES,
13931409
MAX_BOARD_NAMES_LEN, buf, NULL);
@@ -1457,7 +1473,7 @@ int acl_hal_mmd_try_devices(cl_uint num_devices, const cl_device_id *devices,
14571473
physical_device_id = 0;
14581474

14591475
for (unsigned iboard = 0; iboard < num_board_pkgs; ++iboard) {
1460-
acl_mmd_dispatch_t &dispatch = internal_mmd_dispatch[iboard];
1476+
acl_mmd_dispatch_t &dispatch = internal_mmd_dispatch[iboard].dispatch;
14611477

14621478
dispatch.aocl_mmd_get_offline_info(AOCL_MMD_BOARD_NAMES,
14631479
MAX_BOARD_NAMES_LEN, buf, NULL);

0 commit comments

Comments
 (0)