can you also keep Windows version of DynamicLibrary.inc compilable?<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>