From 0df1efc938bdead0e2e12bbba85d49f8a922a060 Mon Sep 17 00:00:00 2001 From: Yossi Itigin Date: Sun, 2 Jun 2019 18:40:24 +0300 Subject: [PATCH] UCS/TEST: Remove entry from configuration list when library is unloaded When UCT is loaded, the static initializer adds it's configuration tables to the global list in UCS. When UCT is unloaded, need to remove them from that list, otherwise the list will point to invalid memory. --- NEWS | 1 + contrib/test_jenkins.sh | 4 +++ src/ucs/config/parser.h | 17 ++++++---- test/apps/Makefile.am | 10 +++++- test/apps/test_dlopen_cfg_print.c | 54 +++++++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 7 deletions(-) create mode 100644 test/apps/test_dlopen_cfg_print.c diff --git a/NEWS b/NEWS index 360e67e2f7d..138217cc700 100644 --- a/NEWS +++ b/NEWS @@ -25,6 +25,7 @@ Bugfixes: - RPM Spec file cleanup - Fixing compilation issues with most recent clang and gcc compilers - Fixing the wrong name of aliases +- Fix segfault when libuct.so is reloaded - issue #3558 Tested configurations: - RDMA: MLNX_OFED 4.5, distribution inbox drivers, rdma-core 22.1 diff --git a/contrib/test_jenkins.sh b/contrib/test_jenkins.sh index 824587e0d12..c9520e6161a 100755 --- a/contrib/test_jenkins.sh +++ b/contrib/test_jenkins.sh @@ -847,6 +847,10 @@ test_ucs_dlopen() { # Make sure UCM is not unloaded echo "==== Running UCS dlopen test with memhooks ====" ./test/apps/test_ucs_dlopen + + # Test global config list integrity after loading/unloading of UCT + echo "==== Running test_dlopen_cfg_print ====" + ./test/apps/test_dlopen_cfg_print } test_ucp_dlopen() { diff --git a/src/ucs/config/parser.h b/src/ucs/config/parser.h index efa57d13893..705de2f7fd5 100644 --- a/src/ucs/config/parser.h +++ b/src/ucs/config/parser.h @@ -88,14 +88,19 @@ typedef struct ucs_config_bw_spec { #define UCS_CONFIG_REGISTER_TABLE(_fields, _name, _prefix, _type) \ + static ucs_config_global_list_entry_t _fields##_config_entry; \ UCS_STATIC_INIT { \ + ucs_config_global_list_entry_t *entry = &_fields##_config_entry; \ extern ucs_list_link_t ucs_config_global_list; \ - static ucs_config_global_list_entry_t entry; \ - entry.fields = _fields; \ - entry.name = _name; \ - entry.prefix = _prefix; \ - entry.size = sizeof(_type); \ - ucs_list_add_tail(&ucs_config_global_list, &entry.list); \ + entry->fields = _fields; \ + entry->name = _name; \ + entry->prefix = _prefix; \ + entry->size = sizeof(_type); \ + ucs_list_add_tail(&ucs_config_global_list, &entry->list); \ + } \ + \ + UCS_STATIC_CLEANUP { \ + ucs_list_del(&_fields##_config_entry.list); \ } diff --git a/test/apps/Makefile.am b/test/apps/Makefile.am index cd47c5534b4..c6258b6471e 100644 --- a/test/apps/Makefile.am +++ b/test/apps/Makefile.am @@ -13,7 +13,8 @@ endif noinst_PROGRAMS = \ test_ucp_dlopen \ test_ucs_dlopen \ - test_link_map + test_link_map \ + test_dlopen_cfg_print objdir = $(shell sed -n -e 's/^objdir=\(.*\)$$/\1/p' $(LIBTOOL)) @@ -34,6 +35,13 @@ test_link_map_CPPFLAGS = $(BASE_CPPFLAGS) test_link_map_CFLAGS = $(BASE_CFLAGS) test_link_map_LDADD = -ldl $(top_builddir)/src/ucp/libucp.la +test_dlopen_cfg_print_SOURCES = test_dlopen_cfg_print.c +test_dlopen_cfg_print_CPPFLAGS = $(BASE_CPPFLAGS) -g \ + -DUCS_LIB_PATH=$(abs_top_builddir)/src/ucs/$(objdir)/libucs.so \ + -DUCT_LIB_PATH=$(abs_top_builddir)/src/uct/$(objdir)/libuct.so +test_dlopen_cfg_print_CFLAGS = $(BASE_CFLAGS) +test_dlopen_cfg_print_LDADD = -ldl + if HAVE_TCMALLOC noinst_PROGRAMS += test_tcmalloc test_tcmalloc_SOURCES = test_tcmalloc.c diff --git a/test/apps/test_dlopen_cfg_print.c b/test/apps/test_dlopen_cfg_print.c new file mode 100644 index 00000000000..21e6c402fef --- /dev/null +++ b/test/apps/test_dlopen_cfg_print.c @@ -0,0 +1,54 @@ +/** + * Copyright (C) Mellanox Technologies Ltd. 2019. ALL RIGHTS RESERVED. + * + * See file LICENSE for terms. + */ + +#include +#include +#include + +#define _QUOTE(x) #x +#define QUOTE(x) _QUOTE(x) + + +static void* do_dlopen_or_exit(const char *filename) +{ + void *handle; + + (void)dlerror(); + printf("opening '%s'\n", filename); + handle = dlopen(filename, RTLD_LAZY); + if (handle == NULL) { + fprintf(stderr, "failed to open %s: %s\n", filename, + dlerror()); + exit(1); + } + + return handle; +} + +int main(int argc, char **argv) +{ + const char *ucs_filename = QUOTE(UCS_LIB_PATH); + const char *uct_filename = QUOTE(UCT_LIB_PATH); + void *ucs_handle, *uct_handle; + int i; + + /* unload and reload uct while ucs is loaded + * would fail if uct global vars are kept on global lists in ucs */ + ucs_handle = do_dlopen_or_exit(ucs_filename); + for (i = 0; i < 2; ++i) { + uct_handle = do_dlopen_or_exit(uct_filename); + dlclose(uct_handle); + } + + /* print all config table, to force going over the global list in ucs */ + void (*print_all_opts)(FILE*, int) = + dlsym(ucs_handle, "ucs_config_parser_print_all_opts"); + print_all_opts(stdout, 0); + dlclose(ucs_handle); + + printf("done\n"); + return 0; +}