<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jul 7, 2009, at 4:46 PM, Howard Su wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">can you also keep Windows version of DynamicLibrary.inc compilable?</blockquote><div><br></div>Is it not? What problem do you see? I don't have a windows box.</div><div><br></div><div>-Chris</div><div><br><blockquote type="cite"><div><br><div class="gmail_quote">On Wed, Jul 8, 2009 at 2:17 AM, Chris Lattner <span dir="ltr"><<a href="mailto:sabre@nondot.org">sabre@nondot.org</a>></span> wrote:<br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Author: lattner<br> Date: Tue Jul 7 13:17:07 2009<br> New Revision: 74927<br> <br> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=74927&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=74927&view=rev</a><br> Log:<br> Eliminate the static constructors and locks from DynamicLibrary.cpp.<br> This fixes PR4512 and eliminating static ctors is always good. Losing<br> thread safety is unfortunate, but the code is just incredibly poorly<br> designed.<br> <br> If someone is interested, the "right" solution is to split<br> DynamicLibrary.cpp into two separate pieces: a stateless piece in<br> libsystem, and a simple support file in libsupport that has the<br> "state" (e.g. AddSymbol) in managed static objects.<br> <br> Doing this would both fix memory leaks we already have, as well as make<br> the code thread safe again. it would also make sense to move all the<br> unix specific code in System/DynamicLibrary.cpp into<br> System/Unix/DynamicLibrary.inc.<br> <br> <br> Modified:<br> llvm/trunk/include/llvm/System/DynamicLibrary.h<br> llvm/trunk/lib/System/DynamicLibrary.cpp<br> <br> Modified: llvm/trunk/include/llvm/System/DynamicLibrary.h<br> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/System/DynamicLibrary.h?rev=74927&r1=74926&r2=74927&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/System/DynamicLibrary.h?rev=74927&r1=74926&r2=74927&view=diff</a><br> <br> ==============================================================================<br> --- llvm/trunk/include/llvm/System/DynamicLibrary.h (original)<br> +++ llvm/trunk/include/llvm/System/DynamicLibrary.h Tue Jul 7 13:17:07 2009<br> @@ -36,6 +36,9 @@<br> /// and will only be unloaded when the program terminates. This returns<br> /// false on success or returns true and fills in *ErrMsg on failure.<br> /// @brief Open a dynamic library permanently.<br> + ///<br> + /// NOTE: This function is not thread safe.<br> + ///<br> static bool LoadLibraryPermanently(const char *filename,<br> std::string *ErrMsg = 0);<br> <br> @@ -46,9 +49,15 @@<br> /// as ephemerally loaded libraries (constructors).<br> /// @throws std::string on error.<br> /// @brief Search through libraries for address of a symbol<br> + ///<br> + /// NOTE: This function is not thread safe.<br> + ///<br> static void *SearchForAddressOfSymbol(const char *symbolName);<br> <br> /// @brief Convenience function for C++ophiles.<br> + ///<br> + /// NOTE: This function is not thread safe.<br> + ///<br> static void *SearchForAddressOfSymbol(const std::string &symbolName) {<br> return SearchForAddressOfSymbol(symbolName.c_str());<br> }<br> @@ -57,9 +66,15 @@<br> /// value \p symbolValue. These symbols are searched before any<br> /// libraries.<br> /// @brief Add searchable symbol/value pair.<br> + ///<br> + /// NOTE: This function is not thread safe.<br> + ///<br> static void AddSymbol(const char *symbolName, void *symbolValue);<br> <br> /// @brief Convenience function for C++ophiles.<br> + ///<br> + /// NOTE: This function is not thread safe.<br> + ///<br> static void AddSymbol(const std::string &symbolName, void *symbolValue) {<br> AddSymbol(symbolName.c_str(), symbolValue);<br> }<br> <br> Modified: llvm/trunk/lib/System/DynamicLibrary.cpp<br> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/System/DynamicLibrary.cpp?rev=74927&r1=74926&r2=74927&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/System/DynamicLibrary.cpp?rev=74927&r1=74926&r2=74927&view=diff</a><br> <br> ==============================================================================<br> --- llvm/trunk/lib/System/DynamicLibrary.cpp (original)<br> +++ llvm/trunk/lib/System/DynamicLibrary.cpp Tue Jul 7 13:17:07 2009<br> @@ -9,11 +9,13 @@<br> //<br> // This header file implements the operating system DynamicLibrary concept.<br> //<br> +// FIXME: This file leaks the ExplicitSymbols and OpenedHandles vector, and is<br> +// not thread safe!<br> +//<br> //===----------------------------------------------------------------------===//<br> <br> #include "llvm/System/DynamicLibrary.h"<br> #include "llvm/Support/ManagedStatic.h"<br> -#include "llvm/System/RWMutex.h"<br> #include "llvm/Config/config.h"<br> #include <cstdio><br> #include <cstring><br> @@ -21,13 +23,13 @@<br> #include <vector><br> <br> // Collection of symbol name/value pairs to be searched prior to any libraries.<br> -static std::map<std::string, void*> symbols;<br> -static llvm::sys::SmartRWMutex<true> SymbolsLock;<br> +static std::map<std::string, void*> *ExplicitSymbols = 0;<br> <br> void llvm::sys::DynamicLibrary::AddSymbol(const char* symbolName,<br> void *symbolValue) {<br> - llvm::sys::SmartScopedWriter<true> Writer(&SymbolsLock);<br> - symbols[symbolName] = symbolValue;<br> + if (ExplicitSymbols == 0)<br> + ExplicitSymbols = new std::map<std::string, void*>();<br> + (*ExplicitSymbols)[symbolName] = symbolValue;<br> }<br> <br> #ifdef LLVM_ON_WIN32<br> @@ -37,7 +39,6 @@<br> #else<br> <br> #include <dlfcn.h><br> -#include <cassert><br> using namespace llvm;<br> using namespace llvm::sys;<br> <br> @@ -46,44 +47,44 @@<br> //=== independent code.<br> //===----------------------------------------------------------------------===//<br> <br> -static std::vector<void *> OpenedHandles;<br> +static std::vector<void *> *OpenedHandles = 0;<br> <br> <br> bool DynamicLibrary::LoadLibraryPermanently(const char *Filename,<br> std::string *ErrMsg) {<br> - SmartScopedWriter<true> Writer(&SymbolsLock);<br> void *H = dlopen(Filename, RTLD_LAZY|RTLD_GLOBAL);<br> if (H == 0) {<br> - if (ErrMsg)<br> - *ErrMsg = dlerror();<br> + if (ErrMsg) *ErrMsg = dlerror();<br> return true;<br> }<br> - OpenedHandles.push_back(H);<br> + if (OpenedHandles == 0)<br> + OpenedHandles = new std::vector<void *>();<br> + OpenedHandles->push_back(H);<br> return false;<br> }<br> <br> void* DynamicLibrary::SearchForAddressOfSymbol(const char* symbolName) {<br> // First check symbols added via AddSymbol().<br> - SymbolsLock.reader_acquire();<br> - std::map<std::string, void *>::iterator I = symbols.find(symbolName);<br> - std::map<std::string, void *>::iterator E = symbols.end();<br> - SymbolsLock.reader_release();<br> + if (ExplicitSymbols) {<br> + std::map<std::string, void *>::iterator I =<br> + ExplicitSymbols->find(symbolName);<br> + std::map<std::string, void *>::iterator E = ExplicitSymbols->end();<br> <br> - if (I != E)<br> - return I->second;<br> + if (I != E)<br> + return I->second;<br> + }<br> <br> - SymbolsLock.writer_acquire();<br> // Now search the libraries.<br> - for (std::vector<void *>::iterator I = OpenedHandles.begin(),<br> - E = OpenedHandles.end(); I != E; ++I) {<br> - //lt_ptr ptr = lt_dlsym(*I, symbolName);<br> - void *ptr = dlsym(*I, symbolName);<br> - if (ptr) {<br> - SymbolsLock.writer_release();<br> - return ptr;<br> + if (OpenedHandles) {<br> + for (std::vector<void *>::iterator I = OpenedHandles->begin(),<br> + E = OpenedHandles->end(); I != E; ++I) {<br> + //lt_ptr ptr = lt_dlsym(*I, symbolName);<br> + void *ptr = dlsym(*I, symbolName);<br> + if (ptr) {<br> + return ptr;<br> + }<br> }<br> }<br> - SymbolsLock.writer_release();<br> <br> #define EXPLICIT_SYMBOL(SYM) \<br> extern void *SYM; if (!strcmp(symbolName, #SYM)) return &SYM<br> <br> <br> _______________________________________________<br> llvm-commits mailing list<br> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br> </blockquote></div><br><br clear="all"><br>-- <br>-Howard<br> </div> _______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></blockquote></div><br></body></html>