diff --git a/lib/Interpreter/DynamicLibraryManager.cpp b/lib/Interpreter/DynamicLibraryManager.cpp index 6ec0e329b..729b44716 100644 --- a/lib/Interpreter/DynamicLibraryManager.cpp +++ b/lib/Interpreter/DynamicLibraryManager.cpp @@ -365,7 +365,7 @@ namespace Cpp { // TODO: !permanent case std::string errMsg; - DyLibHandle dyLibHandle = platform::DLOpen(canonicalLoadedLib, &errMsg); + DyLibHandle dyLibHandle = platform::DLOpen(canonicalLoadedLib, errMsg); if (!dyLibHandle) { // We emit callback to LibraryLoadingFailed when we get error with error message. //TODO: Implement callbacks @@ -403,7 +403,7 @@ namespace Cpp { // TODO: !permanent case std::string errMsg; - platform::DLClose(dyLibHandle, &errMsg); + platform::DLClose(dyLibHandle, errMsg); if (!errMsg.empty()) { LLVM_DEBUG(dbgs() << "cling::DynamicLibraryManager::unloadLibrary(): " << errMsg << '\n'); diff --git a/lib/Interpreter/Paths.cpp b/lib/Interpreter/Paths.cpp index 1a5abbd27..2286e253b 100644 --- a/lib/Interpreter/Paths.cpp +++ b/lib/Interpreter/Paths.cpp @@ -106,53 +106,38 @@ namespace platform { return std::string(Buffer.str()); } -#if defined(LLVM_ON_UNIX) - static void DLErr(std::string* Err) { - if (!Err) - return; - if (const char* DyLibError = ::dlerror()) - *Err = DyLibError; +#if defined(_WIN32) + void* DLOpen(const std::string& Path, std::string& Err) { + auto lib = llvm::sys::DynamicLibrary::getLibrary(Path.c_str(), &Err); + return lib.getOSSpecificHandle(); } - void* DLOpen(const std::string& Path, std::string* Err /* = nullptr */) { - void* Lib = ::dlopen(Path.c_str(), RTLD_LAZY | RTLD_GLOBAL); - DLErr(Err); - return Lib; + void DLClose(void* Lib, std::string& Err) { + auto dl = llvm::sys::DynamicLibrary(Lib); + llvm::sys::DynamicLibrary::closeLibrary(dl); + if (Err.empty()) { + Err = std::string(); + } + } +#elif defined(LLVM_ON_UNIX) + static void DLErr(std::string& Err) { + if (const char* DyLibError = ::dlerror()) + Err = std::string(DyLibError); } - void* DLSym(const std::string& Name, std::string* Err /* = nullptr*/) { - if (void* Self = ::dlopen(nullptr, RTLD_GLOBAL)) { - // get dlopen error if there is one - DLErr(Err); - void* Sym = ::dlsym(Self, Name.c_str()); - // overwrite error if dlsym caused one + void* DLOpen(const std::string& Path, std::string& Err) { + void* Lib = ::dlopen(Path.c_str(), RTLD_LAZY | RTLD_GLOBAL); + if (Lib == nullptr) { DLErr(Err); - // only get dlclose error if dlopen & dlsym haven't emited one - DLClose(Self, Err && Err->empty() ? Err : nullptr); - return Sym; + return nullptr; } - DLErr(Err); - return nullptr; + return Lib; } - void DLClose(void* Lib, std::string* Err /* = nullptr*/) { + void DLClose(void* Lib, std::string& Err) { ::dlclose(Lib); DLErr(Err); } -#elif defined(_WIN32) - - void* DLOpen(const std::string& Path, std::string* Err /* = nullptr */) { - auto lib = llvm::sys::DynamicLibrary::getLibrary(Path.c_str(), Err); - return lib.getOSSpecificHandle(); - } - - void DLClose(void* Lib, std::string* Err /* = nullptr*/) { - auto dl = llvm::sys::DynamicLibrary(Lib); - llvm::sys::DynamicLibrary::closeLibrary(dl); - if (Err) { - *Err = std::string(); - } - } #endif } // namespace platform diff --git a/lib/Interpreter/Paths.h b/lib/Interpreter/Paths.h index a0df72494..6b8862aea 100644 --- a/lib/Interpreter/Paths.h +++ b/lib/Interpreter/Paths.h @@ -47,9 +47,7 @@ std::string NormalizePath(const std::string& Path); /// /// \returns the library handle /// -void* DLOpen(const std::string& Path, std::string* Err = nullptr); - -void* DLSym(const std::string& Name, std::string* Err = nullptr); +void* DLOpen(const std::string& Path, std::string& Err); ///\brief Close a handle to a shared library. /// @@ -58,7 +56,7 @@ void* DLSym(const std::string& Name, std::string* Err = nullptr); /// /// \returns the library handle /// -void DLClose(void* Lib, std::string* Err = nullptr); +void DLClose(void* Lib, std::string& Err); } // namespace platform ///\brief Replace all $TOKENS in a string with environent variable values. diff --git a/unittests/CppInterOp/CMakeLists.txt b/unittests/CppInterOp/CMakeLists.txt index 75dccf556..9fe465428 100644 --- a/unittests/CppInterOp/CMakeLists.txt +++ b/unittests/CppInterOp/CMakeLists.txt @@ -22,7 +22,7 @@ export_executable_symbols(CppInterOpTests) unset(LLVM_LINK_COMPONENTS) -add_cppinterop_unittest(DynamicLibraryManagerTests DynamicLibraryManagerTest.cpp) +add_cppinterop_unittest(DynamicLibraryManagerTests DynamicLibraryManagerTest.cpp ${CMAKE_SOURCE_DIR}/lib/Interpreter/Paths.cpp) target_link_libraries(DynamicLibraryManagerTests PRIVATE clangCppInterOp diff --git a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp index 01624ef78..f61b62fd6 100644 --- a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp +++ b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp @@ -5,6 +5,8 @@ #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" +#include "../../lib/Interpreter/Paths.h" + // This function isn't referenced outside its translation unit, but it // can't use the "static" keyword because its address is used for // GetMainExecutable (since some platforms don't support taking the @@ -40,6 +42,17 @@ TEST(DynamicLibraryManagerTest, Sanity) { << "Cannot find: '" << PathToTestSharedLib << "' in '" << Dir.str() << "'"; + // DLOPEN DLCLOSE Test + std::string err = ""; + auto* dlopen_handle = Cpp::utils::platform::DLOpen(PathToTestSharedLib, err); + EXPECT_TRUE(dlopen_handle) << "Error occurred: " << err << "\n"; + Cpp::utils::platform::DLClose(dlopen_handle, err); + EXPECT_TRUE(err.empty()) << "Error occurred: " << err << "\n"; + Cpp::utils::platform::DLOpen("missing", err); + EXPECT_TRUE(err.find("no such file") != std::string::npos || + err.find("No such file") != std::string::npos); + // DLOPEN DLCLOSE Test end + EXPECT_TRUE(Cpp::LoadLibrary(PathToTestSharedLib.c_str())); // Force ExecutionEngine to be created. Cpp::Process("");