From a9b7d60d67b054541cb8507d697eb94beee32f18 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Tue, 7 Jul 2009 18:17:07 +0000 Subject: [PATCH] Eliminate the static constructors and locks from DynamicLibrary.cpp. This fixes PR4512 and eliminating static ctors is always good. Losing thread safety is unfortunate, but the code is just incredibly poorly designed. If someone is interested, the "right" solution is to split DynamicLibrary.cpp into two separate pieces: a stateless piece in libsystem, and a simple support file in libsupport that has the "state" (e.g. AddSymbol) in managed static objects. Doing this would both fix memory leaks we already have, as well as make the code thread safe again. it would also make sense to move all the unix specific code in System/DynamicLibrary.cpp into System/Unix/DynamicLibrary.inc. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@74927 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/System/DynamicLibrary.h | 15 ++++++++ lib/System/DynamicLibrary.cpp | 53 ++++++++++++++-------------- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/include/llvm/System/DynamicLibrary.h b/include/llvm/System/DynamicLibrary.h index f04734a6f9b..ac58407a194 100644 --- a/include/llvm/System/DynamicLibrary.h +++ b/include/llvm/System/DynamicLibrary.h @@ -36,6 +36,9 @@ namespace sys { /// and will only be unloaded when the program terminates. This returns /// false on success or returns true and fills in *ErrMsg on failure. /// @brief Open a dynamic library permanently. + /// + /// NOTE: This function is not thread safe. + /// static bool LoadLibraryPermanently(const char *filename, std::string *ErrMsg = 0); @@ -46,9 +49,15 @@ namespace sys { /// as ephemerally loaded libraries (constructors). /// @throws std::string on error. /// @brief Search through libraries for address of a symbol + /// + /// NOTE: This function is not thread safe. + /// static void *SearchForAddressOfSymbol(const char *symbolName); /// @brief Convenience function for C++ophiles. + /// + /// NOTE: This function is not thread safe. + /// static void *SearchForAddressOfSymbol(const std::string &symbolName) { return SearchForAddressOfSymbol(symbolName.c_str()); } @@ -57,9 +66,15 @@ namespace sys { /// value \p symbolValue. These symbols are searched before any /// libraries. /// @brief Add searchable symbol/value pair. + /// + /// NOTE: This function is not thread safe. + /// static void AddSymbol(const char *symbolName, void *symbolValue); /// @brief Convenience function for C++ophiles. + /// + /// NOTE: This function is not thread safe. + /// static void AddSymbol(const std::string &symbolName, void *symbolValue) { AddSymbol(symbolName.c_str(), symbolValue); } diff --git a/lib/System/DynamicLibrary.cpp b/lib/System/DynamicLibrary.cpp index b88d03f4943..669422c8427 100644 --- a/lib/System/DynamicLibrary.cpp +++ b/lib/System/DynamicLibrary.cpp @@ -9,11 +9,13 @@ // // This header file implements the operating system DynamicLibrary concept. // +// FIXME: This file leaks the ExplicitSymbols and OpenedHandles vector, and is +// not thread safe! +// //===----------------------------------------------------------------------===// #include "llvm/System/DynamicLibrary.h" #include "llvm/Support/ManagedStatic.h" -#include "llvm/System/RWMutex.h" #include "llvm/Config/config.h" #include #include @@ -21,13 +23,13 @@ #include // Collection of symbol name/value pairs to be searched prior to any libraries. -static std::map symbols; -static llvm::sys::SmartRWMutex SymbolsLock; +static std::map *ExplicitSymbols = 0; void llvm::sys::DynamicLibrary::AddSymbol(const char* symbolName, void *symbolValue) { - llvm::sys::SmartScopedWriter Writer(&SymbolsLock); - symbols[symbolName] = symbolValue; + if (ExplicitSymbols == 0) + ExplicitSymbols = new std::map(); + (*ExplicitSymbols)[symbolName] = symbolValue; } #ifdef LLVM_ON_WIN32 @@ -37,7 +39,6 @@ void llvm::sys::DynamicLibrary::AddSymbol(const char* symbolName, #else #include -#include using namespace llvm; using namespace llvm::sys; @@ -46,44 +47,44 @@ using namespace llvm::sys; //=== independent code. //===----------------------------------------------------------------------===// -static std::vector OpenedHandles; +static std::vector *OpenedHandles = 0; bool DynamicLibrary::LoadLibraryPermanently(const char *Filename, std::string *ErrMsg) { - SmartScopedWriter Writer(&SymbolsLock); void *H = dlopen(Filename, RTLD_LAZY|RTLD_GLOBAL); if (H == 0) { - if (ErrMsg) - *ErrMsg = dlerror(); + if (ErrMsg) *ErrMsg = dlerror(); return true; } - OpenedHandles.push_back(H); + if (OpenedHandles == 0) + OpenedHandles = new std::vector(); + OpenedHandles->push_back(H); return false; } void* DynamicLibrary::SearchForAddressOfSymbol(const char* symbolName) { // First check symbols added via AddSymbol(). - SymbolsLock.reader_acquire(); - std::map::iterator I = symbols.find(symbolName); - std::map::iterator E = symbols.end(); - SymbolsLock.reader_release(); + if (ExplicitSymbols) { + std::map::iterator I = + ExplicitSymbols->find(symbolName); + std::map::iterator E = ExplicitSymbols->end(); - if (I != E) - return I->second; + if (I != E) + return I->second; + } - SymbolsLock.writer_acquire(); // Now search the libraries. - for (std::vector::iterator I = OpenedHandles.begin(), - E = OpenedHandles.end(); I != E; ++I) { - //lt_ptr ptr = lt_dlsym(*I, symbolName); - void *ptr = dlsym(*I, symbolName); - if (ptr) { - SymbolsLock.writer_release(); - return ptr; + if (OpenedHandles) { + for (std::vector::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; + } } } - SymbolsLock.writer_release(); #define EXPLICIT_SYMBOL(SYM) \ extern void *SYM; if (!strcmp(symbolName, #SYM)) return &SYM -- 2.34.1