- User Since
- Jul 25 2016, 10:43 PM (47 w, 5 d)
Mon, Jun 19
Wed, Jun 14
From the summary:
Fixes an issue using RegisterStandardPasses from a statically linked object before PassManagerBuilder::addGlobalExtension is called from a dynamic library.
I'm missing the *why* using a "real function" (I guess you meant "function pointer") matter?
Remove spurious newline.
This should be more 'obviously' correct.
Sorry, some CMake confusion here....
Relocate tests into Transforms/IPO.
Tue, Jun 13
Having trouble with the test part of this and CMake.
Add explicit dependency for CMake when built with LLVM_ENABLE_MODULES.
Conflict resolution & additional comment.
Fri, Jun 9
Mon, Jun 5
Tue, May 30
Add test case.
Allow forward and backward search of loaded modules.
Mon, May 29
Restore default ordering after llvm_shutdown called.
Fri, May 26
Exactly, so there is no way to override the symbol at run-time. This counter-intuitive (well wrong in my opinion) in an interactive-compiler.
I understand you folks feel it is wrong and I have been trying to work towards a solution.
But there are other use cases besides yours.
Add tests for symbol resolution order.
May 26 2017
Indeed we are not. We are arguing the current set of patch (and proposed) patch imposed the order (1) then (maybe) (2) while we need (and the previous behavior was 'close' to) (2) then (1)
Seems this might have broken many bots.
https://reviews.llvm.org/D33529 is attempting to address this can we continue discussion there?
I'm totally understand the order being reversed, though I would like to keep an explicit bool to signify Don't resolve as the OS would
Otherwise from the example above someone who loads a lib with printf will pull that symbol from libA.so or libB.so and they might not actually want to.
They should explicitly request this behaviour.
Lets take a simple example of libRuntime.so, libA.so, libB.so.
libRuntime.so is linked to the binaries and contains stdlib functions, such as printf.
libA.so, and libB.so also contain a locally exported symbols printf, doSomething
It did neither (1) or (2) before, because the order the handles were stored was not defined.
This was particularly noticeable on Windows due to how it uses shim libs for runtime functions.
I assume you guys aren't arguing to going back to undefined behavior just because it worked better for you.
Why does https://reviews.llvm.org/D33529 not address your concerns?
May 25 2017
This works here and is way less of a hack.
This should fix the issue and adds a test case of what you're seeing.
Not sure if I want to be the one committing this as it's a bit ugly...
polly has historically worked as a plugin. In this context, polly isn't technically a "plugin" in the sense that it's not dynamically loaded, but it works like a plugin in every other sense (using the same extension points a plugin would use to integrate with LLVM).
Perhaps we can try to close this one out before moving on to the larger problem?
addGlobalExtension is documented to be used by plugins, where you are using it for a statically linked lib.
There doesn't seem to be any great solution and I going back to how it was (leaking) is one of the worse options.
May 24 2017
Can the discussion of this continue here https://reviews.llvm.org/D33529.
I'm not actually sure I'm in favor of this but wanted another opinion.
To me DynamicLibrary::addPermanentLibrary doesn't really seem all that useful and actually complicates things a bit.
You're saying adding the changes to Hello.cpp,
make -j8 opt LLVMHello
bin/opt -load=lib/LLVMHello.so -S /dev/null
is still crashing?
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.
RTLD_LOCAL means any symbols loaded will not be available for lookup.
If you are relying on symbol lookup via a process handle [ dlsym(dlopen(NULL, Flags), "Symbol") ] then you need to use RTLD_GLOBAL.