[llvm-commits] [llvm] r74927 - in /llvm/trunk: include/llvm/System/DynamicLibrary.h lib/System/DynamicLibrary.cpp
Howard Su
howard0su at gmail.com
Tue Jul 7 16:46:23 PDT 2009
can you also keep Windows version of DynamicLibrary.inc compilable?
On Wed, Jul 8, 2009 at 2:17 AM, Chris Lattner <sabre at nondot.org> wrote:
> Author: lattner
> Date: Tue Jul 7 13:17:07 2009
> New Revision: 74927
>
> URL: http://llvm.org/viewvc/llvm-project?rev=74927&view=rev
> Log:
> 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.
>
>
> Modified:
> llvm/trunk/include/llvm/System/DynamicLibrary.h
> llvm/trunk/lib/System/DynamicLibrary.cpp
>
> Modified: llvm/trunk/include/llvm/System/DynamicLibrary.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/System/DynamicLibrary.h?rev=74927&r1=74926&r2=74927&view=diff
>
>
> ==============================================================================
> --- llvm/trunk/include/llvm/System/DynamicLibrary.h (original)
> +++ llvm/trunk/include/llvm/System/DynamicLibrary.h Tue Jul 7 13:17:07
> 2009
> @@ -36,6 +36,9 @@
> /// 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 @@
> /// 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 @@
> /// 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);
> }
>
> Modified: llvm/trunk/lib/System/DynamicLibrary.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/System/DynamicLibrary.cpp?rev=74927&r1=74926&r2=74927&view=diff
>
>
> ==============================================================================
> --- llvm/trunk/lib/System/DynamicLibrary.cpp (original)
> +++ llvm/trunk/lib/System/DynamicLibrary.cpp Tue Jul 7 13:17:07 2009
> @@ -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 <cstdio>
> #include <cstring>
> @@ -21,13 +23,13 @@
> #include <vector>
>
> // Collection of symbol name/value pairs to be searched prior to any
> libraries.
> -static std::map<std::string, void*> symbols;
> -static llvm::sys::SmartRWMutex<true> SymbolsLock;
> +static std::map<std::string, void*> *ExplicitSymbols = 0;
>
> void llvm::sys::DynamicLibrary::AddSymbol(const char* symbolName,
> void *symbolValue) {
> - llvm::sys::SmartScopedWriter<true> Writer(&SymbolsLock);
> - symbols[symbolName] = symbolValue;
> + if (ExplicitSymbols == 0)
> + ExplicitSymbols = new std::map<std::string, void*>();
> + (*ExplicitSymbols)[symbolName] = symbolValue;
> }
>
> #ifdef LLVM_ON_WIN32
> @@ -37,7 +39,6 @@
> #else
>
> #include <dlfcn.h>
> -#include <cassert>
> using namespace llvm;
> using namespace llvm::sys;
>
> @@ -46,44 +47,44 @@
> //=== independent code.
>
> //===----------------------------------------------------------------------===//
>
> -static std::vector<void *> OpenedHandles;
> +static std::vector<void *> *OpenedHandles = 0;
>
>
> bool DynamicLibrary::LoadLibraryPermanently(const char *Filename,
> std::string *ErrMsg) {
> - SmartScopedWriter<true> 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<void *>();
> + OpenedHandles->push_back(H);
> return false;
> }
>
> void* DynamicLibrary::SearchForAddressOfSymbol(const char* symbolName) {
> // First check symbols added via AddSymbol().
> - SymbolsLock.reader_acquire();
> - std::map<std::string, void *>::iterator I = symbols.find(symbolName);
> - std::map<std::string, void *>::iterator E = symbols.end();
> - SymbolsLock.reader_release();
> + if (ExplicitSymbols) {
> + std::map<std::string, void *>::iterator I =
> + ExplicitSymbols->find(symbolName);
> + std::map<std::string, void *>::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<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) {
> - SymbolsLock.writer_release();
> - return ptr;
> + if (OpenedHandles) {
> + for (std::vector<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;
> + }
> }
> }
> - SymbolsLock.writer_release();
>
> #define EXPLICIT_SYMBOL(SYM) \
> extern void *SYM; if (!strcmp(symbolName, #SYM)) return &SYM
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
--
-Howard
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090708/dd442054/attachment.html>
More information about the llvm-commits
mailing list