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

[Jordy Rose]

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.

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


>> +    /// 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