[PATCH] D30107: Refactor DynamicLibrary so searching for a symbol will have a defined order and libraries are properly unloaded when llvm_shutdown is called.

[Vassil Vassilev via Phabricator via llvm-commits]

v.g.vassilev added a comment.

In https://reviews.llvm.org/D30107#763811, @marsupial wrote:

> In https://reviews.llvm.org/D30107#763741, @v.g.vassilev wrote:
>
> > > Symbol resolution is purposely different, as prior library search order was totally undefined which lead to serious problems (for **cling** in particular), see the //**Summary**//.
> >
> > Right, all I am saying is it doesn't work and asking to revert because it makes the issue much worse on cling/ROOT end. I think you are mostly having in mind Windows which `cling` doesn't officially support, however, this patch breaks Unix.
> >
> > > Do you have more information or a test demonstrating the failure?
> >
> >
> >
> >   I think you should be able to reproduce it if you apply this patch and start up ROOT...
> >   
> >
> > > I'd really like to make sure that these issues aren't easily fixed or relying on prior behavior which was poorly defined before reversion.
> >
> > I'd support that. What about reverting it now while you address the underlying out issues without pressure. If this patch was meant to address a `cling` issue, I'd have been happier to discuss and test it, before landing it.
>
>
> If you could provide an example of failure I'd be happy to look at it.


You should test it with ROOT, there we do a `dlopen(libcling, RTLD_LAZY|RTLD_LOCAL)` and I think there the symbol resolution doesn't work as before.

> I get you're saying it doesn't work, but the tests this patch added show it does work on Unix.

It seems that the test coverage is low.

> Additionally a simple test in cling works as well:
>  **$ .L ./unittests/Support/DynamicLibrary/PipSqueak.so
>  $ extern "C" const char *TestA();
>  $ TestA()
>  (const char *) "LibCall"**


Repository:
  rL LLVM

https://reviews.llvm.org/D30107