Refactor DynamicLibrary so searching for a symbol will have a defined…

… order and

libraries are properly unloaded when llvm_shutdown is called.

Summary:
This was mostly affecting usage of the JIT, where storing the library handles in
a set made iteration unordered/undefined. This lead to disagreement between the
JIT and native code as to what the address and implementation of particularly on
Windows with stdlib functions:

JIT: putenv_s("TEST", "VALUE") // called msvcrt.dll, putenv_s
JIT: getenv("TEST") -> "VALUE" // called msvcrt.dll, getenv
Native: getenv("TEST") -> NULL // called ucrt.dll, getenv

Also fixed is the issue of DynamicLibrary::getPermanentLibrary(0,0) on Windows
not giving priority to the process' symbols as it did on Unix.

Reviewers: chapuni, v.g.vassilev, lhames

Reviewed By: lhames

Subscribers: danalbert, srhines, mgorny, vsk, llvm-commits

Differential Revision: https://reviews.llvm.org/D30107

llvm-svn: 301153

llvm/include/llvm/Support/DynamicLibrary.h

@@ -58,7 +58,7 @@ namespace sys {
     void *getAddressOfSymbol(const char *symbolName);
 
     /// This function permanently loads the dynamic library at the given path.
-    /// The library will only be unloaded when the program terminates.
+    /// The library will only be unloaded when llvm_shutdown() is called.
     /// This returns a valid DynamicLibrary instance on success and an invalid
     /// instance on failure (see isValid()). \p *errMsg will only be modified
     /// if the library fails to load.
@@ -71,7 +71,8 @@ namespace sys {
     /// Registers an externally loaded library. The library will be unloaded
     /// when the program terminates.
     ///
-    /// It is safe to call this function multiple times for the same library.
+    /// It is safe to call this function multiple times for the same library,
+    /// though ownership is only taken if there was no error.
     ///
     /// \returns An empty \p DynamicLibrary if the library was already loaded.
     static DynamicLibrary addPermanentLibrary(void *handle,
@@ -106,6 +107,8 @@ namespace sys {
     /// libraries.
     /// @brief Add searchable symbol/value pair.
     static void AddSymbol(StringRef symbolName, void *symbolValue);
+
+    class HandleSet;
   };
 
 } // End sys namespace

llvm/lib/Support/CMakeLists.txt

@@ -130,7 +130,6 @@ add_llvm_library(LLVMSupport
   Process.cpp
   Program.cpp
   RWMutex.cpp
-  SearchForAddressOfSpecialSymbol.cpp
   Signals.cpp
   TargetRegistry.cpp
   ThreadLocal.cpp

llvm/lib/Support/DynamicLibrary.cpp

@@ -20,169 +20,164 @@
 #include "llvm/Support/Mutex.h"
 #include <cstdio>
 #include <cstring>
+#include <vector>
 
-// Collection of symbol name/value pairs to be searched prior to any libraries.
-static llvm::ManagedStatic<llvm::StringMap<void *> > ExplicitSymbols;
-static llvm::ManagedStatic<llvm::sys::SmartMutex<true> > SymbolsMutex;
-
-void llvm::sys::DynamicLibrary::AddSymbol(StringRef symbolName,
-                                          void *symbolValue) {
-  SmartScopedLock<true> lock(*SymbolsMutex);
-  (*ExplicitSymbols)[symbolName] = symbolValue;
-}
-
-char llvm::sys::DynamicLibrary::Invalid = 0;
-
-#ifdef LLVM_ON_WIN32
-
-#include "Windows/DynamicLibrary.inc"
-
-#else
-
-#if defined(HAVE_DLFCN_H) && defined(HAVE_DLOPEN)
-#include <dlfcn.h>
 using namespace llvm;
 using namespace llvm::sys;
 
-//===----------------------------------------------------------------------===//
-//=== WARNING: Implementation here must contain only TRULY operating system
-//===          independent code.
-//===----------------------------------------------------------------------===//
+// All methods for HandleSet should be used holding SymbolsMutex.
+class DynamicLibrary::HandleSet {
+  typedef std::vector<void *> HandleList;
+  HandleList Handles;
+  void *Process;
 
-static llvm::ManagedStatic<DenseSet<void *> > OpenedHandles;
+public:
+  static void *DLOpen(const char *Filename, std::string *Err);
+  static void DLClose(void *Handle);
+  static void *DLSym(void *Handle, const char *Symbol);
 
-DynamicLibrary DynamicLibrary::getPermanentLibrary(const char *filename,
-                                                   std::string *errMsg) {
-  SmartScopedLock<true> lock(*SymbolsMutex);
+  HandleSet() : Process(nullptr) {}
+  ~HandleSet();
 
-  void *handle = dlopen(filename, RTLD_LAZY|RTLD_GLOBAL);
-  if (!handle) {
-    if (errMsg) *errMsg = dlerror();
-    return DynamicLibrary();
+  HandleList::iterator Find(void *Handle) {
+    return std::find(Handles.begin(), Handles.end(), Handle);
   }
 
-#ifdef __CYGWIN__
-  // Cygwin searches symbols only in the main
-  // with the handle of dlopen(NULL, RTLD_GLOBAL).
-  if (!filename)
-    handle = RTLD_DEFAULT;
-#endif
+  bool Contains(void *Handle) {
+    return Handle == Process || Find(Handle) != Handles.end();
+  }
 
-  // If we've already loaded this library, dlclose() the handle in order to
-  // keep the internal refcount at +1.
-  if (!OpenedHandles->insert(handle).second)
-    dlclose(handle);
+  bool AddLibrary(void *Handle, bool IsProcess = false, bool CanClose = true) {
+#ifdef LLVM_ON_WIN32
+    assert((Handle == this ? IsProcess : !IsProcess) && "Bad Handle.");
+#endif
 
-  return DynamicLibrary(handle);
-}
+    if (LLVM_LIKELY(!IsProcess)) {
+      if (Find(Handle) != Handles.end()) {
+        if (CanClose)
+          DLClose(Handle);
+        return false;
+      }
+      Handles.push_back(Handle);
+    } else {
+#ifndef LLVM_ON_WIN32
+      if (Process) {
+        if (CanClose)
+          DLClose(Process);
+        if (Process == Handle)
+          return false;
+      }
+#endif
+      Process = Handle;
+    }
+    return true;
+  }
 
-DynamicLibrary DynamicLibrary::addPermanentLibrary(void *handle,
-                                                   std::string *errMsg) {
-  SmartScopedLock<true> lock(*SymbolsMutex);
-  // If we've already loaded this library, tell the caller.
-  if (!OpenedHandles->insert(handle).second) {
-    if (errMsg) *errMsg = "Library already loaded";
-    return DynamicLibrary();
+  void *Lookup(const char *Symbol) {
+    // Process handle gets first try.
+    if (Process) {
+      if (void *Ptr = DLSym(Process, Symbol))
+        return Ptr;
+#ifndef NDEBUG
+      for (void *Handle : Handles)
+        assert(!DLSym(Handle, Symbol) && "Symbol exists in non process handle");
+#endif
+    } else {
+      // Iterate in reverse, so newer libraries/symbols override older.
+      for (auto &&I = Handles.rbegin(), E = Handles.rend(); I != E; ++I) {
+        if (void *Ptr = DLSym(*I, Symbol))
+          return Ptr;
+      }
+    }
+    return nullptr;
   }
+};
 
-  return DynamicLibrary(handle);
+namespace {
+// 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;
 }
 
-void *DynamicLibrary::getAddressOfSymbol(const char *symbolName) {
-  if (!isValid())
-    return nullptr;
-  return dlsym(Data, symbolName);
-}
+#ifdef LLVM_ON_WIN32
+
+#include "Windows/DynamicLibrary.inc"
 
 #else
 
-using namespace llvm;
-using namespace llvm::sys;
+#include "Unix/DynamicLibrary.inc"
+
+#endif
 
-DynamicLibrary DynamicLibrary::getPermanentLibrary(const char *filename,
-                                                   std::string *errMsg) {
-  if (errMsg) *errMsg = "dlopen() not supported on this platform";
-  return DynamicLibrary();
+char DynamicLibrary::Invalid;
+
+namespace llvm {
+void *SearchForAddressOfSpecialSymbol(const char *SymbolName) {
+  return DoSearch(SymbolName); // DynamicLibrary.inc
+}
 }
 
-void *DynamicLibrary::getAddressOfSymbol(const char *symbolName) {
-  return NULL;
+void DynamicLibrary::AddSymbol(StringRef SymbolName, void *SymbolValue) {
+  SmartScopedLock<true> Lock(*SymbolsMutex);
+  (*ExplicitSymbols)[SymbolName] = SymbolValue;
 }
 
-#endif
+DynamicLibrary DynamicLibrary::getPermanentLibrary(const char *FileName,
+                                                   std::string *Err) {
+  SmartScopedLock<true> Lock(*SymbolsMutex);
+  void *Handle = HandleSet::DLOpen(FileName, Err);
+  if (Handle != &Invalid)
+    OpenedHandles->AddLibrary(Handle, /*IsProcess*/ FileName == nullptr);
 
-namespace llvm {
-void *SearchForAddressOfSpecialSymbol(const char* symbolName);
+  return DynamicLibrary(Handle);
 }
 
-void* DynamicLibrary::SearchForAddressOfSymbol(const char *symbolName) {
+DynamicLibrary DynamicLibrary::addPermanentLibrary(void *Handle,
+                                                   std::string *Err) {
   SmartScopedLock<true> Lock(*SymbolsMutex);
+  // If we've already loaded this library, tell the caller.
+  if (!OpenedHandles->AddLibrary(Handle, /*IsProcess*/false, /*CanClose*/false))
+    *Err = "Library already loaded";
 
-  // First check symbols added via AddSymbol().
-  if (ExplicitSymbols.isConstructed()) {
-    StringMap<void *>::iterator i = ExplicitSymbols->find(symbolName);
+  return DynamicLibrary(Handle);
+}
 
-    if (i != ExplicitSymbols->end())
-      return i->second;
-  }
+void *DynamicLibrary::getAddressOfSymbol(const char *SymbolName) {
+  if (!isValid())
+    return nullptr;
+  return HandleSet::DLSym(Data, SymbolName);
+}
 
-#if defined(HAVE_DLFCN_H) && defined(HAVE_DLOPEN)
-  // Now search the libraries.
-  if (OpenedHandles.isConstructed()) {
-    for (DenseSet<void *>::iterator I = OpenedHandles->begin(),
-         E = OpenedHandles->end(); I != E; ++I) {
-      //lt_ptr ptr = lt_dlsym(*I, symbolName);
-      void *ptr = dlsym(*I, symbolName);
-      if (ptr) {
-        return ptr;
-      }
-    }
-  }
-#endif
+void *DynamicLibrary::SearchForAddressOfSymbol(const char *SymbolName) {
+  {
+    SmartScopedLock<true> Lock(*SymbolsMutex);
 
-  if (void *Result = llvm::SearchForAddressOfSpecialSymbol(symbolName))
-    return Result;
+    // First check symbols added via AddSymbol().
+    if (ExplicitSymbols.isConstructed()) {
+      StringMap<void *>::iterator i = ExplicitSymbols->find(SymbolName);
 
-// This macro returns the address of a well-known, explicit symbol
-#define EXPLICIT_SYMBOL(SYM) \
-   if (!strcmp(symbolName, #SYM)) return &SYM
+      if (i != ExplicitSymbols->end())
+        return i->second;
+    }
 
-// On linux we have a weird situation. The stderr/out/in symbols are both
-// macros and global variables because of standards requirements. So, we
-// boldly use the EXPLICIT_SYMBOL macro without checking for a #define first.
-#if defined(__linux__) and !defined(__ANDROID__)
-  {
-    EXPLICIT_SYMBOL(stderr);
-    EXPLICIT_SYMBOL(stdout);
-    EXPLICIT_SYMBOL(stdin);
-  }
-#else
-  // For everything else, we want to check to make sure the symbol isn't defined
-  // as a macro before using EXPLICIT_SYMBOL.
-  {
-#ifndef stdin
-    EXPLICIT_SYMBOL(stdin);
-#endif
-#ifndef stdout
-    EXPLICIT_SYMBOL(stdout);
-#endif
-#ifndef stderr
-    EXPLICIT_SYMBOL(stderr);
-#endif
+    // Now search the libraries.
+    if (OpenedHandles.isConstructed()) {
+      if (void *Ptr = OpenedHandles->Lookup(SymbolName))
+        return Ptr;
+    }
   }
-#endif
-#undef EXPLICIT_SYMBOL
 
-  return nullptr;
+  return llvm::SearchForAddressOfSpecialSymbol(SymbolName);
 }
 
-#endif // LLVM_ON_WIN32
-
 //===----------------------------------------------------------------------===//
 // C API.
 //===----------------------------------------------------------------------===//
 
-LLVMBool LLVMLoadLibraryPermanently(const char* Filename) {
+LLVMBool LLVMLoadLibraryPermanently(const char *Filename) {
   return llvm::sys::DynamicLibrary::LoadLibraryPermanently(Filename);
 }