[llvm] 5c7c838 - Revert "ManagedStatic: remove from DynamicLibrary"

Nicolai Hähnle via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 3 08:56:58 PDT 2022


Author: Nicolai Hähnle
Date: 2022-08-03T17:56:46+02:00
New Revision: 5c7c83885fbb0909e972158240e167849ae1c6c0

URL: https://github.com/llvm/llvm-project/commit/5c7c83885fbb0909e972158240e167849ae1c6c0
DIFF: https://github.com/llvm/llvm-project/commit/5c7c83885fbb0909e972158240e167849ae1c6c0.diff

LOG: Revert "ManagedStatic: remove from DynamicLibrary"

This reverts commit 38817af6a7271aee0581c56eaea44b8c4cf5ecc7.

Buildbots report a Windows build error. Revert until I can look at it
more carefully.

Added: 
    

Modified: 
    llvm/lib/Support/DynamicLibrary.cpp
    llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Support/DynamicLibrary.cpp b/llvm/lib/Support/DynamicLibrary.cpp
index 319fbc59d43b..7b9d7abe7545 100644
--- a/llvm/lib/Support/DynamicLibrary.cpp
+++ b/llvm/lib/Support/DynamicLibrary.cpp
@@ -15,6 +15,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Config/config.h"
+#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Mutex.h"
 #include <vector>
 
@@ -106,22 +107,12 @@ class DynamicLibrary::HandleSet {
 };
 
 namespace {
-
-struct Globals {
-  // Collection of symbol name/value pairs to be searched prior to any
-  // libraries.
-  llvm::StringMap<void *> ExplicitSymbols;
-  // Collection of known library handles.
-  DynamicLibrary::HandleSet OpenedHandles;
-  // Lock for ExplicitSymbols and OpenedHandles.
-  llvm::sys::SmartMutex<true> SymbolsMutex;
-};
-
-Globals &getGlobals() {
-  static Globals G;
-  return G;
-}
-
+// Collection of symbol name/value pairs to be searched prior to any libraries.
+static llvm::ManagedStatic<llvm::StringMap<void *>> ExplicitSymbols;
+// Collection of known library handles.
+static llvm::ManagedStatic<DynamicLibrary::HandleSet> OpenedHandles;
+// Lock for ExplicitSymbols and OpenedHandles.
+static llvm::ManagedStatic<llvm::sys::SmartMutex<true>> SymbolsMutex;
 } // namespace
 
 #ifdef _WIN32
@@ -145,18 +136,20 @@ void *SearchForAddressOfSpecialSymbol(const char *SymbolName) {
 } // namespace llvm
 
 void DynamicLibrary::AddSymbol(StringRef SymbolName, void *SymbolValue) {
-  auto &G = getGlobals();
-  SmartScopedLock<true> Lock(G.SymbolsMutex);
-  G.ExplicitSymbols[SymbolName] = SymbolValue;
+  SmartScopedLock<true> Lock(*SymbolsMutex);
+  (*ExplicitSymbols)[SymbolName] = SymbolValue;
 }
 
 DynamicLibrary DynamicLibrary::getPermanentLibrary(const char *FileName,
                                                    std::string *Err) {
-  auto &G = getGlobals();
+  // Force OpenedHandles to be added into the ManagedStatic list before any
+  // ManagedStatic can be added from static constructors in HandleSet::DLOpen.
+  HandleSet& HS = *OpenedHandles;
+
   void *Handle = HandleSet::DLOpen(FileName, Err);
   if (Handle != &Invalid) {
-    SmartScopedLock<true> Lock(G.SymbolsMutex);
-    G.OpenedHandles.AddLibrary(Handle, /*IsProcess*/ FileName == nullptr);
+    SmartScopedLock<true> Lock(*SymbolsMutex);
+    HS.AddLibrary(Handle, /*IsProcess*/ FileName == nullptr);
   }
 
   return DynamicLibrary(Handle);
@@ -164,11 +157,9 @@ DynamicLibrary DynamicLibrary::getPermanentLibrary(const char *FileName,
 
 DynamicLibrary DynamicLibrary::addPermanentLibrary(void *Handle,
                                                    std::string *Err) {
-  auto &G = getGlobals();
-  SmartScopedLock<true> Lock(G.SymbolsMutex);
+  SmartScopedLock<true> Lock(*SymbolsMutex);
   // If we've already loaded this library, tell the caller.
-  if (!G.OpenedHandles.AddLibrary(Handle, /*IsProcess*/ false,
-                                  /*CanClose*/ false))
+  if (!OpenedHandles->AddLibrary(Handle, /*IsProcess*/false, /*CanClose*/false))
     *Err = "Library already loaded";
 
   return DynamicLibrary(Handle);
@@ -182,18 +173,21 @@ void *DynamicLibrary::getAddressOfSymbol(const char *SymbolName) {
 
 void *DynamicLibrary::SearchForAddressOfSymbol(const char *SymbolName) {
   {
-    auto &G = getGlobals();
-    SmartScopedLock<true> Lock(G.SymbolsMutex);
+    SmartScopedLock<true> Lock(*SymbolsMutex);
 
     // First check symbols added via AddSymbol().
-    StringMap<void *>::iterator i = G.ExplicitSymbols.find(SymbolName);
+    if (ExplicitSymbols.isConstructed()) {
+      StringMap<void *>::iterator i = ExplicitSymbols->find(SymbolName);
 
-    if (i != G.ExplicitSymbols.end())
-      return i->second;
+      if (i != ExplicitSymbols->end())
+        return i->second;
+    }
 
     // Now search the libraries.
-    if (void *Ptr = G.OpenedHandles.Lookup(SymbolName, SearchOrder))
-      return Ptr;
+    if (OpenedHandles.isConstructed()) {
+      if (void *Ptr = OpenedHandles->Lookup(SymbolName, SearchOrder))
+        return Ptr;
+    }
   }
 
   return llvm::SearchForAddressOfSpecialSymbol(SymbolName);

diff  --git a/llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp b/llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
index 74de1455a996..784b9c1bb2d2 100644
--- a/llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
+++ b/llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
@@ -9,6 +9,7 @@
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Config/config.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "gtest/gtest.h"
 
@@ -58,6 +59,7 @@ std::string StdString(const char *Ptr) { return Ptr ? Ptr : ""; }
 TEST(DynamicLibrary, Overload) {
   {
     std::string Err;
+    llvm_shutdown_obj Shutdown;
     DynamicLibrary DL =
         DynamicLibrary::getPermanentLibrary(LibPath().c_str(), &Err);
     EXPECT_TRUE(DL.isValid());
@@ -105,6 +107,68 @@ TEST(DynamicLibrary, Overload) {
     EXPECT_EQ(GS, &OverloadTestA);
     EXPECT_EQ(StdString(GS()), "OverloadCall");
   }
+  EXPECT_TRUE(FuncPtr<GetString>(DynamicLibrary::SearchForAddressOfSymbol(
+                  "TestA")) == nullptr);
+
+  // Check serach ordering is reset to default after call to llvm_shutdown
+  EXPECT_EQ(DynamicLibrary::SearchOrder, DynamicLibrary::SO_Linker);
+}
+
+TEST(DynamicLibrary, Shutdown) {
+  std::string A("PipSqueak"), B, C("SecondLib");
+  std::vector<std::string> Order;
+  {
+    std::string Err;
+    llvm_shutdown_obj Shutdown;
+    DynamicLibrary DL =
+        DynamicLibrary::getPermanentLibrary(LibPath(A).c_str(), &Err);
+    EXPECT_TRUE(DL.isValid());
+    EXPECT_TRUE(Err.empty());
+
+    SetStrings SS_0 = FuncPtr<SetStrings>(
+        DynamicLibrary::SearchForAddressOfSymbol("SetStrings"));
+    EXPECT_NE(SS_0, nullptr);
+
+    SS_0(A, B);
+    EXPECT_EQ(B, "Local::Local(PipSqueak)");
+
+    TestOrder TO_0 = FuncPtr<TestOrder>(
+        DynamicLibrary::SearchForAddressOfSymbol("TestOrder"));
+    EXPECT_NE(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_NE(SS_1, nullptr);
+    EXPECT_NE(SS_0, SS_1);
+
+    TestOrder TO_1 = FuncPtr<TestOrder>(
+        DynamicLibrary::SearchForAddressOfSymbol("TestOrder"));
+    EXPECT_NE(TO_1, nullptr);
+    EXPECT_NE(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_EQ(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


        


More information about the llvm-commits mailing list