Backport symbol versioning patch from LLVM 4.0

[ihnorton]

Should help with crashes when loading multiple libLLVM versions in
the same process, as happens with mesa/llvmpipe when mesa is
dynamically linked against libLLVM. See

#19606

patch: https://reviews.llvm.org/D31524
commit: https://reviews.llvm.org/rL300496

[vchuravy]

Fantastic! This should help to alleviate issues we have been encountering in OpenCL.jl and other packages, when they loaded libraries that were using a different version of LLVM. I don't think it solves the issue of loading two versions of LLVM that are the same base version (one Julia build with patches, the other one vanilla) so maybe we can use a similar mechanism to add a jl-(pre|suf)fix.

[ihnorton]

maybe we can use a similar mechanism to add a jl-(pre|suf)fix.

I think it could be as simple as adding JL_ here:

+--- /dev/null
 ++++ b/tools/llvm-shlib/simple_version_script.map.in
 +@@ -0,0 +1 @@
 ++LLVM_@LLVM_VERSION_MAJOR@.@LLVM_VERSION_MINOR@ { global: *; };

Btw

• this only helps on GNU LD platforms (which is ok for now, as I believe the problem only manifests there too)
• RTLD_LOCAL might help in situations where you control dlopen

[maleadt]

If it's going to differ between platforms, depending on eg. the linker, we should have a way of accessing the prefix for packages like LLVM.jl to be able to call into Julia's libLLVM.
EDIT: not strictly necessary though, I could just do a bunch of dlsyms to detect the prefix.

[vtjnash]

as I believe the problem only manifests there too

I also believe this is correct. Windows and macOS always identifies symbols by library id (typically equal to the file base name), for performance, and thereby generally avoiding this problem.

[ihnorton]

@Keno this might have implications for Cxx.

[vchuravy]

I forgot that this patch existed and was about to open my own PR, but instead with https://reviews.llvm.org/D30997 at https://github.com/JuliaLang/julia/tree/vc/vendorllvm

I don't know why LLVM introduced two options to solve the same problem, but my instinct is to prefer this one (#24890) over my patch since it allows us more flexibility.

In any case we should have this for 1.0 since it is a general nuisance on any system that uses LLVM in mesa.

[StefanKarpinski]

Seems like it just needs a rebase and a review by @vchuravy and it should be good to go.

[ihnorton]

Rebased.

[maleadt]

Shouldn't we prefix with JL_LLVM_$X.$Y just to be sure? Given that this is part of upstream, chances are Mesa/... also use this mechanic to prefix with LLVM_$X.$Y

I don't think it solves the issue of loading two versions of LLVM that are the same base version (one Julia build with patches, the other one vanilla) so maybe we can use a similar mechanism to add a jl-(pre|suf)fix.

[ihnorton]

Sounds good. AFK, please go ahead and edit/merge if it passes CI.

[vchuravy]

I will do a round testing today and merge later.

…

On Tue, Jan 16, 2018, 08:44 Isaiah Norton ***@***.***> wrote: Sounds good. AFK, please go ahead and edit/merge if it passes CI. On Tue, Jan 16, 2018 at 3:22 AM Tim Besard ***@***.***> wrote: > Shouldn't we prefix with JL_LLVM_$X.$Y just to be sure? Given that this > is part of upstream, chances are Mesa/... also use this mechanic to prefix > with LLVM_$X.$Y > > I don't think it solves the issue of loading two versions of LLVM that are > the same base version (one Julia build with patches, the other one vanilla) > so maybe we can use a similar mechanism to add a jl-(pre|suf)fix. > > — > > > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#24890 (comment)>, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AAUAGqSU8NSRHNnZ6k3n9g3yE8Ee_s2Oks5tLFw1gaJpZM4Qzl0z > > . > -- Sent from phone. Please excuse typos. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24890 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAI3alQV6UeDCMh2OHTKfAGKAQEQfl9Fks5tLKfHgaJpZM4Qzl0z> .

[vchuravy]

Ok this is looking good:

> nm -D --with-symbol-versions usr/lib/libjulia.so
...
                 U LLVMInitializeX86AsmParser@LLVM_3.9
                 U LLVMInitializeX86AsmPrinter@LLVM_3.9
                 U LLVMInitializeX86Disassembler@LLVM_3.9
                 U LLVMInitializeX86Target@LLVM_3.9
                 U LLVMInitializeX86TargetInfo@LLVM_3.9
                 U LLVMInitializeX86TargetMC@LLVM_3.9
...

Libraries that are using dynamic symbol lookup are unaffected since it is a version suffix.
So LLVM.jl works out of the box (Cxx.jl isn't working on 0.7 right now) and testing with OpenCL.jl and a LLVM based OpenCL implementation also shows that the common clash is hopefully solved.
JuliaGPU/OpenCL.jl#137 (comment)

I have a patch that adds the JL prefix and will open a new PR with that.