[llvm-commits] [PATCH] Look up symbols in a specific library with DynamicLibrary

[Argyrios Kyrtzidis]

On Aug 5, 2011, at 6:20 PM, Jordy Rose wrote:

> 
> On Aug 5, 2011, at 13:04, Argyrios Kyrtzidis wrote:
> 
>> On Aug 2, 2011, at 9:38 PM, Jordy Rose wrote:
>> 
>>> 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.
>> 
>> How about having DynamicLibrary just be a thin wrapper over the handle (no dlopen/dlclose), a RAII DynamicLibrary does not seem very useful.
>> LoadLibraryPermanently can get an optional DynamicLibrary* in order to return a DynamicLibrary instance, on which you would then call its getAddressOfSymbol.
> 
> 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.
> 
> * 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.
> 
> 
>> 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).
> 
> 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.

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.
We get uniqueness without worrying about making sure to call LoadLibraryPermanently before the constructor of DynamicLibrary or how many calls LoadLibraryPermanently got.

> 
> (But the RAII might be useful if we ever support non-permanent loading.)

Not sure why the method had to be LoadLibraryPermanently, it seems to me that having a pair of [Load/Unload]Library with a DynamicLibrary thin wrapper would work well.

-Argyrios

> 
> 
>>> +    /// Returns true if the object refers to a valid library.
>>> +    bool valid() { return Data != NULL; }
>> 
>> 'isValid' seems to be more common in the llvm/clang codebase.
> 
> Changed. Should have checked that first!
> 
> Thanks for the comments...what do you think?
> Jordy

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110805/559d026d/attachment.html>