<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Aug 5, 2011, at 6:20 PM, Jordy Rose wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div><br>On Aug 5, 2011, at 13:04, Argyrios Kyrtzidis wrote:<br><br><blockquote type="cite">On Aug 2, 2011, at 9:38 PM, Jordy Rose wrote:<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><blockquote type="cite">This patch adds the ability to look up symbols in a specific library by instantiating llvm::sys::DynamicLibrary. I've tried to make it minimally intrusive and preserve the existing interface (LoadLibraryPermanently and SearchForAddressOfSymbol). Requiring that DynamicLibrary instances must represent already-loaded libraries also means not having to deal with potentially unloaded libraries, though since both dlfcn handles and HMODULEs are ref-counted, that shouldn't be too hard to deal with if we ever lift this restriction.<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">How about having DynamicLibrary just be a thin wrapper over the handle (no dlopen/dlclose), a RAII DynamicLibrary does not seem very useful.<br></blockquote><blockquote type="cite">LoadLibraryPermanently can get an optional DynamicLibrary* in order to return a DynamicLibrary instance, on which you would then call its getAddressOfSymbol.<br></blockquote><br>I'd prefer this myself, actually, except that currently the dynamic library loading is happening through the use of '-load', which happens before any compiler action is taken. We certainly don't want to make every compiler action responsible for loading plugins*, so we'd have to save the list of loaded plugins in the CompilerInstance somewhere. Which is fine, but it's more intrusive.<br><br>* Or do we? The only time plugins are interesting right now is for plugin actions, and adding pluggable checkers only makes it interesting for -analyze and -analyzer-checker-help. Still, we'd have to be careful to avoid loading twice.<br><br><br><blockquote type="cite">I'd also suggest that the library filename is kept associated with its handle so that there is a static getLoadedLibrary method returning a DynamicLibrary instance (and LoadLibraryPermanently could check with getLoadedLibrary before doing a dlopen).<br></blockquote><br>I also thought about this, but it seems silly to keep a map of string -> DynamicLibrary when the underlying infrastructure (dlopen/GetModule) must already have a similar uniquing scheme. The point of this isn't the RAII-ness of it but rather a way to get access to the underlying uniquing-by-path.<br></div></blockquote><div><br></div><div>Right, we could change LoadLibraryPermanently to keep a set of handles instead of a vector and then use a static getLibrary method returning a DynamicLibrary wrapper which will itself do a LoadLibraryPermanently.</div><div>We get uniqueness without worrying about making sure to call LoadLibraryPermanently before the constructor of DynamicLibrary or how many calls LoadLibraryPermanently got.</div><br><blockquote type="cite"><div><br>(But the RAII might be useful if we ever support non-permanent loading.)<br></div></blockquote><div><br></div><div>Not sure why the method had to be LoadLibrary<b>Permanently</b>, it seems to me that having a pair of [Load/Unload]Library with a DynamicLibrary thin wrapper would work well.</div><div><br></div><div>-Argyrios</div><br><blockquote type="cite"><div><br><br><blockquote type="cite"><blockquote type="cite">+ /// Returns true if the object refers to a valid library.<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">+ bool valid() { return Data != NULL; }<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">'isValid' seems to be more common in the llvm/clang codebase.<br></blockquote><br>Changed. Should have checked that first!<br><br>Thanks for the comments...what do you think?<br>Jordy</div></blockquote></div><br></body></html>