Page MenuHomePhabricator

Add a linker script to version LLVM symbols
ClosedPublic

Authored by sylvestre.ledru on Mar 30 2017, 11:26 PM.

Details

Summary

This patch adds a very simple linker script to version the lib's symbols
and thus trying to avoid crashes if an application loads two different
LLVM versions (as long as they do not share data between them).

Note that we deliberately *don't* make LLVM_5.0 depend on LLVM_4.0:
they're incompatible and the whole point of this patch is
to tell the linker that.

Avoid unexpected crashes when two LLVM versions are used in the same process.

Author: Rebecca N. Palmer <rebecca_palmer@zoho.com>
Author: Lisandro Damían Nicanor Pérez Meyer <lisandro@debian.org>
Author: Sylvestre Ledru <sylvestre@debian.org>
Bug-Debian: https://bugs.debian.org/848368

Diff Detail

Event Timeline

Programs using these libraries WONT have to recompile anything.

rnk added inline comments.Apr 10 2017, 11:58 AM
cmake/modules/AddLLVM.cmake
85

Is this correct for DSOs that aren't libLLVM.so.N.M? This function is called from LLDB and elsewhere.

cmake/modules/AddLLVM.cmake
85

It doesn't have too. libLLVM.so.N.M is for the SONAME.
However, I can rename it, no worries

ping? (FYI, Debian packages are shipping with this change, so will Ubuntu)

rnk accepted this revision.Apr 17 2017, 1:24 PM

Note that we deliberately *don't* make LLVM_5.0 depend on LLVM_5.0:

Can you elaborate on what you mean by this? Do you mean LLVM_5.0 doesn't depend on LLVM_4.0?

cmake/modules/AddLLVM.cmake
85

So, this doesn't refer to LLVM the DSO, it refers to LLVM the project. After this change, libclang.so will have symbols with LLVM_N.M as their version, right?

This revision is now accepted and ready to land.Apr 17 2017, 1:24 PM

Can you elaborate on what you mean by this? Do you mean LLVM_5.0 doesn't depend on LLVM_4.0?

Yeah, sorry. I will fix that in the commit

cmake/modules/AddLLVM.cmake
85

Yes, I can rename it to LLVM_TOOLCHAIN_N.M if you prefer but this is a bit long.

rnk added inline comments.Apr 17 2017, 1:34 PM
cmake/modules/AddLLVM.cmake
85

No, it's fine. Maybe add a comment, since I don't think ELF version script details are common knowledge.

sylvestre.ledru edited the summary of this revision. (Show Details)Apr 17 2017, 1:57 PM