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

[Argyrios Kyrtzidis]

On Aug 6, 2011, at 4:46 PM, Jordy Rose wrote:

> 
> On Aug 5, 2011, at 18:55, Argyrios Kyrtzidis wrote:
> 
>> 
>> 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:
>>>> 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.
> 
> No, I mean dlopen already uniques modules; opening a library a second time returns a handle to the same library structure but increases its refcount. That means that somewhere in the dlcfn code there's already a map from paths (or library identifiers) to handles...and the only way to access /that/ mapping is using dlopen. Which makes loadLibrary and getLibrary equivalent.
> 
> If you think it's worth it to have our own StringMap<DynamicLibrary>, i.e. do our own uniquing based on pathname, then that's fine. I just thought it duplicated exising documented functionality, and interfered with the possibility of allowing unloading in the future. (You can't really safely unload a library without refcounts, right?)

Sorry, I wasn't clear, I totally agree with you that manually uniquing is pointless.

My point is that your patch introduces this API usage model:

DynamicLibrary dylib("mylib.dylib");
void *p = dylib.getAddressOfSymbol("mysymbol");

but wait, I need to also call LoadLibraryPermanently:

DynamicLibrary::LoadLibraryPermanently("mylib.dylib");
DynamicLibrary dylib("mylib.dylib");
void *p = dylib.getAddressOfSymbol("mysymbol");

Now I'm worried, do I need to make sure that LoadLibraryPermanently will only be called once for each library ? Is it OK to call it multiple times with the same library file, should I always do a pair of LoadLibraryPermanently/DynamicLibrary constructor ?

How about this API usage:

DynamicLibrary dylib = DynamicLibrary::getPermanentLibrary("mylib.dylib");  // will be added to permanent libraries if it is not already
void *p = dylib.getAddressOfSymbol("mysymbol");

and we make it explicit that LoadLibraryPermanently/getPermanentLibrary can be called multiple times with the same library file. Internally instead of loading+adding handle to vector, we do not do anything if the handle is already added.

-Argyrios

> 
> Jordy
>