From ab9fa4a528991f34ac044f1742e459d17f95f247 Mon Sep 17 00:00:00 2001 From: Frederich Munch Date: Mon, 5 Jun 2017 16:26:58 +0000 Subject: [PATCH] Close DynamicLibraries in reverse order they were opened. Summary: Matches C++ destruction ordering better and fixes possible problems of loaded libraries having inter-dependencies. Reviewers: efriedma, v.g.vassilev, chapuni Reviewed By: efriedma Subscribers: mgorny, llvm-commits Differential Revision: https://reviews.llvm.org/D33652 llvm-svn: 304720 --- lib/Support/Unix/DynamicLibrary.inc | 3 +- lib/Support/Windows/DynamicLibrary.inc | 2 +- .../Support/DynamicLibrary/CMakeLists.txt | 25 +++++---- .../DynamicLibrary/DynamicLibraryTest.cpp | 51 +++++++++++++++---- .../Support/DynamicLibrary/PipSqueak.cxx | 42 +++++++-------- unittests/Support/DynamicLibrary/PipSqueak.h | 13 +++++ 6 files changed, 95 insertions(+), 41 deletions(-) diff --git a/lib/Support/Unix/DynamicLibrary.inc b/lib/Support/Unix/DynamicLibrary.inc index 30972ffd0ef..aad77f19c35 100644 --- a/lib/Support/Unix/DynamicLibrary.inc +++ b/lib/Support/Unix/DynamicLibrary.inc @@ -15,7 +15,8 @@ #include DynamicLibrary::HandleSet::~HandleSet() { - for (void *Handle : Handles) + // Close the libraries in reverse order. + for (void *Handle : llvm::reverse(Handles)) ::dlclose(Handle); if (Process) ::dlclose(Process); diff --git a/lib/Support/Windows/DynamicLibrary.inc b/lib/Support/Windows/DynamicLibrary.inc index 0b54b5dfdbc..caf1a0a658d 100644 --- a/lib/Support/Windows/DynamicLibrary.inc +++ b/lib/Support/Windows/DynamicLibrary.inc @@ -23,7 +23,7 @@ DynamicLibrary::HandleSet::~HandleSet() { - for (void *Handle : Handles) + for (void *Handle : llvm::reverse(Handles)) FreeLibrary(HMODULE(Handle)); // 'Process' should not be released on Windows. diff --git a/unittests/Support/DynamicLibrary/CMakeLists.txt b/unittests/Support/DynamicLibrary/CMakeLists.txt index f0e945e78b1..2fa4bf237d4 100644 --- a/unittests/Support/DynamicLibrary/CMakeLists.txt +++ b/unittests/Support/DynamicLibrary/CMakeLists.txt @@ -4,16 +4,21 @@ add_llvm_unittest(DynamicLibraryTests DynamicLibraryTest.cpp) export_executable_symbols(DynamicLibraryTests) -add_library(PipSqueak SHARED PipSqueak.cxx) +function(dynlib_add_module NAME) + add_library(${NAME} SHARED PipSqueak.cxx) -set_output_directory(PipSqueak - BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR} - LIBRARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR} - ) + set_output_directory(${NAME} + BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR} + LIBRARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR} + ) -set_target_properties(PipSqueak - PROPERTIES PREFIX "" - SUFFIX ".so" - ) + set_target_properties(${NAME} + PROPERTIES PREFIX "" + SUFFIX ".so" + ) -add_dependencies(DynamicLibraryTests PipSqueak) + add_dependencies(DynamicLibraryTests ${NAME}) +endfunction(dynlib_add_module) + +dynlib_add_module(PipSqueak) +dynlib_add_module(SecondLib) diff --git a/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp b/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp index 0674a91282a..8388375267d 100644 --- a/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp +++ b/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp @@ -15,26 +15,26 @@ #include "gtest/gtest.h" #include "PipSqueak.h" -#include using namespace llvm; using namespace llvm::sys; extern "C" PIPSQUEAK_EXPORT const char *TestA() { return "ProcessCall"; } -std::string LibPath() { +std::string LibPath(const std::string Name = "PipSqueak") { const std::vector& Argvs = testing::internal::GetArgvs(); const char *Argv0 = Argvs.size() > 0 ? Argvs[0].c_str() : "DynamicLibraryTests"; void *Ptr = (void*)(intptr_t)TestA; std::string Path = fs::getMainExecutable(Argv0, Ptr); llvm::SmallString<256> Buf(path::parent_path(Path)); - path::append(Buf, "PipSqueak.so"); + path::append(Buf, (Name+".so").c_str()); return Buf.str(); } #if defined(_WIN32) || (defined(HAVE_DLFCN_H) && defined(HAVE_DLOPEN)) typedef void (*SetStrings)(std::string &GStr, std::string &LStr); +typedef void (*TestOrder)(std::vector &V); typedef const char *(*GetString)(); template static T FuncPtr(void *Ptr) { @@ -100,26 +100,59 @@ TEST(DynamicLibrary, Overload) { } TEST(DynamicLibrary, Shutdown) { - std::string A, B; + std::string A("PipSqueak"), B, C("SecondLib"); + std::vector Order; { std::string Err; llvm_shutdown_obj Shutdown; DynamicLibrary DL = - DynamicLibrary::getPermanentLibrary(LibPath().c_str(), &Err); + DynamicLibrary::getPermanentLibrary(LibPath(A).c_str(), &Err); EXPECT_TRUE(DL.isValid()); EXPECT_TRUE(Err.empty()); - SetStrings SS = FuncPtr( + SetStrings SS_0 = FuncPtr( DynamicLibrary::SearchForAddressOfSymbol("SetStrings")); - EXPECT_TRUE(SS != nullptr); + EXPECT_TRUE(SS_0 != nullptr); - SS(A, B); - EXPECT_EQ(B, "Local::Local"); + SS_0(A, B); + EXPECT_EQ(B, "Local::Local(PipSqueak)"); + + TestOrder TO_0 = FuncPtr( + DynamicLibrary::SearchForAddressOfSymbol("TestOrder")); + EXPECT_TRUE(TO_0 != nullptr); + + DynamicLibrary DL2 = + DynamicLibrary::getPermanentLibrary(LibPath(C).c_str(), &Err); + EXPECT_TRUE(DL2.isValid()); + EXPECT_TRUE(Err.empty()); + + // Should find latest version of symbols in SecondLib + SetStrings SS_1 = FuncPtr( + DynamicLibrary::SearchForAddressOfSymbol("SetStrings")); + EXPECT_TRUE(SS_1 != nullptr); + EXPECT_TRUE(SS_0 != SS_1); + + TestOrder TO_1 = FuncPtr( + DynamicLibrary::SearchForAddressOfSymbol("TestOrder")); + EXPECT_TRUE(TO_1 != nullptr); + EXPECT_TRUE(TO_0 != TO_1); + + B.clear(); + SS_1(C, B); + EXPECT_EQ(B, "Local::Local(SecondLib)"); + + TO_0(Order); + TO_1(Order); } EXPECT_EQ(A, "Global::~Global"); EXPECT_EQ(B, "Local::~Local"); EXPECT_TRUE(FuncPtr(DynamicLibrary::SearchForAddressOfSymbol( "SetStrings")) == nullptr); + + // Test unload/destruction ordering + EXPECT_EQ(Order.size(), 2UL); + EXPECT_EQ(Order.front(), "SecondLib"); + EXPECT_EQ(Order.back(), "PipSqueak"); } #else diff --git a/unittests/Support/DynamicLibrary/PipSqueak.cxx b/unittests/Support/DynamicLibrary/PipSqueak.cxx index d1cf7c042b7..79cf59255a4 100644 --- a/unittests/Support/DynamicLibrary/PipSqueak.cxx +++ b/unittests/Support/DynamicLibrary/PipSqueak.cxx @@ -9,38 +9,40 @@ #include "PipSqueak.h" -#if defined(_WIN32) && !defined(__GNUC__) -// Disable warnings from inclusion of xlocale & exception -#pragma warning(push) -#pragma warning(disable: 4530) -#pragma warning(disable: 4577) -#include -#pragma warning(pop) -#else -#include -#endif - struct Global { std::string *Str; - Global() : Str(nullptr) {} + std::vector *Vec; + Global() : Str(nullptr), Vec(nullptr) {} ~Global() { - if (Str) + if (Str) { + if (Vec) + Vec->push_back(*Str); *Str = "Global::~Global"; + } } }; -struct Local { - std::string &Str; - Local(std::string &S) : Str(S) { Str = "Local::Local"; } - ~Local() { Str = "Local::~Local"; } -}; - static Global Glb; +struct Local { + std::string &Str; + Local(std::string &S) : Str(S) { + Str = "Local::Local"; + if (Glb.Str && !Glb.Str->empty()) + Str += std::string("(") + *Glb.Str + std::string(")"); + } + ~Local() { Str = "Local::~Local"; } +}; + + extern "C" PIPSQUEAK_EXPORT void SetStrings(std::string &GStr, std::string &LStr) { - static Local Lcl(LStr); Glb.Str = &GStr; + static Local Lcl(LStr); +} + +extern "C" PIPSQUEAK_EXPORT void TestOrder(std::vector &V) { + Glb.Vec = &V; } extern "C" PIPSQUEAK_EXPORT const char *TestA() { return "LibCall"; } diff --git a/unittests/Support/DynamicLibrary/PipSqueak.h b/unittests/Support/DynamicLibrary/PipSqueak.h index e6a859d6071..3e4f79a9a6f 100644 --- a/unittests/Support/DynamicLibrary/PipSqueak.h +++ b/unittests/Support/DynamicLibrary/PipSqueak.h @@ -10,6 +10,19 @@ #ifndef LLVM_PIPSQUEAK_H #define LLVM_PIPSQUEAK_H +#if defined(_WIN32) && !defined(__GNUC__) +// Disable warnings from inclusion of xlocale & exception +#pragma warning(push) +#pragma warning(disable: 4530) +#pragma warning(disable: 4577) +#include +#include +#pragma warning(pop) +#else +#include +#include +#endif + #ifdef _WIN32 #define PIPSQUEAK_EXPORT __declspec(dllexport) #else