Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve shared library relative paths handling #320

Merged
merged 8 commits into from
Dec 22, 2020
Merged

Improve shared library relative paths handling #320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8ee8def
Retrieve shared library path after loading.
hidmic Dec 11, 2020
6925a2f
Update shared_library.c tests
hidmic Dec 11, 2020
5915051
Fix bad RCUTILS_SET_ERROR_MSG call.
hidmic Dec 14, 2020
693c728
Fix dyld API typo.
hidmic Dec 14, 2020
dc59b9d
Fix typos in Windows specific shared_library.c bits
hidmic Dec 15, 2020
1d4276a
Address peer review comments.
hidmic Dec 21, 2020
31facfc
Add more regression tests
hidmic Dec 22, 2020
bf6e95c
Fix regression tests in MacOS
hidmic Dec 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 53 additions & 10 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,20 +372,63 @@ if(BUILD_TESTING)
target_link_libraries(test_repl_str ${PROJECT_NAME})
endif()

set(append_library_dirs "$<TARGET_FILE_DIR:${PROJECT_NAME}>")

ament_add_gtest(test_shared_library test/test_shared_library.cpp
APPEND_LIBRARY_DIRS "${append_library_dirs}"
)

if(TARGET test_shared_library)
add_library(dummy_shared_library test/dummy_shared_library/dummy_shared_library.c)
macro(add_dummy_shared_library target)
add_library(${target} test/dummy_shared_library/dummy_shared_library.c)
if(WIN32)
# Causes the visibility macros to use dllexport rather than dllimport
# which is appropriate when building the dll but not consuming it.
target_compile_definitions(dummy_shared_library PRIVATE "DUMMY_SHARED_LIBRARY_BUILDING_DLL")
target_compile_definitions(${target} PRIVATE "DUMMY_SHARED_LIBRARY_BUILDING_DLL")
endif()
endmacro()

ament_add_gtest(test_shared_library_in_run_paths test/test_shared_library.cpp)
if(TARGET test_shared_library_in_run_paths)
# Rely on CMake setting build tree RUNPATHs by default on Unix systems.
add_dummy_shared_library(dummy_shared_library_in_run_paths)
target_compile_definitions(test_shared_library_in_run_paths PRIVATE
"SHARED_LIBRARY_UNDER_TEST=dummy_shared_library_in_run_paths")
if(NOT WIN32 AND NOT APPLE)
# NOTE(hidmic): DT_RUNPATH entries are ignored by dlopen in Linux despite the fact
# documentation says otherwise, so here we fallback to DT_RPATH entries.
target_link_libraries(test_shared_library_in_run_paths "-Wl,--disable-new-dtags")
endif()
target_link_libraries(test_shared_library_in_run_paths ${PROJECT_NAME} mimick)
endif()

set(project_binary_dir "$<TARGET_FILE_DIR:${PROJECT_NAME}>")
set(test_libraries_dir "$<TARGET_FILE_DIR:${PROJECT_NAME}>/test_libraries")
ament_add_gtest(test_shared_library_in_load_paths test/test_shared_library.cpp
APPEND_LIBRARY_DIRS ${project_binary_dir} ${test_libraries_dir}
)
if(TARGET test_shared_library_in_load_paths)
# Make sure CMake adds no RPATHs/RUNPATHs.
set_target_properties(test_shared_library_in_load_paths
PROPERTIES SKIP_BUILD_RPATH TRUE)
add_dummy_shared_library(dummy_shared_library_in_load_paths)
set_target_properties(dummy_shared_library_in_load_paths PROPERTIES
LIBRARY_OUTPUT_DIRECTORY ${test_libraries_dir})
target_compile_definitions(test_shared_library_in_load_paths PRIVATE
"SHARED_LIBRARY_UNDER_TEST=dummy_shared_library_in_load_paths")
target_link_libraries(test_shared_library_in_load_paths ${PROJECT_NAME} mimick)
endif()

ament_add_gtest(test_shared_library_preloaded test/test_shared_library.cpp)
if(TARGET test_shared_library_in_load_paths)
add_dummy_shared_library(dummy_shared_library_preloaded)
set_target_properties(dummy_shared_library_preloaded PROPERTIES
LIBRARY_OUTPUT_DIRECTORY ${test_libraries_dir})
target_compile_definitions(test_shared_library_preloaded PRIVATE
"SHARED_LIBRARY_UNDER_TEST=dummy_shared_library_preloaded")
if(NOT WIN32)
# Force (apparently) unused libraries to be linked in.
if(CMAKE_CXX_COMPILER_ID MATCHES "(C|c)lang")
target_link_libraries(test_shared_library_preloaded "-Wl,-all_load")
else()
target_link_libraries(test_shared_library_preloaded "-Wl,--no-as-needed")
endif()
endif()
target_link_libraries(test_shared_library ${PROJECT_NAME} mimick)
target_link_libraries(test_shared_library_preloaded dummy_shared_library_preloaded)
target_link_libraries(test_shared_library_preloaded ${PROJECT_NAME} mimick)
endif()

rcutils_custom_add_gtest(test_time
Expand Down
129 changes: 115 additions & 14 deletions src/shared_library.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ extern "C"
#include <stdlib.h>

#ifndef _WIN32
#if defined(__APPLE__)
#include <mach-o/dyld.h>
#elif defined (_GNU_SOURCE)
#include <link.h>
#endif
#include <dlfcn.h>
#else
// When building with MSVC 19.28.29333.0 on Windows 10 (as of 2020-11-11),
Expand All @@ -35,6 +40,10 @@ extern "C"
#include <windows.h>
#pragma warning(pop)
C_ASSERT(sizeof(void *) == sizeof(HINSTANCE));
#ifdef UNICODE
#error "rcutils does not support Unicode paths"
#endif
C_ASSERT(sizeof(char) == sizeof(TCHAR));
#endif // _WIN32

#include "rcutils/error_handling.h"
Expand Down Expand Up @@ -65,33 +74,125 @@ rcutils_load_shared_library(
RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib, RCUTILS_RET_INVALID_ARGUMENT);
RCUTILS_CHECK_ARGUMENT_FOR_NULL(library_path, RCUTILS_RET_INVALID_ARGUMENT);
RCUTILS_CHECK_ALLOCATOR(&allocator, return RCUTILS_RET_INVALID_ARGUMENT);

if (lib->library_path != NULL) {
lib->allocator.deallocate(lib->library_path, lib->allocator.state);
if (NULL != lib->lib_pointer) {
RCUTILS_SET_ERROR_MSG("lib argument is not zero-initialized");
return RCUTILS_RET_INVALID_ARGUMENT;
}

rcutils_ret_t ret = RCUTILS_RET_OK;
lib->allocator = allocator;

// Delegate library path resolution to dynamic linker.

// Since the given library path might be relative, let the dynamic
// linker search for the binary object in all standard locations and
// current process space (if already loaded) first.
// Then lookup its full path with the handle obtained.
// See POSIX dlopen() API and Windows' LoadLibrary() API documentation
// for further reference.

#ifndef _WIN32
lib->lib_pointer = dlopen(library_path, RTLD_LAZY);
if (NULL == lib->lib_pointer) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("dlopen error: %s", dlerror());
return RCUTILS_RET_ERROR;
}

#if defined(__APPLE__)
const char * image_name = NULL;
uint32_t image_count = _dyld_image_count();
for (uint32_t i = 0; NULL == image_name && i < image_count; ++i) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like I'm missing something. Is my understanding correct that what ends up in lib->library_path is the absolute path of library_path? If so, can't we just call realpath? Or is it possible library_path and lib->library_path will point to two different files?

Copy link
Contributor Author

@hidmic hidmic Dec 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If so, can't we just call realpath?

The thing is, library_path might simply be the library name e.g. libfoo.so. And that doesn't mean it should be searched in the current working directory. It's only after dlopen performs the search over RPATHs, (DY)LD_LIBRARY_PATHs, and RUNPATHs and actually finds it that you can get the full path to it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that makes sense. Thanks! I don't know what the comment etiquette is for this package, but I think it would be great to have that paragraph as a comment just to explain what the goal of this section of code is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing. See 1d4276a.

// Iterate in reverse as the library is likely near the end of the list.
const char * candidate_name = _dyld_get_image_name(image_count - i - 1);
if (NULL == candidate_name) {
RCUTILS_SET_ERROR_MSG("dyld image index out of range");
ret = RCUTILS_RET_ERROR;
goto fail;
}
void * handle = dlopen(candidate_name, RTLD_LAZY | RTLD_NOLOAD);
if (handle == lib->lib_pointer) {
image_name = candidate_name;
}
if (dlclose(handle) != 0) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("dlclose error: %s", dlerror());
ret = RCUTILS_RET_ERROR;
goto fail;
}
}
if (NULL == image_name) {
RCUTILS_SET_ERROR_MSG("dyld image name could not be found");
ret = RCUTILS_RET_ERROR;
goto fail;
}
lib->library_path = rcutils_strdup(image_name, lib->allocator);
#elif defined(_GNU_SOURCE)
struct link_map * map = NULL;
if (dlinfo(lib->lib_pointer, RTLD_DI_LINKMAP, &map) != 0) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("dlinfo error: %s", dlerror());
ret = RCUTILS_RET_ERROR;
goto fail;
}
lib->library_path = rcutils_strdup(map->l_name, lib->allocator);
#else
lib->library_path = rcutils_strdup(library_path, lib->allocator);
#endif
if (NULL == lib->library_path) {
RCUTILS_SET_ERROR_MSG("unable to allocate memory");
return RCUTILS_RET_BAD_ALLOC;
ret = RCUTILS_RET_BAD_ALLOC;
goto fail;
}

#ifndef _WIN32
lib->lib_pointer = dlopen(lib->library_path, RTLD_LAZY);
if (!lib->lib_pointer) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("LoadLibrary error: %s", dlerror());
return RCUTILS_RET_OK;
fail:
if (dlclose(lib->lib_pointer) != 0) {
RCUTILS_SAFE_FWRITE_TO_STDERR_WITH_FORMAT_STRING(
"dlclose error: %s\n", dlerror());
}
lib->lib_pointer = NULL;
return ret;
#else
lib->lib_pointer = (void *)(LoadLibrary(lib->library_path));
if (!lib->lib_pointer) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("LoadLibrary error: %lu", GetLastError());
#endif // _WIN32
lib->allocator.deallocate(lib->library_path, lib->allocator.state);
lib->library_path = NULL;
HMODULE module = LoadLibrary(library_path);
if (!module) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"LoadLibrary error: %lu", GetLastError());
return RCUTILS_RET_ERROR;
}

for (DWORD buffer_capacity = MAX_PATH; ; buffer_capacity *= 2) {
LPSTR buffer = lib->allocator.allocate(buffer_capacity, lib->allocator.state);
if (NULL == buffer) {
RCUTILS_SET_ERROR_MSG("unable to allocate memory");
ret = RCUTILS_RET_BAD_ALLOC;
goto fail;
}
DWORD buffer_size = GetModuleFileName(module, buffer, buffer_capacity);
if (0 == buffer_size) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"GetModuleFileName error: %lu", GetLastError());
ret = RCUTILS_RET_ERROR;
goto fail;
}
if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
lib->allocator.deallocate(buffer, lib->allocator.state);
continue;
}
lib->library_path = lib->allocator.reallocate(
buffer, buffer_size, lib->allocator.state);
if (NULL == lib->library_path) {
lib->library_path = buffer;
}
break;
}
lib->lib_pointer = (void *)module;

return RCUTILS_RET_OK;
fail:
if (!FreeLibrary(module)) {
RCUTILS_SAFE_FWRITE_TO_STDERR_WITH_FORMAT_STRING(
"FreeLibrary error: %lu\n", GetLastError());
}
return ret;
#endif // _WIN32
}

void *
Expand Down
62 changes: 42 additions & 20 deletions test/test_shared_library.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,17 @@ TEST_F(TestSharedLibrary, basic_load) {

// Check debug name works first because rcutils_load_shared_library should be called on
// non-debug symbol name
ret = rcutils_get_platform_library_name("dummy_shared_library", library_path, 1024, true);
ret = rcutils_get_platform_library_name(
RCUTILS_STRINGIFY(SHARED_LIBRARY_UNDER_TEST), library_path, 1024, true);
ASSERT_EQ(RCUTILS_RET_OK, ret);

ret = rcutils_get_platform_library_name("dummy_shared_library", library_path, 1024, false);
ret = rcutils_get_platform_library_name(
RCUTILS_STRINGIFY(SHARED_LIBRARY_UNDER_TEST), library_path, 1024, false);
ASSERT_EQ(RCUTILS_RET_OK, ret);

// getting shared library
ret = rcutils_load_shared_library(&lib, library_path, rcutils_get_default_allocator());
ASSERT_EQ(RCUTILS_RET_OK, ret);
ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str;
EXPECT_TRUE(rcutils_is_shared_library_loaded(&lib));

// unload shared_library
Expand All @@ -80,7 +82,8 @@ TEST_F(TestSharedLibrary, bad_load) {

TEST_F(TestSharedLibrary, fail_allocator) {
rcutils_ret_t ret;
ret = rcutils_get_platform_library_name("dummy_shared_library", library_path, 1024, false);
ret = rcutils_get_platform_library_name(
RCUTILS_STRINGIFY(SHARED_LIBRARY_UNDER_TEST), library_path, 1024, false);
ASSERT_EQ(RCUTILS_RET_OK, ret);

rcutils_allocator_t failing_allocator = get_failing_allocator();
Expand All @@ -91,46 +94,63 @@ TEST_F(TestSharedLibrary, fail_allocator) {
TEST_F(TestSharedLibrary, load_two_times) {
rcutils_ret_t ret;

ret = rcutils_get_platform_library_name("dummy_shared_library", library_path, 1024, false);
ret = rcutils_get_platform_library_name(
RCUTILS_STRINGIFY(SHARED_LIBRARY_UNDER_TEST), library_path, 1024, false);
ASSERT_EQ(RCUTILS_RET_OK, ret);

// getting shared library
ret = rcutils_load_shared_library(&lib, library_path, rcutils_get_default_allocator());
ASSERT_EQ(RCUTILS_RET_OK, ret);
EXPECT_EQ(RCUTILS_RET_OK, ret);

// getting shared library
ret = rcutils_load_shared_library(&lib, library_path, rcutils_get_default_allocator());
ASSERT_EQ(RCUTILS_RET_OK, ret);
rcutils_shared_library_t clone = rcutils_get_zero_initialized_shared_library();
ret = rcutils_load_shared_library(&clone, library_path, rcutils_get_default_allocator());
EXPECT_EQ(RCUTILS_RET_OK, ret);

// unload shared_library
ret = rcutils_unload_shared_library(&clone);
EXPECT_EQ(RCUTILS_RET_OK, ret);

// unload shared_library
ret = rcutils_unload_shared_library(&lib);
ASSERT_EQ(RCUTILS_RET_OK, ret);
EXPECT_EQ(RCUTILS_RET_OK, ret);
}

TEST_F(TestSharedLibrary, error_load) {
rcutils_ret_t ret;

rcutils_shared_library_t lib_empty;
ret = rcutils_load_shared_library(&lib_empty, NULL, rcutils_get_zero_initialized_allocator());
ASSERT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret);

lib_empty = rcutils_get_zero_initialized_shared_library();
ret = rcutils_load_shared_library(&lib_empty, NULL, rcutils_get_zero_initialized_allocator());
ret = rcutils_load_shared_library(&lib, NULL, rcutils_get_default_allocator());
ASSERT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret);
rcutils_reset_error();

ret = rcutils_get_platform_library_name("dummy_shared_library", library_path, 1024, false);
ret = rcutils_get_platform_library_name(
RCUTILS_STRINGIFY(SHARED_LIBRARY_UNDER_TEST), library_path, 1024, false);
ASSERT_EQ(RCUTILS_RET_OK, ret);

ret = rcutils_load_shared_library(
&lib_empty,
&lib,
library_path, rcutils_get_zero_initialized_allocator());
ASSERT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret);
rcutils_reset_error();

ret = rcutils_load_shared_library(
&lib, library_path, rcutils_get_default_allocator());
ASSERT_EQ(RCUTILS_RET_OK, ret);

ret = rcutils_load_shared_library(
&lib, library_path, rcutils_get_default_allocator());
EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret);
rcutils_reset_error();

ret = rcutils_unload_shared_library(&lib);
EXPECT_EQ(RCUTILS_RET_OK, ret);
}

TEST_F(TestSharedLibrary, error_unload) {
rcutils_ret_t ret;

ret = rcutils_get_platform_library_name("dummy_shared_library", library_path, 1024, false);
ret = rcutils_get_platform_library_name(
RCUTILS_STRINGIFY(SHARED_LIBRARY_UNDER_TEST), library_path, 1024, false);
ASSERT_EQ(RCUTILS_RET_OK, ret);

ret = rcutils_load_shared_library(&lib, library_path, rcutils_get_default_allocator());
Expand All @@ -153,7 +173,8 @@ TEST_F(TestSharedLibrary, error_symbol) {
void * symbol = rcutils_get_symbol(&lib, "print_name");
EXPECT_TRUE(symbol == NULL);

ret = rcutils_get_platform_library_name("dummy_shared_library", library_path, 1024, false);
ret = rcutils_get_platform_library_name(
RCUTILS_STRINGIFY(SHARED_LIBRARY_UNDER_TEST), library_path, 1024, false);
ASSERT_EQ(RCUTILS_RET_OK, ret);
ret = rcutils_load_shared_library(&lib, library_path, rcutils_get_default_allocator());
ASSERT_EQ(RCUTILS_RET_OK, ret);
Expand All @@ -177,7 +198,8 @@ TEST_F(TestSharedLibrary, basic_symbol) {
ret = rcutils_has_symbol(nullptr, "symbol");
EXPECT_FALSE(ret);

ret = rcutils_get_platform_library_name("dummy_shared_library", library_path, 1024, false);
ret = rcutils_get_platform_library_name(
RCUTILS_STRINGIFY(SHARED_LIBRARY_UNDER_TEST), library_path, 1024, false);
ASSERT_EQ(RCUTILS_RET_OK, ret);

// getting shared library
Expand Down