Close DynamicLibraries in reverse order they were opened.

[marsupial]

Summary

Matches C++ destruction ordering better and fixes possible problems of loaded libraries having inter-dependencies.

[efriedma]

Needs a testcase, I think. Otherwise, looks fine.

[marsupial]

Add test case.

[efriedma]

LGTM.

lib/Support/Unix/DynamicLibrary.inc

 // This file provides the UNIX specific implementation of DynamicLibrary.
 //
 //===----------------------------------------------------------------------===//
 
 #if defined(HAVE_DLFCN_H) && defined(HAVE_DLOPEN)
 #include <dlfcn.h>
 
 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);
 
   // llvm_shutdown called, Return to default
   DynamicLibrary::SearchOrder = DynamicLibrary::SO_Linker;
 }
 

lib/Support/Windows/DynamicLibrary.inc

 
 //===----------------------------------------------------------------------===//
 //=== WARNING: Implementation here must contain only Win32 specific code
 //===          and must not be UNIX code.
 //===----------------------------------------------------------------------===//
 
 
 DynamicLibrary::HandleSet::~HandleSet() {
-  for (void *Handle : Handles)
+  for (void *Handle : llvm::reverse(Handles))
     FreeLibrary(HMODULE(Handle));
 
   // 'Process' should not be released on Windows.
   assert((!Process || Process==this) && "Bad Handle");
   // llvm_shutdown called, Return to default
   DynamicLibrary::SearchOrder = DynamicLibrary::SO_Linker;
 }
 

unittests/Support/DynamicLibrary/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS Support)
 
 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
+  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
+  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)

unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp

 #include "llvm/Config/config.h"
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "gtest/gtest.h"
 
 #include "PipSqueak.h"
-#include <string>
 
 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<testing::internal::string>& 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<std::string> &V);
 typedef const char *(*GetString)();
 
 template <class T> static T FuncPtr(void *Ptr) {
   union {
     T F;
     void *P;
   } Tmp;
   Tmp.P = Ptr;
TEST(DynamicLibrary, Overload) {
   EXPECT_TRUE(FuncPtr<GetString>(DynamicLibrary::SearchForAddressOfSymbol(
                   "TestA")) == nullptr);
 
   // Check serach ordering is reset to default after call to llvm_shutdown
   EXPECT_TRUE(DynamicLibrary::SearchOrder == DynamicLibrary::SO_Linker);
 }
 
 TEST(DynamicLibrary, Shutdown) {
-  std::string A, B;
+  std::string A("PipSqueak"), B, C("SecondLib");
+  std::vector<std::string> 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>(
+    SetStrings SS_0 = FuncPtr<SetStrings>(
+        DynamicLibrary::SearchForAddressOfSymbol("SetStrings"));
+    EXPECT_TRUE(SS_0 != nullptr);
+
+    SS_0(A, B);
+    EXPECT_EQ(B, "Local::Local(PipSqueak)");
+
+    TestOrder TO_0 = FuncPtr<TestOrder>(
+        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<SetStrings>(
         DynamicLibrary::SearchForAddressOfSymbol("SetStrings"));
-    EXPECT_TRUE(SS != nullptr);
+    EXPECT_TRUE(SS_1 != nullptr);
+    EXPECT_TRUE(SS_0 != SS_1);
 
-    SS(A, B);
-    EXPECT_EQ(B, "Local::Local");
+    TestOrder TO_1 = FuncPtr<TestOrder>(
+        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<SetStrings>(DynamicLibrary::SearchForAddressOfSymbol(
                   "SetStrings")) == nullptr);
+
+  // Test unload/destruction ordering
+  EXPECT_EQ(Order.size(), 2UL);
+  EXPECT_EQ(Order.front(), "SecondLib");
+  EXPECT_EQ(Order.back(), "PipSqueak");
 }
 
 #else
 
 TEST(DynamicLibrary, Unsupported) {
   std::string Err;
   DynamicLibrary DL =
       DynamicLibrary::getPermanentLibrary(LibPath().c_str(), &Err);
   EXPECT_FALSE(DL.isValid());
   EXPECT_EQ(Err, "dlopen() not supported on this platform");
 }
 
 #endif

unittests/Support/DynamicLibrary/PipSqueak.h

 //===- llvm/unittest/Support/DynamicLibrary/PipSqueak.h -------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #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 <string>
+#include <vector>
+#pragma warning(pop)
+#else
+#include <string>
+#include <vector>
+#endif
+
 #ifdef _WIN32
 #define PIPSQUEAK_EXPORT __declspec(dllexport)
 #else
 #define PIPSQUEAK_EXPORT
 #endif
 
 #endif

unittests/Support/DynamicLibrary/PipSqueak.cxx

 //===- llvm/unittest/Support/DynamicLibrary/PipSqueak.cxx -----------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #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 <string>
-#pragma warning(pop)
-#else
-#include <string>
-#endif
-
 struct Global {
   std::string *Str;
-  Global() : Str(nullptr) {}
+  std::vector<std::string> *Vec;
+  Global() : Str(nullptr), Vec(nullptr) {}
   ~Global() {
-    if (Str)
+    if (Str) {
+      if (Vec)
+        Vec->push_back(*Str);
       *Str = "Global::~Global";
     }
+  }
 };
 
+static Global Glb;
+
 struct Local {
   std::string &Str;
-  Local(std::string &S) : Str(S) { Str = "Local::Local"; }
+  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"; }
 };
 
-static Global Glb;
 
 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<std::string> &V) {
+  Glb.Vec = &V;
 }
 
 extern "C" PIPSQUEAK_EXPORT const char *TestA() { return "LibCall"; }